[PATCH v10 4/8] usb: chipidea: udc: add pullup/pulldown dp at hw_device_state

[peter.chen@freescale.com (Peter Chen)]

On Wed, Feb 27, 2013 at 11:06:05PM +0200, Alexander Shishkin wrote:
>    On Feb 27, 2013 8:33 AM, "Peter Chen" <peter.chen@freescale.com> wrote:
>    >
>    > - During the connect/disconnect host, we need to pullup
>    > and pulldown dp
>    > - Make sure the dp is not pullup until the vbus is on when
>    > flag CI13XXX_PULLUP_ON_VBUS is set
>    > - Using hw_device_state when set run/stop bit
>    >
>    > Signed-off-by: Peter Chen <peter.chen@freescale.com>
>    > ---
>    >  drivers/usb/chipidea/udc.c |   10 ++++++++--
>    >  1 files changed, 8 insertions(+), 2 deletions(-)
>    >
>    > diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
>    > index e82dae4..597ae64 100644
>    > --- a/drivers/usb/chipidea/udc.c
>    > +++ b/drivers/usb/chipidea/udc.c
>    > @@ -91,8 +91,10 @@ static int hw_device_state(struct ci13xxx *ci, u32
>    dma)
>    >                 /* interrupt, error, port change, reset, sleep/suspend
>    */
>    >                 hw_write(ci, OP_USBINTR, ~0,
>    >                            
>     USBi_UI|USBi_UEI|USBi_PCI|USBi_URI|USBi_SLI);
>    > +               hw_write(ci, OP_USBCMD, USBCMD_RS, USBCMD_RS);
>    >         } else {
>    >                 hw_write(ci, OP_USBINTR, ~0, 0);
>    > +               hw_write(ci, OP_USBCMD, USBCMD_RS, 0);
>    >         }
>    >         return 0;
>    >  }
>    > @@ -1424,10 +1426,14 @@ static int ci13xxx_pullup(struct usb_gadget
>    *_gadget, int is_on)
>    >  {
>    >         struct ci13xxx *ci = container_of(_gadget, struct ci13xxx,
>    gadget);
>    >
>    > +       if ((ci->platdata->flags & CI13XXX_PULLUP_ON_VBUS) &&
>    > +                       !ci->vbus_active)
>    > +               return -ENOTSUPP;
> 
>    This doesn't look right, if !ci->vbus_active *and* is_on, then we can't do
>    what we're asked to do, so it makes sense to return error (which, by the
>    looks of it, will be ignored by gadgets anyway, but that's another thing),
>    and I don't think ENOTSUPP is the right choice here.
> 
>    In case !ci->vbus_active and !is_on, it should be safe to return 0.

At later patch, the flag CI13XXX_PULLUP_ON_VBUS is retired. The code likes
below:

static int ci13xxx_pullup(struct usb_gadget *_gadget, int is_on)
{
	struct ci13xxx *ci = container_of(_gadget, struct ci13xxx, gadget);

	if (!ci->vbus_active)
		return -ENOTSUPP;

	if (is_on)
		hw_device_state(ci, ci->ep0out->qh.dma);
	else
		hw_device_state(ci, 0);

	return 0;
}

So you suggested I change like below:

static int ci13xxx_pullup(struct usb_gadget *_gadget, int is_on)
{
	struct ci13xxx *ci = container_of(_gadget, struct ci13xxx, gadget);

	if (!ci->vbus_active)
		return -EPERM;

	if (is_on)
		hw_device_state(ci, ci->ep0out->qh.dma);
	else
		hw_device_state(ci, 0);

	return 0;
}

-- 

Best Regards,
Peter Chen