2014-11-04 04:07:26

by Chen-Yu Tsai

[permalink] [raw]
Subject: [PATCH 0/6] ARM: sun9i: Add USB host controller support for A80

Hi everyone,

This series adds USB host controller (EHCI/OHCI) support for the Allwinner
A80 SoC. The A80 has 3 pairs of host controllers and USB PHYs. The PHYs,
unlike in previous SoCs, do not have low level control registers anymore.

As such, this series forgoes the original phy-sun4i-usb driver, and adds
a new, simpler driver for the USB PHYs. It may be possible to merge the
two, but given that work is being done on the OTG front for the earlier
SoCs, it may be better to merge them after support is complete.

USB0 corresponds to USB1 DP/DM pins; USB1 only has HSIC support; USB2 is
USB2 DP/DM externally. External pins labeled USB0 are for the USB 3.0 OTG
controller.

Patch 1 adds a80 specific support for usb-related clocks and resets.

Patch 2 adds the device tree nodes for the usb clocks.

Patch 3 adds a new generic phy driver for a80 usb phys. This has some
code that is the same as the original phy-sun4i-usb driver, but is simpler.

Patch 4 adds the 3 USB PHY nodes to the a80 dtsi.

Patch 5 adds the USB host controller nodes to the a80 dtsi.

Patch 6 adds the vbus regulator nodes and enables USB on the A80 Optimus
board.


Chen-Yu Tsai (6):
clk: sunxi: Add support for sun9i a80 usb clocks and resets
ARM: dts: sun9i: Add usb clock nodes to a80 dtsi
phy: Add driver to support individual USB PHYs on sun9i
ARM: dts: sun9i: Add usb phy nodes to a80 dtsi
ARM: dts: sun9i: Add USB host controller nodes to a80 dtsi
ARM: dts: sun9i: Enable USB support on A80 Optimus board

Documentation/devicetree/bindings/clock/sunxi.txt | 5 +
.../devicetree/bindings/phy/sun9i-usb-phy.txt | 34 +++
arch/arm/boot/dts/sun9i-a80-optimus.dts | 72 +++++++
arch/arm/boot/dts/sun9i-a80.dtsi | 129 ++++++++++++
drivers/clk/sunxi/Makefile | 1 +
drivers/clk/sunxi/clk-usb.c | 192 +++++++++++++++++
drivers/phy/Kconfig | 12 ++
drivers/phy/Makefile | 1 +
drivers/phy/phy-sun9i-usb.c | 227 +++++++++++++++++++++
9 files changed, 673 insertions(+)
create mode 100644 Documentation/devicetree/bindings/phy/sun9i-usb-phy.txt
create mode 100644 drivers/clk/sunxi/clk-usb.c
create mode 100644 drivers/phy/phy-sun9i-usb.c

--
2.1.1


2014-11-04 04:07:32

by Chen-Yu Tsai

[permalink] [raw]
Subject: [PATCH 4/6] ARM: dts: sun9i: Add usb phy nodes to a80 dtsi

On sun9i, there are 3 independent usb phys. Add device nodes for them.

Signed-off-by: Chen-Yu Tsai <[email protected]>
---
arch/arm/boot/dts/sun9i-a80.dtsi | 37 +++++++++++++++++++++++++++++++++++++
1 file changed, 37 insertions(+)

diff --git a/arch/arm/boot/dts/sun9i-a80.dtsi b/arch/arm/boot/dts/sun9i-a80.dtsi
index 3c25030..1a80e09 100644
--- a/arch/arm/boot/dts/sun9i-a80.dtsi
+++ b/arch/arm/boot/dts/sun9i-a80.dtsi
@@ -305,6 +305,43 @@
*/
ranges = <0 0 0 0x20000000>;

+ usbphy0: phy@00a00800 {
+ compatible = "allwinner,sun9i-a80-usb-phy";
+ reg = <0x00a00800 0x4>;
+ clocks = <&usb_phy_clk 1>;
+ clock-names = "phy";
+ resets = <&usb_phy_clk 17>;
+ reset-names = "phy";
+ status = "disabled";
+ #phy-cells = <0>;
+ };
+
+ usbphy1: phy@00a01800 {
+ compatible = "allwinner,sun9i-a80-usb-phy";
+ reg = <0x00a01800 0x4>;
+ clocks = <&usb_phy_clk 2>, <&usb_phy_clk 10>,
+ <&usb_phy_clk 3>;
+ clock-names = "hsic_480M", "hsic_12M", "phy";
+ resets = <&usb_phy_clk 18>, <&usb_phy_clk 19>;
+ reset-names = "hsic", "phy";
+ status = "disabled";
+ #phy-cells = <0>;
+ /* usb1 is always used with HSIC */
+ phy_type = "hsic";
+ };
+
+ usbphy2: phy@00a02800 {
+ compatible = "allwinner,sun9i-a80-usb-phy";
+ reg = <0x00a02800 0x4>;
+ clocks = <&usb_phy_clk 4>, <&usb_phy_clk 10>,
+ <&usb_phy_clk 5>;
+ clock-names = "hsic_480M", "hsic_12M", "phy";
+ resets = <&usb_phy_clk 20>, <&usb_phy_clk 21>;
+ reset-names = "hsic", "phy";
+ status = "disabled";
+ #phy-cells = <0>;
+ };
+
gic: interrupt-controller@01c41000 {
compatible = "arm,cortex-a7-gic", "arm,cortex-a15-gic";
reg = <0x01c41000 0x1000>,
--
2.1.1

2014-11-04 04:07:34

by Chen-Yu Tsai

[permalink] [raw]
Subject: [PATCH 5/6] ARM: dts: sun9i: Add USB host controller nodes to a80 dtsi

The A80 has 3 EHCI/OHCI USB controllers.

Signed-off-by: Chen-Yu Tsai <[email protected]>
---
arch/arm/boot/dts/sun9i-a80.dtsi | 70 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 70 insertions(+)

diff --git a/arch/arm/boot/dts/sun9i-a80.dtsi b/arch/arm/boot/dts/sun9i-a80.dtsi
index 1a80e09..b6f344f 100644
--- a/arch/arm/boot/dts/sun9i-a80.dtsi
+++ b/arch/arm/boot/dts/sun9i-a80.dtsi
@@ -305,6 +305,28 @@
*/
ranges = <0 0 0 0x20000000>;

+ ehci0: usb@00a00000 {
+ compatible = "allwinner,sun9i-a80-ehci", "generic-ehci";
+ reg = <0x00a00000 0x100>;
+ interrupts = <0 72 4>;
+ clocks = <&usb_mod_clk 1>;
+ resets = <&usb_mod_clk 17>;
+ phys = <&usbphy0>;
+ phy-names = "usb";
+ status = "disabled";
+ };
+
+ ohci0: usb@00a00400 {
+ compatible = "allwinner,sun9i-a80-ohci", "generic-ohci";
+ reg = <0x00a00400 0x100>;
+ interrupts = <0 73 4>;
+ clocks = <&usb_mod_clk 1>, <&usb_mod_clk 2>;
+ resets = <&usb_mod_clk 17>;
+ phys = <&usbphy0>;
+ phy-names = "usb";
+ status = "disabled";
+ };
+
usbphy0: phy@00a00800 {
compatible = "allwinner,sun9i-a80-usb-phy";
reg = <0x00a00800 0x4>;
@@ -316,6 +338,32 @@
#phy-cells = <0>;
};

+ ehci1: usb@00a01000 {
+ compatible = "allwinner,sun9i-a80-ehci", "generic-ehci";
+ reg = <0x00a01000 0x100>;
+ interrupts = <0 74 4>;
+ clocks = <&usb_mod_clk 3>;
+ resets = <&usb_mod_clk 18>;
+ phys = <&usbphy1>;
+ phy-names = "usb";
+ status = "disabled";
+ };
+
+ /*
+ * Even though ohci1 exists, it is never used as
+ * usb1 only has HSIC pins routed externally
+ */
+ ohci1: usb@00a01400 {
+ compatible = "allwinner,sun9i-a80-ohci", "generic-ohci";
+ reg = <0x00a01400 0x100>;
+ interrupts = <0 75 4>;
+ clocks = <&usb_mod_clk 3>, <&usb_mod_clk 4>;
+ resets = <&usb_mod_clk 18>;
+ phys = <&usbphy1>;
+ phy-names = "usb";
+ status = "disabled";
+ };
+
usbphy1: phy@00a01800 {
compatible = "allwinner,sun9i-a80-usb-phy";
reg = <0x00a01800 0x4>;
@@ -330,6 +378,28 @@
phy_type = "hsic";
};

+ ehci2: usb@00a02000 {
+ compatible = "allwinner,sun9i-a80-ehci", "generic-ehci";
+ reg = <0x00a02000 0x100>;
+ interrupts = <0 76 4>;
+ clocks = <&usb_mod_clk 5>;
+ resets = <&usb_mod_clk 19>;
+ phys = <&usbphy2>;
+ phy-names = "usb";
+ status = "disabled";
+ };
+
+ ohci2: usb@00a02400 {
+ compatible = "allwinner,sun9i-a80-ohci", "generic-ohci";
+ reg = <0x00a02400 0x100>;
+ interrupts = <0 77 4>;
+ clocks = <&usb_mod_clk 5>, <&usb_mod_clk 6>;
+ resets = <&usb_mod_clk 19>;
+ phys = <&usbphy2>;
+ phy-names = "usb";
+ status = "disabled";
+ };
+
usbphy2: phy@00a02800 {
compatible = "allwinner,sun9i-a80-usb-phy";
reg = <0x00a02800 0x4>;
--
2.1.1

2014-11-04 04:07:28

by Chen-Yu Tsai

[permalink] [raw]
Subject: [PATCH 2/6] ARM: dts: sun9i: Add usb clock nodes to a80 dtsi

The USB controller and phy clocks and resets have a separate address
block and driver. Add the nodes to represent them.

Signed-off-by: Chen-Yu Tsai <[email protected]>
---
arch/arm/boot/dts/sun9i-a80.dtsi | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)

diff --git a/arch/arm/boot/dts/sun9i-a80.dtsi b/arch/arm/boot/dts/sun9i-a80.dtsi
index 494714f..3c25030 100644
--- a/arch/arm/boot/dts/sun9i-a80.dtsi
+++ b/arch/arm/boot/dts/sun9i-a80.dtsi
@@ -143,6 +143,28 @@
clock-output-names = "osc32k";
};

+ usb_mod_clk: clk@00a08000 {
+ #clock-cells = <1>;
+ #reset-cells = <1>;
+ compatible = "allwinner,sun9i-a80-usb-mod-clk";
+ reg = <0x00a08000 0x4>;
+ clocks = <&ahb1_gates 1>, <&pll4>;
+ clock-output-names = "usb0_ahb", "usb_ohci0",
+ "usb1_ahb", "usb_ohci1",
+ "usb2_ahb", "usb_ohci2";
+ };
+
+ usb_phy_clk: clk@00a08004 {
+ #clock-cells = <1>;
+ #reset-cells = <1>;
+ compatible = "allwinner,sun9i-a80-usb-phy-clk";
+ reg = <0x00a08004 0x4>;
+ clocks = <&ahb1_gates 1>, <&pll4>;
+ clock-output-names = "usb_phy0", "usb_hsic1_480M",
+ "usb_phy1", "usb_hsic2_480M",
+ "usb_phy2", "usb_hsic_12M";
+ };
+
pll4: clk@0600000c {
#clock-cells = <0>;
compatible = "allwinner,sun9i-a80-pll4-clk";
--
2.1.1

2014-11-04 04:07:55

by Chen-Yu Tsai

[permalink] [raw]
Subject: [PATCH 6/6] ARM: dts: sun9i: Enable USB support on A80 Optimus board

Signed-off-by: Chen-Yu Tsai <[email protected]>
---
This patch does not use sunxi-common-regulators.dtsi, but adds the
regulators directly. To use the common regulators file, we would
need to use phandles and switch to preprocessor includes to support
that.

---
arch/arm/boot/dts/sun9i-a80-optimus.dts | 72 +++++++++++++++++++++++++++++++++
1 file changed, 72 insertions(+)

diff --git a/arch/arm/boot/dts/sun9i-a80-optimus.dts b/arch/arm/boot/dts/sun9i-a80-optimus.dts
index 506948f..31010c1 100644
--- a/arch/arm/boot/dts/sun9i-a80-optimus.dts
+++ b/arch/arm/boot/dts/sun9i-a80-optimus.dts
@@ -59,6 +59,40 @@
};

soc {
+ ehci0: usb@00a00000 {
+ status = "okay";
+ };
+
+ ohci0: usb@00a00400 {
+ status = "okay";
+ };
+
+ usbphy0: phy@00a00800 {
+ vbus-supply = <&reg_usb0_vbus>;
+ status = "okay";
+ };
+
+ ehci1: usb@00a01000 {
+ status = "okay";
+ };
+
+ usbphy1: phy@00a01800 {
+ status = "okay";
+ };
+
+ ehci2: usb@00a02000 {
+ status = "okay";
+ };
+
+ ohci2: usb@00a02400 {
+ status = "okay";
+ };
+
+ usbphy2: phy@00a02800 {
+ vbus-supply = <&reg_usb2_vbus>;
+ status = "okay";
+ };
+
pio: pinctrl@06000800 {
i2c3_pins_a: i2c3@0 {
/* Enable internal pull-up */
@@ -76,6 +110,20 @@
/* Enable internal pull-up */
allwinner,pull = <1>;
};
+
+ usb0_vbus_pin_optimus: usb0_vbus_pin@1 {
+ allwinner,pins = "PH4";
+ allwinner,function = "gpio_out";
+ allwinner,drive = <0>;
+ allwinner,pull = <0>;
+ };
+
+ usb2_vbus_pin_optimus: usb2_vbus_pin@1 {
+ allwinner,pins = "PH5";
+ allwinner,function = "gpio_out";
+ allwinner,drive = <0>;
+ allwinner,pull = <0>;
+ };
};

uart0: serial@07000000 {
@@ -116,4 +164,28 @@
gpios = <&pio 7 0 0>;
};
};
+
+ reg_usb0_vbus: usb0-vbus {
+ compatible = "regulator-fixed";
+ pinctrl-names = "default";
+ pinctrl-0 = <&usb0_vbus_pin_optimus>;
+ regulator-name = "usb0-vbus";
+ regulator-min-microvolt = <5000000>;
+ regulator-max-microvolt = <5000000>;
+ enable-active-high;
+ gpio = <&pio 7 4 0>; /* PH4 */
+ status = "okay";
+ };
+
+ reg_usb2_vbus: usb2-vbus {
+ compatible = "regulator-fixed";
+ pinctrl-names = "default";
+ pinctrl-0 = <&usb2_vbus_pin_optimus>;
+ regulator-name = "usb2-vbus";
+ regulator-min-microvolt = <5000000>;
+ regulator-max-microvolt = <5000000>;
+ enable-active-high;
+ gpio = <&pio 7 5 0>; /* PH5 */
+ status = "okay";
+ };
};
--
2.1.1

2014-11-04 04:08:34

