On Wed, May 02, 2012 at 05:13:42PM +0300, Alexander Shishkin wrote:
> Newer versions of the chipidea controller support the "advance" setting
> of usb address, which means instead of setting it immediately, deferring
> it till the status completion. Unfortunately, older versions of the
> controller don't have this feature, so in order to support those too, we
> simply don't use it. It's about 4 lines of extra code, and isn't in any
> way critical to performance.
in fact this is what specification asks:
"The device does not change its device address until after the Status
stage of this request is completed successfully."
> With this patch, ci13xxx_udc driver works with the chipidea controller in
> kirkwood (feroceon SoC), as found in, for example, sheevaplug.
>
> Signed-off-by: Alexander Shishkin <alexander.shishkin@xxxxxxxxxxxxxxx>
> ---
> drivers/usb/gadget/ci13xxx_udc.c | 22 ++++++++++++----------
> drivers/usb/gadget/ci13xxx_udc.h | 1 +
> 2 files changed, 13 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/usb/gadget/ci13xxx_udc.c b/drivers/usb/gadget/ci13xxx_udc.c
> index 8802906..1da415f 100644
> --- a/drivers/usb/gadget/ci13xxx_udc.c
> +++ b/drivers/usb/gadget/ci13xxx_udc.c
> @@ -722,14 +722,13 @@ static int hw_test_and_set_setup_guard(struct ci13xxx *udc)
> * hw_usb_set_address: configures USB address (execute without interruption)
> * @value: new USB address
> *
> - * This function returns an error code
> + * This function explicitly sets the address, without the "USBADRA" (advance)
> + * feature, which is not supported by older versions of the controller.
> */
> -static int hw_usb_set_address(struct ci13xxx *udc, u8 value)
> +static void hw_usb_set_address(struct ci13xxx *udc, u8 value)
changing return from int to void isn't part of $SUBJECT.
> {
> - /* advance */
> - hw_write(udc, OP_DEVICEADDR, DEVICEADDR_USBADR | DEVICEADDR_USBADRA,
> - value << ffs_nr(DEVICEADDR_USBADR) | DEVICEADDR_USBADRA);
> - return 0;
> + hw_write(udc, OP_DEVICEADDR, DEVICEADDR_USBADR,
> + value << ffs_nr(DEVICEADDR_USBADR));
> }
>
> /**
> @@ -1831,6 +1830,11 @@ isr_setup_status_complete(struct usb_ep *ep, struct usb_request *req)
>
> trace(udc->dev, "%s, %p", ep->name, req);
>
> + if (udc->addr_to_set) {
> + hw_usb_set_address(udc, udc->addr_to_set);
> + udc->addr_to_set = 0;
> + }
this won't work always. A SetAddress with address zero is valid and will
put the device into Default State. This is used at least by USB{20,30}CV
tools. I guess it's better to use a flag together with a field to hold
the address.
--
balbi
Attachment:
signature.asc
Description: Digital signature
