Sergei,
On Wed, Nov 10, 2010 at 11:20:37PM +0800, Sergei Shtylyov wrote:
> Hello.
>
> andy.luo wrote:
>
> > This patch adds test mode support for Langwell gadget driver, and it
> > implements debug interface for Host & Client test mode entry.
>
> > Change-Id: Ib82d1e577b0c33ad52866bb227a1a6178340012b
> > Signed-off-by: Henry Yuan <hang.yuan@xxxxxxxxx>
> > Signed-off-by: Andy Luo <yifei.luo@xxxxxxxxx>
> > ---
> > Documentation/ABI/testing/sysfs-bus-usb | 14 +++++
> > drivers/usb/core/sysfs.c | 18 +++++++
> > drivers/usb/gadget/langwell_udc.c | 20 +++++++
> > drivers/usb/host/ehci-dbg.c | 86 +++++++++++++++++++++++++++++++
> > drivers/usb/host/ehci-hub.c | 7 +++
> > drivers/usb/host/ehci.h | 1 +
> > include/linux/usb/ehci_def.h | 4 ++
> > 7 files changed, 150 insertions(+), 0 deletions(-)
>
> There should be several patches: one for the Langwell driver, one for EHCI
> driver, and one for the sysfs change...
>
Thank you for your comment, will implement langwell support only.
> > diff --git a/drivers/usb/core/sysfs.c b/drivers/usb/core/sysfs.c
> > index 448f5b4..e864b8d 100644
> > --- a/drivers/usb/core/sysfs.c
> > +++ b/drivers/usb/core/sysfs.c
> > @@ -581,6 +581,23 @@ static ssize_t usb_remove_store(struct device *dev,
> > }
> > static DEVICE_ATTR(remove, 0200, NULL, usb_remove_store);
> >
> > +static ssize_t usb_testmode_store(struct device *dev,
> > + struct device_attribute *attr,
> > + const char *buf, size_t count)
> > +{
> > + int selector;
> > + struct usb_device *udev = to_usb_device(dev);
> > +
> > + if (strict_strtoul(buf, 16, &selector))
> > + return -EINVAL;
> > + usb_control_msg(udev, usb_sndctrlpipe(udev, 0), USB_REQ_SET_FEATURE,
> > + USB_RECIP_DEVICE, USB_DEVICE_TEST_MODE,
> > + selector << 8, NULL, 0, 1000);
>
> Couldn't this be implemented in userland?
Yes, the test mode interface is not so necessary as libusb can implement it.
> > +
> > + return count;
> > +}
> > +static DEVICE_ATTR(test, 0200, NULL, usb_testmode_store);
> > +
> >
> > static struct attribute *dev_attrs[] = {
> > /* current configuration's attributes */
> [...]
> > diff --git a/drivers/usb/gadget/langwell_udc.c b/drivers/usb/gadget/langwell_udc.c
> > index 3f17ecf..feaa40a 100644
> > --- a/drivers/usb/gadget/langwell_udc.c
> > +++ b/drivers/usb/gadget/langwell_udc.c
> > @@ -2264,6 +2264,7 @@ static void handle_setup_packet(struct langwell_udc *dev,
> > u16 wValue = le16_to_cpu(setup->wValue);
> > u16 wIndex = le16_to_cpu(setup->wIndex);
> > u16 wLength = le16_to_cpu(setup->wLength);
> > + u32 portsc1;
> >
> > dev_vdbg(&dev->pdev->dev, "---> %s()\n", __func__);
> >
> > @@ -2364,6 +2365,25 @@ static void handle_setup_packet(struct langwell_udc *dev,
> > dev->gadget.a_alt_hnp_support = 1;
> > dev->dev_status |= (1 << wValue);
> > break;
> > + case USB_DEVICE_TEST_MODE:
> > + dev_dbg(&dev->pdev->dev, "SETUP: TEST MODE\n");
> > + if ((wIndex & 0xff) ||
> > + (dev->gadget.speed != USB_SPEED_HIGH))
> > + ep0_stall(dev);
>
> An empty line wouldn't hurt here...
Yes, an empty line is better here.
> > + switch (wIndex >> 8) {
> > + case 1:
> > + case 2:
> > + case 3:
> > + case 5:
> > + case 4:
> > + portsc1 = readl(&dev->op_regs->portsc1);
> > + portsc1 |= (wIndex & 0xf00) << 8;
> > + writel(portsc1, &dev->op_regs->portsc1);
> > + break;
> > + default:
> > + rc = -EOPNOTSUPP;
> > + }
> > + break;
>
> Test mode request (as most other requests) should be handled after status
> phase completes.
I think it can be fixed by sending status phase before enable testmode.
> > diff --git a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c
> > index df5546b..a163e32 100644
> > --- a/drivers/usb/host/ehci-dbg.c
> > +++ b/drivers/usb/host/ehci-dbg.c
> [...]
> > @@ -1043,6 +1058,67 @@ static ssize_t debug_lpm_write(struct file *file, const char __user *user_buf,
> > return count;
> > }
> >
> > +static int debug_testmode_show(struct seq_file *s, void *unused)
> > +{
> > + struct usb_hcd *hcd;
> > + struct ehci_hcd *ehci;
> > + u32 __iomem *portsc ;
> > + u32 test;
> > + int port;
> > +
> > + hcd = bus_to_hcd(s->private);
> > + ehci = hcd_to_ehci(hcd);
> > + port = HCS_N_PORTS(ehci->hcs_params);
> > + while (port) {
> > + portsc = &ehci->regs->port_status[port-1];
> > + test = (ehci_readl(ehci, portsc)) & 0xf0000;
> > +
> > + if (test == PORT_TEST_FORCE)
> > + seq_printf(s, "Port%d mode: force enable\n", port);
> > +
> > + if (test == PORT_TEST_PKT)
> > + seq_printf(s, "Port%d mode: test packet\n", port);
> > +
> > + if (test == PORT_TEST_K)
> > + seq_printf(s, "Port%d mode: test K\n", port);
> > +
> > + if (test == PORT_TEST_J)
> > + seq_printf(s, "Port%d mode: test J\n", port);
> > +
>
> Contrarywise, could have saved on the empty lines here...
Will implement it with libusb.
> > + if (test == PORT_TEST_SE0_NAK)
> > + seq_printf(s, "Port%d mode: test SE0 NAK\n", port);
> > + port--;
> > + }
> > +
> > + return 0;
> > +}
>
> WBR, Sergei
Thanks,
Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
