Hello Damien! On Fri, 2023-06-02 at 08:57 +0900, Damien Le Moal wrote: > On 6/1/23 14:45, Nikita Shubin wrote: > > Add YAML bindings for ep93xx SoC PATA. > > > > Signed-off-by: Nikita Shubin <nikita.shubin@xxxxxxxxxxx> > > --- > > > > Notes: > > v0 -> v1: > > > > - renamed file to ep9312-pata > > Looks OK to me but given that this is both for the cirrus,ep9315-pata > and > cirrus,ep9312-pata, wouldn't it be better to name the file > cirrus,ep931x-pata.yaml ? I was advised against using wildcards by Arnd and Krzysztof. See https://lore.kernel.org/all/c981e048-8925-deba-6916-9199844976b9@xxxxxxxxxx/ As i understood we should have at least one fallback, in out case it's "cirrus,ep9312-pata" and one for each SoC variant that supports it. All other comments acknowledged and agreed. I will also change ``` >> + if (!drv_data) >> + return -ENXIO; ``` To ENOMEM, as a part of dt conversion patch in v2. > > > > - changed email to dlemoal@xxxxxxxxxx > > - dropped label > > - fixed ident > > > > .../bindings/ata/cirrus,ep9312-pata.yaml | 44 > > +++++++++++++++++++ > > 1 file changed, 44 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/ata/cirrus,ep9312-pata.yaml > > > > diff --git a/Documentation/devicetree/bindings/ata/cirrus,ep9312- > > pata.yaml b/Documentation/devicetree/bindings/ata/cirrus,ep9312- > > pata.yaml > > new file mode 100644 > > index 000000000000..3489be55a6fe > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/ata/cirrus,ep9312-pata.yaml > > @@ -0,0 +1,44 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/ata/cirrus,ep9312-pata.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Cirrus Logic EP9312 PATA controller > > + > > +maintainers: > > + - Damien Le Moal <dlemoal@xxxxxxxxxx> > > + > > +properties: > > + compatible: > > + oneOf: > > + - const: cirrus,ep9312-pata > > I am not a DT specialist, but isn't this line superfluous since it is > listed in > the items ? > > > + - items: > > + - const: cirrus,ep9315-pata > > + - const: cirrus,ep9312-pata > > + > > + reg: > > + maxItems: 1 > > + > > + interrupts: > > + maxItems: 1 > > + > > +required: > > + - compatible > > + - reg > > + - interrupts > > + > > +additionalProperties: false > > + > > +examples: > > + - | > > + ide@800a0000 { > > + compatible = "cirrus,ep9312-pata"; > > + reg = <0x800a0000 0x38>; > > + interrupt-parent = <&vic1>; > > + interrupts = <8>; > > + pinctrl-names = "default"; > > + pinctrl-0 = <&ide_default_pins>; > > + }; > > + > > +... >
