[PATCH 18/21] usb: phy: tegra: don't call into tegra-ehci directly

2013-04-23 Thread Arnd Bergmann
Both phy-tegra-usb.c and ehci-tegra.c export symbols used by the other one,
which does not work if one of them or both are loadable modules, resulting
in an error like:

drivers/built-in.o: In function `utmi_phy_clk_disable':
drivers/usb/phy/phy-tegra-usb.c:302: undefined reference to 
`tegra_ehci_set_phcd'
drivers/built-in.o: In function `utmi_phy_clk_enable':
drivers/usb/phy/phy-tegra-usb.c:324: undefined reference to 
`tegra_ehci_set_phcd'
drivers/built-in.o: In function `utmi_phy_power_on':
drivers/usb/phy/phy-tegra-usb.c:447: undefined reference to `tegra_ehci_set_pts'

This turns the interface into a one-way dependency by letting the tegra ehci
driver pass two function pointers for callbacks that need to be called by
the phy driver.

Signed-off-by: Arnd Bergmann a...@arndb.de
Cc: Venu Byravarasu vbyravar...@nvidia.com
Cc: Alan Stern st...@rowland.harvard.edu
Cc: Felipe Balbi ba...@ti.com
Cc: Greg Kroah-Hartman gre...@linuxfoundation.org
Cc: Stephen Warren swar...@nvidia.com
Cc: linux-usb@vger.kernel.org
---
 drivers/usb/host/ehci-tegra.c | 10 +-
 drivers/usb/phy/tegra_usb_phy.c   | 13 +
 include/linux/usb/tegra_usb_phy.h | 10 +-
 3 files changed, 19 insertions(+), 14 deletions(-)

diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c
index 568aecc..07419e4 100644
--- a/drivers/usb/host/ehci-tegra.c
+++ b/drivers/usb/host/ehci-tegra.c
@@ -610,7 +610,7 @@ static const struct dev_pm_ops tegra_ehci_pm_ops = {
 /* Bits of PORTSC1, which will get cleared by writing 1 into them */
 #define TEGRA_PORTSC1_RWC_BITS (PORT_CSC | PORT_PEC | PORT_OCC)
 
-void tegra_ehci_set_pts(struct usb_phy *x, u8 pts_val)
+static void tegra_ehci_set_pts(struct usb_phy *x, u8 pts_val)
 {
unsigned long val;
struct usb_hcd *hcd = bus_to_hcd(x-otg-host);
@@ -621,9 +621,8 @@ void tegra_ehci_set_pts(struct usb_phy *x, u8 pts_val)
val |= TEGRA_USB_PORTSC1_PTS(pts_val  3);
writel(val, base + TEGRA_USB_PORTSC1);
 }
-EXPORT_SYMBOL_GPL(tegra_ehci_set_pts);
 
-void tegra_ehci_set_phcd(struct usb_phy *x, bool enable)
+static void tegra_ehci_set_phcd(struct usb_phy *x, bool enable)
 {
unsigned long val;
struct usb_hcd *hcd = bus_to_hcd(x-otg-host);
@@ -636,7 +635,6 @@ void tegra_ehci_set_phcd(struct usb_phy *x, bool enable)
val = ~TEGRA_USB_PORTSC1_PHCD;
writel(val, base + TEGRA_USB_PORTSC1);
 }
-EXPORT_SYMBOL_GPL(tegra_ehci_set_phcd);
 
 static u64 tegra_ehci_dma_mask = DMA_BIT_MASK(32);
 
@@ -733,7 +731,9 @@ static int tegra_ehci_probe(struct platform_device *pdev)
 
tegra-phy = tegra_usb_phy_open(pdev-dev, instance, hcd-regs,
pdata-phy_config,
-   TEGRA_USB_PHY_MODE_HOST);
+   TEGRA_USB_PHY_MODE_HOST,
+   tegra_ehci_set_pts,
+   tegra_ehci_set_phcd);
if (IS_ERR(tegra-phy)) {
dev_err(pdev-dev, Failed to open USB phy\n);
err = -ENXIO;
diff --git a/drivers/usb/phy/tegra_usb_phy.c b/drivers/usb/phy/tegra_usb_phy.c
index 5487d38..17d8112 100644
--- a/drivers/usb/phy/tegra_usb_phy.c
+++ b/drivers/usb/phy/tegra_usb_phy.c
@@ -299,7 +299,7 @@ static void utmi_phy_clk_disable(struct tegra_usb_phy *phy)
val = ~USB_SUSP_SET;
writel(val, base + USB_SUSP_CTRL);
} else
-   tegra_ehci_set_phcd(phy-u_phy, true);
+   phy-set_phcd(phy-u_phy, true);
 
if (utmi_wait_register(base + USB_SUSP_CTRL, USB_PHY_CLK_VALID, 0)  0)
pr_err(%s: timeout waiting for phy to stabilize\n, __func__);
@@ -321,7 +321,7 @@ static void utmi_phy_clk_enable(struct tegra_usb_phy *phy)
val = ~USB_SUSP_CLR;
writel(val, base + USB_SUSP_CTRL);
} else
-   tegra_ehci_set_phcd(phy-u_phy, false);
+   phy-set_phcd(phy-u_phy, false);
 
if (utmi_wait_register(base + USB_SUSP_CTRL, USB_PHY_CLK_VALID,
 USB_PHY_CLK_VALID))
@@ -444,7 +444,7 @@ static int utmi_phy_power_on(struct tegra_usb_phy *phy)
utmi_phy_clk_enable(phy);
 
