linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] usb: host: tegra: code clean up
@ 2012-09-12  7:02 Venu Byravarasu
  2012-09-12 13:51 ` Felipe Balbi
  2012-09-12 18:11 ` Stephen Warren
  0 siblings, 2 replies; 7+ messages in thread
From: Venu Byravarasu @ 2012-09-12  7:02 UTC (permalink / raw)
  To: balbi; +Cc: linux-kernel, linux-usb, Venu Byravarasu

As part of code clean up, used devm counterparts for the APIs
possible.

Signed-off-by: Venu Byravarasu <vbyravarasu@nvidia.com>
---
 drivers/usb/host/ehci-tegra.c |   46 ++++++++++++++--------------------------
 1 files changed, 16 insertions(+), 30 deletions(-)

diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c
index c0d4732..532db04 100644
--- a/drivers/usb/host/ehci-tegra.c
+++ b/drivers/usb/host/ehci-tegra.c
@@ -634,7 +634,8 @@ static int tegra_ehci_probe(struct platform_device *pdev)
 
 	setup_vbus_gpio(pdev, pdata);
 
-	tegra = kzalloc(sizeof(struct tegra_ehci_hcd), GFP_KERNEL);
+	tegra = devm_kzalloc(&pdev->dev, sizeof(struct tegra_ehci_hcd),
+			     GFP_KERNEL);
 	if (!tegra)
 		return -ENOMEM;
 
@@ -642,13 +643,12 @@ static int tegra_ehci_probe(struct platform_device *pdev)
 					dev_name(&pdev->dev));
 	if (!hcd) {
 		dev_err(&pdev->dev, "Unable to create HCD\n");
-		err = -ENOMEM;
-		goto fail_hcd;
+		return -ENOMEM;
 	}
 
 	platform_set_drvdata(pdev, tegra);
 
-	tegra->clk = clk_get(&pdev->dev, NULL);
+	tegra->clk = devm_clk_get(&pdev->dev, NULL);
 	if (IS_ERR(tegra->clk)) {
 		dev_err(&pdev->dev, "Can't get ehci clock\n");
 		err = PTR_ERR(tegra->clk);
@@ -657,9 +657,9 @@ static int tegra_ehci_probe(struct platform_device *pdev)
 
 	err = clk_prepare_enable(tegra->clk);
 	if (err)
-		goto fail_clken;
+		goto fail_clk;
 
-	tegra->emc_clk = clk_get(&pdev->dev, "emc");
+	tegra->emc_clk = devm_clk_get(&pdev->dev, "emc");
 	if (IS_ERR(tegra->emc_clk)) {
 		dev_err(&pdev->dev, "Can't get emc clock\n");
 		err = PTR_ERR(tegra->emc_clk);
@@ -677,7 +677,7 @@ static int tegra_ehci_probe(struct platform_device *pdev)
 	}
 	hcd->rsrc_start = res->start;
 	hcd->rsrc_len = resource_size(res);
-	hcd->regs = ioremap(res->start, resource_size(res));
+	hcd->regs = devm_ioremap(&pdev->dev, res->start, resource_size(res));
 	if (!hcd->regs) {
 		dev_err(&pdev->dev, "Failed to remap I/O memory\n");
 		err = -ENOMEM;
@@ -701,8 +701,8 @@ static int tegra_ehci_probe(struct platform_device *pdev)
 			break;
 		default:
 			err = -ENODEV;
-			dev_err(&pdev->dev, "unknown usb instance\n");
-			goto fail_phy;
+			dev_err(&pdev->dev, "unknown usb inst:%d\n", instance);
+			goto fail_io;
 		}
 	}
 
@@ -712,7 +712,7 @@ static int tegra_ehci_probe(struct platform_device *pdev)
 	if (IS_ERR(tegra->phy)) {
 		dev_err(&pdev->dev, "Failed to open USB phy\n");
 		err = -ENXIO;
-		goto fail_phy;
+		goto fail_io;
 	}
 
 	usb_phy_init(&tegra->phy->u_phy);
