[PATCH 2/8] ehci-platform: add init() method to platform data

[PATCH 2/8] ehci-platform: add init() 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 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(+)

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 */

Re: [PATCH 2/8] ehci-platform: add init() method to platform data

[Alan Stern]

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

Re: [PATCH 2/8] ehci-platform: add init() method to platform data

[Sergei Shtylyov]

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

Re: [PATCH 2/8] ehci-platform: add init() method to platform data

[Sergei Shtylyov]

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

Re: [PATCH 2/8] ehci-platform: add init() method to platform data

[Alan Stern]

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

Re: [PATCH 2/8] ehci-platform: add init() method to platform data

[Alan Stern]

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

Re: [PATCH 2/8] ehci-platform: add init() method to platform data

[Sergei Shtylyov]

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

Re: [PATCH 2/8] ehci-platform: add init() method to platform data

[Alan Stern]

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