[PATCH v2] usb: host: tegra: make use of PHY pointer of HCD

[PATCH v2] usb: host: tegra: make use of PHY pointer of HCD

[Venu Byravarasu]

As pointer to PHY structure can be stored in struct usb_hcd
making use of it, to call Tegra PHY APIs.

Call to usb_phy_shutdown() is moved up in tegra_ehci_remove(),
so that to avoid dereferencing of hcd after its freed up.

Signed-off-by: Venu Byravarasu <vbyravarasu@nvidia.com>
---
First version of patch can be found at
http://marc.info/?l=linux-kernel&m=135581479020213&w=2. As it was not
yet applied, sending v2 of the patch.

delta from v1:
Modified all Tegra PHY APIs to use standard 'structu usb_phy *' as
their parameter instead of proprietary parameter of type 'struct tegra_usb_phy *'.



 drivers/usb/host/ehci-tegra.c     |   27 +++++++++++++--------------
 drivers/usb/phy/tegra_usb_phy.c   |   16 ++++++++++++----
 include/linux/usb/tegra_usb_phy.h |    8 ++++----
 3 files changed, 29 insertions(+), 22 deletions(-)

diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c
index b02622a..568aecc 100644
--- a/drivers/usb/host/ehci-tegra.c
+++ b/drivers/usb/host/ehci-tegra.c
@@ -56,7 +56,7 @@ static void tegra_ehci_power_up(struct usb_hcd *hcd)
 	struct tegra_ehci_hcd *tegra = dev_get_drvdata(hcd->self.controller);
 
 	clk_prepare_enable(tegra->clk);
-	usb_phy_set_suspend(&tegra->phy->u_phy, 0);
+	usb_phy_set_suspend(hcd->phy, 0);
 	tegra->host_resumed = 1;
 }
 
@@ -65,7 +65,7 @@ static void tegra_ehci_power_down(struct usb_hcd *hcd)
 	struct tegra_ehci_hcd *tegra = dev_get_drvdata(hcd->self.controller);
 
 	tegra->host_resumed = 0;
-	usb_phy_set_suspend(&tegra->phy->u_phy, 1);
+	usb_phy_set_suspend(hcd->phy, 1);
 	clk_disable_unprepare(tegra->clk);
 }
 
@@ -159,7 +159,7 @@ static int tegra_ehci_hub_control(
 		if (tegra->port_resuming && !(temp & PORT_SUSPEND)) {
 			/* Resume completed, re-enable disconnect detection */
 			tegra->port_resuming = 0;
-			tegra_usb_phy_postresume(tegra->phy);
+			tegra_usb_phy_postresume(hcd->phy);
 		}
 	}
 
@@ -212,7 +212,7 @@ static int tegra_ehci_hub_control(
 			goto done;
 
 		/* Disable disconnect detection during port resume */
-		tegra_usb_phy_preresume(tegra->phy);
+		tegra_usb_phy_preresume(hcd->phy);
 
 		ehci->reset_done[wIndex-1] = jiffies + msecs_to_jiffies(25);
 
@@ -476,7 +476,7 @@ static int controller_resume(struct device *dev)
 	}
 
 	/* Force the phy to keep data lines in suspend state */
-	tegra_ehci_phy_restore_start(tegra->phy, tegra->port_speed);
+	tegra_ehci_phy_restore_start(hcd->phy, tegra->port_speed);
 
 	/* Enable host mode */
 	tdi_reset(ehci);
@@ -543,17 +543,17 @@ static int controller_resume(struct device *dev)
 		}
 	}
 
-	tegra_ehci_phy_restore_end(tegra->phy);
+	tegra_ehci_phy_restore_end(hcd->phy);
 	goto done;
 
  restart:
 	if (tegra->port_speed <= TEGRA_USB_PHY_PORT_SPEED_HIGH)
-		tegra_ehci_phy_restore_end(tegra->phy);
+		tegra_ehci_phy_restore_end(hcd->phy);
 
 	tegra_ehci_restart(hcd);
 
  done:
-	tegra_usb_phy_preresume(tegra->phy);
+	tegra_usb_phy_preresume(hcd->phy);
 	tegra->port_resuming = 1;
 	return 0;
 }
@@ -740,9 +740,9 @@ static int tegra_ehci_probe(struct platform_device *pdev)
 		goto fail_io;
 	}
 
-	usb_phy_init(&tegra->phy->u_phy);
-
 	hcd->phy = u_phy = &tegra->phy->u_phy;
+	usb_phy_init(hcd->phy);
+
 	u_phy->otg = devm_kzalloc(&pdev->dev, sizeof(struct usb_otg),
 			     GFP_KERNEL);
 	if (!u_phy->otg) {
@@ -752,7 +752,7 @@ static int tegra_ehci_probe(struct platform_device *pdev)
 	}
 	u_phy->otg->host = hcd_to_bus(hcd);
 
-	err = usb_phy_set_suspend(&tegra->phy->u_phy, 0);
+	err = usb_phy_set_suspend(hcd->phy, 0);
 	if (err) {
 		dev_err(&pdev->dev, "Failed to power on the phy\n");
 		goto fail;
@@ -798,7 +798,7 @@ fail:
 	if (!IS_ERR_OR_NULL(tegra->transceiver))
 		otg_set_host(tegra->transceiver->otg, NULL);
 #endif
-	usb_phy_shutdown(&tegra->phy->u_phy);
+	usb_phy_shutdown(hcd->phy);
 fail_io:
 	clk_disable_unprepare(tegra->clk);
 fail_clk:
@@ -820,11 +820,10 @@ static int tegra_ehci_remove(struct platform_device *pdev)
 		otg_set_host(tegra->transceiver->otg, NULL);
 #endif
 
+	usb_phy_shutdown(hcd->phy);
 	usb_remove_hcd(hcd);
 	usb_put_hcd(hcd);
 
-	usb_phy_shutdown(&tegra->phy->u_phy);
-
 	clk_disable_unprepare(tegra->clk);
 
 	return 0;
diff --git a/drivers/usb/phy/tegra_usb_phy.c b/drivers/usb/phy/tegra_usb_phy.c
index d465704..5487d38 100644
--- a/drivers/usb/phy/tegra_usb_phy.c
+++ b/drivers/usb/phy/tegra_usb_phy.c
@@ -759,30 +759,38 @@ err0:
 }
 EXPORT_SYMBOL_GPL(tegra_usb_phy_open);
 
-void tegra_usb_phy_preresume(struct tegra_usb_phy *phy)
+void tegra_usb_phy_preresume(struct usb_phy *x)
 {
+	struct tegra_usb_phy *phy = container_of(x, struct tegra_usb_phy, u_phy);
+
 	if (!phy->is_ulpi_phy)
 		utmi_phy_preresume(phy);
 }
 EXPORT_SYMBOL_GPL(tegra_usb_phy_preresume);
 
-void tegra_usb_phy_postresume(struct tegra_usb_phy *phy)
+void tegra_usb_phy_postresume(struct usb_phy *x)
 {
+	struct tegra_usb_phy *phy = container_of(x, struct tegra_usb_phy, u_phy);
+
 	if (!phy->is_ulpi_phy)
 		utmi_phy_postresume(phy);
 }
 EXPORT_SYMBOL_GPL(tegra_usb_phy_postresume);
 