@@ -735,7 +735,8 @@ static int tegra_ehci_probe(struct platform_device *pdev)
 
 #ifdef CONFIG_USB_OTG_UTILS
 	if (pdata->operating_mode == TEGRA_USB_OTG) {
-		tegra->transceiver = usb_get_phy(USB_PHY_TYPE_USB2);
+		tegra->transceiver =
+			devm_usb_get_phy(&pdev->dev, USB_PHY_TYPE_USB2);
 		if (!IS_ERR_OR_NULL(tegra->transceiver))
 			otg_set_host(tegra->transceiver->otg, &hcd->self);
 	}
@@ -743,7 +744,7 @@ static int tegra_ehci_probe(struct platform_device *pdev)
 
 	err = usb_add_hcd(hcd, irq, IRQF_SHARED);
 	if (err) {
-		dev_err(&pdev->dev, "Failed to add USB HCD\n");
+		dev_err(&pdev->dev, "usb_add_hcd failed with err 0x%x\n", err);
 		goto fail;
 	}
 
@@ -752,32 +753,23 @@ static int tegra_ehci_probe(struct platform_device *pdev)
 
 	/* Don't skip the pm_runtime_forbid call if wakeup isn't working */
 	/* if (!pdata->power_down_on_bus_suspend) */
-		pm_runtime_forbid(&pdev->dev);
+	pm_runtime_forbid(&pdev->dev);
 	pm_runtime_enable(&pdev->dev);
 	pm_runtime_put_sync(&pdev->dev);
 	return err;
 
 fail:
 #ifdef CONFIG_USB_OTG_UTILS
-	if (!IS_ERR_OR_NULL(tegra->transceiver)) {
+	if (!IS_ERR_OR_NULL(tegra->transceiver))
 		otg_set_host(tegra->transceiver->otg, NULL);
-		usb_put_phy(tegra->transceiver);
-	}
 #endif
 	usb_phy_shutdown(&tegra->phy->u_phy);
-fail_phy:
-	iounmap(hcd->regs);
 fail_io:
 	clk_disable_unprepare(tegra->emc_clk);
-	clk_put(tegra->emc_clk);
 fail_emc_clk:
 	clk_disable_unprepare(tegra->clk);
-fail_clken:
-	clk_put(tegra->clk);
 fail_clk:
 	usb_put_hcd(hcd);
-fail_hcd:
-	kfree(tegra);
 	return err;
 }
 
@@ -794,26 +786,20 @@ static int tegra_ehci_remove(struct platform_device *pdev)
 	pm_runtime_put_noidle(&pdev->dev);
 
 #ifdef CONFIG_USB_OTG_UTILS
-	if (!IS_ERR_OR_NULL(tegra->transceiver)) {
+	if (!IS_ERR_OR_NULL(tegra->transceiver))
 		otg_set_host(tegra->transceiver->otg, NULL);
-		usb_put_phy(tegra->transceiver);
-	}
 #endif
 
 	usb_remove_hcd(hcd);
 
 	usb_phy_shutdown(&tegra->phy->u_phy);
-	iounmap(hcd->regs);
 
 	usb_put_hcd(hcd);
 
 	clk_disable_unprepare(tegra->clk);
-	clk_put(tegra->clk);
 
 	clk_disable_unprepare(tegra->emc_clk);
-	clk_put(tegra->emc_clk);
 
-	kfree(tegra);
 	return 0;
 }
 
-- 
1.7.1.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] usb: host: tegra: code clean up
  2012-09-12  7:02 [PATCH] usb: host: tegra: code clean up Venu Byravarasu
@ 2012-09-12 13:51 ` Felipe Balbi
  2012-09-12 18:11 ` Stephen Warren
  1 sibling, 0 replies; 7+ messages in thread
