Re: [PATCH linux dev-5.4 v2 1/3] usb: gadget: aspeed: read vhub config from of_device_id

[Andrew Jeffery]

On Fri, 24 Jan 2020, at 11:53, Tao Ren wrote:
> On Fri, Jan 24, 2020 at 10:39:45AM +1030, Andrew Jeffery wrote:
> > > diff --git a/drivers/usb/gadget/udc/aspeed-vhub/hub.c 
> > > b/drivers/usb/gadget/udc/aspeed-vhub/hub.c
> > > index 19b3517e04c0..aa1c127e9f2f 100644
> > > --- a/drivers/usb/gadget/udc/aspeed-vhub/hub.c
> > > +++ b/drivers/usb/gadget/udc/aspeed-vhub/hub.c
> > > @@ -133,10 +133,9 @@ static const struct ast_vhub_full_cdesc {
> > >  
> > >  #define AST_VHUB_HUB_DESC_SIZE	(USB_DT_HUB_NONVAR_SIZE + 2)
> > >  
> > > -static const struct usb_hub_descriptor ast_vhub_hub_desc = {
> > > +static struct usb_hub_descriptor ast_vhub_hub_desc = {
> > 
> > This seems unfortunate, though we only have one on the SoC... is
> > it worth dynamically allocating it? Or adding a comment?
> > 
> > Andrew
> 
> According to the comment at the beginning of the file (line 39-47), we
> may customize more descriptors in the future. Adding comments involves
> little change in this case, because by allocating the descriptors, we
> will also need a function to free the descriptors when ast_vhub_remove
> is called. Anyways I'm fine with either way.
> 
> There is another option which is to fixup descriptors in request_desc
> callback, like ast_vhub_patch_dev_desc_usb1() in the file. But I don't
> like the approach personally.
> 
> Which way do you prefer?

I'm not wedded to doing anything different from what you've already got
in the patch - we don't have hardware that requires a different solution at
this point. We can always fix the driver if that changes. The approach just
felt a bit icky, but I can live with that :)

Andrew