[PATCH v2 2/9] ehci-platform: add pre_setup() method to platform data

[PATCH v2 2/9] ehci-platform: add pre_setup() method to platform data

[Sergei Shtylyov]

Sometimes there is a need  to initialize some non-standard registers mapped to
the EHCI region before accessing the standard EHCI registers.  Add pre_setup()
method with 'struct usb_hcd *' parameter to be called just before ehci_setup()
to the 'ehci-platform'  driver's platform data for this purpose...

Suggested-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

---
Changes since the original posting:
- changed init() method to pre_setup() with different parameters abd call site.

 drivers/usb/host/ehci-platform.c |    6 ++++++
 include/linux/usb/ehci_pdriver.h |    3 +++
 2 files changed, 9 insertions(+)

Index: renesas/drivers/usb/host/ehci-platform.c
=================================--- renesas.orig/drivers/usb/host/ehci-platform.c
+++ renesas/drivers/usb/host/ehci-platform.c
@@ -46,6 +46,12 @@ static int ehci_platform_reset(struct us
 	ehci->big_endian_desc = pdata->big_endian_desc;
 	ehci->big_endian_mmio = pdata->big_endian_mmio;
 
+	if (pdata->pre_setup) {
+		retval = pdata->pre_setup(hcd);
+		if (retval < 0)
+			return retval;
+	}
+
 	ehci->caps = hcd->regs + pdata->caps_offset;
 	retval = ehci_setup(hcd);
 	if (retval)
Index: renesas/include/linux/usb/ehci_pdriver.h
=================================--- renesas.orig/include/linux/usb/ehci_pdriver.h
+++ renesas/include/linux/usb/ehci_pdriver.h
@@ -19,6 +19,8 @@
 #ifndef __USB_CORE_EHCI_PDRIVER_H
 #define __USB_CORE_EHCI_PDRIVER_H
 
+#include <linux/usb/hcd.h>
+
 /**
  * struct usb_ehci_pdata - platform_data for generic ehci driver
  *
@@ -50,6 +52,7 @@ struct usb_ehci_pdata {
 	/* Turn on only VBUS suspend power and hotplug detection,
 	 * turn off everything else */
 	void (*power_suspend)(struct platform_device *pdev);
+	int (*pre_setup)(struct usb_hcd *hcd);
 };
 
 #endif /* __USB_CORE_EHCI_PDRIVER_H */

Re: [PATCH v2 2/9] ehci-platform: add pre_setup() method to platform data

[Greg KH]

On Tue, Apr 09, 2013 at 01:20:00AM +0400, Sergei Shtylyov wrote:
> Sometimes there is a need  to initialize some non-standard registers mapped to
> the EHCI region before accessing the standard EHCI registers.  Add pre_setup()
> method with 'struct usb_hcd *' parameter to be called just before ehci_setup()
> to the 'ehci-platform'  driver's platform data for this purpose...
> 
> Suggested-by: Alan Stern <stern@rowland.harvard.edu>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> 
> ---
> Changes since the original posting:
> - changed init() method to pre_setup() with different parameters abd call site.
> 
>  drivers/usb/host/ehci-platform.c |    6 ++++++
>  include/linux/usb/ehci_pdriver.h |    3 +++
>  2 files changed, 9 insertions(+)
> 
> Index: renesas/drivers/usb/host/ehci-platform.c
> =================================> --- renesas.orig/drivers/usb/host/ehci-platform.c
> +++ renesas/drivers/usb/host/ehci-platform.c
> @@ -46,6 +46,12 @@ static int ehci_platform_reset(struct us
>  	ehci->big_endian_desc = pdata->big_endian_desc;
>  	ehci->big_endian_mmio = pdata->big_endian_mmio;
>  
> +	if (pdata->pre_setup) {
> +		retval = pdata->pre_setup(hcd);
> +		if (retval < 0)
> +			return retval;
> +	}
> +
>  	ehci->caps = hcd->regs + pdata->caps_offset;
>  	retval = ehci_setup(hcd);
>  	if (retval)
> Index: renesas/include/linux/usb/ehci_pdriver.h
> =================================> --- renesas.orig/include/linux/usb/ehci_pdriver.h
> +++ renesas/include/linux/usb/ehci_pdriver.h
> @@ -19,6 +19,8 @@
>  #ifndef __USB_CORE_EHCI_PDRIVER_H
>  #define __USB_CORE_EHCI_PDRIVER_H
>  
> +#include <linux/usb/hcd.h>

This isn't needed in the .h file, right?  Only the .c file, if it hasn't
already included it (hint, I bet it has...)

thanks,

greg k-h

Re: [PATCH v2 2/9] ehci-platform: add pre_setup() method to platform data

[Sergei Shtylyov]

Hello.

On 04/09/2013 01:26 AM, Greg KH wrote:

>
>> Sometimes there is a need  to initialize some non-standard registers mapped to
>> the EHCI region before accessing the standard EHCI registers.  Add pre_setup()
>> method with 'struct usb_hcd *' parameter to be called just before ehci_setup()
>> to the 'ehci-platform'  driver's platform data for this purpose...
>>
>> Suggested-by: Alan Stern <stern@rowland.harvard.edu>
>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>>
[...]
>> Index: renesas/include/linux/usb/ehci_pdriver.h
>> =================================>> --- renesas.orig/include/linux/usb/ehci_pdriver.h
>> +++ renesas/include/linux/usb/ehci_pdriver.h
>> @@ -19,6 +19,8 @@
>>   #ifndef __USB_CORE_EHCI_PDRIVER_H
>>   #define __USB_CORE_EHCI_PDRIVER_H
>>   
>> +#include <linux/usb/hcd.h>
> This isn't needed in the .h file, right?  Only the .c file, if it hasn't
> already included it (hint, I bet it has...)

    No, it hasn't. And I wouldn't want to include this header in the
platform code.

> thanks,
>
> greg k-h

WBR, Sergei

Re: [PATCH v2 2/9] ehci-platform: add pre_setup() method to platform data

[Sergei Shtylyov]

On 04/09/2013 01:51 AM, Sergei Shtylyov wrote:
> Hello.
>
> On 04/09/2013 01:26 AM, Greg KH wrote:
>
>>
>>> Sometimes there is a need  to initialize some non-standard registers 
>>> mapped to
>>> the EHCI region before accessing the standard EHCI registers. Add 
>>> pre_setup()
>>> method with 'struct usb_hcd *' parameter to be called just before 
>>> ehci_setup()
>>> to the 'ehci-platform'  driver's platform data for this purpose...
>>>
>>> Suggested-by: Alan Stern <stern@rowland.harvard.edu>
>>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>>>
> [...]
>>> Index: renesas/include/linux/usb/ehci_pdriver.h
>>> =================================>>> --- renesas.orig/include/linux/usb/ehci_pdriver.h
>>> +++ renesas/include/linux/usb/ehci_pdriver.h
>>> @@ -19,6 +19,8 @@
>>>   #ifndef __USB_CORE_EHCI_PDRIVER_H
>>>   #define __USB_CORE_EHCI_PDRIVER_H
>>>   +#include <linux/usb/hcd.h>
>> This isn't needed in the .h file, right?  Only the .c file, if it hasn't
>> already included it (hint, I bet it has...)
>
>    No, it hasn't. And I wouldn't want to include this header in the
> platform code.

    Although, if you insist...

    It just occured to me that this file doesn't have 'struct 
platform_device'
pre-declared either -- in the "best" tradition of the USB header files. :-)

WBR, Sergei

Re: [PATCH v2 2/9] ehci-platform: add pre_setup() method to platform data

[Greg KH]

On Tue, Apr 09, 2013 at 01:51:08AM +0400, Sergei Shtylyov wrote:
> Hello.
> 
> On 04/09/2013 01:26 AM, Greg KH wrote:
> 
> >
> >>Sometimes there is a need  to initialize some non-standard registers mapped to
> >>the EHCI region before accessing the standard EHCI registers.  Add pre_setup()
> >>method with 'struct usb_hcd *' parameter to be called just before ehci_setup()
> >>to the 'ehci-platform'  driver's platform data for this purpose...
> >>
> >>Suggested-by: Alan Stern <stern@rowland.harvard.edu>
> >>Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> >>
> [...]
> >>Index: renesas/include/linux/usb/ehci_pdriver.h
> >>=================================> >>--- renesas.orig/include/linux/usb/ehci_pdriver.h
> >>+++ renesas/include/linux/usb/ehci_pdriver.h
> >>@@ -19,6 +19,8 @@
> >>  #ifndef __USB_CORE_EHCI_PDRIVER_H
> >>  #define __USB_CORE_EHCI_PDRIVER_H
> >>+#include <linux/usb/hcd.h>
> >This isn't needed in the .h file, right?  Only the .c file, if it hasn't
> >already included it (hint, I bet it has...)
> 
>    No, it hasn't. And I wouldn't want to include this header in the
> platform code.

Then you don't need it in this .h file either, please remove it.

thanks,

greg k-h

Re: [PATCH v2 2/9] ehci-platform: add pre_setup() method to platform data

[Sergei Shtylyov]

Hello.

On 04/09/2013 02:07 AM, Greg KH wrote:

>>>> Sometimes there is a need  to initialize some non-standard registers mapped to
>>>> the EHCI region before accessing the standard EHCI registers.  Add pre_setup()
>>>> method with 'struct usb_hcd *' parameter to be called just before ehci_setup()
>>>> to the 'ehci-platform'  driver's platform data for this purpose...
>>>>
>>>> Suggested-by: Alan Stern <stern@rowland.harvard.edu>
>>>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>>>>
>> [...]
>>>> Index: renesas/include/linux/usb/ehci_pdriver.h
>>>> =================================>>>> --- renesas.orig/include/linux/usb/ehci_pdriver.h
>>>> +++ renesas/include/linux/usb/ehci_pdriver.h
>>>> @@ -19,6 +19,8 @@
>>>>   #ifndef __USB_CORE_EHCI_PDRIVER_H
>>>>   #define __USB_CORE_EHCI_PDRIVER_H
>>>> +#include <linux/usb/hcd.h>
>>> This isn't needed in the .h file, right?  Only the .c file, if it hasn't
>>> already included it (hint, I bet it has...)
>>     No, it hasn't. And I wouldn't want to include this header in the
>> platform code.
> Then you don't need it in this .h file either, please remove it.

     I do need it in the platform .c file. Well, long live multiple 
redeclarations
of the same thing!

> thanks,
>
> greg k-h

WBR, Sergei

Re: [PATCH v2 2/9] ehci-platform: add pre_setup() method to platform data

[Greg KH]

On Tue, Apr 09, 2013 at 02:04:56AM +0400, Sergei Shtylyov wrote:
> On 04/09/2013 01:51 AM, Sergei Shtylyov wrote:
> >Hello.
> >
> >On 04/09/2013 01:26 AM, Greg KH wrote:
> >
> >>
> >>>Sometimes there is a need  to initialize some non-standard
> >>>registers mapped to
> >>>the EHCI region before accessing the standard EHCI registers.
> >>>Add pre_setup()
> >>>method with 'struct usb_hcd *' parameter to be called just
> >>>before ehci_setup()
> >>>to the 'ehci-platform'  driver's platform data for this purpose...
> >>>
> >>>Suggested-by: Alan Stern <stern@rowland.harvard.edu>
> >>>Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> >>>
> >[...]
> >>>Index: renesas/include/linux/usb/ehci_pdriver.h
> >>>=================================> >>>--- renesas.orig/include/linux/usb/ehci_pdriver.h
> >>>+++ renesas/include/linux/usb/ehci_pdriver.h
> >>>@@ -19,6 +19,8 @@
> >>>  #ifndef __USB_CORE_EHCI_PDRIVER_H
> >>>  #define __USB_CORE_EHCI_PDRIVER_H
> >>>  +#include <linux/usb/hcd.h>
> >>This isn't needed in the .h file, right?  Only the .c file, if it hasn't
> >>already included it (hint, I bet it has...)
> >
> >   No, it hasn't. And I wouldn't want to include this header in the
> >platform code.
> 
>    Although, if you insist...
> 
>    It just occured to me that this file doesn't have 'struct
> platform_device'
> pre-declared either -- in the "best" tradition of the USB header files. :-)

Yes, if the .h file doesn't need it, don't include it in the .h file.
Include it in the .c file instead.

thanks,

greg k-h

Re: [PATCH v2 2/9] ehci-platform: add pre_setup() method to platform data

[Sergei Shtylyov]

Hello.

On 09-04-2013 3:42, Greg KH wrote:

>>>>> Sometimes there is a need  to initialize some non-standard
>>>>> registers mapped to
>>>>> the EHCI region before accessing the standard EHCI registers.
>>>>> Add pre_setup()
>>>>> method with 'struct usb_hcd *' parameter to be called just
>>>>> before ehci_setup()
>>>>> to the 'ehci-platform'  driver's platform data for this purpose...

>>>>> Suggested-by: Alan Stern <stern@rowland.harvard.edu>
>>>>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

>>> [...]
>>>>> Index: renesas/include/linux/usb/ehci_pdriver.h
>>>>> =================================>>>>> --- renesas.orig/include/linux/usb/ehci_pdriver.h
>>>>> +++ renesas/include/linux/usb/ehci_pdriver.h
>>>>> @@ -19,6 +19,8 @@
>>>>>   #ifndef __USB_CORE_EHCI_PDRIVER_H
>>>>>   #define __USB_CORE_EHCI_PDRIVER_H
>>>>>   +#include <linux/usb/hcd.h>

>>>> This isn't needed in the .h file, right?  Only the .c file, if it hasn't
>>>> already included it (hint, I bet it has...)

>>>    No, it hasn't. And I wouldn't want to include this header in the
>>> platform code.

>>     Although, if you insist...

>>     It just occured to me that this file doesn't have 'struct
>> platform_device'
>> pre-declared either -- in the "best" tradition of the USB header files. :-)

> Yes, if the .h file doesn't need it, don't include it in the .h file.
> Include it in the .c file instead.

    The ehci_pdriver.h still needs 'struct platfrom_device' declared. It 
shouldn't rely on the order of other #include's in the .c file that includes 
it. That's simply wrong, and I'm adding incomplete declaration while I am 
touching this file...

> thanks,

> greg k-h

WBR, Sergei

Re: [PATCH v2 2/9] ehci-platform: add pre_setup() method to platform data

[Alan Stern]

On Tue, 9 Apr 2013, Sergei Shtylyov wrote:

> >>>> --- renesas.orig/include/linux/usb/ehci_pdriver.h
> >>>> +++ renesas/include/linux/usb/ehci_pdriver.h
> >>>> @@ -19,6 +19,8 @@
> >>>>   #ifndef __USB_CORE_EHCI_PDRIVER_H
> >>>>   #define __USB_CORE_EHCI_PDRIVER_H
> >>>> +#include <linux/usb/hcd.h>
> >>> This isn't needed in the .h file, right?  Only the .c file, if it hasn't
> >>> already included it (hint, I bet it has...)
> >>     No, it hasn't. And I wouldn't want to include this header in the
> >> platform code.
> > Then you don't need it in this .h file either, please remove it.
> 
>      I do need it in the platform .c file. Well, long live multiple 
> redeclarations
> of the same thing!

If you do remove that line from ehci_pdriver.h, you should add a
declaration of struct usb_hcd.  Like this:

struct usb_hcd;

Otherwise the compiler will complain when it sees this structure
mentioned for the first time in the parameter list of a
function-pointer declaration.

Alan Stern

Re: [PATCH v2 2/9] ehci-platform: add pre_setup() method to platform data

[Sergei Shtylyov]

Hello.

On 04/09/2013 07:27 PM, Alan Stern wrote:

>
>>>>>> --- renesas.orig/include/linux/usb/ehci_pdriver.h
>>>>>> +++ renesas/include/linux/usb/ehci_pdriver.h
>>>>>> @@ -19,6 +19,8 @@
>>>>>>    #ifndef __USB_CORE_EHCI_PDRIVER_H
>>>>>>    #define __USB_CORE_EHCI_PDRIVER_H
>>>>>> +#include <linux/usb/hcd.h>
>>>>> This isn't needed in the .h file, right?  Only the .c file, if it hasn't
>>>>> already included it (hint, I bet it has...)
>>>>      No, it hasn't. And I wouldn't want to include this header in the
>>>> platform code.
>>> Then you don't need it in this .h file either, please remove it.
>>       I do need it in the platform .c file. Well, long live multiple
>> redeclarations
>> of the same thing!
> If you do remove that line from ehci_pdriver.h, you should add a
> declaration of struct usb_hcd.  Like this:
>
> struct usb_hcd;
>
> Otherwise the compiler will complain when it sees this structure
> mentioned for the first time in the parameter list of a
> function-pointer declaration.

    That's what I did, thanks. I've also added 'struct platform_device;' 
line for the same reason.

> Alan Stern
>
WBR, Sergei