| Message ID | 1421418247-30068-4-git-send-email-julien.grall@linaro.org |
|---|---|
| State | Superseded, archived |
| Headers | show |
Hi Stefano, On 27/01/15 15:57, Stefano Stabellini wrote: >> const struct dt_device_node *dt_get_parent(const struct dt_device_node *node) >> diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h >> index 08db8bc..6502369 100644 >> --- a/xen/include/xen/device_tree.h >> +++ b/xen/include/xen/device_tree.h >> @@ -28,6 +28,7 @@ struct dt_device_match { >> const char *type; >> const char *compatible; >> const bool_t not_available; >> + const void *data; > > Why are you adding this field? It doesn't seem to be required by the > changes to dt_match_node you are making in this patch. It's required for the SMMU drivers. The version of the SMMU is stored in the field data. +static const struct of_device_id arm_smmu_of_match[] = { + { .compatible = "arm,smmu-v1", .data = (void *)ARM_SMMU_V1 }, + { .compatible = "arm,smmu-v2", .data = (void *)ARM_SMMU_V2 }, + { .compatible = "arm,mmu-400", .data = (void *)ARM_SMMU_V1 }, + { .compatible = "arm,mmu-401", .data = (void *)ARM_SMMU_V1 }, + { .compatible = "arm,mmu-500", .data = (void *)ARM_SMMU_V2 }, + { }, +}; The main goal of this patch is too add the field data. The change of dt_match_node is only a side effect. Regards,
On 27/01/15 16:10, Stefano Stabellini wrote: > On Tue, 27 Jan 2015, Julien Grall wrote: >> Hi Stefano, >> >> On 27/01/15 15:57, Stefano Stabellini wrote: >>>> const struct dt_device_node *dt_get_parent(const struct dt_device_node *node) >>>> diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h >>>> index 08db8bc..6502369 100644 >>>> --- a/xen/include/xen/device_tree.h >>>> +++ b/xen/include/xen/device_tree.h >>>> @@ -28,6 +28,7 @@ struct dt_device_match { >>>> const char *type; >>>> const char *compatible; >>>> const bool_t not_available; >>>> + const void *data; >>> >>> Why are you adding this field? It doesn't seem to be required by the >>> changes to dt_match_node you are making in this patch. >> >> It's required for the SMMU drivers. The version of the SMMU is stored in >> the field data. >> >> +static const struct of_device_id arm_smmu_of_match[] = { >> + { .compatible = "arm,smmu-v1", .data = (void *)ARM_SMMU_V1 }, >> + { .compatible = "arm,smmu-v2", .data = (void *)ARM_SMMU_V2 }, >> + { .compatible = "arm,mmu-400", .data = (void *)ARM_SMMU_V1 }, >> + { .compatible = "arm,mmu-401", .data = (void *)ARM_SMMU_V1 }, >> + { .compatible = "arm,mmu-500", .data = (void *)ARM_SMMU_V2 }, >> + { }, >> +}; >> >> The main goal of this patch is too add the field data. The change of >> dt_match_node is only a side effect. > > In that case please make sure to write it clearly in the commit message. It's already on the title "xen/dt: Extend dt_device_match to possibly store data" and the reason is the first line of the commit message "Some drivers may want to configure differently the device depending on the compatible string.". I'm not sure how I can make this more clearly. Regards,
On 27/01/15 16:46, Stefano Stabellini wrote: > On Tue, 27 Jan 2015, Julien Grall wrote: >> On 27/01/15 16:10, Stefano Stabellini wrote: >>> On Tue, 27 Jan 2015, Julien Grall wrote: >>>> Hi Stefano, >>>> >>>> On 27/01/15 15:57, Stefano Stabellini wrote: >>>>>> const struct dt_device_node *dt_get_parent(const struct dt_device_node *node) >>>>>> diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h >>>>>> index 08db8bc..6502369 100644 >>>>>> --- a/xen/include/xen/device_tree.h >>>>>> +++ b/xen/include/xen/device_tree.h >>>>>> @@ -28,6 +28,7 @@ struct dt_device_match { >>>>>> const char *type; >>>>>> const char *compatible; >>>>>> const bool_t not_available; >>>>>> + const void *data; >>>>> >>>>> Why are you adding this field? It doesn't seem to be required by the >>>>> changes to dt_match_node you are making in this patch. >>>> >>>> It's required for the SMMU drivers. The version of the SMMU is stored in >>>> the field data. >>>> >>>> +static const struct of_device_id arm_smmu_of_match[] = { >>>> + { .compatible = "arm,smmu-v1", .data = (void *)ARM_SMMU_V1 }, >>>> + { .compatible = "arm,smmu-v2", .data = (void *)ARM_SMMU_V2 }, >>>> + { .compatible = "arm,mmu-400", .data = (void *)ARM_SMMU_V1 }, >>>> + { .compatible = "arm,mmu-401", .data = (void *)ARM_SMMU_V1 }, >>>> + { .compatible = "arm,mmu-500", .data = (void *)ARM_SMMU_V2 }, >>>> + { }, >>>> +}; >>>> >>>> The main goal of this patch is too add the field data. The change of >>>> dt_match_node is only a side effect. >>> >>> In that case please make sure to write it clearly in the commit message. >> >> It's already on the title "xen/dt: Extend dt_device_match to possibly >> store data" and the reason is the first line of the commit message >> "Some drivers may want to configure differently the device depending on >> the compatible string.". > > What data? How are you extending it? Data could be anything. I think it's enough logic to know that dt_device_match is a structure and a new field will be added. Regards,
diff --git a/xen/arch/arm/platform.c b/xen/arch/arm/platform.c index cb4cda8..a79a098 100644 --- a/xen/arch/arm/platform.c +++ b/xen/arch/arm/platform.c @@ -157,7 +157,7 @@ bool_t platform_device_is_blacklisted(const struct dt_device_node *node) if ( platform && platform->blacklist_dev ) blacklist = platform->blacklist_dev; - return dt_match_node(blacklist, node); + return (dt_match_node(blacklist, node) != NULL); } unsigned int platform_dom0_evtchn_ppi(void) diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c index f72b2e9..34a1b9e 100644 --- a/xen/common/device_tree.c +++ b/xen/common/device_tree.c @@ -290,11 +290,12 @@ struct dt_device_node *dt_find_node_by_alias(const char *alias) return NULL; } -bool_t dt_match_node(const struct dt_device_match *matches, - const struct dt_device_node *node) +const struct dt_device_match * +dt_match_node(const struct dt_device_match *matches, + const struct dt_device_node *node) { if ( !matches ) - return 0; + return NULL; while ( matches->path || matches->type || matches->compatible || matches->not_available ) @@ -314,12 +315,11 @@ bool_t dt_match_node(const struct dt_device_match *matches, match &= !dt_device_is_available(node); if ( match ) - return match; - + return matches; matches++; } - return 0; + return NULL; } const struct dt_device_node *dt_get_parent(const struct dt_device_node *node) diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h index 08db8bc..6502369 100644 --- a/xen/include/xen/device_tree.h +++ b/xen/include/xen/device_tree.h @@ -28,6 +28,7 @@ struct dt_device_match { const char *type; const char *compatible; const bool_t not_available; + const void *data; }; #define DT_MATCH_PATH(p) { .path = p } @@ -547,8 +548,9 @@ bool_t dt_device_is_available(const struct dt_device_node *device); * * Returns true if the device node match one of dt_device_match. */ -bool_t dt_match_node(const struct dt_device_match *matches, - const struct dt_device_node *node); +const struct dt_device_match * +dt_match_node(const struct dt_device_match *matches, + const struct dt_device_node *node); /** * dt_find_matching_node - Find a node based on an dt_device_match match table
Some drivers may want to configure differently the device depending on the compatible string. Also modify the return type of dt_match_node to return the matching structure. Signed-off-by: Julien Grall <julien.grall@linaro.org> --- xen/arch/arm/platform.c | 2 +- xen/common/device_tree.c | 12 ++++++------ xen/include/xen/device_tree.h | 6 ++++-- 3 files changed, 11 insertions(+), 9 deletions(-)