Re: [PATCH 1/6] usb: musb: Fix broken use of static variable for multiple instances

[Bin Liu <b-liu-l0cyMroinI0@public.gmane.org>]

On Fri, Nov 11, 2016 at 12:05:19PM -0800, Tony Lindgren wrote:
> * Bin Liu <b-liu-l0cyMroinI0@public.gmane.org> [161111 11:15]:
> > On Fri, Nov 11, 2016 at 10:42:57AM -0800, Tony Lindgren wrote:
> > > --- a/drivers/usb/musb/musb_core.h
> > > +++ b/drivers/usb/musb/musb_core.h
> > > @@ -385,6 +385,8 @@ struct musb {
> > >  	int			a_wait_bcon;	/* VBUS timeout in msecs */
> > >  	unsigned long		idle_timeout;	/* Next timeout in jiffies */
> > >  
> > > +	unsigned		is_initialized:1;
> > > +
> > 
> > The musb strcut is getting bigger and bigger. Is it possible to not add
> > a new variable but use one initialized in musb_init_controller() for
> > this flag purpose? Readability shouldn't be an issue, since it is only
> > used once in musb_runtime_resume().
> 
> Well I was initially checking for delayed work being initialized,
> but Johan did not like the idea of using it. Looking at
> musb_init_controller(), we only initialize musb->nIrq after
> pm_runtime_get_sync(), but that's no better either.. Do you have
> any better ideas for something to use?

I only spent a few seconds on musb_init_controller(), and musb->nIrq is
what I found.

> 
> I agree in the long run we should split struct musb into smaller
> structs though.
> 
> Meanwhile the new flags added in this series are bitfields, so it's
> probably better to use that and overloading something that's not
> strictly related.

Ok, just leave it as is. We will clean up musb struct later. *cough* not
sure when it will ever happen...

> 
> Regards,
> 
> Tony

Regards,
-Bin.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html