by Chen-Yu Tsai

[permalink] [raw]
Subject: [PATCH 3/6] phy: Add driver to support individual USB PHYs on sun9i

Unlike previous Allwinner SoCs, there is no central PHY control block
on the A80. Also, OTG support is completely split off into a different
controller.

This adds a new driver to support the regular USB PHYs.

Signed-off-by: Chen-Yu Tsai <[email protected]>
---
.../devicetree/bindings/phy/sun9i-usb-phy.txt | 34 +++
drivers/phy/Kconfig | 12 ++
drivers/phy/Makefile | 1 +
drivers/phy/phy-sun9i-usb.c | 227 +++++++++++++++++++++
4 files changed, 274 insertions(+)
create mode 100644 Documentation/devicetree/bindings/phy/sun9i-usb-phy.txt
create mode 100644 drivers/phy/phy-sun9i-usb.c

diff --git a/Documentation/devicetree/bindings/phy/sun9i-usb-phy.txt b/Documentation/devicetree/bindings/phy/sun9i-usb-phy.txt
new file mode 100644
index 0000000..27a6067
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/sun9i-usb-phy.txt
@@ -0,0 +1,34 @@
+Allwinner sun9i USB PHY
+-----------------------
+
+Required properties:
+- compatible : should be one of
+ * allwinner,sun9i-a80-usb-phy
+- reg : a list of offset + length pairs
+- #phy-cells : from the generic phy bindings, must be 0
+- clocks : phandle + clock specifier for the phy clocks
+- clock-names :
+ * "phy" for normal USB
+ * "hsic_480M", "hsic_12M" for HSIC
+- resets : a list of phandle + reset specifier pairs
+- reset-names :
+ * "phy" for normal USB
+ * "hsic" for HSIC
+- phy_type : "hsic" for HSIC usage;
+ other values or absence of this property indicates normal USB
+
+It is recommended to list all clocks and resets available.
+The driver will only use those matching the phy_type.
+
+Example:
+ usbphy1: phy@00a01800 {
+ compatible = "allwinner,sun9i-a80-usb-phy";
+ reg = <0x00a01800 0x4>;
+ clocks = <&usb_phy_clk 2>, <&usb_phy_clk 10>,
+ <&usb_phy_clk 3>;
+ clock-names = "hsic_480M", "hsic_12M", "phy";
+ resets = <&usb_phy_clk 18>, <&usb_phy_clk 19>;
+ reset-names = "hsic", "phy";
+ status = "disabled";
+ #phy-cells = <0>;
+ };
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 2a436e6..f5b7fbb 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -153,6 +153,18 @@ config PHY_SUN4I_USB
This driver controls the entire USB PHY block, both the USB OTG
parts, as well as the 2 regular USB 2 host PHYs.

+config PHY_SUN9I_USB
+ tristate "Allwinner sun9i SoC USB PHY driver"
+ depends on ARCH_SUNXI && HAS_IOMEM && OF
+ depends on RESET_CONTROLLER
+ select USB_PHY
+ select GENERIC_PHY
+ help
+ Enable this to support the transceiver that is part of Allwinner
+ sun9i SoCs.
+
+ This driver controls each individual USB 2 host PHY.
+
config PHY_SAMSUNG_USB2
tristate "Samsung USB 2.0 PHY driver"
depends on HAS_IOMEM
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index c4590fc..c3977dc 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -17,6 +17,7 @@ obj-$(CONFIG_TWL4030_USB) += phy-twl4030-usb.o
obj-$(CONFIG_PHY_EXYNOS5250_SATA) += phy-exynos5250-sata.o
obj-$(CONFIG_PHY_HIX5HD2_SATA) += phy-hix5hd2-sata.o
obj-$(CONFIG_PHY_SUN4I_USB) += phy-sun4i-usb.o
+obj-$(CONFIG_PHY_SUN9I_USB) += phy-sun9i-usb.o
obj-$(CONFIG_PHY_SAMSUNG_USB2) += phy-exynos-usb2.o
phy-exynos-usb2-y += phy-samsung-usb2.o
phy-exynos-usb2-$(CONFIG_PHY_EXYNOS4210_USB2) += phy-exynos4210-usb2.o
diff --git a/drivers/phy/phy-sun9i-usb.c b/drivers/phy/phy-sun9i-usb.c
new file mode 100644
index 0000000..f9085d9
--- /dev/null
+++ b/drivers/phy/phy-sun9i-usb.c
@@ -0,0 +1,227 @@
+/*
+ * Allwinner sun9i USB phy driver
+ *
+ * Copyright (C) 2014 Chen-Yu Tsai <[email protected]>
+ *
+ * Based on phy-sun9i.c from
+ * Hans de Goede <[email protected]>
+ *
+ * and code from
+ * Allwinner Technology Co., Ltd. <http://www.allwinnertech.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * 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/err.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/usb/of.h>
+#include <linux/platform_device.h>
+#include <linux/reset.h>
+
+#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)
+
+struct sun9i_usb_phy {
+ struct phy *phy;
+ void __iomem *pmu;
+ struct regulator *vbus;
+ struct reset_control *reset;
+ struct clk *clk;
+ struct clk *hsic_clk;
+};
+
+static void sun9i_usb_phy_passby(struct sun9i_usb_phy *phy, int enable)
+{
+ u32 bits, reg_value;
+
+ 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 sun9i_usb_phy_init(struct phy *_phy)
+{
+ struct sun9i_usb_phy *phy = phy_get_drvdata(_phy);
+ int ret;
+
+ ret = clk_prepare_enable(phy->clk);
+ if (ret)
+ return ret;
+
+ if (phy->hsic_clk) {
+ ret = clk_prepare_enable(phy->hsic_clk);
+ if (ret) {
+ clk_disable_unprepare(phy->clk);
+ return ret;
+ }
+ }
+
+ ret = reset_control_deassert(phy->reset);
+ if (ret) {
+ if (phy->hsic_clk)
+ clk_disable_unprepare(phy->hsic_clk);
+ clk_disable_unprepare(phy->clk);
+ return ret;
+ }
+
+ sun9i_usb_phy_passby(phy, 1);
+
+ return 0;
+}
+
+static int sun9i_usb_phy_exit(struct phy *_phy)
+{
+ struct sun9i_usb_phy *phy = phy_get_drvdata(_phy);
+
+ sun9i_usb_phy_passby(phy, 0);
+ reset_control_assert(phy->reset);
+ if (phy->hsic_clk)
+ clk_disable_unprepare(phy->hsic_clk);
+ clk_disable_unprepare(phy->clk);
+
+ return 0;
+}
+
+static int sun9i_usb_phy_power_on(struct phy *_phy)
+{
+ struct sun9i_usb_phy *phy = phy_get_drvdata(_phy);
+ int ret = 0;
+
+ if (phy->vbus)
+ ret = regulator_enable(phy->vbus);
+
+ return ret;
+}
+
+static int sun9i_usb_phy_power_off(struct phy *_phy)
+{
+ struct sun9i_usb_phy *phy = phy_get_drvdata(_phy);
+
+ if (phy->vbus)
+ regulator_disable(phy->vbus);
+
+ return 0;
+}
+
+static struct phy_ops sun9i_usb_phy_ops = {
+ .init = sun9i_usb_phy_init,
+ .exit = sun9i_usb_phy_exit,
+ .power_on = sun9i_usb_phy_power_on,
+ .power_off = sun9i_usb_phy_power_off,
+ .owner = THIS_MODULE,
+};
+
+static int sun9i_usb_phy_probe(struct platform_device *pdev)
+{
+ struct sun9i_usb_phy *phy;
+ struct device *dev = &pdev->dev;
+ struct device_node *np = dev->of_node;
+ struct phy_provider *phy_provider;
+ struct resource *res;
+ enum usb_phy_interface phy_type;
+
+ phy = devm_kzalloc(dev, sizeof(*phy), GFP_KERNEL);
+ if (!phy)
+ return -ENOMEM;
+
+ phy->vbus = devm_regulator_get_optional(dev, "vbus");
+ if (IS_ERR(phy->vbus)) {
+ if (PTR_ERR(phy->vbus) == -EPROBE_DEFER)
+ return -EPROBE_DEFER;
+ phy->vbus = NULL;
+ }
+
+ phy_type = of_usb_get_phy_mode(np);
+ if (phy_type == USBPHY_INTERFACE_MODE_HSIC) {
+ phy->clk = devm_clk_get(dev, "hsic_480M");
+ if (IS_ERR(phy->clk)) {
+ dev_err(dev, "failed to get hsic_480M clock\n");
+ return PTR_ERR(phy->clk);
+ }
+
+ phy->hsic_clk = devm_clk_get(dev, "hsic_12M");
+ if (IS_ERR(phy->clk)) {
+ dev_err(dev, "failed to get hsic_12M clock\n");
+ return PTR_ERR(phy->clk);
+ }
+
+ phy->reset = devm_reset_control_get(dev, "hsic");
+ if (IS_ERR(phy->reset)) {
+ dev_err(dev, "failed to get reset control\n");
+ return PTR_ERR(phy->reset);
+ }
+ } else {
+ phy->clk = devm_clk_get(dev, "phy");
+ if (IS_ERR(phy->clk)) {
+ dev_err(dev, "failed to get phy clock\n");
+ return PTR_ERR(phy->clk);
+ }
+
+ phy->reset = devm_reset_control_get(dev, "phy");
+ if (IS_ERR(phy->reset)) {
+ dev_err(dev, "failed to get reset control\n");
+ return PTR_ERR(phy->reset);
+ }
+ }
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ phy->pmu = devm_ioremap_resource(dev, res);
+ if (IS_ERR(phy->pmu))
+ return PTR_ERR(phy->pmu);
+
+ phy->phy = devm_phy_create(dev, NULL, &sun9i_usb_phy_ops, NULL);
+ if (IS_ERR(phy->phy)) {
+ dev_err(dev, "failed to create PHY\n");
+ return PTR_ERR(phy->phy);
+ }
+
+ phy_set_drvdata(phy->phy, phy);
+ dev_set_drvdata(dev, phy);
+ phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
+
+ return PTR_ERR_OR_ZERO(phy_provider);
+}
+
+static const struct of_device_id sun9i_usb_phy_of_match[] = {
+ { .compatible = "allwinner,sun9i-a80-usb-phy" },
+ { },
+};
+MODULE_DEVICE_TABLE(of, sun9i_usb_phy_of_match);
+
+static struct platform_driver sun9i_usb_phy_driver = {
+ .probe = sun9i_usb_phy_probe,
+ .driver = {
+ .of_match_table = sun9i_usb_phy_of_match,
+ .name = "sun9i-usb-phy",
+ }
+};
+module_platform_driver(sun9i_usb_phy_driver);
+
+MODULE_DESCRIPTION("Allwinner sun9i USB phy driver");
+MODULE_AUTHOR("Chen-Yu Tsai <[email protected]>");
+MODULE_LICENSE("GPL v2");
--
2.1.1

2014-11-04 04:08:32

by Chen-Yu Tsai

[permalink] [raw]
Subject: [PATCH 1/6] clk: sunxi: Add support for sun9i a80 usb clocks and resets

The USB controller/phy clocks and reset controls are in a separate
address block, unlike previous SoCs where they were in the clock
controller.

This patch copies the original gates clk functions used for usb
clocks into a separate file, and renames them to *_usb_*. Also
add a per-gate parent index, so we can set different parents for
each gate.

In time we may move the other usb clock drivers to this file.

Signed-off-by: Chen-Yu Tsai <[email protected]>
---
Documentation/devicetree/bindings/clock/sunxi.txt | 5 +
drivers/clk/sunxi/Makefile | 1 +
drivers/clk/sunxi/clk-usb.c | 192 ++++++++++++++++++++++
3 files changed, 198 insertions(+)
create mode 100644 drivers/clk/sunxi/clk-usb.c

diff --git a/Documentation/devicetree/bindings/clock/sunxi.txt b/Documentation/devicetree/bindings/clock/sunxi.txt
index 0455cb9..b953fe5 100644
--- a/Documentation/devicetree/bindings/clock/sunxi.txt
+++ b/Documentation/devicetree/bindings/clock/sunxi.txt
@@ -66,6 +66,8 @@ Required properties:
"allwinner,sun4i-a10-usb-clk" - for usb gates + resets on A10 / A20
"allwinner,sun5i-a13-usb-clk" - for usb gates + resets on A13
"allwinner,sun6i-a31-usb-clk" - for usb gates + resets on A31
+ "allwinner,sun9i-a80-usb-mod-clk" - for usb gates + resets on A80
+ "allwinner,sun9i-a80-usb-phy-clk" - for usb phy gates + resets on A80

Required properties for all clocks:
- reg : shall be the control register address for the clock.
@@ -82,6 +84,9 @@ Required properties for all clocks:
And "allwinner,*-usb-clk" clocks also require:
- reset-cells : shall be set to 1

+"allwinner,sun9i-a80-usb-*-clk" clocks require:
+- clocks : shall be the usb hci ahb1 gate and peripheral pll clocks
+
For "allwinner,sun7i-a20-gmac-clk", the parent clocks shall be fixed rate
dummy clocks at 25 MHz and 125 MHz, respectively. See example.

diff --git a/drivers/clk/sunxi/Makefile b/drivers/clk/sunxi/Makefile
index a66953c..f19ce54 100644
--- a/drivers/clk/sunxi/Makefile
+++ b/drivers/clk/sunxi/Makefile
@@ -8,6 +8,7 @@ obj-y += clk-a20-gmac.o
obj-y += clk-mod0.o
obj-y += clk-sun8i-mbus.o
obj-y += clk-sun9i-core.o
+obj-y += clk-usb.o

obj-$(CONFIG_MFD_SUN6I_PRCM) += \
clk-sun6i-ar100.o clk-sun6i-apb0.o clk-sun6i-apb0-gates.o \
diff --git a/drivers/clk/sunxi/clk-usb.c b/drivers/clk/sunxi/clk-usb.c
new file mode 100644
index 0000000..d92ee36
--- /dev/null
+++ b/drivers/clk/sunxi/clk-usb.c
@@ -0,0 +1,192 @@
+/*
+ * Copyright 2013 Emilio López
+ *
+ * Emilio López <[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-provider.h>
+#include <linux/clkdev.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/reset-controller.h>
+#include <linux/spinlock.h>
+
+
+/**
+ * sunxi_usb_reset... - reset bits in usb clk registers handling
+ */
+
+struct usb_reset_data {
+ void __iomem *reg;
+ spinlock_t *lock;
+ struct reset_controller_dev rcdev;
+};
+
+static int sunxi_usb_reset_assert(struct reset_controller_dev *rcdev,
+ unsigned long id)
+{
+ struct usb_reset_data *data = container_of(rcdev,
+ struct usb_reset_data,
+ rcdev);
+ unsigned long flags;
+ u32 reg;
+
+ spin_lock_irqsave(data->lock, flags);
+
+ reg = readl(data->reg);
+ writel(reg & ~BIT(id), data->reg);
+
+ spin_unlock_irqrestore(data->lock, flags);
+
+ return 0;
+}
+
+static int sunxi_usb_reset_deassert(struct reset_controller_dev *rcdev,
+ unsigned long id)
+{
+ struct usb_reset_data *data = container_of(rcdev,
+ struct usb_reset_data,
+ rcdev);
+ unsigned long flags;
+ u32 reg;
+
+ spin_lock_irqsave(data->lock, flags);
+
+ reg = readl(data->reg);
+ writel(reg | BIT(id), data->reg);
+
+ spin_unlock_irqrestore(data->lock, flags);
+
+ return 0;
+}
+
+static struct reset_control_ops sunxi_usb_reset_ops = {
+ .assert = sunxi_usb_reset_assert,
+ .deassert = sunxi_usb_reset_deassert,
+};
+
+/**
+ * sunxi_usb_clk_setup() - Setup function for usb gate clocks
+ */
+
+#define SUNXI_USB_MAX_SIZE 32
+
+struct usb_clk_data {
+ u32 clk_mask;
+ u32 reset_mask;
+ /* which parent to use, should match clock-output-names */
+ char parents[SUNXI_USB_MAX_SIZE];
+};
+
+static void __init sunxi_usb_clk_setup(struct device_node *node,
+ const struct usb_clk_data *data,
+ spinlock_t *lock)
+{
+ struct clk_onecell_data *clk_data;
+ struct usb_reset_data *reset_data;
+ const char *clk_parent;
+ const char *clk_name;
+ void __iomem *reg;
+ int qty;
+ int i = 0;
+ int j = 0;
+
+ reg = of_iomap(node, 0);
+
+ /* Worst-case size approximation and memory allocation */
+ qty = find_last_bit((unsigned long *)&data->clk_mask,
+ SUNXI_USB_MAX_SIZE);
+ clk_data = kmalloc(sizeof(struct clk_onecell_data), GFP_KERNEL);
+ if (!clk_data)
+ return;
+ clk_data->clks = kzalloc((qty+1) * sizeof(struct clk *), GFP_KERNEL);
+ if (!clk_data->clks) {
+ kfree(clk_data);
+ return;
+ }
+
+ for_each_set_bit(i, (unsigned long *)&data->clk_mask,
+ SUNXI_USB_MAX_SIZE) {
+ of_property_read_string_index(node, "clock-output-names",
+ j, &clk_name);
+ clk_parent = of_clk_get_parent_name(node, data->parents[j]);
+
+ clk_data->clks[i] = clk_register_gate(NULL, clk_name,
+ clk_parent, 0,
+ reg, i, 0, lock);
+ WARN_ON(IS_ERR(clk_data->clks[i]));
+ clk_register_clkdev(clk_data->clks[i], clk_name, NULL);
+
+ j++;
+ }
+
+ /* Adjust to the real max */
+ clk_data->clk_num = i;
+
+ of_clk_add_provider(node, of_clk_src_onecell_get, clk_data);
+
+ /* Register a reset controller for usb with reset bits */
+ if (data->reset_mask == 0)
+ return;
+
+ reset_data = kzalloc(sizeof(*reset_data), GFP_KERNEL);
+ if (!reset_data)
+ return;
+
+ reset_data->reg = reg;
+ reset_data->lock = lock;
+ reset_data->rcdev.nr_resets = __fls(data->reset_mask) + 1;
+ reset_data->rcdev.ops = &sunxi_usb_reset_ops;
+ reset_data->rcdev.of_node = node;
+ reset_controller_register(&reset_data->rcdev);
+}
+
+static const struct usb_clk_data sun9i_a80_usb_mod_data __initconst = {
+ .clk_mask = BIT(6) | BIT(5) | BIT(4) | BIT(3) | BIT(2) | BIT(1),
+ .reset_mask = BIT(19) | BIT(18) | BIT(17),
+ .parents = {0, 1, 0, 1, 0, 1},
+};
+
+static DEFINE_SPINLOCK(a80_usb_mod_lock);
+
+static void __init sun9i_a80_usb_mod_setup(struct device_node *node)
+{
+ /* AHB1 gate must be enabled to access registers */
+ struct clk *ahb = of_clk_get(node, 0);
+
+ WARN_ON(IS_ERR(ahb));
+ clk_prepare_enable(ahb);
+
+ sunxi_usb_clk_setup(node, &sun9i_a80_usb_mod_data, &a80_usb_mod_lock);
+}
+CLK_OF_DECLARE(sun9i_a80_usb_mod, "allwinner,sun9i-a80-usb-mod-clk", sun9i_a80_usb_mod_setup);
+
+static const struct usb_clk_data sun9i_a80_usb_phy_data __initconst = {
+ .clk_mask = BIT(10) | BIT(5) | BIT(4) | BIT(3) | BIT(2) | BIT(1),
+ .reset_mask = BIT(21) | BIT(20) | BIT(19) | BIT(18) | BIT(17),
+ .parents = {1, 1, 1, 1, 1, 1},
+};
+
+static DEFINE_SPINLOCK(a80_usb_phy_lock);
+
+static void __init sun9i_a80_usb_phy_setup(struct device_node *node)
+{
+ /* AHB1 gate must be enabled to access registers */
+ struct clk *ahb = of_clk_get(node, 0);
+
+ WARN_ON(IS_ERR(ahb));
+ clk_prepare_enable(ahb);
+
+ sunxi_usb_clk_setup(node, &sun9i_a80_usb_phy_data, &a80_usb_phy_lock);
+}
+CLK_OF_DECLARE(sun9i_a80_usb_phy, "allwinner,sun9i-a80-usb-phy-clk", sun9i_a80_usb_phy_setup);
--
2.1.1

