Re: [PATCH 2/4] net: ethernet: ti: cpsw: add multi queue support

From: Ivan Khoronzhuk
Date: Tue Jul 19 2016 - 09:24:51 EST

Next message: Geert Uytterhoeven: "Re: [PATCH v2] drivers/perf: arm-pmu: Handle per-interrupt affinity mask"
Previous message: Bhuvanchandra DV: "[PATCH RESEND v2 4/8] tty: serial: fsl_lpuart: Fix broken 8m/s1 support"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

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@xxxxxxxxxx>
---
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

Next message: Geert Uytterhoeven: "Re: [PATCH v2] drivers/perf: arm-pmu: Handle per-interrupt affinity mask"
Previous message: Bhuvanchandra DV: "[PATCH RESEND v2 4/8] tty: serial: fsl_lpuart: Fix broken 8m/s1 support"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]