Skip to contentSkip to conversation

ARM: sunxi: Add driver for sunxi usb phy

PATCH Superseded Hans de Goede 3 participants 6 messages Feb 7, 2014 – Feb 10, 2014

Subsystems (3)

v2 of 4 Superseded Latest is v4 →

lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 ARM: sunxi: Add driver for sunxi usb phy

The Allwinner A1x / A2x SoCs have 2 or 3 usb phys which are all accessed through a single set of registers. Besides this there are also some other phy related bits which need poking, which are per phy, but shared between the ohci and ehci controllers, so these are also controlled from this new phy driver.

---
 .../devicetree/bindings/phy/sun4i-usb-phy.txt      |  28 ++
 drivers/phy/Kconfig                                |  11 +
 drivers/phy/Makefile                               |   1 +
 drivers/phy/phy-sun4i-usb.c                        | 326 +++++++++++++++++++++
 4 files changed, 366 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
 create mode 100644 drivers/phy/phy-sun4i-usb.c
Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt new file +28 -0
new file mode 100644
index 0000000..f7eccb2
@@ -0,0 +1,28 @@
+Allwinner sun4i USB PHY
+-----------------------
+
+Required properties:
+- compatible : should be one of "allwinner,sun4i-a10-usb-phy",
+  "allwinner,sun5i-a13-usb-phy" or "allwinner,sun7i-a20-usb-phy"
+- reg : a list of offset + length pairs, the 1st list entry should point to
+  the phy base regs, the 2nd entry to the pmu reg for phy1, and the 3th
+  entry to the pmu reg of phy2 (for devices which have a phy2).
+- #phy-cells : from the generic phy bindings, must be 1
+
+Optional properties:
+- clocks : phandle + clock specifier for the phy clock
+- clock-names : "usb_phy"
+- resets : a list of phandle + reset specifier pairs
+- reset-names : "usb0_reset", "usb1_reset", and / or "usb2_reset"
+
+Example:
+	usbphy: phy at 0x01c13400 {
+		#phy-cells = <1>;
+		compatible = "allwinner,sun4i-a10-usb-phy";
+		/* phy base regs, phy1 pmu reg, phy2 pmu reg */
+		reg = <0x01c13400 0x10 0x01c14800 0x4 0x01c1c800 0x4>;
+		clocks = <&usb_clk 8>;
+		clock-names = "usb_phy";
+		resets = <&usb_clk 1>, <&usb_clk 2>;
+		reset-names = "usb1_reset", "usb2_reset";
+	};
drivers/phy/Kconfig +11 -0
index afa2354..6070c99 100644
@@ -64,4 +64,15 @@ config BCM_KONA_USB2_PHY
 	help
 	  Enable this to support the Broadcom Kona USB 2.0 PHY.
 
+config PHY_SUN4I_USB
+	tristate "Allwinner sunxi SoC USB PHY driver"
+	depends on ARCH_SUNXI
+	select GENERIC_PHY
+	help
+	  Enable this to support the transceiver that is part of Allwinner
+	  sunxi SoCs.
+
+	  This driver controls the entire USB PHY block, both the USB OTG
+	  parts, as well as the 2 regular USB 2 host PHYs.
+
 endmenu
drivers/phy/Makefile +1 -0
index b57c253..9d4f8bb 100644
@@ -9,3 +9,4 @@ obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO)	+= phy-exynos-mipi-video.o
 obj-$(CONFIG_PHY_MVEBU_SATA)		+= phy-mvebu-sata.o
 obj-$(CONFIG_OMAP_USB2)			+= phy-omap-usb2.o
 obj-$(CONFIG_TWL4030_USB)		+= phy-twl4030-usb.o
+obj-$(CONFIG_PHY_SUN4I_USB)		+= phy-sun4i-usb.o
drivers/phy/phy-sun4i-usb.c new file +326 -0

326 lines not on this page · raw message

Signature · 1 line
1.8.4.2
#1 lore raw

PATCH v2 1/1 ARM: sunxi: Add driver for sunxi usb phy

Hi Hans,

It looks very nice, I just have a few comments below though.

Quoted 22 lines · Hans de Goede

The Allwinner A1x / A2x SoCs have 2 or 3 usb phys which are all accessed through a single set of registers. Besides this there are also some other phy related bits which need poking, which are per phy, but shared between the ohci and ehci controllers, so these are also controlled from this new phy driver.

---
 .../devicetree/bindings/phy/sun4i-usb-phy.txt      |  28 ++
 drivers/phy/Kconfig                                |  11 +
 drivers/phy/Makefile                               |   1 +
 drivers/phy/phy-sun4i-usb.c                        | 326 +++++++++++++++++++++
 4 files changed, 366 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
 create mode 100644 drivers/phy/phy-sun4i-usb.c
Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt new file +0 -0
new file mode 100644
index 0000000..f7eccb2

Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt line 9

Quoted 9 lines
Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt new file +9 -0
@@ -0,0 +1,28 @@
+Allwinner sun4i USB PHY
+-----------------------
+
+Required properties:
+- compatible : should be one of "allwinner,sun4i-a10-usb-phy",
+  "allwinner,sun5i-a13-usb-phy" or "allwinner,sun7i-a20-usb-phy"
+- reg : a list of offset + length pairs, the 1st list entry should point to
+  the phy base regs, the 2nd entry to the pmu reg for phy1, and the 3th
+  entry to the pmu reg of phy2 (for devices which have a phy2).

I'm concerned about devices that would only have a phy2 for some reason.

Using reg-names would be much more robust, and is quite painless to use. Just use platform_get_resource_by_name instead of platform_get_resource, and that's pretty much it.

Quoted 291 lines
Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt new file +19 -0
+- #phy-cells : from the generic phy bindings, must be 1
+
+Optional properties:
+- clocks : phandle + clock specifier for the phy clock
+- clock-names : "usb_phy"
+- resets : a list of phandle + reset specifier pairs
+- reset-names : "usb0_reset", "usb1_reset", and / or "usb2_reset"
+
+Example:
+	usbphy: phy at 0x01c13400 {
+		#phy-cells = <1>;
+		compatible = "allwinner,sun4i-a10-usb-phy";
+		/* phy base regs, phy1 pmu reg, phy2 pmu reg */
+		reg = <0x01c13400 0x10 0x01c14800 0x4 0x01c1c800 0x4>;
+		clocks = <&usb_clk 8>;
+		clock-names = "usb_phy";
+		resets = <&usb_clk 1>, <&usb_clk 2>;
+		reset-names = "usb1_reset", "usb2_reset";
+	};
drivers/phy/Kconfig +11 -0
index afa2354..6070c99 100644
@@ -64,4 +64,15 @@ config BCM_KONA_USB2_PHY
 	help
 	  Enable this to support the Broadcom Kona USB 2.0 PHY.
 
+config PHY_SUN4I_USB
+	tristate "Allwinner sunxi SoC USB PHY driver"
+	depends on ARCH_SUNXI
+	select GENERIC_PHY
+	help
+	  Enable this to support the transceiver that is part of Allwinner
+	  sunxi SoCs.
+
+	  This driver controls the entire USB PHY block, both the USB OTG
+	  parts, as well as the 2 regular USB 2 host PHYs.
+
 endmenu
drivers/phy/Makefile +1 -0
index b57c253..9d4f8bb 100644
@@ -9,3 +9,4 @@ obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO)	+= phy-exynos-mipi-video.o
 obj-$(CONFIG_PHY_MVEBU_SATA)		+= phy-mvebu-sata.o
 obj-$(CONFIG_OMAP_USB2)			+= phy-omap-usb2.o
 obj-$(CONFIG_TWL4030_USB)		+= phy-twl4030-usb.o
