[PATCH 0/4] Cleanup dma device in spl and move dma channel[0]
The channel allocation and deallocation for dma copy was happening on every dma transfer. This is a overhead for transactions like NAND, which does page reads recursively for complete data.
So, moving the dma allocation to probe and implement corresponding remove function and cleanup dma device while exiting from spl.
Enable SPL_DM_DEVICE_REMOVE, for device removal capability in SPL.
Prasanth Babu Mantena (2): mach-k3: common.c: Add dma device remove in spl exit configs: k3: Enable device removal in SPL
Santhosh Kumar K (2): dma: ti: k3-udma: Move udma_probe() below all APIs dma: ti: k3-udma: Move DMA channel[0] allocation to probe and add udma_remove()
arch/arm/mach-k3/common.c | 23 ++- configs/am62ax_evm_a53_defconfig | 1 + configs/am62ax_evm_r5_defconfig | 1 + configs/am62x_evm_a53_defconfig | 1 + configs/am62x_evm_r5_defconfig | 1 + configs/j7200_evm_a72_defconfig | 1 + configs/j7200_evm_r5_defconfig | 1 + configs/j721e_evm_a72_defconfig | 1 + configs/j721e_evm_r5_defconfig | 1 + configs/j721s2_evm_a72_defconfig | 1 + configs/j721s2_evm_r5_defconfig | 1 + configs/j784s4_evm_a72_defconfig | 1 + configs/j784s4_evm_r5_defconfig | 1 + drivers/dma/ti/k3-udma.c | 334 ++++++++++++++++--------------- 14 files changed, 208 insertions(+), 161 deletions(-)
While exiting from spl, remove any dma device active through spl_board_prepare_for_boot(). This is required for cleaning up any dma channels being used in spl and avoid issues with overlapping channel allocation in the next stage bootloaders.
Signed-off-by: Prasanth Babu Mantena p-mantena@ti.com --- arch/arm/mach-k3/common.c | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/arch/arm/mach-k3/common.c b/arch/arm/mach-k3/common.c index df48ec8d47..982dc76b00 100644 --- a/arch/arm/mach-k3/common.c +++ b/arch/arm/mach-k3/common.c @@ -28,6 +28,8 @@ #include <env.h> #include <elf.h> #include <soc.h> +#include <dm/uclass-internal.h> +#include <dm/device-internal.h>
#include <asm/arch/k3-qos.h>
@@ -246,12 +248,31 @@ void spl_enable_cache(void) #endif }
-#if !(defined(CONFIG_SYS_ICACHE_OFF) && defined(CONFIG_SYS_DCACHE_OFF)) +static __maybe_unused void k3_dma_remove(void) +{ + struct udevice *dev; + int rc; + + rc = uclass_find_device(UCLASS_DMA, 0, &dev); + if (!rc && dev) { + rc = device_remove(dev, DM_REMOVE_NORMAL); + if (rc) + pr_warn("Cannot remove dma device '%s' (err=%d)\n", + dev->name, rc); + } +} + void spl_board_prepare_for_boot(void) { +#if !(defined(CONFIG_SYS_ICACHE_OFF) && defined(CONFIG_SYS_DCACHE_OFF)) dcache_disable(); +#endif +#if IS_ENABLED(CONFIG_SPL_DMA) && IS_ENABLED(CONFIG_SPL_DM_DEVICE_REMOVE) + k3_dma_remove(); +#endif }
+#if !(defined(CONFIG_SYS_ICACHE_OFF) && defined(CONFIG_SYS_DCACHE_OFF)) void spl_board_prepare_for_linux(void) { dcache_disable();
Hi Prasant,
Thanks for series,
Could we update the subject of patch something like
Remove dma device in spl exit, Sorry but
Add dma device remove in spl exit looks little confusing , are we adding or removing ?
On 10/4/2024 6:50 PM, Prasanth Babu Mantena wrote:
While exiting from spl, remove any dma device active through spl_board_prepare_for_boot(). This is required for cleaning up any dma channels being used in spl and avoid issues with overlapping channel allocation in the next stage bootloaders.
Signed-off-by: Prasanth Babu Mantena p-mantena@ti.com
arch/arm/mach-k3/common.c | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/arch/arm/mach-k3/common.c b/arch/arm/mach-k3/common.c index df48ec8d47..982dc76b00 100644 --- a/arch/arm/mach-k3/common.c +++ b/arch/arm/mach-k3/common.c @@ -28,6 +28,8 @@ #include <env.h> #include <elf.h> #include <soc.h> +#include <dm/uclass-internal.h> +#include <dm/device-internal.h>
#include <asm/arch/k3-qos.h>
@@ -246,12 +248,31 @@ void spl_enable_cache(void) #endif }
-#if !(defined(CONFIG_SYS_ICACHE_OFF) && defined(CONFIG_SYS_DCACHE_OFF)) +static __maybe_unused void k3_dma_remove(void) +{
- struct udevice *dev;
- int rc;
- rc = uclass_find_device(UCLASS_DMA, 0, &dev);
Are you thinking to have DMA device optional ?
Second question, if we enabled multiple DMAs in u-boot, then would you like to remove all ?
- if (!rc && dev) {
rc = device_remove(dev, DM_REMOVE_NORMAL);if (rc)pr_warn("Cannot remove dma device '%s' (err=%d)\n",dev->name, rc);- }
+}
- void spl_board_prepare_for_boot(void) {
+#if !(defined(CONFIG_SYS_ICACHE_OFF) && defined(CONFIG_SYS_DCACHE_OFF)) dcache_disable(); +#endif
Please suggest , why we are protecting dcache_disable with flags ?
+#if IS_ENABLED(CONFIG_SPL_DMA) && IS_ENABLED(CONFIG_SPL_DM_DEVICE_REMOVE)
- k3_dma_remove();
+#endif }
+#if !(defined(CONFIG_SYS_ICACHE_OFF) && defined(CONFIG_SYS_DCACHE_OFF)) void spl_board_prepare_for_linux(void) { dcache_disable();
On 20:09-20241004, Kumar, Udit wrote: Hi Udit,
Hi Prasant,
Thanks for series,
Could we update the subject of patch something like
Remove dma device in spl exit, Sorry but
Add dma device remove in spl exit looks little confusing , are we adding or removing ?
Understood. Should've been more simpler like "Remove dma device in spl exit". Will update in next version.
On 10/4/2024 6:50 PM, Prasanth Babu Mantena wrote:
While exiting from spl, remove any dma device active through spl_board_prepare_for_boot(). This is required for cleaning up any dma channels being used in spl and avoid issues with overlapping channel allocation in the next stage bootloaders.
Signed-off-by: Prasanth Babu Mantena p-mantena@ti.com
arch/arm/mach-k3/common.c | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/arch/arm/mach-k3/common.c b/arch/arm/mach-k3/common.c index df48ec8d47..982dc76b00 100644 --- a/arch/arm/mach-k3/common.c +++ b/arch/arm/mach-k3/common.c @@ -28,6 +28,8 @@ #include <env.h> #include <elf.h> #include <soc.h> +#include <dm/uclass-internal.h> +#include <dm/device-internal.h> #include <asm/arch/k3-qos.h> @@ -246,12 +248,31 @@ void spl_enable_cache(void) #endif } -#if !(defined(CONFIG_SYS_ICACHE_OFF) && defined(CONFIG_SYS_DCACHE_OFF)) +static __maybe_unused void k3_dma_remove(void) +{
- struct udevice *dev;
- int rc;
- rc = uclass_find_device(UCLASS_DMA, 0, &dev);
Are you thinking to have DMA device optional ?
Not really. Sorry, but didn't understand the question completely here.
Second question, if we enabled multiple DMAs in u-boot, then would you like to remove all ?
Considering this function is specific to k3, here we have channel allocation done in probe and there is a need to clean that channel resources which has been done in remove function in this series, call that remove function here. If there are such allocations done for other dma devices as well and have their remove functions implemented, then they should consider adding those as well here. I don't think there is any such case with other dmas.
- if (!rc && dev) {
rc = device_remove(dev, DM_REMOVE_NORMAL);if (rc)pr_warn("Cannot remove dma device '%s' (err=%d)\n",dev->name, rc);- }
+}
- void spl_board_prepare_for_boot(void) {
+#if !(defined(CONFIG_SYS_ICACHE_OFF) && defined(CONFIG_SYS_DCACHE_OFF)) dcache_disable(); +#endif
Please suggest , why we are protecting dcache_disable with flags ?
This is carry forward which was already present. This patch removes the protection from whole spl_board_prepare_for_boot() function to dcache_disable() only. This alone looks like it has been added in this patch, but if we see the whole snippet, it can be understood well.
Thanks, Prasanth
+#if IS_ENABLED(CONFIG_SPL_DMA) && IS_ENABLED(CONFIG_SPL_DM_DEVICE_REMOVE)
- k3_dma_remove();
+#endif } +#if !(defined(CONFIG_SYS_ICACHE_OFF) && defined(CONFIG_SYS_DCACHE_OFF)) void spl_board_prepare_for_linux(void) { dcache_disable();
Hi Prasanth,
On 10/6/2024 2:26 PM, Prasanth Mantena wrote:
On 20:09-20241004, Kumar, Udit wrote: Hi Udit,
Hi Prasant,
Thanks for series,
Could we update the subject of patch something like
Remove dma device in spl exit, Sorry but
Add dma device remove in spl exit looks little confusing , are we adding or removing ?
Understood. Should've been more simpler like "Remove dma device in spl exit". Will update in next version.
On 10/4/2024 6:50 PM, Prasanth Babu Mantena wrote:
[..] Are you thinking to have DMA device optional ?
Not really. Sorry, but didn't understand the question completely here.
+ rc = uclass_find_device(UCLASS_DMA, 0, &dev); + if (!rc && dev) {
I meant , here if there is some return code or device is not found then at user level there is no notification. May be adding some error print in case either some return code or device is NULL will help to user know, that DMA was not cleaned properly.
Second question, if we enabled multiple DMAs in u-boot, then would you like to remove all ?
Considering this function is specific to k3, here we have channel allocation done in probe and there is a need to clean that channel resources which has been done in remove function in this series, call that remove function here. If there are such allocations done for other dma devices as well and have their remove functions implemented, then they should consider adding those as well here. I don't think there is any such case with other dmas.
Understood this is k3 specific , But here i meant multiple dma devices like udma, pdma etc.
- if (!rc && dev) {
rc = device_remove(dev, DM_REMOVE_NORMAL);if (rc)pr_warn("Cannot remove dma device '%s' (err=%d)\n",dev->name, rc);- }
+}
- void spl_board_prepare_for_boot(void) {
+#if !(defined(CONFIG_SYS_ICACHE_OFF) && defined(CONFIG_SYS_DCACHE_OFF)) dcache_disable(); +#endif
Please suggest , why we are protecting dcache_disable with flags ?
This is carry forward which was already present. This patch removes the protection from whole spl_board_prepare_for_boot() function to dcache_disable() only. This alone looks like it has been added in this patch, but if we see the whole snippet, it can be understood well.
Thanks
Thanks, Prasanth
+#if IS_ENABLED(CONFIG_SPL_DMA) && IS_ENABLED(CONFIG_SPL_DM_DEVICE_REMOVE)
- k3_dma_remove();
+#endif } +#if !(defined(CONFIG_SYS_ICACHE_OFF) && defined(CONFIG_SYS_DCACHE_OFF)) void spl_board_prepare_for_linux(void) { dcache_disable();
On 09:59, Kumar, Udit wrote:
Hi Prasanth,
On 10/6/2024 2:26 PM, Prasanth Mantena wrote:
On 20:09-20241004, Kumar, Udit wrote: Hi Udit,
Hi Prasant,
Thanks for series,
Could we update the subject of patch something like
Remove dma device in spl exit, Sorry but
Add dma device remove in spl exit looks little confusing , are we adding or removing ?
Understood. Should've been more simpler like "Remove dma device in spl exit". Will update in next version.
On 10/4/2024 6:50 PM, Prasanth Babu Mantena wrote:
[..] Are you thinking to have DMA device optional ?
Not really. Sorry, but didn't understand the question completely here.
- rc = uclass_find_device(UCLASS_DMA, 0, &dev);
- if (!rc && dev) {
I meant , here if there is some return code or device is not found then at user level there is no notification. May be adding some error print in case either some return code or device is NULL will help to user know, that DMA was not cleaned properly.
Will check and add an error notification here.
Second question, if we enabled multiple DMAs in u-boot, then would you like to remove all ?
Considering this function is specific to k3, here we have channel allocation done in probe and there is a need to clean that channel resources which has been done in remove function in this series, call that remove function here. If there are such allocations done for other dma devices as well and have their remove functions implemented, then they should consider adding those as well here. I don't think there is any such case with other dmas.
Understood this is k3 specific , But here i meant multiple dma devices like udma, pdma etc.
In general there will only one dma either udma(flash boot) or pdma(ethboot) being used in spl at boot time to load the next stage images. There is no case where multiple dmas work at same time atleast for now. If such case comes in future and need a remove, it should be taken care.
Thanks, Prasanth
- if (!rc && dev) {
rc = device_remove(dev, DM_REMOVE_NORMAL);if (rc)pr_warn("Cannot remove dma device '%s' (err=%d)\n",dev->name, rc);- }
+}
- void spl_board_prepare_for_boot(void) {
+#if !(defined(CONFIG_SYS_ICACHE_OFF) && defined(CONFIG_SYS_DCACHE_OFF)) dcache_disable(); +#endif
Please suggest , why we are protecting dcache_disable with flags ?
This is carry forward which was already present. This patch removes the protection from whole spl_board_prepare_for_boot() function to dcache_disable() only. This alone looks like it has been added in this patch, but if we see the whole snippet, it can be understood well.
Thanks
Thanks, Prasanth
+#if IS_ENABLED(CONFIG_SPL_DMA) && IS_ENABLED(CONFIG_SPL_DM_DEVICE_REMOVE)
- k3_dma_remove();
+#endif } +#if !(defined(CONFIG_SYS_ICACHE_OFF) && defined(CONFIG_SYS_DCACHE_OFF)) void spl_board_prepare_for_linux(void) { dcache_disable();
Enable CONFIG_SPL_DM_DEVICE_REMOVE in a72 and r5.
Signed-off-by: Prasanth Babu Mantena p-mantena@ti.com --- configs/am62ax_evm_a53_defconfig | 1 + configs/am62ax_evm_r5_defconfig | 1 + configs/am62x_evm_a53_defconfig | 1 + configs/am62x_evm_r5_defconfig | 1 + configs/j7200_evm_a72_defconfig | 1 + configs/j7200_evm_r5_defconfig | 1 + configs/j721e_evm_a72_defconfig | 1 + configs/j721e_evm_r5_defconfig | 1 + configs/j721s2_evm_a72_defconfig | 1 + configs/j721s2_evm_r5_defconfig | 1 + configs/j784s4_evm_a72_defconfig | 1 + configs/j784s4_evm_r5_defconfig | 1 + 12 files changed, 12 insertions(+)
diff --git a/configs/am62ax_evm_a53_defconfig b/configs/am62ax_evm_a53_defconfig index b905ff74f2..8f507ffb25 100644 --- a/configs/am62ax_evm_a53_defconfig +++ b/configs/am62ax_evm_a53_defconfig @@ -46,6 +46,7 @@ CONFIG_MULTI_DTB_FIT=y CONFIG_SPL_MULTI_DTB_FIT=y CONFIG_SPL_MULTI_DTB_FIT_NO_COMPRESSION=y CONFIG_SPL_DM=y +CONFIG_SPL_DM_DEVICE_REMOVE=y CONFIG_SPL_DM_SEQ_ALIAS=y CONFIG_REGMAP=y CONFIG_SPL_REGMAP=y diff --git a/configs/am62ax_evm_r5_defconfig b/configs/am62ax_evm_r5_defconfig index 092d083062..c93d8eff76 100644 --- a/configs/am62ax_evm_r5_defconfig +++ b/configs/am62ax_evm_r5_defconfig @@ -70,6 +70,7 @@ CONFIG_SYS_RELOC_GD_ENV_ADDR=y CONFIG_SYS_MMC_ENV_PART=1 # CONFIG_NET is not set CONFIG_SPL_DM=y +CONFIG_SPL_DM_DEVICE_REMOVE=y CONFIG_SPL_DM_SEQ_ALIAS=y CONFIG_REGMAP=y CONFIG_SPL_REGMAP=y diff --git a/configs/am62x_evm_a53_defconfig b/configs/am62x_evm_a53_defconfig index 0b7ee942a0..003fa4f868 100644 --- a/configs/am62x_evm_a53_defconfig +++ b/configs/am62x_evm_a53_defconfig @@ -63,6 +63,7 @@ CONFIG_SPL_MULTI_DTB_FIT=y CONFIG_SPL_MULTI_DTB_FIT_NO_COMPRESSION=y CONFIG_NET_RANDOM_ETHADDR=y CONFIG_SPL_DM=y +CONFIG_SPL_DM_DEVICE_REMOVE=y CONFIG_SPL_DM_SEQ_ALIAS=y CONFIG_REGMAP=y CONFIG_SPL_REGMAP=y diff --git a/configs/am62x_evm_r5_defconfig b/configs/am62x_evm_r5_defconfig index 78ed3f62d0..c990e386e9 100644 --- a/configs/am62x_evm_r5_defconfig +++ b/configs/am62x_evm_r5_defconfig @@ -77,6 +77,7 @@ CONFIG_SPL_MULTI_DTB_FIT=y CONFIG_SPL_MULTI_DTB_FIT_NO_COMPRESSION=y CONFIG_SYS_RELOC_GD_ENV_ADDR=y CONFIG_SPL_DM=y +CONFIG_SPL_DM_DEVICE_REMOVE=y CONFIG_SPL_DM_SEQ_ALIAS=y CONFIG_REGMAP=y CONFIG_SPL_REGMAP=y diff --git a/configs/j7200_evm_a72_defconfig b/configs/j7200_evm_a72_defconfig index 137ca3f0d0..2e73559f8c 100644 --- a/configs/j7200_evm_a72_defconfig +++ b/configs/j7200_evm_a72_defconfig @@ -93,6 +93,7 @@ CONFIG_SYS_REDUNDAND_ENVIRONMENT=y CONFIG_SYS_RELOC_GD_ENV_ADDR=y CONFIG_NET_RANDOM_ETHADDR=y CONFIG_SPL_DM=y +CONFIG_SPL_DM_DEVICE_REMOVE=y CONFIG_SPL_DM_SEQ_ALIAS=y CONFIG_REGMAP=y CONFIG_SPL_REGMAP=y diff --git a/configs/j7200_evm_r5_defconfig b/configs/j7200_evm_r5_defconfig index 774a9eff43..eed78ba260 100644 --- a/configs/j7200_evm_r5_defconfig +++ b/configs/j7200_evm_r5_defconfig @@ -78,6 +78,7 @@ CONFIG_OF_CONTROL=y CONFIG_SPL_OF_CONTROL=y CONFIG_SYS_RELOC_GD_ENV_ADDR=y CONFIG_SPL_DM=y +CONFIG_SPL_DM_DEVICE_REMOVE=y CONFIG_SPL_DM_SEQ_ALIAS=y CONFIG_REGMAP=y CONFIG_SPL_REGMAP=y diff --git a/configs/j721e_evm_a72_defconfig b/configs/j721e_evm_a72_defconfig index 640c1be2b9..bd55634924 100644 --- a/configs/j721e_evm_a72_defconfig +++ b/configs/j721e_evm_a72_defconfig @@ -97,6 +97,7 @@ CONFIG_SYS_REDUNDAND_ENVIRONMENT=y CONFIG_SYS_RELOC_GD_ENV_ADDR=y CONFIG_NET_RANDOM_ETHADDR=y CONFIG_SPL_DM=y +CONFIG_SPL_DM_DEVICE_REMOVE=y CONFIG_SPL_DM_SEQ_ALIAS=y CONFIG_REGMAP=y CONFIG_SPL_REGMAP=y diff --git a/configs/j721e_evm_r5_defconfig b/configs/j721e_evm_r5_defconfig index 3b4a7c3c0e..5aaf44661c 100644 --- a/configs/j721e_evm_r5_defconfig +++ b/configs/j721e_evm_r5_defconfig @@ -87,6 +87,7 @@ CONFIG_SPL_MULTI_DTB_FIT_NO_COMPRESSION=y CONFIG_ENV_OVERWRITE=y CONFIG_SYS_RELOC_GD_ENV_ADDR=y CONFIG_SPL_DM=y +CONFIG_SPL_DM_DEVICE_REMOVE=y CONFIG_SPL_DM_SEQ_ALIAS=y CONFIG_REGMAP=y CONFIG_SPL_REGMAP=y diff --git a/configs/j721s2_evm_a72_defconfig b/configs/j721s2_evm_a72_defconfig index d78ebb5cfd..ebefe914b0 100644 --- a/configs/j721s2_evm_a72_defconfig +++ b/configs/j721s2_evm_a72_defconfig @@ -92,6 +92,7 @@ CONFIG_SYS_REDUNDAND_ENVIRONMENT=y CONFIG_SYS_RELOC_GD_ENV_ADDR=y CONFIG_NET_RANDOM_ETHADDR=y CONFIG_SPL_DM=y +CONFIG_SPL_DM_DEVICE_REMOVE=y CONFIG_SPL_DM_SEQ_ALIAS=y CONFIG_REGMAP=y CONFIG_SPL_REGMAP=y diff --git a/configs/j721s2_evm_r5_defconfig b/configs/j721s2_evm_r5_defconfig index b6adb6a77d..4561ce594c 100644 --- a/configs/j721s2_evm_r5_defconfig +++ b/configs/j721s2_evm_r5_defconfig @@ -88,6 +88,7 @@ CONFIG_SPL_MULTI_DTB_FIT_NO_COMPRESSION=y CONFIG_ENV_OVERWRITE=y CONFIG_SYS_RELOC_GD_ENV_ADDR=y CONFIG_SPL_DM=y +CONFIG_SPL_DM_DEVICE_REMOVE=y CONFIG_SPL_DM_SEQ_ALIAS=y CONFIG_REGMAP=y CONFIG_SPL_REGMAP=y diff --git a/configs/j784s4_evm_a72_defconfig b/configs/j784s4_evm_a72_defconfig index f0220170bb..c7fc6bedda 100644 --- a/configs/j784s4_evm_a72_defconfig +++ b/configs/j784s4_evm_a72_defconfig @@ -42,6 +42,7 @@ CONFIG_SPL_SYS_MMCSD_RAW_MODE=y CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR=y CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR=0x1400 CONFIG_SPL_DMA=y +CONFIG_SPL_DM_DEVICE_REMOVE=y CONFIG_SPL_ENV_SUPPORT=y CONFIG_SPL_FS_LOAD_PAYLOAD_NAME="u-boot.img" CONFIG_SPL_I2C=y diff --git a/configs/j784s4_evm_r5_defconfig b/configs/j784s4_evm_r5_defconfig index a116881949..4a74c4c4fe 100644 --- a/configs/j784s4_evm_r5_defconfig +++ b/configs/j784s4_evm_r5_defconfig @@ -75,6 +75,7 @@ CONFIG_SPL_MULTI_DTB_FIT=y CONFIG_SPL_MULTI_DTB_FIT_NO_COMPRESSION=y CONFIG_SYS_RELOC_GD_ENV_ADDR=y CONFIG_SPL_DM=y +CONFIG_SPL_DM_DEVICE_REMOVE=y CONFIG_SPL_DM_SEQ_ALIAS=y CONFIG_REGMAP=y CONFIG_SPL_REGMAP=y
From: Santhosh Kumar K s-k6@ti.com
The udma_probe() function was placed above many important APIs related to bcdma, pktdma, which restricts these APIs to be accessed during probe. So, move udma_probe() below all of them.
Signed-off-by: Santhosh Kumar K s-k6@ti.com Signed-off-by: Prasanth Babu Mantena p-mantena@ti.com --- drivers/dma/ti/k3-udma.c | 270 +++++++++++++++++++-------------------- 1 file changed, 135 insertions(+), 135 deletions(-)
diff --git a/drivers/dma/ti/k3-udma.c b/drivers/dma/ti/k3-udma.c index e23d09e6b8..0543f5f4c8 100644 --- a/drivers/dma/ti/k3-udma.c +++ b/drivers/dma/ti/k3-udma.c @@ -1700,141 +1700,6 @@ static int setup_resources(struct udma_dev *ud) return ch_count; }
-static int udma_probe(struct udevice *dev) -{ - struct dma_dev_priv *uc_priv = dev_get_uclass_priv(dev); - struct udma_dev *ud = dev_get_priv(dev); - int i, ret; - struct udevice *tmp; - struct udevice *tisci_dev = NULL; - struct udma_tisci_rm *tisci_rm = &ud->tisci_rm; - ofnode navss_ofnode = ofnode_get_parent(dev_ofnode(dev)); - - ud->match_data = (void *)dev_get_driver_data(dev); - ret = udma_get_mmrs(dev); - if (ret) - return ret; - - ud->psil_base = ud->match_data->psil_base; - - ret = uclass_get_device_by_phandle(UCLASS_FIRMWARE, dev, - "ti,sci", &tisci_dev); - if (ret) { - debug("Failed to get TISCI phandle (%d)\n", ret); - tisci_rm->tisci = NULL; - return -EINVAL; - } - tisci_rm->tisci = (struct ti_sci_handle *) - (ti_sci_get_handle_from_sysfw(tisci_dev)); - - tisci_rm->tisci_dev_id = -1; - ret = dev_read_u32(dev, "ti,sci-dev-id", &tisci_rm->tisci_dev_id); - if (ret) { - dev_err(dev, "ti,sci-dev-id read failure %d\n", ret); - return ret; - } - - tisci_rm->tisci_navss_dev_id = -1; - ret = ofnode_read_u32(navss_ofnode, "ti,sci-dev-id", - &tisci_rm->tisci_navss_dev_id); - if (ret) { - dev_err(dev, "navss sci-dev-id read failure %d\n", ret); - return ret; - } - - tisci_rm->tisci_udmap_ops = &tisci_rm->tisci->ops.rm_udmap_ops; - tisci_rm->tisci_psil_ops = &tisci_rm->tisci->ops.rm_psil_ops; - - if (ud->match_data->type == DMA_TYPE_UDMA) { - ret = uclass_get_device_by_phandle(UCLASS_MISC, dev, - "ti,ringacc", &tmp); - ud->ringacc = dev_get_priv(tmp); - } else { - struct k3_ringacc_init_data ring_init_data; - - ring_init_data.tisci = ud->tisci_rm.tisci; - ring_init_data.tisci_dev_id = ud->tisci_rm.tisci_dev_id; - if (ud->match_data->type == DMA_TYPE_BCDMA) { - ring_init_data.num_rings = ud->bchan_cnt + - ud->tchan_cnt + - ud->rchan_cnt; - } else { - ring_init_data.num_rings = ud->rflow_cnt + - ud->tflow_cnt; - } - - ud->ringacc = k3_ringacc_dmarings_init(dev, &ring_init_data); - } - if (IS_ERR(ud->ringacc)) - return PTR_ERR(ud->ringacc); - - ud->dev = dev; - ret = setup_resources(ud); - if (ret < 0) - return ret; - - ud->ch_count = ret; - - for (i = 0; i < ud->bchan_cnt; i++) { - struct udma_bchan *bchan = &ud->bchans[i]; - - bchan->id = i; - bchan->reg_rt = ud->mmrs[MMR_BCHANRT] + i * 0x1000; - } - - for (i = 0; i < ud->tchan_cnt; i++) { - struct udma_tchan *tchan = &ud->tchans[i]; - - tchan->id = i; - tchan->reg_chan = ud->mmrs[MMR_TCHAN] + UDMA_CH_100(i); - tchan->reg_rt = ud->mmrs[MMR_TCHANRT] + UDMA_CH_1000(i); - } - - for (i = 0; i < ud->rchan_cnt; i++) { - struct udma_rchan *rchan = &ud->rchans[i]; - - rchan->id = i; - rchan->reg_chan = ud->mmrs[MMR_RCHAN] + UDMA_CH_100(i); - rchan->reg_rt = ud->mmrs[MMR_RCHANRT] + UDMA_CH_1000(i); - } - - for (i = 0; i < ud->rflow_cnt; i++) { - struct udma_rflow *rflow = &ud->rflows[i]; - - rflow->id = i; - rflow->reg_rflow = ud->mmrs[MMR_RFLOW] + UDMA_CH_40(i); - } - - for (i = 0; i < ud->ch_count; i++) { - struct udma_chan *uc = &ud->channels[i]; - - uc->ud = ud; - uc->id = i; - uc->config.remote_thread_id = -1; - uc->bchan = NULL; - uc->tchan = NULL; - uc->rchan = NULL; - uc->config.mapped_channel_id = -1; - uc->config.default_flow_id = -1; - uc->config.dir = DMA_MEM_TO_MEM; - sprintf(uc->name, "UDMA chan%d\n", i); - if (!i) - uc->in_use = true; - } - - pr_debug("%s(rev: 0x%08x) CAP0-3: 0x%08x, 0x%08x, 0x%08x, 0x%08x\n", - dev->name, - udma_read(ud->mmrs[MMR_GCFG], 0), - udma_read(ud->mmrs[MMR_GCFG], 0x20), - udma_read(ud->mmrs[MMR_GCFG], 0x24), - udma_read(ud->mmrs[MMR_GCFG], 0x28), - udma_read(ud->mmrs[MMR_GCFG], 0x2c)); - - uc_priv->supported = DMA_SUPPORTS_MEM_TO_MEM | DMA_SUPPORTS_MEM_TO_DEV; - - return 0; -} - static int udma_push_to_ring(struct k3_nav_ring *ring, void *elem) { u64 addr = 0; @@ -2717,6 +2582,141 @@ static int udma_get_cfg(struct dma *dma, u32 id, void **data) return -EINVAL; }
+static int udma_probe(struct udevice *dev) +{ + struct dma_dev_priv *uc_priv = dev_get_uclass_priv(dev); + struct udma_dev *ud = dev_get_priv(dev); + int i, ret; + struct udevice *tmp; + struct udevice *tisci_dev = NULL; + struct udma_tisci_rm *tisci_rm = &ud->tisci_rm; + ofnode navss_ofnode = ofnode_get_parent(dev_ofnode(dev)); + + ud->match_data = (void *)dev_get_driver_data(dev); + ret = udma_get_mmrs(dev); + if (ret) + return ret; + + ud->psil_base = ud->match_data->psil_base; + + ret = uclass_get_device_by_phandle(UCLASS_FIRMWARE, dev, + "ti,sci", &tisci_dev); + if (ret) { + debug("Failed to get TISCI phandle (%d)\n", ret); + tisci_rm->tisci = NULL; + return -EINVAL; + } + tisci_rm->tisci = (struct ti_sci_handle *) + (ti_sci_get_handle_from_sysfw(tisci_dev)); + + tisci_rm->tisci_dev_id = -1; + ret = dev_read_u32(dev, "ti,sci-dev-id", &tisci_rm->tisci_dev_id); + if (ret) { + dev_err(dev, "ti,sci-dev-id read failure %d\n", ret); + return ret; + } + + tisci_rm->tisci_navss_dev_id = -1; + ret = ofnode_read_u32(navss_ofnode, "ti,sci-dev-id", + &tisci_rm->tisci_navss_dev_id); + if (ret) { + dev_err(dev, "navss sci-dev-id read failure %d\n", ret); + return ret; + } + + tisci_rm->tisci_udmap_ops = &tisci_rm->tisci->ops.rm_udmap_ops; + tisci_rm->tisci_psil_ops = &tisci_rm->tisci->ops.rm_psil_ops; + + if (ud->match_data->type == DMA_TYPE_UDMA) { + ret = uclass_get_device_by_phandle(UCLASS_MISC, dev, + "ti,ringacc", &tmp); + ud->ringacc = dev_get_priv(tmp); + } else { + struct k3_ringacc_init_data ring_init_data; + + ring_init_data.tisci = ud->tisci_rm.tisci; + ring_init_data.tisci_dev_id = ud->tisci_rm.tisci_dev_id; + if (ud->match_data->type == DMA_TYPE_BCDMA) { + ring_init_data.num_rings = ud->bchan_cnt + + ud->tchan_cnt + + ud->rchan_cnt; + } else { + ring_init_data.num_rings = ud->rflow_cnt + + ud->tflow_cnt; + } + + ud->ringacc = k3_ringacc_dmarings_init(dev, &ring_init_data); + } + if (IS_ERR(ud->ringacc)) + return PTR_ERR(ud->ringacc); + + ud->dev = dev; + ret = setup_resources(ud); + if (ret < 0) + return ret; + + ud->ch_count = ret; + + for (i = 0; i < ud->bchan_cnt; i++) { + struct udma_bchan *bchan = &ud->bchans[i]; + + bchan->id = i; + bchan->reg_rt = ud->mmrs[MMR_BCHANRT] + i * 0x1000; + } + + for (i = 0; i < ud->tchan_cnt; i++) { + struct udma_tchan *tchan = &ud->tchans[i]; + + tchan->id = i; + tchan->reg_chan = ud->mmrs[MMR_TCHAN] + UDMA_CH_100(i); + tchan->reg_rt = ud->mmrs[MMR_TCHANRT] + UDMA_CH_1000(i); + } + + for (i = 0; i < ud->rchan_cnt; i++) { + struct udma_rchan *rchan = &ud->rchans[i]; + + rchan->id = i; + rchan->reg_chan = ud->mmrs[MMR_RCHAN] + UDMA_CH_100(i); + rchan->reg_rt = ud->mmrs[MMR_RCHANRT] + UDMA_CH_1000(i); + } + + for (i = 0; i < ud->rflow_cnt; i++) { + struct udma_rflow *rflow = &ud->rflows[i]; + + rflow->id = i; + rflow->reg_rflow = ud->mmrs[MMR_RFLOW] + UDMA_CH_40(i); + } + + for (i = 0; i < ud->ch_count; i++) { + struct udma_chan *uc = &ud->channels[i]; + + uc->ud = ud; + uc->id = i; + uc->config.remote_thread_id = -1; + uc->bchan = NULL; + uc->tchan = NULL; + uc->rchan = NULL; + uc->config.mapped_channel_id = -1; + uc->config.default_flow_id = -1; + uc->config.dir = DMA_MEM_TO_MEM; + sprintf(uc->name, "UDMA chan%d\n", i); + if (!i) + uc->in_use = true; + } + + pr_debug("%s(rev: 0x%08x) CAP0-3: 0x%08x, 0x%08x, 0x%08x, 0x%08x\n", + dev->name, + udma_read(ud->mmrs[MMR_GCFG], 0), + udma_read(ud->mmrs[MMR_GCFG], 0x20), + udma_read(ud->mmrs[MMR_GCFG], 0x24), + udma_read(ud->mmrs[MMR_GCFG], 0x28), + udma_read(ud->mmrs[MMR_GCFG], 0x2c)); + + uc_priv->supported = DMA_SUPPORTS_MEM_TO_MEM | DMA_SUPPORTS_MEM_TO_DEV; + + return 0; +} + static const struct dma_ops udma_ops = { .transfer = udma_transfer, .of_xlate = udma_of_xlate,
From: Santhosh Kumar K s-k6@ti.com
Currently, the allocation of DMA channel[0] for memcpy is happening in udma_transfer() for every transfer, which leads to a huge overhead for each transfer, especially in case of nand page reads. So, move this allocation to udma_probe(), as a result, the allocation is done once during probe.
Introduce udma_remove() for the cleanup of allocated channel during probe.
Signed-off-by: Santhosh Kumar K s-k6@ti.com Signed-off-by: Prasanth Babu Mantena p-mantena@ti.com --- drivers/dma/ti/k3-udma.c | 64 ++++++++++++++++++++++++---------------- 1 file changed, 39 insertions(+), 25 deletions(-)
diff --git a/drivers/dma/ti/k3-udma.c b/drivers/dma/ti/k3-udma.c index 0543f5f4c8..e0328536a2 100644 --- a/drivers/dma/ti/k3-udma.c +++ b/drivers/dma/ti/k3-udma.c @@ -2190,37 +2190,12 @@ static int udma_transfer(struct udevice *dev, int direction, /* Channel0 is reserved for memcpy */ struct udma_chan *uc = &ud->channels[0]; dma_addr_t paddr = 0; - int ret; - - switch (ud->match_data->type) { - case DMA_TYPE_UDMA: - ret = udma_alloc_chan_resources(uc); - break; - case DMA_TYPE_BCDMA: - ret = bcdma_alloc_chan_resources(uc); - break; - default: - return -EINVAL; - }; - if (ret) - return ret;
udma_prep_dma_memcpy(uc, dst, src, len); udma_start(uc); udma_poll_completion(uc, &paddr); udma_stop(uc);
- switch (ud->match_data->type) { - case DMA_TYPE_UDMA: - udma_free_chan_resources(uc); - break; - case DMA_TYPE_BCDMA: - bcdma_free_bchan_resources(uc); - break; - default: - return -EINVAL; - }; - return 0; }
@@ -2590,6 +2565,7 @@ static int udma_probe(struct udevice *dev) struct udevice *tmp; struct udevice *tisci_dev = NULL; struct udma_tisci_rm *tisci_rm = &ud->tisci_rm; + struct udma_chan *uc; ofnode navss_ofnode = ofnode_get_parent(dev_ofnode(dev));
ud->match_data = (void *)dev_get_driver_data(dev); @@ -2714,6 +2690,42 @@ static int udma_probe(struct udevice *dev)
uc_priv->supported = DMA_SUPPORTS_MEM_TO_MEM | DMA_SUPPORTS_MEM_TO_DEV;
+ uc = &ud->channels[0]; + switch (ud->match_data->type) { + case DMA_TYPE_UDMA: + ret = udma_alloc_chan_resources(uc); + break; + case DMA_TYPE_BCDMA: + ret = bcdma_alloc_chan_resources(uc); + break; + default: + return -EINVAL; + }; + + if (ret) { + dev_err(dev, " Channel 0 allocation failure %d\n", ret); + return ret; + } + + return 0; +} + +static int udma_remove(struct udevice *dev) +{ + struct udma_dev *ud = dev_get_priv(dev); + struct udma_chan *uc = &ud->channels[0]; + + switch (ud->match_data->type) { + case DMA_TYPE_UDMA: + udma_free_chan_resources(uc); + break; + case DMA_TYPE_BCDMA: + bcdma_free_bchan_resources(uc); + break; + default: + return -EINVAL; + }; + return 0; }
@@ -2855,5 +2867,7 @@ U_BOOT_DRIVER(ti_edma3) = { .of_match = udma_ids, .ops = &udma_ops, .probe = udma_probe, + .remove = udma_remove, .priv_auto = sizeof(struct udma_dev), + .flags = DM_FLAG_OS_PREPARE, };
On 10/4/2024 6:50 PM, Prasanth Babu Mantena wrote:
From: Santhosh Kumar K s-k6@ti.com
Currently, the allocation of DMA channel[0] for memcpy is happening in udma_transfer() for every transfer, which leads to a huge overhead for each transfer, especially in case of nand page reads. So, move this
[..]
@@ -2590,6 +2565,7 @@ static int udma_probe(struct udevice *dev) struct udevice *tmp; struct udevice *tisci_dev = NULL; struct udma_tisci_rm *tisci_rm = &ud->tisci_rm;
struct udma_chan *uc; ofnode navss_ofnode = ofnode_get_parent(dev_ofnode(dev));
ud->match_data = (void *)dev_get_driver_data(dev);
@@ -2714,6 +2690,42 @@ static int udma_probe(struct udevice *dev)
uc_priv->supported = DMA_SUPPORTS_MEM_TO_MEM | DMA_SUPPORTS_MEM_TO_DEV;
- uc = &ud->channels[0];
- switch (ud->match_data->type) {
- case DMA_TYPE_UDMA:
ret = udma_alloc_chan_resources(uc);break;- case DMA_TYPE_BCDMA:
ret = bcdma_alloc_chan_resources(uc);break;- default:
return -EINVAL;
shouldn't we treat this default case as error and do some dev_err prints ?
- };
- if (ret) {
dev_err(dev, " Channel 0 allocation failure %d\n", ret);return ret;
No need of this ret
- }
- return 0;
do 'return ret' here
+}
+static int udma_remove(struct udevice *dev) +{
- struct udma_dev *ud = dev_get_priv(dev);
- struct udma_chan *uc = &ud->channels[0];
- switch (ud->match_data->type) {
- case DMA_TYPE_UDMA:
udma_free_chan_resources(uc);break;- case DMA_TYPE_BCDMA:
bcdma_free_bchan_resources(uc);break;- default:
return -EINVAL;
shouldn't we treat this default case as error and do some dev_err prints ?
- };
- return 0; }
@@ -2855,5 +2867,7 @@ U_BOOT_DRIVER(ti_edma3) = { .of_match = udma_ids, .ops = &udma_ops, .probe = udma_probe,
- .remove = udma_remove, .priv_auto = sizeof(struct udma_dev),
- .flags = DM_FLAG_OS_PREPARE, };
participants (3)
-
Kumar, Udit -
Prasanth Babu Mantena -
Prasanth Mantena