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

From: Richard Leitner
Date: Wed Feb 08 2017 - 15:32:26 EST

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.