* [PATCH v3 net-next 01/10] dt-bindings: net: dsa: dt bindings for microchip lan937x
2021-07-23 17:30 [PATCH v3 net-next 00/10] net: dsa: microchip: DSA driver support for LAN937x switch Prasanna Vengateshan
@ 2021-07-23 17:30 ` Prasanna Vengateshan
2021-07-26 22:49 ` Rob Herring
2021-07-23 17:31 ` [PATCH v3 net-next 02/10] net: dsa: move mib->cnt_ptr reset code to ksz_common.c Prasanna Vengateshan
` (8 subsequent siblings)
9 siblings, 1 reply; 44+ messages in thread
From: Prasanna Vengateshan @ 2021-07-23 17:30 UTC (permalink / raw)
To: andrew, netdev, olteanv, robh+dt
Cc: UNGLinuxDriver, Woojung.Huh, hkallweit1, linux, davem, kuba,
linux-kernel, vivien.didelot, f.fainelli, devicetree
Documentation in .yaml format and updates to the MAINTAINERS
Also 'make dt_binding_check' is passed
Signed-off-by: Prasanna Vengateshan <prasanna.vengateshan@microchip.com>
---
.../bindings/net/dsa/microchip,lan937x.yaml | 148 ++++++++++++++++++
MAINTAINERS | 1 +
2 files changed, 149 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/dsa/microchip,lan937x.yaml
diff --git a/Documentation/devicetree/bindings/net/dsa/microchip,lan937x.yaml b/Documentation/devicetree/bindings/net/dsa/microchip,lan937x.yaml
new file mode 100644
index 000000000000..69021117d595
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/dsa/microchip,lan937x.yaml
@@ -0,0 +1,148 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/dsa/microchip,lan937x.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: LAN937x Ethernet Switch Series Tree Bindings
+
+maintainers:
+ - UNGLinuxDriver@microchip.com
+
+allOf:
+ - $ref: dsa.yaml#
+
+properties:
+ compatible:
+ enum:
+ - microchip,lan9370
+ - microchip,lan9371
+ - microchip,lan9372
+ - microchip,lan9373
+ - microchip,lan9374
+
+ reg:
+ maxItems: 1
+
+ spi-max-frequency:
+ maximum: 50000000
+
+ reset-gpios:
+ description: Optional gpio specifier for a reset line
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+
+ //Ethernet switch connected via spi to the host
+ ethernet {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ fixed-link {
+ speed = <1000>;
+ full-duplex;
+ };
+ };
+
+ spi {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ lan9374: switch@0 {
+ compatible = "microchip,lan9374";
+ reg = <0>;
+
+ spi-max-frequency = <44000000>;
+
+ ethernet-ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ port@0 {
+ reg = <0>;
+ label = "lan1";
+ phy-mode = "internal";
+ phy-handle = <&t1phy0>;
+ };
+ port@1 {
+ reg = <1>;
+ label = "lan2";
+ phy-mode = "internal";
+ phy-handle = <&t1phy1>;
+ };
+ port@2 {
+ reg = <2>;
+ label = "lan4";
+ phy-mode = "internal";
+ phy-handle = <&t1phy2>;
+ };
+ port@3 {
+ reg = <3>;
+ label = "lan6";
+ phy-mode = "internal";
+ phy-handle = <&t1phy3>;
+ };
+ port@4 {
+ reg = <4>;
+ phy-mode = "rgmii";
+ ethernet = <ðernet>;
+ fixed-link {
+ speed = <1000>;
+ full-duplex;
+ };
+ };
+ port@5 {
+ reg = <5>;
+ label = "lan7";
+ phy-mode = "rgmii";
+ fixed-link {
+ speed = <1000>;
+ full-duplex;
+ };
+ };
+ port@6 {
+ reg = <6>;
+ label = "lan5";
+ phy-mode = "internal";
+ phy-handle = <&t1phy4>;
+ };
+ port@7 {
+ reg = <7>;
+ label = "lan3";
+ phy-mode = "internal";
+ phy-handle = <&t1phy5>;
+ };
+ };
+
+ mdio {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ t1phy0: ethernet-phy@0{
+ reg = <0x0>;
+ };
+ t1phy1: ethernet-phy@1{
+ reg = <0x1>;
+ };
+ t1phy2: ethernet-phy@2{
+ reg = <0x2>;
+ };
+ t1phy3: ethernet-phy@3{
+ reg = <0x3>;
+ };
+ t1phy4: ethernet-phy@6{
+ reg = <0x6>;
+ };
+ t1phy5: ethernet-phy@7{
+ reg = <0x7>;
+ };
+ };
+ };
+ };
diff --git a/MAINTAINERS b/MAINTAINERS
index da478d5c8b0c..f0ce2378b681 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12181,6 +12181,7 @@ M: UNGLinuxDriver@microchip.com
L: netdev@vger.kernel.org
S: Maintained
F: Documentation/devicetree/bindings/net/dsa/microchip,ksz.yaml
+F: Documentation/devicetree/bindings/net/dsa/microchip,lan937x.yaml
F: drivers/net/dsa/microchip/*
F: include/linux/platform_data/microchip-ksz.h
F: net/dsa/tag_ksz.c
--
2.27.0
^ permalink raw reply related [flat|nested] 44+ messages in thread* Re: [PATCH v3 net-next 01/10] dt-bindings: net: dsa: dt bindings for microchip lan937x
2021-07-23 17:30 ` [PATCH v3 net-next 01/10] dt-bindings: net: dsa: dt bindings for microchip lan937x Prasanna Vengateshan
@ 2021-07-26 22:49 ` Rob Herring
0 siblings, 0 replies; 44+ messages in thread
From: Rob Herring @ 2021-07-26 22:49 UTC (permalink / raw)
To: Prasanna Vengateshan
Cc: devicetree, netdev, linux-kernel, robh+dt, olteanv, hkallweit1,
linux, vivien.didelot, f.fainelli, UNGLinuxDriver, Woojung.Huh,
kuba, davem, andrew
On Fri, 23 Jul 2021 23:00:59 +0530, Prasanna Vengateshan wrote:
> Documentation in .yaml format and updates to the MAINTAINERS
> Also 'make dt_binding_check' is passed
>
> Signed-off-by: Prasanna Vengateshan <prasanna.vengateshan@microchip.com>
> ---
> .../bindings/net/dsa/microchip,lan937x.yaml | 148 ++++++++++++++++++
> MAINTAINERS | 1 +
> 2 files changed, 149 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/net/dsa/microchip,lan937x.yaml
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH v3 net-next 02/10] net: dsa: move mib->cnt_ptr reset code to ksz_common.c
2021-07-23 17:30 [PATCH v3 net-next 00/10] net: dsa: microchip: DSA driver support for LAN937x switch Prasanna Vengateshan
2021-07-23 17:30 ` [PATCH v3 net-next 01/10] dt-bindings: net: dsa: dt bindings for microchip lan937x Prasanna Vengateshan
@ 2021-07-23 17:31 ` Prasanna Vengateshan
2021-07-23 18:53 ` Vladimir Oltean
2021-07-23 17:31 ` [PATCH v3 net-next 03/10] net: phy: Add support for LAN937x T1 phy driver Prasanna Vengateshan
` (7 subsequent siblings)
9 siblings, 1 reply; 44+ messages in thread
From: Prasanna Vengateshan @ 2021-07-23 17:31 UTC (permalink / raw)
To: andrew, netdev, olteanv, robh+dt
Cc: UNGLinuxDriver, Woojung.Huh, hkallweit1, linux, davem, kuba,
linux-kernel, vivien.didelot, f.fainelli, devicetree
mib->cnt_ptr resetting is handled in multiple places as part of
port_init_cnt(). Hence moved mib->cnt_ptr code to ksz common layer
and removed from individual product files.
Signed-off-by: Prasanna Vengateshan <prasanna.vengateshan@microchip.com>
---
drivers/net/dsa/microchip/ksz8795.c | 2 --
drivers/net/dsa/microchip/ksz9477.c | 3 ---
drivers/net/dsa/microchip/ksz_common.c | 8 +++++++-
3 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
index 560f6843bb65..52cf2843dbeb 100644
--- a/drivers/net/dsa/microchip/ksz8795.c
+++ b/drivers/net/dsa/microchip/ksz8795.c
@@ -447,8 +447,6 @@ static void ksz8_port_init_cnt(struct ksz_device *dev, int port)
dropped, &mib->counters[mib->cnt_ptr]);
++mib->cnt_ptr;
}
- mib->cnt_ptr = 0;
- memset(mib->counters, 0, dev->mib_cnt * sizeof(u64));
}
static void ksz8_r_table(struct ksz_device *dev, int table, u16 addr, u64 *data)
diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
index 854e25f43fa7..35b430d531de 100644
--- a/drivers/net/dsa/microchip/ksz9477.c
+++ b/drivers/net/dsa/microchip/ksz9477.c
@@ -289,9 +289,6 @@ static void ksz9477_port_init_cnt(struct ksz_device *dev, int port)
ksz_write8(dev, REG_SW_MAC_CTRL_6, SW_MIB_COUNTER_FLUSH);
ksz_pwrite32(dev, port, REG_PORT_MIB_CTRL_STAT__4, 0);
mutex_unlock(&mib->cnt_mutex);
-
- mib->cnt_ptr = 0;
- memset(mib->counters, 0, dev->mib_cnt * sizeof(u64));
}
static enum dsa_tag_protocol ksz9477_get_tag_protocol(struct dsa_switch *ds,
diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 1542bfb8b5e5..2b27d7917903 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -103,8 +103,14 @@ void ksz_init_mib_timer(struct ksz_device *dev)
INIT_DELAYED_WORK(&dev->mib_read, ksz_mib_read_work);
- for (i = 0; i < dev->port_cnt; i++)
+ for (i = 0; i < dev->port_cnt; i++) {
+ struct ksz_port_mib *mib = &dev->ports[i].mib;
+
dev->dev_ops->port_init_cnt(dev, i);
+
+ mib->cnt_ptr = 0;
+ memset(mib->counters, 0, dev->mib_cnt * sizeof(u64));
+ }
}
EXPORT_SYMBOL_GPL(ksz_init_mib_timer);
--
2.27.0
^ permalink raw reply related [flat|nested] 44+ messages in thread* Re: [PATCH v3 net-next 02/10] net: dsa: move mib->cnt_ptr reset code to ksz_common.c
2021-07-23 17:31 ` [PATCH v3 net-next 02/10] net: dsa: move mib->cnt_ptr reset code to ksz_common.c Prasanna Vengateshan
@ 2021-07-23 18:53 ` Vladimir Oltean
0 siblings, 0 replies; 44+ messages in thread
From: Vladimir Oltean @ 2021-07-23 18:53 UTC (permalink / raw)
To: Prasanna Vengateshan
Cc: andrew, netdev, robh+dt, UNGLinuxDriver, Woojung.Huh, hkallweit1,
linux, davem, kuba, linux-kernel, vivien.didelot, f.fainelli,
devicetree
On Fri, Jul 23, 2021 at 11:01:00PM +0530, Prasanna Vengateshan wrote:
> mib->cnt_ptr resetting is handled in multiple places as part of
> port_init_cnt(). Hence moved mib->cnt_ptr code to ksz common layer
> and removed from individual product files.
>
> Signed-off-by: Prasanna Vengateshan <prasanna.vengateshan@microchip.com>
> ---
Reviewed-by: Vladimir Oltean <olteanv@gmail.com>
^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH v3 net-next 03/10] net: phy: Add support for LAN937x T1 phy driver
2021-07-23 17:30 [PATCH v3 net-next 00/10] net: dsa: microchip: DSA driver support for LAN937x switch Prasanna Vengateshan
2021-07-23 17:30 ` [PATCH v3 net-next 01/10] dt-bindings: net: dsa: dt bindings for microchip lan937x Prasanna Vengateshan
2021-07-23 17:31 ` [PATCH v3 net-next 02/10] net: dsa: move mib->cnt_ptr reset code to ksz_common.c Prasanna Vengateshan
@ 2021-07-23 17:31 ` Prasanna Vengateshan
2021-08-11 17:52 ` Prasanna Vengateshan
2021-07-23 17:31 ` [PATCH v3 net-next 04/10] net: dsa: tag_ksz: add tag handling for Microchip LAN937x Prasanna Vengateshan
` (6 subsequent siblings)
9 siblings, 1 reply; 44+ messages in thread
From: Prasanna Vengateshan @ 2021-07-23 17:31 UTC (permalink / raw)
To: andrew, netdev, olteanv, robh+dt
Cc: UNGLinuxDriver, Woojung.Huh, hkallweit1, linux, davem, kuba,
linux-kernel, vivien.didelot, f.fainelli, devicetree
Added support for Microchip LAN937x T1 phy driver. The sequence of
initialization is used commonly for both LAN87xx and LAN937x
drivers. The new initialization sequence is an improvement to
existing LAN87xx and it is shared with LAN937x.
Also relevant comments are added in the existing code and existing
soft-reset customized code has been replaced with
genphy_soft_reset().
access_ereg_clr_poll_timeout() API is introduced for polling phy
bank write and this is linked with PHYACC_ATTR_MODE_POLL.
Finally introduced function table for LAN937X_T1_PHY_ID along with
microchip_t1_phy_driver struct.
Signed-off-by: Prasanna Vengateshan <prasanna.vengateshan@microchip.com>
---
drivers/net/phy/microchip_t1.c | 319 +++++++++++++++++++++++++++------
1 file changed, 260 insertions(+), 59 deletions(-)
diff --git a/drivers/net/phy/microchip_t1.c b/drivers/net/phy/microchip_t1.c
index 4dc00bd5a8d2..a3f1b5d123ce 100644
--- a/drivers/net/phy/microchip_t1.c
+++ b/drivers/net/phy/microchip_t1.c
@@ -30,15 +30,53 @@
#define PHYACC_ATTR_MODE_READ 0
#define PHYACC_ATTR_MODE_WRITE 1
#define PHYACC_ATTR_MODE_MODIFY 2
+#define PHYACC_ATTR_MODE_POLL 3
#define PHYACC_ATTR_BANK_SMI 0
#define PHYACC_ATTR_BANK_MISC 1
#define PHYACC_ATTR_BANK_PCS 2
#define PHYACC_ATTR_BANK_AFE 3
+#define PHYACC_ATTR_BANK_DSP 4
#define PHYACC_ATTR_BANK_MAX 7
+#define T1_M_CTRL_REG 0x09
+#define T1_M_CFG BIT(11)
+
+#define T1_MODE_STAT_REG 0x11
+#define T1_DSCR_LOCK_STATUS_MSK BIT(3)
+#define T1_LINK_UP_MSK BIT(0)
+
+#define T1_REG_BANK_SEL_MASK 0x7
+#define T1_REG_BANK_SEL 8
+#define T1_REG_ADDR_MASK 0xFF
+
+#define T1_M_STATUS_REG 0x0A
+#define T1_LOCAL_RX_OK BIT(13)
+#define T1_REMOTE_RX_OK BIT(12)
+
+#define LAN87XX_PHY_ID 0x0007c150
+#define LAN937X_T1_PHY_ID 0x0007c181
+#define LAN87XX_PHY_ID_MASK 0xfffffff0
+#define LAN937X_PHY_ID_MASK 0xfffffff0
+
+/* T1 Registers */
+#define T1_AFE_PORT_CFG1_REG 0x0B
+#define T1_POWER_DOWN_CONTROL_REG 0x1A
+#define T1_SLV_FD_MULT_CFG_REG 0x18
+#define T1_CDR_CFG_PRE_LOCK_REG 0x05
+#define T1_CDR_CFG_POST_LOCK_REG 0x06
+#define T1_LCK_STG2_MUFACT_CFG_REG 0x1A
+#define T1_LCK_STG3_MUFACT_CFG_REG 0x1B
+#define T1_POST_LCK_MUFACT_CFG_REG 0x1C
+#define T1_TX_RX_FIFO_CFG_REG 0x02
+#define T1_TX_LPF_FIR_CFG_REG 0x55
+#define T1_SQI_CONFIG_REG 0x2E
+#define T1_MDIO_CONTROL2_REG 0x10
+#define T1_INTERRUPT_SOURCE_REG 0x18
+#define T1_INTERRUPT2_SOURCE_REG 0x08
+
#define DRIVER_AUTHOR "Nisar Sayed <nisar.sayed@microchip.com>"
-#define DRIVER_DESC "Microchip LAN87XX T1 PHY driver"
+#define DRIVER_DESC "Microchip LAN87XX/LAN937X T1 PHY driver"
struct access_ereg_val {
u8 mode;
@@ -51,12 +89,16 @@ struct access_ereg_val {
static int access_ereg(struct phy_device *phydev, u8 mode, u8 bank,
u8 offset, u16 val)
{
+ u8 prev_bank;
u16 ereg = 0;
int rc = 0;
+ u16 t;
+ /* return if mode and bank are invalid */
if (mode > PHYACC_ATTR_MODE_WRITE || bank > PHYACC_ATTR_BANK_MAX)
return -EINVAL;
+ /* if the bank is SMI, then call phy_read() & phy_write() directly */
if (bank == PHYACC_ATTR_BANK_SMI) {
if (mode == PHYACC_ATTR_MODE_WRITE)
rc = phy_write(phydev, offset, val);
@@ -66,16 +108,43 @@ static int access_ereg(struct phy_device *phydev, u8 mode, u8 bank,
}
if (mode == PHYACC_ATTR_MODE_WRITE) {
+ /* Initialize to Write Mode */
ereg = LAN87XX_EXT_REG_CTL_WR_CTL;
+
+ /* Write the data to be written in to the Bank */
rc = phy_write(phydev, LAN87XX_EXT_REG_WR_DATA, val);
if (rc < 0)
return rc;
} else {
+ /* Initialize to Read Mode */
ereg = LAN87XX_EXT_REG_CTL_RD_CTL;
}
ereg |= (bank << 8) | offset;
+ /* DSP bank access workaround for lan937x*/
+ if (phydev->phy_id == LAN937X_T1_PHY_ID) {
+ /* Read previous selected bank */
+ rc = phy_read(phydev, LAN87XX_EXT_REG_CTL);
+ if (rc < 0)
+ return rc;
+
+ /* Store the prev_bank */
+ prev_bank = (rc >> T1_REG_BANK_SEL) & T1_REG_BANK_SEL_MASK;
+
+ if (bank != prev_bank && bank == PHYACC_ATTR_BANK_DSP) {
+ t = ereg & ~T1_REG_ADDR_MASK;
+
+ t &= ~LAN87XX_EXT_REG_CTL_WR_CTL;
+ t |= LAN87XX_EXT_REG_CTL_RD_CTL;
+
+ /*access twice for DSP bank change,dummy access*/
+ rc = phy_write(phydev, LAN87XX_EXT_REG_CTL, t);
+ if (rc < 0)
+ return rc;
+ }
+ }
+
rc = phy_write(phydev, LAN87XX_EXT_REG_CTL, ereg);
if (rc < 0)
return rc;
@@ -104,63 +173,152 @@ static int access_ereg_modify_changed(struct phy_device *phydev,
return rc;
}
+static int access_ereg_clr_poll_timeout(struct phy_device *phydev, u8 bank,
+ u8 offset, u16 mask, u16 clr)
+{
+ int val;
+
+ if (bank != PHYACC_ATTR_BANK_SMI)
+ return -EINVAL;
+
+ return phy_read_poll_timeout(phydev, offset, val, (val & mask) == clr,
+ 150, 30000, true);
+}
+
static int lan87xx_phy_init(struct phy_device *phydev)
{
static const struct access_ereg_val init[] = {
- /* TX Amplitude = 5 */
- {PHYACC_ATTR_MODE_MODIFY, PHYACC_ATTR_BANK_AFE, 0x0B,
- 0x000A, 0x001E},
- /* Clear SMI interrupts */
- {PHYACC_ATTR_MODE_READ, PHYACC_ATTR_BANK_SMI, 0x18,
- 0, 0},
- /* Clear MISC interrupts */
- {PHYACC_ATTR_MODE_READ, PHYACC_ATTR_BANK_MISC, 0x08,
- 0, 0},
- /* Turn on TC10 Ring Oscillator (ROSC) */
- {PHYACC_ATTR_MODE_MODIFY, PHYACC_ATTR_BANK_MISC, 0x20,
- 0x0020, 0x0020},
- /* WUR Detect Length to 1.2uS, LPC Detect Length to 1.09uS */
- {PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_PCS, 0x20,
- 0x283C, 0},
- /* Wake_In Debounce Length to 39uS, Wake_Out Length to 79uS */
- {PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_MISC, 0x21,
- 0x274F, 0},
- /* Enable Auto Wake Forward to Wake_Out, ROSC on, Sleep,
- * and Wake_In to wake PHY
- */
- {PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_MISC, 0x20,
- 0x80A7, 0},
- /* Enable WUP Auto Fwd, Enable Wake on MDI, Wakeup Debouncer
- * to 128 uS
- */
- {PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_MISC, 0x24,
- 0xF110, 0},
- /* Enable HW Init */
- {PHYACC_ATTR_MODE_MODIFY, PHYACC_ATTR_BANK_SMI, 0x1A,
- 0x0100, 0x0100},
+ /* TXPD/TXAMP6 Configs*/
+ { PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_AFE,
+ T1_AFE_PORT_CFG1_REG, 0x002D, 0 },
+ /* HW_Init Hi and Force_ED */
+ { PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_SMI,
+ T1_POWER_DOWN_CONTROL_REG, 0x0308, 0 },
+ { PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_DSP,
+ T1_SLV_FD_MULT_CFG_REG, 0x0D53, 0 },
+ { PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_DSP,
+ T1_CDR_CFG_PRE_LOCK_REG, 0x0AB2, 0 },
+ { PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_DSP,
+ T1_CDR_CFG_POST_LOCK_REG, 0x0AB3, 0 },
+ { PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_DSP,
+ T1_LCK_STG2_MUFACT_CFG_REG, 0x0AEA, 0 },
+ { PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_DSP,
+ T1_LCK_STG3_MUFACT_CFG_REG, 0x0AEB, 0 },
+ { PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_DSP,
+ T1_POST_LCK_MUFACT_CFG_REG, 0x0AEB, 0 },
+ /* Pointer delay */
+ { PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_DSP,
+ T1_TX_RX_FIFO_CFG_REG, 0x1C00, 0 },
+ /* Tx iir edits */
+ { PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_DSP,
+ T1_TX_LPF_FIR_CFG_REG, 0x1000, 0 },
+ { PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_DSP,
+ T1_TX_LPF_FIR_CFG_REG, 0x1861, 0 },
+ { PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_DSP,
+ T1_TX_LPF_FIR_CFG_REG, 0x1061, 0 },
+ { PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_DSP,
+ T1_TX_LPF_FIR_CFG_REG, 0x1922, 0 },
+ { PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_DSP,
+ T1_TX_LPF_FIR_CFG_REG, 0x1122, 0 },
+ { PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_DSP,
+ T1_TX_LPF_FIR_CFG_REG, 0x1983, 0 },
+ { PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_DSP,
+ T1_TX_LPF_FIR_CFG_REG, 0x1183, 0 },
+ { PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_DSP,
+ T1_TX_LPF_FIR_CFG_REG, 0x1944, 0 },
+ { PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_DSP,
+ T1_TX_LPF_FIR_CFG_REG, 0x1144, 0 },
+ { PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_DSP,
+ T1_TX_LPF_FIR_CFG_REG, 0x18c5, 0 },
+ { PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_DSP,
+ T1_TX_LPF_FIR_CFG_REG, 0x10c5, 0 },
+ { PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_DSP,
+ T1_TX_LPF_FIR_CFG_REG, 0x1846, 0 },
+ { PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_DSP,
+ T1_TX_LPF_FIR_CFG_REG, 0x1046, 0 },
+ { PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_DSP,
+ T1_TX_LPF_FIR_CFG_REG, 0x1807, 0 },
+ { PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_DSP,
+ T1_TX_LPF_FIR_CFG_REG, 0x1007, 0 },
+ { PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_DSP,
+ T1_TX_LPF_FIR_CFG_REG, 0x1808, 0 },
+ { PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_DSP,
+ T1_TX_LPF_FIR_CFG_REG, 0x1008, 0 },
+ { PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_DSP,
+ T1_TX_LPF_FIR_CFG_REG, 0x1809, 0 },
+ { PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_DSP,
+ T1_TX_LPF_FIR_CFG_REG, 0x1009, 0 },
+ { PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_DSP,
+ T1_TX_LPF_FIR_CFG_REG, 0x180A, 0 },
+ { PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_DSP,
+ T1_TX_LPF_FIR_CFG_REG, 0x100A, 0 },
+ { PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_DSP,
+ T1_TX_LPF_FIR_CFG_REG, 0x180B, 0 },
+ { PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_DSP,
+ T1_TX_LPF_FIR_CFG_REG, 0x100B, 0 },
+ { PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_DSP,
+ T1_TX_LPF_FIR_CFG_REG, 0x180C, 0 },
+ { PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_DSP,
+ T1_TX_LPF_FIR_CFG_REG, 0x100C, 0 },
+ { PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_DSP,
+ T1_TX_LPF_FIR_CFG_REG, 0x180D, 0 },
+ { PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_DSP,
+ T1_TX_LPF_FIR_CFG_REG, 0x100D, 0 },
+ { PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_DSP,
+ T1_TX_LPF_FIR_CFG_REG, 0x180E, 0 },
+ { PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_DSP,
+ T1_TX_LPF_FIR_CFG_REG, 0x100E, 0 },
+ { PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_DSP,
+ T1_TX_LPF_FIR_CFG_REG, 0x180F, 0 },
+ { PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_DSP,
+ T1_TX_LPF_FIR_CFG_REG, 0x100F, 0 },
+ { PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_DSP,
+ T1_TX_LPF_FIR_CFG_REG, 0x1810, 0 },
+ { PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_DSP,
+ T1_TX_LPF_FIR_CFG_REG, 0x1010, 0 },
+ { PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_DSP,
+ T1_TX_LPF_FIR_CFG_REG, 0x1811, 0 },
+ { PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_DSP,
+ T1_TX_LPF_FIR_CFG_REG, 0x1011, 0 },
+ { PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_DSP,
+ T1_TX_LPF_FIR_CFG_REG, 0x1000, 0 },
+ /* SQI enable */
+ { PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_DSP,
+ T1_SQI_CONFIG_REG, 0x9572, 0 },
+ /* Flag LPS and WUR as idle errors */
+ { PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_SMI,
+ T1_MDIO_CONTROL2_REG, 0x0014, 0 },
+ /* Restore state machines without clearing registers */
+ { PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_SMI,
+ T1_POWER_DOWN_CONTROL_REG, 0x0200, 0 },
+ { PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_SMI,
+ T1_MDIO_CONTROL2_REG, 0x0094, 0 },
+ { PHYACC_ATTR_MODE_POLL, PHYACC_ATTR_BANK_SMI,
+ T1_MDIO_CONTROL2_REG, 0x0080, 0 },
+ { PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_AFE,
+ T1_AFE_PORT_CFG1_REG, 0x000C, 0 },
+ /* Read INTERRUPT_SOURCE Register */
+ { PHYACC_ATTR_MODE_READ, PHYACC_ATTR_BANK_SMI,
+ T1_INTERRUPT_SOURCE_REG, 0, 0 },
+ /* Read INTERRUPT_SOURCE Register */
+ { PHYACC_ATTR_MODE_READ, PHYACC_ATTR_BANK_MISC,
+ T1_INTERRUPT2_SOURCE_REG, 0, 0 },
+ /* HW_Init Hi */
+ { PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_SMI,
+ T1_POWER_DOWN_CONTROL_REG, 0x0300, 0 },
};
int rc, i;
- /* Start manual initialization procedures in Managed Mode */
- rc = access_ereg_modify_changed(phydev, PHYACC_ATTR_BANK_SMI,
- 0x1a, 0x0000, 0x0100);
- if (rc < 0)
- return rc;
-
- /* Soft Reset the SMI block */
+ /* Set Master Mode */
rc = access_ereg_modify_changed(phydev, PHYACC_ATTR_BANK_SMI,
- 0x00, 0x8000, 0x8000);
+ T1_M_CTRL_REG, T1_M_CFG, T1_M_CFG);
if (rc < 0)
return rc;
- /* Check to see if the self-clearing bit is cleared */
- usleep_range(1000, 2000);
- rc = access_ereg(phydev, PHYACC_ATTR_MODE_READ,
- PHYACC_ATTR_BANK_SMI, 0x00, 0);
+ /* phy Soft reset */
+ rc = genphy_soft_reset(phydev);
if (rc < 0)
return rc;
- if ((rc & 0x8000) != 0)
- return -ETIMEDOUT;
/* PHY Initialization */
for (i = 0; i < ARRAY_SIZE(init); i++) {
@@ -169,6 +327,11 @@ static int lan87xx_phy_init(struct phy_device *phydev)
init[i].offset,
init[i].val,
init[i].mask);
+ } else if (init[i].mode == PHYACC_ATTR_MODE_POLL) {
+ rc = access_ereg_clr_poll_timeout(phydev, init[i].bank,
+ init[i].offset,
+ init[i].val,
+ init[i].mask);
} else {
rc = access_ereg(phydev, init[i].mode, init[i].bank,
init[i].offset, init[i].val);
@@ -223,32 +386,70 @@ static int lan87xx_config_init(struct phy_device *phydev)
{
int rc = lan87xx_phy_init(phydev);
+ if (rc < 0)
+ phydev_err(phydev, "failed to initialize phy\n");
+
return rc < 0 ? rc : 0;
}
-static struct phy_driver microchip_t1_phy_driver[] = {
- {
- .phy_id = 0x0007c150,
- .phy_id_mask = 0xfffffff0,
- .name = "Microchip LAN87xx T1",
+static int lan937x_read_status(struct phy_device *phydev)
+{
+ int val;
- .features = PHY_BASIC_T1_FEATURES,
+ val = phy_read(phydev, T1_MODE_STAT_REG);
- .config_init = lan87xx_config_init,
+ if (val < 0)
+ return val;
- .config_intr = lan87xx_phy_config_intr,
- .handle_interrupt = lan87xx_handle_interrupt,
+ if (val & T1_LINK_UP_MSK)
+ phydev->link = 1;
+ else
+ phydev->link = 0;
- .suspend = genphy_suspend,
- .resume = genphy_resume,
+ phydev->duplex = DUPLEX_FULL;
+ phydev->speed = SPEED_100;
+ phydev->pause = 0;
+ phydev->asym_pause = 0;
+
+ return 0;
+}
+
+static int lan937x_config_init(struct phy_device *phydev)
+{
+ /* lan87xx & lan937x follows same init sequence */
+ return lan87xx_config_init(phydev);
+}
+
+static struct phy_driver microchip_t1_phy_driver[] = {
+ {
+ .phy_id = LAN87XX_PHY_ID,
+ .phy_id_mask = LAN87XX_PHY_ID_MASK,
+ .name = "LAN87xx T1",
+ .features = PHY_BASIC_T1_FEATURES,
+ .config_init = lan87xx_config_init,
+ .config_intr = lan87xx_phy_config_intr,
+ .handle_interrupt = lan87xx_handle_interrupt,
+ .suspend = genphy_suspend,
+ .resume = genphy_resume,
+ },
+ {
+ .phy_id = LAN937X_T1_PHY_ID,
+ .phy_id_mask = LAN937X_PHY_ID_MASK,
+ .name = "LAN937x T1",
+ .read_status = lan937x_read_status,
+ .features = PHY_BASIC_T1_FEATURES,
+ .config_init = lan937x_config_init,
+ .suspend = genphy_suspend,
+ .resume = genphy_resume,
}
};
module_phy_driver(microchip_t1_phy_driver);
static struct mdio_device_id __maybe_unused microchip_t1_tbl[] = {
- { 0x0007c150, 0xfffffff0 },
- { }
+ { LAN87XX_PHY_ID, LAN87XX_PHY_ID_MASK },
+ { LAN937X_T1_PHY_ID, LAN937X_PHY_ID_MASK },
+ {}
};
MODULE_DEVICE_TABLE(mdio, microchip_t1_tbl);
--
2.27.0
^ permalink raw reply related [flat|nested] 44+ messages in thread* Re: [PATCH v3 net-next 03/10] net: phy: Add support for LAN937x T1 phy driver
2021-07-23 17:31 ` [PATCH v3 net-next 03/10] net: phy: Add support for LAN937x T1 phy driver Prasanna Vengateshan
@ 2021-08-11 17:52 ` Prasanna Vengateshan
0 siblings, 0 replies; 44+ messages in thread
From: Prasanna Vengateshan @ 2021-08-11 17:52 UTC (permalink / raw)
To: andrew
Cc: netdev, olteanv, robh+dt, UNGLinuxDriver, Woojung.Huh,
hkallweit1, linux, davem, kuba, linux-kernel, vivien.didelot,
f.fainelli, devicetree
Andrew,
On Fri, 2021-07-23 at 23:01 +0530, Prasanna Vengateshan wrote:
> Added support for Microchip LAN937x T1 phy driver. The sequence of
> initialization is used commonly for both LAN87xx and LAN937x
> drivers. The new initialization sequence is an improvement to
> existing LAN87xx and it is shared with LAN937x.
>
> Also relevant comments are added in the existing code and existing
> soft-reset customized code has been replaced with
> genphy_soft_reset().
>
> access_ereg_clr_poll_timeout() API is introduced for polling phy
> bank write and this is linked with PHYACC_ATTR_MODE_POLL.
>
> Finally introduced function table for LAN937X_T1_PHY_ID along with
> microchip_t1_phy_driver struct.
>
> Signed-off-by: Prasanna Vengateshan <prasanna.vengateshan@microchip.com>
> ---
> drivers/net/phy/microchip_t1.c | 319 +++++++++++++++++++++++++++------
> 1 file changed, 260 insertions(+), 59 deletions(-)
>
> diff --git a/drivers/net/phy/microchip_t1.c b/drivers/net/phy/microchip_t1.c
> index 4dc00bd5a8d2..a3f1b5d123ce 100644
> --- a/drivers/net/phy/microchip_t1.c
> +++ b/drivers/net/phy/microchip_t1.c
> @@ -30,15 +30,53 @@
> #define PHYACC_ATTR_MODE_READ 0
> #define PHYACC_ATTR_MODE_WRITE 1
> #define PHYACC_ATTR_MODE_MODIFY 2
> +#define PHYACC_ATTR_MODE_POLL 3
>
> #define PHYACC_ATTR_BANK_SMI 0
> #define PHYACC_ATTR_BANK_MISC 1
> #define PHYACC_ATTR_BANK_PCS 2
> #define PHYACC_ATTR_BANK_AFE 3
> +#define PHYACC_ATTR_BANK_DSP 4
> #define PHYACC_ATTR_BANK_MAX 7
Are there any items that need a change in this patch? It will be helpful for me
to include them in the next version. Thanks.
Prasanna V
^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH v3 net-next 04/10] net: dsa: tag_ksz: add tag handling for Microchip LAN937x
2021-07-23 17:30 [PATCH v3 net-next 00/10] net: dsa: microchip: DSA driver support for LAN937x switch Prasanna Vengateshan
` (2 preceding siblings ...)
2021-07-23 17:31 ` [PATCH v3 net-next 03/10] net: phy: Add support for LAN937x T1 phy driver Prasanna Vengateshan
@ 2021-07-23 17:31 ` Prasanna Vengateshan
2021-07-23 19:23 ` Vladimir Oltean
2021-07-23 17:31 ` [PATCH v3 net-next 05/10] net: dsa: microchip: add DSA support for microchip lan937x Prasanna Vengateshan
` (5 subsequent siblings)
9 siblings, 1 reply; 44+ messages in thread
From: Prasanna Vengateshan @ 2021-07-23 17:31 UTC (permalink / raw)
To: andrew, netdev, olteanv, robh+dt
Cc: UNGLinuxDriver, Woojung.Huh, hkallweit1, linux, davem, kuba,
linux-kernel, vivien.didelot, f.fainelli, devicetree
The Microchip LAN937X switches have a tagging protocol which is
very similar to KSZ tagging. So that the implementation is added to
tag_ksz.c and reused common APIs
Signed-off-by: Prasanna Vengateshan <prasanna.vengateshan@microchip.com>
---
include/net/dsa.h | 2 ++
net/dsa/Kconfig | 4 ++--
net/dsa/tag_ksz.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 60 insertions(+), 2 deletions(-)
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 9e5593885357..3c4e1a2e49be 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -51,6 +51,7 @@ struct phylink_link_state;
#define DSA_TAG_PROTO_SEVILLE_VALUE 21
#define DSA_TAG_PROTO_BRCM_LEGACY_VALUE 22
#define DSA_TAG_PROTO_SJA1110_VALUE 23
+#define DSA_TAG_PROTO_LAN937X_VALUE 24
enum dsa_tag_protocol {
DSA_TAG_PROTO_NONE = DSA_TAG_PROTO_NONE_VALUE,
@@ -77,6 +78,7 @@ enum dsa_tag_protocol {
DSA_TAG_PROTO_OCELOT_8021Q = DSA_TAG_PROTO_OCELOT_8021Q_VALUE,
DSA_TAG_PROTO_SEVILLE = DSA_TAG_PROTO_SEVILLE_VALUE,
DSA_TAG_PROTO_SJA1110 = DSA_TAG_PROTO_SJA1110_VALUE,
+ DSA_TAG_PROTO_LAN937X = DSA_TAG_PROTO_LAN937X_VALUE,
};
struct packet_type;
diff --git a/net/dsa/Kconfig b/net/dsa/Kconfig
index bca1b5d66df2..f728e60e0bd3 100644
--- a/net/dsa/Kconfig
+++ b/net/dsa/Kconfig
@@ -87,10 +87,10 @@ config NET_DSA_TAG_MTK
Mediatek switches.
config NET_DSA_TAG_KSZ
- tristate "Tag driver for Microchip 8795/9477/9893 families of switches"
+ tristate "Tag driver for Microchip 8795/937x/9477/9893 families of switches"
help
Say Y if you want to enable support for tagging frames for the
- Microchip 8795/9477/9893 families of switches.
+ Microchip 8795/937x/9477/9893 families of switches.
config NET_DSA_TAG_RTL4_A
tristate "Tag driver for Realtek 4 byte protocol A tags"
diff --git a/net/dsa/tag_ksz.c b/net/dsa/tag_ksz.c
index 53565f48934c..74b2328811d5 100644
--- a/net/dsa/tag_ksz.c
+++ b/net/dsa/tag_ksz.c
@@ -187,10 +187,66 @@ static const struct dsa_device_ops ksz9893_netdev_ops = {
DSA_TAG_DRIVER(ksz9893_netdev_ops);
MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_KSZ9893);
+/* For xmit, 2 bytes are added before FCS.
+ * ---------------------------------------------------------------------------
+ * DA(6bytes)|SA(6bytes)|....|Data(nbytes)|tag0(1byte)|tag1(1byte)|FCS(4bytes)
+ * ---------------------------------------------------------------------------
+ * tag0 : represents tag override, lookup and valid
+ * tag1 : each bit represents port (eg, 0x01=port1, 0x02=port2, 0x80=port8)
+ *
+ * For rcv, 1 byte is added before FCS.
+ * ---------------------------------------------------------------------------
+ * DA(6bytes)|SA(6bytes)|....|Data(nbytes)|tag0(1byte)|FCS(4bytes)
+ * ---------------------------------------------------------------------------
+ * tag0 : zero-based value represents port
+ * (eg, 0x00=port1, 0x02=port3, 0x07=port8)
+ */
+#define LAN937X_EGRESS_TAG_LEN 2
+
+#define LAN937X_TAIL_TAG_BLOCKING_OVERRIDE BIT(11)
+#define LAN937X_TAIL_TAG_LOOKUP BIT(12)
+#define LAN937X_TAIL_TAG_VALID BIT(13)
+#define LAN937X_TAIL_TAG_PORT_MASK 7
+
+static struct sk_buff *lan937x_xmit(struct sk_buff *skb,
+ struct net_device *dev)
+{
+ struct dsa_port *dp = dsa_slave_to_port(dev);
+ const struct ethhdr *hdr = eth_hdr(skb);
+ __be16 *tag;
+ u16 val;
+
+ tag = skb_put(skb, LAN937X_EGRESS_TAG_LEN);
+
+ val = BIT(dp->index);
+
+ if (is_link_local_ether_addr(hdr->h_dest))
+ val |= LAN937X_TAIL_TAG_BLOCKING_OVERRIDE;
+
+ /* Tail tag valid bit - This bit should always be set by the CPU*/
+ val |= LAN937X_TAIL_TAG_VALID;
+
+ *tag = cpu_to_be16(val);
+
+ return skb;
+}
+
+static const struct dsa_device_ops lan937x_netdev_ops = {
+ .name = "lan937x",
+ .proto = DSA_TAG_PROTO_LAN937X,
+ .xmit = lan937x_xmit,
+ .rcv = ksz9477_rcv,
+ .needed_tailroom = LAN937X_EGRESS_TAG_LEN,
+};
+
+DSA_TAG_DRIVER(lan937x_netdev_ops);
+MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_LAN937X);
+
static struct dsa_tag_driver *dsa_tag_driver_array[] = {
&DSA_TAG_DRIVER_NAME(ksz8795_netdev_ops),
&DSA_TAG_DRIVER_NAME(ksz9477_netdev_ops),
&DSA_TAG_DRIVER_NAME(ksz9893_netdev_ops),
+ &DSA_TAG_DRIVER_NAME(lan937x_netdev_ops),
};
module_dsa_tag_drivers(dsa_tag_driver_array);
--
2.27.0
^ permalink raw reply related [flat|nested] 44+ messages in thread* Re: [PATCH v3 net-next 04/10] net: dsa: tag_ksz: add tag handling for Microchip LAN937x
2021-07-23 17:31 ` [PATCH v3 net-next 04/10] net: dsa: tag_ksz: add tag handling for Microchip LAN937x Prasanna Vengateshan
@ 2021-07-23 19:23 ` Vladimir Oltean
0 siblings, 0 replies; 44+ messages in thread
From: Vladimir Oltean @ 2021-07-23 19:23 UTC (permalink / raw)
To: Prasanna Vengateshan
Cc: andrew, netdev, robh+dt, UNGLinuxDriver, Woojung.Huh, hkallweit1,
linux, davem, kuba, linux-kernel, vivien.didelot, f.fainelli,
devicetree
On Fri, Jul 23, 2021 at 11:01:02PM +0530, Prasanna Vengateshan wrote:
> --- a/net/dsa/tag_ksz.c
> +++ b/net/dsa/tag_ksz.c
> @@ -187,10 +187,66 @@ static const struct dsa_device_ops ksz9893_netdev_ops = {
> DSA_TAG_DRIVER(ksz9893_netdev_ops);
> MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_KSZ9893);
>
> +/* For xmit, 2 bytes are added before FCS.
> + * ---------------------------------------------------------------------------
> + * DA(6bytes)|SA(6bytes)|....|Data(nbytes)|tag0(1byte)|tag1(1byte)|FCS(4bytes)
> + * ---------------------------------------------------------------------------
> + * tag0 : represents tag override, lookup and valid
> + * tag1 : each bit represents port (eg, 0x01=port1, 0x02=port2, 0x80=port8)
> + *
> + * For rcv, 1 byte is added before FCS.
> + * ---------------------------------------------------------------------------
> + * DA(6bytes)|SA(6bytes)|....|Data(nbytes)|tag0(1byte)|FCS(4bytes)
> + * ---------------------------------------------------------------------------
> + * tag0 : zero-based value represents port
> + * (eg, 0x00=port1, 0x02=port3, 0x07=port8)
> + */
> +#define LAN937X_EGRESS_TAG_LEN 2
> +
> +#define LAN937X_TAIL_TAG_BLOCKING_OVERRIDE BIT(11)
> +#define LAN937X_TAIL_TAG_LOOKUP BIT(12)
> +#define LAN937X_TAIL_TAG_VALID BIT(13)
> +#define LAN937X_TAIL_TAG_PORT_MASK 7
> +
> +static struct sk_buff *lan937x_xmit(struct sk_buff *skb,
> + struct net_device *dev)
> +{
> + struct dsa_port *dp = dsa_slave_to_port(dev);
> + const struct ethhdr *hdr = eth_hdr(skb);
> + __be16 *tag;
> + u16 val;
There was a recent patch on net.git to fix an issue with DSA master
drivers which set NETIF_F_HW_CSUM in dev->vlan_features, which DSA
inherits.
https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/commit/?id=37120f23ac8998c250573ea3247ff77426551f69
Until we fix the issue at the DSA layer, could you also apply that fix
here please?
> +
> + tag = skb_put(skb, LAN937X_EGRESS_TAG_LEN);
> +
> + val = BIT(dp->index);
> +
> + if (is_link_local_ether_addr(hdr->h_dest))
> + val |= LAN937X_TAIL_TAG_BLOCKING_OVERRIDE;
> +
> + /* Tail tag valid bit - This bit should always be set by the CPU*/
> + val |= LAN937X_TAIL_TAG_VALID;
Please add an extra space here between "CPU" and the comment ending
delimiter.
> +
> + *tag = cpu_to_be16(val);
This probably works for the arch you are testing on, but it is better to
avoid unaligned accesses when the skb->len is odd:
https://www.kernel.org/doc/Documentation/unaligned-memory-access.txt
You can use put_unaligned_be16().
> +
> + return skb;
> +}
^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH v3 net-next 05/10] net: dsa: microchip: add DSA support for microchip lan937x
2021-07-23 17:30 [PATCH v3 net-next 00/10] net: dsa: microchip: DSA driver support for LAN937x switch Prasanna Vengateshan
` (3 preceding siblings ...)
2021-07-23 17:31 ` [PATCH v3 net-next 04/10] net: dsa: tag_ksz: add tag handling for Microchip LAN937x Prasanna Vengateshan
@ 2021-07-23 17:31 ` Prasanna Vengateshan
2021-07-31 15:04 ` Vladimir Oltean
2021-07-23 17:31 ` [PATCH v3 net-next 06/10] net: dsa: microchip: add support for phylink management Prasanna Vengateshan
` (4 subsequent siblings)
9 siblings, 1 reply; 44+ messages in thread
From: Prasanna Vengateshan @ 2021-07-23 17:31 UTC (permalink / raw)
To: andrew, netdev, olteanv, robh+dt
Cc: UNGLinuxDriver, Woojung.Huh, hkallweit1, linux, davem, kuba,
linux-kernel, vivien.didelot, f.fainelli, devicetree
Basic DSA driver support for lan937x and the device will be
configured through SPI interface.
drivers/net/dsa/microchip/ path is already part of MAINTAINERS &
the new files come under this path. Hence no update needed to the
MAINTAINERS
Reused KSZ APIs for port_bridge_join() & port_bridge_leave() and
added support for port_stp_state_set() & port_fast_age().
lan937x_flush_dyn_mac_table() which gets called from
port_fast_age() of KSZ common layer, hence added support for it.
Signed-off-by: Prasanna Vengateshan <prasanna.vengateshan@microchip.com>
---
drivers/net/dsa/microchip/Kconfig | 12 +
drivers/net/dsa/microchip/Makefile | 5 +
drivers/net/dsa/microchip/ksz_common.h | 5 +
drivers/net/dsa/microchip/lan937x_dev.c | 650 ++++++++++++++++++++++
drivers/net/dsa/microchip/lan937x_dev.h | 80 +++
drivers/net/dsa/microchip/lan937x_main.c | 317 +++++++++++
drivers/net/dsa/microchip/lan937x_reg.h | 677 +++++++++++++++++++++++
drivers/net/dsa/microchip/lan937x_spi.c | 223 ++++++++
8 files changed, 1969 insertions(+)
create mode 100644 drivers/net/dsa/microchip/lan937x_dev.c
create mode 100644 drivers/net/dsa/microchip/lan937x_dev.h
create mode 100644 drivers/net/dsa/microchip/lan937x_main.c
create mode 100644 drivers/net/dsa/microchip/lan937x_reg.h
create mode 100644 drivers/net/dsa/microchip/lan937x_spi.c
diff --git a/drivers/net/dsa/microchip/Kconfig b/drivers/net/dsa/microchip/Kconfig
index c9e2a8989556..f329cca934ee 100644
--- a/drivers/net/dsa/microchip/Kconfig
+++ b/drivers/net/dsa/microchip/Kconfig
@@ -3,6 +3,18 @@ config NET_DSA_MICROCHIP_KSZ_COMMON
select NET_DSA_TAG_KSZ
tristate
+config NET_DSA_MICROCHIP_LAN937X
+ tristate "Microchip LAN937X series SPI connected switch support"
+ depends on NET_DSA && SPI
+ select NET_DSA_MICROCHIP_KSZ_COMMON
+ select REGMAP_SPI
+ help
+ This driver adds support for Microchip LAN937X series
+ switch chips.
+
+ Select to enable support for registering switches configured
+ through SPI.
+
menuconfig NET_DSA_MICROCHIP_KSZ9477
tristate "Microchip KSZ9477 series switch support"
depends on NET_DSA
diff --git a/drivers/net/dsa/microchip/Makefile b/drivers/net/dsa/microchip/Makefile
index 2a03b21a3386..28d8eb62a795 100644
--- a/drivers/net/dsa/microchip/Makefile
+++ b/drivers/net/dsa/microchip/Makefile
@@ -6,3 +6,8 @@ obj-$(CONFIG_NET_DSA_MICROCHIP_KSZ9477_SPI) += ksz9477_spi.o
obj-$(CONFIG_NET_DSA_MICROCHIP_KSZ8795) += ksz8795.o
obj-$(CONFIG_NET_DSA_MICROCHIP_KSZ8795_SPI) += ksz8795_spi.o
obj-$(CONFIG_NET_DSA_MICROCHIP_KSZ8863_SMI) += ksz8863_smi.o
+
+obj-$(CONFIG_NET_DSA_MICROCHIP_LAN937X) += lan937x.o
+lan937x-objs := lan937x_dev.o
+lan937x-objs += lan937x_main.o
+lan937x-objs += lan937x_spi.o
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index 2e6bfd333f50..690d339edd7b 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -60,6 +60,8 @@ struct ksz_device {
struct gpio_desc *reset_gpio; /* Optional reset GPIO */
+ struct device_node *mdio_np;
+
/* chip specific data */
u32 chip_id;
int num_vlans;
@@ -145,6 +147,9 @@ void ksz_switch_remove(struct ksz_device *dev);
int ksz8_switch_register(struct ksz_device *dev);
int ksz9477_switch_register(struct ksz_device *dev);
+int lan937x_switch_register(struct ksz_device *dev);
+
+int lan937x_check_device_id(struct ksz_device *dev);
void ksz_update_port_member(struct ksz_device *dev, int port);
void ksz_init_mib_timer(struct ksz_device *dev);
diff --git a/drivers/net/dsa/microchip/lan937x_dev.c b/drivers/net/dsa/microchip/lan937x_dev.c
new file mode 100644
index 000000000000..86e84a79f464
--- /dev/null
+++ b/drivers/net/dsa/microchip/lan937x_dev.c
@@ -0,0 +1,650 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Microchip lan937x dev ops functions
+ * Copyright (C) 2019-2021 Microchip Technology Inc.
+ */
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/iopoll.h>
+#include <linux/of_mdio.h>
+#include <linux/platform_data/microchip-ksz.h>
+#include <linux/phy.h>
+#include <linux/if_bridge.h>
+#include <net/dsa.h>
+#include <net/switchdev.h>
+
+#include "lan937x_reg.h"
+#include "ksz_common.h"
+#include "lan937x_dev.h"
+
+const struct mib_names lan937x_mib_names[] = {
+ { 0x00, "rx_hi" },
+ { 0x01, "rx_undersize" },
+ { 0x02, "rx_fragments" },
+ { 0x03, "rx_oversize" },
+ { 0x04, "rx_jabbers" },
+ { 0x05, "rx_symbol_err" },
+ { 0x06, "rx_crc_err" },
+ { 0x07, "rx_align_err" },
+ { 0x08, "rx_mac_ctrl" },
+ { 0x09, "rx_pause" },
+ { 0x0A, "rx_bcast" },
+ { 0x0B, "rx_mcast" },
+ { 0x0C, "rx_ucast" },
+ { 0x0D, "rx_64_or_less" },
+ { 0x0E, "rx_65_127" },
+ { 0x0F, "rx_128_255" },
+ { 0x10, "rx_256_511" },
+ { 0x11, "rx_512_1023" },
+ { 0x12, "rx_1024_1522" },
+ { 0x13, "rx_1523_2000" },
+ { 0x14, "rx_2001" },
+ { 0x15, "tx_hi" },
+ { 0x16, "tx_late_col" },
+ { 0x17, "tx_pause" },
+ { 0x18, "tx_bcast" },
+ { 0x19, "tx_mcast" },
+ { 0x1A, "tx_ucast" },
+ { 0x1B, "tx_deferred" },
+ { 0x1C, "tx_total_col" },
+ { 0x1D, "tx_exc_col" },
+ { 0x1E, "tx_single_col" },
+ { 0x1F, "tx_mult_col" },
+ { 0x80, "rx_total" },
+ { 0x81, "tx_total" },
+ { 0x82, "rx_discards" },
+ { 0x83, "tx_discards" },
+};
+
+struct prt_init {
+ int addr;
+ u8 mask;
+ bool is_set;
+};
+
+int lan937x_cfg(struct ksz_device *dev, u32 addr, u8 bits, bool set)
+{
+ return regmap_update_bits(dev->regmap[0], addr, bits, set ? bits : 0);
+}
+
+int lan937x_port_cfg(struct ksz_device *dev, int port, int offset, u8 bits,
+ bool set)
+{
+ return regmap_update_bits(dev->regmap[0], PORT_CTRL_ADDR(port, offset),
+ bits, set ? bits : 0);
+}
+
+int lan937x_cfg32(struct ksz_device *dev, u32 addr, u32 bits, bool set)
+{
+ return regmap_update_bits(dev->regmap[2], addr, bits, set ? bits : 0);
+}
+
+int lan937x_pread8(struct ksz_device *dev, int port, int offset, u8 *data)
+{
+ return ksz_read8(dev, PORT_CTRL_ADDR(port, offset), data);
+}
+
+int lan937x_pread16(struct ksz_device *dev, int port, int offset, u16 *data)
+{
+ return ksz_read16(dev, PORT_CTRL_ADDR(port, offset), data);
+}
+
+int lan937x_pread32(struct ksz_device *dev, int port, int offset, u32 *data)
+{
+ return ksz_read32(dev, PORT_CTRL_ADDR(port, offset), data);
+}
+
+int lan937x_pwrite8(struct ksz_device *dev, int port, int offset, u8 data)
+{
+ return ksz_write8(dev, PORT_CTRL_ADDR(port, offset), data);
+}
+
+int lan937x_pwrite16(struct ksz_device *dev, int port, int offset, u16 data)
+{
+ return ksz_write16(dev, PORT_CTRL_ADDR(port, offset), data);
+}
+
+int lan937x_pwrite32(struct ksz_device *dev, int port, int offset, u32 data)
+{
+ return ksz_write32(dev, PORT_CTRL_ADDR(port, offset), data);
+}
+
+int lan937x_port_cfg32(struct ksz_device *dev, int port, int offset, u32 bits,
+ bool set)
+{
+ return regmap_update_bits(dev->regmap[2], PORT_CTRL_ADDR(port, offset),
+ bits, set ? bits : 0);
+}
+
+void lan937x_cfg_port_member(struct ksz_device *dev, int port, u8 member)
+{
+ lan937x_pwrite32(dev, port, REG_PORT_VLAN_MEMBERSHIP__4, member);
+
+ dev->ports[port].member = member;
+}
+
+static void lan937x_flush_dyn_mac_table(struct ksz_device *dev, int port)
+{
+ unsigned int value;
+ u8 data;
+
+ regmap_update_bits(dev->regmap[0], REG_SW_LUE_CTRL_2,
+ SW_FLUSH_OPTION_M << SW_FLUSH_OPTION_S,
+ SW_FLUSH_OPTION_DYN_MAC << SW_FLUSH_OPTION_S);
+
+ if (port < dev->port_cnt) {
+ /* flush individual port */
+ lan937x_pread8(dev, port, P_STP_CTRL, &data);
+ if (!(data & PORT_LEARN_DISABLE))
+ lan937x_pwrite8(dev, port, P_STP_CTRL,
+ (data | PORT_LEARN_DISABLE));
+ lan937x_cfg(dev, S_FLUSH_TABLE_CTRL, SW_FLUSH_DYN_MAC_TABLE,
+ true);
+
+ regmap_read_poll_timeout(dev->regmap[0], S_FLUSH_TABLE_CTRL,
+ value,
+ !(value & SW_FLUSH_DYN_MAC_TABLE), 10,
+ 1000);
+
+ lan937x_pwrite8(dev, port, P_STP_CTRL, data);
+ } else {
+ /* flush all */
+ lan937x_cfg(dev, S_FLUSH_TABLE_CTRL, SW_FLUSH_STP_TABLE, true);
+ }
+}
+
+static void lan937x_r_mib_cnt(struct ksz_device *dev, int port, u16 addr,
+ u64 *cnt)
+{
+ unsigned int val;
+ u32 data;
+ int ret;
+
+ /* Enable MIB Counter read*/
+ data = MIB_COUNTER_READ;
+ data |= (addr << MIB_COUNTER_INDEX_S);
+ lan937x_pwrite32(dev, port, REG_PORT_MIB_CTRL_STAT, data);
+
+ ret = regmap_read_poll_timeout(dev->regmap[2],
+ PORT_CTRL_ADDR(port, REG_PORT_MIB_CTRL_STAT),
+ val, !(val & MIB_COUNTER_READ),
+ 10, 1000);
+ if (ret) {
+ dev_err(dev->dev, "Failed to get MIB\n");
+ return;
+ }
+
+ /* count resets upon read */
+ lan937x_pread32(dev, port, REG_PORT_MIB_DATA, &data);
+ *cnt += data;
+}
+
+static void lan937x_r_mib_pkt(struct ksz_device *dev, int port, u16 addr,
+ u64 *dropped, u64 *cnt)
+{
+ addr = lan937x_mib_names[addr].index;
+ lan937x_r_mib_cnt(dev, port, addr, cnt);
+}
+
+static void lan937x_port_init_cnt(struct ksz_device *dev, int port)
+{
+ struct ksz_port_mib *mib = &dev->ports[port].mib;
+
+ /* flush all enabled port MIB counters */
+ mutex_lock(&mib->cnt_mutex);
+ lan937x_pwrite32(dev, port, REG_PORT_MIB_CTRL_STAT,
+ MIB_COUNTER_FLUSH_FREEZE);
+ ksz_write8(dev, REG_SW_MAC_CTRL_6, SW_MIB_COUNTER_FLUSH);
+ lan937x_pwrite32(dev, port, REG_PORT_MIB_CTRL_STAT, 0);
+ mutex_unlock(&mib->cnt_mutex);
+}
+
+int lan937x_reset_switch(struct ksz_device *dev)
+{
+ u32 data32;
+ u8 data8;
+ int ret;
+
+ /* reset switch */
+ ret = lan937x_cfg(dev, REG_SW_OPERATION, SW_RESET, true);
+ if (ret < 0)
+ return ret;
+
+ ret = ksz_read8(dev, REG_SW_LUE_CTRL_1, &data8);
+ if (ret < 0)
+ return ret;
+
+ /* Enable Auto Aging*/
+ ret = ksz_write8(dev, REG_SW_LUE_CTRL_1, data8 | SW_LINK_AUTO_AGING);
+ if (ret < 0)
+ return ret;
+
+ /* disable interrupts */
+ ret = ksz_write32(dev, REG_SW_INT_MASK__4, SWITCH_INT_MASK);
+ if (ret < 0)
+ return ret;
+
+ ret = ksz_write32(dev, REG_SW_PORT_INT_MASK__4, 0xFF);
+ if (ret < 0)
+ return ret;
+
+ ret = ksz_read32(dev, REG_SW_PORT_INT_STATUS__4, &data32);
+
+ return ret;
+}
+
+static int lan937x_switch_detect(struct ksz_device *dev)
+{
+ u32 id32;
+ int ret;
+
+ /* Read Chip ID */
+ ret = ksz_read32(dev, REG_CHIP_ID0__1, &id32);
+ if (ret < 0)
+ return ret;
+
+ if (id32 != 0 && id32 != 0xffffffff) {
+ dev->chip_id = id32;
+ dev_info(dev->dev, "Chip ID: 0x%x", id32);
+ ret = 0;
+ } else {
+ ret = -EINVAL;
+ }
+
+ return ret;
+}
+
+static void lan937x_switch_exit(struct ksz_device *dev)
+{
+ lan937x_reset_switch(dev);
+
+ if (dev->mdio_np) {
+ mdiobus_unregister(dev->ds->slave_mii_bus);
+ of_node_put(dev->mdio_np);
+ }
+}
+
+int lan937x_enable_spi_indirect_access(struct ksz_device *dev)
+{
+ u16 data16;
+ u8 data8;
+ int ret;
+
+ ret = ksz_read8(dev, REG_GLOBAL_CTRL_0, &data8);
+ if (ret < 0)
+ return ret;
+
+ /* Check if PHY register is blocked */
+ if (data8 & SW_PHY_REG_BLOCK) {
+ /* Enable Phy access through SPI*/
+ data8 &= ~SW_PHY_REG_BLOCK;
+
+ ret = ksz_write8(dev, REG_GLOBAL_CTRL_0, data8);
+ if (ret < 0)
+ return ret;
+ }
+
+ ret = ksz_read16(dev, REG_VPHY_SPECIAL_CTRL__2, &data16);
+ if (ret < 0)
+ return ret;
+
+ /* Allow SPI access */
+ data16 |= VPHY_SPI_INDIRECT_ENABLE;
+ ret = ksz_write16(dev, REG_VPHY_SPECIAL_CTRL__2, data16);
+
+ return ret;
+}
+
+bool lan937x_is_internal_phy_port(struct ksz_device *dev, int port)
+{
+ /* Check if the port is RGMII */
+ if (port == LAN937X_RGMII_1_PORT || port == LAN937X_RGMII_2_PORT)
+ return false;
+
+ /* Check if the port is SGMII */
+ if (port == LAN937X_SGMII_PORT &&
+ GET_CHIP_ID_LSB(dev->chip_id) == CHIP_ID_73)
+ return false;
+
+ return true;
+}
+
+static u32 lan937x_get_port_addr(int port, int offset)
+{
+ return PORT_CTRL_ADDR(port, offset);
+}
+
+bool lan937x_is_internal_100BTX_phy_port(struct ksz_device *dev, int port)
+{
+ /* Check if the port is internal tx phy port */
+ if (lan937x_is_internal_phy_port(dev, port) &&
+ port == LAN937X_TXPHY_PORT)
+ if ((GET_CHIP_ID_LSB(dev->chip_id) == CHIP_ID_71) ||
+ (GET_CHIP_ID_LSB(dev->chip_id) == CHIP_ID_72))
+ return true;
+
+ return false;
+}
+
+bool lan937x_is_internal_t1_phy_port(struct ksz_device *dev, int port)
+{
+ /* Check if the port is internal t1 phy port */
+ if (lan937x_is_internal_phy_port(dev, port) &&
+ !lan937x_is_internal_100BTX_phy_port(dev, port))
+ return true;
+
+ return false;
+}
+
+int lan937x_internal_phy_write(struct ksz_device *dev, int addr, int reg,
+ u16 val)
+{
+ u16 temp, addr_base;
+ unsigned int value;
+ int ret;
+
+ /* Check for internal phy port */
+ if (!lan937x_is_internal_phy_port(dev, addr))
+ return -EOPNOTSUPP;
+
+ if (lan937x_is_internal_100BTX_phy_port(dev, addr))
+ addr_base = REG_PORT_TX_PHY_CTRL_BASE;
+ else
+ addr_base = REG_PORT_T1_PHY_CTRL_BASE;
+
+ temp = PORT_CTRL_ADDR(addr, (addr_base + (reg << 2)));
+
+ ret = ksz_write16(dev, REG_VPHY_IND_ADDR__2, temp);
+ if (ret < 0)
+ return ret;
+
+ /* Write the data to be written to the VPHY reg */
+ ret = ksz_write16(dev, REG_VPHY_IND_DATA__2, val);
+ if (ret < 0)
+ return ret;
+
+ /* Write the Write En and Busy bit */
+ ret = ksz_write16(dev, REG_VPHY_IND_CTRL__2,
+ (VPHY_IND_WRITE | VPHY_IND_BUSY));
+ if (ret < 0)
+ return ret;
+
+ ret = regmap_read_poll_timeout(dev->regmap[1], REG_VPHY_IND_CTRL__2,
+ value, !(value & VPHY_IND_BUSY), 10,
+ 1000);
+ if (ret < 0) {
+ dev_err(dev->dev, "Failed to write phy register\n");
+ return ret;
+ }
+
+ return 0;
+}
+
+int lan937x_internal_phy_read(struct ksz_device *dev, int addr, int reg,
+ u16 *val)
+{
+ u16 temp, addr_base;
+ unsigned int value;
+ int ret;
+
+ /* Check for internal phy port, return 0xffff for non-existent phy*/
+ if (!lan937x_is_internal_phy_port(dev, addr))
+ return 0xffff;
+
+ if (lan937x_is_internal_100BTX_phy_port(dev, addr))
+ addr_base = REG_PORT_TX_PHY_CTRL_BASE;
+ else
+ addr_base = REG_PORT_T1_PHY_CTRL_BASE;
+
+ /* get register address based on the logical port */
+ temp = PORT_CTRL_ADDR(addr, (addr_base + (reg << 2)));
+
+ ret = ksz_write16(dev, REG_VPHY_IND_ADDR__2, temp);
+ if (ret < 0)
+ return ret;
+
+ /* Write Read and Busy bit to start the transaction*/
+ ret = ksz_write16(dev, REG_VPHY_IND_CTRL__2, VPHY_IND_BUSY);
+ if (ret < 0)
+ return ret;
+
+ ret = regmap_read_poll_timeout(dev->regmap[1], REG_VPHY_IND_CTRL__2,
+ value, !(value & VPHY_IND_BUSY), 10,
+ 1000);
+ if (ret < 0) {
+ dev_err(dev->dev, "Failed to read phy register\n");
+ return ret;
+ }
+
+ /* Read the VPHY register which has the PHY data*/
+ ret = ksz_read16(dev, REG_VPHY_IND_DATA__2, val);
+
+ return ret;
+}
+
+static void lan937x_config_gbit(struct ksz_device *dev, bool gbit, u8 *data)
+{
+ if (gbit)
+ *data &= ~PORT_MII_NOT_1GBIT;
+ else
+ *data |= PORT_MII_NOT_1GBIT;
+}
+
+void lan937x_mac_config(struct ksz_device *dev, int port,
+ phy_interface_t interface)
+{
+ u8 data8;
+
+ lan937x_pread8(dev, port, REG_PORT_XMII_CTRL_1, &data8);
+
+ /* clear MII selection & set it based on interface later */
+ data8 &= ~PORT_MII_SEL_M;
+
+ /* configure MAC based on interface */
+ switch (interface) {
+ case PHY_INTERFACE_MODE_MII:
+ lan937x_config_gbit(dev, false, &data8);
+ data8 |= PORT_MII_SEL;
+ break;
+ case PHY_INTERFACE_MODE_RMII:
+ lan937x_config_gbit(dev, false, &data8);
+ data8 |= PORT_RMII_SEL;
+ break;
+ case PHY_INTERFACE_MODE_RGMII:
+ case PHY_INTERFACE_MODE_RGMII_ID:
+ case PHY_INTERFACE_MODE_RGMII_TXID:
+ case PHY_INTERFACE_MODE_RGMII_RXID:
+ lan937x_config_gbit(dev, true, &data8);
+ data8 |= PORT_RGMII_SEL;
+
+ /* Add RGMII internal delay for cpu port*/
+ if (dsa_is_cpu_port(dev->ds, port)) {
+ if (interface == PHY_INTERFACE_MODE_RGMII_ID ||
+ interface == PHY_INTERFACE_MODE_RGMII_RXID)
+ data8 |= PORT_RGMII_ID_IG_ENABLE;
+
+ if (interface == PHY_INTERFACE_MODE_RGMII_ID ||
+ interface == PHY_INTERFACE_MODE_RGMII_TXID)
+ data8 |= PORT_RGMII_ID_EG_ENABLE;
+ }
+ break;
+ default:
+ dev_err(dev->dev, "Unsupported interface '%s' for port %d\n",
+ phy_modes(interface), port);
+ return;
+ }
+
+ /* Write the updated value */
+ lan937x_pwrite8(dev, port, REG_PORT_XMII_CTRL_1, data8);
+}
+
+void lan937x_port_setup(struct ksz_device *dev, int port, bool cpu_port)
+{
+ struct ksz_port *p = &dev->ports[port];
+ u8 member;
+
+ /* enable tag tail for host port */
+ if (cpu_port) {
+ lan937x_port_cfg(dev, port, REG_PORT_CTRL_0,
+ PORT_TAIL_TAG_ENABLE, true);
+ }
+
+ /*disable frame check length field*/
+ lan937x_port_cfg(dev, port, REG_PORT_MAC_CTRL_0, PORT_FR_CHK_LENGTH,
+ false);
+
+ /* set back pressure for half duplex */
+ lan937x_port_cfg(dev, port, REG_PORT_MAC_CTRL_1, PORT_BACK_PRESSURE,
+ true);
+
+ /* enable 802.1p priority */
+ lan937x_port_cfg(dev, port, P_PRIO_CTRL, PORT_802_1P_PRIO_ENABLE, true);
+
+ if (!lan937x_is_internal_phy_port(dev, port)) {
+ /* enable flow control */
+ lan937x_port_cfg(dev, port, REG_PORT_XMII_CTRL_0,
+ PORT_TX_FLOW_CTRL | PORT_RX_FLOW_CTRL,
+ true);
+ lan937x_mac_config(dev, port, p->interface);
+ }
+
+ if (cpu_port)
+ member = dev->port_mask;
+ else
+ member = dev->host_mask | p->vid_member;
+
+ lan937x_cfg_port_member(dev, port, member);
+}
+
+static int lan937x_sw_mdio_read(struct mii_bus *bus, int addr, int regnum)
+{
+ struct ksz_device *dev = bus->priv;
+ u16 val;
+ int ret;
+
+ ret = lan937x_internal_phy_read(dev, addr, regnum, &val);
+ if (ret < 0)
+ return ret;
+
+ return val;
+}
+
+static int lan937x_sw_mdio_write(struct mii_bus *bus, int addr, int regnum,
+ u16 val)
+{
+ struct ksz_device *dev = bus->priv;
+
+ return lan937x_internal_phy_write(dev, addr, regnum, val);
+}
+
+static int lan937x_mdio_register(struct dsa_switch *ds)
+{
+ struct ksz_device *dev = ds->priv;
+ int ret;
+
+ dev->mdio_np = of_get_child_by_name(ds->dev->of_node, "mdio");
+
+ if (!dev->mdio_np) {
+ dev_err(ds->dev, "no MDIO bus node\n");
+ return -ENODEV;
+ }
+
+ ds->slave_mii_bus = devm_mdiobus_alloc(ds->dev);
+ if (!ds->slave_mii_bus)
+ return -ENOMEM;
+
+ ds->slave_mii_bus->priv = ds->priv;
+ ds->slave_mii_bus->read = lan937x_sw_mdio_read;
+ ds->slave_mii_bus->write = lan937x_sw_mdio_write;
+ ds->slave_mii_bus->name = "lan937x slave smi";
+ snprintf(ds->slave_mii_bus->id, MII_BUS_ID_SIZE, "SMI-%d", ds->index);
+ ds->slave_mii_bus->parent = ds->dev;
+ ds->slave_mii_bus->phy_mask = ~ds->phys_mii_mask;
+
+ ret = of_mdiobus_register(ds->slave_mii_bus, dev->mdio_np);
+ if (ret) {
+ dev_err(ds->dev, "unable to register MDIO bus %s\n",
+ ds->slave_mii_bus->id);
+ of_node_put(dev->mdio_np);
+ return ret;
+ }
+
+ return 0;
+}
+
+static int lan937x_switch_init(struct ksz_device *dev)
+{
+ int i, ret;
+
+ dev->ds->ops = &lan937x_switch_ops;
+
+ /* Check device tree */
+ ret = lan937x_check_device_id(dev);
+ if (ret < 0)
+ return ret;
+
+ dev->port_mask = (1 << dev->port_cnt) - 1;
+
+ dev->reg_mib_cnt = SWITCH_COUNTER_NUM;
+ dev->mib_cnt = ARRAY_SIZE(lan937x_mib_names);
+
+ dev->ports = devm_kzalloc(dev->dev,
+ dev->port_cnt * sizeof(struct ksz_port),
+ GFP_KERNEL);
+ if (!dev->ports)
+ return -ENOMEM;
+
+ for (i = 0; i < dev->port_cnt; i++) {
+ mutex_init(&dev->ports[i].mib.cnt_mutex);
+ dev->ports[i].mib.counters =
+ devm_kzalloc(dev->dev,
+ sizeof(u64) * (dev->mib_cnt + 1),
+ GFP_KERNEL);
+
+ if (!dev->ports[i].mib.counters)
+ return -ENOMEM;
+ }
+
+ /* set the real number of ports */
+ dev->ds->num_ports = dev->port_cnt;
+ return 0;
+}
+
+static int lan937x_init(struct ksz_device *dev)
+{
+ int ret;
+
+ ret = lan937x_switch_init(dev);
+ if (ret < 0) {
+ dev_err(dev->dev, "failed to initialize the switch");
+ return ret;
+ }
+
+ /* enable Indirect Access from SPI to the VPHY registers */
+ ret = lan937x_enable_spi_indirect_access(dev);
+ if (ret < 0) {
+ dev_err(dev->dev, "failed to enable spi indirect access");
+ return ret;
+ }
+
+ ret = lan937x_mdio_register(dev->ds);
+ if (ret < 0) {
+ dev_err(dev->dev, "failed to register the mdio");
+ return ret;
+ }
+
+ return 0;
+}
+
+const struct ksz_dev_ops lan937x_dev_ops = {
+ .get_port_addr = lan937x_get_port_addr,
+ .cfg_port_member = lan937x_cfg_port_member,
+ .flush_dyn_mac_table = lan937x_flush_dyn_mac_table,
+ .port_setup = lan937x_port_setup,
+ .r_mib_cnt = lan937x_r_mib_cnt,
+ .r_mib_pkt = lan937x_r_mib_pkt,
+ .port_init_cnt = lan937x_port_init_cnt,
+ .shutdown = lan937x_reset_switch,
+ .detect = lan937x_switch_detect,
+ .init = lan937x_init,
+ .exit = lan937x_switch_exit,
+};
diff --git a/drivers/net/dsa/microchip/lan937x_dev.h b/drivers/net/dsa/microchip/lan937x_dev.h
new file mode 100644
index 000000000000..76c00db05616
--- /dev/null
+++ b/drivers/net/dsa/microchip/lan937x_dev.h
@@ -0,0 +1,80 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Microchip lan937x dev ops headers
+ * Copyright (C) 2019-2021 Microchip Technology Inc.
+ */
+
+#ifndef __LAN937X_CFG_H
+#define __LAN937X_CFG_H
+
+int lan937x_cfg(struct ksz_device *dev, u32 addr, u8 bits, bool set);
+int lan937x_port_cfg(struct ksz_device *dev, int port, int offset,
+ u8 bits, bool set);
+int lan937x_cfg32(struct ksz_device *dev, u32 addr, u32 bits, bool set);
+int lan937x_pread8(struct ksz_device *dev, int port, int offset,
+ u8 *data);
+int lan937x_pread16(struct ksz_device *dev, int port, int offset,
+ u16 *data);
+int lan937x_pread32(struct ksz_device *dev, int port, int offset,
+ u32 *data);
+int lan937x_pwrite8(struct ksz_device *dev, int port,
+ int offset, u8 data);
+int lan937x_pwrite16(struct ksz_device *dev, int port,
+ int offset, u16 data);
+int lan937x_pwrite32(struct ksz_device *dev, int port,
+ int offset, u32 data);
+int lan937x_port_cfg32(struct ksz_device *dev, int port, int offset,
+ u32 bits, bool set);
+int lan937x_internal_phy_write(struct ksz_device *dev, int addr,
+ int reg, u16 val);
+int lan937x_internal_phy_read(struct ksz_device *dev, int addr,
+ int reg, u16 *val);
+bool lan937x_is_internal_100BTX_phy_port(struct ksz_device *dev, int port);
+bool lan937x_is_internal_t1_phy_port(struct ksz_device *dev, int port);
+bool lan937x_is_internal_phy_port(struct ksz_device *dev, int port);
+int lan937x_reset_switch(struct ksz_device *dev);
+void lan937x_cfg_port_member(struct ksz_device *dev, int port,
+ u8 member);
+void lan937x_port_setup(struct ksz_device *dev, int port, bool cpu_port);
+int lan937x_enable_spi_indirect_access(struct ksz_device *dev);
+void lan937x_mac_config(struct ksz_device *dev, int port,
+ phy_interface_t interface);
+
+struct mib_names {
+ int index;
+ char string[ETH_GSTRING_LEN];
+};
+
+struct lan_alu_struct {
+ /* entry 1 */
+ u8 is_static:1;
+ u8 is_src_filter:1;
+ u8 is_dst_filter:1;
+ u8 prio_age:3;
+ u32 _reserv_0_1:23;
+ u8 mstp:3;
+ /* entry 2 */
+ u8 is_override:1;
+ u8 is_use_fid:1;
+ u32 _reserv_1_1:22;
+ u8 port_forward:8;
+ /* entry 3 & 4*/
+ u32 _reserv_2_1:9;
+ u8 fid:7;
+ u8 mac[ETH_ALEN];
+};
+
+struct lan937x_vlan {
+ /* entry 1 */
+ bool valid;
+ u8 fid;
+ /* entry 2 */
+ u32 untag_prtmap;
+ /* entry 3 */
+ u32 fwd_map;
+};
+
+extern const struct dsa_switch_ops lan937x_switch_ops;
+extern const struct ksz_dev_ops lan937x_dev_ops;
+extern const struct mib_names lan937x_mib_names[];
+
+#endif
diff --git a/drivers/net/dsa/microchip/lan937x_main.c b/drivers/net/dsa/microchip/lan937x_main.c
new file mode 100644
index 000000000000..d43db522d07e
--- /dev/null
+++ b/drivers/net/dsa/microchip/lan937x_main.c
@@ -0,0 +1,317 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Microchip LAN937X switch driver main logic
+ * Copyright (C) 2019-2021 Microchip Technology Inc.
+ */
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/iopoll.h>
+#include <linux/phy.h>
+#include <linux/if_bridge.h>
+#include <linux/if_vlan.h>
+#include <net/dsa.h>
+#include <net/switchdev.h>
+
+#include "lan937x_reg.h"
+#include "ksz_common.h"
+#include "lan937x_dev.h"
+
+static enum dsa_tag_protocol lan937x_get_tag_protocol(struct dsa_switch *ds,
+ int port,
+ enum dsa_tag_protocol mp)
+{
+ return DSA_TAG_PROTO_LAN937X_VALUE;
+}
+
+static int lan937x_phy_read16(struct dsa_switch *ds, int addr, int reg)
+{
+ struct ksz_device *dev = ds->priv;
+ u16 val;
+ int ret;
+
+ ret = lan937x_internal_phy_read(dev, addr, reg, &val);
+ if (ret < 0)
+ return ret;
+
+ return val;
+}
+
+static int lan937x_phy_write16(struct dsa_switch *ds, int addr, int reg,
+ u16 val)
+{
+ struct ksz_device *dev = ds->priv;
+
+ return lan937x_internal_phy_write(dev, addr, reg, val);
+}
+
+static void lan937x_port_stp_state_set(struct dsa_switch *ds, int port,
+ u8 state)
+{
+ struct ksz_device *dev = ds->priv;
+ struct ksz_port *p = &dev->ports[port];
+ int forward = dev->member;
+ int member = -1;
+ u8 data;
+
+ lan937x_pread8(dev, port, P_STP_CTRL, &data);
+ data &= ~(PORT_TX_ENABLE | PORT_RX_ENABLE | PORT_LEARN_DISABLE);
+
+ switch (state) {
+ case BR_STATE_DISABLED:
+ data |= PORT_LEARN_DISABLE;
+ break;
+ case BR_STATE_LISTENING:
+ data |= (PORT_RX_ENABLE | PORT_LEARN_DISABLE);
+ if (p->stp_state == BR_STATE_DISABLED)
+ member = dev->host_mask | p->vid_member;
+ break;
+ case BR_STATE_LEARNING:
+ data |= PORT_RX_ENABLE;
+ break;
+ case BR_STATE_FORWARDING:
+ data |= (PORT_TX_ENABLE | PORT_RX_ENABLE);
+
+ member = dev->host_mask | p->vid_member;
+ mutex_lock(&dev->dev_mutex);
+
+ /* Port is a member of a bridge. */
+ if (dev->br_member & (1 << port)) {
+ dev->member |= (1 << port);
+ member = dev->member;
+ }
+ mutex_unlock(&dev->dev_mutex);
+ break;
+ case BR_STATE_BLOCKING:
+ data |= PORT_LEARN_DISABLE;
+ if (p->stp_state == BR_STATE_DISABLED)
+ member = dev->host_mask | p->vid_member;
+ break;
+ default:
+ dev_err(ds->dev, "invalid STP state: %d\n", state);
+ return;
+ }
+
+ lan937x_pwrite8(dev, port, P_STP_CTRL, data);
+
+ p->stp_state = state;
+ mutex_lock(&dev->dev_mutex);
+
+ /* Port membership may share register with STP state. */
+ if (member >= 0 && member != p->member)
+ lan937x_cfg_port_member(dev, port, (u8)member);
+
+ /* Check if forwarding needs to be updated. */
+ if (state != BR_STATE_FORWARDING) {
+ if (dev->br_member & (1 << port))
+ dev->member &= ~(1 << port);
+ }
+
+ /* When topology has changed the function ksz_update_port_member
+ * should be called to modify port forwarding behavior.
+ */
+ if (forward != dev->member)
+ ksz_update_port_member(dev, port);
+
+ mutex_unlock(&dev->dev_mutex);
+}
+
+static phy_interface_t lan937x_get_interface(struct ksz_device *dev, int port)
+{
+ phy_interface_t interface;
+ u8 data8;
+ int ret;
+
+ if (lan937x_is_internal_phy_port(dev, port))
+ return PHY_INTERFACE_MODE_NA;
+
+ /* read interface from REG_PORT_XMII_CTRL_1 register */
+ ret = lan937x_pread8(dev, port, REG_PORT_XMII_CTRL_1, &data8);
+ if (ret < 0)
+ return PHY_INTERFACE_MODE_NA;
+
+ switch (data8 & PORT_MII_SEL_M) {
+ case PORT_RMII_SEL:
+ interface = PHY_INTERFACE_MODE_RMII;
+ break;
+ case PORT_RGMII_SEL:
+ interface = PHY_INTERFACE_MODE_RGMII;
+ if (data8 & PORT_RGMII_ID_EG_ENABLE)
+ interface = PHY_INTERFACE_MODE_RGMII_TXID;
+ if (data8 & PORT_RGMII_ID_IG_ENABLE) {
+ interface = PHY_INTERFACE_MODE_RGMII_RXID;
+ if (data8 & PORT_RGMII_ID_EG_ENABLE)
+ interface = PHY_INTERFACE_MODE_RGMII_ID;
+ }
+ break;
+ case PORT_MII_SEL:
+ default:
+ /* Interface is MII */
+ interface = PHY_INTERFACE_MODE_MII;
+ break;
+ }
+
+ return interface;
+}
+
+static void lan937x_config_cpu_port(struct dsa_switch *ds)
+{
+ struct ksz_device *dev = ds->priv;
+ struct ksz_port *p;
+ int i;
+
+ ds->num_ports = dev->port_cnt;
+
+ for (i = 0; i < dev->port_cnt; i++) {
+ if (dsa_is_cpu_port(ds, i) && (dev->cpu_ports & (1 << i))) {
+ phy_interface_t interface;
+
+ dev->cpu_port = i;
+ dev->host_mask = (1 << dev->cpu_port);
+ dev->port_mask |= dev->host_mask;
+ p = &dev->ports[i];
+
+ /* Check if the device tree have specific interface
+ * setting otherwise read & assign from XMII register
+ * for host port interface
+ */
+ interface = lan937x_get_interface(dev, i);
+ if (!p->interface)
+ p->interface = interface;
+
+ dev_info(dev->dev,
+ "Port%d: using phy mode %s\n",
+ i,
+ phy_modes(p->interface));
+
+ /* enable cpu port */
+ lan937x_port_setup(dev, i, true);
+ p->vid_member = dev->port_mask;
+ }
+ }
+
+ dev->member = dev->host_mask;
+
+ for (i = 0; i < dev->port_cnt; i++) {
+ if (i == dev->cpu_port)
+ continue;
+ p = &dev->ports[i];
+
+ /* Initialize to non-zero so that lan937x_cfg_port_member() will
+ * be called.
+ */
+ p->vid_member = (1 << i);
+ p->member = dev->port_mask;
+ lan937x_port_stp_state_set(ds, i, BR_STATE_DISABLED);
+ }
+}
+
+static int lan937x_setup(struct dsa_switch *ds)
+{
+ struct ksz_device *dev = ds->priv;
+ int ret;
+
+ ret = lan937x_reset_switch(dev);
+ if (ret < 0) {
+ dev_err(ds->dev, "failed to reset switch\n");
+ return ret;
+ }
+ /* The VLAN aware is a global setting. Mixed vlan
+ * filterings are not supported.
+ */
+ ds->vlan_filtering_is_global = true;
+
+ /* Configure cpu port*/
+ lan937x_config_cpu_port(ds);
+
+ /* Enable aggressive back off for half duplex & UNH mode*/
+ lan937x_cfg(dev, REG_SW_MAC_CTRL_0,
+ (SW_PAUSE_UNH_MODE | SW_NEW_BACKOFF | SW_AGGR_BACKOFF),
+ true);
+
+ /* If NO_EXC_COLLISION_DROP bit is set, the switch will not drop
+ * packets when 16 or more collisions occur
+ */
+ lan937x_cfg(dev, REG_SW_MAC_CTRL_1, NO_EXC_COLLISION_DROP, true);
+
+ /* Enable reserved multicast table */
+ lan937x_cfg(dev, REG_SW_LUE_CTRL_0, SW_RESV_MCAST_ENABLE, true);
+
+ /* enable global MIB counter freeze function */
+ lan937x_cfg(dev, REG_SW_MAC_CTRL_6, SW_MIB_COUNTER_FREEZE, true);
+
+ /* disable CLK125 & CLK25, 1: disable, 0: enable*/
+ lan937x_cfg(dev, REG_SW_GLOBAL_OUTPUT_CTRL__1,
+ (SW_CLK125_ENB | SW_CLK25_ENB), true);
+
+ lan937x_enable_spi_indirect_access(dev);
+
+ /* start switch */
+ lan937x_cfg(dev, REG_SW_OPERATION, SW_START, true);
+
+ ksz_init_mib_timer(dev);
+
+ return 0;
+}
+
+static int lan937x_change_mtu(struct dsa_switch *ds, int port, int new_mtu)
+{
+ struct ksz_device *dev = ds->priv;
+ int ret;
+
+ new_mtu += VLAN_ETH_HLEN + ETH_FCS_LEN;
+
+ if (dsa_is_cpu_port(ds, port))
+ new_mtu += LAN937X_TAG_LEN;
+
+ if (new_mtu >= FR_MIN_SIZE)
+ ret = lan937x_port_cfg(dev, port, REG_PORT_MAC_CTRL_0,
+ PORT_JUMBO_EN, true);
+ else
+ ret = lan937x_port_cfg(dev, port, REG_PORT_MAC_CTRL_0,
+ PORT_JUMBO_EN, false);
+ if (ret < 0) {
+ dev_err(ds->dev, "failed to enable jumbo\n");
+ return ret;
+ }
+
+ /* Write the frame size in PORT_MAX_FR_SIZE register */
+ ret = lan937x_pwrite16(dev, port, PORT_MAX_FR_SIZE, new_mtu);
+ if (ret < 0) {
+ dev_err(ds->dev, "failed to change the mtu\n");
+ return ret;
+ }
+
+ return 0;
+}
+
+static int lan937x_get_max_mtu(struct dsa_switch *ds, int port)
+{
+ /* Frame max size is 9000 (= 0x2328) if
+ * jumbo frame support is enabled, PORT_JUMBO_EN bit will be enabled
+ * based on mtu in lan937x_change_mtu() API
+ */
+ return (FR_MAX_SIZE - VLAN_ETH_HLEN - ETH_FCS_LEN);
+}
+
+const struct dsa_switch_ops lan937x_switch_ops = {
+ .get_tag_protocol = lan937x_get_tag_protocol,
+ .setup = lan937x_setup,
+ .phy_read = lan937x_phy_read16,
+ .phy_write = lan937x_phy_write16,
+ .port_enable = ksz_enable_port,
+ .port_bridge_join = ksz_port_bridge_join,
+ .port_bridge_leave = ksz_port_bridge_leave,
+ .port_stp_state_set = lan937x_port_stp_state_set,
+ .port_fast_age = ksz_port_fast_age,
+ .port_max_mtu = lan937x_get_max_mtu,
+ .port_change_mtu = lan937x_change_mtu,
+};
+
+int lan937x_switch_register(struct ksz_device *dev)
+{
+ return ksz_switch_register(dev, &lan937x_dev_ops);
+}
+EXPORT_SYMBOL(lan937x_switch_register);
+
+MODULE_AUTHOR("Prasanna Vengateshan Varadharajan <Prasanna.Vengateshan@microchip.com>");
+MODULE_DESCRIPTION("Microchip LAN937x Series Switch DSA Driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/net/dsa/microchip/lan937x_reg.h b/drivers/net/dsa/microchip/lan937x_reg.h
new file mode 100644
index 000000000000..44f55c2f1fc8
--- /dev/null
+++ b/drivers/net/dsa/microchip/lan937x_reg.h
@@ -0,0 +1,677 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Microchip LAN937X switch register definitions
+ * Copyright (C) 2019-2021 Microchip Technology Inc.
+ */
+#ifndef __LAN937X_REG_H
+#define __LAN937X_REG_H
+
+/* 0 - Operation */
+#define REG_CHIP_ID0__1 0x0000
+#define REG_CHIP_ID1__1 0x0001
+#define REG_CHIP_ID2__1 0x0002
+
+#define CHIP_ID_74 0x74
+#define CHIP_ID_73 0x73
+#define CHIP_ID_72 0x72
+#define CHIP_ID_71 0x71
+#define CHIP_ID_70 0x70
+
+#define REG_CHIP_ID3__1 0x0003
+
+#define REG_GLOBAL_CTRL_0 0x0007
+
+#define SW_PHY_REG_BLOCK BIT(7)
+#define SW_FAST_MODE BIT(3)
+#define SW_FAST_MODE_OVERRIDE BIT(2)
+
+#define REG_GLOBAL_OPTIONS 0x000F
+
+#define REG_SW_INT_STATUS__4 0x0010
+#define REG_SW_INT_MASK__4 0x0014
+
+#define LUE_INT BIT(31)
+#define TRIG_TS_INT BIT(30)
+#define APB_TIMEOUT_INT BIT(29)
+#define OVER_TEMP_INT BIT(28)
+#define HSR_INT BIT(27)
+#define PIO_INT BIT(26)
+#define POR_READY_INT BIT(25)
+
+#define SWITCH_INT_MASK \
+ (LUE_INT | TRIG_TS_INT | APB_TIMEOUT_INT | OVER_TEMP_INT | HSR_INT | \
+ PIO_INT | POR_READY_INT)
+
+#define REG_SW_PORT_INT_STATUS__4 0x0018
+#define REG_SW_PORT_INT_MASK__4 0x001C
+
+/* 1 - Global */
+#define REG_SW_GLOBAL_SERIAL_CTRL_0 0x0100
+
+#define SW_LITTLE_ENDIAN BIT(4)
+#define SPI_AUTO_EDGE_DETECTION BIT(1)
+#define SPI_CLOCK_OUT_RISING_EDGE BIT(0)
+
+#define REG_SW_GLOBAL_OUTPUT_CTRL__1 0x0103
+#define SW_CLK125_ENB BIT(1)
+#define SW_CLK25_ENB BIT(0)
+
+/* 2 - PHY */
+#define REG_SW_POWER_MANAGEMENT_CTRL 0x0201
+
+/* 3 - Operation Control */
+#define REG_SW_OPERATION 0x0300
+
+#define SW_DOUBLE_TAG BIT(7)
+#define SW_OVER_TEMP_ENABLE BIT(2)
+#define SW_RESET BIT(1)
+#define SW_START BIT(0)
+
+#define REG_SW_LUE_CTRL_0 0x0310
+#define SW_VLAN_ENABLE BIT(7)
+#define SW_DROP_INVALID_VID BIT(6)
+#define SW_AGE_CNT_M 0x7
+#define SW_AGE_CNT_S 3
+#define SW_RESV_MCAST_ENABLE BIT(2)
+
+#define REG_SW_LUE_CTRL_1 0x0311
+
+#define UNICAST_LEARN_DISABLE BIT(7)
+#define SW_FLUSH_STP_TABLE BIT(5)
+#define SW_FLUSH_MSTP_TABLE BIT(4)
+#define SW_SRC_ADDR_FILTER BIT(3)
+#define SW_AGING_ENABLE BIT(2)
+#define SW_FAST_AGING BIT(1)
+#define SW_LINK_AUTO_AGING BIT(0)
+
+#define REG_SW_LUE_CTRL_2 0x0312
+
+#define SW_MID_RANGE_AGE BIT(7)
+#define SW_LINK_DOWN_FLUSH BIT(6)
+#define SW_EGRESS_VLAN_FILTER_DYN BIT(5)
+#define SW_EGRESS_VLAN_FILTER_STA BIT(4)
+#define SW_FLUSH_OPTION_M 0x3
+#define SW_FLUSH_OPTION_S 2
+#define SW_FLUSH_OPTION_DYN_MAC 1
+#define SW_FLUSH_OPTION_STA_MAC 2
+#define SW_FLUSH_OPTION_BOTH 3
+
+#define REG_SW_LUE_CTRL_3 0x0313
+#define REG_SW_AGE_PERIOD__1 0x0313
+
+#define REG_SW_LUE_INT_STATUS__1 0x0314
+#define REG_SW_LUE_INT_MASK__1 0x0315
+
+#define LEARN_FAIL_INT BIT(2)
+#define WRITE_FAIL_INT BIT(0)
+
+#define LUE_INT_MASK (LEARN_FAIL_INT | WRITE_FAIL_INT)
+
+#define REG_SW_LUE_INDEX_0__2 0x0316
+
+#define ENTRY_INDEX_M 0x0FFF
+
+#define REG_SW_LUE_INDEX_1__2 0x0318
+
+#define FAIL_INDEX_M 0x03FF
+
+#define REG_SW_LUE_INDEX_2__2 0x031A
+
+#define REG_SW_STATIC_AVAIL_ENTRY__4 0x031C
+
+#define SW_INGRESS_FILTERING_NO_LEARN BIT(15)
+#define SW_STATIC_AVAIL_CNT 0x1FF
+
+#define REG_SW_AGE_PERIOD__2 0x0320
+#define SW_AGE_PERIOD_M 0xFFF
+
+#define REG_SW_LUE_UNK_UCAST_CTRL__2 0x0322
+#define REG_SW_LUE_UNK_CTRL_0__4 0x0322
+
+#define SW_UNK_UCAST_ENABLE BIT(15)
+#define SW_UNK_PORTS_M 0xFF
+
+#define REG_SW_LUE_UNK_MCAST_CTRL__2 0x0324
+#define SW_UNK_MCAST_ENABLE BIT(15)
+
+#define REG_SW_LUE_UNK_VID_CTRL__2 0x0326
+#define SW_UNK_VID_ENABLE BIT(15)
+
+#define SW_VLAN_FLUSH_PORTS_M 0xFF
+
+#define REG_SW_STATIC_ENTRY_LIMIT__4 0x032C
+
+#define REG_SW_MAC_CTRL_0 0x0330
+#define SW_NEW_BACKOFF BIT(7)
+#define SW_PAUSE_UNH_MODE BIT(1)
+#define SW_AGGR_BACKOFF BIT(0)
+
+#define REG_SW_MAC_CTRL_1 0x0331
+#define SW_SHORT_IFG BIT(7)
+#define MULTICAST_STORM_DISABLE BIT(6)
+#define SW_BACK_PRESSURE BIT(5)
+#define FAIR_FLOW_CTRL BIT(4)
+#define NO_EXC_COLLISION_DROP BIT(3)
+#define SW_LEGAL_PACKET_DISABLE BIT(1)
+#define SW_PASS_SHORT_FRAME BIT(0)
+
+#define REG_SW_MAC_CTRL_2 0x0332
+#define SW_REPLACE_VID BIT(3)
+#define BROADCAST_STORM_RATE_HI 0x07
+
+#define REG_SW_MAC_CTRL_3 0x0333
+#define BROADCAST_STORM_RATE_LO 0xFF
+#define BR_STORM_RATE 0x07FF
+
+#define REG_SW_MAC_CTRL_4 0x0334
+#define SW_PASS_PAUSE BIT(3)
+
+#define REG_SW_MAC_CTRL_5 0x0335
+#define SW_OUT_RATE_LIMIT_QUEUE_BASED BIT(3)
+
+#define REG_SW_MAC_CTRL_6 0x0336
+#define SW_MIB_COUNTER_FLUSH BIT(7)
+#define SW_MIB_COUNTER_FREEZE BIT(6)
+
+#define REG_SW_MRI_CTRL_0 0x0370
+#define SW_IGMP_SNOOP BIT(6)
+#define SW_IPV6_MLD_OPTION BIT(3)
+#define SW_IPV6_MLD_SNOOP BIT(2)
+#define SW_MIRROR_RX_TX BIT(0)
+
+#define REG_SW_MRI_CTRL_1__4 0x0374
+#define REG_SW_MRI_CTRL_2__4 0x0378
+#define REG_SW_CLASS_D_IP_CTRL__4 0x0374
+
+#define SW_CLASS_D_IP_ENABLE BIT(31)
+
+#define REG_SW_MRI_CTRL_8 0x0378
+#define SW_RED_COLOR_S 4
+#define SW_YELLOW_COLOR_S 2
+#define SW_GREEN_COLOR_S 0
+#define SW_COLOR_M 0x3
+
+#define REG_PTP_EVENT_PRIO_CTRL 0x037C
+#define REG_PTP_GENERAL_PRIO_CTRL 0x037D
+#define PTP_PRIO_ENABLE BIT(7)
+
+#define REG_SW_QM_CTRL__4 0x0390
+#define PRIO_SCHEME_SELECT_M KS_PRIO_M
+#define PRIO_SCHEME_SELECT_S 6
+#define PRIO_MAP_3_HI 0
+#define PRIO_MAP_2_HI 2
+#define PRIO_MAP_0_LO 3
+#define UNICAST_VLAN_BOUNDARY BIT(1)
+
+#define REG_SW_EEE_QM_CTRL__2 0x03C0
+#define REG_SW_EEE_TXQ_WAIT_TIME__2 0x03C2
+
+/* 4 - */
+#define REG_SW_VLAN_ENTRY__4 0x0400
+#define VLAN_VALID BIT(31)
+#define VLAN_FORWARD_OPTION BIT(27)
+#define VLAN_PRIO_M KS_PRIO_M
+#define VLAN_PRIO_S 24
+#define VLAN_MSTP_M 0x7
+#define VLAN_MSTP_S 12
+#define VLAN_FID_M 0x7F
+
+#define REG_SW_VLAN_ENTRY_UNTAG__4 0x0404
+#define REG_SW_VLAN_ENTRY_PORTS__4 0x0408
+#define REG_SW_VLAN_ENTRY_INDEX__2 0x040C
+
+#define VLAN_INDEX_M 0x0FFF
+
+#define REG_SW_VLAN_CTRL 0x040E
+#define VLAN_START BIT(7)
+#define VLAN_ACTION 0x3
+#define VLAN_WRITE 1
+#define VLAN_READ 2
+#define VLAN_CLEAR 3
+
+#define REG_SW_ALU_INDEX_0 0x0410
+#define ALU_FID_INDEX_S 16
+#define ALU_FID_SIZE 127
+#define ALU_MAC_ADDR_HI 0xFFFF
+
+#define REG_SW_ALU_INDEX_1 0x0414
+#define ALU_DIRECT_INDEX_M (BIT(12) - 1)
+
+#define REG_SW_ALU_CTRL__4 0x0418
+#define REG_SW_ALU_CTRL(num) (REG_SW_ALU_CTRL__4 + ((num) * 4))
+
+#define ALU_STA_DYN_CNT 2
+#define ALU_VALID_CNT_M (BIT(14) - 1)
+#define ALU_VALID_CNT_S 16
+#define ALU_START BIT(7)
+#define ALU_VALID BIT(6)
+#define ALU_VALID_OR_STOP BIT(5)
+#define ALU_DIRECT BIT(2)
+#define ALU_ACTION 0x3
+#define ALU_WRITE 1
+#define ALU_READ 2
+#define ALU_SEARCH 3
+
+#define REG_SW_ALU_STAT_CTRL__4 0x041C
+#define ALU_STAT_VALID_CNT_M (BIT(9) - 1)
+#define ALU_STAT_VALID_CNT_S 20
+#define ALU_STAT_INDEX_M (BIT(8) - 1)
+#define ALU_STAT_INDEX_S 8
+#define ALU_RESV_MCAST_INDEX_M (BIT(6) - 1)
+#define ALU_STAT_START BIT(7)
+#define ALU_STAT_VALID BIT(6)
+#define ALU_STAT_VALID_OR_STOP BIT(5)
+#define ALU_STAT_USE_FID BIT(4)
+#define ALU_STAT_DIRECT BIT(3)
+#define ALU_RESV_MCAST_ADDR BIT(2)
+#define ALU_STAT_ACTION 0x3
+#define ALU_STAT_WRITE 1
+#define ALU_STAT_READ 2
+#define ALU_STAT_SEARCH 3
+
+#define REG_SW_ALU_VAL_A 0x0420
+#define ALU_V_STATIC_VALID BIT(31)
+#define ALU_V_SRC_FILTER BIT(30)
+#define ALU_V_DST_FILTER BIT(29)
+#define ALU_V_PRIO_AGE_CNT_M (BIT(3) - 1)
+#define ALU_V_PRIO_AGE_CNT_S 26
+#define ALU_V_MSTP_M 0x7
+
+#define REG_SW_ALU_VAL_B 0x0424
+#define ALU_V_OVERRIDE BIT(31)
+#define ALU_V_USE_FID BIT(30)
+#define ALU_V_PORT_MAP 0xFF
+
+#define REG_SW_ALU_VAL_C 0x0428
+#define ALU_V_FID_M (BIT(16) - 1)
+#define ALU_V_FID_S 16
+#define ALU_V_MAC_ADDR_HI 0xFFFF
+
+#define REG_SW_ALU_VAL_D 0x042C
+
+#define PORT_CTRL_ADDR(port, addr) ((addr) | (((port) + 1) << 12))
+
+#define REG_GLOBAL_RR_INDEX__1 0x0600
+
+/* VPHY */
+#define REG_VPHY_CTRL__2 0x0700
+#define REG_VPHY_STAT__2 0x0704
+#define REG_VPHY_ID_HI__2 0x0708
+#define REG_VPHY_ID_LO__2 0x070C
+#define REG_VPHY_AUTO_NEG__2 0x0710
+#define REG_VPHY_REMOTE_CAP__2 0x0714
+
+#define REG_VPHY_EXPANSION__2 0x0718
+
+#define REG_VPHY_M_CTRL__2 0x0724
+#define REG_VPHY_M_STAT__2 0x0728
+
+#define REG_VPHY_EXT_STAT__2 0x073C
+#define VPHY_EXT_1000_X_FULL BIT(15)
+#define VPHY_EXT_1000_X_HALF BIT(14)
+#define VPHY_EXT_1000_T_FULL BIT(13)
+#define VPHY_EXT_1000_T_HALF BIT(12)
+
+#define REG_VPHY_DEVAD_0__2 0x0740
+#define REG_VPHY_DEVAD_1__2 0x0744
+#define REG_VPHY_DEVAD_2__2 0x0748
+#define REG_VPHY_DEVAD_3__2 0x074C
+
+#define VPHY_DEVAD_UPDATE BIT(7)
+#define VPHY_DEVAD_M 0x1F
+#define VPHY_DEVAD_S 8
+
+#define REG_VPHY_SMI_ADDR__2 0x0750
+#define REG_VPHY_SMI_DATA_LO__2 0x0754
+#define REG_VPHY_SMI_DATA_HI__2 0x0758
+
+#define REG_VPHY_IND_ADDR__2 0x075C
+#define REG_VPHY_IND_DATA__2 0x0760
+#define REG_VPHY_IND_CTRL__2 0x0768
+
+#define VPHY_IND_WRITE BIT(1)
+#define VPHY_IND_BUSY BIT(0)
+
+#define REG_VPHY_SPECIAL_CTRL__2 0x077C
+#define VPHY_SMI_INDIRECT_ENABLE BIT(15)
+#define VPHY_SW_LOOPBACK BIT(14)
+#define VPHY_MDIO_INTERNAL_ENABLE BIT(13)
+#define VPHY_SPI_INDIRECT_ENABLE BIT(12)
+#define VPHY_PORT_MODE_M 0x3
+#define VPHY_PORT_MODE_S 8
+#define VPHY_MODE_RGMII 0
+#define VPHY_MODE_MII_PHY 1
+#define VPHY_MODE_SGMII 2
+#define VPHY_MODE_RMII_PHY 3
+#define VPHY_SW_COLLISION_TEST BIT(7)
+#define VPHY_SPEED_DUPLEX_STAT_M 0x7
+#define VPHY_SPEED_DUPLEX_STAT_S 2
+#define VPHY_SPEED_1000 BIT(4)
+#define VPHY_SPEED_100 BIT(3)
+#define VPHY_FULL_DUPLEX BIT(2)
+
+/* 0 - Operation */
+#define REG_PORT_DEFAULT_VID 0x0000
+
+#define REG_PORT_CUSTOM_VID 0x0002
+#define REG_PORT_PME_STATUS 0x0013
+
+#define REG_PORT_PME_CTRL 0x0017
+#define PME_WOL_MAGICPKT BIT(2)
+#define PME_WOL_LINKUP BIT(1)
+#define PME_WOL_ENERGY BIT(0)
+
+#define REG_PORT_INT_STATUS 0x001B
+#define REG_PORT_INT_MASK 0x001F
+
+#define PORT_TAS_INT BIT(5)
+#define PORT_SGMII_INT BIT(3)
+#define PORT_PTP_INT BIT(2)
+#define PORT_PHY_INT BIT(1)
+#define PORT_ACL_INT BIT(0)
+
+#define PORT_INT_MASK \
+ ( \
+ PORT_TAS_INT | \
+ PORT_SGMII_INT | PORT_PTP_INT | PORT_PHY_INT | PORT_ACL_INT)
+
+#define REG_PORT_CTRL_0 0x0020
+
+#define PORT_MAC_LOOPBACK BIT(7)
+#define PORT_MAC_REMOTE_LOOPBACK BIT(6)
+#define PORT_K2L_INSERT_ENABLE BIT(5)
+#define PORT_K2L_DEBUG_ENABLE BIT(4)
+#define PORT_TAIL_TAG_ENABLE BIT(2)
+#define PORT_QUEUE_SPLIT_ENABLE 0x3
+
+#define REG_PORT_CTRL_1 0x0021
+#define PORT_SRP_ENABLE 0x3
+
+#define REG_PORT_STATUS_0 0x0030
+#define PORT_INTF_SPEED_M 0x3
+#define PORT_INTF_SPEED_S 3
+#define PORT_INTF_FULL_DUPLEX BIT(2)
+
+#define REG_PORT_STATUS_1 0x0034
+
+/* 1 - PHY */
+#define REG_VPHY_SMI_ADDR 0x14
+#define REG_VPHY_SMI_DATA_LO 0x15
+#define REG_VPHY_SMI_DATA_HI 0x16
+
+#define REG_VPHY_SPECIAL_CTRL_STAT 0x1F
+
+#define REG_PORT_T1_PHY_CTRL_BASE 0x0100
+#define REG_PORT_TX_PHY_CTRL_BASE 0x0280
+#define REG_TX_PHY_CTRL_BASE 0x0980
+
+#define REG_PORT_PHY_1000_CTRL 0x0112
+#define PORT_AUTO_NEG_MANUAL BIT(12)
+#define PORT_AUTO_NEG_M BIT(11)
+#define PORT_AUTO_NEG_M_PREFERRED BIT(10)
+#define PORT_AUTO_NEG_1000BT_FD BIT(9)
+#define PORT_AUTO_NEG_1000BT BIT(8)
+
+#define REG_PORT_PHY_1000_STATUS 0x0114
+
+#define REG_PORT_PHY_RXER_COUNTER 0x012A
+#define REG_PORT_PHY_INT_ENABLE 0x0136
+#define REG_PORT_PHY_INT_STATUS 0x0137
+
+/* Same as PORT_PHY_LOOPBACK */
+#define PORT_PHY_PCS_LOOPBACK BIT(0)
+
+#define REG_PORT_PHY_DIGITAL_DEBUG_2 0x013A
+
+#define REG_PORT_PHY_DIGITAL_DEBUG_3 0x013C
+#define PORT_100BT_FIXED_LATENCY BIT(15)
+
+#define REG_PORT_PHY_PHY_CTRL 0x013E
+#define PORT_INT_PIN_HIGH BIT(14)
+#define PORT_ENABLE_JABBER BIT(9)
+#define PORT_STAT_SPEED_1000MBIT BIT(6)
+#define PORT_STAT_SPEED_100MBIT BIT(5)
+#define PORT_STAT_SPEED_10MBIT BIT(4)
+#define PORT_STAT_FULL_DUPLEX BIT(3)
+
+/* Same as PORT_PHY_STAT_M */
+#define PORT_STAT_M BIT(2)
+#define PORT_RESET BIT(1)
+#define PORT_LINK_STATUS_FAIL BIT(0)
+
+/* 3 - xMII */
+#define REG_PORT_XMII_CTRL_0 0x0300
+#define PORT_SGMII_SEL BIT(7)
+#define PORT_MII_FULL_DUPLEX BIT(6)
+#define PORT_MII_TX_FLOW_CTRL BIT(5)
+#define PORT_MII_100MBIT BIT(4)
+#define PORT_MII_RX_FLOW_CTRL BIT(3)
+#define PORT_GRXC_ENABLE BIT(0)
+
+#define REG_PORT_XMII_CTRL_1 0x0301
+#define PORT_MII_NOT_1GBIT BIT(6)
+#define PORT_MII_SEL_EDGE BIT(5)
+#define PORT_RGMII_ID_IG_ENABLE BIT(4)
+#define PORT_RGMII_ID_EG_ENABLE BIT(3)
+#define PORT_MII_MAC_MODE BIT(2)
+#define PORT_MII_SEL_M 0x3
+#define PORT_RGMII_SEL 0x0
+#define PORT_RMII_SEL 0x1
+#define PORT_MII_SEL 0x2
+
+#define REG_PORT_XMII_CTRL_2 0x0302
+#define PORT_RGMII_RX_STS_ENABLE BIT(0)
+
+#define REG_PORT_XMII_CTRL_3 0x0303
+#define PORT_DUPLEX_STATUS_FULL BIT(3)
+
+#define REG_PORT_XMII_CTRL_4 0x0304
+#define REG_PORT_XMII_CTRL_5 0x0305
+
+/* 4 - MAC */
+#define REG_PORT_MAC_CTRL_0 0x0400
+#define PORT_CHECK_LENGTH BIT(2)
+#define PORT_BROADCAST_STORM BIT(1)
+#define PORT_JUMBO_PACKET BIT(0)
+
+#define REG_PORT_MAC_CTRL_1 0x0401
+#define PORT_BACK_PRESSURE BIT(3)
+#define PORT_PASS_ALL BIT(0)
+
+#define REG_PORT_MAC_CTRL_2 0x0402
+#define PORT_100BT_EEE_DISABLE BIT(7)
+#define PORT_1000BT_EEE_DISABLE BIT(6)
+
+#define REG_PORT_MAC_IN_RATE_LIMIT 0x0403
+
+#define REG_PORT_MTU__2 0x0404
+#define PORT_RATE_LIMIT_M (BIT(7) - 1)
+
+/* 5 - MIB Counters */
+#define REG_PORT_MIB_CTRL_STAT 0x0500
+#define MIB_COUNTER_OVERFLOW BIT(31)
+#define MIB_COUNTER_VALID BIT(30)
+#define MIB_COUNTER_READ BIT(25)
+#define MIB_COUNTER_FLUSH_FREEZE BIT(24)
+#define MIB_COUNTER_INDEX_M (BIT(8) - 1)
+#define MIB_COUNTER_INDEX_S 16
+#define MIB_COUNTER_DATA_HI_M 0xF
+
+#define REG_PORT_MIB_DATA 0x0504
+
+/* 8 - Classification and Policing */
+#define REG_PORT_MRI_MIRROR_CTRL 0x0800
+#define PORT_MIRROR_RX BIT(6)
+#define PORT_MIRROR_TX BIT(5)
+#define PORT_MIRROR_SNIFFER BIT(1)
+
+#define REG_PORT_MRI_PRIO_CTRL 0x0801
+#define PORT_HIGHEST_PRIO BIT(7)
+#define PORT_OR_PRIO BIT(6)
+#define PORT_MAC_PRIO_ENABLE BIT(4)
+#define PORT_VLAN_PRIO_ENABLE BIT(3)
+#define PORT_802_1P_PRIO_ENABLE BIT(2)
+#define PORT_DIFFSERV_PRIO_ENABLE BIT(1)
+#define PORT_ACL_PRIO_ENABLE BIT(0)
+
+#define REG_PORT_MRI_MAC_CTRL 0x0802
+#define PORT_USER_PRIO_CEILING BIT(7)
+#define PORT_DROP_NON_VLAN BIT(4)
+#define PORT_DROP_TAG BIT(3)
+#define PORT_BASED_PRIO_M KS_PRIO_M
+#define PORT_BASED_PRIO_S 0
+
+#define REG_PORT_MRI_TC_MAP__4 0x0808
+
+/* 9 - Shaping */
+#define REG_PORT_MTI_QUEUE_INDEX__4 0x0900
+
+#define REG_PORT_MTI_QUEUE_CTRL_0__4 0x0904
+#define MTI_PVID_REPLACE BIT(0)
+
+#define REG_PORT_MTI_QUEUE_CTRL_0 0x0914
+
+/* A - QM */
+#define REG_PORT_QM_CTRL__4 0x0A00
+#define PORT_QM_DROP_PRIO_M 0x3
+
+#define REG_PORT_VLAN_MEMBERSHIP__4 0x0A04
+
+#define REG_PORT_QM_QUEUE_INDEX__4 0x0A08
+#define PORT_QM_QUEUE_INDEX_S 24
+#define PORT_QM_BURST_SIZE_S 16
+#define PORT_QM_MIN_RESV_SPACE_M (BIT(11) - 1)
+
+#define REG_PORT_QM_WATER_MARK__4 0x0A0C
+#define PORT_QM_HI_WATER_MARK_S 16
+#define PORT_QM_LO_WATER_MARK_S 0
+#define PORT_QM_WATER_MARK_M (BIT(11) - 1)
+
+#define REG_PORT_QM_TX_CNT_0__4 0x0A10
+#define PORT_QM_TX_CNT_USED_S 0
+#define PORT_QM_TX_CNT_M (BIT(11) - 1)
+
+#define REG_PORT_QM_TX_CNT_1__4 0x0A14
+#define PORT_QM_TX_CNT_CALCULATED_S 16
+#define PORT_QM_TX_CNT_AVAIL_S 0
+
+/* B - LUE */
+#define REG_PORT_LUE_CTRL 0x0B00
+
+#define PORT_VLAN_LOOKUP_VID_0 BIT(7)
+#define PORT_INGRESS_FILTER BIT(6)
+#define PORT_DISCARD_NON_VID BIT(5)
+#define PORT_MAC_BASED_802_1X BIT(4)
+#define PORT_SRC_ADDR_FILTER BIT(3)
+
+#define REG_PORT_LUE_MSTP_INDEX 0x0B01
+
+#define REG_PORT_LUE_MSTP_STATE 0x0B04
+
+#define PORT_TX_ENABLE BIT(2)
+#define PORT_RX_ENABLE BIT(1)
+#define PORT_LEARN_DISABLE BIT(0)
+
+#define REG_PORT_LUE_LEARN_CNT__2 0x0B08
+
+#define REG_PORT_LUE_UNK_CTL0 0x0B0E
+#define REG_PORT_LUE_UNK_CTL1 0x0B10
+#define REG_PORT_LUE_UNK_VID_CTRL__2 0x0B12
+
+#define PORT_UNK_UCAST_MCAST_ENABLE BIT(15)
+#define PORT_UCAST_MCAST_MASK 0xFF
+#define PORT_UNK_VID_ENABLE BIT(15)
+
+#define PRIO_QUEUES 8
+#define RX_PRIO_QUEUES 8
+#define KS_PRIO_IN_REG 2
+#define TOTAL_PORT_NUM 8
+
+#define LAN937X_COUNTER_NUM 0x20
+#define TOTAL_LAN937X_COUNTER_NUM (LAN937X_COUNTER_NUM + 2 + 2)
+
+#define SWITCH_COUNTER_NUM LAN937X_COUNTER_NUM
+
+#define P_BCAST_STORM_CTRL REG_PORT_MAC_CTRL_0
+#define P_PRIO_CTRL REG_PORT_MRI_PRIO_CTRL
+#define P_MIRROR_CTRL REG_PORT_MRI_MIRROR_CTRL
+#define P_STP_CTRL REG_PORT_LUE_MSTP_STATE
+#define P_PHY_CTRL REG_PORT_PHY_CTRL
+#define P_NEG_RESTART_CTRL REG_PORT_PHY_CTRL
+#define P_LINK_STATUS REG_PORT_PHY_STATUS
+#define P_SPEED_STATUS REG_PORT_PHY_PHY_CTRL
+#define P_RATE_LIMIT_CTRL REG_PORT_MAC_IN_RATE_LIMIT
+
+#define S_LINK_AGING_CTRL REG_SW_LUE_CTRL_1
+#define S_MIRROR_CTRL REG_SW_MRI_CTRL_0
+#define S_REPLACE_VID_CTRL REG_SW_MAC_CTRL_2
+#define S_802_1P_PRIO_CTRL REG_SW_MAC_802_1P_MAP_0
+#define S_TOS_PRIO_CTRL REG_SW_MAC_TOS_PRIO_0
+#define S_FLUSH_TABLE_CTRL REG_SW_LUE_CTRL_1
+
+#define REG_SWITCH_RESET REG_RESET_CTRL
+
+#define SW_FLUSH_DYN_MAC_TABLE SW_FLUSH_MSTP_TABLE
+
+#define MAX_TIMESTAMP_UNIT 2
+#define MAX_TRIG_UNIT 3
+#define MAX_TIMESTAMP_EVENT_UNIT 8
+#define MAX_GPIO 2
+#define MAX_CLOCK 2
+
+#define PTP_TRIG_UNIT_M (BIT(MAX_TRIG_UNIT) - 1)
+#define PTP_TS_UNIT_M (BIT(MAX_TIMESTAMP_UNIT) - 1)
+
+#define TAIL_TAG_PTP BIT(7)
+#define TAIL_TAG_NEXT_CHIP BIT(6)
+#define TAIL_TAG_K2L BIT(5)
+#define TAIL_TAG_PTP_1_STEP BIT(4)
+#define TAIL_TAG_PTP_P2P BIT(3)
+#define TAIL_TAG_RX_PORTS_M 0x7
+
+/* 148,800 frames * 67 ms / 100 */
+#define BR_STORM_VALUE 9969
+
+#define SW_CHECK_LENGTH BIT(3)
+
+#define FR_MIN_SIZE 1522
+#define FR_MAX_SIZE 9000
+
+#define PORT_JUMBO_EN BIT(0)
+#define PORT_FR_CHK_LENGTH BIT(2)
+#define PORT_MAX_FR_SIZE 0x404
+
+#define FR_SIZE_CPU_PORT 1540
+
+#define REG_PORT_CTRL_0 0x0020
+#define PORT_FULL_DUPLEX BIT(6)
+#define PORT_TX_FLOW_CTRL BIT(5)
+#define PORT_RX_FLOW_CTRL BIT(3)
+#define PORT_MAC_SPEED_100 BIT(4)
+
+#define PORT_QUEUE_SPLIT_ENABLE 0x3
+
+/* Get fid from vid, fid 0 is not used if vid is greater than 127 */
+#define LAN937X_GET_FID(vid) (((vid) % ALU_FID_SIZE) + 1)
+
+/* Driver set switch broadcast storm protection at 10% rate. */
+#define BR_STORM_PROT_RATE 10
+
+#define MII_BMSR_100BASE_TX_FD BIT(14)
+
+#define PHY_LINK_UP 1
+#define PHY_LINK_DOWN 0
+
+/*The port number as per the datasheet*/
+#define RGMII_2_PORT_NUM 5
+#define RGMII_1_PORT_NUM 6
+#define SGMII_PORT_NUM 4
+#define TXPHY_PORT_NUM 4
+
+#define GET_CHIP_ID_LSB(chip_id) (((chip_id) >> 8) & 0xff)
+#define LAN937X_RGMII_2_PORT (RGMII_2_PORT_NUM - 1)
+#define LAN937X_RGMII_1_PORT (RGMII_1_PORT_NUM - 1)
+#define LAN937X_SGMII_PORT (SGMII_PORT_NUM - 1)
+#define LAN937X_TXPHY_PORT (TXPHY_PORT_NUM - 1)
+#define LAN937X_TAG_LEN 2
+
+#endif
diff --git a/drivers/net/dsa/microchip/lan937x_spi.c b/drivers/net/dsa/microchip/lan937x_spi.c
new file mode 100644
index 000000000000..7d2541ba54cb
--- /dev/null
+++ b/drivers/net/dsa/microchip/lan937x_spi.c
@@ -0,0 +1,223 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Microchip LAN937X switch driver register access through SPI
+ * Copyright (C) 2019-2021 Microchip Technology Inc.
+ */
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/spi/spi.h>
+#include <linux/of_device.h>
+
+#include "ksz_common.h"
+
+#define SPI_ADDR_SHIFT 24
+#define SPI_ADDR_ALIGN 3
+#define SPI_TURNAROUND_SHIFT 5
+
+KSZ_REGMAP_TABLE(lan937x, 32, SPI_ADDR_SHIFT, SPI_TURNAROUND_SHIFT,
+ SPI_ADDR_ALIGN);
+
+struct lan937x_chip_data {
+ u32 chip_id;
+ const char *dev_name;
+ int num_vlans;
+ int num_alus;
+ int num_statics;
+ int cpu_ports;
+ int port_cnt;
+};
+
+static const struct of_device_id lan937x_dt_ids[];
+
+static const struct lan937x_chip_data lan937x_switch_chips[] = {
+ {
+ .chip_id = 0x00937010,
+ .dev_name = "LAN9370",
+ .num_vlans = 4096,
+ .num_alus = 1024,
+ .num_statics = 256,
+ /* can be configured as cpu port */
+ .cpu_ports = 0x10,
+ /* total port count */
+ .port_cnt = 5,
+ },
+ {
+ .chip_id = 0x00937110,
+ .dev_name = "LAN9371",
+ .num_vlans = 4096,
+ .num_alus = 1024,
+ .num_statics = 256,
+ /* can be configured as cpu port */
+ .cpu_ports = 0x30,
+ /* total port count */
+ .port_cnt = 6,
+ },
+ {
+ .chip_id = 0x00937210,
+ .dev_name = "LAN9372",
+ .num_vlans = 4096,
+ .num_alus = 1024,
+ .num_statics = 256,
+ /* can be configured as cpu port */
+ .cpu_ports = 0x30,
+ /* total port count */
+ .port_cnt = 8,
+ },
+ {
+ .chip_id = 0x00937310,
+ .dev_name = "LAN9373",
+ .num_vlans = 4096,
+ .num_alus = 1024,
+ .num_statics = 256,
+ /* can be configured as cpu port */
+ .cpu_ports = 0x38,
+ /* total port count */
+ .port_cnt = 5,
+ },
+ {
+ .chip_id = 0x00937410,
+ .dev_name = "LAN9374",
+ .num_vlans = 4096,
+ .num_alus = 1024,
+ .num_statics = 256,
+ /* can be configured as cpu port */
+ .cpu_ports = 0x30,
+ /* total port count */
+ .port_cnt = 8,
+ },
+};
+
+static int lan937x_spi_probe(struct spi_device *spi)
+{
+ struct regmap_config rc;
+ struct ksz_device *dev;
+ int i, ret;
+
+ dev = ksz_switch_alloc(&spi->dev, spi);
+ if (!dev)
+ return -ENOMEM;
+
+ for (i = 0; i < ARRAY_SIZE(lan937x_regmap_config); i++) {
+ rc = lan937x_regmap_config[i];
+ rc.lock_arg = &dev->regmap_mutex;
+ dev->regmap[i] = devm_regmap_init_spi(spi, &rc);
+
+ if (IS_ERR(dev->regmap[i])) {
+ ret = PTR_ERR(dev->regmap[i]);
+ dev_err(&spi->dev,
+ "Failed to initialize regmap%i: %d\n",
+ lan937x_regmap_config[i].val_bits, ret);
+ return ret;
+ }
+ }
+
+ if (spi->dev.platform_data)
+ dev->pdata = spi->dev.platform_data;
+
+ ret = lan937x_switch_register(dev);
+ /* Main DSA driver may not be started yet. */
+ if (ret)
+ return ret;
+
+ spi_set_drvdata(spi, dev);
+
+ return 0;
+}
+
+int lan937x_check_device_id(struct ksz_device *dev)
+{
+ const struct lan937x_chip_data *dt_chip_data;
+ const struct of_device_id *match;
+ int i;
+
+ dt_chip_data = of_device_get_match_data(dev->dev);
+
+ if (!dt_chip_data)
+ return -EINVAL;
+
+ for (match = lan937x_dt_ids; match->compatible[0]; match++) {
+ const struct lan937x_chip_data *chip_data = match->data;
+
+ /* Check for chip id */
+ if (chip_data->chip_id != dev->chip_id)
+ continue;
+
+ /* Check for Device Tree and Chip ID */
+ if (dt_chip_data->chip_id != dev->chip_id) {
+ dev_err(dev->dev,
+ "Device tree specifies chip %s but found %s, please fix it!\n",
+ dt_chip_data->dev_name, chip_data->dev_name);
+ return -ENODEV;
+ }
+
+ break;
+ }
+
+ for (i = 0; i < ARRAY_SIZE(lan937x_switch_chips); i++) {
+ const struct lan937x_chip_data *chip = &lan937x_switch_chips[i];
+
+ if (dev->chip_id == chip->chip_id) {
+ dev->name = chip->dev_name;
+ dev->num_vlans = chip->num_vlans;
+ dev->num_alus = chip->num_alus;
+ dev->num_statics = chip->num_statics;
+ dev->port_cnt = chip->port_cnt;
+ dev->cpu_ports = chip->cpu_ports;
+ break;
+ }
+ }
+
+ /* no switch found */
+ if (!dev->port_cnt)
+ return -ENODEV;
+
+ return 0;
+}
+EXPORT_SYMBOL(lan937x_check_device_id);
+
+static int lan937x_spi_remove(struct spi_device *spi)
+{
+ struct ksz_device *dev = spi_get_drvdata(spi);
+
+ if (dev)
+ ksz_switch_remove(dev);
+
+ return 0;
+}
+
+static void lan937x_spi_shutdown(struct spi_device *spi)
+{
+ struct ksz_device *dev = spi_get_drvdata(spi);
+
+ if (dev && dev->dev_ops->shutdown)
+ dev->dev_ops->shutdown(dev);
+}
+
+static const struct of_device_id lan937x_dt_ids[] = {
+ { .compatible = "microchip,lan9370", .data = &lan937x_switch_chips[0] },
+ { .compatible = "microchip,lan9371", .data = &lan937x_switch_chips[1] },
+ { .compatible = "microchip,lan9372", .data = &lan937x_switch_chips[2] },
+ { .compatible = "microchip,lan9373", .data = &lan937x_switch_chips[3] },
+ { .compatible = "microchip,lan9374", .data = &lan937x_switch_chips[4] },
+ {},
+};
+MODULE_DEVICE_TABLE(of, lan937x_dt_ids);
+
+static struct spi_driver lan937x_spi_driver = {
+ .driver = {
+ .name = "lan937x-switch",
+ .owner = THIS_MODULE,
+ .of_match_table = of_match_ptr(lan937x_dt_ids),
+ },
+ .probe = lan937x_spi_probe,
+ .remove = lan937x_spi_remove,
+ .shutdown = lan937x_spi_shutdown,
+};
+
+module_spi_driver(lan937x_spi_driver);
+
+MODULE_ALIAS("spi:lan937x");
+
+MODULE_AUTHOR("Prasanna Vengateshan Varadharajan <Prasanna.Vengateshan@microchip.com>");
+MODULE_DESCRIPTION("Microchip LAN937x Series Switch SPI access Driver");
+MODULE_LICENSE("GPL");
--
2.27.0
^ permalink raw reply related [flat|nested] 44+ messages in thread* Re: [PATCH v3 net-next 05/10] net: dsa: microchip: add DSA support for microchip lan937x
2021-07-23 17:31 ` [PATCH v3 net-next 05/10] net: dsa: microchip: add DSA support for microchip lan937x Prasanna Vengateshan
@ 2021-07-31 15:04 ` Vladimir Oltean
2021-07-31 22:05 ` Andrew Lunn
2021-08-02 10:45 ` Prasanna Vengateshan
0 siblings, 2 replies; 44+ messages in thread
From: Vladimir Oltean @ 2021-07-31 15:04 UTC (permalink / raw)
To: Prasanna Vengateshan
Cc: andrew, netdev, robh+dt, UNGLinuxDriver, Woojung.Huh, hkallweit1,
linux, davem, kuba, linux-kernel, vivien.didelot, f.fainelli,
devicetree
On Fri, Jul 23, 2021 at 11:01:03PM +0530, Prasanna Vengateshan wrote:
> diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
> index 2e6bfd333f50..690d339edd7b 100644
> --- a/drivers/net/dsa/microchip/ksz_common.h
> +++ b/drivers/net/dsa/microchip/ksz_common.h
> @@ -60,6 +60,8 @@ struct ksz_device {
>
> struct gpio_desc *reset_gpio; /* Optional reset GPIO */
>
> + struct device_node *mdio_np;
> +
I don't think you need to hold a reference to the MDIO bus DT node across
the lifetime of the driver, at least other drivers don't, they drop the
reference to it as soon as they finish with of_mdiobus_register and I
don't think they have seen any adverse effects.
> +int lan937x_cfg32(struct ksz_device *dev, u32 addr, u32 bits, bool set)
> +{
> + return regmap_update_bits(dev->regmap[2], addr, bits, set ? bits : 0);
> +}
This function is unused.
> +int lan937x_port_cfg32(struct ksz_device *dev, int port, int offset, u32 bits,
> + bool set)
> +{
> + return regmap_update_bits(dev->regmap[2], PORT_CTRL_ADDR(port, offset),
> + bits, set ? bits : 0);
> +}
Likewise.
> +static void lan937x_r_mib_cnt(struct ksz_device *dev, int port, u16 addr,
> + u64 *cnt)
> +{
> + unsigned int val;
> + u32 data;
> + int ret;
> +
> + /* Enable MIB Counter read*/
You can try to be more careful with the style of the comments, ensure
that there is a space between the last character and the */ marker, here
and everywhere.
> + data = MIB_COUNTER_READ;
> + data |= (addr << MIB_COUNTER_INDEX_S);
> + lan937x_pwrite32(dev, port, REG_PORT_MIB_CTRL_STAT, data);
> +
> + ret = regmap_read_poll_timeout(dev->regmap[2],
> + PORT_CTRL_ADDR(port, REG_PORT_MIB_CTRL_STAT),
> + val, !(val & MIB_COUNTER_READ),
> + 10, 1000);
> + if (ret) {
> + dev_err(dev->dev, "Failed to get MIB\n");
> + return;
> + }
> +
> + /* count resets upon read */
> + lan937x_pread32(dev, port, REG_PORT_MIB_DATA, &data);
> + *cnt += data;
> +}
> +
> +bool lan937x_is_internal_100BTX_phy_port(struct ksz_device *dev, int port)
In another similar driver (sja1110), instead of adding camel case names,
I went for "base_tx" and "base_t1". Maybe that looks slightly better and
more uniform between your "100BTX" vs "t1".
> +void lan937x_mac_config(struct ksz_device *dev, int port,
> + phy_interface_t interface)
> +{
> + u8 data8;
> +
> + lan937x_pread8(dev, port, REG_PORT_XMII_CTRL_1, &data8);
> +
> + /* clear MII selection & set it based on interface later */
> + data8 &= ~PORT_MII_SEL_M;
> +
> + /* configure MAC based on interface */
> + switch (interface) {
> + case PHY_INTERFACE_MODE_MII:
> + lan937x_config_gbit(dev, false, &data8);
> + data8 |= PORT_MII_SEL;
> + break;
> + case PHY_INTERFACE_MODE_RMII:
> + lan937x_config_gbit(dev, false, &data8);
> + data8 |= PORT_RMII_SEL;
> + break;
> + case PHY_INTERFACE_MODE_RGMII:
> + case PHY_INTERFACE_MODE_RGMII_ID:
> + case PHY_INTERFACE_MODE_RGMII_TXID:
> + case PHY_INTERFACE_MODE_RGMII_RXID:
> + lan937x_config_gbit(dev, true, &data8);
> + data8 |= PORT_RGMII_SEL;
> +
> + /* Add RGMII internal delay for cpu port*/
> + if (dsa_is_cpu_port(dev->ds, port)) {
Why only for the CPU port? I would like Andrew/Florian to have a look
here, I guess the assumption is that if the port has a phy-handle, the
RGMII delays should be dealt with by the PHY, but the logic seems to be
"is a CPU port <=> has a phy-handle / isn't a CPU port <=> doesn't have
a phy-handle"? What if it's a fixed-link port connected to a downstream
switch, for which this one is a DSA master?
> + if (interface == PHY_INTERFACE_MODE_RGMII_ID ||
> + interface == PHY_INTERFACE_MODE_RGMII_RXID)
> + data8 |= PORT_RGMII_ID_IG_ENABLE;
> +
> + if (interface == PHY_INTERFACE_MODE_RGMII_ID ||
> + interface == PHY_INTERFACE_MODE_RGMII_TXID)
> + data8 |= PORT_RGMII_ID_EG_ENABLE;
> + }
> + break;
> + default:
> + dev_err(dev->dev, "Unsupported interface '%s' for port %d\n",
> + phy_modes(interface), port);
> + return;
> + }
> +
> + /* Write the updated value */
> + lan937x_pwrite8(dev, port, REG_PORT_XMII_CTRL_1, data8);
> +}
> +static int lan937x_mdio_register(struct dsa_switch *ds)
> +{
> + struct ksz_device *dev = ds->priv;
> + int ret;
> +
> + dev->mdio_np = of_get_child_by_name(ds->dev->of_node, "mdio");
So you support both the cases where an internal PHY is described using
OF bindings, and where the internal PHY is implicitly accessed using the
slave_mii_bus of the switch, at a PHY address equal to the port number,
and with no phy-handle or fixed-link device tree property for that port?
Do you need both alternatives? The first is already more flexible than
the second.
> +static void lan937x_port_stp_state_set(struct dsa_switch *ds, int port,
> + u8 state)
> +{
> + struct ksz_device *dev = ds->priv;
> + struct ksz_port *p = &dev->ports[port];
> + int forward = dev->member;
> + int member = -1;
> + u8 data;
> +
> + lan937x_pread8(dev, port, P_STP_CTRL, &data);
> + data &= ~(PORT_TX_ENABLE | PORT_RX_ENABLE | PORT_LEARN_DISABLE);
> +
> + switch (state) {
> + case BR_STATE_DISABLED:
> + data |= PORT_LEARN_DISABLE;
> + break;
> + case BR_STATE_LISTENING:
> + data |= (PORT_RX_ENABLE | PORT_LEARN_DISABLE);
> + if (p->stp_state == BR_STATE_DISABLED)
> + member = dev->host_mask | p->vid_member;
> + break;
> + case BR_STATE_LEARNING:
> + data |= PORT_RX_ENABLE;
> + break;
> + case BR_STATE_FORWARDING:
> + data |= (PORT_TX_ENABLE | PORT_RX_ENABLE);
> +
> + member = dev->host_mask | p->vid_member;
> + mutex_lock(&dev->dev_mutex);
I am not convinced that the dev_mutex brings any value in any of the ksz
drivers that use it.
> +
> + /* Port is a member of a bridge. */
> + if (dev->br_member & (1 << port)) {
> + dev->member |= (1 << port);
> + member = dev->member;
> + }
> + mutex_unlock(&dev->dev_mutex);
> + break;
> + case BR_STATE_BLOCKING:
> + data |= PORT_LEARN_DISABLE;
> + if (p->stp_state == BR_STATE_DISABLED)
> + member = dev->host_mask | p->vid_member;
> + break;
> + default:
> + dev_err(ds->dev, "invalid STP state: %d\n", state);
> + return;
> + }
> +
> + lan937x_pwrite8(dev, port, P_STP_CTRL, data);
> +
> + p->stp_state = state;
> + mutex_lock(&dev->dev_mutex);
> +
> + /* Port membership may share register with STP state. */
> + if (member >= 0 && member != p->member)
> + lan937x_cfg_port_member(dev, port, (u8)member);
> +
> + /* Check if forwarding needs to be updated. */
> + if (state != BR_STATE_FORWARDING) {
> + if (dev->br_member & (1 << port))
> + dev->member &= ~(1 << port);
> + }
> +
> + /* When topology has changed the function ksz_update_port_member
> + * should be called to modify port forwarding behavior.
> + */
> + if (forward != dev->member)
> + ksz_update_port_member(dev, port);
> +
> + mutex_unlock(&dev->dev_mutex);
> +}
^ permalink raw reply [flat|nested] 44+ messages in thread* Re: [PATCH v3 net-next 05/10] net: dsa: microchip: add DSA support for microchip lan937x
2021-07-31 15:04 ` Vladimir Oltean
@ 2021-07-31 22:05 ` Andrew Lunn
2021-08-02 21:33 ` Vladimir Oltean
2021-08-02 10:45 ` Prasanna Vengateshan
1 sibling, 1 reply; 44+ messages in thread
From: Andrew Lunn @ 2021-07-31 22:05 UTC (permalink / raw)
To: Vladimir Oltean
Cc: Prasanna Vengateshan, netdev, robh+dt, UNGLinuxDriver,
Woojung.Huh, hkallweit1, linux, davem, kuba, linux-kernel,
vivien.didelot, f.fainelli, devicetree
> > +void lan937x_mac_config(struct ksz_device *dev, int port,
> > + phy_interface_t interface)
> > +{
> > + u8 data8;
> > +
> > + lan937x_pread8(dev, port, REG_PORT_XMII_CTRL_1, &data8);
> > +
> > + /* clear MII selection & set it based on interface later */
> > + data8 &= ~PORT_MII_SEL_M;
> > +
> > + /* configure MAC based on interface */
> > + switch (interface) {
> > + case PHY_INTERFACE_MODE_MII:
> > + lan937x_config_gbit(dev, false, &data8);
> > + data8 |= PORT_MII_SEL;
> > + break;
> > + case PHY_INTERFACE_MODE_RMII:
> > + lan937x_config_gbit(dev, false, &data8);
> > + data8 |= PORT_RMII_SEL;
> > + break;
> > + case PHY_INTERFACE_MODE_RGMII:
> > + case PHY_INTERFACE_MODE_RGMII_ID:
> > + case PHY_INTERFACE_MODE_RGMII_TXID:
> > + case PHY_INTERFACE_MODE_RGMII_RXID:
> > + lan937x_config_gbit(dev, true, &data8);
> > + data8 |= PORT_RGMII_SEL;
> > +
> > + /* Add RGMII internal delay for cpu port*/
> > + if (dsa_is_cpu_port(dev->ds, port)) {
>
> Why only for the CPU port? I would like Andrew/Florian to have a look
> here, I guess the assumption is that if the port has a phy-handle, the
> RGMII delays should be dealt with by the PHY, but the logic seems to be
> "is a CPU port <=> has a phy-handle / isn't a CPU port <=> doesn't have
> a phy-handle"? What if it's a fixed-link port connected to a downstream
> switch, for which this one is a DSA master?
The marvell driver applies delays unconditionally. And as far as i
remember, it is only used in the use case you suggest, a DSA link,
which is using RGMII. For marvell switches, that is pretty unusual,
most boards use 1000BaseX or higher SERDES speeds for links between
switches.
I'm not sure if we have the case of an external PHY using RGMII. I
suspect it might actually be broken, because i think both the MAC and
the PHY might add the same delay. For phylib in general, if the MAC
applies the delays, it needs to manipulate the value passed to the PHY
so it also does not add delays. And i'm not sure DSA does that.
So limiting RGMII delays to only the CPU port is not
unreasonable. However, i suspect you are correct about chained
switches not working.
We might need to look at this at a higher level, when the PHY is
connected to the MAC and what mode gets passed to it.
> > + if (interface == PHY_INTERFACE_MODE_RGMII_ID ||
> > + interface == PHY_INTERFACE_MODE_RGMII_RXID)
> > + data8 |= PORT_RGMII_ID_IG_ENABLE;
> > +
> > + if (interface == PHY_INTERFACE_MODE_RGMII_ID ||
> > + interface == PHY_INTERFACE_MODE_RGMII_TXID)
> > + data8 |= PORT_RGMII_ID_EG_ENABLE;
> > + }
> > + break;
> > + default:
> > + dev_err(dev->dev, "Unsupported interface '%s' for port %d\n",
> > + phy_modes(interface), port);
> > + return;
> > + }
> > +
> > + /* Write the updated value */
> > + lan937x_pwrite8(dev, port, REG_PORT_XMII_CTRL_1, data8);
> > +}
>
> > +static int lan937x_mdio_register(struct dsa_switch *ds)
> > +{
> > + struct ksz_device *dev = ds->priv;
> > + int ret;
> > +
> > + dev->mdio_np = of_get_child_by_name(ds->dev->of_node, "mdio");
>
> So you support both the cases where an internal PHY is described using
> OF bindings, and where the internal PHY is implicitly accessed using the
> slave_mii_bus of the switch, at a PHY address equal to the port number,
> and with no phy-handle or fixed-link device tree property for that port?
>
> Do you need both alternatives? The first is already more flexible than
> the second.
The first is also much more verbose in DT, and the second generally
just works without any DT. What can be tricky with the second is
getting PHY interrupts to work, but it is possible, the mv88e6xxx does
it.
Andrew
^ permalink raw reply [flat|nested] 44+ messages in thread* Re: [PATCH v3 net-next 05/10] net: dsa: microchip: add DSA support for microchip lan937x
2021-07-31 22:05 ` Andrew Lunn
@ 2021-08-02 21:33 ` Vladimir Oltean
2021-08-03 14:43 ` Andrew Lunn
0 siblings, 1 reply; 44+ messages in thread
From: Vladimir Oltean @ 2021-08-02 21:33 UTC (permalink / raw)
To: Andrew Lunn
Cc: Prasanna Vengateshan, netdev, robh+dt, UNGLinuxDriver,
Woojung.Huh, hkallweit1, linux, davem, kuba, linux-kernel,
vivien.didelot, f.fainelli, devicetree
On Sun, Aug 01, 2021 at 12:05:16AM +0200, Andrew Lunn wrote:
> > So you support both the cases where an internal PHY is described using
> > OF bindings, and where the internal PHY is implicitly accessed using the
> > slave_mii_bus of the switch, at a PHY address equal to the port number,
> > and with no phy-handle or fixed-link device tree property for that port?
> >
> > Do you need both alternatives? The first is already more flexible than
> > the second.
>
> The first is also much more verbose in DT, and the second generally
> just works without any DT. What can be tricky with the second is
> getting PHY interrupts to work, but it is possible, the mv88e6xxx does
> it.
- The explicit phy-handle is more verbose as far as the DT description
goes for one particular use case of indirect internal PHY access, but
it also leads to less complex code (more uniform with other usage
patterns in the kernel). What is tricky with an implicit phy-handle is
trivial without it. This makes a difference with DM_DSA in U-Boot,
where I would really like to avoid bloating the code and just support
2 options for a DSA switch port: either a phy-handle or a fixed-link.
These two are already "Turing-complete" (they can describe any system)
so I only see the implicit phy-handle as a helping hand for a few lazy
DT writers. Since I have been pushing back that we shouldn't bloat
U-Boot with implicit phy-handle logic when it doesn't give a concrete
benefit, and have gotten a push back in return that Linux does allow
it and it would be desirable for one DT binding to cover all, I now
need to promote the more generic approach for Linux going forward too.
- If the switch has the ability for its internal PHYs to be accessed
directly over MDIO pins instead of using indirect SPI transfers, using
a phy-handle is a generic way to handle both cases, while the implicit
phy-handle does not give you that option.
- If there is complex pinmuxing in the SoC and one port can either be
connected to an internal 100base-T1 or to a 100base-TX PHY, and this
is not detectable by software, this cannot be dealt with using an
implicit phy-handle if the 100base-T1 and 100base-TX PHYs are not at
the same address.
- In general, if the internal PHYs are not at an MDIO address equal to
the port number, this cannot be dealt with using the implicit
phy-handle method.
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v3 net-next 05/10] net: dsa: microchip: add DSA support for microchip lan937x
2021-08-02 21:33 ` Vladimir Oltean
@ 2021-08-03 14:43 ` Andrew Lunn
2021-08-03 15:05 ` Vladimir Oltean
0 siblings, 1 reply; 44+ messages in thread
From: Andrew Lunn @ 2021-08-03 14:43 UTC (permalink / raw)
To: Vladimir Oltean
Cc: Prasanna Vengateshan, netdev, robh+dt, UNGLinuxDriver,
Woojung.Huh, hkallweit1, linux, davem, kuba, linux-kernel,
vivien.didelot, f.fainelli, devicetree
On Tue, Aug 03, 2021 at 12:33:53AM +0300, Vladimir Oltean wrote:
> On Sun, Aug 01, 2021 at 12:05:16AM +0200, Andrew Lunn wrote:
> > > So you support both the cases where an internal PHY is described using
> > > OF bindings, and where the internal PHY is implicitly accessed using the
> > > slave_mii_bus of the switch, at a PHY address equal to the port number,
> > > and with no phy-handle or fixed-link device tree property for that port?
> > >
> > > Do you need both alternatives? The first is already more flexible than
> > > the second.
> >
> > The first is also much more verbose in DT, and the second generally
> > just works without any DT. What can be tricky with the second is
> > getting PHY interrupts to work, but it is possible, the mv88e6xxx does
> > it.
>
> - The explicit phy-handle is more verbose as far as the DT description
> goes for one particular use case of indirect internal PHY access, but
> it also leads to less complex code (more uniform with other usage
> patterns in the kernel). What is tricky with an implicit phy-handle is
> trivial without it. This makes a difference with DM_DSA in U-Boot,
> where I would really like to avoid bloating the code and just support
> 2 options for a DSA switch port: either a phy-handle or a fixed-link.
> These two are already "Turing-complete" (they can describe any system)
> so I only see the implicit phy-handle as a helping hand for a few lazy
> DT writers. Since I have been pushing back that we shouldn't bloat
> U-Boot with implicit phy-handle logic when it doesn't give a concrete
> benefit, and have gotten a push back in return that Linux does allow
> it and it would be desirable for one DT binding to cover all, I now
> need to promote the more generic approach for Linux going forward too.
>
> - If the switch has the ability for its internal PHYs to be accessed
> directly over MDIO pins instead of using indirect SPI transfers, using
> a phy-handle is a generic way to handle both cases, while the implicit
> phy-handle does not give you that option.
>
> - If there is complex pinmuxing in the SoC and one port can either be
> connected to an internal 100base-T1 or to a 100base-TX PHY, and this
> is not detectable by software, this cannot be dealt with using an
> implicit phy-handle if the 100base-T1 and 100base-TX PHYs are not at
> the same address.
>
> - In general, if the internal PHYs are not at an MDIO address equal to
> the port number, this cannot be dealt with using the implicit
> phy-handle method.
There are good reasons to use an explicit phy-handle, and i would
never block such code. However, implicit is historically how it was
done. There are many DT blobs which assume it works. So implicit is
not going away.
If you want to only support explicit in U-Boot, that is fine. I would
suggest making this clear in the U-Boot documentation.
Andrew
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v3 net-next 05/10] net: dsa: microchip: add DSA support for microchip lan937x
2021-08-03 14:43 ` Andrew Lunn
@ 2021-08-03 15:05 ` Vladimir Oltean
0 siblings, 0 replies; 44+ messages in thread
From: Vladimir Oltean @ 2021-08-03 15:05 UTC (permalink / raw)
To: Andrew Lunn
Cc: Prasanna Vengateshan, netdev, robh+dt, UNGLinuxDriver,
Woojung.Huh, hkallweit1, linux, davem, kuba, linux-kernel,
vivien.didelot, f.fainelli, devicetree
On Tue, Aug 03, 2021 at 04:43:12PM +0200, Andrew Lunn wrote:
> There are good reasons to use an explicit phy-handle, and i would
> never block such code. However, implicit is historically how it was
> done. There are many DT blobs which assume it works. So implicit is
> not going away.
>
> If you want to only support explicit in U-Boot, that is fine. I would
> suggest making this clear in the U-Boot documentation.
I am happy that Prasanna made it possible for OF-based descriptions of
the internal PHYs to be written for the lan937x generation. I did take a
look at the bindings that Prasanna proposed and I think they would work
with what DM_DSA can parse too. The work that Tim Harvey did was for
ksz9897, and it is slightly different: the MDIO controller node has a
compatible string of "microchip,ksz-mdio", and a parent container node
of "mdios".
https://source.denx.de/u-boot/u-boot/-/blob/master/arch/arm/dts/imx8mm-venice-gw7901.dts#L634
However, since the lan937x would probably have a different driver even
in U-Boot, 100% binding consistency between lan937x and ksz9897 is
probably not necessary, since some of that can boil down to driver
author choice too. As long as an OF based choice is available I'm
absolutely fine.
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v3 net-next 05/10] net: dsa: microchip: add DSA support for microchip lan937x
2021-07-31 15:04 ` Vladimir Oltean
2021-07-31 22:05 ` Andrew Lunn
@ 2021-08-02 10:45 ` Prasanna Vengateshan
2021-08-02 12:15 ` Vladimir Oltean
1 sibling, 1 reply; 44+ messages in thread
From: Prasanna Vengateshan @ 2021-08-02 10:45 UTC (permalink / raw)
To: Vladimir Oltean
Cc: andrew, netdev, robh+dt, UNGLinuxDriver, Woojung.Huh, hkallweit1,
linux, davem, kuba, linux-kernel, vivien.didelot, f.fainelli,
devicetree
On Sat, 2021-07-31 at 18:04 +0300, Vladimir Oltean wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
> >
> > +void lan937x_mac_config(struct ksz_device *dev, int port,
> > + phy_interface_t interface)
> > +{
> > + u8 data8;
> > +
> > + lan937x_pread8(dev, port, REG_PORT_XMII_CTRL_1, &data8);
> > +
> > + /* clear MII selection & set it based on interface later */
> > + data8 &= ~PORT_MII_SEL_M;
> > +
> > + /* configure MAC based on interface */
> > + switch (interface) {
> > + case PHY_INTERFACE_MODE_MII:
> > + lan937x_config_gbit(dev, false, &data8);
> > + data8 |= PORT_MII_SEL;
> > + break;
> > + case PHY_INTERFACE_MODE_RMII:
> > + lan937x_config_gbit(dev, false, &data8);
> > + data8 |= PORT_RMII_SEL;
> > + break;
> > + case PHY_INTERFACE_MODE_RGMII:
> > + case PHY_INTERFACE_MODE_RGMII_ID:
> > + case PHY_INTERFACE_MODE_RGMII_TXID:
> > + case PHY_INTERFACE_MODE_RGMII_RXID:
> > + lan937x_config_gbit(dev, true, &data8);
> > + data8 |= PORT_RGMII_SEL;
> > +
> > + /* Add RGMII internal delay for cpu port*/
> > + if (dsa_is_cpu_port(dev->ds, port)) {
>
> Why only for the CPU port? I would like Andrew/Florian to have a look
> here, I guess the assumption is that if the port has a phy-handle, the
> RGMII delays should be dealt with by the PHY, but the logic seems to be
> "is a CPU port <=> has a phy-handle / isn't a CPU port <=> doesn't have
> a phy-handle"? What if it's a fixed-link port connected to a downstream
> switch, for which this one is a DSA master?
> >
Thanks for reviewing the patches. My earlier proposal here was to check if there
is no phydev (dp->slave->phydev) or if PHY is genphy, then apply RGMII delays
assuming delays should be dealt with the phy driver if available. What do you
think of that?
^ permalink raw reply [flat|nested] 44+ messages in thread* Re: [PATCH v3 net-next 05/10] net: dsa: microchip: add DSA support for microchip lan937x
2021-08-02 10:45 ` Prasanna Vengateshan
@ 2021-08-02 12:15 ` Vladimir Oltean
2021-08-02 13:13 ` Andrew Lunn
0 siblings, 1 reply; 44+ messages in thread
From: Vladimir Oltean @ 2021-08-02 12:15 UTC (permalink / raw)
To: Prasanna Vengateshan
Cc: andrew, netdev, robh+dt, UNGLinuxDriver, Woojung.Huh, hkallweit1,
linux, davem, kuba, linux-kernel, vivien.didelot, f.fainelli,
devicetree
On Mon, Aug 02, 2021 at 04:15:08PM +0530, Prasanna Vengateshan wrote:
> On Sat, 2021-07-31 at 18:04 +0300, Vladimir Oltean wrote:
> > > +void lan937x_mac_config(struct ksz_device *dev, int port,
> > > + phy_interface_t interface)
> > > +{
> > > + u8 data8;
> > > +
> > > + lan937x_pread8(dev, port, REG_PORT_XMII_CTRL_1, &data8);
> > > +
> > > + /* clear MII selection & set it based on interface later */
> > > + data8 &= ~PORT_MII_SEL_M;
> > > +
> > > + /* configure MAC based on interface */
> > > + switch (interface) {
> > > + case PHY_INTERFACE_MODE_MII:
> > > + lan937x_config_gbit(dev, false, &data8);
> > > + data8 |= PORT_MII_SEL;
> > > + break;
> > > + case PHY_INTERFACE_MODE_RMII:
> > > + lan937x_config_gbit(dev, false, &data8);
> > > + data8 |= PORT_RMII_SEL;
> > > + break;
> > > + case PHY_INTERFACE_MODE_RGMII:
> > > + case PHY_INTERFACE_MODE_RGMII_ID:
> > > + case PHY_INTERFACE_MODE_RGMII_TXID:
> > > + case PHY_INTERFACE_MODE_RGMII_RXID:
> > > + lan937x_config_gbit(dev, true, &data8);
> > > + data8 |= PORT_RGMII_SEL;
> > > +
> > > + /* Add RGMII internal delay for cpu port*/
> > > + if (dsa_is_cpu_port(dev->ds, port)) {
> >
> > Why only for the CPU port? I would like Andrew/Florian to have a look
> > here, I guess the assumption is that if the port has a phy-handle, the
> > RGMII delays should be dealt with by the PHY, but the logic seems to be
> > "is a CPU port <=> has a phy-handle / isn't a CPU port <=> doesn't have
> > a phy-handle"? What if it's a fixed-link port connected to a downstream
> > switch, for which this one is a DSA master?
>
> Thanks for reviewing the patches. My earlier proposal here was to check if there
> is no phydev (dp->slave->phydev) or if PHY is genphy, then apply RGMII delays
> assuming delays should be dealt with the phy driver if available. What do you
> think of that?
I don't know what to suggest, this is a bit of a minefield.
A while ago and in a different thread, Russell King said that
PHY_INTERFACE_MODE_RGMII_TXID means that the MAC should behave as if it
is connected to a PHY which has applied RGMII delays in the TX direction,
regardless of whether it is in fixed-link or not. So if the MAC was to
add any internal delays in PHY_INTERFACE_MODE_RGMII_TXID mode, those
would have to be RX delays, because the phy-mode specifies what was
already added and nothing more.
However, that is yet another problem. "what is already added" does not
mean "what more needs to be added". The fact that internal delays were
added in the TX direction doesn't necessarily mean that they still need
to be added in the RX direction. If you have a PHY which can only delay
the clock that it drives (the RX clock), and this is connected to a MAC
that cannot add any delays of its own, then you would end up adding PCB
traces on the board in the TX direction. But if you specify the phy-mode
as "rgmii-rxid", the MAC driver would complain that it can't add delays
in the TX direction (under the assumption that these are needed to work),
and if you specify the phy-mode as "rgmii-id", the PHY driver would
complain that it can't add delays in the TX direction.
So you'd have a system that works from a hardware PoV, but you wouldn't
have any way to describe it to Linux, which sucks.
I think the only way to do things correctly today and have a way to
describe any possible hardware setup is to parse the explicit
rx-internal-delay-ps and tx-internal-delay-ps properties in the MAC
driver, as defined in Documentation/bindings/net/ethernet-controller.yaml.
Then only let the MAC add the internal delays if the device tree has
added those properties, leave nothing down to assumptions.
^ permalink raw reply [flat|nested] 44+ messages in thread* Re: [PATCH v3 net-next 05/10] net: dsa: microchip: add DSA support for microchip lan937x
2021-08-02 12:15 ` Vladimir Oltean
@ 2021-08-02 13:13 ` Andrew Lunn
2021-08-02 13:59 ` Vladimir Oltean
0 siblings, 1 reply; 44+ messages in thread
From: Andrew Lunn @ 2021-08-02 13:13 UTC (permalink / raw)
To: Vladimir Oltean
Cc: Prasanna Vengateshan, netdev, robh+dt, UNGLinuxDriver,
Woojung.Huh, hkallweit1, linux, davem, kuba, linux-kernel,
vivien.didelot, f.fainelli, devicetree
On Mon, Aug 02, 2021 at 03:15:50PM +0300, Vladimir Oltean wrote:
> On Mon, Aug 02, 2021 at 04:15:08PM +0530, Prasanna Vengateshan wrote:
> > On Sat, 2021-07-31 at 18:04 +0300, Vladimir Oltean wrote:
> > > > +void lan937x_mac_config(struct ksz_device *dev, int port,
> > > > + phy_interface_t interface)
> > > > +{
> > > > + u8 data8;
> > > > +
> > > > + lan937x_pread8(dev, port, REG_PORT_XMII_CTRL_1, &data8);
> > > > +
> > > > + /* clear MII selection & set it based on interface later */
> > > > + data8 &= ~PORT_MII_SEL_M;
> > > > +
> > > > + /* configure MAC based on interface */
> > > > + switch (interface) {
> > > > + case PHY_INTERFACE_MODE_MII:
> > > > + lan937x_config_gbit(dev, false, &data8);
> > > > + data8 |= PORT_MII_SEL;
> > > > + break;
> > > > + case PHY_INTERFACE_MODE_RMII:
> > > > + lan937x_config_gbit(dev, false, &data8);
> > > > + data8 |= PORT_RMII_SEL;
> > > > + break;
> > > > + case PHY_INTERFACE_MODE_RGMII:
> > > > + case PHY_INTERFACE_MODE_RGMII_ID:
> > > > + case PHY_INTERFACE_MODE_RGMII_TXID:
> > > > + case PHY_INTERFACE_MODE_RGMII_RXID:
> > > > + lan937x_config_gbit(dev, true, &data8);
> > > > + data8 |= PORT_RGMII_SEL;
> > > > +
> > > > + /* Add RGMII internal delay for cpu port*/
> > > > + if (dsa_is_cpu_port(dev->ds, port)) {
> > >
> > > Why only for the CPU port? I would like Andrew/Florian to have a look
> > > here, I guess the assumption is that if the port has a phy-handle, the
> > > RGMII delays should be dealt with by the PHY, but the logic seems to be
> > > "is a CPU port <=> has a phy-handle / isn't a CPU port <=> doesn't have
> > > a phy-handle"? What if it's a fixed-link port connected to a downstream
> > > switch, for which this one is a DSA master?
> >
> > Thanks for reviewing the patches. My earlier proposal here was to check if there
> > is no phydev (dp->slave->phydev) or if PHY is genphy, then apply RGMII delays
> > assuming delays should be dealt with the phy driver if available. What do you
> > think of that?
>
> I don't know what to suggest, this is a bit of a minefield.
In general, the MAC does nothing, and passes the value to the PHY. The
PHY inserts delays as requested. To address Vladimir point,
PHY_INTERFACE_MODE_RGMII_TXID would mean the PHY adds delay in the TX
direction, and assumes the RX delay comes from somewhere else,
probably the PCB.
I only recommend the MAC adds delays when the PHY cannot, or there is
no PHY, e.g. SoC to switch, or switch to switch link. There are a few
MAC drivers that do add delays, mostly because that is how the vendor
crap tree does it.
So as i said, what you propose is O.K, it follows this general rule of
thumb.
Andrew
^ permalink raw reply [flat|nested] 44+ messages in thread* Re: [PATCH v3 net-next 05/10] net: dsa: microchip: add DSA support for microchip lan937x
2021-08-02 13:13 ` Andrew Lunn
@ 2021-08-02 13:59 ` Vladimir Oltean
2021-08-02 20:47 ` Andrew Lunn
2021-08-03 16:54 ` Prasanna Vengateshan
0 siblings, 2 replies; 44+ messages in thread
From: Vladimir Oltean @ 2021-08-02 13:59 UTC (permalink / raw)
To: Andrew Lunn
Cc: Prasanna Vengateshan, netdev, robh+dt, UNGLinuxDriver,
Woojung.Huh, hkallweit1, linux, davem, kuba, linux-kernel,
vivien.didelot, f.fainelli, devicetree
On Mon, Aug 02, 2021 at 03:13:01PM +0200, Andrew Lunn wrote:
> In general, the MAC does nothing, and passes the value to the PHY. The
> PHY inserts delays as requested. To address Vladimir point,
> PHY_INTERFACE_MODE_RGMII_TXID would mean the PHY adds delay in the TX
> direction, and assumes the RX delay comes from somewhere else,
> probably the PCB.
For the PHY, that is the only portion where things are clear.
> I only recommend the MAC adds delays when the PHY cannot, or there is
> no PHY, e.g. SoC to switch, or switch to switch link. There are a few
> MAC drivers that do add delays, mostly because that is how the vendor
> crap tree does it.
>
> So as i said, what you propose is O.K, it follows this general rule of
> thumb.
The "rule of thumb" for a MAC driver is actually applied in reverse by
most MAC drivers compared to what Russell described should be happening.
For example, mv88e6xxx_port_set_rgmii_delay():
switch (mode) {
case PHY_INTERFACE_MODE_RGMII_RXID:
reg |= MV88E6XXX_PORT_MAC_CTL_RGMII_DELAY_RXCLK;
The mv88e6xxx is a MAC, so when it has a phy-mode = "rgmii-rxid", it
should assume it is connected to a link partner (PHY or otherwise) that
has applied the RXCLK delay already. So it should only be concerned with
the TXCLK delay. That is my point. I am just trying to lay out the
points to Prasanna that would make a sane system going forward. I am not
sure that we actually have an in-tree driver that is sane in that
regard.
That discussion, and Russell's point, was here, btw:
https://patchwork.ozlabs.org/project/netdev/patch/20200616074955.GA9092@laureti-dev/#2461123
^ permalink raw reply [flat|nested] 44+ messages in thread* Re: [PATCH v3 net-next 05/10] net: dsa: microchip: add DSA support for microchip lan937x
2021-08-02 13:59 ` Vladimir Oltean
@ 2021-08-02 20:47 ` Andrew Lunn
2021-08-03 16:54 ` Prasanna Vengateshan
1 sibling, 0 replies; 44+ messages in thread
From: Andrew Lunn @ 2021-08-02 20:47 UTC (permalink / raw)
To: Vladimir Oltean
Cc: Prasanna Vengateshan, netdev, robh+dt, UNGLinuxDriver,
Woojung.Huh, hkallweit1, linux, davem, kuba, linux-kernel,
vivien.didelot, f.fainelli, devicetree
On Mon, Aug 02, 2021 at 04:59:11PM +0300, Vladimir Oltean wrote:
> On Mon, Aug 02, 2021 at 03:13:01PM +0200, Andrew Lunn wrote:
> > In general, the MAC does nothing, and passes the value to the PHY. The
> > PHY inserts delays as requested. To address Vladimir point,
> > PHY_INTERFACE_MODE_RGMII_TXID would mean the PHY adds delay in the TX
> > direction, and assumes the RX delay comes from somewhere else,
> > probably the PCB.
>
> For the PHY, that is the only portion where things are clear.
>
> > I only recommend the MAC adds delays when the PHY cannot, or there is
> > no PHY, e.g. SoC to switch, or switch to switch link. There are a few
> > MAC drivers that do add delays, mostly because that is how the vendor
> > crap tree does it.
> >
> > So as i said, what you propose is O.K, it follows this general rule of
> > thumb.
>
> The "rule of thumb" for a MAC driver is actually applied in reverse by
> most MAC drivers compared to what Russell described should be happening.
> For example, mv88e6xxx_port_set_rgmii_delay():
>
> switch (mode) {
> case PHY_INTERFACE_MODE_RGMII_RXID:
> reg |= MV88E6XXX_PORT_MAC_CTL_RGMII_DELAY_RXCLK;
>
> The mv88e6xxx is a MAC, so when it has a phy-mode = "rgmii-rxid", it
> should assume it is connected to a link partner (PHY or otherwise) that
> has applied the RXCLK delay already. So it should only be concerned with
> the TXCLK delay. That is my point. I am just trying to lay out the
> points to Prasanna that would make a sane system going forward. I am not
> sure that we actually have an in-tree driver that is sane in that
> regard.
It is a can or worms. For the used use case for the mv88e6xxx, it is a
DSA link, so there isn't really one side MAC and the other side
PHY. And if i remember correctly, both sides use rgmii-rxid.
Andrew
^ permalink raw reply [flat|nested] 44+ messages in thread* Re: [PATCH v3 net-next 05/10] net: dsa: microchip: add DSA support for microchip lan937x
2021-08-02 13:59 ` Vladimir Oltean
2021-08-02 20:47 ` Andrew Lunn
@ 2021-08-03 16:54 ` Prasanna Vengateshan
2021-08-03 23:54 ` Vladimir Oltean
1 sibling, 1 reply; 44+ messages in thread
From: Prasanna Vengateshan @ 2021-08-03 16:54 UTC (permalink / raw)
To: Vladimir Oltean, Andrew Lunn
Cc: netdev, robh+dt, UNGLinuxDriver, Woojung.Huh, hkallweit1, linux,
davem, kuba, linux-kernel, vivien.didelot, f.fainelli,
devicetree
On Mon, 2021-08-02 at 16:59 +0300, Vladimir Oltean wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
>
> On Mon, Aug 02, 2021 at 03:13:01PM +0200, Andrew Lunn wrote:
> > In general, the MAC does nothing, and passes the value to the PHY. The
> > PHY inserts delays as requested. To address Vladimir point,
> > PHY_INTERFACE_MODE_RGMII_TXID would mean the PHY adds delay in the TX
> > direction, and assumes the RX delay comes from somewhere else,
> > probably the PCB.
>
> For the PHY, that is the only portion where things are clear.
>
> > I only recommend the MAC adds delays when the PHY cannot, or there is
> > no PHY, e.g. SoC to switch, or switch to switch link. There are a few
> > MAC drivers that do add delays, mostly because that is how the vendor
> > crap tree does it.
> >
> > So as i said, what you propose is O.K, it follows this general rule of
> > thumb.
>
> The "rule of thumb" for a MAC driver is actually applied in reverse by
> most MAC drivers compared to what Russell described should be happening.
> For example, mv88e6xxx_port_set_rgmii_delay():
>
> switch (mode) {
> case PHY_INTERFACE_MODE_RGMII_RXID:
> reg |= MV88E6XXX_PORT_MAC_CTL_RGMII_DELAY_RXCLK;
>
> The mv88e6xxx is a MAC, so when it has a phy-mode = "rgmii-rxid", it
> should assume it is connected to a link partner (PHY or otherwise) that
> has applied the RXCLK delay already. So it should only be concerned with
> the TXCLK delay. That is my point. I am just trying to lay out the
> points to Prasanna that would make a sane system going forward. I am not
> sure that we actually have an in-tree driver that is sane in that
> regard.
>
> That discussion, and Russell's point, was here, btw:
> https://patchwork.ozlabs.org/project/netdev/patch/20200616074955.GA9092@laureti-dev/#2461123
Thanks Vladimir & Andrew for the right pointers and info. The thread talks about
"rgmii-*" are going to be applied by the PHY only as per the doc. For fixed-
link, MAC needs to add the delay. This fixed-link can be No-PHY or MAC-MAC or
MAC to in-accessible PHY. In such case, i am not convinced in using rgmii-tx-
delay-ps & rgmii-rx-delay-ps on the MAC side and apply delay. I still think
proposed code in earlier mail thread should still be okay.
^ permalink raw reply [flat|nested] 44+ messages in thread* Re: [PATCH v3 net-next 05/10] net: dsa: microchip: add DSA support for microchip lan937x
2021-08-03 16:54 ` Prasanna Vengateshan
@ 2021-08-03 23:54 ` Vladimir Oltean
2021-08-04 9:59 ` Russell King (Oracle)
0 siblings, 1 reply; 44+ messages in thread
From: Vladimir Oltean @ 2021-08-03 23:54 UTC (permalink / raw)
To: Prasanna Vengateshan
Cc: Andrew Lunn, netdev, robh+dt, UNGLinuxDriver, Woojung.Huh,
hkallweit1, linux, davem, kuba, linux-kernel, vivien.didelot,
f.fainelli, devicetree
On Tue, Aug 03, 2021 at 10:24:27PM +0530, Prasanna Vengateshan wrote:
> Thanks Vladimir & Andrew for the right pointers and info. The thread talks about
> "rgmii-*" are going to be applied by the PHY only as per the doc. For fixed-
> link, MAC needs to add the delay. This fixed-link can be No-PHY or MAC-MAC or
> MAC to in-accessible PHY. In such case, i am not convinced in using rgmii-tx-
> delay-ps & rgmii-rx-delay-ps on the MAC side and apply delay. I still think
> proposed code in earlier mail thread should still be okay.
Why? I genuinely do not understand your reasoning
- I read a thread that brings some arguments for which MACs should not
add delays based on the delay type in the "rgmii-*" phy-mode string
[ but based on explicit rgmii-tx-delay-ps and rgmii-rx-delay-ps
properties under the MAC OF node; this is written in the same
message as the quote that you chose ]
- I acknowledge that in certain configurations I need the MAC to apply
internal delays.
=> I disagree that I should parse the rgmii-tx-delay-ps and
rgmii-rx-delay-ps OF properties of the MAC, just apply RGMII delays
based on the "rgmii-*" phy-mode string value, when I am a DSA CPU
port and in no other circumstance
?!
I mean, feel free to feel convinced or not, but you have not actually
brought any argument to the table here, or I'm not seeing it.
Anyway, I don't believe that whatever you decide to do with the RGMII
delays is likely to be a decisive factor in whether the patches are
accepted or not, considering the fact that traditionally, everyone did
what suited their board best and that's about it; I will stop pushing back.
I have a theory that all the RGMII setups driven by the Linux PHY
library cannot all work at the same time, with the same code base.
Someone will sooner or later come and change a driver to make it do what
they need, which will break what the original author intended, which
will then be again patched, which will again break ..., which ....
If a perpetual motion device will ever be built by mankind, I am sure it
will be powered by RGMII delays.
^ permalink raw reply [flat|nested] 44+ messages in thread* Re: [PATCH v3 net-next 05/10] net: dsa: microchip: add DSA support for microchip lan937x
2021-08-03 23:54 ` Vladimir Oltean
@ 2021-08-04 9:59 ` Russell King (Oracle)
2021-08-04 10:46 ` Vladimir Oltean
0 siblings, 1 reply; 44+ messages in thread
From: Russell King (Oracle) @ 2021-08-04 9:59 UTC (permalink / raw)
To: Vladimir Oltean
Cc: Prasanna Vengateshan, Andrew Lunn, netdev, robh+dt,
UNGLinuxDriver, Woojung.Huh, hkallweit1, davem, kuba,
linux-kernel, vivien.didelot, f.fainelli, devicetree
On Wed, Aug 04, 2021 at 02:54:01AM +0300, Vladimir Oltean wrote:
> On Tue, Aug 03, 2021 at 10:24:27PM +0530, Prasanna Vengateshan wrote:
> > Thanks Vladimir & Andrew for the right pointers and info. The thread talks about
> > "rgmii-*" are going to be applied by the PHY only as per the doc. For fixed-
> > link, MAC needs to add the delay. This fixed-link can be No-PHY or MAC-MAC or
> > MAC to in-accessible PHY. In such case, i am not convinced in using rgmii-tx-
> > delay-ps & rgmii-rx-delay-ps on the MAC side and apply delay. I still think
> > proposed code in earlier mail thread should still be okay.
>
> Why? I genuinely do not understand your reasoning
>
> - I read a thread that brings some arguments for which MACs should not
> add delays based on the delay type in the "rgmii-*" phy-mode string
> [ but based on explicit rgmii-tx-delay-ps and rgmii-rx-delay-ps
> properties under the MAC OF node; this is written in the same
> message as the quote that you chose ]
>
> - I acknowledge that in certain configurations I need the MAC to apply
> internal delays.
>
> => I disagree that I should parse the rgmii-tx-delay-ps and
> rgmii-rx-delay-ps OF properties of the MAC, just apply RGMII delays
> based on the "rgmii-*" phy-mode string value, when I am a DSA CPU
> port and in no other circumstance
>
> ?!
>
> I mean, feel free to feel convinced or not, but you have not actually
> brought any argument to the table here, or I'm not seeing it.
>
> Anyway, I don't believe that whatever you decide to do with the RGMII
> delays is likely to be a decisive factor in whether the patches are
> accepted or not, considering the fact that traditionally, everyone did
> what suited their board best and that's about it; I will stop pushing back.
>
> I have a theory that all the RGMII setups driven by the Linux PHY
> library cannot all work at the same time, with the same code base.
> Someone will sooner or later come and change a driver to make it do what
> they need, which will break what the original author intended, which
> will then be again patched, which will again break ..., which ....
This is why we need to have a clear definition of what the various
RGMII interface types are, how and where they are applied, and make
sure everyone sticks to that. We have this documented in
Documentation/networking/phy.rst.
The RGMII interface modes _only_ determine how the PHY should be
configured - they do not determine how the MAC should be configured
(with /maybe/ the exception of PHY_INTERFACE_MODE_RGMII allowing the
MAC to insert "default delays".)
In the case of a fixed link, there is no "PHY" as such, but the PHY
interface mode describes the properties of the link from the MAC
perspective, since it is specified in the MAC node. So, if we have
e.g. PHY_INTERFACE_MODE_RGMII_TXID, then from the MAC perspective,
we expect the device on the other end of the fixed link to be adding
the transmit data line delay. Since we have a fixed link though, we
have no way to communicate that to the other side - but the delays do
need to be configured to conform with RGMII.
An interesting point here, however, is the mv88e6xxx DSA driver - it
appears to set the RGMII delays for _all_ ports based on what is in
the DT node for the port. So, even if you have a port operating in
RGMII with an external PHY, specifying a phy-mode of rgmii-txid results
in that being configured at the DSA end of the RGMII link. Andrew - we
may need to look at that since it doesn't conform to what we have in
the documentation...
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v3 net-next 05/10] net: dsa: microchip: add DSA support for microchip lan937x
2021-08-04 9:59 ` Russell King (Oracle)
@ 2021-08-04 10:46 ` Vladimir Oltean
2021-08-04 14:28 ` Prasanna Vengateshan
2021-08-07 15:40 ` Andrew Lunn
0 siblings, 2 replies; 44+ messages in thread
From: Vladimir Oltean @ 2021-08-04 10:46 UTC (permalink / raw)
To: Russell King (Oracle)
Cc: Prasanna Vengateshan, Andrew Lunn, netdev, robh+dt,
UNGLinuxDriver, Woojung.Huh, hkallweit1, davem, kuba,
linux-kernel, vivien.didelot, f.fainelli, devicetree
On Wed, Aug 04, 2021 at 10:59:54AM +0100, Russell King (Oracle) wrote:
> This is why we need to have a clear definition of what the various
> RGMII interface types are, how and where they are applied, and make
> sure everyone sticks to that. We have this documented in
> Documentation/networking/phy.rst.
The problem is that I have no clear migration path for the drivers I
maintain, like sja1105, and I suspect that others might be in the exact
same situation.
Currently, if the sja1105 needs to add internal delays in a MAC-to-MAC
(fixed-link) setup, it does that based on the phy-mode string. So
"rgmii-id" + "fixed-link" means for sja1105 "add RX and TX RGMII
internal delays", even though the documentation now says "the MAC should
not add the RX or TX delays in this case".
There are 2 cases to think about, old driver with new DT blob and new
driver with old DT blob. If breakage is involved, I am not actually very
interested in doing the migration, because even though the interpretation
of the phy-mode string is inconsistent between the phy-handle and fixed-link
case (which was deliberate), at least it currently does all that I need it to.
I am not even clear what is the expected canonical behavior for a MAC
driver. It parses rx-internal-delay-ps and tx-internal-delay-ps, and
then what? It treats all "rgmii*" phy-mode strings identically? Or is it
an error to have "rgmii-rxid" for phy-mode and non-zero rx-internal-delay-ps?
If it is an error, should all MAC drivers check for it? And if it is an
error, does it not make migration even more difficult (adding an
rx-internal-delay-ps property to a MAC OF node which already uses
"rgmii-id" would be preferable to also having to change the "rgmii-id"
to "rgmii", because an old kernel might also need to work with that DT
blob, and that will ignore the new rx-internal-delay-ps property).
Does qca8k_setup_of_rgmii_delay(), a very recent function, even do the
right thing with rx-internal-delay-ps, or is it doing the exact opposite
of the right thing (it applies rx-internal-delay-ps when in rgmii-id or
rgmii-rxid mode).
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v3 net-next 05/10] net: dsa: microchip: add DSA support for microchip lan937x
2021-08-04 10:46 ` Vladimir Oltean
@ 2021-08-04 14:28 ` Prasanna Vengateshan
2021-08-04 14:51 ` Vladimir Oltean
2021-08-07 15:40 ` Andrew Lunn
1 sibling, 1 reply; 44+ messages in thread
From: Prasanna Vengateshan @ 2021-08-04 14:28 UTC (permalink / raw)
To: Vladimir Oltean, Russell King (Oracle)
Cc: Andrew Lunn, netdev, robh+dt, UNGLinuxDriver, Woojung.Huh,
hkallweit1, davem, kuba, linux-kernel, vivien.didelot,
f.fainelli, devicetree
On Wed, 2021-08-04 at 13:46 +0300, Vladimir Oltean wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
>
> The problem is that I have no clear migration path for the drivers I
> maintain, like sja1105, and I suspect that others might be in the exact
> same situation.
>
> Currently, if the sja1105 needs to add internal delays in a MAC-to-MAC
> (fixed-link) setup, it does that based on the phy-mode string. So
> "rgmii-id" + "fixed-link" means for sja1105 "add RX and TX RGMII
> internal delays", even though the documentation now says "the MAC should
> not add the RX or TX delays in this case".
>
> There are 2 cases to think about, old driver with new DT blob and new
> driver with old DT blob. If breakage is involved, I am not actually very
> interested in doing the migration, because even though the interpretation
> of the phy-mode string is inconsistent between the phy-handle and fixed-link
> case (which was deliberate), at least it currently does all that I need it to.
>
> I am not even clear what is the expected canonical behavior for a MAC
> driver. It parses rx-internal-delay-ps and tx-internal-delay-ps, and
> then what? It treats all "rgmii*" phy-mode strings identically? Or is it
> an error to have "rgmii-rxid" for phy-mode and non-zero rx-internal-delay-ps?
> If it is an error, should all MAC drivers check for it? And if it is an
> error, does it not make migration even more difficult (adding an
> rx-internal-delay-ps property to a MAC OF node which already uses
> "rgmii-id" would be preferable to also having to change the "rgmii-id"
> to "rgmii", because an old kernel might also need to work with that DT
> blob, and that will ignore the new rx-internal-delay-ps property).
Considering the PHY is responsible to add internal delays w.r.to phy-mode, "*-
tx-internal-delay-ps" approach that i was applying to different connections as
shown below by bringing up different examples.
1) Fixed-link MAC-MAC:
port@4 {
.....
phy-mode = "rgmii";
rx-internal-delay-ps = <xxx>;
tx-internal-delay-ps = <xxx>;
ethernet = <ðernet>;
fixed-link {
......
};
};
2) Fixed-link MAC-Unknown:
port@5 {
......
phy-mode = "rgmii-id";
rx-internal-delay-ps = <xxx>;
tx-internal-delay-ps = <xxx>;
fixed-link {
. ....
};
};
3) Fixed-link :
port@5 {
......
phy-mode = "rgmii-id";
fixed-link {
.....
};
};
From above examples,
a) MAC node is responsible to add RGMII delay by parsing "*-internal-
delay-ps" for (1) & (2). Its a known item in this discussion.
b) Is rgmii-* to be ignored by the MAC in (2) and just apply the delays
from MAC side? Because if its forced to have "rgmii", would it become just -
>interface=*_MODE_RGMII and affects legacy?
c) if MAC follows standard delay, then it needs to be validated against
"*-internal-delay-ps", may be validating against single value and throw an
error. Might be okay.
d) For 3), Neither MAC nor other side will apply delays. Expected.
3) MAC-PHY
i) &test3 {
phy-handle = <&phy0>;
phy-mode = "rgmii-id";
phy0: ethernet-phy@xx {
.....
rx-internal-delay = <xxx>;
tx-internal-delay = <xxx>;
};
};
ii) &test4 {
phy-handle = <&phy0>;
phy-mode = "rgmii";
rx-internal-delay-ps = <xxx>;
tx-internal-delay-ps = <xxx>;
phy0: ethernet-phy@xx {
reg = <x>;
};
};
For 3(i), I assume phy would apply internal delay values by checking its phydev-
>interface.
For 3(ii), MAC would apply the delays.
Overall, only (b) need a right decision? or any other items are missed?
Prasanna V
^ permalink raw reply [flat|nested] 44+ messages in thread* Re: [PATCH v3 net-next 05/10] net: dsa: microchip: add DSA support for microchip lan937x
2021-08-04 14:28 ` Prasanna Vengateshan
@ 2021-08-04 14:51 ` Vladimir Oltean
0 siblings, 0 replies; 44+ messages in thread
From: Vladimir Oltean @ 2021-08-04 14:51 UTC (permalink / raw)
To: Prasanna Vengateshan
Cc: Russell King (Oracle),
Andrew Lunn, netdev, robh+dt, UNGLinuxDriver, Woojung.Huh,
hkallweit1, davem, kuba, linux-kernel, vivien.didelot,
f.fainelli, devicetree
On Wed, Aug 04, 2021 at 07:58:15PM +0530, Prasanna Vengateshan wrote:
> On Wed, 2021-08-04 at 13:46 +0300, Vladimir Oltean wrote:
> > The problem is that I have no clear migration path for the drivers I
> > maintain, like sja1105, and I suspect that others might be in the exact
> > same situation.
> >
> > Currently, if the sja1105 needs to add internal delays in a MAC-to-MAC
> > (fixed-link) setup, it does that based on the phy-mode string. So
> > "rgmii-id" + "fixed-link" means for sja1105 "add RX and TX RGMII
> > internal delays", even though the documentation now says "the MAC should
> > not add the RX or TX delays in this case".
> >
> > There are 2 cases to think about, old driver with new DT blob and new
> > driver with old DT blob. If breakage is involved, I am not actually very
> > interested in doing the migration, because even though the interpretation
> > of the phy-mode string is inconsistent between the phy-handle and fixed-link
> > case (which was deliberate), at least it currently does all that I need it to.
> >
> > I am not even clear what is the expected canonical behavior for a MAC
> > driver. It parses rx-internal-delay-ps and tx-internal-delay-ps, and
> > then what? It treats all "rgmii*" phy-mode strings identically? Or is it
> > an error to have "rgmii-rxid" for phy-mode and non-zero rx-internal-delay-ps?
> > If it is an error, should all MAC drivers check for it? And if it is an
> > error, does it not make migration even more difficult (adding an
> > rx-internal-delay-ps property to a MAC OF node which already uses
> > "rgmii-id" would be preferable to also having to change the "rgmii-id"
> > to "rgmii", because an old kernel might also need to work with that DT
> > blob, and that will ignore the new rx-internal-delay-ps property).
>
>
> Considering the PHY is responsible to add internal delays w.r.to phy-mode, "*-
> tx-internal-delay-ps" approach that i was applying to different connections as
> shown below by bringing up different examples.
>
> 1) Fixed-link MAC-MAC:
> port@4 {
> .....
> phy-mode = "rgmii";
> rx-internal-delay-ps = <xxx>;
> tx-internal-delay-ps = <xxx>;
> ethernet = <ðernet>;
> fixed-link {
> ......
> };
> };
>
> 2) Fixed-link MAC-Unknown:
> port@5 {
> ......
> phy-mode = "rgmii-id";
> rx-internal-delay-ps = <xxx>;
> tx-internal-delay-ps = <xxx>;
> fixed-link {
> . ....
> };
> };
>
> 3) Fixed-link :
> port@5 {
> ......
> phy-mode = "rgmii-id";
> fixed-link {
> .....
> };
> };
>
> From above examples,
> a) MAC node is responsible to add RGMII delay by parsing "*-internal-
> delay-ps" for (1) & (2). Its a known item in this discussion.
> b) Is rgmii-* to be ignored by the MAC in (2) and just apply the delays
> from MAC side? Because if its forced to have "rgmii", would it become just -
> >interface=*_MODE_RGMII and affects legacy?
Yes, I think the MAC would have to accept any "rgmii*" phy-mode in
fixed-link. The legacy behavior would be do to whatever it did before,
and the new behavior would be to NOT apply any MAC-level delays based on
the phy-mode value, but only based on the {rx,tx}-internal-delay-ps
properties if these are present, or fall back to the legacy behavior if
they aren't.
This way:
- New kernel with old DT blob falls back to legacy behavior
- New kernel with new DT blob finds the explicit {rx,tx}-internal-delay-ps
properties and applies MAC-level delays only according to those, while
accepting any phy-mode string
- Old kernel with new DT blob behaves the same as before, because it
does not parse {rx,tx}-internal-delay-ps and we will not change its
phy-mode.
> c) if MAC follows standard delay, then it needs to be validated against
> "*-internal-delay-ps", may be validating against single value and throw an
> error. Might be okay.
Drivers with no legacy might throw an error if:
- phy-mode == "rgmii-id" or "rgmii-rxid" and there is a non-zero rx-internal-delay-ps
- phy-mode == "rgmii-id" or "rgmii-txid" and there is a non-zero tx-internal-delay-ps
but considering that most drivers already have a legacy to support, I'm
not sure how useful that error will be.
> d) For 3), Neither MAC nor other side will apply delays. Expected.
In the "new" behavior, correct. In "legacy" behavior, they might have to.
> 3) MAC-PHY
>
> i) &test3 {
> phy-handle = <&phy0>;
> phy-mode = "rgmii-id";
> phy0: ethernet-phy@xx {
> .....
> rx-internal-delay = <xxx>;
> tx-internal-delay = <xxx>;
> };
> };
>
> ii) &test4 {
> phy-handle = <&phy0>;
> phy-mode = "rgmii";
> rx-internal-delay-ps = <xxx>;
> tx-internal-delay-ps = <xxx>;
> phy0: ethernet-phy@xx {
> reg = <x>;
> };
> };
>
>
> For 3(i), I assume phy would apply internal delay values by checking its phydev-
> >interface.
PHY drivers have a phy_get_internal_delay() helper that takes into
consideration both the phy-mode value and the {rx,tx}-internal-delay
properties. In example 3(i), the {rx,tx}-internal-delay properties would
prevail as long as the PHY driver uses that helper.
> For 3(ii), MAC would apply the delays.
>
> Overall, only (b) need a right decision? or any other items are missed?
>
>
> Prasanna V
>
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v3 net-next 05/10] net: dsa: microchip: add DSA support for microchip lan937x
2021-08-04 10:46 ` Vladimir Oltean
2021-08-04 14:28 ` Prasanna Vengateshan
@ 2021-08-07 15:40 ` Andrew Lunn
2021-08-07 17:00 ` Vladimir Oltean
2021-08-11 17:44 ` Prasanna Vengateshan
1 sibling, 2 replies; 44+ messages in thread
From: Andrew Lunn @ 2021-08-07 15:40 UTC (permalink / raw)
To: Vladimir Oltean
Cc: Russell King (Oracle),
Prasanna Vengateshan, netdev, robh+dt, UNGLinuxDriver,
Woojung.Huh, hkallweit1, davem, kuba, linux-kernel,
vivien.didelot, f.fainelli, devicetree
> I am not even clear what is the expected canonical behavior for a MAC
> driver. It parses rx-internal-delay-ps and tx-internal-delay-ps, and
> then what?
So best practices are based around a MAC-PHY link. phy-mode is passed
to the PHY, and the MAC does not act upon it. MAC rx-internal-delay-ps
and tx-internal-delay-ps can be used to fine tune the link. You can
use them to add and sometimes subtract small amounts of delay.
> It treats all "rgmii*" phy-mode strings identically? Or is it an
> error to have "rgmii-rxid" for phy-mode and non-zero
> rx-internal-delay-ps?
I would say the first is correct, the second statement is false. You
should always be able to fine tune the link, independent of the PHY
mode.
We also have to consider the case when the PHY is not actually able to
implement the delay. It hopefully returns -EOPNOTSUPP for anything
other than "rgmii". You can then put the full 2ns delay into
tx-internal-delay-ps nd rx-internal-delay-ps.
And lastly there is one MAC driver which mostly ignores these best
practices because the vendor crap tree always did the delay in the
MAC. It correctly masks the phy-mode, so the PHY does not add delays.
For MAC-MAC and fixed link best practices are very fuzzily defined.
It is not something we have much of in the kernel. We might also want
to narrow the discussion down to MACs within a switch. MACs within in
NIC should probably follow the best practices for a MAC-PHY link, even
if it is actually a switch on the other end.
I also agree with Russell that mv88e6xxx is probably broken for a
MAC-PHY link. It is known to work for a Marvell DSA in MAC-MAC link,
we have boards doing that.
It seems like a switch MAC should parse rx-internal-delay-ps and
tx-internal-delay-ps and apply them independent of the phy-mode. That
keeps it consistent with MAC-PHY. And if there is a PHY connected,
pass the phy-mode on unmasked.
So that we don't break mv88e6xxx, for a CPU or DSA port, we probably
should continue to locally implement the delay, with the assumption
there is no PHY, it is a MAC-MAC link. We probably want to patch
mv88e6xxx to do nothing for user ports.
I suspect it is a 50/50 roll of a dice what rx and tx actually
mean. Is it from the perspective of the MAC or the PHY? Luckily,
rgmii-rxid and rgmii-txid don't appear in DT very often.
Andrew
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v3 net-next 05/10] net: dsa: microchip: add DSA support for microchip lan937x
2021-08-07 15:40 ` Andrew Lunn
@ 2021-08-07 17:00 ` Vladimir Oltean
2021-08-11 17:44 ` Prasanna Vengateshan
1 sibling, 0 replies; 44+ messages in thread
From: Vladimir Oltean @ 2021-08-07 17:00 UTC (permalink / raw)
To: Andrew Lunn
Cc: Russell King (Oracle),
Prasanna Vengateshan, netdev, robh+dt, UNGLinuxDriver,
Woojung.Huh, hkallweit1, davem, kuba, linux-kernel,
vivien.didelot, f.fainelli, devicetree
On Sat, Aug 07, 2021 at 05:40:35PM +0200, Andrew Lunn wrote:
> I suspect it is a 50/50 roll of a dice what rx and tx actually
> mean. Is it from the perspective of the MAC or the PHY? Luckily,
> rgmii-rxid and rgmii-txid don't appear in DT very often.
I checked an NXP board schematic which has both, and:
When you connect a MAC to a PHY using RGMII, the RXD[3:0] pins of the
MAC connect to the RXD[3:0] pins of the PHY, and the TXD[3:0] goes to
TXD[3:0]. So it is neither the perspective of the MAC nor of the PHY.
When you connect a MAC to a MAC using RGMII, the RXD[3:0] of one MAC
goes to the TXD[3:0] of the other, and vice versa.
Nonetheless, a phy-mode of "rgmii-rxid" always means to the local MAC
that "the RX delays have been dealt with" - either by PCB traces, or a
remote MAC applying TX delay, or the PHY applying RX delay.
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v3 net-next 05/10] net: dsa: microchip: add DSA support for microchip lan937x
2021-08-07 15:40 ` Andrew Lunn
2021-08-07 17:00 ` Vladimir Oltean
@ 2021-08-11 17:44 ` Prasanna Vengateshan
2021-08-11 18:23 ` Andrew Lunn
1 sibling, 1 reply; 44+ messages in thread
From: Prasanna Vengateshan @ 2021-08-11 17:44 UTC (permalink / raw)
To: Andrew Lunn, Vladimir Oltean
Cc: Russell King (Oracle),
netdev, robh+dt, UNGLinuxDriver, Woojung.Huh, hkallweit1, davem,
kuba, linux-kernel, vivien.didelot, f.fainelli, devicetree
On Sat, 2021-08-07 at 17:40 +0200, Andrew Lunn wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
>
> > I am not even clear what is the expected canonical behavior for a MAC
> > driver. It parses rx-internal-delay-ps and tx-internal-delay-ps, and
> > then what?
>
> So best practices are based around a MAC-PHY link. phy-mode is passed
> to the PHY, and the MAC does not act upon it. MAC rx-internal-delay-ps
> and tx-internal-delay-ps can be used to fine tune the link. You can
> use them to add and sometimes subtract small amounts of delay.
>
> > It treats all "rgmii*" phy-mode strings identically? Or is it an
> > error to have "rgmii-rxid" for phy-mode and non-zero
> > rx-internal-delay-ps?
>
> I would say the first is correct, the second statement is false. You
> should always be able to fine tune the link, independent of the PHY
> mode.
>
> We also have to consider the case when the PHY is not actually able to
> implement the delay. It hopefully returns -EOPNOTSUPP for anything
> other than "rgmii". You can then put the full 2ns delay into
> tx-internal-delay-ps nd rx-internal-delay-ps.
I hope that using "*-internal-delay-ps" for Mac would be the right option.
Shall i include these changes as we discussed in next revision of the patch?
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v3 net-next 05/10] net: dsa: microchip: add DSA support for microchip lan937x
2021-08-11 17:44 ` Prasanna Vengateshan
@ 2021-08-11 18:23 ` Andrew Lunn
2021-08-11 20:14 ` Russell King (Oracle)
0 siblings, 1 reply; 44+ messages in thread
From: Andrew Lunn @ 2021-08-11 18:23 UTC (permalink / raw)
To: Prasanna Vengateshan
Cc: Vladimir Oltean, Russell King (Oracle),
netdev, robh+dt, UNGLinuxDriver, Woojung.Huh, hkallweit1, davem,
kuba, linux-kernel, vivien.didelot, f.fainelli, devicetree
> I hope that using "*-internal-delay-ps" for Mac would be the right option.
> Shall i include these changes as we discussed in next revision of the patch?
Yes, that seems sensible. But please limit them to the CPU port. Maybe
return -EINVAL for other ports.
Andrew
^ permalink raw reply [flat|nested] 44+ messages in thread* Re: [PATCH v3 net-next 05/10] net: dsa: microchip: add DSA support for microchip lan937x
2021-08-11 18:23 ` Andrew Lunn
@ 2021-08-11 20:14 ` Russell King (Oracle)
2021-08-11 20:20 ` Vladimir Oltean
0 siblings, 1 reply; 44+ messages in thread
From: Russell King (Oracle) @ 2021-08-11 20:14 UTC (permalink / raw)
To: Andrew Lunn
Cc: Prasanna Vengateshan, Vladimir Oltean, netdev, robh+dt,
UNGLinuxDriver, Woojung.Huh, hkallweit1, davem, kuba,
linux-kernel, vivien.didelot, f.fainelli, devicetree
On Wed, Aug 11, 2021 at 08:23:04PM +0200, Andrew Lunn wrote:
> > I hope that using "*-internal-delay-ps" for Mac would be the right option.
> > Shall i include these changes as we discussed in next revision of the patch?
>
> Yes, that seems sensible. But please limit them to the CPU port. Maybe
> return -EINVAL for other ports.
Hmm. Don't we want ports that are "MAC like" to behave "MAC like" ?
In other words, shouldn't a DSA port that can be connected to an
external PHY should accept the same properties as a conventional
Ethernet MAC e.g. in a SoC device?
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v3 net-next 05/10] net: dsa: microchip: add DSA support for microchip lan937x
2021-08-11 20:14 ` Russell King (Oracle)
@ 2021-08-11 20:20 ` Vladimir Oltean
2021-08-11 20:22 ` Andrew Lunn
0 siblings, 1 reply; 44+ messages in thread
From: Vladimir Oltean @ 2021-08-11 20:20 UTC (permalink / raw)
To: Russell King (Oracle)
Cc: Andrew Lunn, Prasanna Vengateshan, netdev, robh+dt,
UNGLinuxDriver, Woojung.Huh, hkallweit1, davem, kuba,
linux-kernel, vivien.didelot, f.fainelli, devicetree
On Wed, Aug 11, 2021 at 09:14:14PM +0100, Russell King (Oracle) wrote:
> On Wed, Aug 11, 2021 at 08:23:04PM +0200, Andrew Lunn wrote:
> > > I hope that using "*-internal-delay-ps" for Mac would be the right option.
> > > Shall i include these changes as we discussed in next revision of the patch?
> >
> > Yes, that seems sensible. But please limit them to the CPU port. Maybe
> > return -EINVAL for other ports.
>
> Hmm. Don't we want ports that are "MAC like" to behave "MAC like" ?
> In other words, shouldn't a DSA port that can be connected to an
> external PHY should accept the same properties as a conventional
> Ethernet MAC e.g. in a SoC device?
+1, I thought the whole purpose of the discussion was to stop singling
out the CPU port as being special in any way w.r.t. RGMII delays.
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v3 net-next 05/10] net: dsa: microchip: add DSA support for microchip lan937x
2021-08-11 20:20 ` Vladimir Oltean
@ 2021-08-11 20:22 ` Andrew Lunn
0 siblings, 0 replies; 44+ messages in thread
From: Andrew Lunn @ 2021-08-11 20:22 UTC (permalink / raw)
To: Vladimir Oltean
Cc: Russell King (Oracle),
Prasanna Vengateshan, netdev, robh+dt, UNGLinuxDriver,
Woojung.Huh, hkallweit1, davem, kuba, linux-kernel,
vivien.didelot, f.fainelli, devicetree
On Wed, Aug 11, 2021 at 11:20:19PM +0300, Vladimir Oltean wrote:
> On Wed, Aug 11, 2021 at 09:14:14PM +0100, Russell King (Oracle) wrote:
> > On Wed, Aug 11, 2021 at 08:23:04PM +0200, Andrew Lunn wrote:
> > > > I hope that using "*-internal-delay-ps" for Mac would be the right option.
> > > > Shall i include these changes as we discussed in next revision of the patch?
> > >
> > > Yes, that seems sensible. But please limit them to the CPU port. Maybe
> > > return -EINVAL for other ports.
> >
> > Hmm. Don't we want ports that are "MAC like" to behave "MAC like" ?
> > In other words, shouldn't a DSA port that can be connected to an
> > external PHY should accept the same properties as a conventional
> > Ethernet MAC e.g. in a SoC device?
>
> +1, I thought the whole purpose of the discussion was to stop singling
> out the CPU port as being special in any way w.r.t. RGMII delays.
Yes, sorry.
What we don't want is it acting upon phy-mode.
Andrew
^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH v3 net-next 06/10] net: dsa: microchip: add support for phylink management
2021-07-23 17:30 [PATCH v3 net-next 00/10] net: dsa: microchip: DSA driver support for LAN937x switch Prasanna Vengateshan
` (4 preceding siblings ...)
2021-07-23 17:31 ` [PATCH v3 net-next 05/10] net: dsa: microchip: add DSA support for microchip lan937x Prasanna Vengateshan
@ 2021-07-23 17:31 ` Prasanna Vengateshan
2021-07-31 15:27 ` Vladimir Oltean
2021-07-23 17:31 ` [PATCH v3 net-next 07/10] net: dsa: microchip: add support for ethtool port counters Prasanna Vengateshan
` (3 subsequent siblings)
9 siblings, 1 reply; 44+ messages in thread
From: Prasanna Vengateshan @ 2021-07-23 17:31 UTC (permalink / raw)
To: andrew, netdev, olteanv, robh+dt
Cc: UNGLinuxDriver, Woojung.Huh, hkallweit1, linux, davem, kuba,
linux-kernel, vivien.didelot, f.fainelli, devicetree
Support for phylink_validate() and reused KSZ commmon API for
phylink_mac_link_down() operation
lan937x_phylink_mac_config configures the interface using
lan937x_mac_config and lan937x_phylink_mac_link_up configures
the speed/duplex/flow control.
Currently SGMII & in-band neg are not supported & it will be
added later.
Signed-off-by: Prasanna Vengateshan <prasanna.vengateshan@microchip.com>
---
drivers/net/dsa/microchip/lan937x_dev.c | 46 ++++++++++++
drivers/net/dsa/microchip/lan937x_dev.h | 3 +
drivers/net/dsa/microchip/lan937x_main.c | 90 ++++++++++++++++++++++++
3 files changed, 139 insertions(+)
diff --git a/drivers/net/dsa/microchip/lan937x_dev.c b/drivers/net/dsa/microchip/lan937x_dev.c
index 86e84a79f464..c36fdc4692ff 100644
--- a/drivers/net/dsa/microchip/lan937x_dev.c
+++ b/drivers/net/dsa/microchip/lan937x_dev.c
@@ -477,6 +477,52 @@ void lan937x_mac_config(struct ksz_device *dev, int port,
lan937x_pwrite8(dev, port, REG_PORT_XMII_CTRL_1, data8);
}
+void lan937x_config_interface(struct ksz_device *dev, int port,
+ int speed, int duplex,
+ bool tx_pause, bool rx_pause)
+{
+ u8 xmii_ctrl0, xmii_ctrl1;
+
+ lan937x_pread8(dev, port, REG_PORT_XMII_CTRL_0, &xmii_ctrl0);
+ lan937x_pread8(dev, port, REG_PORT_XMII_CTRL_1, &xmii_ctrl1);
+
+ switch (speed) {
+ case SPEED_1000:
+ lan937x_config_gbit(dev, true, &xmii_ctrl1);
+ break;
+ case SPEED_100:
+ lan937x_config_gbit(dev, false, &xmii_ctrl1);
+ xmii_ctrl0 |= PORT_MAC_SPEED_100;
+ break;
+ case SPEED_10:
+ lan937x_config_gbit(dev, false, &xmii_ctrl1);
+ xmii_ctrl0 &= ~PORT_MAC_SPEED_100;
+ break;
+ default:
+ dev_err(dev->dev, "Unsupported speed on port %d: %d\n",
+ port, speed);
+ return;
+ }
+
+ if (duplex)
+ xmii_ctrl0 |= PORT_FULL_DUPLEX;
+ else
+ xmii_ctrl0 &= ~PORT_FULL_DUPLEX;
+
+ if (tx_pause)
+ xmii_ctrl0 |= PORT_TX_FLOW_CTRL;
+ else
+ xmii_ctrl1 &= ~PORT_TX_FLOW_CTRL;
+
+ if (rx_pause)
+ xmii_ctrl0 |= PORT_RX_FLOW_CTRL;
+ else
+ xmii_ctrl0 &= ~PORT_RX_FLOW_CTRL;
+
+ lan937x_pwrite8(dev, port, REG_PORT_XMII_CTRL_0, xmii_ctrl0);
+ lan937x_pwrite8(dev, port, REG_PORT_XMII_CTRL_1, xmii_ctrl1);
+}
+
void lan937x_port_setup(struct ksz_device *dev, int port, bool cpu_port)
{
struct ksz_port *p = &dev->ports[port];
diff --git a/drivers/net/dsa/microchip/lan937x_dev.h b/drivers/net/dsa/microchip/lan937x_dev.h
index 76c00db05616..0ddd7aa87468 100644
--- a/drivers/net/dsa/microchip/lan937x_dev.h
+++ b/drivers/net/dsa/microchip/lan937x_dev.h
@@ -36,6 +36,9 @@ void lan937x_cfg_port_member(struct ksz_device *dev, int port,
u8 member);
void lan937x_port_setup(struct ksz_device *dev, int port, bool cpu_port);
int lan937x_enable_spi_indirect_access(struct ksz_device *dev);
+void lan937x_config_interface(struct ksz_device *dev, int port,
+ int speed, int duplex,
+ bool tx_pause, bool rx_pause);
void lan937x_mac_config(struct ksz_device *dev, int port,
phy_interface_t interface);
diff --git a/drivers/net/dsa/microchip/lan937x_main.c b/drivers/net/dsa/microchip/lan937x_main.c
index d43db522d07e..74b6c9563dc1 100644
--- a/drivers/net/dsa/microchip/lan937x_main.c
+++ b/drivers/net/dsa/microchip/lan937x_main.c
@@ -292,6 +292,92 @@ static int lan937x_get_max_mtu(struct dsa_switch *ds, int port)
return (FR_MAX_SIZE - VLAN_ETH_HLEN - ETH_FCS_LEN);
}
+static void lan937x_phylink_mac_config(struct dsa_switch *ds, int port,
+ unsigned int mode,
+ const struct phylink_link_state *state)
+{
+ struct ksz_device *dev = ds->priv;
+
+ /* Internal PHYs */
+ if (lan937x_is_internal_phy_port(dev, port))
+ return;
+
+ if (phylink_autoneg_inband(mode)) {
+ dev_err(ds->dev, "In-band AN not supported!\n");
+ return;
+ }
+ lan937x_mac_config(dev, port, state->interface);
+}
+
+static void lan937x_phylink_mac_link_up(struct dsa_switch *ds, int port,
+ unsigned int mode,
+ phy_interface_t interface,
+ struct phy_device *phydev,
+ int speed, int duplex,
+ bool tx_pause, bool rx_pause)
+{
+ struct ksz_device *dev = ds->priv;
+
+ /* Internal PHYs */
+ if (lan937x_is_internal_phy_port(dev, port))
+ return;
+
+ if (phylink_autoneg_inband(mode)) {
+ dev_err(ds->dev, "In-band AN not supported!\n");
+ return;
+ }
+ lan937x_config_interface(dev, port, speed, duplex,
+ tx_pause, rx_pause);
+}
+
+static void lan937x_phylink_validate(struct dsa_switch *ds, int port,
+ unsigned long *supported,
+ struct phylink_link_state *state)
+{
+ struct ksz_device *dev = ds->priv;
+ __ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
+
+ /* Check for unsupported interfaces */
+ if (!phy_interface_mode_is_rgmii(state->interface) &&
+ state->interface != PHY_INTERFACE_MODE_RMII &&
+ state->interface != PHY_INTERFACE_MODE_MII &&
+ state->interface != PHY_INTERFACE_MODE_INTERNAL) {
+ bitmap_zero(supported, __ETHTOOL_LINK_MODE_MASK_NBITS);
+ dev_err(ds->dev, "Unsupported interface '%s' for port %d\n",
+ phy_modes(state->interface), port);
+ return;
+ }
+
+ /* For RGMII, RMII, MII and internal TX phy port */
+ if (phy_interface_mode_is_rgmii(state->interface) ||
+ state->interface == PHY_INTERFACE_MODE_RMII ||
+ state->interface == PHY_INTERFACE_MODE_MII ||
+ lan937x_is_internal_100BTX_phy_port(dev, port)) {
+ phylink_set(mask, 10baseT_Half);
+ phylink_set(mask, 10baseT_Full);
+ phylink_set(mask, 100baseT_Half);
+ phylink_set(mask, 100baseT_Full);
+ phylink_set(mask, Autoneg);
+ phylink_set_port_modes(mask);
+ phylink_set(mask, Pause);
+ phylink_set(mask, Asym_Pause);
+ }
+
+ /* For RGMII interface */
+ if (phy_interface_mode_is_rgmii(state->interface))
+ phylink_set(mask, 1000baseT_Full);
+
+ /* For T1 PHY */
+ if (lan937x_is_internal_t1_phy_port(dev, port)) {
+ phylink_set(mask, 100baseT1_Full);
+ phylink_set_port_modes(mask);
+ }
+
+ bitmap_and(supported, supported, mask, __ETHTOOL_LINK_MODE_MASK_NBITS);
+ bitmap_and(state->advertising, state->advertising, mask,
+ __ETHTOOL_LINK_MODE_MASK_NBITS);
+}
+
const struct dsa_switch_ops lan937x_switch_ops = {
.get_tag_protocol = lan937x_get_tag_protocol,
.setup = lan937x_setup,
@@ -304,6 +390,10 @@ const struct dsa_switch_ops lan937x_switch_ops = {
.port_fast_age = ksz_port_fast_age,
.port_max_mtu = lan937x_get_max_mtu,
.port_change_mtu = lan937x_change_mtu,
+ .phylink_validate = lan937x_phylink_validate,
+ .phylink_mac_link_down = ksz_mac_link_down,
+ .phylink_mac_config = lan937x_phylink_mac_config,
+ .phylink_mac_link_up = lan937x_phylink_mac_link_up,
};
int lan937x_switch_register(struct ksz_device *dev)
--
2.27.0
^ permalink raw reply related [flat|nested] 44+ messages in thread* Re: [PATCH v3 net-next 06/10] net: dsa: microchip: add support for phylink management
2021-07-23 17:31 ` [PATCH v3 net-next 06/10] net: dsa: microchip: add support for phylink management Prasanna Vengateshan
@ 2021-07-31 15:27 ` Vladimir Oltean
2021-08-03 17:04 ` Prasanna Vengateshan
0 siblings, 1 reply; 44+ messages in thread
From: Vladimir Oltean @ 2021-07-31 15:27 UTC (permalink / raw)
To: Prasanna Vengateshan
Cc: andrew, netdev, robh+dt, UNGLinuxDriver, Woojung.Huh, hkallweit1,
linux, davem, kuba, linux-kernel, vivien.didelot, f.fainelli,
devicetree
On Fri, Jul 23, 2021 at 11:01:04PM +0530, Prasanna Vengateshan wrote:
> +static void lan937x_phylink_validate(struct dsa_switch *ds, int port,
> + unsigned long *supported,
> + struct phylink_link_state *state)
> +{
> + struct ksz_device *dev = ds->priv;
> + __ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
> +
> + /* Check for unsupported interfaces */
> + if (!phy_interface_mode_is_rgmii(state->interface) &&
> + state->interface != PHY_INTERFACE_MODE_RMII &&
> + state->interface != PHY_INTERFACE_MODE_MII &&
> + state->interface != PHY_INTERFACE_MODE_INTERNAL) {
According to include/linux/phylink.h, when phylink passes
state->interface == PHY_INTERFACE_MODE_NA, you are expected to return
all supported link modes.
^ permalink raw reply [flat|nested] 44+ messages in thread* Re: [PATCH v3 net-next 06/10] net: dsa: microchip: add support for phylink management
2021-07-31 15:27 ` Vladimir Oltean
@ 2021-08-03 17:04 ` Prasanna Vengateshan
0 siblings, 0 replies; 44+ messages in thread
From: Prasanna Vengateshan @ 2021-08-03 17:04 UTC (permalink / raw)
To: Vladimir Oltean
Cc: andrew, netdev, robh+dt, UNGLinuxDriver, Woojung.Huh, hkallweit1,
linux, davem, kuba, linux-kernel, vivien.didelot, f.fainelli,
devicetree
On Sat, 2021-07-31 at 18:27 +0300, Vladimir Oltean wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
>
> On Fri, Jul 23, 2021 at 11:01:04PM +0530, Prasanna Vengateshan wrote:
> > +static void lan937x_phylink_validate(struct dsa_switch *ds, int port,
> > + unsigned long *supported,
> > + struct phylink_link_state *state)
> > +{
> > + struct ksz_device *dev = ds->priv;
> > + __ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
> > +
> > + /* Check for unsupported interfaces */
> > + if (!phy_interface_mode_is_rgmii(state->interface) &&
> > + state->interface != PHY_INTERFACE_MODE_RMII &&
> > + state->interface != PHY_INTERFACE_MODE_MII &&
> > + state->interface != PHY_INTERFACE_MODE_INTERNAL) {
>
> According to include/linux/phylink.h, when phylink passes
> state->interface == PHY_INTERFACE_MODE_NA, you are expected to return
> all supported link modes.
Okay, Noted. i think PHY_INTERFACE_MODE_NA check should be added in all of the
'if' (==) checks down to the above one and including the above one (!=).
^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH v3 net-next 07/10] net: dsa: microchip: add support for ethtool port counters
2021-07-23 17:30 [PATCH v3 net-next 00/10] net: dsa: microchip: DSA driver support for LAN937x switch Prasanna Vengateshan
` (5 preceding siblings ...)
2021-07-23 17:31 ` [PATCH v3 net-next 06/10] net: dsa: microchip: add support for phylink management Prasanna Vengateshan
@ 2021-07-23 17:31 ` Prasanna Vengateshan
2021-07-23 17:31 ` [PATCH v3 net-next 08/10] net: dsa: microchip: add support for port mirror operations Prasanna Vengateshan
` (2 subsequent siblings)
9 siblings, 0 replies; 44+ messages in thread
From: Prasanna Vengateshan @ 2021-07-23 17:31 UTC (permalink / raw)
To: andrew, netdev, olteanv, robh+dt
Cc: UNGLinuxDriver, Woojung.Huh, hkallweit1, linux, davem, kuba,
linux-kernel, vivien.didelot, f.fainelli, devicetree
Reused the KSZ common APIs for get_ethtool_stats() & get_sset_count()
along with relevant lan937x hooks for KSZ common layer and added
support for get_strings()
Signed-off-by: Prasanna Vengateshan <prasanna.vengateshan@microchip.com>
---
drivers/net/dsa/microchip/lan937x_main.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/drivers/net/dsa/microchip/lan937x_main.c b/drivers/net/dsa/microchip/lan937x_main.c
index 74b6c9563dc1..3380a4617725 100644
--- a/drivers/net/dsa/microchip/lan937x_main.c
+++ b/drivers/net/dsa/microchip/lan937x_main.c
@@ -43,6 +43,21 @@ static int lan937x_phy_write16(struct dsa_switch *ds, int addr, int reg,
return lan937x_internal_phy_write(dev, addr, reg, val);
}
+static void lan937x_get_strings(struct dsa_switch *ds, int port, u32 stringset,
+ uint8_t *buf)
+{
+ struct ksz_device *dev = ds->priv;
+ int i;
+
+ if (stringset != ETH_SS_STATS)
+ return;
+
+ for (i = 0; i < dev->mib_cnt; i++) {
+ memcpy(buf + i * ETH_GSTRING_LEN, lan937x_mib_names[i].string,
+ ETH_GSTRING_LEN);
+ }
+}
+
static void lan937x_port_stp_state_set(struct dsa_switch *ds, int port,
u8 state)
{
@@ -384,6 +399,9 @@ const struct dsa_switch_ops lan937x_switch_ops = {
.phy_read = lan937x_phy_read16,
.phy_write = lan937x_phy_write16,
.port_enable = ksz_enable_port,
+ .get_strings = lan937x_get_strings,
+ .get_ethtool_stats = ksz_get_ethtool_stats,
+ .get_sset_count = ksz_sset_count,
.port_bridge_join = ksz_port_bridge_join,
.port_bridge_leave = ksz_port_bridge_leave,
.port_stp_state_set = lan937x_port_stp_state_set,
--
2.27.0
^ permalink raw reply related [flat|nested] 44+ messages in thread* [PATCH v3 net-next 08/10] net: dsa: microchip: add support for port mirror operations
2021-07-23 17:30 [PATCH v3 net-next 00/10] net: dsa: microchip: DSA driver support for LAN937x switch Prasanna Vengateshan
` (6 preceding siblings ...)
2021-07-23 17:31 ` [PATCH v3 net-next 07/10] net: dsa: microchip: add support for ethtool port counters Prasanna Vengateshan
@ 2021-07-23 17:31 ` Prasanna Vengateshan
2021-07-31 15:24 ` Vladimir Oltean
2021-07-23 17:31 ` [PATCH v3 net-next 09/10] net: dsa: microchip: add support for fdb and mdb management Prasanna Vengateshan
2021-07-23 17:31 ` [PATCH v3 net-next 10/10] net: dsa: microchip: add support for vlan operations Prasanna Vengateshan
9 siblings, 1 reply; 44+ messages in thread
From: Prasanna Vengateshan @ 2021-07-23 17:31 UTC (permalink / raw)
To: andrew, netdev, olteanv, robh+dt
Cc: UNGLinuxDriver, Woojung.Huh, hkallweit1, linux, davem, kuba,
linux-kernel, vivien.didelot, f.fainelli, devicetree
Added support for port_mirror_add() and port_mirror_del operations
Sniffing is limited to one port & alert the user if any new
sniffing port is selected
Signed-off-by: Prasanna Vengateshan <prasanna.vengateshan@microchip.com>
---
drivers/net/dsa/microchip/lan937x_main.c | 83 ++++++++++++++++++++++++
1 file changed, 83 insertions(+)
diff --git a/drivers/net/dsa/microchip/lan937x_main.c b/drivers/net/dsa/microchip/lan937x_main.c
index 3380a4617725..171c46f37fa4 100644
--- a/drivers/net/dsa/microchip/lan937x_main.c
+++ b/drivers/net/dsa/microchip/lan937x_main.c
@@ -129,6 +129,87 @@ static void lan937x_port_stp_state_set(struct dsa_switch *ds, int port,
mutex_unlock(&dev->dev_mutex);
}
+static int lan937x_port_mirror_add(struct dsa_switch *ds, int port,
+ struct dsa_mall_mirror_tc_entry *mirror,
+ bool ingress)
+{
+ struct ksz_device *dev = ds->priv;
+ int ret, p;
+ u8 data;
+
+ /* Configure ingress/egress mirroring*/
+ if (ingress)
+ ret = lan937x_port_cfg(dev, port, P_MIRROR_CTRL, PORT_MIRROR_RX,
+ true);
+ else
+ ret = lan937x_port_cfg(dev, port, P_MIRROR_CTRL, PORT_MIRROR_TX,
+ true);
+ if (ret < 0)
+ return ret;
+
+ /* Configure sniffer port meanwhile limit to one sniffer port
+ * Check if any of the port is already set for sniffing
+ * If yes, instruct the user to remove the previous entry & exit
+ */
+ for (p = 0; p < dev->port_cnt; p++) {
+ /*Skip the current sniffing port*/
+ if (p == mirror->to_local_port)
+ continue;
+
+ ret = lan937x_pread8(dev, p, P_MIRROR_CTRL, &data);
+ if (ret < 0)
+ return ret;
+
+ if (data & PORT_MIRROR_SNIFFER) {
+ dev_err(dev->dev,
+ "Delete existing rules towards %s & try\n",
+ dsa_to_port(ds, p)->name);
+ return -EBUSY;
+ }
+ }
+
+ ret = lan937x_port_cfg(dev, mirror->to_local_port, P_MIRROR_CTRL,
+ PORT_MIRROR_SNIFFER, true);
+ if (ret < 0)
+ return ret;
+
+ ret = lan937x_cfg(dev, S_MIRROR_CTRL, SW_MIRROR_RX_TX, false);
+
+ return ret;
+}
+
+static void lan937x_port_mirror_del(struct dsa_switch *ds, int port,
+ struct dsa_mall_mirror_tc_entry *mirror)
+{
+ struct ksz_device *dev = ds->priv;
+ bool in_use = false;
+ u8 data;
+ int p;
+
+ /* clear ingress/egress mirroring port*/
+ if (mirror->ingress)
+ lan937x_port_cfg(dev, port, P_MIRROR_CTRL, PORT_MIRROR_RX,
+ false);
+ else
+ lan937x_port_cfg(dev, port, P_MIRROR_CTRL, PORT_MIRROR_TX,
+ false);
+
+ /* Check if any of the port is still referring to sniffer port*/
+ for (p = 0; p < dev->port_cnt; p++) {
+ lan937x_pread8(dev, p, P_MIRROR_CTRL, &data);
+
+ if ((data & (PORT_MIRROR_RX | PORT_MIRROR_TX))) {
+ in_use = true;
+ break;
+ }
+ }
+
+ /* delete sniffing if there are no other mirroring rule exist*/
+ if (!in_use)
+ lan937x_port_cfg(dev, mirror->to_local_port, P_MIRROR_CTRL,
+ PORT_MIRROR_SNIFFER, false);
+}
+
static phy_interface_t lan937x_get_interface(struct ksz_device *dev, int port)
{
phy_interface_t interface;
@@ -406,6 +487,8 @@ const struct dsa_switch_ops lan937x_switch_ops = {
.port_bridge_leave = ksz_port_bridge_leave,
.port_stp_state_set = lan937x_port_stp_state_set,
.port_fast_age = ksz_port_fast_age,
+ .port_mirror_add = lan937x_port_mirror_add,
+ .port_mirror_del = lan937x_port_mirror_del,
.port_max_mtu = lan937x_get_max_mtu,
.port_change_mtu = lan937x_change_mtu,
.phylink_validate = lan937x_phylink_validate,
--
2.27.0
^ permalink raw reply related [flat|nested] 44+ messages in thread* Re: [PATCH v3 net-next 08/10] net: dsa: microchip: add support for port mirror operations
2021-07-23 17:31 ` [PATCH v3 net-next 08/10] net: dsa: microchip: add support for port mirror operations Prasanna Vengateshan
@ 2021-07-31 15:24 ` Vladimir Oltean
0 siblings, 0 replies; 44+ messages in thread
From: Vladimir Oltean @ 2021-07-31 15:24 UTC (permalink / raw)
To: Prasanna Vengateshan
Cc: andrew, netdev, robh+dt, UNGLinuxDriver, Woojung.Huh, hkallweit1,
linux, davem, kuba, linux-kernel, vivien.didelot, f.fainelli,
devicetree
On Fri, Jul 23, 2021 at 11:01:06PM +0530, Prasanna Vengateshan wrote:
> Added support for port_mirror_add() and port_mirror_del operations
>
> Sniffing is limited to one port & alert the user if any new
> sniffing port is selected
>
> Signed-off-by: Prasanna Vengateshan <prasanna.vengateshan@microchip.com>
> ---
> drivers/net/dsa/microchip/lan937x_main.c | 83 ++++++++++++++++++++++++
> 1 file changed, 83 insertions(+)
>
> diff --git a/drivers/net/dsa/microchip/lan937x_main.c b/drivers/net/dsa/microchip/lan937x_main.c
> index 3380a4617725..171c46f37fa4 100644
> --- a/drivers/net/dsa/microchip/lan937x_main.c
> +++ b/drivers/net/dsa/microchip/lan937x_main.c
> @@ -129,6 +129,87 @@ static void lan937x_port_stp_state_set(struct dsa_switch *ds, int port,
> mutex_unlock(&dev->dev_mutex);
> }
>
> +static int lan937x_port_mirror_add(struct dsa_switch *ds, int port,
> + struct dsa_mall_mirror_tc_entry *mirror,
> + bool ingress)
> +{
> + struct ksz_device *dev = ds->priv;
> + int ret, p;
> + u8 data;
> +
> + /* Configure ingress/egress mirroring*/
> + if (ingress)
> + ret = lan937x_port_cfg(dev, port, P_MIRROR_CTRL, PORT_MIRROR_RX,
> + true);
> + else
> + ret = lan937x_port_cfg(dev, port, P_MIRROR_CTRL, PORT_MIRROR_TX,
> + true);
> + if (ret < 0)
> + return ret;
> +
> + /* Configure sniffer port meanwhile limit to one sniffer port
> + * Check if any of the port is already set for sniffing
> + * If yes, instruct the user to remove the previous entry & exit
> + */
> + for (p = 0; p < dev->port_cnt; p++) {
> + /*Skip the current sniffing port*/
> + if (p == mirror->to_local_port)
> + continue;
> +
> + ret = lan937x_pread8(dev, p, P_MIRROR_CTRL, &data);
> + if (ret < 0)
> + return ret;
> +
> + if (data & PORT_MIRROR_SNIFFER) {
> + dev_err(dev->dev,
> + "Delete existing rules towards %s & try\n",
> + dsa_to_port(ds, p)->name);
> + return -EBUSY;
> + }
> + }
I think this check should be placed before you enable
PORT_MIRROR_RX/PORT_MIRROR_TX.
> +
> + ret = lan937x_port_cfg(dev, mirror->to_local_port, P_MIRROR_CTRL,
> + PORT_MIRROR_SNIFFER, true);
> + if (ret < 0)
> + return ret;
> +
> + ret = lan937x_cfg(dev, S_MIRROR_CTRL, SW_MIRROR_RX_TX, false);
> +
> + return ret;
You can forgo an assignment to "ret" here and do "return lan937x_cfg(...)"
^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH v3 net-next 09/10] net: dsa: microchip: add support for fdb and mdb management
2021-07-23 17:30 [PATCH v3 net-next 00/10] net: dsa: microchip: DSA driver support for LAN937x switch Prasanna Vengateshan
` (7 preceding siblings ...)
2021-07-23 17:31 ` [PATCH v3 net-next 08/10] net: dsa: microchip: add support for port mirror operations Prasanna Vengateshan
@ 2021-07-23 17:31 ` Prasanna Vengateshan
2021-07-31 15:19 ` Vladimir Oltean
2021-07-23 17:31 ` [PATCH v3 net-next 10/10] net: dsa: microchip: add support for vlan operations Prasanna Vengateshan
9 siblings, 1 reply; 44+ messages in thread
From: Prasanna Vengateshan @ 2021-07-23 17:31 UTC (permalink / raw)
To: andrew, netdev, olteanv, robh+dt
Cc: UNGLinuxDriver, Woojung.Huh, hkallweit1, linux, davem, kuba,
linux-kernel, vivien.didelot, f.fainelli, devicetree
Support for fdb_add, mdb_add, fdb_del, mdb_del and
fdb_dump operations. ALU1 and ALU2 are used for fdb operations.
fdb_add: find any existing entries and update the port map.
if ALU1 write is failed and attempt to write ALU2.
If ALU2 is also failed then exit. Clear WRITE_FAIL for both ALU1
& ALU2.
fdb_del: find the matching entry and clear the respective port
in the port map by writing the ALU tables
fdb_dump: read and dump 2 ALUs upto last entry. ALU_START bit is
used to find the last entry. If the read is timed out, then pass
the error message.
mdb_add: Find the empty slot in ALU and update the port map &
mac address by writing the ALU
mdb_del: find the matching entry and delete the respective port
in port map by writing the ALU
Signed-off-by: Prasanna Vengateshan <prasanna.vengateshan@microchip.com>
---
drivers/net/dsa/microchip/lan937x_main.c | 525 +++++++++++++++++++++++
1 file changed, 525 insertions(+)
diff --git a/drivers/net/dsa/microchip/lan937x_main.c b/drivers/net/dsa/microchip/lan937x_main.c
index 171c46f37fa4..fb780678ef8d 100644
--- a/drivers/net/dsa/microchip/lan937x_main.c
+++ b/drivers/net/dsa/microchip/lan937x_main.c
@@ -15,6 +15,74 @@
#include "ksz_common.h"
#include "lan937x_dev.h"
+static u8 lan937x_get_fid(u16 vid)
+{
+ if (vid > ALU_FID_SIZE)
+ return LAN937X_GET_FID(vid);
+ else
+ return vid;
+}
+
+static int lan937x_read_table(struct ksz_device *dev, u32 *table)
+{
+ int ret;
+
+ /* read alu table */
+ ret = ksz_read32(dev, REG_SW_ALU_VAL_A, &table[0]);
+ if (ret < 0)
+ return ret;
+
+ ret = ksz_read32(dev, REG_SW_ALU_VAL_B, &table[1]);
+ if (ret < 0)
+ return ret;
+
+ ret = ksz_read32(dev, REG_SW_ALU_VAL_C, &table[2]);
+ if (ret < 0)
+ return ret;
+
+ ret = ksz_read32(dev, REG_SW_ALU_VAL_D, &table[3]);
+
+ return ret;
+}
+
+static int lan937x_write_table(struct ksz_device *dev, u32 *table)
+{
+ int ret;
+
+ /* write alu table */
+ ret = ksz_write32(dev, REG_SW_ALU_VAL_A, table[0]);
+ if (ret < 0)
+ return ret;
+
+ ret = ksz_write32(dev, REG_SW_ALU_VAL_B, table[1]);
+ if (ret < 0)
+ return ret;
+
+ ret = ksz_write32(dev, REG_SW_ALU_VAL_C, table[2]);
+ if (ret < 0)
+ return ret;
+
+ ret = ksz_write32(dev, REG_SW_ALU_VAL_D, table[3]);
+
+ return ret;
+}
+
+static int lan937x_wait_alu_ready(int alu, struct ksz_device *dev)
+{
+ unsigned int val;
+
+ return regmap_read_poll_timeout(dev->regmap[2], REG_SW_ALU_CTRL(alu),
+ val, !(val & ALU_START), 10, 1000);
+}
+
+static int lan937x_wait_alu_sta_ready(struct ksz_device *dev)
+{
+ unsigned int val;
+
+ return regmap_read_poll_timeout(dev->regmap[2], REG_SW_ALU_STAT_CTRL__4,
+ val, !(val & ALU_STAT_START), 10, 1000);
+}
+
static enum dsa_tag_protocol lan937x_get_tag_protocol(struct dsa_switch *ds,
int port,
enum dsa_tag_protocol mp)
@@ -129,6 +197,458 @@ static void lan937x_port_stp_state_set(struct dsa_switch *ds, int port,
mutex_unlock(&dev->dev_mutex);
}
+static int lan937x_port_fdb_add(struct dsa_switch *ds, int port,
+ const unsigned char *addr, u16 vid)
+{
+ struct ksz_device *dev = ds->priv;
+ u8 fid = lan937x_get_fid(vid);
+ u32 alu_table[4];
+ int ret, i;
+ u32 data;
+ u8 val;
+
+ mutex_lock(&dev->alu_mutex);
+
+ /* Accessing two ALU tables through loop */
+ for (i = 0; i < ALU_STA_DYN_CNT; i++) {
+ /* find any entry with mac & fid */
+ data = fid << ALU_FID_INDEX_S;
+ data |= ((addr[0] << 8) | addr[1]);
+
+ ret = ksz_write32(dev, REG_SW_ALU_INDEX_0, data);
+ if (ret < 0)
+ break;
+
+ data = ((addr[2] << 24) | (addr[3] << 16));
+ data |= ((addr[4] << 8) | addr[5]);
+
+ ret = ksz_write32(dev, REG_SW_ALU_INDEX_1, data);
+ if (ret < 0)
+ break;
+
+ /* start read operation */
+ ret = ksz_write32(dev, REG_SW_ALU_CTRL(i),
+ ALU_READ | ALU_START);
+ if (ret < 0)
+ break;
+
+ /* wait to be finished */
+ ret = lan937x_wait_alu_ready(i, dev);
+ if (ret < 0) {
+ dev_err(dev->dev, "Failed to read ALU\n");
+ break;
+ }
+
+ /* read ALU entry */
+ ret = lan937x_read_table(dev, alu_table);
+ if (ret < 0) {
+ dev_err(dev->dev, "Failed to read ALU\n");
+ break;
+ }
+
+ /* update ALU entry */
+ alu_table[0] = ALU_V_STATIC_VALID;
+
+ /* update port number */
+ alu_table[1] |= BIT(port);
+
+ if (fid)
+ alu_table[1] |= ALU_V_USE_FID;
+
+ alu_table[2] = (fid << ALU_V_FID_S);
+ alu_table[2] |= ((addr[0] << 8) | addr[1]);
+ alu_table[3] = ((addr[2] << 24) | (addr[3] << 16));
+ alu_table[3] |= ((addr[4] << 8) | addr[5]);
+
+ ret = lan937x_write_table(dev, alu_table);
+ if (ret < 0)
+ break;
+
+ ret = ksz_write32(dev, REG_SW_ALU_CTRL(i),
+ (ALU_WRITE | ALU_START));
+ if (ret < 0)
+ break;
+
+ /* wait to be finished */
+ ret = lan937x_wait_alu_ready(i, dev);
+ if (ret < 0) {
+ dev_err(dev->dev, "Failed to write ALU\n");
+ break;
+ }
+
+ ret = ksz_read8(dev, REG_SW_LUE_INT_STATUS__1, &val);
+ if (ret < 0)
+ break;
+
+ /* ALU2 write failed */
+ if (val & WRITE_FAIL_INT && i == 1)
+ dev_err(dev->dev, "Failed to write ALU\n");
+
+ /* if ALU1 write is failed and attempt to write ALU2,
+ * otherwise exit. Clear Write fail for both ALU1 & ALU2
+ */
+ if (val & WRITE_FAIL_INT) {
+ /* Write to clear the Write Fail */
+ ret = ksz_write8(dev, REG_SW_LUE_INT_STATUS__1,
+ WRITE_FAIL_INT);
+ if (ret < 0)
+ break;
+ } else {
+ break;
+ }
+ }
+
+ mutex_unlock(&dev->alu_mutex);
+
+ return ret;
+}
+
+static int lan937x_port_fdb_del(struct dsa_switch *ds, int port,
+ const unsigned char *addr, u16 vid)
+{
+ struct ksz_device *dev = ds->priv;
+ u8 fid = lan937x_get_fid(vid);
+ u32 alu_table[4];
+ int ret, i;
+ u32 data;
+
+ mutex_lock(&dev->alu_mutex);
+
+ /* Accessing two ALU tables through loop */
+ for (i = 0; i < ALU_STA_DYN_CNT; i++) {
+ /* read any entry with mac & fid */
+ data = fid << ALU_FID_INDEX_S;
+ data |= ((addr[0] << 8) | addr[1]);
+ ret = ksz_write32(dev, REG_SW_ALU_INDEX_0, data);
+ if (ret < 0)
+ break;
+
+ data = ((addr[2] << 24) | (addr[3] << 16));
+ data |= ((addr[4] << 8) | addr[5]);
+ ret = ksz_write32(dev, REG_SW_ALU_INDEX_1, data);
+ if (ret < 0)
+ break;
+
+ /* start read operation */
+ ret = ksz_write32(dev, REG_SW_ALU_CTRL(i),
+ (ALU_READ | ALU_START));
+ if (ret < 0)
+ break;
+
+ /* wait to be finished */
+ ret = lan937x_wait_alu_ready(i, dev);
+ if (ret < 0) {
+ dev_err(dev->dev, "Failed to read ALU\n");
+ break;
+ }
+
+ ret = ksz_read32(dev, REG_SW_ALU_VAL_A, &alu_table[0]);
+ if (ret < 0)
+ break;
+
+ if (alu_table[0] & ALU_V_STATIC_VALID) {
+ /* read ALU entry */
+ ret = lan937x_read_table(dev, alu_table);
+ if (ret < 0) {
+ dev_err(dev->dev, "Failed to read ALU table\n");
+ break;
+ }
+
+ /* clear forwarding port */
+ alu_table[1] &= ~BIT(port);
+
+ /* if there is no port to forward, clear table */
+ if ((alu_table[1] & ALU_V_PORT_MAP) == 0) {
+ alu_table[0] = 0;
+ alu_table[1] = 0;
+ alu_table[2] = 0;
+ alu_table[3] = 0;
+ }
+ } else {
+ alu_table[0] = 0;
+ alu_table[1] = 0;
+ alu_table[2] = 0;
+ alu_table[3] = 0;
+ }
+
+ ret = lan937x_write_table(dev, alu_table);
+ if (ret < 0)
+ break;
+
+ ret = ksz_write32(dev, REG_SW_ALU_CTRL(i),
+ (ALU_WRITE | ALU_START));
+ if (ret < 0)
+ break;
+
+ /* wait to be finished */
+ ret = lan937x_wait_alu_ready(i, dev);
+ if (ret < 0) {
+ dev_err(dev->dev, "Failed to delete ALU Entries\n");
+ break;
+ }
+ }
+
+ mutex_unlock(&dev->alu_mutex);
+
+ return ret;
+}
+
+static void lan937x_convert_alu(struct lan_alu_struct *alu, u32 *alu_table)
+{
+ alu->is_static = !!(alu_table[0] & ALU_V_STATIC_VALID);
+ alu->is_src_filter = !!(alu_table[0] & ALU_V_SRC_FILTER);
+ alu->is_dst_filter = !!(alu_table[0] & ALU_V_DST_FILTER);
+ alu->prio_age = (alu_table[0] >> ALU_V_PRIO_AGE_CNT_S) &
+ ALU_V_PRIO_AGE_CNT_M;
+ alu->mstp = alu_table[0] & ALU_V_MSTP_M;
+
+ alu->is_override = !!(alu_table[1] & ALU_V_OVERRIDE);
+ alu->is_use_fid = !!(alu_table[1] & ALU_V_USE_FID);
+ alu->port_forward = alu_table[1] & ALU_V_PORT_MAP;
+
+ alu->fid = (alu_table[2] >> ALU_V_FID_S) & ALU_V_FID_M;
+
+ alu->mac[0] = (alu_table[2] >> 8) & 0xFF;
+ alu->mac[1] = alu_table[2] & 0xFF;
+ alu->mac[2] = (alu_table[3] >> 24) & 0xFF;
+ alu->mac[3] = (alu_table[3] >> 16) & 0xFF;
+ alu->mac[4] = (alu_table[3] >> 8) & 0xFF;
+ alu->mac[5] = alu_table[3] & 0xFF;
+}
+
+static int lan937x_port_fdb_dump(struct dsa_switch *ds, int port,
+ dsa_fdb_dump_cb_t *cb, void *data)
+{
+ struct ksz_device *dev = ds->priv;
+ struct lan_alu_struct alu;
+ u32 lan937x_data;
+ u32 alu_table[4];
+ int timeout;
+ int ret, i;
+
+ mutex_lock(&dev->alu_mutex);
+
+ /* Accessing two ALU tables through loop */
+ for (i = 0; i < ALU_STA_DYN_CNT; i++) {
+ /* start ALU search */
+ ret = ksz_write32(dev, REG_SW_ALU_CTRL(i),
+ (ALU_START | ALU_SEARCH));
+ if (ret < 0)
+ goto exit;
+
+ do {
+ timeout = 1000;
+ do {
+ ret = ksz_read32(dev, REG_SW_ALU_CTRL(i),
+ &lan937x_data);
+ if (ret < 0)
+ goto exit;
+
+ if ((lan937x_data & ALU_VALID) ||
+ !(lan937x_data & ALU_START))
+ break;
+ usleep_range(1, 10);
+ } while (timeout-- > 0);
+
+ if (!timeout) {
+ dev_err(dev->dev, "Failed to search ALU\n");
+ ret = -ETIMEDOUT;
+ goto exit;
+ }
+
+ /* read ALU table */
+ ret = lan937x_read_table(dev, alu_table);
+ if (ret < 0)
+ goto exit;
+
+ lan937x_convert_alu(&alu, alu_table);
+
+ if (alu.port_forward & BIT(port)) {
+ ret = cb(alu.mac, alu.fid, alu.is_static, data);
+ if (ret)
+ goto exit;
+ }
+ } while (lan937x_data & ALU_START);
+
+exit:
+ /* stop ALU search & continue to next ALU if available */
+ ret = ksz_write32(dev, REG_SW_ALU_CTRL(i), 0);
+ }
+
+ mutex_unlock(&dev->alu_mutex);
+
+ return ret;
+}
+
+static int lan937x_port_mdb_add(struct dsa_switch *ds, int port,
+ const struct switchdev_obj_port_mdb *mdb)
+{
+ struct ksz_device *dev = ds->priv;
+ u8 fid = lan937x_get_fid(mdb->vid);
+ u32 static_table[4];
+ u32 mac_hi, mac_lo;
+ int index, ret;
+ u32 data;
+
+ mac_hi = ((mdb->addr[0] << 8) | mdb->addr[1]);
+ mac_lo = ((mdb->addr[2] << 24) | (mdb->addr[3] << 16));
+ mac_lo |= ((mdb->addr[4] << 8) | mdb->addr[5]);
+
+ mutex_lock(&dev->alu_mutex);
+
+ /* Access the entries in the table */
+ for (index = 0; index < dev->num_statics; index++) {
+ /* find empty slot first */
+ data = (index << ALU_STAT_INDEX_S) |
+ ALU_STAT_READ | ALU_STAT_START;
+
+ ret = ksz_write32(dev, REG_SW_ALU_STAT_CTRL__4, data);
+ if (ret < 0)
+ goto exit;
+
+ /* wait to be finished */
+ ret = lan937x_wait_alu_sta_ready(dev);
+ if (ret < 0) {
+ dev_err(dev->dev, "Failed to read ALU STATIC\n");
+ goto exit;
+ }
+
+ /* read ALU static table */
+ ret = lan937x_read_table(dev, static_table);
+ if (ret < 0)
+ goto exit;
+
+ if (static_table[0] & ALU_V_STATIC_VALID) {
+ /* check this has same fid & mac address */
+ if (((static_table[2] >> ALU_V_FID_S) == fid) &&
+ ((static_table[2] & ALU_V_MAC_ADDR_HI) == mac_hi) &&
+ static_table[3] == mac_lo) {
+ /* found matching one */
+ break;
+ }
+ } else {
+ /* found empty one */
+ break;
+ }
+ }
+
+ /* no available entry */
+ if (index == dev->num_statics) {
+ ret = -ENOSPC;
+ goto exit;
+ }
+
+ /* add entry */
+ static_table[0] = ALU_V_STATIC_VALID;
+
+ static_table[1] |= BIT(port);
+ if (fid)
+ static_table[1] |= ALU_V_USE_FID;
+ static_table[2] = (fid << ALU_V_FID_S);
+ static_table[2] |= mac_hi;
+ static_table[3] = mac_lo;
+
+ ret = lan937x_write_table(dev, static_table);
+ if (ret < 0)
+ goto exit;
+
+ data = (index << ALU_STAT_INDEX_S) | ALU_STAT_START;
+ ret = ksz_write32(dev, REG_SW_ALU_STAT_CTRL__4, data);
+ if (ret < 0)
+ goto exit;
+
+ /* wait to be finished */
+ ret = lan937x_wait_alu_sta_ready(dev);
+ if (ret < 0)
+ dev_err(dev->dev, "Failed to read ALU STATIC\n");
+
+exit:
+ mutex_unlock(&dev->alu_mutex);
+ return ret;
+}
+
+static int lan937x_port_mdb_del(struct dsa_switch *ds, int port,
+ const struct switchdev_obj_port_mdb *mdb)
+{
+ struct ksz_device *dev = ds->priv;
+ u8 fid = lan937x_get_fid(mdb->vid);
+ u32 static_table[4];
+ u32 mac_hi, mac_lo;
+ int index, ret;
+ u32 data;
+
+ mac_hi = ((mdb->addr[0] << 8) | mdb->addr[1]);
+ mac_lo = ((mdb->addr[2] << 24) | (mdb->addr[3] << 16));
+ mac_lo |= ((mdb->addr[4] << 8) | mdb->addr[5]);
+
+ mutex_lock(&dev->alu_mutex);
+
+ /* Access the entries in the table */
+ for (index = 0; index < dev->num_statics; index++) {
+ data = (index << ALU_STAT_INDEX_S) |
+ ALU_STAT_READ | ALU_STAT_START;
+ ret = ksz_write32(dev, REG_SW_ALU_STAT_CTRL__4, data);
+ if (ret < 0)
+ goto exit;
+
+ /* wait to be finished */
+ ret = lan937x_wait_alu_sta_ready(dev);
+ if (ret < 0) {
+ dev_err(dev->dev, "Failed to read ALU STATIC\n");
+ goto exit;
+ }
+
+ /* read ALU static table */
+ ret = lan937x_read_table(dev, static_table);
+ if (ret < 0)
+ goto exit;
+
+ if (static_table[0] & ALU_V_STATIC_VALID) {
+ /* check this has same fid & mac address */
+ if (((static_table[2] >> ALU_V_FID_S) == fid) &&
+ ((static_table[2] & ALU_V_MAC_ADDR_HI) == mac_hi) &&
+ static_table[3] == mac_lo) {
+ /* found matching one */
+ break;
+ }
+ }
+ }
+
+ /* no available entry */
+ if (index == dev->num_statics)
+ goto exit;
+
+ /* clear port based on port arg */
+ static_table[1] &= ~BIT(port);
+
+ if ((static_table[1] & ALU_V_PORT_MAP) == 0) {
+ /* delete entry */
+ static_table[0] = 0;
+ static_table[1] = 0;
+ static_table[2] = 0;
+ static_table[3] = 0;
+ }
+
+ ret = lan937x_write_table(dev, static_table);
+ if (ret < 0)
+ goto exit;
+
+ data = (index << ALU_STAT_INDEX_S) | ALU_STAT_START;
+ ret = ksz_write32(dev, REG_SW_ALU_STAT_CTRL__4, data);
+ if (ret < 0)
+ goto exit;
+
+ /* wait to be finished */
+ ret = lan937x_wait_alu_sta_ready(dev);
+ if (ret < 0)
+ dev_err(dev->dev, "Failed to read ALU STATIC\n");
+
+exit:
+ mutex_unlock(&dev->alu_mutex);
+
+ return ret;
+}
+
static int lan937x_port_mirror_add(struct dsa_switch *ds, int port,
struct dsa_mall_mirror_tc_entry *mirror,
bool ingress)
@@ -487,6 +1007,11 @@ const struct dsa_switch_ops lan937x_switch_ops = {
.port_bridge_leave = ksz_port_bridge_leave,
.port_stp_state_set = lan937x_port_stp_state_set,
.port_fast_age = ksz_port_fast_age,
+ .port_fdb_dump = lan937x_port_fdb_dump,
+ .port_fdb_add = lan937x_port_fdb_add,
+ .port_fdb_del = lan937x_port_fdb_del,
+ .port_mdb_add = lan937x_port_mdb_add,
+ .port_mdb_del = lan937x_port_mdb_del,
.port_mirror_add = lan937x_port_mirror_add,
.port_mirror_del = lan937x_port_mirror_del,
.port_max_mtu = lan937x_get_max_mtu,
--
2.27.0
^ permalink raw reply related [flat|nested] 44+ messages in thread* Re: [PATCH v3 net-next 09/10] net: dsa: microchip: add support for fdb and mdb management
2021-07-23 17:31 ` [PATCH v3 net-next 09/10] net: dsa: microchip: add support for fdb and mdb management Prasanna Vengateshan
@ 2021-07-31 15:19 ` Vladimir Oltean
0 siblings, 0 replies; 44+ messages in thread
From: Vladimir Oltean @ 2021-07-31 15:19 UTC (permalink / raw)
To: Prasanna Vengateshan
Cc: andrew, netdev, robh+dt, UNGLinuxDriver, Woojung.Huh, hkallweit1,
linux, davem, kuba, linux-kernel, vivien.didelot, f.fainelli,
devicetree
On Fri, Jul 23, 2021 at 11:01:07PM +0530, Prasanna Vengateshan wrote:
> +static int lan937x_port_fdb_add(struct dsa_switch *ds, int port,
> + const unsigned char *addr, u16 vid)
> +{
> + struct ksz_device *dev = ds->priv;
> + u8 fid = lan937x_get_fid(vid);
> + u32 alu_table[4];
> + int ret, i;
> + u32 data;
> + u8 val;
> +
> + mutex_lock(&dev->alu_mutex);
> +
> + /* Accessing two ALU tables through loop */
> + for (i = 0; i < ALU_STA_DYN_CNT; i++) {
> + /* find any entry with mac & fid */
> + data = fid << ALU_FID_INDEX_S;
> + data |= ((addr[0] << 8) | addr[1]);
Maybe upper_32_bits(ether_addr_to_u64(addr)) and
lower_32_bits(ether_addr_to_u64(addr)) would be slightly easier on the
eye?
> + if (alu_table[0] & ALU_V_STATIC_VALID) {
> + /* read ALU entry */
> + ret = lan937x_read_table(dev, alu_table);
> + if (ret < 0) {
> + dev_err(dev->dev, "Failed to read ALU table\n");
> + break;
> + }
> +
> + /* clear forwarding port */
> + alu_table[1] &= ~BIT(port);
> +
> + /* if there is no port to forward, clear table */
> + if ((alu_table[1] & ALU_V_PORT_MAP) == 0) {
> + alu_table[0] = 0;
> + alu_table[1] = 0;
> + alu_table[2] = 0;
> + alu_table[3] = 0;
memset?
> + }
> + } else {
> + alu_table[0] = 0;
> + alu_table[1] = 0;
> + alu_table[2] = 0;
> + alu_table[3] = 0;
> + }
> +
> + ret = lan937x_write_table(dev, alu_table);
> + if (ret < 0)
> + break;
^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH v3 net-next 10/10] net: dsa: microchip: add support for vlan operations
2021-07-23 17:30 [PATCH v3 net-next 00/10] net: dsa: microchip: DSA driver support for LAN937x switch Prasanna Vengateshan
` (8 preceding siblings ...)
2021-07-23 17:31 ` [PATCH v3 net-next 09/10] net: dsa: microchip: add support for fdb and mdb management Prasanna Vengateshan
@ 2021-07-23 17:31 ` Prasanna Vengateshan
2021-07-31 15:08 ` Vladimir Oltean
9 siblings, 1 reply; 44+ messages in thread
From: Prasanna Vengateshan @ 2021-07-23 17:31 UTC (permalink / raw)
To: andrew, netdev, olteanv, robh+dt
Cc: UNGLinuxDriver, Woojung.Huh, hkallweit1, linux, davem, kuba,
linux-kernel, vivien.didelot, f.fainelli, devicetree
Support for VLAN add, del, prepare and filtering operations.
The VLAN aware is a global setting. Mixed vlan filterings
are not supported. vlan_filtering_is_global is made as true
in lan937x_setup function.
Signed-off-by: Prasanna Vengateshan <prasanna.vengateshan@microchip.com>
---
drivers/net/dsa/microchip/lan937x_main.c | 198 +++++++++++++++++++++++
1 file changed, 198 insertions(+)
diff --git a/drivers/net/dsa/microchip/lan937x_main.c b/drivers/net/dsa/microchip/lan937x_main.c
index fb780678ef8d..963b066a8ad1 100644
--- a/drivers/net/dsa/microchip/lan937x_main.c
+++ b/drivers/net/dsa/microchip/lan937x_main.c
@@ -15,6 +15,14 @@
#include "ksz_common.h"
#include "lan937x_dev.h"
+static int lan937x_wait_vlan_ctrl_ready(struct ksz_device *dev)
+{
+ unsigned int val;
+
+ return regmap_read_poll_timeout(dev->regmap[0], REG_SW_VLAN_CTRL, val,
+ !(val & VLAN_START), 10, 1000);
+}
+
static u8 lan937x_get_fid(u16 vid)
{
if (vid > ALU_FID_SIZE)
@@ -23,6 +31,97 @@ static u8 lan937x_get_fid(u16 vid)
return vid;
}
+static int lan937x_get_vlan_table(struct ksz_device *dev, u16 vid,
+ struct lan937x_vlan *vlan_entry)
+{
+ u32 data;
+ int ret;
+
+ mutex_lock(&dev->vlan_mutex);
+
+ ret = ksz_write16(dev, REG_SW_VLAN_ENTRY_INDEX__2, vid & VLAN_INDEX_M);
+ if (ret < 0)
+ goto exit;
+
+ ret = ksz_write8(dev, REG_SW_VLAN_CTRL, VLAN_READ | VLAN_START);
+ if (ret < 0)
+ goto exit;
+
+ /* wait to be cleared */
+ ret = lan937x_wait_vlan_ctrl_ready(dev);
+ if (ret < 0)
+ goto exit;
+
+ ret = ksz_read32(dev, REG_SW_VLAN_ENTRY__4, &data);
+ if (ret < 0)
+ goto exit;
+
+ vlan_entry->valid = !!(data & VLAN_VALID);
+ vlan_entry->fid = data & VLAN_FID_M;
+
+ ret = ksz_read32(dev, REG_SW_VLAN_ENTRY_UNTAG__4,
+ &vlan_entry->untag_prtmap);
+ if (ret < 0)
+ goto exit;
+
+ ret = ksz_read32(dev, REG_SW_VLAN_ENTRY_PORTS__4,
+ &vlan_entry->fwd_map);
+ if (ret < 0)
+ goto exit;
+
+ ret = ksz_write8(dev, REG_SW_VLAN_CTRL, 0);
+ if (ret < 0)
+ goto exit;
+
+exit:
+ mutex_unlock(&dev->vlan_mutex);
+
+ return ret;
+}
+
+static int lan937x_set_vlan_table(struct ksz_device *dev, u16 vid,
+ struct lan937x_vlan *vlan_entry)
+{
+ u32 data;
+ int ret;
+
+ mutex_lock(&dev->vlan_mutex);
+
+ data = vlan_entry->valid ? VLAN_VALID : 0;
+ data |= vlan_entry->fid;
+
+ ret = ksz_write32(dev, REG_SW_VLAN_ENTRY__4, data);
+ if (ret < 0)
+ goto exit;
+
+ ret = ksz_write32(dev, REG_SW_VLAN_ENTRY_UNTAG__4,
+ vlan_entry->untag_prtmap);
+ if (ret < 0)
+ goto exit;
+
+ ret = ksz_write32(dev, REG_SW_VLAN_ENTRY_PORTS__4, vlan_entry->fwd_map);
+ if (ret < 0)
+ goto exit;
+
+ ret = ksz_write16(dev, REG_SW_VLAN_ENTRY_INDEX__2, vid & VLAN_INDEX_M);
+ if (ret < 0)
+ goto exit;
+
+ ret = ksz_write8(dev, REG_SW_VLAN_CTRL, VLAN_START | VLAN_WRITE);
+ if (ret < 0)
+ goto exit;
+
+ /* wait to be cleared */
+ ret = lan937x_wait_vlan_ctrl_ready(dev);
+ if (ret < 0)
+ goto exit;
+
+exit:
+ mutex_unlock(&dev->vlan_mutex);
+
+ return ret;
+}
+
static int lan937x_read_table(struct ksz_device *dev, u32 *table)
{
int ret;
@@ -197,6 +296,102 @@ static void lan937x_port_stp_state_set(struct dsa_switch *ds, int port,
mutex_unlock(&dev->dev_mutex);
}
+static int lan937x_port_vlan_filtering(struct dsa_switch *ds, int port,
+ bool flag,
+ struct netlink_ext_ack *extack)
+{
+ struct ksz_device *dev = ds->priv;
+ int ret;
+
+ ret = lan937x_cfg(dev, REG_SW_LUE_CTRL_0, SW_VLAN_ENABLE,
+ flag);
+
+ return ret;
+}
+
+static int lan937x_port_vlan_add(struct dsa_switch *ds, int port,
+ const struct switchdev_obj_port_vlan *vlan,
+ struct netlink_ext_ack *extack)
+{
+ bool untagged = vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED;
+ struct ksz_device *dev = ds->priv;
+ struct lan937x_vlan vlan_entry;
+ int ret;
+
+ ret = lan937x_get_vlan_table(dev, vlan->vid, &vlan_entry);
+ if (ret < 0) {
+ NL_SET_ERR_MSG_MOD(extack, "Failed to get vlan table\n");
+ return ret;
+ }
+
+ vlan_entry.fid = lan937x_get_fid(vlan->vid);
+ vlan_entry.valid = true;
+
+ /* set/clear switch port when updating vlan table registers */
+ if (untagged)
+ vlan_entry.untag_prtmap |= BIT(port);
+ else
+ vlan_entry.untag_prtmap &= ~BIT(port);
+
+ vlan_entry.fwd_map |= BIT(port);
+
+ ret = lan937x_set_vlan_table(dev, vlan->vid, &vlan_entry);
+ if (ret < 0) {
+ NL_SET_ERR_MSG_MOD(extack, "Failed to set vlan table\n");
+ return ret;
+ }
+
+ /* change PVID */
+ if (vlan->flags & BRIDGE_VLAN_INFO_PVID) {
+ ret = lan937x_pwrite16(dev, port, REG_PORT_DEFAULT_VID,
+ vlan->vid);
+ if (ret < 0) {
+ NL_SET_ERR_MSG_MOD(extack, "Failed to set pvid\n");
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
+static int lan937x_port_vlan_del(struct dsa_switch *ds, int port,
+ const struct switchdev_obj_port_vlan *vlan)
+{
+ bool untagged = vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED;
+ struct ksz_device *dev = ds->priv;
+ struct lan937x_vlan vlan_entry;
+ u16 pvid;
+ int ret;
+
+ lan937x_pread16(dev, port, REG_PORT_DEFAULT_VID, &pvid);
+ pvid &= 0xFFF;
+
+ ret = lan937x_get_vlan_table(dev, vlan->vid, &vlan_entry);
+ if (ret < 0) {
+ dev_err(dev->dev, "Failed to get vlan table\n");
+ return ret;
+ }
+ /* clear port fwd map */
+ vlan_entry.fwd_map &= ~BIT(port);
+
+ if (untagged)
+ vlan_entry.untag_prtmap &= ~BIT(port);
+
+ ret = lan937x_set_vlan_table(dev, vlan->vid, &vlan_entry);
+ if (ret < 0) {
+ dev_err(dev->dev, "Failed to set vlan table\n");
+ return ret;
+ }
+
+ ret = lan937x_pwrite16(dev, port, REG_PORT_DEFAULT_VID, pvid);
+ if (ret < 0) {
+ dev_err(dev->dev, "Failed to set pvid\n");
+ return ret;
+ }
+
+ return 0;
+}
+
static int lan937x_port_fdb_add(struct dsa_switch *ds, int port,
const unsigned char *addr, u16 vid)
{
@@ -1007,6 +1202,9 @@ const struct dsa_switch_ops lan937x_switch_ops = {
.port_bridge_leave = ksz_port_bridge_leave,
.port_stp_state_set = lan937x_port_stp_state_set,
.port_fast_age = ksz_port_fast_age,
+ .port_vlan_filtering = lan937x_port_vlan_filtering,
+ .port_vlan_add = lan937x_port_vlan_add,
+ .port_vlan_del = lan937x_port_vlan_del,
.port_fdb_dump = lan937x_port_fdb_dump,
.port_fdb_add = lan937x_port_fdb_add,
.port_fdb_del = lan937x_port_fdb_del,
--
2.27.0
^ permalink raw reply related [flat|nested] 44+ messages in thread* Re: [PATCH v3 net-next 10/10] net: dsa: microchip: add support for vlan operations
2021-07-23 17:31 ` [PATCH v3 net-next 10/10] net: dsa: microchip: add support for vlan operations Prasanna Vengateshan
@ 2021-07-31 15:08 ` Vladimir Oltean
2021-08-02 10:48 ` Prasanna Vengateshan
0 siblings, 1 reply; 44+ messages in thread
From: Vladimir Oltean @ 2021-07-31 15:08 UTC (permalink / raw)
To: Prasanna Vengateshan
Cc: andrew, netdev, robh+dt, UNGLinuxDriver, Woojung.Huh, hkallweit1,
linux, davem, kuba, linux-kernel, vivien.didelot, f.fainelli,
devicetree
On Fri, Jul 23, 2021 at 11:01:08PM +0530, Prasanna Vengateshan wrote:
> +static int lan937x_port_vlan_add(struct dsa_switch *ds, int port,
> + const struct switchdev_obj_port_vlan *vlan,
> + struct netlink_ext_ack *extack)
> +{
> + bool untagged = vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED;
> + struct ksz_device *dev = ds->priv;
> + struct lan937x_vlan vlan_entry;
> + int ret;
> +
> + ret = lan937x_get_vlan_table(dev, vlan->vid, &vlan_entry);
> + if (ret < 0) {
> + NL_SET_ERR_MSG_MOD(extack, "Failed to get vlan table\n");
The NL_SET_ERR_MSG_MOD function already adds the \n at the end.
> + return ret;
> + }
> +
> + vlan_entry.fid = lan937x_get_fid(vlan->vid);
> + vlan_entry.valid = true;
> +
> + /* set/clear switch port when updating vlan table registers */
> + if (untagged)
> + vlan_entry.untag_prtmap |= BIT(port);
> + else
> + vlan_entry.untag_prtmap &= ~BIT(port);
> +
> + vlan_entry.fwd_map |= BIT(port);
> +
> + ret = lan937x_set_vlan_table(dev, vlan->vid, &vlan_entry);
> + if (ret < 0) {
> + NL_SET_ERR_MSG_MOD(extack, "Failed to set vlan table\n");
> + return ret;
> + }
> +
> + /* change PVID */
> + if (vlan->flags & BRIDGE_VLAN_INFO_PVID) {
> + ret = lan937x_pwrite16(dev, port, REG_PORT_DEFAULT_VID,
> + vlan->vid);
> + if (ret < 0) {
> + NL_SET_ERR_MSG_MOD(extack, "Failed to set pvid\n");
> + return ret;
> + }
> + }
> +
> + return 0;
> +}
Side question: do you think the ds->configure_vlan_while_not_filtering = false
from ksz9477.c and ksz8795.c serve any purpose, considering that you did
not need this setting for lan937x? If not, could you please send a patch
to remove that setting from those 2 other KSZ drivers? Thanks.
^ permalink raw reply [flat|nested] 44+ messages in thread* Re: [PATCH v3 net-next 10/10] net: dsa: microchip: add support for vlan operations
2021-07-31 15:08 ` Vladimir Oltean
@ 2021-08-02 10:48 ` Prasanna Vengateshan
0 siblings, 0 replies; 44+ messages in thread
From: Prasanna Vengateshan @ 2021-08-02 10:48 UTC (permalink / raw)
To: Vladimir Oltean
Cc: andrew, netdev, robh+dt, UNGLinuxDriver, Woojung.Huh, hkallweit1,
linux, davem, kuba, linux-kernel, vivien.didelot, f.fainelli,
devicetree
On Sat, 2021-07-31 at 18:08 +0300, Vladimir Oltean wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
>
> On Fri, Jul 23, 2021 at 11:01:08PM +0530, Prasanna Vengateshan wrote:
> > +static int lan937x_port_vlan_add(struct dsa_switch *ds, int port,
> > + const struct switchdev_obj_port_vlan *vlan,
> > + struct netlink_ext_ack *extack)
> > +{
> > + bool untagged = vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED;
> > + struct ksz_device *dev = ds->priv;
> > + struct lan937x_vlan vlan_entry;
> > + int ret;
> > +
> > + ret = lan937x_get_vlan_table(dev, vlan->vid, &vlan_entry);
> > + if (ret < 0) {
> > + NL_SET_ERR_MSG_MOD(extack, "Failed to get vlan table\n");
>
> The NL_SET_ERR_MSG_MOD function already adds the \n at the end.
i will remove \n from other places as well for NL_SET_ERR_MSG_MOD.
>
> > + return ret;
> >
> > +
> > + /* change PVID */
> > + if (vlan->flags & BRIDGE_VLAN_INFO_PVID) {
> > + ret = lan937x_pwrite16(dev, port, REG_PORT_DEFAULT_VID,
> > + vlan->vid);
> > + if (ret < 0) {
> > + NL_SET_ERR_MSG_MOD(extack, "Failed to set pvid\n");
> > + return ret;
> > + }
> > + }
> > +
> > + return 0;
> > +}
>
> Side question: do you think the ds->configure_vlan_while_not_filtering = false
> from ksz9477.c and ksz8795.c serve any purpose, considering that you did
> not need this setting for lan937x? If not, could you please send a patch
> to remove that setting from those 2 other KSZ drivers? Thanks.
Sure, I will add this patch in my next submission.
^ permalink raw reply [flat|nested] 44+ messages in thread