[PATCH v1 net-next 0/3] net: dsa: felix: tc taprio and CBS offload support

[PATCH v1 net-next 0/3] net: dsa: felix: tc taprio and CBS offload support

[Xiaoliang Yang]

This patch series support tc taprio and CBS hardware offload according
to IEEE 802.1Qbv and IEEE-802.1Qav on VSC9959.

Xiaoliang Yang (3):
  net: dsa: felix: qos classified based on pcp
  net: dsa: felix: Configure Time-Aware Scheduler via taprio offload
  net: dsa: felix: add support Credit Based Shaper(CBS) for hardware
    offload

 drivers/net/dsa/ocelot/felix.c         |  16 +-
 drivers/net/dsa/ocelot/felix.h         |   6 +
 drivers/net/dsa/ocelot/felix_vsc9959.c | 215 ++++++++++++++++++++++++-
 3 files changed, 235 insertions(+), 2 deletions(-)

-- 
2.17.1

[PATCH v1 net-next 1/3] net: dsa: felix: qos classified based on pcp

[Xiaoliang Yang]

Set the default QoS Classification based on PCP and DEI of vlan tag,
after that, frames can be Classified to different Qos based on PCP tag.
If there is no vlan tag or vlan ignored, use port default Qos.

Signed-off-by: Xiaoliang Yang <xiaoliang.yang_1@nxp.com>
---
 drivers/net/dsa/ocelot/felix.c         |  6 ++++++
 drivers/net/dsa/ocelot/felix.h         |  1 +
 drivers/net/dsa/ocelot/felix_vsc9959.c | 23 +++++++++++++++++++++++
 3 files changed, 30 insertions(+)

diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
index a2dfd73f8a1a..0afdc6fc3f57 100644
--- a/drivers/net/dsa/ocelot/felix.c
+++ b/drivers/net/dsa/ocelot/felix.c
@@ -547,6 +547,12 @@ static int felix_setup(struct dsa_switch *ds)
 			ocelot_configure_cpu(ocelot, port,
 					     OCELOT_TAG_PREFIX_NONE,
 					     OCELOT_TAG_PREFIX_LONG);
+
+		/* Set the default QoS Classification based on PCP and DEI
+		 * bits of vlan tag.
+		 */
+		if (felix->info->port_qos_map_init)
+			felix->info->port_qos_map_init(ocelot, port);
 	}
 
 	/* Include the CPU port module in the forwarding mask for unknown
diff --git a/drivers/net/dsa/ocelot/felix.h b/drivers/net/dsa/ocelot/felix.h
index b94386fa8d63..0d4ec34309c7 100644
--- a/drivers/net/dsa/ocelot/felix.h
+++ b/drivers/net/dsa/ocelot/felix.h
@@ -35,6 +35,7 @@ struct felix_info {
 				  struct phylink_link_state *state);
 	int	(*prevalidate_phy_mode)(struct ocelot *ocelot, int port,
 					phy_interface_t phy_mode);
+	void	(*port_qos_map_init)(struct ocelot *ocelot, int port);
 };
 
 extern struct felix_info		felix_info_vsc9959;
diff --git a/drivers/net/dsa/ocelot/felix_vsc9959.c b/drivers/net/dsa/ocelot/felix_vsc9959.c
index 1c56568d5aca..5c931fb3e4cd 100644
--- a/drivers/net/dsa/ocelot/felix_vsc9959.c
+++ b/drivers/net/dsa/ocelot/felix_vsc9959.c
@@ -4,6 +4,7 @@
  */
 #include <linux/fsl/enetc_mdio.h>
 #include <soc/mscc/ocelot_vcap.h>
+#include <soc/mscc/ocelot_ana.h>
 #include <soc/mscc/ocelot_sys.h>
 #include <soc/mscc/ocelot.h>
 #include <linux/iopoll.h>
@@ -1209,6 +1210,27 @@ static void vsc9959_mdio_bus_free(struct ocelot *ocelot)
 	mdiobus_unregister(felix->imdio);
 }
 
