This series adds USB PHYs and EHCI host drivers for the Marvell PXA1928
SOC.
The OTG block is based on ChipIdea and works with "chipidea,usb2"
compatible driver as is just by adding the PHY driver. Yay!
Rob
Rob Herring (5):
dt-bindings: Add Marvell PXA1928 USB and HSIC PHY bindings
dt-bindings: Add Marvell PXA1928 USB EHCI controller binding
phy: Add Marvell USB 2.0 OTG 28nm PHY
phy: add Marvell HSIC 28nm PHY
usb: add pxa1928 ehci support
.../devicetree/bindings/phy/pxa1928-usb-phy.txt | 18 ++
.../devicetree/bindings/usb/ehci-pxa1928.txt | 19 ++
drivers/phy/Kconfig | 20 ++
drivers/phy/Makefile | 2 +
drivers/phy/phy-mv-hsic.c | 208 +++++++++++++
drivers/phy/phy-mv-usb2.c | 329 +++++++++++++++++++++
drivers/usb/host/Kconfig | 15 +-
drivers/usb/host/Makefile | 1 +
drivers/usb/host/ehci-mv-of.c | 243 +++++++++++++++
9 files changed, 854 insertions(+), 1 deletion(-)
create mode 100644 Documentation/devicetree/bindings/phy/pxa1928-usb-phy.txt
create mode 100644 Documentation/devicetree/bindings/usb/ehci-pxa1928.txt
create mode 100644 drivers/phy/phy-mv-hsic.c
create mode 100644 drivers/phy/phy-mv-usb2.c
create mode 100644 drivers/usb/host/ehci-mv-of.c
--
2.1.0
Add PHY binding for Marvell PXA1928 SOC's USB and HSIC PHYs.
Signed-off-by: Rob Herring <[email protected]>
Cc: Pawel Moll <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: Ian Campbell <[email protected]>
Cc: Kumar Gala <[email protected]>
Cc: [email protected]
---
.../devicetree/bindings/phy/pxa1928-usb-phy.txt | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
create mode 100644 Documentation/devicetree/bindings/phy/pxa1928-usb-phy.txt
diff --git a/Documentation/devicetree/bindings/phy/pxa1928-usb-phy.txt b/Documentation/devicetree/bindings/phy/pxa1928-usb-phy.txt
new file mode 100644
index 0000000..660a13c
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/pxa1928-usb-phy.txt
@@ -0,0 +1,18 @@
+* Marvell PXA1928 USB and HSIC PHYs
+
+Required properties:
+- compatible: "marvell,pxa1928-usb-phy" or "marvell,pxa1928-hsic-phy"
+- reg: base address and length of the registers
+- clocks - A single clock. From common clock binding.
+- #phys-cells: should be 0. From commmon phy binding.
+- resets: reference to the reset controller
+
+Example:
+
+ usbphy: phy@7000 {
+ compatible = "marvell,pxa1928-usb-phy";
+ reg = <0x7000 0xe0>;
+ clocks = <&apmu_clocks PXA1928_CLK_USB>;
+ #phy-cells = <0>;
+ };
+
--
2.1.0
Add binding for Marvell PXA1928 SOC's USB EHCI host controller.
Signed-off-by: Rob Herring <[email protected]>
Cc: Pawel Moll <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: Ian Campbell <[email protected]>
Cc: Kumar Gala <[email protected]>
Cc: [email protected]
---
.../devicetree/bindings/usb/ehci-pxa1928.txt | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
create mode 100644 Documentation/devicetree/bindings/usb/ehci-pxa1928.txt
diff --git a/Documentation/devicetree/bindings/usb/ehci-pxa1928.txt b/Documentation/devicetree/bindings/usb/ehci-pxa1928.txt
new file mode 100644
index 0000000..25881be
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/ehci-pxa1928.txt
@@ -0,0 +1,19 @@
+* EHCI controller, Marvell PXA1928
+
+Required properties:
+- compatible: must be "marvell,pxa1928-ehci"
+- reg: physical base address of the controller and length of memory mapped
+ region.
+- interrupts: The EHCI interrupt
+- clocks: reference to the clock
+- phys: phandle reference to the USB PHY
+
+Example:
+
+ hsic: usb@601100 {
+ compatible = "marvell,pxa1928-ehci";
+ reg = <0x601100 0x200>;
+ interrupts = <0 22 0x4>;
+ clocks = <&apmu_clocks PXA1928_CLK_HSIC>;
+ phys = <&hsicphy>;
+ };
--
2.1.0
Add driver for USB PHY found in Marvell PXA1928 SOC.
Signed-off-by: Rob Herring <[email protected]>
Cc: Kishon Vijay Abraham I <[email protected]>
---
drivers/phy/Kconfig | 10 ++
drivers/phy/Makefile | 1 +
drivers/phy/phy-mv-usb2.c | 329 ++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 340 insertions(+)
create mode 100644 drivers/phy/phy-mv-usb2.c
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index a53bd5b..ef7634f 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -52,6 +52,16 @@ config PHY_EXYNOS_MIPI_VIDEO
Support for MIPI CSI-2 and MIPI DSI DPHY found on Samsung S5P
and EXYNOS SoCs.
+config PHY_MV_USB2
+ tristate "Marvell USB 2.0 28nm PHY Driver"
+ select GENERIC_PHY
+ help
+ Enable this to support Marvell USB 2.0 PHY driver for Marvell
+ SoC. This driver will do the PHY initialization and shutdown.
+ The PHY driver will be used by Marvell udc/ehci/otg driver.
+
+ To compile this driver as a module, choose M here.
+
config PHY_MVEBU_SATA
def_bool y
depends on ARCH_DOVE || MACH_DOVE || MACH_KIRKWOOD
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index f126251..768e55a 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_ARMADA375_USBCLUSTER_PHY) += phy-armada375-usb2.o
obj-$(CONFIG_BCM_KONA_USB2_PHY) += phy-bcm-kona-usb2.o
obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO) += phy-exynos-dp-video.o
obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO) += phy-exynos-mipi-video.o
+obj-$(CONFIG_PHY_MV_USB2) += phy-mv-usb2.o
obj-$(CONFIG_PHY_MVEBU_SATA) += phy-mvebu-sata.o
obj-$(CONFIG_PHY_MIPHY28LP) += phy-miphy28lp.o
obj-$(CONFIG_PHY_MIPHY365X) += phy-miphy365x.o
diff --git a/drivers/phy/phy-mv-usb2.c b/drivers/phy/phy-mv-usb2.c
new file mode 100644
index 0000000..c48d111
--- /dev/null
+++ b/drivers/phy/phy-mv-usb2.c
@@ -0,0 +1,329 @@
+/*
+ * Copyright (C) 2015 Linaro, Ltd.
+ * Rob Herring <[email protected]>
+ *
+ * Based on vendor driver:
+ * Copyright (C) 2013 Marvell Inc.
+ * Author: Chao Xie <[email protected]>
+ *
+ * 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/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/module.h>
+#include <linux/platform_device.h>
+#include <linux/phy/phy.h>
+
+/* USB PXA1928 PHY mapping */
+#define PHY_28NM_PLL_REG0 0x0
+#define PHY_28NM_PLL_REG1 0x4
+#define PHY_28NM_CAL_REG 0x8
+#define PHY_28NM_TX_REG0 0x0c
+#define PHY_28NM_TX_REG1 0x10
+#define PHY_28NM_RX_REG0 0x14
+#define PHY_28NM_RX_REG1 0x18
+#define PHY_28NM_DIG_REG0 0x1c
+#define PHY_28NM_DIG_REG1 0x20
+#define PHY_28NM_TEST_REG0 0x24
+#define PHY_28NM_TEST_REG1 0x28
+#define PHY_28NM_MOC_REG 0x2c
+#define PHY_28NM_PHY_RESERVE 0x30
+#define PHY_28NM_OTG_REG 0x34
+#define PHY_28NM_CHRG_DET 0x38
+#define PHY_28NM_CTRL_REG0 0xc4
+#define PHY_28NM_CTRL_REG1 0xc8
+#define PHY_28NM_CTRL_REG2 0xd4
+#define PHY_28NM_CTRL_REG3 0xdc
+
+/* PHY_28NM_PLL_REG0 */
+#define PHY_28NM_PLL_READY BIT(31)
+
+#define PHY_28NM_PLL_SELLPFR_SHIFT 28
+#define PHY_28NM_PLL_SELLPFR_MASK (0x3 << 28)
+
+#define PHY_28NM_PLL_FBDIV_SHIFT 16
+#define PHY_28NM_PLL_FBDIV_MASK (0x1ff << 16)
+
+#define PHY_28NM_PLL_ICP_SHIFT 8
+#define PHY_28NM_PLL_ICP_MASK (0x7 << 8)
+
+#define PHY_28NM_PLL_REFDIV_SHIFT 0
+#define PHY_28NM_PLL_REFDIV_MASK 0x7f
+
+/* PHY_28NM_PLL_REG1 */
+#define PHY_28NM_PLL_PU_BY_REG BIT(1)
+
+#define PHY_28NM_PLL_PU_PLL BIT(0)
+
+/* PHY_28NM_CAL_REG */
+#define PHY_28NM_PLL_PLLCAL_DONE BIT(31)
+
+#define PHY_28NM_PLL_IMPCAL_DONE BIT(23)
+
+#define PHY_28NM_PLL_KVCO_SHIFT 16
+#define PHY_28NM_PLL_KVCO_MASK (0x7 << 16)
+
+#define PHY_28NM_PLL_CAL12_SHIFT 20
+#define PHY_28NM_PLL_CAL12_MASK (0x3 << 20)
+
+#define PHY_28NM_IMPCAL_VTH_SHIFT 8
+#define PHY_28NM_IMPCAL_VTH_MASK (0x7 << 8)
+
+#define PHY_28NM_PLLCAL_START_SHIFT 22
+#define PHY_28NM_IMPCAL_START_SHIFT 13
+
+/* PHY_28NM_TX_REG0 */
+#define PHY_28NM_TX_PU_BY_REG BIT(25)
+
+#define PHY_28NM_TX_PU_ANA BIT(24)
+
+#define PHY_28NM_TX_AMP_SHIFT 20
+#define PHY_28NM_TX_AMP_MASK (0x7 << 20)
+
+/* PHY_28NM_RX_REG0 */
+#define PHY_28NM_RX_SQ_THRESH_SHIFT 0
+#define PHY_28NM_RX_SQ_THRESH_MASK (0xf << 0)
+
+/* PHY_28NM_RX_REG1 */
+#define PHY_28NM_RX_SQCAL_DONE BIT(31)
+
+/* PHY_28NM_DIG_REG0 */
+#define PHY_28NM_DIG_BITSTAFFING_ERR BIT(31)
+#define PHY_28NM_DIG_SYNC_ERR BIT(30)
+
+#define PHY_28NM_DIG_SQ_FILT_SHIFT 16
+#define PHY_28NM_DIG_SQ_FILT_MASK (0x7 << 16)
+
+#define PHY_28NM_DIG_SQ_BLK_SHIFT 12
+#define PHY_28NM_DIG_SQ_BLK_MASK (0x7 << 12)
+
+#define PHY_28NM_DIG_SYNC_NUM_SHIFT 0
+#define PHY_28NM_DIG_SYNC_NUM_MASK (0x3 << 0)
+
+#define PHY_28NM_PLL_LOCK_BYPASS BIT(7)
+
+/* PHY_28NM_OTG_REG */
+#define PHY_28NM_OTG_CONTROL_BY_PIN BIT(5)
+#define PHY_28NM_OTG_PU_OTG BIT(4)
+
+#define PHY_28NM_CHGDTC_ENABLE_SWITCH_DM_SHIFT_28 13
+#define PHY_28NM_CHGDTC_ENABLE_SWITCH_DP_SHIFT_28 12
+#define PHY_28NM_CHGDTC_VSRC_CHARGE_SHIFT_28 10
+#define PHY_28NM_CHGDTC_VDAT_CHARGE_SHIFT_28 8
+#define PHY_28NM_CHGDTC_CDP_DM_AUTO_SWITCH_SHIFT_28 7
+#define PHY_28NM_CHGDTC_DP_DM_SWAP_SHIFT_28 6
+#define PHY_28NM_CHGDTC_PU_CHRG_DTC_SHIFT_28 5
+#define PHY_28NM_CHGDTC_PD_EN_SHIFT_28 4
+#define PHY_28NM_CHGDTC_DCP_EN_SHIFT_28 3
+#define PHY_28NM_CHGDTC_CDP_EN_SHIFT_28 2
+#define PHY_28NM_CHGDTC_TESTMON_CHRGDTC_SHIFT_28 0
+
+#define PHY_28NM_CTRL1_CHRG_DTC_OUT_SHIFT_28 4
+#define PHY_28NM_CTRL1_VBUSDTC_OUT_SHIFT_28 2
+
+#define PHY_28NM_CTRL3_OVERWRITE BIT(0)
+#define PHY_28NM_CTRL3_VBUS_VALID BIT(4)
+#define PHY_28NM_CTRL3_AVALID BIT(5)
+#define PHY_28NM_CTRL3_BVALID BIT(6)
+
+struct mv_usb2_phy {
+ struct phy *phy;
+ struct platform_device *pdev;
+ void __iomem *base;
+ struct clk *clk;
+};
+
+static bool wait_for_reg(void __iomem *reg, u32 mask, unsigned long timeout)
+{
+ timeout += jiffies;
+ while (time_is_after_eq_jiffies(timeout)) {
+ if ((readl(reg) & mask) == mask)
+ return true;
+ msleep(1);
+ }
+ return false;
+}
+
+static int mv_usb2_phy_28nm_init(struct phy *phy)
+{
+ struct mv_usb2_phy *mv_phy = phy_get_drvdata(phy);
+ struct platform_device *pdev = mv_phy->pdev;
+ void __iomem *base = mv_phy->base;
+ u32 reg;
+
+ clk_prepare_enable(mv_phy->clk);
+
+ /* PHY_28NM_PLL_REG0 */
+ reg = readl(base + PHY_28NM_PLL_REG0) &
+ ~(PHY_28NM_PLL_SELLPFR_MASK | PHY_28NM_PLL_FBDIV_MASK
+ | PHY_28NM_PLL_ICP_MASK | PHY_28NM_PLL_REFDIV_MASK);
+ writel(reg | (0x1 << PHY_28NM_PLL_SELLPFR_SHIFT
+ | 0xf0 << PHY_28NM_PLL_FBDIV_SHIFT
+ | 0x3 << PHY_28NM_PLL_ICP_SHIFT
+ | 0xd << PHY_28NM_PLL_REFDIV_SHIFT),
+ base + PHY_28NM_PLL_REG0);
+
+ /* PHY_28NM_PLL_REG1 */
+ reg = readl(base + PHY_28NM_PLL_REG1);
+ writel(reg | PHY_28NM_PLL_PU_PLL | PHY_28NM_PLL_PU_BY_REG,
+ base + PHY_28NM_PLL_REG1);
+
+ /* PHY_28NM_TX_REG0 */
+ reg = readl(base + PHY_28NM_TX_REG0) & ~PHY_28NM_TX_AMP_MASK;
+ writel(reg | PHY_28NM_TX_PU_BY_REG | 0x3 << PHY_28NM_TX_AMP_SHIFT |
+ PHY_28NM_TX_PU_ANA,
+ base + PHY_28NM_TX_REG0);
+
+ /* PHY_28NM_RX_REG0 */
+ reg = readl(base + PHY_28NM_RX_REG0) & ~PHY_28NM_RX_SQ_THRESH_MASK;
+ writel(reg | 0xa << PHY_28NM_RX_SQ_THRESH_SHIFT,
+ base + PHY_28NM_RX_REG0);
+
+ /* PHY_28NM_DIG_REG0 */
+ reg = readl(base + PHY_28NM_DIG_REG0) &
+ ~(PHY_28NM_DIG_BITSTAFFING_ERR | PHY_28NM_DIG_SYNC_ERR |
+ PHY_28NM_DIG_SQ_FILT_MASK | PHY_28NM_DIG_SQ_BLK_MASK |
+ PHY_28NM_DIG_SYNC_NUM_MASK);
+ writel(reg | (0x1 << PHY_28NM_DIG_SYNC_NUM_SHIFT |
+ PHY_28NM_PLL_LOCK_BYPASS),
+ base + PHY_28NM_DIG_REG0);
+
+ /* PHY_28NM_OTG_REG */
+ reg = readl(base + PHY_28NM_OTG_REG) | PHY_28NM_OTG_PU_OTG;
+ writel(reg & ~PHY_28NM_OTG_CONTROL_BY_PIN, base + PHY_28NM_OTG_REG);
+
+ /* Calibration Timing
+ * ____________________________
+ * CAL START ___|
+ * ____________________
+ * CAL_DONE ___________|
+ * | 400us |
+ */
+
+ /* Make sure PHY Calibration is ready */
+ if (!wait_for_reg(base + PHY_28NM_CAL_REG,
+ PHY_28NM_PLL_PLLCAL_DONE | PHY_28NM_PLL_IMPCAL_DONE,
+ HZ / 10)) {
+ dev_warn(&pdev->dev, "USB PHY PLL calibrate not done after 100mS.");
+ return -ETIMEDOUT;
+ }
+ if (!wait_for_reg(base + PHY_28NM_RX_REG1,
+ PHY_28NM_RX_SQCAL_DONE, HZ / 10)) {
+ dev_warn(&pdev->dev, "USB PHY RX SQ calibrate not done after 100mS.");
+ return -ETIMEDOUT;
+ }
+
+ /* Make sure PHY PLL is ready */
+ if (!wait_for_reg(base + PHY_28NM_PLL_REG0,
+ PHY_28NM_PLL_READY, HZ / 10)) {
+ dev_warn(&pdev->dev, "PLL_READY not set after 100mS.");
+ return -ETIMEDOUT;
+ }
+
+ writel(readl(base + PHY_28NM_CTRL_REG3) |
+ (PHY_28NM_CTRL3_OVERWRITE | PHY_28NM_CTRL3_VBUS_VALID
+ | PHY_28NM_CTRL3_AVALID | PHY_28NM_CTRL3_BVALID),
+ base + PHY_28NM_CTRL_REG3);
+
+ return 0;
+}
+
+static int mv_usb2_phy_28nm_power_off(struct phy *phy)
+{
+ struct mv_usb2_phy *mv_phy = phy_get_drvdata(phy);
+ void __iomem *base = mv_phy->base;
+ unsigned int val;
+
+ writel(readl(base + PHY_28NM_CTRL_REG3) |
+ ~(PHY_28NM_CTRL3_OVERWRITE | PHY_28NM_CTRL3_VBUS_VALID
+ | PHY_28NM_CTRL3_AVALID | PHY_28NM_CTRL3_BVALID),
+ base + PHY_28NM_CTRL_REG3);
+
+ val = readw(base + PHY_28NM_PLL_REG1);
+ val &= ~PHY_28NM_PLL_PU_PLL;
+ writew(val, base + PHY_28NM_PLL_REG1);
+
+ /* power down PHY Analog part */
+ val = readw(base + PHY_28NM_TX_REG0);
+ val &= ~PHY_28NM_TX_PU_ANA;
+ writew(val, base + PHY_28NM_TX_REG0);
+
+ /* power down PHY OTG part */
+ val = readw(base + PHY_28NM_OTG_REG);
+ val &= ~PHY_28NM_OTG_PU_OTG;
+ writew(val, base + PHY_28NM_OTG_REG);
+
+ clk_disable_unprepare(mv_phy->clk);
+ return 0;
+}
+
+static const struct phy_ops usb_ops = {
+ .init = mv_usb2_phy_28nm_init,
+ .power_off = mv_usb2_phy_28nm_power_off,
+};
+
+static int mv_usb2_phy_probe(struct platform_device *pdev)
+{
+ struct phy_provider *phy_provider;
+ struct mv_usb2_phy *mv_phy;
+ struct resource *r;
+
+ mv_phy = devm_kzalloc(&pdev->dev, sizeof(*mv_phy), GFP_KERNEL);
+ if (!mv_phy)
+ return -ENOMEM;
+
+ mv_phy->pdev = pdev;
+
+ mv_phy->clk = devm_clk_get(&pdev->dev, NULL);
+ if (IS_ERR(mv_phy->clk)) {
+ dev_err(&pdev->dev, "failed to get clock.\n");
+ return PTR_ERR(mv_phy->clk);
+ }
+
+ r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ mv_phy->base = devm_ioremap_resource(&pdev->dev, r);
+ if (IS_ERR(mv_phy->base))
+ return PTR_ERR(mv_phy->base);
+
+ mv_phy->phy = devm_phy_create(&pdev->dev, pdev->dev.of_node, &usb_ops);
+ if (IS_ERR(mv_phy->phy))
+ return PTR_ERR(mv_phy->phy);
+
+ phy_set_drvdata(mv_phy->phy, mv_phy);
+
+ phy_provider = devm_of_phy_provider_register(&pdev->dev, of_phy_simple_xlate);
+ return PTR_ERR_OR_ZERO(phy_provider);
+}
+
+static const struct of_device_id mv_usbphy_dt_match[] = {
+ { .compatible = "marvell,pxa1928-usb-phy", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, mv_usbphy_dt_match);
+
+static struct platform_driver mv_usb2_phy_driver = {
+ .probe = mv_usb2_phy_probe,
+ .driver = {
+ .name = "mv-usb2-phy",
+ .of_match_table = of_match_ptr(mv_usbphy_dt_match),
+ },
+};
+module_platform_driver(mv_usb2_phy_driver);
+
+MODULE_AUTHOR("Rob Herring <[email protected]>");
+MODULE_DESCRIPTION("Marvell USB2 phy driver");
+MODULE_LICENSE("GPL v2");
--
2.1.0
Add PHY driver for the Marvell HSIC 28nm PHY. This PHY is found in PXA1928
SOC.
Signed-off-by: Rob Herring <[email protected]>
Cc: Kishon Vijay Abraham I <[email protected]>
---
drivers/phy/Kconfig | 10 +++
drivers/phy/Makefile | 1 +
drivers/phy/phy-mv-hsic.c | 208 ++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 219 insertions(+)
create mode 100644 drivers/phy/phy-mv-hsic.c
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index ef7634f..cfcae72 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -52,6 +52,16 @@ config PHY_EXYNOS_MIPI_VIDEO
Support for MIPI CSI-2 and MIPI DSI DPHY found on Samsung S5P
and EXYNOS SoCs.
+config PHY_MV_HSIC
+ tristate "Marvell USB HSIC 28nm PHY Driver"
+ select GENERIC_PHY
+ help
+ Enable this to support Marvell USB HSIC PHY driver for Marvell
+ SoC. This driver will do the PHY initialization and shutdown.
+ The PHY driver will be used by Marvell ehci driver.
+
+ To compile this driver as a module, choose M here.
+
config PHY_MV_USB2
tristate "Marvell USB 2.0 28nm PHY Driver"
select GENERIC_PHY
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index 768e55a..472bf0a 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_ARMADA375_USBCLUSTER_PHY) += phy-armada375-usb2.o
obj-$(CONFIG_BCM_KONA_USB2_PHY) += phy-bcm-kona-usb2.o
obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO) += phy-exynos-dp-video.o
obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO) += phy-exynos-mipi-video.o
+obj-$(CONFIG_PHY_MV_HSIC) += phy-mv-hsic.o
obj-$(CONFIG_PHY_MV_USB2) += phy-mv-usb2.o
obj-$(CONFIG_PHY_MVEBU_SATA) += phy-mvebu-sata.o
obj-$(CONFIG_PHY_MIPHY28LP) += phy-miphy28lp.o
diff --git a/drivers/phy/phy-mv-hsic.c b/drivers/phy/phy-mv-hsic.c
new file mode 100644
index 0000000..aa6cccd
--- /dev/null
+++ b/drivers/phy/phy-mv-hsic.c
@@ -0,0 +1,208 @@
+/*
+ * Copyright (C) 2015 Linaro, Ltd.
+ * Rob Herring <[email protected]>
+ *
+ * Based on vendor driver:
+ * Copyright (C) 2013 Marvell Inc.
+ * Author: Chao Xie <[email protected]>
+ *
+ * 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/delay.h>
+#include <linux/slab.h>
+#include <linux/of.h>
+#include <linux/io.h>
+#include <linux/err.h>
+#include <linux/clk.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/phy/phy.h>
+
+#define PHY_28NM_HSIC_CTRL 0x08
+#define PHY_28NM_HSIC_IMPCAL_CAL 0x18
+#define PHY_28NM_HSIC_PLL_CTRL01 0x1c
+#define PHY_28NM_HSIC_PLL_CTRL2 0x20
+#define PHY_28NM_HSIC_INT 0x28
+
+#define PHY_28NM_HSIC_PLL_SELLPFR_SHIFT 26
+#define PHY_28NM_HSIC_PLL_FBDIV_SHIFT 0
+#define PHY_28NM_HSIC_PLL_REFDIV_SHIFT 9
+
+#define PHY_28NM_HSIC_S2H_PU_PLL BIT(10)
+#define PHY_28NM_HSIC_H2S_PLL_LOCK BIT(15)
+#define PHY_28NM_HSIC_S2H_HSIC_EN BIT(7)
+#define S2H_DRV_SE0_4RESUME BIT(14)
+#define PHY_28NM_HSIC_H2S_IMPCAL_DONE BIT(27)
+
+#define PHY_28NM_HSIC_CONNECT_INT BIT(1)
+#define PHY_28NM_HSIC_HS_READY_INT BIT(2)
+
+struct mv_hsic_phy {
+ struct phy *phy;
+ struct platform_device *pdev;
+ void __iomem *base;
+ struct clk *clk;
+};
+
+static bool wait_for_reg(void __iomem *reg, u32 mask, unsigned long timeout)
+{
+ timeout += jiffies;
+ while (time_is_after_eq_jiffies(timeout)) {
+ if ((readl(reg) & mask) == mask)
+ return true;
+ msleep(1);
+ }
+ return false;
+}
+
+static int mv_hsic_phy_init(struct phy *phy)
+{
+ struct mv_hsic_phy *mv_phy = phy_get_drvdata(phy);
+ struct platform_device *pdev = mv_phy->pdev;
+ void __iomem *base = mv_phy->base;
+
+ clk_prepare_enable(mv_phy->clk);
+
+ /* Set reference clock */
+ writel(0x1 << PHY_28NM_HSIC_PLL_SELLPFR_SHIFT |
+ 0xf0 << PHY_28NM_HSIC_PLL_FBDIV_SHIFT |
+ 0xd << PHY_28NM_HSIC_PLL_REFDIV_SHIFT,
+ base + PHY_28NM_HSIC_PLL_CTRL01);
+
+ /* Turn on PLL */
+ writel(readl(base + PHY_28NM_HSIC_PLL_CTRL2) |
+ PHY_28NM_HSIC_S2H_PU_PLL,
+ base + PHY_28NM_HSIC_PLL_CTRL2);
+
+ /* Make sure PHY PLL is locked */
+ if (!wait_for_reg(base + PHY_28NM_HSIC_PLL_CTRL2,
+ PHY_28NM_HSIC_H2S_PLL_LOCK, HZ / 10)) {
+ dev_err(&pdev->dev, "HSIC PHY PLL not locked after 100mS.");
+ return -ETIMEDOUT;
+ }
+
+ /* Avoid SE0 state when resume for some device will take it as reset */
+ writel(readl(base + PHY_28NM_HSIC_CTRL) & ~S2H_DRV_SE0_4RESUME,
+ base + PHY_28NM_HSIC_CTRL);
+
+ /* Enable HSIC PHY */
+ writel(readl(base + PHY_28NM_HSIC_CTRL) | PHY_28NM_HSIC_S2H_HSIC_EN,
+ base + PHY_28NM_HSIC_CTRL);
+
+ /* Calibration Timing
+ * ____________________________
+ * CAL START ___|
+ * ____________________
+ * CAL_DONE ___________|
+ * | 400us |
+ */
+
+ /* Make sure PHY Calibration is ready */
+ if (!wait_for_reg(base + PHY_28NM_HSIC_IMPCAL_CAL,
+ PHY_28NM_HSIC_H2S_IMPCAL_DONE, HZ / 10)) {
+ dev_warn(&pdev->dev, "HSIC PHY READY not set after 100mS.");
+ return -ETIMEDOUT;
+ }
+
+ /* Waiting for HSIC connect int*/
+ if (!wait_for_reg(base + PHY_28NM_HSIC_INT,
+ PHY_28NM_HSIC_CONNECT_INT, HZ / 5)) {
+ dev_warn(&pdev->dev, "HSIC wait for connect interrupt timeout.");
+ return -ETIMEDOUT;
+ }
+
+ return 0;
+}
+
+static int mv_hsic_phy_power_on(struct phy *phy)
+{
+ struct mv_hsic_phy *mv_phy = phy_get_drvdata(phy);
+ struct platform_device *pdev = mv_phy->pdev;
+
+ /* check HS ready */
+ if (!wait_for_reg(mv_phy->base + PHY_28NM_HSIC_INT,
+ PHY_28NM_HSIC_HS_READY_INT, HZ / 10)) {
+ dev_err(&pdev->dev, "HSIC HS_READY not set\n");
+ return -ETIMEDOUT;
+ }
+ return 0;
+}
+
+static int mv_hsic_phy_power_off(struct phy *phy)
+{
+ struct mv_hsic_phy *mv_phy = phy_get_drvdata(phy);
+ void __iomem *base = mv_phy->base;
+
+ writel(readl(base + PHY_28NM_HSIC_CTRL) & ~PHY_28NM_HSIC_S2H_HSIC_EN,
+ base + PHY_28NM_HSIC_CTRL);
+
+ clk_disable_unprepare(mv_phy->clk);
+ return 0;
+}
+
+static const struct phy_ops hsic_ops = {
+ .init = mv_hsic_phy_init,
+ .power_on = mv_hsic_phy_power_on,
+ .power_off = mv_hsic_phy_power_off,
+};
+
+static int mv_hsic_phy_probe(struct platform_device *pdev)
+{
+ struct phy_provider *phy_provider;
+ struct mv_hsic_phy *mv_phy;
+ struct resource *r;
+
+ mv_phy = devm_kzalloc(&pdev->dev, sizeof(*mv_phy), GFP_KERNEL);
+ if (!mv_phy)
+ return -ENOMEM;
+
+ mv_phy->pdev = pdev;
+
+ mv_phy->clk = devm_clk_get(&pdev->dev, NULL);
+ if (IS_ERR(mv_phy->clk)) {
+ dev_err(&pdev->dev, "failed to get clock.\n");
+ return PTR_ERR(mv_phy->clk);
+ }
+
+ r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ mv_phy->base = devm_ioremap_resource(&pdev->dev, r);
+ if (IS_ERR(mv_phy->base))
+ return PTR_ERR(mv_phy->base);
+
+ mv_phy->phy = devm_phy_create(&pdev->dev, pdev->dev.of_node, &hsic_ops);
+ if (IS_ERR(mv_phy->phy))
+ return PTR_ERR(mv_phy->phy);
+
+ phy_set_drvdata(mv_phy->phy, mv_phy);
+
+ phy_provider = devm_of_phy_provider_register(&pdev->dev, of_phy_simple_xlate);
+ return PTR_ERR_OR_ZERO(phy_provider);
+}
+
+static const struct of_device_id mv_hsic_phy_dt_match[] = {
+ { .compatible = "marvell,pxa1928-hsic-phy", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, mv_hsic_phy_dt_match);
+
+static struct platform_driver mv_hsic_phy_driver = {
+ .probe = mv_hsic_phy_probe,
+ .driver = {
+ .name = "mv-hsic-phy",
+ .of_match_table = of_match_ptr(mv_hsic_phy_dt_match),
+ },
+};
+module_platform_driver(mv_hsic_phy_driver);
+
+MODULE_AUTHOR("Rob Herring <[email protected]>");
+MODULE_DESCRIPTION("Marvell HSIC phy driver");
+MODULE_LICENSE("GPL v2");
--
2.1.0
Add platform driver for Marvell PXA1928 SOC. This is different from the
mv-ehci driver in that it uses the generic phy framework, uses DT, does
not use platform_data, is host only, and has a specific HSIC PHY and
controller initialization handshake.
Signed-off-by: Rob Herring <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Alan Stern <[email protected]>
Cc: [email protected]
---
drivers/usb/host/Kconfig | 15 ++-
drivers/usb/host/Makefile | 1 +
drivers/usb/host/ehci-mv-of.c | 243 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 258 insertions(+), 1 deletion(-)
create mode 100644 drivers/usb/host/ehci-mv-of.c
diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index 197a6a3..4d8cfbc 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -251,6 +251,19 @@ config USB_EHCI_MV
Dova, Armada 370 and Armada XP. See "Support for Marvell EBU
on-chip EHCI USB controller" for those.
+config USB_EHCI_MV_OF
+ bool "EHCI OF support for Marvell PXA/MMP USB controller"
+ depends on (ARCH_PXA || ARCH_MMP)
+ select USB_EHCI_ROOT_HUB_TT
+ ---help---
+ Enables support for Marvell (including PXA and MMP series) on-chip
+ USB SPH and OTG controller. SPH is a single port host, and it can
+ only be EHCI host. OTG is controller that can switch to host mode.
+ Note that this driver will not work on Marvell's other EHCI
+ controller used by the EBU-type SoCs including Orion, Kirkwood,
+ Dova, Armada 370 and Armada XP. See "Support for Marvell EBU
+ on-chip EHCI USB controller" for those.
+
config USB_W90X900_EHCI
tristate "W90X900(W90P910) EHCI support"
depends on ARCH_W90X900
@@ -663,7 +676,7 @@ config USB_SL811_HCD
help
The SL811HS is a single-port USB controller that supports either
host side or peripheral side roles. Enable this option if your
- board has this chip, and you want to use it as a host controller.
+ board has this chip, and you want to use it as a host controller.
If unsure, say N.
To compile this driver as a module, choose M here: the
diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
index 65b0b6a..5ed93ff 100644
--- a/drivers/usb/host/Makefile
+++ b/drivers/usb/host/Makefile
@@ -32,6 +32,7 @@ obj-$(CONFIG_USB_XHCI_PLATFORM) += xhci-plat-hcd.o
obj-$(CONFIG_USB_EHCI_HCD) += ehci-hcd.o
obj-$(CONFIG_USB_EHCI_PCI) += ehci-pci.o
obj-$(CONFIG_USB_EHCI_HCD_PLATFORM) += ehci-platform.o
+obj-$(CONFIG_USB_EHCI_MV_OF) += ehci-mv-of.o
obj-$(CONFIG_USB_EHCI_MXC) += ehci-mxc.o
obj-$(CONFIG_USB_EHCI_HCD_OMAP) += ehci-omap.o
obj-$(CONFIG_USB_EHCI_HCD_ORION) += ehci-orion.o
diff --git a/drivers/usb/host/ehci-mv-of.c b/drivers/usb/host/ehci-mv-of.c
new file mode 100644
index 0000000..3783299
--- /dev/null
+++ b/drivers/usb/host/ehci-mv-of.c
@@ -0,0 +1,243 @@
+/*
+ * Copyright (C) 2015 Linaro, Ltd.
+ * Rob Herring <[email protected]>
+ *
+ * Based on vendor driver modifications to ehci-mv.c:
+ * Copyright (C) 2011 Marvell International Ltd. All rights reserved.
+ * Author: Chao Xie <[email protected]>
+ * Neil Zhang <[email protected]>
+ *
+ * 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.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/clk.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/phy/phy.h>
+#include <linux/err.h>
+#include <linux/usb.h>
+#include <linux/usb/hcd.h>
+
+#include "ehci.h"
+
+struct ehci_hcd_mv {
+ struct usb_hcd *hcd;
+ struct phy *phy;
+ struct clk *clk;
+};
+
+#define hcd_to_mv_priv(h) ((struct ehci_hcd_mv *)hcd_to_ehci(h)->priv)
+
+static struct hc_driver ehci_mv_hc_driver;
+
+static bool wait_for_reg(void __iomem *reg, u32 mask, unsigned long timeout)
+{
+ timeout += jiffies;
+ while (time_is_after_eq_jiffies(timeout)) {
+ if ((readl(reg) & mask) == mask)
+ return true;
+ msleep(1);
+ }
+ return false;
+}
+
+static int mv_ehci_enable(struct ehci_hcd_mv *ehci_mv)
+{
+ struct ehci_hcd *ehci = hcd_to_ehci(ehci_mv->hcd);
+ struct ehci_regs *ehci_regs = ehci->regs;
+ int retval;
+ u32 status;
+
+ /* enable port power and reserved bit 25 */
+ status = __raw_readl(&ehci_regs->port_status[0]);
+ status |= (PORT_POWER) | (1 << 25);
+ /* Clear bits 30:31 for HSIC to be enabled */
+ status &= ~(0x3 << 30);
+ __raw_writel(status, &ehci_regs->port_status[0]);
+
+ /* test mode: force enable hs */
+ status = __raw_readl(&ehci_regs->port_status[0]);
+ status &= ~(0xf << 16);
+ status |= (0x5 << 16);
+ __raw_writel(status, &ehci_regs->port_status[0]);
+
+ /* disable test mode */
+ status = __raw_readl(&ehci_regs->port_status[0]);
+ status &= ~(0xf << 16);
+ __raw_writel(status, &ehci_regs->port_status[0]);
+
+ retval = phy_power_on(ehci_mv->hcd->phy);
+ if (retval)
+ return retval;
+
+ /* issue port reset */
+ status = __raw_readl(&ehci_regs->port_status[0]);
+ status |= PORT_RESET;
+ status &= ~PORT_PE;
+ __raw_writel(status, &ehci_regs->port_status[0]);
+
+ /* check reset done */
+ if (!wait_for_reg(&ehci_regs->port_status[0], PORT_RESET, HZ / 10)) {
+ pr_err("HSIC port reset not done: port_status 0x%x\n", status);
+ return -ETIMEDOUT;
+ }
+ return 0;
+}
+
+static int mv_ehci_probe(struct platform_device *pdev)
+{
+ struct usb_hcd *hcd;
+ struct ehci_hcd *ehci;
+ struct ehci_hcd_mv *ehci_mv;
+ struct resource *r;
+ int retval = -ENODEV;
+
+ hcd = usb_create_hcd(&ehci_mv_hc_driver, &pdev->dev, "mv ehci");
+ if (!hcd)
+ return -ENOMEM;
+ ehci = hcd_to_ehci(hcd);
+ ehci_mv = hcd_to_mv_priv(hcd);
+ ehci_mv->hcd = hcd;
+
+ platform_set_drvdata(pdev, ehci_mv);
+
+ ehci_mv->clk = devm_clk_get(&pdev->dev, NULL);
+ if (IS_ERR(ehci_mv->clk)) {
+ dev_err(&pdev->dev, "error getting clock\n");
+ retval = PTR_ERR(ehci_mv->clk);
+ goto err_put_hcd;
+ }
+
+ r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ ehci->caps = devm_ioremap_resource(&pdev->dev, r);
+ if (IS_ERR(ehci->caps)) {
+ retval = PTR_ERR(ehci->caps);
+ goto err_put_hcd;
+ }
+
+ hcd->phy = devm_of_phy_get(&pdev->dev, pdev->dev.of_node, NULL);
+ if (IS_ERR_OR_NULL(hcd->phy)) {
+ retval = PTR_ERR(hcd->phy);
+ if (retval != -EPROBE_DEFER && retval != -ENODEV)
+ dev_err(&pdev->dev, "failed to get the phy\n");
+ else
+ return -EPROBE_DEFER;
+ goto err_put_hcd;
+ }
+
+ hcd->rsrc_start = r->start;
+ hcd->rsrc_len = resource_size(r);
+
+ hcd->irq = platform_get_irq(pdev, 0);
+ if (!hcd->irq) {
+ dev_err(&pdev->dev, "Cannot get irq.");
+ retval = -ENODEV;
+ goto err_put_hcd;
+ }
+
+ retval = phy_init(hcd->phy);
+ if (retval)
+ goto err_put_hcd;
+
+ clk_prepare(ehci_mv->clk);
+ clk_enable(ehci_mv->clk);
+
+ retval = usb_add_hcd(hcd, hcd->irq, IRQF_SHARED);
+ if (retval) {
+ dev_err(&pdev->dev,
+ "failed to add hcd with err %d\n", retval);
+ goto err_disable_clk;
+ }
+
+ retval = mv_ehci_enable(ehci_mv);
+ if (retval) {
+ dev_err(&pdev->dev,
+ "EHCI: private init failed with err %d\n", retval);
+ goto err_disable_clk;
+ }
+
+ device_wakeup_enable(hcd->self.controller);
+
+ return 0;
+
+err_disable_clk:
+ clk_disable(ehci_mv->clk);
+ clk_unprepare(ehci_mv->clk);
+ phy_exit(hcd->phy);
+err_put_hcd:
+ usb_put_hcd(hcd);
+ return retval;
+}
+
+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;
+
+ phy_power_off(hcd->phy);
+ phy_exit(hcd->phy);
+ clk_disable(ehci_mv->clk);
+ clk_unprepare(ehci_mv->clk);
+
+ usb_remove_hcd(hcd);
+ usb_put_hcd(hcd);
+
+ return 0;
+}
+
+static const struct of_device_id mv_ehci_dt_match[] = {
+ {.compatible = "marvell,pxa1928-ehci"},
+ {},
+};
+MODULE_DEVICE_TABLE(of, mv_ehci_dt_match);
+
+static void mv_ehci_shutdown(struct platform_device *pdev)
+{
+ struct ehci_hcd_mv *ehci_mv = platform_get_drvdata(pdev);
+ struct usb_hcd *hcd = ehci_mv->hcd;
+
+ if (!hcd->rh_registered)
+ return;
+
+ if (hcd->driver->shutdown)
+ hcd->driver->shutdown(hcd);
+}
+
+static const struct ehci_driver_overrides ehci_mv_overrides __initconst = {
+ .extra_priv_size = sizeof(struct ehci_hcd_mv),
+};
+
+static struct platform_driver ehci_mv_driver = {
+ .probe = mv_ehci_probe,
+ .remove = mv_ehci_remove,
+ .shutdown = mv_ehci_shutdown,
+ .driver = {
+ .name = "mv-ehci-of",
+ .of_match_table = of_match_ptr(mv_ehci_dt_match),
+ },
+};
+
+static int __init ehci_mv_init(void)
+{
+ if (usb_disabled())
+ return -ENODEV;
+
+ ehci_init_driver(&ehci_mv_hc_driver, &ehci_mv_overrides);
+ return platform_driver_register(&ehci_mv_driver);
+}
+module_init(ehci_mv_init);
+
+static void __exit ehci_mv_cleanup(void)
+{
+ platform_driver_unregister(&ehci_mv_driver);
+}
+module_exit(ehci_mv_cleanup);
+
+MODULE_AUTHOR("Rob Herring <[email protected]>");
+MODULE_LICENSE("GPL v2");
--
2.1.0
On Wed, 2015-05-13 at 17:49 -0500, Rob Herring wrote:
> --- a/drivers/usb/host/Kconfig
> +++ b/drivers/usb/host/Kconfig
> +config USB_EHCI_MV_OF
> + bool "EHCI OF support for Marvell PXA/MMP USB controller"
> + depends on (ARCH_PXA || ARCH_MMP)
> + select USB_EHCI_ROOT_HUB_TT
> + ---help---
> + Enables support for Marvell (including PXA and MMP series) on-chip
> + USB SPH and OTG controller. SPH is a single port host, and it can
> + only be EHCI host. OTG is controller that can switch to host mode.
> + Note that this driver will not work on Marvell's other EHCI
> + controller used by the EBU-type SoCs including Orion, Kirkwood,
> + Dova, Armada 370 and Armada XP. See "Support for Marvell EBU
> + on-chip EHCI USB controller" for those.
> --- a/drivers/usb/host/Makefile
> +++ b/drivers/usb/host/Makefile
> +obj-$(CONFIG_USB_EHCI_MV_OF) += ehci-mv-of.o
USB_EHCI_MV_OF is a bool symbol so ehci-mv-of.o will never be part of a
module, correct?
> --- /dev/null
> +++ b/drivers/usb/host/ehci-mv-of.c
> +MODULE_DEVICE_TABLE(of, mv_ehci_dt_match);
> +module_init(ehci_mv_init);
> +
> +static void __exit ehci_mv_cleanup(void)
> +{
> + platform_driver_unregister(&ehci_mv_driver);
> +}
> +module_exit(ehci_mv_cleanup);
> +
> +MODULE_AUTHOR("Rob Herring <[email protected]>");
> +MODULE_LICENSE("GPL v2");
The code contains a few module-specific constructs. (These will be
preprocessed away, replaced with a built-in equivalent, etc.) Was it
your intention to make USB_EHCI_MV_OF tristate?
Paul Bolle
On Thursday 14 May 2015 10:24:38 Paul Bolle wrote:
>
> > --- a/drivers/usb/host/Makefile
> > +++ b/drivers/usb/host/Makefile
>
> > +obj-$(CONFIG_USB_EHCI_MV_OF) += ehci-mv-of.o
>
> USB_EHCI_MV_OF is a bool symbol so ehci-mv-of.o will never be part of a
> module, correct?
>
>
I think that's a bug.
Arnd
On Wednesday 13 May 2015 17:49:00 Rob Herring wrote:
> +
> + /* enable port power and reserved bit 25 */
> + status = __raw_readl(&ehci_regs->port_status[0]);
> + status |= (PORT_POWER) | (1 << 25);
> + /* Clear bits 30:31 for HSIC to be enabled */
> + status &= ~(0x3 << 30);
> + __raw_writel(status, &ehci_regs->port_status[0]);
> +
> + /* test mode: force enable hs */
> + status = __raw_readl(&ehci_regs->port_status[0]);
> + status &= ~(0xf << 16);
> + status |= (0x5 << 16);
> + __raw_writel(status, &ehci_regs->port_status[0]);
> +
> + /* disable test mode */
> + status = __raw_readl(&ehci_regs->port_status[0]);
> + status &= ~(0xf << 16);
> + __raw_writel(status, &ehci_regs->port_status[0]);
Please make this endian-safe.
> +
> +static const struct of_device_id mv_ehci_dt_match[] = {
> + {.compatible = "marvell,pxa1928-ehci"},
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, mv_ehci_dt_match);
> +static struct platform_driver ehci_mv_driver = {
> + .probe = mv_ehci_probe,
> + .remove = mv_ehci_remove,
> + .shutdown = mv_ehci_shutdown,
> + .driver = {
> + .name = "mv-ehci-of",
> + .of_match_table = of_match_ptr(mv_ehci_dt_match),
> + },
> +};
Warning: unused symbol 'mv_ehci_dt_match'.
Just remove the of_match_ptr().
Arnd
On Thu, May 14, 2015 at 5:42 AM, Arnd Bergmann <[email protected]> wrote:
> On Thursday 14 May 2015 10:24:38 Paul Bolle wrote:
>>
>> > --- a/drivers/usb/host/Makefile
>> > +++ b/drivers/usb/host/Makefile
>>
>> > +obj-$(CONFIG_USB_EHCI_MV_OF) += ehci-mv-of.o
>>
>> USB_EHCI_MV_OF is a bool symbol so ehci-mv-of.o will never be part of a
>> module, correct?
>>
>>
>
> I think that's a bug.
Indeed.
Rob
On Wed, 13 May 2015, Rob Herring wrote:
> Add platform driver for Marvell PXA1928 SOC. This is different from the
> mv-ehci driver in that it uses the generic phy framework, uses DT, does
> not use platform_data, is host only, and has a specific HSIC PHY and
> controller initialization handshake.
Are those differences sufficient to make a separate source file
necessary? There are plenty of other drivers that work for both DT and
non-DT, for example.
> Signed-off-by: Rob Herring <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: Alan Stern <[email protected]>
> Cc: [email protected]
> ---
> drivers/usb/host/Kconfig | 15 ++-
> drivers/usb/host/Makefile | 1 +
> drivers/usb/host/ehci-mv-of.c | 243 ++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 258 insertions(+), 1 deletion(-)
> create mode 100644 drivers/usb/host/ehci-mv-of.c
>
> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
> index 197a6a3..4d8cfbc 100644
> --- a/drivers/usb/host/Kconfig
> +++ b/drivers/usb/host/Kconfig
> @@ -251,6 +251,19 @@ config USB_EHCI_MV
> Dova, Armada 370 and Armada XP. See "Support for Marvell EBU
> on-chip EHCI USB controller" for those.
>
> +config USB_EHCI_MV_OF
> + bool "EHCI OF support for Marvell PXA/MMP USB controller"
> + depends on (ARCH_PXA || ARCH_MMP)
> + select USB_EHCI_ROOT_HUB_TT
> + ---help---
> + Enables support for Marvell (including PXA and MMP series) on-chip
> + USB SPH and OTG controller. SPH is a single port host, and it can
> + only be EHCI host. OTG is controller that can switch to host mode.
> + Note that this driver will not work on Marvell's other EHCI
> + controller used by the EBU-type SoCs including Orion, Kirkwood,
> + Dova, Armada 370 and Armada XP. See "Support for Marvell EBU
> + on-chip EHCI USB controller" for those.
This is identical to the help text for USB_EHCI_MV. How is a user
supposed to know which option to enable?
> +
> config USB_W90X900_EHCI
> tristate "W90X900(W90P910) EHCI support"
> depends on ARCH_W90X900
> @@ -663,7 +676,7 @@ config USB_SL811_HCD
> help
> The SL811HS is a single-port USB controller that supports either
> host side or peripheral side roles. Enable this option if your
> - board has this chip, and you want to use it as a host controller.
> + board has this chip, and you want to use it as a host controller.
This doesn't belong in the patch.
> If unsure, say N.
>
> To compile this driver as a module, choose M here: the
> diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
> index 65b0b6a..5ed93ff 100644
> --- a/drivers/usb/host/Makefile
> +++ b/drivers/usb/host/Makefile
> @@ -32,6 +32,7 @@ obj-$(CONFIG_USB_XHCI_PLATFORM) += xhci-plat-hcd.o
> obj-$(CONFIG_USB_EHCI_HCD) += ehci-hcd.o
> obj-$(CONFIG_USB_EHCI_PCI) += ehci-pci.o
> obj-$(CONFIG_USB_EHCI_HCD_PLATFORM) += ehci-platform.o
> +obj-$(CONFIG_USB_EHCI_MV_OF) += ehci-mv-of.o
> obj-$(CONFIG_USB_EHCI_MXC) += ehci-mxc.o
> obj-$(CONFIG_USB_EHCI_HCD_OMAP) += ehci-omap.o
> obj-$(CONFIG_USB_EHCI_HCD_ORION) += ehci-orion.o
> diff --git a/drivers/usb/host/ehci-mv-of.c b/drivers/usb/host/ehci-mv-of.c
> new file mode 100644
> index 0000000..3783299
> --- /dev/null
> +++ b/drivers/usb/host/ehci-mv-of.c
> @@ -0,0 +1,243 @@
> +/*
> + * Copyright (C) 2015 Linaro, Ltd.
> + * Rob Herring <[email protected]>
> + *
> + * Based on vendor driver modifications to ehci-mv.c:
> + * Copyright (C) 2011 Marvell International Ltd. All rights reserved.
> + * Author: Chao Xie <[email protected]>
> + * Neil Zhang <[email protected]>
> + *
> + * 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.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/clk.h>
> +#include <linux/io.h>
> +#include <linux/of.h>
> +#include <linux/phy/phy.h>
> +#include <linux/err.h>
> +#include <linux/usb.h>
> +#include <linux/usb/hcd.h>
> +
> +#include "ehci.h"
> +
> +struct ehci_hcd_mv {
> + struct usb_hcd *hcd;
> + struct phy *phy;
> + struct clk *clk;
> +};
Where does the .phy member get used?
> +
> +#define hcd_to_mv_priv(h) ((struct ehci_hcd_mv *)hcd_to_ehci(h)->priv)
> +
> +static struct hc_driver ehci_mv_hc_driver;
> +
> +static bool wait_for_reg(void __iomem *reg, u32 mask, unsigned long timeout)
> +{
> + timeout += jiffies;
> + while (time_is_after_eq_jiffies(timeout)) {
> + if ((readl(reg) & mask) == mask)
> + return true;
> + msleep(1);
> + }
> + return false;
> +}
> +
> +static int mv_ehci_enable(struct ehci_hcd_mv *ehci_mv)
> +{
> + struct ehci_hcd *ehci = hcd_to_ehci(ehci_mv->hcd);
> + struct ehci_regs *ehci_regs = ehci->regs;
> + int retval;
> + u32 status;
> +
> + /* enable port power and reserved bit 25 */
> + status = __raw_readl(&ehci_regs->port_status[0]);
> + status |= (PORT_POWER) | (1 << 25);
> + /* Clear bits 30:31 for HSIC to be enabled */
> + status &= ~(0x3 << 30);
> + __raw_writel(status, &ehci_regs->port_status[0]);
> +
> + /* test mode: force enable hs */
> + status = __raw_readl(&ehci_regs->port_status[0]);
> + status &= ~(0xf << 16);
> + status |= (0x5 << 16);
> + __raw_writel(status, &ehci_regs->port_status[0]);
> +
> + /* disable test mode */
> + status = __raw_readl(&ehci_regs->port_status[0]);
> + status &= ~(0xf << 16);
> + __raw_writel(status, &ehci_regs->port_status[0]);
> +
> + retval = phy_power_on(ehci_mv->hcd->phy);
> + if (retval)
> + return retval;
> +
> + /* issue port reset */
> + status = __raw_readl(&ehci_regs->port_status[0]);
> + status |= PORT_RESET;
> + status &= ~PORT_PE;
> + __raw_writel(status, &ehci_regs->port_status[0]);
> +
> + /* check reset done */
> + if (!wait_for_reg(&ehci_regs->port_status[0], PORT_RESET, HZ / 10)) {
> + pr_err("HSIC port reset not done: port_status 0x%x\n", status);
> + return -ETIMEDOUT;
> + }
> + return 0;
> +}
> +
> +static int mv_ehci_probe(struct platform_device *pdev)
> +{
> + struct usb_hcd *hcd;
> + struct ehci_hcd *ehci;
> + struct ehci_hcd_mv *ehci_mv;
> + struct resource *r;
> + int retval = -ENODEV;
> +
> + hcd = usb_create_hcd(&ehci_mv_hc_driver, &pdev->dev, "mv ehci");
> + if (!hcd)
> + return -ENOMEM;
> + ehci = hcd_to_ehci(hcd);
> + ehci_mv = hcd_to_mv_priv(hcd);
> + ehci_mv->hcd = hcd;
> +
> + platform_set_drvdata(pdev, ehci_mv);
> +
> + ehci_mv->clk = devm_clk_get(&pdev->dev, NULL);
> + if (IS_ERR(ehci_mv->clk)) {
> + dev_err(&pdev->dev, "error getting clock\n");
> + retval = PTR_ERR(ehci_mv->clk);
> + goto err_put_hcd;
> + }
> +
> + r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + ehci->caps = devm_ioremap_resource(&pdev->dev, r);
> + if (IS_ERR(ehci->caps)) {
> + retval = PTR_ERR(ehci->caps);
> + goto err_put_hcd;
> + }
> +
> + hcd->phy = devm_of_phy_get(&pdev->dev, pdev->dev.of_node, NULL);
> + if (IS_ERR_OR_NULL(hcd->phy)) {
> + retval = PTR_ERR(hcd->phy);
> + if (retval != -EPROBE_DEFER && retval != -ENODEV)
> + dev_err(&pdev->dev, "failed to get the phy\n");
> + else
> + return -EPROBE_DEFER;
> + goto err_put_hcd;
> + }
> +
> + hcd->rsrc_start = r->start;
> + hcd->rsrc_len = resource_size(r);
> +
> + hcd->irq = platform_get_irq(pdev, 0);
> + if (!hcd->irq) {
> + dev_err(&pdev->dev, "Cannot get irq.");
> + retval = -ENODEV;
> + goto err_put_hcd;
> + }
> +
> + retval = phy_init(hcd->phy);
> + if (retval)
> + goto err_put_hcd;
> +
> + clk_prepare(ehci_mv->clk);
> + clk_enable(ehci_mv->clk);
> +
> + retval = usb_add_hcd(hcd, hcd->irq, IRQF_SHARED);
> + if (retval) {
> + dev_err(&pdev->dev,
> + "failed to add hcd with err %d\n", retval);
> + goto err_disable_clk;
> + }
> +
> + retval = mv_ehci_enable(ehci_mv);
Is this call in the right place? I doubt it. The rest of the system
expects the controller to be running by the time usb_add_hcd() returns.
Probably you ought to put mv_ehci_enable in the ehci_mv_overrides
structure as the .reset entry.
> + if (retval) {
> + dev_err(&pdev->dev,
> + "EHCI: private init failed with err %d\n", retval);
> + goto err_disable_clk;
> + }
> +
> + device_wakeup_enable(hcd->self.controller);
> +
> + return 0;
> +
> +err_disable_clk:
> + clk_disable(ehci_mv->clk);
> + clk_unprepare(ehci_mv->clk);
> + phy_exit(hcd->phy);
> +err_put_hcd:
> + usb_put_hcd(hcd);
> + return retval;
> +}
This doesn't set hcd->has_tt anywhere. So why bother selecting
USB_EHCI_ROOT_HUB_TT?
> +
> +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;
> +
> + phy_power_off(hcd->phy);
> + phy_exit(hcd->phy);
> + clk_disable(ehci_mv->clk);
> + clk_unprepare(ehci_mv->clk);
> +
> + usb_remove_hcd(hcd);
> + usb_put_hcd(hcd);
The remove actions should be carried out in reverse order of the probe
actions. As it is, this code briefly leaves the controller running but
with the clock turned off. That doesn't seem like a good idea.
> +
> + return 0;
> +}
> +
> +static const struct of_device_id mv_ehci_dt_match[] = {
> + {.compatible = "marvell,pxa1928-ehci"},
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, mv_ehci_dt_match);
> +
> +static void mv_ehci_shutdown(struct platform_device *pdev)
> +{
> + struct ehci_hcd_mv *ehci_mv = platform_get_drvdata(pdev);
> + struct usb_hcd *hcd = ehci_mv->hcd;
> +
> + if (!hcd->rh_registered)
> + return;
> +
> + if (hcd->driver->shutdown)
> + hcd->driver->shutdown(hcd);
> +}
This is kind of strange. It's the same as usb_hcd_platform_shutdown()
except for the hcd->rh_registered test. Is there some reason why that
test is needed here but not in the generic routine? If not, can the
test be removed? Or should it be added to the generic routine?
> +
> +static const struct ehci_driver_overrides ehci_mv_overrides __initconst = {
> + .extra_priv_size = sizeof(struct ehci_hcd_mv),
> +};
> +
> +static struct platform_driver ehci_mv_driver = {
> + .probe = mv_ehci_probe,
> + .remove = mv_ehci_remove,
> + .shutdown = mv_ehci_shutdown,
> + .driver = {
> + .name = "mv-ehci-of",
> + .of_match_table = of_match_ptr(mv_ehci_dt_match),
> + },
> +};
> +
> +static int __init ehci_mv_init(void)
> +{
> + if (usb_disabled())
> + return -ENODEV;
> +
> + ehci_init_driver(&ehci_mv_hc_driver, &ehci_mv_overrides);
> + return platform_driver_register(&ehci_mv_driver);
> +}
> +module_init(ehci_mv_init);
> +
> +static void __exit ehci_mv_cleanup(void)
> +{
> + platform_driver_unregister(&ehci_mv_driver);
> +}
> +module_exit(ehci_mv_cleanup);
> +
> +MODULE_AUTHOR("Rob Herring <[email protected]>");
> +MODULE_LICENSE("GPL v2");
Alan Stern
On Wed, 13 May 2015, Rob Herring wrote:
> Add platform driver for Marvell PXA1928 SOC. This is different from the
> mv-ehci driver in that it uses the generic phy framework, uses DT, does
> not use platform_data, is host only, and has a specific HSIC PHY and
> controller initialization handshake.
One more thing, which I had intended to include in my earlier review
but then forgot about...
> +static int mv_ehci_probe(struct platform_device *pdev)
> +{
...
> + hcd->phy = devm_of_phy_get(&pdev->dev, pdev->dev.of_node, NULL);
> + if (IS_ERR_OR_NULL(hcd->phy)) {
> + retval = PTR_ERR(hcd->phy);
> + if (retval != -EPROBE_DEFER && retval != -ENODEV)
> + dev_err(&pdev->dev, "failed to get the phy\n");
> + else
> + return -EPROBE_DEFER;
> + goto err_put_hcd;
> + }
Please straighten out this convoluted logic. It should go like this:
hcd->phy = devm_of_phy_get(&pdev->dev, pdev->dev.of_node, NULL);
if (IS_ERR_OR_NULL(hcd->phy)) {
retval = PTR_ERR(hcd->phy);
if (retval == -EPROBE_DEFER || retval == -ENODEV)
return -EPROBE_DEFER;
dev_err(&pdev->dev, "failed to get the phy\n");
goto err_put_hcd;
}
Alan Stern
On Thu, May 14, 2015 at 9:56 AM, Alan Stern <[email protected]> wrote:
> On Wed, 13 May 2015, Rob Herring wrote:
>
>> Add platform driver for Marvell PXA1928 SOC. This is different from the
>> mv-ehci driver in that it uses the generic phy framework, uses DT, does
>> not use platform_data, is host only, and has a specific HSIC PHY and
>> controller initialization handshake.
>
> Are those differences sufficient to make a separate source file
> necessary? There are plenty of other drivers that work for both DT and
> non-DT, for example.
IMO, yes. If it were only DT support, then no it would not make sense.
I don't disagree there are too many ehci drivers mostly varying in phy
control.
Actually, ehci-platform is a closer match to this driver. I could use
it perhaps and either add my custom power_on function and a match
table entry to it or export ehci-platform functions (probe,
power_{on,off}, and use them here. Opinion on which direction you
prefer?
There's also a resume issue I have to figure out how support too. This
is what the vendor driver does:
@@ -469,6 +470,40 @@ static int ehci_bus_resume (struct usb_hcd *hcd)
set_bit(i, &resume_needed);
}
ehci_writel(ehci, temp, &ehci->regs->port_status [i]);
+
+ /* for HSIC phy, we have already disable SE0 state when
+ * resume back. we need do following step to pull USB
+ * controller back. */
+#ifdef CONFIG_USB_EHCI_MV_U2H_HSIC
+#define PHY_TEST_GRP_0 0x10
+
+#define S2H_TEST_UTMI_SEL (0x1 << 7)
+#define S2H_TEST_SUSPENDM (0x1 << 14)
+#define S2H_TEST_TX_BITSTUFF_EN (0x1 << 29)
+
+ if (hcd->usb_phy->is_hsic) {
+ void __iomem *phy_base = hcd->usb_phy->io_priv;
+ unsigned int count;
+ mdelay(10);
+
+ temp = readl(phy_base + PHY_TEST_GRP_0);
+ writel(temp | (S2H_TEST_UTMI_SEL
+ | S2H_TEST_SUSPENDM
+ | S2H_TEST_TX_BITSTUFF_EN),
+ phy_base + PHY_TEST_GRP_0);
+
+ count = 1000;
+ while (--count && (ehci_readl(ehci,
+ &ehci->regs->port_status[i]) & PORT_RESUME))
+ udelay(50);
+ if (count <= 0)
+ ehci_dbg(ehci, "resume time out\n");
+ writel(readl(phy_base + PHY_TEST_GRP_0)
+ & ~S2H_TEST_UTMI_SEL,
+ phy_base + PHY_TEST_GRP_0);
+ }
+#endif
+
}
/*
>> +config USB_EHCI_MV_OF
>> + bool "EHCI OF support for Marvell PXA/MMP USB controller"
>> + depends on (ARCH_PXA || ARCH_MMP)
>> + select USB_EHCI_ROOT_HUB_TT
>> + ---help---
>> + Enables support for Marvell (including PXA and MMP series) on-chip
>> + USB SPH and OTG controller. SPH is a single port host, and it can
>> + only be EHCI host. OTG is controller that can switch to host mode.
>> + Note that this driver will not work on Marvell's other EHCI
>> + controller used by the EBU-type SoCs including Orion, Kirkwood,
>> + Dova, Armada 370 and Armada XP. See "Support for Marvell EBU
>> + on-chip EHCI USB controller" for those.
>
> This is identical to the help text for USB_EHCI_MV. How is a user
> supposed to know which option to enable?
The OTG part needs to go. Perhaps I need to make it PXA1928 specific.
All I know about Marvell IP is it is scattered all over the place, so
determining which SOCs are the same IP is difficult. The PHYs for sure
seem to be different on every chip as I did not find any existing
Marvell phy drivers that match.
Rob
On Thu, 14 May 2015, Rob Herring wrote:
> On Thu, May 14, 2015 at 9:56 AM, Alan Stern <[email protected]> wrote:
> > On Wed, 13 May 2015, Rob Herring wrote:
> >
> >> Add platform driver for Marvell PXA1928 SOC. This is different from the
> >> mv-ehci driver in that it uses the generic phy framework, uses DT, does
> >> not use platform_data, is host only, and has a specific HSIC PHY and
> >> controller initialization handshake.
> >
> > Are those differences sufficient to make a separate source file
> > necessary? There are plenty of other drivers that work for both DT and
> > non-DT, for example.
>
> IMO, yes. If it were only DT support, then no it would not make sense.
> I don't disagree there are too many ehci drivers mostly varying in phy
> control.
>
> Actually, ehci-platform is a closer match to this driver. I could use
> it perhaps and either add my custom power_on function and a match
> table entry to it or export ehci-platform functions (probe,
> power_{on,off}, and use them here. Opinion on which direction you
> prefer?
If you can use ehci-platform in place of a more specific driver, that
would be great. Custom power-on, power-suspend, and power-off routines
aren't a problem.
> There's also a resume issue I have to figure out how support too. This
> is what the vendor driver does:
>
> @@ -469,6 +470,40 @@ static int ehci_bus_resume (struct usb_hcd *hcd)
> set_bit(i, &resume_needed);
> }
> ehci_writel(ehci, temp, &ehci->regs->port_status [i]);
> +
> + /* for HSIC phy, we have already disable SE0 state when
> + * resume back. we need do following step to pull USB
> + * controller back. */
> +#ifdef CONFIG_USB_EHCI_MV_U2H_HSIC
> +#define PHY_TEST_GRP_0 0x10
> +
> +#define S2H_TEST_UTMI_SEL (0x1 << 7)
> +#define S2H_TEST_SUSPENDM (0x1 << 14)
> +#define S2H_TEST_TX_BITSTUFF_EN (0x1 << 29)
> +
> + if (hcd->usb_phy->is_hsic) {
> + void __iomem *phy_base = hcd->usb_phy->io_priv;
> + unsigned int count;
> + mdelay(10);
> +
> + temp = readl(phy_base + PHY_TEST_GRP_0);
> + writel(temp | (S2H_TEST_UTMI_SEL
> + | S2H_TEST_SUSPENDM
> + | S2H_TEST_TX_BITSTUFF_EN),
> + phy_base + PHY_TEST_GRP_0);
> +
> + count = 1000;
> + while (--count && (ehci_readl(ehci,
> + &ehci->regs->port_status[i]) & PORT_RESUME))
> + udelay(50);
> + if (count <= 0)
> + ehci_dbg(ehci, "resume time out\n");
> + writel(readl(phy_base + PHY_TEST_GRP_0)
> + & ~S2H_TEST_UTMI_SEL,
> + phy_base + PHY_TEST_GRP_0);
> + }
> +#endif
It's not entirely clear why this is needed. For example, why wait for
PORT_RESUME to turn off if you never turned it on in the first place?
Anyway, it looks like this won't be so easy to integrate with
ehci-platform. Perhaps this could be done in the custom power-on
routine; I don't know if that's feasible.
> >> +config USB_EHCI_MV_OF
> >> + bool "EHCI OF support for Marvell PXA/MMP USB controller"
> >> + depends on (ARCH_PXA || ARCH_MMP)
> >> + select USB_EHCI_ROOT_HUB_TT
> >> + ---help---
> >> + Enables support for Marvell (including PXA and MMP series) on-chip
> >> + USB SPH and OTG controller. SPH is a single port host, and it can
> >> + only be EHCI host. OTG is controller that can switch to host mode.
> >> + Note that this driver will not work on Marvell's other EHCI
> >> + controller used by the EBU-type SoCs including Orion, Kirkwood,
> >> + Dova, Armada 370 and Armada XP. See "Support for Marvell EBU
> >> + on-chip EHCI USB controller" for those.
> >
> > This is identical to the help text for USB_EHCI_MV. How is a user
> > supposed to know which option to enable?
>
> The OTG part needs to go. Perhaps I need to make it PXA1928 specific.
> All I know about Marvell IP is it is scattered all over the place, so
> determining which SOCs are the same IP is difficult. The PHYs for sure
> seem to be different on every chip as I did not find any existing
> Marvell phy drivers that match.
Obviously this is the kind of thing that a developer is supposed to
sort out, rather then making users responsible for finding the right
combination of settings for their hardware.
Alan Stern
On Thursday 14 May 2015 11:55:36 Alan Stern wrote:
>
> > + hcd->phy = devm_of_phy_get(&pdev->dev, pdev->dev.of_node, NULL);
> > + if (IS_ERR_OR_NULL(hcd->phy)) {
> > + retval = PTR_ERR(hcd->phy);
> > + if (retval != -EPROBE_DEFER && retval != -ENODEV)
> > + dev_err(&pdev->dev, "failed to get the phy\n");
> > + else
> > + return -EPROBE_DEFER;
> > + goto err_put_hcd;
> > + }
>
> Please straighten out this convoluted logic. It should go like this:
>
> hcd->phy = devm_of_phy_get(&pdev->dev, pdev->dev.of_node, NULL);
> if (IS_ERR_OR_NULL(hcd->phy)) {
> retval = PTR_ERR(hcd->phy);
> if (retval == -EPROBE_DEFER || retval == -ENODEV)
> return -EPROBE_DEFER;
> dev_err(&pdev->dev, "failed to get the phy\n");
> goto err_put_hcd;
> }
>
IS_ERR_OR_NULL is almost always wrong. Kernel interfaces that can return
an error pointer should never return a NULL pointer, and I'm pretty sure
that is also the case for the phy subsystem (or else it should be fixed).
In some cases, we have interfaces that return NULL pointers or error pointers,
but those are designed to treat NULL as success.
Arnd
On 14.05.2015 00:48, Rob Herring wrote:
> This series adds USB PHYs and EHCI host drivers for the Marvell PXA1928
> SOC.
>
> The OTG block is based on ChipIdea and works with "chipidea,usb2"
> compatible driver as is just by adding the PHY driver. Yay!
>
> Rob
>
> Rob Herring (5):
> dt-bindings: Add Marvell PXA1928 USB and HSIC PHY bindings
> dt-bindings: Add Marvell PXA1928 USB EHCI controller binding
> phy: Add Marvell USB 2.0 OTG 28nm PHY
> phy: add Marvell HSIC 28nm PHY
> usb: add pxa1928 ehci support
>
> .../devicetree/bindings/phy/pxa1928-usb-phy.txt | 18 ++
> .../devicetree/bindings/usb/ehci-pxa1928.txt | 19 ++
> drivers/phy/Kconfig | 20 ++
> drivers/phy/Makefile | 2 +
> drivers/phy/phy-mv-hsic.c | 208 +++++++++++++
> drivers/phy/phy-mv-usb2.c | 329 +++++++++++++++++++++
[Adding some MVEBU guys]
Rob,
I had a look at the USB PHYs of some of the other Marvell SoCs a while
ago for the barebox bootloader [1]. Marvell seems to distinguish the
USB PHY type by technology node, e.g. 28nm like the one above. For the
most used Marvell SoCs, i.e. Kirkwood, Dove, and Armada 370/XP, they all
use a different technology node and we could either use the SoC name
or the technology node as compatible.
Anyway, if you are introducing new PHY drivers with _that_ generic
names, it will either clash with every other Marvell USB PHYs -
or we'll have to add the PHY code into the drivers above.
[1] http://lists.infradead.org/pipermail/barebox/2014-June/019600.html
> drivers/usb/host/Kconfig | 15 +-
> drivers/usb/host/Makefile | 1 +
> drivers/usb/host/ehci-mv-of.c | 243 +++++++++++++++
> 9 files changed, 854 insertions(+), 1 deletion(-)
> create mode 100644 Documentation/devicetree/bindings/phy/pxa1928-usb-phy.txt
> create mode 100644 Documentation/devicetree/bindings/usb/ehci-pxa1928.txt
> create mode 100644 drivers/phy/phy-mv-hsic.c
> create mode 100644 drivers/phy/phy-mv-usb2.c
> create mode 100644 drivers/usb/host/ehci-mv-of.c
>
On Fri, May 15, 2015 at 4:55 AM, Sebastian Hesselbarth
<[email protected]> wrote:
> On 14.05.2015 00:48, Rob Herring wrote:
>>
>> This series adds USB PHYs and EHCI host drivers for the Marvell PXA1928
>> SOC.
>>
>> The OTG block is based on ChipIdea and works with "chipidea,usb2"
>> compatible driver as is just by adding the PHY driver. Yay!
>>
>> Rob
>>
>> Rob Herring (5):
>> dt-bindings: Add Marvell PXA1928 USB and HSIC PHY bindings
>> dt-bindings: Add Marvell PXA1928 USB EHCI controller binding
>> phy: Add Marvell USB 2.0 OTG 28nm PHY
>> phy: add Marvell HSIC 28nm PHY
>> usb: add pxa1928 ehci support
>>
>> .../devicetree/bindings/phy/pxa1928-usb-phy.txt | 18 ++
>> .../devicetree/bindings/usb/ehci-pxa1928.txt | 19 ++
>> drivers/phy/Kconfig | 20 ++
>> drivers/phy/Makefile | 2 +
>> drivers/phy/phy-mv-hsic.c | 208 +++++++++++++
>> drivers/phy/phy-mv-usb2.c | 329
>> +++++++++++++++++++++
>
>
> [Adding some MVEBU guys]
>
> Rob,
>
> I had a look at the USB PHYs of some of the other Marvell SoCs a while
> ago for the barebox bootloader [1]. Marvell seems to distinguish the
> USB PHY type by technology node, e.g. 28nm like the one above. For the
> most used Marvell SoCs, i.e. Kirkwood, Dove, and Armada 370/XP, they all
> use a different technology node and we could either use the SoC name
> or the technology node as compatible.
It also seems to be by business unit. The registers in the 40nm
version I have look nothing like what are in Armada. It could be the
same IP, but PHY IP tends to just be signals and they leave the
register interface up the to licensee's imagination. Any idea if there
are 28nm Armada parts which have the same phy?
It's a good idea to always include the SOC name in the compatible.
Every new chip is an opportunity for changes or bugs.
> Anyway, if you are introducing new PHY drivers with _that_ generic
> names, it will either clash with every other Marvell USB PHYs -
> or we'll have to add the PHY code into the drivers above.
How about phy-pxa-28nm-{usb2,hsic}?
Rob
On Friday 15 May 2015 07:48:24 Rob Herring wrote:
>
> > Anyway, if you are introducing new PHY drivers with _that_ generic
> > names, it will either clash with every other Marvell USB PHYs -
> > or we'll have to add the PHY code into the drivers above.
>
> How about phy-pxa-28nm-{usb2,hsic}?
>
Seems ok to me, but please verify that these are actually usb2 and hsic
specific devices. There are a lot of PHY implementations that share the
basic layout between devices for different purposes, e.g. USB3 and PCIe
and they should have a name that is not too specific for one connection
when the same driver gets reused by another part.
Arnd
On Fri, May 15, 2015 at 9:11 AM, Arnd Bergmann <[email protected]> wrote:
> On Friday 15 May 2015 07:48:24 Rob Herring wrote:
>>
>> > Anyway, if you are introducing new PHY drivers with _that_ generic
>> > names, it will either clash with every other Marvell USB PHYs -
>> > or we'll have to add the PHY code into the drivers above.
>>
>> How about phy-pxa-28nm-{usb2,hsic}?
>>
>
> Seems ok to me, but please verify that these are actually usb2 and hsic
> specific devices. There are a lot of PHY implementations that share the
> basic layout between devices for different purposes, e.g. USB3 and PCIe
> and they should have a name that is not too specific for one connection
> when the same driver gets reused by another part.
Yes, I'm aware of that as Calxeda "comboPHYs" did 10G XAUI, PCIe and
SATA. Those are cases are supporting similar electrical specs. USB3 is
similar to those. I've not seen USB 2.0 PHYs shared with anything
else, and HSIC is also very unique.
Rob
>
> Arnd
Hi,
On Thursday 14 May 2015 04:18 AM, Rob Herring wrote:
> Add driver for USB PHY found in Marvell PXA1928 SOC.
>
> Signed-off-by: Rob Herring <[email protected]>
> Cc: Kishon Vijay Abraham I <[email protected]>
> ---
> drivers/phy/Kconfig | 10 ++
> drivers/phy/Makefile | 1 +
> drivers/phy/phy-mv-usb2.c | 329 ++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 340 insertions(+)
> create mode 100644 drivers/phy/phy-mv-usb2.c
>
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index a53bd5b..ef7634f 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -52,6 +52,16 @@ config PHY_EXYNOS_MIPI_VIDEO
> Support for MIPI CSI-2 and MIPI DSI DPHY found on Samsung S5P
> and EXYNOS SoCs.
>
> +config PHY_MV_USB2
> + tristate "Marvell USB 2.0 28nm PHY Driver"
> + select GENERIC_PHY
> + help
> + Enable this to support Marvell USB 2.0 PHY driver for Marvell
> + SoC. This driver will do the PHY initialization and shutdown.
> + The PHY driver will be used by Marvell udc/ehci/otg driver.
> +
> + To compile this driver as a module, choose M here.
> +
> config PHY_MVEBU_SATA
> def_bool y
> depends on ARCH_DOVE || MACH_DOVE || MACH_KIRKWOOD
> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> index f126251..768e55a 100644
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -10,6 +10,7 @@ obj-$(CONFIG_ARMADA375_USBCLUSTER_PHY) += phy-armada375-usb2.o
> obj-$(CONFIG_BCM_KONA_USB2_PHY) += phy-bcm-kona-usb2.o
> obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO) += phy-exynos-dp-video.o
> obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO) += phy-exynos-mipi-video.o
> +obj-$(CONFIG_PHY_MV_USB2) += phy-mv-usb2.o
> obj-$(CONFIG_PHY_MVEBU_SATA) += phy-mvebu-sata.o
> obj-$(CONFIG_PHY_MIPHY28LP) += phy-miphy28lp.o
> obj-$(CONFIG_PHY_MIPHY365X) += phy-miphy365x.o
> diff --git a/drivers/phy/phy-mv-usb2.c b/drivers/phy/phy-mv-usb2.c
> new file mode 100644
> index 0000000..c48d111
> --- /dev/null
> +++ b/drivers/phy/phy-mv-usb2.c
> @@ -0,0 +1,329 @@
> +/*
> + * Copyright (C) 2015 Linaro, Ltd.
> + * Rob Herring <[email protected]>
> + *
> + * Based on vendor driver:
> + * Copyright (C) 2013 Marvell Inc.
> + * Author: Chao Xie <[email protected]>
> + *
> + * 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/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/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/phy/phy.h>
> +
> +/* USB PXA1928 PHY mapping */
> +#define PHY_28NM_PLL_REG0 0x0
> +#define PHY_28NM_PLL_REG1 0x4
> +#define PHY_28NM_CAL_REG 0x8
> +#define PHY_28NM_TX_REG0 0x0c
> +#define PHY_28NM_TX_REG1 0x10
> +#define PHY_28NM_RX_REG0 0x14
> +#define PHY_28NM_RX_REG1 0x18
> +#define PHY_28NM_DIG_REG0 0x1c
> +#define PHY_28NM_DIG_REG1 0x20
> +#define PHY_28NM_TEST_REG0 0x24
> +#define PHY_28NM_TEST_REG1 0x28
> +#define PHY_28NM_MOC_REG 0x2c
> +#define PHY_28NM_PHY_RESERVE 0x30
> +#define PHY_28NM_OTG_REG 0x34
> +#define PHY_28NM_CHRG_DET 0x38
> +#define PHY_28NM_CTRL_REG0 0xc4
> +#define PHY_28NM_CTRL_REG1 0xc8
> +#define PHY_28NM_CTRL_REG2 0xd4
> +#define PHY_28NM_CTRL_REG3 0xdc
> +
> +/* PHY_28NM_PLL_REG0 */
> +#define PHY_28NM_PLL_READY BIT(31)
> +
> +#define PHY_28NM_PLL_SELLPFR_SHIFT 28
> +#define PHY_28NM_PLL_SELLPFR_MASK (0x3 << 28)
> +
> +#define PHY_28NM_PLL_FBDIV_SHIFT 16
> +#define PHY_28NM_PLL_FBDIV_MASK (0x1ff << 16)
> +
> +#define PHY_28NM_PLL_ICP_SHIFT 8
> +#define PHY_28NM_PLL_ICP_MASK (0x7 << 8)
> +
> +#define PHY_28NM_PLL_REFDIV_SHIFT 0
> +#define PHY_28NM_PLL_REFDIV_MASK 0x7f
> +
> +/* PHY_28NM_PLL_REG1 */
> +#define PHY_28NM_PLL_PU_BY_REG BIT(1)
> +
> +#define PHY_28NM_PLL_PU_PLL BIT(0)
> +
> +/* PHY_28NM_CAL_REG */
> +#define PHY_28NM_PLL_PLLCAL_DONE BIT(31)
> +
> +#define PHY_28NM_PLL_IMPCAL_DONE BIT(23)
> +
> +#define PHY_28NM_PLL_KVCO_SHIFT 16
> +#define PHY_28NM_PLL_KVCO_MASK (0x7 << 16)
> +
> +#define PHY_28NM_PLL_CAL12_SHIFT 20
> +#define PHY_28NM_PLL_CAL12_MASK (0x3 << 20)
> +
> +#define PHY_28NM_IMPCAL_VTH_SHIFT 8
> +#define PHY_28NM_IMPCAL_VTH_MASK (0x7 << 8)
> +
> +#define PHY_28NM_PLLCAL_START_SHIFT 22
> +#define PHY_28NM_IMPCAL_START_SHIFT 13
> +
> +/* PHY_28NM_TX_REG0 */
> +#define PHY_28NM_TX_PU_BY_REG BIT(25)
> +
> +#define PHY_28NM_TX_PU_ANA BIT(24)
> +
> +#define PHY_28NM_TX_AMP_SHIFT 20
> +#define PHY_28NM_TX_AMP_MASK (0x7 << 20)
> +
> +/* PHY_28NM_RX_REG0 */
> +#define PHY_28NM_RX_SQ_THRESH_SHIFT 0
> +#define PHY_28NM_RX_SQ_THRESH_MASK (0xf << 0)
> +
> +/* PHY_28NM_RX_REG1 */
> +#define PHY_28NM_RX_SQCAL_DONE BIT(31)
> +
> +/* PHY_28NM_DIG_REG0 */
> +#define PHY_28NM_DIG_BITSTAFFING_ERR BIT(31)
> +#define PHY_28NM_DIG_SYNC_ERR BIT(30)
> +
> +#define PHY_28NM_DIG_SQ_FILT_SHIFT 16
> +#define PHY_28NM_DIG_SQ_FILT_MASK (0x7 << 16)
> +
> +#define PHY_28NM_DIG_SQ_BLK_SHIFT 12
> +#define PHY_28NM_DIG_SQ_BLK_MASK (0x7 << 12)
> +
> +#define PHY_28NM_DIG_SYNC_NUM_SHIFT 0
> +#define PHY_28NM_DIG_SYNC_NUM_MASK (0x3 << 0)
> +
> +#define PHY_28NM_PLL_LOCK_BYPASS BIT(7)
> +
> +/* PHY_28NM_OTG_REG */
> +#define PHY_28NM_OTG_CONTROL_BY_PIN BIT(5)
> +#define PHY_28NM_OTG_PU_OTG BIT(4)
> +
> +#define PHY_28NM_CHGDTC_ENABLE_SWITCH_DM_SHIFT_28 13
> +#define PHY_28NM_CHGDTC_ENABLE_SWITCH_DP_SHIFT_28 12
> +#define PHY_28NM_CHGDTC_VSRC_CHARGE_SHIFT_28 10
> +#define PHY_28NM_CHGDTC_VDAT_CHARGE_SHIFT_28 8
> +#define PHY_28NM_CHGDTC_CDP_DM_AUTO_SWITCH_SHIFT_28 7
> +#define PHY_28NM_CHGDTC_DP_DM_SWAP_SHIFT_28 6
> +#define PHY_28NM_CHGDTC_PU_CHRG_DTC_SHIFT_28 5
> +#define PHY_28NM_CHGDTC_PD_EN_SHIFT_28 4
> +#define PHY_28NM_CHGDTC_DCP_EN_SHIFT_28 3
> +#define PHY_28NM_CHGDTC_CDP_EN_SHIFT_28 2
> +#define PHY_28NM_CHGDTC_TESTMON_CHRGDTC_SHIFT_28 0
> +
> +#define PHY_28NM_CTRL1_CHRG_DTC_OUT_SHIFT_28 4
> +#define PHY_28NM_CTRL1_VBUSDTC_OUT_SHIFT_28 2
> +
> +#define PHY_28NM_CTRL3_OVERWRITE BIT(0)
> +#define PHY_28NM_CTRL3_VBUS_VALID BIT(4)
> +#define PHY_28NM_CTRL3_AVALID BIT(5)
> +#define PHY_28NM_CTRL3_BVALID BIT(6)
> +
> +struct mv_usb2_phy {
> + struct phy *phy;
> + struct platform_device *pdev;
> + void __iomem *base;
> + struct clk *clk;
> +};
> +
> +static bool wait_for_reg(void __iomem *reg, u32 mask, unsigned long timeout)
> +{
> + timeout += jiffies;
> + while (time_is_after_eq_jiffies(timeout)) {
> + if ((readl(reg) & mask) == mask)
> + return true;
> + msleep(1);
> + }
> + return false;
> +}
> +
> +static int mv_usb2_phy_28nm_init(struct phy *phy)
> +{
> + struct mv_usb2_phy *mv_phy = phy_get_drvdata(phy);
> + struct platform_device *pdev = mv_phy->pdev;
> + void __iomem *base = mv_phy->base;
> + u32 reg;
> +
> + clk_prepare_enable(mv_phy->clk);
> +
> + /* PHY_28NM_PLL_REG0 */
> + reg = readl(base + PHY_28NM_PLL_REG0) &
> + ~(PHY_28NM_PLL_SELLPFR_MASK | PHY_28NM_PLL_FBDIV_MASK
> + | PHY_28NM_PLL_ICP_MASK | PHY_28NM_PLL_REFDIV_MASK);
> + writel(reg | (0x1 << PHY_28NM_PLL_SELLPFR_SHIFT
> + | 0xf0 << PHY_28NM_PLL_FBDIV_SHIFT
> + | 0x3 << PHY_28NM_PLL_ICP_SHIFT
> + | 0xd << PHY_28NM_PLL_REFDIV_SHIFT),
> + base + PHY_28NM_PLL_REG0);
> +
> + /* PHY_28NM_PLL_REG1 */
> + reg = readl(base + PHY_28NM_PLL_REG1);
> + writel(reg | PHY_28NM_PLL_PU_PLL | PHY_28NM_PLL_PU_BY_REG,
> + base + PHY_28NM_PLL_REG1);
> +
> + /* PHY_28NM_TX_REG0 */
> + reg = readl(base + PHY_28NM_TX_REG0) & ~PHY_28NM_TX_AMP_MASK;
> + writel(reg | PHY_28NM_TX_PU_BY_REG | 0x3 << PHY_28NM_TX_AMP_SHIFT |
> + PHY_28NM_TX_PU_ANA,
> + base + PHY_28NM_TX_REG0);
> +
> + /* PHY_28NM_RX_REG0 */
> + reg = readl(base + PHY_28NM_RX_REG0) & ~PHY_28NM_RX_SQ_THRESH_MASK;
> + writel(reg | 0xa << PHY_28NM_RX_SQ_THRESH_SHIFT,
> + base + PHY_28NM_RX_REG0);
> +
> + /* PHY_28NM_DIG_REG0 */
> + reg = readl(base + PHY_28NM_DIG_REG0) &
> + ~(PHY_28NM_DIG_BITSTAFFING_ERR | PHY_28NM_DIG_SYNC_ERR |
> + PHY_28NM_DIG_SQ_FILT_MASK | PHY_28NM_DIG_SQ_BLK_MASK |
> + PHY_28NM_DIG_SYNC_NUM_MASK);
> + writel(reg | (0x1 << PHY_28NM_DIG_SYNC_NUM_SHIFT |
> + PHY_28NM_PLL_LOCK_BYPASS),
> + base + PHY_28NM_DIG_REG0);
> +
> + /* PHY_28NM_OTG_REG */
> + reg = readl(base + PHY_28NM_OTG_REG) | PHY_28NM_OTG_PU_OTG;
> + writel(reg & ~PHY_28NM_OTG_CONTROL_BY_PIN, base + PHY_28NM_OTG_REG);
> +
> + /* Calibration Timing
> + * ____________________________
> + * CAL START ___|
> + * ____________________
> + * CAL_DONE ___________|
> + * | 400us |
> + */
> +
> + /* Make sure PHY Calibration is ready */
> + if (!wait_for_reg(base + PHY_28NM_CAL_REG,
> + PHY_28NM_PLL_PLLCAL_DONE | PHY_28NM_PLL_IMPCAL_DONE,
> + HZ / 10)) {
> + dev_warn(&pdev->dev, "USB PHY PLL calibrate not done after 100mS.");
> + return -ETIMEDOUT;
> + }
> + if (!wait_for_reg(base + PHY_28NM_RX_REG1,
> + PHY_28NM_RX_SQCAL_DONE, HZ / 10)) {
> + dev_warn(&pdev->dev, "USB PHY RX SQ calibrate not done after 100mS.");
> + return -ETIMEDOUT;
> + }
> +
> + /* Make sure PHY PLL is ready */
> + if (!wait_for_reg(base + PHY_28NM_PLL_REG0,
> + PHY_28NM_PLL_READY, HZ / 10)) {
> + dev_warn(&pdev->dev, "PLL_READY not set after 100mS.");
> + return -ETIMEDOUT;
> + }
> +
> + writel(readl(base + PHY_28NM_CTRL_REG3) |
> + (PHY_28NM_CTRL3_OVERWRITE | PHY_28NM_CTRL3_VBUS_VALID
> + | PHY_28NM_CTRL3_AVALID | PHY_28NM_CTRL3_BVALID),
> + base + PHY_28NM_CTRL_REG3);
IMO only the PLL programming should be done in init. The VBUS_VALID etc..
should be done as part of power_on callback.
Thanks
Kishon
Hi,
On Thursday 14 May 2015 04:18 AM, Rob Herring wrote:
> Add PHY driver for the Marvell HSIC 28nm PHY. This PHY is found in PXA1928
> SOC.
>
> Signed-off-by: Rob Herring <[email protected]>
> Cc: Kishon Vijay Abraham I <[email protected]>
> ---
> drivers/phy/Kconfig | 10 +++
> drivers/phy/Makefile | 1 +
> drivers/phy/phy-mv-hsic.c | 208 ++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 219 insertions(+)
> create mode 100644 drivers/phy/phy-mv-hsic.c
>
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index ef7634f..cfcae72 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -52,6 +52,16 @@ config PHY_EXYNOS_MIPI_VIDEO
> Support for MIPI CSI-2 and MIPI DSI DPHY found on Samsung S5P
> and EXYNOS SoCs.
>
> +config PHY_MV_HSIC
> + tristate "Marvell USB HSIC 28nm PHY Driver"
> + select GENERIC_PHY
> + help
> + Enable this to support Marvell USB HSIC PHY driver for Marvell
> + SoC. This driver will do the PHY initialization and shutdown.
> + The PHY driver will be used by Marvell ehci driver.
> +
> + To compile this driver as a module, choose M here.
> +
> config PHY_MV_USB2
> tristate "Marvell USB 2.0 28nm PHY Driver"
> select GENERIC_PHY
> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> index 768e55a..472bf0a 100644
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -10,6 +10,7 @@ obj-$(CONFIG_ARMADA375_USBCLUSTER_PHY) += phy-armada375-usb2.o
> obj-$(CONFIG_BCM_KONA_USB2_PHY) += phy-bcm-kona-usb2.o
> obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO) += phy-exynos-dp-video.o
> obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO) += phy-exynos-mipi-video.o
> +obj-$(CONFIG_PHY_MV_HSIC) += phy-mv-hsic.o
> obj-$(CONFIG_PHY_MV_USB2) += phy-mv-usb2.o
> obj-$(CONFIG_PHY_MVEBU_SATA) += phy-mvebu-sata.o
> obj-$(CONFIG_PHY_MIPHY28LP) += phy-miphy28lp.o
> diff --git a/drivers/phy/phy-mv-hsic.c b/drivers/phy/phy-mv-hsic.c
> new file mode 100644
> index 0000000..aa6cccd
> --- /dev/null
> +++ b/drivers/phy/phy-mv-hsic.c
> @@ -0,0 +1,208 @@
> +/*
> + * Copyright (C) 2015 Linaro, Ltd.
> + * Rob Herring <[email protected]>
> + *
> + * Based on vendor driver:
> + * Copyright (C) 2013 Marvell Inc.
> + * Author: Chao Xie <[email protected]>
> + *
> + * 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/delay.h>
> +#include <linux/slab.h>
> +#include <linux/of.h>
> +#include <linux/io.h>
> +#include <linux/err.h>
> +#include <linux/clk.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/phy/phy.h>
> +
> +#define PHY_28NM_HSIC_CTRL 0x08
> +#define PHY_28NM_HSIC_IMPCAL_CAL 0x18
> +#define PHY_28NM_HSIC_PLL_CTRL01 0x1c
> +#define PHY_28NM_HSIC_PLL_CTRL2 0x20
> +#define PHY_28NM_HSIC_INT 0x28
> +
> +#define PHY_28NM_HSIC_PLL_SELLPFR_SHIFT 26
> +#define PHY_28NM_HSIC_PLL_FBDIV_SHIFT 0
> +#define PHY_28NM_HSIC_PLL_REFDIV_SHIFT 9
> +
> +#define PHY_28NM_HSIC_S2H_PU_PLL BIT(10)
> +#define PHY_28NM_HSIC_H2S_PLL_LOCK BIT(15)
> +#define PHY_28NM_HSIC_S2H_HSIC_EN BIT(7)
> +#define S2H_DRV_SE0_4RESUME BIT(14)
> +#define PHY_28NM_HSIC_H2S_IMPCAL_DONE BIT(27)
> +
> +#define PHY_28NM_HSIC_CONNECT_INT BIT(1)
> +#define PHY_28NM_HSIC_HS_READY_INT BIT(2)
> +
> +struct mv_hsic_phy {
> + struct phy *phy;
> + struct platform_device *pdev;
> + void __iomem *base;
> + struct clk *clk;
> +};
> +
> +static bool wait_for_reg(void __iomem *reg, u32 mask, unsigned long timeout)
> +{
> + timeout += jiffies;
> + while (time_is_after_eq_jiffies(timeout)) {
> + if ((readl(reg) & mask) == mask)
> + return true;
> + msleep(1);
> + }
> + return false;
> +}
> +
> +static int mv_hsic_phy_init(struct phy *phy)
> +{
> + struct mv_hsic_phy *mv_phy = phy_get_drvdata(phy);
> + struct platform_device *pdev = mv_phy->pdev;
> + void __iomem *base = mv_phy->base;
> +
> + clk_prepare_enable(mv_phy->clk);
> +
> + /* Set reference clock */
> + writel(0x1 << PHY_28NM_HSIC_PLL_SELLPFR_SHIFT |
> + 0xf0 << PHY_28NM_HSIC_PLL_FBDIV_SHIFT |
> + 0xd << PHY_28NM_HSIC_PLL_REFDIV_SHIFT,
> + base + PHY_28NM_HSIC_PLL_CTRL01);
> +
> + /* Turn on PLL */
> + writel(readl(base + PHY_28NM_HSIC_PLL_CTRL2) |
> + PHY_28NM_HSIC_S2H_PU_PLL,
> + base + PHY_28NM_HSIC_PLL_CTRL2);
> +
> + /* Make sure PHY PLL is locked */
> + if (!wait_for_reg(base + PHY_28NM_HSIC_PLL_CTRL2,
> + PHY_28NM_HSIC_H2S_PLL_LOCK, HZ / 10)) {
> + dev_err(&pdev->dev, "HSIC PHY PLL not locked after 100mS.");
> + return -ETIMEDOUT;
> + }
> +
> + /* Avoid SE0 state when resume for some device will take it as reset */
> + writel(readl(base + PHY_28NM_HSIC_CTRL) & ~S2H_DRV_SE0_4RESUME,
> + base + PHY_28NM_HSIC_CTRL);
> +
> + /* Enable HSIC PHY */
> + writel(readl(base + PHY_28NM_HSIC_CTRL) | PHY_28NM_HSIC_S2H_HSIC_EN,
> + base + PHY_28NM_HSIC_CTRL);
> +
> + /* Calibration Timing
> + * ____________________________
> + * CAL START ___|
> + * ____________________
> + * CAL_DONE ___________|
> + * | 400us |
> + */
Please use the comment style used here
http://lxr.free-electrons.com/source/Documentation/CodingStyle#L464
> +
> + /* Make sure PHY Calibration is ready */
> + if (!wait_for_reg(base + PHY_28NM_HSIC_IMPCAL_CAL,
> + PHY_28NM_HSIC_H2S_IMPCAL_DONE, HZ / 10)) {
> + dev_warn(&pdev->dev, "HSIC PHY READY not set after 100mS.");
> + return -ETIMEDOUT;
> + }
> +
> + /* Waiting for HSIC connect int*/
> + if (!wait_for_reg(base + PHY_28NM_HSIC_INT,
> + PHY_28NM_HSIC_CONNECT_INT, HZ / 5)) {
> + dev_warn(&pdev->dev, "HSIC wait for connect interrupt timeout.");
> + return -ETIMEDOUT;
> + }
> +
> + return 0;
> +}
> +
> +static int mv_hsic_phy_power_on(struct phy *phy)
> +{
> + struct mv_hsic_phy *mv_phy = phy_get_drvdata(phy);
> + struct platform_device *pdev = mv_phy->pdev;
> +
> + /* check HS ready */
> + if (!wait_for_reg(mv_phy->base + PHY_28NM_HSIC_INT,
> + PHY_28NM_HSIC_HS_READY_INT, HZ / 10)) {
> + dev_err(&pdev->dev, "HSIC HS_READY not set\n");
> + return -ETIMEDOUT;
> + }
> + return 0;
> +}
> +
> +static int mv_hsic_phy_power_off(struct phy *phy)
> +{
> + struct mv_hsic_phy *mv_phy = phy_get_drvdata(phy);
> + void __iomem *base = mv_phy->base;
> +
> + writel(readl(base + PHY_28NM_HSIC_CTRL) & ~PHY_28NM_HSIC_S2H_HSIC_EN,
> + base + PHY_28NM_HSIC_CTRL);
> +
> + clk_disable_unprepare(mv_phy->clk);
> + return 0;
> +}
> +
> +static const struct phy_ops hsic_ops = {
> + .init = mv_hsic_phy_init,
> + .power_on = mv_hsic_phy_power_on,
> + .power_off = mv_hsic_phy_power_off,
exit callback is missing? Shouldn't we turn off the PLLs in exit callback?
Thanks
Kishon
On Thursday 21 May 2015 06:15 PM, Kishon Vijay Abraham I wrote:
> Hi,
>
> On Thursday 14 May 2015 04:18 AM, Rob Herring wrote:
>> Add PHY driver for the Marvell HSIC 28nm PHY. This PHY is found in PXA1928
>> SOC.
>>
>> Signed-off-by: Rob Herring <[email protected]>
>> Cc: Kishon Vijay Abraham I <[email protected]>
>> ---
>> drivers/phy/Kconfig | 10 +++
>> drivers/phy/Makefile | 1 +
>> drivers/phy/phy-mv-hsic.c | 208 ++++++++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 219 insertions(+)
>> create mode 100644 drivers/phy/phy-mv-hsic.c
>>
>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
>> index ef7634f..cfcae72 100644
>> --- a/drivers/phy/Kconfig
>> +++ b/drivers/phy/Kconfig
>> @@ -52,6 +52,16 @@ config PHY_EXYNOS_MIPI_VIDEO
>> Support for MIPI CSI-2 and MIPI DSI DPHY found on Samsung S5P
>> and EXYNOS SoCs.
>>
>> +config PHY_MV_HSIC
>> + tristate "Marvell USB HSIC 28nm PHY Driver"
>> + select GENERIC_PHY
>> + help
>> + Enable this to support Marvell USB HSIC PHY driver for Marvell
>> + SoC. This driver will do the PHY initialization and shutdown.
>> + The PHY driver will be used by Marvell ehci driver.
>> +
>> + To compile this driver as a module, choose M here.
>> +
>> config PHY_MV_USB2
>> tristate "Marvell USB 2.0 28nm PHY Driver"
>> select GENERIC_PHY
>> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
>> index 768e55a..472bf0a 100644
>> --- a/drivers/phy/Makefile
>> +++ b/drivers/phy/Makefile
>> @@ -10,6 +10,7 @@ obj-$(CONFIG_ARMADA375_USBCLUSTER_PHY) += phy-armada375-usb2.o
>> obj-$(CONFIG_BCM_KONA_USB2_PHY) += phy-bcm-kona-usb2.o
>> obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO) += phy-exynos-dp-video.o
>> obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO) += phy-exynos-mipi-video.o
>> +obj-$(CONFIG_PHY_MV_HSIC) += phy-mv-hsic.o
>> obj-$(CONFIG_PHY_MV_USB2) += phy-mv-usb2.o
>> obj-$(CONFIG_PHY_MVEBU_SATA) += phy-mvebu-sata.o
>> obj-$(CONFIG_PHY_MIPHY28LP) += phy-miphy28lp.o
>> diff --git a/drivers/phy/phy-mv-hsic.c b/drivers/phy/phy-mv-hsic.c
>> new file mode 100644
>> index 0000000..aa6cccd
>> --- /dev/null
>> +++ b/drivers/phy/phy-mv-hsic.c
>> @@ -0,0 +1,208 @@
>> +/*
>> + * Copyright (C) 2015 Linaro, Ltd.
>> + * Rob Herring <[email protected]>
>> + *
>> + * Based on vendor driver:
>> + * Copyright (C) 2013 Marvell Inc.
>> + * Author: Chao Xie <[email protected]>
>> + *
>> + * 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/delay.h>
>> +#include <linux/slab.h>
>> +#include <linux/of.h>
>> +#include <linux/io.h>
>> +#include <linux/err.h>
>> +#include <linux/clk.h>
>> +#include <linux/module.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/phy/phy.h>
>> +
>> +#define PHY_28NM_HSIC_CTRL 0x08
>> +#define PHY_28NM_HSIC_IMPCAL_CAL 0x18
>> +#define PHY_28NM_HSIC_PLL_CTRL01 0x1c
>> +#define PHY_28NM_HSIC_PLL_CTRL2 0x20
>> +#define PHY_28NM_HSIC_INT 0x28
>> +
>> +#define PHY_28NM_HSIC_PLL_SELLPFR_SHIFT 26
>> +#define PHY_28NM_HSIC_PLL_FBDIV_SHIFT 0
>> +#define PHY_28NM_HSIC_PLL_REFDIV_SHIFT 9
>> +
>> +#define PHY_28NM_HSIC_S2H_PU_PLL BIT(10)
>> +#define PHY_28NM_HSIC_H2S_PLL_LOCK BIT(15)
>> +#define PHY_28NM_HSIC_S2H_HSIC_EN BIT(7)
>> +#define S2H_DRV_SE0_4RESUME BIT(14)
>> +#define PHY_28NM_HSIC_H2S_IMPCAL_DONE BIT(27)
>> +
>> +#define PHY_28NM_HSIC_CONNECT_INT BIT(1)
>> +#define PHY_28NM_HSIC_HS_READY_INT BIT(2)
>> +
>> +struct mv_hsic_phy {
>> + struct phy *phy;
>> + struct platform_device *pdev;
>> + void __iomem *base;
>> + struct clk *clk;
>> +};
>> +
>> +static bool wait_for_reg(void __iomem *reg, u32 mask, unsigned long timeout)
>> +{
>> + timeout += jiffies;
>> + while (time_is_after_eq_jiffies(timeout)) {
>> + if ((readl(reg) & mask) == mask)
>> + return true;
>> + msleep(1);
>> + }
>> + return false;
>> +}
>> +
>> +static int mv_hsic_phy_init(struct phy *phy)
>> +{
>> + struct mv_hsic_phy *mv_phy = phy_get_drvdata(phy);
>> + struct platform_device *pdev = mv_phy->pdev;
>> + void __iomem *base = mv_phy->base;
>> +
>> + clk_prepare_enable(mv_phy->clk);
>> +
>> + /* Set reference clock */
>> + writel(0x1 << PHY_28NM_HSIC_PLL_SELLPFR_SHIFT |
>> + 0xf0 << PHY_28NM_HSIC_PLL_FBDIV_SHIFT |
>> + 0xd << PHY_28NM_HSIC_PLL_REFDIV_SHIFT,
>> + base + PHY_28NM_HSIC_PLL_CTRL01);
>> +
>> + /* Turn on PLL */
>> + writel(readl(base + PHY_28NM_HSIC_PLL_CTRL2) |
>> + PHY_28NM_HSIC_S2H_PU_PLL,
>> + base + PHY_28NM_HSIC_PLL_CTRL2);
>> +
>> + /* Make sure PHY PLL is locked */
>> + if (!wait_for_reg(base + PHY_28NM_HSIC_PLL_CTRL2,
>> + PHY_28NM_HSIC_H2S_PLL_LOCK, HZ / 10)) {
>> + dev_err(&pdev->dev, "HSIC PHY PLL not locked after 100mS.");
>> + return -ETIMEDOUT;
>> + }
>> +
>> + /* Avoid SE0 state when resume for some device will take it as reset */
>> + writel(readl(base + PHY_28NM_HSIC_CTRL) & ~S2H_DRV_SE0_4RESUME,
>> + base + PHY_28NM_HSIC_CTRL);
>> +
>> + /* Enable HSIC PHY */
>> + writel(readl(base + PHY_28NM_HSIC_CTRL) | PHY_28NM_HSIC_S2H_HSIC_EN,
>> + base + PHY_28NM_HSIC_CTRL);
>> +
>> + /* Calibration Timing
>> + * ____________________________
>> + * CAL START ___|
>> + * ____________________
>> + * CAL_DONE ___________|
>> + * | 400us |
>> + */
>
> Please use the comment style used here
> http://lxr.free-electrons.com/source/Documentation/CodingStyle#L464
>
>> +
>> + /* Make sure PHY Calibration is ready */
>> + if (!wait_for_reg(base + PHY_28NM_HSIC_IMPCAL_CAL,
>> + PHY_28NM_HSIC_H2S_IMPCAL_DONE, HZ / 10)) {
>> + dev_warn(&pdev->dev, "HSIC PHY READY not set after 100mS.");
>> + return -ETIMEDOUT;
>> + }
>> +
>> + /* Waiting for HSIC connect int*/
>> + if (!wait_for_reg(base + PHY_28NM_HSIC_INT,
>> + PHY_28NM_HSIC_CONNECT_INT, HZ / 5)) {
>> + dev_warn(&pdev->dev, "HSIC wait for connect interrupt timeout.");
>> + return -ETIMEDOUT;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int mv_hsic_phy_power_on(struct phy *phy)
>> +{
>> + struct mv_hsic_phy *mv_phy = phy_get_drvdata(phy);
>> + struct platform_device *pdev = mv_phy->pdev;
>> +
>> + /* check HS ready */
>> + if (!wait_for_reg(mv_phy->base + PHY_28NM_HSIC_INT,
>> + PHY_28NM_HSIC_HS_READY_INT, HZ / 10)) {
>> + dev_err(&pdev->dev, "HSIC HS_READY not set\n");
>> + return -ETIMEDOUT;
>> + }
>> + return 0;
>> +}
>> +
>> +static int mv_hsic_phy_power_off(struct phy *phy)
>> +{
>> + struct mv_hsic_phy *mv_phy = phy_get_drvdata(phy);
>> + void __iomem *base = mv_phy->base;
>> +
>> + writel(readl(base + PHY_28NM_HSIC_CTRL) & ~PHY_28NM_HSIC_S2H_HSIC_EN,
>> + base + PHY_28NM_HSIC_CTRL);
>> +
>> + clk_disable_unprepare(mv_phy->clk);
>> + return 0;
>> +}
>> +
>> +static const struct phy_ops hsic_ops = {
>> + .init = mv_hsic_phy_init,
>> + .power_on = mv_hsic_phy_power_on,
>> + .power_off = mv_hsic_phy_power_off,
>
> exit callback is missing? Shouldn't we turn off the PLLs in exit callback?
Also add the .owner member since this driver can be used as module.
Thanks
Kishon
On Thu, May 21, 2015 at 7:51 AM, Kishon Vijay Abraham I <[email protected]> wrote:
> On Thursday 21 May 2015 06:15 PM, Kishon Vijay Abraham I wrote:
>>
>> Hi,
>>
>> On Thursday 14 May 2015 04:18 AM, Rob Herring wrote:
>>>
>>> Add PHY driver for the Marvell HSIC 28nm PHY. This PHY is found in
>>> PXA1928
>>> SOC.
[...]
>>> + writel(readl(base + PHY_28NM_HSIC_CTRL) &
>>> ~PHY_28NM_HSIC_S2H_HSIC_EN,
>>> + base + PHY_28NM_HSIC_CTRL);
>>> +
>>> + clk_disable_unprepare(mv_phy->clk);
>>> + return 0;
>>> +}
>>> +
>>> +static const struct phy_ops hsic_ops = {
>>> + .init = mv_hsic_phy_init,
>>> + .power_on = mv_hsic_phy_power_on,
>>> + .power_off = mv_hsic_phy_power_off,
>>
>>
>> exit callback is missing? Shouldn't we turn off the PLLs in exit callback?
I really don't understand the division of the ops functions. It seems
backwards to me. Don't you need to power on the phy before you can
initialize it? Or init is supposed to be s/w init of some kind.
AFAICT, all the drivers just call init and power_on back to back.
>
> Also add the .owner member since this driver can be used as module.
Strange. Generally an ops struct just has ops.
Rob
Hi,
On Saturday 23 May 2015 01:24 AM, Rob Herring wrote:
> On Thu, May 21, 2015 at 7:51 AM, Kishon Vijay Abraham I <[email protected]> wrote:
>> On Thursday 21 May 2015 06:15 PM, Kishon Vijay Abraham I wrote:
>>>
>>> Hi,
>>>
>>> On Thursday 14 May 2015 04:18 AM, Rob Herring wrote:
>>>>
>>>> Add PHY driver for the Marvell HSIC 28nm PHY. This PHY is found in
>>>> PXA1928
>>>> SOC.
>
> [...]
>
>>>> + writel(readl(base + PHY_28NM_HSIC_CTRL) &
>>>> ~PHY_28NM_HSIC_S2H_HSIC_EN,
>>>> + base + PHY_28NM_HSIC_CTRL);
>>>> +
>>>> + clk_disable_unprepare(mv_phy->clk);
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static const struct phy_ops hsic_ops = {
>>>> + .init = mv_hsic_phy_init,
>>>> + .power_on = mv_hsic_phy_power_on,
>>>> + .power_off = mv_hsic_phy_power_off,
>>>
>>>
>>> exit callback is missing? Shouldn't we turn off the PLLs in exit callback?
>
> I really don't understand the division of the ops functions. It seems
> backwards to me. Don't you need to power on the phy before you can
> initialize it? Or init is supposed to be s/w init of some kind.
Generally the order is init and then power on. During init the driver can
perform the various setting for initializing the PHY which includes PLL
programming, calibration etc.. And then power on which actually starts the PHY
so that PHY can now transfer and receive data.
> AFAICT, all the drivers just call init and power_on back to back.
>
>>
>> Also add the .owner member since this driver can be used as module.
>
> Strange. Generally an ops struct just has ops.
phy-core has a call to try_module_get with this .owner so that the module is
not removed after phy_get. Maybe we can make phy-core to handle it in some
other way. Need to check.
Thanks
Kishon