if (!phy-is_legacy_phy)
-   tegra_ehci_set_pts(phy-u_phy, 0);
+   phy-set_pts(phy-u_phy, 0);
 
return 0;
 }
@@ -688,7 +688,10 @@ static int tegra_usb_phy_suspend(struct usb_phy *x, int 
suspend)
 }
 
 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 __iomem *regs, void *config, enum tegra_usb_phy_mode phy_mode,
+   void (*set_pts)(struct usb_phy *x, u8 pts_val),
+   void (*set_phcd)(struct usb_phy *x, bool enable))
+
 {
struct tegra_usb_phy *phy;
unsigned long parent_rate;
@@ -707,6 +710,8 @@ struct tegra_usb_phy 

Re: [PATCH 18/21] usb: phy: tegra: don't call into tegra-ehci directly

2013-04-23 Thread Alan Stern
On Tue, 23 Apr 2013, Arnd Bergmann wrote:

 Both phy-tegra-usb.c and ehci-tegra.c export symbols used by the other one,
 which does not work if one of them or both are loadable modules, resulting
 in an error like:
 
 drivers/built-in.o: In function `utmi_phy_clk_disable':
 drivers/usb/phy/phy-tegra-usb.c:302: undefined reference to 
 `tegra_ehci_set_phcd'
 drivers/built-in.o: In function `utmi_phy_clk_enable':
 drivers/usb/phy/phy-tegra-usb.c:324: undefined reference to 
 `tegra_ehci_set_phcd'
 drivers/built-in.o: In function `utmi_phy_power_on':
 drivers/usb/phy/phy-tegra-usb.c:447: undefined reference to 
 `tegra_ehci_set_pts'
 
 This turns the interface into a one-way dependency by letting the tegra ehci
 driver pass two function pointers for callbacks that need to be called by
 the phy driver.

 @@ -733,7 +731,9 @@ static int tegra_ehci_probe(struct platform_device *pdev)
  
   tegra-phy = tegra_usb_phy_open(pdev-dev, instance, hcd-regs,
   pdata-phy_config,
 - TEGRA_USB_PHY_MODE_HOST);
 + TEGRA_USB_PHY_MODE_HOST,
 + tegra_ehci_set_pts,
 + tegra_ehci_set_phcd);

Does the compiler warn about the unnecessary ''?  In any case, it
looks strange to have one function pointer with an '' and another
without.

Aside from that minor detail, the ehci-tegra.c changes are fine.

Alan Stern

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


Re: [PATCH 18/21] usb: phy: tegra: don't call into tegra-ehci directly

2013-04-23 Thread Greg Kroah-Hartman
On Tue, Apr 23, 2013 at 06:30:50PM +0200, Arnd Bergmann wrote:
 Both phy-tegra-usb.c and ehci-tegra.c export symbols used by the other one,
 which does not work if one of them or both are loadable modules, resulting
 in an error like:
 
 drivers/built-in.o: In function `utmi_phy_clk_disable':
 drivers/usb/phy/phy-tegra-usb.c:302: undefined reference to 
 `tegra_ehci_set_phcd'
 drivers/built-in.o: In function `utmi_phy_clk_enable':
 drivers/usb/phy/phy-tegra-usb.c:324: undefined reference to 
 `tegra_ehci_set_phcd'
 drivers/built-in.o: In function `utmi_phy_power_on':
 drivers/usb/phy/phy-tegra-usb.c:447: undefined reference to 
 `tegra_ehci_set_pts'
 
 This turns the interface into a one-way dependency by letting the tegra ehci
 driver pass two function pointers for callbacks that need to be called by
 the phy driver.
 
 Signed-off-by: Arnd Bergmann a...@arndb.de
 Cc: Venu Byravarasu vbyravar...@nvidia.com
 Cc: Alan Stern st...@rowland.harvard.edu
 Cc: Felipe Balbi ba...@ti.com
 Cc: Greg Kroah-Hartman gre...@linuxfoundation.org
 Cc: Stephen Warren swar...@nvidia.com
 Cc: linux-usb@vger.kernel.org
 ---
  drivers/usb/host/ehci-tegra.c | 10 +-
  drivers/usb/phy/tegra_usb_phy.c   | 13 +
  include/linux/usb/tegra_usb_phy.h | 10 +-
  3 files changed, 19 insertions(+), 14 deletions(-)

Not all of these files are in my usb-next tree, so I can't take this
patch, sorry.

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 18/21] usb: phy: tegra: don't call into tegra-ehci directly

2013-04-23 Thread Arnd Bergmann
On Tuesday 23 April 2013, Alan Stern wrote:
 On Tue, 23 Apr 2013, Arnd Bergmann wrote:

  @@ -733,7 +731,9 @@ static int tegra_ehci_probe(struct platform_device 
  *pdev)
   
  tegra-phy = tegra_usb_phy_open(pdev-dev, instance, hcd-regs,
  pdata-phy_config,
  -   TEGRA_USB_PHY_MODE_HOST);
  +   TEGRA_USB_PHY_MODE_HOST,
  +   tegra_ehci_set_pts,
  +   tegra_ehci_set_phcd);
 
 Does the compiler warn about the unnecessary ''? 

No, AFAIK, both variants are equally acceptable C.

 In any case, it
 looks strange to have one function pointer with an '' and another
 without.

Yes, that was certainly not intentional. I've removed the '' now.

 Aside from that minor detail, the ehci-tegra.c changes are fine.

Ok, thanks for the feedback.

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