[U-Boot] [PATCH v1 1/4] usb: host: ehci-vf: Migrate Vybrid USB to driver model
maitysanchayan at gmail.com
maitysanchayan at gmail.com
Tue Aug 9 17:33:11 CEST 2016
Hello Lukasz,
On 16-08-09 15:25:50, Lukasz Majewski wrote:
> Hi maitysanchayan at gmail.com,
>
> > Hello,
> >
> > On 16-08-08 18:56:21, maitysanchayan at gmail.com wrote:
> > > Hello,
> > >
> > > Adding Lukasz's second mail ID to cc
> > >
> > > On 16-08-08 11:45:35, 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
> > > > 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 ...
> > > >
> > > > 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.
> > >
> > > @Lukasz
> > > Can you comment?
> >
> > diff --git a/drivers/usb/host/usb-uclass.c
> > b/drivers/usb/host/usb-uclass.c index be114fc..1bcd73f 100644
> > --- a/drivers/usb/host/usb-uclass.c
> > +++ b/drivers/usb/host/usb-uclass.c
> > @@ -355,7 +355,7 @@ int usb_setup_ehci_gadget(struct ehci_ctrl
> > **ctlrp) int ret;
> >
> > /* Find the old device and remove it */
> > - ret = uclass_find_device_by_seq(UCLASS_USB, 0, true, &dev);
> > + ret = uclass_find_device_by_seq(UCLASS_USB, 0, false, &dev);
> > if (ret)
> > return ret;
> > ret = device_remove(dev);
> >
> > Having the above change, the following sequence of commands seems to
> > work?
> >
> > U-Boot 2016.09-rc1-00329-g4e72f10-dirty (Aug 08 2016 - 19:19:38 +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
> > ERROR: g_dnl_register failed
> > at cmd/dfu.c:59/do_dfu()
> > Colibri VFxx # usb start
> > starting USB...
> > USB0: USB EHCI 1.00
> > USB1: USB EHCI 1.00
> > scanning bus 0 for devices... 1 USB Device(s) found
> > scanning bus 1 for devices... 2 USB Device(s) found
> > Colibri VFxx # dfu 0 nand 4
> > using id 'nand0,0'
> > using id 'nand0,1'
> > using id 'nand0,3'
> >
> > Using the third parameter as false makes it look for probed devices.
> > Since it got probed because of the "usb start" it now works. With the
> > original parameter as "true" it will look for a device that will
> > request the concerned sequence number if probed. However it seems to
> > not work?
>
> Could you confirm that for your platform DFU is working without
> CONFIG_DM_USB enabled?
Yes DFU works without CONFIG_DM_USB enabled.
Regards,
Sanchayan.
>
> >
> > Ideally it should have worked with just a command of "dfu 0 nand 4".
> >
> > Regards,
> > Sanchayan.
> >
> > >
> > > Regards,
> > > Sanchayan.
> > >
> > > > > >
> > > > > > 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
> >
>
>
>
> --
> Best regards,
>
> Lukasz Majewski
>
> Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
More information about the U-Boot
mailing list