+obj-$(CONFIG_PHY_SUN4I_USB)		+= phy-sun4i-usb.o
drivers/phy/phy-sun4i-usb.c new file +237 -0
new file mode 100644
index 0000000..bd9cb7fa
@@ -0,0 +1,326 @@
+/*
+ * Allwinner sun4i USB phy driver
+ *
+ * Copyright (C) 2014 Hans de Goede <[email protected]>
+ *
+ * Based on code from
+ * Allwinner Technology Co., Ltd. <www.allwinnertech.com>
+ *
+ * Modelled after: Samsung S5P/EXYNOS SoC series MIPI CSIS/DSIM DPHY driver
+ * Copyright (C) 2013 Samsung Electronics Co., Ltd.
+ * Author: Sylwester Nawrocki <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * 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/clk.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/consumer.h>
+#include <linux/reset.h>
+
+#define REG_ISCR			0x00
+#define REG_PHYCTL			0x04
+#define REG_PHYBIST			0x08
+#define REG_PHYTUNE			0x0c
+
+#define SUNXI_AHB_ICHR8_EN		BIT(10)
+#define SUNXI_AHB_INCR4_BURST_EN	BIT(9)
+#define SUNXI_AHB_INCRX_ALIGN_EN	BIT(8)
+#define SUNXI_ULPI_BYPASS_EN		BIT(0)
+
+/* Common Control Bits for Both PHYs */
+#define PHY_PLL_BW			0x03
+#define PHY_RES45_CAL_EN		0x0c
+
+/* Private Control Bits for Each PHY */
+#define PHY_TX_AMPLITUDE_TUNE		0x20
+#define PHY_TX_SLEWRATE_TUNE		0x22
+#define PHY_VBUSVALID_TH_SEL		0x25
+#define PHY_PULLUP_RES_SEL		0x27
+#define PHY_OTG_FUNC_EN			0x28
+#define PHY_VBUS_DET_EN			0x29
+#define PHY_DISCON_TH_SEL		0x2a
+
+#define MAX_PHYS			3
+
+struct sun4i_usb_phy_data {
+	struct clk *clk;
+	void __iomem *base;
+	struct mutex mutex;
+	int num_phys;
+	u32 disc_thresh;
+	struct sun4i_usb_phy {
+		struct phy *phy;
+		void __iomem *pmu;
+		struct regulator *vbus;
+		struct reset_control *reset;
+		int index;
+	} phys[MAX_PHYS];
+};
+
+#define to_sun4i_usb_phy_data(phy) \
+	container_of((phy), struct sun4i_usb_phy_data, phys[(phy)->index])
+
+static void sun4i_usb_phy_write(struct sun4i_usb_phy *phy, u32 addr, u32 data,
+				int len)
+{
+	struct sun4i_usb_phy_data *phy_data = to_sun4i_usb_phy_data(phy);
+	u32 temp, usbc_bit = BIT(phy->index * 2);
+	int i;
+
+	mutex_lock(&phy_data->mutex);
+
+	for (i = 0; i < len; i++) {
+		temp = readl(phy_data->base + REG_PHYCTL);
+
+		/* clear the address portion */
+		temp &= ~(0xff << 8);
+
+		/* set the address */
+		temp |= ((addr + i) << 8);
+		writel(temp, phy_data->base + REG_PHYCTL);
+
+		/* set the data bit and clear usbc bit*/
+		temp = readb(phy_data->base + REG_PHYCTL);
+		if (data & 0x1)
+			temp |= BIT(7);
+		else
+			temp &= ~BIT(7);
+		temp &= ~usbc_bit;
+		writeb(temp, phy_data->base + REG_PHYCTL);
+
+		/* pulse usbc_bit */
+		temp = readb(phy_data->base + REG_PHYCTL);
+		temp |= usbc_bit;
+		writeb(temp, phy_data->base + REG_PHYCTL);
+
+		temp = readb(phy_data->base + REG_PHYCTL);
+		temp &= ~usbc_bit;
+		writeb(temp, phy_data->base + REG_PHYCTL);
+
+		data >>= 1;
+	}
+	mutex_unlock(&phy_data->mutex);
+}
+
+static void sun4i_usb_phy_passby(struct sun4i_usb_phy *phy, int enable)
+{
+	u32 bits, reg_value;
+
+	if (!phy->pmu)
+		return;
+
+	bits = SUNXI_AHB_ICHR8_EN | SUNXI_AHB_INCR4_BURST_EN |
+		SUNXI_AHB_INCRX_ALIGN_EN | SUNXI_ULPI_BYPASS_EN;
+
+	reg_value = readl(phy->pmu);
+
+	if (enable)
+		reg_value |= bits;
+	else
+		reg_value &= ~bits;
+
+	writel(reg_value, phy->pmu);
+}
+
+static int sun4i_usb_phy_init(struct phy *_phy)
+{
+	struct sun4i_usb_phy *phy = phy_get_drvdata(_phy);
+	struct sun4i_usb_phy_data *data = to_sun4i_usb_phy_data(phy);
+	int ret;
+
+	ret = clk_prepare_enable(data->clk);
+	if (ret)
+		return ret;
+
+	ret = reset_control_deassert(phy->reset);
+	if (ret) {
+		clk_disable_unprepare(data->clk);
+		return ret;
+	}
+
+	/* Adjust PHY's magnitude and rate */
+	sun4i_usb_phy_write(phy, PHY_TX_AMPLITUDE_TUNE, 0x14, 5);
+
+	/* Disconnect threshold adjustment */
+	sun4i_usb_phy_write(phy, PHY_DISCON_TH_SEL, data->disc_thresh, 2);
+
+	sun4i_usb_phy_passby(phy, 1);
+
+	return 0;
+}
+
+static int sun4i_usb_phy_exit(struct phy *_phy)
+{
+	struct sun4i_usb_phy *phy = phy_get_drvdata(_phy);
+	struct sun4i_usb_phy_data *data = to_sun4i_usb_phy_data(phy);
+
+	sun4i_usb_phy_passby(phy, 0);
+	reset_control_assert(phy->reset);
+	clk_disable_unprepare(data->clk);
+
+	return 0;
+}
+
+static int sun4i_usb_phy_power_on(struct phy *_phy)
+{
+	struct sun4i_usb_phy *phy = phy_get_drvdata(_phy);
+	int ret = 0;
+
+	if (phy->vbus)
+		ret = regulator_enable(phy->vbus);
+
+	return ret;
+}
+
+static int sun4i_usb_phy_power_off(struct phy *_phy)
+{
+	struct sun4i_usb_phy *phy = phy_get_drvdata(_phy);
+
+	if (phy->vbus)
+		regulator_disable(phy->vbus);
+
+	return 0;
+}
+
+static struct phy_ops sun4i_usb_phy_ops = {
+	.init		= sun4i_usb_phy_init,
+	.exit		= sun4i_usb_phy_exit,
+	.power_on	= sun4i_usb_phy_power_on,
+	.power_off	= sun4i_usb_phy_power_off,
+	.owner		= THIS_MODULE,
+};
+
+static struct phy *sun4i_usb_phy_xlate(struct device *dev,
+					struct of_phandle_args *args)
+{
+	struct sun4i_usb_phy_data *data = dev_get_drvdata(dev);
+
+	if (WARN_ON(args->args[0] == 0 || args->args[0] >= data->num_phys))
+		return ERR_PTR(-ENODEV);
+
+	return data->phys[args->args[0]].phy;
+}
+
+static int sun4i_usb_phy_probe(struct platform_device *pdev)
+{
+	struct sun4i_usb_phy_data *data;
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	void __iomem *pmu = NULL;
+	struct phy_provider *phy_provider;
+	struct reset_control *reset;
+	struct regulator *vbus;
+	struct phy *phy;
+	char name[16];
+	int i;
+
+	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	mutex_init(&data->mutex);

drivers/phy/phy-sun4i-usb.c line 247

Quoted 10 lines
drivers/phy/phy-sun4i-usb.c new file +10 -0
+
+	if (of_device_is_compatible(np, "allwinner,sun5i-a13-usb-phy"))
+		data->num_phys = 2;
+	else
+		data->num_phys = 3;
+
+	if (of_device_is_compatible(np, "allwinner,sun4i-a10-usb-phy"))
+		data->disc_thresh = 3;
+	else
+		data->disc_thresh = 2;

I'd still prefer to pass this through the .data field of of_device_id, but it looks much cleaner already :)

Quoted 81 lines
drivers/phy/phy-sun4i-usb.c new file +78 -0
+	data->clk = devm_clk_get(dev, "usb_phy");
+	if (IS_ERR(data->clk)) {
+		dev_err(dev, "could not get usb_phy clock\n");
+		return PTR_ERR(data->clk);
+	}
+
+	/* Skip 0, 0 is the phy for otg which is not yet supported. */
+	for (i = 1; i < data->num_phys; i++) {
+		snprintf(name, sizeof(name), "usb%d_vbus", i);
+		vbus = devm_regulator_get_optional(dev, name);
+		if (IS_ERR(vbus)) {
+			if (PTR_ERR(vbus) == -EPROBE_DEFER)
+				return -EPROBE_DEFER;
+			vbus = NULL;
+		}
+
+		snprintf(name, sizeof(name), "usb%d_reset", i);
+		reset = devm_reset_control_get(dev, name);
+		if (IS_ERR(phy)) {
+			dev_err(dev, "failed to get reset %s\n", name);
+			return PTR_ERR(phy);
+		}
+
+		if (i) { /* No pmu for usbc0 */
+			pmu = devm_ioremap_resource(dev,
+			      platform_get_resource(pdev, IORESOURCE_MEM, i));
+			if (IS_ERR(pmu))
+				return PTR_ERR(pmu);
+		}
+
+		phy = devm_phy_create(dev, &sun4i_usb_phy_ops, NULL);
+		if (IS_ERR(phy)) {
+			dev_err(dev, "failed to create PHY %d\n", i);
+			return PTR_ERR(phy);
+		}
+
+		data->phys[i].phy = phy;
+		data->phys[i].pmu = pmu;
+		data->phys[i].vbus = vbus;
+		data->phys[i].reset = reset;
+		data->phys[i].index = i;
+		phy_set_drvdata(phy, &data->phys[i]);
+	}
+
+	data->base = devm_ioremap_resource(dev,
+			platform_get_resource(pdev, IORESOURCE_MEM, 0));
+	if (IS_ERR(data->base))
+		return PTR_ERR(data->base);
+
+	dev_set_drvdata(dev, data);
+	phy_provider = devm_of_phy_provider_register(dev, sun4i_usb_phy_xlate);
+	if (IS_ERR(phy_provider))
+		return PTR_ERR(phy_provider);
+
+	return 0;
+}
+
+static const struct of_device_id sun4i_usb_phy_of_match[] = {
+	{ .compatible = "allwinner,sun4i-a10-usb-phy" },
+	{ .compatible = "allwinner,sun5i-a13-usb-phy" },
+	{ .compatible = "allwinner,sun7i-a20-usb-phy" },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, sun4i_usb_phy_of_match);
+
+static struct platform_driver sun4i_usb_phy_driver = {
+	.probe	= sun4i_usb_phy_probe,
+	.driver = {
+		.of_match_table	= sun4i_usb_phy_of_match,
+		.name  = "sun4i-usb-phy",
+		.owner = THIS_MODULE,
+	}
+};
+module_platform_driver(sun4i_usb_phy_driver);
+
+MODULE_DESCRIPTION("Allwinner sun4i USB phy driver");
+MODULE_AUTHOR("Hans de Goede <[email protected]>");
+MODULE_LICENSE("GPL v2");
Signature · 1 line
1.8.4.2
Signature · 10 lines
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140207/e5533146/attachment.sig>
#2 lore raw

