Re: [U-Boot] [PATCH v4 2/4] USB: host: Add the USB3 host driver
Hi Jean,
>
> Hi Sherry,
>
> On 03/09/2019 14:44, Sherry Sun wrote:
> > Hi Jean,
> >
> >>
> >> On 02/09/2019 13:29, Sherry Sun wrote:
> >>> Hi Vignesh,
> >>>
> Hi Sherry,
>
> [...]
> >> AFAIK, U-Boot does not support runtime switching of USB port to
> >> host from device and vice versa. This is the case for existing
> >> driver like
> DWC3/MUSB etc.
> >> Ideally we would need a role switch driver that unbinds and
> >> rebinds host vs device driver as when required based on U-Boot
> >> cmd or via an API (if required to be done during SPL stage etc).
> > I wonder if we can add two subnodes under the wrapper node as
> > below,
> one bind to usb gadget driver and another bind to usb host driver.
> > So if we want to use usb device, just call the gadget driver, and
> > if want to
> use usb host, just call the host driver. The driver will probe
> correspond
> >> node.
> > I'm not sure if it is feasible, what do you think about it?
> >
> > usbss0: cdns_usb@4104000 {
> > compatible = "ti,j721e-usb";
> > []
> > usb0: usb@601 { /* xhci reg address*/
> > compatible = "cdns,usb3-1.0.1";
> > dr_mode = "host";
> > []
> > }
> > usb1: usb@602 { /* dev reg address*/
> > compatible = "cdns,usb3-1.0.1";
> > dr_mode = "peripheral";
> > []
> > }
> > }
> >
> But this is not how DT would look in kernel. There will be single
> DT node representing both Host and Device node. DT repesentation
> should not be changed based on OS/bootloader, its HW description
> and must
> >> remain same.
> Unbinding host/gadget driver and rebinding with a different role
> should not be hard as the U-Boot core infrastructure exists.
>
> Moreover if xhci reg and dev reg are separated into different nodes
> dont we still need access to OTG register block to switch b/w host
> and
> >> device mode?
> >>> I think we may not need to access OTG register if we add two
> >>> subnodes for
> >> gadget and host.
> >>> But I see a for loop in dwc3_glue_bind() as follows, if there only
> >>> one single
> >> node representing both Host and Device node under usb wrapper node,
> >> why we need this for loop?
> >>> 212 for (node = fdt_first_subnode(fdt, dev_of_offset(parent)); node >
> 0;
> >>> 213 node = fdt_next_subnode(fdt, node)) {
> >> I believe this is a legacy from the code it was inspired from.
> >>
> >> Indeed the loop is not required.
> >>
> >> Like Vignesh I think we must stick with the dt-bindings used by the kernel.
> > Thanks for your reply, I understand now.
> >
> >> BTW we are working on the USB3 support right now that is lacking in our
> tree.
> >> I choose to keep the PHY drivers as close as possible to their linux
> >> version. I'll probably start posting preliminary patches this week.
> >>
> >> If you have the USB3 working in the kernel, can you point to a tree
> >> where I can have a look at the drivers and dt-bindings ?
> > Sure, you can see our downstream kernel code at my github, here is the
> link address:
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub
> .com%2Fsherrysun1%2Flinux-
> imx.gitdata=02%7C01%7Csherry.sun%40nxp.com%7C368ba21c33c048
> fd137208d735199870%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7
> C637036256383501199sdata=y4AB0RpG%2F7Skk34yccpkW9TQsDWpi
> 6OJhtlRWid1DM0%3Dreserved=0.
> > And look forward to seeing your patches in uboot maillist.
>
> It will take some times before I can post on the mailing list because I did
> the
> work on top TI's u-boot. But you can find the patches under
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub
> .com%2Fjjhiblot%2Fu-
> boot%2Ftree%2Fusb3_cleanup_v2data=02%7C01%7Csherry.sun%40n
> xp.com%7C368ba21c33c048fd137208d735199870%7C686ea1d3bc2b4c6fa92c
> d99c5c301635%7C0%7C0%7C637036256383511193sdata=ozU9gYDTO
> %2FkoB4NwU1R2omGw9%2F%2Fyf6MPxBz1%2FtQ8Lgg%3Dreserved=
> 0.
> b.com%2Fjjhiblot%2Fu-
> boot%2Ftree%2Fusb3_cleanup_v2data=02%7C01%7Csherry.sun%40n
> xp.com%7C368ba21c33c048fd137208d735199870%7C686ea1d3bc2b4c6fa92c
> d99c5c301635%7C0%7C0%7C637036256383511193sdata=ozU9gYDTO
> %2FkoB4NwU1R2omGw9%2F%2Fyf6MPxBz1%2FtQ8Lgg%3Dreserved=
> 0>
>
> The DTS bindings are the same as under Linux. The CDNS3 driver is a port of
> the iteration v11 of the linux driver that has just been merged in the linux
> usb
> tree
> (https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.ke
> rnel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fbalbi%2Fusb.git%2Flog
> %2F%3Fh%3Dnextdata=02%7C01%7Csherry.sun%40nxp.com%7C368b
> a21c33c048fd137208d735199870%7C686ea1d3bc2b4c6fa92cd99c5c301635%
>
Re: [U-Boot] [PATCH v4 2/4] USB: host: Add the USB3 host driver
Hi Sherry,
On 03/09/2019 14:44, Sherry Sun wrote:
Hi Jean,
On 02/09/2019 13:29, Sherry Sun wrote:
Hi Vignesh,
Hi Sherry,
[...]
AFAIK, U-Boot does not support runtime switching of USB port to
host from device and vice versa. This is the case for existing
driver like
DWC3/MUSB etc.
Ideally we would need a role switch driver that unbinds and rebinds
host vs device driver as when required based on U-Boot cmd or via
an API (if required to be done during SPL stage etc).
I wonder if we can add two subnodes under the wrapper node as below,
one bind to usb gadget driver and another bind to usb host driver.
So if we want to use usb device, just call the gadget driver, and if
want to
use usb host, just call the host driver. The driver will probe correspond
node.
I'm not sure if it is feasible, what do you think about it?
usbss0: cdns_usb@4104000 {
compatible = "ti,j721e-usb";
[]
usb0: usb@601 { /* xhci reg address*/
compatible = "cdns,usb3-1.0.1";
dr_mode = "host";
[]
}
usb1: usb@602 { /* dev reg address*/
compatible = "cdns,usb3-1.0.1";
dr_mode = "peripheral";
[]
}
}
But this is not how DT would look in kernel. There will be single DT
node representing both Host and Device node. DT repesentation should
not be changed based on OS/bootloader, its HW description and must
remain same.
Unbinding host/gadget driver and rebinding with a different role
should not be hard as the U-Boot core infrastructure exists.
Moreover if xhci reg and dev reg are separated into different nodes
dont we still need access to OTG register block to switch b/w host and
device mode?
I think we may not need to access OTG register if we add two subnodes for
gadget and host.
But I see a for loop in dwc3_glue_bind() as follows, if there only one single
node representing both Host and Device node under usb wrapper node, why
we need this for loop?
212 for (node = fdt_first_subnode(fdt, dev_of_offset(parent)); node > 0;
213 node = fdt_next_subnode(fdt, node)) {
I believe this is a legacy from the code it was inspired from.
Indeed the loop is not required.
Like Vignesh I think we must stick with the dt-bindings used by the kernel.
Thanks for your reply, I understand now.
BTW we are working on the USB3 support right now that is lacking in our tree.
I choose to keep the PHY drivers as close as possible to their linux version.
I'll
probably start posting preliminary patches this week.
If you have the USB3 working in the kernel, can you point to a tree where I
can have a look at the drivers and dt-bindings ?
Sure, you can see our downstream kernel code at my github, here is the link
address: https://github.com/sherrysun1/linux-imx.git.
And look forward to seeing your patches in uboot maillist.
It will take some times before I can post on the mailing list because I
did the work on top TI's u-boot. But you can find the patches under
https://github.com/jjhiblot/u-boot/tree/usb3_cleanup_v2.
The DTS bindings are the same as under Linux. The CDNS3 driver is a port
of the iteration v11 of the linux driver that has just been merged in
the linux usb tree
(https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git/log/?h=next)
Care has been taken to keep the changes with linux minimal to ease the
maintenance: the drivers are quite new and will probably evolve quite a
bit in the next few months.
JJ
Best regards
Sherry sun
JJ
Best regards
Sherry sun
Regards
Vignesh
Best regards
Sherry sun
Regards
Vignesh
Best regards
Sherry sun
JJ
Then when binding the wrapper node, it will hard-coded the two
subnodes
to different driver(gadge/host driver) according to the dr_mode
property
in
nodes.
Do you think I understand it right?
Best regards
Sherry sun
Best regards
Sherry sun
JJ
+ { }
+};
___
U-Boot mailing list
U-Boot@lists.denx.de
https://l
ists.ddata=02%7C01%7Csherry.sun%40nxp.com%7C7d1d76a7124f4c3f
e9
9d08d72d3ddf82%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63
70276
16080089878sdata=70BPVQkomokcNpxsHRD3njfZQvuG51VSD1QKBjQ
o1kA%3
Dreserved=0
enx.de%2Flistinfo%2Fu-
bootdata=02%7C01%7Csherry.sun%40nxp.com%7C35f1d34da1ea4b7
670ba08d72b823e9a%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7
C637025710721487079sdata=Nfk0qWtSViz60wJHAOr2m5tgIwTWjNwI
GrNOxDH6HC0%3Dreserved=0
--
Regards
Vignesh
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v4 2/4] USB: host: Add the USB3 host driver
Hi Jean,
>
>
> On 02/09/2019 13:29, Sherry Sun wrote:
> > Hi Vignesh,
> >
> >> Hi Sherry,
> >>
> >> [...]
> AFAIK, U-Boot does not support runtime switching of USB port to
> host from device and vice versa. This is the case for existing
> driver like
> >> DWC3/MUSB etc.
> Ideally we would need a role switch driver that unbinds and rebinds
> host vs device driver as when required based on U-Boot cmd or via
> an API (if required to be done during SPL stage etc).
> >>> I wonder if we can add two subnodes under the wrapper node as below,
> >> one bind to usb gadget driver and another bind to usb host driver.
> >>> So if we want to use usb device, just call the gadget driver, and if
> >>> want to
> >> use usb host, just call the host driver. The driver will probe correspond
> node.
> >>> I'm not sure if it is feasible, what do you think about it?
> >>>
> >>>usbss0: cdns_usb@4104000 {
> >>>compatible = "ti,j721e-usb";
> >>>[]
> >>>usb0: usb@601 {/* xhci reg address*/
> >>>compatible = "cdns,usb3-1.0.1";
> >>> dr_mode = "host";
> >>> []
> >>> }
> >>>usb1: usb@602 {/* dev reg address*/
> >>>compatible = "cdns,usb3-1.0.1";
> >>> dr_mode = "peripheral";
> >>> []
> >>> }
> >>> }
> >>>
> >> But this is not how DT would look in kernel. There will be single DT
> >> node representing both Host and Device node. DT repesentation should
> >> not be changed based on OS/bootloader, its HW description and must
> remain same.
> >> Unbinding host/gadget driver and rebinding with a different role
> >> should not be hard as the U-Boot core infrastructure exists.
> >>
> >> Moreover if xhci reg and dev reg are separated into different nodes
> >> dont we still need access to OTG register block to switch b/w host and
> device mode?
> > I think we may not need to access OTG register if we add two subnodes for
> gadget and host.
> >
> > But I see a for loop in dwc3_glue_bind() as follows, if there only one
> > single
> node representing both Host and Device node under usb wrapper node, why
> we need this for loop?
> >
> > 212 for (node = fdt_first_subnode(fdt, dev_of_offset(parent)); node > 0;
> > 213 node = fdt_next_subnode(fdt, node)) {
>
> I believe this is a legacy from the code it was inspired from.
>
> Indeed the loop is not required.
>
> Like Vignesh I think we must stick with the dt-bindings used by the kernel.
Thanks for your reply, I understand now.
>
> BTW we are working on the USB3 support right now that is lacking in our tree.
> I choose to keep the PHY drivers as close as possible to their linux version.
> I'll
> probably start posting preliminary patches this week.
>
> If you have the USB3 working in the kernel, can you point to a tree where I
> can have a look at the drivers and dt-bindings ?
Sure, you can see our downstream kernel code at my github, here is the link
address: https://github.com/sherrysun1/linux-imx.git.
And look forward to seeing your patches in uboot maillist.
Best regards
Sherry sun
>
> JJ
>
> >
> > Best regards
> > Sherry sun
> >
> >> Regards
> >> Vignesh
> >>
> >>> Best regards
> >>> Sherry sun
> >>>
> Regards
> Vignesh
>
> > Best regards
> > Sherry sun
> >
> >> JJ
> >>
> Then when binding the wrapper node, it will hard-coded the two
> >> subnodes
> to different driver(gadge/host driver) according to the dr_mode
> property
> >> in
> nodes.
>
> >>> Do you think I understand it right?
> >>> Best regards
> >>> Sherry sun
> >>>
> Best regards
> Sherry sun
>
> > JJ
> >
> >
> >
> >>> + { }
> >>> +};
> ___
> U-Boot mailing list
> U-Boot@lists.denx.de
> https://l
>
> >>
> ists.ddata=02%7C01%7Csherry.sun%40nxp.com%7C7d1d76a7124f4c3f
> e9
> >>
> 9d08d72d3ddf82%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63
> 70276
> >>
> 16080089878sdata=70BPVQkomokcNpxsHRD3njfZQvuG51VSD1QKBjQ
> o1kA%3
> Dreserved=0
> enx.de%2Flistinfo%2Fu-
>
> >>
> bootdata=02%7C01%7Csherry.sun%40nxp.com%7C35f1d34da1ea4b7
> >>
> 670ba08d72b823e9a%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7
> >>
> C637025710721487079sdata=Nfk0qWtSViz60wJHAOr2m5tgIwTWjNwI
> GrNOxDH6HC0%3Dreserved=0
> --
> Regards
> Vignesh
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v4 2/4] USB: host: Add the USB3 host driver
On 02/09/2019 13:29, Sherry Sun wrote:
Hi Vignesh,
Hi Sherry,
[...]
AFAIK, U-Boot does not support runtime switching of USB port to host
from device and vice versa. This is the case for existing driver like
DWC3/MUSB etc.
Ideally we would need a role switch driver that unbinds and rebinds
host vs device driver as when required based on U-Boot cmd or via an
API (if required to be done during SPL stage etc).
I wonder if we can add two subnodes under the wrapper node as below,
one bind to usb gadget driver and another bind to usb host driver.
So if we want to use usb device, just call the gadget driver, and if want to
use usb host, just call the host driver. The driver will probe correspond node.
I'm not sure if it is feasible, what do you think about it?
usbss0: cdns_usb@4104000 {
compatible = "ti,j721e-usb";
[]
usb0: usb@601 { /* xhci reg address*/
compatible = "cdns,usb3-1.0.1";
dr_mode = "host";
[]
}
usb1: usb@602 { /* dev reg address*/
compatible = "cdns,usb3-1.0.1";
dr_mode = "peripheral";
[]
}
}
But this is not how DT would look in kernel. There will be single DT node
representing both Host and Device node. DT repesentation should not be
changed based on OS/bootloader, its HW description and must remain same.
Unbinding host/gadget driver and rebinding with a different role should not
be hard as the U-Boot core infrastructure exists.
Moreover if xhci reg and dev reg are separated into different nodes dont we
still need access to OTG register block to switch b/w host and device mode?
I think we may not need to access OTG register if we add two subnodes for
gadget and host.
But I see a for loop in dwc3_glue_bind() as follows, if there only one single
node representing both Host and Device node under usb wrapper node, why we need
this for loop?
212 for (node = fdt_first_subnode(fdt, dev_of_offset(parent)); node > 0;
213 node = fdt_next_subnode(fdt, node)) {
I believe this is a legacy from the code it was inspired from.
Indeed the loop is not required.
Like Vignesh I think we must stick with the dt-bindings used by the kernel.
BTW we are working on the USB3 support right now that is lacking in our
tree. I choose to keep the PHY drivers as close as possible to their
linux version. I'll probably start posting preliminary patches this week.
If you have the USB3 working in the kernel, can you point to a tree
where I can have a look at the drivers and dt-bindings ?
JJ
Best regards
Sherry sun
Regards
Vignesh
Best regards
Sherry sun
Regards
Vignesh
Best regards
Sherry sun
JJ
Then when binding the wrapper node, it will hard-coded the two
subnodes
to different driver(gadge/host driver) according to the dr_mode
property
in
nodes.
Do you think I understand it right?
Best regards
Sherry sun
Best regards
Sherry sun
JJ
+ { }
+};
___
U-Boot mailing list
U-Boot@lists.denx.de
https://l
ists.ddata=02%7C01%7Csherry.sun%40nxp.com%7C7d1d76a7124f4c3f
e9
9d08d72d3ddf82%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63
70276
16080089878sdata=70BPVQkomokcNpxsHRD3njfZQvuG51VSD1QKBjQ
o1kA%3
Dreserved=0
enx.de%2Flistinfo%2Fu-
bootdata=02%7C01%7Csherry.sun%40nxp.com%7C35f1d34da1ea4b7
670ba08d72b823e9a%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7
C637025710721487079sdata=Nfk0qWtSViz60wJHAOr2m5tgIwTWjNwI
GrNOxDH6HC0%3Dreserved=0
--
Regards
Vignesh
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v4 2/4] USB: host: Add the USB3 host driver
Hi Vignesh,
>
> Hi Sherry,
>
> [...]
> >> AFAIK, U-Boot does not support runtime switching of USB port to host
> >> from device and vice versa. This is the case for existing driver like
> DWC3/MUSB etc.
> >>
> >> Ideally we would need a role switch driver that unbinds and rebinds
> >> host vs device driver as when required based on U-Boot cmd or via an
> >> API (if required to be done during SPL stage etc).
> >
> > I wonder if we can add two subnodes under the wrapper node as below,
> one bind to usb gadget driver and another bind to usb host driver.
> > So if we want to use usb device, just call the gadget driver, and if want to
> use usb host, just call the host driver. The driver will probe correspond
> node.
> > I'm not sure if it is feasible, what do you think about it?
> >
> > usbss0: cdns_usb@4104000 {
> > compatible = "ti,j721e-usb";
> > []
> > usb0: usb@601 { /* xhci reg address*/
> > compatible = "cdns,usb3-1.0.1";
> > dr_mode = "host";
> > []
> > }
> > usb1: usb@602 { /* dev reg address*/
> > compatible = "cdns,usb3-1.0.1";
> > dr_mode = "peripheral";
> > []
> > }
> > }
> >
>
> But this is not how DT would look in kernel. There will be single DT node
> representing both Host and Device node. DT repesentation should not be
> changed based on OS/bootloader, its HW description and must remain same.
> Unbinding host/gadget driver and rebinding with a different role should not
> be hard as the U-Boot core infrastructure exists.
>
> Moreover if xhci reg and dev reg are separated into different nodes dont we
> still need access to OTG register block to switch b/w host and device mode?
I think we may not need to access OTG register if we add two subnodes for
gadget and host.
But I see a for loop in dwc3_glue_bind() as follows, if there only one single
node representing both Host and Device node under usb wrapper node, why we need
this for loop?
212 for (node = fdt_first_subnode(fdt, dev_of_offset(parent)); node > 0;
213 node = fdt_next_subnode(fdt, node)) {
Best regards
Sherry sun
>
> Regards
> Vignesh
>
> > Best regards
> > Sherry sun
> >
> >>
> >> Regards
> >> Vignesh
> >>
> >>> Best regards
> >>> Sherry sun
> >>>
>
> JJ
>
> >> Then when binding the wrapper node, it will hard-coded the two
> subnodes
> >> to different driver(gadge/host driver) according to the dr_mode
> >> property
> in
> >> nodes.
> >>
> > Do you think I understand it right?
>
> >
> > Best regards
> > Sherry sun
> >
> >> Best regards
> >> Sherry sun
> >>
> >>> JJ
> >>>
> >>>
> >>>
> > + { }
> > +};
> >> ___
> >> U-Boot mailing list
> >> U-Boot@lists.denx.de
> >> https://l
> >>
> >>
> ists.ddata=02%7C01%7Csherry.sun%40nxp.com%7C7d1d76a7124f4c3f
> >> e9
> >>
> >>
> 9d08d72d3ddf82%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63
> >> 70276
> >>
> >>
> 16080089878sdata=70BPVQkomokcNpxsHRD3njfZQvuG51VSD1QKBjQ
> >> o1kA%3
> >> Dreserved=0
> >> enx.de%2Flistinfo%2Fu-
> >>
>
> >>
> bootdata=02%7C01%7Csherry.sun%40nxp.com%7C35f1d34da1ea4b7
> >>
>
> >>
> 670ba08d72b823e9a%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7
> >>
>
> >>
> C637025710721487079sdata=Nfk0qWtSViz60wJHAOr2m5tgIwTWjNwI
> >> GrNOxDH6HC0%3Dreserved=0
> >>
> >> --
> >> Regards
> >> Vignesh
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v4 2/4] USB: host: Add the USB3 host driver
Hi Sherry,
[...]
>> AFAIK, U-Boot does not support runtime switching of USB port to host from
>> device and vice versa. This is the case for existing driver like DWC3/MUSB
>> etc.
>>
>> Ideally we would need a role switch driver that unbinds and rebinds host vs
>> device driver as when required based on U-Boot cmd or via an API (if
>> required to be done during SPL stage etc).
>
> I wonder if we can add two subnodes under the wrapper node as below, one bind
> to usb gadget driver and another bind to usb host driver.
> So if we want to use usb device, just call the gadget driver, and if want to
> use usb host, just call the host driver. The driver will probe correspond
> node.
> I'm not sure if it is feasible, what do you think about it?
>
> usbss0: cdns_usb@4104000 {
> compatible = "ti,j721e-usb";
> []
> usb0: usb@601 { /* xhci reg address*/
> compatible = "cdns,usb3-1.0.1";
> dr_mode = "host";
> []
> }
> usb1: usb@602 { /* dev reg address*/
> compatible = "cdns,usb3-1.0.1";
> dr_mode = "peripheral";
> []
> }
> }
>
But this is not how DT would look in kernel. There will be single DT
node representing both Host and Device node. DT repesentation should not
be changed based on OS/bootloader, its HW description and must remain same.
Unbinding host/gadget driver and rebinding with a different role should
not be hard as the U-Boot core infrastructure exists.
Moreover if xhci reg and dev reg are separated into different nodes dont
we still need access to OTG register block to switch b/w host and device
mode?
Regards
Vignesh
> Best regards
> Sherry sun
>
>>
>> Regards
>> Vignesh
>>
>>> Best regards
>>> Sherry sun
>>>
JJ
>> Then when binding the wrapper node, it will hard-coded the two
subnodes
>> to different driver(gadge/host driver) according to the dr_mode
>> property
in
>> nodes.
>>
> Do you think I understand it right?
>
> Best regards
> Sherry sun
>
>> Best regards
>> Sherry sun
>>
>>> JJ
>>>
>>>
>>>
> + { }
> +};
>> ___
>> U-Boot mailing list
>> U-Boot@lists.denx.de
>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fl
>>
>> ists.ddata=02%7C01%7Csherry.sun%40nxp.com%7C7d1d76a7124f4c3f
>> e9
>>
>> 9d08d72d3ddf82%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63
>> 70276
>>
>> 16080089878sdata=70BPVQkomokcNpxsHRD3njfZQvuG51VSD1QKBjQ
>> o1kA%3
>> Dreserved=0
>> enx.de%2Flistinfo%2Fu-
>>
>> bootdata=02%7C01%7Csherry.sun%40nxp.com%7C35f1d34da1ea4b7
>>
>> 670ba08d72b823e9a%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7
>>
>> C637025710721487079sdata=Nfk0qWtSViz60wJHAOr2m5tgIwTWjNwI
>> GrNOxDH6HC0%3Dreserved=0
>>
>> --
>> Regards
>> Vignesh
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v4 2/4] USB: host: Add the USB3 host driver
Hi Vignesh,
>
>
>
> On 30/08/19 3:36 PM, Sherry Sun wrote:
> > Hi Jean,
> >
> >>
> >>
> >> On 28/08/2019 13:50, Sherry Sun wrote:
> >>> Hi Jean,
> >>>
> Hi Jean,
>
> > Hi Marek, Sherry,
> >
> >
> > we keep the cdns3 node for usb gadget driver, then add a usb
> > host node for
> > xhci-imx8 driver in *-uboot.dtsi. so here is no need to change
> > the host driver
> compatible.
> > But the compatible in gadget driver should be changed later.
> We should try avoiding ABI breaks in DT.
> >>> But the cdns3 usb gaget driver and host driver in different
> >>> uclass need two
> > dt nodes to bind with.
> >>> And the compatible of the two node cannot be same.
> >>> So for gadget driver, the compatible may use "cdns,usb3-1.0.0",
> >>> for host
> > driver, the compatible will use "cdns,usb3-1.0.0-host".
> >>> What do you think about it?
> >> CCing Jean, since I think he did solve similar topic for his platform.
> > I've been OOO for a few weeks and didn't look at the whole series.
> >
> > For this particular issue, the solution I used is to let the
> > wrapper do the binding job. The name of the driver to use is
> > hard-coded in the
> wrapper diver.
> > This is done in dwc3_glue_bind().
> Thanks for your suggestions.
>
> So if I want to use the cdns3 usb node as both usb gadget device
> and usb host device, do you mean that I should make the cdns3 usb
> node as a usb wrapper device, and create two subnodes in it.
> >>
> >> I think we should not change the binding to adapt to out driver but
> >> keep the bindings that exist in linux and adapt the u-boot driver
> >>
> >> In the version used by our platform, there is a wrapper around the USB:
> >>
> >> usbss0: cdns_usb@4104000 {
> >> compatible = "ti,j721e-usb";
> >> []
> >> usb0: usb@600 {
> >> compatible = "cdns,usb3-1.0.1";
> >>
> >> The driver selection (host or device) could be done by the wrapper
> >> when it binds its children (same as the dwc3).
> >>
> >> OR
> >>
> >> The "cdns,usb3-1.0.1" could be a dumb driver the role of which would
> >> be only to bind a new driver (host or device) based on "dr_mode". The
> >> binding could be done using the same node, it doesn't have to be a
> subnode.
> >>
> >>
> >> Maybe the second solution will be better, as it would work for
> >> platforms that do not use a wrapper.
> >>
> >
> > I just communicated with Vignesh Raghavendra , and he
> suggest that I should keep this cdns3 driver under Linux kernel and U-Boot in
> sync. And show me your downstream code with v10 of Cadence USB3 kernel
> driver ported to U-Boot. So I decide to follow your way to deal with this
> issue.
> >
> > But I want to ask another question:
> > The two solutions you gave before both make the usb node with
> compatible "cdns,usb3-1.0.1" as a definite device (host or gadget) by its
> dr_mode property. If I want use this usb device works as both host and
> gadget driver, which means I want to change its status runtime, such as I
> want to use this usb device to run both fastboot or usb start command, how
> can we deal with this? .
> >
>
> AFAIK, U-Boot does not support runtime switching of USB port to host from
> device and vice versa. This is the case for existing driver like DWC3/MUSB
> etc.
>
> Ideally we would need a role switch driver that unbinds and rebinds host vs
> device driver as when required based on U-Boot cmd or via an API (if
> required to be done during SPL stage etc).
I wonder if we can add two subnodes under the wrapper node as below, one bind
to usb gadget driver and another bind to usb host driver.
So if we want to use usb device, just call the gadget driver, and if want to
use usb host, just call the host driver. The driver will probe correspond node.
I'm not sure if it is feasible, what do you think about it?
usbss0: cdns_usb@4104000 {
compatible = "ti,j721e-usb";
[]
usb0: usb@601 { /* xhci reg address*/
compatible = "cdns,usb3-1.0.1";
dr_mode = "host";
[]
}
usb1: usb@602 { /* dev reg address*/
compatible = "cdns,usb3-1.0.1";
dr_mode = "peripheral";
[]
}
}
Best regards
Sherry sun
>
> Regards
> Vignesh
>
> > Best regards
> > Sherry sun
> >
> >>
> >> JJ
> >>
> Then when binding the wrapper node, it will hard-coded the two
> >> subnodes
> to different driver(gadge/host driver) according to the dr_mode
> property
> >> in
> nodes.
>
> >>> Do you think I understand it right?
> >>
> >>>
> >>> Best regards
> >>> Sherry sun
> >>>
> Best regards
> Sherry sun
>
> > JJ
> >
> >
> >
> >>> + { }
> >>> +};
> ___
> U-Boot
Re: [U-Boot] [PATCH v4 2/4] USB: host: Add the USB3 host driver
On 30/08/19 3:36 PM, Sherry Sun wrote:
> Hi Jean,
>
>>
>>
>> On 28/08/2019 13:50, Sherry Sun wrote:
>>> Hi Jean,
>>>
Hi Jean,
> Hi Marek, Sherry,
>
>
> we keep the cdns3 node for usb gadget driver, then add a usb
> host node for
> xhci-imx8 driver in *-uboot.dtsi. so here is no need to change
> the host driver
compatible.
> But the compatible in gadget driver should be changed later.
We should try avoiding ABI breaks in DT.
>>> But the cdns3 usb gaget driver and host driver in different uclass
>>> need two
> dt nodes to bind with.
>>> And the compatible of the two node cannot be same.
>>> So for gadget driver, the compatible may use "cdns,usb3-1.0.0",
>>> for host
> driver, the compatible will use "cdns,usb3-1.0.0-host".
>>> What do you think about it?
>> CCing Jean, since I think he did solve similar topic for his platform.
> I've been OOO for a few weeks and didn't look at the whole series.
>
> For this particular issue, the solution I used is to let the wrapper
> do the binding job. The name of the driver to use is hard-coded in
> the
wrapper diver.
> This is done in dwc3_glue_bind().
Thanks for your suggestions.
So if I want to use the cdns3 usb node as both usb gadget device and
usb host device, do you mean that I should make the cdns3 usb node as
a usb wrapper device, and create two subnodes in it.
>>
>> I think we should not change the binding to adapt to out driver but keep the
>> bindings that exist in linux and adapt the u-boot driver
>>
>> In the version used by our platform, there is a wrapper around the USB:
>>
>> usbss0: cdns_usb@4104000 {
>> compatible = "ti,j721e-usb";
>> []
>> usb0: usb@600 {
>> compatible = "cdns,usb3-1.0.1";
>>
>> The driver selection (host or device) could be done by the wrapper when it
>> binds its children (same as the dwc3).
>>
>> OR
>>
>> The "cdns,usb3-1.0.1" could be a dumb driver the role of which would be
>> only to bind a new driver (host or device) based on "dr_mode". The binding
>> could be done using the same node, it doesn't have to be a subnode.
>>
>>
>> Maybe the second solution will be better, as it would work for platforms that
>> do not use a wrapper.
>>
>
> I just communicated with Vignesh Raghavendra , and he
> suggest that I should keep this cdns3 driver under Linux kernel and U-Boot in
> sync. And show me your downstream code with v10 of Cadence USB3 kernel
> driver ported to U-Boot. So I decide to follow your way to deal with this
> issue.
>
> But I want to ask another question:
> The two solutions you gave before both make the usb node with compatible
> "cdns,usb3-1.0.1" as a definite device (host or gadget) by its dr_mode
> property. If I want use this usb device works as both host and gadget driver,
> which means I want to change its status runtime, such as I want to use this
> usb device to run both fastboot or usb start command, how can we deal with
> this? .
>
AFAIK, U-Boot does not support runtime switching of USB port to host
from device and vice versa. This is the case for existing driver like
DWC3/MUSB etc.
Ideally we would need a role switch driver that unbinds and rebinds host
vs device driver as when required based on U-Boot cmd or via an API (if
required to be done during SPL stage etc).
Regards
Vignesh
> Best regards
> Sherry sun
>
>>
>> JJ
>>
Then when binding the wrapper node, it will hard-coded the two
>> subnodes
to different driver(gadge/host driver) according to the dr_mode property
>> in
nodes.
>>> Do you think I understand it right?
>>
>>>
>>> Best regards
>>> Sherry sun
>>>
Best regards
Sherry sun
> JJ
>
>
>
>>> + { }
>>> +};
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.d
enx.de%2Flistinfo%2Fu-
>> bootdata=02%7C01%7Csherry.sun%40nxp.com%7C35f1d34da1ea4b7
>> 670ba08d72b823e9a%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7
>> C637025710721487079sdata=Nfk0qWtSViz60wJHAOr2m5tgIwTWjNwI
GrNOxDH6HC0%3Dreserved=0
--
Regards
Vignesh
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v4 2/4] USB: host: Add the USB3 host driver
Hi Jean,
>
>
> On 28/08/2019 13:50, Sherry Sun wrote:
> > Hi Jean,
> >
> >> Hi Jean,
> >>
> >>> Hi Marek, Sherry,
> >>>
> >>>
> >>> we keep the cdns3 node for usb gadget driver, then add a usb
> >>> host node for
> >>> xhci-imx8 driver in *-uboot.dtsi. so here is no need to change
> >>> the host driver
> >> compatible.
> >>> But the compatible in gadget driver should be changed later.
> >> We should try avoiding ABI breaks in DT.
> > But the cdns3 usb gaget driver and host driver in different uclass
> > need two
> >>> dt nodes to bind with.
> > And the compatible of the two node cannot be same.
> > So for gadget driver, the compatible may use "cdns,usb3-1.0.0",
> > for host
> >>> driver, the compatible will use "cdns,usb3-1.0.0-host".
> > What do you think about it?
> CCing Jean, since I think he did solve similar topic for his platform.
> >>> I've been OOO for a few weeks and didn't look at the whole series.
> >>>
> >>> For this particular issue, the solution I used is to let the wrapper
> >>> do the binding job. The name of the driver to use is hard-coded in
> >>> the
> >> wrapper diver.
> >>> This is done in dwc3_glue_bind().
> >> Thanks for your suggestions.
> >>
> >> So if I want to use the cdns3 usb node as both usb gadget device and
> >> usb host device, do you mean that I should make the cdns3 usb node as
> >> a usb wrapper device, and create two subnodes in it.
>
> I think we should not change the binding to adapt to out driver but keep the
> bindings that exist in linux and adapt the u-boot driver
>
> In the version used by our platform, there is a wrapper around the USB:
>
> usbss0: cdns_usb@4104000 {
> compatible = "ti,j721e-usb";
> []
> usb0: usb@600 {
> compatible = "cdns,usb3-1.0.1";
>
> The driver selection (host or device) could be done by the wrapper when it
> binds its children (same as the dwc3).
>
> OR
>
> The "cdns,usb3-1.0.1" could be a dumb driver the role of which would be
> only to bind a new driver (host or device) based on "dr_mode". The binding
> could be done using the same node, it doesn't have to be a subnode.
>
>
> Maybe the second solution will be better, as it would work for platforms that
> do not use a wrapper.
>
I just communicated with Vignesh Raghavendra , and he suggest
that I should keep this cdns3 driver under Linux kernel and U-Boot in sync. And
show me your downstream code with v10 of Cadence USB3 kernel driver ported to
U-Boot. So I decide to follow your way to deal with this issue.
But I want to ask another question:
The two solutions you gave before both make the usb node with compatible
"cdns,usb3-1.0.1" as a definite device (host or gadget) by its dr_mode
property. If I want use this usb device works as both host and gadget driver,
which means I want to change its status runtime, such as I want to use this usb
device to run both fastboot or usb start command, how can we deal with this? .
Best regards
Sherry sun
>
> JJ
>
> >> Then when binding the wrapper node, it will hard-coded the two
> subnodes
> >> to different driver(gadge/host driver) according to the dr_mode property
> in
> >> nodes.
> >>
> > Do you think I understand it right?
>
> >
> > Best regards
> > Sherry sun
> >
> >> Best regards
> >> Sherry sun
> >>
> >>> JJ
> >>>
> >>>
> >>>
> > + { }
> > +};
> >> ___
> >> U-Boot mailing list
> >> U-Boot@lists.denx.de
> >> https://lists.d
> >> enx.de%2Flistinfo%2Fu-
> >>
> bootdata=02%7C01%7Csherry.sun%40nxp.com%7C35f1d34da1ea4b7
> >>
> 670ba08d72b823e9a%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7
> >>
> C637025710721487079sdata=Nfk0qWtSViz60wJHAOr2m5tgIwTWjNwI
> >> GrNOxDH6HC0%3Dreserved=0
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v4 2/4] USB: host: Add the USB3 host driver
On 28/08/2019 13:50, Sherry Sun wrote:
Hi Jean,
Hi Jean,
Hi Marek, Sherry,
we keep the cdns3 node for usb gadget driver, then add a usb host
node for
xhci-imx8 driver in *-uboot.dtsi. so here is no need to change
the host driver
compatible.
But the compatible in gadget driver should be changed later.
We should try avoiding ABI breaks in DT.
But the cdns3 usb gaget driver and host driver in different uclass
need two
dt nodes to bind with.
And the compatible of the two node cannot be same.
So for gadget driver, the compatible may use "cdns,usb3-1.0.0", for
host
driver, the compatible will use "cdns,usb3-1.0.0-host".
What do you think about it?
CCing Jean, since I think he did solve similar topic for his platform.
I've been OOO for a few weeks and didn't look at the whole series.
For this particular issue, the solution I used is to let the wrapper
do the binding job. The name of the driver to use is hard-coded in the
wrapper diver.
This is done in dwc3_glue_bind().
Thanks for your suggestions.
So if I want to use the cdns3 usb node as both usb gadget device and usb
host device, do you mean that I should make the cdns3 usb node as a usb
wrapper device, and create two subnodes in it.
I think we should not change the binding to adapt to out driver but keep
the bindings that exist in linux and adapt the u-boot driver
In the version used by our platform, there is a wrapper around the USB:
usbss0: cdns_usb@4104000 {
compatible = "ti,j721e-usb";
[]
usb0: usb@600 {
compatible = "cdns,usb3-1.0.1";
The driver selection (host or device) could be done by the wrapper when
it binds its children (same as the dwc3).
OR
The "cdns,usb3-1.0.1" could be a dumb driver the role of which would be
only to bind a new driver (host or device) based on "dr_mode". The
binding could be done using the same node, it doesn't have to be a subnode.
Maybe the second solution will be better, as it would work for platforms
that do not use a wrapper.
JJ
Then when binding the wrapper node, it will hard-coded the two subnodes
to different driver(gadge/host driver) according to the dr_mode property in
nodes.
Do you think I understand it right?
Best regards
Sherry sun
Best regards
Sherry sun
JJ
+ { }
+};
___
U-Boot mailing list
U-Boot@lists.denx.de
https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.d
enx.de%2Flistinfo%2Fu-
bootdata=02%7C01%7Csherry.sun%40nxp.com%7C35f1d34da1ea4b7
670ba08d72b823e9a%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7
C637025710721487079sdata=Nfk0qWtSViz60wJHAOr2m5tgIwTWjNwI
GrNOxDH6HC0%3Dreserved=0
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v4 2/4] USB: host: Add the USB3 host driver
Hi Jean,
>
> Hi Jean,
>
> >
> > Hi Marek, Sherry,
> >
> >
> > we keep the cdns3 node for usb gadget driver, then add a usb host
> > node for
> > xhci-imx8 driver in *-uboot.dtsi. so here is no need to change
> > the host driver
> > >>> compatible.
> > But the compatible in gadget driver should be changed later.
> > >>> We should try avoiding ABI breaks in DT.
> > >> But the cdns3 usb gaget driver and host driver in different uclass
> > >> need two
> > dt nodes to bind with.
> > >> And the compatible of the two node cannot be same.
> > >> So for gadget driver, the compatible may use "cdns,usb3-1.0.0", for
> > >> host
> > driver, the compatible will use "cdns,usb3-1.0.0-host".
> > >> What do you think about it?
> > > CCing Jean, since I think he did solve similar topic for his platform.
> >
> > I've been OOO for a few weeks and didn't look at the whole series.
> >
> > For this particular issue, the solution I used is to let the wrapper
> > do the binding job. The name of the driver to use is hard-coded in the
> wrapper diver.
> >
> > This is done in dwc3_glue_bind().
>
> Thanks for your suggestions.
>
> So if I want to use the cdns3 usb node as both usb gadget device and usb
> host device, do you mean that I should make the cdns3 usb node as a usb
> wrapper device, and create two subnodes in it.
> Then when binding the wrapper node, it will hard-coded the two subnodes
> to different driver(gadge/host driver) according to the dr_mode property in
> nodes.
>
Do you think I understand it right?
Best regards
Sherry sun
> Best regards
> Sherry sun
>
> >
> > JJ
> >
> >
> >
> > >
> > >> +{ }
> > >> +};
> > >
> ___
> U-Boot mailing list
> U-Boot@lists.denx.de
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.d
> enx.de%2Flistinfo%2Fu-
> bootdata=02%7C01%7Csherry.sun%40nxp.com%7C35f1d34da1ea4b7
> 670ba08d72b823e9a%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7
> C637025710721487079sdata=Nfk0qWtSViz60wJHAOr2m5tgIwTWjNwI
> GrNOxDH6HC0%3Dreserved=0
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v4 2/4] USB: host: Add the USB3 host driver
Hi Jean,
>
> Hi Marek, Sherry,
>
>
> we keep the cdns3 node for usb gadget driver, then add a usb host
> node for
> xhci-imx8 driver in *-uboot.dtsi. so here is no need to change the
> host driver
> >>> compatible.
> But the compatible in gadget driver should be changed later.
> >>> We should try avoiding ABI breaks in DT.
> >> But the cdns3 usb gaget driver and host driver in different uclass need two
> dt nodes to bind with.
> >> And the compatible of the two node cannot be same.
> >> So for gadget driver, the compatible may use "cdns,usb3-1.0.0", for host
> driver, the compatible will use "cdns,usb3-1.0.0-host".
> >> What do you think about it?
> > CCing Jean, since I think he did solve similar topic for his platform.
>
> I've been OOO for a few weeks and didn't look at the whole series.
>
> For this particular issue, the solution I used is to let the wrapper do the
> binding job. The name of the driver to use is hard-coded in the wrapper diver.
>
> This is done in dwc3_glue_bind().
Thanks for your suggestions.
So if I want to use the cdns3 usb node as both usb gadget device and usb host
device,
do you mean that I should make the cdns3 usb node as a usb wrapper device, and
create two subnodes in it.
Then when binding the wrapper node, it will hard-coded the two subnodes to
different driver(gadge/host driver)
according to the dr_mode property in nodes.
Best regards
Sherry sun
>
> JJ
>
>
>
> >
> >> + { }
> >> +};
> >
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v4 2/4] USB: host: Add the USB3 host driver
Hi Marek, Sherry,
we keep the cdns3 node for usb gadget driver, then add a usb host node
for
xhci-imx8 driver in *-uboot.dtsi. so here is no need to change the host driver
compatible.
But the compatible in gadget driver should be changed later.
We should try avoiding ABI breaks in DT.
But the cdns3 usb gaget driver and host driver in different uclass need two dt
nodes to bind with.
And the compatible of the two node cannot be same.
So for gadget driver, the compatible may use "cdns,usb3-1.0.0", for host driver, the
compatible will use "cdns,usb3-1.0.0-host".
What do you think about it?
CCing Jean, since I think he did solve similar topic for his platform.
I've been OOO for a few weeks and didn't look at the whole series.
For this particular issue, the solution I used is to let the wrapper do
the binding job. The name of the driver to use is hard-coded in the
wrapper diver.
This is done in dwc3_glue_bind().
JJ
+ { }
+};
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v4 2/4] USB: host: Add the USB3 host driver
Hi Marek,
>
> On 8/20/19 5:42 PM, Sherry Sun wrote:
> [...]
>
> >>> +U_BOOT_DRIVER(xhci_imx8) = {
> >>> + .name = "xhci_imx8",
> >>> + .id = UCLASS_USB,
> >>> + .of_match = xhci_usb_ids,
> >>> + .probe = xhci_imx8_probe,
> >>> + .remove = xhci_imx8_remove,
> >>> + .ops= _usb_ops,
> >>> + .platdata_auto_alloc_size = sizeof(struct usb_platdata),
> >>> + .priv_auto_alloc_size = sizeof(struct xhci_ctrl),
> >>> + .flags = DM_FLAG_ALLOC_PRIV_DMA,
> >>
> >> I think you also need DM_FLAG_OS_PREPARE to trigger .remove
> >> before booting Linux, but I might be wrong. Please check that.
> >
> > When use usb stop command to stop the usb host driver,
> > device_remove(bus, DM_REMOVE_NORMAL) is called by usb_stop(), so
> > normally we don’t need the DM_FLAG_OS_PREPARE flag.
> > But considering some special circumstances, maybe add this flag is
> helpful.
>
> I'm not talking about "usb stop", I am talking about booting Linux.
> That's when then .remove should be called to tear down the driver. Is it?
> >>>
> >>> I know what you mean. The only function of DM_FLAG_OS_PREPARE is to
> >>> call .remove before booting kernel, to make sure the device is
> >>> removed.
> >>> But normally, the host device is removed every time after we use it.
> >>
> >> How so ? The controller is left running until you call .remove()
> >
> > We usually start the host driver in uboot by using the usb start command,
> right?
> > And we will call usb stop command to finish the usb start command.
> > .remove will be called in usb_stop(). So I think the controller has already
> been teared before booting kernel.
>
> That is only if you assume that every user will manually call usb stop,
> always,
> and never fail to do so. And also every piece of code which might have inited
> the USB will also tear down the controller. That is not realistic.
>
> Hence. you should call the .remove() before boot to tear down the controller.
Okay, you are right, I will add this flag, thanks.
Best regards
Sherry sun
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v4 2/4] USB: host: Add the USB3 host driver
On 8/20/19 5:42 PM, Sherry Sun wrote:
[...]
>>> +U_BOOT_DRIVER(xhci_imx8) = {
>>> + .name = "xhci_imx8",
>>> + .id = UCLASS_USB,
>>> + .of_match = xhci_usb_ids,
>>> + .probe = xhci_imx8_probe,
>>> + .remove = xhci_imx8_remove,
>>> + .ops= _usb_ops,
>>> + .platdata_auto_alloc_size = sizeof(struct usb_platdata),
>>> + .priv_auto_alloc_size = sizeof(struct xhci_ctrl),
>>> + .flags = DM_FLAG_ALLOC_PRIV_DMA,
>>
>> I think you also need DM_FLAG_OS_PREPARE to trigger .remove before
>> booting Linux, but I might be wrong. Please check that.
>
> When use usb stop command to stop the usb host driver,
> device_remove(bus, DM_REMOVE_NORMAL) is called by usb_stop(), so
> normally we don’t need the DM_FLAG_OS_PREPARE flag.
> But considering some special circumstances, maybe add this flag is
> helpful.
I'm not talking about "usb stop", I am talking about booting Linux.
That's when then .remove should be called to tear down the driver. Is it?
>>>
>>> I know what you mean. The only function of DM_FLAG_OS_PREPARE is to
>>> call .remove before booting kernel, to make sure the device is
>>> removed.
>>> But normally, the host device is removed every time after we use it.
>>
>> How so ? The controller is left running until you call .remove()
>
> We usually start the host driver in uboot by using the usb start command,
> right?
> And we will call usb stop command to finish the usb start command.
> .remove will be called in usb_stop(). So I think the controller has already
> been teared before booting kernel.
That is only if you assume that every user will manually call usb stop,
always, and never fail to do so. And also every piece of code which
might have inited the USB will also tear down the controller. That is
not realistic.
Hence. you should call the .remove() before boot to tear down the
controller.
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v4 2/4] USB: host: Add the USB3 host driver
Hi Marek,
>
> On 8/20/19 5:24 PM, Sherry Sun wrote:
> > Hi Marek,
>
> Hi,
>
> >> On 8/20/19 10:31 AM, Sherry Sun wrote:
> >>> Hi Marek,
> >>>
>
> On 8/19/19 8:10 AM, Sherry Sun wrote:
> > Add the USB3 host driver for NXP imx8 platform, and the cadence IP
> > is in it. The USB3 host driver support DM mode, it will probe USB3
> > host node in dts.
> >
> > Signed-off-by: Sherry Sun
>
> [...]
>
> > +static void xhci_imx8_get_reg_addr(struct udevice *dev) {
> > + imx8_data.usb3_ctrl_base =
> > + (void __iomem *)devfdt_get_addr_index(dev, 0);
> > + imx8_data.usb3_core_base =
> > + (void __iomem *)devfdt_get_addr_index(dev, 4); }
>
> Inline this.
>
> Also, look at drivers/mtd/renesas_rpc_hf.c to handle both 32bit and
> 64bit DTs with multiple "reg" entries.
>
> >>>
> >>> Actually, I don't think it's needed to change the way getting reg in dts.
> >>> For two reasons:
> >>> 1. This xhci-imx8 driver is only target on imx8 platform which all
> >>> use 64bit
> >> dts.
> >>> 2. devfdt_get_addr_index() can handle both 32/64 bits dts too.
> >>
> >> By hard-coding this "4" offset, you assume the DT is using 64bit
> >> addressing, that's wrong, so please fix it. The iMX8 does support
> >> aarch32, right ? So someone might pass in 32bit DT and this would break.
> >
> > Sorry, maybe there are some misunderstandings in the code.
> > There five reg elements in dts node, which are none_core_regs/ xhci_regs/
> dev_regs/ phy_regs/ otg_regs.
> > devfdt_get_addr_index(dev, 4) only want to get the fifth reg value, instead
> > of
> meanning 64bit address.
> >
> > 586 usbotg3: usb3@5b11 {
> > 587 compatible = "cdns,usb3";
> > 588 reg = <0x0 0x5B11 0x0 0x1>,
> > 589 <0x0 0x5B13 0x0 0x1>,
> > 590 <0x0 0x5B14 0x0 0x1>,
> > 591 <0x0 0x5B16 0x0 0x4>,
> > 592 <0x0 0x5B12 0x0 0x1>;
>
> Look the registers up by name then, each entry should have it's own name in
> the DT. git grep for "reg-names" .
>
Okay, I understand, will change it.
> [...]
>
> >>> And note that in uboot, one dts node can only bind with one driver,
> >>> so we keep the cdns3 node for usb gadget driver, then add a usb host
> >>> node for
> >>> xhci-imx8 driver in *-uboot.dtsi. so here is no need to change the
> >>> host driver
> >> compatible.
> >>> But the compatible in gadget driver should be changed later.
> >>
> >> We should try avoiding ABI breaks in DT.
> >
> > But the cdns3 usb gaget driver and host driver in different uclass need two
> > dt
> nodes to bind with.
> > And the compatible of the two node cannot be same.
> > So for gadget driver, the compatible may use "cdns,usb3-1.0.0", for host
> driver, the compatible will use "cdns,usb3-1.0.0-host".
> > What do you think about it?
>
> CCing Jean, since I think he did solve similar topic for his platform.
>
Hi, Jean, may you please give me some advice about this problem? Thanks.
> > + { }
> > +};
> > +
> > +U_BOOT_DRIVER(xhci_imx8) = {
> > + .name = "xhci_imx8",
> > + .id = UCLASS_USB,
> > + .of_match = xhci_usb_ids,
> > + .probe = xhci_imx8_probe,
> > + .remove = xhci_imx8_remove,
> > + .ops= _usb_ops,
> > + .platdata_auto_alloc_size = sizeof(struct usb_platdata),
> > + .priv_auto_alloc_size = sizeof(struct xhci_ctrl),
> > + .flags = DM_FLAG_ALLOC_PRIV_DMA,
>
> I think you also need DM_FLAG_OS_PREPARE to trigger .remove before
> booting Linux, but I might be wrong. Please check that.
> >>>
> >>> When use usb stop command to stop the usb host driver,
> >>> device_remove(bus, DM_REMOVE_NORMAL) is called by usb_stop(), so
> >>> normally we don’t need the DM_FLAG_OS_PREPARE flag.
> >>> But considering some special circumstances, maybe add this flag is
> >>> helpful.
> >>
> >> I'm not talking about "usb stop", I am talking about booting Linux.
> >> That's when then .remove should be called to tear down the driver. Is it?
> >
> > I know what you mean. The only function of DM_FLAG_OS_PREPARE is to
> > call .remove before booting kernel, to make sure the device is
> > removed.
> > But normally, the host device is removed every time after we use it.
>
> How so ? The controller is left running until you call .remove()
We usually start the host driver in uboot by using the usb start command, right?
And we will call usb stop command to finish the usb start command.
.remove will be called in usb_stop(). So I think the controller has already
been teared before booting kernel.
>
> > So even the flag is set, the .remove won't be called again before
> > booting kernel because the device is checked to be removed already.
> > Add the flag just make sure the device will be removed before booting
> > kernel.
> > Do you
Re: [U-Boot] [PATCH v4 2/4] USB: host: Add the USB3 host driver
On 8/20/19 5:24 PM, Sherry Sun wrote:
> Hi Marek,
Hi,
>> On 8/20/19 10:31 AM, Sherry Sun wrote:
>>> Hi Marek,
>>>
On 8/19/19 8:10 AM, Sherry Sun wrote:
> Add the USB3 host driver for NXP imx8 platform, and the cadence IP
> is in it. The USB3 host driver support DM mode, it will probe USB3
> host node in dts.
>
> Signed-off-by: Sherry Sun
[...]
> +static void xhci_imx8_get_reg_addr(struct udevice *dev) {
> + imx8_data.usb3_ctrl_base =
> + (void __iomem *)devfdt_get_addr_index(dev, 0);
> + imx8_data.usb3_core_base =
> + (void __iomem *)devfdt_get_addr_index(dev, 4); }
Inline this.
Also, look at drivers/mtd/renesas_rpc_hf.c to handle both 32bit and
64bit DTs with multiple "reg" entries.
>>>
>>> Actually, I don't think it's needed to change the way getting reg in dts.
>>> For two reasons:
>>> 1. This xhci-imx8 driver is only target on imx8 platform which all use 64bit
>> dts.
>>> 2. devfdt_get_addr_index() can handle both 32/64 bits dts too.
>>
>> By hard-coding this "4" offset, you assume the DT is using 64bit addressing,
>> that's wrong, so please fix it. The iMX8 does support aarch32, right ? So
>> someone might pass in 32bit DT and this would break.
>
> Sorry, maybe there are some misunderstandings in the code.
> There five reg elements in dts node, which are none_core_regs/ xhci_regs/
> dev_regs/ phy_regs/ otg_regs.
> devfdt_get_addr_index(dev, 4) only want to get the fifth reg value, instead
> of meanning 64bit address.
>
> 586 usbotg3: usb3@5b11 {
> 587 compatible = "cdns,usb3";
> 588 reg = <0x0 0x5B11 0x0 0x1>,
> 589 <0x0 0x5B13 0x0 0x1>,
> 590 <0x0 0x5B14 0x0 0x1>,
> 591 <0x0 0x5B16 0x0 0x4>,
> 592 <0x0 0x5B12 0x0 0x1>;
Look the registers up by name then, each entry should have it's own name
in the DT. git grep for "reg-names" .
[...]
>>> And note that in uboot, one dts node can only bind with one driver, so
>>> we keep the cdns3 node for usb gadget driver, then add a usb host node
>>> for
>>> xhci-imx8 driver in *-uboot.dtsi. so here is no need to change the host
>>> driver
>> compatible.
>>> But the compatible in gadget driver should be changed later.
>>
>> We should try avoiding ABI breaks in DT.
>
> But the cdns3 usb gaget driver and host driver in different uclass need two
> dt nodes to bind with.
> And the compatible of the two node cannot be same.
> So for gadget driver, the compatible may use "cdns,usb3-1.0.0", for host
> driver, the compatible will use "cdns,usb3-1.0.0-host".
> What do you think about it?
CCing Jean, since I think he did solve similar topic for his platform.
> + { }
> +};
> +
> +U_BOOT_DRIVER(xhci_imx8) = {
> + .name = "xhci_imx8",
> + .id = UCLASS_USB,
> + .of_match = xhci_usb_ids,
> + .probe = xhci_imx8_probe,
> + .remove = xhci_imx8_remove,
> + .ops= _usb_ops,
> + .platdata_auto_alloc_size = sizeof(struct usb_platdata),
> + .priv_auto_alloc_size = sizeof(struct xhci_ctrl),
> + .flags = DM_FLAG_ALLOC_PRIV_DMA,
I think you also need DM_FLAG_OS_PREPARE to trigger .remove before
booting Linux, but I might be wrong. Please check that.
>>>
>>> When use usb stop command to stop the usb host driver,
>>> device_remove(bus, DM_REMOVE_NORMAL) is called by usb_stop(), so
>>> normally we don’t need the DM_FLAG_OS_PREPARE flag.
>>> But considering some special circumstances, maybe add this flag is helpful.
>>
>> I'm not talking about "usb stop", I am talking about booting Linux.
>> That's when then .remove should be called to tear down the driver. Is it?
>
> I know what you mean. The only function of DM_FLAG_OS_PREPARE
> is to call .remove before booting kernel, to make sure the device is
> removed.
> But normally, the host device is removed every time after we use it.
How so ? The controller is left running until you call .remove()
> So even the flag is set, the .remove won't be called again before booting
> kernel
> because the device is checked to be removed already.
> Add the flag just make sure the device will be removed before booting kernel.
> Do you think so?
You want to tear the controller down before booting kernel, e.g. to
prevent it from doing some rogue DMA into memory, while the kernel
starts up. This might mess the system up.
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v4 2/4] USB: host: Add the USB3 host driver
Hi Marek,
>
> On 8/20/19 10:31 AM, Sherry Sun wrote:
> > Hi Marek,
> >
> >>
> >> On 8/19/19 8:10 AM, Sherry Sun wrote:
> >>> Add the USB3 host driver for NXP imx8 platform, and the cadence IP
> >>> is in it. The USB3 host driver support DM mode, it will probe USB3
> >>> host node in dts.
> >>>
> >>> Signed-off-by: Sherry Sun
> >>
> >> [...]
> >>
> >>> +static void xhci_imx8_get_reg_addr(struct udevice *dev) {
> >>> + imx8_data.usb3_ctrl_base =
> >>> + (void __iomem *)devfdt_get_addr_index(dev, 0);
> >>> + imx8_data.usb3_core_base =
> >>> + (void __iomem *)devfdt_get_addr_index(dev, 4); }
> >>
> >> Inline this.
> >>
> >> Also, look at drivers/mtd/renesas_rpc_hf.c to handle both 32bit and
> >> 64bit DTs with multiple "reg" entries.
> >>
> >
> > Actually, I don't think it's needed to change the way getting reg in dts.
> > For two reasons:
> > 1. This xhci-imx8 driver is only target on imx8 platform which all use 64bit
> dts.
> > 2. devfdt_get_addr_index() can handle both 32/64 bits dts too.
>
> By hard-coding this "4" offset, you assume the DT is using 64bit addressing,
> that's wrong, so please fix it. The iMX8 does support aarch32, right ? So
> someone might pass in 32bit DT and this would break.
Sorry, maybe there are some misunderstandings in the code.
There five reg elements in dts node, which are none_core_regs/ xhci_regs/
dev_regs/ phy_regs/ otg_regs.
devfdt_get_addr_index(dev, 4) only want to get the fifth reg value, instead of
meanning 64bit address.
586 usbotg3: usb3@5b11 {
587 compatible = "cdns,usb3";
588 reg = <0x0 0x5B11 0x0 0x1>,
589 <0x0 0x5B13 0x0 0x1>,
590 <0x0 0x5B14 0x0 0x1>,
591 <0x0 0x5B16 0x0 0x4>,
592 <0x0 0x5B12 0x0 0x1>;
>
> >> [...]
> >>
> >>> +static const struct udevice_id xhci_usb_ids[] = {
> >>> + { .compatible = "cdns,usb3-host", },
> >>
> >> https://lore.ker
> >>
> nel.org%2Fpatchwork%2Fpatch%2F1059917%2Fdata=02%7C01%7Cshe
> >>
> rry.sun%40nxp.com%7C70e116ce152a4060dbd508d724987b54%7C686ea1d
> >>
> 3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637018109631327748sd
> >>
> ata=GzX5KNQ8m0CCeBa9tPSV%2BOEwwuSsxDdAiJd1T6or8P0%3Dreser
> >> ved=0 would suggest that
> >> cdns,usb3-1.0.0 is the compatible. But I might be wrong.
> >
> > This patch has not been accepted by the kernel, right?
> > So I think maybe we can do this change to synchronize with kernel
> > after the cdns3 kernel patch been accepted.
>
> Or, we can start with at least reasonably OK DT compatible right away to
> reasonably avoid ABI breakage.
Sure, will change the compatible from "cdns,usb3" to "cdns,usb3-1.0.0".
>
> > And note that in uboot, one dts node can only bind with one driver, so
> > we keep the cdns3 node for usb gadget driver, then add a usb host node
> > for
> > xhci-imx8 driver in *-uboot.dtsi. so here is no need to change the host
> > driver
> compatible.
> > But the compatible in gadget driver should be changed later.
>
> We should try avoiding ABI breaks in DT.
But the cdns3 usb gaget driver and host driver in different uclass need two dt
nodes to bind with.
And the compatible of the two node cannot be same.
So for gadget driver, the compatible may use "cdns,usb3-1.0.0", for host
driver, the compatible will use "cdns,usb3-1.0.0-host".
What do you think about it?
>
> >>> + { }
> >>> +};
> >>> +
> >>> +U_BOOT_DRIVER(xhci_imx8) = {
> >>> + .name = "xhci_imx8",
> >>> + .id = UCLASS_USB,
> >>> + .of_match = xhci_usb_ids,
> >>> + .probe = xhci_imx8_probe,
> >>> + .remove = xhci_imx8_remove,
> >>> + .ops= _usb_ops,
> >>> + .platdata_auto_alloc_size = sizeof(struct usb_platdata),
> >>> + .priv_auto_alloc_size = sizeof(struct xhci_ctrl),
> >>> + .flags = DM_FLAG_ALLOC_PRIV_DMA,
> >>
> >> I think you also need DM_FLAG_OS_PREPARE to trigger .remove before
> >> booting Linux, but I might be wrong. Please check that.
> >
> > When use usb stop command to stop the usb host driver,
> > device_remove(bus, DM_REMOVE_NORMAL) is called by usb_stop(), so
> > normally we don’t need the DM_FLAG_OS_PREPARE flag.
> > But considering some special circumstances, maybe add this flag is helpful.
>
> I'm not talking about "usb stop", I am talking about booting Linux.
> That's when then .remove should be called to tear down the driver. Is it?
I know what you mean. The only function of DM_FLAG_OS_PREPARE
is to call .remove before booting kernel, to make sure the device is
removed.
But normally, the host device is removed every time after we use it.
So even the flag is set, the .remove won't be called again before booting kernel
because the device is checked to be removed already.
Add the flag just make sure the device will be removed before booting kernel.
Do you think so?
Best regard
Sherry sun
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v4 2/4] USB: host: Add the USB3 host driver
On 8/20/19 10:31 AM, Sherry Sun wrote:
> Hi Marek,
>
>>
>> On 8/19/19 8:10 AM, Sherry Sun wrote:
>>> Add the USB3 host driver for NXP imx8 platform, and the cadence IP is
>>> in it. The USB3 host driver support DM mode, it will probe USB3 host
>>> node in dts.
>>>
>>> Signed-off-by: Sherry Sun
>>
>> [...]
>>
>>> +static void xhci_imx8_get_reg_addr(struct udevice *dev) {
>>> + imx8_data.usb3_ctrl_base =
>>> + (void __iomem *)devfdt_get_addr_index(dev, 0);
>>> + imx8_data.usb3_core_base =
>>> + (void __iomem *)devfdt_get_addr_index(dev, 4); }
>>
>> Inline this.
>>
>> Also, look at drivers/mtd/renesas_rpc_hf.c to handle both 32bit and 64bit DTs
>> with multiple "reg" entries.
>>
>
> Actually, I don't think it's needed to change the way getting reg in dts.
> For two reasons:
> 1. This xhci-imx8 driver is only target on imx8 platform which all use 64bit
> dts.
> 2. devfdt_get_addr_index() can handle both 32/64 bits dts too.
By hard-coding this "4" offset, you assume the DT is using 64bit
addressing, that's wrong, so please fix it. The iMX8 does support
aarch32, right ? So someone might pass in 32bit DT and this would break.
>> [...]
>>
>>> +static const struct udevice_id xhci_usb_ids[] = {
>>> + { .compatible = "cdns,usb3-host", },
>>
>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.ker
>> nel.org%2Fpatchwork%2Fpatch%2F1059917%2Fdata=02%7C01%7Cshe
>> rry.sun%40nxp.com%7C70e116ce152a4060dbd508d724987b54%7C686ea1d
>> 3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637018109631327748sd
>> ata=GzX5KNQ8m0CCeBa9tPSV%2BOEwwuSsxDdAiJd1T6or8P0%3Dreser
>> ved=0 would suggest that
>> cdns,usb3-1.0.0 is the compatible. But I might be wrong.
>
> This patch has not been accepted by the kernel, right?
> So I think maybe we can do this change to synchronize with kernel after the
> cdns3
> kernel patch been accepted.
Or, we can start with at least reasonably OK DT compatible right away to
reasonably avoid ABI breakage.
> And note that in uboot, one dts node can only bind with
> one driver, so we keep the cdns3 node for usb gadget driver, then add a usb
> host node for
> xhci-imx8 driver in *-uboot.dtsi. so here is no need to change the host
> driver compatible.
> But the compatible in gadget driver should be changed later.
We should try avoiding ABI breaks in DT.
>>> + { }
>>> +};
>>> +
>>> +U_BOOT_DRIVER(xhci_imx8) = {
>>> + .name = "xhci_imx8",
>>> + .id = UCLASS_USB,
>>> + .of_match = xhci_usb_ids,
>>> + .probe = xhci_imx8_probe,
>>> + .remove = xhci_imx8_remove,
>>> + .ops= _usb_ops,
>>> + .platdata_auto_alloc_size = sizeof(struct usb_platdata),
>>> + .priv_auto_alloc_size = sizeof(struct xhci_ctrl),
>>> + .flags = DM_FLAG_ALLOC_PRIV_DMA,
>>
>> I think you also need DM_FLAG_OS_PREPARE to trigger .remove before booting
>> Linux, but I might be wrong. Please check that.
>
> When use usb stop command to stop the usb host driver,
> device_remove(bus, DM_REMOVE_NORMAL) is called by usb_stop(),
> so normally we don’t need the DM_FLAG_OS_PREPARE flag.
> But considering some special circumstances, maybe add this flag is helpful.
I'm not talking about "usb stop", I am talking about booting Linux.
That's when then .remove should be called to tear down the driver. Is it?
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v4 2/4] USB: host: Add the USB3 host driver
Hi Marek,
>
> On 8/19/19 8:10 AM, Sherry Sun wrote:
> > Add the USB3 host driver for NXP imx8 platform, and the cadence IP is
> > in it. The USB3 host driver support DM mode, it will probe USB3 host
> > node in dts.
> >
> > Signed-off-by: Sherry Sun
>
> [...]
>
> > +static void xhci_imx8_get_reg_addr(struct udevice *dev) {
> > + imx8_data.usb3_ctrl_base =
> > + (void __iomem *)devfdt_get_addr_index(dev, 0);
> > + imx8_data.usb3_core_base =
> > + (void __iomem *)devfdt_get_addr_index(dev, 4); }
>
> Inline this.
>
> Also, look at drivers/mtd/renesas_rpc_hf.c to handle both 32bit and 64bit DTs
> with multiple "reg" entries.
>
Actually, I don't think it's needed to change the way getting reg in dts.
For two reasons:
1. This xhci-imx8 driver is only target on imx8 platform which all use 64bit
dts.
2. devfdt_get_addr_index() can handle both 32/64 bits dts too.
> [...]
>
> > +static const struct udevice_id xhci_usb_ids[] = {
> > + { .compatible = "cdns,usb3-host", },
>
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.ker
> nel.org%2Fpatchwork%2Fpatch%2F1059917%2Fdata=02%7C01%7Cshe
> rry.sun%40nxp.com%7C70e116ce152a4060dbd508d724987b54%7C686ea1d
> 3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637018109631327748sd
> ata=GzX5KNQ8m0CCeBa9tPSV%2BOEwwuSsxDdAiJd1T6or8P0%3Dreser
> ved=0 would suggest that
> cdns,usb3-1.0.0 is the compatible. But I might be wrong.
This patch has not been accepted by the kernel, right?
So I think maybe we can do this change to synchronize with kernel after the
cdns3
kernel patch been accepted. And note that in uboot, one dts node can only bind
with
one driver, so we keep the cdns3 node for usb gadget driver, then add a usb
host node for
xhci-imx8 driver in *-uboot.dtsi. so here is no need to change the host driver
compatible.
But the compatible in gadget driver should be changed later.
>
> > + { }
> > +};
> > +
> > +U_BOOT_DRIVER(xhci_imx8) = {
> > + .name = "xhci_imx8",
> > + .id = UCLASS_USB,
> > + .of_match = xhci_usb_ids,
> > + .probe = xhci_imx8_probe,
> > + .remove = xhci_imx8_remove,
> > + .ops= _usb_ops,
> > + .platdata_auto_alloc_size = sizeof(struct usb_platdata),
> > + .priv_auto_alloc_size = sizeof(struct xhci_ctrl),
> > + .flags = DM_FLAG_ALLOC_PRIV_DMA,
>
> I think you also need DM_FLAG_OS_PREPARE to trigger .remove before booting
> Linux, but I might be wrong. Please check that.
When use usb stop command to stop the usb host driver,
device_remove(bus, DM_REMOVE_NORMAL) is called by usb_stop(),
so normally we don’t need the DM_FLAG_OS_PREPARE flag.
But considering some special circumstances, maybe add this flag is helpful.
Best regard
Sherry sun
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v4 2/4] USB: host: Add the USB3 host driver
On 8/19/19 8:10 AM, Sherry Sun wrote:
> Add the USB3 host driver for NXP imx8 platform, and the
> cadence IP is in it. The USB3 host driver support DM
> mode, it will probe USB3 host node in dts.
>
> Signed-off-by: Sherry Sun
[...]
> +static void xhci_imx8_get_reg_addr(struct udevice *dev)
> +{
> + imx8_data.usb3_ctrl_base =
> + (void __iomem *)devfdt_get_addr_index(dev, 0);
> + imx8_data.usb3_core_base =
> + (void __iomem *)devfdt_get_addr_index(dev, 4);
> +}
Inline this.
Also, look at drivers/mtd/renesas_rpc_hf.c to handle both 32bit and
64bit DTs with multiple "reg" entries.
[...]
> +static const struct udevice_id xhci_usb_ids[] = {
> + { .compatible = "cdns,usb3-host", },
https://lore.kernel.org/patchwork/patch/1059917/ would suggest that
cdns,usb3-1.0.0 is the compatible. But I might be wrong.
> + { }
> +};
> +
> +U_BOOT_DRIVER(xhci_imx8) = {
> + .name = "xhci_imx8",
> + .id = UCLASS_USB,
> + .of_match = xhci_usb_ids,
> + .probe = xhci_imx8_probe,
> + .remove = xhci_imx8_remove,
> + .ops= _usb_ops,
> + .platdata_auto_alloc_size = sizeof(struct usb_platdata),
> + .priv_auto_alloc_size = sizeof(struct xhci_ctrl),
> + .flags = DM_FLAG_ALLOC_PRIV_DMA,
I think you also need DM_FLAG_OS_PREPARE to trigger .remove before
booting Linux, but I might be wrong. Please check that.
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
