On 3/11/21 2:04 PM, Rafał Miłecki wrote:
> On 10.03.2021 18:19, Florian Fainelli wrote:
>> On 3/10/21 3:59 AM, Rafał Miłecki wrote:
>>> From: Rafał Miłecki <ra...@milecki.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 <ra...@milecki.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.
>
> Thanks, I didn't know about those 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.
>
> Sounds great, but I experienced a problem while trying to implement
> that.
>
> On Asus GT-AC5300 I have:
>
> /* External BCM53134S switch */ port@7 { label = "sw"; reg = <7>;
>
> fixed-link { speed = <1000>; full-duplex; }; };
>
> after adding phy-mode = "rgmii"; to it, my PHYs stop working because
> of SF2.
>
> bcm_sf2_sw_mac_link_up() calls: bcm_sf2_sw_mac_link_set(ds, 7,
> PHY_INTERFACE_MODE_RGMII, true); which results in setting
> RGMII_MODE_EN bit in the REG_RGMII_CNTRL_P(7).
>
> For some reason setting above bit results in stopping internal PHYs.
> unimac_mdio_read() starts getting MDIO_READ_FAIL.
>
> Do you have any idea why it happens?
RGMII_MODE_EN enables the RGMII data pad, but this usually has no
incidence on the MDIO which is separate, unless there is something I do
not understand about how the crossbar works.
--
Florian
