Re: [PATCH] extcon: Add support for qcom SPMI PMIC USB id detection hardware

[Chanwoo Choi]

On 2016ë 06ì 30ì 03:48, Stephen Boyd wrote:
> Quoting Chanwoo Choi (2016-06-28 23:25:57)
>> On 2016ë 06ì 29ì 06:59, Stephen Boyd wrote:
>>> Quoting Chanwoo Choi (2016-06-28 05:06:48)
>>>> 2016-06-28 4:11 GMT+09:00 Stephen Boyd <stephen.boyd@xxxxxxxxxx>:
>>>>> Quoting Chanwoo Choi (2016-06-26 04:20:43)
>>>>>> 2016-06-26 14:56 GMT+09:00 Stephen Boyd <stephen.boyd@xxxxxxxxxx>:
>>>>>>> +PROPERTIES
>>>>>>> +
>>>>>>> +- compatible:
>>>>>>> +    Usage: required
>>>>>>> +    Value type: <string>
>>>>>>> +    Definition: Should contain "qcom,pm8941-misc";
>>>>>>> +
>>>>>>> +- reg:
>>>>>>> +    Usage: required
>>>>>>> +    Value type: <u32>
>>>>>>> +    Definition: Should contain the offset to the misc address space
>>>>>>
>>>>>> 'reg' property is used on extcon-qcom-spmi-misc.c?
>>>>>> I think that you don't need to include this property.
>>>>>
>>>>> No it isn't used in the driver right now, but there is a register offset
>>>>> for this module and there are registers that can be read/written in this
>>>>> module. I'd like to keep it as required so we can easily read the
>>>>> registers in the future if needed.
>>>>
>>>> OK.
>>>> But, If you want to remain the reg property, you should add the code to get
>>>> the register offset by using OF functions. This patch don't include the OF
>>>> function to handle it.
>>>>
>>>
>>> Sorry I don't follow the argument. I've put the reg property here for
>>> future proofing so that the binding doesn't have to change in backwards
>>> incompatible ways in the future if we do need to get the property later.
>>
>> I don't mention that 'reg' property should be removed.
> 
> Ok good. We need to keep reg property as this device is on a bus that
> uses reg property for addressing.
> 
>> Just if you want to remain it, you should add some codes as following:
>> For exmaple,
>> - of_get_address() to get the address information from device-tree.
>>
>> If documentation include the some properties, you should add the handling code
>> in device driver. When you add the code to get the offset from device-tree,
>> it doesn't influence the some behavior in the future.
> 
> Sorry I don't understand that argument. We can put properties into
> bindings and not use them in drivers if there isn't any immediate need
> to use them.
> 
> From what I can tell you're suggesting we call of_get_address() in the
> driver and then do nothing with the value of the property? Is that just
> to check that the node is compliant with the binding and actually has a
> reg property? We don't add code in the kernel to check dts compliance,
> so I'm not inclined to do anything more here.

I don't agree.

When he DT binding document include the 'reg' property.
But, the device driver don't include any code to handle the 'reg' property
(just to get the offset). It is obviously wrong.

It is just basic principle to write the Device-tree binding document.

Other developer who don' know the history about 'reg' property
would be embarrassed. Why don't extcon driver include the code to
handle the 'reg' property? There is no method to explain it.