Re: [PATCH v4] usb: misc: add USB251xB/xBi Hi-Speed Hub Controller Driver

[Richard Leitner]

On 02/08/2017 08:20 PM, Andy Shevchenko wrote:
> On Wed, 2017-02-08 at 19:45 +0100, Richard Leitner wrote:
> > On 02/08/2017 05:40 PM, Andy Shevchenko wrote:
> > > On Wed, 2017-02-08 at 16:17 +0100, Richard Leitner wrote:
> > > > On 02/08/2017 02:59 PM, Greg KH wrote:
> > > > > On Wed, Feb 08, 2017 at 03:21:08PM +0200, Andy Shevchenko wrote:
> > > > > > On Wed, 2017-02-08 at 09:52 +0100, Richard Leitner wrote:
> >
> > So the preferred solution is the BIT() stuff?
>
> I think BIT() macro in place. Otherwise you'll need a temporary unsigned
> long variable. If you already have one, then __*_bit() would work.

As I have no temporary unsigned long variable in that scope I'll go for 
the BIT() approach. Should I keep my inline {clr,set}_bit_in_byte() 
functions an use BIT() in there, or delete them and use BIT() directly 
in usb251xb_get_ofdata() ?

> > > > > > > +static int usb251xb_get_ofdata(struct usb251xb *hub,
> > > > > > > +			       struct usb251xb_data *data)
> > > > > > > +{
> > > > > > > +	return 0;
> > > > > > > +}
> > > > > > > +#endif /* CONFIG_OF */
> > > > > >
> > > > > > I don't think it's a good idea to have those ugly #ifdef.
> > > > >
> > > > > How can it be removed?
> > >
> > > __maybe_unused for function, device_property_*() instead of
> > > of_property_*() calls.
> > >
> > > Something like that. But if you are insisting this is *only* OF
> > > available hardware or we don't care, I'll not object.
> >
> > In usb3503.c and usb4604.c we have that #ifdef CONFIG_OF too. IMHO those
> > drivers should use the same solution here. But you guys are the ones
> > with tons of kernel coding experience, so just say how it should be
> > done :-)
>
> I'll agree with whatever Greg wants here.

Ok. So I'll wait for Gregs response before posting a v5.