+static void vsc9959_port_qos_map_init(struct ocelot *ocelot, int port)
+{
+	int i;
+
+	ocelot_rmw_gix(ocelot,
+		       ANA_PORT_QOS_CFG_QOS_PCP_ENA,
+		       ANA_PORT_QOS_CFG_QOS_PCP_ENA,
+		       ANA_PORT_QOS_CFG,
+		       port);
+
+	for (i = 0; i < FELIX_NUM_TC * 2; i++) {
+		ocelot_rmw_ix(ocelot,
+			      (ANA_PORT_PCP_DEI_MAP_DP_PCP_DEI_VAL & i) |
+			      ANA_PORT_PCP_DEI_MAP_QOS_PCP_DEI_VAL(i),
+			      ANA_PORT_PCP_DEI_MAP_DP_PCP_DEI_VAL |
+			      ANA_PORT_PCP_DEI_MAP_QOS_PCP_DEI_VAL_M,
+			      ANA_PORT_PCP_DEI_MAP,
+			      port, i);
+	}
+}
+
 struct felix_info felix_info_vsc9959 = {
 	.target_io_res		= vsc9959_target_io_res,
 	.port_io_res		= vsc9959_port_io_res,
@@ -1232,4 +1254,5 @@ struct felix_info felix_info_vsc9959 = {
 	.pcs_an_restart		= vsc9959_pcs_an_restart,
 	.pcs_link_state		= vsc9959_pcs_link_state,
 	.prevalidate_phy_mode	= vsc9959_prevalidate_phy_mode,
+	.port_qos_map_init	= vsc9959_port_qos_map_init,
 };
-- 
2.17.1

[PATCH v1 net-next 2/3] net: dsa: felix: Configure Time-Aware Scheduler via taprio offload

[Xiaoliang Yang]

Ocelot VSC9959 switch supports time-based egress shaping in hardware
according to IEEE 802.1Qbv. This patch add support for TAS configuration
on egress port of VSC9959 switch.

Felix driver is an instance of Ocelot family, with a DSA front-end. The
patch uses tc taprio hardware offload to setup TAS set function on felix
driver.

Signed-off-by: Xiaoliang Yang <xiaoliang.yang_1@nxp.com>
Reviewed-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/dsa/ocelot/felix.c         |  10 +-
 drivers/net/dsa/ocelot/felix.h         |   5 +
 drivers/net/dsa/ocelot/felix_vsc9959.c | 140 +++++++++++++++++++++++++
 3 files changed, 154 insertions(+), 1 deletion(-)

diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
index 0afdc6fc3f57..edd693d59b8e 100644
--- a/drivers/net/dsa/ocelot/felix.c
+++ b/drivers/net/dsa/ocelot/felix.c
@@ -237,6 +237,10 @@ static void felix_phylink_mac_config(struct dsa_switch *ds, int port,
 
 	if (felix->info->pcs_init)
 		felix->info->pcs_init(ocelot, port, link_an_mode, state);
+
+	if (felix->info->port_sched_speed_set)
+		felix->info->port_sched_speed_set(ocelot, port,
+						  state->speed);
 }
 
 static void felix_phylink_mac_an_restart(struct dsa_switch *ds, int port)
@@ -710,7 +714,7 @@ static void felix_port_policer_del(struct dsa_switch *ds, int port)
 	ocelot_port_policer_del(ocelot, port);
 }
 
