usb: host: tegra: don't touch EMC clock

The patch to add USB PHY nodes to device tree was written before Tegra supported the clocks property in device tree. Now that it does, add the required clocks properties to these nodes.

This will allow all clk_get_sys() calls in tegra_usb_phy.c to be replaced by clk_get(phy->dev, clock_name), as part of converting the PHY driver to a platform driver.

arch/arm/boot/dts/tegra20.dtsi +2 -0

 
+		clocks = <&tegra_car 22>, <&tegra_car 127>;
+		clock-names = "utmi", "pll_u";

-----Original Message-----
From: Stephen Warren [mailto:swarren at wwwdotorg.org]
Sent: Wednesday, January 23, 2013 6:03 AM
To: Venu Byravarasu
Cc: Alan Stern; Greg Kroah-Hartman; linux-tegra at vger.kernel.org; linux-arm- kernel at lists.infradead.org; linux-usb at vger.kernel.org; Stephen Warren
Subject: Re: [PATCH 2/2] ARM: tegra: add clocks properties to USB PHY nodes

Review

Quoted 9 lines · Stephen Warren

The patch to add USB PHY nodes to device tree was written before Tegra supported the clocks property in device tree. Now that it does, add the required clocks properties to these nodes.

This will allow all clk_get_sys() calls in tegra_usb_phy.c to be replaced by clk_get(phy->dev, clock_name), as part of converting the PHY driver to a platform driver.

(quoted diff) +0 -0

b/arch/arm/boot/dts/tegra20.dtsi

Quoted 2 lines

(quoted diff) +2 -0

+		clocks = <&tegra_car 22>, <&tegra_car 127>;
+		clock-names = "utmi", "pll_u";

...

arch/arm/boot/dts/tegra20.dtsi line 454

Quoted 2 lines

arch/arm/boot/dts/tegra20.dtsi +2 -0

+		clocks = <&tegra_car 94>, <&tegra_car 127>;
+		clock-names = "ulpi", "pll_u";

Hmmm. Thinking about that first clock more, if we name it just "phy" in both the UTMI and ULPI PHY nodes, we could make tegra_phy_init() perform the clk_get() for all PHY types, and use the same clock name everywhere, and hence remove the type-specific clk_get()s from tegra_phy_init() and utmip_pad_open().

Venu, will this work for other chips such as Tegra30/Tegra114 and so on into the future, or do chips after Tegra20 introduce any new clocks, and hence break this scheme?

Stephen Warren wrote at Wednesday, January 23, 2013 6:03 AM:

Quoted 24 lines · Stephen Warren

Review

Quoted 9 lines

The patch to add USB PHY nodes to device tree was written before Tegra supported the clocks property in device tree. Now that it does, add the required clocks properties to these nodes.

This will allow all clk_get_sys() calls in tegra_usb_phy.c to be replaced by clk_get(phy->dev, clock_name), as part of converting the PHY driver to a platform driver.

(quoted diff) +0 -0

b/arch/arm/boot/dts/tegra20.dtsi

Quoted 2 lines

(quoted diff) +2 -0

+		clocks = <&tegra_car 22>, <&tegra_car 127>;
+		clock-names = "utmi", "pll_u";

...

arch/arm/boot/dts/tegra20.dtsi line 454

Quoted 2 lines

arch/arm/boot/dts/tegra20.dtsi +2 -0

+		clocks = <&tegra_car 94>, <&tegra_car 127>;
+		clock-names = "ulpi", "pll_u";

Hmmm. Thinking about that first clock more, if we name it just "phy" in both the UTMI and ULPI PHY nodes, we could make tegra_phy_init() perform the clk_get() for all PHY types, and use the same clock name everywhere, and hence remove the type-specific clk_get()s from tegra_phy_init() and utmip_pad_open().

Quoted 3 lines

Venu, will this work for other chips such as Tegra30/Tegra114 and so on into the future, or do chips after Tegra20 introduce any new clocks, and hence break this scheme?

Should be fine, as same clocks are used across all chips.

Acked-by Venu Byravarasu <[email protected]>

From: Stephen Warren <[email protected]>

Clock "emc" is for the External Memory Controller. The USB driver has no business touching this clock directly. Remove the code that does so.

Signed-off-by Stephen Warren <[email protected]>

---
Greg, Alan, I'd like to take this patch through the Tegra tree to avoid any merge conflicts with the Tegra USB changes that have recently happened there.

Venu, When creating your patch to convert the Tegra USB PHY driver to a platform driver, can you assume these patches are applied first? Thanks. I assume that these patches make sense to you; could you ack them if so.

---
 drivers/usb/host/ehci-tegra.c |   17 -----------------
 1 file changed, 17 deletions(-)

-----Original Message-----
From: linux-tegra-owner at vger.kernel.org [mailto:linux-tegra- owner at vger.kernel.org] On Behalf Of Stephen Warren
Sent: Wednesday, January 23, 2013 5:58 AM
To: Alan Stern; Greg Kroah-Hartman; Stephen Warren
Cc: Venu Byravarasu; linux-tegra at vger.kernel.org; linux-arm- kernel at lists.infradead.org; linux-usb at vger.kernel.org; Stephen Warren
Subject: [PATCH 1/2] usb: host: tegra: don't touch EMC clock

From: Stephen Warren <[email protected]>

Clock "emc" is for the External Memory Controller. The USB driver has no business touching this clock directly. Remove the code that does so.

Signed-off-by Stephen Warren <[email protected]>

