* [PATCH 2/5] dt-bindings: Add Marvell PXA1928 USB EHCI controller binding
2015-05-13 22:48 [PATCH 0/5] Marvell PXA1928 USB support Rob Herring
@ 2015-05-13 22:48 ` Rob Herring
[not found] ` <1431557340-5421-1-git-send-email-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
` (2 subsequent siblings)
3 siblings, 0 replies; 24+ messages in thread
From: Rob Herring @ 2015-05-13 22:48 UTC (permalink / raw)
To: Greg Kroah-Hartman, Alan Stern, Kishon Vijay Abraham I
Cc: linux-kernel, devicetree, linux-usb, Rob Herring, Pawel Moll,
Mark Rutland, Ian Campbell, Kumar Gala
Add binding for Marvell PXA1928 SOC's USB EHCI host controller.
Signed-off-by: Rob Herring <robh@kernel.org>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: Kumar Gala <galak@codeaurora.org>
Cc: devicetree@vger.kernel.org
---
.../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
^ permalink raw reply related [flat|nested] 24+ messages in thread[parent not found: <1431557340-5421-1-git-send-email-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]
* [PATCH 1/5] dt-bindings: Add Marvell PXA1928 USB and HSIC PHY bindings
[not found] ` <1431557340-5421-1-git-send-email-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
@ 2015-05-13 22:48 ` Rob Herring
2015-05-13 22:48 ` [PATCH 3/5] phy: Add Marvell USB 2.0 OTG 28nm PHY Rob Herring
2015-05-15 9:55 ` [PATCH 0/5] Marvell PXA1928 USB support Sebastian Hesselbarth
2 siblings, 0 replies; 24+ messages in thread
From: Rob Herring @ 2015-05-13 22:48 UTC (permalink / raw)
To: Greg Kroah-Hartman, Alan Stern, Kishon Vijay Abraham I
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Pawel Moll,
Mark Rutland, Ian Campbell, Kumar Gala
Add PHY binding for Marvell PXA1928 SOC's USB and HSIC PHYs.
Signed-off-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Pawel Moll <pawel.moll-5wv7dgnIgG8@public.gmane.org>
Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
Cc: Ian Campbell <ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org>
Cc: Kumar Gala <galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
---
.../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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 3/5] phy: Add Marvell USB 2.0 OTG 28nm PHY
[not found] ` <1431557340-5421-1-git-send-email-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2015-05-13 22:48 ` [PATCH 1/5] dt-bindings: Add Marvell PXA1928 USB and HSIC PHY bindings Rob Herring
@ 2015-05-13 22:48 ` Rob Herring
[not found] ` <1431557340-5421-4-git-send-email-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2015-05-15 9:55 ` [PATCH 0/5] Marvell PXA1928 USB support Sebastian Hesselbarth
2 siblings, 1 reply; 24+ messages in thread
From: Rob Herring @ 2015-05-13 22:48 UTC (permalink / raw)
To: Greg Kroah-Hartman, Alan Stern, Kishon Vijay Abraham I
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA, Rob Herring
Add driver for USB PHY found in Marvell PXA1928 SOC.
Signed-off-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Kishon Vijay Abraham I <kishon-l0cyMroinI0@public.gmane.org>
---
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 <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
+ *
+ * Based on vendor driver:
+ * Copyright (C) 2013 Marvell Inc.
+ * Author: Chao Xie <xiechao.mail-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
+ *
+ * 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 <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>");
+MODULE_DESCRIPTION("Marvell USB2 phy driver");
+MODULE_LICENSE("GPL v2");
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH 0/5] Marvell PXA1928 USB support
[not found] ` <1431557340-5421-1-git-send-email-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2015-05-13 22:48 ` [PATCH 1/5] dt-bindings: Add Marvell PXA1928 USB and HSIC PHY bindings Rob Herring
2015-05-13 22:48 ` [PATCH 3/5] phy: Add Marvell USB 2.0 OTG 28nm PHY Rob Herring
@ 2015-05-15 9:55 ` Sebastian Hesselbarth
[not found] ` <5555C2A5.8010509-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2 siblings, 1 reply; 24+ messages in thread
From: Sebastian Hesselbarth @ 2015-05-15 9:55 UTC (permalink / raw)
To: Rob Herring, Greg Kroah-Hartman, Alan Stern,
Kishon Vijay Abraham I
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA, Jason Cooper, Andrew Lunn,
Gregory Clement, Thomas Petazzoni
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
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 4/5] phy: add Marvell HSIC 28nm PHY
2015-05-13 22:48 [PATCH 0/5] Marvell PXA1928 USB support Rob Herring
2015-05-13 22:48 ` [PATCH 2/5] dt-bindings: Add Marvell PXA1928 USB EHCI controller binding Rob Herring
[not found] ` <1431557340-5421-1-git-send-email-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
@ 2015-05-13 22:48 ` Rob Herring
2015-05-21 12:45 ` Kishon Vijay Abraham I
2015-05-13 22:49 ` [PATCH 5/5] usb: add pxa1928 ehci support Rob Herring
3 siblings, 1 reply; 24+ messages in thread
From: Rob Herring @ 2015-05-13 22:48 UTC (permalink / raw)
To: Greg Kroah-Hartman, Alan Stern, Kishon Vijay Abraham I
Cc: linux-kernel, devicetree, linux-usb, Rob Herring
Add PHY driver for the Marvell HSIC 28nm PHY. This PHY is found in PXA1928
SOC.
Signed-off-by: Rob Herring <robh@kernel.org>
Cc: Kishon Vijay Abraham I <kishon@ti.com>
---
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 <robh@kernel.org>
+ *
+ * Based on vendor driver:
+ * 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/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 <robh@kernel.org>");
+MODULE_DESCRIPTION("Marvell HSIC phy driver");
+MODULE_LICENSE("GPL v2");
--
2.1.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH 4/5] phy: add Marvell HSIC 28nm PHY
2015-05-13 22:48 ` [PATCH 4/5] phy: add Marvell HSIC 28nm PHY Rob Herring
@ 2015-05-21 12:45 ` Kishon Vijay Abraham I
2015-05-21 12:51 ` Kishon Vijay Abraham I
0 siblings, 1 reply; 24+ messages in thread
From: Kishon Vijay Abraham I @ 2015-05-21 12:45 UTC (permalink / raw)
To: Rob Herring, Greg Kroah-Hartman, Alan Stern
Cc: linux-kernel, devicetree, linux-usb
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 <robh@kernel.org>
> Cc: Kishon Vijay Abraham I <kishon@ti.com>
> ---
> 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 <robh@kernel.org>
> + *
> + * Based on vendor driver:
> + * 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/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
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 4/5] phy: add Marvell HSIC 28nm PHY
2015-05-21 12:45 ` Kishon Vijay Abraham I
@ 2015-05-21 12:51 ` Kishon Vijay Abraham I
[not found] ` <555DD4C2.80406-l0cyMroinI0@public.gmane.org>
0 siblings, 1 reply; 24+ messages in thread
From: Kishon Vijay Abraham I @ 2015-05-21 12:51 UTC (permalink / raw)
To: Rob Herring, Greg Kroah-Hartman, Alan Stern
Cc: linux-kernel, devicetree, linux-usb
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 <robh@kernel.org>
>> Cc: Kishon Vijay Abraham I <kishon@ti.com>
>> ---
>> 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 <robh@kernel.org>
>> + *
>> + * Based on vendor driver:
>> + * 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/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
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 5/5] usb: add pxa1928 ehci support
2015-05-13 22:48 [PATCH 0/5] Marvell PXA1928 USB support Rob Herring
` (2 preceding siblings ...)
2015-05-13 22:48 ` [PATCH 4/5] phy: add Marvell HSIC 28nm PHY Rob Herring
@ 2015-05-13 22:49 ` Rob Herring
2015-05-14 8:24 ` Paul Bolle
` (3 more replies)
3 siblings, 4 replies; 24+ messages in thread
From: Rob Herring @ 2015-05-13 22:49 UTC (permalink / raw)
To: Greg Kroah-Hartman, Alan Stern, Kishon Vijay Abraham I
Cc: linux-kernel, devicetree, linux-usb, Rob Herring
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 <robh@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: linux-usb@vger.kernel.org
---
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 <robh@kernel.org>
+ *
+ * Based on vendor driver modifications to ehci-mv.c:
+ * Copyright (C) 2011 Marvell International Ltd. All rights reserved.
+ * Author: Chao Xie <chao.xie@marvell.com>
+ * Neil Zhang <zhangwm@marvell.com>
+ *
+ * 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 <robh@kernel.org>");
+MODULE_LICENSE("GPL v2");
--
2.1.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH 5/5] usb: add pxa1928 ehci support
2015-05-13 22:49 ` [PATCH 5/5] usb: add pxa1928 ehci support Rob Herring
@ 2015-05-14 8:24 ` Paul Bolle
2015-05-14 10:42 ` Arnd Bergmann
2015-05-14 10:43 ` Arnd Bergmann
` (2 subsequent siblings)
3 siblings, 1 reply; 24+ messages in thread
From: Paul Bolle @ 2015-05-14 8:24 UTC (permalink / raw)
To: Rob Herring
Cc: Greg Kroah-Hartman, Alan Stern, Kishon Vijay Abraham I,
linux-kernel, devicetree, linux-usb
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 <robh@kernel.org>");
> +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
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 5/5] usb: add pxa1928 ehci support
2015-05-14 8:24 ` Paul Bolle
@ 2015-05-14 10:42 ` Arnd Bergmann
2015-05-14 13:25 ` Rob Herring
0 siblings, 1 reply; 24+ messages in thread
From: Arnd Bergmann @ 2015-05-14 10:42 UTC (permalink / raw)
To: Paul Bolle
Cc: Rob Herring, Greg Kroah-Hartman, Alan Stern,
Kishon Vijay Abraham I, linux-kernel, devicetree, linux-usb
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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 5/5] usb: add pxa1928 ehci support
2015-05-14 10:42 ` Arnd Bergmann
@ 2015-05-14 13:25 ` Rob Herring
0 siblings, 0 replies; 24+ messages in thread
From: Rob Herring @ 2015-05-14 13:25 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Paul Bolle, Greg Kroah-Hartman, Alan Stern,
Kishon Vijay Abraham I,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Linux USB List
On Thu, May 14, 2015 at 5:42 AM, Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> 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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 5/5] usb: add pxa1928 ehci support
2015-05-13 22:49 ` [PATCH 5/5] usb: add pxa1928 ehci support Rob Herring
2015-05-14 8:24 ` Paul Bolle
@ 2015-05-14 10:43 ` Arnd Bergmann
[not found] ` <1431557340-5421-6-git-send-email-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2015-05-14 15:55 ` Alan Stern
3 siblings, 0 replies; 24+ messages in thread
From: Arnd Bergmann @ 2015-05-14 10:43 UTC (permalink / raw)
To: Rob Herring
Cc: Greg Kroah-Hartman, Alan Stern, Kishon Vijay Abraham I,
linux-kernel, devicetree, linux-usb
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
^ permalink raw reply [flat|nested] 24+ messages in thread[parent not found: <1431557340-5421-6-git-send-email-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]
* Re: [PATCH 5/5] usb: add pxa1928 ehci support
[not found] ` <1431557340-5421-6-git-send-email-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
@ 2015-05-14 14:56 ` Alan Stern
[not found] ` <Pine.LNX.4.44L0.1505141025280.1582-100000-IYeN2dnnYyZXsRXLowluHWD2FQJk+8+b@public.gmane.org>
0 siblings, 1 reply; 24+ messages in thread
From: Alan Stern @ 2015-05-14 14:56 UTC (permalink / raw)
To: Rob Herring
Cc: Greg Kroah-Hartman, Kishon Vijay Abraham I,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA
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 <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Cc: Greg Kroah-Hartman <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>
> Cc: Alan Stern <stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz@public.gmane.org>
> Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> ---
> 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 <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> + *
> + * Based on vendor driver modifications to ehci-mv.c:
> + * Copyright (C) 2011 Marvell International Ltd. All rights reserved.
> + * Author: Chao Xie <chao.xie-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
> + * Neil Zhang <zhangwm-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
> + *
> + * 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 <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>");
> +MODULE_LICENSE("GPL v2");
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 5/5] usb: add pxa1928 ehci support
2015-05-13 22:49 ` [PATCH 5/5] usb: add pxa1928 ehci support Rob Herring
` (2 preceding siblings ...)
[not found] ` <1431557340-5421-6-git-send-email-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
@ 2015-05-14 15:55 ` Alan Stern
2015-05-15 9:32 ` Arnd Bergmann
3 siblings, 1 reply; 24+ messages in thread
From: Alan Stern @ 2015-05-14 15:55 UTC (permalink / raw)
To: Rob Herring
Cc: Greg Kroah-Hartman, Kishon Vijay Abraham I, linux-kernel,
devicetree, linux-usb
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
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 5/5] usb: add pxa1928 ehci support
2015-05-14 15:55 ` Alan Stern
@ 2015-05-15 9:32 ` Arnd Bergmann
0 siblings, 0 replies; 24+ messages in thread
From: Arnd Bergmann @ 2015-05-15 9:32 UTC (permalink / raw)
To: Alan Stern
Cc: Rob Herring, Greg Kroah-Hartman, Kishon Vijay Abraham I,
linux-kernel, devicetree, linux-usb
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
^ permalink raw reply [flat|nested] 24+ messages in thread