[PATCH 4/4] usb: Add APIs to access host registers from Tegra PHY
As Tegra PHY driver needs to access one of the Host registers,
added few APIs.
Signed-off-by: Venu Byravarasu vbyravar...@nvidia.com
---
drivers/usb/host/ehci-tegra.c | 71 -
drivers/usb/phy/tegra_usb_phy.c | 51 +++
include/linux/usb/tegra_usb_phy.h |6 +++
3 files changed, 89 insertions(+), 39 deletions(-)
diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c
index 55a9cde..5299b01 100644
--- a/drivers/usb/host/ehci-tegra.c
+++ b/drivers/usb/host/ehci-tegra.c
@@ -2,7 +2,7 @@
* EHCI-compliant USB host controller driver for NVIDIA Tegra SoCs
*
* Copyright (C) 2010 Google, Inc.
- * Copyright (C) 2009 NVIDIA Corporation
+ * Copyright (C) 2009 - 2013 NVIDIA Corporation
*
* This program is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License as published by the
@@ -33,6 +33,20 @@
#define TEGRA_USB2_BASE0xC5004000
#define TEGRA_USB3_BASE0xC5008000
+/* PORTSC registers */
+#define USB_PORTSC10x184
+#define USB_PORTSC1_PTS(x) (((x) 0x3) 30)
+#define USB_PORTSC1_PSPD(x)(((x) 0x3) 26)
+#define USB_PORTSC1_PHCD (1 23)
+#define USB_PORTSC1_WKOC (1 22)
+#define USB_PORTSC1_WKDS (1 21)
+#define USB_PORTSC1_WKCN (1 20)
+#define USB_PORTSC1_PTC(x) (((x) 0xf) 16)
+#define USB_PORTSC1_PP (1 12)
+#define USB_PORTSC1_SUSP (1 7)
+#define USB_PORTSC1_PE (1 2)
+#define USB_PORTSC1_CCS(1 0)
+
#define TEGRA_USB_DMA_ALIGN 32
struct tegra_ehci_hcd {
@@ -605,6 +619,50 @@ static const struct dev_pm_ops tegra_ehci_pm_ops = {
#endif
+void tegra_ehci_set_wakeon_events(struct usb_phy *x, bool enable)
+{
+ unsigned long val;
+ struct usb_hcd *hcd = bus_to_hcd(x-otg-host);
+ void __iomem *base = hcd-regs;
+ u32 wake = USB_PORTSC1_WKOC | USB_PORTSC1_WKDS | USB_PORTSC1_WKCN;
+
+ val = readl(base + USB_PORTSC1);
+ if (enable)
+ val |= wake;
+ else
+ val = ~wake;
+ writel(val, base + USB_PORTSC1);
+}
+EXPORT_SYMBOL_GPL(tegra_ehci_set_wakeon_events);
+
+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);
+ void __iomem *base = hcd-regs;
+
+ val = readl(base + USB_PORTSC1);
+ val = ~USB_PORTSC1_PTS(3);
+ val |= USB_PORTSC1_PTS(pts_val 3);
+ writel(val, base + USB_PORTSC1);
+}
+EXPORT_SYMBOL_GPL(tegra_ehci_set_pts);
+
+void tegra_ehci_set_phcd(struct usb_phy *x, bool enable)
+{
+ unsigned long val;
+ struct usb_hcd *hcd = bus_to_hcd(x-otg-host);
+ void __iomem *base = hcd-regs;
+
+ val = readl(base + USB_PORTSC1);
+ if (enable)
+ val |= USB_PORTSC1_PHCD;
+ else
+ val = ~USB_PORTSC1_PHCD;
+ writel(val, base + USB_PORTSC1);
+}
+EXPORT_SYMBOL_GPL(tegra_ehci_set_phcd);
+
static u64 tegra_ehci_dma_mask = DMA_BIT_MASK(32);
static int tegra_ehci_probe(struct platform_device *pdev)
@@ -616,6 +674,7 @@ static int tegra_ehci_probe(struct platform_device *pdev)
int err = 0;
int irq;
int instance = pdev-id;
+ struct usb_phy *u_phy;
pdata = pdev-dev.platform_data;
if (!pdata) {
@@ -718,6 +777,16 @@ static int tegra_ehci_probe(struct platform_device *pdev)
usb_phy_init(tegra-phy-u_phy);
+ hcd-phy = u_phy = tegra-phy-u_phy;
+ u_phy-otg = devm_kzalloc(pdev-dev, sizeof(struct usb_otg),
+GFP_KERNEL);
+ if (!u_phy-otg) {
+ dev_err(pdev-dev, Failed to alloc memory for otg\n);
+ err = -ENOMEM;
+ goto fail_io;
+ }
+ u_phy-otg-host = hcd_to_bus(hcd);
+
err = usb_phy_set_suspend(tegra-phy-u_phy, 0);
if (err) {
dev_err(pdev-dev, Failed to power on the phy\n);
diff --git a/drivers/usb/phy/tegra_usb_phy.c b/drivers/usb/phy/tegra_usb_phy.c
index ce1ff2a..f3b73b3 100644
--- a/drivers/usb/phy/tegra_usb_phy.c
+++ b/drivers/usb/phy/tegra_usb_phy.c
@@ -36,19 +36,6 @@
#define ULPI_VIEWPORT 0x170
-#define USB_PORTSC10x184
-#define USB_PORTSC1_PTS(x) (((x) 0x3) 30)
-#define USB_PORTSC1_PSPD(x) (((x) 0x3) 26)
-#define USB_PORTSC1_PHCD (1 23)
-#define USB_PORTSC1_WKOC (1 22)
-#define USB_PORTSC1_WKDS (1 21)
-#define USB_PORTSC1_WKCN (1 20)
-#define USB_PORTSC1_PTC(x) (((x) 0xf) 16)
-#define USB_PORTSC1_PP (1 12)
-#define USB_PORTSC1_SUSP (1 7)
-#define USB_PORTSC1_PE (1 2)
-#define USB_PORTSC1_CCS (1 0)
-
#define USB_SUSP_CTRL 0x400
#define USB_WAKE_ON_CNNT_EN_DEV (1 3)
#define USB_WAKE_ON_DISCON_EN_DEV(1 4)
@@ -311,11 +298,8 @@ static void utmi_phy_clk_disable(struct
Re: [PATCH 4/4] usb: Add APIs to access host registers from Tegra PHY
On Wed, 16 Jan 2013, Venu Byravarasu wrote:
As Tegra PHY driver needs to access one of the Host registers,
added few APIs.
Signed-off-by: Venu Byravarasu vbyravar...@nvidia.com
--- a/drivers/usb/host/ehci-tegra.c
+++ b/drivers/usb/host/ehci-tegra.c
@@ -605,6 +619,50 @@ static const struct dev_pm_ops tegra_ehci_pm_ops = {
#endif
+void tegra_ehci_set_wakeon_events(struct usb_phy *x, bool enable)
+{
+ unsigned long val;
+ struct usb_hcd *hcd = bus_to_hcd(x-otg-host);
+ void __iomem *base = hcd-regs;
+ u32 wake = USB_PORTSC1_WKOC | USB_PORTSC1_WKDS | USB_PORTSC1_WKCN;
+
+ val = readl(base + USB_PORTSC1);
+ if (enable)
+ val |= wake;
+ else
+ val = ~wake;
+ writel(val, base + USB_PORTSC1);
+}
Here and below, this sort of code is highly questionable. You
evidently don't realize that some of the bits in the PORTSC registers
are R/WC. This means writing a 1 to these bits will clear them.
Consequently it is almost always wrong to read a PORTSC register and
then write back the same (or a slightly modified) value.
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 4/4] usb: Add APIs to access host registers from Tegra PHY
On 01/16/2013 08:08 AM, Alan Stern wrote:
On Wed, 16 Jan 2013, Venu Byravarasu wrote:
As Tegra PHY driver needs to access one of the Host registers,
added few APIs.
--- a/drivers/usb/host/ehci-tegra.c
+++ b/drivers/usb/host/ehci-tegra.c
+void tegra_ehci_set_wakeon_events(struct usb_phy *x, bool enable)
+{
+unsigned long val;
+struct usb_hcd *hcd = bus_to_hcd(x-otg-host);
+void __iomem *base = hcd-regs;
+u32 wake = USB_PORTSC1_WKOC | USB_PORTSC1_WKDS | USB_PORTSC1_WKCN;
+
+val = readl(base + USB_PORTSC1);
+if (enable)
+val |= wake;
+else
+val = ~wake;
+writel(val, base + USB_PORTSC1);
+}
Here and below, this sort of code is highly questionable. You
evidently don't realize that some of the bits in the PORTSC registers
are R/WC. This means writing a 1 to these bits will clear them.
Consequently it is almost always wrong to read a PORTSC register and
then write back the same (or a slightly modified) value.
Sorry I'm not familiar with USB... Are the bits being manipulated here
(i.e. USB_PORTSC1_WKOC | USB_PORTSC1_WKDS | USB_PORTSC1_WKCN)
standardized USBisms, or some custom Tegra stuff?
Anyway, is the solution here to do:
val = readl(addr)
// i.e. add the following line:
val = ~(all write-to-clear bits);
if (enable) val |= wake; else val = ~wake;
writel(val, addr)
... or is there more broken than just that?
Also note that the driver is already doing exactly what is in these new
functions; the code is just being split out so that only the EHCI driver
touches EHCI registers, and the PHY driver only touches PHY registers.
Still, I'll admit it's a good time to fix any mistakes in this part of
the code.
--
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 4/4] usb: Add APIs to access host registers from Tegra PHY
On Wed, 16 Jan 2013, Stephen Warren wrote:
On 01/16/2013 08:08 AM, Alan Stern wrote:
On Wed, 16 Jan 2013, Venu Byravarasu wrote:
As Tegra PHY driver needs to access one of the Host registers,
added few APIs.
--- a/drivers/usb/host/ehci-tegra.c
+++ b/drivers/usb/host/ehci-tegra.c
+void tegra_ehci_set_wakeon_events(struct usb_phy *x, bool enable)
+{
+ unsigned long val;
+ struct usb_hcd *hcd = bus_to_hcd(x-otg-host);
+ void __iomem *base = hcd-regs;
+ u32 wake = USB_PORTSC1_WKOC | USB_PORTSC1_WKDS | USB_PORTSC1_WKCN;
+
+ val = readl(base + USB_PORTSC1);
+ if (enable)
+ val |= wake;
+ else
+ val = ~wake;
+ writel(val, base + USB_PORTSC1);
+}
Here and below, this sort of code is highly questionable. You
evidently don't realize that some of the bits in the PORTSC registers
are R/WC. This means writing a 1 to these bits will clear them.
Consequently it is almost always wrong to read a PORTSC register and
then write back the same (or a slightly modified) value.
Sorry I'm not familiar with USB... Are the bits being manipulated here
(i.e. USB_PORTSC1_WKOC | USB_PORTSC1_WKDS | USB_PORTSC1_WKCN)
standardized USBisms, or some custom Tegra stuff?
The _symbols_ are not standard (i.e., not used in the regular ehci-hcd
driver) but the _values_ are: compare with PORT_WKOC_E etc. in
include/linux/usb/ehci_defs.h. I'm not sure about the register
addresses, however.
Anyway, is the solution here to do:
val = readl(addr)
// i.e. add the following line:
val = ~(all write-to-clear bits);
if (enable) val |= wake; else val = ~wake;
writel(val, addr)
... or is there more broken than just that?
Nope, that's the right way to do it. There are loads of examples in
drivers/usb/host/ehci-hub.c; look at usages of PORT_RWC_BITS.
Also note that the driver is already doing exactly what is in these new
functions; the code is just being split out so that only the EHCI driver
touches EHCI registers, and the PHY driver only touches PHY registers.
Still, I'll admit it's a good time to fix any mistakes in this part of
the code.
Agreed.
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
