Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.38 as permitted sender) client-ip=23.128.96.38;
MIME-Version: 1.0
References: <20231013-marvell-88e6152-wan-led-v1-0-0712ba99857c@linaro.org>
 <20231013-marvell-88e6152-wan-led-v1-2-0712ba99857c@linaro.org> <d971d7c1-c6b5-44a4-81cf-4f634e760e87@lunn.ch>
In-Reply-To: <d971d7c1-c6b5-44a4-81cf-4f634e760e87@lunn.ch>
From:   Linus Walleij <linus.walleij@linaro.org>
Date:   Fri, 13 Oct 2023 15:04:10 +0200
Message-ID: <CACRpkdYocdsrsydHwe_FF--6g-Y_YwxHXF6GUTe3wRY0suSCCg@mail.gmail.com>
Subject: Re: [PATCH 2/3] RFC: dt-bindings: marvell: Rewrite in schema
To:     Andrew Lunn <andrew@lunn.ch>
Cc:     Gregory Clement <gregory.clement@bootlin.com>,
        Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Conor Dooley <conor+dt@kernel.org>,
        Russell King <linux@armlinux.org.uk>,
        Florian Fainelli <f.fainelli@gmail.com>,
        Vladimir Oltean <olteanv@gmail.com>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        Christian Marangi <ansuelsmth@gmail.com>,
        linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org, netdev@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Hi Andrew,

thanks for reviewing!

On Fri, Oct 13, 2023 at 2:43=E2=80=AFPM Andrew Lunn <andrew@lunn.ch> wrote:

> > +properties:
> > +  compatible:
> > +    oneOf:
> > +      - enum:
> > +          - marvell,mv88e6060
>
> The 6060 is a separate driver. Its not part of mv88e6xxx. So it should
> have a binding document of its own.

It really doesn't matter to the DT bindings.
It is not the job of DT to reflect the state of Linux.

In another operating system they might all be the same driver.
Or all four variants have their own driver.

If the hardware is distinctly different so a lot of the properties
are unique then it may be warranted with a separate DT
binding, for the sake of keeping bindings simpler and
coherent.

> > +  '#interrupt-cells':
> > +    description: The internal interrupt controller only supports trigg=
ering
> > +      on IRQ_TYPE_LEVEL_HIGH
> > +      # FIXME: what is this? this should be one cell should it not?
> > +      # the Linux mv88e6xxx driver does not implement .irq_set_type in=
 its irq_chip
> > +      # so at least in that implementation the type is flat out ignore=
d.
> > +    const: 2
>
> This interrupt controller is for the embedded PHYs. Its is hard wired
> active high.

Hmm.... I need feedback from the DT people here. It does have a
polarity, but the polarity cannot be changed. So shall we encode this
always the same polarity in the flags cell or skip it altogether?

I'm uncertain. The currens scheme does reflect a reality.

> > +  mdio1:
> > +    $ref: /schemas/net/mdio.yaml#
> > +    unevaluatedProperties: false
> > +    description: Older version of mdio-external
> > +    deprecated: true
> > +    properties:
> > +      compatible:
> > +        const: marvell,mv88e6xxx-mdio-external
>
> The driver only looks at the compatible. It does not care what the
> node is called. So you are going to need to change the driver if you
> want this in the schema.

Yeah, thats what patch 3/3 does :D

Yours,
Linus Walleij