Re: [PATCH v2 2/2] ARM: dts: r9a06g032: add r9a06g032-rzn1d400-eb board device-tree

[Clément Léger]

Le Wed, 15 Feb 2023 09:29:33 +0100,
Clément Léger <clement.leger@xxxxxxxxxxx> a écrit :

> Le Tue, 14 Feb 2023 17:25:14 +0100,
> Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> a écrit :
> 
> > Hi Clément,
> > 
> > CC Gareth
> > 
> > On Thu, Feb 9, 2023 at 2:32 PM Clément Léger <clement.leger@xxxxxxxxxxx> 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@xxxxxxxxxxx>    
> > 
> > Thanks for your patch!
> >   
> > > --- /dev/null
> > > +++ b/arch/arm/boot/dts/r9a06g032-rzn1d400-eb.dts
> > > @@ -0,0 +1,94 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Device Tree Source for the RZN1D-EB Board
> > > + *
> > > + * Copyright (C) 2023 Schneider-Electric
> > > + *
> > > + */
> > > +
> > > +#include "r9a06g032-rzn1d400-db.dts"
> > > +
> > > +/ {
> > > +       model = "RZN1D-EB Board";
> > > +       compatible = "renesas,rzn1d400-eb", "renesas,rzn1d400-db",
> > > +                    "renesas,r9a06g032";
> > > +};
> > > +
> > > +&mii_conv2 {
> > > +       renesas,miic-input = <MIIC_SWITCH_PORTD>;
> > > +       status = "okay";
> > > +};
> > > +
> > > +&mii_conv3 {
> > > +       renesas,miic-input = <MIIC_SWITCH_PORTC>;
> > > +       status = "okay";
> > > +};
> > > +
> > > +&pinctrl{
> > > +       pins_eth1: pins-eth1 {
> > > +               pinmux = <RZN1_PINMUX(12, RZN1_FUNC_CLK_ETH_MII_RGMII_RMII)>,
> > > +                        <RZN1_PINMUX(13, RZN1_FUNC_CLK_ETH_MII_RGMII_RMII)>,
> > > +                        <RZN1_PINMUX(14, RZN1_FUNC_CLK_ETH_MII_RGMII_RMII)>,
> > > +                        <RZN1_PINMUX(15, RZN1_FUNC_CLK_ETH_MII_RGMII_RMII)>,
> > > +                        <RZN1_PINMUX(16, RZN1_FUNC_CLK_ETH_MII_RGMII_RMII)>,
> > > +                        <RZN1_PINMUX(17, RZN1_FUNC_CLK_ETH_MII_RGMII_RMII)>,
> > > +                        <RZN1_PINMUX(18, RZN1_FUNC_CLK_ETH_MII_RGMII_RMII)>,
> > > +                        <RZN1_PINMUX(19, RZN1_FUNC_CLK_ETH_MII_RGMII_RMII)>,
> > > +                        <RZN1_PINMUX(20, RZN1_FUNC_CLK_ETH_MII_RGMII_RMII)>,
> > > +                        <RZN1_PINMUX(21, RZN1_FUNC_CLK_ETH_MII_RGMII_RMII)>,
> > > +                        <RZN1_PINMUX(22, RZN1_FUNC_CLK_ETH_MII_RGMII_RMII)>,
> > > +                        <RZN1_PINMUX(23, RZN1_FUNC_CLK_ETH_MII_RGMII_RMII)>;
> > > +               drive-strength = <6>;
> > > +               bias-disable;
> > > +       };
> > > +
> > > +       pins_eth2: pins-eth2 {
> > > +               pinmux = <RZN1_PINMUX(24, RZN1_FUNC_CLK_ETH_MII_RGMII_RMII)>,
> > > +                        <RZN1_PINMUX(25, RZN1_FUNC_CLK_ETH_MII_RGMII_RMII)>,
> > > +                        <RZN1_PINMUX(26, RZN1_FUNC_CLK_ETH_MII_RGMII_RMII)>,
> > > +                        <RZN1_PINMUX(27, RZN1_FUNC_CLK_ETH_MII_RGMII_RMII)>,
> > > +                        <RZN1_PINMUX(28, RZN1_FUNC_CLK_ETH_MII_RGMII_RMII)>,
> > > +                        <RZN1_PINMUX(29, RZN1_FUNC_CLK_ETH_MII_RGMII_RMII)>,
> > > +                        <RZN1_PINMUX(30, RZN1_FUNC_CLK_ETH_MII_RGMII_RMII)>,
> > > +                        <RZN1_PINMUX(31, RZN1_FUNC_CLK_ETH_MII_RGMII_RMII)>,
> > > +                        <RZN1_PINMUX(32, RZN1_FUNC_CLK_ETH_MII_RGMII_RMII)>,
> > > +                        <RZN1_PINMUX(33, RZN1_FUNC_CLK_ETH_MII_RGMII_RMII)>,
> > > +                        <RZN1_PINMUX(34, RZN1_FUNC_CLK_ETH_MII_RGMII_RMII)>,
> > > +                        <RZN1_PINMUX(35, RZN1_FUNC_CLK_ETH_MII_RGMII_RMII)>;
> > > +               drive-strength = <6>;
> > > +               bias-disable;
> > > +       };
> > > +};
> > > +
> > > +&switch {
> > > +       pinctrl-names = "default";    
> > 
> > No need to specify pinctrl-names, as it is inherited from
> > r9a06g032-rzn1d400-db.dts.  
> 
> Acked.
> 
> >   
> > > +       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.
> 

Hi Geert,

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.

-- 
Clément Léger,
Embedded Linux and Kernel engineer at Bootlin
https://bootlin.com