| Message ID | 201304050259.51025.sergei.shtylyov@cogentembedded.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
On Fri, 5 Apr 2013, 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 the init() > method to the 'ehci-platform' platform data for this purpose. "init" isn't such a good name for this. It's too vague; there are already a lot of initialization steps here. How about "pre_setup" instead? > --- renesas.orig/drivers/usb/host/ehci-platform.c > +++ renesas/drivers/usb/host/ehci-platform.c > @@ -110,6 +110,13 @@ static int ehci_platform_probe(struct pl > err = PTR_ERR(hcd->regs); > goto err_put_hcd; > } > + > + if (pdata->init) { > + err = pdata->init(dev, hcd->regs); > + if (err < 0) > + goto err_put_hcd; > + } Also, I think this code should go in the ehci_platform_reset() routine, just before the call to ehci_setup(). That way more of the setup will already have been carried out. And instead of passing hcd->regs, wouldn't it be better to pass hcd? Other users of this interface might need to initialize something other than a non-standard register. 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/05/2013 06:43 PM, Alan Stern wrote: > >> Sometimes there is a need to initialize some non-standard registers mapped to >> the EHCI region before accessing the standard EHCI registers. Add the init() >> method to the 'ehci-platform' platform data for this purpose. > "init" isn't such a good name for this. It's too vague; there are > already a lot of initialization steps here. How about "pre_setup" > instead? Quite vague too. :-) But can't think of a better name... > >> --- renesas.orig/drivers/usb/host/ehci-platform.c >> +++ renesas/drivers/usb/host/ehci-platform.c >> @@ -110,6 +110,13 @@ static int ehci_platform_probe(struct pl >> err = PTR_ERR(hcd->regs); >> goto err_put_hcd; >> } >> + >> + if (pdata->init) { >> + err = pdata->init(dev, hcd->regs); >> + if (err < 0) >> + goto err_put_hcd; >> + } > Also, I think this code should go in the ehci_platform_reset() routine, > just before the call to ehci_setup(). That way more of the setup will > already have been carried out. You're right, of course. > And instead of passing hcd->regs, wouldn't it be better to pass hcd? I really don't know. > Other users of this interface might need to initialize something other > than a non-standard register. Hm, maybe... if passing 'struct usb_hcd *' would indeed help here. Do you think it's worth passing 'struct platform_device *' along with it? > 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
On 04/06/2013 12:40 AM, Sergei Shtylyov wrote: > >> And instead of passing hcd->regs, wouldn't it be better to pass hcd? > > I really don't know. > >> Other users of this interface might need to initialize something other >> than a non-standard register. > > Hm, maybe... if passing 'struct usb_hcd *' would indeed help here. > Do you think it's > worth passing 'struct platform_device *' along with it? Probably not as it can be extracted from 'hcd->self.controller'... 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 Sat, 6 Apr 2013, Sergei Shtylyov wrote: > > And instead of passing hcd->regs, wouldn't it be better to pass hcd? > > I really don't know. > > > Other users of this interface might need to initialize something other > > than a non-standard register. > > Hm, maybe... if passing 'struct usb_hcd *' would indeed help here. > Do you think it's > worth passing 'struct platform_device *' along with it? Yes. After all, the routine being called is part of the platform code. 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
On Sat, 6 Apr 2013, Sergei Shtylyov wrote: > On 04/06/2013 12:40 AM, Sergei Shtylyov wrote: > > > > >> And instead of passing hcd->regs, wouldn't it be better to pass hcd? > > > > I really don't know. > > > >> Other users of this interface might need to initialize something other > >> than a non-standard register. > > > > Hm, maybe... if passing 'struct usb_hcd *' would indeed help here. > > Do you think it's > > worth passing 'struct platform_device *' along with it? > > Probably not as it can be extracted from 'hcd->self.controller'... It's up to you. Generally I think it's easier to pass an extra argument than to force the function being called to dig it out. 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
On 04/06/2013 12:56 AM, Alan Stern wrote: >>>> And instead of passing hcd->regs, wouldn't it be better to pass hcd? >>> I really don't know. >>> >>>> Other users of this interface might need to initialize something other >>>> than a non-standard register. >>> Hm, maybe... if passing 'struct usb_hcd *' would indeed help here. >>> Do you think it's >>> worth passing 'struct platform_device *' along with it? >> Probably not as it can be extracted from 'hcd->self.controller'... > It's up to you. Generally I think it's easier to pass an extra > argument than to force the function being called to dig it out. Note that in our use case we don't even need this argument. > 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
On Sat, 6 Apr 2013, Sergei Shtylyov wrote: > On 04/06/2013 12:56 AM, Alan Stern wrote: > > >>>> And instead of passing hcd->regs, wouldn't it be better to pass hcd? > >>> I really don't know. > >>> > >>>> Other users of this interface might need to initialize something other > >>>> than a non-standard register. > >>> Hm, maybe... if passing 'struct usb_hcd *' would indeed help here. > >>> Do you think it's > >>> worth passing 'struct platform_device *' along with it? > >> Probably not as it can be extracted from 'hcd->self.controller'... > > It's up to you. Generally I think it's easier to pass an extra > > argument than to force the function being called to dig it out. > > Note that in our use case we don't even need this argument. Okay, then don't pass it. 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
Index: renesas/drivers/usb/host/ehci-platform.c =================================================================== --- renesas.orig/drivers/usb/host/ehci-platform.c +++ renesas/drivers/usb/host/ehci-platform.c @@ -110,6 +110,13 @@ static int ehci_platform_probe(struct pl err = PTR_ERR(hcd->regs); goto err_put_hcd; } + + if (pdata->init) { + err = pdata->init(dev, hcd->regs); + if (err < 0) + goto err_put_hcd; + } + err = usb_add_hcd(hcd, irq, IRQF_SHARED); if (err) goto err_put_hcd; Index: renesas/include/linux/usb/ehci_pdriver.h =================================================================== --- renesas.orig/include/linux/usb/ehci_pdriver.h +++ renesas/include/linux/usb/ehci_pdriver.h @@ -50,6 +50,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 (*init)(struct platform_device *pdev, void __iomem *regs); }; #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 the init() method to the 'ehci-platform' platform data for this purpose. Suggested-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> --- drivers/usb/host/ehci-platform.c | 7 +++++++ include/linux/usb/ehci_pdriver.h | 1 + 2 files changed, 8 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