* [V8 PATCH 01/16] usb: phy: mv_usb2: add PHY driver for marvell usb2 controller
2013-02-21 4:07 [V8 PATCH 00/16] mv-usb phy enhancement patches Chao Xie
@ 2013-02-21 4:07 ` Chao Xie
2013-03-04 14:21 ` Felipe Balbi
2013-02-21 4:07 ` [V8 PATCH 02/16] usb: gadget: mv_udc: use PHY driver for udc Chao Xie
` (15 subsequent siblings)
16 siblings, 1 reply; 34+ messages in thread
From: Chao Xie @ 2013-02-21 4:07 UTC (permalink / raw)
To: linux-arm-kernel
The PHY is seperated from usb controller.
The usb controller used in marvell pxa168/pxa910/mmp2 are same,
but PHY initialization may be different.
the usb controller can support udc/otg/ehci, and for each of
the mode, it need PHY to initialized before use the controller.
Direclty writing the phy driver will make the usb controller
driver to be simple and portable.
The PHY driver will be used by marvell udc/otg/ehci.
Signed-off-by: Chao Xie <chao.xie@marvell.com>
---
drivers/usb/phy/Kconfig | 7 +
drivers/usb/phy/Makefile | 1 +
drivers/usb/phy/mv_usb2_phy.c | 402 ++++++++++++++++++++++++++++++++++
include/linux/platform_data/mv_usb.h | 9 +-
include/linux/usb/mv_usb2.h | 32 +++
5 files changed, 448 insertions(+), 3 deletions(-)
create mode 100644 drivers/usb/phy/mv_usb2_phy.c
create mode 100644 include/linux/usb/mv_usb2.h
diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
index 65217a5..5479760 100644
--- a/drivers/usb/phy/Kconfig
+++ b/drivers/usb/phy/Kconfig
@@ -73,3 +73,10 @@ config SAMSUNG_USBPHY
help
Enable this to support Samsung USB phy controller for samsung
SoCs.
+
+config MV_USB2_PHY
+ tristate "Marvell USB 2.0 PHY Driver"
+ depends on USB || USB_GADGET
+ help
+ Enable this to support Marvell USB 2.0 phy driver for Marvell
+ SoC.
diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile
index b13faa1..3835316 100644
--- a/drivers/usb/phy/Makefile
+++ b/drivers/usb/phy/Makefile
@@ -12,3 +12,4 @@ obj-$(CONFIG_MV_U3D_PHY) += mv_u3d_phy.o
obj-$(CONFIG_USB_EHCI_TEGRA) += tegra_usb_phy.o
obj-$(CONFIG_USB_RCAR_PHY) += rcar-phy.o
obj-$(CONFIG_SAMSUNG_USBPHY) += samsung-usbphy.o
+obj-$(CONFIG_MV_USB2_PHY) += mv_usb2_phy.o
diff --git a/drivers/usb/phy/mv_usb2_phy.c b/drivers/usb/phy/mv_usb2_phy.c
new file mode 100644
index 0000000..a81e5e4
--- /dev/null
+++ b/drivers/usb/phy/mv_usb2_phy.c
@@ -0,0 +1,402 @@
+/*
+ * Copyright (C) 2013 Marvell Inc.
+ *
+ * Author:
+ * Chao Xie <xiechao.mail@gmail.com>
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include <linux/resource.h>
+#include <linux/delay.h>
+#include <linux/slab.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/io.h>
+#include <linux/err.h>
+#include <linux/clk.h>
+#include <linux/export.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/platform_data/mv_usb.h>
+#include <linux/usb/phy.h>
+#include <linux/usb/mv_usb2.h>
+
+/* phy regs */
+#define UTMI_REVISION 0x0
+#define UTMI_CTRL 0x4
+#define UTMI_PLL 0x8
+#define UTMI_TX 0xc
+#define UTMI_RX 0x10
+#define UTMI_IVREF 0x14
+#define UTMI_T0 0x18
+#define UTMI_T1 0x1c
+#define UTMI_T2 0x20
+#define UTMI_T3 0x24
+#define UTMI_T4 0x28
+#define UTMI_T5 0x2c
+#define UTMI_RESERVE 0x30
+#define UTMI_USB_INT 0x34
+#define UTMI_DBG_CTL 0x38
+#define UTMI_OTG_ADDON 0x3c
+
+/* For UTMICTRL Register */
+#define UTMI_CTRL_USB_CLK_EN (1 << 31)
+/* pxa168 */
+#define UTMI_CTRL_SUSPEND_SET1 (1 << 30)
+#define UTMI_CTRL_SUSPEND_SET2 (1 << 29)
+#define UTMI_CTRL_RXBUF_PDWN (1 << 24)
+#define UTMI_CTRL_TXBUF_PDWN (1 << 11)
+
+#define UTMI_CTRL_INPKT_DELAY_SHIFT 30
+#define UTMI_CTRL_INPKT_DELAY_SOF_SHIFT 28
+#define UTMI_CTRL_PU_REF_SHIFT 20
+#define UTMI_CTRL_ARC_PULLDN_SHIFT 12
+#define UTMI_CTRL_PLL_PWR_UP_SHIFT 1
+#define UTMI_CTRL_PWR_UP_SHIFT 0
+
+/* For UTMI_PLL Register */
+#define UTMI_PLL_PLLCALI12_SHIFT 29
+#define UTMI_PLL_PLLCALI12_MASK (0x3 << 29)
+
+#define UTMI_PLL_PLLVDD18_SHIFT 27
+#define UTMI_PLL_PLLVDD18_MASK (0x3 << 27)
+
+#define UTMI_PLL_PLLVDD12_SHIFT 25
+#define UTMI_PLL_PLLVDD12_MASK (0x3 << 25)
+
+#define UTMI_PLL_CLK_BLK_EN_SHIFT 24
+#define CLK_BLK_EN (0x1 << 24)
+#define PLL_READY (0x1 << 23)
+#define KVCO_EXT (0x1 << 22)
+#define VCOCAL_START (0x1 << 21)
+
+#define UTMI_PLL_KVCO_SHIFT 15
+#define UTMI_PLL_KVCO_MASK (0x7 << 15)
+
+#define UTMI_PLL_ICP_SHIFT 12
+#define UTMI_PLL_ICP_MASK (0x7 << 12)
+
+#define UTMI_PLL_FBDIV_SHIFT 4
+#define UTMI_PLL_FBDIV_MASK (0xFF << 4)
+
+#define UTMI_PLL_REFDIV_SHIFT 0
+#define UTMI_PLL_REFDIV_MASK (0xF << 0)
+
+/* For UTMI_TX Register */
+#define UTMI_TX_REG_EXT_FS_RCAL_SHIFT 27
+#define UTMI_TX_REG_EXT_FS_RCAL_MASK (0xf << 27)
+
+#define UTMI_TX_REG_EXT_FS_RCAL_EN_SHIFT 26
+#define UTMI_TX_REG_EXT_FS_RCAL_EN_MASK (0x1 << 26)
+
+#define UTMI_TX_TXVDD12_SHIFT 22
+#define UTMI_TX_TXVDD12_MASK (0x3 << 22)
+
+#define UTMI_TX_CK60_PHSEL_SHIFT 17
+#define UTMI_TX_CK60_PHSEL_MASK (0xf << 17)
+
+#define UTMI_TX_IMPCAL_VTH_SHIFT 14
+#define UTMI_TX_IMPCAL_VTH_MASK (0x7 << 14)
+
+#define REG_RCAL_START (0x1 << 12)
+
+#define UTMI_TX_LOW_VDD_EN_SHIFT 11
+
+#define UTMI_TX_AMP_SHIFT 0
+#define UTMI_TX_AMP_MASK (0x7 << 0)
+
+/* For UTMI_RX Register */
+#define UTMI_REG_SQ_LENGTH_SHIFT 15
+#define UTMI_REG_SQ_LENGTH_MASK (0x3 << 15)
+
+#define UTMI_RX_SQ_THRESH_SHIFT 4
+#define UTMI_RX_SQ_THRESH_MASK (0xf << 4)
+
+#define UTMI_OTG_ADDON_OTG_ON (1 << 0)
+
+enum mv_usb2_phy_type {
+ PXA168_USB,
+ PXA910_USB,
+ MMP2_USB,
+};
+
+static unsigned int u2o_get(void __iomem *base, unsigned int offset)
+{
+ return readl(base + offset);
+}
+
+static void u2o_set(void __iomem *base, unsigned int offset,
+ unsigned int value)
+{
+ u32 reg;
+
+ reg = readl(base + offset);
+ reg |= value;
+ writel(reg, base + offset);
+ /*
+ * read after write. It will make sure writing takes effect.
+ * It is suggested by PHY design engineer.
+ */
+ readl(base + offset);
+}
+
+static void u2o_clear(void __iomem *base, unsigned int offset,
+ unsigned int value)
+{
+ u32 reg;
+
+ reg = readl(base + offset);
+ reg &= ~value;
+ writel(reg, base + offset);
+ /*
+ * read after write. It will make sure writing takes effect.
+ * It is suggested by PHY design engineer.
+ */
+ readl(base + offset);
+}
+
+static void u2o_write(void __iomem *base, unsigned int offset,
+ unsigned int value)
+{
+ writel(value, base + offset);
+ /*
+ * read after write. It will make sure writing takes effect.
+ * It is suggested by PHY design engineer.
+ */
+ readl(base + offset);
+}
+
+static int _mv_usb2_phy_init(struct mv_usb2_phy *mv_phy)
+{
+ struct platform_device *pdev = mv_phy->pdev;
+ unsigned int loops = 0;
+ void __iomem *base = mv_phy->base;
+
+ dev_dbg(&pdev->dev, "phy init\n");
+
+ /* Initialize the USB PHY power */
+ if (mv_phy->type == PXA910_USB) {
+ u2o_set(base, UTMI_CTRL, (1<<UTMI_CTRL_INPKT_DELAY_SOF_SHIFT)
+ | (1<<UTMI_CTRL_PU_REF_SHIFT));
+ }
+
+ u2o_set(base, UTMI_CTRL, 1<<UTMI_CTRL_PLL_PWR_UP_SHIFT);
+ u2o_set(base, UTMI_CTRL, 1<<UTMI_CTRL_PWR_UP_SHIFT);
+
+ /* UTMI_PLL settings */
+ u2o_clear(base, UTMI_PLL, UTMI_PLL_PLLVDD18_MASK
+ | UTMI_PLL_PLLVDD12_MASK | UTMI_PLL_PLLCALI12_MASK
+ | UTMI_PLL_FBDIV_MASK | UTMI_PLL_REFDIV_MASK
+ | UTMI_PLL_ICP_MASK | UTMI_PLL_KVCO_MASK);
+
+ u2o_set(base, UTMI_PLL, 0xee<<UTMI_PLL_FBDIV_SHIFT
+ | 0xb<<UTMI_PLL_REFDIV_SHIFT | 3<<UTMI_PLL_PLLVDD18_SHIFT
+ | 3<<UTMI_PLL_PLLVDD12_SHIFT | 3<<UTMI_PLL_PLLCALI12_SHIFT
+ | 1<<UTMI_PLL_ICP_SHIFT | 3<<UTMI_PLL_KVCO_SHIFT);
+
+ /* UTMI_TX */
+ u2o_clear(base, UTMI_TX, UTMI_TX_REG_EXT_FS_RCAL_EN_MASK
+ | UTMI_TX_TXVDD12_MASK | UTMI_TX_CK60_PHSEL_MASK
+ | UTMI_TX_IMPCAL_VTH_MASK | UTMI_TX_REG_EXT_FS_RCAL_MASK
+ | UTMI_TX_AMP_MASK);
+ u2o_set(base, UTMI_TX, 3<<UTMI_TX_TXVDD12_SHIFT
+ | 4<<UTMI_TX_CK60_PHSEL_SHIFT | 4<<UTMI_TX_IMPCAL_VTH_SHIFT
+ | 8<<UTMI_TX_REG_EXT_FS_RCAL_SHIFT | 3<<UTMI_TX_AMP_SHIFT);
+
+ /* UTMI_RX */
+ u2o_clear(base, UTMI_RX, UTMI_RX_SQ_THRESH_MASK
+ | UTMI_REG_SQ_LENGTH_MASK);
+ u2o_set(base, UTMI_RX, 7<<UTMI_RX_SQ_THRESH_SHIFT
+ | 2<<UTMI_REG_SQ_LENGTH_SHIFT);
+
+ /* UTMI_IVREF */
+ if (mv_phy->type == PXA168_USB)
+ /* fixing Microsoft Altair board interface with NEC hub issue -
+ * Set UTMI_IVREF from 0x4a3 to 0x4bf */
+ u2o_write(base, UTMI_IVREF, 0x4bf);
+
+ /* toggle VCOCAL_START bit of UTMI_PLL */
+ udelay(200);
+ u2o_set(base, UTMI_PLL, VCOCAL_START);
+ udelay(40);
+ u2o_clear(base, UTMI_PLL, VCOCAL_START);
+
+ /* toggle REG_RCAL_START bit of UTMI_TX */
+ udelay(400);
+ u2o_set(base, UTMI_TX, REG_RCAL_START);
+ udelay(40);
+ u2o_clear(base, UTMI_TX, REG_RCAL_START);
+ udelay(400);
+
+ /* Make sure PHY PLL is ready */
+ loops = 0;
+ while ((u2o_get(base, UTMI_PLL) & PLL_READY) == 0) {
+ mdelay(1);
+ loops++;
+ if (loops > 100) {
+ dev_warn(&pdev->dev, "calibrate timeout, UTMI_PLL %x\n",
+ u2o_get(base, UTMI_PLL));
+ break;
+ }
+ }
+
+ if (mv_phy->type == PXA168_USB) {
+ u2o_set(base, UTMI_RESERVE, 1 << 5);
+ /* Turn on UTMI PHY OTG extension */
+ u2o_write(base, UTMI_OTG_ADDON, 1);
+ }
+
+ return 0;
+}
+
+static int _mv_usb2_phy_shutdown(struct mv_usb2_phy *mv_phy)
+{
+ void __iomem *base = mv_phy->base;
+
+ if (mv_phy->type == PXA168_USB)
+ u2o_clear(base, UTMI_OTG_ADDON, UTMI_OTG_ADDON_OTG_ON);
+
+ u2o_clear(base, UTMI_CTRL, UTMI_CTRL_RXBUF_PDWN);
+ u2o_clear(base, UTMI_CTRL, UTMI_CTRL_TXBUF_PDWN);
+ u2o_clear(base, UTMI_CTRL, UTMI_CTRL_USB_CLK_EN);
+ u2o_clear(base, UTMI_CTRL, 1<<UTMI_CTRL_PWR_UP_SHIFT);
+ u2o_clear(base, UTMI_CTRL, 1<<UTMI_CTRL_PLL_PWR_UP_SHIFT);
+
+ return 0;
+}
+
+static int mv_usb2_phy_init(struct usb_phy *phy)
+{
+ struct mv_usb2_phy *mv_phy = container_of(phy, struct mv_usb2_phy, phy);
+ int i = 0;
+
+ mutex_lock(&mv_phy->phy_lock);
+ if (mv_phy->refcount++ == 0) {
+ for (i = 0; i < mv_phy->clks_num; i++)
+ clk_prepare_enable(mv_phy->clks[i]);
+ _mv_usb2_phy_init(mv_phy);
+ }
+ mutex_unlock(&mv_phy->phy_lock);
+ return 0;
+}
+
+static void mv_usb2_phy_shutdown(struct usb_phy *phy)
+{
+ struct mv_usb2_phy *mv_phy = container_of(phy, struct mv_usb2_phy, phy);
+ int i = 0;
+
+ mutex_lock(&mv_phy->phy_lock);
+ if (--mv_phy->refcount == 0) {
+ _mv_usb2_phy_shutdown(mv_phy);
+ for (i = 0; i < mv_phy->clks_num; i++)
+ clk_disable_unprepare(mv_phy->clks[i]);
+ }
+ mutex_unlock(&mv_phy->phy_lock);
+}
+
+static int mv_usb2_phy_probe(struct platform_device *pdev)
+{
+ struct mv_usb2_phy *mv_phy;
+ struct resource *r;
+ int i;
+ struct mv_usb_phy_platform_data *pdata;
+ const struct platform_device_id *id;
+
+ mv_phy = devm_kzalloc(&pdev->dev, sizeof(*mv_phy), GFP_KERNEL);
+ if (mv_phy == NULL) {
+ dev_err(&pdev->dev, "failed to allocate memory\n");
+ return -ENOMEM;
+ }
+ mutex_init(&mv_phy->phy_lock);
+
+ pdata = pdev->dev.platform_data;
+ id = platform_get_device_id(pdev);
+
+ if (pdata == NULL || id == NULL) {
+ dev_err(&pdev->dev,
+ "missing platform_data or id_entry\n");
+ return -ENODEV;
+ }
+ mv_phy->type = (unsigned int)(id->driver_data);
+ mv_phy->clks_num = pdata->clknum;
+ mv_phy->clks = devm_kzalloc(&pdev->dev,
+ sizeof(struct clk *) * mv_phy->clks_num, GFP_KERNEL);
+ if (mv_phy->clks == NULL) {
+ dev_err(&pdev->dev,
+ "failed to allocate mempory for clocks");
+ return -ENOMEM;
+ }
+ for (i = 0; i < mv_phy->clks_num; i++) {
+ mv_phy->clks[i] = devm_clk_get(&pdev->dev,
+ pdata->clkname[i]);
+ if (IS_ERR(mv_phy->clks[i])) {
+ dev_err(&pdev->dev, "failed to get clock %s\n",
+ pdata->clkname[i]);
+ return PTR_ERR(mv_phy->clks[i]);
+ }
+ }
+ r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (r == NULL) {
+ dev_err(&pdev->dev, "no phy I/O memory resource defined\n");
+ return -ENODEV;
+ }
+ mv_phy->base = devm_ioremap(&pdev->dev, r->start, resource_size(r));
+ if (mv_phy->base == NULL) {
+ dev_err(&pdev->dev, "error map register base\n");
+ return -EBUSY;
+ }
+
+ mv_phy->phy.dev = &pdev->dev;
+ mv_phy->phy.label = "mv-usb2";
+ mv_phy->phy.type = USB_PHY_TYPE_USB2;
+ mv_phy->phy.init = mv_usb2_phy_init;
+ mv_phy->phy.shutdown = mv_usb2_phy_shutdown;
+
+ usb_add_phy_dev(&mv_phy->phy);
+
+ platform_set_drvdata(pdev, mv_phy);
+
+ return 0;
+}
+
+static int mv_usb2_phy_remove(struct platform_device *pdev)
+{
+ struct mv_usb2_phy *mv_phy = platform_get_drvdata(pdev);
+
+ usb_remove_phy(&mv_phy->phy);
+
+ platform_set_drvdata(pdev, NULL);
+
+ return 0;
+}
+
+static struct platform_device_id mv_usb2_phy_ids[] = {
+ { .name = "pxa168-usb-phy", .driver_data = PXA168_USB },
+ { .name = "pxa910-usb-phy", .driver_data = PXA910_USB },
+ { .name = "mmp2-usb-phy", .driver_data = MMP2_USB },
+ {}
+};
+
+static struct platform_driver mv_usb2_phy_driver = {
+ .probe = mv_usb2_phy_probe,
+ .remove = mv_usb2_phy_remove,
+ .driver = {
+ .name = "pxa168-usb-phy",
+ },
+ .id_table = mv_usb2_phy_ids,
+};
+
+static int __init mv_usb2_phy_driver_init(void)
+{
+ return platform_driver_register(&mv_usb2_phy_driver);
+}
+arch_initcall(mv_usb2_phy_driver_init);
diff --git a/include/linux/platform_data/mv_usb.h b/include/linux/platform_data/mv_usb.h
index 944b01d..fd3d1b4 100644
--- a/include/linux/platform_data/mv_usb.h
+++ b/include/linux/platform_data/mv_usb.h
@@ -47,9 +47,12 @@ struct mv_usb_platform_data {
/* Force a_bus_req to be asserted */
unsigned int otg_force_a_bus_req:1;
- int (*phy_init)(void __iomem *regbase);
- void (*phy_deinit)(void __iomem *regbase);
int (*set_vbus)(unsigned int vbus);
- int (*private_init)(void __iomem *opregs, void __iomem *phyregs);
};
+
+struct mv_usb_phy_platform_data {
+ unsigned int clknum;
+ char **clkname;
+};
+
#endif
diff --git a/include/linux/usb/mv_usb2.h b/include/linux/usb/mv_usb2.h
new file mode 100644
index 0000000..77250f5
--- /dev/null
+++ b/include/linux/usb/mv_usb2.h
@@ -0,0 +1,32 @@
+/*
+ * Copyright (C) 2013 Marvell Inc.
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#ifndef __MV_USB2_H
+#define __MV_USB2_H
+
+#define MV_USB2_PHY_INDEX 0
+#define MV_USB2_OTG_PHY_INDEX 1
+
+struct mv_usb2_phy {
+ struct usb_phy phy;
+ struct platform_device *pdev;
+ struct mutex phy_lock;
+ unsigned int refcount;
+ unsigned int type;
+ void __iomem *base;
+ struct clk **clks;
+ unsigned int clks_num;
+};
+
+#endif
--
1.7.4.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* [V8 PATCH 01/16] usb: phy: mv_usb2: add PHY driver for marvell usb2 controller
2013-02-21 4:07 ` [V8 PATCH 01/16] usb: phy: mv_usb2: add PHY driver for marvell usb2 controller Chao Xie
@ 2013-03-04 14:21 ` Felipe Balbi
2013-03-05 2:03 ` Chao Xie
0 siblings, 1 reply; 34+ messages in thread
From: Felipe Balbi @ 2013-03-04 14:21 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Feb 20, 2013 at 11:07:11PM -0500, Chao Xie wrote:
> The PHY is seperated from usb controller.
> The usb controller used in marvell pxa168/pxa910/mmp2 are same,
> but PHY initialization may be different.
> the usb controller can support udc/otg/ehci, and for each of
> the mode, it need PHY to initialized before use the controller.
> Direclty writing the phy driver will make the usb controller
> driver to be simple and portable.
> The PHY driver will be used by marvell udc/otg/ehci.
>
> Signed-off-by: Chao Xie <chao.xie@marvell.com>
> ---
> drivers/usb/phy/Kconfig | 7 +
> drivers/usb/phy/Makefile | 1 +
> drivers/usb/phy/mv_usb2_phy.c | 402 ++++++++++++++++++++++++++++++++++
> include/linux/platform_data/mv_usb.h | 9 +-
> include/linux/usb/mv_usb2.h | 32 +++
> 5 files changed, 448 insertions(+), 3 deletions(-)
> create mode 100644 drivers/usb/phy/mv_usb2_phy.c
> create mode 100644 include/linux/usb/mv_usb2.h
>
> diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
> index 65217a5..5479760 100644
> --- a/drivers/usb/phy/Kconfig
> +++ b/drivers/usb/phy/Kconfig
> @@ -73,3 +73,10 @@ config SAMSUNG_USBPHY
> help
> Enable this to support Samsung USB phy controller for samsung
> SoCs.
> +
> +config MV_USB2_PHY
> + tristate "Marvell USB 2.0 PHY Driver"
> + depends on USB || USB_GADGET
NAK, PHYs don't depend on the gadget framework.
> + help
> + Enable this to support Marvell USB 2.0 phy driver for Marvell
> + SoC.
> diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile
> index b13faa1..3835316 100644
> --- a/drivers/usb/phy/Makefile
> +++ b/drivers/usb/phy/Makefile
> @@ -12,3 +12,4 @@ obj-$(CONFIG_MV_U3D_PHY) += mv_u3d_phy.o
> obj-$(CONFIG_USB_EHCI_TEGRA) += tegra_usb_phy.o
> obj-$(CONFIG_USB_RCAR_PHY) += rcar-phy.o
> obj-$(CONFIG_SAMSUNG_USBPHY) += samsung-usbphy.o
> +obj-$(CONFIG_MV_USB2_PHY) += mv_usb2_phy.o
> diff --git a/drivers/usb/phy/mv_usb2_phy.c b/drivers/usb/phy/mv_usb2_phy.c
> new file mode 100644
> index 0000000..a81e5e4
> --- /dev/null
> +++ b/drivers/usb/phy/mv_usb2_phy.c
> @@ -0,0 +1,402 @@
> +/*
> + * Copyright (C) 2013 Marvell Inc.
> + *
> + * Author:
> + * Chao Xie <xiechao.mail@gmail.com>
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include <linux/resource.h>
> +#include <linux/delay.h>
> +#include <linux/slab.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/io.h>
> +#include <linux/err.h>
> +#include <linux/clk.h>
> +#include <linux/export.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/platform_data/mv_usb.h>
> +#include <linux/usb/phy.h>
> +#include <linux/usb/mv_usb2.h>
> +
> +/* phy regs */
> +#define UTMI_REVISION 0x0
> +#define UTMI_CTRL 0x4
> +#define UTMI_PLL 0x8
> +#define UTMI_TX 0xc
> +#define UTMI_RX 0x10
> +#define UTMI_IVREF 0x14
> +#define UTMI_T0 0x18
> +#define UTMI_T1 0x1c
> +#define UTMI_T2 0x20
> +#define UTMI_T3 0x24
> +#define UTMI_T4 0x28
> +#define UTMI_T5 0x2c
> +#define UTMI_RESERVE 0x30
> +#define UTMI_USB_INT 0x34
> +#define UTMI_DBG_CTL 0x38
> +#define UTMI_OTG_ADDON 0x3c
prepend these with MV_USB_
> +enum mv_usb2_phy_type {
> + PXA168_USB,
> + PXA910_USB,
> + MMP2_USB,
> +};
> +
> +static unsigned int u2o_get(void __iomem *base, unsigned int offset)
> +{
> + return readl(base + offset);
> +}
> +
> +static void u2o_set(void __iomem *base, unsigned int offset,
> + unsigned int value)
> +{
> + u32 reg;
> +
> + reg = readl(base + offset);
> + reg |= value;
> + writel(reg, base + offset);
> + /*
> + * read after write. It will make sure writing takes effect.
> + * It is suggested by PHY design engineer.
> + */
> + readl(base + offset);
ewww... you really don't need (and *shouldn't* use) u2o_set() or
u2o_clear(). They clearly prevent compiler from optimizing variable
usage and could cause pressure on your interconnect. By writing and
reading multiple times for no reason.
> +static int _mv_usb2_phy_init(struct mv_usb2_phy *mv_phy)
> +{
> + struct platform_device *pdev = mv_phy->pdev;
> + unsigned int loops = 0;
> + void __iomem *base = mv_phy->base;
> +
> + dev_dbg(&pdev->dev, "phy init\n");
remove this debugging message.
> + /* Initialize the USB PHY power */
> + if (mv_phy->type == PXA910_USB) {
you _do_ have a REVISION register. Are you 100% certain that revision is
the same on all your devices ? It seems to me that you should be doing
proper revision detection instead of adding the hacky enumeration of
yours.
> + /* UTMI_IVREF */
> + if (mv_phy->type == PXA168_USB)
> + /* fixing Microsoft Altair board interface with NEC hub issue -
> + * Set UTMI_IVREF from 0x4a3 to 0x4bf */
wrong comment style. Run *ALL* your patches through checkpatch.pl
--strict and sparse.
> + u2o_write(base, UTMI_IVREF, 0x4bf);
> +
> + /* toggle VCOCAL_START bit of UTMI_PLL */
> + udelay(200);
why the udelay() calls ? Add a comment to code explaining them.
> + u2o_set(base, UTMI_PLL, VCOCAL_START);
> + udelay(40);
> + u2o_clear(base, UTMI_PLL, VCOCAL_START);
> +
> + /* toggle REG_RCAL_START bit of UTMI_TX */
> + udelay(400);
> + u2o_set(base, UTMI_TX, REG_RCAL_START);
> + udelay(40);
> + u2o_clear(base, UTMI_TX, REG_RCAL_START);
> + udelay(400);
> +
> + /* Make sure PHY PLL is ready */
> + loops = 0;
> + while ((u2o_get(base, UTMI_PLL) & PLL_READY) == 0) {
> + mdelay(1);
> + loops++;
> + if (loops > 100) {
> + dev_warn(&pdev->dev, "calibrate timeout, UTMI_PLL %x\n",
> + u2o_get(base, UTMI_PLL));
if this fails, shouldn't you return an error code ?
> +static int _mv_usb2_phy_shutdown(struct mv_usb2_phy *mv_phy)
> +{
> + void __iomem *base = mv_phy->base;
> +
> + if (mv_phy->type == PXA168_USB)
> + u2o_clear(base, UTMI_OTG_ADDON, UTMI_OTG_ADDON_OTG_ON);
> +
> + u2o_clear(base, UTMI_CTRL, UTMI_CTRL_RXBUF_PDWN);
> + u2o_clear(base, UTMI_CTRL, UTMI_CTRL_TXBUF_PDWN);
> + u2o_clear(base, UTMI_CTRL, UTMI_CTRL_USB_CLK_EN);
> + u2o_clear(base, UTMI_CTRL, 1<<UTMI_CTRL_PWR_UP_SHIFT);
> + u2o_clear(base, UTMI_CTRL, 1<<UTMI_CTRL_PLL_PWR_UP_SHIFT);
NAK, this is stupid, read once, clear bits you want to clear and write
only once.
> + return 0;
> +}
> +
> +static int mv_usb2_phy_init(struct usb_phy *phy)
> +{
> + struct mv_usb2_phy *mv_phy = container_of(phy, struct mv_usb2_phy, phy);
> + int i = 0;
> +
> + mutex_lock(&mv_phy->phy_lock);
what's this mutex for ?
> + if (mv_phy->refcount++ == 0) {
can this device really be used simultaneously by multiple devices ?
> + for (i = 0; i < mv_phy->clks_num; i++) {
> + mv_phy->clks[i] = devm_clk_get(&pdev->dev,
> + pdata->clkname[i]);
*NEVER* pass clock names via platform_data, this is utterly wrong.
> + if (IS_ERR(mv_phy->clks[i])) {
> + dev_err(&pdev->dev, "failed to get clock %s\n",
> + pdata->clkname[i]);
> + return PTR_ERR(mv_phy->clks[i]);
> + }
> + }
> + r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (r == NULL) {
> + dev_err(&pdev->dev, "no phy I/O memory resource defined\n");
> + return -ENODEV;
> + }
> + mv_phy->base = devm_ioremap(&pdev->dev, r->start, resource_size(r));
use devm_ioremap_resource() instead.
> +static struct platform_driver mv_usb2_phy_driver = {
> + .probe = mv_usb2_phy_probe,
> + .remove = mv_usb2_phy_remove,
> + .driver = {
> + .name = "pxa168-usb-phy",
> + },
> + .id_table = mv_usb2_phy_ids,
> +};
> +
> +static int __init mv_usb2_phy_driver_init(void)
> +{
> + return platform_driver_register(&mv_usb2_phy_driver);
> +}
> +arch_initcall(mv_usb2_phy_driver_init);
NAK, use module_platform_driver() like everybody else and handle
registration ordering with -EPROBE_DEFER.
> diff --git a/include/linux/platform_data/mv_usb.h b/include/linux/platform_data/mv_usb.h
> index 944b01d..fd3d1b4 100644
> --- a/include/linux/platform_data/mv_usb.h
> +++ b/include/linux/platform_data/mv_usb.h
> @@ -47,9 +47,12 @@ struct mv_usb_platform_data {
> /* Force a_bus_req to be asserted */
> unsigned int otg_force_a_bus_req:1;
>
> - int (*phy_init)(void __iomem *regbase);
> - void (*phy_deinit)(void __iomem *regbase);
> int (*set_vbus)(unsigned int vbus);
> - int (*private_init)(void __iomem *opregs, void __iomem *phyregs);
should be part of a separate patch.
> };
> +
> +struct mv_usb_phy_platform_data {
> + unsigned int clknum;
> + char **clkname;
> +};
NAK for this platform_data.
--
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130304/def7411d/attachment.sig>
^ permalink raw reply [flat|nested] 34+ messages in thread* [V8 PATCH 01/16] usb: phy: mv_usb2: add PHY driver for marvell usb2 controller
2013-03-04 14:21 ` Felipe Balbi
@ 2013-03-05 2:03 ` Chao Xie
2013-03-05 11:04 ` Felipe Balbi
2013-03-06 16:45 ` Russell King - ARM Linux
0 siblings, 2 replies; 34+ messages in thread
From: Chao Xie @ 2013-03-05 2:03 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Mar 4, 2013 at 10:21 PM, Felipe Balbi <balbi@ti.com> wrote:
> On Wed, Feb 20, 2013 at 11:07:11PM -0500, Chao Xie wrote:
>> The PHY is seperated from usb controller.
>> The usb controller used in marvell pxa168/pxa910/mmp2 are same,
>> but PHY initialization may be different.
>> the usb controller can support udc/otg/ehci, and for each of
>> the mode, it need PHY to initialized before use the controller.
>> Direclty writing the phy driver will make the usb controller
>> driver to be simple and portable.
>> The PHY driver will be used by marvell udc/otg/ehci.
>>
>> Signed-off-by: Chao Xie <chao.xie@marvell.com>
>> ---
>> drivers/usb/phy/Kconfig | 7 +
>> drivers/usb/phy/Makefile | 1 +
>> drivers/usb/phy/mv_usb2_phy.c | 402 ++++++++++++++++++++++++++++++++++
>> include/linux/platform_data/mv_usb.h | 9 +-
>> include/linux/usb/mv_usb2.h | 32 +++
>> 5 files changed, 448 insertions(+), 3 deletions(-)
>> create mode 100644 drivers/usb/phy/mv_usb2_phy.c
>> create mode 100644 include/linux/usb/mv_usb2.h
>>
>> diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
>> index 65217a5..5479760 100644
>> --- a/drivers/usb/phy/Kconfig
>> +++ b/drivers/usb/phy/Kconfig
>> @@ -73,3 +73,10 @@ config SAMSUNG_USBPHY
>> help
>> Enable this to support Samsung USB phy controller for samsung
>> SoCs.
>> +
>> +config MV_USB2_PHY
>> + tristate "Marvell USB 2.0 PHY Driver"
>> + depends on USB || USB_GADGET
>
> NAK, PHYs don't depend on the gadget framework.
Sure, i will remove it.
>
>> + help
>> + Enable this to support Marvell USB 2.0 phy driver for Marvell
>> + SoC.
>> diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile
>> index b13faa1..3835316 100644
>> --- a/drivers/usb/phy/Makefile
>> +++ b/drivers/usb/phy/Makefile
>> @@ -12,3 +12,4 @@ obj-$(CONFIG_MV_U3D_PHY) += mv_u3d_phy.o
>> obj-$(CONFIG_USB_EHCI_TEGRA) += tegra_usb_phy.o
>> obj-$(CONFIG_USB_RCAR_PHY) += rcar-phy.o
>> obj-$(CONFIG_SAMSUNG_USBPHY) += samsung-usbphy.o
>> +obj-$(CONFIG_MV_USB2_PHY) += mv_usb2_phy.o
>> diff --git a/drivers/usb/phy/mv_usb2_phy.c b/drivers/usb/phy/mv_usb2_phy.c
>> new file mode 100644
>> index 0000000..a81e5e4
>> --- /dev/null
>> +++ b/drivers/usb/phy/mv_usb2_phy.c
>> @@ -0,0 +1,402 @@
>> +/*
>> + * Copyright (C) 2013 Marvell Inc.
>> + *
>> + * Author:
>> + * Chao Xie <xiechao.mail@gmail.com>
>> + *
>> + * This software is licensed under the terms of the GNU General Public
>> + * License version 2, as published by the Free Software Foundation, and
>>
>> +#define UTMI_CTRL 0x4
>> +#define UTMI_PLL 0x8
>> +#define UTMI_TX 0xc
>> +#define UTMI_RX 0x10
>> +#define UTMI_IVREF 0x14
>> +#define UTMI_T0 0x18
>> +#define UTMI_T1 0x1c
>> +#define UTMI_T2 0x20
>> +#define UTMI_T3 0x24
>> +#define UTMI_T4 0x28
>> +#define UTMI_T5 0x2c
>> +#define UTMI_RESERVE 0x30
>> +#define UTMI_USB_INT 0x34
>> +#define UTMI_DBG_CTL 0x38
>> +#define UTMI_OTG_ADDON 0x3c
>
> prepend these with MV_USB_
Fine.
>
>> +enum mv_usb2_phy_type {
>> + PXA168_USB,
>> + PXA910_USB,
>> + MMP2_USB,
>> +};
>
>
> ewww... you really don't need (and *shouldn't* use) u2o_set() or
> u2o_clear(). They clearly prevent compiler from optimizing variable
> usage and could cause pressure on your interconnect. By writing and
> reading multiple times for no reason.
>
The APIs defined here is for device operation. The device register
read/write is not same as memory.
When a silicon comes, it may not be stable, and will have done some
workaround epecially for the
device register read/write. to define the APIs for the register
read/write will help to implement the workaround
without changing phy init code every time.
Now the only constrain is should read back the registers if you have
writen to it.
It will low down the performance, but it does not matter. Because phy
init will only done once when you initialize it.
I will think about reove the u2o_xxx APIs.
>> +static int _mv_usb2_phy_init(struct mv_usb2_phy *mv_phy)
>> +{
>> + struct platform_device *pdev = mv_phy->pdev;
>> + unsigned int loops = 0;
>> + void __iomem *base = mv_phy->base;
>> +
>> + dev_dbg(&pdev->dev, "phy init\n");
>
> remove this debugging message.
>
>> + /* Initialize the USB PHY power */
>> + if (mv_phy->type == PXA910_USB) {
>
> you _do_ have a REVISION register. Are you 100% certain that revision is
> the same on all your devices ? It seems to me that you should be doing
> proper revision detection instead of adding the hacky enumeration of
> yours.
We do not have revison registers and will do not want ot define
#ifdef CPU_PXA910 or CPU_PXA_XXX
or
use is_cpu_pxa910 or cpu_pxa_xxx
because it is not acceptable. for example, if we have new SOC and it
use the same PHY as pxa910
i have to change the USB driver code to support it. for example
#ifdef CPU_PXA910 || CPU_XXX
So i have to depends on the device_id to do the work.
>
>> + /* UTMI_IVREF */
>> + if (mv_phy->type == PXA168_USB)
>> + /* fixing Microsoft Altair board interface with NEC hub issue -
>> + * Set UTMI_IVREF from 0x4a3 to 0x4bf */
>
> wrong comment style. Run *ALL* your patches through checkpatch.pl
> --strict and sparse.
>
It seems that checkpatch.pl can not detect everything. I really use
checpatch.pl for
every patch i sent to maillist.
sorry for that, i will fix it.
>> + u2o_write(base, UTMI_IVREF, 0x4bf);
>> +
>> + /* toggle VCOCAL_START bit of UTMI_PLL */
>> + udelay(200);
>
> why the udelay() calls ? Add a comment to code explaining them.
>
>> + u2o_set(base, UTMI_PLL, VCOCAL_START);
>> + udelay(40);
>> + u2o_clear(base, UTMI_PLL, VCOCAL_START);
>> +
>> + /* toggle REG_RCAL_START bit of UTMI_TX */
>> + udelay(400);
>> + u2o_set(base, UTMI_TX, REG_RCAL_START);
>> + udelay(40);
>> + u2o_clear(base, UTMI_TX, REG_RCAL_START);
>> + udelay(400);
>> +
>> + /* Make sure PHY PLL is ready */
>> + loops = 0;
>> + while ((u2o_get(base, UTMI_PLL) & PLL_READY) == 0) {
>> + mdelay(1);
>> + loops++;
>> + if (loops > 100) {
>> + dev_warn(&pdev->dev, "calibrate timeout, UTMI_PLL %x\n",
>> + u2o_get(base, UTMI_PLL));
>
> if this fails, shouldn't you return an error code ?
>
yes. it should return the error code.
>> +static int _mv_usb2_phy_shutdown(struct mv_usb2_phy *mv_phy)
>> +{
>> + void __iomem *base = mv_phy->base;
>> +
>> + if (mv_phy->type == PXA168_USB)
>> + u2o_clear(base, UTMI_OTG_ADDON, UTMI_OTG_ADDON_OTG_ON);
>> +
>> + u2o_clear(base, UTMI_CTRL, UTMI_CTRL_RXBUF_PDWN);
>> + u2o_clear(base, UTMI_CTRL, UTMI_CTRL_TXBUF_PDWN);
>> + u2o_clear(base, UTMI_CTRL, UTMI_CTRL_USB_CLK_EN);
>> + u2o_clear(base, UTMI_CTRL, 1<<UTMI_CTRL_PWR_UP_SHIFT);
>> + u2o_clear(base, UTMI_CTRL, 1<<UTMI_CTRL_PLL_PWR_UP_SHIFT);
>
> NAK, this is stupid, read once, clear bits you want to clear and write
> only once.
>
I will check with silicon design engineer. becuase all the phy init
code is delivered by them.
I can not tell that if there are any speciall reason they will do so
because the device register
read/write is not same as normal memory.
>> + return 0;
>> +}
>> +
>> +static int mv_usb2_phy_init(struct usb_phy *phy)
>> +{
>> + struct mv_usb2_phy *mv_phy = container_of(phy, struct mv_usb2_phy, phy);
>> + int i = 0;
>> +
>> + mutex_lock(&mv_phy->phy_lock);
>
> what's this mutex for ?
>
>> + if (mv_phy->refcount++ == 0) {
>
> can this device really be used simultaneously by multiple devices ?
>
Sure, we have another EHCI device, it will share the PHY with this USB
controller.
So we will use refcount and mutext to protect the phy init.
>> + for (i = 0; i < mv_phy->clks_num; i++) {
>> + mv_phy->clks[i] = devm_clk_get(&pdev->dev,
>> + pdata->clkname[i]);
>
> *NEVER* pass clock names via platform_data, this is utterly wrong.
>
without device tree support, the only way we can get the clock is the pdata.
the use phy have mutiple clocks.
So what do you suggest to handle it?
>> + if (IS_ERR(mv_phy->clks[i])) {
>> + dev_err(&pdev->dev, "failed to get clock %s\n",
>> + pdata->clkname[i]);
>> + return PTR_ERR(mv_phy->clks[i]);
>> + }
>> + }
>> + r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> + if (r == NULL) {
>> + dev_err(&pdev->dev, "no phy I/O memory resource defined\n");
>> + return -ENODEV;
>> + }
>> + mv_phy->base = devm_ioremap(&pdev->dev, r->start, resource_size(r));
>
> use devm_ioremap_resource() instead.
>
fine.
>> +static struct platform_driver mv_usb2_phy_driver = {
>> + .probe = mv_usb2_phy_probe,
>> + .remove = mv_usb2_phy_remove,
>> + .driver = {
>> + .name = "pxa168-usb-phy",
>> + },
>> + .id_table = mv_usb2_phy_ids,
>> +};
>> +
>> +static int __init mv_usb2_phy_driver_init(void)
>> +{
>> + return platform_driver_register(&mv_usb2_phy_driver);
>> +}
>> +arch_initcall(mv_usb2_phy_driver_init);
>
> NAK, use module_platform_driver() like everybody else and handle
> registration ordering with -EPROBE_DEFER.
>
The reason i do not use module_platform_driver is the compiling
sequence of each directory of driver/usb/
the phy is compiled after otg/ehci. So it means that it can not find
the usb phy, and will register fail.
>> diff --git a/include/linux/platform_data/mv_usb.h b/include/linux/platform_data/mv_usb.h
>> index 944b01d..fd3d1b4 100644
>> --- a/include/linux/platform_data/mv_usb.h
>> +++ b/include/linux/platform_data/mv_usb.h
>> @@ -47,9 +47,12 @@ struct mv_usb_platform_data {
>> /* Force a_bus_req to be asserted */
>> unsigned int otg_force_a_bus_req:1;
>>
>> - int (*phy_init)(void __iomem *regbase);
>> - void (*phy_deinit)(void __iomem *regbase);
>> int (*set_vbus)(unsigned int vbus);
>> - int (*private_init)(void __iomem *opregs, void __iomem *phyregs);
>
> should be part of a separate patch.
>
>> };
>> +
>> +struct mv_usb_phy_platform_data {
>> + unsigned int clknum;
>> + char **clkname;
>> +};
>
> NAK for this platform_data.
>
> --
> balbi
^ permalink raw reply [flat|nested] 34+ messages in thread* [V8 PATCH 01/16] usb: phy: mv_usb2: add PHY driver for marvell usb2 controller
2013-03-05 2:03 ` Chao Xie
@ 2013-03-05 11:04 ` Felipe Balbi
2013-03-05 16:43 ` Alan Stern
2013-03-06 2:11 ` Chao Xie
2013-03-06 16:45 ` Russell King - ARM Linux
1 sibling, 2 replies; 34+ messages in thread
From: Felipe Balbi @ 2013-03-05 11:04 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
On Tue, Mar 05, 2013 at 10:03:01AM +0800, Chao Xie wrote:
> >> +enum mv_usb2_phy_type {
> >> + PXA168_USB,
> >> + PXA910_USB,
> >> + MMP2_USB,
> >> +};
> >
> >
> > ewww... you really don't need (and *shouldn't* use) u2o_set() or
> > u2o_clear(). They clearly prevent compiler from optimizing variable
> > usage and could cause pressure on your interconnect. By writing and
> > reading multiple times for no reason.
> >
>
> The APIs defined here is for device operation. The device register
> read/write is not same as memory.
> When a silicon comes, it may not be stable, and will have done some
> workaround epecially for the device register read/write. to define the
> APIs for the register read/write will help to implement the workaround
> without changing phy init code every time.
> Now the only constrain is should read back the registers if you have
> writen to it.
> It will low down the performance, but it does not matter. Because phy
> init will only done once when you initialize it.
> I will think about reove the u2o_xxx APIs.
You didn't even understand what I meant. Seriously.
Anyway, details are as follows:
readl() and writel() always add a memory barrier around each operation.
This is good because it makes sure memory mapped I/O region is always
ordered, but your current usage will post reads and writes on the
interconnect over and over again for no reason. What you should do, with
any register access is:
reg = readl();
reg &= ~BITS_TO_DISABLE;
reg |= BITS_TO_ENABLE;
writel(reg);
Whereas your current code does:
reg = readl();
reg &= ~BITS_TO_DISABLE;
writel(reg);
reg = readl();
reg |= BITS_TO_ENABLE;
writel(reg);
You do that for each bit, in some cases.
> >> +static int _mv_usb2_phy_init(struct mv_usb2_phy *mv_phy)
> >> +{
> >> + struct platform_device *pdev = mv_phy->pdev;
> >> + unsigned int loops = 0;
> >> + void __iomem *base = mv_phy->base;
> >> +
> >> + dev_dbg(&pdev->dev, "phy init\n");
> >
> > remove this debugging message.
> >
> >> + /* Initialize the USB PHY power */
> >> + if (mv_phy->type == PXA910_USB) {
> >
> > you _do_ have a REVISION register. Are you 100% certain that revision is
> > the same on all your devices ? It seems to me that you should be doing
> > proper revision detection instead of adding the hacky enumeration of
> > yours.
>
> We do not have revison registers and will do not want ot define
> #ifdef CPU_PXA910 or CPU_PXA_XXX
> or
> use is_cpu_pxa910 or cpu_pxa_xxx
> because it is not acceptable. for example, if we have new SOC and it
> use the same PHY as pxa910
> i have to change the USB driver code to support it. for example
> #ifdef CPU_PXA910 || CPU_XXX
what a load of crap.
*read your code!!!*
There is a UTMI_REVISION register defined at offset 0.
> So i have to depends on the device_id to do the work.
> >
> >> + /* UTMI_IVREF */
> >> + if (mv_phy->type == PXA168_USB)
> >> + /* fixing Microsoft Altair board interface with NEC hub issue -
> >> + * Set UTMI_IVREF from 0x4a3 to 0x4bf */
> >
> > wrong comment style. Run *ALL* your patches through checkpatch.pl
> > --strict and sparse.
> >
>
> It seems that checkpatch.pl can not detect everything. I really use
> checpatch.pl for
> every patch i sent to maillist.
> sorry for that, i will fix it.
did you use --strict ?
> >> +static int _mv_usb2_phy_shutdown(struct mv_usb2_phy *mv_phy)
> >> +{
> >> + void __iomem *base = mv_phy->base;
> >> +
> >> + if (mv_phy->type == PXA168_USB)
> >> + u2o_clear(base, UTMI_OTG_ADDON, UTMI_OTG_ADDON_OTG_ON);
> >> +
> >> + u2o_clear(base, UTMI_CTRL, UTMI_CTRL_RXBUF_PDWN);
> >> + u2o_clear(base, UTMI_CTRL, UTMI_CTRL_TXBUF_PDWN);
> >> + u2o_clear(base, UTMI_CTRL, UTMI_CTRL_USB_CLK_EN);
> >> + u2o_clear(base, UTMI_CTRL, 1<<UTMI_CTRL_PWR_UP_SHIFT);
> >> + u2o_clear(base, UTMI_CTRL, 1<<UTMI_CTRL_PLL_PWR_UP_SHIFT);
> >
> > NAK, this is stupid, read once, clear bits you want to clear and write
> > only once.
> >
>
> I will check with silicon design engineer. becuase all the phy init
> code is delivered by them.
right, but you need to be critical with any code you read. You can't
simply blindly make it compile on linux and hope that it'll work
efficiently.
I doubt your silicon validation evironment has support for the memory
barriers which are added on each readl()/writel() calls.
> I can not tell that if there are any speciall reason they will do so
> because the device register read/write is not same as normal memory.
seriously ? Read your documentation dude, there's nothing that will
require you to enable one bit in the register, then flush the write,
then write another bit, flush the write, write another bit, flush the
write and so on.
Those cases are extremely rare (MUSB has only *ONE* such case with
regards to DMA_MODE bit) and in 99% of the cases, you can write
everything to a variable and post a single write to your interconnect.
Stop trying to come up with nonsensical excuses that "register
read/write is not same as normal memory", of course it's not same as
normal memory, and that's why we have standardized I/O functions.
> >> + return 0;
> >> +}
> >> +
> >> +static int mv_usb2_phy_init(struct usb_phy *phy)
> >> +{
> >> + struct mv_usb2_phy *mv_phy = container_of(phy, struct mv_usb2_phy, phy);
> >> + int i = 0;
> >> +
> >> + mutex_lock(&mv_phy->phy_lock);
> >
> > what's this mutex for ?
> >
> >> + if (mv_phy->refcount++ == 0) {
> >
> > can this device really be used simultaneously by multiple devices ?
> >
>
> Sure, we have another EHCI device, it will share the PHY with this USB
> controller.
what is "this USB controller", current driver is for the PHY only. Do
you mean to say that PHY will be shared by EHCI and UDC ?
> So we will use refcount and mutext to protect the phy init.
in that case, it might be better to add a generic refcounting to the PHY
layer as a whole, instead of cooking your own private solutions.
> >> + for (i = 0; i < mv_phy->clks_num; i++) {
> >> + mv_phy->clks[i] = devm_clk_get(&pdev->dev,
> >> + pdata->clkname[i]);
> >
> > *NEVER* pass clock names via platform_data, this is utterly wrong.
> >
> without device tree support, the only way we can get the clock is the pdata.
> the use phy have mutiple clocks.
> So what do you suggest to handle it?
clkdev
> >> +static struct platform_driver mv_usb2_phy_driver = {
> >> + .probe = mv_usb2_phy_probe,
> >> + .remove = mv_usb2_phy_remove,
> >> + .driver = {
> >> + .name = "pxa168-usb-phy",
> >> + },
> >> + .id_table = mv_usb2_phy_ids,
> >> +};
> >> +
> >> +static int __init mv_usb2_phy_driver_init(void)
> >> +{
> >> + return platform_driver_register(&mv_usb2_phy_driver);
> >> +}
> >> +arch_initcall(mv_usb2_phy_driver_init);
> >
> > NAK, use module_platform_driver() like everybody else and handle
> > registration ordering with -EPROBE_DEFER.
> >
>
> The reason i do not use module_platform_driver is the compiling
> sequence of each directory of driver/usb/
> the phy is compiled after otg/ehci. So it means that it can not find
> the usb phy, and will register fail.
it doesn't matter, you should teach EHCI about -EPROBE_DEFER. If it
can't grab the PHY, return -EPROBE_DEFER.
--
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130305/6bef20d9/attachment.sig>
^ permalink raw reply [flat|nested] 34+ messages in thread* [V8 PATCH 01/16] usb: phy: mv_usb2: add PHY driver for marvell usb2 controller
2013-03-05 11:04 ` Felipe Balbi
@ 2013-03-05 16:43 ` Alan Stern
2013-03-05 17:20 ` Felipe Balbi
2013-03-06 2:11 ` Chao Xie
1 sibling, 1 reply; 34+ messages in thread
From: Alan Stern @ 2013-03-05 16:43 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, 5 Mar 2013, Felipe Balbi wrote:
> Anyway, details are as follows:
>
> readl() and writel() always add a memory barrier around each operation.
Is that supposed to be true on all architectures or only on ARM?
Alan Stern
^ permalink raw reply [flat|nested] 34+ messages in thread
* [V8 PATCH 01/16] usb: phy: mv_usb2: add PHY driver for marvell usb2 controller
2013-03-05 16:43 ` Alan Stern
@ 2013-03-05 17:20 ` Felipe Balbi
0 siblings, 0 replies; 34+ messages in thread
From: Felipe Balbi @ 2013-03-05 17:20 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Mar 05, 2013 at 11:43:35AM -0500, Alan Stern wrote:
> On Tue, 5 Mar 2013, Felipe Balbi wrote:
>
> > Anyway, details are as follows:
> >
> > readl() and writel() always add a memory barrier around each operation.
>
> Is that supposed to be true on all architectures or only on ARM?
I believe it's true for every architecture which doesn't have strongly
ordered memory accesses. ARM is just one example of that ;-)
In any case, his read->set 1 bit->write loop isn't good even for
architectures with strongly ordered memory accesses. It will continue to
post unnecessary reads and writes to the interconnect.
--
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130305/24df4eb1/attachment.sig>
^ permalink raw reply [flat|nested] 34+ messages in thread
* [V8 PATCH 01/16] usb: phy: mv_usb2: add PHY driver for marvell usb2 controller
2013-03-05 11:04 ` Felipe Balbi
2013-03-05 16:43 ` Alan Stern
@ 2013-03-06 2:11 ` Chao Xie
2013-03-06 8:10 ` Felipe Balbi
1 sibling, 1 reply; 34+ messages in thread
From: Chao Xie @ 2013-03-06 2:11 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Mar 5, 2013 at 7:04 PM, Felipe Balbi <balbi@ti.com> wrote:
> Hi,
>
> On Tue, Mar 05, 2013 at 10:03:01AM +0800, Chao Xie wrote:
>> >> +enum mv_usb2_phy_type {
>> >> + PXA168_USB,
>> >> + PXA910_USB,
>> >> + MMP2_USB,
>> >> +};
>> >
>> >
>> > ewww... you really don't need (and *shouldn't* use) u2o_set() or
>> > u2o_clear(). They clearly prevent compiler from optimizing variable
>> > usage and could cause pressure on your interconnect. By writing and
>> > reading multiple times for no reason.
>> >
>>
>> The APIs defined here is for device operation. The device register
>> read/write is not same as memory.
>> When a silicon comes, it may not be stable, and will have done some
>> workaround epecially for the device register read/write. to define the
>> APIs for the register read/write will help to implement the workaround
>> without changing phy init code every time.
>> Now the only constrain is should read back the registers if you have
>> writen to it.
>> It will low down the performance, but it does not matter. Because phy
>> init will only done once when you initialize it.
>> I will think about reove the u2o_xxx APIs.
>
> You didn't even understand what I meant. Seriously.
>
> Anyway, details are as follows:
>
> readl() and writel() always add a memory barrier around each operation.
>
> This is good because it makes sure memory mapped I/O region is always
> ordered, but your current usage will post reads and writes on the
> interconnect over and over again for no reason. What you should do, with
> any register access is:
>
> reg = readl();
>
> reg &= ~BITS_TO_DISABLE;
> reg |= BITS_TO_ENABLE;
>
> writel(reg);
>
> Whereas your current code does:
>
> reg = readl();
> reg &= ~BITS_TO_DISABLE;
> writel(reg);
>
> reg = readl();
> reg |= BITS_TO_ENABLE;
> writel(reg);
>
> You do that for each bit, in some cases.
>
>> >> +static int _mv_usb2_phy_init(struct mv_usb2_phy *mv_phy)
>> >> +{
>> >> + struct platform_device *pdev = mv_phy->pdev;
>> >> + unsigned int loops = 0;
>> >> + void __iomem *base = mv_phy->base;
>> >> +
>> >> + dev_dbg(&pdev->dev, "phy init\n");
>> >
>> > remove this debugging message.
>> >
>> >> + /* Initialize the USB PHY power */
>> >> + if (mv_phy->type == PXA910_USB) {
>> >
>> > you _do_ have a REVISION register. Are you 100% certain that revision is
>> > the same on all your devices ? It seems to me that you should be doing
>> > proper revision detection instead of adding the hacky enumeration of
>> > yours.
>>
>> We do not have revison registers and will do not want ot define
>> #ifdef CPU_PXA910 or CPU_PXA_XXX
>> or
>> use is_cpu_pxa910 or cpu_pxa_xxx
>> because it is not acceptable. for example, if we have new SOC and it
>> use the same PHY as pxa910
>> i have to change the USB driver code to support it. for example
>> #ifdef CPU_PXA910 || CPU_XXX
>
> what a load of crap.
>
> *read your code!!!*
>
> There is a UTMI_REVISION register defined at offset 0.
>
>> So i have to depends on the device_id to do the work.
>> >
>> >> + /* UTMI_IVREF */
>> >> + if (mv_phy->type == PXA168_USB)
>> >> + /* fixing Microsoft Altair board interface with NEC hub issue -
>> >> + * Set UTMI_IVREF from 0x4a3 to 0x4bf */
>> >
>> > wrong comment style. Run *ALL* your patches through checkpatch.pl
>> > --strict and sparse.
>> >
>>
>> It seems that checkpatch.pl can not detect everything. I really use
>> checpatch.pl for
>> every patch i sent to maillist.
>> sorry for that, i will fix it.
>
> did you use --strict ?
>
>> >> +static int _mv_usb2_phy_shutdown(struct mv_usb2_phy *mv_phy)
>> >> +{
>> >> + void __iomem *base = mv_phy->base;
>> >> +
>> >> + if (mv_phy->type == PXA168_USB)
>> >> + u2o_clear(base, UTMI_OTG_ADDON, UTMI_OTG_ADDON_OTG_ON);
>> >> +
>> >> + u2o_clear(base, UTMI_CTRL, UTMI_CTRL_RXBUF_PDWN);
>> >> + u2o_clear(base, UTMI_CTRL, UTMI_CTRL_TXBUF_PDWN);
>> >> + u2o_clear(base, UTMI_CTRL, UTMI_CTRL_USB_CLK_EN);
>> >> + u2o_clear(base, UTMI_CTRL, 1<<UTMI_CTRL_PWR_UP_SHIFT);
>> >> + u2o_clear(base, UTMI_CTRL, 1<<UTMI_CTRL_PLL_PWR_UP_SHIFT);
>> >
>> > NAK, this is stupid, read once, clear bits you want to clear and write
>> > only once.
>> >
>>
>> I will check with silicon design engineer. becuase all the phy init
>> code is delivered by them.
>
> right, but you need to be critical with any code you read. You can't
> simply blindly make it compile on linux and hope that it'll work
> efficiently.
>
> I doubt your silicon validation evironment has support for the memory
> barriers which are added on each readl()/writel() calls.
>
>> I can not tell that if there are any speciall reason they will do so
>> because the device register read/write is not same as normal memory.
>
> seriously ? Read your documentation dude, there's nothing that will
> require you to enable one bit in the register, then flush the write,
> then write another bit, flush the write, write another bit, flush the
> write and so on.
>
> Those cases are extremely rare (MUSB has only *ONE* such case with
> regards to DMA_MODE bit) and in 99% of the cases, you can write
> everything to a variable and post a single write to your interconnect.
>
> Stop trying to come up with nonsensical excuses that "register
> read/write is not same as normal memory", of course it's not same as
> normal memory, and that's why we have standardized I/O functions.
>
>> >> + return 0;
>> >> +}
>> >> +
>> >> +static int mv_usb2_phy_init(struct usb_phy *phy)
>> >> +{
>> >> + struct mv_usb2_phy *mv_phy = container_of(phy, struct mv_usb2_phy, phy);
>> >> + int i = 0;
>> >> +
>> >> + mutex_lock(&mv_phy->phy_lock);
>> >
>> > what's this mutex for ?
>> >
>> >> + if (mv_phy->refcount++ == 0) {
>> >
>> > can this device really be used simultaneously by multiple devices ?
>> >
>>
>> Sure, we have another EHCI device, it will share the PHY with this USB
>> controller.
>
> what is "this USB controller", current driver is for the PHY only. Do
> you mean to say that PHY will be shared by EHCI and UDC ?
>
>> So we will use refcount and mutext to protect the phy init.
>
> in that case, it might be better to add a generic refcounting to the PHY
> layer as a whole, instead of cooking your own private solutions.
>
>> >> + for (i = 0; i < mv_phy->clks_num; i++) {
>> >> + mv_phy->clks[i] = devm_clk_get(&pdev->dev,
>> >> + pdata->clkname[i]);
>> >
>> > *NEVER* pass clock names via platform_data, this is utterly wrong.
>> >
>> without device tree support, the only way we can get the clock is the pdata.
>> the use phy have mutiple clocks.
>> So what do you suggest to handle it?
>
> clkdev
>
>> >> +static struct platform_driver mv_usb2_phy_driver = {
>> >> + .probe = mv_usb2_phy_probe,
>> >> + .remove = mv_usb2_phy_remove,
>> >> + .driver = {
>> >> + .name = "pxa168-usb-phy",
>> >> + },
>> >> + .id_table = mv_usb2_phy_ids,
>> >> +};
>> >> +
>> >> +static int __init mv_usb2_phy_driver_init(void)
>> >> +{
>> >> + return platform_driver_register(&mv_usb2_phy_driver);
>> >> +}
>> >> +arch_initcall(mv_usb2_phy_driver_init);
>> >
>> > NAK, use module_platform_driver() like everybody else and handle
>> > registration ordering with -EPROBE_DEFER.
>> >
>>
>> The reason i do not use module_platform_driver is the compiling
>> sequence of each directory of driver/usb/
>> the phy is compiled after otg/ehci. So it means that it can not find
>> the usb phy, and will register fail.
>
> it doesn't matter, you should teach EHCI about -EPROBE_DEFER. If it
> can't grab the PHY, return -EPROBE_DEFER.
>
> --
> balbi
Thanks for the comments, i summary the feedbacks and suggestions as below,
1. For the phy setting and u2o_xx APIs something, i will optimize it.
If there are some special cases, i will add comments.
2. For the module_platform_driver, -EPROBE_DEFER is what i missed. I
will check it and thanks for suggestion.
3. For the revison register. It exists in some SOCes(pxa168), but for
some SOCes, the register dispears(pxa910, armada610). These SOCes are
developed by different desgin teams, and it need to be enhanced, but
for current products i have to use the device_id to detect the PHY
controller.
4. For the mutex and refcount. The "USB controller" includes two
blocks - the udc and ehci. In fact they will not be used at same time,
but for some SOCes it duplicates the ehci block. It means that the
SOCes have two or more ehci blocks. The added ehci blocks depend on
the PHY to be initialized, and they can be used at same time as the
"USB controller". That is the reason i add mutex and refcount for phy
driver.
5. For the clock name. Can you describe it in details? For clkdev, if
it want to find the clock, it still depends on the dev_name and
con_id.
^ permalink raw reply [flat|nested] 34+ messages in thread* [V8 PATCH 01/16] usb: phy: mv_usb2: add PHY driver for marvell usb2 controller
2013-03-06 2:11 ` Chao Xie
@ 2013-03-06 8:10 ` Felipe Balbi
2013-03-06 8:24 ` Chao Xie
0 siblings, 1 reply; 34+ messages in thread
From: Felipe Balbi @ 2013-03-06 8:10 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
On Wed, Mar 06, 2013 at 10:11:41AM +0800, Chao Xie wrote:
> 3. For the revison register. It exists in some SOCes(pxa168), but for
> some SOCes, the register dispears(pxa910, armada610). These SOCes are
> developed by different desgin teams, and it need to be enhanced, but
> for current products i have to use the device_id to detect the PHY
> controller.
fair enough, make sure to add a comment to the driver about this so
grumpy maintainers stop complaining ;-)
> 4. For the mutex and refcount. The "USB controller" includes two
> blocks - the udc and ehci. In fact they will not be used at same time,
> but for some SOCes it duplicates the ehci block. It means that the
> SOCes have two or more ehci blocks. The added ehci blocks depend on
> the PHY to be initialized, and they can be used at same time as the
> "USB controller". That is the reason i add mutex and refcount for phy
> driver.
alright, let's add refcounting to the generic PHY layer then.
> 5. For the clock name. Can you describe it in details? For clkdev, if
> it want to find the clock, it still depends on the dev_name and
> con_id.
right, the idea of clkdev is that you associate a clock provider with
its consumer by means of dev_name. Then you can fetch the clock from
driver with any name you want. Which means if you do you clkdev properly
you could:
clk_get(dev, "clk1");
clk_get(dev, "clk2");
...
and so on.
good luck
--
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130306/c75c0f9c/attachment.sig>
^ permalink raw reply [flat|nested] 34+ messages in thread
* [V8 PATCH 01/16] usb: phy: mv_usb2: add PHY driver for marvell usb2 controller
2013-03-06 8:10 ` Felipe Balbi
@ 2013-03-06 8:24 ` Chao Xie
2013-03-06 8:53 ` Felipe Balbi
2013-03-06 16:48 ` Russell King - ARM Linux
0 siblings, 2 replies; 34+ messages in thread
From: Chao Xie @ 2013-03-06 8:24 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Mar 6, 2013 at 4:10 PM, Felipe Balbi <balbi@ti.com> wrote:
> Hi,
>
> On Wed, Mar 06, 2013 at 10:11:41AM +0800, Chao Xie wrote:
>> 3. For the revison register. It exists in some SOCes(pxa168), but for
>> some SOCes, the register dispears(pxa910, armada610). These SOCes are
>> developed by different desgin teams, and it need to be enhanced, but
>> for current products i have to use the device_id to detect the PHY
>> controller.
>
> fair enough, make sure to add a comment to the driver about this so
> grumpy maintainers stop complaining ;-)
>
>> 4. For the mutex and refcount. The "USB controller" includes two
>> blocks - the udc and ehci. In fact they will not be used at same time,
>> but for some SOCes it duplicates the ehci block. It means that the
>> SOCes have two or more ehci blocks. The added ehci blocks depend on
>> the PHY to be initialized, and they can be used at same time as the
>> "USB controller". That is the reason i add mutex and refcount for phy
>> driver.
>
> alright, let's add refcounting to the generic PHY layer then.
>
>> 5. For the clock name. Can you describe it in details? For clkdev, if
>> it want to find the clock, it still depends on the dev_name and
>> con_id.
>
> right, the idea of clkdev is that you associate a clock provider with
> its consumer by means of dev_name. Then you can fetch the clock from
> driver with any name you want. Which means if you do you clkdev properly
> you could:
>
> clk_get(dev, "clk1");
> clk_get(dev, "clk2");
> ...
>
> and so on.
>
It is same as what i think.
The clock numbers and names are depent of SOCes, i do not want to fix
it in ther driver. So i use pdata to pass the clknum and clk names
"clk1", "clk2" and so on.
in the driver, i use
devm_clk_get(&pdev->dev, pdata->clkname[i]);
Do you think it is acceptable?
When device tree for clock framework are supported, the things get
easier, and it is my next step.
> good luck
>
> --
> balbi
^ permalink raw reply [flat|nested] 34+ messages in thread
* [V8 PATCH 01/16] usb: phy: mv_usb2: add PHY driver for marvell usb2 controller
2013-03-06 8:24 ` Chao Xie
@ 2013-03-06 8:53 ` Felipe Balbi
2013-03-06 9:02 ` Chao Xie
2013-03-06 16:48 ` Russell King - ARM Linux
1 sibling, 1 reply; 34+ messages in thread
From: Felipe Balbi @ 2013-03-06 8:53 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Mar 06, 2013 at 04:24:58PM +0800, Chao Xie wrote:
> On Wed, Mar 6, 2013 at 4:10 PM, Felipe Balbi <balbi@ti.com> wrote:
> > Hi,
> >
> > On Wed, Mar 06, 2013 at 10:11:41AM +0800, Chao Xie wrote:
> >> 3. For the revison register. It exists in some SOCes(pxa168), but for
> >> some SOCes, the register dispears(pxa910, armada610). These SOCes are
> >> developed by different desgin teams, and it need to be enhanced, but
> >> for current products i have to use the device_id to detect the PHY
> >> controller.
> >
> > fair enough, make sure to add a comment to the driver about this so
> > grumpy maintainers stop complaining ;-)
> >
> >> 4. For the mutex and refcount. The "USB controller" includes two
> >> blocks - the udc and ehci. In fact they will not be used at same time,
> >> but for some SOCes it duplicates the ehci block. It means that the
> >> SOCes have two or more ehci blocks. The added ehci blocks depend on
> >> the PHY to be initialized, and they can be used at same time as the
> >> "USB controller". That is the reason i add mutex and refcount for phy
> >> driver.
> >
> > alright, let's add refcounting to the generic PHY layer then.
> >
> >> 5. For the clock name. Can you describe it in details? For clkdev, if
> >> it want to find the clock, it still depends on the dev_name and
> >> con_id.
> >
> > right, the idea of clkdev is that you associate a clock provider with
> > its consumer by means of dev_name. Then you can fetch the clock from
> > driver with any name you want. Which means if you do you clkdev properly
> > you could:
> >
> > clk_get(dev, "clk1");
> > clk_get(dev, "clk2");
> > ...
> >
> > and so on.
> >
>
> It is same as what i think.
> The clock numbers and names are depent of SOCes, i do not want to fix
no, they're not dependent on anything, not if you use clkdev correctly.
> it in ther driver. So i use pdata to pass the clknum and clk names
> "clk1", "clk2" and so on.
> in the driver, i use
> devm_clk_get(&pdev->dev, pdata->clkname[i]);
> Do you think it is acceptable?
no
> When device tree for clock framework are supported, the things get
> easier, and it is my next step.
not strong enough argument to accept passing clock names via
platform_data.
--
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130306/826acfb5/attachment.sig>
^ permalink raw reply [flat|nested] 34+ messages in thread
* [V8 PATCH 01/16] usb: phy: mv_usb2: add PHY driver for marvell usb2 controller
2013-03-06 8:53 ` Felipe Balbi
@ 2013-03-06 9:02 ` Chao Xie
2013-03-06 9:26 ` Felipe Balbi
0 siblings, 1 reply; 34+ messages in thread
From: Chao Xie @ 2013-03-06 9:02 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Mar 6, 2013 at 4:53 PM, Felipe Balbi <balbi@ti.com> wrote:
> On Wed, Mar 06, 2013 at 04:24:58PM +0800, Chao Xie wrote:
>> On Wed, Mar 6, 2013 at 4:10 PM, Felipe Balbi <balbi@ti.com> wrote:
>> > Hi,
>> >
>> > On Wed, Mar 06, 2013 at 10:11:41AM +0800, Chao Xie wrote:
>> >> 3. For the revison register. It exists in some SOCes(pxa168), but for
>> >> some SOCes, the register dispears(pxa910, armada610). These SOCes are
>> >> developed by different desgin teams, and it need to be enhanced, but
>> >> for current products i have to use the device_id to detect the PHY
>> >> controller.
>> >
>> > fair enough, make sure to add a comment to the driver about this so
>> > grumpy maintainers stop complaining ;-)
>> >
>> >> 4. For the mutex and refcount. The "USB controller" includes two
>> >> blocks - the udc and ehci. In fact they will not be used at same time,
>> >> but for some SOCes it duplicates the ehci block. It means that the
>> >> SOCes have two or more ehci blocks. The added ehci blocks depend on
>> >> the PHY to be initialized, and they can be used at same time as the
>> >> "USB controller". That is the reason i add mutex and refcount for phy
>> >> driver.
>> >
>> > alright, let's add refcounting to the generic PHY layer then.
>> >
>> >> 5. For the clock name. Can you describe it in details? For clkdev, if
>> >> it want to find the clock, it still depends on the dev_name and
>> >> con_id.
>> >
>> > right, the idea of clkdev is that you associate a clock provider with
>> > its consumer by means of dev_name. Then you can fetch the clock from
>> > driver with any name you want. Which means if you do you clkdev properly
>> > you could:
>> >
>> > clk_get(dev, "clk1");
>> > clk_get(dev, "clk2");
>> > ...
>> >
>> > and so on.
>> >
>>
>> It is same as what i think.
>> The clock numbers and names are depent of SOCes, i do not want to fix
>
> no, they're not dependent on anything, not if you use clkdev correctly.
>
So
1. Does it mean that all SOCes clock driver should define same names
such as "clk1", "clk2"?
2. Does it mean that if driver failed at clk_get, the probing will not
stop because this SOC may define this clock?
>> it in ther driver. So i use pdata to pass the clknum and clk names
>> "clk1", "clk2" and so on.
>> in the driver, i use
>> devm_clk_get(&pdev->dev, pdata->clkname[i]);
>> Do you think it is acceptable?
>
> no
>
>> When device tree for clock framework are supported, the things get
>> easier, and it is my next step.
>
> not strong enough argument to accept passing clock names via
> platform_data.
>
> --
> balbi
^ permalink raw reply [flat|nested] 34+ messages in thread
* [V8 PATCH 01/16] usb: phy: mv_usb2: add PHY driver for marvell usb2 controller
2013-03-06 9:02 ` Chao Xie
@ 2013-03-06 9:26 ` Felipe Balbi
0 siblings, 0 replies; 34+ messages in thread
From: Felipe Balbi @ 2013-03-06 9:26 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Mar 06, 2013 at 05:02:28PM +0800, Chao Xie wrote:
> On Wed, Mar 6, 2013 at 4:53 PM, Felipe Balbi <balbi@ti.com> wrote:
> > On Wed, Mar 06, 2013 at 04:24:58PM +0800, Chao Xie wrote:
> >> On Wed, Mar 6, 2013 at 4:10 PM, Felipe Balbi <balbi@ti.com> wrote:
> >> > Hi,
> >> >
> >> > On Wed, Mar 06, 2013 at 10:11:41AM +0800, Chao Xie wrote:
> >> >> 3. For the revison register. It exists in some SOCes(pxa168), but for
> >> >> some SOCes, the register dispears(pxa910, armada610). These SOCes are
> >> >> developed by different desgin teams, and it need to be enhanced, but
> >> >> for current products i have to use the device_id to detect the PHY
> >> >> controller.
> >> >
> >> > fair enough, make sure to add a comment to the driver about this so
> >> > grumpy maintainers stop complaining ;-)
> >> >
> >> >> 4. For the mutex and refcount. The "USB controller" includes two
> >> >> blocks - the udc and ehci. In fact they will not be used at same time,
> >> >> but for some SOCes it duplicates the ehci block. It means that the
> >> >> SOCes have two or more ehci blocks. The added ehci blocks depend on
> >> >> the PHY to be initialized, and they can be used at same time as the
> >> >> "USB controller". That is the reason i add mutex and refcount for phy
> >> >> driver.
> >> >
> >> > alright, let's add refcounting to the generic PHY layer then.
> >> >
> >> >> 5. For the clock name. Can you describe it in details? For clkdev, if
> >> >> it want to find the clock, it still depends on the dev_name and
> >> >> con_id.
> >> >
> >> > right, the idea of clkdev is that you associate a clock provider with
> >> > its consumer by means of dev_name. Then you can fetch the clock from
> >> > driver with any name you want. Which means if you do you clkdev properly
> >> > you could:
> >> >
> >> > clk_get(dev, "clk1");
> >> > clk_get(dev, "clk2");
> >> > ...
> >> >
> >> > and so on.
> >> >
> >>
> >> It is same as what i think.
> >> The clock numbers and names are depent of SOCes, i do not want to fix
> >
> > no, they're not dependent on anything, not if you use clkdev correctly.
> >
>
> So
> 1. Does it mean that all SOCes clock driver should define same names
> such as "clk1", "clk2"?
clk1, clk2, clk3 were just examples. Ideally you would have more
meaningfull names like "fck" for functional clock.
> 2. Does it mean that if driver failed at clk_get, the probing will not
> stop because this SOC may define this clock?
that depends on how you write your driver. If platform A has 3 clocks
and platform B has only 2 you have two options:
a) make the third clock optional by not bailing out if clk_get() fails
b) define a dummy no-op clock node to satisfy the third clock.
--
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130306/0317a612/attachment-0001.sig>
^ permalink raw reply [flat|nested] 34+ messages in thread
* [V8 PATCH 01/16] usb: phy: mv_usb2: add PHY driver for marvell usb2 controller
2013-03-06 8:24 ` Chao Xie
2013-03-06 8:53 ` Felipe Balbi
@ 2013-03-06 16:48 ` Russell King - ARM Linux
2013-03-07 0:57 ` Chao Xie
1 sibling, 1 reply; 34+ messages in thread
From: Russell King - ARM Linux @ 2013-03-06 16:48 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Mar 06, 2013 at 04:24:58PM +0800, Chao Xie wrote:
> The clock numbers and names are depent of SOCes,
No they aren't. The clock names used to describe them in your documentation
may vary, but their _purpose_ for the sake of the device will be fixed -
and you should name them appropriately from the _device_ point of view.
Not the SoC point of view. That way leads to total madness. We've
proven this over the years that we've had the clk API and people have
come up with trash implementations that do that crap. After many
years of struggling, they've seen the light and fixed their shite up
to work the way I originally intended, and... instantly benefited from
it.
^ permalink raw reply [flat|nested] 34+ messages in thread
* [V8 PATCH 01/16] usb: phy: mv_usb2: add PHY driver for marvell usb2 controller
2013-03-06 16:48 ` Russell King - ARM Linux
@ 2013-03-07 0:57 ` Chao Xie
0 siblings, 0 replies; 34+ messages in thread
From: Chao Xie @ 2013-03-07 0:57 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Mar 7, 2013 at 12:48 AM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Wed, Mar 06, 2013 at 04:24:58PM +0800, Chao Xie wrote:
>> The clock numbers and names are depent of SOCes,
>
> No they aren't. The clock names used to describe them in your documentation
> may vary, but their _purpose_ for the sake of the device will be fixed -
> and you should name them appropriately from the _device_ point of view.
>
> Not the SoC point of view. That way leads to total madness. We've
> proven this over the years that we've had the clk API and people have
> come up with trash implementations that do that crap. After many
> years of struggling, they've seen the light and fixed their shite up
> to work the way I originally intended, and... instantly benefited from
> it.
Yes, you are right, and you explained it very clearly.. I understand
what you said.
Our SOC document is lack of this kind of inoformation. I will talk
with the desgin engneer to make clear it, and make a new version of
patches.
Thanks for your review and comments.
^ permalink raw reply [flat|nested] 34+ messages in thread
* [V8 PATCH 01/16] usb: phy: mv_usb2: add PHY driver for marvell usb2 controller
2013-03-05 2:03 ` Chao Xie
2013-03-05 11:04 ` Felipe Balbi
@ 2013-03-06 16:45 ` Russell King - ARM Linux
1 sibling, 0 replies; 34+ messages in thread
From: Russell King - ARM Linux @ 2013-03-06 16:45 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Mar 05, 2013 at 10:03:01AM +0800, Chao Xie wrote:
> On Mon, Mar 4, 2013 at 10:21 PM, Felipe Balbi <balbi@ti.com> wrote:
> > On Wed, Feb 20, 2013 at 11:07:11PM -0500, Chao Xie wrote:
> >> + for (i = 0; i < mv_phy->clks_num; i++) {
> >> + mv_phy->clks[i] = devm_clk_get(&pdev->dev,
> >> + pdata->clkname[i]);
> >
> > *NEVER* pass clock names via platform_data, this is utterly wrong.
> >
> without device tree support, the only way we can get the clock is the pdata.
> the use phy have mutiple clocks.
> So what do you suggest to handle it?
Then you don't understand the clk API at all.
Read the documentation in include/linux/clk.h for clk_get().
The first parameter is the device which you're interested in getting the
clock for.
The second parameter defines the INPUT as a string to THAT DEVICE. It
is specific to the device. It is NOT the system name of the clock.
So, if you have a function clock and an interface clock to a device,
then use a name like "fck" for the function clock and "ick" for the
interface clock.
Do _NOT_ make the mistake of using "global" clock names. People have done
that many times in the past and got into horrid sticky problems - and
ended up with _far_ more code than is really necessary if you do things
the right way.
^ permalink raw reply [flat|nested] 34+ messages in thread
* [V8 PATCH 02/16] usb: gadget: mv_udc: use PHY driver for udc
2013-02-21 4:07 [V8 PATCH 00/16] mv-usb phy enhancement patches Chao Xie
2013-02-21 4:07 ` [V8 PATCH 01/16] usb: phy: mv_usb2: add PHY driver for marvell usb2 controller Chao Xie
@ 2013-02-21 4:07 ` Chao Xie
2013-03-04 14:24 ` Felipe Balbi
2013-02-21 4:07 ` [V8 PATCH 03/16] usb: ehci: ehci-mv: use PHY driver for ehci Chao Xie
` (14 subsequent siblings)
16 siblings, 1 reply; 34+ messages in thread
From: Chao Xie @ 2013-02-21 4:07 UTC (permalink / raw)
To: linux-arm-kernel
Originaly, udc driver will call the callbacks in platform data
for PHY initialization and shut down.
With PHY driver, it will call the APIs provided by PHY driver
for PHY initialization and shut down. It removes the callbacks
in platform data, and at same time it removes one block in the
way of enabling device tree for udc driver.
Signed-off-by: Chao Xie <chao.xie@marvell.com>
---
drivers/usb/gadget/mv_udc.h | 2 +-
drivers/usb/gadget/mv_udc_core.c | 45 ++++++++++++++-----------------------
2 files changed, 18 insertions(+), 29 deletions(-)
diff --git a/drivers/usb/gadget/mv_udc.h b/drivers/usb/gadget/mv_udc.h
index 9073436..f339df4 100644
--- a/drivers/usb/gadget/mv_udc.h
+++ b/drivers/usb/gadget/mv_udc.h
@@ -180,7 +180,6 @@ struct mv_udc {
struct mv_cap_regs __iomem *cap_regs;
struct mv_op_regs __iomem *op_regs;
- void __iomem *phy_regs;
unsigned int max_eps;
struct mv_dqh *ep_dqh;
size_t ep_dqh_size;
@@ -217,6 +216,7 @@ struct mv_udc {
struct work_struct vbus_work;
struct workqueue_struct *qwork;
+ struct usb_phy *phy;
struct usb_phy *transceiver;
struct mv_usb_platform_data *pdata;
diff --git a/drivers/usb/gadget/mv_udc_core.c b/drivers/usb/gadget/mv_udc_core.c
index c8cf959..4876d2f 100644
--- a/drivers/usb/gadget/mv_udc_core.c
+++ b/drivers/usb/gadget/mv_udc_core.c
@@ -35,6 +35,7 @@
#include <linux/platform_device.h>
#include <linux/clk.h>
#include <linux/platform_data/mv_usb.h>
+#include <linux/usb/mv_usb2.h>
#include <asm/unaligned.h>
#include "mv_udc.h"
@@ -1121,15 +1122,14 @@ static int mv_udc_enable_internal(struct mv_udc *udc)
dev_dbg(&udc->dev->dev, "enable udc\n");
udc_clock_enable(udc);
- if (udc->pdata->phy_init) {
- retval = udc->pdata->phy_init(udc->phy_regs);
- if (retval) {
- dev_err(&udc->dev->dev,
- "init phy error %d\n", retval);
- udc_clock_disable(udc);
- return retval;
- }
+ retval = usb_phy_init(udc->phy);
+ if (retval) {
+ dev_err(&udc->dev->dev,
+ "init phy error %d\n", retval);
+ udc_clock_disable(udc);
+ return retval;
}
+
udc->active = 1;
return 0;
@@ -1147,8 +1147,7 @@ static void mv_udc_disable_internal(struct mv_udc *udc)
{
if (udc->active) {
dev_dbg(&udc->dev->dev, "disable udc\n");
- if (udc->pdata->phy_deinit)
- udc->pdata->phy_deinit(udc->phy_regs);
+ usb_phy_shutdown(udc->phy);
udc_clock_disable(udc);
udc->active = 0;
}
@@ -2175,12 +2174,10 @@ static int mv_udc_probe(struct platform_device *pdev)
#ifdef CONFIG_USB_OTG_UTILS
if (pdata->mode == MV_USB_MODE_OTG) {
- udc->transceiver = devm_usb_get_phy(&pdev->dev,
- USB_PHY_TYPE_USB2);
- if (IS_ERR_OR_NULL(udc->transceiver)) {
- udc->transceiver = NULL;
- return -ENODEV;
- }
+ udc->transceiver = devm_usb_get_phy_dev(&pdev->dev,
+ MV_USB2_OTG_PHY_INDEX);
+ if (IS_ERR_OR_NULL(udc->transceiver))
+ return PTR_ERR(udc->transceiver);
}
#endif
@@ -2194,7 +2191,7 @@ static int mv_udc_probe(struct platform_device *pdev)
}
}
- r = platform_get_resource_byname(udc->dev, IORESOURCE_MEM, "capregs");
+ r = platform_get_resource(udc->dev, IORESOURCE_MEM, 0);
if (r == NULL) {
dev_err(&pdev->dev, "no I/O memory resource defined\n");
return -ENODEV;
@@ -2207,17 +2204,9 @@ static int mv_udc_probe(struct platform_device *pdev)
return -EBUSY;
}
- r = platform_get_resource_byname(udc->dev, IORESOURCE_MEM, "phyregs");
- if (r == NULL) {
- dev_err(&pdev->dev, "no phy I/O memory resource defined\n");
- return -ENODEV;
- }
-
- udc->phy_regs = ioremap(r->start, resource_size(r));
- if (udc->phy_regs == NULL) {
- dev_err(&pdev->dev, "failed to map phy I/O memory\n");
- return -EBUSY;
- }
+ udc->phy = devm_usb_get_phy_dev(&pdev->dev, MV_USB2_PHY_INDEX);
+ if (IS_ERR_OR_NULL(udc->phy))
+ return PTR_ERR(udc->phy);
/* we will acces controller register, so enable the clk */
retval = mv_udc_enable_internal(udc);
--
1.7.4.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* [V8 PATCH 02/16] usb: gadget: mv_udc: use PHY driver for udc
2013-02-21 4:07 ` [V8 PATCH 02/16] usb: gadget: mv_udc: use PHY driver for udc Chao Xie
@ 2013-03-04 14:24 ` Felipe Balbi
2013-03-05 2:11 ` Chao Xie
0 siblings, 1 reply; 34+ messages in thread
From: Felipe Balbi @ 2013-03-04 14:24 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Feb 20, 2013 at 11:07:12PM -0500, Chao Xie wrote:
> Originaly, udc driver will call the callbacks in platform data
> for PHY initialization and shut down.
> With PHY driver, it will call the APIs provided by PHY driver
> for PHY initialization and shut down. It removes the callbacks
> in platform data, and at same time it removes one block in the
> way of enabling device tree for udc driver.
>
> Signed-off-by: Chao Xie <chao.xie@marvell.com>
> ---
> drivers/usb/gadget/mv_udc.h | 2 +-
> drivers/usb/gadget/mv_udc_core.c | 45 ++++++++++++++-----------------------
> 2 files changed, 18 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/usb/gadget/mv_udc.h b/drivers/usb/gadget/mv_udc.h
> index 9073436..f339df4 100644
> --- a/drivers/usb/gadget/mv_udc.h
> +++ b/drivers/usb/gadget/mv_udc.h
> @@ -180,7 +180,6 @@ struct mv_udc {
>
> struct mv_cap_regs __iomem *cap_regs;
> struct mv_op_regs __iomem *op_regs;
> - void __iomem *phy_regs;
> unsigned int max_eps;
> struct mv_dqh *ep_dqh;
> size_t ep_dqh_size;
> @@ -217,6 +216,7 @@ struct mv_udc {
> struct work_struct vbus_work;
> struct workqueue_struct *qwork;
>
> + struct usb_phy *phy;
> struct usb_phy *transceiver;
>
> struct mv_usb_platform_data *pdata;
> diff --git a/drivers/usb/gadget/mv_udc_core.c b/drivers/usb/gadget/mv_udc_core.c
> index c8cf959..4876d2f 100644
> --- a/drivers/usb/gadget/mv_udc_core.c
> +++ b/drivers/usb/gadget/mv_udc_core.c
> @@ -35,6 +35,7 @@
> #include <linux/platform_device.h>
> #include <linux/clk.h>
> #include <linux/platform_data/mv_usb.h>
> +#include <linux/usb/mv_usb2.h>
> #include <asm/unaligned.h>
>
> #include "mv_udc.h"
> @@ -1121,15 +1122,14 @@ static int mv_udc_enable_internal(struct mv_udc *udc)
>
> dev_dbg(&udc->dev->dev, "enable udc\n");
> udc_clock_enable(udc);
> - if (udc->pdata->phy_init) {
> - retval = udc->pdata->phy_init(udc->phy_regs);
> - if (retval) {
> - dev_err(&udc->dev->dev,
> - "init phy error %d\n", retval);
> - udc_clock_disable(udc);
> - return retval;
> - }
dude, you really don't test your patches, do you ? Your previous patch
removed ->phy_init(), ->phy_deinit() and ->private_init() from your
platform_data.
Are you seriously telling me you didn't compile test your patches ?
Rewrite your series again and make sure that each and every patch
compiles and works on its own. You shouldn't cause any regressions or
build breaks or build warnings while converting this driver.
I won't review the rest of this series and I'm purging it from my TODO
list.
cheers
--
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130304/be03f1ea/attachment.sig>
^ permalink raw reply [flat|nested] 34+ messages in thread* [V8 PATCH 02/16] usb: gadget: mv_udc: use PHY driver for udc
2013-03-04 14:24 ` Felipe Balbi
@ 2013-03-05 2:11 ` Chao Xie
0 siblings, 0 replies; 34+ messages in thread
From: Chao Xie @ 2013-03-05 2:11 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Mar 4, 2013 at 10:24 PM, Felipe Balbi <balbi@ti.com> wrote:
> On Wed, Feb 20, 2013 at 11:07:12PM -0500, Chao Xie wrote:
>> Originaly, udc driver will call the callbacks in platform data
>> for PHY initialization and shut down.
>> With PHY driver, it will call the APIs provided by PHY driver
>> for PHY initialization and shut down. It removes the callbacks
>> in platform data, and at same time it removes one block in the
>> way of enabling device tree for udc driver.
>>
>> Signed-off-by: Chao Xie <chao.xie@marvell.com>
>> ---
>> drivers/usb/gadget/mv_udc.h | 2 +-
>> drivers/usb/gadget/mv_udc_core.c | 45 ++++++++++++++-----------------------
>> 2 files changed, 18 insertions(+), 29 deletions(-)
>>
>> diff --git a/drivers/usb/gadget/mv_udc.h b/drivers/usb/gadget/mv_udc.h
>> index 9073436..f339df4 100644
>> --- a/drivers/usb/gadget/mv_udc.h
>> +++ b/drivers/usb/gadget/mv_udc.h
>> @@ -180,7 +180,6 @@ struct mv_udc {
>>
>> struct mv_cap_regs __iomem *cap_regs;
>> struct mv_op_regs __iomem *op_regs;
>> - void __iomem *phy_regs;
>> unsigned int max_eps;
>> struct mv_dqh *ep_dqh;
>> size_t ep_dqh_size;
>> @@ -217,6 +216,7 @@ struct mv_udc {
>> struct work_struct vbus_work;
>> struct workqueue_struct *qwork;
>>
>> + struct usb_phy *phy;
>> struct usb_phy *transceiver;
>>
>> struct mv_usb_platform_data *pdata;
>> diff --git a/drivers/usb/gadget/mv_udc_core.c b/drivers/usb/gadget/mv_udc_core.c
>> index c8cf959..4876d2f 100644
>> --- a/drivers/usb/gadget/mv_udc_core.c
>> +++ b/drivers/usb/gadget/mv_udc_core.c
>> @@ -35,6 +35,7 @@
>> #include <linux/platform_device.h>
>> #include <linux/clk.h>
>> #include <linux/platform_data/mv_usb.h>
>> +#include <linux/usb/mv_usb2.h>
>> #include <asm/unaligned.h>
>>
>> #include "mv_udc.h"
>> @@ -1121,15 +1122,14 @@ static int mv_udc_enable_internal(struct mv_udc *udc)
>>
>> dev_dbg(&udc->dev->dev, "enable udc\n");
>> udc_clock_enable(udc);
>> - if (udc->pdata->phy_init) {
>> - retval = udc->pdata->phy_init(udc->phy_regs);
>> - if (retval) {
>> - dev_err(&udc->dev->dev,
>> - "init phy error %d\n", retval);
>> - udc_clock_disable(udc);
>> - return retval;
>> - }
>
> dude, you really don't test your patches, do you ? Your previous patch
> removed ->phy_init(), ->phy_deinit() and ->private_init() from your
> platform_data.
>
> Are you seriously telling me you didn't compile test your patches ?
>
> Rewrite your series again and make sure that each and every patch
> compiles and works on its own. You shouldn't cause any regressions or
> build breaks or build warnings while converting this driver.
>
> I won't review the rest of this series and I'm purging it from my TODO
> list.
>
Thanks for your review.
The patches are seperated into two parts. for each part it includes
some patches.
I have compiled and tested each part for our configurartion and x86
configuration, but
not for every patch. i will do it.
> cheers
>
> --
> balbi
^ permalink raw reply [flat|nested] 34+ messages in thread
* [V8 PATCH 03/16] usb: ehci: ehci-mv: use PHY driver for ehci
2013-02-21 4:07 [V8 PATCH 00/16] mv-usb phy enhancement patches Chao Xie
2013-02-21 4:07 ` [V8 PATCH 01/16] usb: phy: mv_usb2: add PHY driver for marvell usb2 controller Chao Xie
2013-02-21 4:07 ` [V8 PATCH 02/16] usb: gadget: mv_udc: use PHY driver for udc Chao Xie
@ 2013-02-21 4:07 ` Chao Xie
2013-02-21 4:07 ` [V8 PATCH 04/16] usb: otg: mv_otg: use PHY driver for otg Chao Xie
` (13 subsequent siblings)
16 siblings, 0 replies; 34+ messages in thread
From: Chao Xie @ 2013-02-21 4:07 UTC (permalink / raw)
To: linux-arm-kernel
Originaly, ehci driver will call the callbacks in platform data
for PHY initialization and shut down.
With PHY driver, it will call the APIs provided by PHY driver
for PHY initialization and shut down. It removes the callbacks
in platform data, and at same time it removes one block in the
way of enabling device tree for ehci driver.
Signed-off-by: Chao Xie <chao.xie@marvell.com>
Acked-by: Alan Stern <stern@rowland.harvard.edu>
---
drivers/usb/host/ehci-mv.c | 48 ++++++++++++++-----------------------------
1 files changed, 16 insertions(+), 32 deletions(-)
diff --git a/drivers/usb/host/ehci-mv.c b/drivers/usb/host/ehci-mv.c
index 3065809..3e89bd4 100644
--- a/drivers/usb/host/ehci-mv.c
+++ b/drivers/usb/host/ehci-mv.c
@@ -15,17 +15,18 @@
#include <linux/clk.h>
#include <linux/err.h>
#include <linux/usb/otg.h>
+#include <linux/usb/mv_usb2.h>
#include <linux/platform_data/mv_usb.h>
#define CAPLENGTH_MASK (0xff)
struct ehci_hcd_mv {
struct usb_hcd *hcd;
+ struct usb_phy *phy;
/* Which mode does this ehci running OTG/Host ? */
int mode;
- void __iomem *phy_regs;
void __iomem *cap_regs;
void __iomem *op_regs;
@@ -56,22 +57,15 @@ static void ehci_clock_disable(struct ehci_hcd_mv *ehci_mv)
static int mv_ehci_enable(struct ehci_hcd_mv *ehci_mv)
{
- int retval;
-
ehci_clock_enable(ehci_mv);
- if (ehci_mv->pdata->phy_init) {
- retval = ehci_mv->pdata->phy_init(ehci_mv->phy_regs);
- if (retval)
- return retval;
- }
- return 0;
+ return usb_phy_init(ehci_mv->phy);
}
static void mv_ehci_disable(struct ehci_hcd_mv *ehci_mv)
{
- if (ehci_mv->pdata->phy_deinit)
- ehci_mv->pdata->phy_deinit(ehci_mv->phy_regs);
+ usb_phy_shutdown(ehci_mv->phy);
+
ehci_clock_disable(ehci_mv);
}
@@ -184,22 +178,7 @@ static int mv_ehci_probe(struct platform_device *pdev)
}
}
- r = platform_get_resource_byname(pdev, IORESOURCE_MEM, "phyregs");
- if (r == NULL) {
- dev_err(&pdev->dev, "no phy I/O memory resource defined\n");
- retval = -ENODEV;
- goto err_clear_drvdata;
- }
-
- ehci_mv->phy_regs = devm_ioremap(&pdev->dev, r->start,
- resource_size(r));
- if (ehci_mv->phy_regs == 0) {
- dev_err(&pdev->dev, "failed to map phy I/O memory\n");
- retval = -EFAULT;
- goto err_clear_drvdata;
- }
-
- r = platform_get_resource_byname(pdev, IORESOURCE_MEM, "capregs");
+ r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!r) {
dev_err(&pdev->dev, "no I/O memory resource defined\n");
retval = -ENODEV;
@@ -214,6 +193,13 @@ static int mv_ehci_probe(struct platform_device *pdev)
goto err_clear_drvdata;
}
+ ehci_mv->phy = devm_usb_get_phy_dev(&pdev->dev, MV_USB2_PHY_INDEX);
+ if (IS_ERR_OR_NULL(ehci_mv->phy)) {
+ dev_err(&pdev->dev, "failed to get the outer phy\n");
+ retval = PTR_ERR(ehci_mv->phy);
+ goto err_clear_drvdata;
+ }
+
retval = mv_ehci_enable(ehci_mv);
if (retval) {
dev_err(&pdev->dev, "init phy error %d\n", retval);
@@ -241,11 +227,12 @@ static int mv_ehci_probe(struct platform_device *pdev)
ehci_mv->mode = pdata->mode;
if (ehci_mv->mode == MV_USB_MODE_OTG) {
#ifdef CONFIG_USB_OTG_UTILS
- ehci_mv->otg = devm_usb_get_phy(&pdev->dev, USB_PHY_TYPE_USB2);
+ ehci_mv->otg = devm_usb_get_phy_dev(&pdev->dev,
+ MV_USB2_OTG_PHY_INDEX);
if (IS_ERR_OR_NULL(ehci_mv->otg)) {
dev_err(&pdev->dev,
"unable to find transceiver\n");
- retval = -ENODEV;
+ retval = PTR_ERR(ehci_mv->otg);
goto err_disable_clk;
}
@@ -275,9 +262,6 @@ static int mv_ehci_probe(struct platform_device *pdev)
}
}
- if (pdata->private_init)
- pdata->private_init(ehci_mv->op_regs, ehci_mv->phy_regs);
-
dev_info(&pdev->dev,
"successful find EHCI device with regs 0x%p irq %d"
" working in %s mode\n", hcd->regs, hcd->irq,
--
1.7.4.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* [V8 PATCH 04/16] usb: otg: mv_otg: use PHY driver for otg
2013-02-21 4:07 [V8 PATCH 00/16] mv-usb phy enhancement patches Chao Xie
` (2 preceding siblings ...)
2013-02-21 4:07 ` [V8 PATCH 03/16] usb: ehci: ehci-mv: use PHY driver for ehci Chao Xie
@ 2013-02-21 4:07 ` Chao Xie
2013-02-21 4:07 ` [V8 PATCH 05/16] arm: mmp2: change the defintion of usb devices Chao Xie
` (12 subsequent siblings)
16 siblings, 0 replies; 34+ messages in thread
From: Chao Xie @ 2013-02-21 4:07 UTC (permalink / raw)
To: linux-arm-kernel
Originaly, otg driver will call the callbacks in platform data
for PHY initialization and shut down.
With PHY driver, it will call the APIs provided by PHY driver
for PHY initialization and shut down. It removes the callbacks
in platform data, and at same time it removes one block in the
way of enabling device tree for otg driver.
Signed-off-by: Chao Xie <chao.xie@marvell.com>
---
drivers/usb/otg/mv_otg.c | 50 ++++++++++++++++++---------------------------
drivers/usb/otg/mv_otg.h | 2 +-
2 files changed, 21 insertions(+), 31 deletions(-)
diff --git a/drivers/usb/otg/mv_otg.c b/drivers/usb/otg/mv_otg.c
index b6a9be3..7282b0d 100644
--- a/drivers/usb/otg/mv_otg.c
+++ b/drivers/usb/otg/mv_otg.c
@@ -25,6 +25,7 @@
#include <linux/usb/otg.h>
#include <linux/usb/gadget.h>
#include <linux/usb/hcd.h>
+#include <linux/usb/mv_usb2.h>
#include <linux/platform_data/mv_usb.h>
#include "mv_otg.h"
@@ -261,14 +262,12 @@ static int mv_otg_enable_internal(struct mv_otg *mvotg)
dev_dbg(&mvotg->pdev->dev, "otg enabled\n");
otg_clock_enable(mvotg);
- if (mvotg->pdata->phy_init) {
- retval = mvotg->pdata->phy_init(mvotg->phy_regs);
- if (retval) {
- dev_err(&mvotg->pdev->dev,
- "init phy error %d\n", retval);
- otg_clock_disable(mvotg);
- return retval;
- }
+ retval = usb_phy_init(mvotg->outer_phy);
+ if (retval) {
+ dev_err(&mvotg->pdev->dev,
+ "init phy error %d\n", retval);
+ otg_clock_disable(mvotg);
+ return retval;
}
mvotg->active = 1;
@@ -288,8 +287,7 @@ static void mv_otg_disable_internal(struct mv_otg *mvotg)
{
if (mvotg->active) {
dev_dbg(&mvotg->pdev->dev, "otg disabled\n");
- if (mvotg->pdata->phy_deinit)
- mvotg->pdata->phy_deinit(mvotg->phy_regs);
+ usb_phy_shutdown(mvotg->outer_phy);
otg_clock_disable(mvotg);
mvotg->active = 0;
}
@@ -729,6 +727,7 @@ static int mv_otg_probe(struct platform_device *pdev)
/* OTG common part */
mvotg->pdev = pdev;
mvotg->phy.dev = &pdev->dev;
+ mvotg->phy.type = USB_PHY_TYPE_USB2;
mvotg->phy.otg = otg;
mvotg->phy.label = driver_name;
mvotg->phy.state = OTG_STATE_UNDEFINED;
@@ -741,23 +740,8 @@ static int mv_otg_probe(struct platform_device *pdev)
for (i = 0; i < OTG_TIMER_NUM; i++)
init_timer(&mvotg->otg_ctrl.timer[i]);
- r = platform_get_resource_byname(mvotg->pdev,
- IORESOURCE_MEM, "phyregs");
- if (r == NULL) {
- dev_err(&pdev->dev, "no phy I/O memory resource defined\n");
- retval = -ENODEV;
- goto err_destroy_workqueue;
- }
-
- mvotg->phy_regs = devm_ioremap(&pdev->dev, r->start, resource_size(r));
- if (mvotg->phy_regs == NULL) {
- dev_err(&pdev->dev, "failed to map phy I/O memory\n");
- retval = -EFAULT;
- goto err_destroy_workqueue;
- }
-
- r = platform_get_resource_byname(mvotg->pdev,
- IORESOURCE_MEM, "capregs");
+ r = platform_get_resource(mvotg->pdev,
+ IORESOURCE_MEM, 0);
if (r == NULL) {
dev_err(&pdev->dev, "no I/O memory resource defined\n");
retval = -ENODEV;
@@ -770,6 +754,12 @@ static int mv_otg_probe(struct platform_device *pdev)
retval = -EFAULT;
goto err_destroy_workqueue;
}
+ mvotg->outer_phy = devm_usb_get_phy_dev(&pdev->dev, MV_USB2_PHY_INDEX);
+ if (IS_ERR_OR_NULL(mvotg->outer_phy)) {
+ dev_err(&pdev->dev, "can not find outer phy\n");
+ retval = PTR_ERR(mvotg->outer_phy);
+ goto err_destroy_workqueue;
+ }
/* we will acces controller register, so enable the udc controller */
retval = mv_otg_enable_internal(mvotg);
@@ -830,7 +820,7 @@ static int mv_otg_probe(struct platform_device *pdev)
goto err_disable_clk;
}
- retval = usb_add_phy(&mvotg->phy, USB_PHY_TYPE_USB2);
+ retval = usb_add_phy_dev(&mvotg->phy);
if (retval < 0) {
dev_err(&pdev->dev, "can't register transceiver, %d\n",
retval);
@@ -841,7 +831,7 @@ static int mv_otg_probe(struct platform_device *pdev)
if (retval < 0) {
dev_dbg(&pdev->dev,
"Can't register sysfs attr group: %d\n", retval);
- goto err_remove_phy;
+ goto err_remove_otg_phy;
}
spin_lock_init(&mvotg->wq_lock);
@@ -856,7 +846,7 @@ static int mv_otg_probe(struct platform_device *pdev)
return 0;
-err_remove_phy:
+err_remove_otg_phy:
usb_remove_phy(&mvotg->phy);
err_disable_clk:
mv_otg_disable_internal(mvotg);
diff --git a/drivers/usb/otg/mv_otg.h b/drivers/usb/otg/mv_otg.h
index 8a9e351..65e7fd1 100644
--- a/drivers/usb/otg/mv_otg.h
+++ b/drivers/usb/otg/mv_otg.h
@@ -137,10 +137,10 @@ struct mv_otg_regs {
struct mv_otg {
struct usb_phy phy;
+ struct usb_phy *outer_phy;
struct mv_otg_ctrl otg_ctrl;
/* base address */
- void __iomem *phy_regs;
void __iomem *cap_regs;
struct mv_otg_regs __iomem *op_regs;
--
1.7.4.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* [V8 PATCH 05/16] arm: mmp2: change the defintion of usb devices
2013-02-21 4:07 [V8 PATCH 00/16] mv-usb phy enhancement patches Chao Xie
` (3 preceding siblings ...)
2013-02-21 4:07 ` [V8 PATCH 04/16] usb: otg: mv_otg: use PHY driver for otg Chao Xie
@ 2013-02-21 4:07 ` Chao Xie
2013-02-21 4:07 ` [V8 PATCH 06/16] arm: pxa910: " Chao Xie
` (11 subsequent siblings)
16 siblings, 0 replies; 34+ messages in thread
From: Chao Xie @ 2013-02-21 4:07 UTC (permalink / raw)
To: linux-arm-kernel
add the udc/otg/ehci devices for mmp2
Signed-off-by: Chao Xie <chao.xie@marvell.com>
---
arch/arm/mach-mmp/include/mach/mmp2.h | 4 ++++
arch/arm/mach-mmp/mmp2.c | 4 ++++
2 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-mmp/include/mach/mmp2.h b/arch/arm/mach-mmp/include/mach/mmp2.h
index c4ca4d1..58e96b0 100644
--- a/arch/arm/mach-mmp/include/mach/mmp2.h
+++ b/arch/arm/mach-mmp/include/mach/mmp2.h
@@ -31,6 +31,10 @@ extern struct pxa_device_desc mmp2_device_sdh2;
extern struct pxa_device_desc mmp2_device_sdh3;
extern struct pxa_device_desc mmp2_device_asram;
extern struct pxa_device_desc mmp2_device_isram;
+extern struct pxa_device_desc mmp2_device_u2o;
+extern struct pxa_device_desc mmp2_device_u2ootg;
+extern struct pxa_device_desc mmp2_device_u2oehci;
+extern struct pxa_device_desc mmp2_device_u2ophy;
extern struct platform_device mmp2_device_gpio;
diff --git a/arch/arm/mach-mmp/mmp2.c b/arch/arm/mach-mmp/mmp2.c
index 3a3768c..73edbfc 100644
--- a/arch/arm/mach-mmp/mmp2.c
+++ b/arch/arm/mach-mmp/mmp2.c
@@ -153,6 +153,10 @@ MMP2_DEVICE(sdh3, "sdhci-pxav3", 3, MMC4, 0xd4281800, 0x120);
MMP2_DEVICE(asram, "asram", -1, NONE, 0xe0000000, 0x4000);
/* 0xd1000000 ~ 0xd101ffff is reserved for secure processor */
MMP2_DEVICE(isram, "isram", -1, NONE, 0xd1020000, 0x18000);
+MMP2_DEVICE(u2ophy, "mmp2-usb-phy", -1, NONE, 0xd4207000, 0x1ff);
+MMP2_DEVICE(u2o, "mv-udc", -1, USB_OTG, 0xd4208100, 0x1ff);
+MMP2_DEVICE(u2ootg, "mv-otg", -1, USB_OTG, 0xd4208100, 0x1ff);
+MMP2_DEVICE(u2oehci, "mv-ehci", -1, USB_OTG, 0xd4208100, 0x1ff);
struct resource mmp2_resource_gpio[] = {
{
--
1.7.4.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* [V8 PATCH 06/16] arm: pxa910: change the defintion of usb devices
2013-02-21 4:07 [V8 PATCH 00/16] mv-usb phy enhancement patches Chao Xie
` (4 preceding siblings ...)
2013-02-21 4:07 ` [V8 PATCH 05/16] arm: mmp2: change the defintion of usb devices Chao Xie
@ 2013-02-21 4:07 ` Chao Xie
2013-02-21 4:07 ` [V8 PATCH 07/16] arm: brownstone: add usb support for the board Chao Xie
` (10 subsequent siblings)
16 siblings, 0 replies; 34+ messages in thread
From: Chao Xie @ 2013-02-21 4:07 UTC (permalink / raw)
To: linux-arm-kernel
because phy is seperated from the usb controller driver,
we can use the common pxa_device_desc for device register.
Signed-off-by: Chao Xie <chao.xie@marvell.com>
---
arch/arm/mach-mmp/include/mach/pxa910.h | 7 ++++---
arch/arm/mach-mmp/pxa910.c | 4 ++++
2 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/arch/arm/mach-mmp/include/mach/pxa910.h b/arch/arm/mach-mmp/include/mach/pxa910.h
index 3b58a3b..26ea4fe 100644
--- a/arch/arm/mach-mmp/include/mach/pxa910.h
+++ b/arch/arm/mach-mmp/include/mach/pxa910.h
@@ -20,9 +20,10 @@ extern struct pxa_device_desc pxa910_device_pwm2;
extern struct pxa_device_desc pxa910_device_pwm3;
extern struct pxa_device_desc pxa910_device_pwm4;
extern struct pxa_device_desc pxa910_device_nand;
-extern struct platform_device pxa168_device_u2o;
-extern struct platform_device pxa168_device_u2ootg;
-extern struct platform_device pxa168_device_u2oehci;
+extern struct pxa_device_desc pxa910_device_u2o;
+extern struct pxa_device_desc pxa910_device_u2ootg;
+extern struct pxa_device_desc pxa910_device_u2oehci;
+extern struct pxa_device_desc pxa910_device_u2ophy;
extern struct platform_device pxa910_device_gpio;
extern struct platform_device pxa910_device_rtc;
diff --git a/arch/arm/mach-mmp/pxa910.c b/arch/arm/mach-mmp/pxa910.c
index 8b1e16f..65174f7 100644
--- a/arch/arm/mach-mmp/pxa910.c
+++ b/arch/arm/mach-mmp/pxa910.c
@@ -138,6 +138,10 @@ PXA910_DEVICE(pwm2, "pxa910-pwm", 1, NONE, 0xd401a400, 0x10);
PXA910_DEVICE(pwm3, "pxa910-pwm", 2, NONE, 0xd401a800, 0x10);
PXA910_DEVICE(pwm4, "pxa910-pwm", 3, NONE, 0xd401ac00, 0x10);
PXA910_DEVICE(nand, "pxa3xx-nand", -1, NAND, 0xd4283000, 0x80, 97, 99);
+PXA910_DEVICE(u2ophy, "pxa910-usb-phy", -1, NONE, 0xd4207000, 0x1ff);
+PXA910_DEVICE(u2o, "mv-udc", -1, USB1, 0xd4208100, 0x1ff);
+PXA910_DEVICE(u2ootg, "mv-otg", -1, USB1, 0xd4208100, 0x1ff);
+PXA910_DEVICE(u2oehci, "mv-ehci", -1, USB1, 0xd4208100, 0x1ff);
struct resource pxa910_resource_gpio[] = {
{
--
1.7.4.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* [V8 PATCH 07/16] arm: brownstone: add usb support for the board
2013-02-21 4:07 [V8 PATCH 00/16] mv-usb phy enhancement patches Chao Xie
` (5 preceding siblings ...)
2013-02-21 4:07 ` [V8 PATCH 06/16] arm: pxa910: " Chao Xie
@ 2013-02-21 4:07 ` Chao Xie
2013-02-21 4:07 ` [V8 PATCH 08/16] arm: ttc_dkb: add usb support Chao Xie
` (9 subsequent siblings)
16 siblings, 0 replies; 34+ messages in thread
From: Chao Xie @ 2013-02-21 4:07 UTC (permalink / raw)
To: linux-arm-kernel
for brownstone board, add the udc/otg/ehci support
Signed-off-by: Chao Xie <chao.xie@marvell.com>
---
arch/arm/mach-mmp/brownstone.c | 68 ++++++++++++++++++++++++++++++++++++++++
1 files changed, 68 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-mmp/brownstone.c b/arch/arm/mach-mmp/brownstone.c
index 5cb769c..f84613a 100644
--- a/arch/arm/mach-mmp/brownstone.c
+++ b/arch/arm/mach-mmp/brownstone.c
@@ -18,6 +18,9 @@
#include <linux/regulator/max8649.h>
#include <linux/regulator/fixed.h>
#include <linux/mfd/max8925.h>
+#include <linux/usb/phy.h>
+#include <linux/usb/mv_usb2.h>
+#include <linux/platform_data/mv_usb.h>
#include <asm/mach-types.h>
#include <asm/mach/arch.h>
@@ -195,6 +198,31 @@ static struct sram_platdata mmp2_isram_platdata = {
.granularity = SRAM_GRANULARITY,
};
+#ifdef CONFIG_USB_SUPPORT
+
+static char *mmp2_usb_clock_name[] = {
+ [0] = "usb_clk",
+};
+
+static struct mv_usb_phy_platform_data brownstone_usb_phy_pdata = {
+ .clknum = 1,
+ .clkname = mmp2_usb_clock_name,
+};
+
+#if defined(CONFIG_USB_MV_UDC) || defined(CONFIG_USB_EHCI_MV_U2O)
+
+static struct mv_usb_platform_data brownstone_usb_pdata = {
+ .clknum = 1,
+ .clkname = mmp2_usb_clock_name,
+ .vbus = NULL,
+ .mode = MV_USB_MODE_OTG,
+ .otg_force_a_bus_req = 1,
+ .set_vbus = NULL,
+};
+#endif
+#endif
+
+
static void __init brownstone_init(void)
{
mfp_config(ARRAY_AND_SIZE(brownstone_pin_config));
@@ -211,6 +239,46 @@ static void __init brownstone_init(void)
/* enable 5v regulator */
platform_device_register(&brownstone_v_5vp_device);
+
+#ifdef CONFIG_USB_SUPPORT
+ pxa_register_device(&mmp2_device_u2ophy, &brownstone_usb_phy_pdata,
+ sizeof(brownstone_usb_phy_pdata));
+#endif
+
+#ifdef CONFIG_USB_MV_UDC
+ /* for usb2 phy */
+ usb_bind_phy(mmp2_device_u2o.drv_name, MV_USB2_PHY_INDEX,
+ mmp2_device_u2ophy.drv_name);
+#ifdef CONFIG_USB_MV_OTG
+ /* for usb2 otg phy */
+ usb_bind_phy(mmp2_device_u2o.drv_name, MV_USB2_OTG_PHY_INDEX,
+ mmp2_device_u2ootg.drv_name);
+#endif
+ pxa_register_device(&mmp2_device_u2o, &brownstone_usb_pdata,
+ sizeof(brownstone_usb_pdata));
+#endif
+
+#ifdef CONFIG_USB_EHCI_MV_U2O
+ /* for usb2 phy */
+ usb_bind_phy(mmp2_device_u2oehci.dev_name, MV_USB2_PHY_INDEX,
+ mmp2_device_u2ophy.dev_name);
+#ifdef CONFIG_USB_MV_OTG
+ /* for usb2 otg phy */
+ usb_bind_phy(mmp2_device_u2oehci.drv_name, MV_USB2_OTG_PHY_INDEX,
+ mmp2_device_u2ootg.drv_name);
+#endif
+ pxa_register_device(&mmp2_device_u2oehci, &brownstone_usb_pdata,
+ sizeof(brownstone_usb_pdata));
+#endif
+
+#ifdef CONFIG_USB_MV_OTG
+ /* for usb2 phy */
+ usb_bind_phy(mmp2_device_u2ootg.dev_name, MV_USB2_PHY_INDEX,
+ mmp2_device_u2ophy.dev_name);
+ pxa_register_device(&mmp2_device_u2ootg, &brownstone_usb_pdata,
+ sizeof(brownstone_usb_pdata));
+#endif
+
}
MACHINE_START(BROWNSTONE, "Brownstone Development Platform")
--
1.7.4.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* [V8 PATCH 08/16] arm: ttc_dkb: add usb support
2013-02-21 4:07 [V8 PATCH 00/16] mv-usb phy enhancement patches Chao Xie
` (6 preceding siblings ...)
2013-02-21 4:07 ` [V8 PATCH 07/16] arm: brownstone: add usb support for the board Chao Xie
@ 2013-02-21 4:07 ` Chao Xie
2013-02-21 4:07 ` [V8 PATCH 09/16] arm: mmp: remove the usb phy setting Chao Xie
` (8 subsequent siblings)
16 siblings, 0 replies; 34+ messages in thread
From: Chao Xie @ 2013-02-21 4:07 UTC (permalink / raw)
To: linux-arm-kernel
for ttc_dkb board, add udc/otg/ehci support
Signed-off-by: Chao Xie <chao.xie@marvell.com>
---
arch/arm/mach-mmp/ttc_dkb.c | 50 +++++++++++++++++++++++++++++++++---------
1 files changed, 39 insertions(+), 11 deletions(-)
diff --git a/arch/arm/mach-mmp/ttc_dkb.c b/arch/arm/mach-mmp/ttc_dkb.c
index ce55fd8..5622092 100644
--- a/arch/arm/mach-mmp/ttc_dkb.c
+++ b/arch/arm/mach-mmp/ttc_dkb.c
@@ -18,6 +18,8 @@
#include <linux/i2c/pca953x.h>
#include <linux/gpio.h>
#include <linux/mfd/88pm860x.h>
+#include <linux/usb/phy.h>
+#include <linux/usb/mv_usb2.h>
#include <linux/platform_data/mv_usb.h>
#include <asm/mach-types.h>
@@ -27,7 +29,6 @@
#include <mach/mfp-pxa910.h>
#include <mach/pxa910.h>
#include <mach/irqs.h>
-#include <mach/regs-usb.h>
#include "common.h"
@@ -158,20 +159,24 @@ static struct i2c_board_info ttc_dkb_i2c_info[] = {
};
#ifdef CONFIG_USB_SUPPORT
-#if defined(CONFIG_USB_MV_UDC) || defined(CONFIG_USB_EHCI_MV_U2O)
static char *pxa910_usb_clock_name[] = {
- [0] = "U2OCLK",
+ [0] = "usb_clk",
};
+static struct mv_usb_phy_platform_data ttc_usb_phy_pdata = {
+ .clknum = 1,
+ .clkname = pxa910_usb_clock_name,
+};
+
+#if defined(CONFIG_USB_MV_UDC) || defined(CONFIG_USB_EHCI_MV_U2O)
+
static struct mv_usb_platform_data ttc_usb_pdata = {
.clknum = 1,
.clkname = pxa910_usb_clock_name,
.vbus = NULL,
.mode = MV_USB_MODE_OTG,
.otg_force_a_bus_req = 1,
- .phy_init = pxa_usb_phy_init,
- .phy_deinit = pxa_usb_phy_deinit,
.set_vbus = NULL,
};
#endif
@@ -198,19 +203,42 @@ static void __init ttc_dkb_init(void)
pxa910_add_twsi(0, NULL, ARRAY_AND_SIZE(ttc_dkb_i2c_info));
platform_add_devices(ARRAY_AND_SIZE(ttc_dkb_devices));
+#ifdef CONFIG_USB_SUPPORT
+ pxa_register_device(&pxa910_device_u2ophy, &ttc_usb_phy_pdata,
+ sizeof(ttc_usb_phy_pdata));
+#endif
+
#ifdef CONFIG_USB_MV_UDC
- pxa168_device_u2o.dev.platform_data = &ttc_usb_pdata;
- platform_device_register(&pxa168_device_u2o);
+ /* for usb2 phy */
+ usb_bind_phy(pxa910_device_u2o.drv_name, MV_USB2_PHY_INDEX,
+ pxa910_device_u2ophy.drv_name);
+#ifdef CONFIG_USB_MV_OTG
+ /* for usb2 otg phy */
+ usb_bind_phy(pxa910_device_u2o.drv_name, MV_USB2_OTG_PHY_INDEX,
+ pxa910_device_u2ootg.drv_name);
+#endif
+ pxa_register_device(&pxa910_device_u2o, &ttc_usb_pdata,
+ sizeof(ttc_usb_pdata));
#endif
#ifdef CONFIG_USB_EHCI_MV_U2O
- pxa168_device_u2oehci.dev.platform_data = &ttc_usb_pdata;
- platform_device_register(&pxa168_device_u2oehci);
+ /* for usb2 phy */
+ usb_bind_phy(pxa910_device_u2oehci.drv_name, MV_USB2_PHY_INDEX,
+ pxa910_device_u2ophy.drv_name);
+#ifdef CONFIG_USB_MV_OTG
+ /* for usb2 otg phy */
+ usb_bind_phy(pxa910_device_u2oehci.drv_name, MV_USB2_OTG_PHY_INDEX,
+ pxa910_device_u2ootg.drv_name);
+#endif
+ pxa_register_device(&pxa910_device_u2oehci, &ttc_usb_pdata,
+ sizeof(ttc_usb_pdata));
#endif
#ifdef CONFIG_USB_MV_OTG
- pxa168_device_u2ootg.dev.platform_data = &ttc_usb_pdata;
- platform_device_register(&pxa168_device_u2ootg);
+ usb_bind_phy(pxa910_device_u2ootg.drv_name, MV_USB2_PHY_INDEX,
+ pxa910_device_u2ophy.drv_name);
+ pxa_register_device(&pxa910_device_u2ootg, &ttc_usb_pdata,
+ sizeof(ttc_usb_pdata));
#endif
}
--
1.7.4.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* [V8 PATCH 09/16] arm: mmp: remove the usb phy setting
2013-02-21 4:07 [V8 PATCH 00/16] mv-usb phy enhancement patches Chao Xie
` (7 preceding siblings ...)
2013-02-21 4:07 ` [V8 PATCH 08/16] arm: ttc_dkb: add usb support Chao Xie
@ 2013-02-21 4:07 ` Chao Xie
2013-02-21 4:07 ` [V8 PATCH 10/16] arm: mmp: remove usb devices from pxa168 Chao Xie
` (7 subsequent siblings)
16 siblings, 0 replies; 34+ messages in thread
From: Chao Xie @ 2013-02-21 4:07 UTC (permalink / raw)
To: linux-arm-kernel
phy setting are formatted into a phy driver at drivers/usb/phy,
we do not need do the setting in SOC files.
Signed-off-by: Chao Xie <chao.xie@marvell.com>
---
arch/arm/mach-mmp/devices.c | 278 -----------------------------
arch/arm/mach-mmp/include/mach/regs-usb.h | 253 --------------------------
2 files changed, 0 insertions(+), 531 deletions(-)
delete mode 100644 arch/arm/mach-mmp/include/mach/regs-usb.h
diff --git a/arch/arm/mach-mmp/devices.c b/arch/arm/mach-mmp/devices.c
index dd2d8b1..af341e5 100644
--- a/arch/arm/mach-mmp/devices.c
+++ b/arch/arm/mach-mmp/devices.c
@@ -15,7 +15,6 @@
#include <mach/irqs.h>
#include <mach/devices.h>
#include <mach/cputype.h>
-#include <mach/regs-usb.h>
int __init pxa_register_device(struct pxa_device_desc *desc,
void *data, size_t size)
@@ -72,280 +71,3 @@ int __init pxa_register_device(struct pxa_device_desc *desc,
return platform_device_add(pdev);
}
-#if defined(CONFIG_USB) || defined(CONFIG_USB_GADGET)
-
-/*****************************************************************************
- * The registers read/write routines
- *****************************************************************************/
-
-static unsigned int u2o_get(void __iomem *base, unsigned int offset)
-{
- return readl_relaxed(base + offset);
-}
-
-static void u2o_set(void __iomem *base, unsigned int offset,
- unsigned int value)
-{
- u32 reg;
-
- reg = readl_relaxed(base + offset);
- reg |= value;
- writel_relaxed(reg, base + offset);
- readl_relaxed(base + offset);
-}
-
-static void u2o_clear(void __iomem *base, unsigned int offset,
- unsigned int value)
-{
- u32 reg;
-
- reg = readl_relaxed(base + offset);
- reg &= ~value;
- writel_relaxed(reg, base + offset);
- readl_relaxed(base + offset);
-}
-
-static void u2o_write(void __iomem *base, unsigned int offset,
- unsigned int value)
-{
- writel_relaxed(value, base + offset);
- readl_relaxed(base + offset);
-}
-
-#if defined(CONFIG_USB_MV_UDC) || defined(CONFIG_USB_EHCI_MV)
-
-#if defined(CONFIG_CPU_PXA910) || defined(CONFIG_CPU_PXA168)
-
-static DEFINE_MUTEX(phy_lock);
-static int phy_init_cnt;
-
-static int usb_phy_init_internal(void __iomem *base)
-{
- int loops;
-
- pr_info("Init usb phy!!!\n");
-
- /* Initialize the USB PHY power */
- if (cpu_is_pxa910()) {
- u2o_set(base, UTMI_CTRL, (1<<UTMI_CTRL_INPKT_DELAY_SOF_SHIFT)
- | (1<<UTMI_CTRL_PU_REF_SHIFT));
- }
-
- u2o_set(base, UTMI_CTRL, 1<<UTMI_CTRL_PLL_PWR_UP_SHIFT);
- u2o_set(base, UTMI_CTRL, 1<<UTMI_CTRL_PWR_UP_SHIFT);
-
- /* UTMI_PLL settings */
- u2o_clear(base, UTMI_PLL, UTMI_PLL_PLLVDD18_MASK
- | UTMI_PLL_PLLVDD12_MASK | UTMI_PLL_PLLCALI12_MASK
- | UTMI_PLL_FBDIV_MASK | UTMI_PLL_REFDIV_MASK
- | UTMI_PLL_ICP_MASK | UTMI_PLL_KVCO_MASK);
-
- u2o_set(base, UTMI_PLL, 0xee<<UTMI_PLL_FBDIV_SHIFT
- | 0xb<<UTMI_PLL_REFDIV_SHIFT | 3<<UTMI_PLL_PLLVDD18_SHIFT
- | 3<<UTMI_PLL_PLLVDD12_SHIFT | 3<<UTMI_PLL_PLLCALI12_SHIFT
- | 1<<UTMI_PLL_ICP_SHIFT | 3<<UTMI_PLL_KVCO_SHIFT);
-
- /* UTMI_TX */
- u2o_clear(base, UTMI_TX, UTMI_TX_REG_EXT_FS_RCAL_EN_MASK
- | UTMI_TX_TXVDD12_MASK | UTMI_TX_CK60_PHSEL_MASK
- | UTMI_TX_IMPCAL_VTH_MASK | UTMI_TX_REG_EXT_FS_RCAL_MASK
- | UTMI_TX_AMP_MASK);
- u2o_set(base, UTMI_TX, 3<<UTMI_TX_TXVDD12_SHIFT
- | 4<<UTMI_TX_CK60_PHSEL_SHIFT | 4<<UTMI_TX_IMPCAL_VTH_SHIFT
- | 8<<UTMI_TX_REG_EXT_FS_RCAL_SHIFT | 3<<UTMI_TX_AMP_SHIFT);
-
- /* UTMI_RX */
- u2o_clear(base, UTMI_RX, UTMI_RX_SQ_THRESH_MASK
- | UTMI_REG_SQ_LENGTH_MASK);
- u2o_set(base, UTMI_RX, 7<<UTMI_RX_SQ_THRESH_SHIFT
- | 2<<UTMI_REG_SQ_LENGTH_SHIFT);
-
- /* UTMI_IVREF */
- if (cpu_is_pxa168())
- /* fixing Microsoft Altair board interface with NEC hub issue -
- * Set UTMI_IVREF from 0x4a3 to 0x4bf */
- u2o_write(base, UTMI_IVREF, 0x4bf);
-
- /* toggle VCOCAL_START bit of UTMI_PLL */
- udelay(200);
- u2o_set(base, UTMI_PLL, VCOCAL_START);
- udelay(40);
- u2o_clear(base, UTMI_PLL, VCOCAL_START);
-
- /* toggle REG_RCAL_START bit of UTMI_TX */
- udelay(400);
- u2o_set(base, UTMI_TX, REG_RCAL_START);
- udelay(40);
- u2o_clear(base, UTMI_TX, REG_RCAL_START);
- udelay(400);
-
- /* Make sure PHY PLL is ready */
- loops = 0;
- while ((u2o_get(base, UTMI_PLL) & PLL_READY) == 0) {
- mdelay(1);
- loops++;
- if (loops > 100) {
- printk(KERN_WARNING "calibrate timeout, UTMI_PLL %x\n",
- u2o_get(base, UTMI_PLL));
- break;
- }
- }
-
- if (cpu_is_pxa168()) {
- u2o_set(base, UTMI_RESERVE, 1 << 5);
- /* Turn on UTMI PHY OTG extension */
- u2o_write(base, UTMI_OTG_ADDON, 1);
- }
-
- return 0;
-}
-
-static int usb_phy_deinit_internal(void __iomem *base)
-{
- pr_info("Deinit usb phy!!!\n");
-
- if (cpu_is_pxa168())
- u2o_clear(base, UTMI_OTG_ADDON, UTMI_OTG_ADDON_OTG_ON);
-
- u2o_clear(base, UTMI_CTRL, UTMI_CTRL_RXBUF_PDWN);
- u2o_clear(base, UTMI_CTRL, UTMI_CTRL_TXBUF_PDWN);
- u2o_clear(base, UTMI_CTRL, UTMI_CTRL_USB_CLK_EN);
- u2o_clear(base, UTMI_CTRL, 1<<UTMI_CTRL_PWR_UP_SHIFT);
- u2o_clear(base, UTMI_CTRL, 1<<UTMI_CTRL_PLL_PWR_UP_SHIFT);
-
- return 0;
-}
-
-int pxa_usb_phy_init(void __iomem *phy_reg)
-{
- mutex_lock(&phy_lock);
- if (phy_init_cnt++ == 0)
- usb_phy_init_internal(phy_reg);
- mutex_unlock(&phy_lock);
- return 0;
-}
-
-void pxa_usb_phy_deinit(void __iomem *phy_reg)
-{
- WARN_ON(phy_init_cnt == 0);
-
- mutex_lock(&phy_lock);
- if (--phy_init_cnt == 0)
- usb_phy_deinit_internal(phy_reg);
- mutex_unlock(&phy_lock);
-}
-#endif
-#endif
-#endif
-
-#ifdef CONFIG_USB_SUPPORT
-static u64 usb_dma_mask = ~(u32)0;
-
-#ifdef CONFIG_USB_MV_UDC
-struct resource pxa168_u2o_resources[] = {
- /* regbase */
- [0] = {
- .start = PXA168_U2O_REGBASE + U2x_CAPREGS_OFFSET,
- .end = PXA168_U2O_REGBASE + USB_REG_RANGE,
- .flags = IORESOURCE_MEM,
- .name = "capregs",
- },
- /* phybase */
- [1] = {
- .start = PXA168_U2O_PHYBASE,
- .end = PXA168_U2O_PHYBASE + USB_PHY_RANGE,
- .flags = IORESOURCE_MEM,
- .name = "phyregs",
- },
- [2] = {
- .start = IRQ_PXA168_USB1,
- .end = IRQ_PXA168_USB1,
- .flags = IORESOURCE_IRQ,
- },
-};
-
-struct platform_device pxa168_device_u2o = {
- .name = "mv-udc",
- .id = -1,
- .resource = pxa168_u2o_resources,
- .num_resources = ARRAY_SIZE(pxa168_u2o_resources),
- .dev = {
- .dma_mask = &usb_dma_mask,
- .coherent_dma_mask = 0xffffffff,
- }
-};
-#endif /* CONFIG_USB_MV_UDC */
-
-#ifdef CONFIG_USB_EHCI_MV_U2O
-struct resource pxa168_u2oehci_resources[] = {
- /* regbase */
- [0] = {
- .start = PXA168_U2O_REGBASE + U2x_CAPREGS_OFFSET,
- .end = PXA168_U2O_REGBASE + USB_REG_RANGE,
- .flags = IORESOURCE_MEM,
- .name = "capregs",
- },
- /* phybase */
- [1] = {
- .start = PXA168_U2O_PHYBASE,
- .end = PXA168_U2O_PHYBASE + USB_PHY_RANGE,
- .flags = IORESOURCE_MEM,
- .name = "phyregs",
- },
- [2] = {
- .start = IRQ_PXA168_USB1,
- .end = IRQ_PXA168_USB1,
- .flags = IORESOURCE_IRQ,
- },
-};
-
-struct platform_device pxa168_device_u2oehci = {
- .name = "pxa-u2oehci",
- .id = -1,
- .dev = {
- .dma_mask = &usb_dma_mask,
- .coherent_dma_mask = 0xffffffff,
- },
-
- .num_resources = ARRAY_SIZE(pxa168_u2oehci_resources),
- .resource = pxa168_u2oehci_resources,
-};
-#endif
-
-#if defined(CONFIG_USB_MV_OTG)
-struct resource pxa168_u2ootg_resources[] = {
- /* regbase */
- [0] = {
- .start = PXA168_U2O_REGBASE + U2x_CAPREGS_OFFSET,
- .end = PXA168_U2O_REGBASE + USB_REG_RANGE,
- .flags = IORESOURCE_MEM,
- .name = "capregs",
- },
- /* phybase */
- [1] = {
- .start = PXA168_U2O_PHYBASE,
- .end = PXA168_U2O_PHYBASE + USB_PHY_RANGE,
- .flags = IORESOURCE_MEM,
- .name = "phyregs",
- },
- [2] = {
- .start = IRQ_PXA168_USB1,
- .end = IRQ_PXA168_USB1,
- .flags = IORESOURCE_IRQ,
- },
-};
-
-struct platform_device pxa168_device_u2ootg = {
- .name = "mv-otg",
- .id = -1,
- .dev = {
- .dma_mask = &usb_dma_mask,
- .coherent_dma_mask = 0xffffffff,
- },
-
- .num_resources = ARRAY_SIZE(pxa168_u2ootg_resources),
- .resource = pxa168_u2ootg_resources,
-};
-#endif /* CONFIG_USB_MV_OTG */
-
-#endif
diff --git a/arch/arm/mach-mmp/include/mach/regs-usb.h b/arch/arm/mach-mmp/include/mach/regs-usb.h
deleted file mode 100644
index b047bf4..0000000
--- a/arch/arm/mach-mmp/include/mach/regs-usb.h
+++ /dev/null
@@ -1,253 +0,0 @@
-/*
- * Copyright (C) 2011 Marvell International Ltd. All rights reserved.
- *
- * 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
- * Free Software Foundation; either version 2 of the License, or (at your
- * option) any later version.
- */
-
-#ifndef __ASM_ARCH_REGS_USB_H
-#define __ASM_ARCH_REGS_USB_H
-
-#define PXA168_U2O_REGBASE (0xd4208000)
-#define PXA168_U2O_PHYBASE (0xd4207000)
-
-#define PXA168_U2H_REGBASE (0xd4209000)
-#define PXA168_U2H_PHYBASE (0xd4206000)
-
-#define MMP3_HSIC1_REGBASE (0xf0001000)
-#define MMP3_HSIC1_PHYBASE (0xf0001800)
-
-#define MMP3_HSIC2_REGBASE (0xf0002000)
-#define MMP3_HSIC2_PHYBASE (0xf0002800)
-
-#define MMP3_FSIC_REGBASE (0xf0003000)
-#define MMP3_FSIC_PHYBASE (0xf0003800)
-
-
-#define USB_REG_RANGE (0x1ff)
-#define USB_PHY_RANGE (0xff)
-
-/* registers */
-#define U2x_CAPREGS_OFFSET 0x100
-
-/* phy regs */
-#define UTMI_REVISION 0x0
-#define UTMI_CTRL 0x4
-#define UTMI_PLL 0x8
-#define UTMI_TX 0xc
-#define UTMI_RX 0x10
-#define UTMI_IVREF 0x14
-#define UTMI_T0 0x18
-#define UTMI_T1 0x1c
-#define UTMI_T2 0x20
-#define UTMI_T3 0x24
-#define UTMI_T4 0x28
-#define UTMI_T5 0x2c
-#define UTMI_RESERVE 0x30
-#define UTMI_USB_INT 0x34
-#define UTMI_DBG_CTL 0x38
-#define UTMI_OTG_ADDON 0x3c
-
-/* For UTMICTRL Register */
-#define UTMI_CTRL_USB_CLK_EN (1 << 31)
-/* pxa168 */
-#define UTMI_CTRL_SUSPEND_SET1 (1 << 30)
-#define UTMI_CTRL_SUSPEND_SET2 (1 << 29)
-#define UTMI_CTRL_RXBUF_PDWN (1 << 24)
-#define UTMI_CTRL_TXBUF_PDWN (1 << 11)
-
-#define UTMI_CTRL_INPKT_DELAY_SHIFT 30
-#define UTMI_CTRL_INPKT_DELAY_SOF_SHIFT 28
-#define UTMI_CTRL_PU_REF_SHIFT 20
-#define UTMI_CTRL_ARC_PULLDN_SHIFT 12
-#define UTMI_CTRL_PLL_PWR_UP_SHIFT 1
-#define UTMI_CTRL_PWR_UP_SHIFT 0
-
-/* For UTMI_PLL Register */
-#define UTMI_PLL_PLLCALI12_SHIFT 29
-#define UTMI_PLL_PLLCALI12_MASK (0x3 << 29)
-
-#define UTMI_PLL_PLLVDD18_SHIFT 27
-#define UTMI_PLL_PLLVDD18_MASK (0x3 << 27)
-
-#define UTMI_PLL_PLLVDD12_SHIFT 25
-#define UTMI_PLL_PLLVDD12_MASK (0x3 << 25)
-
-#define UTMI_PLL_CLK_BLK_EN_SHIFT 24
-#define CLK_BLK_EN (0x1 << 24)
-#define PLL_READY (0x1 << 23)
-#define KVCO_EXT (0x1 << 22)
-#define VCOCAL_START (0x1 << 21)
-
-#define UTMI_PLL_KVCO_SHIFT 15
-#define UTMI_PLL_KVCO_MASK (0x7 << 15)
-
-#define UTMI_PLL_ICP_SHIFT 12
-#define UTMI_PLL_ICP_MASK (0x7 << 12)
-
-#define UTMI_PLL_FBDIV_SHIFT 4
-#define UTMI_PLL_FBDIV_MASK (0xFF << 4)
-
-#define UTMI_PLL_REFDIV_SHIFT 0
-#define UTMI_PLL_REFDIV_MASK (0xF << 0)
-
-/* For UTMI_TX Register */
-#define UTMI_TX_REG_EXT_FS_RCAL_SHIFT 27
-#define UTMI_TX_REG_EXT_FS_RCAL_MASK (0xf << 27)
-
-#define UTMI_TX_REG_EXT_FS_RCAL_EN_SHIFT 26
-#define UTMI_TX_REG_EXT_FS_RCAL_EN_MASK (0x1 << 26)
-
-#define UTMI_TX_TXVDD12_SHIFT 22
-#define UTMI_TX_TXVDD12_MASK (0x3 << 22)
-
-#define UTMI_TX_CK60_PHSEL_SHIFT 17
-#define UTMI_TX_CK60_PHSEL_MASK (0xf << 17)
-
-#define UTMI_TX_IMPCAL_VTH_SHIFT 14
-#define UTMI_TX_IMPCAL_VTH_MASK (0x7 << 14)
-
-#define REG_RCAL_START (0x1 << 12)
-
-#define UTMI_TX_LOW_VDD_EN_SHIFT 11
-
-#define UTMI_TX_AMP_SHIFT 0
-#define UTMI_TX_AMP_MASK (0x7 << 0)
-
-/* For UTMI_RX Register */
-#define UTMI_REG_SQ_LENGTH_SHIFT 15
-#define UTMI_REG_SQ_LENGTH_MASK (0x3 << 15)
-
-#define UTMI_RX_SQ_THRESH_SHIFT 4
-#define UTMI_RX_SQ_THRESH_MASK (0xf << 4)
-
-#define UTMI_OTG_ADDON_OTG_ON (1 << 0)
-
-/* For MMP3 USB Phy */
-#define USB2_PLL_REG0 0x4
-#define USB2_PLL_REG1 0x8
-#define USB2_TX_REG0 0x10
-#define USB2_TX_REG1 0x14
-#define USB2_TX_REG2 0x18
-#define USB2_RX_REG0 0x20
-#define USB2_RX_REG1 0x24
-#define USB2_RX_REG2 0x28
-#define USB2_ANA_REG0 0x30
-#define USB2_ANA_REG1 0x34
-#define USB2_ANA_REG2 0x38
-#define USB2_DIG_REG0 0x3C
-#define USB2_DIG_REG1 0x40
-#define USB2_DIG_REG2 0x44
-#define USB2_DIG_REG3 0x48
-#define USB2_TEST_REG0 0x4C
-#define USB2_TEST_REG1 0x50
-#define USB2_TEST_REG2 0x54
-#define USB2_CHARGER_REG0 0x58
-#define USB2_OTG_REG0 0x5C
-#define USB2_PHY_MON0 0x60
-#define USB2_RESETVE_REG0 0x64
-#define USB2_ICID_REG0 0x78
-#define USB2_ICID_REG1 0x7C
-
-/* USB2_PLL_REG0 */
-/* This is for Ax stepping */
-#define USB2_PLL_FBDIV_SHIFT_MMP3 0
-#define USB2_PLL_FBDIV_MASK_MMP3 (0xFF << 0)
-
-#define USB2_PLL_REFDIV_SHIFT_MMP3 8
-#define USB2_PLL_REFDIV_MASK_MMP3 (0xF << 8)
-
-#define USB2_PLL_VDD12_SHIFT_MMP3 12
-#define USB2_PLL_VDD18_SHIFT_MMP3 14
-
-/* This is for B0 stepping */
-#define USB2_PLL_FBDIV_SHIFT_MMP3_B0 0
-#define USB2_PLL_REFDIV_SHIFT_MMP3_B0 9
-#define USB2_PLL_VDD18_SHIFT_MMP3_B0 14
-#define USB2_PLL_FBDIV_MASK_MMP3_B0 0x01FF
-#define USB2_PLL_REFDIV_MASK_MMP3_B0 0x3E00
-
-#define USB2_PLL_CAL12_SHIFT_MMP3 0
-#define USB2_PLL_CALI12_MASK_MMP3 (0x3 << 0)
-
-#define USB2_PLL_VCOCAL_START_SHIFT_MMP3 2
-
-#define USB2_PLL_KVCO_SHIFT_MMP3 4
-#define USB2_PLL_KVCO_MASK_MMP3 (0x7<<4)
-
-#define USB2_PLL_ICP_SHIFT_MMP3 8
-#define USB2_PLL_ICP_MASK_MMP3 (0x7<<8)
-
-#define USB2_PLL_LOCK_BYPASS_SHIFT_MMP3 12
-
-#define USB2_PLL_PU_PLL_SHIFT_MMP3 13
-#define USB2_PLL_PU_PLL_MASK (0x1 << 13)
-
-#define USB2_PLL_READY_MASK_MMP3 (0x1 << 15)
-
-/* USB2_TX_REG0 */
-#define USB2_TX_IMPCAL_VTH_SHIFT_MMP3 8
-#define USB2_TX_IMPCAL_VTH_MASK_MMP3 (0x7 << 8)
-
-#define USB2_TX_RCAL_START_SHIFT_MMP3 13
-
-/* USB2_TX_REG1 */
-#define USB2_TX_CK60_PHSEL_SHIFT_MMP3 0
-#define USB2_TX_CK60_PHSEL_MASK_MMP3 (0xf << 0)
-
-#define USB2_TX_AMP_SHIFT_MMP3 4
-#define USB2_TX_AMP_MASK_MMP3 (0x7 << 4)
-
-#define USB2_TX_VDD12_SHIFT_MMP3 8
-#define USB2_TX_VDD12_MASK_MMP3 (0x3 << 8)
-
-/* USB2_TX_REG2 */
-#define USB2_TX_DRV_SLEWRATE_SHIFT 10
-
-/* USB2_RX_REG0 */
-#define USB2_RX_SQ_THRESH_SHIFT_MMP3 4
-#define USB2_RX_SQ_THRESH_MASK_MMP3 (0xf << 4)
-
-#define USB2_RX_SQ_LENGTH_SHIFT_MMP3 10
-#define USB2_RX_SQ_LENGTH_MASK_MMP3 (0x3 << 10)
-
-/* USB2_ANA_REG1*/
-#define USB2_ANA_PU_ANA_SHIFT_MMP3 14
-
-/* USB2_OTG_REG0 */
-#define USB2_OTG_PU_OTG_SHIFT_MMP3 3
-
-/* fsic registers */
-#define FSIC_MISC 0x4
-#define FSIC_INT 0x28
-#define FSIC_CTRL 0x30
-
-/* HSIC registers */
-#define HSIC_PAD_CTRL 0x4
-
-#define HSIC_CTRL 0x8
-#define HSIC_CTRL_HSIC_ENABLE (1<<7)
-#define HSIC_CTRL_PLL_BYPASS (1<<4)
-
-#define TEST_GRP_0 0xc
-#define TEST_GRP_1 0x10
-
-#define HSIC_INT 0x14
-#define HSIC_INT_READY_INT_EN (1<<10)
-#define HSIC_INT_CONNECT_INT_EN (1<<9)
-#define HSIC_INT_CORE_INT_EN (1<<8)
-#define HSIC_INT_HS_READY (1<<2)
-#define HSIC_INT_CONNECT (1<<1)
-#define HSIC_INT_CORE (1<<0)
-
-#define HSIC_CONFIG 0x18
-#define USBHSIC_CTRL 0x20
-
-#define HSIC_USB_CTRL 0x28
-#define HSIC_USB_CTRL_CLKEN 1
-#define HSIC_USB_CLK_PHY 0x0
-#define HSIC_USB_CLK_PMU 0x1
-
-#endif /* __ASM_ARCH_PXA_U2O_H */
--
1.7.4.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* [V8 PATCH 10/16] arm: mmp: remove usb devices from pxa168
2013-02-21 4:07 [V8 PATCH 00/16] mv-usb phy enhancement patches Chao Xie
` (8 preceding siblings ...)
2013-02-21 4:07 ` [V8 PATCH 09/16] arm: mmp: remove the usb phy setting Chao Xie
@ 2013-02-21 4:07 ` Chao Xie
2013-02-21 4:07 ` [V8 PATCH 11/16] usb: phy: mv_usb2_phy: add externel chip support Chao Xie
` (6 subsequent siblings)
16 siblings, 0 replies; 34+ messages in thread
From: Chao Xie @ 2013-02-21 4:07 UTC (permalink / raw)
To: linux-arm-kernel
Signed-off-by: Chao Xie <chao.xie@marvell.com>
---
arch/arm/mach-mmp/pxa168.c | 42 ------------------------------------------
1 files changed, 0 insertions(+), 42 deletions(-)
diff --git a/arch/arm/mach-mmp/pxa168.c b/arch/arm/mach-mmp/pxa168.c
index b7f074f..dd3a68b 100644
--- a/arch/arm/mach-mmp/pxa168.c
+++ b/arch/arm/mach-mmp/pxa168.c
@@ -28,7 +28,6 @@
#include <mach/mfp.h>
#include <linux/dma-mapping.h>
#include <mach/pxa168.h>
-#include <mach/regs-usb.h>
#include "common.h"
#include "clock.h"
@@ -135,47 +134,6 @@ struct platform_device pxa168_device_gpio = {
.resource = pxa168_resource_gpio,
};
-struct resource pxa168_usb_host_resources[] = {
- /* USB Host conroller register base */
- [0] = {
- .start = PXA168_U2H_REGBASE + U2x_CAPREGS_OFFSET,
- .end = PXA168_U2H_REGBASE + USB_REG_RANGE,
- .flags = IORESOURCE_MEM,
- .name = "capregs",
- },
- /* USB PHY register base */
- [1] = {
- .start = PXA168_U2H_PHYBASE,
- .end = PXA168_U2H_PHYBASE + USB_PHY_RANGE,
- .flags = IORESOURCE_MEM,
- .name = "phyregs",
- },
- [2] = {
- .start = IRQ_PXA168_USB2,
- .end = IRQ_PXA168_USB2,
- .flags = IORESOURCE_IRQ,
- },
-};
-
-static u64 pxa168_usb_host_dmamask = DMA_BIT_MASK(32);
-struct platform_device pxa168_device_usb_host = {
- .name = "pxa-sph",
- .id = -1,
- .dev = {
- .dma_mask = &pxa168_usb_host_dmamask,
- .coherent_dma_mask = DMA_BIT_MASK(32),
- },
-
- .num_resources = ARRAY_SIZE(pxa168_usb_host_resources),
- .resource = pxa168_usb_host_resources,
-};
-
-int __init pxa168_add_usb_host(struct mv_usb_platform_data *pdata)
-{
- pxa168_device_usb_host.dev.platform_data = pdata;
- return platform_device_register(&pxa168_device_usb_host);
-}
-
void pxa168_restart(char mode, const char *cmd)
{
soft_restart(0xffff0000);
--
1.7.4.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* [V8 PATCH 11/16] usb: phy: mv_usb2_phy: add externel chip support
2013-02-21 4:07 [V8 PATCH 00/16] mv-usb phy enhancement patches Chao Xie
` (9 preceding siblings ...)
2013-02-21 4:07 ` [V8 PATCH 10/16] arm: mmp: remove usb devices from pxa168 Chao Xie
@ 2013-02-21 4:07 ` Chao Xie
2013-02-21 4:07 ` [V8 PATCH 12/16] usb: gadget: mv_udc: add extern " Chao Xie
` (5 subsequent siblings)
16 siblings, 0 replies; 34+ messages in thread
From: Chao Xie @ 2013-02-21 4:07 UTC (permalink / raw)
To: linux-arm-kernel
For the vbus and idpin detection. It may be completed by some
external chip, for example the pmic chip 88pm860x in driver/mfd
can do it.
Although the usb controller can detect the vbus and id pin, but
it need clock on and PHY enabled to detect the
vbus/idpin. It will increase the power.
Using the external chip to detect vbus/idpin can save the power.
Signed-off-by: Chao Xie <chao.xie@marvell.com>
---
drivers/usb/phy/mv_usb2_phy.c | 49 +++++++++++++++++++
include/linux/platform_data/mv_usb.h | 15 ++----
include/linux/usb/mv_usb2.h | 89 ++++++++++++++++++++++++++++++++++
3 files changed, 143 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/phy/mv_usb2_phy.c b/drivers/usb/phy/mv_usb2_phy.c
index a81e5e4..cbf207c 100644
--- a/drivers/usb/phy/mv_usb2_phy.c
+++ b/drivers/usb/phy/mv_usb2_phy.c
@@ -129,6 +129,53 @@ enum mv_usb2_phy_type {
MMP2_USB,
};
+int mv_usb2_register_notifier(struct mv_usb2_phy *phy,
+ struct notifier_block *nb)
+{
+ int ret;
+
+ if (!phy)
+ return -ENODEV;
+
+ ret = atomic_notifier_chain_register(&phy->extern_chip.head, nb);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+EXPORT_SYMBOL(mv_usb2_register_notifier);
+
+int mv_usb2_unregister_notifier(struct mv_usb2_phy *phy,
+ struct notifier_block *nb)
+{
+ int ret;
+
+ if (!phy)
+ return -ENODEV;
+
+ ret = atomic_notifier_chain_unregister(&phy->extern_chip.head, nb);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+EXPORT_SYMBOL(mv_usb2_unregister_notifier);
+
+int mv_usb2_notify(struct mv_usb2_phy *phy, unsigned long val, void *v)
+{
+ int ret;
+
+ if (!phy)
+ return -ENODEV;
+
+ ret = atomic_notifier_call_chain(&phy->extern_chip.head, val, v);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+EXPORT_SYMBOL(mv_usb2_notify);
+
static unsigned int u2o_get(void __iomem *base, unsigned int offset)
{
return readl(base + offset);
@@ -363,6 +410,8 @@ static int mv_usb2_phy_probe(struct platform_device *pdev)
usb_add_phy_dev(&mv_phy->phy);
+ ATOMIC_INIT_NOTIFIER_HEAD(&mv_phy->extern_chip.head);
+
platform_set_drvdata(pdev, mv_phy);
return 0;
diff --git a/include/linux/platform_data/mv_usb.h b/include/linux/platform_data/mv_usb.h
index fd3d1b4..dc25d60 100644
--- a/include/linux/platform_data/mv_usb.h
+++ b/include/linux/platform_data/mv_usb.h
@@ -28,16 +28,13 @@ enum {
VBUS_HIGH = 1 << 0,
};
-struct mv_usb_addon_irq {
- unsigned int irq;
- int (*poll)(void);
-};
-
+#define MV_USB_HAS_VBUS_DETECTION (1 << 0)
+#define MV_USB_HAS_IDPIN_DETECTION (1 << 1)
struct mv_usb_platform_data {
unsigned int clknum;
char **clkname;
- struct mv_usb_addon_irq *id; /* Only valid for OTG. ID pin change*/
- struct mv_usb_addon_irq *vbus; /* valid for OTG/UDC. VBUS change*/
+
+ unsigned int extern_attr;
/* only valid for HCD. OTG or Host only*/
unsigned int mode;
@@ -45,9 +42,7 @@ struct mv_usb_platform_data {
/* This flag is used for that needs id pin checked by otg */
unsigned int disable_otg_clock_gating:1;
/* Force a_bus_req to be asserted */
- unsigned int otg_force_a_bus_req:1;
-
- int (*set_vbus)(unsigned int vbus);
+ unsigned int otg_force_a_bus_req:1;
};
struct mv_usb_phy_platform_data {
diff --git a/include/linux/usb/mv_usb2.h b/include/linux/usb/mv_usb2.h
index 77250f5..f0bfacb 100644
--- a/include/linux/usb/mv_usb2.h
+++ b/include/linux/usb/mv_usb2.h
@@ -18,6 +18,33 @@
#define MV_USB2_PHY_INDEX 0
#define MV_USB2_OTG_PHY_INDEX 1
+enum {
+ EVENT_VBUS,
+ EVENT_ID,
+};
+
+struct pxa_usb_vbus_ops {
+ int (*get_vbus)(unsigned int *level);
+ int (*set_vbus)(unsigned int level);
+ int (*init)(void);
+};
+
+struct pxa_usb_idpin_ops {
+ int (*get_idpin)(unsigned int *level);
+ int (*init)(void);
+};
+
+struct pxa_usb_extern_ops {
+ struct pxa_usb_vbus_ops vbus;
+ struct pxa_usb_idpin_ops idpin;
+};
+
+struct mv_usb2_extern_chip {
+ unsigned int id;
+ struct pxa_usb_extern_ops ops;
+ struct atomic_notifier_head head;
+};
+
struct mv_usb2_phy {
struct usb_phy phy;
struct platform_device *pdev;
@@ -27,6 +54,68 @@ struct mv_usb2_phy {
void __iomem *base;
struct clk **clks;
unsigned int clks_num;
+ struct mv_usb2_extern_chip extern_chip;
};
+#if defined(CONFIG_MV_USB2_PHY) || defined(CONFIG_MV_USB2_PHY_MODULE)
+#define mv_usb2_has_extern_call(phy, o, f, arg...)( \
+{ \
+ int ret; \
+ ret = (!phy ? 0 : ((phy->extern_chip.ops.o.f) ? \
+ 1 : 0)); \
+ ret; \
+} \
+)
+
+#define mv_usb2_extern_call(phy, o, f, args...)( \
+{ \
+ int ret; \
+ ret = (!phy ? -ENODEV : ((phy->extern_chip.ops.o.f) ? \
+ phy->extern_chip.ops.o.f(args) : -ENOIOCTLCMD));\
+ ret; \
+} \
+)
+
+#define mv_usb2_set_extern_call(phy, o, f, p)( \
+{ \
+ int ret; \
+ ret = !phy ? -ENODEV : ((phy->extern_chip.ops.o.f) ? \
+ -EINVAL : ({phy->extern_chip.ops.o.f = p; 0; })); \
+ ret; \
+} \
+)
+
+extern int mv_usb2_register_notifier(struct mv_usb2_phy *phy,
+ struct notifier_block *nb);
+extern int mv_usb2_unregister_notifier(struct mv_usb2_phy *phy,
+ struct notifier_block *nb);
+extern int mv_usb2_notify(struct mv_usb2_phy *phy, unsigned long val, void *v);
+
+#else
+
+#define mv_usb2_has_extern_call(phy, o, f, arg...)(0)
+
+#define mv_usb2_extern_call(phy, o, f, args...)(0)
+
+#define mv_usb2_set_extern_call(phy, o, f, p)(0)
+
+int mv_usb2_register_notifier(struct mv_usb2_phy *phy,
+ struct notifier_block *nb)
+{
+ return -EINVAL;
+}
+
+int mv_usb2_unregister_notifier(struct mv_usb2_phy *phy,
+ struct notifier_block *nb)
+{
+ return -EINVAL;
+}
+
+int mv_usb2_notify(struct mv_usb2_phy *phy, unsigned long val, void *v)
+{
+ return 0;
+}
+
+#endif
+
#endif
--
1.7.4.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* [V8 PATCH 12/16] usb: gadget: mv_udc: add extern chip support
2013-02-21 4:07 [V8 PATCH 00/16] mv-usb phy enhancement patches Chao Xie
` (10 preceding siblings ...)
2013-02-21 4:07 ` [V8 PATCH 11/16] usb: phy: mv_usb2_phy: add externel chip support Chao Xie
@ 2013-02-21 4:07 ` Chao Xie
2013-02-21 4:07 ` [V8 PATCH 13/16] usb: ehci: ehci-mv: " Chao Xie
` (4 subsequent siblings)
16 siblings, 0 replies; 34+ messages in thread
From: Chao Xie @ 2013-02-21 4:07 UTC (permalink / raw)
To: linux-arm-kernel
Because arch-mmp will make use of irq domain for irq
allocation, the irqs allocated for PMIC is dynamical.
The vbus/idpin irqs from PMIC can not be passed by platform data.
Using the extern chip APIs provides by PHY driver can solve this
problem.
Marvell usb PHY driver provides a middle layer.
The PMIC usb drivers will help to register the callbacks in the
marvell usb PHY driver.
udc/otg/ehci driver will call the callbacks.
Then we do not need pass the information in platform data. It will
remove another block in the way of enabling device tree for usb.
Signed-off-by: Chao Xie <chao.xie@marvell.com>
---
drivers/usb/gadget/mv_udc.h | 3 ++
drivers/usb/gadget/mv_udc_core.c | 56 +++++++++++++++++++++++++-------------
2 files changed, 40 insertions(+), 19 deletions(-)
diff --git a/drivers/usb/gadget/mv_udc.h b/drivers/usb/gadget/mv_udc.h
index f339df4..5d44a6f 100644
--- a/drivers/usb/gadget/mv_udc.h
+++ b/drivers/usb/gadget/mv_udc.h
@@ -178,6 +178,9 @@ struct mv_udc {
struct platform_device *dev;
int irq;
+ unsigned int extern_attr;
+ struct notifier_block notifier;
+
struct mv_cap_regs __iomem *cap_regs;
struct mv_op_regs __iomem *op_regs;
unsigned int max_eps;
diff --git a/drivers/usb/gadget/mv_udc_core.c b/drivers/usb/gadget/mv_udc_core.c
index 4876d2f..04b4195 100644
--- a/drivers/usb/gadget/mv_udc_core.c
+++ b/drivers/usb/gadget/mv_udc_core.c
@@ -2075,27 +2075,32 @@ static irqreturn_t mv_udc_irq(int irq, void *dev)
return IRQ_HANDLED;
}
-static irqreturn_t mv_udc_vbus_irq(int irq, void *dev)
+static int mv_udc_vbus_notifier_call(struct notifier_block *nb,
+ unsigned long val, void *v)
{
- struct mv_udc *udc = (struct mv_udc *)dev;
+ struct mv_udc *udc = container_of(nb, struct mv_udc, notifier);
/* polling VBUS and init phy may cause too much time*/
- if (udc->qwork)
+ if (udc->qwork && val == EVENT_VBUS)
queue_work(udc->qwork, &udc->vbus_work);
- return IRQ_HANDLED;
+ return 0;
}
static void mv_udc_vbus_work(struct work_struct *work)
{
struct mv_udc *udc;
- unsigned int vbus;
+ struct mv_usb2_phy *mv_phy;
+ unsigned int vbus = VBUS_LOW;
udc = container_of(work, struct mv_udc, vbus_work);
- if (!udc->pdata->vbus)
+ if (!(udc->extern_attr & MV_USB_HAS_VBUS_DETECTION))
+ return;
+
+ mv_phy = container_of(udc->phy, struct mv_usb2_phy, phy);
+ if (mv_usb2_extern_call(mv_phy, vbus, get_vbus, &vbus))
return;
- vbus = udc->pdata->vbus->poll();
dev_info(&udc->dev->dev, "vbus is %d\n", vbus);
if (vbus == VBUS_HIGH)
@@ -2117,11 +2122,17 @@ static void gadget_release(struct device *_dev)
static int mv_udc_remove(struct platform_device *pdev)
{
struct mv_udc *udc;
+ struct mv_usb2_phy *mv_phy;
udc = platform_get_drvdata(pdev);
+ mv_phy = container_of(udc->phy, struct mv_usb2_phy, phy);
+
usb_del_gadget_udc(&udc->gadget);
+ if (udc->extern_attr & MV_USB_HAS_VBUS_DETECTION)
+ mv_usb2_unregister_notifier(mv_phy, &udc->notifier);
+
if (udc->qwork) {
flush_workqueue(udc->qwork);
destroy_workqueue(udc->qwork);
@@ -2149,6 +2160,7 @@ static int mv_udc_probe(struct platform_device *pdev)
{
struct mv_usb_platform_data *pdata = pdev->dev.platform_data;
struct mv_udc *udc;
+ struct mv_usb2_phy *mv_phy;
int retval = 0;
int clk_i = 0;
struct resource *r;
@@ -2167,6 +2179,7 @@ static int mv_udc_probe(struct platform_device *pdev)
}
udc->done = &release_done;
+ udc->extern_attr = pdata->extern_attr;
udc->pdata = pdev->dev.platform_data;
spin_lock_init(&udc->lock);
@@ -2203,6 +2216,7 @@ static int mv_udc_probe(struct platform_device *pdev)
dev_err(&pdev->dev, "failed to map I/O memory\n");
return -EBUSY;
}
+ mv_phy = container_of(udc->phy, struct mv_usb2_phy, phy);
udc->phy = devm_usb_get_phy_dev(&pdev->dev, MV_USB2_PHY_INDEX);
if (IS_ERR_OR_NULL(udc->phy))
@@ -2315,17 +2329,8 @@ static int mv_udc_probe(struct platform_device *pdev)
/* VBUS detect: we can disable/enable clock on demand.*/
if (udc->transceiver)
udc->clock_gating = 1;
- else if (pdata->vbus) {
+ else if (udc->extern_attr & MV_USB_HAS_VBUS_DETECTION) {
udc->clock_gating = 1;
- retval = devm_request_threaded_irq(&pdev->dev,
- pdata->vbus->irq, NULL,
- mv_udc_vbus_irq, IRQF_ONESHOT, "vbus", udc);
- if (retval) {
- dev_info(&pdev->dev,
- "Can not request irq for VBUS, "
- "disable clock gating\n");
- udc->clock_gating = 0;
- }
udc->qwork = create_singlethread_workqueue("mv_udc_queue");
if (!udc->qwork) {
@@ -2335,6 +2340,8 @@ static int mv_udc_probe(struct platform_device *pdev)
}
INIT_WORK(&udc->vbus_work, mv_udc_vbus_work);
+ udc->notifier.notifier_call = mv_udc_vbus_notifier_call;
+ mv_usb2_register_notifier(mv_phy, &udc->notifier);
}
/*
@@ -2358,6 +2365,9 @@ static int mv_udc_probe(struct platform_device *pdev)
return 0;
err_create_workqueue:
+ if (!udc->transceiver
+ && (udc->extern_attr & MV_USB_HAS_VBUS_DETECTION))
+ mv_usb2_unregister_notifier(mv_phy, &udc->notifier);
destroy_workqueue(udc->qwork);
err_unregister:
device_unregister(&udc->gadget.dev);
@@ -2376,6 +2386,8 @@ err_disable_clock:
static int mv_udc_suspend(struct device *dev)
{
struct mv_udc *udc;
+ struct mv_usb2_phy *mv_phy;
+ unsigned int vbus;
udc = dev_get_drvdata(dev);
@@ -2383,11 +2395,17 @@ static int mv_udc_suspend(struct device *dev)
if (udc->transceiver)
return 0;
- if (udc->pdata->vbus && udc->pdata->vbus->poll)
- if (udc->pdata->vbus->poll() == VBUS_HIGH) {
+ mv_phy = container_of(udc->phy, struct mv_usb2_phy, phy);
+ if ((udc->extern_attr & MV_USB_HAS_VBUS_DETECTION) &&
+ mv_usb2_has_extern_call(mv_phy, vbus, get_vbus)) {
+ if (mv_usb2_extern_call(mv_phy, vbus, get_vbus, &vbus))
+ return -EAGAIN;
+
+ if (vbus == VBUS_HIGH) {
dev_info(&udc->dev->dev, "USB cable is connected!\n");
return -EAGAIN;
}
+ }
/*
* only cable is unplugged, udc can suspend.
--
1.7.4.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* [V8 PATCH 13/16] usb: ehci: ehci-mv: add extern chip support
2013-02-21 4:07 [V8 PATCH 00/16] mv-usb phy enhancement patches Chao Xie
` (11 preceding siblings ...)
2013-02-21 4:07 ` [V8 PATCH 12/16] usb: gadget: mv_udc: add extern " Chao Xie
@ 2013-02-21 4:07 ` Chao Xie
2013-02-21 4:07 ` [V8 PATCH 14/16] usb: otg: mv_otg: " Chao Xie
` (3 subsequent siblings)
16 siblings, 0 replies; 34+ messages in thread
From: Chao Xie @ 2013-02-21 4:07 UTC (permalink / raw)
To: linux-arm-kernel
It does the similar things as what we do for udc driver.
Signed-off-by: Chao Xie <chao.xie@marvell.com>
Acked-by: Alan Stern <stern@rowland.harvard.edu>
---
drivers/usb/host/ehci-mv.c | 16 ++++++++++------
1 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/host/ehci-mv.c b/drivers/usb/host/ehci-mv.c
index 3e89bd4..ae5d8dc 100644
--- a/drivers/usb/host/ehci-mv.c
+++ b/drivers/usb/host/ehci-mv.c
@@ -138,6 +138,7 @@ static int mv_ehci_probe(struct platform_device *pdev)
struct ehci_hcd *ehci;
struct ehci_hcd_mv *ehci_mv;
struct resource *r;
+ struct mv_usb2_phy *mv_phy;
int clk_i, retval = -ENODEV;
u32 offset;
size_t size;
@@ -199,6 +200,7 @@ static int mv_ehci_probe(struct platform_device *pdev)
retval = PTR_ERR(ehci_mv->phy);
goto err_clear_drvdata;
}
+ mv_phy = container_of(ehci_mv->phy, struct mv_usb2_phy, phy);
retval = mv_ehci_enable(ehci_mv);
if (retval) {
@@ -251,8 +253,8 @@ static int mv_ehci_probe(struct platform_device *pdev)
goto err_disable_clk;
#endif
} else {
- if (pdata->set_vbus)
- pdata->set_vbus(1);
+ if (mv_usb2_has_extern_call(mv_phy, vbus, set_vbus))
+ mv_usb2_extern_call(mv_phy, vbus, set_vbus, 1);
retval = usb_add_hcd(hcd, hcd->irq, IRQF_SHARED);
if (retval) {
@@ -270,8 +272,8 @@ static int mv_ehci_probe(struct platform_device *pdev)
return 0;
err_set_vbus:
- if (pdata->set_vbus)
- pdata->set_vbus(0);
+ if (mv_usb2_has_extern_call(mv_phy, vbus, set_vbus))
+ mv_usb2_extern_call(mv_phy, vbus, set_vbus, 0);
err_disable_clk:
mv_ehci_disable(ehci_mv);
err_clear_drvdata:
@@ -286,6 +288,7 @@ static int mv_ehci_remove(struct platform_device *pdev)
{
struct ehci_hcd_mv *ehci_mv = platform_get_drvdata(pdev);
struct usb_hcd *hcd = ehci_mv->hcd;
+ struct mv_usb2_phy *mv_phy;
if (hcd->rh_registered)
usb_remove_hcd(hcd);
@@ -293,9 +296,10 @@ static int mv_ehci_remove(struct platform_device *pdev)
if (!IS_ERR_OR_NULL(ehci_mv->otg))
otg_set_host(ehci_mv->otg->otg, NULL);
+ mv_phy = container_of(ehci_mv->phy, struct mv_usb2_phy, phy);
if (ehci_mv->mode == MV_USB_MODE_HOST) {
- if (ehci_mv->pdata->set_vbus)
- ehci_mv->pdata->set_vbus(0);
+ if (mv_usb2_has_extern_call(mv_phy, vbus, set_vbus))
+ mv_usb2_extern_call(mv_phy, vbus, set_vbus, 1);
mv_ehci_disable(ehci_mv);
}
--
1.7.4.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* [V8 PATCH 14/16] usb: otg: mv_otg: add extern chip support
2013-02-21 4:07 [V8 PATCH 00/16] mv-usb phy enhancement patches Chao Xie
` (12 preceding siblings ...)
2013-02-21 4:07 ` [V8 PATCH 13/16] usb: ehci: ehci-mv: " Chao Xie
@ 2013-02-21 4:07 ` Chao Xie
2013-02-21 4:07 ` [V8 PATCH 15/16] arm: mmp: add extern chip support for brownstone Chao Xie
` (2 subsequent siblings)
16 siblings, 0 replies; 34+ messages in thread
From: Chao Xie @ 2013-02-21 4:07 UTC (permalink / raw)
To: linux-arm-kernel
It does the similar things as what we do for udc driver.
Signed-off-by: Chao Xie <chao.xie@marvell.com>
---
drivers/usb/otg/mv_otg.c | 72 +++++++++++++++++++++++----------------------
drivers/usb/otg/mv_otg.h | 3 ++
2 files changed, 40 insertions(+), 35 deletions(-)
diff --git a/drivers/usb/otg/mv_otg.c b/drivers/usb/otg/mv_otg.c
index 7282b0d..9b3789a 100644
--- a/drivers/usb/otg/mv_otg.c
+++ b/drivers/usb/otg/mv_otg.c
@@ -59,10 +59,13 @@ static char *state_string[] = {
static int mv_otg_set_vbus(struct usb_otg *otg, bool on)
{
struct mv_otg *mvotg = container_of(otg->phy, struct mv_otg, phy);
- if (mvotg->pdata->set_vbus == NULL)
+ struct mv_usb2_phy *mv_phy = container_of(mvotg->outer_phy,
+ struct mv_usb2_phy, phy);
+
+ if (!mv_usb2_has_extern_call(mv_phy, vbus, set_vbus))
return -ENODEV;
- return mvotg->pdata->set_vbus(on);
+ return mv_usb2_extern_call(mv_phy, vbus, set_vbus, on);
}
static int mv_otg_set_host(struct usb_otg *otg,
@@ -184,14 +187,14 @@ static void mv_otg_init_irq(struct mv_otg *mvotg)
mvotg->irq_status = OTGSC_INTSTS_A_SESSION_VALID
| OTGSC_INTSTS_A_VBUS_VALID;
- if (mvotg->pdata->vbus == NULL) {
+ if (!(mvotg->extern_attr & MV_USB_HAS_VBUS_DETECTION)) {
mvotg->irq_en |= OTGSC_INTR_B_SESSION_VALID
| OTGSC_INTR_B_SESSION_END;
mvotg->irq_status |= OTGSC_INTSTS_B_SESSION_VALID
| OTGSC_INTSTS_B_SESSION_END;
}
- if (mvotg->pdata->id == NULL) {
+ if (!(mvotg->extern_attr & MV_USB_HAS_IDPIN_DETECTION)) {
mvotg->irq_en |= OTGSC_INTR_USB_ID;
mvotg->irq_status |= OTGSC_INTSTS_USB_ID;
}
@@ -303,11 +306,16 @@ static void mv_otg_update_inputs(struct mv_otg *mvotg)
{
struct mv_otg_ctrl *otg_ctrl = &mvotg->otg_ctrl;
u32 otgsc;
+ unsigned int vbus, idpin;
+ struct mv_usb2_phy *mv_phy = container_of(mvotg->outer_phy,
+ struct mv_usb2_phy, phy);
otgsc = readl(&mvotg->op_regs->otgsc);
- if (mvotg->pdata->vbus) {
- if (mvotg->pdata->vbus->poll() == VBUS_HIGH) {
+ if (mvotg->extern_attr & MV_USB_HAS_VBUS_DETECTION) {
+ if (mv_usb2_extern_call(mv_phy, vbus, get_vbus, &vbus))
+ return;
+ if (vbus == VBUS_HIGH) {
otg_ctrl->b_sess_vld = 1;
otg_ctrl->b_sess_end = 0;
} else {
@@ -319,8 +327,11 @@ static void mv_otg_update_inputs(struct mv_otg *mvotg)
otg_ctrl->b_sess_end = !!(otgsc & OTGSC_STS_B_SESSION_END);
}
- if (mvotg->pdata->id)
- otg_ctrl->id = !!mvotg->pdata->id->poll();
+ if (mvotg->extern_attr & MV_USB_HAS_IDPIN_DETECTION) {
+ if (mv_usb2_extern_call(mv_phy, idpin, get_idpin, &idpin))
+ return;
+ otg_ctrl->id = !!idpin;
+ }
else
otg_ctrl->id = !!(otgsc & OTGSC_STS_USB_ID);
@@ -502,7 +513,7 @@ static irqreturn_t mv_otg_irq(int irq, void *dev)
* if we have vbus, then the vbus detection for B-device
* will be done by mv_otg_inputs_irq().
*/
- if (mvotg->pdata->vbus)
+ if (mvotg->extern_attr & MV_USB_HAS_VBUS_DETECTION)
if ((otgsc & OTGSC_STS_USB_ID) &&
!(otgsc & OTGSC_INTSTS_USB_ID))
return IRQ_NONE;
@@ -515,9 +526,10 @@ static irqreturn_t mv_otg_irq(int irq, void *dev)
return IRQ_HANDLED;
}
-static irqreturn_t mv_otg_inputs_irq(int irq, void *dev)
+static int mv_otg_notifier_call(struct notifier_block *nb,
+ unsigned long val, void *v)
{
- struct mv_otg *mvotg = dev;
+ struct mv_otg *mvotg = container_of(nb, struct mv_otg, notifier);
/* The clock may disabled at this time */
if (!mvotg->active) {
@@ -527,7 +539,7 @@ static irqreturn_t mv_otg_inputs_irq(int irq, void *dev)
mv_otg_run_state_machine(mvotg, 0);
- return IRQ_HANDLED;
+ return 0;
}
static ssize_t
@@ -660,9 +672,15 @@ static struct attribute_group inputs_attr_group = {
int mv_otg_remove(struct platform_device *pdev)
{
struct mv_otg *mvotg = platform_get_drvdata(pdev);
+ struct mv_usb2_phy *mv_phy = container_of(mvotg->outer_phy,
+ struct mv_usb2_phy, phy);
sysfs_remove_group(&mvotg->pdev->dev.kobj, &inputs_attr_group);
+ if ((mvotg->extern_attr & MV_USB_HAS_VBUS_DETECTION) ||
+ (mvotg->extern_attr & MV_USB_HAS_IDPIN_DETECTION))
+ mv_usb2_unregister_notifier(mv_phy, &mvotg->notifier);
+
if (mvotg->qwork) {
flush_workqueue(mvotg->qwork);
destroy_workqueue(mvotg->qwork);
@@ -682,6 +700,7 @@ static int mv_otg_probe(struct platform_device *pdev)
struct mv_otg *mvotg;
struct usb_otg *otg;
struct resource *r;
+ struct mv_usb2_phy *mv_phy;
int retval = 0, clk_i, i;
size_t size;
@@ -704,6 +723,7 @@ static int mv_otg_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, mvotg);
mvotg->pdev = pdev;
+ mvotg->extern_attr = pdata->extern_attr;
mvotg->pdata = pdata;
mvotg->clknum = pdata->clknum;
@@ -760,6 +780,7 @@ static int mv_otg_probe(struct platform_device *pdev)
retval = PTR_ERR(mvotg->outer_phy);
goto err_destroy_workqueue;
}
+ mv_phy = container_of(mvotg->outer_phy, struct mv_usb2_phy, phy);
/* we will acces controller register, so enable the udc controller */
retval = mv_otg_enable_internal(mvotg);
@@ -772,29 +793,10 @@ static int mv_otg_probe(struct platform_device *pdev)
(struct mv_otg_regs __iomem *) ((unsigned long) mvotg->cap_regs
+ (readl(mvotg->cap_regs) & CAPLENGTH_MASK));
- if (pdata->id) {
- retval = devm_request_threaded_irq(&pdev->dev, pdata->id->irq,
- NULL, mv_otg_inputs_irq,
- IRQF_ONESHOT, "id", mvotg);
- if (retval) {
- dev_info(&pdev->dev,
- "Failed to request irq for ID\n");
- pdata->id = NULL;
- }
- }
-
- if (pdata->vbus) {
- mvotg->clock_gating = 1;
- retval = devm_request_threaded_irq(&pdev->dev, pdata->vbus->irq,
- NULL, mv_otg_inputs_irq,
- IRQF_ONESHOT, "vbus", mvotg);
- if (retval) {
- dev_info(&pdev->dev,
- "Failed to request irq for VBUS, "
- "disable clock gating\n");
- mvotg->clock_gating = 0;
- pdata->vbus = NULL;
- }
+ if ((mvotg->extern_attr & MV_USB_HAS_VBUS_DETECTION) ||
+ (mvotg->extern_attr & MV_USB_HAS_IDPIN_DETECTION)) {
+ mvotg->notifier.notifier_call = mv_otg_notifier_call;
+ mv_usb2_register_notifier(mv_phy, &mvotg->notifier);
}
if (pdata->disable_otg_clock_gating)
diff --git a/drivers/usb/otg/mv_otg.h b/drivers/usb/otg/mv_otg.h
index 65e7fd1..2a9ecae 100644
--- a/drivers/usb/otg/mv_otg.h
+++ b/drivers/usb/otg/mv_otg.h
@@ -149,6 +149,9 @@ struct mv_otg {
u32 irq_status;
u32 irq_en;
+ unsigned int extern_attr;
+ struct notifier_block notifier;
+
struct delayed_work work;
struct workqueue_struct *qwork;
--
1.7.4.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* [V8 PATCH 15/16] arm: mmp: add extern chip support for brownstone
2013-02-21 4:07 [V8 PATCH 00/16] mv-usb phy enhancement patches Chao Xie
` (13 preceding siblings ...)
2013-02-21 4:07 ` [V8 PATCH 14/16] usb: otg: mv_otg: " Chao Xie
@ 2013-02-21 4:07 ` Chao Xie
2013-02-21 4:07 ` [V8 PATCH 16/16] arm: mmp: add extern chip support for ttc_dkb Chao Xie
2013-02-21 8:04 ` [V8 PATCH 00/16] mv-usb phy enhancement patches Greg KH
16 siblings, 0 replies; 34+ messages in thread
From: Chao Xie @ 2013-02-21 4:07 UTC (permalink / raw)
To: linux-arm-kernel
Change the board support for usb as extern chip is supported
in marvell usb PHY driver
Signed-off-by: Chao Xie <chao.xie@marvell.com>
---
arch/arm/mach-mmp/brownstone.c | 2 --
1 files changed, 0 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-mmp/brownstone.c b/arch/arm/mach-mmp/brownstone.c
index f84613a..eb92419 100644
--- a/arch/arm/mach-mmp/brownstone.c
+++ b/arch/arm/mach-mmp/brownstone.c
@@ -214,10 +214,8 @@ static struct mv_usb_phy_platform_data brownstone_usb_phy_pdata = {
static struct mv_usb_platform_data brownstone_usb_pdata = {
.clknum = 1,
.clkname = mmp2_usb_clock_name,
- .vbus = NULL,
.mode = MV_USB_MODE_OTG,
.otg_force_a_bus_req = 1,
- .set_vbus = NULL,
};
#endif
#endif
--
1.7.4.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* [V8 PATCH 16/16] arm: mmp: add extern chip support for ttc_dkb
2013-02-21 4:07 [V8 PATCH 00/16] mv-usb phy enhancement patches Chao Xie
` (14 preceding siblings ...)
2013-02-21 4:07 ` [V8 PATCH 15/16] arm: mmp: add extern chip support for brownstone Chao Xie
@ 2013-02-21 4:07 ` Chao Xie
2013-02-21 8:04 ` [V8 PATCH 00/16] mv-usb phy enhancement patches Greg KH
16 siblings, 0 replies; 34+ messages in thread
From: Chao Xie @ 2013-02-21 4:07 UTC (permalink / raw)
To: linux-arm-kernel
Change the board support for usb as extern chip is supported
in marvell usb PHY driver.
Signed-off-by: Chao Xie <chao.xie@marvell.com>
---
arch/arm/mach-mmp/ttc_dkb.c | 2 --
1 files changed, 0 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-mmp/ttc_dkb.c b/arch/arm/mach-mmp/ttc_dkb.c
index 5622092..0ba89e9 100644
--- a/arch/arm/mach-mmp/ttc_dkb.c
+++ b/arch/arm/mach-mmp/ttc_dkb.c
@@ -174,10 +174,8 @@ static struct mv_usb_phy_platform_data ttc_usb_phy_pdata = {
static struct mv_usb_platform_data ttc_usb_pdata = {
.clknum = 1,
.clkname = pxa910_usb_clock_name,
- .vbus = NULL,
.mode = MV_USB_MODE_OTG,
.otg_force_a_bus_req = 1,
- .set_vbus = NULL,
};
#endif
#endif
--
1.7.4.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* [V8 PATCH 00/16] mv-usb phy enhancement patches
2013-02-21 4:07 [V8 PATCH 00/16] mv-usb phy enhancement patches Chao Xie
` (15 preceding siblings ...)
2013-02-21 4:07 ` [V8 PATCH 16/16] arm: mmp: add extern chip support for ttc_dkb Chao Xie
@ 2013-02-21 8:04 ` Greg KH
16 siblings, 0 replies; 34+ messages in thread
From: Greg KH @ 2013-02-21 8:04 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Feb 20, 2013 at 11:07:10PM -0500, Chao Xie wrote:
> The patches are divied into 2 parts
<snip>
You do realize that we are currently in the middle of the merge window,
and don't have any time at all to look at these patches until 2 weeks
from now, at the earliest?
greg k-h
^ permalink raw reply [flat|nested] 34+ messages in thread