2022-02-09 10:24:42

by Po Liu

[permalink] [raw]
Subject: [v1,net-next 3/3] net:enetc: enetc qos using the CBDR dma alloc function

Now we can use the enetc_cbd_alloc_data_mem() to replace complicated DMA
data alloc method and CBDR memory basic seting.

Signed-off-by: Po Liu <[email protected]>
---
.../net/ethernet/freescale/enetc/enetc_qos.c | 91 +++++--------------
1 file changed, 21 insertions(+), 70 deletions(-)

diff --git a/drivers/net/ethernet/freescale/enetc/enetc_qos.c b/drivers/net/ethernet/freescale/enetc/enetc_qos.c
index d3d7172e0fcc..147c2457292f 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_qos.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_qos.c
@@ -45,7 +45,6 @@ void enetc_sched_speed_set(struct enetc_ndev_priv *priv, int speed)
| pspeed);
}

-#define ENETC_QOS_ALIGN 64
static int enetc_setup_taprio(struct net_device *ndev,
struct tc_taprio_qopt_offload *admin_conf)
{
@@ -53,7 +52,7 @@ static int enetc_setup_taprio(struct net_device *ndev,
struct enetc_cbd cbd = {.cmd = 0};
struct tgs_gcl_conf *gcl_config;
struct tgs_gcl_data *gcl_data;
- dma_addr_t dma, dma_align;
+ dma_addr_t dma;
struct gce *gce;
u16 data_size;
u16 gcl_len;
@@ -84,16 +83,10 @@ static int enetc_setup_taprio(struct net_device *ndev,
gcl_config = &cbd.gcl_conf;

data_size = struct_size(gcl_data, entry, gcl_len);
- tmp = dma_alloc_coherent(&priv->si->pdev->dev,
- data_size + ENETC_QOS_ALIGN,
- &dma, GFP_KERNEL);
- if (!tmp) {
- dev_err(&priv->si->pdev->dev,
- "DMA mapping of taprio gate list failed!\n");
+ tmp = enetc_cbd_alloc_data_mem(priv->si, &cbd, data_size,
+ &dma, (void *)&gcl_data);
+ if (!tmp)
return -ENOMEM;
- }
- dma_align = ALIGN(dma, ENETC_QOS_ALIGN);
- gcl_data = (struct tgs_gcl_data *)PTR_ALIGN(tmp, ENETC_QOS_ALIGN);

gce = (struct gce *)(gcl_data + 1);

@@ -116,11 +109,8 @@ static int enetc_setup_taprio(struct net_device *ndev,
temp_gce->period = cpu_to_le32(temp_entry->interval);
}

- cbd.length = cpu_to_le16(data_size);
cbd.status_flags = 0;

- cbd.addr[0] = cpu_to_le32(lower_32_bits(dma_align));
- cbd.addr[1] = cpu_to_le32(upper_32_bits(dma_align));
cbd.cls = BDCR_CMD_PORT_GCL;
cbd.status_flags = 0;

@@ -133,8 +123,7 @@ static int enetc_setup_taprio(struct net_device *ndev,
ENETC_QBV_PTGCR_OFFSET,
tge & (~ENETC_QBV_TGE));

- dma_free_coherent(&priv->si->pdev->dev, data_size + ENETC_QOS_ALIGN,
- tmp, dma);
+ enetc_cbd_free_data_mem(priv->si, data_size, tmp, &dma);

return err;
}
@@ -464,7 +453,7 @@ static int enetc_streamid_hw_set(struct enetc_ndev_priv *priv,
struct enetc_cbd cbd = {.cmd = 0};
struct streamid_data *si_data;
struct streamid_conf *si_conf;
- dma_addr_t dma, dma_align;
+ dma_addr_t dma;
u16 data_size;
void *tmp;
int port;
@@ -487,20 +476,11 @@ static int enetc_streamid_hw_set(struct enetc_ndev_priv *priv,
cbd.status_flags = 0;

data_size = sizeof(struct streamid_data);
- tmp = dma_alloc_coherent(&priv->si->pdev->dev,
- data_size + ENETC_QOS_ALIGN,
- &dma, GFP_KERNEL);
- if (!tmp) {
- dev_err(&priv->si->pdev->dev,
- "DMA mapping of stream identify failed!\n");
+ tmp = enetc_cbd_alloc_data_mem(priv->si, &cbd, data_size,
+ &dma, (void *)&si_data);
+ if (!tmp)
return -ENOMEM;
- }
- dma_align = ALIGN(dma, ENETC_QOS_ALIGN);
- si_data = (struct streamid_data *)PTR_ALIGN(tmp, ENETC_QOS_ALIGN);

- cbd.length = cpu_to_le16(data_size);
- cbd.addr[0] = cpu_to_le32(lower_32_bits(dma_align));
- cbd.addr[1] = cpu_to_le32(upper_32_bits(dma_align));
eth_broadcast_addr(si_data->dmac);
si_data->vid_vidm_tg = (ENETC_CBDR_SID_VID_MASK
+ ((0x3 << 14) | ENETC_CBDR_SID_VIDM));
@@ -538,11 +518,6 @@ static int enetc_streamid_hw_set(struct enetc_ndev_priv *priv,

memset(si_data, 0, data_size);

- cbd.length = cpu_to_le16(data_size);
-
- cbd.addr[0] = cpu_to_le32(lower_32_bits(dma_align));
- cbd.addr[1] = cpu_to_le32(upper_32_bits(dma_align));
-
/* VIDM default to be 1.
* VID Match. If set (b1) then the VID must match, otherwise
* any VID is considered a match. VIDM setting is only used
@@ -562,8 +537,7 @@ static int enetc_streamid_hw_set(struct enetc_ndev_priv *priv,

err = enetc_send_cmd(priv->si, &cbd);
out:
- dma_free_coherent(&priv->si->pdev->dev, data_size + ENETC_QOS_ALIGN,
- tmp, dma);
+ enetc_cbd_free_data_mem(priv->si, data_size, tmp, &dma);

return err;
}
@@ -632,7 +606,7 @@ static int enetc_streamcounter_hw_get(struct enetc_ndev_priv *priv,
{
struct enetc_cbd cbd = { .cmd = 2 };
struct sfi_counter_data *data_buf;
- dma_addr_t dma, dma_align;
+ dma_addr_t dma;
u16 data_size;
void *tmp;
int err;
@@ -643,21 +617,11 @@ static int enetc_streamcounter_hw_get(struct enetc_ndev_priv *priv,
cbd.status_flags = 0;

data_size = sizeof(struct sfi_counter_data);
- tmp = dma_alloc_coherent(&priv->si->pdev->dev,
- data_size + ENETC_QOS_ALIGN,
- &dma, GFP_KERNEL);
- if (!tmp) {
- dev_err(&priv->si->pdev->dev,
- "DMA mapping of stream counter failed!\n");
- return -ENOMEM;
- }
- dma_align = ALIGN(dma, ENETC_QOS_ALIGN);
- data_buf = (struct sfi_counter_data *)PTR_ALIGN(tmp, ENETC_QOS_ALIGN);

- cbd.addr[0] = cpu_to_le32(lower_32_bits(dma_align));
- cbd.addr[1] = cpu_to_le32(upper_32_bits(dma_align));
-
- cbd.length = cpu_to_le16(data_size);
+ tmp = enetc_cbd_alloc_data_mem(priv->si, &cbd, data_size,
+ &dma, (void *)&data_buf);
+ if (!tmp)
+ return -ENOMEM;

err = enetc_send_cmd(priv->si, &cbd);
if (err)
@@ -684,8 +648,7 @@ static int enetc_streamcounter_hw_get(struct enetc_ndev_priv *priv,
data_buf->flow_meter_dropl;

exit:
- dma_free_coherent(&priv->si->pdev->dev, data_size + ENETC_QOS_ALIGN,
- tmp, dma);
+ enetc_cbd_free_data_mem(priv->si, data_size, tmp, &dma);

return err;
}
@@ -725,7 +688,7 @@ static int enetc_streamgate_hw_set(struct enetc_ndev_priv *priv,
struct sgcl_conf *sgcl_config;
struct sgcl_data *sgcl_data;
struct sgce *sgce;
- dma_addr_t dma, dma_align;
+ dma_addr_t dma;
u16 data_size;
int err, i;
void *tmp;
@@ -775,20 +738,10 @@ static int enetc_streamgate_hw_set(struct enetc_ndev_priv *priv,
sgcl_config->acl_len = (sgi->num_entries - 1) & 0x3;

data_size = struct_size(sgcl_data, sgcl, sgi->num_entries);
- tmp = dma_alloc_coherent(&priv->si->pdev->dev,
- data_size + ENETC_QOS_ALIGN,
- &dma, GFP_KERNEL);
- if (!tmp) {
- dev_err(&priv->si->pdev->dev,
- "DMA mapping of stream counter failed!\n");
+ tmp = enetc_cbd_alloc_data_mem(priv->si, &cbd, data_size,
+ &dma, (void *)&sgcl_data);
+ if (!tmp)
return -ENOMEM;
- }
- dma_align = ALIGN(dma, ENETC_QOS_ALIGN);
- sgcl_data = (struct sgcl_data *)PTR_ALIGN(tmp, ENETC_QOS_ALIGN);
-
- cbd.length = cpu_to_le16(data_size);
- cbd.addr[0] = cpu_to_le32(lower_32_bits(dma_align));
- cbd.addr[1] = cpu_to_le32(upper_32_bits(dma_align));

sgce = &sgcl_data->sgcl[0];

@@ -843,9 +796,7 @@ static int enetc_streamgate_hw_set(struct enetc_ndev_priv *priv,
err = enetc_send_cmd(priv->si, &cbd);

exit:
- dma_free_coherent(&priv->si->pdev->dev, data_size + ENETC_QOS_ALIGN,
- tmp, dma);
-
+ enetc_cbd_free_data_mem(priv->si, data_size, tmp, &dma);
return err;
}

--
2.17.1



2022-02-09 13:49:02

by Po Liu

[permalink] [raw]
Subject: [v2,net-next 1/3] net:enetc: allocate CBD ring data memory using DMA coherent methods

To replace the dma_map_single() stream DMA mapping with DMA coherent
method dma_alloc_coherent() which is more simple.

dma_map_single() found by Tim Gardner not proper. Suggested by Claudiu
Manoil and Jakub Kicinski to use dma_alloc_coherent(). Discussion at:

https://lore.kernel.org/netdev/AM9PR04MB8397F300DECD3C44D2EBD07796BD9@AM9PR04MB8397.eurprd04.prod.outlook.com/t/

Fixes: 888ae5a3952ba ("net: enetc: add tc flower psfp offload driver")
cc: Claudiu Manoil <[email protected]>
Reported-by: Tim Gardner <[email protected]>
Signed-off-by: Po Liu <[email protected]>
---
changelog:
v2: no changes

.../net/ethernet/freescale/enetc/enetc_qos.c | 128 +++++++++---------
1 file changed, 64 insertions(+), 64 deletions(-)

diff --git a/drivers/net/ethernet/freescale/enetc/enetc_qos.c b/drivers/net/ethernet/freescale/enetc/enetc_qos.c
index 3555c12edb45..d3d7172e0fcc 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_qos.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_qos.c
@@ -45,6 +45,7 @@ void enetc_sched_speed_set(struct enetc_ndev_priv *priv, int speed)
| pspeed);
}

+#define ENETC_QOS_ALIGN 64
static int enetc_setup_taprio(struct net_device *ndev,
struct tc_taprio_qopt_offload *admin_conf)
{
@@ -52,10 +53,11 @@ static int enetc_setup_taprio(struct net_device *ndev,
struct enetc_cbd cbd = {.cmd = 0};
struct tgs_gcl_conf *gcl_config;
struct tgs_gcl_data *gcl_data;
+ dma_addr_t dma, dma_align;
struct gce *gce;
- dma_addr_t dma;
u16 data_size;
u16 gcl_len;
+ void *tmp;
u32 tge;
int err;
int i;
@@ -82,9 +84,16 @@ static int enetc_setup_taprio(struct net_device *ndev,
gcl_config = &cbd.gcl_conf;

data_size = struct_size(gcl_data, entry, gcl_len);
- gcl_data = kzalloc(data_size, __GFP_DMA | GFP_KERNEL);
- if (!gcl_data)
+ tmp = dma_alloc_coherent(&priv->si->pdev->dev,
+ data_size + ENETC_QOS_ALIGN,
+ &dma, GFP_KERNEL);
+ if (!tmp) {
+ dev_err(&priv->si->pdev->dev,
+ "DMA mapping of taprio gate list failed!\n");
return -ENOMEM;
+ }
+ dma_align = ALIGN(dma, ENETC_QOS_ALIGN);
+ gcl_data = (struct tgs_gcl_data *)PTR_ALIGN(tmp, ENETC_QOS_ALIGN);

gce = (struct gce *)(gcl_data + 1);

@@ -110,16 +119,8 @@ static int enetc_setup_taprio(struct net_device *ndev,
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] = cpu_to_le32(lower_32_bits(dma));
- cbd.addr[1] = cpu_to_le32(upper_32_bits(dma));
+ cbd.addr[0] = cpu_to_le32(lower_32_bits(dma_align));
+ cbd.addr[1] = cpu_to_le32(upper_32_bits(dma_align));
cbd.cls = BDCR_CMD_PORT_GCL;
cbd.status_flags = 0;

@@ -132,8 +133,8 @@ static int enetc_setup_taprio(struct net_device *ndev,
ENETC_QBV_PTGCR_OFFSET,
tge & (~ENETC_QBV_TGE));

- dma_unmap_single(&priv->si->pdev->dev, dma, data_size, DMA_TO_DEVICE);
- kfree(gcl_data);
+ dma_free_coherent(&priv->si->pdev->dev, data_size + ENETC_QOS_ALIGN,
+ tmp, dma);

return err;
}
@@ -463,8 +464,9 @@ static int enetc_streamid_hw_set(struct enetc_ndev_priv *priv,
struct enetc_cbd cbd = {.cmd = 0};
struct streamid_data *si_data;
struct streamid_conf *si_conf;
+ dma_addr_t dma, dma_align;
u16 data_size;
- dma_addr_t dma;
+ void *tmp;
int port;
int err;

@@ -485,21 +487,20 @@ static int enetc_streamid_hw_set(struct enetc_ndev_priv *priv,
cbd.status_flags = 0;

data_size = sizeof(struct streamid_data);
- si_data = kzalloc(data_size, __GFP_DMA | GFP_KERNEL);
- if (!si_data)
+ tmp = dma_alloc_coherent(&priv->si->pdev->dev,
+ data_size + ENETC_QOS_ALIGN,
+ &dma, GFP_KERNEL);
+ if (!tmp) {
+ dev_err(&priv->si->pdev->dev,
+ "DMA mapping of stream identify failed!\n");
return -ENOMEM;
- cbd.length = cpu_to_le16(data_size);
-
- dma = dma_map_single(&priv->si->pdev->dev, si_data,
- data_size, DMA_FROM_DEVICE);
- if (dma_mapping_error(&priv->si->pdev->dev, dma)) {
- netdev_err(priv->si->ndev, "DMA mapping failed!\n");
- err = -ENOMEM;
- goto out;
}
+ dma_align = ALIGN(dma, ENETC_QOS_ALIGN);
+ si_data = (struct streamid_data *)PTR_ALIGN(tmp, ENETC_QOS_ALIGN);

- cbd.addr[0] = cpu_to_le32(lower_32_bits(dma));
- cbd.addr[1] = cpu_to_le32(upper_32_bits(dma));
+ cbd.length = cpu_to_le16(data_size);
+ cbd.addr[0] = cpu_to_le32(lower_32_bits(dma_align));
+ cbd.addr[1] = cpu_to_le32(upper_32_bits(dma_align));
eth_broadcast_addr(si_data->dmac);
si_data->vid_vidm_tg = (ENETC_CBDR_SID_VID_MASK
+ ((0x3 << 14) | ENETC_CBDR_SID_VIDM));
@@ -539,8 +540,8 @@ static int enetc_streamid_hw_set(struct enetc_ndev_priv *priv,

cbd.length = cpu_to_le16(data_size);

- cbd.addr[0] = cpu_to_le32(lower_32_bits(dma));
- cbd.addr[1] = cpu_to_le32(upper_32_bits(dma));
+ cbd.addr[0] = cpu_to_le32(lower_32_bits(dma_align));
+ cbd.addr[1] = cpu_to_le32(upper_32_bits(dma_align));

/* VIDM default to be 1.
* VID Match. If set (b1) then the VID must match, otherwise
@@ -561,10 +562,8 @@ static int enetc_streamid_hw_set(struct enetc_ndev_priv *priv,

err = enetc_send_cmd(priv->si, &cbd);
out:
- if (!dma_mapping_error(&priv->si->pdev->dev, dma))
- dma_unmap_single(&priv->si->pdev->dev, dma, data_size, DMA_FROM_DEVICE);
-
- kfree(si_data);
+ dma_free_coherent(&priv->si->pdev->dev, data_size + ENETC_QOS_ALIGN,
+ tmp, dma);

return err;
}
@@ -633,8 +632,9 @@ static int enetc_streamcounter_hw_get(struct enetc_ndev_priv *priv,
{
struct enetc_cbd cbd = { .cmd = 2 };
struct sfi_counter_data *data_buf;
- dma_addr_t dma;
+ dma_addr_t dma, dma_align;
u16 data_size;
+ void *tmp;
int err;

cbd.index = cpu_to_le16((u16)index);
@@ -643,19 +643,19 @@ static int enetc_streamcounter_hw_get(struct enetc_ndev_priv *priv,
cbd.status_flags = 0;

data_size = sizeof(struct sfi_counter_data);
- data_buf = kzalloc(data_size, __GFP_DMA | GFP_KERNEL);
- if (!data_buf)
+ tmp = dma_alloc_coherent(&priv->si->pdev->dev,
+ data_size + ENETC_QOS_ALIGN,
+ &dma, GFP_KERNEL);
+ if (!tmp) {
+ dev_err(&priv->si->pdev->dev,
+ "DMA mapping of stream counter failed!\n");
return -ENOMEM;
-
- dma = dma_map_single(&priv->si->pdev->dev, data_buf,
- data_size, DMA_FROM_DEVICE);
- if (dma_mapping_error(&priv->si->pdev->dev, dma)) {
- netdev_err(priv->si->ndev, "DMA mapping failed!\n");
- err = -ENOMEM;
- goto exit;
}
- cbd.addr[0] = cpu_to_le32(lower_32_bits(dma));
- cbd.addr[1] = cpu_to_le32(upper_32_bits(dma));
+ dma_align = ALIGN(dma, ENETC_QOS_ALIGN);
+ data_buf = (struct sfi_counter_data *)PTR_ALIGN(tmp, ENETC_QOS_ALIGN);
+
+ cbd.addr[0] = cpu_to_le32(lower_32_bits(dma_align));
+ cbd.addr[1] = cpu_to_le32(upper_32_bits(dma_align));

cbd.length = cpu_to_le16(data_size);

@@ -684,7 +684,9 @@ static int enetc_streamcounter_hw_get(struct enetc_ndev_priv *priv,
data_buf->flow_meter_dropl;

exit:
- kfree(data_buf);
+ dma_free_coherent(&priv->si->pdev->dev, data_size + ENETC_QOS_ALIGN,
+ tmp, dma);
+
return err;
}

@@ -723,9 +725,10 @@ static int enetc_streamgate_hw_set(struct enetc_ndev_priv *priv,
struct sgcl_conf *sgcl_config;
struct sgcl_data *sgcl_data;
struct sgce *sgce;
- dma_addr_t dma;
+ dma_addr_t dma, dma_align;
u16 data_size;
int err, i;
+ void *tmp;
u64 now;

cbd.index = cpu_to_le16(sgi->index);
@@ -772,24 +775,20 @@ static int enetc_streamgate_hw_set(struct enetc_ndev_priv *priv,
sgcl_config->acl_len = (sgi->num_entries - 1) & 0x3;

data_size = struct_size(sgcl_data, sgcl, sgi->num_entries);
-
- sgcl_data = kzalloc(data_size, __GFP_DMA | GFP_KERNEL);
- if (!sgcl_data)
- return -ENOMEM;
-
- cbd.length = cpu_to_le16(data_size);
-
- dma = dma_map_single(&priv->si->pdev->dev,
- sgcl_data, data_size,
- DMA_FROM_DEVICE);
- if (dma_mapping_error(&priv->si->pdev->dev, dma)) {
- netdev_err(priv->si->ndev, "DMA mapping failed!\n");
- kfree(sgcl_data);
+ tmp = dma_alloc_coherent(&priv->si->pdev->dev,
+ data_size + ENETC_QOS_ALIGN,
+ &dma, GFP_KERNEL);
+ if (!tmp) {
+ dev_err(&priv->si->pdev->dev,
+ "DMA mapping of stream counter failed!\n");
return -ENOMEM;
}
+ dma_align = ALIGN(dma, ENETC_QOS_ALIGN);
+ sgcl_data = (struct sgcl_data *)PTR_ALIGN(tmp, ENETC_QOS_ALIGN);

- cbd.addr[0] = cpu_to_le32(lower_32_bits(dma));
- cbd.addr[1] = cpu_to_le32(upper_32_bits(dma));
+ cbd.length = cpu_to_le16(data_size);
+ cbd.addr[0] = cpu_to_le32(lower_32_bits(dma_align));
+ cbd.addr[1] = cpu_to_le32(upper_32_bits(dma_align));

sgce = &sgcl_data->sgcl[0];

@@ -844,7 +843,8 @@ static int enetc_streamgate_hw_set(struct enetc_ndev_priv *priv,
err = enetc_send_cmd(priv->si, &cbd);

exit:
- kfree(sgcl_data);
+ dma_free_coherent(&priv->si->pdev->dev, data_size + ENETC_QOS_ALIGN,
+ tmp, dma);

return err;
}
--
2.17.1


2022-02-09 13:53:23

by Claudiu Manoil

[permalink] [raw]
Subject: RE: [v1,net-next 3/3] net:enetc: enetc qos using the CBDR dma alloc function

> -----Original Message-----
> From: Po Liu <[email protected]>
> Sent: Wednesday, February 9, 2022 7:49 AM
[...]
> Subject: [v1,net-next 3/3] net:enetc: enetc qos using the CBDR dma alloc
> function
>
> Now we can use the enetc_cbd_alloc_data_mem() to replace complicated
> DMA
> data alloc method and CBDR memory basic seting.
>
> Signed-off-by: Po Liu <[email protected]>

Reviewed-by: Claudiu Manoil <[email protected]>

2022-02-09 14:07:55

by Po Liu

[permalink] [raw]
Subject: RE: [v1,net-next 3/3] net:enetc: enetc qos using the CBDR dma alloc function


> -----Original Message-----
> From: Po Liu <[email protected]>
> Sent: 2022??2??9?? 13:49
> To: [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> Claudiu Manoil <[email protected]>; Vladimir Oltean
> <[email protected]>
> Cc: Xiaoliang Yang <[email protected]>; Po Liu <[email protected]>
> Subject: [v1,net-next 3/3] net:enetc: enetc qos using the CBDR dma alloc
> function
>
> Now we can use the enetc_cbd_alloc_data_mem() to replace complicated
> DMA data alloc method and CBDR memory basic seting.
>
> Signed-off-by: Po Liu <[email protected]>
> ---
> .../net/ethernet/freescale/enetc/enetc_qos.c | 91 +++++--------------
> 1 file changed, 21 insertions(+), 70 deletions(-)
>
>
> memset(si_data, 0, data_size);
>
> - cbd.length = cpu_to_le16(data_size);
> -
> - cbd.addr[0] = cpu_to_le32(lower_32_bits(dma_align));
> - cbd.addr[1] = cpu_to_le32(upper_32_bits(dma_align));
> -

Sorry, found there is hardware setting bug here. Will update soon for v2.

> /* VIDM default to be 1.
> * VID Match. If set (b1) then the VID must match, otherwise
> * any VID is considered a match. VIDM setting is only used @@ -562,8
> +537,7 @@ static int enetc_streamid_hw_set(struct enetc_ndev_priv *priv,

2022-02-09 15:36:23

by patchwork-bot+netdevbpf

[permalink] [raw]
Subject: Re: [v2,net-next 1/3] net:enetc: allocate CBD ring data memory using DMA coherent methods

Hello:

This series was applied to netdev/net-next.git (master)
by David S. Miller <[email protected]>:

On Wed, 9 Feb 2022 20:33:01 +0800 you wrote:
> To replace the dma_map_single() stream DMA mapping with DMA coherent
> method dma_alloc_coherent() which is more simple.
>
> dma_map_single() found by Tim Gardner not proper. Suggested by Claudiu
> Manoil and Jakub Kicinski to use dma_alloc_coherent(). Discussion at:
>
> https://lore.kernel.org/netdev/AM9PR04MB8397F300DECD3C44D2EBD07796BD9@AM9PR04MB8397.eurprd04.prod.outlook.com/t/
>
> [...]

Here is the summary with links:
- [v2,net-next,1/3] net:enetc: allocate CBD ring data memory using DMA coherent methods
https://git.kernel.org/netdev/net-next/c/b3a723dbc94a
- [v2,net-next,2/3] net:enetc: command BD ring data memory alloc as one function alone
https://git.kernel.org/netdev/net-next/c/0cc11cdbcb39
- [v2,net-next,3/3] net:enetc: enetc qos using the CBDR dma alloc function
https://git.kernel.org/netdev/net-next/c/237d20c208db

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



2022-02-09 19:16:25

by Po Liu

[permalink] [raw]
Subject: [v2,net-next 2/3] net:enetc: command BD ring data memory alloc as one function alone

Separate the CBDR data memory alloc standalone. It is convenient for
other part loading, for example the ENETC QOS part.

Reported-and-suggested-by: Vladimir Oltean <[email protected]>
Signed-off-by: Po Liu <[email protected]>
---
change log:
v2: no changes

drivers/net/ethernet/freescale/enetc/enetc.h | 38 +++++++++++++++++
.../net/ethernet/freescale/enetc/enetc_cbdr.c | 41 +++++--------------
2 files changed, 49 insertions(+), 30 deletions(-)

diff --git a/drivers/net/ethernet/freescale/enetc/enetc.h b/drivers/net/ethernet/freescale/enetc/enetc.h
index fb39e406b7fc..68d806dc3701 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc.h
+++ b/drivers/net/ethernet/freescale/enetc/enetc.h
@@ -18,6 +18,8 @@
#define ENETC_MAX_MTU (ENETC_MAC_MAXFRM_SIZE - \
(ETH_FCS_LEN + ETH_HLEN + VLAN_HLEN))

+#define ENETC_CBD_DATA_MEM_ALIGN 64
+
struct enetc_tx_swbd {
union {
struct sk_buff *skb;
@@ -415,6 +417,42 @@ 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);

+static inline void *enetc_cbd_alloc_data_mem(struct enetc_si *si,
+ struct enetc_cbd *cbd,
+ int size, dma_addr_t *dma,
+ void **data_align)
+{
+ struct enetc_cbdr *ring = &si->cbd_ring;
+ dma_addr_t dma_align;
+ void *data;
+
+ data = dma_alloc_coherent(ring->dma_dev,
+ size + ENETC_CBD_DATA_MEM_ALIGN,
+ dma, GFP_KERNEL);
+ if (!data) {
+ dev_err(ring->dma_dev, "CBD alloc data memory failed!\n");
+ return NULL;
+ }
+
+ dma_align = ALIGN(*dma, ENETC_CBD_DATA_MEM_ALIGN);
+ *data_align = PTR_ALIGN(data, ENETC_CBD_DATA_MEM_ALIGN);
+
+ cbd->addr[0] = cpu_to_le32(lower_32_bits(dma_align));
+ cbd->addr[1] = cpu_to_le32(upper_32_bits(dma_align));
+ cbd->length = cpu_to_le16(size);
+
+ return data;
+}
+
+static inline void enetc_cbd_free_data_mem(struct enetc_si *si, int size,
+ void *data, dma_addr_t *dma)
+{
+ struct enetc_cbdr *ring = &si->cbd_ring;
+
+ dma_free_coherent(ring->dma_dev, size + ENETC_CBD_DATA_MEM_ALIGN,
+ data, *dma);
+}
+
#ifdef CONFIG_FSL_ENETC_QOS
int enetc_setup_tc_taprio(struct net_device *ndev, void *type_data);
void enetc_sched_speed_set(struct enetc_ndev_priv *priv, int speed);
diff --git a/drivers/net/ethernet/freescale/enetc/enetc_cbdr.c b/drivers/net/ethernet/freescale/enetc/enetc_cbdr.c
index 073e56dcca4e..af68dc46a795 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_cbdr.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_cbdr.c
@@ -166,70 +166,55 @@ int enetc_set_mac_flt_entry(struct enetc_si *si, int index,
return enetc_send_cmd(si, &cbd);
}

-#define RFSE_ALIGN 64
/* Set entry in RFS table */
int enetc_set_fs_entry(struct enetc_si *si, struct enetc_cmd_rfse *rfse,
int index)
{
struct enetc_cbdr *ring = &si->cbd_ring;
struct enetc_cbd cbd = {.cmd = 0};
- dma_addr_t dma, dma_align;
void *tmp, *tmp_align;
+ dma_addr_t dma;
int err;

/* fill up the "set" descriptor */
cbd.cmd = 0;
cbd.cls = 4;
cbd.index = cpu_to_le16(index);
- cbd.length = cpu_to_le16(sizeof(*rfse));
cbd.opt[3] = cpu_to_le32(0); /* SI */

- tmp = dma_alloc_coherent(ring->dma_dev, sizeof(*rfse) + RFSE_ALIGN,
- &dma, GFP_KERNEL);
- if (!tmp) {
- dev_err(ring->dma_dev, "DMA mapping of RFS entry failed!\n");
+ tmp = enetc_cbd_alloc_data_mem(si, &cbd, sizeof(*rfse),
+ &dma, &tmp_align);
+ if (!tmp)
return -ENOMEM;
- }

- dma_align = ALIGN(dma, RFSE_ALIGN);
- tmp_align = PTR_ALIGN(tmp, RFSE_ALIGN);
memcpy(tmp_align, rfse, sizeof(*rfse));

- cbd.addr[0] = cpu_to_le32(lower_32_bits(dma_align));
- cbd.addr[1] = cpu_to_le32(upper_32_bits(dma_align));
-
err = enetc_send_cmd(si, &cbd);
if (err)
dev_err(ring->dma_dev, "FS entry add failed (%d)!", err);

- dma_free_coherent(ring->dma_dev, sizeof(*rfse) + RFSE_ALIGN,
- tmp, dma);
+ enetc_cbd_free_data_mem(si, sizeof(*rfse), tmp, &dma);

return err;
}

-#define RSSE_ALIGN 64
static int enetc_cmd_rss_table(struct enetc_si *si, u32 *table, int count,
bool read)
{
struct enetc_cbdr *ring = &si->cbd_ring;
struct enetc_cbd cbd = {.cmd = 0};
- dma_addr_t dma, dma_align;
u8 *tmp, *tmp_align;
+ dma_addr_t dma;
int err, i;

- if (count < RSSE_ALIGN)
+ if (count < ENETC_CBD_DATA_MEM_ALIGN)
/* HW only takes in a full 64 entry table */
return -EINVAL;

- tmp = dma_alloc_coherent(ring->dma_dev, count + RSSE_ALIGN,
- &dma, GFP_KERNEL);
- if (!tmp) {
- dev_err(ring->dma_dev, "DMA mapping of RSS table failed!\n");
+ tmp = enetc_cbd_alloc_data_mem(si, &cbd, count,
+ &dma, (void *)&tmp_align);
+ if (!tmp)
return -ENOMEM;
- }
- dma_align = ALIGN(dma, RSSE_ALIGN);
- tmp_align = PTR_ALIGN(tmp, RSSE_ALIGN);

if (!read)
for (i = 0; i < count; i++)
@@ -238,10 +223,6 @@ static int enetc_cmd_rss_table(struct enetc_si *si, u32 *table, int count,
/* fill up the descriptor */
cbd.cmd = read ? 2 : 1;
cbd.cls = 3;
- cbd.length = cpu_to_le16(count);
-
- cbd.addr[0] = cpu_to_le32(lower_32_bits(dma_align));
- cbd.addr[1] = cpu_to_le32(upper_32_bits(dma_align));

err = enetc_send_cmd(si, &cbd);
if (err)
@@ -251,7 +232,7 @@ static int enetc_cmd_rss_table(struct enetc_si *si, u32 *table, int count,
for (i = 0; i < count; i++)
table[i] = tmp_align[i];

- dma_free_coherent(ring->dma_dev, count + RSSE_ALIGN, tmp, dma);
+ enetc_cbd_free_data_mem(si, count, tmp, &dma);

return err;
}
--
2.17.1