* [PATCH] dmaengine: ti: k3-udma-glue: do not create glue dma devices for udma channels
@ 2023-04-04 8:11 Siddharth Vadapalli
2023-04-04 20:23 ` Péter Ujfalusi
0 siblings, 1 reply; 4+ messages in thread
From: Siddharth Vadapalli @ 2023-04-04 8:11 UTC (permalink / raw)
To: peter.ujfalusi, vkoul
Cc: dmaengine, linux-kernel, linux-arm-kernel, srk, vigneshr,
s-vadapalli
From: Grygorii Strashko <grygorii.strashko@ti.com>
In case K3 DMA glue layer is using UDMA channels (AM65/J721E/J7200) it
doesn't need to create own DMA devices per RX/TX channels as they are never
used and just waste resources. The UDMA based platforms are coherent and
UDMA device iteslf is used for DMA memory management.
Hence, update K3 DMA glue layer to create K3 DMA glue DMA devices per RX/TX
channels only in case of PKTDMA (AM64) where coherency configurable per DMA
channel.
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
---
drivers/dma/ti/k3-udma-glue.c | 70 +++++++++++++++++------------------
1 file changed, 34 insertions(+), 36 deletions(-)
diff --git a/drivers/dma/ti/k3-udma-glue.c b/drivers/dma/ti/k3-udma-glue.c
index 789193ed0386..b0c9572b0d02 100644
--- a/drivers/dma/ti/k3-udma-glue.c
+++ b/drivers/dma/ti/k3-udma-glue.c
@@ -293,19 +293,18 @@ struct k3_udma_glue_tx_channel *k3_udma_glue_request_tx_chn(struct device *dev,
}
tx_chn->udma_tchan_id = xudma_tchan_get_id(tx_chn->udma_tchanx);
- tx_chn->common.chan_dev.class = &k3_udma_glue_devclass;
- tx_chn->common.chan_dev.parent = xudma_get_device(tx_chn->common.udmax);
- dev_set_name(&tx_chn->common.chan_dev, "tchan%d-0x%04x",
- tx_chn->udma_tchan_id, tx_chn->common.dst_thread);
- ret = device_register(&tx_chn->common.chan_dev);
- if (ret) {
- dev_err(dev, "Channel Device registration failed %d\n", ret);
- put_device(&tx_chn->common.chan_dev);
- tx_chn->common.chan_dev.parent = NULL;
- goto err;
- }
-
if (xudma_is_pktdma(tx_chn->common.udmax)) {
+ tx_chn->common.chan_dev.class = &k3_udma_glue_devclass;
+ tx_chn->common.chan_dev.parent = xudma_get_device(tx_chn->common.udmax);
+ dev_set_name(&tx_chn->common.chan_dev, "tchan%d-0x%04x",
+ tx_chn->udma_tchan_id, tx_chn->common.dst_thread);
+ ret = device_register(&tx_chn->common.chan_dev);
+ if (ret) {
+ dev_err(dev, "Channel Device registration failed %d\n", ret);
+ tx_chn->common.chan_dev.parent = NULL;
+ goto err;
+ }
+
/* prepare the channel device as coherent */
tx_chn->common.chan_dev.dma_coherent = true;
dma_coerce_mask_and_coherent(&tx_chn->common.chan_dev,
@@ -912,19 +911,18 @@ k3_udma_glue_request_rx_chn_priv(struct device *dev, const char *name,
}
rx_chn->udma_rchan_id = xudma_rchan_get_id(rx_chn->udma_rchanx);
- rx_chn->common.chan_dev.class = &k3_udma_glue_devclass;
- rx_chn->common.chan_dev.parent = xudma_get_device(rx_chn->common.udmax);
- dev_set_name(&rx_chn->common.chan_dev, "rchan%d-0x%04x",
- rx_chn->udma_rchan_id, rx_chn->common.src_thread);
- ret = device_register(&rx_chn->common.chan_dev);
- if (ret) {
- dev_err(dev, "Channel Device registration failed %d\n", ret);
- put_device(&rx_chn->common.chan_dev);
- rx_chn->common.chan_dev.parent = NULL;
- goto err;
- }
-
if (xudma_is_pktdma(rx_chn->common.udmax)) {
+ rx_chn->common.chan_dev.class = &k3_udma_glue_devclass;
+ rx_chn->common.chan_dev.parent = xudma_get_device(rx_chn->common.udmax);
+ dev_set_name(&rx_chn->common.chan_dev, "rchan%d-0x%04x",
+ rx_chn->udma_rchan_id, rx_chn->common.src_thread);
+ ret = device_register(&rx_chn->common.chan_dev);
+ if (ret) {
+ dev_err(dev, "Channel Device registration failed %d\n", ret);
+ rx_chn->common.chan_dev.parent = NULL;
+ goto err;
+ }
+
/* prepare the channel device as coherent */
rx_chn->common.chan_dev.dma_coherent = true;
dma_coerce_mask_and_coherent(&rx_chn->common.chan_dev,
@@ -1044,19 +1042,19 @@ k3_udma_glue_request_remote_rx_chn(struct device *dev, const char *name,
goto err;
}
- rx_chn->common.chan_dev.class = &k3_udma_glue_devclass;
- rx_chn->common.chan_dev.parent = xudma_get_device(rx_chn->common.udmax);
- dev_set_name(&rx_chn->common.chan_dev, "rchan_remote-0x%04x",
- rx_chn->common.src_thread);
- ret = device_register(&rx_chn->common.chan_dev);
- if (ret) {
- dev_err(dev, "Channel Device registration failed %d\n", ret);
- put_device(&rx_chn->common.chan_dev);
- rx_chn->common.chan_dev.parent = NULL;
- goto err;
- }
-
if (xudma_is_pktdma(rx_chn->common.udmax)) {
+ rx_chn->common.chan_dev.class = &k3_udma_glue_devclass;
+ rx_chn->common.chan_dev.parent = xudma_get_device(rx_chn->common.udmax);
+ dev_set_name(&rx_chn->common.chan_dev, "rchan_remote-0x%04x",
+ rx_chn->common.src_thread);
+
+ ret = device_register(&rx_chn->common.chan_dev);
+ if (ret) {
+ dev_err(dev, "Channel Device registration failed %d\n", ret);
+ rx_chn->common.chan_dev.parent = NULL;
+ goto err;
+ }
+
/* prepare the channel device as coherent */
rx_chn->common.chan_dev.dma_coherent = true;
dma_coerce_mask_and_coherent(&rx_chn->common.chan_dev,
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] dmaengine: ti: k3-udma-glue: do not create glue dma devices for udma channels
2023-04-04 8:11 [PATCH] dmaengine: ti: k3-udma-glue: do not create glue dma devices for udma channels Siddharth Vadapalli
@ 2023-04-04 20:23 ` Péter Ujfalusi
2023-04-05 3:47 ` Vignesh Raghavendra
0 siblings, 1 reply; 4+ messages in thread
From: Péter Ujfalusi @ 2023-04-04 20:23 UTC (permalink / raw)
To: Siddharth Vadapalli, vkoul
Cc: dmaengine, linux-kernel, linux-arm-kernel, srk, vigneshr
On 04/04/2023 11:11, Siddharth Vadapalli wrote:
> From: Grygorii Strashko <grygorii.strashko@ti.com>
>
> In case K3 DMA glue layer is using UDMA channels (AM65/J721E/J7200) it
> doesn't need to create own DMA devices per RX/TX channels as they are never
> used and just waste resources. The UDMA based platforms are coherent and
> UDMA device iteslf is used for DMA memory management.
>
> Hence, update K3 DMA glue layer to create K3 DMA glue DMA devices per RX/TX
> channels only in case of PKTDMA (AM64) where coherency configurable per DMA
> channel.
>
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
> ---
> drivers/dma/ti/k3-udma-glue.c | 70 +++++++++++++++++------------------
> 1 file changed, 34 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/dma/ti/k3-udma-glue.c b/drivers/dma/ti/k3-udma-glue.c
> index 789193ed0386..b0c9572b0d02 100644
> --- a/drivers/dma/ti/k3-udma-glue.c
> +++ b/drivers/dma/ti/k3-udma-glue.c
> @@ -293,19 +293,18 @@ struct k3_udma_glue_tx_channel *k3_udma_glue_request_tx_chn(struct device *dev,
> }
> tx_chn->udma_tchan_id = xudma_tchan_get_id(tx_chn->udma_tchanx);
>
> - tx_chn->common.chan_dev.class = &k3_udma_glue_devclass;
> - tx_chn->common.chan_dev.parent = xudma_get_device(tx_chn->common.udmax);
> - dev_set_name(&tx_chn->common.chan_dev, "tchan%d-0x%04x",
> - tx_chn->udma_tchan_id, tx_chn->common.dst_thread);
> - ret = device_register(&tx_chn->common.chan_dev);
> - if (ret) {
> - dev_err(dev, "Channel Device registration failed %d\n", ret);
> - put_device(&tx_chn->common.chan_dev);
> - tx_chn->common.chan_dev.parent = NULL;
> - goto err;
> - }
> -
> if (xudma_is_pktdma(tx_chn->common.udmax)) {
it might be possible to narrow it down to include a test for atype_asel
14 or 15, but then I would move that test to a helper (passing common as
parameter) and re-use it in other places to avoid getting out o sync
overtime.
Might not worth the effort, just an observation.
> + tx_chn->common.chan_dev.class = &k3_udma_glue_devclass;
> + tx_chn->common.chan_dev.parent = xudma_get_device(tx_chn->common.udmax);
> + dev_set_name(&tx_chn->common.chan_dev, "tchan%d-0x%04x",
> + tx_chn->udma_tchan_id, tx_chn->common.dst_thread);
> + ret = device_register(&tx_chn->common.chan_dev);
> + if (ret) {
> + dev_err(dev, "Channel Device registration failed %d\n", ret);
my guess is that the put_device() is still needed, no?
> + tx_chn->common.chan_dev.parent = NULL;
> + goto err;
> + }
> +
> /* prepare the channel device as coherent */
> tx_chn->common.chan_dev.dma_coherent = true;
> dma_coerce_mask_and_coherent(&tx_chn->common.chan_dev,
> @@ -912,19 +911,18 @@ k3_udma_glue_request_rx_chn_priv(struct device *dev, const char *name,
> }
> rx_chn->udma_rchan_id = xudma_rchan_get_id(rx_chn->udma_rchanx);
>
> - rx_chn->common.chan_dev.class = &k3_udma_glue_devclass;
> - rx_chn->common.chan_dev.parent = xudma_get_device(rx_chn->common.udmax);
> - dev_set_name(&rx_chn->common.chan_dev, "rchan%d-0x%04x",
> - rx_chn->udma_rchan_id, rx_chn->common.src_thread);
> - ret = device_register(&rx_chn->common.chan_dev);
> - if (ret) {
> - dev_err(dev, "Channel Device registration failed %d\n", ret);
> - put_device(&rx_chn->common.chan_dev);
> - rx_chn->common.chan_dev.parent = NULL;
> - goto err;
> - }
> -
> if (xudma_is_pktdma(rx_chn->common.udmax)) {
> + rx_chn->common.chan_dev.class = &k3_udma_glue_devclass;
> + rx_chn->common.chan_dev.parent = xudma_get_device(rx_chn->common.udmax);
> + dev_set_name(&rx_chn->common.chan_dev, "rchan%d-0x%04x",
> + rx_chn->udma_rchan_id, rx_chn->common.src_thread);
> + ret = device_register(&rx_chn->common.chan_dev);
> + if (ret) {
> + dev_err(dev, "Channel Device registration failed %d\n", ret);
> + rx_chn->common.chan_dev.parent = NULL;
> + goto err;
> + }
> +
> /* prepare the channel device as coherent */
> rx_chn->common.chan_dev.dma_coherent = true;
> dma_coerce_mask_and_coherent(&rx_chn->common.chan_dev,
> @@ -1044,19 +1042,19 @@ k3_udma_glue_request_remote_rx_chn(struct device *dev, const char *name,
> goto err;
> }
>
> - rx_chn->common.chan_dev.class = &k3_udma_glue_devclass;
> - rx_chn->common.chan_dev.parent = xudma_get_device(rx_chn->common.udmax);
> - dev_set_name(&rx_chn->common.chan_dev, "rchan_remote-0x%04x",
> - rx_chn->common.src_thread);
> - ret = device_register(&rx_chn->common.chan_dev);
> - if (ret) {
> - dev_err(dev, "Channel Device registration failed %d\n", ret);
> - put_device(&rx_chn->common.chan_dev);
> - rx_chn->common.chan_dev.parent = NULL;
> - goto err;
> - }
> -
> if (xudma_is_pktdma(rx_chn->common.udmax)) {
> + rx_chn->common.chan_dev.class = &k3_udma_glue_devclass;
> + rx_chn->common.chan_dev.parent = xudma_get_device(rx_chn->common.udmax);
> + dev_set_name(&rx_chn->common.chan_dev, "rchan_remote-0x%04x",
> + rx_chn->common.src_thread);
> +
> + ret = device_register(&rx_chn->common.chan_dev);
> + if (ret) {
> + dev_err(dev, "Channel Device registration failed %d\n", ret);
> + rx_chn->common.chan_dev.parent = NULL;
> + goto err;
> + }
> +
> /* prepare the channel device as coherent */
> rx_chn->common.chan_dev.dma_coherent = true;
> dma_coerce_mask_and_coherent(&rx_chn->common.chan_dev,
--
Péter
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] dmaengine: ti: k3-udma-glue: do not create glue dma devices for udma channels
2023-04-04 20:23 ` Péter Ujfalusi
@ 2023-04-05 3:47 ` Vignesh Raghavendra
2023-04-05 5:00 ` Siddharth Vadapalli
0 siblings, 1 reply; 4+ messages in thread
From: Vignesh Raghavendra @ 2023-04-05 3:47 UTC (permalink / raw)
To: Siddharth Vadapalli
Cc: dmaengine, linux-kernel, linux-arm-kernel, srk, vkoul,
Péter Ujfalusi
On 05/04/23 01:53, Péter Ujfalusi wrote:
>
>
> On 04/04/2023 11:11, Siddharth Vadapalli wrote:
>> From: Grygorii Strashko <grygorii.strashko@ti.com>
>>
>> In case K3 DMA glue layer is using UDMA channels (AM65/J721E/J7200) it
>> doesn't need to create own DMA devices per RX/TX channels as they are
>> never
>> used and just waste resources. The UDMA based platforms are coherent and
>> UDMA device iteslf is used for DMA memory management.
>>
>> Hence, update K3 DMA glue layer to create K3 DMA glue DMA devices per
>> RX/TX
>> channels only in case of PKTDMA (AM64) where coherency configurable
>> per DMA
>> channel.
>>
>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
>> ---
>> drivers/dma/ti/k3-udma-glue.c | 70 +++++++++++++++++------------------
>> 1 file changed, 34 insertions(+), 36 deletions(-)
>>
>> diff --git a/drivers/dma/ti/k3-udma-glue.c
>> b/drivers/dma/ti/k3-udma-glue.c
>> index 789193ed0386..b0c9572b0d02 100644
>> --- a/drivers/dma/ti/k3-udma-glue.c
>> +++ b/drivers/dma/ti/k3-udma-glue.c
>> @@ -293,19 +293,18 @@ struct k3_udma_glue_tx_channel
>> *k3_udma_glue_request_tx_chn(struct device *dev,
>> }
>> tx_chn->udma_tchan_id = xudma_tchan_get_id(tx_chn->udma_tchanx);
>> - tx_chn->common.chan_dev.class = &k3_udma_glue_devclass;
>> - tx_chn->common.chan_dev.parent =
>> xudma_get_device(tx_chn->common.udmax);
>> - dev_set_name(&tx_chn->common.chan_dev, "tchan%d-0x%04x",
>> - tx_chn->udma_tchan_id, tx_chn->common.dst_thread);
>> - ret = device_register(&tx_chn->common.chan_dev);
>> - if (ret) {
>> - dev_err(dev, "Channel Device registration failed %d\n", ret);
>> - put_device(&tx_chn->common.chan_dev);
>> - tx_chn->common.chan_dev.parent = NULL;
>> - goto err;
>> - }
>> -
>> if (xudma_is_pktdma(tx_chn->common.udmax)) {
>
> it might be possible to narrow it down to include a test for atype_asel
> 14 or 15, but then I would move that test to a helper (passing common as
> parameter) and re-use it in other places to avoid getting out o sync
> overtime.
> Might not worth the effort, just an observation.
Irrespective, we should at least add check for atype_asel == 14/15 along
with xudma_is_pktdma().
Refractoring these checks to separate function can be patch of its own
>
>> + tx_chn->common.chan_dev.class = &k3_udma_glue_devclass;
>> + tx_chn->common.chan_dev.parent =
>> xudma_get_device(tx_chn->common.udmax);
>> + dev_set_name(&tx_chn->common.chan_dev, "tchan%d-0x%04x",
>> + tx_chn->udma_tchan_id, tx_chn->common.dst_thread);
>> + ret = device_register(&tx_chn->common.chan_dev);
>> + if (ret) {
>> + dev_err(dev, "Channel Device registration failed %d\n",
>> ret);
>
> my guess is that the put_device() is still needed, no?
Agree
>
>> + tx_chn->common.chan_dev.parent = NULL;
>> + goto err;
>> + }
>> +
>> /* prepare the channel device as coherent */
[...]
--
Regards
Vignesh
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] dmaengine: ti: k3-udma-glue: do not create glue dma devices for udma channels
2023-04-05 3:47 ` Vignesh Raghavendra
@ 2023-04-05 5:00 ` Siddharth Vadapalli
0 siblings, 0 replies; 4+ messages in thread
From: Siddharth Vadapalli @ 2023-04-05 5:00 UTC (permalink / raw)
To: Péter Ujfalusi, Vignesh Raghavendra
Cc: dmaengine, linux-kernel, linux-arm-kernel, srk, vkoul,
s-vadapalli
Péter, Vignesh,
Thank you for reviewing the patch.
On 05/04/23 09:17, Vignesh Raghavendra wrote:
>
> On 05/04/23 01:53, Péter Ujfalusi wrote:
>>
>>
>> On 04/04/2023 11:11, Siddharth Vadapalli wrote:
>>> From: Grygorii Strashko <grygorii.strashko@ti.com>
>>>
>>> In case K3 DMA glue layer is using UDMA channels (AM65/J721E/J7200) it
>>> doesn't need to create own DMA devices per RX/TX channels as they are
>>> never
>>> used and just waste resources. The UDMA based platforms are coherent and
>>> UDMA device iteslf is used for DMA memory management.
>>>
>>> Hence, update K3 DMA glue layer to create K3 DMA glue DMA devices per
>>> RX/TX
>>> channels only in case of PKTDMA (AM64) where coherency configurable
>>> per DMA
>>> channel.
>>>
>>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>>> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
>>> ---
>>> drivers/dma/ti/k3-udma-glue.c | 70 +++++++++++++++++------------------
>>> 1 file changed, 34 insertions(+), 36 deletions(-)
>>>
>>> diff --git a/drivers/dma/ti/k3-udma-glue.c
>>> b/drivers/dma/ti/k3-udma-glue.c
>>> index 789193ed0386..b0c9572b0d02 100644
>>> --- a/drivers/dma/ti/k3-udma-glue.c
>>> +++ b/drivers/dma/ti/k3-udma-glue.c
>>> @@ -293,19 +293,18 @@ struct k3_udma_glue_tx_channel
>>> *k3_udma_glue_request_tx_chn(struct device *dev,
>>> }
>>> tx_chn->udma_tchan_id = xudma_tchan_get_id(tx_chn->udma_tchanx);
>>> - tx_chn->common.chan_dev.class = &k3_udma_glue_devclass;
>>> - tx_chn->common.chan_dev.parent =
>>> xudma_get_device(tx_chn->common.udmax);
>>> - dev_set_name(&tx_chn->common.chan_dev, "tchan%d-0x%04x",
>>> - tx_chn->udma_tchan_id, tx_chn->common.dst_thread);
>>> - ret = device_register(&tx_chn->common.chan_dev);
>>> - if (ret) {
>>> - dev_err(dev, "Channel Device registration failed %d\n", ret);
>>> - put_device(&tx_chn->common.chan_dev);
>>> - tx_chn->common.chan_dev.parent = NULL;
>>> - goto err;
>>> - }
>>> -
>>> if (xudma_is_pktdma(tx_chn->common.udmax)) {
>>
>> it might be possible to narrow it down to include a test for atype_asel
>> 14 or 15, but then I would move that test to a helper (passing common as
>> parameter) and re-use it in other places to avoid getting out o sync
>> overtime.
>> Might not worth the effort, just an observation.
>
> Irrespective, we should at least add check for atype_asel == 14/15 along
> with xudma_is_pktdma().
>
> Refractoring these checks to separate function can be patch of its own
>
>>
>>> + tx_chn->common.chan_dev.class = &k3_udma_glue_devclass;
>>> + tx_chn->common.chan_dev.parent =
>>> xudma_get_device(tx_chn->common.udmax);
>>> + dev_set_name(&tx_chn->common.chan_dev, "tchan%d-0x%04x",
>>> + tx_chn->udma_tchan_id, tx_chn->common.dst_thread);
>>> + ret = device_register(&tx_chn->common.chan_dev);
>>> + if (ret) {
>>> + dev_err(dev, "Channel Device registration failed %d\n",
>>> ret);
>>
>> my guess is that the put_device() is still needed, no?
>
> Agree
I will fix this at all 3 occurrences and post the v2 patch.
>
>>
>>> + tx_chn->common.chan_dev.parent = NULL;
>>> + goto err;
>>> + }
>>> +
>>> /* prepare the channel device as coherent */
>
> [...]
>
--
Regards,
Siddharth.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-04-05 5:01 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-04-04 8:11 [PATCH] dmaengine: ti: k3-udma-glue: do not create glue dma devices for udma channels Siddharth Vadapalli
2023-04-04 20:23 ` Péter Ujfalusi
2023-04-05 3:47 ` Vignesh Raghavendra
2023-04-05 5:00 ` Siddharth Vadapalli
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox