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

[Richard Leitner <dev@g0hl1n.net>]

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.