PATCH v2 1/1 ARM: sunxi: Add driver for sunxi usb phy

Hi,

Quoted 31 lines · Maxime Ripard

Hi Hans,

It looks very nice, I just have a few comments below though.

Quoted 26 lines · Hans de Goede

The Allwinner A1x / A2x SoCs have 2 or 3 usb phys which are all accessed through a single set of registers. Besides this there are also some other phy related bits which need poking, which are per phy, but shared between the ohci and ehci controllers, so these are also controlled from this new phy driver.

---
  .../devicetree/bindings/phy/sun4i-usb-phy.txt      |  28 ++
  drivers/phy/Kconfig                                |  11 +
  drivers/phy/Makefile                               |   1 +
  drivers/phy/phy-sun4i-usb.c                        | 326 +++++++++++++++++++++
  4 files changed, 366 insertions(+)

create mode 100644 Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt create mode 100644 drivers/phy/phy-sun4i-usb.c

Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt new file +4 -0
new file mode 100644
index 0000000..f7eccb2
@@ -0,0 +1,28 @@
+Allwinner sun4i USB PHY
+-----------------------
+
+Required properties:
Quoted 8 lines

Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt line 9

Quoted 5 lines
Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt new file +5 -0
+- compatible : should be one of "allwinner,sun4i-a10-usb-phy",
+  "allwinner,sun5i-a13-usb-phy" or "allwinner,sun7i-a20-usb-phy"
+- reg : a list of offset + length pairs, the 1st list entry should point to
+  the phy base regs, the 2nd entry to the pmu reg for phy1, and the 3th
+  entry to the pmu reg of phy2 (for devices which have a phy2).

I'm concerned about devices that would only have a phy2 for some reason.

phy1 and phy2 are identical, so in that case we would just call the phy phy1 I guess, and specify its register where ever it lives and be done with it.

Quoted 3 lines

Using reg-names would be much more robust, and is quite painless to use. Just use platform_get_resource_by_name instead of platform_get_resource, and that's pretty much it.

The above argument does not really help to convince me to use register-names, I don't really see them as useful / necessary, adding support for them will just grow the driver-code, as well as the devicetree bindings docs, as well as the dts files.

But if you really really want me to use register-names, just say so and I'll modify the patch.

Quoted 297 lines
Quoted 297 lines
Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt new file +19 -0
+- #phy-cells : from the generic phy bindings, must be 1
+
+Optional properties:
+- clocks : phandle + clock specifier for the phy clock
+- clock-names : "usb_phy"
+- resets : a list of phandle + reset specifier pairs
+- reset-names : "usb0_reset", "usb1_reset", and / or "usb2_reset"
+
+Example:
+	usbphy: phy at 0x01c13400 {
+		#phy-cells = <1>;
+		compatible = "allwinner,sun4i-a10-usb-phy";
+		/* phy base regs, phy1 pmu reg, phy2 pmu reg */
+		reg = <0x01c13400 0x10 0x01c14800 0x4 0x01c1c800 0x4>;
+		clocks = <&usb_clk 8>;
+		clock-names = "usb_phy";
+		resets = <&usb_clk 1>, <&usb_clk 2>;
+		reset-names = "usb1_reset", "usb2_reset";
+	};
drivers/phy/Kconfig +11 -0
index afa2354..6070c99 100644
@@ -64,4 +64,15 @@ config BCM_KONA_USB2_PHY
  	help
  	  Enable this to support the Broadcom Kona USB 2.0 PHY.
 
+config PHY_SUN4I_USB
+	tristate "Allwinner sunxi SoC USB PHY driver"
+	depends on ARCH_SUNXI
+	select GENERIC_PHY
+	help
+	  Enable this to support the transceiver that is part of Allwinner
+	  sunxi SoCs.
+
+	  This driver controls the entire USB PHY block, both the USB OTG
+	  parts, as well as the 2 regular USB 2 host PHYs.
+
  endmenu
drivers/phy/Makefile +1 -0
index b57c253..9d4f8bb 100644
@@ -9,3 +9,4 @@ obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO)	+= phy-exynos-mipi-video.o
  obj-$(CONFIG_PHY_MVEBU_SATA)		+= phy-mvebu-sata.o
  obj-$(CONFIG_OMAP_USB2)			+= phy-omap-usb2.o
  obj-$(CONFIG_TWL4030_USB)		+= phy-twl4030-usb.o
