[PATCH 1/2] EHCI : introduce a common ehci_setup

[PATCH 1/2] EHCI : introduce a common ehci_setup

[Matthieu CASTET]

This allow to clean duplicated code in most of SOC driver.

Signed-off-by: Matthieu CASTET <castet.matthieu@free.fr>
---
 drivers/usb/host/ehci-hcd.c |   29 +++++++++++++++++++++++++++++
 1 files changed, 29 insertions(+), 0 deletions(-)

diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index f8030ee..f990b0f 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -763,6 +763,35 @@ static int ehci_run (struct usb_hcd *hcd)
 	return 0;
 }
 
+static int __maybe_unused ehci_setup (struct usb_hcd *hcd)
+{
+	struct ehci_hcd *ehci = hcd_to_ehci(hcd);
+	int retval;
+
+	ehci->regs = (void __iomem *)ehci->caps +
+	    HC_LENGTH(ehci, ehci_readl(ehci, &ehci->caps->hc_capbase));
+	dbg_hcs_params(ehci, "reset");
+	dbg_hcc_params(ehci, "reset");
+
+	/* cache this readonly data; minimize chip reads */
+	ehci->hcs_params = ehci_readl(ehci, &ehci->caps->hcs_params);
+
+	ehci->sbrn = HCD_USB2;
+
+	retval = ehci_halt(ehci);
+	if (retval)
+		return retval;
+
+	/* data structure init */
+	retval = ehci_init(hcd);
+	if (retval)
+		return retval;
+
+	ehci_reset(ehci);
+
+	return 0;
+}
+
 /*-------------------------------------------------------------------------*/
 
 static irqreturn_t ehci_irq (struct usb_hcd *hcd)
-- 
1.7.5.4

[PATCH 2/2] ehci-msm : use ehci_setup

[Matthieu CASTET]

Signed-off-by: Matthieu CASTET <castet.matthieu@free.fr>
Tested-by: Pavankumar Kondeti <pkondeti@codeaurora.org>
---
 drivers/usb/host/ehci-msm.c |   20 +-------------------
 1 files changed, 1 insertions(+), 19 deletions(-)

diff --git a/drivers/usb/host/ehci-msm.c b/drivers/usb/host/ehci-msm.c
index b5a0bf6..592d5f7 100644
--- a/drivers/usb/host/ehci-msm.c
+++ b/drivers/usb/host/ehci-msm.c
@@ -40,27 +40,9 @@ static int ehci_msm_reset(struct usb_hcd *hcd)
 	int retval;
 
 	ehci->caps = USB_CAPLENGTH;
-	ehci->regs = USB_CAPLENGTH +
-		HC_LENGTH(ehci, ehci_readl(ehci, &ehci->caps->hc_capbase));
-	dbg_hcs_params(ehci, "reset");
-	dbg_hcc_params(ehci, "reset");
-
-	/* cache the data to minimize the chip reads*/
-	ehci->hcs_params = ehci_readl(ehci, &ehci->caps->hcs_params);
-
 	hcd->has_tt = 1;
-	ehci->sbrn = HCD_USB2;
-
-	retval = ehci_halt(ehci);
-	if (retval)
-		return retval;
-
-	/* data structure init */
-	retval = ehci_init(hcd);
-	if (retval)
-		return retval;
 
-	retval = ehci_reset(ehci);
+	retval = ehci_setup(hcd);
 	if (retval)
 		return retval;
 
-- 
1.7.5.4

Re: [PATCH 1/2] EHCI : introduce a common ehci_setup

[Alan Stern]

On Sat, 2 Jul 2011, Matthieu CASTET wrote:

> This allow to clean duplicated code in most of SOC driver.
> 
> Signed-off-by: Matthieu CASTET <castet.matthieu@free.fr>
> ---
>  drivers/usb/host/ehci-hcd.c |   29 +++++++++++++++++++++++++++++
>  1 files changed, 29 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
> index f8030ee..f990b0f 100644
> --- a/drivers/usb/host/ehci-hcd.c
> +++ b/drivers/usb/host/ehci-hcd.c
> @@ -763,6 +763,35 @@ static int ehci_run (struct usb_hcd *hcd)
>  	return 0;
>  }
>  
> +static int __maybe_unused ehci_setup (struct usb_hcd *hcd)
> +{
> +	struct ehci_hcd *ehci = hcd_to_ehci(hcd);
> +	int retval;

...

> +}
> +

Instead of putting this routine in ehci-hcd.c, where it will silently 
use up unnecessary codespace on thousands or millions of desktop 
systems, why not put it into a new separate source file?  Then the SOC 
drivers can #include that file.

Alan Stern

Re: [PATCH 1/2] EHCI : introduce a common ehci_setup

[matthieu castet]

Le Sat, 2 Jul 2011 16:35:15 -0400 (EDT),
Alan Stern <stern@rowland.harvard.edu> a écrit :

> On Sat, 2 Jul 2011, Matthieu CASTET wrote:
> 
> > This allow to clean duplicated code in most of SOC driver.
> > 
> > Signed-off-by: Matthieu CASTET <castet.matthieu@free.fr>
> > ---
> >  drivers/usb/host/ehci-hcd.c |   29 +++++++++++++++++++++++++++++
> >  1 files changed, 29 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/usb/host/ehci-hcd.c
> > b/drivers/usb/host/ehci-hcd.c index f8030ee..f990b0f 100644
> > --- a/drivers/usb/host/ehci-hcd.c
> > +++ b/drivers/usb/host/ehci-hcd.c
> > @@ -763,6 +763,35 @@ static int ehci_run (struct usb_hcd *hcd)
> >  	return 0;
> >  }
> >  
> > +static int __maybe_unused ehci_setup (struct usb_hcd *hcd)
> > +{
> > +	struct ehci_hcd *ehci = hcd_to_ehci(hcd);
> > +	int retval;
> 
> ...
> 
> > +}
> > +
> 
> Instead of putting this routine in ehci-hcd.c, where it will silently 
> use up unnecessary codespace on thousands or millions of desktop 
> systems, why not put it into a new separate source file?  Then the
> SOC drivers can #include that file.
Ok if you prefer that.
But if we allow someday to built more than one
SOC driver at a time (this will come for distribution doing generic arm
kernel or case like [1]). How it will work ? 

Also because the function is static, the compiler will remove
it, if it is not used. So it won't use extra codespace on desktop.


Matthieu


[1] http://marc.info/?l=linux-usb&m=130831671316362&w=2

Re: [PATCH 1/2] EHCI : introduce a common ehci_setup

[Alan Stern]

On Sun, 3 Jul 2011, matthieu castet wrote:

> > Instead of putting this routine in ehci-hcd.c, where it will silently 
> > use up unnecessary codespace on thousands or millions of desktop 
> > systems, why not put it into a new separate source file?  Then the
> > SOC drivers can #include that file.
> Ok if you prefer that.
> But if we allow someday to built more than one
> SOC driver at a time (this will come for distribution doing generic arm
> kernel or case like [1]). How it will work ? 

Good point.

> Also because the function is static, the compiler will remove
> it, if it is not used. So it won't use extra codespace on desktop.

Ah, I hadn't thought of that.  Thanks for pointing it out.  Okay, you 
can add my Acked-by to this patch.

Alan Stern