-static const struct dsa_switch_ops felix_switch_ops = {
+static struct dsa_switch_ops felix_switch_ops = {
 	.get_tag_protocol	= felix_get_tag_protocol,
 	.setup			= felix_setup,
 	.teardown		= felix_teardown,
@@ -827,6 +831,9 @@ static int felix_pci_probe(struct pci_dev *pdev,
 
 	ocelot->ptp = 1;
 
+	if (felix->info->port_setup_tc)
+		felix_switch_ops.port_setup_tc = felix->info->port_setup_tc;
+
 	ds = kzalloc(sizeof(struct dsa_switch), GFP_KERNEL);
 	if (!ds) {
 		err = -ENOMEM;
@@ -836,6 +843,7 @@ static int felix_pci_probe(struct pci_dev *pdev,
 
 	ds->dev = &pdev->dev;
 	ds->num_ports = felix->info->num_ports;
+	ds->num_tx_queues = felix->info->num_tx_queues;
 	ds->ops = &felix_switch_ops;
 	ds->priv = ocelot;
 	felix->ds = ds;
diff --git a/drivers/net/dsa/ocelot/felix.h b/drivers/net/dsa/ocelot/felix.h
index 0d4ec34309c7..24b13526fcf2 100644
--- a/drivers/net/dsa/ocelot/felix.h
+++ b/drivers/net/dsa/ocelot/felix.h
@@ -20,6 +20,7 @@ struct felix_info {
 	const struct ocelot_stat_layout	*stats_layout;
 	unsigned int			num_stats;
 	int				num_ports;
+	int                             num_tx_queues;
 	struct vcap_field		*vcap_is2_keys;
 	struct vcap_field		*vcap_is2_actions;
 	const struct vcap_props		*vcap;
@@ -36,6 +37,10 @@ struct felix_info {
 	int	(*prevalidate_phy_mode)(struct ocelot *ocelot, int port,
 					phy_interface_t phy_mode);
 	void	(*port_qos_map_init)(struct ocelot *ocelot, int port);
+	int	(*port_setup_tc)(struct dsa_switch *ds, int port,
+				 enum tc_setup_type type, void *type_data);
+	void	(*port_sched_speed_set)(struct ocelot *ocelot, int port,
+					u32 speed);
 };
 
 extern struct felix_info		felix_info_vsc9959;
diff --git a/drivers/net/dsa/ocelot/felix_vsc9959.c b/drivers/net/dsa/ocelot/felix_vsc9959.c
index 5c931fb3e4cd..ccbd875c7a47 100644
--- a/drivers/net/dsa/ocelot/felix_vsc9959.c
+++ b/drivers/net/dsa/ocelot/felix_vsc9959.c
@@ -3,10 +3,13 @@
  * Copyright 2018-2019 NXP Semiconductors
  */
 #include <linux/fsl/enetc_mdio.h>
+#include <soc/mscc/ocelot_qsys.h>
 #include <soc/mscc/ocelot_vcap.h>
 #include <soc/mscc/ocelot_ana.h>
+#include <soc/mscc/ocelot_ptp.h>
 #include <soc/mscc/ocelot_sys.h>
 #include <soc/mscc/ocelot.h>
+#include <net/pkt_sched.h>
 #include <linux/iopoll.h>
 #include <linux/pci.h>
 #include "felix.h"
@@ -28,6 +31,8 @@
 #define USXGMII_LPA_DUPLEX(lpa)		(((lpa) & GENMASK(12, 12)) >> 12)
 #define USXGMII_LPA_SPEED(lpa)		(((lpa) & GENMASK(11, 9)) >> 9)
 
+#define VSC9959_TAS_GCL_ENTRY_MAX	63
+
 enum usxgmii_speed {
 	USXGMII_SPEED_10	= 0,
 	USXGMII_SPEED_100	= 1,
@@ -1231,6 +1236,138 @@ static void vsc9959_port_qos_map_init(struct ocelot *ocelot, int port)
 	}
 }
 
+static void vsc9959_sched_speed_set(struct ocelot *ocelot, int port,
+				    u32 speed)
+{
+	ocelot_rmw_rix(ocelot,
+		       QSYS_TAG_CONFIG_LINK_SPEED(speed),
+		       QSYS_TAG_CONFIG_LINK_SPEED_M,
+		       QSYS_TAG_CONFIG, port);
+}
+
+static void vsc9959_new_base_time(struct ocelot *ocelot, ktime_t base_time,
+				  u64 cycle_time,
+				  struct timespec64 *new_base_ts)
+{
+	struct timespec64 ts;
+	ktime_t new_base_time;
+	ktime_t current_time;
+
+	ocelot_ptp_gettime64(&ocelot->ptp_info, &ts);
+	current_time = timespec64_to_ktime(ts);
+	new_base_time = base_time;
+
+	if (base_time < current_time) {
+		u64 nr_of_cycles = current_time - base_time;
+
+		do_div(nr_of_cycles, cycle_time);
+		new_base_time += cycle_time * (nr_of_cycles + 1);
+	}
+
+	*new_base_ts = ktime_to_timespec64(new_base_time);
+}
+
+static u32 vsc9959_tas_read_cfg_status(struct ocelot *ocelot)
+{
+	return ocelot_read(ocelot, QSYS_TAS_PARAM_CFG_CTRL);
+}
+
+static void vsc9959_tas_gcl_set(struct ocelot *ocelot, const u32 gcl_ix,
+				struct tc_taprio_sched_entry *entry)
+{
+	ocelot_write(ocelot,
+		     QSYS_GCL_CFG_REG_1_GCL_ENTRY_NUM(gcl_ix) |
+		     QSYS_GCL_CFG_REG_1_GATE_STATE(entry->gate_mask),
+		     QSYS_GCL_CFG_REG_1);
+	ocelot_write(ocelot, entry->interval, QSYS_GCL_CFG_REG_2);
+}
+
+static int vsc9959_qos_port_tas_set(struct ocelot *ocelot, int port,
+				    struct tc_taprio_qopt_offload *taprio)
+{
+	struct timespec64 base_ts;
+	int ret, i;
+	u32 val;
+
+	if (!taprio->enable) {
+		ocelot_rmw_rix(ocelot,
+			       QSYS_TAG_CONFIG_INIT_GATE_STATE(0xFF),
+			       QSYS_TAG_CONFIG_ENABLE |
+			       QSYS_TAG_CONFIG_INIT_GATE_STATE_M,
+			       QSYS_TAG_CONFIG, port);
+
+		return 0;
+	}
+
+	if (taprio->cycle_time > NSEC_PER_SEC ||
+	    taprio->cycle_time_extension >= NSEC_PER_SEC)
+		return -EINVAL;
+
+	if (taprio->num_entries > VSC9959_TAS_GCL_ENTRY_MAX)
+		return -ERANGE;
+
+	ocelot_rmw(ocelot, QSYS_TAS_PARAM_CFG_CTRL_PORT_NUM(port) |
+		   QSYS_TAS_PARAM_CFG_CTRL_ALWAYS_GUARD_BAND_SCH_Q,
+		   QSYS_TAS_PARAM_CFG_CTRL_PORT_NUM_M |
+		   QSYS_TAS_PARAM_CFG_CTRL_ALWAYS_GUARD_BAND_SCH_Q,
+		   QSYS_TAS_PARAM_CFG_CTRL);
+
+	/* Hardware errata -  Admin config could not be overwritten if
+	 * config is pending, need reset the TAS module
+	 */
+	val = ocelot_read(ocelot, QSYS_PARAM_STATUS_REG_8);
+	if (val & QSYS_PARAM_STATUS_REG_8_CONFIG_PENDING)
+		return  -EBUSY;
+
+	ocelot_rmw_rix(ocelot,
+		       QSYS_TAG_CONFIG_ENABLE |
+		       QSYS_TAG_CONFIG_INIT_GATE_STATE(0xFF) |
+		       QSYS_TAG_CONFIG_SCH_TRAFFIC_QUEUES(0xFF),
+		       QSYS_TAG_CONFIG_ENABLE |
+		       QSYS_TAG_CONFIG_INIT_GATE_STATE_M |
+		       QSYS_TAG_CONFIG_SCH_TRAFFIC_QUEUES_M,
+		       QSYS_TAG_CONFIG, port);
+
+	vsc9959_new_base_time(ocelot, taprio->base_time,
+			      taprio->cycle_time, &base_ts);
+	ocelot_write(ocelot, base_ts.tv_nsec, QSYS_PARAM_CFG_REG_1);
+	ocelot_write(ocelot, lower_32_bits(base_ts.tv_sec), QSYS_PARAM_CFG_REG_2);
+	val = upper_32_bits(base_ts.tv_sec);
+	ocelot_write(ocelot,
+		     QSYS_PARAM_CFG_REG_3_BASE_TIME_SEC_MSB(val) |
+		     QSYS_PARAM_CFG_REG_3_LIST_LENGTH(taprio->num_entries),
+		     QSYS_PARAM_CFG_REG_3);
+	ocelot_write(ocelot, taprio->cycle_time, QSYS_PARAM_CFG_REG_4);
+	ocelot_write(ocelot, taprio->cycle_time_extension, QSYS_PARAM_CFG_REG_5);
+
+	for (i = 0; i < taprio->num_entries; i++)
+		vsc9959_tas_gcl_set(ocelot, i, &taprio->entries[i]);
+
+	ocelot_rmw(ocelot, QSYS_TAS_PARAM_CFG_CTRL_CONFIG_CHANGE,
+		   QSYS_TAS_PARAM_CFG_CTRL_CONFIG_CHANGE,
+		   QSYS_TAS_PARAM_CFG_CTRL);
+
+	ret = readx_poll_timeout(vsc9959_tas_read_cfg_status, ocelot, val,
+				 !(val & QSYS_TAS_PARAM_CFG_CTRL_CONFIG_CHANGE),
+				 10, 100000);
+
+	return ret;
+}
+
+static int vsc9959_port_setup_tc(struct dsa_switch *ds, int port,
+				 enum tc_setup_type type,
+				 void *type_data)
+{
+	struct ocelot *ocelot = ds->priv;
+
+	switch (type) {
+	case TC_SETUP_QDISC_TAPRIO:
+		return vsc9959_qos_port_tas_set(ocelot, port, type_data);
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
 struct felix_info felix_info_vsc9959 = {
 	.target_io_res		= vsc9959_target_io_res,
 	.port_io_res		= vsc9959_port_io_res,
@@ -1246,6 +1383,7 @@ struct felix_info felix_info_vsc9959 = {
 	.shared_queue_sz	= 128 * 1024,
 	.num_mact_rows		= 2048,
 	.num_ports		= 6,
+	.num_tx_queues		= FELIX_NUM_TC,
 	.switch_pci_bar		= 4,
 	.imdio_pci_bar		= 0,
 	.mdio_bus_alloc		= vsc9959_mdio_bus_alloc,
@@ -1255,4 +1393,6 @@ struct felix_info felix_info_vsc9959 = {
 	.pcs_link_state		= vsc9959_pcs_link_state,
 	.prevalidate_phy_mode	= vsc9959_prevalidate_phy_mode,
 	.port_qos_map_init	= vsc9959_port_qos_map_init,
+	.port_setup_tc          = vsc9959_port_setup_tc,
+	.port_sched_speed_set   = vsc9959_sched_speed_set,
 };
-- 
2.17.1

[PATCH v1 net-next 3/3] net: dsa: felix: add support Credit Based Shaper(CBS) for hardware offload

[Xiaoliang Yang]

VSC9959 hardware support the Credit Based Shaper(CBS) which part
of the IEEE-802.1Qav. This patch support sch_cbs set for VSC9959.

Signed-off-by: Xiaoliang Yang <xiaoliang.yang_1@nxp.com>
---
 drivers/net/dsa/ocelot/felix_vsc9959.c | 52 +++++++++++++++++++++++++-
 1 file changed, 51 insertions(+), 1 deletion(-)

diff --git a/drivers/net/dsa/ocelot/felix_vsc9959.c b/drivers/net/dsa/ocelot/felix_vsc9959.c
index ccbd875c7a47..d8d1657ee8ba 100644
--- a/drivers/net/dsa/ocelot/felix_vsc9959.c
+++ b/drivers/net/dsa/ocelot/felix_vsc9959.c
@@ -208,7 +208,7 @@ static const u32 vsc9959_qsys_regmap[] = {
 	REG(QSYS_QMAXSDU_CFG_6,			0x00f62c),
 	REG(QSYS_QMAXSDU_CFG_7,			0x00f648),
 	REG(QSYS_PREEMPTION_CFG,		0x00f664),
-	REG_RESERVED(QSYS_CIR_CFG),
+	REG(QSYS_CIR_CFG,			0x000000),
 	REG(QSYS_EIR_CFG,			0x000004),
 	REG(QSYS_SE_CFG,			0x000008),
 	REG(QSYS_SE_DWRR_CFG,			0x00000c),
@@ -1354,6 +1354,54 @@ static int vsc9959_qos_port_tas_set(struct ocelot *ocelot, int port,
 	return ret;
 }
 
+int vsc9959_qos_port_cbs_set(struct dsa_switch *ds, int port,
+			     struct tc_cbs_qopt_offload *cbs_qopt)
+{
+	struct ocelot *ocelot = ds->priv;
+	int port_ix = port * 8 + cbs_qopt->queue;
+	u32 cbs = 0;
+	u32 cir = 0;
+
+	if (cbs_qopt->queue >= ds->num_tx_queues)
+		return -EINVAL;
+
+	if (!cbs_qopt->enable) {
+		ocelot_write_gix(ocelot, QSYS_CIR_CFG_CIR_RATE(0) |
+				 QSYS_CIR_CFG_CIR_BURST(0),
+				 QSYS_CIR_CFG, port_ix);
+
+		ocelot_rmw_gix(ocelot, 0, QSYS_SE_CFG_SE_AVB_ENA,
+			       QSYS_SE_CFG, port_ix);
+
+		return 0;
+	}
+
+	/* Rate unit is 100 kbps */
+	cir = DIV_ROUND_UP(cbs_qopt->idleslope, 100);
+	cir = (cir ? cir : 1);
+	cir = min_t(u32, GENMASK(14, 0), cir);
+	/* Burst unit is 4kB */
+	cbs = DIV_ROUND_UP(cbs_qopt->hicredit, 4096);
+	/* Avoid using zero burst size */
+	cbs = (cbs ? cbs : 1);
+	cbs = min_t(u32, GENMASK(5, 0), cbs);
+	ocelot_write_gix(ocelot,
+			 QSYS_CIR_CFG_CIR_RATE(cir) |
+			 QSYS_CIR_CFG_CIR_BURST(cbs),
+			 QSYS_CIR_CFG,
+			 port_ix);
+
+	ocelot_rmw_gix(ocelot,
+		       QSYS_SE_CFG_SE_FRM_MODE(0) |
+		       QSYS_SE_CFG_SE_AVB_ENA,
+		       QSYS_SE_CFG_SE_AVB_ENA |
+		       QSYS_SE_CFG_SE_FRM_MODE_M,
+		       QSYS_SE_CFG,
+		       port_ix);
+
+	return 0;
+}
+
 static int vsc9959_port_setup_tc(struct dsa_switch *ds, int port,
 				 enum tc_setup_type type,
 				 void *type_data)
@@ -1363,6 +1411,8 @@ static int vsc9959_port_setup_tc(struct dsa_switch *ds, int port,
 	switch (type) {
 	case TC_SETUP_QDISC_TAPRIO:
 		return vsc9959_qos_port_tas_set(ocelot, port, type_data);
+	case TC_SETUP_QDISC_CBS:
+		return vsc9959_qos_port_cbs_set(ds, port, type_data);
 	default:
 		return -EOPNOTSUPP;
 	}
-- 
2.17.1

Re: [PATCH v1 net-next 1/3] net: dsa: felix: qos classified based on pcp

[Vladimir Oltean]

Hi Jiri, Jakub, Ido,

On Mon, 11 May 2020 at 08:50, Xiaoliang Yang <xiaoliang.yang_1@nxp.com> wrote:
>
> Set the default QoS Classification based on PCP and DEI of vlan tag,
> after that, frames can be Classified to different Qos based on PCP tag.
> If there is no vlan tag or vlan ignored, use port default Qos.
>
> Signed-off-by: Xiaoliang Yang <xiaoliang.yang_1@nxp.com>
> ---

The new skbedit priority offload action looks interesting to me.
But it also raises the question of what to do in the default case
where such rules are not installed. I think it is ok to support a
1-to-1 VLAN PCP to TC mapping by default? This should also be needed
for features such as Priority Flow Control.

>  drivers/net/dsa/ocelot/felix.c         |  6 ++++++
>  drivers/net/dsa/ocelot/felix.h         |  1 +
>  drivers/net/dsa/ocelot/felix_vsc9959.c | 23 +++++++++++++++++++++++
>  3 files changed, 30 insertions(+)
>
> diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
> index a2dfd73f8a1a..0afdc6fc3f57 100644
> --- a/drivers/net/dsa/ocelot/felix.c
> +++ b/drivers/net/dsa/ocelot/felix.c
> @@ -547,6 +547,12 @@ static int felix_setup(struct dsa_switch *ds)
>                         ocelot_configure_cpu(ocelot, port,
>                                              OCELOT_TAG_PREFIX_NONE,
>                                              OCELOT_TAG_PREFIX_LONG);
> +
> +               /* Set the default QoS Classification based on PCP and DEI
> +                * bits of vlan tag.
> +                */
> +               if (felix->info->port_qos_map_init)
> +                       felix->info->port_qos_map_init(ocelot, port);

Xiaoliang, just a small comment in case you need to resend.
The felix->info structure is intended to hold SoC-specific data that
is likely to differ between chips (like for example if vsc7511 support
ever appears in felix). But I see ANA:PORT:QOS_CFG and
ANA:PORT:QOS_PCP_DEI_MAP_CFG are common registers, so are there any
specific reasons why you put this in felix_vsc9959 and not in the
common ocelot code?

>         }
>
>         /* Include the CPU port module in the forwarding mask for unknown
> diff --git a/drivers/net/dsa/ocelot/felix.h b/drivers/net/dsa/ocelot/felix.h
> index b94386fa8d63..0d4ec34309c7 100644
> --- a/drivers/net/dsa/ocelot/felix.h
> +++ b/drivers/net/dsa/ocelot/felix.h
> @@ -35,6 +35,7 @@ struct felix_info {
>                                   struct phylink_link_state *state);
>         int     (*prevalidate_phy_mode)(struct ocelot *ocelot, int port,
>                                         phy_interface_t phy_mode);
> +       void    (*port_qos_map_init)(struct ocelot *ocelot, int port);
>  };
>
>  extern struct felix_info               felix_info_vsc9959;
> diff --git a/drivers/net/dsa/ocelot/felix_vsc9959.c b/drivers/net/dsa/ocelot/felix_vsc9959.c
> index 1c56568d5aca..5c931fb3e4cd 100644
> --- a/drivers/net/dsa/ocelot/felix_vsc9959.c
> +++ b/drivers/net/dsa/ocelot/felix_vsc9959.c
> @@ -4,6 +4,7 @@
>   */
>  #include <linux/fsl/enetc_mdio.h>
>  #include <soc/mscc/ocelot_vcap.h>
> +#include <soc/mscc/ocelot_ana.h>
>  #include <soc/mscc/ocelot_sys.h>
>  #include <soc/mscc/ocelot.h>
>  #include <linux/iopoll.h>
> @@ -1209,6 +1210,27 @@ static void vsc9959_mdio_bus_free(struct ocelot *ocelot)
>         mdiobus_unregister(felix->imdio);
>  }
>
> +static void vsc9959_port_qos_map_init(struct ocelot *ocelot, int port)
> +{
> +       int i;
> +
> +       ocelot_rmw_gix(ocelot,
> +                      ANA_PORT_QOS_CFG_QOS_PCP_ENA,
> +                      ANA_PORT_QOS_CFG_QOS_PCP_ENA,
> +                      ANA_PORT_QOS_CFG,
> +                      port);
> +
> +       for (i = 0; i < FELIX_NUM_TC * 2; i++) {
> +               ocelot_rmw_ix(ocelot,
> +                             (ANA_PORT_PCP_DEI_MAP_DP_PCP_DEI_VAL & i) |

ANA_PORT_PCP_DEI_MAP_DP_PCP_DEI_VAL is 1 bit. Are you sure this should
be % i and not % 2?

> +                             ANA_PORT_PCP_DEI_MAP_QOS_PCP_DEI_VAL(i),
> +                             ANA_PORT_PCP_DEI_MAP_DP_PCP_DEI_VAL |
> +                             ANA_PORT_PCP_DEI_MAP_QOS_PCP_DEI_VAL_M,
> +                             ANA_PORT_PCP_DEI_MAP,
> +                             port, i);
> +       }
> +}
> +
>  struct felix_info felix_info_vsc9959 = {
>         .target_io_res          = vsc9959_target_io_res,
>         .port_io_res            = vsc9959_port_io_res,
> @@ -1232,4 +1254,5 @@ struct felix_info felix_info_vsc9959 = {
>         .pcs_an_restart         = vsc9959_pcs_an_restart,
>         .pcs_link_state         = vsc9959_pcs_link_state,
>         .prevalidate_phy_mode   = vsc9959_prevalidate_phy_mode,
> +       .port_qos_map_init      = vsc9959_port_qos_map_init,
>  };
> --
> 2.17.1
>

Thanks,
-Vladimir

Re: [PATCH v1 net-next 3/3] net: dsa: felix: add support Credit Based Shaper(CBS) for hardware offload

[Jakub Kicinski]

On Mon, 11 May 2020 13:43:32 +0800 Xiaoliang Yang wrote:
> +int vsc9959_qos_port_cbs_set(struct dsa_switch *ds, int port,
> +			     struct tc_cbs_qopt_offload *cbs_qopt)

static

Re: [PATCH v1 net-next 2/3] net: dsa: felix: Configure Time-Aware Scheduler via taprio offload

[David Miller]

From: Xiaoliang Yang <xiaoliang.yang_1@nxp.com>
Date: Mon, 11 May 2020 13:43:31 +0800

> @@ -710,7 +714,7 @@ static void felix_port_policer_del(struct dsa_switch *ds, int port)
>  	ocelot_port_policer_del(ocelot, port);
>  }
>  
> -static const struct dsa_switch_ops felix_switch_ops = {
> +static struct dsa_switch_ops felix_switch_ops = {
>  	.get_tag_protocol	= felix_get_tag_protocol,
>  	.setup			= felix_setup,
>  	.teardown		= felix_teardown,

There has to be a better way to do this, removing const for operation
structs is very undesirable.

Re: [PATCH v1 net-next 2/3] net: dsa: felix: Configure Time-Aware Scheduler via taprio offload

[Vladimir Oltean]

On Tue, 12 May 2020 at 02:54, David Miller <davem@davemloft.net> wrote:
>
> From: Xiaoliang Yang <xiaoliang.yang_1@nxp.com>
> Date: Mon, 11 May 2020 13:43:31 +0800
>
> > @@ -710,7 +714,7 @@ static void felix_port_policer_del(struct dsa_switch *ds, int port)
> >       ocelot_port_policer_del(ocelot, port);
> >  }
> >
> > -static const struct dsa_switch_ops felix_switch_ops = {
> > +static struct dsa_switch_ops felix_switch_ops = {
> >       .get_tag_protocol       = felix_get_tag_protocol,
> >       .setup                  = felix_setup,
> >       .teardown               = felix_teardown,
>
> There has to be a better way to do this, removing const for operation
> structs is very undesirable.

Actually I think this was at my suggestion, but now I agree that it is
a bit undesirable.
struct felix_info is on its toes in case vsc7511 or any other switch
revision will be added to the felix driver. It is likely that those
other chips will have hardware support for a different set of qdiscs.
So we thought of populating the .port_setup_tc conditionally,
depending on chip version (while also having DSA return -EOPNOTSUPP
automatically when the function pointer is NULL). Otherwise an extra
function call would be needed. But I think that keeping felix flexible
for more hardware revisions is a concern to be had for another
time/somebody else.

Re: [PATCH v1 net-next 3/3] net: dsa: felix: add support Credit Based Shaper(CBS) for hardware offload

[Vinicius Costa Gomes]

Xiaoliang Yang <xiaoliang.yang_1@nxp.com> writes:

> VSC9959 hardware support the Credit Based Shaper(CBS) which part
> of the IEEE-802.1Qav. This patch support sch_cbs set for VSC9959.
>
> Signed-off-by: Xiaoliang Yang <xiaoliang.yang_1@nxp.com>
> ---
>  drivers/net/dsa/ocelot/felix_vsc9959.c | 52 +++++++++++++++++++++++++-
>  1 file changed, 51 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/dsa/ocelot/felix_vsc9959.c b/drivers/net/dsa/ocelot/felix_vsc9959.c
> index ccbd875c7a47..d8d1657ee8ba 100644
> --- a/drivers/net/dsa/ocelot/felix_vsc9959.c
> +++ b/drivers/net/dsa/ocelot/felix_vsc9959.c
> @@ -208,7 +208,7 @@ static const u32 vsc9959_qsys_regmap[] = {
>  	REG(QSYS_QMAXSDU_CFG_6,			0x00f62c),
>  	REG(QSYS_QMAXSDU_CFG_7,			0x00f648),
>  	REG(QSYS_PREEMPTION_CFG,		0x00f664),
> -	REG_RESERVED(QSYS_CIR_CFG),
> +	REG(QSYS_CIR_CFG,			0x000000),
>  	REG(QSYS_EIR_CFG,			0x000004),
>  	REG(QSYS_SE_CFG,			0x000008),
>  	REG(QSYS_SE_DWRR_CFG,			0x00000c),
> @@ -1354,6 +1354,54 @@ static int vsc9959_qos_port_tas_set(struct ocelot *ocelot, int port,
>  	return ret;
>  }
>  
> +int vsc9959_qos_port_cbs_set(struct dsa_switch *ds, int port,
> +			     struct tc_cbs_qopt_offload *cbs_qopt)
> +{
> +	struct ocelot *ocelot = ds->priv;
> +	int port_ix = port * 8 + cbs_qopt->queue;
> +	u32 cbs = 0;
> +	u32 cir = 0;
> +
> +	if (cbs_qopt->queue >= ds->num_tx_queues)
> +		return -EINVAL;
> +
> +	if (!cbs_qopt->enable) {
> +		ocelot_write_gix(ocelot, QSYS_CIR_CFG_CIR_RATE(0) |
> +				 QSYS_CIR_CFG_CIR_BURST(0),
> +				 QSYS_CIR_CFG, port_ix);
> +
> +		ocelot_rmw_gix(ocelot, 0, QSYS_SE_CFG_SE_AVB_ENA,
> +			       QSYS_SE_CFG, port_ix);
> +
> +		return 0;
> +	}
> +
> +	/* Rate unit is 100 kbps */
> +	cir = DIV_ROUND_UP(cbs_qopt->idleslope, 100);
> +	cir = (cir ? cir : 1);
> +	cir = min_t(u32, GENMASK(14, 0), cir);

Please rename 'cir' to "rate" or "idleslope".

Also consider using clamp_t here and below (I just found out about it).

> +	/* Burst unit is 4kB */
> +	cbs = DIV_ROUND_UP(cbs_qopt->hicredit, 4096);
> +	/* Avoid using zero burst size */
> +	cbs = (cbs ? cbs : 1);
> +	cbs = min_t(u32, GENMASK(5, 0), cbs);

And please(!) rename 'cbs' to "burst" or "hicredit". Re-using the name
"cbs" with a completely different meaning here is confusing.

> +	ocelot_write_gix(ocelot,
> +			 QSYS_CIR_CFG_CIR_RATE(cir) |
> +			 QSYS_CIR_CFG_CIR_BURST(cbs),
> +			 QSYS_CIR_CFG,
> +			 port_ix);
> +
> +	ocelot_rmw_gix(ocelot,
> +		       QSYS_SE_CFG_SE_FRM_MODE(0) |
> +		       QSYS_SE_CFG_SE_AVB_ENA,
> +		       QSYS_SE_CFG_SE_AVB_ENA |
> +		       QSYS_SE_CFG_SE_FRM_MODE_M,
> +		       QSYS_SE_CFG,
> +		       port_ix);
> +
> +	return 0;
> +}
> +
>  static int vsc9959_port_setup_tc(struct dsa_switch *ds, int port,
>  				 enum tc_setup_type type,
>  				 void *type_data)
> @@ -1363,6 +1411,8 @@ static int vsc9959_port_setup_tc(struct dsa_switch *ds, int port,
>  	switch (type) {
>  	case TC_SETUP_QDISC_TAPRIO:
>  		return vsc9959_qos_port_tas_set(ocelot, port, type_data);
> +	case TC_SETUP_QDISC_CBS:
> +		return vsc9959_qos_port_cbs_set(ds, port, type_data);
>  	default:
>  		return -EOPNOTSUPP;
>  	}
> -- 
> 2.17.1
>

-- 
Vinicius

RE: [EXT] Re: [PATCH v1 net-next 1/3] net: dsa: felix: qos classified based on pcp

[Xiaoliang Yang]

Hi Vladimir,

On Mon, 11 May 2020 at 08:19, Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
> 
> The new skbedit priority offload action looks interesting to me.
> But it also raises the question of what to do in the default case where such rules are not installed. I think it is ok to support a
> 1-to-1 VLAN PCP to TC mapping by default? This should also be needed for features such as Priority Flow Control.

skbedit priority offload seems only support port based prority set now, I haven't found how to set a priority for each port and QoS. So I set a 1-to-1 VLAN PCP to TC mapping by default.

> Xiaoliang, just a small comment in case you need to resend.
> The felix->info structure is intended to hold SoC-specific data that 
> is likely to differ between chips (like for example if vsc7511 support 
> ever appears in felix). But I see ANA:PORT:QOS_CFG and 
> ANA:PORT:QOS_PCP_DEI_MAP_CFG are common registers, so are there any 
> specific reasons why you put this in felix_vsc9959 and not in the 
> common ocelot code?

All right, I have checked they are common registers, I will move port_qos_map_init() function to felix.c.

> > +       for (i = 0; i < FELIX_NUM_TC * 2; i++) {
> > +               ocelot_rmw_ix(ocelot,
> > +                             (ANA_PORT_PCP_DEI_MAP_DP_PCP_DEI_VAL & 
> > + i) |
> > +								 ANA_PORT_PCP_DEI_MAP_QOS_PCP_DEI_VAL(i),
> > +                             ANA_PORT_PCP_DEI_MAP_DP_PCP_DEI_VAL |
> > +                             ANA_PORT_PCP_DEI_MAP_QOS_PCP_DEI_VAL_M,
> > +                             ANA_PORT_PCP_DEI_MAP,
> > +                             port, i);
> 
> ANA_PORT_PCP_DEI_MAP_DP_PCP_DEI_VAL is 1 bit. Are you sure this should be % i and not % 2?

Because in QOS_PCP_DEI_MAP_CFG register, BIT(3) is DP value, BIT(2, 0) is QOS value. QoS class=QOS_PCP_DEI_MAP_CFG[i].QOS_PC
P_DEI_VAL, i=8*DEI + PCP, so DP value need to be set BIT(3)&i.

Regards,
Xiaoliang Yang

RE: [EXT] Re: [PATCH v1 net-next 3/3] net: dsa: felix: add support Credit Based Shaper(CBS) for hardware offload

[Xiaoliang Yang]

Hi Jakub,

On Mon, 11 May 2020 22:34:32 Jakub Kicinski <kuba@kernel.org> wrote:
> > +int vsc9959_qos_port_cbs_set(struct dsa_switch *ds, int port,
> > +                          struct tc_cbs_qopt_offload *cbs_qopt)
>
> static

I will update this in v2, thanks.

Regards,
Xiaoliang Yang

RE: [EXT] Re: [PATCH v1 net-next 3/3] net: dsa: felix: add support Credit Based Shaper(CBS) for hardware offload

[Xiaoliang Yang]

Hi Vinicius,


On Tue, 12 May 2020 9:42:23 Vinicius Costa Gomes wrote:
> > +
> > +     /* Rate unit is 100 kbps */
> > +     cir = DIV_ROUND_UP(cbs_qopt->idleslope, 100);
> > +     cir = (cir ? cir : 1);
> > +     cir = min_t(u32, GENMASK(14, 0), cir);
>
> Please rename 'cir' to "rate" or "idleslope".
>
> Also consider using clamp_t here and below (I just found out about it).
>
> > +     /* Burst unit is 4kB */
> > +     cbs = DIV_ROUND_UP(cbs_qopt->hicredit, 4096);
> > +     /* Avoid using zero burst size */
> > +     cbs = (cbs ? cbs : 1);
> > +     cbs = min_t(u32, GENMASK(5, 0), cbs);
> 
> And please(!) rename 'cbs' to "burst" or "hicredit". Re-using the name "cbs" with a completely different meaning here is confusing.
> 
I will update this, using clamp_t seems more concise in the codes.

Regards,
Xiaoliang