2014-11-04 06:24:39

by Priit Laes

[permalink] [raw]
Subject: Re: [linux-sunxi] [PATCH 3/6] phy: Add driver to support individual USB PHYs on sun9i


On Tue, 2014-11-04 at 12:07 +0800, Chen-Yu Tsai wrote:
> Unlike previous Allwinner SoCs, there is no central PHY control block
> on the A80. Also, OTG support is completely split off into a
> different
> controller.
>
> This adds a new driver to support the regular USB PHYs.
>
> Signed-off-by: Chen-Yu Tsai <[email protected]>
> ---
> .../devicetree/bindings/phy/sun9i-usb-phy.txt | 34 +++
> drivers/phy/Kconfig | 12 ++
> drivers/phy/Makefile | 1 +
> drivers/phy/phy-sun9i-usb.c | 227
> +++++++++++++++++++++
> 4 files changed, 274 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/phy/sun9i-usb-
> phy.txt
> create mode 100644 drivers/phy/phy-sun9i-usb.c
>
> diff --git a/Documentation/devicetree/bindings/phy/sun9i-usb-phy.txt
> b/Documentation/devicetree/bindings/phy/sun9i-usb-phy.txt
> new file mode 100644
> index 0000000..27a6067
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/sun9i-usb-phy.txt
> @@ -0,0 +1,34 @@
> +Allwinner sun9i USB PHY
> +-----------------------
> +
> +Required properties:
> +- compatible : should be one of
> + * allwinner,sun9i-a80-usb-phy
> +- reg : a list of offset + length pairs
> +- #phy-cells : from the generic phy bindings, must be 0
> +- clocks : phandle + clock specifier for the phy clocks
> +- clock-names :
> + * "phy" for normal USB
> + * "hsic_480M", "hsic_12M" for HSIC
> +- resets : a list of phandle + reset specifier pairs
> +- reset-names :
> + * "phy" for normal USB
> + * "hsic" for HSIC
> +- phy_type : "hsic" for HSIC usage;
> + other values or
> absence of this property indicates normal USB
> +
> +It is recommended to list all clocks and resets available.
> +The driver will only use those matching the phy_type.
> +
> +Example:
> + usbphy1: phy@00a01800 {
> + compatible = "allwinner,sun9i-a80-usb-phy";
> + reg = <0x00a01800 0x4>;
> + clocks = <&usb_phy_clk 2>, <&usb_phy_clk 10>,
> +
> <&usb_phy_clk 3>;
> + clock-names = "hsic_480M", "hsic_12M", "phy";
> + resets = <&usb_phy_clk 18>, <&usb_phy_clk 19>;
> + reset-names = "hsic", "phy";
> + status = "disabled";
> + #phy-cells = <0>;
> + };
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index 2a436e6..f5b7fbb 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -153,6 +153,18 @@ config PHY_SUN4I_USB
> This driver controls the entire USB PHY
> block, both the USB OTG
> parts, as well as the 2 regular USB 2 host
> PHYs.
>
> +config PHY_SUN9I_USB
> + tristate "Allwinner sun9i SoC USB PHY driver"

Adding 'A80' to the user visible data makes things a bit easier for
end users:
"AllWinner sun9i (A80) SoC USB PHY driver"



> + depends on ARCH_SUNXI && HAS_IOMEM && OF
> + depends on RESET_CONTROLLER
> + select USB_PHY
> + select GENERIC_PHY
> + help
> + Enable this to support the transceiver that
> is part of Allwinner
> + sun9i SoCs.

And extra 'A80' here wouldn't hurt either.

