Re: [PATCH v3 1/3] dt-bindings: pinctrl: Add support for Amlogic A4 SoCs

[Krzysztof Kozlowski <krzk@kernel.org>]

On 21/10/2024 08:31, Krzysztof Kozlowski wrote:
>>>>
>>>>  reset-gpios = <&gpio 42 GPIO_ACTIVE_LOW>;
>>>>
>>>> And others will go in the driver to see that is maps to GPIOX_10 ? the number
>>>> being completly made up, with no link to anything HW/Datasheet
>>>> whatsoever ?
>>>>
>>>> This is how things should be done now ?
>>>
>>> Why would you need to do this? Why it cannot be <&gpio 10
>>> GPIO_ACTIVE_LOW>, assuming it is GPIO 10?
>>>
>>> Bindings have absolutely nothing to do with it. You have GPIO 10, not
>>> 42, right?
>>
>> That's what being proposed here, as far as I can see.
>>
>> GPIOX_10 (not GPIO 10) maps to 42. If this goes through, for DTs to be
>> valid in any OS, all need to share the same definition. That looks like
>> a binding to me.
>>
>> On these SOC, gpios in each controller are organized in bank with
>> different number of pins. So far, this was represented as single linear
>> array and that was not a problem since the mapping was part of the binding.
>>
>> Are you suggesting 2 params instead of one ? something like this maybe ?
>>
>> reset-gpios = <&gpio BANK_X 10 GPIO_ACTIVE_LOW>;
> 
> No, I propose the same as you wrote:
> <&gpio 10 GPIO_ACTIVE_LOW>
> 
> but I don't mind putting bank there.
> 
>>
>> This means this A4 controller will be software incompatible with the
>> previous generation. It will need to handled differently eventhough the
>> HW is exactly the same.
>>
>> Note that some form of binding would still be required to define the
>> banks which are referenced by arbitrary letter in doc, not numbers.
> 
> Usually banks are considered separate gpio controllers, so numbering
> always start from 0 because phandle encodes the bank.
> 
> And this is exactly what Rob already asked in v1 review.

Ha, actually I misread his reply, I think he proposed your syntax:
<&gpio BANK_X 10 GPIO_ACTIVE_LOW>

Best regards,
Krzysztof