Re: [PATCH mvebu-dt v2 4/6] ARM: dts: turris-omnia: add SFP node

["Marek Behún" <kabel@kernel.org>]

On Sat, 14 Nov 2020 22:36:09 +0100
Andreas Färber <afaerber@suse.de> wrote:

> Hi Marek,
> 
> On 14.11.20 19:32, Marek Behún wrote:
> > Turris Omnia has a SFP cage that, together with WAN PHY is connected to  
> 
> "an SFP"
> Comma missing after PHY (or drop before together).
> 
> > eth2 SerDes via a SerDes multiplexor. Describe the SFP cage, but leave
> > it disabled. Until phylink has support for such multiplexor we will
> > leave it to U-Boot to enable SFP and disable WAN PHY at boot time
> > depending on whether a SFP module is present.  
> 
> multiplexor vs. multiplexer may be a British thing? Thunderbird
> underlines it fwiw.
> 
> > 
> > Signed-off-by: Marek Behún <kabel@kernel.org>
> > Fixes: 26ca8b52d6e1 ("ARM: dts: add support for Turris Omnia")
> > Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> > Cc: Uwe Kleine-König <uwe@kleine-koenig.org>
> > Cc: Jason Cooper <jason@lakedaemon.net>
> > Cc: Gregory CLEMENT <gregory.clement@bootlin.com>
> > Cc: Andreas Färber <afaerber@suse.de>
> > Cc: Rob Herring <robh+dt@kernel.org>
> > Cc: devicetree@vger.kernel.org
> > ---
> >  arch/arm/boot/dts/armada-385-turris-omnia.dts | 19 ++++++++++++++++++-
> >  1 file changed, 18 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm/boot/dts/armada-385-turris-omnia.dts b/arch/arm/boot/dts/armada-385-turris-omnia.dts
> > index 7ccebf7d1757..14c21cddef72 100644
> > --- a/arch/arm/boot/dts/armada-385-turris-omnia.dts
> > +++ b/arch/arm/boot/dts/armada-385-turris-omnia.dts
> > @@ -82,6 +82,22 @@ pcie@3,0 {
> >  			};
> >  		};
> >  	};
> > +
> > +	sfp: sfp {
> > +		compatible = "sff,sfp";
> > +		i2c-bus = <&sfp_i2c>;
> > +		tx-fault-gpios = <&pcawan 0 GPIO_ACTIVE_HIGH>;
> > +		tx-disable-gpios = <&pcawan 1 GPIO_ACTIVE_HIGH>;
> > +		rate-select0-gpios = <&pcawan 2 GPIO_ACTIVE_HIGH>;
> > +		los-gpios = <&pcawan 3 GPIO_ACTIVE_HIGH>;
> > +		mod-def0-gpios = <&pcawan 4 GPIO_ACTIVE_LOW>;
> > +
> > +		/*
> > +		 * For now this has to be enabled at boot time by U-Boot when
> > +		 * a SFP module is present.
> > +		 */
> > +		status = "disabled";
> > +	};
> >  };
> >  
> >  &bm {
> > @@ -130,6 +146,7 @@ &eth2 {
> >  	phy-mode = "sgmii";
> >  	phy = <&phy1>;
> >  	phys = <&comphy5 2>;
> > +	sfp = <&sfp>;
> >  	buffer-manager = <&bm>;
> >  	bm,pool-long = <2>;
> >  	bm,pool-short = <3>;
> > @@ -195,7 +212,7 @@ i2c@3 {
> >  			/* routed to PCIe2 connector (CN62A) */
> >  		};
> >  
> > -		i2c@4 {
> > +		sfp_i2c: i2c@4 {
> >  			#address-cells = <1>;
> >  			#size-cells = <0>;
> >  			reg = <4>;  
> 
> Matches what I've come up with,
> 
> Reviewed-by: Andreas Färber <afaerber@suse.de>
> 
> However, I also needed to set managed = "in-band-status" when enabling
> SFP node and removing phy property. Shall we prepare it with its default
> value of "auto" and add a comment? (unlike disabled -> okay,
> in-band-status is longer than auto, so not sure whether it helps U-Boot,
> but it may help humans.

The idea is that for now when U-Boot detects that SFP is present, it
shall change the device tree accordingly:
  remove phy-handle
  add managed = in-band-status
  enable sfp node

This is the only way to support this reasobaly until the multiplexer is
somehow supported by kernel.

Marek