Re: [Xen-devel] [PATCH v2 03/12] xen/dt: Extend dt_device_match to possibly store data

[Julien Grall <julien.grall@xxxxxxxxxx>]

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,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel