linux-usb.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [v2,05/10] phy: add A3700 UTMI PHY driver
@ 2019-01-11 13:31 Miquel Raynal
  0 siblings, 0 replies; 6+ messages in thread
From: Miquel Raynal @ 2019-01-11 13:31 UTC (permalink / raw)
  To: Kishon Vijay Abraham I, Gregory Clement, Jason Cooper,
	Andrew Lunn, Sebastian Hesselbarth, Rob Herring, Mark Rutland,
	Greg Kroah-Hartman, Mathias Nyman, Alan Stern
  Cc: devicetree, linux-arm-kernel, linux-usb, Thomas Petazzoni,
	Antoine Tenart, Maxime Chevallier, Nadav Haklai, Miquel Raynal,
	Igal Liberman

Marvell Armada 3700 SoC has two USB controllers, each of them being
wired to an internal UTMI PHY. Add a driver to control them.

Igal Liberman worked on supporting the PHY, I took the while 'register
configuration' from his work and rewrote almost entirely the
driver/bindings around it.

Co-developed-by: Igal Liberman <igall@marvell.com>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Signed-off-by: Igal Liberman <igall@marvell.com>
---
 drivers/phy/marvell/Kconfig                |   9 +
 drivers/phy/marvell/Makefile               |   1 +
 drivers/phy/marvell/phy-mvebu-a3700-utmi.c | 297 +++++++++++++++++++++
 3 files changed, 307 insertions(+)
 create mode 100644 drivers/phy/marvell/phy-mvebu-a3700-utmi.c

diff --git a/drivers/phy/marvell/Kconfig b/drivers/phy/marvell/Kconfig
index 9c90c0408ea3..b8e9dd38ad0d 100644
--- a/drivers/phy/marvell/Kconfig
+++ b/drivers/phy/marvell/Kconfig
@@ -33,6 +33,15 @@ config PHY_MVEBU_A3700_COMPHY
 	  shared serdes PHYs on Marvell Armada 3700. Its serdes lanes can be
 	  used by various controllers: Ethernet, SATA, USB3, PCIe.
 
+config PHY_MVEBU_A3700_UTMI
+	tristate "Marvell A3700 UTMI driver"
+	depends on ARCH_MVEBU || COMPILE_TEST
+	depends on OF
+	default y
+	select GENERIC_PHY
+	help
+	  Enable this to support Marvell A3700 UTMI PHY driver.
+
 config PHY_MVEBU_CP110_COMPHY
 	tristate "Marvell CP110 comphy driver"
 	depends on ARCH_MVEBU || COMPILE_TEST
diff --git a/drivers/phy/marvell/Makefile b/drivers/phy/marvell/Makefile
index c13a0c8ab6f0..82f291cf59ee 100644
--- a/drivers/phy/marvell/Makefile
+++ b/drivers/phy/marvell/Makefile
@@ -3,6 +3,7 @@ obj-$(CONFIG_ARMADA375_USBCLUSTER_PHY)	+= phy-armada375-usb2.o
 obj-$(CONFIG_PHY_BERLIN_SATA)		+= phy-berlin-sata.o
 obj-$(CONFIG_PHY_BERLIN_USB)		+= phy-berlin-usb.o
 obj-$(CONFIG_PHY_MVEBU_A3700_COMPHY)	+= phy-mvebu-a3700-comphy.o
+obj-$(CONFIG_PHY_MVEBU_A3700_UTMI)	+= phy-mvebu-a3700-utmi.o
 obj-$(CONFIG_PHY_MVEBU_CP110_COMPHY)	+= phy-mvebu-cp110-comphy.o
 obj-$(CONFIG_PHY_MVEBU_SATA)		+= phy-mvebu-sata.o
 obj-$(CONFIG_PHY_PXA_28NM_HSIC)		+= phy-pxa-28nm-hsic.o
diff --git a/drivers/phy/marvell/phy-mvebu-a3700-utmi.c b/drivers/phy/marvell/phy-mvebu-a3700-utmi.c
new file mode 100644
index 000000000000..97d8235d661d
--- /dev/null
+++ b/drivers/phy/marvell/phy-mvebu-a3700-utmi.c
@@ -0,0 +1,297 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2018 Marvell
+ *
+ * Authors:
+ *   Evan Wang <xswang@marvell.com>
+ *   Miquèl Raynal <miquel.raynal@bootlin.com>
+ *
+ * Marvell A3700 UTMI PHY driver
+ */
+
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+/* Armada 3700 UTMI PHY registers */
+#define USB2_PHY_PLL_CTRL_REG0			0x0
+#define   PLL_REF_DIV_OFF			0
+#define   PLL_REF_DIV_MASK			(0x7F << PLL_REF_DIV_OFF)
+#define   PLL_REF_DIV_5				0x5
+#define   PLL_FB_DIV_OFF			16
+#define   PLL_FB_DIV_MASK			(0x1FF << PLL_FB_DIV_OFF)
+#define   PLL_FB_DIV_96				96
+#define   PLL_SEL_LPFR_OFF			28
+#define   PLL_SEL_LPFR_MASK			(0x3 << PLL_SEL_LPFR_OFF)
+#define   PLL_READY				BIT(31)
+#define USB2_PHY_CAL_CTRL			0x8
+#define   PHY_PLLCAL_DONE			BIT(31)
+#define   PHY_IMPCAL_DONE			BIT(23)
+#define USB2_RX_CHAN_CTRL1			0x18
+#define   USB2PHY_SQCAL_DONE			BIT(31)
+#define USB2_PHY_OTG_CTRL			0x34
+#define   PHY_PU_OTG				BIT(4)
+#define USB2_PHY_CHRGR_DETECT			0x38
+#define   PHY_CDP_EN				BIT(2)
+#define   PHY_DCP_EN				BIT(3)
+#define   PHY_PD_EN				BIT(4)
+#define   PHY_PU_CHRG_DTC			BIT(5)
+#define   PHY_CDP_DM_AUTO			BIT(7)
+#define   PHY_ENSWITCH_DP			BIT(12)
+#define   PHY_ENSWITCH_DM			BIT(13)
+
+/* Armada 3700 USB miscellaneous registers */
+#define USB2_PHY_CTRL(usb32)			(usb32 ? 0x20 : 0x4)
+#define   RB_USB2PHY_PU				BIT(0)
+#define   USB2_DP_PULLDN_DEV_MODE		BIT(5)
+#define   USB2_DM_PULLDN_DEV_MODE		BIT(6)
+#define   RB_USB2PHY_SUSPM(usb32)		(usb32 ? BIT(14) : BIT(7))
+
+#define PLL_LOCK_DELAY_US			10000
+#define PLL_LOCK_TIMEOUT_US			1000000
+
+/**
+ * struct mvebu_a3700_utmi_caps - PHY capabilities
+ *
+ * @usb32: Flag indicating which PHY is in use (impacts the register map):
+ *           - The UTMI PHY wired to the USB3/USB2 controller (otg)
+ *           - The UTMI PHY wired to the USB2 controller (host only)
+ * @ops: PHY operations
+ */
+struct mvebu_a3700_utmi_caps {
+	int usb32;
+	const struct phy_ops *ops;
+};
+
+/**
+ * struct mvebu_a3700_utmi - PHY driver data
+ *
+ * @regs: PHY registers
+ * @usb_mis: Regmap with USB miscellaneous registers including PHY ones
+ * @caps: PHY capabilities
+ * @phy: PHY handle
+ */
+struct mvebu_a3700_utmi {
+	void __iomem *regs;
+	struct regmap *usb_misc;
+	const struct mvebu_a3700_utmi_caps *caps;
+	struct phy *phy;
+};
+
+static int mvebu_a3700_utmi_phy_power_on(struct phy *phy)
+{
+	struct mvebu_a3700_utmi *utmi = phy_get_drvdata(phy);
+	struct device *dev = &phy->dev;
+	int usb32 = utmi->caps->usb32;
+	int ret = 0;
+	u32 reg;
+
+	if (!utmi)
+		return -ENODEV;
+
+	/*
+	 * Setup PLL. 40MHz clock used to be the default, bein 25MHz now.
+	 * See "PLL Settings for Typical REFCLK" table.
+	 */
+	reg = readl(utmi->regs + USB2_PHY_PLL_CTRL_REG0);
+	reg &= ~(PLL_REF_DIV_MASK | PLL_FB_DIV_MASK | PLL_SEL_LPFR_MASK);
+	reg |= (PLL_REF_DIV_5 << PLL_REF_DIV_OFF |
+		PLL_FB_DIV_96 << PLL_FB_DIV_OFF);
+	writel(reg, utmi->regs + USB2_PHY_PLL_CTRL_REG0);
+
+	/* Enable PHY pull up and disable USB2 suspend */
+	regmap_update_bits(utmi->usb_misc, USB2_PHY_CTRL(usb32),
+			   RB_USB2PHY_SUSPM(usb32) | RB_USB2PHY_PU,
+			   RB_USB2PHY_SUSPM(usb32) | RB_USB2PHY_PU);
+
+	if (usb32) {
+		/* Power up OTG module */
+		reg = readl(utmi->regs + USB2_PHY_OTG_CTRL);
+		reg |= PHY_PU_OTG;
+		writel(reg, utmi->regs + USB2_PHY_OTG_CTRL);
+
+		/* Disable PHY charger detection */
+		reg = readl(utmi->regs + USB2_PHY_CHRGR_DETECT);
+		reg &= ~(PHY_CDP_EN | PHY_DCP_EN | PHY_PD_EN | PHY_PU_CHRG_DTC |
+			 PHY_CDP_DM_AUTO | PHY_ENSWITCH_DP | PHY_ENSWITCH_DM);
+		writel(reg, utmi->regs + USB2_PHY_CHRGR_DETECT);
+
+		/* Disable PHY DP/DM pull-down (used for device mode) */
+		regmap_update_bits(utmi->usb_misc, USB2_PHY_CTRL(usb32),
+				   USB2_DP_PULLDN_DEV_MODE |
+				   USB2_DM_PULLDN_DEV_MODE, 0);
+	}
+
+	/* Wait for PLL calibration */
+	ret = readl_poll_timeout(utmi->regs + USB2_PHY_CAL_CTRL, reg,
+				 reg & PHY_PLLCAL_DONE,
+				 PLL_LOCK_DELAY_US, PLL_LOCK_TIMEOUT_US);
+	if (ret) {
+		dev_err(dev, "Failed to end USB2 PLL calibration\n");
+		return ret;
+	}
+
+	/* Wait for impedance calibration */
+	ret = readl_poll_timeout(utmi->regs + USB2_PHY_CAL_CTRL, reg,
+				 reg & PHY_IMPCAL_DONE,
+				 PLL_LOCK_DELAY_US, PLL_LOCK_TIMEOUT_US);
+	if (ret) {
+		dev_err(dev, "Failed to end USB2 impedance calibration\n");
+		return ret;
+	}
+
+	/* Wait for squetch calibration */
+	ret = readl_poll_timeout(utmi->regs + USB2_RX_CHAN_CTRL1, reg,
+				 reg & USB2PHY_SQCAL_DONE,
+				 PLL_LOCK_DELAY_US, PLL_LOCK_TIMEOUT_US);
+	if (ret) {
+		dev_err(dev, "Failed to end USB2 unknown calibration\n");
+		return ret;
+	}
+
+	/* Wait for PLL to be locked */
+	ret = readl_poll_timeout(utmi->regs + USB2_PHY_PLL_CTRL_REG0, reg,
+				 reg & PLL_READY,
+				 PLL_LOCK_DELAY_US, PLL_LOCK_TIMEOUT_US);
+	if (ret)
+		dev_err(dev, "Failed to lock USB2 PLL\n");
+
+	return ret;
+}
+
+static int mvebu_a3700_utmi_phy_power_off(struct phy *phy)
+{
+	struct mvebu_a3700_utmi *utmi = phy_get_drvdata(phy);
+	int usb32 = utmi->caps->usb32;
+	u32 reg;
+
+	if (!utmi)
+		return -ENODEV;
+
+	/* Disable PHY pull-up and enable USB2 suspend */
+	reg = readl(utmi->regs + USB2_PHY_CTRL(usb32));
+	reg &= ~(RB_USB2PHY_PU | RB_USB2PHY_SUSPM(usb32));
+	writel(reg, utmi->regs + USB2_PHY_CTRL(usb32));
+
+	/* Power down OTG module */
+	if (usb32) {
+		reg = readl(utmi->regs + USB2_PHY_OTG_CTRL);
+		reg &= ~PHY_PU_OTG;
+		writel(reg, utmi->regs + USB2_PHY_OTG_CTRL);
+	}
+
+	return 0;
+}
+
+static const struct phy_ops mvebu_a3700_utmi_phy_ops = {
+	.power_on = mvebu_a3700_utmi_phy_power_on,
+	.power_off = mvebu_a3700_utmi_phy_power_off,
+	.owner = THIS_MODULE,
+};
+
+static const struct mvebu_a3700_utmi_caps mvebu_a3700_utmi_otg_phy_caps = {
+	.usb32 = true,
+	.ops = &mvebu_a3700_utmi_phy_ops,
+};
+
+static const struct mvebu_a3700_utmi_caps mvebu_a3700_utmi_host_phy_caps = {
+	.usb32 = false,
+	.ops = &mvebu_a3700_utmi_phy_ops,
+};
+
+static const struct of_device_id mvebu_a3700_utmi_of_match[] = {
+	{
+		.compatible = "marvell,a3700-utmi-otg-phy",
+		.data = &mvebu_a3700_utmi_otg_phy_caps,
+	},
+	{
+		.compatible = "marvell,a3700-utmi-host-phy",
+		.data = &mvebu_a3700_utmi_host_phy_caps,
+	},
+	{},
+};
+MODULE_DEVICE_TABLE(of, mvebu_a3700_utmi_of_match);
+
+static int mvebu_a3700_utmi_phy_probe(struct platform_device *pdev)
+{
+	const struct of_device_id *dev_id;
+	struct device *dev = &pdev->dev;
+	struct mvebu_a3700_utmi *utmi;
+	struct phy_provider *provider;
+	const struct phy_ops *ops;
+	struct resource *res;
+
+	utmi = devm_kzalloc(dev, sizeof(*utmi), GFP_KERNEL);
+	if (!utmi)
+		return -ENOMEM;
+
+	/* Get UTMI memory region */
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		dev_err(dev, "Missing UTMI PHY memory resource\n");
+		return -ENODEV;
+	}
+
+	utmi->regs = devm_ioremap_resource(dev, res);
+	if (IS_ERR(utmi->regs))
+		return PTR_ERR(utmi->regs);
+
+	/* Get miscellaneous Host/PHY region */
+	utmi->usb_misc = syscon_regmap_lookup_by_phandle(dev->of_node,
+							 "marvell,usb-misc-reg");
+	if (IS_ERR(utmi->usb_misc)) {
+		dev_err(dev,
+			"Missing USB misc purpose system controller\n");
+		return PTR_ERR(utmi->usb_misc);
+	}
+
+	/* Retrieve PHY capabilities */
+	dev_id = of_match_device(mvebu_a3700_utmi_of_match, dev);
+	if (!dev_id)
+		return -EINVAL;
+
+	utmi->caps = dev_id->data;
+	ops = utmi->caps->ops;
+
+	/* Instantiate the PHY */
+	utmi->phy = devm_phy_create(dev, NULL, ops);
+	if (IS_ERR(utmi->phy)) {
+		dev_err(dev, "Failed to create the UTMI PHY\n");
+		return PTR_ERR(utmi->phy);
+	}
+
+	phy_set_drvdata(utmi->phy, utmi);
+
+	/* Ensure the PHY is powered off */
+	ops->power_off(utmi->phy);
+
+	provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
+
+	return PTR_ERR_OR_ZERO(provider);
+}
+
+static int mvebu_a3700_utmi_phy_remove(struct platform_device *pdev)
+{
+	return 0;
+}
+
+static struct platform_driver mvebu_a3700_utmi_driver = {
+	.probe	= mvebu_a3700_utmi_phy_probe,
+	.remove = mvebu_a3700_utmi_phy_remove,
+	.driver	= {
+		.name		= "mvebu-a3700-utmi-phy",
+		.owner		= THIS_MODULE,
+		.of_match_table	= mvebu_a3700_utmi_of_match,
+	 },
+};
+module_platform_driver(mvebu_a3700_utmi_driver);
+
+MODULE_AUTHOR("Evan Wang <xswang@marvell.com>");
+MODULE_AUTHOR("Miquel Raynal <miquel.raynal@bootlin.com>");
+MODULE_DESCRIPTION("Marvell EBU A3700 UTMI PHY driver");
+MODULE_LICENSE("GPL");

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [v2,05/10] phy: add A3700 UTMI PHY driver
@ 2019-01-15  2:40 Chunfeng Yun
  0 siblings, 0 replies; 6+ messages in thread
From: Chunfeng Yun @ 2019-01-15  2:40 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Kishon Vijay Abraham I, Gregory Clement, Jason Cooper,
	Andrew Lunn, Sebastian Hesselbarth, Rob Herring, Mark Rutland,
	Greg Kroah-Hartman, Mathias Nyman, Alan Stern, devicetree,
	linux-arm-kernel, linux-usb, Thomas Petazzoni, Antoine Tenart,
	Maxime Chevallier, Nadav Haklai, Igal Liberman