From: Felipe Balbi @ 2012-09-12 13:51 UTC (permalink / raw)
  To: Venu Byravarasu; +Cc: balbi, linux-kernel, linux-usb, Alan Stern

[-- Attachment #1: Type: text/plain, Size: 5285 bytes --]

Hi,

On Wed, Sep 12, 2012 at 12:32:42PM +0530, Venu Byravarasu wrote:
> As part of code clean up, used devm counterparts for the APIs
> possible.
> 
> Signed-off-by: Venu Byravarasu <vbyravarasu@nvidia.com>

EHCI is Alan's domain.

> ---
>  drivers/usb/host/ehci-tegra.c |   46 ++++++++++++++--------------------------
>  1 files changed, 16 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c
> index c0d4732..532db04 100644
> --- a/drivers/usb/host/ehci-tegra.c
> +++ b/drivers/usb/host/ehci-tegra.c
> @@ -634,7 +634,8 @@ static int tegra_ehci_probe(struct platform_device *pdev)
>  
>  	setup_vbus_gpio(pdev, pdata);
>  
> -	tegra = kzalloc(sizeof(struct tegra_ehci_hcd), GFP_KERNEL);
> +	tegra = devm_kzalloc(&pdev->dev, sizeof(struct tegra_ehci_hcd),
> +			     GFP_KERNEL);
>  	if (!tegra)
>  		return -ENOMEM;
>  
> @@ -642,13 +643,12 @@ static int tegra_ehci_probe(struct platform_device *pdev)
>  					dev_name(&pdev->dev));
>  	if (!hcd) {
>  		dev_err(&pdev->dev, "Unable to create HCD\n");
> -		err = -ENOMEM;
> -		goto fail_hcd;
> +		return -ENOMEM;
>  	}
>  
>  	platform_set_drvdata(pdev, tegra);
>  
> -	tegra->clk = clk_get(&pdev->dev, NULL);
> +	tegra->clk = devm_clk_get(&pdev->dev, NULL);
>  	if (IS_ERR(tegra->clk)) {
>  		dev_err(&pdev->dev, "Can't get ehci clock\n");
>  		err = PTR_ERR(tegra->clk);
> @@ -657,9 +657,9 @@ static int tegra_ehci_probe(struct platform_device *pdev)
>  
>  	err = clk_prepare_enable(tegra->clk);
>  	if (err)
> -		goto fail_clken;
> +		goto fail_clk;
>  
> -	tegra->emc_clk = clk_get(&pdev->dev, "emc");
> +	tegra->emc_clk = devm_clk_get(&pdev->dev, "emc");
>  	if (IS_ERR(tegra->emc_clk)) {
>  		dev_err(&pdev->dev, "Can't get emc clock\n");
>  		err = PTR_ERR(tegra->emc_clk);
> @@ -677,7 +677,7 @@ static int tegra_ehci_probe(struct platform_device *pdev)
>  	}
>  	hcd->rsrc_start = res->start;
>  	hcd->rsrc_len = resource_size(res);
> -	hcd->regs = ioremap(res->start, resource_size(res));
> +	hcd->regs = devm_ioremap(&pdev->dev, res->start, resource_size(res));
>  	if (!hcd->regs) {
>  		dev_err(&pdev->dev, "Failed to remap I/O memory\n");
>  		err = -ENOMEM;
> @@ -701,8 +701,8 @@ static int tegra_ehci_probe(struct platform_device *pdev)
>  			break;
>  		default:
>  			err = -ENODEV;
> -			dev_err(&pdev->dev, "unknown usb instance\n");
> -			goto fail_phy;
> +			dev_err(&pdev->dev, "unknown usb inst:%d\n", instance);
> +			goto fail_io;
>  		}
>  	}
>  
> @@ -712,7 +712,7 @@ static int tegra_ehci_probe(struct platform_device *pdev)
>  	if (IS_ERR(tegra->phy)) {
>  		dev_err(&pdev->dev, "Failed to open USB phy\n");
>  		err = -ENXIO;
> -		goto fail_phy;
> +		goto fail_io;
>  	}
>  
>  	usb_phy_init(&tegra->phy->u_phy);
> @@ -735,7 +735,8 @@ static int tegra_ehci_probe(struct platform_device *pdev)
>  
>  #ifdef CONFIG_USB_OTG_UTILS
>  	if (pdata->operating_mode == TEGRA_USB_OTG) {
> -		tegra->transceiver = usb_get_phy(USB_PHY_TYPE_USB2);
> +		tegra->transceiver =
> +			devm_usb_get_phy(&pdev->dev, USB_PHY_TYPE_USB2);
>  		if (!IS_ERR_OR_NULL(tegra->transceiver))
>  			otg_set_host(tegra->transceiver->otg, &hcd->self);
>  	}
> @@ -743,7 +744,7 @@ static int tegra_ehci_probe(struct platform_device *pdev)
>  
>  	err = usb_add_hcd(hcd, irq, IRQF_SHARED);
>  	if (err) {
> -		dev_err(&pdev->dev, "Failed to add USB HCD\n");
> +		dev_err(&pdev->dev, "usb_add_hcd failed with err 0x%x\n", err);
>  		goto fail;
>  	}
>  
> @@ -752,32 +753,23 @@ static int tegra_ehci_probe(struct platform_device *pdev)
>  
>  	/* Don't skip the pm_runtime_forbid call if wakeup isn't working */
>  	/* if (!pdata->power_down_on_bus_suspend) */
> -		pm_runtime_forbid(&pdev->dev);
> +	pm_runtime_forbid(&pdev->dev);
>  	pm_runtime_enable(&pdev->dev);
>  	pm_runtime_put_sync(&pdev->dev);
>  	return err;
>  
>  fail:
>  #ifdef CONFIG_USB_OTG_UTILS
> -	if (!IS_ERR_OR_NULL(tegra->transceiver)) {
> +	if (!IS_ERR_OR_NULL(tegra->transceiver))
>  		otg_set_host(tegra->transceiver->otg, NULL);
> -		usb_put_phy(tegra->transceiver);
> -	}
>  #endif
>  	usb_phy_shutdown(&tegra->phy->u_phy);
> -fail_phy:
> -	iounmap(hcd->regs);
>  fail_io:
>  	clk_disable_unprepare(tegra->emc_clk);
> -	clk_put(tegra->emc_clk);
>  fail_emc_clk:
>  	clk_disable_unprepare(tegra->clk);
> -fail_clken:
> -	clk_put(tegra->clk);
>  fail_clk:
>  	usb_put_hcd(hcd);
> -fail_hcd:
> -	kfree(tegra);
>  	return err;
>  }
>  
> @@ -794,26 +786,20 @@ static int tegra_ehci_remove(struct platform_device *pdev)
>  	pm_runtime_put_noidle(&pdev->dev);
>  
>  #ifdef CONFIG_USB_OTG_UTILS
> -	if (!IS_ERR_OR_NULL(tegra->transceiver)) {
> +	if (!IS_ERR_OR_NULL(tegra->transceiver))
>  		otg_set_host(tegra->transceiver->otg, NULL);
> -		usb_put_phy(tegra->transceiver);
> -	}
>  #endif
>  
>  	usb_remove_hcd(hcd);
>  
>  	usb_phy_shutdown(&tegra->phy->u_phy);
> -	iounmap(hcd->regs);
>  
>  	usb_put_hcd(hcd);
>  
>  	clk_disable_unprepare(tegra->clk);
> -	clk_put(tegra->clk);
>  
>  	clk_disable_unprepare(tegra->emc_clk);
> -	clk_put(tegra->emc_clk);
>  
> -	kfree(tegra);
>  	return 0;
>  }
>  
> -- 
> 1.7.1.1
> 

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] usb: host: tegra: code clean up
  2012-09-12  7:02 [PATCH] usb: host: tegra: code clean up Venu Byravarasu
  2012-09-12 13:51 ` Felipe Balbi
@ 2012-09-12 18:11 ` Stephen Warren
  2012-09-13  3:42   ` Venu Byravarasu
  1 sibling, 1 reply; 7+ messages in thread
From: Stephen Warren @ 2012-09-12 18:11 UTC (permalink / raw)
  To: Venu Byravarasu; +Cc: balbi, linux-kernel, linux-usb

On 09/12/2012 01:02 AM, Venu Byravarasu wrote:
> As part of code clean up, used devm counterparts for the APIs
> possible.

Almost all of this patch has already been applied as:

bc2ff98 drivers/usb/host/ehci-tegra.c: use devm_ functions

(btw, that patch has a much better patch subject than this one)

The only additions in your patch are shown below, and those changes
should indeed be a separate patch.

> diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c
> index 6223d17..dba9f07 100644
> --- a/drivers/usb/host/ehci-tegra.c
> +++ b/drivers/usb/host/ehci-tegra.c
> @@ -701,7 +701,7 @@ static int tegra_ehci_probe(struct platform_device *pdev)
>                         break;
>                 default:
>                         err = -ENODEV;
> -                       dev_err(&pdev->dev, "unknown usb instance\n");
> +                       dev_err(&pdev->dev, "unknown usb inst:%d\n", instance);
>                         goto fail_io;
>                 }
>         }
> @@ -744,7 +744,7 @@ static int tegra_ehci_probe(struct platform_device *pdev)
>  
>         err = usb_add_hcd(hcd, irq, IRQF_SHARED);
>         if (err) {
> -               dev_err(&pdev->dev, "Failed to add USB HCD\n");
> +               dev_err(&pdev->dev, "usb_add_hcd failed with err 0x%x\n", err);
>                 goto fail;
>         }
>  
> @@ -753,7 +753,7 @@ static int tegra_ehci_probe(struct platform_device *pdev)
>  
>         /* Don't skip the pm_runtime_forbid call if wakeup isn't working */
>         /* if (!pdata->power_down_on_bus_suspend) */
> -               pm_runtime_forbid(&pdev->dev);
> +       pm_runtime_forbid(&pdev->dev);
>         pm_runtime_enable(&pdev->dev);
>         pm_runtime_put_sync(&pdev->dev);
>         return err;

I'm not sure that last change is worth making; hopefully, you'll fix the
bug the causes the "if" to be commented out, and we can re-enabled it
again. Removing the indent makes it much less obvious which lines of
code the "if" was intended to cover.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH] usb: host: tegra: code clean up
  2012-09-12 18:11 ` Stephen Warren
