2024-04-03 09:30:18

by Oleksij Rempel

[permalink] [raw]
Subject: [PATCH net-next v2 2/9] net: dsa: microchip: add IPV information support

Most of Microchip KSZ switches use Internal Priority Value associated
with every frame. For example, it is possible to map any VLAN PCP or
DSCP value to IPV and at the end, map IPV to a queue.

Since amount of IPVs is not equal to amount of queues, add this
information and make use of it in some functions.

Signed-off-by: Oleksij Rempel <[email protected]>
Acked-by: Arun Ramadoss <[email protected]>
---
drivers/net/dsa/microchip/ksz_common.c | 28 ++++++++++++++++++++++++--
drivers/net/dsa/microchip/ksz_common.h | 2 +-
2 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 42330e8fd26e7..cf81739d91dae 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -1194,6 +1194,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
.port_cnt = 3, /* total port count */
.port_nirqs = 3,
.num_tx_queues = 4,
+ .max_ipvs = 8,
.tc_cbs_supported = true,
.tc_ets_supported = true,
.ops = &ksz9477_dev_ops,
@@ -1223,6 +1224,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
.cpu_ports = 0x10, /* can be configured as cpu port */
.port_cnt = 5, /* total cpu and user ports */
.num_tx_queues = 4,
+ .max_ipvs = 4,
.ops = &ksz8_dev_ops,
.ksz87xx_eee_link_erratum = true,
.mib_names = ksz9477_mib_names,
@@ -1262,6 +1264,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
.cpu_ports = 0x10, /* can be configured as cpu port */
.port_cnt = 5, /* total cpu and user ports */
.num_tx_queues = 4,
+ .max_ipvs = 4,
.ops = &ksz8_dev_ops,
.ksz87xx_eee_link_erratum = true,
.mib_names = ksz9477_mib_names,
@@ -1287,6 +1290,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
.cpu_ports = 0x10, /* can be configured as cpu port */
.port_cnt = 5, /* total cpu and user ports */
.num_tx_queues = 4,
+ .max_ipvs = 4,
.ops = &ksz8_dev_ops,
.ksz87xx_eee_link_erratum = true,
.mib_names = ksz9477_mib_names,
@@ -1312,6 +1316,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
.cpu_ports = 0x4, /* can be configured as cpu port */
.port_cnt = 3,
.num_tx_queues = 4,
+ .max_ipvs = 4,
.ops = &ksz8_dev_ops,
.mib_names = ksz88xx_mib_names,
.mib_cnt = ARRAY_SIZE(ksz88xx_mib_names),
@@ -1336,6 +1341,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
.port_cnt = 7, /* total physical port count */
.port_nirqs = 4,
.num_tx_queues = 4,
+ .max_ipvs = 8,
.tc_cbs_supported = true,
.tc_ets_supported = true,
.ops = &ksz9477_dev_ops,
@@ -1370,6 +1376,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
.port_cnt = 6, /* total physical port count */
.port_nirqs = 2,
.num_tx_queues = 4,
+ .max_ipvs = 8,
.ops = &ksz9477_dev_ops,
.mib_names = ksz9477_mib_names,
.mib_cnt = ARRAY_SIZE(ksz9477_mib_names),
@@ -1402,6 +1409,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
.port_cnt = 7, /* total physical port count */
.port_nirqs = 2,
.num_tx_queues = 4,
+ .max_ipvs = 8,
.ops = &ksz9477_dev_ops,
.mib_names = ksz9477_mib_names,
.mib_cnt = ARRAY_SIZE(ksz9477_mib_names),
@@ -1432,6 +1440,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
.port_cnt = 3, /* total port count */
.port_nirqs = 2,
.num_tx_queues = 4,
+ .max_ipvs = 8,
.ops = &ksz9477_dev_ops,
.mib_names = ksz9477_mib_names,
.mib_cnt = ARRAY_SIZE(ksz9477_mib_names),
@@ -1458,6 +1467,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
.port_cnt = 3, /* total port count */
.port_nirqs = 3,
.num_tx_queues = 4,
+ .max_ipvs = 8,
.tc_cbs_supported = true,
.tc_ets_supported = true,
.ops = &ksz9477_dev_ops,
@@ -1486,6 +1496,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
.port_cnt = 7, /* total port count */
.port_nirqs = 3,
.num_tx_queues = 4,
+ .max_ipvs = 8,
.tc_cbs_supported = true,
.tc_ets_supported = true,
.ops = &ksz9477_dev_ops,
@@ -1519,6 +1530,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
.port_cnt = 7, /* total physical port count */
.port_nirqs = 3,
.num_tx_queues = 4,
+ .max_ipvs = 8,
.tc_cbs_supported = true,
.tc_ets_supported = true,
.ops = &ksz9477_dev_ops,
@@ -1551,6 +1563,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
.port_cnt = 5, /* total physical port count */
.port_nirqs = 6,
.num_tx_queues = 8,
+ .max_ipvs = 8,
.tc_cbs_supported = true,
.tc_ets_supported = true,
.ops = &lan937x_dev_ops,
@@ -1578,6 +1591,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
.port_cnt = 6, /* total physical port count */
.port_nirqs = 6,
.num_tx_queues = 8,
+ .max_ipvs = 8,
.tc_cbs_supported = true,
.tc_ets_supported = true,
.ops = &lan937x_dev_ops,
@@ -1605,6 +1619,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
.port_cnt = 8, /* total physical port count */
.port_nirqs = 6,
.num_tx_queues = 8,
+ .max_ipvs = 8,
.tc_cbs_supported = true,
.tc_ets_supported = true,
.ops = &lan937x_dev_ops,
@@ -1636,6 +1651,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
.port_cnt = 5, /* total physical port count */
.port_nirqs = 6,
.num_tx_queues = 8,
+ .max_ipvs = 8,
.tc_cbs_supported = true,
.tc_ets_supported = true,
.ops = &lan937x_dev_ops,
@@ -1667,6 +1683,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
.port_cnt = 8, /* total physical port count */
.port_nirqs = 6,
.num_tx_queues = 8,
+ .max_ipvs = 8,
.tc_cbs_supported = true,
.tc_ets_supported = true,
.ops = &lan937x_dev_ops,
@@ -2296,6 +2313,13 @@ static int ksz_setup(struct dsa_switch *ds)