+obj-$(CONFIG_PHY_SUN4I_USB)		+= phy-sun4i-usb.o
drivers/phy/phy-sun4i-usb.c new file +242 -0
new file mode 100644
index 0000000..bd9cb7fa
@@ -0,0 +1,326 @@
+/*
+ * Allwinner sun4i USB phy driver
+ *
+ * Copyright (C) 2014 Hans de Goede <[email protected]>
+ *
+ * Based on code from
+ * Allwinner Technology Co., Ltd. <www.allwinnertech.com>
+ *
+ * Modelled after: Samsung S5P/EXYNOS SoC series MIPI CSIS/DSIM DPHY driver
+ * Copyright (C) 2013 Samsung Electronics Co., Ltd.
+ * Author: Sylwester Nawrocki <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * 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/clk.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/consumer.h>
+#include <linux/reset.h>
+
+#define REG_ISCR			0x00
+#define REG_PHYCTL			0x04
+#define REG_PHYBIST			0x08
+#define REG_PHYTUNE			0x0c
+
+#define SUNXI_AHB_ICHR8_EN		BIT(10)
+#define SUNXI_AHB_INCR4_BURST_EN	BIT(9)
+#define SUNXI_AHB_INCRX_ALIGN_EN	BIT(8)
+#define SUNXI_ULPI_BYPASS_EN		BIT(0)
+
+/* Common Control Bits for Both PHYs */
+#define PHY_PLL_BW			0x03
+#define PHY_RES45_CAL_EN		0x0c
+
+/* Private Control Bits for Each PHY */
+#define PHY_TX_AMPLITUDE_TUNE		0x20
+#define PHY_TX_SLEWRATE_TUNE		0x22
+#define PHY_VBUSVALID_TH_SEL		0x25
+#define PHY_PULLUP_RES_SEL		0x27
+#define PHY_OTG_FUNC_EN			0x28
+#define PHY_VBUS_DET_EN			0x29
+#define PHY_DISCON_TH_SEL		0x2a
+
+#define MAX_PHYS			3
+
+struct sun4i_usb_phy_data {
+	struct clk *clk;
+	void __iomem *base;
+	struct mutex mutex;
+	int num_phys;
+	u32 disc_thresh;
+	struct sun4i_usb_phy {
+		struct phy *phy;
+		void __iomem *pmu;
+		struct regulator *vbus;
+		struct reset_control *reset;
+		int index;
+	} phys[MAX_PHYS];
+};
+
+#define to_sun4i_usb_phy_data(phy) \
+	container_of((phy), struct sun4i_usb_phy_data, phys[(phy)->index])
+
+static void sun4i_usb_phy_write(struct sun4i_usb_phy *phy, u32 addr, u32 data,
+				int len)
+{
+	struct sun4i_usb_phy_data *phy_data = to_sun4i_usb_phy_data(phy);
+	u32 temp, usbc_bit = BIT(phy->index * 2);
+	int i;
+
+	mutex_lock(&phy_data->mutex);
+
+	for (i = 0; i < len; i++) {
+		temp = readl(phy_data->base + REG_PHYCTL);
+
+		/* clear the address portion */
+		temp &= ~(0xff << 8);
+
+		/* set the address */
+		temp |= ((addr + i) << 8);
+		writel(temp, phy_data->base + REG_PHYCTL);
+
+		/* set the data bit and clear usbc bit*/
+		temp = readb(phy_data->base + REG_PHYCTL);
+		if (data & 0x1)
+			temp |= BIT(7);
+		else
+			temp &= ~BIT(7);
+		temp &= ~usbc_bit;
+		writeb(temp, phy_data->base + REG_PHYCTL);
+
+		/* pulse usbc_bit */
+		temp = readb(phy_data->base + REG_PHYCTL);
+		temp |= usbc_bit;
+		writeb(temp, phy_data->base + REG_PHYCTL);
+
+		temp = readb(phy_data->base + REG_PHYCTL);
+		temp &= ~usbc_bit;
+		writeb(temp, phy_data->base + REG_PHYCTL);
+
+		data >>= 1;
+	}
+	mutex_unlock(&phy_data->mutex);
+}
+
+static void sun4i_usb_phy_passby(struct sun4i_usb_phy *phy, int enable)
+{
+	u32 bits, reg_value;
+
+	if (!phy->pmu)
+		return;
+
+	bits = SUNXI_AHB_ICHR8_EN | SUNXI_AHB_INCR4_BURST_EN |
+		SUNXI_AHB_INCRX_ALIGN_EN | SUNXI_ULPI_BYPASS_EN;
+
+	reg_value = readl(phy->pmu);
+
+	if (enable)
+		reg_value |= bits;
+	else
+		reg_value &= ~bits;
+
+	writel(reg_value, phy->pmu);
+}
+
+static int sun4i_usb_phy_init(struct phy *_phy)
+{
+	struct sun4i_usb_phy *phy = phy_get_drvdata(_phy);
+	struct sun4i_usb_phy_data *data = to_sun4i_usb_phy_data(phy);
+	int ret;
+
+	ret = clk_prepare_enable(data->clk);
+	if (ret)
+		return ret;
+
+	ret = reset_control_deassert(phy->reset);
+	if (ret) {
+		clk_disable_unprepare(data->clk);
+		return ret;
+	}
+
+	/* Adjust PHY's magnitude and rate */
+	sun4i_usb_phy_write(phy, PHY_TX_AMPLITUDE_TUNE, 0x14, 5);
+
+	/* Disconnect threshold adjustment */
+	sun4i_usb_phy_write(phy, PHY_DISCON_TH_SEL, data->disc_thresh, 2);
+
+	sun4i_usb_phy_passby(phy, 1);
+
+	return 0;
+}
+
+static int sun4i_usb_phy_exit(struct phy *_phy)
+{
+	struct sun4i_usb_phy *phy = phy_get_drvdata(_phy);
+	struct sun4i_usb_phy_data *data = to_sun4i_usb_phy_data(phy);
+
+	sun4i_usb_phy_passby(phy, 0);
+	reset_control_assert(phy->reset);
+	clk_disable_unprepare(data->clk);
+
+	return 0;
+}
+
+static int sun4i_usb_phy_power_on(struct phy *_phy)
+{
+	struct sun4i_usb_phy *phy = phy_get_drvdata(_phy);
+	int ret = 0;
+
+	if (phy->vbus)
+		ret = regulator_enable(phy->vbus);
+
+	return ret;
+}
+
+static int sun4i_usb_phy_power_off(struct phy *_phy)
+{
+	struct sun4i_usb_phy *phy = phy_get_drvdata(_phy);
+
+	if (phy->vbus)
+		regulator_disable(phy->vbus);
+
+	return 0;
+}
+
+static struct phy_ops sun4i_usb_phy_ops = {
+	.init		= sun4i_usb_phy_init,
+	.exit		= sun4i_usb_phy_exit,
+	.power_on	= sun4i_usb_phy_power_on,
+	.power_off	= sun4i_usb_phy_power_off,
+	.owner		= THIS_MODULE,
+};
+
+static struct phy *sun4i_usb_phy_xlate(struct device *dev,
+					struct of_phandle_args *args)
+{
+	struct sun4i_usb_phy_data *data = dev_get_drvdata(dev);
+
+	if (WARN_ON(args->args[0] == 0 || args->args[0] >= data->num_phys))
+		return ERR_PTR(-ENODEV);
+
+	return data->phys[args->args[0]].phy;
+}
+
+static int sun4i_usb_phy_probe(struct platform_device *pdev)
+{
+	struct sun4i_usb_phy_data *data;
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	void __iomem *pmu = NULL;
+	struct phy_provider *phy_provider;
+	struct reset_control *reset;
+	struct regulator *vbus;
+	struct phy *phy;
+	char name[16];
+	int i;
+
+	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	mutex_init(&data->mutex);
+
+	if (of_device_is_compatible(np, "allwinner,sun5i-a13-usb-phy"))
+		data->num_phys = 2;
+	else
+		data->num_phys = 3;
Quoted 8 lines

drivers/phy/phy-sun4i-usb.c line 247

Quoted 5 lines
drivers/phy/phy-sun4i-usb.c new file +5 -0
+
+	if (of_device_is_compatible(np, "allwinner,sun4i-a10-usb-phy"))
+		data->disc_thresh = 3;
+	else
+		data->disc_thresh = 2;

I'd still prefer to pass this through the .data field of of_device_id, but it looks much cleaner already :)

The problem with using the .data field is that I can only store a single integer there. To store 2 I need to: define a struct, create an array of these structs with initialization. Create an enum for indexing the array which must be kept in sync with the initializers manually, store either the index, or a direct pointer to the correct array entry into the .data field, add code to get the of_device_id from the compatible string, and then finally extract the settings from the struct again.

See IE how this is done in drivers/ata/ahci_platform.c, I've tried to come up with a simpler way and failed, for ahci_platform.c the struct with per compatible-string data is quite big so it makes some sense to use this construction. Here however not so much, this adds a whole lot of unnecessary extra code + indirection. I esp. object against the indirection as that unnecessarily makes it harder to follow whats going on.

Quoted 74 lines
Quoted 74 lines
drivers/phy/phy-sun4i-usb.c new file +73 -0
+	data->clk = devm_clk_get(dev, "usb_phy");
+	if (IS_ERR(data->clk)) {
+		dev_err(dev, "could not get usb_phy clock\n");
+		return PTR_ERR(data->clk);
+	}
+
+	/* Skip 0, 0 is the phy for otg which is not yet supported. */
+	for (i = 1; i < data->num_phys; i++) {
+		snprintf(name, sizeof(name), "usb%d_vbus", i);
+		vbus = devm_regulator_get_optional(dev, name);
+		if (IS_ERR(vbus)) {
+			if (PTR_ERR(vbus) == -EPROBE_DEFER)
+				return -EPROBE_DEFER;
+			vbus = NULL;
+		}
+
+		snprintf(name, sizeof(name), "usb%d_reset", i);
+		reset = devm_reset_control_get(dev, name);
+		if (IS_ERR(phy)) {
+			dev_err(dev, "failed to get reset %s\n", name);
+			return PTR_ERR(phy);
+		}
+
+		if (i) { /* No pmu for usbc0 */
+			pmu = devm_ioremap_resource(dev,
+			      platform_get_resource(pdev, IORESOURCE_MEM, i));
+			if (IS_ERR(pmu))
+				return PTR_ERR(pmu);
+		}
+
+		phy = devm_phy_create(dev, &sun4i_usb_phy_ops, NULL);
+		if (IS_ERR(phy)) {
+			dev_err(dev, "failed to create PHY %d\n", i);
+			return PTR_ERR(phy);
+		}
+
+		data->phys[i].phy = phy;
+		data->phys[i].pmu = pmu;
+		data->phys[i].vbus = vbus;
+		data->phys[i].reset = reset;
+		data->phys[i].index = i;
+		phy_set_drvdata(phy, &data->phys[i]);
+	}
+
+	data->base = devm_ioremap_resource(dev,
+			platform_get_resource(pdev, IORESOURCE_MEM, 0));
+	if (IS_ERR(data->base))
+		return PTR_ERR(data->base);
+
+	dev_set_drvdata(dev, data);
+	phy_provider = devm_of_phy_provider_register(dev, sun4i_usb_phy_xlate);
+	if (IS_ERR(phy_provider))
+		return PTR_ERR(phy_provider);
+
+	return 0;
+}
+
+static const struct of_device_id sun4i_usb_phy_of_match[] = {
+	{ .compatible = "allwinner,sun4i-a10-usb-phy" },
+	{ .compatible = "allwinner,sun5i-a13-usb-phy" },
+	{ .compatible = "allwinner,sun7i-a20-usb-phy" },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, sun4i_usb_phy_of_match);
+
+static struct platform_driver sun4i_usb_phy_driver = {
+	.probe	= sun4i_usb_phy_probe,
+	.driver = {
+		.of_match_table	= sun4i_usb_phy_of_match,
+		.name  = "sun4i-usb-phy",
+		.owner = THIS_MODULE,
+	}
+};
Quoted 9 lines
Quoted 8 lines
drivers/phy/phy-sun4i-usb.c new file +5 -1
+module_platform_driver(sun4i_usb_phy_driver);
+
+MODULE_DESCRIPTION("Allwinner sun4i USB phy driver");
+MODULE_AUTHOR("Hans de Goede <[email protected]>");
+MODULE_LICENSE("GPL v2");
--