@ 2012-09-13  3:42   ` Venu Byravarasu
  2012-09-13  4:50     ` Stephen Warren
  0 siblings, 1 reply; 7+ messages in thread
From: Venu Byravarasu @ 2012-09-13  3:42 UTC (permalink / raw)
  To: Stephen Warren; +Cc: balbi, linux-kernel, linux-usb

> -----Original Message-----
> From: Stephen Warren [mailto:swarren@wwwdotorg.org]
> Sent: Wednesday, September 12, 2012 11:41 PM
> To: Venu Byravarasu
> Cc: balbi@ti.com; linux-kernel@vger.kernel.org; linux-usb@vger.kernel.org
> Subject: Re: [PATCH] usb: host: tegra: code clean up
> 
> On 09/12/2012 01:02 AM, Venu Byravarasu wrote:
> > As part of code clean up, used devm counterparts for the APIs
> > possible.
> 
> Almost all of this patch has already been applied as:

Agree. 
Currently Balbi's tree has bit old ehci-tegra.c.
Because of this the patches prepared with linux-next need to be rebased onto this tree and prepare a new patch.
My main intention behind pushing this patch was to get all changes of ehci-tegra.c from linux-next into balbi's code base so that I can push the same patch against either balbi's tree or linux-next.

> bc2ff98 drivers/usb/host/ehci-tegra.c: use devm_ functions
> 
> (btw, that patch has a much better patch subject than this one)
> 
> The only additions in your patch are shown below, and those changes
> should indeed be a separate patch.
> 
> > diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c
> > index 6223d17..dba9f07 100644
> > --- a/drivers/usb/host/ehci-tegra.c
> > +++ b/drivers/usb/host/ehci-tegra.c
> > @@ -701,7 +701,7 @@ static int tegra_ehci_probe(struct platform_device
> *pdev)
> >                         break;
> >                 default:
> >                         err = -ENODEV;
> > -                       dev_err(&pdev->dev, "unknown usb instance\n");
> > +                       dev_err(&pdev->dev, "unknown usb inst:%d\n", instance);
> >                         goto fail_io;
> >                 }
> >         }
> > @@ -744,7 +744,7 @@ static int tegra_ehci_probe(struct platform_device
> *pdev)
> >
> >         err = usb_add_hcd(hcd, irq, IRQF_SHARED);
> >         if (err) {
> > -               dev_err(&pdev->dev, "Failed to add USB HCD\n");
> > +               dev_err(&pdev->dev, "usb_add_hcd failed with err 0x%x\n", err);
> >                 goto fail;
> >         }
> >
> > @@ -753,7 +753,7 @@ static int tegra_ehci_probe(struct platform_device
> *pdev)
> >
> >         /* Don't skip the pm_runtime_forbid call if wakeup isn't working */
> >         /* if (!pdata->power_down_on_bus_suspend) */
> > -               pm_runtime_forbid(&pdev->dev);
> > +       pm_runtime_forbid(&pdev->dev);
> >         pm_runtime_enable(&pdev->dev);
> >         pm_runtime_put_sync(&pdev->dev);
> >         return err;
> 
> I'm not sure that last change is worth making; hopefully, you'll fix the
> bug the causes the "if" to be commented out, and we can re-enabled it
> again. Removing the indent makes it much less obvious which lines of
> code the "if" was intended to cover.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] usb: host: tegra: code clean up
  2012-09-13  3:42   ` Venu Byravarasu
@ 2012-09-13  4:50     ` Stephen Warren
  0 siblings, 0 replies; 7+ messages in thread
