| Message ID | 20191112082823.28998-1-Po.Liu@nxp.com |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | [v2,net-next,1/2] enetc: Configure the Time-Aware Scheduler via tc-taprio offload | expand |
From: Po Liu <po.liu@nxp.com> Date: Tue, 12 Nov 2019 08:42:49 +0000 > ENETC supports in hardware for time-based egress shaping according > to IEEE 802.1Qbv. This patch implement the Qbv enablement by the > hardware offload method qdisc tc-taprio method. > Also update cbdr writeback to up level since control bd ring may > writeback data to control bd ring. > > Signed-off-by: Po Liu <Po.Liu@nxp.com> > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> > Signed-off-by: Claudiu Manoil <claudiu.manoil@nxp.com> Applied.
Hello, On Tue, Nov 12, 2019 at 08:42:49AM +0000, Po Liu wrote: >ENETC supports in hardware for time-based egress shaping according >to IEEE 802.1Qbv. This patch implement the Qbv enablement by the >hardware offload method qdisc tc-taprio method. >Also update cbdr writeback to up level since control bd ring may >writeback data to control bd ring. > >Signed-off-by: Po Liu <Po.Liu@nxp.com> >Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> >Signed-off-by: Claudiu Manoil <claudiu.manoil@nxp.com> >--- >changes: >- introduce a local define CONFIG_FSL_ENETC_QOS to fix the various > configurations will result in link errors. > Since the CONFIG_NET_SCH_TAPRIO depends on many Qos configs. Not > to use it directly in driver. Add it to CONFIG_FSL_ENETC_QOS depends > on list, so only CONFIG_NET_SCH_TAPRIO enabled, user can enable this > tsn feature, or else, return not support. > > drivers/net/ethernet/freescale/enetc/Kconfig | 10 ++ > drivers/net/ethernet/freescale/enetc/Makefile | 1 + > drivers/net/ethernet/freescale/enetc/enetc.c | 19 ++- > drivers/net/ethernet/freescale/enetc/enetc.h | 7 + > .../net/ethernet/freescale/enetc/enetc_cbdr.c | 5 +- > .../net/ethernet/freescale/enetc/enetc_hw.h | 150 ++++++++++++++++-- > .../net/ethernet/freescale/enetc/enetc_qos.c | 130 +++++++++++++++ > 7 files changed, 300 insertions(+), 22 deletions(-) > create mode 100644 drivers/net/ethernet/freescale/enetc/enetc_qos.c > [...] > >@@ -1483,6 +1479,19 @@ int enetc_setup_tc(struct net_device *ndev, enum tc_setup_type type, > return 0; > } > >+int enetc_setup_tc(struct net_device *ndev, enum tc_setup_type type, >+ void *type_data) >+{ >+ switch (type) { >+ case TC_SETUP_QDISC_MQPRIO: >+ return enetc_setup_tc_mqprio(ndev, type_data); Sorry didn't see v2, so i duplicate my question here: This patch is for taprio offload, I see that mqprio is related and is part of taprio offload configuration. But taprio offload has own mqprio settings. The taprio mqprio part is not offloaded with TC_SETUP_QDISC_MQPRIO. So, a combination of mqprio and tario qdiscs used. Could you please share the commands were used for your setup? And couple interesting questions about all of this: - The taprio qdisc has to have mqprio settings, but if it's done with mqprio then it just skipped (by reading tc class num). - If no separate mqprio qdisc configuration then mqprio conf from taprio is set, who should restore tc mappings when taprio qdisc is unloaded? Maybe there is reason to implement TC_SETUP_QDISC_MQPRIO offload in taprio since it's required feature? Would be better to move changes for mqprio in separate patch with explanation. >+ case TC_SETUP_QDISC_TAPRIO: >+ return enetc_setup_tc_taprio(ndev, type_data); >+ default: >+ return -EOPNOTSUPP; >+ } >+} >+ [...] >diff --git a/drivers/net/ethernet/freescale/enetc/enetc_qos.c b/drivers/net/ethernet/freescale/enetc/enetc_qos.c >new file mode 100644 >index 000000000000..036bb39c7a0b >--- /dev/null >+++ b/drivers/net/ethernet/freescale/enetc/enetc_qos.c [...] >+static int enetc_setup_taprio(struct net_device *ndev, >+ struct tc_taprio_qopt_offload *admin_conf) >+{ >+ struct enetc_ndev_priv *priv = netdev_priv(ndev); >+ struct enetc_cbd cbd = {.cmd = 0}; >+ struct tgs_gcl_conf *gcl_config; >+ struct tgs_gcl_data *gcl_data; >+ struct gce *gce; >+ dma_addr_t dma; >+ u16 data_size; >+ u16 gcl_len; >+ u32 temp; >+ int i; >+ >+ gcl_len = admin_conf->num_entries; >+ if (gcl_len > enetc_get_max_gcl_len(&priv->si->hw)) >+ return -EINVAL; >+ >+ if (admin_conf->enable) { >+ enetc_wr(&priv->si->hw, >+ ENETC_QBV_PTGCR_OFFSET, >+ temp & (~ENETC_QBV_TGE)); >+ usleep_range(10, 20); >+ enetc_wr(&priv->si->hw, >+ ENETC_QBV_PTGCR_OFFSET, >+ temp | ENETC_QBV_TGE); >+ } else { >+ enetc_wr(&priv->si->hw, >+ ENETC_QBV_PTGCR_OFFSET, >+ temp & (~ENETC_QBV_TGE)); >+ return 0; >+ } Better do the upper qbv enable/disable procedure closer to enetc_send_cmd() or at least after kzalloc that can fail, no need to restore then. >+ >+ /* Configure the (administrative) gate control list using the >+ * control BD descriptor. >+ */ >+ gcl_config = &cbd.gcl_conf; >+ >+ data_size = sizeof(struct tgs_gcl_data) + gcl_len * sizeof(struct gce); >+ >+ gcl_data = kzalloc(data_size, __GFP_DMA | GFP_KERNEL); >+ if (!gcl_data) >+ return -ENOMEM; >+ >+ gce = (struct gce *)(gcl_data + 1); >+ >+ /* Since no initial state config in taprio, set gates open as default. >+ */ tc-taprio and IEEE Qbv allows to change configuration in flight, so that oper state is active till new admin start time. So, here comment says it does initial state config, if in-flight feature is not supported then error has to be returned instead of silently rewriting configuration. But if it can be implemented then state should be remembered/verified in order to not brake oper configuration? >+ gcl_config->atc = 0xff; >+ gcl_config->acl_len = cpu_to_le16(gcl_len); Ok, this is maximum number of schedules. According to tc-taprio it's possible to set cycle period more then schedules actually can consume. If cycle time is more, then last gate's state can be kept till the end of cycle. But if last schedule has it's own interval set then gates should be closed till the end of cycle or no? if it has to be closed, then one more endl schedule should be present closing gates at the end of list for the rest cycle time. Can be implemented in h/w but just to be sure, how it's done in h/w? >+ >+ if (!admin_conf->base_time) { >+ gcl_data->btl = >+ cpu_to_le32(enetc_rd(&priv->si->hw, ENETC_SICTR0)); >+ gcl_data->bth = >+ cpu_to_le32(enetc_rd(&priv->si->hw, ENETC_SICTR1)); >+ } else { >+ gcl_data->btl = >+ cpu_to_le32(lower_32_bits(admin_conf->base_time)); >+ gcl_data->bth = >+ cpu_to_le32(upper_32_bits(admin_conf->base_time)); >+ } >+ >+ gcl_data->ct = cpu_to_le32(admin_conf->cycle_time); >+ gcl_data->cte = cpu_to_le32(admin_conf->cycle_time_extension); Not sure it's good idea to write values w/o verification, The cycle time and time extension is 64 values, so it's supposed them to be more then 4,3 seconds, it's probably not a case, but better return error if it's more. >+ >+ for (i = 0; i < gcl_len; i++) { >+ struct tc_taprio_sched_entry *temp_entry; >+ struct gce *temp_gce = gce + i; >+ >+ temp_entry = &admin_conf->entries[i]; >+ >+ temp_gce->gate = cpu_to_le32(temp_entry->gate_mask); So, gate_mask can have up to 32 traffic classes? :-|. >+ temp_gce->period = cpu_to_le32(temp_entry->interval); So, the interval can be up to 4.3 seconds for one schedule? That is, one cycle can be one schedule. great. >+ } There is no schedule cmd set, so only SetGateStates is supported? But anyway it's Ok. >+ >+ cbd.length = cpu_to_le16(data_size); >+ cbd.status_flags = 0; >+ >+ dma = dma_map_single(&priv->si->pdev->dev, gcl_data, >+ data_size, DMA_TO_DEVICE); >+ if (dma_mapping_error(&priv->si->pdev->dev, dma)) { >+ netdev_err(priv->si->ndev, "DMA mapping failed!\n"); >+ kfree(gcl_data); >+ return -ENOMEM; >+ } >+ >+ cbd.addr[0] = lower_32_bits(dma); >+ cbd.addr[1] = upper_32_bits(dma); >+ cbd.cls = BDCR_CMD_PORT_GCL; >+ >+ /* Updated by ENETC on completion of the configuration >+ * command. A zero value indicates success. >+ */ >+ cbd.status_flags = 0; It's updated on completion by setting 0 on success, then why it's here set to 0 but not 1 and not verified afterwards? >+ >+ enetc_send_cmd(priv->si, &cbd); >+ >+ dma_unmap_single(&priv->si->pdev->dev, dma, data_size, DMA_TO_DEVICE); >+ kfree(gcl_data); >+ >+ return 0; >+} >+ >+int enetc_setup_tc_taprio(struct net_device *ndev, void *type_data) >+{ >+ struct tc_taprio_qopt_offload *taprio = type_data; >+ struct enetc_ndev_priv *priv = netdev_priv(ndev); >+ int i; >+ >+ for (i = 0; i < priv->num_tx_rings; i++) >+ enetc_set_bdr_prio(&priv->si->hw, >+ priv->tx_ring[i]->index, >+ taprio->enable ? i : 0); then why enable/disable at the beginning for whole qbv scheduler, maybe better do it together? Or better say, what if setup_taprio failed, who restore configuration? >+ >+ return enetc_setup_taprio(ndev, taprio); >+} >-- >2.17.1 >
Hi Ivan, > -----Original Message----- > From: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> > Sent: 2019年11月13日 5:10 > To: Po Liu <po.liu@nxp.com> > Cc: Claudiu Manoil <claudiu.manoil@nxp.com>; davem@davemloft.net; linux- > kernel@vger.kernel.org; netdev@vger.kernel.org; vinicius.gomes@intel.com; > Vladimir Oltean <vladimir.oltean@nxp.com>; Alexandru Marginean > <alexandru.marginean@nxp.com>; Xiaoliang Yang > <xiaoliang.yang_1@nxp.com>; Roy Zang <roy.zang@nxp.com>; Mingkai Hu > <mingkai.hu@nxp.com>; Jerry Huang <jerry.huang@nxp.com>; Leo Li > <leoyang.li@nxp.com> > Subject: [EXT] Re: [v2,net-next, 1/2] enetc: Configure the Time-Aware Scheduler > via tc-taprio offload > > Caution: EXT Email > > Hello, > > On Tue, Nov 12, 2019 at 08:42:49AM +0000, Po Liu wrote: > >ENETC supports in hardware for time-based egress shaping according to > >IEEE 802.1Qbv. This patch implement the Qbv enablement by the hardware > >offload method qdisc tc-taprio method. > >Also update cbdr writeback to up level since control bd ring may > >writeback data to control bd ring. > > > >Signed-off-by: Po Liu <Po.Liu@nxp.com> > >Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> > >Signed-off-by: Claudiu Manoil <claudiu.manoil@nxp.com> > >--- > >changes: > >- introduce a local define CONFIG_FSL_ENETC_QOS to fix the various > > configurations will result in link errors. > > Since the CONFIG_NET_SCH_TAPRIO depends on many Qos configs. Not > > to use it directly in driver. Add it to CONFIG_FSL_ENETC_QOS depends > > on list, so only CONFIG_NET_SCH_TAPRIO enabled, user can enable this > > tsn feature, or else, return not support. > > > > drivers/net/ethernet/freescale/enetc/Kconfig | 10 ++ > > drivers/net/ethernet/freescale/enetc/Makefile | 1 + > > drivers/net/ethernet/freescale/enetc/enetc.c | 19 ++- > > drivers/net/ethernet/freescale/enetc/enetc.h | 7 + > > .../net/ethernet/freescale/enetc/enetc_cbdr.c | 5 +- > > .../net/ethernet/freescale/enetc/enetc_hw.h | 150 ++++++++++++++++-- > > .../net/ethernet/freescale/enetc/enetc_qos.c | 130 +++++++++++++++ > > 7 files changed, 300 insertions(+), 22 deletions(-) create mode 100644 > > drivers/net/ethernet/freescale/enetc/enetc_qos.c > > > > [...] > > > > >@@ -1483,6 +1479,19 @@ int enetc_setup_tc(struct net_device *ndev, enum > tc_setup_type type, > > return 0; > > } > > > >+int enetc_setup_tc(struct net_device *ndev, enum tc_setup_type type, > >+ void *type_data) > >+{ > >+ switch (type) { > >+ case TC_SETUP_QDISC_MQPRIO: > >+ return enetc_setup_tc_mqprio(ndev, type_data); > Sorry didn't see v2, so i duplicate my question here: > > This patch is for taprio offload, I see that mqprio is related and is part of taprio > offload configuration. But taprio offload has own mqprio settings. > The taprio mqprio part is not offloaded with TC_SETUP_QDISC_MQPRIO. > > So, a combination of mqprio and tario qdiscs used. > Could you please share the commands were used for your setup? > Example command: tc qdisc replace dev eth0 parent root handle 100 taprio num_tc 8 map 0 1 2 3 4 5 6 7 queues 1@0 1@1 1@2 1@3 1@4 1@5 1@6 1@7 base-time 01 sched-entry S 02 300000 flags 0x2 > And couple interesting questions about all of this: > - The taprio qdisc has to have mqprio settings, but if it's done with mqprio then > it just skipped (by reading tc class num). > - If no separate mqprio qdisc configuration then mqprio conf from taprio is set, > who should restore tc mappings when taprio qdisc is unloaded? > Maybe there is reason to implement TC_SETUP_QDISC_MQPRIO offload in > taprio since it's required feature? Mqprio offload with TC_SETUP_QDISC_MQPRIO would be good or even the plus with num_tc would fix some hack. This has a discussion with Vinicius Gomes for a future patch fix. I know the problem is the mqprio will be set after the offload function. But offload function may use some for hardware set. > Would be better to move changes for mqprio in separate patch with > explanation. > > >+ case TC_SETUP_QDISC_TAPRIO: > >+ return enetc_setup_tc_taprio(ndev, type_data); > >+ default: > >+ return -EOPNOTSUPP; > >+ } > >+} > >+ > > [...] > > >diff --git a/drivers/net/ethernet/freescale/enetc/enetc_qos.c > >b/drivers/net/ethernet/freescale/enetc/enetc_qos.c > >new file mode 100644 > >index 000000000000..036bb39c7a0b > >--- /dev/null > >+++ b/drivers/net/ethernet/freescale/enetc/enetc_qos.c > > [...] > > >+static int enetc_setup_taprio(struct net_device *ndev, > >+ struct tc_taprio_qopt_offload *admin_conf) > >+{ > >+ struct enetc_ndev_priv *priv = netdev_priv(ndev); > >+ struct enetc_cbd cbd = {.cmd = 0}; > >+ struct tgs_gcl_conf *gcl_config; > >+ struct tgs_gcl_data *gcl_data; > >+ struct gce *gce; > >+ dma_addr_t dma; > >+ u16 data_size; > >+ u16 gcl_len; > >+ u32 temp; > >+ int i; > >+ > >+ gcl_len = admin_conf->num_entries; > >+ if (gcl_len > enetc_get_max_gcl_len(&priv->si->hw)) > >+ return -EINVAL; > >+ > >+ if (admin_conf->enable) { > >+ enetc_wr(&priv->si->hw, > >+ ENETC_QBV_PTGCR_OFFSET, > >+ temp & (~ENETC_QBV_TGE)); > >+ usleep_range(10, 20); > >+ enetc_wr(&priv->si->hw, > >+ ENETC_QBV_PTGCR_OFFSET, > >+ temp | ENETC_QBV_TGE); > >+ } else { > >+ enetc_wr(&priv->si->hw, > >+ ENETC_QBV_PTGCR_OFFSET, > >+ temp & (~ENETC_QBV_TGE)); > >+ return 0; > >+ } > > Better do the upper qbv enable/disable procedure closer to enetc_send_cmd() > or at least after kzalloc that can fail, no need to restore then. After saving the 'if' suggest by Simon Horman, the enable part could move close to the cmd(). > > >+ > >+ /* Configure the (administrative) gate control list using the > >+ * control BD descriptor. > >+ */ > >+ gcl_config = &cbd.gcl_conf; > >+ > >+ data_size = sizeof(struct tgs_gcl_data) + gcl_len * > >+ sizeof(struct gce); > >+ > >+ gcl_data = kzalloc(data_size, __GFP_DMA | GFP_KERNEL); > >+ if (!gcl_data) > >+ return -ENOMEM; > >+ > >+ gce = (struct gce *)(gcl_data + 1); > >+ > >+ /* Since no initial state config in taprio, set gates open as default. > >+ */ > tc-taprio and IEEE Qbv allows to change configuration in flight, so that oper > state is active till new admin start time. So, here comment says it does initial > state config, if in-flight feature is not supported then error has to be returned > instead of silently rewriting configuration. But if it can be implemented then > state should be remembered/verified in order to not brake oper configuration? I think this is ok as per standard. Also see this comment in net/sched/sch_taprio.c: /* Until the schedule starts, all the queues are open */ I would change the comment. > >+ gcl_config->atc = 0xff; > >+ gcl_config->acl_len = cpu_to_le16(gcl_len); > > Ok, this is maximum number of schedules. > According to tc-taprio it's possible to set cycle period more then schedules > actually can consume. If cycle time is more, then last gate's state can be kept > till the end of cycle. But if last schedule has it's own interval set then gates > should be closed till the end of cycle or no? if it has to be closed, then one more > endl schedule should be present closing gates at the end of list for the rest cycle > time. Can be implemented in h/w but just to be sure, how it's done in h/w? > There is already check the list len in up code. if (admin_conf->num_entries > enetc_get_max_gcl_len(&priv->si->hw)) return -EINVAL; gcl_len = admin_conf->num_entries; > >+ > >+ if (!admin_conf->base_time) { > >+ gcl_data->btl = > >+ cpu_to_le32(enetc_rd(&priv->si->hw, ENETC_SICTR0)); > >+ gcl_data->bth = > >+ cpu_to_le32(enetc_rd(&priv->si->hw, ENETC_SICTR1)); > >+ } else { > >+ gcl_data->btl = > >+ cpu_to_le32(lower_32_bits(admin_conf->base_time)); > >+ gcl_data->bth = > >+ cpu_to_le32(upper_32_bits(admin_conf->base_time)); > >+ } > >+ > >+ gcl_data->ct = cpu_to_le32(admin_conf->cycle_time); > >+ gcl_data->cte = cpu_to_le32(admin_conf->cycle_time_extension); > > Not sure it's good idea to write values w/o verification, The cycle time and time > extension is 64 values, so it's supposed them to be more then 4,3 seconds, it's > probably not a case, but better return error if it's more. > Can add a check for cycle time and extension since type not match. > >+ > >+ for (i = 0; i < gcl_len; i++) { > >+ struct tc_taprio_sched_entry *temp_entry; > >+ struct gce *temp_gce = gce + i; > >+ > >+ temp_entry = &admin_conf->entries[i]; > >+ > >+ temp_gce->gate = cpu_to_le32(temp_entry->gate_mask); > > So, gate_mask can have up to 32 traffic classes? :-|. Gate_mask is defined u32 type. Simon has suggested endian issue. Would change in next patch. > > >+ temp_gce->period = cpu_to_le32(temp_entry->interval); > > So, the interval can be up to 4.3 seconds for one schedule? > That is, one cycle can be one schedule. > great. > > >+ } > > There is no schedule cmd set, so only SetGateStates is supported? > But anyway it's Ok. > > >+ > >+ cbd.length = cpu_to_le16(data_size); > >+ cbd.status_flags = 0; > >+ > >+ dma = dma_map_single(&priv->si->pdev->dev, gcl_data, > >+ data_size, DMA_TO_DEVICE); > >+ if (dma_mapping_error(&priv->si->pdev->dev, dma)) { > >+ netdev_err(priv->si->ndev, "DMA mapping failed!\n"); > >+ kfree(gcl_data); > >+ return -ENOMEM; > >+ } > >+ > >+ cbd.addr[0] = lower_32_bits(dma); > >+ cbd.addr[1] = upper_32_bits(dma); > >+ cbd.cls = BDCR_CMD_PORT_GCL; > >+ > >+ /* Updated by ENETC on completion of the configuration > >+ * command. A zero value indicates success. > >+ */ > >+ cbd.status_flags = 0; > > It's updated on completion by setting 0 on success, then why it's here set to 0 > but not 1 and not verified afterwards? > This byte is feedback by hardware after enetc_send_cmd. Hardware require the cbd space set status_flags 0 before send to hardware. No choice of larger than 0 before send to hardware. But you mind me the enetc_send_cmd() need to check return value. > >+ > >+ enetc_send_cmd(priv->si, &cbd); > >+ > >+ dma_unmap_single(&priv->si->pdev->dev, dma, data_size, > DMA_TO_DEVICE); > >+ kfree(gcl_data); > >+ > >+ return 0; > >+} > >+ > >+int enetc_setup_tc_taprio(struct net_device *ndev, void *type_data) { > >+ struct tc_taprio_qopt_offload *taprio = type_data; > >+ struct enetc_ndev_priv *priv = netdev_priv(ndev); > >+ int i; > >+ > >+ for (i = 0; i < priv->num_tx_rings; i++) > >+ enetc_set_bdr_prio(&priv->si->hw, > >+ priv->tx_ring[i]->index, > >+ taprio->enable ? i : 0); > > then why enable/disable at the beginning for whole qbv scheduler, maybe > better do it together? Or better say, what if setup_taprio failed, who restore > configuration? You remind me the enetc_send_cmd() need to check return value. > >+ > >+ return enetc_setup_taprio(ndev, taprio); } > >-- > >2.17.1 > > > > -- > Regards, > Ivan Khoronzhuk
On Wed, Nov 13, 2019 at 03:45:08AM +0000, Po Liu wrote: >Hi Ivan, > >> -----Original Message----- >> From: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> >> Sent: 2019年11月13日 5:10 >> To: Po Liu <po.liu@nxp.com> >> Cc: Claudiu Manoil <claudiu.manoil@nxp.com>; davem@davemloft.net; linux- >> kernel@vger.kernel.org; netdev@vger.kernel.org; vinicius.gomes@intel.com; >> Vladimir Oltean <vladimir.oltean@nxp.com>; Alexandru Marginean >> <alexandru.marginean@nxp.com>; Xiaoliang Yang >> <xiaoliang.yang_1@nxp.com>; Roy Zang <roy.zang@nxp.com>; Mingkai Hu >> <mingkai.hu@nxp.com>; Jerry Huang <jerry.huang@nxp.com>; Leo Li >> <leoyang.li@nxp.com> >> Subject: [EXT] Re: [v2,net-next, 1/2] enetc: Configure the Time-Aware Scheduler >> via tc-taprio offload >> >> Caution: EXT Email >> >> Hello, >> >> On Tue, Nov 12, 2019 at 08:42:49AM +0000, Po Liu wrote: >> >ENETC supports in hardware for time-based egress shaping according to >> >IEEE 802.1Qbv. This patch implement the Qbv enablement by the hardware >> >offload method qdisc tc-taprio method. >> >Also update cbdr writeback to up level since control bd ring may >> >writeback data to control bd ring. >> > >> >Signed-off-by: Po Liu <Po.Liu@nxp.com> >> >Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> >> >Signed-off-by: Claudiu Manoil <claudiu.manoil@nxp.com> >> >--- >> >changes: >> >- introduce a local define CONFIG_FSL_ENETC_QOS to fix the various >> > configurations will result in link errors. >> > Since the CONFIG_NET_SCH_TAPRIO depends on many Qos configs. Not >> > to use it directly in driver. Add it to CONFIG_FSL_ENETC_QOS depends >> > on list, so only CONFIG_NET_SCH_TAPRIO enabled, user can enable this >> > tsn feature, or else, return not support. >> > >> > drivers/net/ethernet/freescale/enetc/Kconfig | 10 ++ >> > drivers/net/ethernet/freescale/enetc/Makefile | 1 + >> > drivers/net/ethernet/freescale/enetc/enetc.c | 19 ++- >> > drivers/net/ethernet/freescale/enetc/enetc.h | 7 + >> > .../net/ethernet/freescale/enetc/enetc_cbdr.c | 5 +- >> > .../net/ethernet/freescale/enetc/enetc_hw.h | 150 ++++++++++++++++-- >> > .../net/ethernet/freescale/enetc/enetc_qos.c | 130 +++++++++++++++ >> > 7 files changed, 300 insertions(+), 22 deletions(-) create mode 100644 >> > drivers/net/ethernet/freescale/enetc/enetc_qos.c >> > >> >> [...] >> >> > >> >@@ -1483,6 +1479,19 @@ int enetc_setup_tc(struct net_device *ndev, enum >> tc_setup_type type, >> > return 0; >> > } >> > >> >+int enetc_setup_tc(struct net_device *ndev, enum tc_setup_type type, >> >+ void *type_data) >> >+{ >> >+ switch (type) { >> >+ case TC_SETUP_QDISC_MQPRIO: >> >+ return enetc_setup_tc_mqprio(ndev, type_data); >> Sorry didn't see v2, so i duplicate my question here: >> >> This patch is for taprio offload, I see that mqprio is related and is part of taprio >> offload configuration. But taprio offload has own mqprio settings. >> The taprio mqprio part is not offloaded with TC_SETUP_QDISC_MQPRIO. >> >> So, a combination of mqprio and tario qdiscs used. >> Could you please share the commands were used for your setup? >> > >Example command: >tc qdisc replace dev eth0 parent root handle 100 taprio num_tc 8 map 0 1 2 3 4 5 6 7 queues 1@0 1@1 1@2 1@3 1@4 1@5 1@6 1@7 base-time 01 sched-entry S 02 300000 flags 0x2 So, the TC_SETUP_QDISC_MQPRIO is really not required here, and mqprio qdisc is not used. Then why is it here, should be placed in separate patch at least. But even the comb mqprio qdisc and taprio qdisc are used together then taprio requires hw offload also. I'm Ok to add it later to taprio, and I'm asking about it because I need it also, what ever way it could be added. Not clear how you combined mqprio qdisc and taprio now to get it working From this command: tc qdisc replace dev eth0 parent root handle 100 taprio num_tc 8 map 0 1 2 3 4 5 6 7 queues 1@0 1@1 1@2 1@3 1@4 1@5 1@6 1@7 base-time 01 sched-entry S 02 300000 flags 0x2 I can conclude that some default h/w mapping (one to one) were used and no need to use mqprio qdisc, the command above is enough. Is it true? Then move it to spearate patch please as it's not taprio related yet. > >> And couple interesting questions about all of this: >> - The taprio qdisc has to have mqprio settings, but if it's done with mqprio then >> it just skipped (by reading tc class num). >> - If no separate mqprio qdisc configuration then mqprio conf from taprio is set, >> who should restore tc mappings when taprio qdisc is unloaded? >> Maybe there is reason to implement TC_SETUP_QDISC_MQPRIO offload in >> taprio since it's required feature? > [...] >> >> >+ >> >+ /* Configure the (administrative) gate control list using the >> >+ * control BD descriptor. >> >+ */ >> >+ gcl_config = &cbd.gcl_conf; >> >+ >> >+ data_size = sizeof(struct tgs_gcl_data) + gcl_len * >> >+ sizeof(struct gce); >> >+ >> >+ gcl_data = kzalloc(data_size, __GFP_DMA | GFP_KERNEL); >> >+ if (!gcl_data) >> >+ return -ENOMEM; >> >+ >> >+ gce = (struct gce *)(gcl_data + 1); >> >+ >> >+ /* Since no initial state config in taprio, set gates open as default. >> >+ */ >> tc-taprio and IEEE Qbv allows to change configuration in flight, so that oper >> state is active till new admin start time. So, here comment says it does initial >> state config, if in-flight feature is not supported then error has to be returned >> instead of silently rewriting configuration. But if it can be implemented then >> state should be remembered/verified in order to not brake oper configuration? > >I think this is ok as per standard. Also see this comment in >net/sched/sch_taprio.c: From the code above (duplicate for convenience): if (admin_conf->enable) { enetc_wr(&priv->si->hw, ENETC_QBV_PTGCR_OFFSET, temp & (~ENETC_QBV_TGE)); usleep_range(10, 20); enetc_wr(&priv->si->hw, ENETC_QBV_PTGCR_OFFSET, temp | ENETC_QBV_TGE); } else { enetc_wr(&priv->si->hw, ENETC_QBV_PTGCR_OFFSET, temp & (~ENETC_QBV_TGE)); return 0; } I see that's not true, as Simon noted, you have same command: enetc_wr(&priv->si->hw, ENETC_QBV_PTGCR_OFFSET, temp & (~ENETC_QBV_TGE)); before enabling and for disabling. I guess it's stop command, that is disable qbv. So, before enabling new configuration with enetc_setup_tc(), the tario is inited|cleared|reseted|rebooted|defaulted|offed but not updated. It means no in-flight capabilities or they are ignored. JFI, it's possible to do first time: tc qdisc replace dev eth0 parent root handle 100 taprio num_tc 8 map 0 1 2 3 4 5 6 7 queues 1@0 1@1 1@2 1@3 1@4 1@5 1@6 1@7 base-time 01 sched-entry S 02 300000 flags 0x2 and then: tc qdisc replace dev eth0 parent root handle 100 taprio base-time 01 sched-entry S 02 200000 flags 0x2 Do it many times, but w/o mqprio configuration. So, this function must return error if it cannot be done, as above commands suppose that configuration can be updated in runtime, that is, set ADMIN cycle while OPER cycle is still active for some time and not broken like it's done now. If it can be achieved then no need to do enetc_wr(&priv->si->hw, ENETC_QBV_PTGCR_OFFSET, temp & (~ENETC_QBV_TGE)); when admin_conf->enable. So or return error, or do it appropriately. > > /* Until the schedule starts, all the queues are open */ >I would change the comment. > >> >+ gcl_config->atc = 0xff; >> >+ gcl_config->acl_len = cpu_to_le16(gcl_len); >> >> Ok, this is maximum number of schedules. >> According to tc-taprio it's possible to set cycle period more then schedules >> actually can consume. If cycle time is more, then last gate's state can be kept >> till the end of cycle. But if last schedule has it's own interval set then gates >> should be closed till the end of cycle or no? if it has to be closed, then one more >> endl schedule should be present closing gates at the end of list for the rest cycle >> time. Can be implemented in h/w but just to be sure, how it's done in h/w? >> >There is already check the list len in up code. >if (admin_conf->num_entries > enetc_get_max_gcl_len(&priv->si->hw)) > return -EINVAL; >gcl_len = admin_conf->num_entries; I mean +1 schedule to finalize the cycle with closed gates if last schedule has provided time interval. If I set couple schedules, with intervals, and cycle time more then those schedules consume, then I suppose the gates closed for the rest time of cycle, probably. Example: sched1 ----> shced2 ----> time w/o scheds -> cycle end gate 1 gate 2 no gates But if shced2 is last one then gate 2 is opened till the end of cycle. So, to close gate one more shched is needed with closed gates to finalize it. Or it's supposed that gate2 is opened till the end of cycle, How is it in you case. Or this is can be provided by configuration from tc? It's question not statement. Just though. Anyway i'll verify later it can be done with tc and how it's done in sw version, just interesting how your h/w works. > >> >+ >> >+ if (!admin_conf->base_time) { >> >+ gcl_data->btl = >> >+ cpu_to_le32(enetc_rd(&priv->si->hw, ENETC_SICTR0)); >> >+ gcl_data->bth = >> >+ cpu_to_le32(enetc_rd(&priv->si->hw, ENETC_SICTR1)); [...]
Hi Ivan, > -----Original Message----- > From: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> > Sent: 2019年11月13日 21:42 > To: Po Liu <po.liu@nxp.com> > Cc: Claudiu Manoil <claudiu.manoil@nxp.com>; davem@davemloft.net; linux- > kernel@vger.kernel.org; netdev@vger.kernel.org; vinicius.gomes@intel.com; > Vladimir Oltean <vladimir.oltean@nxp.com>; Alexandru Marginean > <alexandru.marginean@nxp.com>; Xiaoliang Yang > <xiaoliang.yang_1@nxp.com>; Roy Zang <roy.zang@nxp.com>; Mingkai Hu > <mingkai.hu@nxp.com>; Jerry Huang <jerry.huang@nxp.com>; Leo Li > <leoyang.li@nxp.com> > Subject: Re: [EXT] Re: [v2,net-next, 1/2] enetc: Configure the Time-Aware > Scheduler via tc-taprio offload > > Caution: EXT Email > > On Wed, Nov 13, 2019 at 03:45:08AM +0000, Po Liu wrote: > >Hi Ivan, > > > >> -----Original Message----- > >> From: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> > >> Sent: 2019年11月13日 5:10 > >> To: Po Liu <po.liu@nxp.com> > >> Cc: Claudiu Manoil <claudiu.manoil@nxp.com>; davem@davemloft.net; > >> linux- kernel@vger.kernel.org; netdev@vger.kernel.org; > >> vinicius.gomes@intel.com; Vladimir Oltean <vladimir.oltean@nxp.com>; > >> Alexandru Marginean <alexandru.marginean@nxp.com>; Xiaoliang Yang > >> <xiaoliang.yang_1@nxp.com>; Roy Zang <roy.zang@nxp.com>; Mingkai Hu > >> <mingkai.hu@nxp.com>; Jerry Huang <jerry.huang@nxp.com>; Leo Li > >> <leoyang.li@nxp.com> > >> Subject: [EXT] Re: [v2,net-next, 1/2] enetc: Configure the Time-Aware > >> Scheduler via tc-taprio offload > >> > >> Caution: EXT Email > >> > >> Hello, > >> > >> On Tue, Nov 12, 2019 at 08:42:49AM +0000, Po Liu wrote: > >> >ENETC supports in hardware for time-based egress shaping according > >> >to IEEE 802.1Qbv. This patch implement the Qbv enablement by the > >> >hardware offload method qdisc tc-taprio method. > >> >Also update cbdr writeback to up level since control bd ring may > >> >writeback data to control bd ring. > >> > > >> >Signed-off-by: Po Liu <Po.Liu@nxp.com> > >> >Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> > >> >Signed-off-by: Claudiu Manoil <claudiu.manoil@nxp.com> > >> >--- > >> >changes: > >> >- introduce a local define CONFIG_FSL_ENETC_QOS to fix the various > >> > configurations will result in link errors. > >> > Since the CONFIG_NET_SCH_TAPRIO depends on many Qos configs. Not > >> > to use it directly in driver. Add it to CONFIG_FSL_ENETC_QOS > >> >depends > >> > on list, so only CONFIG_NET_SCH_TAPRIO enabled, user can enable > >> >this > >> > tsn feature, or else, return not support. > >> > > >> > drivers/net/ethernet/freescale/enetc/Kconfig | 10 ++ > >> > drivers/net/ethernet/freescale/enetc/Makefile | 1 + > >> > drivers/net/ethernet/freescale/enetc/enetc.c | 19 ++- > >> > drivers/net/ethernet/freescale/enetc/enetc.h | 7 + > >> > .../net/ethernet/freescale/enetc/enetc_cbdr.c | 5 +- > >> > .../net/ethernet/freescale/enetc/enetc_hw.h | 150 ++++++++++++++++-- > >> > .../net/ethernet/freescale/enetc/enetc_qos.c | 130 +++++++++++++++ > >> > 7 files changed, 300 insertions(+), 22 deletions(-) create mode > >> > 100644 drivers/net/ethernet/freescale/enetc/enetc_qos.c > >> > > >> > >> [...] > >> > >> > > >> >@@ -1483,6 +1479,19 @@ int enetc_setup_tc(struct net_device *ndev, > >> >enum > >> tc_setup_type type, > >> > return 0; > >> > } > >> > > >> >+int enetc_setup_tc(struct net_device *ndev, enum tc_setup_type type, > >> >+ void *type_data) > >> >+{ > >> >+ switch (type) { > >> >+ case TC_SETUP_QDISC_MQPRIO: > >> >+ return enetc_setup_tc_mqprio(ndev, type_data); > >> Sorry didn't see v2, so i duplicate my question here: > >> > >> This patch is for taprio offload, I see that mqprio is related and is > >> part of taprio offload configuration. But taprio offload has own mqprio > settings. > >> The taprio mqprio part is not offloaded with TC_SETUP_QDISC_MQPRIO. > >> > >> So, a combination of mqprio and tario qdiscs used. > >> Could you please share the commands were used for your setup? > >> > > > >Example command: > >tc qdisc replace dev eth0 parent root handle 100 taprio num_tc 8 map 0 > >1 2 3 4 5 6 7 queues 1@0 1@1 1@2 1@3 1@4 1@5 1@6 1@7 base-time 01 > >sched-entry S 02 300000 flags 0x2 > > So, the TC_SETUP_QDISC_MQPRIO is really not required here, and mqprio qdisc > is not used. Then why is it here, should be placed in separate patch at least. > Taprio is not fully copy the mqprio but refer the mqprio feature. I don't think there is Problem for them all list in driver. Some person may not use taprio at all. > But even the comb mqprio qdisc and taprio qdisc are used together then taprio > requires hw offload also. I'm Ok to add it later to taprio, and I'm asking about it > because I need it also, what ever way it could be added. > > Not clear how you combined mqprio qdisc and taprio now to get it working > From this command: > > tc qdisc replace dev eth0 parent root handle 100 taprio num_tc 8 map 0 1 2 3 > 4 > 5 6 7 queues 1@0 1@1 1@2 1@3 1@4 1@5 1@6 1@7 base-time 01 sched- > entry S 02 > 300000 flags 0x2 > > I can conclude that some default h/w mapping (one to one) were used and no > need to use mqprio qdisc, the command above is enough. > > Is it true? > Then move it to spearate patch please as it's not taprio related yet. As mentioned, to set the queue mapping it is require the data_type include the mqprio parameter. So here hardware set it as default. Without the settting, taprio would not say it is implement the offload. So I don’t think it is proper to seperate another patch. > > > >> And couple interesting questions about all of this: > >> - The taprio qdisc has to have mqprio settings, but if it's done with > >> mqprio then it just skipped (by reading tc class num). > >> - If no separate mqprio qdisc configuration then mqprio conf from > >> taprio is set, who should restore tc mappings when taprio qdisc is unloaded? > >> Maybe there is reason to implement TC_SETUP_QDISC_MQPRIO offload in > >> taprio since it's required feature? > > > > [...] > > >> > >> >+ > >> >+ /* Configure the (administrative) gate control list using the > >> >+ * control BD descriptor. > >> >+ */ > >> >+ gcl_config = &cbd.gcl_conf; > >> >+ > >> >+ data_size = sizeof(struct tgs_gcl_data) + gcl_len * > >> >+ sizeof(struct gce); > >> >+ > >> >+ gcl_data = kzalloc(data_size, __GFP_DMA | GFP_KERNEL); > >> >+ if (!gcl_data) > >> >+ return -ENOMEM; > >> >+ > >> >+ gce = (struct gce *)(gcl_data + 1); > >> >+ > >> >+ /* Since no initial state config in taprio, set gates open as default. > >> >+ */ > >> tc-taprio and IEEE Qbv allows to change configuration in flight, so > >> that oper state is active till new admin start time. So, here comment > >> says it does initial state config, if in-flight feature is not > >> supported then error has to be returned instead of silently rewriting > >> configuration. But if it can be implemented then state should be > remembered/verified in order to not brake oper configuration? > > > >I think this is ok as per standard. Also see this comment in > >net/sched/sch_taprio.c: > > From the code above (duplicate for convenience): > if (admin_conf->enable) { > enetc_wr(&priv->si->hw, > ENETC_QBV_PTGCR_OFFSET, > temp & (~ENETC_QBV_TGE)); > usleep_range(10, 20); > enetc_wr(&priv->si->hw, > ENETC_QBV_PTGCR_OFFSET, > temp | ENETC_QBV_TGE); > } else { > enetc_wr(&priv->si->hw, > ENETC_QBV_PTGCR_OFFSET, > temp & (~ENETC_QBV_TGE)); > return 0; > } > > I see that's not true, as Simon noted, you have same command: > > enetc_wr(&priv->si->hw, ENETC_QBV_PTGCR_OFFSET, temp & > (~ENETC_QBV_TGE)); > > before enabling and for disabling. I guess it's stop command, that is disable qbv. > So, before enabling new configuration with enetc_setup_tc(), the tario is > inited|cleared|reseted|rebooted|defaulted|offed but not updated. It > inited|cleared|reseted|rebooted|defaulted|means no > in-flight capabilities or they are ignored. The Qbv spec do not show the disable before enable. Here is hardware workaround to avoid the hardware run into unknow state. > > JFI, it's possible to do first time: > > tc qdisc replace dev eth0 parent root handle 100 taprio num_tc 8 map 0 1 2 3 > 4 > 5 6 7 queues 1@0 1@1 1@2 1@3 1@4 1@5 1@6 1@7 base-time 01 sched- > entry S 02 > 300000 flags 0x2 > > and then: > > tc qdisc replace dev eth0 parent root handle 100 taprio base-time 01 sched- > entry S 02 200000 flags 0x2 > > Do it many times, but w/o mqprio configuration. > > So, this function must return error if it cannot be done, as above commands > suppose that configuration can be updated in runtime, that is, set ADMIN cycle > while OPER cycle is still active for some time and not broken like it's done now. > If it can be achieved then no need to do enetc_wr(&priv->si->hw, > ENETC_QBV_PTGCR_OFFSET, temp & (~ENETC_QBV_TGE)); when admin_conf- > >enable. > > So or return error, or do it appropriately. > > > > > /* Until the schedule starts, all the queues are open */ I would > >change the comment. > > > >> >+ gcl_config->atc = 0xff; > >> >+ gcl_config->acl_len = cpu_to_le16(gcl_len); > >> > >> Ok, this is maximum number of schedules. > >> According to tc-taprio it's possible to set cycle period more then > >> schedules actually can consume. If cycle time is more, then last > >> gate's state can be kept till the end of cycle. But if last schedule > >> has it's own interval set then gates should be closed till the end of > >> cycle or no? if it has to be closed, then one more endl schedule > >> should be present closing gates at the end of list for the rest cycle time. Can > be implemented in h/w but just to be sure, how it's done in h/w? > >> > >There is already check the list len in up code. > >if (admin_conf->num_entries > enetc_get_max_gcl_len(&priv->si->hw)) > > return -EINVAL; > >gcl_len = admin_conf->num_entries; > > I mean +1 schedule to finalize the cycle with closed gates if last schedule has > provided time interval. If I set couple schedules, with intervals, and cycle time > more then those schedules consume, then I suppose the gates closed for the > rest time of cycle, probably. > > Example: > > sched1 ----> shced2 ----> time w/o scheds -> cycle end > gate 1 gate 2 no gates > > But if shced2 is last one then gate 2 is opened till the end of cycle. > So, to close gate one more shched is needed with closed gates to finalize it. > Or it's supposed that gate2 is opened till the end of cycle, How is it in you case. > Or this is can be provided by configuration from tc? > > It's question not statement. Just though. Anyway i'll verify later it can be done > with tc and how it's done in sw version, just interesting how your h/w works. What user command set the gate list would set into the hardware. We have detail setting in the user manual which you can have it try. > > > >> >+ > >> >+ if (!admin_conf->base_time) { > >> >+ gcl_data->btl = > >> >+ cpu_to_le32(enetc_rd(&priv->si->hw, ENETC_SICTR0)); > >> >+ gcl_data->bth = > >> >+ cpu_to_le32(enetc_rd(&priv->si->hw, > >> >+ ENETC_SICTR1)); > > [...] > > -- > Regards, > Ivan Khoronzhuk
diff --git a/drivers/net/ethernet/freescale/enetc/Kconfig b/drivers/net/ethernet/freescale/enetc/Kconfig index c219587bd334..017ade2d0e50 100644 --- a/drivers/net/ethernet/freescale/enetc/Kconfig +++ b/drivers/net/ethernet/freescale/enetc/Kconfig @@ -50,3 +50,13 @@ config FSL_ENETC_HW_TIMESTAMPING allocation has not been supported and it is too expensive to use extended RX BDs if timestamping is not used, this option enables extended RX BDs in order to support hardware timestamping. + +config FSL_ENETC_QOS + bool "ENETC hardware Time-sensitive Network support" + depends on FSL_ENETC && NET_SCH_TAPRIO + help + There are Time-Sensitive Network(TSN) capabilities(802.1Qbv/802.1Qci + /802.1Qbu etc.) supported by ENETC. These TSN capabilities can be set + enable/disable from user space via Qos commands(tc). In the kernel + side, it can be loaded by Qos driver. Currently, it is only support + taprio(802.1Qbv). diff --git a/drivers/net/ethernet/freescale/enetc/Makefile b/drivers/net/ethernet/freescale/enetc/Makefile index d200c27c3bf6..7a80680a7634 100644 --- a/drivers/net/ethernet/freescale/enetc/Makefile +++ b/drivers/net/ethernet/freescale/enetc/Makefile @@ -5,6 +5,7 @@ common-objs := enetc.o enetc_cbdr.o enetc_ethtool.o obj-$(CONFIG_FSL_ENETC) += fsl-enetc.o fsl-enetc-y := enetc_pf.o enetc_mdio.o $(common-objs) fsl-enetc-$(CONFIG_PCI_IOV) += enetc_msg.o +fsl-enetc-$(CONFIG_FSL_ENETC_QOS) += enetc_qos.o obj-$(CONFIG_FSL_ENETC_VF) += fsl-enetc-vf.o fsl-enetc-vf-y := enetc_vf.o $(common-objs) diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c b/drivers/net/ethernet/freescale/enetc/enetc.c index 3e8f9819f08c..d58dbc2c4270 100644 --- a/drivers/net/ethernet/freescale/enetc/enetc.c +++ b/drivers/net/ethernet/freescale/enetc/enetc.c @@ -1427,8 +1427,7 @@ int enetc_close(struct net_device *ndev) return 0; } -int enetc_setup_tc(struct net_device *ndev, enum tc_setup_type type, - void *type_data) +int enetc_setup_tc_mqprio(struct net_device *ndev, void *type_data) { struct enetc_ndev_priv *priv = netdev_priv(ndev); struct tc_mqprio_qopt *mqprio = type_data; @@ -1436,9 +1435,6 @@ int enetc_setup_tc(struct net_device *ndev, enum tc_setup_type type, u8 num_tc; int i; - if (type != TC_SETUP_QDISC_MQPRIO) - return -EOPNOTSUPP; - mqprio->hw = TC_MQPRIO_HW_OFFLOAD_TCS; num_tc = mqprio->num_tc; @@ -1483,6 +1479,19 @@ int enetc_setup_tc(struct net_device *ndev, enum tc_setup_type type, return 0; } +int enetc_setup_tc(struct net_device *ndev, enum tc_setup_type type, + void *type_data) +{ + switch (type) { + case TC_SETUP_QDISC_MQPRIO: + return enetc_setup_tc_mqprio(ndev, type_data); + case TC_SETUP_QDISC_TAPRIO: + return enetc_setup_tc_taprio(ndev, type_data); + default: + return -EOPNOTSUPP; + } +} + struct net_device_stats *enetc_get_stats(struct net_device *ndev) { struct enetc_ndev_priv *priv = netdev_priv(ndev); diff --git a/drivers/net/ethernet/freescale/enetc/enetc.h b/drivers/net/ethernet/freescale/enetc/enetc.h index 541b4e2073fe..8ca2f97050c8 100644 --- a/drivers/net/ethernet/freescale/enetc/enetc.h +++ b/drivers/net/ethernet/freescale/enetc/enetc.h @@ -244,3 +244,10 @@ int enetc_set_fs_entry(struct enetc_si *si, struct enetc_cmd_rfse *rfse, void enetc_set_rss_key(struct enetc_hw *hw, const u8 *bytes); int enetc_get_rss_table(struct enetc_si *si, u32 *table, int count); int enetc_set_rss_table(struct enetc_si *si, const u32 *table, int count); +int enetc_send_cmd(struct enetc_si *si, struct enetc_cbd *cbd); + +#ifdef CONFIG_FSL_ENETC_QOS +int enetc_setup_tc_taprio(struct net_device *ndev, void *type_data); +#else +#define enetc_setup_tc_taprio(ndev, type_data) -EOPNOTSUPP +#endif diff --git a/drivers/net/ethernet/freescale/enetc/enetc_cbdr.c b/drivers/net/ethernet/freescale/enetc/enetc_cbdr.c index de466b71bf8f..201cbc362e33 100644 --- a/drivers/net/ethernet/freescale/enetc/enetc_cbdr.c +++ b/drivers/net/ethernet/freescale/enetc/enetc_cbdr.c @@ -32,7 +32,7 @@ static int enetc_cbd_unused(struct enetc_cbdr *r) r->bd_count; } -static int enetc_send_cmd(struct enetc_si *si, struct enetc_cbd *cbd) +int enetc_send_cmd(struct enetc_si *si, struct enetc_cbd *cbd) { struct enetc_cbdr *ring = &si->cbd_ring; int timeout = ENETC_CBDR_TIMEOUT; @@ -66,6 +66,9 @@ static int enetc_send_cmd(struct enetc_si *si, struct enetc_cbd *cbd) if (!timeout) return -EBUSY; + /* CBD may writeback data, feedback up level */ + *cbd = *dest_cbd; + enetc_clean_cbdr(si); return 0; diff --git a/drivers/net/ethernet/freescale/enetc/enetc_hw.h b/drivers/net/ethernet/freescale/enetc/enetc_hw.h index 88276299f447..75a7c0f1f8ce 100644 --- a/drivers/net/ethernet/freescale/enetc/enetc_hw.h +++ b/drivers/net/ethernet/freescale/enetc/enetc_hw.h @@ -18,6 +18,7 @@ #define ENETC_SICTR0 0x18 #define ENETC_SICTR1 0x1c #define ENETC_SIPCAPR0 0x20 +#define ENETC_SIPCAPR0_QBV BIT(4) #define ENETC_SIPCAPR0_RSS BIT(8) #define ENETC_SIPCAPR1 0x24 #define ENETC_SITGTGR 0x30 @@ -148,6 +149,12 @@ enum enetc_bdr_type {TX, RX}; #define ENETC_PORT_BASE 0x10000 #define ENETC_PMR 0x0000 #define ENETC_PMR_EN GENMASK(18, 16) +#define ENETC_PMR_PSPEED_MASK GENMASK(11, 8) +#define ENETC_PMR_PSPEED_10M 0x000 +#define ENETC_PMR_PSPEED_100M 0x100 +#define ENETC_PMR_PSPEED_1000M 0x200 +#define ENETC_PMR_PSPEED_2500M 0x400 + #define ENETC_PSR 0x0004 /* RO */ #define ENETC_PSIPMR 0x0018 #define ENETC_PSIPMR_SET_UP(n) BIT(n) /* n = SI index */ @@ -440,22 +447,6 @@ union enetc_rx_bd { #define EMETC_MAC_ADDR_FILT_RES 3 /* # of reserved entries at the beginning */ #define ENETC_MAX_NUM_VFS 2 -struct enetc_cbd { - union { - struct { - __le32 addr[2]; - __le32 opt[4]; - }; - __le32 data[6]; - }; - __le16 index; - __le16 length; - u8 cmd; - u8 cls; - u8 _res; - u8 status_flags; -}; - #define ENETC_CBD_FLAGS_SF BIT(7) /* short format */ #define ENETC_CBD_STATUS_MASK 0xf @@ -554,3 +545,130 @@ static inline void enetc_set_bdr_prio(struct enetc_hw *hw, int bdr_idx, val |= ENETC_TBMR_SET_PRIO(prio); enetc_txbdr_wr(hw, bdr_idx, ENETC_TBMR, val); } + +enum bdcr_cmd_class { + BDCR_CMD_UNSPEC = 0, + BDCR_CMD_MAC_FILTER, + BDCR_CMD_VLAN_FILTER, + BDCR_CMD_RSS, + BDCR_CMD_RFS, + BDCR_CMD_PORT_GCL, + BDCR_CMD_RECV_CLASSIFIER, + __BDCR_CMD_MAX_LEN, + BDCR_CMD_MAX_LEN = __BDCR_CMD_MAX_LEN - 1, +}; + +/* class 5, command 0 */ +struct tgs_gcl_conf { + u8 atc; /* init gate value */ + u8 res[7]; + union { + struct { + u8 res1[4]; + __le16 acl_len; + u8 res2[2]; + }; + struct { + u32 cctl; + u32 ccth; + }; + }; +}; + +#define ENETC_CBDR_SGL_IOMEN BIT(0) +#define ENETC_CBDR_SGL_IPVEN BIT(3) +#define ENETC_CBDR_SGL_GTST BIT(4) +#define ENETC_CBDR_SGL_IPV_MASK 0xe + +/* gate control list entry */ +struct gce { + u32 period; + u8 gate; + u8 res[3]; +}; + +/* tgs_gcl_conf address point to this data space */ +struct tgs_gcl_data { + u32 btl; + u32 bth; + u32 ct; + u32 cte; +}; + +/* class 5, command 1 */ +struct tgs_gcl_query { + u8 res[12]; + union { + struct { + __le16 acl_len; /* admin list length */ + __le16 ocl_len; /* operation list length */ + }; + struct { + u16 admin_list_len; + u16 oper_list_len; + }; + }; +}; + +/* tgs_gcl_query command response data format */ +struct tgs_gcl_resp { + u32 abtl; /* base time */ + u32 abth; + u32 act; /* cycle time */ + u32 acte; /* cycle time extend */ + u32 cctl; /* config change time */ + u32 ccth; + u32 obtl; /* operation base time */ + u32 obth; + u32 oct; /* operation cycle time */ + u32 octe; /* operation cycle time extend */ + u32 ccel; /* config change error */ + u32 cceh; +}; + +struct enetc_cbd { + union{ + struct { + __le32 addr[2]; + union { + __le32 opt[4]; + struct tgs_gcl_conf gcl_conf; + struct tgs_gcl_query gcl_query; + }; + }; /* Long format */ + __le32 data[6]; + }; + __le16 index; + __le16 length; + u8 cmd; + u8 cls; + u8 _res; + u8 status_flags; +}; + +#define ENETC_PTCFPR(n) (0x1910 + (n) * 4) /* n = [0 ..7] */ +#define ENETC_FPE BIT(31) + +/* Port capability register 0 */ +#define ENETC_PCAPR0_PSFPM BIT(10) +#define ENETC_PCAPR0_PSFP BIT(9) +#define ENETC_PCAPR0_TSN BIT(4) +#define ENETC_PCAPR0_QBU BIT(3) + +/* port time gating control register */ +#define ENETC_QBV_PTGCR_OFFSET 0x11a00 +#define ENETC_QBV_TGE 0x80000000 +#define ENETC_QBV_TGPE BIT(30) +#define ENETC_QBV_TGDROP_DISABLE BIT(29) + +/* Port time gating capability register */ +#define ENETC_QBV_PTGCAPR_OFFSET 0x11a08 +#define ENETC_QBV_MAX_GCL_LEN_MASK 0xffff + +/* Port time gating admin gate list status register */ +#define ENETC_QBV_PTGAGLSR_OFFSET 0x11a10 +#define ENETC_QBV_CFG_PEND_MASK 0x00000002 + +#define ENETC_TGLSTR 0xa200 +#define ENETC_TGS_MIN_DIS_MASK 0x80000000 +#define ENETC_MIN_LOOKAHEAD_MASK 0xffff diff --git a/drivers/net/ethernet/freescale/enetc/enetc_qos.c b/drivers/net/ethernet/freescale/enetc/enetc_qos.c new file mode 100644 index 000000000000..036bb39c7a0b --- /dev/null +++ b/drivers/net/ethernet/freescale/enetc/enetc_qos.c @@ -0,0 +1,130 @@ +// SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause) +/* Copyright 2019 NXP */ + +#include "enetc.h" + +#include <net/pkt_sched.h> + +static u16 enetc_get_max_gcl_len(struct enetc_hw *hw) +{ + return enetc_rd(hw, ENETC_QBV_PTGCAPR_OFFSET) + & ENETC_QBV_MAX_GCL_LEN_MASK; +} + +static int enetc_setup_taprio(struct net_device *ndev, + struct tc_taprio_qopt_offload *admin_conf) +{ + struct enetc_ndev_priv *priv = netdev_priv(ndev); + struct enetc_cbd cbd = {.cmd = 0}; + struct tgs_gcl_conf *gcl_config; + struct tgs_gcl_data *gcl_data; + struct gce *gce; + dma_addr_t dma; + u16 data_size; + u16 gcl_len; + u32 temp; + int i; + + gcl_len = admin_conf->num_entries; + if (gcl_len > enetc_get_max_gcl_len(&priv->si->hw)) + return -EINVAL; + + if (admin_conf->enable) { + enetc_wr(&priv->si->hw, + ENETC_QBV_PTGCR_OFFSET, + temp & (~ENETC_QBV_TGE)); + usleep_range(10, 20); + enetc_wr(&priv->si->hw, + ENETC_QBV_PTGCR_OFFSET, + temp | ENETC_QBV_TGE); + } else { + enetc_wr(&priv->si->hw, + ENETC_QBV_PTGCR_OFFSET, + temp & (~ENETC_QBV_TGE)); + return 0; + } + + /* Configure the (administrative) gate control list using the + * control BD descriptor. + */ + gcl_config = &cbd.gcl_conf; + + data_size = sizeof(struct tgs_gcl_data) + gcl_len * sizeof(struct gce); + + gcl_data = kzalloc(data_size, __GFP_DMA | GFP_KERNEL); + if (!gcl_data) + return -ENOMEM; + + gce = (struct gce *)(gcl_data + 1); + + /* Since no initial state config in taprio, set gates open as default. + */ + gcl_config->atc = 0xff; + gcl_config->acl_len = cpu_to_le16(gcl_len); + + if (!admin_conf->base_time) { + gcl_data->btl = + cpu_to_le32(enetc_rd(&priv->si->hw, ENETC_SICTR0)); + gcl_data->bth = + cpu_to_le32(enetc_rd(&priv->si->hw, ENETC_SICTR1)); + } else { + gcl_data->btl = + cpu_to_le32(lower_32_bits(admin_conf->base_time)); + gcl_data->bth = + cpu_to_le32(upper_32_bits(admin_conf->base_time)); + } + + gcl_data->ct = cpu_to_le32(admin_conf->cycle_time); + gcl_data->cte = cpu_to_le32(admin_conf->cycle_time_extension); + + for (i = 0; i < gcl_len; i++) { + struct tc_taprio_sched_entry *temp_entry; + struct gce *temp_gce = gce + i; + + temp_entry = &admin_conf->entries[i]; + + temp_gce->gate = cpu_to_le32(temp_entry->gate_mask); + temp_gce->period = cpu_to_le32(temp_entry->interval); + } + + cbd.length = cpu_to_le16(data_size); + cbd.status_flags = 0; + + dma = dma_map_single(&priv->si->pdev->dev, gcl_data, + data_size, DMA_TO_DEVICE); + if (dma_mapping_error(&priv->si->pdev->dev, dma)) { + netdev_err(priv->si->ndev, "DMA mapping failed!\n"); + kfree(gcl_data); + return -ENOMEM; + } + + cbd.addr[0] = lower_32_bits(dma); + cbd.addr[1] = upper_32_bits(dma); + cbd.cls = BDCR_CMD_PORT_GCL; + + /* Updated by ENETC on completion of the configuration + * command. A zero value indicates success. + */ + cbd.status_flags = 0; + + enetc_send_cmd(priv->si, &cbd); + + dma_unmap_single(&priv->si->pdev->dev, dma, data_size, DMA_TO_DEVICE); + kfree(gcl_data); + + return 0; +} + +int enetc_setup_tc_taprio(struct net_device *ndev, void *type_data) +{ + struct tc_taprio_qopt_offload *taprio = type_data; + struct enetc_ndev_priv *priv = netdev_priv(ndev); + int i; + + for (i = 0; i < priv->num_tx_rings; i++) + enetc_set_bdr_prio(&priv->si->hw, + priv->tx_ring[i]->index, + taprio->enable ? i : 0); + + return enetc_setup_taprio(ndev, taprio); +}