[U-Boot] [PATCH v1 1/4] usb: host: ehci-vf: Migrate Vybrid USB to driver model
maitysanchayan at gmail.com
maitysanchayan at gmail.com
Mon Aug 8 10:02:35 CEST 2016
Hello Stefan,
On 16-08-08 00:25:16, Stefan Agner wrote:
> On 2016-08-07 23:15, maitysanchayan at gmail.com wrote:
> > Hello,
> >
> > On 16-08-03 17:13:11, Marek Vasut wrote:
> >> On 08/03/2016 01:58 PM, Sanchayan Maity wrote:
> >> > Add driver model support for Vybrid USB driver.
> >> >
> >> > Signed-off-by: Sanchayan Maity <maitysanchayan at gmail.com>
> >>
> >> CCing Lukasz.
> >>
> >> > ---
> >> > Hello,
> >> >
> >> > I am trying to migrate the Vybrid USB driver to driver model.
> >> > Patches are based on top of uboot master branch. With this
> >> > implementation, host works perfectly fine on both USB ports
> >> > but I have problems using it in client mode.
> >> >
> >> > I tried DFU to test client mode and I get the following
> >> >
> >> > Colibri VFxx # version
> >> >
> >> > U-Boot 2016.09-rc1-00235-g4e8c122 (Aug 03 2016 - 17:07:48 +0530)
> >> > arm-linux-gnueabihf-gcc (Linaro GCC 5.2-2015.11-2) 5.2.1 20151005
> >> > GNU ld (GNU Binutils) 2.25.0 Linaro 2015_10
> >> > Colibri VFxx # dfu 0 nand 4
> >> > using id 'nand0,0'
> >> > using id 'nand0,1'
> >> > using id 'nand0,3'
> >> > g_dnl_register: failed!, error: -19
> >> > data abort
> >> > pc : [<8ff80f18>] lr : [<8ff612a9>]
> >> > reloc pc : [<3f431f18>] lr : [<3f4122a9>]
> >> > sp : 8fd15000 ip : 00000000 fp : 00002710
> >> > r10: 8ffb50cc r9 : 8fd16ee8 r8 : 8ffbc574
> >> > r7 : 00000000 r6 : 00000000 r5 : 00000000 r4 : 00000000
> >> > r3 : 0000f4b9 r2 : 80000000 r1 : 00000001 r0 : 00000000
> >> > Flags: nZCv IRQs off FIQs off Mode SVC_32
> >> > Resetting CPU ...
> >> >
> >> > resetting ...
> >
> > FWIW here is output with DEBUG enabled in uclass.c
> >
> > when testing with DFU:
> >
> > uclass_find_device_by_seq: 1 0
> > - -1 -1
> > - -1 -1
> > - not found
> > g_dnl_register: failed!, error: -19
>
> Hm, I guess this comes from usb_setup_ehci_gadget called through
> usb_gadget_register_driver. The call in there seems rather hardcoded:
Yes it comes from that function.
>
> ret = uclass_find_device_by_seq(UCLASS_USB, 0, true, &dev);
> (seq = 0, and find_req_seq=true, maybe find_req_seq=false helps? Maybe
> it is because we use the second USB DR controller for OTG?)
I have already tried using that value along with what I see during
use as USB host.
While eventually the second USB controller is to be used as OTG for
us, the point of failure is even before the device_probe call in
usb_setup_ehci_gadget. Also before the device probe call the plat->
init_type is set to USB_DEVICE and I have not yet added the type
check in usb_probe function akin to the non DM case in ehci_hcd_init
which takes care of OTG handling.
Something related to sequence numbers and uclass device. I tried
DM_UC_FLAG_SEQ_ALIAS but that seems to be specified for UCLASS USB
anyways.
Regards,
Sanchayan,
>
>
> > data abort
> > pc : [<8ff80fb0>] lr : [<8ff612a9>]
> > reloc pc : [<3f431fb0>] lr : [<3f4122a9>]
> > sp : 8fd15000 ip : 00000000 fp : 00002710
> > r10: 8ffb5274 r9 : 8fd16ee8 r8 : 8ffbc714
> > r7 : 00000000 r6 : 00000000 r5 : 00000000 r4 : 00000000
> > r3 : 0000f4b9 r2 : 80000000 r1 : 00000001 r0 : 00000000
> > Flags: nZCv IRQs off FIQs off Mode SVC_32
> > Resetting CPU ...
>
> I guess that comes from cmd/dfu.c not handling the error code of
> g_dnl_register...
>
> --
> Stefan
>
> >
> > resetting ...
> >
> > When testing host mode with usb start
> >
> > Colibri VFxx # usb start
> > starting USB...
> > USB0: uclass_find_device_by_seq: 0 -1
> > uclass_find_device_by_seq: 0 0
> > - -1 -1
> > - -1 -1
> > - not found
> > USB EHCI 1.00
> > USB1: uclass_find_device_by_seq: 0 -1
> > uclass_find_device_by_seq: 0 0
> > - -1 0
> > - found
> > uclass_find_device_by_seq: 0 1
> > - -1 0
> > - -1 1
> > - found
> > uclass_find_device_by_seq: 0 2
> > - -1 0
> > - -1 1
> > - -1 -1
> > - not found
> > uclass_find_device_by_seq: 0 -1
> > uclass_find_device_by_seq: 0 0
> > - -1 0
> > - found
> > uclass_find_device_by_seq: 0 1
> > - -1 0
> > - -1 -1
> > - not found
> > USB EHCI 1.00
> > scanning bus 0 for devices... uclass_find_device_by_seq: 0 -1
> > uclass_find_device_by_seq: 0 0
> > - -1 -1
> > - not found
> > uclass_find_device_by_seq: 0 -1
> > uclass_find_device_by_seq: 0 0
> > - -1 -1
> > - not found
> > 2 USB Device(s) found
> > scanning bus 1 for devices... uclass_find_device_by_seq: 0 -1
> > uclass_find_device_by_seq: 0 0
> > - -1 0
> > - found
> > uclass_find_device_by_seq: 0 1
> > - -1 0
> > - -1 -1
> > - not found
> > uclass_find_device_by_seq: 0 -1
> > uclass_find_device_by_seq: 0 0
> > - -1 0
> > - found
> > uclass_find_device_by_seq: 0 1
> > - -1 0
> > - -1 1
> > - found
> > uclass_find_device_by_seq: 0 2
> > - -1 0
> > - -1 1
> > - -1 -1
> > - not found
> > 2 USB Device(s) found
> >
> > Regards,
> > Sanchayan.
> >
> >> >
> >> > It seems to return ENODEV from usb_setup_ehci_gadget after calling
> >> > uclass_find_device_by_seq. I am not sure what I am missing in the
> >> > current implementation. Can someone point me in the correct direction?
> >> > Since host works on both ports I would assume the device tree nodes
> >> > are correct.
> >> >
> >> > Tried to look in documentation and usb-info.txt mentions that gadget
> >> > framework does not use driver model. Does this imply I cannot use
> >> > any usb gadget functionality if I am using USB DM? However the function
> >> > usb_gadget_register_driver in ci_udc.c does have a CONFIG_DM_USB and
> >> > calls into usb_setup_ehci_gadget which is in usb-uclass.c.
> >> >
> >> > Regards,
> >> > Sanchayan.
> >> > ---
> >> > drivers/usb/host/ehci-vf.c | 123 ++++++++++++++++++++++++++++++++++++++++++---
> >> > 1 file changed, 117 insertions(+), 6 deletions(-)
> >> >
> >> > diff --git a/drivers/usb/host/ehci-vf.c b/drivers/usb/host/ehci-vf.c
> >> > index 61789dd..8c5c593 100644
> >> > --- a/drivers/usb/host/ehci-vf.c
> >> > +++ b/drivers/usb/host/ehci-vf.c
> >> > @@ -8,6 +8,7 @@
> >> > */
> >> >
> >> > #include <common.h>
> >> > +#include <dm.h>
> >> > #include <usb.h>
> >> > #include <errno.h>
> >> > #include <linux/compiler.h>
> >> > @@ -131,6 +132,24 @@ int __weak board_ehci_hcd_init(int port)
> >> > return 0;
> >> > }
> >> >
> >> > +int ehci_vf_common_init(struct usb_ehci *ehci, int index)
> >> > +{
> >> > + int ret;
> >> > +
> >> > + /* Do board specific initialisation */
> >> > + ret = board_ehci_hcd_init(index);
> >> > + if (ret)
> >> > + return ret;
> >> > +
> >> > + usb_power_config(index);
> >> > + usb_oc_config(index);
> >> > + usb_internal_phy_clock_gate(index);
> >> > + usb_phy_enable(index, ehci);
> >> > +
> >> > + return 0;
> >> > +}
> >> > +
> >> > +#ifndef CONFIG_DM_USB
> >> > int ehci_hcd_init(int index, enum usb_init_type init,
> >> > struct ehci_hccr **hccr, struct ehci_hcor **hcor)
> >> > {
> >> > @@ -143,12 +162,9 @@ int ehci_hcd_init(int index, enum usb_init_type init,
> >> > ehci = (struct usb_ehci *)nc_reg_bases[index];
> >> >
> >> > /* Do board specific initialisation */
> >> > - board_ehci_hcd_init(index);
> >> > -
> >> > - usb_power_config(index);
> >> > - usb_oc_config(index);
> >> > - usb_internal_phy_clock_gate(index);
> >> > - usb_phy_enable(index, ehci);
> >> > + ret = ehci_vf_common_init(index);
> >> > + if (ret)
> >> > + return ret;
> >> >
> >> > *hccr = (struct ehci_hccr *)((uint32_t)&ehci->caplength);
> >> > *hcor = (struct ehci_hcor *)((uint32_t)*hccr +
> >> > @@ -175,3 +191,98 @@ int ehci_hcd_stop(int index)
> >> > {
> >> > return 0;
> >> > }
> >> > +#else
> >> > +struct ehci_vf_priv_data {
> >> > + struct ehci_ctrl ctrl;
> >> > + struct usb_ehci *ehci;
> >> > + enum usb_init_type init_type;
> >> > + int portnr;
> >> > +};
> >> > +
> >> > +static int vf_init_after_reset(struct ehci_ctrl *dev)
> >> > +{
> >> > + struct ehci_vf_priv_data *priv = dev->priv;
> >> > + enum usb_init_type type = priv->init_type;
> >> > + struct usb_ehci *ehci = priv->ehci;
> >> > + int ret;
> >> > +
> >> > + ret = ehci_vf_common_init(priv->ehci, priv->portnr);
> >> > + if (ret)
> >> > + return ret;
> >> > +
> >> > + if (type == USB_INIT_DEVICE)
> >> > + return 0;
> >> > +
> >> > + setbits_le32(&ehci->usbmode, CM_HOST);
> >> > + writel((PORT_PTS_UTMI | PORT_PTS_PTW), &ehci->portsc);
> >> > + setbits_le32(&ehci->portsc, USB_EN);
> >> > +
> >> > + mdelay(10);
> >> > +
> >> > + return 0;
> >> > +}
> >> > +
> >> > +static const struct ehci_ops vf_ehci_ops = {
> >> > + .init_after_reset = vf_init_after_reset
> >> > +};
> >> > +
> >> > +static int ehci_usb_probe(struct udevice *dev)
> >> > +{
> >> > + struct usb_platdata *plat = dev_get_platdata(dev);
> >> > + struct usb_ehci *ehci = (struct usb_ehci *)dev_get_addr(dev);
> >> > + struct ehci_vf_priv_data *priv = dev_get_priv(dev);
> >> > + struct ehci_hccr *hccr;
> >> > + struct ehci_hcor *hcor;
> >> > + int ret;
> >> > +
> >> > + priv->ehci = ehci;
> >> > + priv->portnr = dev->seq;
> >> > + priv->init_type = plat->init_type;
> >> > +
> >> > + ret = ehci_vf_common_init(ehci, priv->portnr);
> >> > + if (ret)
> >> > + return ret;
> >> > +
> >> > + if (priv->init_type == USB_INIT_HOST) {
> >> > + setbits_le32(&ehci->usbmode, CM_HOST);
> >> > + writel((PORT_PTS_UTMI | PORT_PTS_PTW), &ehci->portsc);
> >> > + setbits_le32(&ehci->portsc, USB_EN);
> >> > + }
> >> > +
> >> > + mdelay(10);
> >> > +
> >> > + hccr = (struct ehci_hccr *)((uint32_t)&ehci->caplength);
> >> > + hcor = (struct ehci_hcor *)((uint32_t)hccr +
> >> > + HC_LENGTH(ehci_readl(&hccr->cr_capbase)));
> >> > +
> >> > + return ehci_register(dev, hccr, hcor, &vf_ehci_ops, 0, plat->init_type);
> >> > +}
> >> > +
> >> > +static int ehci_usb_remove(struct udevice *dev)
> >> > +{
> >> > + int ret;
> >> > +
> >> > + ret = ehci_deregister(dev);
> >> > + if (ret)
> >> > + return ret;
> >> > +
> >> > + return 0;
> >> > +}
> >> > +
> >> > +static const struct udevice_id vf_usb_ids[] = {
> >> > + { .compatible = "fsl,vf610-usb" },
> >> > + { }
> >> > +};
> >> > +
> >> > +U_BOOT_DRIVER(usb_ehci) = {
> >> > + .name = "ehci_vf",
> >> > + .id = UCLASS_USB,
> >> > + .of_match = vf_usb_ids,
> >> > + .probe = ehci_usb_probe,
> >> > + .remove = ehci_usb_remove,
> >> > + .ops = &ehci_usb_ops,
> >> > + .platdata_auto_alloc_size = sizeof(struct usb_platdata),
> >> > + .priv_auto_alloc_size = sizeof(struct ehci_vf_priv_data),
> >> > + .flags = DM_FLAG_ALLOC_PRIV_DMA,
> >> > +};
> >> > +#endif
> >> >
> >>
> >>
> >> --
> >> Best regards,
> >> Marek Vasut
> > _______________________________________________
> > U-Boot mailing list
> > U-Boot at lists.denx.de
> > http://lists.denx.de/mailman/listinfo/u-boot
More information about the U-Boot
mailing list