[U-Boot] [PATCH] usb: xhci: add struct devrequest declaration

[Marek Vasut <marex@denx.de>]

On 05/06/2016 01:31 PM, Masahiro Yamada wrote:
> Hi Marek,

Hi!

> 2016-05-06 19:50 GMT+09:00 Marek Vasut <marex@denx.de>:
>> On 05/06/2016 12:36 PM, Masahiro Yamada wrote:
>>> This should be declared for xhci_ctrl_tx() to avoid build error.
>>
>> Can you please include the build error in the commit message ?
>> That is extremely useful.
>>
>>> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>>> ---
>>>
>>>  drivers/usb/host/xhci.h | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
>>> index 2afa386..16dd61a 100644
>>> --- a/drivers/usb/host/xhci.h
>>> +++ b/drivers/usb/host/xhci.h
>>> @@ -1252,6 +1252,8 @@ void xhci_acknowledge_event(struct xhci_ctrl *ctrl);
>>>  union xhci_trb *xhci_wait_for_event(struct xhci_ctrl *ctrl, trb_type expected);
>>>  int xhci_bulk_tx(struct usb_device *udev, unsigned long pipe,
>>>                int length, void *buffer);
>>> +
>>> +struct devrequest;
>>
>> I don't think that's the right thing to do, since the structure
>> devrequest is not defined anywhere in this file or the headers
>> which are included in this file.
>>
>> Will this patch work for you instead? It includes usb.h , which
>> defines the struct devrequest.
> 
> Of course, works.
> But why?

Because you want to have definition of every symbol you use in your
headers when you include that header. I am not a big fan of huge stack
of #include statements in a driver.

> xhci_ctrl_tx() only takes a pointer to struct devrequest.
> 
> This header does not need to know
> the members of struct devrequest, or sizeof(struct devrequest).
> 
> We need to teach it that devrequest is a structure.
> That's enough.
> 
> 
> This is a very common way to fix
> "warning: 'struct devrequest' declared inside parameter list" error.
> 
> For example,
> 
> head -25 include/linux/clk.h
> head -30 include/linux/pinctrl/pinctrl.h
> 
> in Linux Kernel.

Ha, I didn't know about that. Is that some new recommended practice or
is this a matter of taste ?

CCing Tom and Simon, so I can get some more input on this. I cannot
decide either way myself.

>> --->8---
>> diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
>> index 2afa386..b5a2ea5 100644
>> --- a/drivers/usb/host/xhci.h
>> +++ b/drivers/usb/host/xhci.h
>> @@ -22,6 +22,7 @@
>>  #include <asm/io.h>
>>  #include <linux/list.h>
>>  #include <linux/compat.h>
>> +#include <usb.h>
>>
>>  #define MAX_EP_CTX_NUM         31
>>  #define XHCI_ALIGNMENT         64
>> ---8<---
>>
>>>  int xhci_ctrl_tx(struct usb_device *udev, unsigned long pipe,
>>>                struct devrequest *req, int length, void *buffer);
>>>  int xhci_check_maxpacket(struct usb_device *udev);
>>>
>>
>>
>> --
>> Best regards,
>> Marek Vasut
>> _______________________________________________
>> U-Boot mailing list
>> U-Boot at lists.denx.de
>> http://lists.denx.de/mailman/listinfo/u-boot
> 
> 
> 


-- 
Best regards,
Marek Vasut