---
Greg, Alan, I'd like to take this patch through the Tegra tree to avoid any merge conflicts with the Tegra USB changes that have recently happened there.

Venu, When creating your patch to convert the Tegra USB PHY driver to a platform driver, can you assume these patches are applied first? Thanks. I assume that these patches make sense to you; could you ack them if so.

---
 drivers/usb/host/ehci-tegra.c |   17 -----------------
 1 file changed, 17 deletions(-)

drivers/usb/host/ehci-tegra.c +0 -3

index 1f596fb..b02622a 100644
@@ -44,7 +44,6 @@ struct tegra_ehci_hcd {
 	struct ehci_hcd *ehci;
 	struct tegra_usb_phy *phy;
 	struct clk *clk;
-	struct clk *emc_clk;
 	struct usb_phy *transceiver;
 	int host_resumed;
 	int port_resuming;

@@ -56,7 +55,6 @@ 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->emc_clk);
 	clk_prepare_enable(tegra->clk);
 	usb_phy_set_suspend(&tegra->phy->u_phy, 0);
 	tegra->host_resumed = 1;

@@ -69,7 +67,6 @@ static void tegra_ehci_power_down(struct usb_hcd
 *hcd)
 	tegra->host_resumed = 0;
 	usb_phy_set_suspend(&tegra->phy->u_phy, 1);
 	clk_disable_unprepare(tegra->clk);
-	clk_disable_unprepare(tegra->emc_clk);
 }
 

static int tegra_ehci_internal_port_reset(

drivers/usb/host/ehci-tegra.c +0 -10

@@ -694,16 +691,6 @@ static int tegra_ehci_probe(struct platform_device
 *pdev)
 	if (err)
 		goto fail_clk;
 
-	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);
-		goto fail_emc_clk;
-	}
-
-	clk_prepare_enable(tegra->emc_clk);
-	clk_set_rate(tegra->emc_clk, 400000000);
-
 	tegra->needs_double_reset = of_property_read_bool(pdev-
 >dev.of_node,

drivers/usb/host/ehci-tegra.c +0 -5

 		"nvidia,needs-double-reset");

@@ -813,8 +800,6 @@ fail:
 #endif
 	usb_phy_shutdown(&tegra->phy->u_phy);
 fail_io:
-	clk_disable_unprepare(tegra->emc_clk);
-fail_emc_clk:
 	clk_disable_unprepare(tegra->clk);
 fail_clk:
 	usb_put_hcd(hcd);

@@ -842,8 +827,6 @@ static int tegra_ehci_remove(struct platform_device
 *pdev)
 
 	clk_disable_unprepare(tegra->clk);
 
-	clk_disable_unprepare(tegra->emc_clk);
-
 	return 0;
 }
 
--

1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 11 lines

-----Original Message-----
From: linux-tegra-owner at vger.kernel.org [mailto:linux-tegra- owner at vger.kernel.org] On Behalf Of Stephen Warren
Sent: Wednesday, January 23, 2013 5:58 AM
To: Alan Stern; Greg Kroah-Hartman; Stephen Warren
Cc: Venu Byravarasu; linux-tegra at vger.kernel.org; linux-arm- kernel at lists.infradead.org; linux-usb at vger.kernel.org; Stephen Warren
Subject: [PATCH 1/2] usb: host: tegra: don't touch EMC clock

From: Stephen Warren <[email protected]>

Quoted 2 lines

Clock "emc" is for the External Memory Controller. The USB driver has no business touching this clock directly. Remove the code that does so.

Stephen,
This was primarily done to make sure that EMC is set to a minimum frequency, below which data errors may occur during USB transfers. If we plan to remove this, how should we make sure that the EMC is programmed for the required frequency during USB transfers?

Am Mittwoch, den 23.01.2013, 12:25 +0530 schrieb Venu Byravarasu:

Quoted 11 lines

Quoted 11 lines

-----Original Message-----
From: linux-tegra-owner at vger.kernel.org [mailto:linux-tegra- owner at vger.kernel.org] On Behalf Of Stephen Warren
Sent: Wednesday, January 23, 2013 5:58 AM
To: Alan Stern; Greg Kroah-Hartman; Stephen Warren
Cc: Venu Byravarasu; linux-tegra at vger.kernel.org; linux-arm- kernel at lists.infradead.org; linux-usb at vger.kernel.org; Stephen Warren
Subject: [PATCH 1/2] usb: host: tegra: don't touch EMC clock

From: Stephen Warren <[email protected]>

Quoted 8 lines

Quoted 2 lines

Clock "emc" is for the External Memory Controller. The USB driver has no business touching this clock directly. Remove the code that does so.

Stephen,
This was primarily done to make sure that EMC is set to a minimum frequency, below which data errors may occur during USB transfers. If we plan to remove this, how should we make sure that the EMC is programmed for the required frequency during USB transfers?

You could use something like the API I added in "ARM: tegra: add EMC clock scaling API". This needs some rework and I looked into integrating this with the DEVFREQ framework, but I don't think it fits too well.

Bandwidth requirements should always be communicated to the EMC driver and not described by clock rates.

From: Stephen Warren <[email protected]>

Clock "emc" is for the External Memory Controller. The USB driver has no business touching this clock directly. Remove the code that does so.

Signed-off-by Stephen Warren <[email protected]>

---
Greg, Alan, I'd like to take this patch through the Tegra tree to avoid any merge conflicts with the Tegra USB changes that have recently happened there.