Re: [PATCH v3 4/7] dt-bindings: pinctrl: add bindings for Mediatek MT8365 SoC

Krzysztof Kozlowski
2022-11-20T10:40:15Z

On 18/11/2022 20:52, Kevin Hilman wrote:
> Hi Krzysztof,
>
> Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> writes:
>
>> On 17/11/2022 22:03, Bernhard Rosenkränzer wrote:
>>> Add devicetree bindings for Mediatek MT8365 pinctrl driver.
>>>
>>> Signed-off-by: Bernhard Rosenkränzer <bero@baylibre.com>
>>
>> Thank you for your patch. There is something to discuss/improve.
>>
>>> +
>>> + pins-are-numbered:
>>> + $ref: /schemas/types.yaml#/definitions/flag
>>> + description: |
>>> + Specify the subnodes are using numbered pinmux to specify pins.
>>
>> Why would you name pins differently per board? And why this different
>> naming of the same pins is a property of hardware?
>>
>> This looks like something to drop.
>
> Yeah, having this as a flag kind of implies that this could be present
> for some boards but not others. But in practice, the driver requires it
> to be present or just fails[1]. What's the right way to describe that?
> We're just trying to add a binding that reflects the existing driver.

Uh, what an interesting property. What's the point of it then? Why
failing to probe on a missing property which does nothing else?

The solution is also to drop that property from the driver.

> We also noticed that there's another documented binding with this
> same flag[2] where similiarily, the driver simply requires it to be
> present[2].
>
> So is the way this flag is documented in the stm32 binding OK for the
> mediatek one also? If not, what would you suggest?

I would like to understand why do we need this property and what is
described by it. Because if it's purpose is only to fail or not fail
driver probe, then we should just drop it everywhere.

>

Best regards,
Krzysztof