Hi,
On Tue, Jul 02, 2013 at 10:06:23AM +0200, Sebastian Andrzej Siewior wrote:
> On 07/01/2013 09:18 AM, Felipe Balbi wrote:
> > diff --git a/drivers/usb/usb-common.c b/drivers/usb/usb-common.c
> > index 675384d..b87f202 100644
> > --- a/drivers/usb/usb-common.c
> > +++ b/drivers/usb/usb-common.c
> > @@ -112,6 +112,41 @@ enum usb_dr_mode of_usb_get_dr_mode(struct device_node *np)
> > return USB_DR_MODE_UNKNOWN;
> > }
> > EXPORT_SYMBOL_GPL(of_usb_get_dr_mode);
> > +
> > +static const char *const usb_maximum_speed[] = {
> > + [USB_SPEED_UNKNOWN] = "",
> > + [USB_SPEED_LOW] = "lowspeed",
> > + [USB_SPEED_FULL] = "fullspeed",
> > + [USB_SPEED_HIGH] = "highspeed",
> > + [USB_SPEED_SUPER] = "superspeed",
>
> I somehow would love if you could move that table + the lookup function
> into usb-common where allready have the reverse one called
> usb_speed_string(). Then we could have always "low-speed" instead
> mixing the wording.
uhm... this is usb-common.c
But I agree that we should use low-speed instead of lowspeed.
> Also it would be nice if you would Cc: the devicetree-discuss@ folks and
> document the binding in Documentation/devicetree/bindings/usb/
sure, will do.
> > +};
> > +
> > +/**
> > + * of_usb_get_maximum_speed - Get maximum requested speed for a given USB
> > + * controller.
> > + * @np: Pointer to the given device_node
> > + *
> > + * The function gets the maximum speed string from property "maximum-speed",
> > + * and returns the corresponding enum usb_device_speed.
> > + */
> > +enum usb_device_speed of_usb_get_maximum_speed(struct device_node *np)
> > +{
> > + const char *maximum_speed;
> > + int err;
> > + int i;
> > +
> > + err = of_property_read_string(np, "maximum-speed", &maximum_speed);
> > + if (err < 0)
> > + return USB_SPEED_UNKNOWN;
> > +
> > + for (i = 0; i < ARRAY_SIZE(usb_maximum_speed); i++)
> > + if (strcmp(maximum_speed, usb_maximum_speed[i]) == 0)
> > + return i;
> > +
> > + return USB_SPEED_UNKNOWN;
>
> This looks tricky. If you copy that property unconditionally than you
> will end up with UNKNOWN speed if that property is missing and no
> gadget will load, right? If so, maybe a second argument would be nice
UDC should be required to treat UNKOWN as maximum-speed not being
passed and, thus, fixing it to the maximum the HW supports.
> which would be used as the default speed if none is found in DT. Unless
> you want to do that check in each and every driver.
I don't mind either way although I find it funny to pass a 'default'
argument which just gets returned back to the caller if maximum-speed
isn't available.
--
balbi
Attachment:
signature.asc
Description: Digital signature
