netdev - Re: [PATCH] net: dsa: bcm_sf2: setup BCM4908 internal crossbar

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 10 Mar 2021 09:19:49 -0800
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Rafał Miłecki <zajec5@...il.com>,
        Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        Vladimir Oltean <olteanv@...il.com>
Cc:     "David S . Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>, netdev@...r.kernel.org,
        bcm-kernel-feedback-list@...adcom.com,
        Rafał Miłecki <rafal@...ecki.pl>
Subject: Re: [PATCH] net: dsa: bcm_sf2: setup BCM4908 internal crossbar

On 3/10/21 3:59 AM, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@...ecki.pl>
> 
> On some SoCs (e.g. BCM4908, BCM631[345]8) SF2 has an integrated
> crossbar. It allows connecting its selected external ports to internal
> ports. It's used by vendors to handle custom Ethernet setups.
> 
> BCM4908 has following 3x2 crossbar. On Asus GT-AC5300 rgmii is used for
> connecting external BCM53134S switch. GPHY4 is usually used for WAN
> port. More fancy devices use SerDes for 2.5 Gbps Ethernet.
> 
>               ┌──────────┐
> SerDes ─── 0 ─┤          │
>               │   3x2    ├─ 0 ─── switch port 7
>  GPHY4 ─── 1 ─┤          │
>               │ crossbar ├─ 1 ─── runner (accelerator)
>  rgmii ─── 2 ─┤          │
>               └──────────┘
> 
> Use setup data based on DT info to configure BCM4908's switch port 7.
> Right now only GPHY and rgmii variants are supported. Handling SerDes
> can be implemented later.
> 
> Signed-off-by: Rafał Miłecki <rafal@...ecki.pl>
> ---
>  drivers/net/dsa/bcm_sf2.c      | 41 ++++++++++++++++++++++++++++++++++
>  drivers/net/dsa/bcm_sf2.h      |  1 +
>  drivers/net/dsa/bcm_sf2_regs.h |  7 ++++++
>  3 files changed, 49 insertions(+)
> 
> diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
> index 8f50e91d4004..b4b36408f069 100644
> --- a/drivers/net/dsa/bcm_sf2.c
> +++ b/drivers/net/dsa/bcm_sf2.c
> @@ -432,6 +432,40 @@ static int bcm_sf2_sw_rst(struct bcm_sf2_priv *priv)
>  	return 0;
>  }
>  
> +static void bcm_sf2_crossbar_setup(struct bcm_sf2_priv *priv)
> +{
> +	struct device *dev = priv->dev->ds->dev;
> +	int shift;
> +	u32 mask;
> +	u32 reg;
> +	int i;
> +
> +	reg = 0;

I believe you need to do a read/modify/write here otherwise you are
clobbering the other settings for the p_wan_link_status and
p_wan_link_sel bits.

> +	switch (priv->type) {
> +	case BCM4908_DEVICE_ID:
> +		shift = CROSSBAR_BCM4908_INT_P7 * priv->num_crossbar_int_ports;
> +		if (priv->int_phy_mask & BIT(7))
> +			reg |= CROSSBAR_BCM4908_EXT_GPHY4 << shift;
> +		else if (0) /* FIXME */
> +			reg |= CROSSBAR_BCM4908_EXT_SERDES << shift;
> +		else

Maybe what you can do is change bcm_sf2_identify_ports() such that when
the 'phy-interface' property is retrieved from Device Tree, we also
store the 'mode' variable into the per-port structure
(bcm_sf2_port_status) and when you call bcm_sf2_crossbar_setup() for
each port that has been setup, and you update the logic to look like this:

if (priv->int_phy_mask & BIT(7))
	reg |= CROSSBAR_BCM4908_EXT_GPHY4 << shift;
else if (phy_interface_mode_is_rgmii(mode))
	reg |= CROSSBAR_BCM4908_EXT_RGMII

and we add support for SerDes when we get to that point. This would also
allow you to detect if an invalid configuration is specified via Device
Tree.
	
Other than that, this looks good to me, thanks!
-- 
Florian