[PATCH 0/2] net: dsa: b53: mdio: add support for BCM53134

[PATCH 0/2] net: dsa: b53: mdio: add support for BCM53134

[Álvaro Fernández Rojas]

This is based on the initial work from Paul Geurts that was sent to the
incorrect linux development lists and recipients.
I've modified it by removing BCM53134_DEVICE_ID from is531x5() and therefore
adding is53134() where needed.
I also added a separate RGMII handling block for is53134() since according to
Paul, BCM53134 doesn't support RGMII_CTRL_TIMING_SEL as opposed to is531x5().

Paul Geurts (1):
  net: dsa: b53: mdio: add support for BCM53134

Álvaro Fernández Rojas (1):
  dt-bindings: net: dsa: b53: add BCM53134 support

 .../devicetree/bindings/net/dsa/brcm,b53.yaml |  1 +
 drivers/net/dsa/b53/b53_common.c              | 53 ++++++++++++++++++-
 drivers/net/dsa/b53/b53_mdio.c                |  5 +-
 drivers/net/dsa/b53/b53_priv.h                |  9 +++-
 4 files changed, 65 insertions(+), 3 deletions(-)

-- 
2.30.2

[PATCH 1/2] dt-bindings: net: dsa: b53: add BCM53134 support

[Álvaro Fernández Rojas]

BCM53134 are B53 switches connected by MDIO.

Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
---
 Documentation/devicetree/bindings/net/dsa/brcm,b53.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/net/dsa/brcm,b53.yaml b/Documentation/devicetree/bindings/net/dsa/brcm,b53.yaml
index 57e0ef93b134..4c78c546343f 100644
--- a/Documentation/devicetree/bindings/net/dsa/brcm,b53.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/brcm,b53.yaml
@@ -19,6 +19,7 @@ properties:
       - const: brcm,bcm53115
       - const: brcm,bcm53125
       - const: brcm,bcm53128
+      - const: brcm,bcm53134
       - const: brcm,bcm5365
       - const: brcm,bcm5395
       - const: brcm,bcm5389
-- 
2.30.2

[PATCH 2/2] net: dsa: b53: mdio: add support for BCM53134

[Álvaro Fernández Rojas]

From: Paul Geurts <paul.geurts@prodrive-technologies.com>

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@prodrive-technologies.com>
Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
---
 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)) {
+		if (port == dev->imp_port)
+			off = B53_RGMII_CTRL_IMP;
+		else
+			off = B53_RGMII_CTRL_P(port);
+
+		/* Configure the port RGMII clock delay by DLL disabled and
+		 * tx_clk aligned timing (restoring to reset defaults)
+		 */
+		b53_read8(dev, B53_CTRL_PAGE, off, &rgmii_ctrl);
+		rgmii_ctrl &= ~(RGMII_CTRL_DLL_RXC | RGMII_CTRL_DLL_TXC);
+
+		/* PHY_INTERFACE_MODE_RGMII_TXID means TX internal delay, make
+		 * sure that we enable the port TX clock internal delay to
+		 * account for this internal delay that is inserted, otherwise
+		 * the switch won't be able to receive correctly.
+		 *
+		 * PHY_INTERFACE_MODE_RGMII means that we are not introducing
+		 * any delay neither on transmission nor reception, so the
+		 * BCM53134 must also be configured accordingly to account for
+		 * the lack of delay and introduce
+		 *
+		 * The BCM53134 switch has its RX clock and TX clock control
+		 * swapped, hence the reason why we modify the TX clock path in
+		 * the "RGMII" case
+		 */
+		if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID)
+			rgmii_ctrl |= RGMII_CTRL_DLL_TXC;
+		if (phydev->interface == PHY_INTERFACE_MODE_RGMII)
+			rgmii_ctrl |= RGMII_CTRL_DLL_TXC | RGMII_CTRL_DLL_RXC;
+		b53_write8(dev, B53_CTRL_PAGE, off, rgmii_ctrl);
+
+		dev_info(ds->dev, "Configured port %d for %s\n", port,
+			 phy_modes(phydev->interface));
+	}
+
 	if (is531x5(dev) && phy_interface_is_rgmii(phydev)) {
 		if (port == dev->imp_port)
 			off = B53_RGMII_CTRL_IMP;
@@ -2613,6 +2649,20 @@ static const struct b53_chip_data b53_switch_chips[] = {
 		.jumbo_pm_reg = B53_JUMBO_PORT_MASK,
 		.jumbo_size_reg = B53_JUMBO_MAX_SIZE,
 	},
+	{
+		.chip_id = BCM53134_DEVICE_ID,
+		.dev_name = "BCM53134",
+		.vlans = 4096,
+		.enabled_ports = 0x12f,
+		.imp_port = 8,
+		.cpu_port = B53_CPU_PORT,
+		.vta_regs = B53_VTA_REGS,
+		.arl_bins = 4,
+		.arl_buckets = 1024,
+		.duplex_reg = B53_DUPLEX_STAT_GE,
+		.jumbo_pm_reg = B53_JUMBO_PORT_MASK,
+		.jumbo_size_reg = B53_JUMBO_MAX_SIZE,
+	},
 };
 
 static int b53_switch_init(struct b53_device *dev)
