On Tue, Mar 14, 2023, at 20:57, Krzysztof Kozlowski wrote:
> On 14/03/2023 17:30, Arnd Bergmann wrote:
>
> Please split bindings to separate patch.
>
>> MAINTAINERS | 1 +
>> arch/arm/boot/dts/omap2.dtsi | 4 ++
>> arch/arm/boot/dts/omap2420-n8x0-common.dtsi | 12 ++++
>
> DTS as well...
ok
>> +maintainers:
>> + - Johannes Berg <johannes@xxxxxxxxxxxxxxxx>
>> + - Christian Lamparter <chunkeey@xxxxxx>
>> +
>> +description: |
>
> You can drop '|'.
ok
>> +properties:
>> + compatible:
>> + enum:
>> + - st,stlc4550
>> + - st,stlc4560
>> + - isil,p54spi
>> + - cnxt,3110x
>
> Order above entries by name.
ok
>> +
>> + power-gpios:
>
> If this is GPIO driving some power pin, then it should be
> "powerdown-gpios" (like in /bindings/gpio/gpio-consumer-common.yaml)
As far as I can tell, it's the opposite: the gpio turns the power on
in 'high' state. I could make it GPIO_ACTIVE_LOW and call it powerdown,
if you think that's better, but I don't think that is how it was
meant.
>> +examples:
>> + - |
>> + gpio {
>
> Align example with above |, so four spaces. Or better indent entire
> example with four spaces.
Ok, that makes it much more readable.
>> + gpio-controller;
>> + #gpio-cells = <1>;
>> + #interupt-cells = <1>;
>> + };
>
> Drop "gpio" node. It's not needed for the example.
ok.
>> + spi {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + wifi@0 {
>> + reg = <0>;
>> + compatible = "st,stlc4560";
>
> compatible before reg.
ok.
Arnd
