Hello.
I'm the author of the da8xx.c file you're patching. :-)
On 07/23/2015 07:43 PM, Andrew Holcomb wrote:
Trying this again as plain text... sorry about that.
Please place such comments under the --- tear line (that goes after
signoff), else they end up in the patch change log and a maintainer will have
to remove them by hand.
Attached is a patch
It's not really attached.
that adds device tree support to the da8xx musb driver.
I prefer to call it glue layer, not a driver.
The current driver expects a board file to setup the platform device and perform the initialization.
> With this patch all of the setup is done through the device tree.
Please wrap your change log at 80 columns (or even less).
diffstat for this patch is:
You can have diffstat under --- without such comments.
Documentation/devicetree/bindings/usb/da8xx-usb.txt | 18 ++
drivers/usb/musb/Kconfig | 1
drivers/usb/musb/da8xx.c | 139 ++++++++++++++------
3 files changed, 119 insertions(+), 39 deletions(-)
To apply this patch, in the root of a kernel tree use:
patch -p1 < da8xx-musb.patch
No need to tell that, -p1 is a default.
Please let me know any feedback you have on this patch.
Thanks
Andrew Holcomb
Software Engineer
RELM Wireless
The patch change log is not a place for your signature.
It's better to completely omit that.
Signed-off-by: Andrew T Holcomb <aholcomb@xxxxxxxxxx>
-----------------------------------------------------------------------------------------------------------------------------
No need for such separator.
diff -pruN linux-4.1/Documentation/devicetree/bindings/usb/da8xx-usb.txt linux-4.1.musb/Documentation/devicetree/bindings/usb/da8xx-usb.txt
--- linux-4.1/Documentation/devicetree/bindings/usb/da8xx-usb.txt 1969-12-31 18:00:00.000000000 -0600
+++ linux-4.1.musb/Documentation/devicetree/bindings/usb/da8xx-usb.txt 2015-07-23 10:49:35.926160500 -0500
@@ -0,0 +1,18 @@
+DA8XX MUSB
I'd prefer to call it DA8xx/OMAP-L1x.
+
+Required properties:
+ - compatible : Should be "ti,da8xx-musb"
+ - reg : Offset and length of registers
+ - interrupts : Interrupt number
+ - mode : Dual-role; either host mode "host", peripheral mode "peripheral"
+ or both "otg"
Isn't there a standardized property called "dr_mode" for that?
And shouldn't be also e.g. "clocks" property?
+
+Example:
+
+musb@1e00000 {
Just "usb@1e00000" please, to conform to the ePAPR standard.
+ compatible = "ti,da8xx-musb";
+ reg = <0x01e00000 0x10000>;
+ interrupts = <58>;
+ interrupt-names = "mc";
You didn't mention it above.
+ mode = "peripheral";
+};
diff -pruN linux-4.1/drivers/usb/musb/da8xx.c linux-4.1.musb/drivers/usb/musb/da8xx.c
--- linux-4.1/drivers/usb/musb/da8xx.c 2015-06-22 00:05:43.000000000 -0500
+++ linux-4.1.musb/drivers/usb/musb/da8xx.c 2015-07-23 10:49:35.926160500 -0500
@@ -89,6 +89,36 @@ struct da8xx_glue {
[...]
+static struct musb_hdrc_platform_data usb_data = {
+ /* OTG requires a Mini-AB connector */
+ .mode = MUSB_PERIPHERAL,
Hm, why are you defaulting to gadget?
[...]
@@ -105,6 +135,9 @@ static inline void phy_on(void)
*/
cfgchip2 &= ~(CFGCHIP2_RESET | CFGCHIP2_PHYPWRDN | CFGCHIP2_OTGPWRDN);
cfgchip2 |= CFGCHIP2_PHY_PLLON;
+ /* USB2.0 PHY reference clock is 24 MHz */
+ cfgchip2 &= ~CFGCHIP2_REFFREQ;
+ cfgchip2 |= CFGCHIP2_REFFREQ_24MHZ;
I think this depends on the board.
[...]
@@ -480,44 +513,68 @@ static const struct platform_device_info
static int da8xx_probe(struct platform_device *pdev)
{
- struct resource musb_resources[2];
+ struct resource musb_resources[2];
struct musb_hdrc_platform_data *pdata = dev_get_platdata(&pdev->dev);
+ struct device_node *np = pdev->dev.of_node;
struct platform_device *musb;
struct da8xx_glue *glue;
- struct platform_device_info pinfo;
struct clk *clk;
-
int ret = -ENOMEM;
+ const char *mode;
+ int strlen;
Why not just 'len'?
- glue = kzalloc(sizeof(*glue), GFP_KERNEL);
- if (!glue) {
- dev_err(&pdev->dev, "failed to allocate glue context\n");
+ glue = devm_kzalloc(&pdev->dev, sizeof(*glue), GFP_KERNEL);
+ if (!glue)
+ goto err0;
+
This change is not related to device tree support.
+ musb = platform_device_alloc("musb-hdrc", PLATFORM_DEVID_AUTO);
+ if (!musb) {
+ dev_err(&pdev->dev, "failed to allocate musb device\n");
goto err0;
}
- clk = clk_get(&pdev->dev, "usb20");
+ clk = devm_clk_get(&pdev->dev, "usb20");
This change is not related to device tree support.
if (IS_ERR(clk)) {
dev_err(&pdev->dev, "failed to get clock\n");
ret = PTR_ERR(clk);
- goto err3;
+ goto err1;
}
- ret = clk_enable(clk);
+ ret = clk_prepare_enable(clk);
This change is not related to device tree support.
[...]
+ if (np) {
+ pdata = &usb_data;
+ /* Mode can be overridden */
+ mode = of_get_property(np, "mode", &strlen);
+ if (!mode) {
+ dev_info(&pdev->dev, "No 'mode' property found... defaulting to MUSB_PERIPHERAL.\n");
+ pdata->mode = MUSB_PERIPHERAL;
Dubious default. And BTW, isn't it already initialized to this value?
[...]
diff -pruN linux-4.1/drivers/usb/musb/Kconfig linux-4.1.musb/drivers/usb/musb/Kconfig
--- linux-4.1/drivers/usb/musb/Kconfig 2015-06-22 00:05:43.000000000 -0500
+++ linux-4.1.musb/drivers/usb/musb/Kconfig 2015-07-23 10:49:35.926160500 -0500
@@ -70,7 +70,6 @@ config USB_MUSB_DA8XX
tristate "DA8xx/OMAP-L1x"
depends on ARCH_DAVINCI_DA8XX
depends on NOP_USB_XCEIV
- depends on BROKEN
It's not marked broken because of missing DT support. :-)
[...]
MBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
