Messages in this thread | | | Subject | Re: [PATCH 2/4] net: ethernet: ti: cpsw: add multi queue support | From | Ivan Khoronzhuk <> | Date | Tue, 19 Jul 2016 16:24:41 +0300 |
| |
On 08.07.16 16:12, Grygorii Strashko wrote: > On 06/30/2016 10:04 PM, Ivan Khoronzhuk wrote: >> The cpsw h/w supports up to 8 tx and 8 rx channels.This patch adds >> multi-queue support to the driver. An ability to configure h/w >> shaper will be added with separate patch. Default shaper mode, as >> before, priority mode. >> >> The poll function handles all unprocessed channels, till all of >> them are free, beginning from hi priority channel. >> >> The statistic for every channel can be read with: >> ethtool -S ethX >> >> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> >> --- >> drivers/net/ethernet/ti/cpsw.c | 334 +++++++++++++++++++++----------- >> drivers/net/ethernet/ti/davinci_cpdma.c | 12 ++ >> drivers/net/ethernet/ti/davinci_cpdma.h | 2 + >> 3 files changed, 237 insertions(+), 111 deletions(-) >> >> diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c >> index a713336..14d53eb 100644 >> --- a/drivers/net/ethernet/ti/cpsw.c >> +++ b/drivers/net/ethernet/ti/cpsw.c >> @@ -140,6 +140,8 @@ do { \ >> #define CPSW_CMINTMAX_INTVL (1000 / CPSW_CMINTMIN_CNT) >> #define CPSW_CMINTMIN_INTVL ((1000 / CPSW_CMINTMAX_CNT) + 1) >> >> +#define CPSW_MAX_QUEUES 8 >> + >> #define cpsw_slave_index(priv) \ >> ((priv->data.dual_emac) ? priv->emac_port : \ >> priv->data.active_slave) >> @@ -383,7 +385,8 @@ struct cpsw_priv { >> u8 mac_addr[ETH_ALEN]; >> struct cpsw_slave *slaves; >> struct cpdma_ctlr *dma; >> - struct cpdma_chan *txch, *rxch; >> + struct cpdma_chan *txch[CPSW_MAX_QUEUES]; >> + struct cpdma_chan *rxch[CPSW_MAX_QUEUES]; >> struct cpsw_ale *ale; >> bool rx_pause; >> bool tx_pause; >> @@ -395,6 +398,7 @@ struct cpsw_priv { >> u32 num_irqs; >> struct cpts *cpts; >> u32 emac_port; >> + int rx_ch_num, tx_ch_num; >> }; >> > > [...] > >> >> @@ -989,26 +1024,50 @@ update_return: >> >> static int cpsw_get_sset_count(struct net_device *ndev, int sset) >> { >> + struct cpsw_priv *priv = netdev_priv(ndev); >> + >> switch (sset) { >> case ETH_SS_STATS: >> - return CPSW_STATS_LEN; >> + return (CPSW_STATS_COMMON_LEN + >> + (priv->rx_ch_num + priv->tx_ch_num) * >> + CPSW_STATS_CH_LEN); >> default: >> return -EOPNOTSUPP; >> } >> } >> >> +static void cpsw_add_ch_strings(u8 **p, int ch_num, int rx_dir) >> +{ >> + int ch_stats_len; >> + int line; >> + int i; >> + >> + ch_stats_len = CPSW_STATS_CH_LEN * ch_num; >> + for (i = 0; i < ch_stats_len; i++) { >> + line = i % CPSW_STATS_CH_LEN; >> + sprintf(*p, "%s DMA chan %d: %s", rx_dir ? "Rx" : "Tx", >> + i / CPSW_STATS_CH_LEN, > > snprintf(,ETH_GSTRING_LEN,) ? It's number of channel.
> >> + cpsw_gstrings_ch_stats[line].stat_string); >> + *p += ETH_GSTRING_LEN; >> + } >> +} >> + >> static void cpsw_get_strings(struct net_device *ndev, u32 stringset, u8 *data) >> { >> + struct cpsw_priv *priv = netdev_priv(ndev); >> u8 *p = data; >> int i; >> >> switch (stringset) { >> case ETH_SS_STATS: >> - for (i = 0; i < CPSW_STATS_LEN; i++) { >> + for (i = 0; i < CPSW_STATS_COMMON_LEN; i++) { >> memcpy(p, cpsw_gstrings_stats[i].stat_string, >> ETH_GSTRING_LEN); >> p += ETH_GSTRING_LEN; >> } >> + >> + cpsw_add_ch_strings(&p, priv->rx_ch_num, 1); >> + cpsw_add_ch_strings(&p, priv->tx_ch_num, 0); >> break; >> } >> } >> @@ -1017,35 +1076,38 @@ static void cpsw_get_ethtool_stats(struct net_device *ndev, >> struct ethtool_stats *stats, u64 *data) >> { >> struct cpsw_priv *priv = netdev_priv(ndev); >> - struct cpdma_chan_stats rx_stats; >> - struct cpdma_chan_stats tx_stats; >> - u32 val; >> + struct cpdma_chan_stats ch_stats; >> + int i, l, ch, ret; >> u8 *p; >> - int i; >> + >> + ret = pm_runtime_get_sync(&priv->pdev->dev); >> + if (ret < 0) { >> + pm_runtime_put_noidle(&priv->pdev->dev); >> + return; >> + } > > You probably need to base you work on top of net-next.git Yep. Will correct.
> >> >> /* Collect Davinci CPDMA stats for Rx and Tx Channel */ >> - cpdma_chan_get_stats(priv->rxch, &rx_stats); >> - cpdma_chan_get_stats(priv->txch, &tx_stats); >> - >> - for (i = 0; i < CPSW_STATS_LEN; i++) { >> - switch (cpsw_gstrings_stats[i].type) { >> - case CPSW_STATS: >> - val = readl(priv->hw_stats + >> - cpsw_gstrings_stats[i].stat_offset); >> - data[i] = val; >> - break; >> + for (l = 0; l < CPSW_STATS_COMMON_LEN; l++) >> + data[l] = readl(priv->hw_stats + >> + cpsw_gstrings_stats[l].stat_offset); >> >> - case CPDMA_RX_STATS: >> - p = (u8 *)&rx_stats + >> - cpsw_gstrings_stats[i].stat_offset; >> - data[i] = *(u32 *)p; >> - break; >> + pm_runtime_put(&priv->pdev->dev); >> >> - case CPDMA_TX_STATS: >> - p = (u8 *)&tx_stats + >> - cpsw_gstrings_stats[i].stat_offset; >> - data[i] = *(u32 *)p; >> - break; >> + for (ch = 0; ch < priv->rx_ch_num; ch++) { >> + cpdma_chan_get_stats(priv->rxch[ch], &ch_stats); >> + for (i = 0; i < CPSW_STATS_CH_LEN; i++, l++) { >> + p = (u8 *)&ch_stats + >> + cpsw_gstrings_ch_stats[i].stat_offset; >> + data[l] = *(u32 *)p; >> + } >> + } >> + >> + for (ch = 0; ch < priv->tx_ch_num; ch++) { >> + cpdma_chan_get_stats(priv->txch[ch], &ch_stats); >> + for (i = 0; i < CPSW_STATS_CH_LEN; i++, l++) { >> + p = (u8 *)&ch_stats + >> + cpsw_gstrings_ch_stats[i].stat_offset; >> + data[l] = *(u32 *)p; >> } >> } > > I think, it's better to do pm_runtime_put() here even if now cpdma does'n access > HW from cpdma_chan_get_stats it may change in fufture. > And it's not critical from PM point of view This part is not going to access h/w. The function is ethtool op, and after adding pm_runtime in begin/end ops no need in it. Will correct it in next version.
> >> } >> @@ -1065,19 +1127,29 @@ static int cpsw_common_res_usage_state(struct cpsw_priv *priv) >> return usage_count; >> } >> >> +static inline struct cpdma_chan * >> +cpsw_tx_queue_mapping(struct cpsw_priv *priv, struct sk_buff *skb) >> +{ >> + unsigned int q_idx = skb_get_queue_mapping(skb); >> + >> + if (q_idx >= priv->tx_ch_num) >> + q_idx = q_idx % priv->tx_ch_num; >> + >> + return priv->txch[q_idx]; >> +} >> + >> static inline int cpsw_tx_packet_submit(struct net_device *ndev, >> - struct cpsw_priv *priv, struct sk_buff *skb) >> + struct cpsw_priv *priv, >> + struct sk_buff *skb, >> + struct cpdma_chan *txch) >> { >> if (!priv->data.dual_emac) >> - return cpdma_chan_submit(priv->txch, skb, skb->data, >> - skb->len, 0); >> + return cpdma_chan_submit(txch, skb, skb->data, skb->len, 0); >> >> if (ndev == cpsw_get_slave_ndev(priv, 0)) >> - return cpdma_chan_submit(priv->txch, skb, skb->data, >> - skb->len, 1); >> + return cpdma_chan_submit(txch, skb, skb->data, skb->len, 1); >> else >> - return cpdma_chan_submit(priv->txch, skb, skb->data, >> - skb->len, 2); >> + return cpdma_chan_submit(txch, skb, skb->data, skb->len, 2); >> } >> >> static inline void cpsw_add_dual_emac_def_ale_entries( > > [...] > >> >> @@ -1614,12 +1713,16 @@ static int cpsw_ndo_ioctl(struct net_device *dev, struct ifreq *req, int cmd) >> static void cpsw_ndo_tx_timeout(struct net_device *ndev) >> { >> struct cpsw_priv *priv = netdev_priv(ndev); >> + int ch; >> >> cpsw_err(priv, tx_err, "transmit timeout, restarting dma\n"); >> ndev->stats.tx_errors++; >> cpsw_intr_disable(priv); >> - cpdma_chan_stop(priv->txch); >> - cpdma_chan_start(priv->txch); >> + for (ch = 0; ch < priv->tx_ch_num; ch++) { >> + cpdma_chan_stop(priv->txch[ch]); >> + cpdma_chan_start(priv->txch[ch]); >> + } >> + >> cpsw_intr_enable(priv); >> } >> >> @@ -1833,7 +1936,7 @@ static void cpsw_get_drvinfo(struct net_device *ndev, >> struct cpsw_priv *priv = netdev_priv(ndev); >> >> strlcpy(info->driver, "cpsw", sizeof(info->driver)); >> - strlcpy(info->version, "1.0", sizeof(info->version)); >> + strlcpy(info->version, "1.1", sizeof(info->version)); > > Not sure about this change, at least not as part of this patch. The possibilities of the driver are changed. Now it's multichannel. If you think it's not needed I can drop it otherwise will send it with separate patch.
> >> strlcpy(info->bus_info, priv->pdev->name, sizeof(info->bus_info)); >> } >> >> @@ -2181,7 +2284,7 @@ static int cpsw_probe_dual_emac(struct platform_device *pdev, >> struct cpsw_priv *priv_sl2; >> int ret = 0, i; >> >> - ndev = alloc_etherdev(sizeof(struct cpsw_priv)); >> + ndev = alloc_etherdev_mq(sizeof(struct cpsw_priv), CPSW_MAX_QUEUES); >> if (!ndev) { >> dev_err(&pdev->dev, "cpsw: error allocating net_device\n"); >> return -ENOMEM; >> @@ -2216,8 +2319,15 @@ static int cpsw_probe_dual_emac(struct platform_device *pdev, >> priv_sl2->wr_regs = priv->wr_regs; >> priv_sl2->hw_stats = priv->hw_stats; >> priv_sl2->dma = priv->dma; >> - priv_sl2->txch = priv->txch; >> - priv_sl2->rxch = priv->rxch; > > [...] > >> >> - if (WARN_ON(!priv->txch || !priv->rxch)) { >> + if (WARN_ON(!priv->rxch[0] || !priv->txch[0])) { >> dev_err(priv->dev, "error initializing dma channels\n"); >> ret = -ENOMEM; >> goto clean_dma_ret; >> diff --git a/drivers/net/ethernet/ti/davinci_cpdma.c b/drivers/net/ethernet/ti/davinci_cpdma.c >> index 2f4b571..a4b299d 100644 >> --- a/drivers/net/ethernet/ti/davinci_cpdma.c >> +++ b/drivers/net/ethernet/ti/davinci_cpdma.c >> @@ -481,6 +481,18 @@ void cpdma_ctlr_eoi(struct cpdma_ctlr *ctlr, u32 value) >> } >> EXPORT_SYMBOL_GPL(cpdma_ctlr_eoi); >> >> +u32 cpdma_ctrl_rxchs_state(struct cpdma_ctlr *ctlr) >> +{ >> + return dma_reg_read(ctlr, CPDMA_RXINTSTATMASKED); >> +} >> +EXPORT_SYMBOL_GPL(cpdma_ctrl_rxchs_state); >> + >> +u32 cpdma_ctrl_txchs_state(struct cpdma_ctlr *ctlr) >> +{ >> + return dma_reg_read(ctlr, CPDMA_TXINTSTATMASKED); > > TRM: CPDMA_INT TX INTERRUPT STATUS REGISTER (MASKED VALUE) > >> +} >> +EXPORT_SYMBOL_GPL(cpdma_ctrl_txchs_state); > > This is interrupt status, so may be cpdma_ctrl_tx[rx]chs_intr_status() name > will be more appropriate? Not sure. It's not exacly interrupt status as can give status of channel even if the intertupt was disabled. And it can be used w/o interrupt at all, just polling. The interrupt status can be read with another register. This one continue to mirror descriptors presence for channels till they are all correctly handled.
> >> + >> /** >> * cpdma_chan_split_pool - Splits ctrl pool between all channels. >> * Has to be called under ctlr lock >> diff --git a/drivers/net/ethernet/ti/davinci_cpdma.h b/drivers/net/ethernet/ti/davinci_cpdma.h >> index 0308b67..3ce91a1 100644 >> --- a/drivers/net/ethernet/ti/davinci_cpdma.h >> +++ b/drivers/net/ethernet/ti/davinci_cpdma.h >> @@ -96,6 +96,8 @@ int cpdma_chan_process(struct cpdma_chan *chan, int quota); >> int cpdma_ctlr_int_ctrl(struct cpdma_ctlr *ctlr, bool enable); >> void cpdma_ctlr_eoi(struct cpdma_ctlr *ctlr, u32 value); >> int cpdma_chan_int_ctrl(struct cpdma_chan *chan, bool enable); >> +u32 cpdma_ctrl_rxchs_state(struct cpdma_ctlr *ctlr); >> +u32 cpdma_ctrl_txchs_state(struct cpdma_ctlr *ctlr); >> bool cpdma_check_free_tx_desc(struct cpdma_chan *chan); >> >> enum cpdma_control { >> > >
-- Regards, Ivan Khoronzhuk
| |