From: Stephen Warren @ 2012-09-13  4:50 UTC (permalink / raw)
  To: Venu Byravarasu; +Cc: balbi, linux-kernel, linux-usb

On 09/12/2012 09:42 PM, Venu Byravarasu wrote:
> Stephen Warren wrote at Wednesday, September 12, 2012 11:41 PM:
>> On 09/12/2012 01:02 AM, Venu Byravarasu wrote:
>>> As part of code clean up, used devm counterparts for the APIs
>>> possible.
>>
>> Almost all of this patch has already been applied as:
> 
> Agree. 
> Currently Balbi's tree has bit old ehci-tegra.c.

Quite possibly. That would happen if patches to ehci-tegra.c were
checked into other branches, which have not yet been merged to Linus's
tree, and hence have not made it into the baseline for Felipe's tree.
This is especially likely as ehci-tegra.c isn't something that Felipe's
xceiv branch would usually take patches for IIRC.

> Because of this the patches prepared with linux-next need to be rebased onto this tree and prepare a new patch.
> My main intention behind pushing this patch was to get all changes of ehci-tegra.c from linux-next into balbi's code base so that I can push the same patch against either balbi's tree or linux-next.

That's not the way the Linux branching model works. The primary way for
Felipe's branch to pick up changes from another branch is for the other
branch to be merged by Linus, and then Felipe to either merge Linus's
branch into his, or start a new branch based on a more recent tag from
Linus's tree. If you send patches to Felipe that duplicate work that's
already happened in another branch, you'll most likely end up causing
merge conflicts when Felipe's branch is merged with the other branch
containing the same work in linux-next, Greg's USB tree, and Linus's
tree. Of course, you might get lucky and avoid conflicts if the patches
are identical, but duplicate patches are still not a good idea.

