| Message ID | 1374237277-17769-4-git-send-email-george.cherian@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, On Friday 19 July 2013 06:04 PM, George Cherian wrote: > Add phy nodes for AM33XX platform and split the musb nodes > per instance. > > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> > Signed-off-by: George Cherian <george.cherian@ti.com> > --- > arch/arm/boot/dts/am33xx.dtsi | 68 +++++++++++++++++++++++++++++++++---------- > 1 file changed, 53 insertions(+), 15 deletions(-) > > diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi > index 8e1248f..e3890c4 100644 > --- a/arch/arm/boot/dts/am33xx.dtsi > +++ b/arch/arm/boot/dts/am33xx.dtsi > @@ -326,21 +326,59 @@ > 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>; > - ti,hwmods = "usb_otg_hs"; > + phy1: am335x-usb0@44e10620 { *44e10620* is not needed if you are not having the reg property. > + compatible = "ti,am335x-usb2"; > + #phy-cells = <0>; > + id= <0>; > + }; > + > + phy2: am335x-usb1@44e10628 { > + compatible = "ti,am335x-usb2"; ditto.. > + #phy-cells = <0>; > + id= <1>; > + }; > + > + omap_control_usb: omap-control-usb@44e10620 { > + compatible = "ti,omap-control-usb"; > + reg = <0x44e10620 0x10>; > + reg-names = "control_dev_conf"; > + ti,type = <3>; > + }; > + > + musb: usb@47400000 { > + compatible = "ti,musb-am33xx"; > + reg = <0x47400000 0x1000>; > + ranges; > + #address-cells = <1>; > + #size-cells = <1>; > + interrupts = <17>; > + ti,hwmods = "usb_otg_hs"; > + > + usb0@47401000 { > + reg = <0x47401000 0x800>; > + interrupts = <18>; > + interrupt-names = "mc"; > + multipoint = <1>; > + num-eps = <16>; > + ram-bits = <12>; > + port-mode = <3>; > + power = <250>; > + phys = <&phy1>; > + phy-names = "am335x-usb0"; Looks like alignment has gone wrong here. > + }; > + > + usb1@47401800 { > + reg = <0x47401800 0x800>; > + interrupts = <19>; > + interrupt-names = "mc"; > + multipoint = <1>; > + num-eps = <16>; > + ram-bits = <12>; > + port-mode = <3>; > + power = <250>; > + phys = <&phy2>; > + phy-names = "am335x-usb1"; ditto. Thanks Kishon -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello. On 19-07-2013 16:34, George Cherian wrote: > Add phy nodes for AM33XX platform and split the musb nodes > per instance. > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> > Signed-off-by: George Cherian <george.cherian@ti.com> > --- > arch/arm/boot/dts/am33xx.dtsi | 68 +++++++++++++++++++++++++++++++++---------- > 1 file changed, 53 insertions(+), 15 deletions(-) > diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi > index 8e1248f..e3890c4 100644 > --- a/arch/arm/boot/dts/am33xx.dtsi > +++ b/arch/arm/boot/dts/am33xx.dtsi > @@ -326,21 +326,59 @@ > 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>; > - ti,hwmods = "usb_otg_hs"; > + phy1: am335x-usb0@44e10620 { Shouldn't the PHYs be *under* the usb0/1 device nodes in the hierarchy? They're not on the same bus as the MUSB controllers for sure. > + compatible = "ti,am335x-usb2"; > + #phy-cells = <0>; > + id= <0>; Forgot space before =. > + }; > + > + phy2: am335x-usb1@44e10628 { > + compatible = "ti,am335x-usb2"; > + #phy-cells = <0>; > + id= <1>; Same here. > + }; > + > + omap_control_usb: omap-control-usb@44e10620 { > + compatible = "ti,omap-control-usb"; > + reg = <0x44e10620 0x10>; > + reg-names = "control_dev_conf"; > + ti,type = <3>; > + }; > + > + musb: usb@47400000 { > + compatible = "ti,musb-am33xx"; > + reg = <0x47400000 0x1000>; > + ranges; > + #address-cells = <1>; > + #size-cells = <1>; > + interrupts = <17>; > + ti,hwmods = "usb_otg_hs"; > + > + usb0@47401000 { I don't think you need index in the name since you have the address as a part of the name anyway. That way it's closer to the ePAPR spec. > + reg = <0x47401000 0x800>; > + interrupts = <18>; > + interrupt-names = "mc"; > + multipoint = <1>; > + num-eps = <16>; > + ram-bits = <12>; > + port-mode = <3>; > + power = <250>; > + phys = <&phy1>; The above lines are indented with spaces, the below one with tabs. Use tabs please. > + phy-names = "am335x-usb0"; > + }; > + > + usb1@47401800 { > + reg = <0x47401800 0x800>; > + interrupts = <19>; > + interrupt-names = "mc"; > + multipoint = <1>; > + num-eps = <16>; > + ram-bits = <12>; > + port-mode = <3>; > + power = <250>; > + phys = <&phy2>; The above lines are indented with spaces, the below one with tabs. Use tabs please. > + phy-names = "am335x-usb1"; > + }; > }; WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 07/19/2013 03:56 PM, Sergei Shtylyov wrote: > On 19-07-2013 16:34, George Cherian wrote: > >> diff --git a/arch/arm/boot/dts/am33xx.dtsi >> b/arch/arm/boot/dts/am33xx.dtsi >> index 8e1248f..e3890c4 100644 >> --- a/arch/arm/boot/dts/am33xx.dtsi >> +++ b/arch/arm/boot/dts/am33xx.dtsi >> @@ -326,21 +326,59 @@ >> 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>; >> - ti,hwmods = "usb_otg_hs"; >> + phy1: am335x-usb0@44e10620 { > > Shouldn't the PHYs be *under* the usb0/1 device nodes in the hierarchy? > They're not on the same bus as the MUSB controllers for sure. I redo the complete thing. I have now: usb: usb@47400000 { compatible = "ti,am33xx-usb"; usb0_phy: phy@47401300 { compatible = "ti,am335x-usb-phy"; } usb0: usb@47401000 { musb0: usb@47401400 { compatible = "mg,musbmhdrc"; } } usb1_phy: phy@47402300 { compatible = "ti,am335x-usb-phy"; } usb1: usb@47402000 { musb1: usb@47402400 { compatible = "mg,musbmhdrc"; } } } And you want usb0_phy to be child of usb0? In the TRM they are all in the same block. > > WBR, Sergei > Sebastian -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello. On 07/19/2013 06:20 PM, Sebastian Andrzej Siewior wrote: >>> diff --git a/arch/arm/boot/dts/am33xx.dtsi >>> b/arch/arm/boot/dts/am33xx.dtsi >>> index 8e1248f..e3890c4 100644 >>> --- a/arch/arm/boot/dts/am33xx.dtsi >>> +++ b/arch/arm/boot/dts/am33xx.dtsi >>> @@ -326,21 +326,59 @@ >>> 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>; >>> - ti,hwmods = "usb_otg_hs"; >>> + phy1: am335x-usb0@44e10620 { >> Shouldn't the PHYs be *under* the usb0/1 device nodes in the hierarchy? >> They're not on the same bus as the MUSB controllers for sure. > I redo the complete thing. I have now: > usb: usb@47400000 { > compatible = "ti,am33xx-usb"; > > usb0_phy: phy@47401300 { > compatible = "ti,am335x-usb-phy"; > } > usb0: usb@47401000 { > musb0: usb@47401400 { > compatible = "mg,musbmhdrc"; > } > } > usb1_phy: phy@47402300 { > compatible = "ti,am335x-usb-phy"; > } > usb1: usb@47402000 { > musb1: usb@47402400 { > compatible = "mg,musbmhdrc"; > } > } > } > And you want usb0_phy to be child of usb0? In the TRM they are all in > the same block. Ah, the fact that PHYs didn't have the "reg" property got me muddled, I didn't pay attention to the address part of the node names... BTW, where is the "reg" prop? I see PHYs share the address space with "omap-control-usb@44e10620" device -- what's the point with this? > Sebastian WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 07/19/2013 08:33 PM, Sergei Shtylyov wrote: > Hello. Hello, >> usb: usb@47400000 { >> compatible = "ti,am33xx-usb"; >> >> usb0_phy: phy@47401300 { >> compatible = "ti,am335x-usb-phy"; >> } >> usb0: usb@47401000 { >> musb0: usb@47401400 { >> compatible = "mg,musbmhdrc"; >> } >> } >> usb1_phy: phy@47402300 { >> compatible = "ti,am335x-usb-phy"; >> } >> usb1: usb@47402000 { >> musb1: usb@47402400 { >> compatible = "mg,musbmhdrc"; >> } >> } >> } > >> And you want usb0_phy to be child of usb0? In the TRM they are all in >> the same block. > > Ah, the fact that PHYs didn't have the "reg" property got me muddled, > I didn't pay attention to the address part of the node names... BTW, > where is the "reg" prop? I skipped it for the general idea. I planned to repost is today but I messed up dsps and need to get it working first… > I see PHYs share the address space with > "omap-control-usb@44e10620" device -- what's the point with this? I decided to get rid of this. Both phys have 8 bytes (2 registers) which are exclusive for them. There is one register for the wakeup which is shared by both. I changed this to limit it only to the 8bytes per phy. I care about wakeup later - hopefully George will take care of this :) > >> Sebastian > > WBR, Sergei > Sebastian -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 7/20/2013 12:03 AM, Sergei Shtylyov wrote: > Hello. > > On 07/19/2013 06:20 PM, Sebastian Andrzej Siewior wrote: > >>>> diff --git a/arch/arm/boot/dts/am33xx.dtsi >>>> b/arch/arm/boot/dts/am33xx.dtsi >>>> index 8e1248f..e3890c4 100644 >>>> --- a/arch/arm/boot/dts/am33xx.dtsi >>>> +++ b/arch/arm/boot/dts/am33xx.dtsi >>>> @@ -326,21 +326,59 @@ >>>> 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>; >>>> - ti,hwmods = "usb_otg_hs"; >>>> + phy1: am335x-usb0@44e10620 { > >>> Shouldn't the PHYs be *under* the usb0/1 device nodes in the >>> hierarchy? >>> They're not on the same bus as the MUSB controllers for sure. > >> I redo the complete thing. I have now: > >> usb: usb@47400000 { >> compatible = "ti,am33xx-usb"; >> >> usb0_phy: phy@47401300 { >> compatible = "ti,am335x-usb-phy"; >> } >> usb0: usb@47401000 { >> musb0: usb@47401400 { >> compatible = "mg,musbmhdrc"; >> } >> } >> usb1_phy: phy@47402300 { >> compatible = "ti,am335x-usb-phy"; >> } >> usb1: usb@47402000 { >> musb1: usb@47402400 { >> compatible = "mg,musbmhdrc"; >> } >> } >> } > >> And you want usb0_phy to be child of usb0? In the TRM they are all in >> the same block. > > Ah, the fact that PHYs didn't have the "reg" property got me > muddled, I didn't pay attention to the address part of the node > names... BTW, where is the "reg" prop? I see PHYs share the address > space with "omap-control-usb@44e10620" device -- what's the point with > this? In control module(CM) each USB has got 2 registers in which one is a shared register( wakeup register) So all the CM access are done using the control-usb driver (phy-omap-control-usb.c). Same reason why phy's dont have a reg property. > >> Sebastian > > WBR, Sergei >
On 7/20/2013 12:12 AM, Sebastian Andrzej Siewior wrote: > On 07/19/2013 08:33 PM, Sergei Shtylyov wrote: >> Hello. > Hello, > >>> usb: usb@47400000 { >>> compatible = "ti,am33xx-usb"; >>> >>> usb0_phy: phy@47401300 { >>> compatible = "ti,am335x-usb-phy"; >>> } >>> usb0: usb@47401000 { >>> musb0: usb@47401400 { >>> compatible = "mg,musbmhdrc"; >>> } >>> } >>> usb1_phy: phy@47402300 { >>> compatible = "ti,am335x-usb-phy"; >>> } >>> usb1: usb@47402000 { >>> musb1: usb@47402400 { >>> compatible = "mg,musbmhdrc"; >>> } >>> } >>> } >>> And you want usb0_phy to be child of usb0? In the TRM they are all in >>> the same block. >> Ah, the fact that PHYs didn't have the "reg" property got me muddled, >> I didn't pay attention to the address part of the node names... BTW, >> where is the "reg" prop? > I skipped it for the general idea. I planned to repost is today but I > messed up dsps and need to get it working first… > >> I see PHYs share the address space with >> "omap-control-usb@44e10620" device -- what's the point with this? > I decided to get rid of this. Both phys have 8 bytes (2 registers) > which are exclusive for them. > There is one register for the wakeup which is shared by both. > I changed this to limit it only to the 8bytes per phy. I care about > wakeup later - hopefully George will take care of this :) But for wakeup how can we map it since its the same register. That is the main reason i took the omap-control-usb route. > >>> Sebastian >> WBR, Sergei >> > Sebastian
On 7/20/2013 9:11 AM, George Cherian wrote: > On 7/20/2013 12:12 AM, Sebastian Andrzej Siewior wrote: >> On 07/19/2013 08:33 PM, Sergei Shtylyov wrote: >>> Hello. >> Hello, >> >>>> usb: usb@47400000 { >>>> compatible = "ti,am33xx-usb"; >>>> usb0_phy: phy@47401300 { >>>> compatible = "ti,am335x-usb-phy"; >>>> } >>>> usb0: usb@47401000 { >>>> musb0: usb@47401400 { >>>> compatible = "mg,musbmhdrc"; >>>> } >>>> } >>>> usb1_phy: phy@47402300 { >>>> compatible = "ti,am335x-usb-phy"; >>>> } >>>> usb1: usb@47402000 { >>>> musb1: usb@47402400 { >>>> compatible = "mg,musbmhdrc"; >>>> } >>>> } >>>> } How about something like this ? Here am making wrapper -> musb instance 0 -> phy0 wrapper -> musb instance 1 -> phy1 musb: usb@47400000 { compatible = "ti,musb-am33xx"; reg = <0x47400000 0x1000>; ranges; #address-cells = <1>; #size-cells = <1>; interrupts = <17>; ti,hwmods = "usb_otg_hs"; usb0@47401000 { reg = <0x47401000 0x800>; interrupts = <18>; interrupt-names = "mc"; multipoint = <1>; num-eps = <16>; ram-bits = <12>; port-mode = <3>; power = <250>; phys = <&phy1>; phy-names = "am335x-usb0"; phy1: am335x-usb0 { compatible = "ti,am335x-usb2"; #phy-cells = <0>; id= <0>; }; }; usb1@47401800 { reg = <0x47401800 0x800>; interrupts = <19>; interrupt-names = "mc"; multipoint = <1>; num-eps = <16>; ram-bits = <12>; port-mode = <3>; power = <250>; phys = <&phy2>; phy-names = "am335x-usb1"; phy2: am335x-usb1 { compatible = "ti,am335x-usb2"; #phy-cells = <0>; id= <1>; }; }; };
diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi index 8e1248f..e3890c4 100644 --- a/arch/arm/boot/dts/am33xx.dtsi +++ b/arch/arm/boot/dts/am33xx.dtsi @@ -326,21 +326,59 @@ 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>; - ti,hwmods = "usb_otg_hs"; + phy1: am335x-usb0@44e10620 { + compatible = "ti,am335x-usb2"; + #phy-cells = <0>; + id= <0>; + }; + + phy2: am335x-usb1@44e10628 { + compatible = "ti,am335x-usb2"; + #phy-cells = <0>; + id= <1>; + }; + + omap_control_usb: omap-control-usb@44e10620 { + compatible = "ti,omap-control-usb"; + reg = <0x44e10620 0x10>; + reg-names = "control_dev_conf"; + ti,type = <3>; + }; + + musb: usb@47400000 { + compatible = "ti,musb-am33xx"; + reg = <0x47400000 0x1000>; + ranges; + #address-cells = <1>; + #size-cells = <1>; + interrupts = <17>; + ti,hwmods = "usb_otg_hs"; + + usb0@47401000 { + reg = <0x47401000 0x800>; + interrupts = <18>; + interrupt-names = "mc"; + multipoint = <1>; + num-eps = <16>; + ram-bits = <12>; + port-mode = <3>; + power = <250>; + phys = <&phy1>; + phy-names = "am335x-usb0"; + }; + + usb1@47401800 { + reg = <0x47401800 0x800>; + interrupts = <19>; + interrupt-names = "mc"; + multipoint = <1>; + num-eps = <16>; + ram-bits = <12>; + port-mode = <3>; + power = <250>; + phys = <&phy2>; + phy-names = "am335x-usb1"; + }; }; mac: ethernet@4a100000 {