> +
> + This driver controls each individual USB 2
> host PHY.
> +
> config PHY_SAMSUNG_USB2
> tristate "Samsung USB 2.0 PHY driver"
> depends on HAS_IOMEM
> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> index c4590fc..c3977dc 100644
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -17,6 +17,7 @@ obj-$(CONFIG_TWL4030_USB) += phy-
> twl4030-usb.o
> obj-$(CONFIG_PHY_EXYNOS5250_SATA) += phy-exynos5250-sata.o
> obj-$(CONFIG_PHY_HIX5HD2_SATA) += phy-hix5hd2-sata.o
> obj-$(CONFIG_PHY_SUN4I_USB) += phy-sun4i-usb.o
> +obj-$(CONFIG_PHY_SUN9I_USB) += phy-sun9i-usb.o
> obj-$(CONFIG_PHY_SAMSUNG_USB2) += phy-exynos-usb2.o
> phy-exynos-usb2-y += phy-samsung-usb2.o
> phy-exynos-usb2-$(CONFIG_PHY_EXYNOS4210_USB2) += phy-exynos4210-
> usb2.o
> diff --git a/drivers/phy/phy-sun9i-usb.c b/drivers/phy/phy-sun9i-
> usb.c
> new file mode 100644
> index 0000000..f9085d9
> --- /dev/null
> +++ b/drivers/phy/phy-sun9i-usb.c
> @@ -0,0 +1,227 @@
> +/*
> + * Allwinner sun9i USB phy driver
> + *
> + * Copyright (C) 2014 Chen-Yu Tsai <[email protected]>
> + *
> + * Based on phy-sun9i.c from
> + * Hans de Goede <[email protected]>
> + *
> + * and code from
> + * Allwinner Technology Co., Ltd. <http://www.allwinnertech.com>
> + *
> + * This program is free software; you can redistribute it and/or
> modify
> + * it under the terms of the GNU General Public License as
> published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * 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/err.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/usb/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/reset.h>
> +
> +#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)
> +
> +struct sun9i_usb_phy {
> + struct phy *phy;
> + void __iomem *pmu;
> + struct regulator *vbus;
> + struct reset_control *reset;
> + struct clk *clk;
> + struct clk *hsic_clk;
> +};
> +
> +static void sun9i_usb_phy_passby(struct sun9i_usb_phy *phy, int
> enable)
> +{
> + u32 bits, reg_value;
> +
> + 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 sun9i_usb_phy_init(struct phy *_phy)
> +{
> + struct sun9i_usb_phy *phy = phy_get_drvdata(_phy);
> + int ret;
> +
> + ret = clk_prepare_enable(phy->clk);
> + if (ret)
> + return ret;
> +
> + if (phy->hsic_clk) {
> + ret = clk_prepare_enable(phy->hsic_clk);
> + if (ret) {
> + clk_disable_unprepare(phy->clk);
> + return ret;
> + }
> + }
> +
> + ret = reset_control_deassert(phy->reset);
> + if (ret) {
> + if (phy->hsic_clk)
> + clk_disable_unprepare(phy->hsic_clk);
> + clk_disable_unprepare(phy->clk);
> + return ret;
> + }
> +
> + sun9i_usb_phy_passby(phy, 1);
> +
> + return 0;
> +}
> +
> +static int sun9i_usb_phy_exit(struct phy *_phy)
> +{
> + struct sun9i_usb_phy *phy = phy_get_drvdata(_phy);
> +
> + sun9i_usb_phy_passby(phy, 0);
> + reset_control_assert(phy->reset);
> + if (phy->hsic_clk)
> + clk_disable_unprepare(phy->hsic_clk);
> + clk_disable_unprepare(phy->clk);
> +
> + return 0;
> +}
> +
> +static int sun9i_usb_phy_power_on(struct phy *_phy)
> +{
> + struct sun9i_usb_phy *phy = phy_get_drvdata(_phy);
> + int ret = 0;
> +
> + if (phy->vbus)
> + ret = regulator_enable(phy->vbus);
> +
> + return ret;
> +}
> +
> +static int sun9i_usb_phy_power_off(struct phy *_phy)
> +{
> + struct sun9i_usb_phy *phy = phy_get_drvdata(_phy);
> +
> + if (phy->vbus)
> + regulator_disable(phy->vbus);
> +
> + return 0;
> +}
> +
> +static struct phy_ops sun9i_usb_phy_ops = {
> + .init = sun9i_usb_phy_init,
> + .exit = sun9i_usb_phy_exit,
> + .power_on= sun9i_usb_phy_power_on,
> + .power_off= sun9i_usb_phy_power_off,
> + .owner = THIS_MODULE,
> +};
> +
> +static int sun9i_usb_phy_probe(struct platform_device *pdev)
> +{
> + struct sun9i_usb_phy *phy;
> + struct device *dev = &pdev->dev;
> + struct device_node *np = dev->of_node;
> + struct phy_provider *phy_provider;
> + struct resource *res;
> + enum usb_phy_interface phy_type;
> +
> + phy = devm_kzalloc(dev, sizeof(*phy), GFP_KERNEL);
> + if (!phy)
> + return -ENOMEM;
> +
> + phy->vbus = devm_regulator_get_optional(dev, "vbus");
> + if (IS_ERR(phy->vbus)) {
> + if (PTR_ERR(phy->vbus) == -EPROBE_DEFER)
> + return -EPROBE_DEFER;
> + phy->vbus = NULL;
> + }
> +
> + phy_type = of_usb_get_phy_mode(np);
> + if (phy_type == USBPHY_INTERFACE_MODE_HSIC) {
> + phy->clk = devm_clk_get(dev, "hsic_480M");
> + if (IS_ERR(phy->clk)) {
> + dev_err(dev, "failed to get hsic_480M
> clock\n");
> + return PTR_ERR(phy->clk);
> + }
> +
> + phy->hsic_clk = devm_clk_get(dev, "hsic_12M");
> + if (IS_ERR(phy->clk)) {
> + dev_err(dev, "failed to get hsic_12M
> clock\n");
> + return PTR_ERR(phy->clk);
> + }
> +
> + phy->reset = devm_reset_control_get(dev, "hsic");
> + if (IS_ERR(phy->reset)) {
> + dev_err(dev, "failed to get reset
> control\n");
> + return PTR_ERR(phy->reset);
> + }
> + } else {
> + phy->clk = devm_clk_get(dev, "phy");
> + if (IS_ERR(phy->clk)) {
> + dev_err(dev, "failed to get phy clock\n");
> + return PTR_ERR(phy->clk);
> + }
> +
> + phy->reset = devm_reset_control_get(dev, "phy");
> + if (IS_ERR(phy->reset)) {
> + dev_err(dev, "failed to get reset
> control\n");
> + return PTR_ERR(phy->reset);
> + }
> + }
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + phy->pmu = devm_ioremap_resource(dev, res);
> + if (IS_ERR(phy->pmu))
> + return PTR_ERR(phy->pmu);
> +
> + phy->phy = devm_phy_create(dev, NULL, &sun9i_usb_phy_ops,
> NULL);
> + if (IS_ERR(phy->phy)) {
> + dev_err(dev, "failed to create PHY\n");
> + return PTR_ERR(phy->phy);
> + }
> +
> + phy_set_drvdata(phy->phy, phy);
> + dev_set_drvdata(dev, phy);
> + phy_provider = devm_of_phy_provider_register(dev,
> of_phy_simple_xlate);
> +
> + return PTR_ERR_OR_ZERO(phy_provider);
> +}
> +
> +static const struct of_device_id sun9i_usb_phy_of_match[] = {
> + { .compatible = "allwinner,sun9i-a80-usb-phy" },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, sun9i_usb_phy_of_match);
> +
> +static struct platform_driver sun9i_usb_phy_driver = {
> + .probe = sun9i_usb_phy_probe,
> + .driver = {
> + .of_match_table= sun9i_usb_phy_of_match,
> + .name = "sun9i-usb-phy",
> + }
> +};
> +module_platform_driver(sun9i_usb_phy_driver);
> +
> +MODULE_DESCRIPTION("Allwinner sun9i USB phy driver");
> +MODULE_AUTHOR("Chen-Yu Tsai <[email protected]>");
> +MODULE_LICENSE("GPL v2");
> --
> 2.1.1
>
>

2014-11-04 17:00:08

by Maxime Ripard

[permalink] [raw]
Subject: Re: [PATCH 1/6] clk: sunxi: Add support for sun9i a80 usb clocks and resets

Hi,

On Tue, Nov 04, 2014 at 12:07:14PM +0800, Chen-Yu Tsai wrote:
> The USB controller/phy clocks and reset controls are in a separate
> address block, unlike previous SoCs where they were in the clock
> controller.
>
> This patch copies the original gates clk functions used for usb
> clocks into a separate file, and renames them to *_usb_*. Also
> add a per-gate parent index, so we can set different parents for
> each gate.
>
> In time we may move the other usb clock drivers to this file.
>
> Signed-off-by: Chen-Yu Tsai <[email protected]>
> ---
> Documentation/devicetree/bindings/clock/sunxi.txt | 5 +
> drivers/clk/sunxi/Makefile | 1 +
> drivers/clk/sunxi/clk-usb.c | 192 ++++++++++++++++++++++
> 3 files changed, 198 insertions(+)
> create mode 100644 drivers/clk/sunxi/clk-usb.c
>
> diff --git a/Documentation/devicetree/bindings/clock/sunxi.txt b/Documentation/devicetree/bindings/clock/sunxi.txt
> index 0455cb9..b953fe5 100644
> --- a/Documentation/devicetree/bindings/clock/sunxi.txt
> +++ b/Documentation/devicetree/bindings/clock/sunxi.txt
> @@ -66,6 +66,8 @@ Required properties:
> "allwinner,sun4i-a10-usb-clk" - for usb gates + resets on A10 / A20
> "allwinner,sun5i-a13-usb-clk" - for usb gates + resets on A13
> "allwinner,sun6i-a31-usb-clk" - for usb gates + resets on A31
> + "allwinner,sun9i-a80-usb-mod-clk" - for usb gates + resets on A80
> + "allwinner,sun9i-a80-usb-phy-clk" - for usb phy gates + resets on A80
>
> Required properties for all clocks:
> - reg : shall be the control register address for the clock.
> @@ -82,6 +84,9 @@ Required properties for all clocks:
> And "allwinner,*-usb-clk" clocks also require:
> - reset-cells : shall be set to 1
>
> +"allwinner,sun9i-a80-usb-*-clk" clocks require:
> +- clocks : shall be the usb hci ahb1 gate and peripheral pll clocks
> +

In this particular order, I assume?

> For "allwinner,sun7i-a20-gmac-clk", the parent clocks shall be fixed rate
> dummy clocks at 25 MHz and 125 MHz, respectively. See example.
>
> diff --git a/drivers/clk/sunxi/Makefile b/drivers/clk/sunxi/Makefile
> index a66953c..f19ce54 100644
> --- a/drivers/clk/sunxi/Makefile
> +++ b/drivers/clk/sunxi/Makefile
> @@ -8,6 +8,7 @@ obj-y += clk-a20-gmac.o
> obj-y += clk-mod0.o
> obj-y += clk-sun8i-mbus.o
> obj-y += clk-sun9i-core.o
> +obj-y += clk-usb.o
>
> obj-$(CONFIG_MFD_SUN6I_PRCM) += \
> clk-sun6i-ar100.o clk-sun6i-apb0.o clk-sun6i-apb0-gates.o \
> diff --git a/drivers/clk/sunxi/clk-usb.c b/drivers/clk/sunxi/clk-usb.c
> new file mode 100644
> index 0000000..d92ee36
> --- /dev/null
> +++ b/drivers/clk/sunxi/clk-usb.c
> @@ -0,0 +1,192 @@
> +/*
> + * Copyright 2013 Emilio L?pez
> + *
> + * Emilio L?pez <[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-provider.h>
> +#include <linux/clkdev.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/reset-controller.h>
> +#include <linux/spinlock.h>
> +
> +
> +/**
> + * sunxi_usb_reset... - reset bits in usb clk registers handling
> + */
> +
> +struct usb_reset_data {
> + void __iomem *reg;
> + spinlock_t *lock;
> + struct reset_controller_dev rcdev;
> +};
> +
> +static int sunxi_usb_reset_assert(struct reset_controller_dev *rcdev,
> + unsigned long id)
> +{
> + struct usb_reset_data *data = container_of(rcdev,
> + struct usb_reset_data,
> + rcdev);
> + unsigned long flags;
> + u32 reg;
> +
> + spin_lock_irqsave(data->lock, flags);
> +
> + reg = readl(data->reg);
> + writel(reg & ~BIT(id), data->reg);
> +
> + spin_unlock_irqrestore(data->lock, flags);
> +
> + return 0;
> +}
> +
> +static int sunxi_usb_reset_deassert(struct reset_controller_dev *rcdev,
> + unsigned long id)
> +{
> + struct usb_reset_data *data = container_of(rcdev,
> + struct usb_reset_data,
> + rcdev);
> + unsigned long flags;
> + u32 reg;
> +
> + spin_lock_irqsave(data->lock, flags);
> +
> + reg = readl(data->reg);
> + writel(reg | BIT(id), data->reg);
> +
> + spin_unlock_irqrestore(data->lock, flags);
> +
> + return 0;
> +}
> +
> +static struct reset_control_ops sunxi_usb_reset_ops = {
> + .assert = sunxi_usb_reset_assert,
> + .deassert = sunxi_usb_reset_deassert,
> +};
> +
> +/**
> + * sunxi_usb_clk_setup() - Setup function for usb gate clocks
> + */
> +
> +#define SUNXI_USB_MAX_SIZE 32
> +
> +struct usb_clk_data {
> + u32 clk_mask;
> + u32 reset_mask;
> + /* which parent to use, should match clock-output-names */
> + char parents[SUNXI_USB_MAX_SIZE];
> +};
> +
> +static void __init sunxi_usb_clk_setup(struct device_node *node,
> + const struct usb_clk_data *data,
> + spinlock_t *lock)
> +{
> + struct clk_onecell_data *clk_data;
> + struct usb_reset_data *reset_data;
> + const char *clk_parent;
> + const char *clk_name;
> + void __iomem *reg;
> + int qty;
> + int i = 0;
> + int j = 0;
> +
> + reg = of_iomap(node, 0);

of_io_request_and_map?

> +
> + /* Worst-case size approximation and memory allocation */
> + qty = find_last_bit((unsigned long *)&data->clk_mask,
> + SUNXI_USB_MAX_SIZE);
> + clk_data = kmalloc(sizeof(struct clk_onecell_data), GFP_KERNEL);
> + if (!clk_data)
> + return;
> + clk_data->clks = kzalloc((qty+1) * sizeof(struct clk *), GFP_KERNEL);
> + if (!clk_data->clks) {
> + kfree(clk_data);
> + return;
> + }
> +
> + for_each_set_bit(i, (unsigned long *)&data->clk_mask,
> + SUNXI_USB_MAX_SIZE) {
> + of_property_read_string_index(node, "clock-output-names",
> + j, &clk_name);
> + clk_parent = of_clk_get_parent_name(node, data->parents[j]);
> +
> + clk_data->clks[i] = clk_register_gate(NULL, clk_name,
> + clk_parent, 0,
> + reg, i, 0, lock);
> + WARN_ON(IS_ERR(clk_data->clks[i]));
> + clk_register_clkdev(clk_data->clks[i], clk_name, NULL);
> +
> + j++;
> + }
> +
> + /* Adjust to the real max */
> + clk_data->clk_num = i;
> +
> + of_clk_add_provider(node, of_clk_src_onecell_get, clk_data);
> +
> + /* Register a reset controller for usb with reset bits */
> + if (data->reset_mask == 0)
> + return;
> +
> + reset_data = kzalloc(sizeof(*reset_data), GFP_KERNEL);
> + if (!reset_data)
> + return;
> +
> + reset_data->reg = reg;
> + reset_data->lock = lock;
> + reset_data->rcdev.nr_resets = __fls(data->reset_mask) + 1;
> + reset_data->rcdev.ops = &sunxi_usb_reset_ops;
> + reset_data->rcdev.of_node = node;
> + reset_controller_register(&reset_data->rcdev);
> +}
> +
> +static const struct usb_clk_data sun9i_a80_usb_mod_data __initconst = {
> + .clk_mask = BIT(6) | BIT(5) | BIT(4) | BIT(3) | BIT(2) | BIT(1),
> + .reset_mask = BIT(19) | BIT(18) | BIT(17),
> + .parents = {0, 1, 0, 1, 0, 1},
> +};
> +
> +static DEFINE_SPINLOCK(a80_usb_mod_lock);
> +
> +static void __init sun9i_a80_usb_mod_setup(struct device_node *node)
> +{
> + /* AHB1 gate must be enabled to access registers */
> + struct clk *ahb = of_clk_get(node, 0);
> +
> + WARN_ON(IS_ERR(ahb));
> + clk_prepare_enable(ahb);

Hmmmm. That look off.

Why do you need the clock to be enabled all the time? Isn't the CCF
already taking care of enabling the parent clock whenever it needs to
access any register?

Thanks,
Maxime

--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com


Attachments:
(No filename) (7.63 kB)
signature.asc (819.00 B)
Digital signature
Download all attachments

2014-11-04 17:05:07

by Maxime Ripard

[permalink] [raw]
Subject: Re: [PATCH 3/6] phy: Add driver to support individual USB PHYs on sun9i

On Tue, Nov 04, 2014 at 12:07:16PM +0800, Chen-Yu Tsai wrote:
> Unlike previous Allwinner SoCs, there is no central PHY control block
> on the A80. Also, OTG support is completely split off into a different
> controller.
>
> This adds a new driver to support the regular USB PHYs.
>
> Signed-off-by: Chen-Yu Tsai <[email protected]>
> ---
> .../devicetree/bindings/phy/sun9i-usb-phy.txt | 34 +++
> drivers/phy/Kconfig | 12 ++
> drivers/phy/Makefile | 1 +
> drivers/phy/phy-sun9i-usb.c | 227 +++++++++++++++++++++
> 4 files changed, 274 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/phy/sun9i-usb-phy.txt
> create mode 100644 drivers/phy/phy-sun9i-usb.c
>
> diff --git a/Documentation/devicetree/bindings/phy/sun9i-usb-phy.txt b/Documentation/devicetree/bindings/phy/sun9i-usb-phy.txt
> new file mode 100644
> index 0000000..27a6067
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/sun9i-usb-phy.txt
> @@ -0,0 +1,34 @@
> +Allwinner sun9i USB PHY
> +-----------------------
> +
> +Required properties:
> +- compatible : should be one of
> + * allwinner,sun9i-a80-usb-phy
> +- reg : a list of offset + length pairs
> +- #phy-cells : from the generic phy bindings, must be 0
> +- clocks : phandle + clock specifier for the phy clocks
> +- clock-names :
> + * "phy" for normal USB
> + * "hsic_480M", "hsic_12M" for HSIC

Do you need all of them? phy and one of the hsic one?

> +- resets : a list of phandle + reset specifier pairs
> +- reset-names :
> + * "phy" for normal USB
> + * "hsic" for HSIC
> +- phy_type : "hsic" for HSIC usage;
> + other values or absence of this property indicates normal USB
> +
> +It is recommended to list all clocks and resets available.
> +The driver will only use those matching the phy_type.
> +
> +Example:
> + usbphy1: phy@00a01800 {
> + compatible = "allwinner,sun9i-a80-usb-phy";
> + reg = <0x00a01800 0x4>;
> + clocks = <&usb_phy_clk 2>, <&usb_phy_clk 10>,
> + <&usb_phy_clk 3>;
> + clock-names = "hsic_480M", "hsic_12M", "phy";
> + resets = <&usb_phy_clk 18>, <&usb_phy_clk 19>;
> + reset-names = "hsic", "phy";
> + status = "disabled";
> + #phy-cells = <0>;
> + };
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index 2a436e6..f5b7fbb 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -153,6 +153,18 @@ config PHY_SUN4I_USB
> This driver controls the entire USB PHY block, both the USB OTG
> parts, as well as the 2 regular USB 2 host PHYs.
>
> +config PHY_SUN9I_USB
> + tristate "Allwinner sun9i SoC USB PHY driver"
> + depends on ARCH_SUNXI && HAS_IOMEM && OF
> + depends on RESET_CONTROLLER
> + select USB_PHY
> + select GENERIC_PHY
> + help
> + Enable this to support the transceiver that is part of Allwinner
> + sun9i SoCs.
> +
> + This driver controls each individual USB 2 host PHY.
> +
> config PHY_SAMSUNG_USB2
> tristate "Samsung USB 2.0 PHY driver"
> depends on HAS_IOMEM
> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> index c4590fc..c3977dc 100644
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -17,6 +17,7 @@ obj-$(CONFIG_TWL4030_USB) += phy-twl4030-usb.o
> obj-$(CONFIG_PHY_EXYNOS5250_SATA) += phy-exynos5250-sata.o
> obj-$(CONFIG_PHY_HIX5HD2_SATA) += phy-hix5hd2-sata.o
> obj-$(CONFIG_PHY_SUN4I_USB) += phy-sun4i-usb.o
> +obj-$(CONFIG_PHY_SUN9I_USB) += phy-sun9i-usb.o
> obj-$(CONFIG_PHY_SAMSUNG_USB2) += phy-exynos-usb2.o
> phy-exynos-usb2-y += phy-samsung-usb2.o
> phy-exynos-usb2-$(CONFIG_PHY_EXYNOS4210_USB2) += phy-exynos4210-usb2.o
> diff --git a/drivers/phy/phy-sun9i-usb.c b/drivers/phy/phy-sun9i-usb.c
> new file mode 100644
> index 0000000..f9085d9
> --- /dev/null
> +++ b/drivers/phy/phy-sun9i-usb.c
> @@ -0,0 +1,227 @@
> +/*
> + * Allwinner sun9i USB phy driver
> + *
> + * Copyright (C) 2014 Chen-Yu Tsai <[email protected]>
> + *
> + * Based on phy-sun9i.c from

I doubt this is the file you were thinking of :)

> + * Hans de Goede <[email protected]>
> + *
> + * and code from
> + * Allwinner Technology Co., Ltd. <http://www.allwinnertech.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * 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/err.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/usb/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/reset.h>
> +
> +#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)
> +
> +struct sun9i_usb_phy {
> + struct phy *phy;
> + void __iomem *pmu;
> + struct regulator *vbus;
> + struct reset_control *reset;
> + struct clk *clk;
> + struct clk *hsic_clk;
> +};
> +
> +static void sun9i_usb_phy_passby(struct sun9i_usb_phy *phy, int enable)
> +{
> + u32 bits, reg_value;
> +
> + 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 sun9i_usb_phy_init(struct phy *_phy)
> +{
> + struct sun9i_usb_phy *phy = phy_get_drvdata(_phy);
> + int ret;
> +
> + ret = clk_prepare_enable(phy->clk);
> + if (ret)
> + return ret;
> +
> + if (phy->hsic_clk) {
> + ret = clk_prepare_enable(phy->hsic_clk);
> + if (ret) {
> + clk_disable_unprepare(phy->clk);
> + return ret;
> + }
> + }
> +
> + ret = reset_control_deassert(phy->reset);
> + if (ret) {
> + if (phy->hsic_clk)
> + clk_disable_unprepare(phy->hsic_clk);
> + clk_disable_unprepare(phy->clk);
> + return ret;

Gotos in your exit path please.

> + }
> +
> + sun9i_usb_phy_passby(phy, 1);
> +
> + return 0;
> +}
> +
> +static int sun9i_usb_phy_exit(struct phy *_phy)
> +{
> + struct sun9i_usb_phy *phy = phy_get_drvdata(_phy);
> +
> + sun9i_usb_phy_passby(phy, 0);
> + reset_control_assert(phy->reset);
> + if (phy->hsic_clk)
> + clk_disable_unprepare(phy->hsic_clk);
> + clk_disable_unprepare(phy->clk);
> +
> + return 0;
> +}
> +
> +static int sun9i_usb_phy_power_on(struct phy *_phy)
> +{
> + struct sun9i_usb_phy *phy = phy_get_drvdata(_phy);
> + int ret = 0;
> +
> + if (phy->vbus)
> + ret = regulator_enable(phy->vbus);
> +
> + return ret;
> +}
> +
> +static int sun9i_usb_phy_power_off(struct phy *_phy)
> +{
> + struct sun9i_usb_phy *phy = phy_get_drvdata(_phy);
> +
> + if (phy->vbus)
> + regulator_disable(phy->vbus);
> +
> + return 0;
> +}
> +
> +static struct phy_ops sun9i_usb_phy_ops = {
> + .init = sun9i_usb_phy_init,
> + .exit = sun9i_usb_phy_exit,
> + .power_on = sun9i_usb_phy_power_on,
> + .power_off = sun9i_usb_phy_power_off,
> + .owner = THIS_MODULE,
> +};
> +
> +static int sun9i_usb_phy_probe(struct platform_device *pdev)
> +{
> + struct sun9i_usb_phy *phy;
> + struct device *dev = &pdev->dev;
> + struct device_node *np = dev->of_node;
> + struct phy_provider *phy_provider;
> + struct resource *res;
> + enum usb_phy_interface phy_type;
> +
> + phy = devm_kzalloc(dev, sizeof(*phy), GFP_KERNEL);
> + if (!phy)
> + return -ENOMEM;
> +
> + phy->vbus = devm_regulator_get_optional(dev, "vbus");
> + if (IS_ERR(phy->vbus)) {
> + if (PTR_ERR(phy->vbus) == -EPROBE_DEFER)
> + return -EPROBE_DEFER;
> + phy->vbus = NULL;
> + }
> +
> + phy_type = of_usb_get_phy_mode(np);
> + if (phy_type == USBPHY_INTERFACE_MODE_HSIC) {
> + phy->clk = devm_clk_get(dev, "hsic_480M");
> + if (IS_ERR(phy->clk)) {
> + dev_err(dev, "failed to get hsic_480M clock\n");
> + return PTR_ERR(phy->clk);
> + }
> +
> + phy->hsic_clk = devm_clk_get(dev, "hsic_12M");
> + if (IS_ERR(phy->clk)) {
> + dev_err(dev, "failed to get hsic_12M clock\n");
> + return PTR_ERR(phy->clk);
> + }
> +
> + phy->reset = devm_reset_control_get(dev, "hsic");
> + if (IS_ERR(phy->reset)) {
> + dev_err(dev, "failed to get reset control\n");
> + return PTR_ERR(phy->reset);
> + }
> + } else {
> + phy->clk = devm_clk_get(dev, "phy");
> + if (IS_ERR(phy->clk)) {
> + dev_err(dev, "failed to get phy clock\n");
> + return PTR_ERR(phy->clk);
> + }
> +
> + phy->reset = devm_reset_control_get(dev, "phy");
> + if (IS_ERR(phy->reset)) {
> + dev_err(dev, "failed to get reset control\n");
> + return PTR_ERR(phy->reset);
> + }
> + }
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + phy->pmu = devm_ioremap_resource(dev, res);
> + if (IS_ERR(phy->pmu))
> + return PTR_ERR(phy->pmu);
> +
> + phy->phy = devm_phy_create(dev, NULL, &sun9i_usb_phy_ops, NULL);
> + if (IS_ERR(phy->phy)) {
> + dev_err(dev, "failed to create PHY\n");
> + return PTR_ERR(phy->phy);
> + }
> +
> + phy_set_drvdata(phy->phy, phy);
> + dev_set_drvdata(dev, phy);
> + phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
> +
> + return PTR_ERR_OR_ZERO(phy_provider);
> +}
> +
> +static const struct of_device_id sun9i_usb_phy_of_match[] = {
> + { .compatible = "allwinner,sun9i-a80-usb-phy" },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, sun9i_usb_phy_of_match);
> +
> +static struct platform_driver sun9i_usb_phy_driver = {
> + .probe = sun9i_usb_phy_probe,
> + .driver = {
> + .of_match_table = sun9i_usb_phy_of_match,
> + .name = "sun9i-usb-phy",
> + }
> +};
> +module_platform_driver(sun9i_usb_phy_driver);
> +
> +MODULE_DESCRIPTION("Allwinner sun9i USB phy driver");
> +MODULE_AUTHOR("Chen-Yu Tsai <[email protected]>");
> +MODULE_LICENSE("GPL v2");

Thanks!
Maxime

--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com


Attachments:
(No filename) (9.99 kB)
signature.asc (819.00 B)
Digital signature
Download all attachments

2014-11-04 18:12:42

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [PATCH 1/6] clk: sunxi: Add support for sun9i a80 usb clocks and resets

On Tue, Nov 04, 2014 at 12:07:14PM +0800, Chen-Yu Tsai wrote:
> + spin_lock_irqsave(data->lock, flags);
> +
> + reg = readl(data->reg);
> + writel(reg & ~BIT(id), data->reg);
> +
> + spin_unlock_irqrestore(data->lock, flags);

Don't we have generic support for atomic modification of register
values? Hmm, we have it for ARM only - atomic_io_modify() and
atomic_io_modify_relaxed().

I guess we should push for those to become cross-arch if we end up
with generic drivers shared between other architectures.

--
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.

2014-11-05 09:31:54

by Chen-Yu Tsai

[permalink] [raw]
Subject: Re: [PATCH 3/6] phy: Add driver to support individual USB PHYs on sun9i

On Wed, Nov 5, 2014 at 1:03 AM, Maxime Ripard
<[email protected]> wrote:
> On Tue, Nov 04, 2014 at 12:07:16PM +0800, Chen-Yu Tsai wrote:
>> Unlike previous Allwinner SoCs, there is no central PHY control block
>> on the A80. Also, OTG support is completely split off into a different
>> controller.
>>
>> This adds a new driver to support the regular USB PHYs.
>>
>> Signed-off-by: Chen-Yu Tsai <[email protected]>
>> ---
>> .../devicetree/bindings/phy/sun9i-usb-phy.txt | 34 +++
>> drivers/phy/Kconfig | 12 ++
>> drivers/phy/Makefile | 1 +
>> drivers/phy/phy-sun9i-usb.c | 227 +++++++++++++++++++++
>> 4 files changed, 274 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/phy/sun9i-usb-phy.txt
>> create mode 100644 drivers/phy/phy-sun9i-usb.c
>>
>> diff --git a/Documentation/devicetree/bindings/phy/sun9i-usb-phy.txt b/Documentation/devicetree/bindings/phy/sun9i-usb-phy.txt
>> new file mode 100644
>> index 0000000..27a6067
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/phy/sun9i-usb-phy.txt
>> @@ -0,0 +1,34 @@
>> +Allwinner sun9i USB PHY
>> +-----------------------
>> +
>> +Required properties:
>> +- compatible : should be one of
>> + * allwinner,sun9i-a80-usb-phy
>> +- reg : a list of offset + length pairs
>> +- #phy-cells : from the generic phy bindings, must be 0
>> +- clocks : phandle + clock specifier for the phy clocks
>> +- clock-names :
>> + * "phy" for normal USB
>> + * "hsic_480M", "hsic_12M" for HSIC
>
> Do you need all of them? phy and one of the hsic one?

It would be either "phy" or the hsic ones, depending on the
value of the "phy_type" property. I will make it clearer.

>> +- resets : a list of phandle + reset specifier pairs
>> +- reset-names :
>> + * "phy" for normal USB
>> + * "hsic" for HSIC
>> +- phy_type : "hsic" for HSIC usage;
>> + other values or absence of this property indicates normal USB
>> +
>> +It is recommended to list all clocks and resets available.
>> +The driver will only use those matching the phy_type.
>> +
>> +Example:
>> + usbphy1: phy@00a01800 {
>> + compatible = "allwinner,sun9i-a80-usb-phy";
>> + reg = <0x00a01800 0x4>;
>> + clocks = <&usb_phy_clk 2>, <&usb_phy_clk 10>,
>> + <&usb_phy_clk 3>;
>> + clock-names = "hsic_480M", "hsic_12M", "phy";
>> + resets = <&usb_phy_clk 18>, <&usb_phy_clk 19>;
>> + reset-names = "hsic", "phy";
>> + status = "disabled";
>> + #phy-cells = <0>;
>> + };
>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
>> index 2a436e6..f5b7fbb 100644
>> --- a/drivers/phy/Kconfig
>> +++ b/drivers/phy/Kconfig
>> @@ -153,6 +153,18 @@ config PHY_SUN4I_USB
>> This driver controls the entire USB PHY block, both the USB OTG
>> parts, as well as the 2 regular USB 2 host PHYs.
>>
>> +config PHY_SUN9I_USB
>> + tristate "Allwinner sun9i SoC USB PHY driver"
>> + depends on ARCH_SUNXI && HAS_IOMEM && OF
>> + depends on RESET_CONTROLLER
>> + select USB_PHY
>> + select GENERIC_PHY
>> + help
>> + Enable this to support the transceiver that is part of Allwinner
>> + sun9i SoCs.
>> +
>> + This driver controls each individual USB 2 host PHY.
>> +
>> config PHY_SAMSUNG_USB2
>> tristate "Samsung USB 2.0 PHY driver"
>> depends on HAS_IOMEM
>> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
>> index c4590fc..c3977dc 100644
>> --- a/drivers/phy/Makefile
>> +++ b/drivers/phy/Makefile
>> @@ -17,6 +17,7 @@ obj-$(CONFIG_TWL4030_USB) += phy-twl4030-usb.o
>> obj-$(CONFIG_PHY_EXYNOS5250_SATA) += phy-exynos5250-sata.o
>> obj-$(CONFIG_PHY_HIX5HD2_SATA) += phy-hix5hd2-sata.o
>> obj-$(CONFIG_PHY_SUN4I_USB) += phy-sun4i-usb.o
>> +obj-$(CONFIG_PHY_SUN9I_USB) += phy-sun9i-usb.o
>> obj-$(CONFIG_PHY_SAMSUNG_USB2) += phy-exynos-usb2.o
>> phy-exynos-usb2-y += phy-samsung-usb2.o
>> phy-exynos-usb2-$(CONFIG_PHY_EXYNOS4210_USB2) += phy-exynos4210-usb2.o
>> diff --git a/drivers/phy/phy-sun9i-usb.c b/drivers/phy/phy-sun9i-usb.c
>> new file mode 100644
>> index 0000000..f9085d9
>> --- /dev/null
>> +++ b/drivers/phy/phy-sun9i-usb.c
>> @@ -0,0 +1,227 @@
>> +/*
>> + * Allwinner sun9i USB phy driver
>> + *
>> + * Copyright (C) 2014 Chen-Yu Tsai <[email protected]>
>> + *
>> + * Based on phy-sun9i.c from
>
> I doubt this is the file you were thinking of :)

This is what happens when you do a global replace. :|

>> + * Hans de Goede <[email protected]>
>> + *
>> + * and code from
>> + * Allwinner Technology Co., Ltd. <http://www.allwinnertech.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * 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/err.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/usb/of.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/reset.h>
>> +
>> +#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)
>> +
>> +struct sun9i_usb_phy {
>> + struct phy *phy;
>> + void __iomem *pmu;
>> + struct regulator *vbus;
>> + struct reset_control *reset;
>> + struct clk *clk;
>> + struct clk *hsic_clk;
>> +};
>> +
>> +static void sun9i_usb_phy_passby(struct sun9i_usb_phy *phy, int enable)
>> +{
>> + u32 bits, reg_value;
>> +
>> + 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 sun9i_usb_phy_init(struct phy *_phy)
>> +{
>> + struct sun9i_usb_phy *phy = phy_get_drvdata(_phy);
>> + int ret;
>> +
>> + ret = clk_prepare_enable(phy->clk);
>> + if (ret)
>> + return ret;
>> +
>> + if (phy->hsic_clk) {
>> + ret = clk_prepare_enable(phy->hsic_clk);
>> + if (ret) {
>> + clk_disable_unprepare(phy->clk);
>> + return ret;
>> + }
>> + }
>> +
>> + ret = reset_control_deassert(phy->reset);
>> + if (ret) {
>> + if (phy->hsic_clk)
>> + clk_disable_unprepare(phy->hsic_clk);
>> + clk_disable_unprepare(phy->clk);
>> + return ret;
>
> Gotos in your exit path please.

I will try to clean it up more.

>> + }
>> +
>> + sun9i_usb_phy_passby(phy, 1);
>> +
>> + return 0;
>> +}
>> +
>> +static int sun9i_usb_phy_exit(struct phy *_phy)
>> +{
>> + struct sun9i_usb_phy *phy = phy_get_drvdata(_phy);
>> +
>> + sun9i_usb_phy_passby(phy, 0);
>> + reset_control_assert(phy->reset);
>> + if (phy->hsic_clk)
>> + clk_disable_unprepare(phy->hsic_clk);
>> + clk_disable_unprepare(phy->clk);
>> +
>> + return 0;
>> +}
>> +
>> +static int sun9i_usb_phy_power_on(struct phy *_phy)
>> +{
>> + struct sun9i_usb_phy *phy = phy_get_drvdata(_phy);
>> + int ret = 0;
>> +
>> + if (phy->vbus)
>> + ret = regulator_enable(phy->vbus);
>> +
>> + return ret;
>> +}
>> +
>> +static int sun9i_usb_phy_power_off(struct phy *_phy)
>> +{
>> + struct sun9i_usb_phy *phy = phy_get_drvdata(_phy);
>> +
>> + if (phy->vbus)
>> + regulator_disable(phy->vbus);
>> +
>> + return 0;
>> +}
>> +
>> +static struct phy_ops sun9i_usb_phy_ops = {
>> + .init = sun9i_usb_phy_init,
>> + .exit = sun9i_usb_phy_exit,
>> + .power_on = sun9i_usb_phy_power_on,
>> + .power_off = sun9i_usb_phy_power_off,
>> + .owner = THIS_MODULE,
>> +};
>> +
>> +static int sun9i_usb_phy_probe(struct platform_device *pdev)
>> +{
>> + struct sun9i_usb_phy *phy;
>> + struct device *dev = &pdev->dev;
>> + struct device_node *np = dev->of_node;
>> + struct phy_provider *phy_provider;
>> + struct resource *res;
>> + enum usb_phy_interface phy_type;
>> +
>> + phy = devm_kzalloc(dev, sizeof(*phy), GFP_KERNEL);
>> + if (!phy)
>> + return -ENOMEM;
>> +
>> + phy->vbus = devm_regulator_get_optional(dev, "vbus");
>> + if (IS_ERR(phy->vbus)) {
>> + if (PTR_ERR(phy->vbus) == -EPROBE_DEFER)
>> + return -EPROBE_DEFER;
>> + phy->vbus = NULL;
>> + }
>> +
>> + phy_type = of_usb_get_phy_mode(np);
>> + if (phy_type == USBPHY_INTERFACE_MODE_HSIC) {
>> + phy->clk = devm_clk_get(dev, "hsic_480M");
>> + if (IS_ERR(phy->clk)) {
>> + dev_err(dev, "failed to get hsic_480M clock\n");
>> + return PTR_ERR(phy->clk);
>> + }
>> +
>> + phy->hsic_clk = devm_clk_get(dev, "hsic_12M");
>> + if (IS_ERR(phy->clk)) {
>> + dev_err(dev, "failed to get hsic_12M clock\n");
>> + return PTR_ERR(phy->clk);
>> + }
>> +
>> + phy->reset = devm_reset_control_get(dev, "hsic");
>> + if (IS_ERR(phy->reset)) {
>> + dev_err(dev, "failed to get reset control\n");
>> + return PTR_ERR(phy->reset);
>> + }
>> + } else {
>> + phy->clk = devm_clk_get(dev, "phy");
>> + if (IS_ERR(phy->clk)) {
>> + dev_err(dev, "failed to get phy clock\n");
>> + return PTR_ERR(phy->clk);
>> + }
>> +
>> + phy->reset = devm_reset_control_get(dev, "phy");
>> + if (IS_ERR(phy->reset)) {
>> + dev_err(dev, "failed to get reset control\n");
>> + return PTR_ERR(phy->reset);
>> + }
>> + }
>> +
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> + phy->pmu = devm_ioremap_resource(dev, res);
>> + if (IS_ERR(phy->pmu))
>> + return PTR_ERR(phy->pmu);
>> +
>> + phy->phy = devm_phy_create(dev, NULL, &sun9i_usb_phy_ops, NULL);
>> + if (IS_ERR(phy->phy)) {
>> + dev_err(dev, "failed to create PHY\n");
>> + return PTR_ERR(phy->phy);
>> + }
>> +
>> + phy_set_drvdata(phy->phy, phy);
>> + dev_set_drvdata(dev, phy);
>> + phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
>> +
>> + return PTR_ERR_OR_ZERO(phy_provider);
>> +}
>> +
>> +static const struct of_device_id sun9i_usb_phy_of_match[] = {
>> + { .compatible = "allwinner,sun9i-a80-usb-phy" },
>> + { },
>> +};
>> +MODULE_DEVICE_TABLE(of, sun9i_usb_phy_of_match);
>> +
>> +static struct platform_driver sun9i_usb_phy_driver = {
>> + .probe = sun9i_usb_phy_probe,
>> + .driver = {
>> + .of_match_table = sun9i_usb_phy_of_match,
>> + .name = "sun9i-usb-phy",
>> + }
>> +};
>> +module_platform_driver(sun9i_usb_phy_driver);
>> +
>> +MODULE_DESCRIPTION("Allwinner sun9i USB phy driver");
>> +MODULE_AUTHOR("Chen-Yu Tsai <[email protected]>");
>> +MODULE_LICENSE("GPL v2");
>
> Thanks!
> Maxime

Thanks for the review.

ChenYu

2014-11-05 09:35:28

by Chen-Yu Tsai

[permalink] [raw]
Subject: Re: [linux-sunxi] [PATCH 3/6] phy: Add driver to support individual USB PHYs on sun9i

Hi,

On Tue, Nov 4, 2014 at 2:16 PM, Priit Laes <[email protected]> wrote:
>
> On Tue, 2014-11-04 at 12:07 +0800, Chen-Yu Tsai wrote:
>> Unlike previous Allwinner SoCs, there is no central PHY control block
>> on the A80. Also, OTG support is completely split off into a
>> different
>> controller.
>>
>> This adds a new driver to support the regular USB PHYs.
>>
>> Signed-off-by: Chen-Yu Tsai <[email protected]>
>> ---
>> .../devicetree/bindings/phy/sun9i-usb-phy.txt | 34 +++
>> drivers/phy/Kconfig | 12 ++
>> drivers/phy/Makefile | 1 +
>> drivers/phy/phy-sun9i-usb.c | 227
>> +++++++++++++++++++++
>> 4 files changed, 274 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/phy/sun9i-usb-
>> phy.txt
>> create mode 100644 drivers/phy/phy-sun9i-usb.c
>>
>> diff --git a/Documentation/devicetree/bindings/phy/sun9i-usb-phy.txt
>> b/Documentation/devicetree/bindings/phy/sun9i-usb-phy.txt
>> new file mode 100644
>> index 0000000..27a6067
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/phy/sun9i-usb-phy.txt
>> @@ -0,0 +1,34 @@
>> +Allwinner sun9i USB PHY
>> +-----------------------
>> +
>> +Required properties:
>> +- compatible : should be one of
>> + * allwinner,sun9i-a80-usb-phy
>> +- reg : a list of offset + length pairs
>> +- #phy-cells : from the generic phy bindings, must be 0
>> +- clocks : phandle + clock specifier for the phy clocks
>> +- clock-names :
>> + * "phy" for normal USB
>> + * "hsic_480M", "hsic_12M" for HSIC
>> +- resets : a list of phandle + reset specifier pairs
>> +- reset-names :
>> + * "phy" for normal USB
>> + * "hsic" for HSIC
>> +- phy_type : "hsic" for HSIC usage;
>> + other values or
>> absence of this property indicates normal USB
>> +
>> +It is recommended to list all clocks and resets available.
>> +The driver will only use those matching the phy_type.
>> +
>> +Example:
>> + usbphy1: phy@00a01800 {
>> + compatible = "allwinner,sun9i-a80-usb-phy";
>> + reg = <0x00a01800 0x4>;
>> + clocks = <&usb_phy_clk 2>, <&usb_phy_clk 10>,
>> +
>> <&usb_phy_clk 3>;
>> + clock-names = "hsic_480M", "hsic_12M", "phy";
>> + resets = <&usb_phy_clk 18>, <&usb_phy_clk 19>;
>> + reset-names = "hsic", "phy";
>> + status = "disabled";
>> + #phy-cells = <0>;
>> + };
>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
>> index 2a436e6..f5b7fbb 100644
>> --- a/drivers/phy/Kconfig
>> +++ b/drivers/phy/Kconfig
>> @@ -153,6 +153,18 @@ config PHY_SUN4I_USB
>> This driver controls the entire USB PHY
>> block, both the USB OTG
>> parts, as well as the 2 regular USB 2 host
>> PHYs.
>>
>> +config PHY_SUN9I_USB
>> + tristate "Allwinner sun9i SoC USB PHY driver"
>
> Adding 'A80' to the user visible data makes things a bit easier for
> end users:
> "AllWinner sun9i (A80) SoC USB PHY driver"

We explicitly left out "A80" from the description of the MACH_SUN9I
Kconfig option. Keeping it the same here.

But I will add a few more patches to this series to enable this
by default in (sunxi and multiv7) defconfigs.

>> + depends on ARCH_SUNXI && HAS_IOMEM && OF
>> + depends on RESET_CONTROLLER
>> + select USB_PHY
>> + select GENERIC_PHY
>> + help
>> + Enable this to support the transceiver that
>> is part of Allwinner
>> + sun9i SoCs.
>
> And extra 'A80' here wouldn't hurt either.
>
>> +
>> + This driver controls each individual USB 2
>> host PHY.
>> +
>> config PHY_SAMSUNG_USB2
>> tristate "Samsung USB 2.0 PHY driver"
>> depends on HAS_IOMEM
>> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
>> index c4590fc..c3977dc 100644
>> --- a/drivers/phy/Makefile
>> +++ b/drivers/phy/Makefile
>> @@ -17,6 +17,7 @@ obj-$(CONFIG_TWL4030_USB) += phy-
>> twl4030-usb.o
>> obj-$(CONFIG_PHY_EXYNOS5250_SATA) += phy-exynos5250-sata.o
>> obj-$(CONFIG_PHY_HIX5HD2_SATA) += phy-hix5hd2-sata.o
>> obj-$(CONFIG_PHY_SUN4I_USB) += phy-sun4i-usb.o
>> +obj-$(CONFIG_PHY_SUN9I_USB) += phy-sun9i-usb.o
>> obj-$(CONFIG_PHY_SAMSUNG_USB2) += phy-exynos-usb2.o
>> phy-exynos-usb2-y += phy-samsung-usb2.o
>> phy-exynos-usb2-$(CONFIG_PHY_EXYNOS4210_USB2) += phy-exynos4210-
>> usb2.o
>> diff --git a/drivers/phy/phy-sun9i-usb.c b/drivers/phy/phy-sun9i-
>> usb.c
>> new file mode 100644
>> index 0000000..f9085d9
>> --- /dev/null
>> +++ b/drivers/phy/phy-sun9i-usb.c
>> @@ -0,0 +1,227 @@
>> +/*
>> + * Allwinner sun9i USB phy driver
>> + *
>> + * Copyright (C) 2014 Chen-Yu Tsai <[email protected]>
>> + *
>> + * Based on phy-sun9i.c from
>> + * Hans de Goede <[email protected]>
>> + *
>> + * and code from
>> + * Allwinner Technology Co., Ltd. <http://www.allwinnertech.com>
>> + *
>> + * This program is free software; you can redistribute it and/or
>> modify
>> + * it under the terms of the GNU General Public License as
>> published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * 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/err.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/usb/of.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/reset.h>
>> +
>> +#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)
>> +
>> +struct sun9i_usb_phy {
>> + struct phy *phy;
>> + void __iomem *pmu;
>> + struct regulator *vbus;
>> + struct reset_control *reset;
>> + struct clk *clk;
>> + struct clk *hsic_clk;
>> +};
>> +
>> +static void sun9i_usb_phy_passby(struct sun9i_usb_phy *phy, int
>> enable)
>> +{
>> + u32 bits, reg_value;
>> +
>> + 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 sun9i_usb_phy_init(struct phy *_phy)
>> +{
>> + struct sun9i_usb_phy *phy = phy_get_drvdata(_phy);
>> + int ret;
>> +
>> + ret = clk_prepare_enable(phy->clk);
>> + if (ret)
>> + return ret;
>> +
>> + if (phy->hsic_clk) {
>> + ret = clk_prepare_enable(phy->hsic_clk);
>> + if (ret) {
>> + clk_disable_unprepare(phy->clk);
>> + return ret;
>> + }
>> + }
>> +
>> + ret = reset_control_deassert(phy->reset);
>> + if (ret) {
>> + if (phy->hsic_clk)
>> + clk_disable_unprepare(phy->hsic_clk);
>> + clk_disable_unprepare(phy->clk);
>> + return ret;
>> + }
>> +
>> + sun9i_usb_phy_passby(phy, 1);
>> +
>> + return 0;
>> +}
>> +
>> +static int sun9i_usb_phy_exit(struct phy *_phy)
>> +{
>> + struct sun9i_usb_phy *phy = phy_get_drvdata(_phy);
>> +
>> + sun9i_usb_phy_passby(phy, 0);
>> + reset_control_assert(phy->reset);
>> + if (phy->hsic_clk)
>> + clk_disable_unprepare(phy->hsic_clk);
>> + clk_disable_unprepare(phy->clk);
>> +
>> + return 0;
>> +}
>> +
>> +static int sun9i_usb_phy_power_on(struct phy *_phy)
>> +{
>> + struct sun9i_usb_phy *phy = phy_get_drvdata(_phy);
>> + int ret = 0;
>> +
>> + if (phy->vbus)
>> + ret = regulator_enable(phy->vbus);
>> +
>> + return ret;
>> +}
>> +
>> +static int sun9i_usb_phy_power_off(struct phy *_phy)
>> +{
>> + struct sun9i_usb_phy *phy = phy_get_drvdata(_phy);
>> +
>> + if (phy->vbus)
>> + regulator_disable(phy->vbus);
>> +
>> + return 0;
>> +}
>> +
>> +static struct phy_ops sun9i_usb_phy_ops = {
>> + .init = sun9i_usb_phy_init,
>> + .exit = sun9i_usb_phy_exit,
>> + .power_on= sun9i_usb_phy_power_on,
>> + .power_off= sun9i_usb_phy_power_off,
>> + .owner = THIS_MODULE,
>> +};
>> +
>> +static int sun9i_usb_phy_probe(struct platform_device *pdev)
>> +{
>> + struct sun9i_usb_phy *phy;
>> + struct device *dev = &pdev->dev;
>> + struct device_node *np = dev->of_node;
>> + struct phy_provider *phy_provider;
>> + struct resource *res;
>> + enum usb_phy_interface phy_type;
>> +
>> + phy = devm_kzalloc(dev, sizeof(*phy), GFP_KERNEL);
>> + if (!phy)
>> + return -ENOMEM;
>> +
>> + phy->vbus = devm_regulator_get_optional(dev, "vbus");
>> + if (IS_ERR(phy->vbus)) {
>> + if (PTR_ERR(phy->vbus) == -EPROBE_DEFER)
>> + return -EPROBE_DEFER;
>> + phy->vbus = NULL;
>> + }
>> +
>> + phy_type = of_usb_get_phy_mode(np);
>> + if (phy_type == USBPHY_INTERFACE_MODE_HSIC) {
>> + phy->clk = devm_clk_get(dev, "hsic_480M");
>> + if (IS_ERR(phy->clk)) {
>> + dev_err(dev, "failed to get hsic_480M
>> clock\n");
>> + return PTR_ERR(phy->clk);
>> + }
>> +
>> + phy->hsic_clk = devm_clk_get(dev, "hsic_12M");
>> + if (IS_ERR(phy->clk)) {
>> + dev_err(dev, "failed to get hsic_12M
>> clock\n");
>> + return PTR_ERR(phy->clk);
>> + }
>> +
>> + phy->reset = devm_reset_control_get(dev, "hsic");
>> + if (IS_ERR(phy->reset)) {
>> + dev_err(dev, "failed to get reset
>> control\n");
>> + return PTR_ERR(phy->reset);
>> + }
>> + } else {
>> + phy->clk = devm_clk_get(dev, "phy");
>> + if (IS_ERR(phy->clk)) {
>> + dev_err(dev, "failed to get phy clock\n");
>> + return PTR_ERR(phy->clk);
>> + }
>> +
>> + phy->reset = devm_reset_control_get(dev, "phy");
>> + if (IS_ERR(phy->reset)) {
>> + dev_err(dev, "failed to get reset
>> control\n");
>> + return PTR_ERR(phy->reset);
>> + }
>> + }
>> +
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> + phy->pmu = devm_ioremap_resource(dev, res);
>> + if (IS_ERR(phy->pmu))
>> + return PTR_ERR(phy->pmu);
>> +
>> + phy->phy = devm_phy_create(dev, NULL, &sun9i_usb_phy_ops,
>> NULL);
>> + if (IS_ERR(phy->phy)) {
>> + dev_err(dev, "failed to create PHY\n");
>> + return PTR_ERR(phy->phy);
>> + }
>> +
>> + phy_set_drvdata(phy->phy, phy);
>> + dev_set_drvdata(dev, phy);
>> + phy_provider = devm_of_phy_provider_register(dev,
>> of_phy_simple_xlate);
>> +
>> + return PTR_ERR_OR_ZERO(phy_provider);
>> +}
>> +
>> +static const struct of_device_id sun9i_usb_phy_of_match[] = {
>> + { .compatible = "allwinner,sun9i-a80-usb-phy" },
>> + { },
>> +};
>> +MODULE_DEVICE_TABLE(of, sun9i_usb_phy_of_match);
>> +
>> +static struct platform_driver sun9i_usb_phy_driver = {
>> + .probe = sun9i_usb_phy_probe,
>> + .driver = {
>> + .of_match_table= sun9i_usb_phy_of_match,
>> + .name = "sun9i-usb-phy",
>> + }
>> +};
>> +module_platform_driver(sun9i_usb_phy_driver);
>> +
>> +MODULE_DESCRIPTION("Allwinner sun9i USB phy driver");
>> +MODULE_AUTHOR("Chen-Yu Tsai <[email protected]>");
>> +MODULE_LICENSE("GPL v2");
>> --
>> 2.1.1
>>
>>

2014-11-05 09:45:24

by Maxime Ripard

[permalink] [raw]
Subject: Re: [PATCH 1/6] clk: sunxi: Add support for sun9i a80 usb clocks and resets

Hi Russell,

On Tue, Nov 04, 2014 at 06:12:19PM +0000, Russell King - ARM Linux wrote:
> On Tue, Nov 04, 2014 at 12:07:14PM +0800, Chen-Yu Tsai wrote:
> > + spin_lock_irqsave(data->lock, flags);
> > +
> > + reg = readl(data->reg);
> > + writel(reg & ~BIT(id), data->reg);
> > +
> > + spin_unlock_irqrestore(data->lock, flags);
>
> Don't we have generic support for atomic modification of register
> values? Hmm, we have it for ARM only - atomic_io_modify() and
> atomic_io_modify_relaxed().
>
> I guess we should push for those to become cross-arch if we end up
> with generic drivers shared between other architectures.

IIRC, the atomic MMIO accessors are doing exactly that, but with a
global lock for all MMIO accesses, while here we protect a single
register.

I'm not really sure that sharing this spinlock across the whole system
is worth it and scales that well.

Maxime

--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com


Attachments:
(No filename) (990.00 B)
signature.asc (819.00 B)
Digital signature
Download all attachments

2014-11-05 09:51:15

by Maxime Ripard

[permalink] [raw]
Subject: Re: [linux-sunxi] [PATCH 3/6] phy: Add driver to support individual USB PHYs on sun9i

On Tue, Nov 04, 2014 at 08:16:13AM +0200, Priit Laes wrote:
> > --- a/drivers/phy/Kconfig
> > +++ b/drivers/phy/Kconfig
> > @@ -153,6 +153,18 @@ config PHY_SUN4I_USB
> > This driver controls the entire USB PHY
> > block, both the USB OTG
> > parts, as well as the 2 regular USB 2 host
> > PHYs.
> >
> > +config PHY_SUN9I_USB
> > + tristate "Allwinner sun9i SoC USB PHY driver"
>
> Adding 'A80' to the user visible data makes things a bit easier for
> end users:
> "AllWinner sun9i (A80) SoC USB PHY driver"

Until some new SoC part of the sun9i family comes up, and it will just
add to the confusion.

Plus, who the "end user" is for a Kconfig file? My guess is that it is
a kernel developper, or at least someone with a good kernel
experience.

Such a person would know that the A80 is part of the sun9i family if
it has some knowledge of the Allwinner SocS. And if they don't, A80 or
sun9i or both won't make any kind of difference.

Maxime

--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com


Attachments:
(No filename) (1.08 kB)
signature.asc (819.00 B)
Digital signature
Download all attachments

2014-11-05 10:03:03

by Chen-Yu Tsai

[permalink] [raw]
Subject: Re: [PATCH 1/6] clk: sunxi: Add support for sun9i a80 usb clocks and resets

Hi,

On Wed, Nov 5, 2014 at 12:57 AM, Maxime Ripard
<[email protected]> wrote:
> Hi,
>
> On Tue, Nov 04, 2014 at 12:07:14PM +0800, Chen-Yu Tsai wrote:
>> The USB controller/phy clocks and reset controls are in a separate
>> address block, unlike previous SoCs where they were in the clock
>> controller.
>>
>> This patch copies the original gates clk functions used for usb
>> clocks into a separate file, and renames them to *_usb_*. Also
>> add a per-gate parent index, so we can set different parents for
>> each gate.
>>
>> In time we may move the other usb clock drivers to this file.
>>
>> Signed-off-by: Chen-Yu Tsai <[email protected]>
>> ---
>> Documentation/devicetree/bindings/clock/sunxi.txt | 5 +
>> drivers/clk/sunxi/Makefile | 1 +
>> drivers/clk/sunxi/clk-usb.c | 192 ++++++++++++++++++++++
>> 3 files changed, 198 insertions(+)
>> create mode 100644 drivers/clk/sunxi/clk-usb.c
>>
>> diff --git a/Documentation/devicetree/bindings/clock/sunxi.txt b/Documentation/devicetree/bindings/clock/sunxi.txt
>> index 0455cb9..b953fe5 100644
>> --- a/Documentation/devicetree/bindings/clock/sunxi.txt
>> +++ b/Documentation/devicetree/bindings/clock/sunxi.txt
>> @@ -66,6 +66,8 @@ Required properties:
>> "allwinner,sun4i-a10-usb-clk" - for usb gates + resets on A10 / A20
>> "allwinner,sun5i-a13-usb-clk" - for usb gates + resets on A13
>> "allwinner,sun6i-a31-usb-clk" - for usb gates + resets on A31
>> + "allwinner,sun9i-a80-usb-mod-clk" - for usb gates + resets on A80
>> + "allwinner,sun9i-a80-usb-phy-clk" - for usb phy gates + resets on A80
>>
>> Required properties for all clocks:
>> - reg : shall be the control register address for the clock.
>> @@ -82,6 +84,9 @@ Required properties for all clocks:
>> And "allwinner,*-usb-clk" clocks also require:
>> - reset-cells : shall be set to 1
>>
>> +"allwinner,sun9i-a80-usb-*-clk" clocks require:
>> +- clocks : shall be the usb hci ahb1 gate and peripheral pll clocks
>> +
>
> In this particular order, I assume?

Yes, I will make it clear.

>> For "allwinner,sun7i-a20-gmac-clk", the parent clocks shall be fixed rate
>> dummy clocks at 25 MHz and 125 MHz, respectively. See example.
>>
>> diff --git a/drivers/clk/sunxi/Makefile b/drivers/clk/sunxi/Makefile
>> index a66953c..f19ce54 100644
>> --- a/drivers/clk/sunxi/Makefile
>> +++ b/drivers/clk/sunxi/Makefile
>> @@ -8,6 +8,7 @@ obj-y += clk-a20-gmac.o
>> obj-y += clk-mod0.o
>> obj-y += clk-sun8i-mbus.o
>> obj-y += clk-sun9i-core.o
>> +obj-y += clk-usb.o
>>
>> obj-$(CONFIG_MFD_SUN6I_PRCM) += \
>> clk-sun6i-ar100.o clk-sun6i-apb0.o clk-sun6i-apb0-gates.o \
>> diff --git a/drivers/clk/sunxi/clk-usb.c b/drivers/clk/sunxi/clk-usb.c
>> new file mode 100644
>> index 0000000..d92ee36
>> --- /dev/null
>> +++ b/drivers/clk/sunxi/clk-usb.c
>> @@ -0,0 +1,192 @@
>> +/*
>> + * Copyright 2013 Emilio López
>> + *
>> + * Emilio López <[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-provider.h>
>> +#include <linux/clkdev.h>
>> +#include <linux/of.h>
>> +#include <linux/of_address.h>
>> +#include <linux/reset-controller.h>
>> +#include <linux/spinlock.h>
>> +
>> +
>> +/**
>> + * sunxi_usb_reset... - reset bits in usb clk registers handling
>> + */
>> +
>> +struct usb_reset_data {
>> + void __iomem *reg;
>> + spinlock_t *lock;
>> + struct reset_controller_dev rcdev;
>> +};
>> +
>> +static int sunxi_usb_reset_assert(struct reset_controller_dev *rcdev,
>> + unsigned long id)
>> +{
>> + struct usb_reset_data *data = container_of(rcdev,
>> + struct usb_reset_data,
>> + rcdev);
>> + unsigned long flags;
>> + u32 reg;
>> +
>> + spin_lock_irqsave(data->lock, flags);
>> +
>> + reg = readl(data->reg);
>> + writel(reg & ~BIT(id), data->reg);
>> +
>> + spin_unlock_irqrestore(data->lock, flags);
>> +
>> + return 0;
>> +}
>> +
>> +static int sunxi_usb_reset_deassert(struct reset_controller_dev *rcdev,
>> + unsigned long id)
>> +{
>> + struct usb_reset_data *data = container_of(rcdev,
>> + struct usb_reset_data,
>> + rcdev);
>> + unsigned long flags;
>> + u32 reg;
>> +
>> + spin_lock_irqsave(data->lock, flags);
>> +
>> + reg = readl(data->reg);
>> + writel(reg | BIT(id), data->reg);
>> +
>> + spin_unlock_irqrestore(data->lock, flags);
>> +
>> + return 0;
>> +}
>> +
>> +static struct reset_control_ops sunxi_usb_reset_ops = {
>> + .assert = sunxi_usb_reset_assert,
>> + .deassert = sunxi_usb_reset_deassert,
>> +};
>> +
>> +/**
>> + * sunxi_usb_clk_setup() - Setup function for usb gate clocks
>> + */
>> +
>> +#define SUNXI_USB_MAX_SIZE 32
>> +
>> +struct usb_clk_data {
>> + u32 clk_mask;
>> + u32 reset_mask;
>> + /* which parent to use, should match clock-output-names */
>> + char parents[SUNXI_USB_MAX_SIZE];
>> +};
>> +
>> +static void __init sunxi_usb_clk_setup(struct device_node *node,
>> + const struct usb_clk_data *data,
>> + spinlock_t *lock)
>> +{
>> + struct clk_onecell_data *clk_data;
>> + struct usb_reset_data *reset_data;
>> + const char *clk_parent;
>> + const char *clk_name;
>> + void __iomem *reg;
>> + int qty;
>> + int i = 0;
>> + int j = 0;
>> +
>> + reg = of_iomap(node, 0);
>
> of_io_request_and_map?

OK. About that, any recommended naming style for the 3rd argument?
Maybe the driver name "clk_sun9i_usb"? Or just a generic name like
"usb_clk"?

I'm asking now as we'll likely be changing the existing drivers to
use it as well.

>> +
>> + /* Worst-case size approximation and memory allocation */
>> + qty = find_last_bit((unsigned long *)&data->clk_mask,
>> + SUNXI_USB_MAX_SIZE);
>> + clk_data = kmalloc(sizeof(struct clk_onecell_data), GFP_KERNEL);
>> + if (!clk_data)
>> + return;
>> + clk_data->clks = kzalloc((qty+1) * sizeof(struct clk *), GFP_KERNEL);
>> + if (!clk_data->clks) {
>> + kfree(clk_data);
>> + return;
>> + }
>> +
>> + for_each_set_bit(i, (unsigned long *)&data->clk_mask,
>> + SUNXI_USB_MAX_SIZE) {
>> + of_property_read_string_index(node, "clock-output-names",
>> + j, &clk_name);
>> + clk_parent = of_clk_get_parent_name(node, data->parents[j]);
>> +
>> + clk_data->clks[i] = clk_register_gate(NULL, clk_name,
>> + clk_parent, 0,
>> + reg, i, 0, lock);
>> + WARN_ON(IS_ERR(clk_data->clks[i]));
>> + clk_register_clkdev(clk_data->clks[i], clk_name, NULL);
>> +
>> + j++;
>> + }
>> +
>> + /* Adjust to the real max */
>> + clk_data->clk_num = i;
>> +
>> + of_clk_add_provider(node, of_clk_src_onecell_get, clk_data);
>> +
>> + /* Register a reset controller for usb with reset bits */
>> + if (data->reset_mask == 0)
>> + return;
>> +
>> + reset_data = kzalloc(sizeof(*reset_data), GFP_KERNEL);
>> + if (!reset_data)
>> + return;
>> +
>> + reset_data->reg = reg;
>> + reset_data->lock = lock;
>> + reset_data->rcdev.nr_resets = __fls(data->reset_mask) + 1;
>> + reset_data->rcdev.ops = &sunxi_usb_reset_ops;
>> + reset_data->rcdev.of_node = node;
>> + reset_controller_register(&reset_data->rcdev);
>> +}
>> +
>> +static const struct usb_clk_data sun9i_a80_usb_mod_data __initconst = {
>> + .clk_mask = BIT(6) | BIT(5) | BIT(4) | BIT(3) | BIT(2) | BIT(1),
>> + .reset_mask = BIT(19) | BIT(18) | BIT(17),
>> + .parents = {0, 1, 0, 1, 0, 1},
>> +};
>> +
>> +static DEFINE_SPINLOCK(a80_usb_mod_lock);
>> +
>> +static void __init sun9i_a80_usb_mod_setup(struct device_node *node)
>> +{
>> + /* AHB1 gate must be enabled to access registers */
>> + struct clk *ahb = of_clk_get(node, 0);
>> +
>> + WARN_ON(IS_ERR(ahb));
>> + clk_prepare_enable(ahb);
>
> Hmmmm. That look off.
>
> Why do you need the clock to be enabled all the time? Isn't the CCF
> already taking care of enabling the parent clock whenever it needs to
> access any register?

There are also resets in the same block. That and I couldn't get it
working without enabling the clock beforehand.


ChenYu

2014-11-05 10:11:05

by Maxime Ripard

[permalink] [raw]
Subject: Re: [PATCH 1/6] clk: sunxi: Add support for sun9i a80 usb clocks and resets

On Wed, Nov 05, 2014 at 06:02:35PM +0800, Chen-Yu Tsai wrote:
> >> +static void __init sunxi_usb_clk_setup(struct device_node *node,
> >> + const struct usb_clk_data *data,
> >> + spinlock_t *lock)
> >> +{
> >> + struct clk_onecell_data *clk_data;
> >> + struct usb_reset_data *reset_data;
> >> + const char *clk_parent;
> >> + const char *clk_name;
> >> + void __iomem *reg;
> >> + int qty;
> >> + int i = 0;
> >> + int j = 0;
> >> +
> >> + reg = of_iomap(node, 0);
> >
> > of_io_request_and_map?
>
> OK. About that, any recommended naming style for the 3rd argument?
> Maybe the driver name "clk_sun9i_usb"? Or just a generic name like
> "usb_clk"?
>
> I'm asking now as we'll likely be changing the existing drivers to
> use it as well.

I don't really have a preference. Maybe the DT node name would be both
the easier and better solution.

[...]

> >> +static void __init sun9i_a80_usb_mod_setup(struct device_node *node)
> >> +{
> >> + /* AHB1 gate must be enabled to access registers */
> >> + struct clk *ahb = of_clk_get(node, 0);
> >> +
> >> + WARN_ON(IS_ERR(ahb));
> >> + clk_prepare_enable(ahb);
> >
> > Hmmmm. That look off.
> >
> > Why do you need the clock to be enabled all the time? Isn't the CCF
> > already taking care of enabling the parent clock whenever it needs to
> > access any register?
>
> There are also resets in the same block. That and I couldn't get it
> working without enabling the clock beforehand.

Ah, right.

What happens if you just enable and disable the clocks in the
reset_assert and reset_deassert right before and after accessing the
registers?

Maxime

--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com


Attachments:
(No filename) (1.79 kB)
signature.asc (819.00 B)
Digital signature
Download all attachments

2014-11-06 02:09:49

by Chen-Yu Tsai

[permalink] [raw]
Subject: Re: [PATCH 1/6] clk: sunxi: Add support for sun9i a80 usb clocks and resets

On Wed, Nov 5, 2014 at 6:09 PM, Maxime Ripard
<[email protected]> wrote:
> On Wed, Nov 05, 2014 at 06:02:35PM +0800, Chen-Yu Tsai wrote:
>> >> +static void __init sunxi_usb_clk_setup(struct device_node *node,
>> >> + const struct usb_clk_data *data,
>> >> + spinlock_t *lock)
>> >> +{
>> >> + struct clk_onecell_data *clk_data;
>> >> + struct usb_reset_data *reset_data;
>> >> + const char *clk_parent;
>> >> + const char *clk_name;
>> >> + void __iomem *reg;
>> >> + int qty;
>> >> + int i = 0;
>> >> + int j = 0;
>> >> +
>> >> + reg = of_iomap(node, 0);
>> >
>> > of_io_request_and_map?
>>
>> OK. About that, any recommended naming style for the 3rd argument?
>> Maybe the driver name "clk_sun9i_usb"? Or just a generic name like
>> "usb_clk"?
>>
>> I'm asking now as we'll likely be changing the existing drivers to
>> use it as well.
>
> I don't really have a preference. Maybe the DT node name would be both
> the easier and better solution.

Using of_node_full_name() then.

> [...]
>
>> >> +static void __init sun9i_a80_usb_mod_setup(struct device_node *node)
>> >> +{
>> >> + /* AHB1 gate must be enabled to access registers */
>> >> + struct clk *ahb = of_clk_get(node, 0);
>> >> +
>> >> + WARN_ON(IS_ERR(ahb));
>> >> + clk_prepare_enable(ahb);
>> >
>> > Hmmmm. That look off.
>> >
>> > Why do you need the clock to be enabled all the time? Isn't the CCF
>> > already taking care of enabling the parent clock whenever it needs to
>> > access any register?
>>
>> There are also resets in the same block. That and I couldn't get it
>> working without enabling the clock beforehand.
>
> Ah, right.
>
> What happens if you just enable and disable the clocks in the
> reset_assert and reset_deassert right before and after accessing the
> registers?

That doesn't work either. I forgot to mention that most of the clock
gates have the peripheral pll as their parent, not the ahb clock gate.

Since most of the clocks are special clocks @ 12M, 48M, or 480M, which
on previous SoCs were driven by PLL6 or another special PLL, it seems
to make sense to assume the same on A80 and set them this way.

ChenYu

2014-11-06 08:55:08

by Maxime Ripard

[permalink] [raw]
Subject: Re: [PATCH 1/6] clk: sunxi: Add support for sun9i a80 usb clocks and resets

On Thu, Nov 06, 2014 at 10:09:27AM +0800, Chen-Yu Tsai wrote:
> >> >> +static void __init sun9i_a80_usb_mod_setup(struct device_node *node)
> >> >> +{
> >> >> + /* AHB1 gate must be enabled to access registers */
> >> >> + struct clk *ahb = of_clk_get(node, 0);
> >> >> +
> >> >> + WARN_ON(IS_ERR(ahb));
> >> >> + clk_prepare_enable(ahb);
> >> >
> >> > Hmmmm. That look off.
> >> >
> >> > Why do you need the clock to be enabled all the time? Isn't the CCF
> >> > already taking care of enabling the parent clock whenever it needs to
> >> > access any register?
> >>
> >> There are also resets in the same block. That and I couldn't get it
> >> working without enabling the clock beforehand.
> >
> > Ah, right.
> >
> > What happens if you just enable and disable the clocks in the
> > reset_assert and reset_deassert right before and after accessing the
> > registers?
>
> That doesn't work either. I forgot to mention that most of the clock
> gates have the peripheral pll as their parent, not the ahb clock gate.

Why it doesn't work? The clock needs more time to stabilize? The reset
line is set back in reset if the clocks are disabled?

Maxime

--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com


Attachments:
(No filename) (1.24 kB)
signature.asc (819.00 B)
Digital signature
Download all attachments

2014-11-06 09:19:50

by Chen-Yu Tsai

[permalink] [raw]
Subject: Re: [PATCH 1/6] clk: sunxi: Add support for sun9i a80 usb clocks and resets

On Thu, Nov 6, 2014 at 4:54 PM, Maxime Ripard
<[email protected]> wrote:
> On Thu, Nov 06, 2014 at 10:09:27AM +0800, Chen-Yu Tsai wrote:
>> >> >> +static void __init sun9i_a80_usb_mod_setup(struct device_node *node)
>> >> >> +{
>> >> >> + /* AHB1 gate must be enabled to access registers */
>> >> >> + struct clk *ahb = of_clk_get(node, 0);
>> >> >> +
>> >> >> + WARN_ON(IS_ERR(ahb));
>> >> >> + clk_prepare_enable(ahb);
>> >> >
>> >> > Hmmmm. That look off.
>> >> >
>> >> > Why do you need the clock to be enabled all the time? Isn't the CCF
>> >> > already taking care of enabling the parent clock whenever it needs to
>> >> > access any register?
>> >>
>> >> There are also resets in the same block. That and I couldn't get it
>> >> working without enabling the clock beforehand.
>> >
>> > Ah, right.
>> >
>> > What happens if you just enable and disable the clocks in the
>> > reset_assert and reset_deassert right before and after accessing the
>> > registers?
>>
>> That doesn't work either. I forgot to mention that most of the clock
>> gates have the peripheral pll as their parent, not the ahb clock gate.
>
> Why it doesn't work? The clock needs more time to stabilize? The reset
> line is set back in reset if the clocks are disabled?

Let me clarify, what you proposed will work for the resets.

However the clock gates won't work if we use the generic clk-gate driver.
The problem is most of the gates don't have the ahb gate as their parent,
but pll4 (peripheral pll). When we enable the clock, the ahb gate isn't
its parent, and doesn't get enabled as a result. This is especially true
for the usb phy clocks: all of them use pll4 as their parent.

I think this is a better representation of the hardware, but without
documents this is really just a guess.

As a whole, I think enabling the clock gate at the beginning is simpler.

ChenYu

2014-11-14 08:40:05

by Maxime Ripard

[permalink] [raw]
Subject: Re: [PATCH 1/6] clk: sunxi: Add support for sun9i a80 usb clocks and resets

Hi,

Sorry for the belated answer.

On Thu, Nov 06, 2014 at 05:19:24PM +0800, Chen-Yu Tsai wrote:
> On Thu, Nov 6, 2014 at 4:54 PM, Maxime Ripard
> <[email protected]> wrote:
> > On Thu, Nov 06, 2014 at 10:09:27AM +0800, Chen-Yu Tsai wrote:
> >> >> >> +static void __init sun9i_a80_usb_mod_setup(struct device_node *node)
> >> >> >> +{
> >> >> >> + /* AHB1 gate must be enabled to access registers */
> >> >> >> + struct clk *ahb = of_clk_get(node, 0);
> >> >> >> +
> >> >> >> + WARN_ON(IS_ERR(ahb));
> >> >> >> + clk_prepare_enable(ahb);
> >> >> >
> >> >> > Hmmmm. That look off.
> >> >> >
> >> >> > Why do you need the clock to be enabled all the time? Isn't the CCF
> >> >> > already taking care of enabling the parent clock whenever it needs to
> >> >> > access any register?
> >> >>
> >> >> There are also resets in the same block. That and I couldn't get it
> >> >> working without enabling the clock beforehand.
> >> >
> >> > Ah, right.
> >> >
> >> > What happens if you just enable and disable the clocks in the
> >> > reset_assert and reset_deassert right before and after accessing the
> >> > registers?
> >>
> >> That doesn't work either. I forgot to mention that most of the clock
> >> gates have the peripheral pll as their parent, not the ahb clock gate.
> >
> > Why it doesn't work? The clock needs more time to stabilize? The reset
> > line is set back in reset if the clocks are disabled?
>
> Let me clarify, what you proposed will work for the resets.
>
> However the clock gates won't work if we use the generic clk-gate driver.
> The problem is most of the gates don't have the ahb gate as their parent,
> but pll4 (peripheral pll). When we enable the clock, the ahb gate isn't
> its parent, and doesn't get enabled as a result. This is especially true
> for the usb phy clocks: all of them use pll4 as their parent.

I'm not sure I get this right. You mean that this USB clock needs
*both* pll4 and its AHB gates to be enabled in order to run properly?

Or that the PHY needs its AHB gate to be enabled?

Both ways, I still don't think it's the right thing to do.

Maxime

--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com


Attachments:
(No filename) (2.18 kB)
signature.asc (819.00 B)
Digital signature
Download all attachments

2014-11-14 19:59:16

by Chen-Yu Tsai

[permalink] [raw]
Subject: Re: [linux-sunxi] Re: [PATCH 1/6] clk: sunxi: Add support for sun9i a80 usb clocks and resets

Hi,

On Fri, Nov 14, 2014 at 4:39 PM, Maxime Ripard
<[email protected]> wrote:
> Hi,
>
> Sorry for the belated answer.
>
> On Thu, Nov 06, 2014 at 05:19:24PM +0800, Chen-Yu Tsai wrote:
>> On Thu, Nov 6, 2014 at 4:54 PM, Maxime Ripard
>> <[email protected]> wrote:
>> > On Thu, Nov 06, 2014 at 10:09:27AM +0800, Chen-Yu Tsai wrote:
>> >> >> >> +static void __init sun9i_a80_usb_mod_setup(struct device_node *node)
>> >> >> >> +{
>> >> >> >> + /* AHB1 gate must be enabled to access registers */
>> >> >> >> + struct clk *ahb = of_clk_get(node, 0);
>> >> >> >> +
>> >> >> >> + WARN_ON(IS_ERR(ahb));
>> >> >> >> + clk_prepare_enable(ahb);
>> >> >> >
>> >> >> > Hmmmm. That look off.
>> >> >> >
>> >> >> > Why do you need the clock to be enabled all the time? Isn't the CCF
>> >> >> > already taking care of enabling the parent clock whenever it needs to
>> >> >> > access any register?
>> >> >>
>> >> >> There are also resets in the same block. That and I couldn't get it
>> >> >> working without enabling the clock beforehand.
>> >> >
>> >> > Ah, right.
>> >> >
>> >> > What happens if you just enable and disable the clocks in the
>> >> > reset_assert and reset_deassert right before and after accessing the
>> >> > registers?
>> >>
>> >> That doesn't work either. I forgot to mention that most of the clock
>> >> gates have the peripheral pll as their parent, not the ahb clock gate.
>> >
>> > Why it doesn't work? The clock needs more time to stabilize? The reset
>> > line is set back in reset if the clocks are disabled?
>>
>> Let me clarify, what you proposed will work for the resets.
>>
>> However the clock gates won't work if we use the generic clk-gate driver.
>> The problem is most of the gates don't have the ahb gate as their parent,
>> but pll4 (peripheral pll). When we enable the clock, the ahb gate isn't
>> its parent, and doesn't get enabled as a result. This is especially true
>> for the usb phy clocks: all of them use pll4 as their parent.
>
> I'm not sure I get this right. You mean that this USB clock needs
> *both* pll4 and its AHB gates to be enabled in order to run properly?
>
> Or that the PHY needs its AHB gate to be enabled?

What I meant was the USB clocks should have some backing PLL.
But let's just use the AHB gate for all the parents and keep
it simple for now. We can model it accurately after we get
documentation.

> Both ways, I still don't think it's the right thing to do.

As said above, I'll just use ahb gate as the parent, and do
what you said earlier about the reset controls (toggle the
gate before and after touching the register).


ChenYu