1.8.4.2

Regards,

Hans

#3 lore raw

PATCH v2 1/1 ARM: sunxi: Add driver for sunxi usb phy

Hi,

Quoted 51 lines · Hans de Goede

Hi,

Quoted 31 lines · Maxime Ripard

Hi Hans,

It looks very nice, I just have a few comments below though.

Quoted 26 lines · Hans de Goede

The Allwinner A1x / A2x SoCs have 2 or 3 usb phys which are all accessed through a single set of registers. Besides this there are also some other phy related bits which need poking, which are per phy, but shared between the ohci and ehci controllers, so these are also controlled from this new phy driver.

---
 .../devicetree/bindings/phy/sun4i-usb-phy.txt      |  28 ++
 drivers/phy/Kconfig                                |  11 +
 drivers/phy/Makefile                               |   1 +
 drivers/phy/phy-sun4i-usb.c                        | 326 +++++++++++++++++++++
 4 files changed, 366 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
 create mode 100644 drivers/phy/phy-sun4i-usb.c
Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt new file +4 -0
new file mode 100644
index 0000000..f7eccb2
@@ -0,0 +1,28 @@
+Allwinner sun4i USB PHY
+-----------------------
+
+Required properties:
Quoted 8 lines

Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt line 9

Quoted 5 lines
Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt new file +5 -0
+- compatible : should be one of "allwinner,sun4i-a10-usb-phy",
+  "allwinner,sun5i-a13-usb-phy" or "allwinner,sun7i-a20-usb-phy"
+- reg : a list of offset + length pairs, the 1st list entry should point to
+  the phy base regs, the 2nd entry to the pmu reg for phy1, and the 3th
+  entry to the pmu reg of phy2 (for devices which have a phy2).

I'm concerned about devices that would only have a phy2 for some reason.

phy1 and phy2 are identical, so in that case we would just call the phy phy1 I guess, and specify its register where ever it lives and be done with it.

Quoted 3 lines

Using reg-names would be much more robust, and is quite painless to use. Just use platform_get_resource_by_name instead of platform_get_resource, and that's pretty much it.

Quoted 7 lines

The above argument does not really help to convince me to use register-names, I don't really see them as useful / necessary, adding support for them will just grow the driver-code, as well as the devicetree bindings docs, as well as the dts files.

But if you really really want me to use register-names, just say so and I'll modify the patch.

Yep, overall, whenever there's several resources involved, I very much prefer to differentiate them by name, rather than by index, which is much more fragile.

And the overhead is of one single line in the DT, one single line in the doc, and a few of them in the driver. It doesn't look like it bloats the code that much...

Quoted 316 lines
Quoted 299 lines
Quoted 299 lines
Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt new file +19 -0
+- #phy-cells : from the generic phy bindings, must be 1
+
+Optional properties:
+- clocks : phandle + clock specifier for the phy clock
+- clock-names : "usb_phy"
+- resets : a list of phandle + reset specifier pairs
+- reset-names : "usb0_reset", "usb1_reset", and / or "usb2_reset"
+
+Example:
+	usbphy: phy at 0x01c13400 {
+		#phy-cells = <1>;
+		compatible = "allwinner,sun4i-a10-usb-phy";
+		/* phy base regs, phy1 pmu reg, phy2 pmu reg */
+		reg = <0x01c13400 0x10 0x01c14800 0x4 0x01c1c800 0x4>;
+		clocks = <&usb_clk 8>;
+		clock-names = "usb_phy";
+		resets = <&usb_clk 1>, <&usb_clk 2>;
+		reset-names = "usb1_reset", "usb2_reset";
+	};
drivers/phy/Kconfig +11 -0
index afa2354..6070c99 100644
@@ -64,4 +64,15 @@ config BCM_KONA_USB2_PHY
 	help
 	  Enable this to support the Broadcom Kona USB 2.0 PHY.
 
