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

["Andreas Färber" <afaerber@suse.de>]

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.

Thanks,
Andreas

-- 
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer
HRB 36809 (AG Nürnberg)