Hi,
On Fri, 2019-01-11 at 14:31 +0100, Miquel Raynal wrote:
> Marvell Armada 3700 SoC has two USB controllers, each of them being
> wired to an internal UTMI PHY. Add a driver to control them.
> 
> Igal Liberman worked on supporting the PHY, I took the while 'register
> configuration' from his work and rewrote almost entirely the
> driver/bindings around it.
> 
> Co-developed-by: Igal Liberman <igall@marvell.com>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> Signed-off-by: Igal Liberman <igall@marvell.com>
> ---
>  drivers/phy/marvell/Kconfig                |   9 +
>  drivers/phy/marvell/Makefile               |   1 +
>  drivers/phy/marvell/phy-mvebu-a3700-utmi.c | 297 +++++++++++++++++++++
>  3 files changed, 307 insertions(+)
>  create mode 100644 drivers/phy/marvell/phy-mvebu-a3700-utmi.c
> 
> diff --git a/drivers/phy/marvell/Kconfig b/drivers/phy/marvell/Kconfig
> index 9c90c0408ea3..b8e9dd38ad0d 100644
> --- a/drivers/phy/marvell/Kconfig
> +++ b/drivers/phy/marvell/Kconfig
> @@ -33,6 +33,15 @@ config PHY_MVEBU_A3700_COMPHY
>  	  shared serdes PHYs on Marvell Armada 3700. Its serdes lanes can be
>  	  used by various controllers: Ethernet, SATA, USB3, PCIe.
>  
> +config PHY_MVEBU_A3700_UTMI
> +	tristate "Marvell A3700 UTMI driver"
> +	depends on ARCH_MVEBU || COMPILE_TEST
> +	depends on OF
> +	default y
> +	select GENERIC_PHY
> +	help
> +	  Enable this to support Marvell A3700 UTMI PHY driver.
> +
>  config PHY_MVEBU_CP110_COMPHY
>  	tristate "Marvell CP110 comphy driver"
>  	depends on ARCH_MVEBU || COMPILE_TEST
> diff --git a/drivers/phy/marvell/Makefile b/drivers/phy/marvell/Makefile
> index c13a0c8ab6f0..82f291cf59ee 100644
> --- a/drivers/phy/marvell/Makefile
> +++ b/drivers/phy/marvell/Makefile
> @@ -3,6 +3,7 @@ obj-$(CONFIG_ARMADA375_USBCLUSTER_PHY)	+= phy-armada375-usb2.o
>  obj-$(CONFIG_PHY_BERLIN_SATA)		+= phy-berlin-sata.o
>  obj-$(CONFIG_PHY_BERLIN_USB)		+= phy-berlin-usb.o
>  obj-$(CONFIG_PHY_MVEBU_A3700_COMPHY)	+= phy-mvebu-a3700-comphy.o
> +obj-$(CONFIG_PHY_MVEBU_A3700_UTMI)	+= phy-mvebu-a3700-utmi.o
>  obj-$(CONFIG_PHY_MVEBU_CP110_COMPHY)	+= phy-mvebu-cp110-comphy.o
>  obj-$(CONFIG_PHY_MVEBU_SATA)		+= phy-mvebu-sata.o
>  obj-$(CONFIG_PHY_PXA_28NM_HSIC)		+= phy-pxa-28nm-hsic.o
> diff --git a/drivers/phy/marvell/phy-mvebu-a3700-utmi.c b/drivers/phy/marvell/phy-mvebu-a3700-utmi.c
> new file mode 100644
> index 000000000000..97d8235d661d
> --- /dev/null
> +++ b/drivers/phy/marvell/phy-mvebu-a3700-utmi.c
> @@ -0,0 +1,297 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2018 Marvell
> + *
> + * Authors:
> + *   Evan Wang <xswang@marvell.com>
> + *   Miquèl Raynal <miquel.raynal@bootlin.com>
> + *
> + * Marvell A3700 UTMI PHY driver
> + */
> +
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +
> +/* Armada 3700 UTMI PHY registers */
> +#define USB2_PHY_PLL_CTRL_REG0			0x0
> +#define   PLL_REF_DIV_OFF			0
> +#define   PLL_REF_DIV_MASK			(0x7F << PLL_REF_DIV_OFF)
use GENMASK?
> +#define   PLL_REF_DIV_5				0x5
> +#define   PLL_FB_DIV_OFF			16
> +#define   PLL_FB_DIV_MASK			(0x1FF << PLL_FB_DIV_OFF)
> +#define   PLL_FB_DIV_96				96
> +#define   PLL_SEL_LPFR_OFF			28
> +#define   PLL_SEL_LPFR_MASK			(0x3 << PLL_SEL_LPFR_OFF)
> +#define   PLL_READY				BIT(31)
> +#define USB2_PHY_CAL_CTRL			0x8
> +#define   PHY_PLLCAL_DONE			BIT(31)
> +#define   PHY_IMPCAL_DONE			BIT(23)
> +#define USB2_RX_CHAN_CTRL1			0x18
> +#define   USB2PHY_SQCAL_DONE			BIT(31)
> +#define USB2_PHY_OTG_CTRL			0x34
> +#define   PHY_PU_OTG				BIT(4)
> +#define USB2_PHY_CHRGR_DETECT			0x38
> +#define   PHY_CDP_EN				BIT(2)
> +#define   PHY_DCP_EN				BIT(3)
> +#define   PHY_PD_EN				BIT(4)
> +#define   PHY_PU_CHRG_DTC			BIT(5)
> +#define   PHY_CDP_DM_AUTO			BIT(7)
> +#define   PHY_ENSWITCH_DP			BIT(12)
> +#define   PHY_ENSWITCH_DM			BIT(13)
> +
> +/* Armada 3700 USB miscellaneous registers */
> +#define USB2_PHY_CTRL(usb32)			(usb32 ? 0x20 : 0x4)
> +#define   RB_USB2PHY_PU				BIT(0)
> +#define   USB2_DP_PULLDN_DEV_MODE		BIT(5)
> +#define   USB2_DM_PULLDN_DEV_MODE		BIT(6)
> +#define   RB_USB2PHY_SUSPM(usb32)		(usb32 ? BIT(14) : BIT(7))
> +
> +#define PLL_LOCK_DELAY_US			10000
> +#define PLL_LOCK_TIMEOUT_US			1000000
> +
> +/**
> + * struct mvebu_a3700_utmi_caps - PHY capabilities
> + *
> + * @usb32: Flag indicating which PHY is in use (impacts the register map):
> + *           - The UTMI PHY wired to the USB3/USB2 controller (otg)
> + *           - The UTMI PHY wired to the USB2 controller (host only)
> + * @ops: PHY operations
> + */
> +struct mvebu_a3700_utmi_caps {
> +	int usb32;
> +	const struct phy_ops *ops;
> +};
> +
> +/**
> + * struct mvebu_a3700_utmi - PHY driver data
> + *
> + * @regs: PHY registers
> + * @usb_mis: Regmap with USB miscellaneous registers including PHY ones
> + * @caps: PHY capabilities
> + * @phy: PHY handle
> + */
> +struct mvebu_a3700_utmi {
> +	void __iomem *regs;
> +	struct regmap *usb_misc;
> +	const struct mvebu_a3700_utmi_caps *caps;
> +	struct phy *phy;
> +};
> +
> +static int mvebu_a3700_utmi_phy_power_on(struct phy *phy)
> +{
> +	struct mvebu_a3700_utmi *utmi = phy_get_drvdata(phy);
> +	struct device *dev = &phy->dev;
> +	int usb32 = utmi->caps->usb32;
> +	int ret = 0;
> +	u32 reg;
> +
> +	if (!utmi)
> +		return -ENODEV;
the check here may be not necessary
> +
> +	/*
> +	 * Setup PLL. 40MHz clock used to be the default, bein 25MHz now.
> +	 * See "PLL Settings for Typical REFCLK" table.
> +	 */
> +	reg = readl(utmi->regs + USB2_PHY_PLL_CTRL_REG0);
> +	reg &= ~(PLL_REF_DIV_MASK | PLL_FB_DIV_MASK | PLL_SEL_LPFR_MASK);
> +	reg |= (PLL_REF_DIV_5 << PLL_REF_DIV_OFF |
> +		PLL_FB_DIV_96 << PLL_FB_DIV_OFF);
> +	writel(reg, utmi->regs + USB2_PHY_PLL_CTRL_REG0);
> +
> +	/* Enable PHY pull up and disable USB2 suspend */
> +	regmap_update_bits(utmi->usb_misc, USB2_PHY_CTRL(usb32),
> +			   RB_USB2PHY_SUSPM(usb32) | RB_USB2PHY_PU,
> +			   RB_USB2PHY_SUSPM(usb32) | RB_USB2PHY_PU);
> +
> +	if (usb32) {
> +		/* Power up OTG module */
> +		reg = readl(utmi->regs + USB2_PHY_OTG_CTRL);
> +		reg |= PHY_PU_OTG;
> +		writel(reg, utmi->regs + USB2_PHY_OTG_CTRL);
> +
> +		/* Disable PHY charger detection */
> +		reg = readl(utmi->regs + USB2_PHY_CHRGR_DETECT);
> +		reg &= ~(PHY_CDP_EN | PHY_DCP_EN | PHY_PD_EN | PHY_PU_CHRG_DTC |
> +			 PHY_CDP_DM_AUTO | PHY_ENSWITCH_DP | PHY_ENSWITCH_DM);
> +		writel(reg, utmi->regs + USB2_PHY_CHRGR_DETECT);
> +
> +		/* Disable PHY DP/DM pull-down (used for device mode) */
> +		regmap_update_bits(utmi->usb_misc, USB2_PHY_CTRL(usb32),
> +				   USB2_DP_PULLDN_DEV_MODE |
> +				   USB2_DM_PULLDN_DEV_MODE, 0);
> +	}
> +
> +	/* Wait for PLL calibration */
> +	ret = readl_poll_timeout(utmi->regs + USB2_PHY_CAL_CTRL, reg,
> +				 reg & PHY_PLLCAL_DONE,
> +				 PLL_LOCK_DELAY_US, PLL_LOCK_TIMEOUT_US);
> +	if (ret) {
> +		dev_err(dev, "Failed to end USB2 PLL calibration\n");
> +		return ret;
> +	}
> +
> +	/* Wait for impedance calibration */
> +	ret = readl_poll_timeout(utmi->regs + USB2_PHY_CAL_CTRL, reg,
> +				 reg & PHY_IMPCAL_DONE,
> +				 PLL_LOCK_DELAY_US, PLL_LOCK_TIMEOUT_US);
> +	if (ret) {
> +		dev_err(dev, "Failed to end USB2 impedance calibration\n");
> +		return ret;
> +	}
> +
> +	/* Wait for squetch calibration */
> +	ret = readl_poll_timeout(utmi->regs + USB2_RX_CHAN_CTRL1, reg,
> +				 reg & USB2PHY_SQCAL_DONE,
> +				 PLL_LOCK_DELAY_US, PLL_LOCK_TIMEOUT_US);
> +	if (ret) {
> +		dev_err(dev, "Failed to end USB2 unknown calibration\n");
> +		return ret;
> +	}
> +
> +	/* Wait for PLL to be locked */
> +	ret = readl_poll_timeout(utmi->regs + USB2_PHY_PLL_CTRL_REG0, reg,
> +				 reg & PLL_READY,
> +				 PLL_LOCK_DELAY_US, PLL_LOCK_TIMEOUT_US);
> +	if (ret)
> +		dev_err(dev, "Failed to lock USB2 PLL\n");
> +
> +	return ret;
> +}
> +
> +static int mvebu_a3700_utmi_phy_power_off(struct phy *phy)
> +{
> +	struct mvebu_a3700_utmi *utmi = phy_get_drvdata(phy);
> +	int usb32 = utmi->caps->usb32;
> +	u32 reg;
> +
> +	if (!utmi)
> +		return -ENODEV;
> +
> +	/* Disable PHY pull-up and enable USB2 suspend */
> +	reg = readl(utmi->regs + USB2_PHY_CTRL(usb32));
> +	reg &= ~(RB_USB2PHY_PU | RB_USB2PHY_SUSPM(usb32));
> +	writel(reg, utmi->regs + USB2_PHY_CTRL(usb32));
> +
> +	/* Power down OTG module */
> +	if (usb32) {
> +		reg = readl(utmi->regs + USB2_PHY_OTG_CTRL);
> +		reg &= ~PHY_PU_OTG;
> +		writel(reg, utmi->regs + USB2_PHY_OTG_CTRL);
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct phy_ops mvebu_a3700_utmi_phy_ops = {
> +	.power_on = mvebu_a3700_utmi_phy_power_on,
> +	.power_off = mvebu_a3700_utmi_phy_power_off,
> +	.owner = THIS_MODULE,
> +};
> +
> +static const struct mvebu_a3700_utmi_caps mvebu_a3700_utmi_otg_phy_caps = {
> +	.usb32 = true,
> +	.ops = &mvebu_a3700_utmi_phy_ops,
> +};
> +
> +static const struct mvebu_a3700_utmi_caps mvebu_a3700_utmi_host_phy_caps = {
> +	.usb32 = false,
> +	.ops = &mvebu_a3700_utmi_phy_ops,
> +};
> +
> +static const struct of_device_id mvebu_a3700_utmi_of_match[] = {
> +	{
> +		.compatible = "marvell,a3700-utmi-otg-phy",
> +		.data = &mvebu_a3700_utmi_otg_phy_caps,
> +	},
> +	{
> +		.compatible = "marvell,a3700-utmi-host-phy",
> +		.data = &mvebu_a3700_utmi_host_phy_caps,
> +	},
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, mvebu_a3700_utmi_of_match);
> +
> +static int mvebu_a3700_utmi_phy_probe(struct platform_device *pdev)
> +{
> +	const struct of_device_id *dev_id;
> +	struct device *dev = &pdev->dev;
> +	struct mvebu_a3700_utmi *utmi;
> +	struct phy_provider *provider;
> +	const struct phy_ops *ops;
> +	struct resource *res;
> +
> +	utmi = devm_kzalloc(dev, sizeof(*utmi), GFP_KERNEL);
> +	if (!utmi)
> +		return -ENOMEM;
> +
> +	/* Get UTMI memory region */
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res) {
> +		dev_err(dev, "Missing UTMI PHY memory resource\n");
> +		return -ENODEV;
> +	}
> +
> +	utmi->regs = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(utmi->regs))
> +		return PTR_ERR(utmi->regs);
> +
> +	/* Get miscellaneous Host/PHY region */
> +	utmi->usb_misc = syscon_regmap_lookup_by_phandle(dev->of_node,
> +							 "marvell,usb-misc-reg");
> +	if (IS_ERR(utmi->usb_misc)) {
> +		dev_err(dev,
> +			"Missing USB misc purpose system controller\n");
> +		return PTR_ERR(utmi->usb_misc);
> +	}
> +
> +	/* Retrieve PHY capabilities */
> +	dev_id = of_match_device(mvebu_a3700_utmi_of_match, dev);
> +	if (!dev_id)
> +		return -EINVAL;
> +
> +	utmi->caps = dev_id->data;
a useful API to get @data: of_device_get_match_data()

> +	ops = utmi->caps->ops;
> +
> +	/* Instantiate the PHY */
> +	utmi->phy = devm_phy_create(dev, NULL, ops);
> +	if (IS_ERR(utmi->phy)) {
> +		dev_err(dev, "Failed to create the UTMI PHY\n");
> +		return PTR_ERR(utmi->phy);
> +	}
> +
> +	phy_set_drvdata(utmi->phy, utmi);
> +
> +	/* Ensure the PHY is powered off */
> +	ops->power_off(utmi->phy);
> +
> +	provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
> +
> +	return PTR_ERR_OR_ZERO(provider);
> +}
> +
> +static int mvebu_a3700_utmi_phy_remove(struct platform_device *pdev)
> +{
> +	return 0;
> +}
> +
> +static struct platform_driver mvebu_a3700_utmi_driver = {
> +	.probe	= mvebu_a3700_utmi_phy_probe,
> +	.remove = mvebu_a3700_utmi_phy_remove,
> +	.driver	= {
> +		.name		= "mvebu-a3700-utmi-phy",
> +		.owner		= THIS_MODULE,
> +		.of_match_table	= mvebu_a3700_utmi_of_match,
> +	 },
> +};
> +module_platform_driver(mvebu_a3700_utmi_driver);
> +
> +MODULE_AUTHOR("Evan Wang <xswang@marvell.com>");
> +MODULE_AUTHOR("Miquel Raynal <miquel.raynal@bootlin.com>");
> +MODULE_DESCRIPTION("Marvell EBU A3700 UTMI PHY driver");
> +MODULE_LICENSE("GPL");

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [v2,05/10] phy: add A3700 UTMI PHY driver
@ 2019-01-16  9:20 Kishon Vijay Abraham I
  0 siblings, 0 replies; 6+ messages in thread
