On 3/23/23 13:10, Paul Geurts wrote:
-----Original Message-----
From: Florian Fainelli <f.fainelli@xxxxxxxxx>
Sent: donderdag 23 maart 2023 17:43
To: Álvaro Fernández Rojas <noltari@xxxxxxxxx>; Paul Geurts
<paul.geurts@xxxxxxxxxxxxxxxxxxxxxxxxx>; jonas.gorski@xxxxxxxxx;
andrew@xxxxxxx; olteanv@xxxxxxxxx; davem@xxxxxxxxxxxxx;
edumazet@xxxxxxxxxx; kuba@xxxxxxxxxx; pabeni@xxxxxxxxxx;
robh+dt@xxxxxxxxxx; krzysztof.kozlowski+dt@xxxxxxxxxx;
netdev@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux-
kernel@xxxxxxxxxxxxxxx
Subject: Re: [PATCH 2/2] net: dsa: b53: mdio: add support for BCM53134
On 3/23/23 05:18, Álvaro Fernández Rojas wrote:
From: Paul Geurts <paul.geurts@xxxxxxxxxxxxxxxxxxxxxxxxx>
Add support for the BCM53134 Ethernet switch in the existing b53 dsa
driver.
BCM53134 is very similar to the BCM58XX series.
Signed-off-by: Paul Geurts <paul.geurts@xxxxxxxxxxxxxxxxxxxxxxxxx>
Signed-off-by: Álvaro Fernández Rojas <noltari@xxxxxxxxx>
---
drivers/net/dsa/b53/b53_common.c | 53
+++++++++++++++++++++++++++++++-
drivers/net/dsa/b53/b53_mdio.c | 5 ++-
drivers/net/dsa/b53/b53_priv.h | 9 +++++-
3 files changed, 64 insertions(+), 3 deletions(-)
diff --git a/drivers/net/dsa/b53/b53_common.c
b/drivers/net/dsa/b53/b53_common.c
index 1f9b251a5452..aaa0813e6f59 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -1282,6 +1282,42 @@ static void b53_adjust_link(struct dsa_switch
*ds, int port,
if (is63xx(dev) && port >= B53_63XX_RGMII0)
b53_adjust_63xx_rgmii(ds, port, phydev->interface);
+ if (is53134(dev) && phy_interface_is_rgmii(phydev)) {
Why is not this in the same code block as the one for the is531x5() device like
this:
diff --git a/drivers/net/dsa/b53/b53_common.c
b/drivers/net/dsa/b53/b53_common.c
index 59cdfc51ce06..1c64b6ce7e78 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -1235,7 +1235,7 @@ static void b53_adjust_link(struct dsa_switch *ds,
int port,
tx_pause, rx_pause);
b53_force_link(dev, port, phydev->link);
- if (is531x5(dev) && phy_interface_is_rgmii(phydev)) {
+ if ((is531x5(dev) || is53134(dev)) &&
phy_interface_is_rgmii(phydev)) {
if (port == dev->imp_port)
off = B53_RGMII_CTRL_IMP;
else
Other than that, LGTM!
--
Florian
I think the only reason is that the BCM53134 does not support the
RGMII_CTRL_TIMING_SEL bit, which is set in the original block. I agree
Putting a if statement around
rgmii_ctrl |= RGMII_CTRL_TIMING_SEL;
would prevent a lot of code duplication. _however_, after looking at it again,
I don’t think the device does not support the bit. When looking at the datasheet,
The same bit in the this register is called BYPASS_2NS_DEL. It's very uncommon
For Broadcom to make such a change in the register interface, so maybe they
Just renamed it. Do you think this could be the same bit?
Yes, I think this is exactly the same bit, just named differently. What
strikes me as odd is that neither of the 53115, 53125 or 53128 which are
guarded by the is531x5() conditional have it defined.
--
Florian