It's best if you send patches that apply and operate correctly on one
particular branch, e.g. just Felipe's or some other USB
(sub-)maintainer's branch.

If your patches actually rely on changes from multiple different
branches, then that is problematic. The simplest answer is to simply
wait for the (end of the) next kernel merge window when everything has
been merged together, and then start sending patches based on the merged
result.

In some cases, branches can be merged together other than by Linus,
although you do have to be quite careful to avoid pain when doing this.
It's best to plan out what patches you'll send, where you expect they
will be applied, and point out any such dependencies to the maintainers
ahead of time. Developing all your big patches first and then sending
them, rather than developing them piecemeal, may help you plan this out
better, at least at first.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] usb: host: tegra: code clean up
  2012-04-05  5:55 Venu Byravarasu
@ 2012-04-05 14:03 ` Alan Stern
  0 siblings, 0 replies; 7+ messages in thread
From: Alan Stern @ 2012-04-05 14:03 UTC (permalink / raw)
  To: Venu Byravarasu; +Cc: gregkh, linux-usb, linux-kernel

On Thu, 5 Apr 2012, Venu Byravarasu wrote:

> With this patch:
> 	1. Renamed structure and function names to be more meaningful.
> 	2. Removed unnecessary local variables.
> 
> Signed-off-by: Venu Byravarasu <vbyravarasu@nvidia.com>

Acked-by: Alan Stern <stern@rowland.harvard.edu>


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH] usb: host: tegra: code clean up
@ 2012-04-05  5:55 Venu Byravarasu
  2012-04-05 14:03 ` Alan Stern
  0 siblings, 1 reply; 7+ messages in thread