+config PHY_SUN4I_USB
+	tristate "Allwinner sunxi SoC USB PHY driver"
+	depends on ARCH_SUNXI
+	select GENERIC_PHY
+	help
+	  Enable this to support the transceiver that is part of Allwinner
+	  sunxi SoCs.
+
+	  This driver controls the entire USB PHY block, both the USB OTG
+	  parts, as well as the 2 regular USB 2 host PHYs.
+
 endmenu
drivers/phy/Makefile +1 -0
index b57c253..9d4f8bb 100644
@@ -9,3 +9,4 @@ obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO)	+= phy-exynos-mipi-video.o
 obj-$(CONFIG_PHY_MVEBU_SATA)		+= phy-mvebu-sata.o
 obj-$(CONFIG_OMAP_USB2)			+= phy-omap-usb2.o
 obj-$(CONFIG_TWL4030_USB)		+= phy-twl4030-usb.o
+obj-$(CONFIG_PHY_SUN4I_USB)		+= phy-sun4i-usb.o
drivers/phy/phy-sun4i-usb.c new file +242 -0
new file mode 100644
index 0000000..bd9cb7fa
@@ -0,0 +1,326 @@
+/*
+ * Allwinner sun4i USB phy driver
+ *
+ * Copyright (C) 2014 Hans de Goede <[email protected]>
+ *
+ * Based on code from
+ * Allwinner Technology Co., Ltd. <www.allwinnertech.com>
+ *
+ * Modelled after: Samsung S5P/EXYNOS SoC series MIPI CSIS/DSIM DPHY driver
+ * Copyright (C) 2013 Samsung Electronics Co., Ltd.
+ * Author: Sylwester Nawrocki <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * 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/clk.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/consumer.h>
+#include <linux/reset.h>
+
+#define REG_ISCR			0x00
+#define REG_PHYCTL			0x04
+#define REG_PHYBIST			0x08
+#define REG_PHYTUNE			0x0c
+
+#define SUNXI_AHB_ICHR8_EN		BIT(10)
+#define SUNXI_AHB_INCR4_BURST_EN	BIT(9)
+#define SUNXI_AHB_INCRX_ALIGN_EN	BIT(8)
+#define SUNXI_ULPI_BYPASS_EN		BIT(0)
+
+/* Common Control Bits for Both PHYs */
+#define PHY_PLL_BW			0x03
+#define PHY_RES45_CAL_EN		0x0c
+
+/* Private Control Bits for Each PHY */
+#define PHY_TX_AMPLITUDE_TUNE		0x20
+#define PHY_TX_SLEWRATE_TUNE		0x22
+#define PHY_VBUSVALID_TH_SEL		0x25
+#define PHY_PULLUP_RES_SEL		0x27
+#define PHY_OTG_FUNC_EN			0x28
+#define PHY_VBUS_DET_EN			0x29
+#define PHY_DISCON_TH_SEL		0x2a
+
+#define MAX_PHYS			3
+
+struct sun4i_usb_phy_data {
+	struct clk *clk;
+	void __iomem *base;
+	struct mutex mutex;
+	int num_phys;
+	u32 disc_thresh;
+	struct sun4i_usb_phy {
+		struct phy *phy;
+		void __iomem *pmu;
+		struct regulator *vbus;
+		struct reset_control *reset;
+		int index;
+	} phys[MAX_PHYS];
+};
+
+#define to_sun4i_usb_phy_data(phy) \
+	container_of((phy), struct sun4i_usb_phy_data, phys[(phy)->index])
+
+static void sun4i_usb_phy_write(struct sun4i_usb_phy *phy, u32 addr, u32 data,
+				int len)
+{
+	struct sun4i_usb_phy_data *phy_data = to_sun4i_usb_phy_data(phy);
+	u32 temp, usbc_bit = BIT(phy->index * 2);
+	int i;
+
+	mutex_lock(&phy_data->mutex);
+
+	for (i = 0; i < len; i++) {
+		temp = readl(phy_data->base + REG_PHYCTL);
+
+		/* clear the address portion */
+		temp &= ~(0xff << 8);
+
+		/* set the address */
+		temp |= ((addr + i) << 8);
+		writel(temp, phy_data->base + REG_PHYCTL);
+
+		/* set the data bit and clear usbc bit*/
+		temp = readb(phy_data->base + REG_PHYCTL);
+		if (data & 0x1)
+			temp |= BIT(7);
+		else
+			temp &= ~BIT(7);
+		temp &= ~usbc_bit;
+		writeb(temp, phy_data->base + REG_PHYCTL);
+
+		/* pulse usbc_bit */
+		temp = readb(phy_data->base + REG_PHYCTL);
+		temp |= usbc_bit;
+		writeb(temp, phy_data->base + REG_PHYCTL);
+
+		temp = readb(phy_data->base + REG_PHYCTL);
+		temp &= ~usbc_bit;
+		writeb(temp, phy_data->base + REG_PHYCTL);
+
+		data >>= 1;
+	}
+	mutex_unlock(&phy_data->mutex);
+}
+
+static void sun4i_usb_phy_passby(struct sun4i_usb_phy *phy, int enable)
+{
+	u32 bits, reg_value;
+
+	if (!phy->pmu)
+		return;
+
+	bits = SUNXI_AHB_ICHR8_EN | SUNXI_AHB_INCR4_BURST_EN |
+		SUNXI_AHB_INCRX_ALIGN_EN | SUNXI_ULPI_BYPASS_EN;
+
+	reg_value = readl(phy->pmu);
+
+	if (enable)
+		reg_value |= bits;
+	else
+		reg_value &= ~bits;
+
+	writel(reg_value, phy->pmu);
+}
+
+static int sun4i_usb_phy_init(struct phy *_phy)
+{
+	struct sun4i_usb_phy *phy = phy_get_drvdata(_phy);
+	struct sun4i_usb_phy_data *data = to_sun4i_usb_phy_data(phy);
+	int ret;
+
+	ret = clk_prepare_enable(data->clk);
+	if (ret)
+		return ret;
+
+	ret = reset_control_deassert(phy->reset);
+	if (ret) {
+		clk_disable_unprepare(data->clk);
+		return ret;
+	}
+
+	/* Adjust PHY's magnitude and rate */
+	sun4i_usb_phy_write(phy, PHY_TX_AMPLITUDE_TUNE, 0x14, 5);
+
+	/* Disconnect threshold adjustment */
+	sun4i_usb_phy_write(phy, PHY_DISCON_TH_SEL, data->disc_thresh, 2);
+
+	sun4i_usb_phy_passby(phy, 1);
+
+	return 0;
+}
+
+static int sun4i_usb_phy_exit(struct phy *_phy)
+{
+	struct sun4i_usb_phy *phy = phy_get_drvdata(_phy);
+	struct sun4i_usb_phy_data *data = to_sun4i_usb_phy_data(phy);
+
+	sun4i_usb_phy_passby(phy, 0);
+	reset_control_assert(phy->reset);
+	clk_disable_unprepare(data->clk);
+
+	return 0;
+}
+
+static int sun4i_usb_phy_power_on(struct phy *_phy)
+{
+	struct sun4i_usb_phy *phy = phy_get_drvdata(_phy);
+	int ret = 0;
+
+	if (phy->vbus)
+		ret = regulator_enable(phy->vbus);
+
+	return ret;
+}
+
+static int sun4i_usb_phy_power_off(struct phy *_phy)
+{
+	struct sun4i_usb_phy *phy = phy_get_drvdata(_phy);
+
+	if (phy->vbus)
+		regulator_disable(phy->vbus);
+
+	return 0;
+}
+
+static struct phy_ops sun4i_usb_phy_ops = {
+	.init		= sun4i_usb_phy_init,
+	.exit		= sun4i_usb_phy_exit,
+	.power_on	= sun4i_usb_phy_power_on,
+	.power_off	= sun4i_usb_phy_power_off,
+	.owner		= THIS_MODULE,
+};
+
+static struct phy *sun4i_usb_phy_xlate(struct device *dev,
+					struct of_phandle_args *args)
+{
+	struct sun4i_usb_phy_data *data = dev_get_drvdata(dev);
+
+	if (WARN_ON(args->args[0] == 0 || args->args[0] >= data->num_phys))
+		return ERR_PTR(-ENODEV);
+
+	return data->phys[args->args[0]].phy;
+}
+
+static int sun4i_usb_phy_probe(struct platform_device *pdev)
+{
+	struct sun4i_usb_phy_data *data;
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	void __iomem *pmu = NULL;
+	struct phy_provider *phy_provider;
+	struct reset_control *reset;
+	struct regulator *vbus;
+	struct phy *phy;
+	char name[16];
+	int i;
+
+	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	mutex_init(&data->mutex);
+
+	if (of_device_is_compatible(np, "allwinner,sun5i-a13-usb-phy"))
+		data->num_phys = 2;
+	else
+		data->num_phys = 3;
Quoted 8 lines

drivers/phy/phy-sun4i-usb.c line 247

Quoted 5 lines
drivers/phy/phy-sun4i-usb.c new file +5 -0
+
+	if (of_device_is_compatible(np, "allwinner,sun4i-a10-usb-phy"))
+		data->disc_thresh = 3;
+	else
+		data->disc_thresh = 2;

I'd still prefer to pass this through the .data field of of_device_id, but it looks much cleaner already :)

The problem with using the .data field is that I can only store a single integer there. To store 2 I need to: define a struct, create an array of these structs with initialization. Create an enum for indexing the array which must be kept in sync with the initializers manually, store either the index, or a direct pointer to the correct array entry into the .data field, add code to get the of_device_id from the compatible string, and then finally extract the settings from the struct again.

Quoted 7 lines

See IE how this is done in drivers/ata/ahci_platform.c, I've tried to come up with a simpler way and failed, for ahci_platform.c the struct with per compatible-string data is quite big so it makes some sense to use this construction. Here however not so much, this adds a whole lot of unnecessary extra code + indirection. I esp. object against the indirection as that unnecessarily makes it harder to follow whats going on.

Ok, thanks!
Maxime

Signature · 10 lines
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140210/03b2d275/attachment.sig>
#4 lore raw

PATCH v2 1/1 ARM: sunxi: Add driver for sunxi usb phy

In reply to Hans de Goede
Quoted 16 lines · Hans de Goede

The Allwinner A1x / A2x SoCs have 2 or 3 usb phys which are all accessed through a single set of registers. Besides this there are also some other phy related bits which need poking, which are per phy, but shared between the ohci and ehci controllers, so these are also controlled from this new phy driver.

---
 .../devicetree/bindings/phy/sun4i-usb-phy.txt      |  28 ++
 drivers/phy/Kconfig                                |  11 +
 drivers/phy/Makefile                               |   1 +
 drivers/phy/phy-sun4i-usb.c                        | 326 +++++++++++++++++++++
 4 files changed, 366 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
 create mode 100644 drivers/phy/phy-sun4i-usb.c

[...]

Quoted 267 lines
drivers/phy/phy-sun4i-usb.c new file +261 -0
new file mode 100644
index 0000000..bd9cb7fa
@@ -0,0 +1,326 @@
+/*
+ * Allwinner sun4i USB phy driver
+ *
+ * Copyright (C) 2014 Hans de Goede <[email protected]>
+ *
+ * Based on code from
+ * Allwinner Technology Co., Ltd. <www.allwinnertech.com>
+ *
+ * Modelled after: Samsung S5P/EXYNOS SoC series MIPI CSIS/DSIM DPHY driver
+ * Copyright (C) 2013 Samsung Electronics Co., Ltd.
+ * Author: Sylwester Nawrocki <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * 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/clk.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/consumer.h>
+#include <linux/reset.h>
+
+#define REG_ISCR                       0x00
+#define REG_PHYCTL                     0x04
+#define REG_PHYBIST                    0x08
+#define REG_PHYTUNE                    0x0c
+
+#define SUNXI_AHB_ICHR8_EN             BIT(10)
+#define SUNXI_AHB_INCR4_BURST_EN       BIT(9)
+#define SUNXI_AHB_INCRX_ALIGN_EN       BIT(8)
+#define SUNXI_ULPI_BYPASS_EN           BIT(0)
+
+/* Common Control Bits for Both PHYs */
+#define PHY_PLL_BW                     0x03
+#define PHY_RES45_CAL_EN               0x0c
+
+/* Private Control Bits for Each PHY */
+#define PHY_TX_AMPLITUDE_TUNE          0x20
+#define PHY_TX_SLEWRATE_TUNE           0x22
+#define PHY_VBUSVALID_TH_SEL           0x25
+#define PHY_PULLUP_RES_SEL             0x27
+#define PHY_OTG_FUNC_EN                        0x28
+#define PHY_VBUS_DET_EN                        0x29
+#define PHY_DISCON_TH_SEL              0x2a
+
+#define MAX_PHYS                       3
+
+struct sun4i_usb_phy_data {
+       struct clk *clk;
+       void __iomem *base;
+       struct mutex mutex;
+       int num_phys;
+       u32 disc_thresh;
+       struct sun4i_usb_phy {
+               struct phy *phy;
+               void __iomem *pmu;
+               struct regulator *vbus;
+               struct reset_control *reset;
+               int index;
+       } phys[MAX_PHYS];
+};
+
+#define to_sun4i_usb_phy_data(phy) \
+       container_of((phy), struct sun4i_usb_phy_data, phys[(phy)->index])
+
+static void sun4i_usb_phy_write(struct sun4i_usb_phy *phy, u32 addr, u32 data,
+                               int len)
+{
+       struct sun4i_usb_phy_data *phy_data = to_sun4i_usb_phy_data(phy);
+       u32 temp, usbc_bit = BIT(phy->index * 2);
+       int i;
+
+       mutex_lock(&phy_data->mutex);
+
+       for (i = 0; i < len; i++) {
+               temp = readl(phy_data->base + REG_PHYCTL);
+
+               /* clear the address portion */
+               temp &= ~(0xff << 8);
+
+               /* set the address */
+               temp |= ((addr + i) << 8);
+               writel(temp, phy_data->base + REG_PHYCTL);
+
+               /* set the data bit and clear usbc bit*/
+               temp = readb(phy_data->base + REG_PHYCTL);
+               if (data & 0x1)
+                       temp |= BIT(7);
+               else
+                       temp &= ~BIT(7);
+               temp &= ~usbc_bit;
+               writeb(temp, phy_data->base + REG_PHYCTL);
+
+               /* pulse usbc_bit */
+               temp = readb(phy_data->base + REG_PHYCTL);
+               temp |= usbc_bit;
+               writeb(temp, phy_data->base + REG_PHYCTL);
+
+               temp = readb(phy_data->base + REG_PHYCTL);
+               temp &= ~usbc_bit;
+               writeb(temp, phy_data->base + REG_PHYCTL);
+
+               data >>= 1;
+       }
+       mutex_unlock(&phy_data->mutex);
+}
+
+static void sun4i_usb_phy_passby(struct sun4i_usb_phy *phy, int enable)
+{
+       u32 bits, reg_value;
+
+       if (!phy->pmu)
+               return;
+
+       bits = SUNXI_AHB_ICHR8_EN | SUNXI_AHB_INCR4_BURST_EN |
+               SUNXI_AHB_INCRX_ALIGN_EN | SUNXI_ULPI_BYPASS_EN;
+
+       reg_value = readl(phy->pmu);
+
+       if (enable)
+               reg_value |= bits;
+       else
+               reg_value &= ~bits;
+
+       writel(reg_value, phy->pmu);
+}
+
+static int sun4i_usb_phy_init(struct phy *_phy)
+{
+       struct sun4i_usb_phy *phy = phy_get_drvdata(_phy);
+       struct sun4i_usb_phy_data *data = to_sun4i_usb_phy_data(phy);
+       int ret;
+
+       ret = clk_prepare_enable(data->clk);
+       if (ret)
+               return ret;
+
+       ret = reset_control_deassert(phy->reset);
+       if (ret) {
+               clk_disable_unprepare(data->clk);
+               return ret;
+       }
+
+       /* Adjust PHY's magnitude and rate */
+       sun4i_usb_phy_write(phy, PHY_TX_AMPLITUDE_TUNE, 0x14, 5);
+
+       /* Disconnect threshold adjustment */
+       sun4i_usb_phy_write(phy, PHY_DISCON_TH_SEL, data->disc_thresh, 2);
+
+       sun4i_usb_phy_passby(phy, 1);
+
+       return 0;
+}
+
+static int sun4i_usb_phy_exit(struct phy *_phy)
+{
+       struct sun4i_usb_phy *phy = phy_get_drvdata(_phy);
+       struct sun4i_usb_phy_data *data = to_sun4i_usb_phy_data(phy);
+
+       sun4i_usb_phy_passby(phy, 0);
+       reset_control_assert(phy->reset);
+       clk_disable_unprepare(data->clk);
+
+       return 0;
+}
+
+static int sun4i_usb_phy_power_on(struct phy *_phy)
+{
+       struct sun4i_usb_phy *phy = phy_get_drvdata(_phy);
+       int ret = 0;
+
+       if (phy->vbus)
+               ret = regulator_enable(phy->vbus);
+
+       return ret;
+}
+
+static int sun4i_usb_phy_power_off(struct phy *_phy)
+{
+       struct sun4i_usb_phy *phy = phy_get_drvdata(_phy);
+
+       if (phy->vbus)
+               regulator_disable(phy->vbus);
+
+       return 0;
+}
+
+static struct phy_ops sun4i_usb_phy_ops = {
+       .init           = sun4i_usb_phy_init,
+       .exit           = sun4i_usb_phy_exit,
+       .power_on       = sun4i_usb_phy_power_on,
+       .power_off      = sun4i_usb_phy_power_off,
+       .owner          = THIS_MODULE,
+};
+
+static struct phy *sun4i_usb_phy_xlate(struct device *dev,
+                                       struct of_phandle_args *args)
+{
+       struct sun4i_usb_phy_data *data = dev_get_drvdata(dev);
+
+       if (WARN_ON(args->args[0] == 0 || args->args[0] >= data->num_phys))
+               return ERR_PTR(-ENODEV);
+
+       return data->phys[args->args[0]].phy;
+}
+
+static int sun4i_usb_phy_probe(struct platform_device *pdev)
+{
+       struct sun4i_usb_phy_data *data;
+       struct device *dev = &pdev->dev;
+       struct device_node *np = dev->of_node;
+       void __iomem *pmu = NULL;
+       struct phy_provider *phy_provider;
+       struct reset_control *reset;
+       struct regulator *vbus;
+       struct phy *phy;
+       char name[16];
+       int i;
+
+       data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+       if (!data)
+               return -ENOMEM;
+
+       mutex_init(&data->mutex);
+
+       if (of_device_is_compatible(np, "allwinner,sun5i-a13-usb-phy"))
+               data->num_phys = 2;
+       else
+               data->num_phys = 3;
+
+       if (of_device_is_compatible(np, "allwinner,sun4i-a10-usb-phy"))
+               data->disc_thresh = 3;
+       else
+               data->disc_thresh = 2;
+
+       data->clk = devm_clk_get(dev, "usb_phy");
+       if (IS_ERR(data->clk)) {
+               dev_err(dev, "could not get usb_phy clock\n");
+               return PTR_ERR(data->clk);
+       }
+
+       /* Skip 0, 0 is the phy for otg which is not yet supported. */
+       for (i = 1; i < data->num_phys; i++) {
+               snprintf(name, sizeof(name), "usb%d_vbus", i);
+               vbus = devm_regulator_get_optional(dev, name);
+               if (IS_ERR(vbus)) {
+                       if (PTR_ERR(vbus) == -EPROBE_DEFER)
+                               return -EPROBE_DEFER;

drivers/phy/phy-sun4i-usb.c line 270

Quoted 10 lines
drivers/phy/phy-sun4i-usb.c new file +9 -0
+                       vbus = NULL;
+               }
+
 
+               snprintf(name, sizeof(name), "usb%d_reset", i);
+               reset = devm_reset_control_get(dev, name);
+               if (IS_ERR(phy)) {
+                       dev_err(dev, "failed to get reset %s\n", name);
+                       return PTR_ERR(phy);
+               }

Wrong variable checked for error here that I pointed out before is still wrong.

Cheers
ChenYu

Quoted 63 lines
drivers/phy/phy-sun4i-usb.c new file +56 -1
+
+               if (i) { /* No pmu for usbc0 */
+                       pmu = devm_ioremap_resource(dev,
+                             platform_get_resource(pdev, IORESOURCE_MEM, i));
+                       if (IS_ERR(pmu))
+                               return PTR_ERR(pmu);
+               }
+
+               phy = devm_phy_create(dev, &sun4i_usb_phy_ops, NULL);
+               if (IS_ERR(phy)) {
+                       dev_err(dev, "failed to create PHY %d\n", i);
+                       return PTR_ERR(phy);
+               }
+
+               data->phys[i].phy = phy;
+               data->phys[i].pmu = pmu;
+               data->phys[i].vbus = vbus;
+               data->phys[i].reset = reset;
+               data->phys[i].index = i;
+               phy_set_drvdata(phy, &data->phys[i]);
+       }
+
+       data->base = devm_ioremap_resource(dev,
+                       platform_get_resource(pdev, IORESOURCE_MEM, 0));
+       if (IS_ERR(data->base))
+               return PTR_ERR(data->base);
+
+       dev_set_drvdata(dev, data);
+       phy_provider = devm_of_phy_provider_register(dev, sun4i_usb_phy_xlate);
+       if (IS_ERR(phy_provider))
+               return PTR_ERR(phy_provider);
+
+       return 0;
+}
+
+static const struct of_device_id sun4i_usb_phy_of_match[] = {
+       { .compatible = "allwinner,sun4i-a10-usb-phy" },
+       { .compatible = "allwinner,sun5i-a13-usb-phy" },
+       { .compatible = "allwinner,sun7i-a20-usb-phy" },
+       { },
+};
+MODULE_DEVICE_TABLE(of, sun4i_usb_phy_of_match);
+
+static struct platform_driver sun4i_usb_phy_driver = {
+       .probe  = sun4i_usb_phy_probe,
+       .driver = {
+               .of_match_table = sun4i_usb_phy_of_match,
+               .name  = "sun4i-usb-phy",
+               .owner = THIS_MODULE,
+       }
+};
+module_platform_driver(sun4i_usb_phy_driver);
+
+MODULE_DESCRIPTION("Allwinner sun4i USB phy driver");
+MODULE_AUTHOR("Hans de Goede <[email protected]>");
+MODULE_LICENSE("GPL v2");
--

1.8.4.2

--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe at googlegroups.com. For more options, visit https://groups.google.com/groups/opt_out.

#5 lore raw

PATCH v2 1/1 ARM: sunxi: Add driver for sunxi usb phy

Hi,

Quoted 7 lines · Chen-Yu Tsai
Quoted 6 lines · Hans de Goede

The Allwinner A1x / A2x SoCs have 2 or 3 usb phys which are all accessed through a single set of registers. Besides this there are also some other phy related bits which need poking, which are per phy, but shared between the ohci and ehci controllers, so these are also controlled from this new phy driver.

<snip>

Quoted 8 lines

drivers/phy/phy-sun4i-usb.c line 270

Quoted 6 lines
drivers/phy/phy-sun4i-usb.c new file +6 -0
+               snprintf(name, sizeof(name), "usb%d_reset", i);
+               reset = devm_reset_control_get(dev, name);
+               if (IS_ERR(phy)) {
+                       dev_err(dev, "failed to get reset %s\n", name);
+                       return PTR_ERR(phy);
+               }

Wrong variable checked for error here that I pointed out before is still wrong.

Ah, yes my bad, I'll do a v3 fixing this, and switching to using register names as Maxime requested. Don't know whether I'll be able to send v3 today as I'm currently on the road. If not I'll try to get it out the door tomorrow.

Regards,

Hans