Message-ID: <CAMuHMdVAJRhSLYbt27P-AzwHc89+MYKi-3KmkhT=hhXq27UFbQ@mail.gmail.com>
Date: Fri, 14 Feb 2025 14:42:14 +0100
From: Geert Uytterhoeven <geert@...ux-m68k.org>
To: Thomas Bonnefille <thomas.bonnefille@...tlin.com>
Cc: Miquel Raynal <miquel.raynal@...tlin.com>, Romain Gantois <romain.gantois@...tlin.com>, 
	Magnus Damm <magnus.damm@...il.com>, Rob Herring <robh+dt@...nel.org>, 
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>, 
	Thomas Petazzoni <thomas.petazzoni@...tlin.com>, Herve Codina <herve.codina@...tlin.com>, 
	Milan Stevanovic <milan.stevanovic@...com>, Jimmy Lalande <jimmy.lalande@...com>, 
	Pascal Eberhard <pascal.eberhard@...com>, linux-renesas-soc@...r.kernel.org, 
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, 
	Gareth Williams <gareth.williams.jx@...esas.com>, 
	Wolfram Sang <wsa+renesas@...g-engineering.com>
Subject: Re: [PATCH v2 2/2] ARM: dts: r9a06g032: add r9a06g032-rzn1d400-eb
 board device-tree

Hi Thomas,

On Fri, 14 Feb 2025 at 14:20, Thomas Bonnefille
<thomas.bonnefille@...tlin.com> wrote:
> >> On Wed, Feb 15, 2023 at 12:31 PM Geert Uytterhoeven
> >> <geert@...ux-m68k.org> wrote:
> >>> On Wed, Feb 15, 2023 at 11:52 AM Clément Léger
> >>> <clement.leger@...tlin.com> wrote:
> >>> > Le Wed, 15 Feb 2023 09:29:33 +0100,
> >>> > Clément Léger <clement.leger@...tlin.com> a écrit :
> >>> > > Le Tue, 14 Feb 2023 17:25:14 +0100,
> >>> > > Geert Uytterhoeven <geert@...ux-m68k.org> a écrit :
> >>> > > > On Thu, Feb 9, 2023 at 2:32 PM Clément Léger <clement.leger@...tlin.com> wrote:
> >>> > > > > The EB board (Expansion board) supports both RZ/N1D and RZ-N1S. Since this
> >>> > > > > configuration targets only the RZ/N1D, it is named r9a06g032-rzn1d400-eb.
> >>> > > > > It adds support for the 2 additional switch ports (port C and D) that are
> >>> > > > > available on that board.
> >>> > > > >
> >>> > > > > Signed-off-by: Clément Léger <clement.leger@...tlin.com>
> >>> > > >
> >>> > > > Thanks for your patch!
> >>> > > >
> >>> > > > > --- /dev/null
> >>> > > > > +++ b/arch/arm/boot/dts/r9a06g032-rzn1d400-eb.dts
> >>>
> >>> > > > > +       pinctrl-0 = <&pins_eth1>, <&pins_eth2>, <&pins_eth3>, <&pins_eth4>,
> >>> > > > > +                   <&pins_mdio1>;
> >>> > > > > +
> >>> > > > > +       mdio {
> >>> > > > > +               /* CN15 and CN16 switches must be configured in MDIO2 mode */
> >>> > > > > +               switch0phy1: ethernet-phy@1 {
> >>> > > > > +                       reg = <1>;
> >>> > > > > +                       marvell,reg-init = <3 16 0 0x1010>;
> >>> > > >
> >>> > > > marvell,reg-init is not documented in any DT bindings document?
> >>> > >
> >>> > > Indeed, this is not somethiong that should be made available here. It's
> >>> > > only inverting the LED polarity but supported by some internal patch.
> >>> > > I'll remove that.
> >>>
> >>> > I actually was confused by a property I added in another device-tree but
> >>> > marvell,reg-init exists, is handled by the marvell phy driver and used
> >>> > in a few device-trees. Strangely, it is not documented anywhere. So I
> >>> > can either remove that (and the LED won't work properly) or let it live
> >>> > depending on what you prefer.
> >>>
> >>> In that case, please keep it.
> >>> But the property really should be documented, one day...
>
> As Clément mentioned, this property is used to set up the LEDs for
> Marvell PHY. However, Marvell's PHYs have no dedicated bindings; only
> their associated switches do. PHY's usually don't have their own yaml,
> so there is no easy place where to add this property. We could however
> describe them in the numerous switch bindings that embed a Marvell PHY,
> which are: Qualcomm ETHQOS, Cadence MACB/GEM, Gianfar, Freescale FEC,
> Renesas switches and of course Marvell switches.
>
> I already thought of doing it in the binding of the renesas switch, like
> this :
>
> Documentation/devicetree/bindings/net/dsa/renesas,rzn1-a5psw.yaml:
> ```
>
> \[...\]
>
> mdio:
> $ref: /schemas/net/mdio.yaml#
> patternProperties:
> '@\[0-9a-f\]+$':
> properties:
> marvel,reg-init:
> - description: Lorem Ipsum
>
> unevaluatedProperties: false
>
> \[...\]
>
> ```
> but it would document it only for this particular switch.
> It is also possible to do it in the main mdio.yaml on the model of this:
> https://elixir.bootlin.com/linux/v6.13.1/source/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml#L121
>
> What's your opinion on this ?

Oh, so this is a similar issue as the one preventing us from converting
the Micrel PHY bindings to dt-schema[1]?

You could still document it in a text binding file:
Documentation/devicetree/bindings/net/marvell,phy.txt
That cannot be used for validation, but at least people can find the
property using git grep...

> Moreover, everywhere this property is used in the kernel, it is to set
> up the LEDs. Nowadays, the Marvell PHY driver should be able to handle
> LEDs without this property. Therefore, this property should be
> deprecated in this case.

So the LED works now on this board without the property?
Then the property can be dropped?

[1] "[PATCH] [RFC] dt-bindings: net: micrel: Convert to json-schema"
    https://lore.kernel.org/linux-renesas-soc/943cb31d01d0da3a63911326e24fbf9b328f7206.1731580776.git.geert+renesas@glider.be

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Powered by blists - more mailing lists