Adding legacy USB driver #2851

UtsavAgarwalADI · 2025-07-15T15:03:50Z

PR Description

Adding legacy musb driver for sc57x/58x and enabling DMA where applicable

PR Type

Bug fix (a change that fixes an issue)
New feature (a change that adds new functionality)
Breaking change (a change that affects other repos or cause CIs to fail)

PR Checklist

I have conducted a self-review of my own code changes
I have tested the changes on the relevant hardware
I have updated the documentation outside this repo accordingly (if there is the case)

pamolloy · 2025-07-15T15:12:22Z

Documentation/devicetree/bindings/usb/adi,musb.yaml

@@ -0,0 +1,85 @@
+


Suggested change

pamolloy · 2025-07-15T15:13:29Z

Documentation/devicetree/bindings/usb/adi,musb.yaml

+
+maintainers:
+  - Nathan Barrett-Morrison <nathan.morrison@timesys.com>
+  - Greg Malysa <greg.malysa@timesys.com>


Do they still want to maintain these if they ever make it upstream? Either way, I assume these e-mail addresses aren't valid anymore?

probably should be just adsp-linux@analog.com

pamolloy · 2025-07-15T15:16:40Z

drivers/usb/musb/adi.c

+ *
+ * Contact: Nathan Barrett-Morrison <nathan.morrison@timesys.com>
+ * Contact: Greg Malysa <greg.malysa@timesys.com>
+ *


Suggested change

*

pamolloy · 2025-07-15T15:19:34Z

drivers/usb/musb/adi.h

+ *
+ * Contact: Nathan Barrett-Morrison <nathan.morrison@timesys.com>
+ * Contact: Greg Malysa <greg.malysa@timesys.com>
+ *


Suggested change

*

pamolloy · 2025-07-15T15:22:07Z

drivers/usb/musb/adi.c

+module_platform_driver(adi_musb_driver);
+
+MODULE_DESCRIPTION("ADI MUSB Glue Layer");
+MODULE_AUTHOR("Hao Liang <hliang1025@gmail.com>");


Curious, I see one mainline contribution from Hao Liang:

1569039db006 bfin uart: it will hang when read current y count if not disable dma irq

I wonder why he's listed as the author.

pamolloy · 2025-07-15T15:39:33Z

arch/arm/configs/sc589-mini_defconfig

@@ -120,4 +120,6 @@ CONFIG_CRC_CCITT=y
 CONFIG_PRINTK_TIME=y
 CONFIG_DEBUG_KERNEL=y
 CONFIG_DEBUG_FS=y
+CONFIG_USB_MUSB_ADI=y
+CONFIG_USB_INVENTRA_DMA=y


Looks like there is a conflict, but also it is good practice to select through menuconfig and savedefconfig or to run make sc5890-mini_defconfig; make savedefconfig before committing.

Ideally the commit message would be declarative and have a prefix matching mainline:

ARM: configs: sc589-mini: Enable MUSB DMA

ARM: defconfig: would refer to multiple different defconfigs.

pamolloy · 2025-07-15T15:49:53Z

drivers/usb/musb/adi.c

+	}
+	pdata.mode = class;
+
+	pdata.power = get_int_prop(pdev->dev.of_node, "mentor,power")/2;


I hope checkpatch would catch this:

Suggested change

pdata.power = get_int_prop(pdev->dev.of_node, "mentor,power")/2;

pdata.power = get_int_prop(pdev->dev.of_node, "mentor,power") / 2;

nunojsa

Some initial comments. The commit message should be improved a lot... That you're adding some glue code we can see :). You need to explain why we need it for our platforms and some explanation on why all those platforms devices are needed would also be great.

nunojsa · 2025-07-16T07:02:30Z

drivers/usb/musb/adi.c

+		   .of_match_table = adi_musb_match,
+		   },
+};
+


nit: no need for blank line

nunojsa · 2025-07-16T07:02:59Z

drivers/usb/musb/adi.h

+#define __ADI_H__
+
+static int get_int_prop(struct device_node *node, const char *s);
+


nunojsa · 2025-07-16T07:03:31Z

drivers/usb/musb/adi.c

+
+	platform_device_unregister(glue->musb);
+	usb_phy_generic_unregister(glue->phy);
+	kfree(glue);


fairly sure the above adi_musb_remove() is not needed

The other musb glue layer implementations seem to have kept it:
https://elixir.bootlin.com/linux/v6.12/source/drivers/usb/musb/da8xx.c#L588
https://elixir.bootlin.com/linux/v6.12/source/drivers/usb/musb/mediatek.c#L507

Would this change be more in line with our particular case?

nunojsa · 2025-07-16T07:04:04Z

drivers/usb/musb/adi.c

+	kfree(glue);
+
+err0:
+	dev_err(&pdev->dev, "ADI MUSB device FAILED\n");


use device managed APIs and you should be able to remove all the above labels.

nunojsa · 2025-07-16T07:06:29Z

drivers/usb/musb/adi.c

+	{},
+};
+
+MODULE_DEVICE_TABLE(of, adi_musb_match);


This should be closer to the driver initialization

nunojsa · 2025-07-16T07:08:28Z

drivers/usb/musb/adi.c