@@ -2671,7 +2721,7 @@ static int b53_switch_init(struct b53_device *dev)
 	dev->ds->num_ports = min_t(unsigned int, dev->num_ports, DSA_MAX_PORTS);
 
 	/* Include non standard CPU port built-in PHYs to be probed */
-	if (is539x(dev) || is531x5(dev)) {
+	if (is539x(dev) || is531x5(dev) || is53134(dev)) {
 		for (i = 0; i < dev->num_ports; i++) {
 			if (!(dev->ds->phys_mii_mask & BIT(i)) &&
 			    !b53_possible_cpu_port(dev->ds, i))
@@ -2790,6 +2840,7 @@ int b53_switch_detect(struct b53_device *dev)
 		case BCM53012_DEVICE_ID:
 		case BCM53018_DEVICE_ID:
 		case BCM53019_DEVICE_ID:
+		case BCM53134_DEVICE_ID:
 			dev->chip_id = id32;
 			break;
 		default:
diff --git a/drivers/net/dsa/b53/b53_mdio.c b/drivers/net/dsa/b53/b53_mdio.c
index 6ddc03b58b28..8b422b298cd5 100644
--- a/drivers/net/dsa/b53/b53_mdio.c
+++ b/drivers/net/dsa/b53/b53_mdio.c
@@ -286,6 +286,7 @@ static const struct b53_io_ops b53_mdio_ops = {
 #define B53_BRCM_OUI_2	0x03625c00
 #define B53_BRCM_OUI_3	0x00406000
 #define B53_BRCM_OUI_4	0x01410c00
+#define B53_BRCM_OUI_5	0xae025000
 
 static int b53_mdio_probe(struct mdio_device *mdiodev)
 {
@@ -313,7 +314,8 @@ static int b53_mdio_probe(struct mdio_device *mdiodev)
 	if ((phy_id & 0xfffffc00) != B53_BRCM_OUI_1 &&
 	    (phy_id & 0xfffffc00) != B53_BRCM_OUI_2 &&
 	    (phy_id & 0xfffffc00) != B53_BRCM_OUI_3 &&
-	    (phy_id & 0xfffffc00) != B53_BRCM_OUI_4) {
+	    (phy_id & 0xfffffc00) != B53_BRCM_OUI_4 &&
+	    (phy_id & 0xfffffc00) != B53_BRCM_OUI_5) {
 		dev_err(&mdiodev->dev, "Unsupported device: 0x%08x\n", phy_id);
 		return -ENODEV;
 	}
@@ -375,6 +377,7 @@ static const struct of_device_id b53_of_match[] = {
 	{ .compatible = "brcm,bcm53115" },
 	{ .compatible = "brcm,bcm53125" },
 	{ .compatible = "brcm,bcm53128" },
+	{ .compatible = "brcm,bcm53134" },
 	{ .compatible = "brcm,bcm5365" },
 	{ .compatible = "brcm,bcm5389" },
 	{ .compatible = "brcm,bcm5395" },
diff --git a/drivers/net/dsa/b53/b53_priv.h b/drivers/net/dsa/b53/b53_priv.h
index a689a6950189..7888be1fd23f 100644
--- a/drivers/net/dsa/b53/b53_priv.h
+++ b/drivers/net/dsa/b53/b53_priv.h
@@ -80,6 +80,7 @@ enum {
 	BCM583XX_DEVICE_ID = 0x58300,
 	BCM7445_DEVICE_ID = 0x7445,
 	BCM7278_DEVICE_ID = 0x7278,
+	BCM53134_DEVICE_ID = 0x5075,
 };
 
 struct b53_pcs {
@@ -215,7 +216,13 @@ static inline int is58xx(struct b53_device *dev)
 	return dev->chip_id == BCM58XX_DEVICE_ID ||
 		dev->chip_id == BCM583XX_DEVICE_ID ||
 		dev->chip_id == BCM7445_DEVICE_ID ||
-		dev->chip_id == BCM7278_DEVICE_ID;
+		dev->chip_id == BCM7278_DEVICE_ID ||
+		dev->chip_id == BCM53134_DEVICE_ID;
+}
+
+static inline int is53134(struct b53_device *dev)
+{
+	return dev->chip_id == BCM53134_DEVICE_ID;
 }
 
 #define B53_63XX_RGMII0	4
-- 
2.30.2

Re: [PATCH 1/2] dt-bindings: net: dsa: b53: add BCM53134 support

[Florian Fainelli]

On 3/23/23 05:18, Álvaro Fernández Rojas wrote:
> BCM53134 are B53 switches connected by MDIO.
> 
> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

Re: [PATCH 2/2] net: dsa: b53: mdio: add support for BCM53134

[Florian Fainelli]

On 3/23/23 05:18, Álvaro Fernández Rojas wrote:
> From: Paul Geurts <paul.geurts@prodrive-technologies.com>
> 
> 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@prodrive-technologies.com>
> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
> ---
>   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

Re: [PATCH 2/2] net: dsa: b53: mdio: add support for BCM53134

[Álvaro Fernández Rojas]

El jue, 23 mar 2023 a las 17:43, Florian Fainelli
(<f.fainelli@gmail.com>) escribió:
>
> On 3/23/23 05:18, Álvaro Fernández Rojas wrote:
> > From: Paul Geurts <paul.geurts@prodrive-technologies.com>
> >
> > 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@prodrive-technologies.com>
> > Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
> > ---
> >   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:

This is what I asked on the cover letter:
> I also added a separate RGMII handling block for is53134() since according to
> Paul, BCM53134 doesn't support RGMII_CTRL_TIMING_SEL as opposed to is531x5().

Should I add it to the same block and avoid adding
RGMII_CTRL_TIMING_SEL if is53134() or is it compatible with
RGMII_CTRL_TIMING_SEL?

>
> 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
>

--
Álvaro.

Re: [PATCH 2/2] net: dsa: b53: mdio: add support for BCM53134

[Florian Fainelli]

On 3/23/23 09:49, Álvaro Fernández Rojas wrote:
> El jue, 23 mar 2023 a las 17:43, Florian Fainelli
> (<f.fainelli@gmail.com>) escribió:
>>
>> On 3/23/23 05:18, Álvaro Fernández Rojas wrote:
>>> From: Paul Geurts <paul.geurts@prodrive-technologies.com>
>>>
>>> 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@prodrive-technologies.com>
>>> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
>>> ---
>>>    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:
> 
> This is what I asked on the cover letter:
>> I also added a separate RGMII handling block for is53134() since according to
>> Paul, BCM53134 doesn't support RGMII_CTRL_TIMING_SEL as opposed to is531x5().
> 
> Should I add it to the same block and avoid adding
> RGMII_CTRL_TIMING_SEL if is53134() or is it compatible with
> RGMII_CTRL_TIMING_SEL?

RGMII_CTRL_TIMING_SEL is not defined for the 53125 or 53128, and for 
53115, the register 0x60 is not even defined in the datasheet. 
RGMII_CTRL_TIMING_SEL is defined for the 53134 however and would control 
how the two other bits behave with respect to RGMII timing. My guess is 
that if we removed it entirely we would not be breaking anything, and we 
could just support all of the 531x5() and 53134 families within the same 
block.

Jonas, do you remember why setting RGMII_CTRL_TIMING_SEL might have been 
necessary?
--
Florian

RE: [PATCH 0/2] net: dsa: b53: mdio: add support for BCM53134

[Paul Geurts]

> -----Original Message-----
> From: Álvaro Fernández Rojas <noltari@gmail.com>
> Sent: donderdag 23 maart 2023 13:18
> To: Paul Geurts <paul.geurts@prodrive-technologies.com>;
> f.fainelli@gmail.com; jonas.gorski@gmail.com; andrew@lunn.ch;
> olteanv@gmail.com; davem@davemloft.net; edumazet@google.com;
> kuba@kernel.org; pabeni@redhat.com; robh+dt@kernel.org;
> krzysztof.kozlowski+dt@linaro.org; netdev@vger.kernel.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org
> Cc: Álvaro Fernández Rojas <noltari@gmail.com>
> Subject: [PATCH 0/2] net: dsa: b53: mdio: add support for BCM53134
> 
> This is based on the initial work from Paul Geurts that was sent to the
> incorrect linux development lists and recipients.
> I've modified it by removing BCM53134_DEVICE_ID from is531x5() and
> therefore adding is53134() where needed.
> I also added a separate RGMII handling block for is53134() since according to
> Paul, BCM53134 doesn't support RGMII_CTRL_TIMING_SEL as opposed to
> is531x5().
> 
> Paul Geurts (1):
>   net: dsa: b53: mdio: add support for BCM53134
> 
> Álvaro Fernández Rojas (1):
>   dt-bindings: net: dsa: b53: add BCM53134 support
> 
>  .../devicetree/bindings/net/dsa/brcm,b53.yaml |  1 +
>  drivers/net/dsa/b53/b53_common.c              | 53 ++++++++++++++++++-
>  drivers/net/dsa/b53/b53_mdio.c                |  5 +-
>  drivers/net/dsa/b53/b53_priv.h                |  9 +++-
>  4 files changed, 65 insertions(+), 3 deletions(-)
> 
> --
> 2.30.2

Thank you for resending my patches! I didn't get to it yet. Any particular reason you didn't include the optional GPIO patch I had in my set?
---
Paul

RE: [PATCH 2/2] net: dsa: b53: mdio: add support for BCM53134

[Paul Geurts]

> -----Original Message-----
> From: Florian Fainelli <f.fainelli@gmail.com>
> Sent: donderdag 23 maart 2023 17:43
> To: Álvaro Fernández Rojas <noltari@gmail.com>; Paul Geurts
> <paul.geurts@prodrive-technologies.com>; jonas.gorski@gmail.com;
> andrew@lunn.ch; olteanv@gmail.com; davem@davemloft.net;
> edumazet@google.com; kuba@kernel.org; pabeni@redhat.com;
> robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org;
> netdev@vger.kernel.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org
> 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@prodrive-technologies.com>
> >
> > 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@prodrive-technologies.com>
> > Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
> > ---
> >   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?

---
Paul

Re: [PATCH 2/2] net: dsa: b53: mdio: add support for BCM53134

[Florian Fainelli]

On 3/23/23 13:10, Paul Geurts wrote:
>> -----Original Message-----
>> From: Florian Fainelli <f.fainelli@gmail.com>
>> Sent: donderdag 23 maart 2023 17:43
>> To: Álvaro Fernández Rojas <noltari@gmail.com>; Paul Geurts
>> <paul.geurts@prodrive-technologies.com>; jonas.gorski@gmail.com;
>> andrew@lunn.ch; olteanv@gmail.com; davem@davemloft.net;
>> edumazet@google.com; kuba@kernel.org; pabeni@redhat.com;
>> robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org;
>> netdev@vger.kernel.org; devicetree@vger.kernel.org; linux-
>> kernel@vger.kernel.org
>> 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@prodrive-technologies.com>
>>>
>>> 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@prodrive-technologies.com>
>>> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
>>> ---
>>>    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

Re: [PATCH 0/2] net: dsa: b53: mdio: add support for BCM53134

[Álvaro Fernández Rojas]

El jue, 23 mar 2023 a las 20:58, Paul Geurts
(<paul.geurts@prodrive-technologies.com>) escribió:
>
> > -----Original Message-----
> > From: Álvaro Fernández Rojas <noltari@gmail.com>
> > Sent: donderdag 23 maart 2023 13:18
> > To: Paul Geurts <paul.geurts@prodrive-technologies.com>;
> > f.fainelli@gmail.com; jonas.gorski@gmail.com; andrew@lunn.ch;
> > olteanv@gmail.com; davem@davemloft.net; edumazet@google.com;
> > kuba@kernel.org; pabeni@redhat.com; robh+dt@kernel.org;
> > krzysztof.kozlowski+dt@linaro.org; netdev@vger.kernel.org;
> > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org
> > Cc: Álvaro Fernández Rojas <noltari@gmail.com>
> > Subject: [PATCH 0/2] net: dsa: b53: mdio: add support for BCM53134
> >
> > This is based on the initial work from Paul Geurts that was sent to the
> > incorrect linux development lists and recipients.
> > I've modified it by removing BCM53134_DEVICE_ID from is531x5() and
> > therefore adding is53134() where needed.
> > I also added a separate RGMII handling block for is53134() since according to
> > Paul, BCM53134 doesn't support RGMII_CTRL_TIMING_SEL as opposed to
> > is531x5().
> >
> > Paul Geurts (1):
> >   net: dsa: b53: mdio: add support for BCM53134
> >
> > Álvaro Fernández Rojas (1):
> >   dt-bindings: net: dsa: b53: add BCM53134 support
> >
> >  .../devicetree/bindings/net/dsa/brcm,b53.yaml |  1 +
> >  drivers/net/dsa/b53/b53_common.c              | 53 ++++++++++++++++++-
> >  drivers/net/dsa/b53/b53_mdio.c                |  5 +-
> >  drivers/net/dsa/b53/b53_priv.h                |  9 +++-
> >  4 files changed, 65 insertions(+), 3 deletions(-)
> >
> > --
> > 2.30.2
>
> Thank you for resending my patches! I didn't get to it yet. Any particular reason you didn't include the optional GPIO patch I had in my set?

I'm using it for a Sercomm H500-s which doesn't seem to need it.
However, I've just realized that it's documented here as GPIO 18:
https://openwrt.org/toh/sercomm/h500-s#other
Anyway, I think that the GPIO patch can be added later...

> ---
> Paul

--
Álvaro

Re: [PATCH 2/2] net: dsa: b53: mdio: add support for BCM53134

[Álvaro Fernández Rojas]

El jue, 23 mar 2023 a las 21:10, Paul Geurts
(<paul.geurts@prodrive-technologies.com>) escribió:
>
> > -----Original Message-----
> > From: Florian Fainelli <f.fainelli@gmail.com>
> > Sent: donderdag 23 maart 2023 17:43
> > To: Álvaro Fernández Rojas <noltari@gmail.com>; Paul Geurts
> > <paul.geurts@prodrive-technologies.com>; jonas.gorski@gmail.com;
> > andrew@lunn.ch; olteanv@gmail.com; davem@davemloft.net;
> > edumazet@google.com; kuba@kernel.org; pabeni@redhat.com;
> > robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org;
> > netdev@vger.kernel.org; devicetree@vger.kernel.org; linux-
> > kernel@vger.kernel.org
> > 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@prodrive-technologies.com>
> > >
> > > 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@prodrive-technologies.com>
> > > Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
> > > ---
> > >   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?

What happens if you add that bit on your device? It doesn't work?
I will try with my device and see what happens when I add it...

>
> ---
> Paul

--
Álvaro

Re: [PATCH 2/2] net: dsa: b53: mdio: add support for BCM53134

[Álvaro Fernández Rojas]

El jue, 23 mar 2023 a las 22:02, Florian Fainelli
(<f.fainelli@gmail.com>) escribió:
>
> On 3/23/23 13:10, Paul Geurts wrote:
> >> -----Original Message-----
> >> From: Florian Fainelli <f.fainelli@gmail.com>
> >> Sent: donderdag 23 maart 2023 17:43
> >> To: Álvaro Fernández Rojas <noltari@gmail.com>; Paul Geurts
> >> <paul.geurts@prodrive-technologies.com>; jonas.gorski@gmail.com;
> >> andrew@lunn.ch; olteanv@gmail.com; davem@davemloft.net;
> >> edumazet@google.com; kuba@kernel.org; pabeni@redhat.com;
> >> robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org;
> >> netdev@vger.kernel.org; devicetree@vger.kernel.org; linux-
> >> kernel@vger.kernel.org
> >> 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@prodrive-technologies.com>
> >>>
> >>> 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@prodrive-technologies.com>
> >>> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
> >>> ---
> >>>    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.

If this is true then we can safely add 53134 to is531x5() conditional
and reuse the existing code.

> --
> Florian
>

--
Álvaro

RE: [PATCH 2/2] net: dsa: b53: mdio: add support for BCM53134

[Paul Geurts]

> -----Original Message-----
> From: Álvaro Fernández Rojas <noltari@gmail.com>
> Sent: donderdag 23 maart 2023 23:13
> To: Florian Fainelli <f.fainelli@gmail.com>
> Cc: Paul Geurts <paul.geurts@prodrive-technologies.com>;
> jonas.gorski@gmail.com; andrew@lunn.ch; olteanv@gmail.com;
> davem@davemloft.net; edumazet@google.com; kuba@kernel.org;
> pabeni@redhat.com; robh+dt@kernel.org;
> krzysztof.kozlowski+dt@linaro.org; netdev@vger.kernel.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 2/2] net: dsa: b53: mdio: add support for BCM53134
> 
> El jue, 23 mar 2023 a las 22:02, Florian Fainelli
> (<f.fainelli@gmail.com>) escribió:
> >
> > On 3/23/23 13:10, Paul Geurts wrote:
> > >> -----Original Message-----
> > >> From: Florian Fainelli <f.fainelli@gmail.com>
> > >> Sent: donderdag 23 maart 2023 17:43
> > >> To: Álvaro Fernández Rojas <noltari@gmail.com>; Paul Geurts
> > >> <paul.geurts@prodrive-technologies.com>; jonas.gorski@gmail.com;
> > >> andrew@lunn.ch; olteanv@gmail.com; davem@davemloft.net;
> > >> edumazet@google.com; kuba@kernel.org; pabeni@redhat.com;
> > >> robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org;
> > >> netdev@vger.kernel.org; devicetree@vger.kernel.org; linux-
> > >> kernel@vger.kernel.org
> > >> 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@prodrive-technologies.com>
> > >>>
> > >>> 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@prodrive-technologies.com>
> > >>> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
> > >>> ---
> > >>>    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.
> 
> If this is true then we can safely add 53134 to is531x5() conditional and reuse
> the existing code.
> 
> > --
> > Florian
> >
> 
> --
> Álvaro

With the bit set on my device, everything keeps working just fine. I indeed think
This is just the same bit. I have requested some clarity from our FAE at
Broadcom. We could wait for their answer (which could take a while), or just
assume setting this bit is fine an push the patches. I'll leave that up to you.

---
Paul

[PATCH v2 0/2] net: dsa: b53: mdio: add support for BCM53134

[Álvaro Fernández Rojas]

This is based on the initial work from Paul Geurts that was sent to the
incorrect linux development lists and recipients.
I've simplified his patches by adding BCM53134 to the is531x5() block since it
seems that the switch doesn't need a special RGMII config.

Paul Geurts (1):
  net: dsa: b53: mdio: add support for BCM53134

Álvaro Fernández Rojas (1):
  dt-bindings: net: dsa: b53: add BCM53134 support

 .../devicetree/bindings/net/dsa/brcm,b53.yaml     |  1 +
 drivers/net/dsa/b53/b53_common.c                  | 15 +++++++++++++++
 drivers/net/dsa/b53/b53_mdio.c                    |  5 ++++-
 drivers/net/dsa/b53/b53_priv.h                    |  7 +++++--
 4 files changed, 25 insertions(+), 3 deletions(-)

-- 
2.30.2

[PATCH v2 1/2] dt-bindings: net: dsa: b53: add BCM53134 support

[Álvaro Fernández Rojas]

BCM53134 are B53 switches connected by MDIO.

Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
 v2: no changes

 Documentation/devicetree/bindings/net/dsa/brcm,b53.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/net/dsa/brcm,b53.yaml b/Documentation/devicetree/bindings/net/dsa/brcm,b53.yaml
index 57e0ef93b134..4c78c546343f 100644
--- a/Documentation/devicetree/bindings/net/dsa/brcm,b53.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/brcm,b53.yaml
@@ -19,6 +19,7 @@ properties:
       - const: brcm,bcm53115
       - const: brcm,bcm53125
       - const: brcm,bcm53128
+      - const: brcm,bcm53134
       - const: brcm,bcm5365
       - const: brcm,bcm5395
       - const: brcm,bcm5389
-- 
2.30.2

[PATCH v2 2/2] net: dsa: b53: mdio: add support for BCM53134

[Álvaro Fernández Rojas]

From: Paul Geurts <paul.geurts@prodrive-technologies.com>

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@prodrive-technologies.com>
Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
---
 v2: add BCM53134 to is531x5() and remove special RGMII config

 drivers/net/dsa/b53/b53_common.c | 15 +++++++++++++++
 drivers/net/dsa/b53/b53_mdio.c   |  5 ++++-
 drivers/net/dsa/b53/b53_priv.h   |  7 +++++--
 3 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index 1f9b251a5452..3464ce5e7470 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -2613,6 +2613,20 @@ static const struct b53_chip_data b53_switch_chips[] = {
 		.jumbo_pm_reg = B53_JUMBO_PORT_MASK,
 		.jumbo_size_reg = B53_JUMBO_MAX_SIZE,
 	},
+	{
+		.chip_id = BCM53134_DEVICE_ID,
+		.dev_name = "BCM53134",
+		.vlans = 4096,
+		.enabled_ports = 0x12f,
+		.imp_port = 8,
+		.cpu_port = B53_CPU_PORT,
+		.vta_regs = B53_VTA_REGS,
+		.arl_bins = 4,
+		.arl_buckets = 1024,
+		.duplex_reg = B53_DUPLEX_STAT_GE,
+		.jumbo_pm_reg = B53_JUMBO_PORT_MASK,
+		.jumbo_size_reg = B53_JUMBO_MAX_SIZE,
+	},
 };
 
 static int b53_switch_init(struct b53_device *dev)
@@ -2790,6 +2804,7 @@ int b53_switch_detect(struct b53_device *dev)
 		case BCM53012_DEVICE_ID:
 		case BCM53018_DEVICE_ID:
 		case BCM53019_DEVICE_ID:
+		case BCM53134_DEVICE_ID:
 			dev->chip_id = id32;
 			break;
 		default:
diff --git a/drivers/net/dsa/b53/b53_mdio.c b/drivers/net/dsa/b53/b53_mdio.c
index 6ddc03b58b28..8b422b298cd5 100644
--- a/drivers/net/dsa/b53/b53_mdio.c
+++ b/drivers/net/dsa/b53/b53_mdio.c
@@ -286,6 +286,7 @@ static const struct b53_io_ops b53_mdio_ops = {
 #define B53_BRCM_OUI_2	0x03625c00
 #define B53_BRCM_OUI_3	0x00406000
 #define B53_BRCM_OUI_4	0x01410c00
+#define B53_BRCM_OUI_5	0xae025000
 
 static int b53_mdio_probe(struct mdio_device *mdiodev)
 {
@@ -313,7 +314,8 @@ static int b53_mdio_probe(struct mdio_device *mdiodev)
 	if ((phy_id & 0xfffffc00) != B53_BRCM_OUI_1 &&
 	    (phy_id & 0xfffffc00) != B53_BRCM_OUI_2 &&
 	    (phy_id & 0xfffffc00) != B53_BRCM_OUI_3 &&
-	    (phy_id & 0xfffffc00) != B53_BRCM_OUI_4) {
+	    (phy_id & 0xfffffc00) != B53_BRCM_OUI_4 &&
+	    (phy_id & 0xfffffc00) != B53_BRCM_OUI_5) {
 		dev_err(&mdiodev->dev, "Unsupported device: 0x%08x\n", phy_id);
 		return -ENODEV;
 	}
@@ -375,6 +377,7 @@ static const struct of_device_id b53_of_match[] = {
 	{ .compatible = "brcm,bcm53115" },
 	{ .compatible = "brcm,bcm53125" },
 	{ .compatible = "brcm,bcm53128" },
+	{ .compatible = "brcm,bcm53134" },
 	{ .compatible = "brcm,bcm5365" },
 	{ .compatible = "brcm,bcm5389" },
 	{ .compatible = "brcm,bcm5395" },
diff --git a/drivers/net/dsa/b53/b53_priv.h b/drivers/net/dsa/b53/b53_priv.h
index a689a6950189..fdcfd5081c28 100644
--- a/drivers/net/dsa/b53/b53_priv.h
+++ b/drivers/net/dsa/b53/b53_priv.h
@@ -80,6 +80,7 @@ enum {
 	BCM583XX_DEVICE_ID = 0x58300,
 	BCM7445_DEVICE_ID = 0x7445,
 	BCM7278_DEVICE_ID = 0x7278,
+	BCM53134_DEVICE_ID = 0x5075,
 };
 
 struct b53_pcs {
@@ -187,7 +188,8 @@ static inline int is531x5(struct b53_device *dev)
 {
 	return dev->chip_id == BCM53115_DEVICE_ID ||
 		dev->chip_id == BCM53125_DEVICE_ID ||
-		dev->chip_id == BCM53128_DEVICE_ID;
+		dev->chip_id == BCM53128_DEVICE_ID ||
+		dev->chip_id == BCM53134_DEVICE_ID;
 }
 
 static inline int is63xx(struct b53_device *dev)
@@ -215,7 +217,8 @@ static inline int is58xx(struct b53_device *dev)
 	return dev->chip_id == BCM58XX_DEVICE_ID ||
 		dev->chip_id == BCM583XX_DEVICE_ID ||
 		dev->chip_id == BCM7445_DEVICE_ID ||
-		dev->chip_id == BCM7278_DEVICE_ID;
+		dev->chip_id == BCM7278_DEVICE_ID ||
+		dev->chip_id == BCM53134_DEVICE_ID;
 }
 
 #define B53_63XX_RGMII0	4
-- 
2.30.2

Re: [PATCH 2/2] net: dsa: b53: mdio: add support for BCM53134

[Álvaro Fernández Rojas]

El vie, 24 mar 2023 a las 9:29, Paul Geurts
(<paul.geurts@prodrive-technologies.com>) escribió:
>
> > -----Original Message-----
> > From: Álvaro Fernández Rojas <noltari@gmail.com>
> > Sent: donderdag 23 maart 2023 23:13
> > To: Florian Fainelli <f.fainelli@gmail.com>
> > Cc: Paul Geurts <paul.geurts@prodrive-technologies.com>;
> > jonas.gorski@gmail.com; andrew@lunn.ch; olteanv@gmail.com;
> > davem@davemloft.net; edumazet@google.com; kuba@kernel.org;
> > pabeni@redhat.com; robh+dt@kernel.org;
> > krzysztof.kozlowski+dt@linaro.org; netdev@vger.kernel.org;
> > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org
> > Subject: Re: [PATCH 2/2] net: dsa: b53: mdio: add support for BCM53134
> >
> > El jue, 23 mar 2023 a las 22:02, Florian Fainelli
> > (<f.fainelli@gmail.com>) escribió:
> > >
> > > On 3/23/23 13:10, Paul Geurts wrote:
> > > >> -----Original Message-----
> > > >> From: Florian Fainelli <f.fainelli@gmail.com>
> > > >> Sent: donderdag 23 maart 2023 17:43
> > > >> To: Álvaro Fernández Rojas <noltari@gmail.com>; Paul Geurts
> > > >> <paul.geurts@prodrive-technologies.com>; jonas.gorski@gmail.com;
> > > >> andrew@lunn.ch; olteanv@gmail.com; davem@davemloft.net;
> > > >> edumazet@google.com; kuba@kernel.org; pabeni@redhat.com;
> > > >> robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org;
> > > >> netdev@vger.kernel.org; devicetree@vger.kernel.org; linux-
> > > >> kernel@vger.kernel.org
> > > >> 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@prodrive-technologies.com>
> > > >>>
> > > >>> 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@prodrive-technologies.com>
> > > >>> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
> > > >>> ---
> > > >>>    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.
> >
> > If this is true then we can safely add 53134 to is531x5() conditional and reuse
> > the existing code.
> >
> > > --
> > > Florian
> > >
> >
> > --
> > Álvaro
>
> With the bit set on my device, everything keeps working just fine. I indeed think
> This is just the same bit. I have requested some clarity from our FAE at
> Broadcom. We could wait for their answer (which could take a while), or just
> assume setting this bit is fine an push the patches. I'll leave that up to you.

I've checked and mine works fine too, so I simplified the patch in v2.

>
> ---
> Paul

--
Álvaro

Re: [PATCH v2 0/2] net: dsa: b53: mdio: add support for BCM53134

[Jakub Kicinski]

On Fri, 24 Mar 2023 09:41:36 +0100 Álvaro Fernández Rojas wrote:
> This is based on the initial work from Paul Geurts that was sent to the
> incorrect linux development lists and recipients.
> I've simplified his patches by adding BCM53134 to the is531x5() block since it
> seems that the switch doesn't need a special RGMII config.

In the future please don't send the new version in-reply to.
Makes it harder to organize patches that need review for maintainers.
Or at least IDK how to maintain a queue ordered by submission date 
when people do this :\

Re: [PATCH v2 2/2] net: dsa: b53: mdio: add support for BCM53134

[Florian Fainelli]

On 3/24/23 01:41, Álvaro Fernández Rojas wrote:
> From: Paul Geurts <paul.geurts@prodrive-technologies.com>
> 
> 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@prodrive-technologies.com>
> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

Re: [PATCH 1/2] dt-bindings: net: dsa: b53: add BCM53134 support

[Krzysztof Kozlowski]

On 23/03/2023 13:18, Álvaro Fernández Rojas wrote:
> BCM53134 are B53 switches connected by MDIO.
> 
> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
> ---
>  Documentation/devicetree/bindings/net/dsa/brcm,b53.yaml | 1 +
>  1 file changed, 1 insertion(+)


Acked-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof

Re: [PATCH v2 1/2] dt-bindings: net: dsa: b53: add BCM53134 support

[Krzysztof Kozlowski]

On 24/03/2023 09:41, Álvaro Fernández Rojas wrote:
> BCM53134 are B53 switches connected by MDIO.

Do not attach (thread) your patchsets to some other threads (unrelated
or older versions). This buries them deep in the mailbox and might
interfere with applying entire sets.

> 
> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>


Acked-by: Krzysztof Kozlowski <krzk@kernel.org>



Best regards,
Krzysztof

Re: [PATCH v2 0/2] net: dsa: b53: mdio: add support for BCM53134

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by David S. Miller <davem@davemloft.net>:

On Fri, 24 Mar 2023 09:41:36 +0100 you wrote:
> This is based on the initial work from Paul Geurts that was sent to the
> incorrect linux development lists and recipients.
> I've simplified his patches by adding BCM53134 to the is531x5() block since it
> seems that the switch doesn't need a special RGMII config.
> 
> Paul Geurts (1):
>   net: dsa: b53: mdio: add support for BCM53134
> 
> [...]

Here is the summary with links:
  - [v2,1/2] dt-bindings: net: dsa: b53: add BCM53134 support
    https://git.kernel.org/netdev/net-next/c/a20869b3a785
  - [v2,2/2] net: dsa: b53: mdio: add support for BCM53134
    https://git.kernel.org/netdev/net-next/c/f927e8ef1e93

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html