Re: [PATCH v3 1/2] dt-bindings: gpio: gpio-mvebu: convert txt binding to YAML

[Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx>]

On 14/05/2022 04:20, Chris Packham wrote:
> 
>>> +
>>> +allOf:
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          contains:
>>> +            const: marvell,armada-8k-gpio
>>> +    then:
>>> +      required:
>>> +        - offset
>>> +    else:
>>> +      required:
>>> +        - reg
>> one blank line please
>>
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          contains:
>>> +            const: marvell,armadaxp-gpio
>> Original bindings are saying that second reg is optional for
>> marvell,armada-370-gpio. What about other cases, e.g. mv78200-gpio? Is
>> it also allowed (and optional) there?
> This is where things get interesting. The armadaxp (and only the 
> armadaxp) requires a second register value for some per-cpu registers. 
> All of the other SoCs can have an optional 2nd register value if they 
> want to use the PWM function. I guess that implies that the armadaxp 
> can't do PWM.
>>> +    then:
>>> +      properties:
>>> +        reg:
>>> +          minItems: 2
>> Then you also should require two reg-names.
> 
> Simple enough to add. But currently we've said that the reg-names are 
> "gpio" and "pwm" but on the armadaxp the 2nd one is not "pwm" but 
> something else ("per-cpu" perhaps?)

In such case they would be failing with current bindings, because they
expect "pwm" as second name, right?

> 
> On the other hand this is all completely moot because the 
> armada-xp-mv78*.dtsi actually use the "marvell,armada-370-gpio" 
> compatible so this appears to be documenting something that is no longer 
> used. Indeed it appears that the armadaxp specific usage was remove in 
> 5f79c651e81e ("arm: mvebu: use global interrupts for GPIOs on Armada XP").
> 
> So perhaps the best course of action is to drop marvell,armadaxp-gpio 
> from the new binding (noting that we've done so in the commit message).


That's fine, maybe in a separate patch (2nd one)?


Best regards,
Krzysztof