+	config = devm_kzalloc(&pdev->dev, sizeof(*config), GFP_KERNEL);
+	if (!config) {
+		ret = -ENOMEM;
+		goto err2;


return dev_err_probe(). Ditto for all other places

nunojsa · 2025-07-16T07:08:51Z

drivers/usb/musb/adi.c

+
+	musb->dev.parent	= &pdev->dev;
+	musb->dev.dma_mask	= &musb_dmamask;
+	musb->dev.coherent_dma_mask = musb_dmamask;


nit: use spaces

nunojsa · 2025-07-16T07:09:03Z

drivers/usb/musb/adi.c

+	}
+
+	/*alloc musb platform_device and register */
+	musb = platform_device_alloc("musb-hdrc", PLATFORM_DEVID_AUTO);


no new line

nunojsa · 2025-07-16T07:09:49Z

drivers/usb/musb/adi.c

+	if (!match) {
+		dev_err(&pdev->dev, "failed to matching of_match node\n");
+		return -EINVAL;
+	}


Why having the above?

nunojsa · 2025-07-16T07:10:10Z

drivers/usb/musb/adi.c

+		return 0;
+
+	return val;
+}


The above helper does not bring much

nunojsa

Some initial comments. The commit message should be improved a lot... That you're adding some glue code we can see :). You need to explain why we need it for our platforms and some explanation on why all those platforms devices are needed would also be great.

UtsavAgarwalADI · 2025-07-17T14:19:12Z

Changelog:

Driver:

Replaced maintainer info
Removed get_int_prop() due to lack of any meaningful contribution
Removed unnecessary static function declaration within header
Removed explicit dma mask value assignment since we already use the default
adi_musb_set_mode() now returns errors upon failure
Removed node matching from probe which does not contribute to device functionality
Replaced #defined() with #IF_ENABLED()
Updated error handling within probe to utilize dev_err_probe()
Updated error handling within probe to unregister phy upon failure as well
Updated commit description to explain why this driver is required for the kernel
Updated maintainer info

Documentation:

Updated maintainer info

Configs:

Updated SC589-mini defconfig with options generated from make savedefconfig
Updated commit subject from defconfig to configs to indicate single platform config change

Adding glue layer for adsp platforms sc57x/sc58x. This is enables support for the following: * usbboot: Mount and boot into file systems contained on USB devices by switching OTG to host mode. * usb gadget audio: Utilize linux's USB gadget for streaming audio from another device by switching OTG to device mode. Signed-off-by: Utsav Agarwal <utsav.agarwal@analog.com>

Adding documentation from legacy kernel for ADI musb glue layer Signed-off-by: Utsav Agarwal <utsav.agarwal@analog.com>

Enabling ADI musb to utilize DMA Signed-off-by: UtsavAgarwalADI <utsav.agarwal@analog.com>

UtsavAgarwalADI · 2025-07-17T14:24:43Z

Changelog:

Adding signature to last commit
Updating maintainer contact in adi.c

artursartamonovsadi

can mount sdcard from usb adapter on sc589

UtsavAgarwalADI requested review from pamolloy, nunojsa, vasbimpikasadi and artursartamonovsadi July 15, 2025 15:03

UtsavAgarwalADI self-assigned this Jul 15, 2025

pamolloy reviewed Jul 15, 2025

View reviewed changes

Documentation/devicetree/bindings/usb/adi,musb.yaml

@@ -0,0 +1,85 @@

Copy link

Collaborator

pamolloy Jul 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change

pamolloy reviewed Jul 15, 2025

View reviewed changes

nunojsa reviewed Jul 16, 2025

View reviewed changes

artursartamonovsadi force-pushed the adsp-main-6.12 branch from f6f0111 to f137331 Compare July 17, 2025 10:00

UtsavAgarwalADI force-pushed the adsp-main-6.12-legacy-usb branch from 98afa1d to 898097b Compare July 17, 2025 14:16

UtsavAgarwalADI requested review from pamolloy and nunojsa July 17, 2025 14:19

UtsavAgarwalADI added 3 commits July 17, 2025 15:20

musb: adi: documentation: Adding ADI musb driver

36544b2

Adding documentation from legacy kernel for ADI musb glue layer Signed-off-by: Utsav Agarwal <utsav.agarwal@analog.com>

ARM: configs: sc589-mini: Enabling musb DMA

0362992

Enabling ADI musb to utilize DMA Signed-off-by: UtsavAgarwalADI <utsav.agarwal@analog.com>

UtsavAgarwalADI force-pushed the adsp-main-6.12-legacy-usb branch 2 times, most recently from c0e11e5 to 0362992 Compare July 17, 2025 14:24

pamolloy approved these changes Jul 20, 2025

View reviewed changes

artursartamonovsadi approved these changes Jul 21, 2025

View reviewed changes

UtsavAgarwalADI merged commit bde0fbf into adsp-main-6.12 Jul 21, 2025
1 check failed

UtsavAgarwalADI deleted the adsp-main-6.12-legacy-usb branch July 21, 2025 11:24

	pdata.power = get_int_prop(pdev->dev.of_node, "mentor,power")/2;
	pdata.power = get_int_prop(pdev->dev.of_node, "mentor,power") / 2;

		#define __ADI_H__

		static int get_int_prop(struct device_node node, const char s);

Adding legacy USB driver #2851

Adding legacy USB driver #2851

Conversation

UtsavAgarwalADI commented Jul 15, 2025

PR Description

PR Type

PR Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nunojsa left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nunojsa left a comment

Choose a reason for hiding this comment

Uh oh!

UtsavAgarwalADI commented Jul 17, 2025

Uh oh!

UtsavAgarwalADI commented Jul 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

artursartamonovsadi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

UtsavAgarwalADI commented Jul 17, 2025 •

edited

Loading