-void tegra_ehci_phy_restore_start(struct tegra_usb_phy *phy,
+void tegra_ehci_phy_restore_start(struct usb_phy *x,
 				 enum tegra_usb_phy_port_speed port_speed)
 {
+	struct tegra_usb_phy *phy = container_of(x, struct tegra_usb_phy, u_phy);
+
 	if (!phy->is_ulpi_phy)
 		utmi_phy_restore_start(phy, port_speed);
 }
 EXPORT_SYMBOL_GPL(tegra_ehci_phy_restore_start);
 
-void tegra_ehci_phy_restore_end(struct tegra_usb_phy *phy)
+void tegra_ehci_phy_restore_end(struct usb_phy *x)
 {
+	struct tegra_usb_phy *phy = container_of(x, struct tegra_usb_phy, u_phy);
+
 	if (!phy->is_ulpi_phy)
 		utmi_phy_restore_end(phy);
 }
diff --git a/include/linux/usb/tegra_usb_phy.h b/include/linux/usb/tegra_usb_phy.h
index 9259d46..9ebebe9 100644
--- a/include/linux/usb/tegra_usb_phy.h
+++ b/include/linux/usb/tegra_usb_phy.h
@@ -66,14 +66,14 @@ struct tegra_usb_phy {
 struct tegra_usb_phy *tegra_usb_phy_open(struct device *dev, int instance,
 	void __iomem *regs, void *config, enum tegra_usb_phy_mode phy_mode);
 
-void tegra_usb_phy_preresume(struct tegra_usb_phy *phy);
+void tegra_usb_phy_preresume(struct usb_phy *phy);
 
-void tegra_usb_phy_postresume(struct tegra_usb_phy *phy);
+void tegra_usb_phy_postresume(struct usb_phy *phy);
 
-void tegra_ehci_phy_restore_start(struct tegra_usb_phy *phy,
+void tegra_ehci_phy_restore_start(struct usb_phy *phy,
 				 enum tegra_usb_phy_port_speed port_speed);
 
-void tegra_ehci_phy_restore_end(struct tegra_usb_phy *phy);
+void tegra_ehci_phy_restore_end(struct usb_phy *phy);
 
 void tegra_ehci_set_pts(struct usb_phy *x, u8 pts_val);
 
-- 
1.7.0.4

Re: [PATCH v2] usb: host: tegra: make use of PHY pointer of HCD

[Felipe Balbi]

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

Hi,

On Thu, Jan 24, 2013 at 03:57:03PM +0530, Venu Byravarasu wrote:
> As pointer to PHY structure can be stored in struct usb_hcd
> making use of it, to call Tegra PHY APIs.
> 
> Call to usb_phy_shutdown() is moved up in tegra_ehci_remove(),
> so that to avoid dereferencing of hcd after its freed up.
> 
> Signed-off-by: Venu Byravarasu <vbyravarasu@nvidia.com>
> ---
> First version of patch can be found at
> http://marc.info/?l=linux-kernel&m=135581479020213&w=2. As it was not
> yet applied, sending v2 of the patch.
> 
> delta from v1:
> Modified all Tegra PHY APIs to use standard 'structu usb_phy *' as
> their parameter instead of proprietary parameter of type 'struct tegra_usb_phy *'.

I would like to see those tegra-specific functions vanish from the
driver. Can that be done for v3.10 merge window ?

-- 
balbi

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

Re: [PATCH v2] usb: host: tegra: make use of PHY pointer of HCD

[BVG Rao]

On Thu, Jan 24, 2013 at 7:35 PM, Felipe Balbi <balbi@ti.com> wrote:
> Hi,
>
> On Thu, Jan 24, 2013 at 03:57:03PM +0530, Venu Byravarasu wrote:
>> As pointer to PHY structure can be stored in struct usb_hcd
>> making use of it, to call Tegra PHY APIs.
>>
>> Call to usb_phy_shutdown() is moved up in tegra_ehci_remove(),
>> so that to avoid dereferencing of hcd after its freed up.
>>
>> Signed-off-by: Venu Byravarasu <vbyravarasu@nvidia.com>
>> ---
>> First version of patch can be found at
>> http://marc.info/?l=linux-kernel&m=135581479020213&w=2. As it was not
>> yet applied, sending v2 of the patch.
>>
>> delta from v1:
>> Modified all Tegra PHY APIs to use standard 'structu usb_phy *' as
>> their parameter instead of proprietary parameter of type 'struct tegra_usb_phy *'.
>
> I would like to see those tegra-specific functions vanish from the
> driver. Can that be done for v3.10 merge window ?

Deleting these functions may not be possible because of the way,
Tegra20 USB PHY and controllers are designed.
Hence I'm trying to remove tegra specific structure pointers as
function parameter
and use generic USB PHY structure pointers instead.

>
> --
> balbi

Re: [PATCH v2] usb: host: tegra: make use of PHY pointer of HCD

[Felipe Balbi]

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

On Thu, Jan 24, 2013 at 09:28:32PM +0530, BVG Rao wrote:
> On Thu, Jan 24, 2013 at 7:35 PM, Felipe Balbi <balbi@ti.com> wrote:
> > Hi,
> >
> > On Thu, Jan 24, 2013 at 03:57:03PM +0530, Venu Byravarasu wrote:
> >> As pointer to PHY structure can be stored in struct usb_hcd
> >> making use of it, to call Tegra PHY APIs.
> >>
> >> Call to usb_phy_shutdown() is moved up in tegra_ehci_remove(),
> >> so that to avoid dereferencing of hcd after its freed up.
> >>
> >> Signed-off-by: Venu Byravarasu <vbyravarasu@nvidia.com>
> >> ---
> >> First version of patch can be found at
> >> http://marc.info/?l=linux-kernel&m=135581479020213&w=2. As it was not
> >> yet applied, sending v2 of the patch.
> >>
> >> delta from v1:
> >> Modified all Tegra PHY APIs to use standard 'structu usb_phy *' as
> >> their parameter instead of proprietary parameter of type 'struct tegra_usb_phy *'.
> >
> > I would like to see those tegra-specific functions vanish from the
> > driver. Can that be done for v3.10 merge window ?
> 
> Deleting these functions may not be possible because of the way,
> Tegra20 USB PHY and controllers are designed.
> Hence I'm trying to remove tegra specific structure pointers as
> function parameter
> and use generic USB PHY structure pointers instead.

yes, you need to use generic function pointers, that's what I meant.

-- 
balbi

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

Re: [PATCH v2] usb: host: tegra: make use of PHY pointer of HCD

[Alan Stern]

On Thu, 24 Jan 2013, Venu Byravarasu wrote:

> As pointer to PHY structure can be stored in struct usb_hcd
> making use of it, to call Tegra PHY APIs.
> 
> Call to usb_phy_shutdown() is moved up in tegra_ehci_remove(),
> so that to avoid dereferencing of hcd after its freed up.
> 
> Signed-off-by: Venu Byravarasu <vbyravarasu@nvidia.com>
> ---
> First version of patch can be found at
> http://marc.info/?l=linux-kernel&m=135581479020213&w=2. As it was not
> yet applied, sending v2 of the patch.
> 
> delta from v1:
> Modified all Tegra PHY APIs to use standard 'structu usb_phy *' as
> their parameter instead of proprietary parameter of type 'struct tegra_usb_phy *'.

For ehci-tegra.c:

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

Re: [PATCH v2] usb: host: tegra: make use of PHY pointer of HCD

[Stephen Warren]

On 01/24/2013 06:05 AM, Felipe Balbi wrote:
> Hi,
> 
> On Thu, Jan 24, 2013 at 03:57:03PM +0530, Venu Byravarasu wrote:
>> As pointer to PHY structure can be stored in struct usb_hcd 
>> making use of it, to call Tegra PHY APIs.
>> 
>> Call to usb_phy_shutdown() is moved up in tegra_ehci_remove(), so
>> that to avoid dereferencing of hcd after its freed up.
>> 
>> Signed-off-by: Venu Byravarasu <vbyravarasu@nvidia.com> --- First
>> version of patch can be found at 
>> http://marc.info/?l=linux-kernel&m=135581479020213&w=2. As it was
>> not yet applied, sending v2 of the patch.
>> 
>> delta from v1: Modified all Tegra PHY APIs to use standard
>> 'structu usb_phy *' as their parameter instead of proprietary
>> parameter of type 'struct tegra_usb_phy *'.
> 
> I would like to see those tegra-specific functions vanish from the 
> driver. Can that be done for v3.10 merge window ?

Felipe, I assume that means you're OK with me taking this patch for
3.9 then?

Re: [PATCH v2] usb: host: tegra: make use of PHY pointer of HCD

[Felipe Balbi]

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

On Thu, Jan 24, 2013 at 08:57:01AM -0800, Stephen Warren wrote:
> On 01/24/2013 06:05 AM, Felipe Balbi wrote:
> > Hi,
> > 
> > On Thu, Jan 24, 2013 at 03:57:03PM +0530, Venu Byravarasu wrote:
> >> As pointer to PHY structure can be stored in struct usb_hcd 
> >> making use of it, to call Tegra PHY APIs.
> >> 
> >> Call to usb_phy_shutdown() is moved up in tegra_ehci_remove(), so
> >> that to avoid dereferencing of hcd after its freed up.
> >> 
> >> Signed-off-by: Venu Byravarasu <vbyravarasu@nvidia.com> --- First
> >> version of patch can be found at 
> >> http://marc.info/?l=linux-kernel&m=135581479020213&w=2. As it was
> >> not yet applied, sending v2 of the patch.
> >> 
> >> delta from v1: Modified all Tegra PHY APIs to use standard
> >> 'structu usb_phy *' as their parameter instead of proprietary
> >> parameter of type 'struct tegra_usb_phy *'.
> > 
> > I would like to see those tegra-specific functions vanish from the 
> > driver. Can that be done for v3.10 merge window ?
> 
> Felipe, I assume that means you're OK with me taking this patch for
> 3.9 then?

right,

Acked-by: Felipe Balbi <balbi@ti.com>

But I need to see those vanish on v3.10 ;-)

-- 
balbi

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

Re: [PATCH v2] usb: host: tegra: make use of PHY pointer of HCD

[Greg KH]

On Thu, Jan 24, 2013 at 08:57:01AM -0800, Stephen Warren wrote:
> On 01/24/2013 06:05 AM, Felipe Balbi wrote:
> > Hi,
> > 
> > On Thu, Jan 24, 2013 at 03:57:03PM +0530, Venu Byravarasu wrote:
> >> As pointer to PHY structure can be stored in struct usb_hcd 
> >> making use of it, to call Tegra PHY APIs.
> >> 
> >> Call to usb_phy_shutdown() is moved up in tegra_ehci_remove(), so
> >> that to avoid dereferencing of hcd after its freed up.
> >> 
> >> Signed-off-by: Venu Byravarasu <vbyravarasu@nvidia.com> --- First
> >> version of patch can be found at 
> >> http://marc.info/?l=linux-kernel&m=135581479020213&w=2. As it was
> >> not yet applied, sending v2 of the patch.
> >> 
> >> delta from v1: Modified all Tegra PHY APIs to use standard
> >> 'structu usb_phy *' as their parameter instead of proprietary
> >> parameter of type 'struct tegra_usb_phy *'.
> > 
> > I would like to see those tegra-specific functions vanish from the 
> > driver. Can that be done for v3.10 merge window ?
> 
> Felipe, I assume that means you're OK with me taking this patch for
> 3.9 then?

No objection from me either:

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Re: [PATCH v2] usb: host: tegra: make use of PHY pointer of HCD

[Stephen Warren]

On 01/24/2013 03:27 AM, Venu Byravarasu wrote:
> As pointer to PHY structure can be stored in struct usb_hcd
> making use of it, to call Tegra PHY APIs.
> 
> Call to usb_phy_shutdown() is moved up in tegra_ehci_remove(),
> so that to avoid dereferencing of hcd after its freed up.

I have applied this to Tegra's for-3.9/soc-usb branch.