[RFC,net-next,03/11] net: dsa: microchip: lan937x: enable cascade port
Commit Message
Get index of cascaded port (if any) from device tree and enable
the feature. These ports referenced as dev->dsa_port and will be
used for processing further based on cascaded connection.
For the second switch in cascaded connection, no dev->cpu_port will
be assigned, and same happens for dev->dsa_port variable for switches
without cascading. For the single switch design, there is no way
dev->cpu_port will be unassigned. But coming to cascaded connection,
it can be unassigned, and they will be having value zero. Keeping the
initial value as zero will create error in other features like port
forwarding since DSA will misunderstood these as port index zero. So
keep the default values as 0xFF which is of invalid value so that if
nothing assigned, taking bitmap of the cpu_port or dsa_port will not
cause any harm.
Signed-off-by: Rakesh Sankaranarayanan <rakesh.sankaranarayanan@microchip.com>
---
drivers/net/dsa/microchip/ksz_common.c | 4 +++
drivers/net/dsa/microchip/ksz_common.h | 2 ++
drivers/net/dsa/microchip/lan937x.h | 1 +
drivers/net/dsa/microchip/lan937x_main.c | 31 ++++++++++++++++++++++++
drivers/net/dsa/microchip/lan937x_reg.h | 3 +++
5 files changed, 41 insertions(+)
Comments
On Thu, Feb 02, 2023 at 06:29:22PM +0530, Rakesh Sankaranarayanan wrote:
> diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
> index aab60f2587bf..c3c3eee178f4 100644
> --- a/drivers/net/dsa/microchip/ksz_common.h
> +++ b/drivers/net/dsa/microchip/ksz_common.h
> @@ -147,6 +147,7 @@ struct ksz_device {
> u32 chip_id;
> u8 chip_rev;
> int cpu_port; /* port connected to CPU */
> + int dsa_port; /* Port used as cascaded port */
nitpick: since "dsa_port" is typically a keyword that I use for the DSA
framework's generic port structure (struct dsa_port *dp), I would appreciate
if you could name this in some other way, like "cascade_port". I don't
believe the explanatory comment would even be necessary in that case.
And btw, the comment is inconsistent in alignment (tabs vs spaces) with
the line immediately above it.
> u32 smi_index;
> int phy_port_cnt;
> phy_interface_t compat_interface;
> @@ -358,6 +359,7 @@ struct ksz_dev_ops {
> void (*setup_rgmii_delay)(struct ksz_device *dev, int port);
> int (*tc_cbs_set_cinc)(struct ksz_device *dev, int port, u32 val);
> void (*config_cpu_port)(struct dsa_switch *ds);
> + void (*config_dsa_port)(struct dsa_switch *ds);
> int (*enable_stp_addr)(struct ksz_device *dev);
> int (*reset)(struct ksz_device *dev);
> int (*init)(struct ksz_device *dev);
> diff --git a/drivers/net/dsa/microchip/lan937x.h b/drivers/net/dsa/microchip/lan937x.h
> index 3388d91dbc44..ef84abc31556 100644
> --- a/drivers/net/dsa/microchip/lan937x.h
> +++ b/drivers/net/dsa/microchip/lan937x.h
> @@ -11,6 +11,7 @@ int lan937x_setup(struct dsa_switch *ds);
> void lan937x_teardown(struct dsa_switch *ds);
> void lan937x_port_setup(struct ksz_device *dev, int port, bool cpu_port);
> void lan937x_config_cpu_port(struct dsa_switch *ds);
> +void lan937x_config_dsa_port(struct dsa_switch *ds);
> int lan937x_switch_init(struct ksz_device *dev);
> void lan937x_switch_exit(struct ksz_device *dev);
> int lan937x_r_phy(struct ksz_device *dev, u16 addr, u16 reg, u16 *data);
> diff --git a/drivers/net/dsa/microchip/lan937x_main.c b/drivers/net/dsa/microchip/lan937x_main.c
> index 399a3905e6ca..5108a3f4bf76 100644
> --- a/drivers/net/dsa/microchip/lan937x_main.c
> +++ b/drivers/net/dsa/microchip/lan937x_main.c
> @@ -205,11 +205,42 @@ void lan937x_port_setup(struct ksz_device *dev, int port, bool cpu_port)
> dev->dev_ops->cfg_port_member(dev, port, member);
> }
>
> +void lan937x_config_dsa_port(struct dsa_switch *ds)
> +{
> + struct ksz_device *dev = ds->priv;
> + struct dsa_port *dp;
> +
> + dev->dsa_port = 0xFF;
> +
> + dsa_switch_for_each_port(dp, ds) {
> + if (dsa_is_dsa_port(ds, dp->index)) {
Would be good if you introduced dsa_switch_for_each_dsa_port() as a
separate patch, and used it here first.
Not sure if you realize this, but dsa_is_dsa_port() contains a list
iteration hidden in it, in dsa_to_port(). So dsa_switch_for_each_port()
-> dsa_is_dsa_port() effectively does an O(n^2) list walk (apart from
uselessly increasing the code indentation).
> + ksz_rmw32(dev, REG_SW_CASCADE_MODE_CTL,
> + CASCADE_PORT_SEL, dp->index);
> + dev->dsa_port = dp->index;
> +
> + /* Tail tag should be enabled for switch 0
> + * in cascaded connection.
> + */
> + if (dev->smi_index == 0) {
> + lan937x_port_cfg(dev, dp->index, REG_PORT_CTRL_0,
> + PORT_TAIL_TAG_ENABLE, true);
> + }
> +
> + /* Frame check length should be disabled for cascaded ports */
> + lan937x_port_cfg(dev, dp->index, REG_PORT_MAC_CTRL_0,
> + PORT_CHECK_LENGTH, false);
break?
> + }
> + }
> +}
> +
> void lan937x_config_cpu_port(struct dsa_switch *ds)
> {
> struct ksz_device *dev = ds->priv;
> struct dsa_port *dp;
>
> + /* Initializing cpu_port parameter into invalid value */
> + dev->cpu_port = 0xFF;
> +
> dsa_switch_for_each_cpu_port(dp, ds) {
> if (dev->info->cpu_ports & (1 << dp->index)) {
> dev->cpu_port = dp->index;
> diff --git a/drivers/net/dsa/microchip/lan937x_reg.h b/drivers/net/dsa/microchip/lan937x_reg.h
> index 45b606b6429f..4f30bc12f7a9 100644
> --- a/drivers/net/dsa/microchip/lan937x_reg.h
> +++ b/drivers/net/dsa/microchip/lan937x_reg.h
> @@ -32,6 +32,9 @@
> #define REG_SW_PORT_INT_STATUS__4 0x0018
> #define REG_SW_PORT_INT_MASK__4 0x001C
>
> +#define REG_SW_CASCADE_MODE_CTL 0x0030
> +#define CASCADE_PORT_SEL 7
> +
> /* 1 - Global */
> #define REG_SW_GLOBAL_OUTPUT_CTRL__1 0x0103
> #define SW_CLK125_ENB BIT(1)
> --
> 2.34.1
>
@@ -292,6 +292,7 @@ static const struct ksz_dev_ops lan937x_dev_ops = {
.change_mtu = lan937x_change_mtu,
.phylink_mac_link_up = ksz9477_phylink_mac_link_up,
.config_cpu_port = lan937x_config_cpu_port,
+ .config_dsa_port = lan937x_config_dsa_port,
.tc_cbs_set_cinc = lan937x_tc_cbs_set_cinc,
.enable_stp_addr = ksz9477_enable_stp_addr,
.reset = lan937x_reset_switch,
@@ -2095,6 +2096,9 @@ static int ksz_setup(struct dsa_switch *ds)
dev->dev_ops->config_cpu_port(ds);
+ if (dev->dev_ops->config_dsa_port)
+ dev->dev_ops->config_dsa_port(ds);
+
dev->dev_ops->enable_stp_addr(dev);
ds->num_tx_queues = dev->info->num_tx_queues;
@@ -147,6 +147,7 @@ struct ksz_device {
u32 chip_id;
u8 chip_rev;
int cpu_port; /* port connected to CPU */
+ int dsa_port; /* Port used as cascaded port */
u32 smi_index;
int phy_port_cnt;
phy_interface_t compat_interface;
@@ -358,6 +359,7 @@ struct ksz_dev_ops {
void (*setup_rgmii_delay)(struct ksz_device *dev, int port);
int (*tc_cbs_set_cinc)(struct ksz_device *dev, int port, u32 val);
void (*config_cpu_port)(struct dsa_switch *ds);
+ void (*config_dsa_port)(struct dsa_switch *ds);
int (*enable_stp_addr)(struct ksz_device *dev);
int (*reset)(struct ksz_device *dev);
int (*init)(struct ksz_device *dev);
@@ -11,6 +11,7 @@ int lan937x_setup(struct dsa_switch *ds);
void lan937x_teardown(struct dsa_switch *ds);
void lan937x_port_setup(struct ksz_device *dev, int port, bool cpu_port);
void lan937x_config_cpu_port(struct dsa_switch *ds);
+void lan937x_config_dsa_port(struct dsa_switch *ds);
int lan937x_switch_init(struct ksz_device *dev);
void lan937x_switch_exit(struct ksz_device *dev);
int lan937x_r_phy(struct ksz_device *dev, u16 addr, u16 reg, u16 *data);
@@ -205,11 +205,42 @@ void lan937x_port_setup(struct ksz_device *dev, int port, bool cpu_port)
dev->dev_ops->cfg_port_member(dev, port, member);
}
+void lan937x_config_dsa_port(struct dsa_switch *ds)
+{
+ struct ksz_device *dev = ds->priv;
+ struct dsa_port *dp;
+
+ dev->dsa_port = 0xFF;
+
+ dsa_switch_for_each_port(dp, ds) {
+ if (dsa_is_dsa_port(ds, dp->index)) {
+ ksz_rmw32(dev, REG_SW_CASCADE_MODE_CTL,
+ CASCADE_PORT_SEL, dp->index);
+ dev->dsa_port = dp->index;
+
+ /* Tail tag should be enabled for switch 0
+ * in cascaded connection.
+ */
+ if (dev->smi_index == 0) {
+ lan937x_port_cfg(dev, dp->index, REG_PORT_CTRL_0,
+ PORT_TAIL_TAG_ENABLE, true);
+ }
+
+ /* Frame check length should be disabled for cascaded ports */
+ lan937x_port_cfg(dev, dp->index, REG_PORT_MAC_CTRL_0,
+ PORT_CHECK_LENGTH, false);
+ }
+ }
+}
+
void lan937x_config_cpu_port(struct dsa_switch *ds)
{
struct ksz_device *dev = ds->priv;
struct dsa_port *dp;
+ /* Initializing cpu_port parameter into invalid value */
+ dev->cpu_port = 0xFF;
+
dsa_switch_for_each_cpu_port(dp, ds) {
if (dev->info->cpu_ports & (1 << dp->index)) {
dev->cpu_port = dp->index;
@@ -32,6 +32,9 @@
#define REG_SW_PORT_INT_STATUS__4 0x0018
#define REG_SW_PORT_INT_MASK__4 0x001C
+#define REG_SW_CASCADE_MODE_CTL 0x0030
+#define CASCADE_PORT_SEL 7
+
/* 1 - Global */
#define REG_SW_GLOBAL_OUTPUT_CTRL__1 0x0103
#define SW_CLK125_ENB BIT(1)