dev->dev_ops->enable_stp_addr(dev);

+ /* Make sure driver provide plausible queue and IPV values */
+ if (!dev->info->num_tx_queues ||
+ dev->info->num_tx_queues > dev->info->max_ipvs) {
+ dev_err(dev->dev, "Number of TX queues exceeds maximum supported IPVs\n");
+ return -EINVAL;
+ }
+
ds->num_tx_queues = dev->info->num_tx_queues;

regmap_update_bits(ksz_regmap_8(dev), regs[S_MULTICAST_CTRL],
@@ -3522,7 +3546,7 @@ static int ksz_tc_ets_add(struct ksz_device *dev, int port,
for (tc_prio = 0; tc_prio < ARRAY_SIZE(p->priomap); tc_prio++) {
int queue;

- if (tc_prio > KSZ9477_MAX_TC_PRIO)
+ if (tc_prio >= dev->info->max_ipvs)
break;

queue = ksz_ets_band_to_queue(p, p->priomap[tc_prio]);
@@ -3564,7 +3588,7 @@ static int ksz_tc_ets_del(struct ksz_device *dev, int port)
/* Revert the queue mapping for TC-priority to its default setting on
* the chip.
*/
- for (tc_prio = 0; tc_prio <= KSZ9477_MAX_TC_PRIO; tc_prio++) {
+ for (tc_prio = 0; tc_prio < dev->info->max_ipvs; tc_prio++) {
int queue;

queue = tc_prio >> s;
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index 40c11b0d6b625..1bedd240cbbe4 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -58,6 +58,7 @@ struct ksz_chip_data {
int port_cnt;
u8 port_nirqs;
u8 num_tx_queues;
+ u8 max_ipvs; /* max number of Internal Priority Values */
bool tc_cbs_supported;
bool tc_ets_supported;
const struct ksz_dev_ops *ops;
@@ -722,7 +723,6 @@ static inline int is_lan937x(struct ksz_device *dev)
#define KSZ9477_PORT_MRI_TC_MAP__4 0x0808

#define KSZ9477_PORT_TC_MAP_S 4
-#define KSZ9477_MAX_TC_PRIO 7

/* CBS related registers */
#define REG_PORT_MTI_QUEUE_INDEX__4 0x0900
--
2.39.2



2024-04-03 10:58:26

by Ratheesh Kannoth

[permalink] [raw]
Subject: Re: [PATCH net-next v2 2/9] net: dsa: microchip: add IPV information support

On 2024-04-03 at 14:58:58, Oleksij Rempel ([email protected]) wrote:
> Most of Microchip KSZ switches use Internal Priority Value associated
> with every frame. For example, it is possible to map any VLAN PCP or
> DSCP value to IPV and at the end, map IPV to a queue.
>
> Since amount of IPVs is not equal to amount of queues, add this
> information and make use of it in some functions.
>
> Signed-off-by: Oleksij Rempel <[email protected]>
> Acked-by: Arun Ramadoss <[email protected]>
> ---
>
> dev->dev_ops->enable_stp_addr(dev);
>
> + /* Make sure driver provide plausible queue and IPV values */
> + if (!dev->info->num_tx_queues ||
> + dev->info->num_tx_queues > dev->info->max_ipvs) {
> + dev_err(dev->dev, "Number of TX queues exceeds maximum supported IPVs\n");
if dev->info->num_tx_queues == 0, error message seems to be wrong.

> + return -EINVAL;
> + }
>
> #define KSZ9477_PORT_TC_MAP_S 4
> -#define KSZ9477_MAX_TC_PRIO 7
>
> /* CBS related registers */
> #define REG_PORT_MTI_QUEUE_INDEX__4 0x0900
> --
> 2.39.2
>

2024-04-03 11:36:50

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [PATCH net-next v2 2/9] net: dsa: microchip: add IPV information support

On Wed, Apr 03, 2024 at 11:28:58AM +0200, Oleksij Rempel wrote:
> Most of Microchip KSZ switches use Internal Priority Value associated
> with every frame. For example, it is possible to map any VLAN PCP or
> DSCP value to IPV and at the end, map IPV to a queue.
>
> Since amount of IPVs is not equal to amount of queues, add this
> information and make use of it in some functions.
>
> Signed-off-by: Oleksij Rempel <[email protected]>
> Acked-by: Arun Ramadoss <[email protected]>
> ---
> @@ -2296,6 +2313,13 @@ static int ksz_setup(struct dsa_switch *ds)
>
> dev->dev_ops->enable_stp_addr(dev);
>
> + /* Make sure driver provide plausible queue and IPV values */

provides

> + if (!dev->info->num_tx_queues ||
> + dev->info->num_tx_queues > dev->info->max_ipvs) {

This only works because "max_ipvs" actually holds the _number_ of IPVs.
If it actually held _max_, it would have been off by one. Conclusion:
please rename it num_ipvs.

> + dev_err(dev->dev, "Number of TX queues exceeds maximum supported IPVs\n");
> + return -EINVAL;

Is the validation actually that helpful? FWIW, more TX queues than IPVs
is nothing out of the ordinary (maybe not for this hardware IP). You can
do multi-queue egress scheduling for a traffic class, even mqprio allows
configuring this (e.g. "2@0 2@2" which means 4 queues using 2 TCs).

> + }
> +
> ds->num_tx_queues = dev->info->num_tx_queues;
>
> regmap_update_bits(ksz_regmap_8(dev), regs[S_MULTICAST_CTRL],
> @@ -3522,7 +3546,7 @@ static int ksz_tc_ets_add(struct ksz_device *dev, int port,
> for (tc_prio = 0; tc_prio < ARRAY_SIZE(p->priomap); tc_prio++) {
> int queue;
>
> - if (tc_prio > KSZ9477_MAX_TC_PRIO)
> + if (tc_prio >= dev->info->max_ipvs)

Same here (please rename max_ipvs to num_ipvs). Otherwise, the operator
change makes no sense (we have "max" -> "max" but ">" -> ">=").

> break;
>
> queue = ksz_ets_band_to_queue(p, p->priomap[tc_prio]);
> @@ -3564,7 +3588,7 @@ static int ksz_tc_ets_del(struct ksz_device *dev, int port)
> /* Revert the queue mapping for TC-priority to its default setting on
> * the chip.
> */
> - for (tc_prio = 0; tc_prio <= KSZ9477_MAX_TC_PRIO; tc_prio++) {
> + for (tc_prio = 0; tc_prio < dev->info->max_ipvs; tc_prio++) {
> int queue;
>
> queue = tc_prio >> s;
> diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
> index 40c11b0d6b625..1bedd240cbbe4 100644
> --- a/drivers/net/dsa/microchip/ksz_common.h
> +++ b/drivers/net/dsa/microchip/ksz_common.h
> @@ -58,6 +58,7 @@ struct ksz_chip_data {
> int port_cnt;
> u8 port_nirqs;
> u8 num_tx_queues;
> + u8 max_ipvs; /* max number of Internal Priority Values */
> bool tc_cbs_supported;
> bool tc_ets_supported;
> const struct ksz_dev_ops *ops;
> @@ -722,7 +723,6 @@ static inline int is_lan937x(struct ksz_device *dev)
> #define KSZ9477_PORT_MRI_TC_MAP__4 0x0808
>
> #define KSZ9477_PORT_TC_MAP_S 4
> -#define KSZ9477_MAX_TC_PRIO 7
>
> /* CBS related registers */
> #define REG_PORT_MTI_QUEUE_INDEX__4 0x0900
> --
> 2.39.2
>