From: Venu Byravarasu @ 2012-04-05  5:55 UTC (permalink / raw)
  To: stern, gregkh; +Cc: linux-usb, linux-kernel, Venu Byravarasu

With this patch:
	1. Renamed structure and function names to be more meaningful.
	2. Removed unnecessary local variables.

Signed-off-by: Venu Byravarasu <vbyravarasu@nvidia.com>
---
 drivers/usb/host/ehci-tegra.c |   35 ++++++++++++++---------------------
 1 files changed, 14 insertions(+), 21 deletions(-)

diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c
index 96c942c..65f4283 100644
--- a/drivers/usb/host/ehci-tegra.c
+++ b/drivers/usb/host/ehci-tegra.c
@@ -462,26 +462,23 @@ static int tegra_ehci_bus_resume(struct usb_hcd *hcd)
 }
 #endif
 
-struct temp_buffer {
+struct dma_aligned_buffer {
 	void *kmalloc_ptr;
 	void *old_xfer_buffer;
 	u8 data[0];
 };
 
-static void free_temp_buffer(struct urb *urb)
+static void free_dma_aligned_buffer(struct urb *urb)
 {
-	enum dma_data_direction dir;
-	struct temp_buffer *temp;
+	struct dma_aligned_buffer *temp;
 
 	if (!(urb->transfer_flags & URB_ALIGNED_TEMP_BUFFER))
 		return;
 
-	dir = usb_urb_dir_in(urb) ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
+	temp = container_of(urb->transfer_buffer,
+		struct dma_aligned_buffer, data);
 
-	temp = container_of(urb->transfer_buffer, struct temp_buffer,
-			    data);
-
-	if (dir == DMA_FROM_DEVICE)
+	if (usb_urb_dir_in(urb))
 		memcpy(temp->old_xfer_buffer, temp->data,
 		       urb->transfer_buffer_length);
 	urb->transfer_buffer = temp->old_xfer_buffer;
@@ -490,10 +487,9 @@ static void free_temp_buffer(struct urb *urb)
 	urb->transfer_flags &= ~URB_ALIGNED_TEMP_BUFFER;
 }
 
-static int alloc_temp_buffer(struct urb *urb, gfp_t mem_flags)
+static int alloc_dma_aligned_buffer(struct urb *urb, gfp_t mem_flags)
 {
-	enum dma_data_direction dir;
-	struct temp_buffer *temp, *kmalloc_ptr;
+	struct dma_aligned_buffer *temp, *kmalloc_ptr;
 	size_t kmalloc_size;
 
 	if (urb->num_sgs || urb->sg ||
@@ -501,22 +497,19 @@ static int alloc_temp_buffer(struct urb *urb, gfp_t mem_flags)
 	    !((uintptr_t)urb->transfer_buffer & (TEGRA_USB_DMA_ALIGN - 1)))
 		return 0;
 
-	dir = usb_urb_dir_in(urb) ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
-
 	/* Allocate a buffer with enough padding for alignment */
 	kmalloc_size = urb->transfer_buffer_length +
-		sizeof(struct temp_buffer) + TEGRA_USB_DMA_ALIGN - 1;
+		sizeof(struct dma_aligned_buffer) + TEGRA_USB_DMA_ALIGN - 1;
 
 	kmalloc_ptr = kmalloc(kmalloc_size, mem_flags);
 	if (!kmalloc_ptr)
 		return -ENOMEM;
 
-	/* Position our struct temp_buffer such that data is aligned */
+	/* Position our struct dma_aligned_buffer such that data is aligned */
 	temp = PTR_ALIGN(kmalloc_ptr + 1, TEGRA_USB_DMA_ALIGN) - 1;
-
 	temp->kmalloc_ptr = kmalloc_ptr;
 	temp->old_xfer_buffer = urb->transfer_buffer;
-	if (dir == DMA_TO_DEVICE)
+	if (usb_urb_dir_out(urb))
 		memcpy(temp->data, urb->transfer_buffer,
 		       urb->transfer_buffer_length);
 	urb->transfer_buffer = temp->data;
@@ -531,13 +524,13 @@ static int tegra_ehci_map_urb_for_dma(struct usb_hcd *hcd, struct urb *urb,
 {
 	int ret;
 
-	ret = alloc_temp_buffer(urb, mem_flags);
+	ret = alloc_dma_aligned_buffer(urb, mem_flags);
 	if (ret)
 		return ret;
 
 	ret = usb_hcd_map_urb_for_dma(hcd, urb, mem_flags);
 	if (ret)
-		free_temp_buffer(urb);
+		free_dma_aligned_buffer(urb);
 
 	return ret;
 }
@@ -545,7 +538,7 @@ static int tegra_ehci_map_urb_for_dma(struct usb_hcd *hcd, struct urb *urb,
 static void tegra_ehci_unmap_urb_for_dma(struct usb_hcd *hcd, struct urb *urb)
 {
 	usb_hcd_unmap_urb_for_dma(hcd, urb);
-	free_temp_buffer(urb);
+	free_dma_aligned_buffer(urb);
 }
 
 static const struct hc_driver tegra_ehci_hc_driver = {
-- 
1.7.1.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2012-09-13  4:50 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-09-12  7:02 [PATCH] usb: host: tegra: code clean up Venu Byravarasu
2012-09-12 13:51 ` Felipe Balbi
2012-09-12 18:11 ` Stephen Warren
2012-09-13  3:42   ` Venu Byravarasu
2012-09-13  4:50     ` Stephen Warren
  -- strict thread matches above, loose matches on Subject: below --
2012-04-05  5:55 Venu Byravarasu
2012-04-05 14:03 ` Alan Stern

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).