From: Kishon Vijay Abraham I @ 2019-01-16  9:20 UTC (permalink / raw)
  To: Chunfeng Yun, Miquel Raynal
  Cc: Gregory Clement, Jason Cooper, Andrew Lunn, Sebastian Hesselbarth,
	Rob Herring, Mark Rutland, Greg Kroah-Hartman, Mathias Nyman,
	Alan Stern, devicetree, linux-arm-kernel, linux-usb,
	Thomas Petazzoni, Antoine Tenart, Maxime Chevallier, Nadav Haklai,
	Igal Liberman

Hi,

On 15/01/19 8:10 AM, Chunfeng Yun wrote:
> Hi,
> On Fri, 2019-01-11 at 14:31 +0100, Miquel Raynal wrote:
>> Marvell Armada 3700 SoC has two USB controllers, each of them being
>> wired to an internal UTMI PHY. Add a driver to control them.
>>
>> Igal Liberman worked on supporting the PHY, I took the while 'register
>> configuration' from his work and rewrote almost entirely the
>> driver/bindings around it.
>>
>> Co-developed-by: Igal Liberman <igall@marvell.com>
>> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
>> Signed-off-by: Igal Liberman <igall@marvell.com>
>> ---
>>  drivers/phy/marvell/Kconfig                |   9 +
>>  drivers/phy/marvell/Makefile               |   1 +
>>  drivers/phy/marvell/phy-mvebu-a3700-utmi.c | 297 +++++++++++++++++++++
>>  3 files changed, 307 insertions(+)
>>  create mode 100644 drivers/phy/marvell/phy-mvebu-a3700-utmi.c
>>
>> diff --git a/drivers/phy/marvell/Kconfig b/drivers/phy/marvell/Kconfig
>> index 9c90c0408ea3..b8e9dd38ad0d 100644
>> --- a/drivers/phy/marvell/Kconfig
>> +++ b/drivers/phy/marvell/Kconfig
>> @@ -33,6 +33,15 @@ config PHY_MVEBU_A3700_COMPHY
>>  	  shared serdes PHYs on Marvell Armada 3700. Its serdes lanes can be
>>  	  used by various controllers: Ethernet, SATA, USB3, PCIe.
>>  
>> +config PHY_MVEBU_A3700_UTMI
>> +	tristate "Marvell A3700 UTMI driver"
>> +	depends on ARCH_MVEBU || COMPILE_TEST
>> +	depends on OF
>> +	default y
>> +	select GENERIC_PHY
>> +	help
>> +	  Enable this to support Marvell A3700 UTMI PHY driver.
>> +
>>  config PHY_MVEBU_CP110_COMPHY
>>  	tristate "Marvell CP110 comphy driver"
>>  	depends on ARCH_MVEBU || COMPILE_TEST
>> diff --git a/drivers/phy/marvell/Makefile b/drivers/phy/marvell/Makefile
>> index c13a0c8ab6f0..82f291cf59ee 100644
>> --- a/drivers/phy/marvell/Makefile
>> +++ b/drivers/phy/marvell/Makefile
>> @@ -3,6 +3,7 @@ obj-$(CONFIG_ARMADA375_USBCLUSTER_PHY)	+= phy-armada375-usb2.o
>>  obj-$(CONFIG_PHY_BERLIN_SATA)		+= phy-berlin-sata.o
>>  obj-$(CONFIG_PHY_BERLIN_USB)		+= phy-berlin-usb.o
>>  obj-$(CONFIG_PHY_MVEBU_A3700_COMPHY)	+= phy-mvebu-a3700-comphy.o
>> +obj-$(CONFIG_PHY_MVEBU_A3700_UTMI)	+= phy-mvebu-a3700-utmi.o
>>  obj-$(CONFIG_PHY_MVEBU_CP110_COMPHY)	+= phy-mvebu-cp110-comphy.o
>>  obj-$(CONFIG_PHY_MVEBU_SATA)		+= phy-mvebu-sata.o
>>  obj-$(CONFIG_PHY_PXA_28NM_HSIC)		+= phy-pxa-28nm-hsic.o
>> diff --git a/drivers/phy/marvell/phy-mvebu-a3700-utmi.c b/drivers/phy/marvell/phy-mvebu-a3700-utmi.c
>> new file mode 100644
>> index 000000000000..97d8235d661d
>> --- /dev/null
>> +++ b/drivers/phy/marvell/phy-mvebu-a3700-utmi.c
>> @@ -0,0 +1,297 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (C) 2018 Marvell
>> + *
>> + * Authors:
>> + *   Evan Wang <xswang@marvell.com>
>> + *   Miquèl Raynal <miquel.raynal@bootlin.com>
>> + *
>> + * Marvell A3700 UTMI PHY driver
>> + */
>> +
>> +#include <linux/io.h>
>> +#include <linux/iopoll.h>
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/module.h>
>> +#include <linux/of_device.h>
>> +#include <linux/phy/phy.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/regmap.h>
>> +
>> +/* Armada 3700 UTMI PHY registers */
>> +#define USB2_PHY_PLL_CTRL_REG0			0x0
>> +#define   PLL_REF_DIV_OFF			0
>> +#define   PLL_REF_DIV_MASK			(0x7F << PLL_REF_DIV_OFF)
> use GENMASK?
>> +#define   PLL_REF_DIV_5				0x5
>> +#define   PLL_FB_DIV_OFF			16
>> +#define   PLL_FB_DIV_MASK			(0x1FF << PLL_FB_DIV_OFF)
>> +#define   PLL_FB_DIV_96				96
>> +#define   PLL_SEL_LPFR_OFF			28
>> +#define   PLL_SEL_LPFR_MASK			(0x3 << PLL_SEL_LPFR_OFF)
>> +#define   PLL_READY				BIT(31)
>> +#define USB2_PHY_CAL_CTRL			0x8
>> +#define   PHY_PLLCAL_DONE			BIT(31)
>> +#define   PHY_IMPCAL_DONE			BIT(23)
>> +#define USB2_RX_CHAN_CTRL1			0x18
>> +#define   USB2PHY_SQCAL_DONE			BIT(31)
>> +#define USB2_PHY_OTG_CTRL			0x34
>> +#define   PHY_PU_OTG				BIT(4)
>> +#define USB2_PHY_CHRGR_DETECT			0x38
>> +#define   PHY_CDP_EN				BIT(2)
>> +#define   PHY_DCP_EN				BIT(3)
>> +#define   PHY_PD_EN				BIT(4)
>> +#define   PHY_PU_CHRG_DTC			BIT(5)
>> +#define   PHY_CDP_DM_AUTO			BIT(7)
>> +#define   PHY_ENSWITCH_DP			BIT(12)
>> +#define   PHY_ENSWITCH_DM			BIT(13)
>> +
>> +/* Armada 3700 USB miscellaneous registers */
>> +#define USB2_PHY_CTRL(usb32)			(usb32 ? 0x20 : 0x4)
>> +#define   RB_USB2PHY_PU				BIT(0)
>> +#define   USB2_DP_PULLDN_DEV_MODE		BIT(5)
>> +#define   USB2_DM_PULLDN_DEV_MODE		BIT(6)
>> +#define   RB_USB2PHY_SUSPM(usb32)		(usb32 ? BIT(14) : BIT(7))
>> +
>> +#define PLL_LOCK_DELAY_US			10000
>> +#define PLL_LOCK_TIMEOUT_US			1000000
>> +
>> +/**
>> + * struct mvebu_a3700_utmi_caps - PHY capabilities
>> + *
>> + * @usb32: Flag indicating which PHY is in use (impacts the register map):
>> + *           - The UTMI PHY wired to the USB3/USB2 controller (otg)
>> + *           - The UTMI PHY wired to the USB2 controller (host only)
>> + * @ops: PHY operations
>> + */
>> +struct mvebu_a3700_utmi_caps {
>> +	int usb32;
>> +	const struct phy_ops *ops;
>> +};
>> +
>> +/**
>> + * struct mvebu_a3700_utmi - PHY driver data
>> + *
>> + * @regs: PHY registers
>> + * @usb_mis: Regmap with USB miscellaneous registers including PHY ones
>> + * @caps: PHY capabilities
>> + * @phy: PHY handle
>> + */
>> +struct mvebu_a3700_utmi {
>> +	void __iomem *regs;
>> +	struct regmap *usb_misc;
>> +	const struct mvebu_a3700_utmi_caps *caps;
>> +	struct phy *phy;
>> +};
>> +
>> +static int mvebu_a3700_utmi_phy_power_on(struct phy *phy)
>> +{
>> +	struct mvebu_a3700_utmi *utmi = phy_get_drvdata(phy);
>> +	struct device *dev = &phy->dev;
>> +	int usb32 = utmi->caps->usb32;
>> +	int ret = 0;
>> +	u32 reg;
>> +
>> +	if (!utmi)
>> +		return -ENODEV;
> the check here may be not necessary

right. utmi is already referenced above.
>> +
>> +	/*
>> +	 * Setup PLL. 40MHz clock used to be the default, bein 25MHz now.
>> +	 * See "PLL Settings for Typical REFCLK" table.
>> +	 */
>> +	reg = readl(utmi->regs + USB2_PHY_PLL_CTRL_REG0);
>> +	reg &= ~(PLL_REF_DIV_MASK | PLL_FB_DIV_MASK | PLL_SEL_LPFR_MASK);
>> +	reg |= (PLL_REF_DIV_5 << PLL_REF_DIV_OFF |
>> +		PLL_FB_DIV_96 << PLL_FB_DIV_OFF);
>> +	writel(reg, utmi->regs + USB2_PHY_PLL_CTRL_REG0);
>> +
>> +	/* Enable PHY pull up and disable USB2 suspend */
>> +	regmap_update_bits(utmi->usb_misc, USB2_PHY_CTRL(usb32),
>> +			   RB_USB2PHY_SUSPM(usb32) | RB_USB2PHY_PU,
>> +			   RB_USB2PHY_SUSPM(usb32) | RB_USB2PHY_PU);
>> +
>> +	if (usb32) {
>> +		/* Power up OTG module */
>> +		reg = readl(utmi->regs + USB2_PHY_OTG_CTRL);
>> +		reg |= PHY_PU_OTG;
>> +		writel(reg, utmi->regs + USB2_PHY_OTG_CTRL);
>> +
>> +		/* Disable PHY charger detection */
>> +		reg = readl(utmi->regs + USB2_PHY_CHRGR_DETECT);
>> +		reg &= ~(PHY_CDP_EN | PHY_DCP_EN | PHY_PD_EN | PHY_PU_CHRG_DTC |
>> +			 PHY_CDP_DM_AUTO | PHY_ENSWITCH_DP | PHY_ENSWITCH_DM);
>> +		writel(reg, utmi->regs + USB2_PHY_CHRGR_DETECT);
>> +
>> +		/* Disable PHY DP/DM pull-down (used for device mode) */
>> +		regmap_update_bits(utmi->usb_misc, USB2_PHY_CTRL(usb32),
>> +				   USB2_DP_PULLDN_DEV_MODE |
>> +				   USB2_DM_PULLDN_DEV_MODE, 0);
>> +	}
>> +
>> +	/* Wait for PLL calibration */
>> +	ret = readl_poll_timeout(utmi->regs + USB2_PHY_CAL_CTRL, reg,
>> +				 reg & PHY_PLLCAL_DONE,
>> +				 PLL_LOCK_DELAY_US, PLL_LOCK_TIMEOUT_US);
>> +	if (ret) {
>> +		dev_err(dev, "Failed to end USB2 PLL calibration\n");
>> +		return ret;
>> +	}
>> +
>> +	/* Wait for impedance calibration */
>> +	ret = readl_poll_timeout(utmi->regs + USB2_PHY_CAL_CTRL, reg,
>> +				 reg & PHY_IMPCAL_DONE,
>> +				 PLL_LOCK_DELAY_US, PLL_LOCK_TIMEOUT_US);
>> +	if (ret) {
>> +		dev_err(dev, "Failed to end USB2 impedance calibration\n");
>> +		return ret;
>> +	}
>> +
>> +	/* Wait for squetch calibration */
>> +	ret = readl_poll_timeout(utmi->regs + USB2_RX_CHAN_CTRL1, reg,
>> +				 reg & USB2PHY_SQCAL_DONE,
>> +				 PLL_LOCK_DELAY_US, PLL_LOCK_TIMEOUT_US);
>> +	if (ret) {
>> +		dev_err(dev, "Failed to end USB2 unknown calibration\n");
>> +		return ret;
>> +	}
>> +
>> +	/* Wait for PLL to be locked */
>> +	ret = readl_poll_timeout(utmi->regs + USB2_PHY_PLL_CTRL_REG0, reg,
>> +				 reg & PLL_READY,
>> +				 PLL_LOCK_DELAY_US, PLL_LOCK_TIMEOUT_US);
>> +	if (ret)
>> +		dev_err(dev, "Failed to lock USB2 PLL\n");
>> +
>> +	return ret;
>> +}
>> +
>> +static int mvebu_a3700_utmi_phy_power_off(struct phy *phy)
>> +{
>> +	struct mvebu_a3700_utmi *utmi = phy_get_drvdata(phy);
>> +	int usb32 = utmi->caps->usb32;
>> +	u32 reg;
>> +
>> +	if (!utmi)
>> +		return -ENODEV;

here too, utmi is referenced above.
>> +
>> +	/* Disable PHY pull-up and enable USB2 suspend */
>> +	reg = readl(utmi->regs + USB2_PHY_CTRL(usb32));
>> +	reg &= ~(RB_USB2PHY_PU | RB_USB2PHY_SUSPM(usb32));
>> +	writel(reg, utmi->regs + USB2_PHY_CTRL(usb32));
>> +
>> +	/* Power down OTG module */
>> +	if (usb32) {
>> +		reg = readl(utmi->regs + USB2_PHY_OTG_CTRL);
>> +		reg &= ~PHY_PU_OTG;
>> +		writel(reg, utmi->regs + USB2_PHY_OTG_CTRL);
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct phy_ops mvebu_a3700_utmi_phy_ops = {
>> +	.power_on = mvebu_a3700_utmi_phy_power_on,
>> +	.power_off = mvebu_a3700_utmi_phy_power_off,
>> +	.owner = THIS_MODULE,
>> +};
>> +
>> +static const struct mvebu_a3700_utmi_caps mvebu_a3700_utmi_otg_phy_caps = {
>> +	.usb32 = true,
>> +	.ops = &mvebu_a3700_utmi_phy_ops,
>> +};
>> +
>> +static const struct mvebu_a3700_utmi_caps mvebu_a3700_utmi_host_phy_caps = {
>> +	.usb32 = false,
>> +	.ops = &mvebu_a3700_utmi_phy_ops,
>> +};
>> +
>> +static const struct of_device_id mvebu_a3700_utmi_of_match[] = {
>> +	{
>> +		.compatible = "marvell,a3700-utmi-otg-phy",
>> +		.data = &mvebu_a3700_utmi_otg_phy_caps,
>> +	},
>> +	{
>> +		.compatible = "marvell,a3700-utmi-host-phy",
>> +		.data = &mvebu_a3700_utmi_host_phy_caps,
>> +	},
>> +	{},
>> +};
>> +MODULE_DEVICE_TABLE(of, mvebu_a3700_utmi_of_match);
>> +
>> +static int mvebu_a3700_utmi_phy_probe(struct platform_device *pdev)
>> +{
>> +	const struct of_device_id *dev_id;
>> +	struct device *dev = &pdev->dev;
>> +	struct mvebu_a3700_utmi *utmi;
>> +	struct phy_provider *provider;
>> +	const struct phy_ops *ops;
>> +	struct resource *res;
>> +
>> +	utmi = devm_kzalloc(dev, sizeof(*utmi), GFP_KERNEL);
>> +	if (!utmi)
>> +		return -ENOMEM;
>> +
>> +	/* Get UTMI memory region */
>> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +	if (!res) {
>> +		dev_err(dev, "Missing UTMI PHY memory resource\n");
>> +		return -ENODEV;
>> +	}
>> +
>> +	utmi->regs = devm_ioremap_resource(dev, res);
>> +	if (IS_ERR(utmi->regs))
>> +		return PTR_ERR(utmi->regs);
>> +
>> +	/* Get miscellaneous Host/PHY region */
>> +	utmi->usb_misc = syscon_regmap_lookup_by_phandle(dev->of_node,
>> +							 "marvell,usb-misc-reg");
>> +	if (IS_ERR(utmi->usb_misc)) {
>> +		dev_err(dev,
>> +			"Missing USB misc purpose system controller\n");
>> +		return PTR_ERR(utmi->usb_misc);
>> +	}
>> +
>> +	/* Retrieve PHY capabilities */
>> +	dev_id = of_match_device(mvebu_a3700_utmi_of_match, dev);
>> +	if (!dev_id)
>> +		return -EINVAL;
>> +
>> +	utmi->caps = dev_id->data;
> a useful API to get @data: of_device_get_match_data()
> 
>> +	ops = utmi->caps->ops;
>> +
>> +	/* Instantiate the PHY */
>> +	utmi->phy = devm_phy_create(dev, NULL, ops);
>> +	if (IS_ERR(utmi->phy)) {
>> +		dev_err(dev, "Failed to create the UTMI PHY\n");
>> +		return PTR_ERR(utmi->phy);
>> +	}
>> +
>> +	phy_set_drvdata(utmi->phy, utmi);
>> +
>> +	/* Ensure the PHY is powered off */
>> +	ops->power_off(utmi->phy);
>> +
>> +	provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
>> +
>> +	return PTR_ERR_OR_ZERO(provider);
>> +}
>> +
>> +static int mvebu_a3700_utmi_phy_remove(struct platform_device *pdev)
>> +{
>> +	return 0;
>> +}

This empty remove function can be removed.

Thanks
Kishon

>> +
>> +static struct platform_driver mvebu_a3700_utmi_driver = {
>> +	.probe	= mvebu_a3700_utmi_phy_probe,
>> +	.remove = mvebu_a3700_utmi_phy_remove,
>> +	.driver	= {
>> +		.name		= "mvebu-a3700-utmi-phy",
>> +		.owner		= THIS_MODULE,
>> +		.of_match_table	= mvebu_a3700_utmi_of_match,
>> +	 },
>> +};
>> +module_platform_driver(mvebu_a3700_utmi_driver);
>> +
>> +MODULE_AUTHOR("Evan Wang <xswang@marvell.com>");
>> +MODULE_AUTHOR("Miquel Raynal <miquel.raynal@bootlin.com>");
>> +MODULE_DESCRIPTION("Marvell EBU A3700 UTMI PHY driver");
>> +MODULE_LICENSE("GPL");
> 
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [v2,05/10] phy: add A3700 UTMI PHY driver
@ 2019-01-18 16:36 Gregory CLEMENT
  0 siblings, 0 replies; 6+ messages in thread
From: Gregory CLEMENT @ 2019-01-18 16:36 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Kishon Vijay Abraham I, Jason Cooper, Andrew Lunn,
	Sebastian Hesselbarth, Rob Herring, Mark Rutland,
	Greg Kroah-Hartman, Mathias Nyman, Alan Stern, devicetree,
	linux-arm-kernel, linux-usb, Thomas Petazzoni, Antoine Tenart,
	Maxime Chevallier, Nadav Haklai, Igal Liberman

Hi Miquel,

I have only a few smallish remarks:

 On ven., janv. 11 2019, Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Marvell Armada 3700 SoC has two USB controllers, each of them being
> wired to an internal UTMI PHY. Add a driver to control them.
>
> Igal Liberman worked on supporting the PHY, I took the while 'register
> configuration' from his work and rewrote almost entirely the
> driver/bindings around it.
>
> Co-developed-by: Igal Liberman <igall@marvell.com>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> Signed-off-by: Igal Liberman <igall@marvell.com>
> ---
[...]

> diff --git a/drivers/phy/marvell/phy-mvebu-a3700-utmi.c b/drivers/phy/marvell/phy-mvebu-a3700-utmi.c
> new file mode 100644
> index 000000000000..97d8235d661d
> --- /dev/null
> +++ b/drivers/phy/marvell/phy-mvebu-a3700-utmi.c
> @@ -0,0 +1,297 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2018 Marvell
> + *
> + * Authors:
> + *   Evan Wang <xswang@marvell.com>
> + *   Miquèl Raynal <miquel.raynal@bootlin.com>

If it is co-developed with Igal Liberman, shouldn't be added here too?
[...]

> +static int mvebu_a3700_utmi_phy_power_on(struct phy *phy)
> +{
> +	struct mvebu_a3700_utmi *utmi = phy_get_drvdata(phy);
> +	struct device *dev = &phy->dev;
> +	int usb32 = utmi->caps->usb32;
> +	int ret = 0;
> +	u32 reg;
> +
> +	if (!utmi)
> +		return -ENODEV;
> +
> +	/*
> +	 * Setup PLL. 40MHz clock used to be the default, bein 25MHz now.
                                                          bein?
> +	 * See "PLL Settings for Typical REFCLK" table.

[...]

> +
> +	/* Wait for squetch calibration */
                    squetch?

Gregory

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [v2,05/10] phy: add A3700 UTMI PHY driver
@ 2019-01-21 10:24 Miquel Raynal
  0 siblings, 0 replies; 6+ messages in thread
From: Miquel Raynal @ 2019-01-21 10:24 UTC (permalink / raw)
  To: Gregory CLEMENT
  Cc: Kishon Vijay Abraham I, Jason Cooper, Andrew Lunn,
	Sebastian Hesselbarth, Rob Herring, Mark Rutland,
	Greg Kroah-Hartman, Mathias Nyman, Alan Stern, devicetree,
	linux-arm-kernel, linux-usb, Thomas Petazzoni, Antoine Tenart,
	Maxime Chevallier, Nadav Haklai, Igal Liberman

Hi Gregory,

Gregory CLEMENT <gregory.clement@bootlin.com> wrote on Fri, 18 Jan 2019
17:36:36 +0100:

> Hi Miquel,
> 
> I have only a few smallish remarks:
> 
>  On ven., janv. 11 2019, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> 
> > Marvell Armada 3700 SoC has two USB controllers, each of them being
> > wired to an internal UTMI PHY. Add a driver to control them.
> >
> > Igal Liberman worked on supporting the PHY, I took the while 'register
> > configuration' from his work and rewrote almost entirely the
> > driver/bindings around it.
> >
> > Co-developed-by: Igal Liberman <igall@marvell.com>
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > Signed-off-by: Igal Liberman <igall@marvell.com>
> > ---  
> [...]
> 
> > diff --git a/drivers/phy/marvell/phy-mvebu-a3700-utmi.c b/drivers/phy/marvell/phy-mvebu-a3700-utmi.c
> > new file mode 100644
> > index 000000000000..97d8235d661d
> > --- /dev/null
> > +++ b/drivers/phy/marvell/phy-mvebu-a3700-utmi.c
> > @@ -0,0 +1,297 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2018 Marvell
> > + *
> > + * Authors:
> > + *   Evan Wang <xswang@marvell.com>
> > + *   Miquèl Raynal <miquel.raynal@bootlin.com>  
> 
> If it is co-developed with Igal Liberman, shouldn't be added here too?

In the original patch, the author in the commit log was Igal and the
MODULE_AUTHOR macro was saying Evan. I think the macro is wrong, I will
keep Igal.

> [...]
> 
> > +static int mvebu_a3700_utmi_phy_power_on(struct phy *phy)
> > +{
> > +	struct mvebu_a3700_utmi *utmi = phy_get_drvdata(phy);
> > +	struct device *dev = &phy->dev;
> > +	int usb32 = utmi->caps->usb32;
> > +	int ret = 0;
> > +	u32 reg;
> > +
> > +	if (!utmi)
> > +		return -ENODEV;
> > +
> > +	/*
> > +	 * Setup PLL. 40MHz clock used to be the default, bein 25MHz now.  
>                                                           bein?
> > +	 * See "PLL Settings for Typical REFCLK" table.  
> 
> [...]
> 
> > +
> > +	/* Wait for squetch calibration */  
>                     squetch?

Good catch. It's written "squelch" in the datasheet, but I don't know
what it means exactly.


Thanks,
Miquèl

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [v2,05/10] phy: add A3700 UTMI PHY driver
@ 2019-01-21 10:37 Miquel Raynal
  0 siblings, 0 replies; 6+ messages in thread
From: Miquel Raynal @ 2019-01-21 10:37 UTC (permalink / raw)
  To: Kishon Vijay Abraham I
  Cc: Chunfeng Yun, Gregory Clement, Jason Cooper, Andrew Lunn,
	Sebastian Hesselbarth, Rob Herring, Mark Rutland,
	Greg Kroah-Hartman, Mathias Nyman, Alan Stern, devicetree,
	linux-arm-kernel, linux-usb, Thomas Petazzoni, Antoine Tenart,
	Maxime Chevallier, Nadav Haklai, Igal Liberman

Hi Kishon, Chunfeng,

Kishon Vijay Abraham I <kishon@ti.com> wrote on Wed, 16 Jan 2019
14:50:58 +0530:

> Hi,
> 
> On 15/01/19 8:10 AM, Chunfeng Yun wrote:
> > Hi,
> > On Fri, 2019-01-11 at 14:31 +0100, Miquel Raynal wrote:  
> >> Marvell Armada 3700 SoC has two USB controllers, each of them being
> >> wired to an internal UTMI PHY. Add a driver to control them.
> >>
> >> Igal Liberman worked on supporting the PHY, I took the while 'register
> >> configuration' from his work and rewrote almost entirely the
> >> driver/bindings around it.
> >>
> >> Co-developed-by: Igal Liberman <igall@marvell.com>
> >> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> >> Signed-off-by: Igal Liberman <igall@marvell.com>
> >> ---
> >>  drivers/phy/marvell/Kconfig                |   9 +
> >>  drivers/phy/marvell/Makefile               |   1 +
> >>  drivers/phy/marvell/phy-mvebu-a3700-utmi.c | 297 +++++++++++++++++++++
> >>  3 files changed, 307 insertions(+)
> >>  create mode 100644 drivers/phy/marvell/phy-mvebu-a3700-utmi.c
> >>
> >> diff --git a/drivers/phy/marvell/Kconfig b/drivers/phy/marvell/Kconfig
> >> index 9c90c0408ea3..b8e9dd38ad0d 100644
> >> --- a/drivers/phy/marvell/Kconfig
> >> +++ b/drivers/phy/marvell/Kconfig
> >> @@ -33,6 +33,15 @@ config PHY_MVEBU_A3700_COMPHY
> >>  	  shared serdes PHYs on Marvell Armada 3700. Its serdes lanes can be
> >>  	  used by various controllers: Ethernet, SATA, USB3, PCIe.
> >>  
> >> +config PHY_MVEBU_A3700_UTMI
> >> +	tristate "Marvell A3700 UTMI driver"
> >> +	depends on ARCH_MVEBU || COMPILE_TEST
> >> +	depends on OF
> >> +	default y
> >> +	select GENERIC_PHY
> >> +	help
> >> +	  Enable this to support Marvell A3700 UTMI PHY driver.
> >> +
> >>  config PHY_MVEBU_CP110_COMPHY
> >>  	tristate "Marvell CP110 comphy driver"
> >>  	depends on ARCH_MVEBU || COMPILE_TEST
> >> diff --git a/drivers/phy/marvell/Makefile b/drivers/phy/marvell/Makefile
> >> index c13a0c8ab6f0..82f291cf59ee 100644
> >> --- a/drivers/phy/marvell/Makefile
> >> +++ b/drivers/phy/marvell/Makefile
> >> @@ -3,6 +3,7 @@ obj-$(CONFIG_ARMADA375_USBCLUSTER_PHY)	+= phy-armada375-usb2.o
> >>  obj-$(CONFIG_PHY_BERLIN_SATA)		+= phy-berlin-sata.o
> >>  obj-$(CONFIG_PHY_BERLIN_USB)		+= phy-berlin-usb.o
> >>  obj-$(CONFIG_PHY_MVEBU_A3700_COMPHY)	+= phy-mvebu-a3700-comphy.o
> >> +obj-$(CONFIG_PHY_MVEBU_A3700_UTMI)	+= phy-mvebu-a3700-utmi.o
> >>  obj-$(CONFIG_PHY_MVEBU_CP110_COMPHY)	+= phy-mvebu-cp110-comphy.o
> >>  obj-$(CONFIG_PHY_MVEBU_SATA)		+= phy-mvebu-sata.o
> >>  obj-$(CONFIG_PHY_PXA_28NM_HSIC)		+= phy-pxa-28nm-hsic.o
> >> diff --git a/drivers/phy/marvell/phy-mvebu-a3700-utmi.c b/drivers/phy/marvell/phy-mvebu-a3700-utmi.c
> >> new file mode 100644
> >> index 000000000000..97d8235d661d
> >> --- /dev/null
> >> +++ b/drivers/phy/marvell/phy-mvebu-a3700-utmi.c
> >> @@ -0,0 +1,297 @@
> >> +// SPDX-License-Identifier: GPL-2.0
> >> +/*
> >> + * Copyright (C) 2018 Marvell
> >> + *
> >> + * Authors:
> >> + *   Evan Wang <xswang@marvell.com>
> >> + *   Miquèl Raynal <miquel.raynal@bootlin.com>
> >> + *
> >> + * Marvell A3700 UTMI PHY driver
> >> + */
> >> +
> >> +#include <linux/io.h>
> >> +#include <linux/iopoll.h>
> >> +#include <linux/mfd/syscon.h>
> >> +#include <linux/module.h>
> >> +#include <linux/of_device.h>
> >> +#include <linux/phy/phy.h>
> >> +#include <linux/platform_device.h>
> >> +#include <linux/regmap.h>
> >> +
> >> +/* Armada 3700 UTMI PHY registers */
> >> +#define USB2_PHY_PLL_CTRL_REG0			0x0
> >> +#define   PLL_REF_DIV_OFF			0
> >> +#define   PLL_REF_DIV_MASK			(0x7F << PLL_REF_DIV_OFF)  
> > use GENMASK?  

Ok

> >> +#define   PLL_REF_DIV_5				0x5
> >> +#define   PLL_FB_DIV_OFF			16
> >> +#define   PLL_FB_DIV_MASK			(0x1FF << PLL_FB_DIV_OFF)
> >> +#define   PLL_FB_DIV_96				96
> >> +#define   PLL_SEL_LPFR_OFF			28
> >> +#define   PLL_SEL_LPFR_MASK			(0x3 << PLL_SEL_LPFR_OFF)
> >> +#define   PLL_READY				BIT(31)
> >> +#define USB2_PHY_CAL_CTRL			0x8
> >> +#define   PHY_PLLCAL_DONE			BIT(31)
> >> +#define   PHY_IMPCAL_DONE			BIT(23)
> >> +#define USB2_RX_CHAN_CTRL1			0x18
> >> +#define   USB2PHY_SQCAL_DONE			BIT(31)
> >> +#define USB2_PHY_OTG_CTRL			0x34
> >> +#define   PHY_PU_OTG				BIT(4)
> >> +#define USB2_PHY_CHRGR_DETECT			0x38
> >> +#define   PHY_CDP_EN				BIT(2)
> >> +#define   PHY_DCP_EN				BIT(3)
> >> +#define   PHY_PD_EN				BIT(4)
> >> +#define   PHY_PU_CHRG_DTC			BIT(5)
> >> +#define   PHY_CDP_DM_AUTO			BIT(7)
> >> +#define   PHY_ENSWITCH_DP			BIT(12)
> >> +#define   PHY_ENSWITCH_DM			BIT(13)
> >> +
> >> +/* Armada 3700 USB miscellaneous registers */
> >> +#define USB2_PHY_CTRL(usb32)			(usb32 ? 0x20 : 0x4)
> >> +#define   RB_USB2PHY_PU				BIT(0)
> >> +#define   USB2_DP_PULLDN_DEV_MODE		BIT(5)
> >> +#define   USB2_DM_PULLDN_DEV_MODE		BIT(6)
> >> +#define   RB_USB2PHY_SUSPM(usb32)		(usb32 ? BIT(14) : BIT(7))
> >> +
> >> +#define PLL_LOCK_DELAY_US			10000
> >> +#define PLL_LOCK_TIMEOUT_US			1000000
> >> +
> >> +/**
> >> + * struct mvebu_a3700_utmi_caps - PHY capabilities
> >> + *
> >> + * @usb32: Flag indicating which PHY is in use (impacts the register map):
> >> + *           - The UTMI PHY wired to the USB3/USB2 controller (otg)
> >> + *           - The UTMI PHY wired to the USB2 controller (host only)
> >> + * @ops: PHY operations
> >> + */
> >> +struct mvebu_a3700_utmi_caps {
> >> +	int usb32;
> >> +	const struct phy_ops *ops;
> >> +};
> >> +
> >> +/**
> >> + * struct mvebu_a3700_utmi - PHY driver data
> >> + *
> >> + * @regs: PHY registers
> >> + * @usb_mis: Regmap with USB miscellaneous registers including PHY ones
> >> + * @caps: PHY capabilities
> >> + * @phy: PHY handle
> >> + */
> >> +struct mvebu_a3700_utmi {
> >> +	void __iomem *regs;
> >> +	struct regmap *usb_misc;
> >> +	const struct mvebu_a3700_utmi_caps *caps;
> >> +	struct phy *phy;
> >> +};
> >> +
> >> +static int mvebu_a3700_utmi_phy_power_on(struct phy *phy)
> >> +{
> >> +	struct mvebu_a3700_utmi *utmi = phy_get_drvdata(phy);
> >> +	struct device *dev = &phy->dev;
> >> +	int usb32 = utmi->caps->usb32;
> >> +	int ret = 0;
> >> +	u32 reg;
> >> +
> >> +	if (!utmi)
> >> +		return -ENODEV;  
> > the check here may be not necessary  
> 
> right. utmi is already referenced above.

Removed.

> >> +
> >> +	/*
> >> +	 * Setup PLL. 40MHz clock used to be the default, bein 25MHz now.
> >> +	 * See "PLL Settings for Typical REFCLK" table.
> >> +	 */
> >> +	reg = readl(utmi->regs + USB2_PHY_PLL_CTRL_REG0);
> >> +	reg &= ~(PLL_REF_DIV_MASK | PLL_FB_DIV_MASK | PLL_SEL_LPFR_MASK);
> >> +	reg |= (PLL_REF_DIV_5 << PLL_REF_DIV_OFF |
> >> +		PLL_FB_DIV_96 << PLL_FB_DIV_OFF);
> >> +	writel(reg, utmi->regs + USB2_PHY_PLL_CTRL_REG0);
> >> +
> >> +	/* Enable PHY pull up and disable USB2 suspend */
> >> +	regmap_update_bits(utmi->usb_misc, USB2_PHY_CTRL(usb32),
> >> +			   RB_USB2PHY_SUSPM(usb32) | RB_USB2PHY_PU,
> >> +			   RB_USB2PHY_SUSPM(usb32) | RB_USB2PHY_PU);
> >> +
> >> +	if (usb32) {
> >> +		/* Power up OTG module */
> >> +		reg = readl(utmi->regs + USB2_PHY_OTG_CTRL);
> >> +		reg |= PHY_PU_OTG;
> >> +		writel(reg, utmi->regs + USB2_PHY_OTG_CTRL);
> >> +
> >> +		/* Disable PHY charger detection */
> >> +		reg = readl(utmi->regs + USB2_PHY_CHRGR_DETECT);
> >> +		reg &= ~(PHY_CDP_EN | PHY_DCP_EN | PHY_PD_EN | PHY_PU_CHRG_DTC |
> >> +			 PHY_CDP_DM_AUTO | PHY_ENSWITCH_DP | PHY_ENSWITCH_DM);
> >> +		writel(reg, utmi->regs + USB2_PHY_CHRGR_DETECT);
> >> +
> >> +		/* Disable PHY DP/DM pull-down (used for device mode) */
> >> +		regmap_update_bits(utmi->usb_misc, USB2_PHY_CTRL(usb32),
> >> +				   USB2_DP_PULLDN_DEV_MODE |
> >> +				   USB2_DM_PULLDN_DEV_MODE, 0);
> >> +	}
> >> +
> >> +	/* Wait for PLL calibration */
> >> +	ret = readl_poll_timeout(utmi->regs + USB2_PHY_CAL_CTRL, reg,
> >> +				 reg & PHY_PLLCAL_DONE,
> >> +				 PLL_LOCK_DELAY_US, PLL_LOCK_TIMEOUT_US);
> >> +	if (ret) {
> >> +		dev_err(dev, "Failed to end USB2 PLL calibration\n");
> >> +		return ret;
> >> +	}
> >> +
> >> +	/* Wait for impedance calibration */
> >> +	ret = readl_poll_timeout(utmi->regs + USB2_PHY_CAL_CTRL, reg,
> >> +				 reg & PHY_IMPCAL_DONE,
> >> +				 PLL_LOCK_DELAY_US, PLL_LOCK_TIMEOUT_US);
> >> +	if (ret) {
> >> +		dev_err(dev, "Failed to end USB2 impedance calibration\n");
> >> +		return ret;
> >> +	}
> >> +
> >> +	/* Wait for squetch calibration */
> >> +	ret = readl_poll_timeout(utmi->regs + USB2_RX_CHAN_CTRL1, reg,
> >> +				 reg & USB2PHY_SQCAL_DONE,
> >> +				 PLL_LOCK_DELAY_US, PLL_LOCK_TIMEOUT_US);
> >> +	if (ret) {
> >> +		dev_err(dev, "Failed to end USB2 unknown calibration\n");
> >> +		return ret;
> >> +	}
> >> +
> >> +	/* Wait for PLL to be locked */
> >> +	ret = readl_poll_timeout(utmi->regs + USB2_PHY_PLL_CTRL_REG0, reg,
> >> +				 reg & PLL_READY,
> >> +				 PLL_LOCK_DELAY_US, PLL_LOCK_TIMEOUT_US);
> >> +	if (ret)
> >> +		dev_err(dev, "Failed to lock USB2 PLL\n");
> >> +
> >> +	return ret;
> >> +}
> >> +
> >> +static int mvebu_a3700_utmi_phy_power_off(struct phy *phy)
> >> +{
> >> +	struct mvebu_a3700_utmi *utmi = phy_get_drvdata(phy);
> >> +	int usb32 = utmi->caps->usb32;
> >> +	u32 reg;
> >> +
> >> +	if (!utmi)
> >> +		return -ENODEV;  
> 
> here too, utmi is referenced above.

Removed too.

> >> +
> >> +	/* Disable PHY pull-up and enable USB2 suspend */
> >> +	reg = readl(utmi->regs + USB2_PHY_CTRL(usb32));
> >> +	reg &= ~(RB_USB2PHY_PU | RB_USB2PHY_SUSPM(usb32));
> >> +	writel(reg, utmi->regs + USB2_PHY_CTRL(usb32));
> >> +
> >> +	/* Power down OTG module */
> >> +	if (usb32) {
> >> +		reg = readl(utmi->regs + USB2_PHY_OTG_CTRL);
> >> +		reg &= ~PHY_PU_OTG;
> >> +		writel(reg, utmi->regs + USB2_PHY_OTG_CTRL);
> >> +	}
> >> +
> >> +	return 0;
> >> +}
> >> +
> >> +static const struct phy_ops mvebu_a3700_utmi_phy_ops = {
> >> +	.power_on = mvebu_a3700_utmi_phy_power_on,
> >> +	.power_off = mvebu_a3700_utmi_phy_power_off,
> >> +	.owner = THIS_MODULE,
> >> +};
> >> +
> >> +static const struct mvebu_a3700_utmi_caps mvebu_a3700_utmi_otg_phy_caps = {
> >> +	.usb32 = true,
> >> +	.ops = &mvebu_a3700_utmi_phy_ops,
> >> +};
> >> +
> >> +static const struct mvebu_a3700_utmi_caps mvebu_a3700_utmi_host_phy_caps = {
> >> +	.usb32 = false,
> >> +	.ops = &mvebu_a3700_utmi_phy_ops,
> >> +};
> >> +
> >> +static const struct of_device_id mvebu_a3700_utmi_of_match[] = {
> >> +	{
> >> +		.compatible = "marvell,a3700-utmi-otg-phy",
> >> +		.data = &mvebu_a3700_utmi_otg_phy_caps,
> >> +	},
> >> +	{
> >> +		.compatible = "marvell,a3700-utmi-host-phy",
> >> +		.data = &mvebu_a3700_utmi_host_phy_caps,
> >> +	},
> >> +	{},
> >> +};
> >> +MODULE_DEVICE_TABLE(of, mvebu_a3700_utmi_of_match);
> >> +
> >> +static int mvebu_a3700_utmi_phy_probe(struct platform_device *pdev)
> >> +{
> >> +	const struct of_device_id *dev_id;
> >> +	struct device *dev = &pdev->dev;
> >> +	struct mvebu_a3700_utmi *utmi;
> >> +	struct phy_provider *provider;
> >> +	const struct phy_ops *ops;
> >> +	struct resource *res;
> >> +
> >> +	utmi = devm_kzalloc(dev, sizeof(*utmi), GFP_KERNEL);
> >> +	if (!utmi)
> >> +		return -ENOMEM;
> >> +
> >> +	/* Get UTMI memory region */
> >> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >> +	if (!res) {
> >> +		dev_err(dev, "Missing UTMI PHY memory resource\n");
> >> +		return -ENODEV;
> >> +	}
> >> +
> >> +	utmi->regs = devm_ioremap_resource(dev, res);
> >> +	if (IS_ERR(utmi->regs))
> >> +		return PTR_ERR(utmi->regs);
> >> +
> >> +	/* Get miscellaneous Host/PHY region */
> >> +	utmi->usb_misc = syscon_regmap_lookup_by_phandle(dev->of_node,
> >> +							 "marvell,usb-misc-reg");
> >> +	if (IS_ERR(utmi->usb_misc)) {
> >> +		dev_err(dev,
> >> +			"Missing USB misc purpose system controller\n");
> >> +		return PTR_ERR(utmi->usb_misc);
> >> +	}
> >> +
> >> +	/* Retrieve PHY capabilities */
> >> +	dev_id = of_match_device(mvebu_a3700_utmi_of_match, dev);
> >> +	if (!dev_id)
> >> +		return -EINVAL;
> >> +
> >> +	utmi->caps = dev_id->data;  
> > a useful API to get @data: of_device_get_match_data()

Nice one, I forgot about it.

> >   
> >> +	ops = utmi->caps->ops;
> >> +
> >> +	/* Instantiate the PHY */
> >> +	utmi->phy = devm_phy_create(dev, NULL, ops);
> >> +	if (IS_ERR(utmi->phy)) {
> >> +		dev_err(dev, "Failed to create the UTMI PHY\n");
> >> +		return PTR_ERR(utmi->phy);
> >> +	}
> >> +
> >> +	phy_set_drvdata(utmi->phy, utmi);
> >> +
> >> +	/* Ensure the PHY is powered off */
> >> +	ops->power_off(utmi->phy);
> >> +
> >> +	provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
> >> +
> >> +	return PTR_ERR_OR_ZERO(provider);
> >> +}
> >> +
> >> +static int mvebu_a3700_utmi_phy_remove(struct platform_device *pdev)
> >> +{
> >> +	return 0;
> >> +}  
> 
> This empty remove function can be removed.

Sure.

> 
> Thanks
> Kishon
> 
> >> +
> >> +static struct platform_driver mvebu_a3700_utmi_driver = {
> >> +	.probe	= mvebu_a3700_utmi_phy_probe,
> >> +	.remove = mvebu_a3700_utmi_phy_remove,
> >> +	.driver	= {
> >> +		.name		= "mvebu-a3700-utmi-phy",
> >> +		.owner		= THIS_MODULE,
> >> +		.of_match_table	= mvebu_a3700_utmi_of_match,
> >> +	 },
> >> +};
> >> +module_platform_driver(mvebu_a3700_utmi_driver);
> >> +
> >> +MODULE_AUTHOR("Evan Wang <xswang@marvell.com>");
> >> +MODULE_AUTHOR("Miquel Raynal <miquel.raynal@bootlin.com>");
> >> +MODULE_DESCRIPTION("Marvell EBU A3700 UTMI PHY driver");
> >> +MODULE_LICENSE("GPL");  
> > 
> >   

Thanks for reviewing!
Miquèl

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-01-21 10:37 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-01-21 10:37 [v2,05/10] phy: add A3700 UTMI PHY driver Miquel Raynal
  -- strict thread matches above, loose matches on Subject: below --
2019-01-21 10:24 Miquel Raynal
2019-01-18 16:36 Gregory CLEMENT
2019-01-16  9:20 Kishon Vijay Abraham I
2019-01-15  2:40 Chunfeng Yun
2019-01-11 13:31 Miquel Raynal

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).