Re: [PATCH v2 3/5] arm64: dts: allwinner: a523: Add EMAC0 ethernet MAC

[Andre Przywara <andre.przywara@arm.com>]

On Fri, 25 Apr 2025 22:35:59 +0800
Chen-Yu Tsai <wens@csie.org> wrote:

adding LinusW for a more generic pinctrl question ...

> On Fri, Apr 25, 2025 at 5:41 PM Andre Przywara <andre.przywara@arm.com> wrote:
> >
> > On Fri, 25 Apr 2025 13:26:25 +0800
> > Chen-Yu Tsai <wens@csie.org> wrote:
> >
> > Hi Chen-Yu,
> >  
> > > On Thu, Apr 24, 2025 at 6:09 PM Yixun Lan <dlan@gentoo.org> wrote:  
> > > >
> > > > Add EMAC0 ethernet MAC support which found on A523 variant SoCs,
> > > > including the A527/T527 chips. MAC0 is compatible to the A64 chip which
> > > > requires an external PHY. This patch only add RGMII pins for now.
> > > >
> > > > Signed-off-by: Yixun Lan <dlan@gentoo.org>
> > > > ---
> > > >  arch/arm64/boot/dts/allwinner/sun55i-a523.dtsi | 40 ++++++++++++++++++++++++++
> > > >  1 file changed, 40 insertions(+)
> > > >
> > > > diff --git a/arch/arm64/boot/dts/allwinner/sun55i-a523.dtsi b/arch/arm64/boot/dts/allwinner/sun55i-a523.dtsi
> > > > index ee485899ba0af69f32727a53de20051a2e31be1d..c9a9b9dd479af05ba22fe9d783e32f6d61a74ef7 100644
> > > > --- a/arch/arm64/boot/dts/allwinner/sun55i-a523.dtsi
> > > > +++ b/arch/arm64/boot/dts/allwinner/sun55i-a523.dtsi
> > > > @@ -126,6 +126,15 @@ pio: pinctrl@2000000 {
> > > >                         interrupt-controller;
> > > >                         #interrupt-cells = <3>;
> > > >
> > > > +                       rgmii0_pins: rgmii0-pins {
> > > > +                               pins = "PH0", "PH1", "PH2", "PH3", "PH4",
> > > > +                                      "PH5", "PH6", "PH7", "PH9", "PH10",
> > > > +                                      "PH14", "PH15", "PH16", "PH17", "PH18";
> > > > +                               allwinner,pinmux = <5>;
> > > > +                               function = "emac0";
> > > > +                               drive-strength = <40>;  
> > >
> > > We should probably add
> > >
> > >                                   bias-disable;
> > >
> > > to explicitly turn off pull-up and pull-down.  
> >
> > Should we? I don't see this anywhere else for sunxi, probably because it is
> > the (reset) default (0b00).
> > I wonder if we have a hidden assumption about this? As in: if no bias is
> > specified, we assume bias-disable? Then we should maybe enforce this is in
> > the driver?  
> 
> There isn't any assumption, as in we were fine with either the reset
> default or whatever the bootloader left it in. However in projects at
> work I learned that it's better to have explicit settings despite
> working defaults.

I totally agree, but my point was that this applies basically to every
pinctrl user. I usually think of the bias settings as "do we need
pull-ups or pull-downs", and if nothing is specified, I somewhat assume
bias-disable.

So I am fine with this being added here, but was wondering if we should
look at a more generic solution.

Linus: is bias-disable assumed to be the default, that pinctrl drivers
should set in absence of explicit properties? Or is this "whatever is in
the registers at boot" the default we have to live with?

Cheers,
Andre