am33xx: create child nodes for the two musb controllers
RFC 1/1 am33xx: create child nodes for the two musb controllers
I've been thinkig about creating two child nodes for the independent musb controllers on the am33. I've been thinking about the following:
arch/arm/boot/dts/am33xx.dtsi +51 -12
@@ -326,21 +326,78 @@ status = "disabled"; }; usb@47400000 { compatible = "ti,musb-am33xx"; - reg = <0x47400000 0x1000 /* usbss */ - 0x47401000 0x800 /* musb instance 0 */ - 0x47401800 0x800>; /* musb instance 1 */ - interrupts = <17 /* usbss */ - 18 /* musb instance 0 */ - 19>; /* musb instance 1 */ - multipoint = <1>; - num-eps = <16>; - ram-bits = <12>; - port0-mode = <3>; - port1-mode = <3>; - power = <250>; + reg = <0x47400000 0x1000>; /* usbss */ + interrupts = <17>; /* usbss */ ti,hwmods = "usb_otg_hs"; + + usb0@0x47401000 { + reg = <0x47401000 0x800>; /* musb instance 0 */ + interrupts = <18>; /* musb instance 0 */ + multipoint = <1>; + num-eps = <16>; + ram-bits = <12>; + port-mode = <3>; + power = <250>; + + phys = <&nopphy0>; + dmas = <&cppi41dma 0 + &cppi41dma 1 + &cppi41dma 2 + &cppi41dma 3 + &cppi41dma 4 + &cppi41dma 32 + &cppi41dma 34 + &cppi41dma 36 + &cppi41dma 38 + &cppi41dma 40>; + dma-names = "rx1", "rx2", "rx3", "rx4", "rx5", + "tx1", "tx2", "tx3", "tx4", "tx5"; + }; + + usb0@0x47401800 { + reg = <0x47401800 0x800>; /* musb instance 1 */ + interrupts = <19>; /* musb instance 1 */ + multipoint = <1>; + num-eps = <16>; + ram-bits = <12>; + port-mode = <3>; + power = <250>; + + phys = <&nopphy1>; + dmas = <&cppi41dma 0 + &cppi41dma 1 + &cppi41dma 2 + &cppi41dma 3 + &cppi41dma 4 + &cppi41dma 62 + &cppi41dma 64 + &cppi41dma 66 + &cppi41dma 68 + &cppi41dma 70>; + dma-names = "rx1", "rx2", "rx3", "rx4", "rx5", + "tx1", "tx2", "tx3", "tx4", "tx5"; + }; + };
Please keep in mind that his is not a proper patch it should point out what I have in mind.
This would easy the for instance the dma channel assignment. Also, it will move back to the port-mode property instead of portX-mode one which it has now. This is kind of confusing since it is not a root hub or anything like that it is a complete musb controller. Plus for the phys property could easily take the one avaible and would not require which instance it really it is.
Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
A message that is not in the archive (sent off-list or not yet received): [email protected]
Hi,
Quoted 78 lines · Sebastian Andrzej Siewior
I've been thinkig about creating two child nodes for the independent musb controllers on the am33. I've been thinking about the following:
arch/arm/boot/dts/am33xx.dtsi +51
-12@@ -326,21 +326,78 @@ status = "disabled"; }; usb@47400000 { compatible = "ti,musb-am33xx"; - reg = <0x47400000 0x1000 /* usbss */ - 0x47401000 0x800 /* musb instance 0 */ - 0x47401800 0x800>; /* musb instance 1 */ - interrupts = <17 /* usbss */ - 18 /* musb instance 0 */ - 19>; /* musb instance 1 */ - multipoint = <1>; - num-eps = <16>; - ram-bits = <12>; - port0-mode = <3>; - port1-mode = <3>; - power = <250>; + reg = <0x47400000 0x1000>; /* usbss */ + interrupts = <17>; /* usbss */ ti,hwmods = "usb_otg_hs"; + + usb0@0x47401000 { + reg = <0x47401000 0x800>; /* musb instance 0 */ + interrupts = <18>; /* musb instance 0 */ + multipoint = <1>; + num-eps = <16>; + ram-bits = <12>; + port-mode = <3>; + power = <250>; + + phys = <&nopphy0>; + dmas = <&cppi41dma 0 + &cppi41dma 1 + &cppi41dma 2 + &cppi41dma 3 + &cppi41dma 4 + &cppi41dma 32 + &cppi41dma 34 + &cppi41dma 36 + &cppi41dma 38 + &cppi41dma 40>; + dma-names = "rx1", "rx2", "rx3", "rx4", "rx5", + "tx1", "tx2", "tx3", "tx4", "tx5"; + }; + + usb0@0x47401800 { + reg = <0x47401800 0x800>; /* musb instance 1 */ + interrupts = <19>; /* musb instance 1 */ + multipoint = <1>; + num-eps = <16>; + ram-bits = <12>; + port-mode = <3>; + power = <250>; + + phys = <&nopphy1>; + dmas = <&cppi41dma 0 + &cppi41dma 1 + &cppi41dma 2 + &cppi41dma 3 + &cppi41dma 4 + &cppi41dma 62 + &cppi41dma 64 + &cppi41dma 66 + &cppi41dma 68 + &cppi41dma 70>; + dma-names = "rx1", "rx2", "rx3", "rx4", "rx5", + "tx1", "tx2", "tx3", "tx4", "tx5"; + }; + };
Quoted 8 lines
Please keep in mind that his is not a proper patch it should point out what I have in mind.
This would easy the for instance the dma channel assignment. Also, it will move back to the port-mode property instead of portX-mode one which it has now. This is kind of confusing since it is not a root hub or anything like that it is a complete musb controller. Plus for the phys property could easily take the one avaible and would not require which instance it really it is.
the patch is alright, but what about the giant amoutn of function pointers we have ? Are you planning to use of_dev_auxdata ??
Signature · 1 line
balbi
Quoted 1 line · Felipe Balbi
Hi,
Hi Felipe,
Quoted 2 lines
the patch is alright, but what about the giant amoutn of function pointers we have ? Are you planning to use
of_dev_auxdata??
I didn't plan to use of_dev_auxdata. What do you mean by "giant amount of function pointers"?
Sebastian
A message that is not in the archive (sent off-list or not yet received): [email protected]
Hi,
Quoted 5 lines · Sebastian Andrzej Siewior
Quoted 2 lines
the patch is alright, but what about the giant amoutn of function pointers we have ? Are you planning to use
of_dev_auxdata??I didn't plan to use
of_dev_auxdata. What do you mean by "giant amount of function pointers"?
every glue layer has:
467 static const struct musb_platform_ops omap2430_ops = {
468 .init = omap2430_musb_init, 469 .exit = omap2430_musb_exit, 470 471 .set_mode = omap2430_musb_set_mode, 472 .try_idle = omap2430_musb_try_idle, 473 474 .set_vbus = omap2430_musb_set_vbus, 475 476 .enable = omap2430_musb_enable, 477 .disable = omap2430_musb_disable,
478 };
note that in the glue layer, they have access to musb itself (biggest mistake of my life, btw :-)
Signature · 1 line
balbi
Hi Sebastian,
Quoted 17 lines · Sebastian Andrzej Siewior
I've been thinkig about creating two child nodes for the independent musb controllers on the am33. I've been thinking about the following:
arch/arm/boot/dts/am33xx.dtsi +0
-4@@ -326,21 +326,78 @@ status = "disabled"; }; usb@47400000 { compatible = "ti,musb-am33xx"; - reg = <0x47400000 0x1000 /* usbss */ - 0x47401000 0x800 /* musb instance 0 */ - 0x47401800 0x800>; /* musb instance 1 */ - interrupts = <17 /* usbss */
arch/arm/boot/dts/am33xx.dtsi line 332
Quoted 10 lines
arch/arm/boot/dts/am33xx.dtsi +2
-8- 18 /* musb instance 0 */ - 19>; /* musb instance 1 */ - multipoint = <1>; - num-eps = <16>; - ram-bits = <12>; - port0-mode = <3>; - port1-mode = <3>; - power = <250>; + reg = <0x47400000 0x1000>; /* usbss */ + interrupts = <17>;
BTW, why do have so many DMA compared to the previous version? /* usbss */
Quoted 51 lines
arch/arm/boot/dts/am33xx.dtsi +49
-0ti,hwmods = "usb_otg_hs"; + + usb0@0x47401000 { + reg = <0x47401000 0x800>; /* musb instance 0 */ + interrupts = <18>; /* musb instance 0 */ + multipoint = <1>; + num-eps = <16>; + ram-bits = <12>; + port-mode = <3>; + power = <250>; + + phys = <&nopphy0>; + dmas = <&cppi41dma 0 + &cppi41dma 1 + &cppi41dma 2 + &cppi41dma 3 + &cppi41dma 4 + &cppi41dma 32 + &cppi41dma 34 + &cppi41dma 36 + &cppi41dma 38 + &cppi41dma 40>; + dma-names = "rx1", "rx2", "rx3", "rx4", "rx5", + "tx1", "tx2", "tx3", "tx4", "tx5"; + }; + + usb0@0x47401800 { + reg = <0x47401800 0x800>; /* musb instance 1 */ + interrupts = <19>; /* musb instance 1 */ + multipoint = <1>; + num-eps = <16>; + ram-bits = <12>; + port-mode = <3>; + power = <250>; + + phys = <&nopphy1>; + dmas = <&cppi41dma 0 + &cppi41dma 1 + &cppi41dma 2 + &cppi41dma 3 + &cppi41dma 4 + &cppi41dma 62 + &cppi41dma 64 + &cppi41dma 66 + &cppi41dma 68 + &cppi41dma 70>; + dma-names = "rx1", "rx2", "rx3", "rx4", "rx5", + "tx1", "tx2", "tx3", "tx4", "tx5"; + }; + };
Quoted 8 lines
Please keep in mind that his is not a proper patch it should point out what I have in mind.
This would easy the for instance the dma channel assignment. Also, it will move back to the port-mode property instead of portX-mode one which it has now. This is kind of confusing since it is not a root hub or anything like that it is a complete musb controller. Plus for the phys property could easily take the one avaible and would not require which instance it really it is.
I don't know the HW in detail, but that looks indeed much better and seems to reflect the partitioning with multiple instances accurately.
If Felipe is OK with that partitioning, I'll take it.
Thanks,
Benoit
Quoted 1 line · Benoit Cousson
Hi Sebastian,
Hi Benoit,
Quoted 1 line
BTW, why do have so many DMA compared to the previous version?
I added them, the previous had none and is PIO only.
I currently use three cells per dma channel (the posted example had two).
In general I think I have to request 2 * 15 dma channels for the 15 TX and RX endpoints. So we end up with 30 channels per device (and we have two of them).
The DMA engine has specific queues which depend on the USB endpoint that is used. Instead of hacking this information in some kind of platform data I decided to push this detail into device tree.
Quoted 4 lines
I don't know the HW in detail, but that looks indeed much better and seems to reflect the partitioning with multiple instances accurately.
If Felipe is OK with that partitioning, I'll take it.
Okay, thanks.
Quoted 3 lines
Thanks,
Benoit
Sebastian
Felipe Balbi
Acked-byQuoted 14 lines · Benoit Cousson
Hi Sebastian,
Quoted 11 lines · Sebastian Andrzej Siewior
I've been thinkig about creating two child nodes for the independent musb controllers on the am33. I've been thinking about the following:
arch/arm/boot/dts/am33xx.dtsi +0
-0@@ -326,21 +326,78 @@ status = "disabled"; };
Quoted 16 lines
Quoted 6 lines
arch/arm/boot/dts/am33xx.dtsi +0
-4usb@47400000 { compatible = "ti,musb-am33xx"; - reg = <0x47400000 0x1000 /* usbss */ - 0x47401000 0x800 /* musb instance 0 */ - 0x47401800 0x800>; /* musb instance 1 */ - interrupts = <17 /* usbss */arch/arm/boot/dts/am33xx.dtsi
Quoted 10 lines
arch/arm/boot/dts/am33xx.dtsi +2
-8- 18 /* musb instance 0 */ - 19>; /* musb instance 1 */ - multipoint = <1>; - num-eps = <16>; - ram-bits = <12>; - port0-mode = <3>; - port1-mode = <3>; - power = <250>; + reg = <0x47400000 0x1000>; /* usbss */ + interrupts = <17>;
trailing whitespaces here
Quoted 65 lines
BTW, why do have so many DMA compared to the previous version? /* usbss */
Quoted 51 lines
arch/arm/boot/dts/am33xx.dtsi +49
-0ti,hwmods = "usb_otg_hs"; + + usb0@0x47401000 { + reg = <0x47401000 0x800>; /* musb instance 0 */ + interrupts = <18>; /* musb instance 0 */ + multipoint = <1>; + num-eps = <16>; + ram-bits = <12>; + port-mode = <3>; + power = <250>; + + phys = <&nopphy0>; + dmas = <&cppi41dma 0 + &cppi41dma 1 + &cppi41dma 2 + &cppi41dma 3 + &cppi41dma 4 + &cppi41dma 32 + &cppi41dma 34 + &cppi41dma 36 + &cppi41dma 38 + &cppi41dma 40>; + dma-names = "rx1", "rx2", "rx3", "rx4", "rx5", + "tx1", "tx2", "tx3", "tx4", "tx5"; + }; + + usb0@0x47401800 { + reg = <0x47401800 0x800>; /* musb instance 1 */ + interrupts = <19>; /* musb instance 1 */ + multipoint = <1>; + num-eps = <16>; + ram-bits = <12>; + port-mode = <3>; + power = <250>; + + phys = <&nopphy1>; + dmas = <&cppi41dma 0 + &cppi41dma 1 + &cppi41dma 2 + &cppi41dma 3 + &cppi41dma 4 + &cppi41dma 62 + &cppi41dma 64 + &cppi41dma 66 + &cppi41dma 68 + &cppi41dma 70>; + dma-names = "rx1", "rx2", "rx3", "rx4", "rx5", + "tx1", "tx2", "tx3", "tx4", "tx5"; + }; + };Quoted 8 lines
Please keep in mind that his is not a proper patch it should point out what I have in mind.
This would easy the for instance the dma channel assignment. Also, it will move back to the port-mode property instead of portX-mode one which it has now. This is kind of confusing since it is not a root hub or anything like that it is a complete musb controller. Plus for the phys property could easily take the one avaible and would not require which instance it really it is.I don't know the HW in detail, but that looks indeed much better and seems to reflect the partitioning with multiple instances accurately.
Quoted 1 line
If Felipe is OK with that partitioning, I'll take it.
it's alright with me
Signature · 1 line
balbi