| Message ID | 201304090120.00626.sergei.shtylyov@cogentembedded.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
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 -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 */
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(+) -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html