Patches to add support for Rockchip usb phys.Add a new Rockchip
usb phy driver and modify dwc2 controller driver to make dwc2
platform devices support a generic PHY framework driver. This
patch set has been tested on my rk3288-evb and power off the usb
phys would reduce about 60mW power budget in total during sustem
suspend.
Changes in v6:
- Rename SIDDQ_MSK to SIDDQ_WRITE_ENA.
- Use phandle args to find a phy struct directly.
Changes in v5:
- Adjust entry order of example devicetree node in document.
- reorder the phy dt node to a correct position.
Changes in v4:
- Get number of PHYs from device tree.
- Model each PHY as subnode of the phy provider node.
- Updata description for phy device tree subnode.
- Add phy subnodes.
Changes in v3:
- Use BIT macro instead of bit shift ops.
- Rename the config entry to PHY_ROCKCHIP_USB.
- Fix coding style: both branches of the if() which only one
branch of the conditional statement is a single statement
should have braces.
- No need to test dwc2->phy for NULL before calling generic phy
APIs.
Yunzhi Li (5):
phy: add a driver for the Rockchip SoC internal USB2.0 PHY
Documentation: bindings: add dt documentation for Rockchip usb PHY
usb: dwc2: add generic PHY framework support for dwc2 usb controler
platform driver.
ARM: dts: rockchip: add rk3288 usb PHY
ARM: dts: rockchip: Enable usb PHY on rk3288-evb board
.../devicetree/bindings/phy/rockchip-usb-phy.txt | 32 ++++
arch/arm/boot/dts/rk3288-evb.dtsi | 4 +
arch/arm/boot/dts/rk3288.dtsi | 27 +++
drivers/phy/Kconfig | 7 +
drivers/phy/Makefile | 1 +
drivers/phy/phy-rockchip-usb.c | 198 +++++++++++++++++++++
drivers/usb/dwc2/gadget.c | 33 ++--
drivers/usb/dwc2/platform.c | 36 +++-
8 files changed, 315 insertions(+), 23 deletions(-)
create mode 100644 Documentation/devicetree/bindings/phy/rockchip-usb-phy.txt
create mode 100644 drivers/phy/phy-rockchip-usb.c
--
2.0.0
This patch to add a generic PHY driver for ROCKCHIP usb PHYs,
currently this driver can support RK3288. The RK3288 SoC have
three independent USB PHY IPs which are all configured through a
set of registers located in the GRF (general register files)
module.
Signed-off-by: Yunzhi Li <[email protected]>
---
Changes in v6:
- Rename SIDDQ_MSK to SIDDQ_WRITE_ENA.
- Use phandle args to find a phy struct directly.
Changes in v5: None
Changes in v4:
- Get number of PHYs from device tree.
- Model each PHY as subnode of the phy provider node.
Changes in v3:
- Use BIT macro instead of bit shift ops.
- Rename the config entry to PHY_ROCKCHIP_USB.
drivers/phy/Kconfig | 7 ++
drivers/phy/Makefile | 1 +
drivers/phy/phy-rockchip-usb.c | 198 +++++++++++++++++++++++++++++++++++++++++
3 files changed, 206 insertions(+)
create mode 100644 drivers/phy/phy-rockchip-usb.c
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index ccad880..b24500a 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -239,6 +239,13 @@ config PHY_QCOM_IPQ806X_SATA
depends on OF
select GENERIC_PHY
+config PHY_ROCKCHIP_USB
+ tristate "Rockchip USB2 PHY Driver"
+ depends on ARCH_ROCKCHIP && OF
+ select GENERIC_PHY
+ help
+ Enable this to support the Rockchip USB 2.0 PHY.
+
config PHY_ST_SPEAR1310_MIPHY
tristate "ST SPEAR1310-MIPHY driver"
select GENERIC_PHY
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index aa74f96..48bf5a1 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -28,6 +28,7 @@ phy-exynos-usb2-$(CONFIG_PHY_EXYNOS5250_USB2) += phy-exynos5250-usb2.o
phy-exynos-usb2-$(CONFIG_PHY_S5PV210_USB2) += phy-s5pv210-usb2.o
obj-$(CONFIG_PHY_EXYNOS5_USBDRD) += phy-exynos5-usbdrd.o
obj-$(CONFIG_PHY_QCOM_APQ8064_SATA) += phy-qcom-apq8064-sata.o
+obj-$(CONFIG_PHY_ROCKCHIP_USB) += phy-rockchip-usb.o
obj-$(CONFIG_PHY_QCOM_IPQ806X_SATA) += phy-qcom-ipq806x-sata.o
obj-$(CONFIG_PHY_ST_SPEAR1310_MIPHY) += phy-spear1310-miphy.o
obj-$(CONFIG_PHY_ST_SPEAR1340_MIPHY) += phy-spear1340-miphy.o
diff --git a/drivers/phy/phy-rockchip-usb.c b/drivers/phy/phy-rockchip-usb.c
new file mode 100644
index 0000000..dad5194
--- /dev/null
+++ b/drivers/phy/phy-rockchip-usb.c
@@ -0,0 +1,198 @@
+/*
+ * Rockchip usb PHY driver
+ *
+ * Copyright (C) 2014 Yunzhi Li <[email protected]>
+ * Copyright (C) 2014 ROCKCHIP, Inc.
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/clk.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/consumer.h>
+#include <linux/reset.h>
+#include <linux/regmap.h>
+#include <linux/mfd/syscon.h>
+
+#define ROCKCHIP_RK3288_UOC(n) (0x320 + n * 0x14)
+
+/*
+ * The higher 16-bit of this register is used for write protection
+ * only if BIT(13 + 16) set to 1 the BIT(13) can be written.
+ */
+#define SIDDQ_WRITE_ENA BIT(29)
+#define SIDDQ_ON BIT(13)
+#define SIDDQ_OFF (0 << 13)
+
+struct rockchip_usb_phy {
+ struct regmap *reg_base;
+ unsigned int reg_offset;
+ struct clk *clk;
+ struct phy *phy;
+};
+
+struct rockchip_usb_phy_priv {
+ struct rockchip_usb_phy *phys;
+ unsigned nphys;
+};
+
+static int rockchip_usb_phy_power(struct rockchip_usb_phy *phy,
+ bool siddq)
+{
+ return regmap_write(phy->reg_base, phy->reg_offset,
+ SIDDQ_WRITE_ENA | (siddq ? SIDDQ_ON : SIDDQ_OFF));
+}
+
+static int rockchip_usb_phy_power_off(struct phy *_phy)
+{
+ struct rockchip_usb_phy *phy = phy_get_drvdata(_phy);
+ int ret = 0;
+
+ /* Power down usb phy analog blocks by set siddq 1 */
+ ret = rockchip_usb_phy_power(phy, 1);
+ if (ret)
+ return ret;
+
+ clk_disable_unprepare(phy->clk);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+static int rockchip_usb_phy_power_on(struct phy *_phy)
+{
+ struct rockchip_usb_phy *phy = phy_get_drvdata(_phy);
+ int ret = 0;
+
+ ret = clk_prepare_enable(phy->clk);
+ if (ret)
+ return ret;
+
+ /* Power up usb phy analog blocks by set siddq 0 */
+ ret = rockchip_usb_phy_power(phy, 0);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+static struct phy *rockchip_usb_phy_xlate(struct device *dev,
+ struct of_phandle_args *args)
+{
+ struct rockchip_usb_phy_priv *priv = dev_get_drvdata(dev);
+ unsigned int phy_id = args->args[0];
+
+ if (WARN_ON(phy_id < 0 || phy_id >= priv->nphys))
+ return ERR_PTR(-ENODEV);
+
+ return priv->phys[phy_id].phy;
+}
+
+static struct phy_ops ops = {
+ .power_on = rockchip_usb_phy_power_on,
+ .power_off = rockchip_usb_phy_power_off,
+ .owner = THIS_MODULE,
+};
+
+static int rockchip_usb_phy_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct rockchip_usb_phy *rk_phy;
+ struct rockchip_usb_phy_priv *priv;
+ struct phy_provider *phy_provider;
+ struct device_node *child;
+ struct regmap *grf;
+ unsigned int phy_id;
+
+ grf = syscon_regmap_lookup_by_phandle(dev->of_node, "rockchip,grf");
+ if (IS_ERR(grf)) {
+ dev_err(&pdev->dev, "Missing rockchip,grf property\n");
+ return PTR_ERR(grf);
+ }
+
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ /* Get number of phys from device tree */
+ priv->nphys = of_get_child_count(dev->of_node);
+ if (priv->nphys == 0)
+ return -ENODEV;
+
+ priv->phys = devm_kzalloc(dev, priv->nphys * sizeof(*priv->phys),
+ GFP_KERNEL);
+ if (!priv->phys)
+ return -ENOMEM;
+
+ for_each_available_child_of_node(dev->of_node, child) {
+ if (of_property_read_u32(child, "reg", &phy_id)) {
+ dev_err(dev, "missing reg property in node %s\n",
+ child->name);
+ return -EINVAL;
+ }
+
+ if (phy_id < 0 || phy_id >= priv->nphys) {
+ dev_err(dev, "invalid phy id\n");
+ return -EINVAL;
+ }
+ rk_phy = &priv->phys[phy_id];
+ rk_phy->reg_offset = ROCKCHIP_RK3288_UOC(phy_id);
+ rk_phy->reg_base = grf;
+
+ rk_phy->clk = of_clk_get(child, 0);
+ if (IS_ERR(rk_phy->clk)) {
+ dev_warn(dev, "failed to get clock\n");
+ rk_phy->clk = NULL;
+ }
+
+ rk_phy->phy = devm_phy_create(dev, NULL, &ops);
+ if (IS_ERR(rk_phy->phy)) {
+ dev_err(dev, "failed to create PHY %d\n", phy_id);
+ return PTR_ERR(rk_phy->phy);
+ }
+ phy_set_drvdata(rk_phy->phy, rk_phy);
+ }
+
+ platform_set_drvdata(pdev, priv);
+
+ phy_provider = devm_of_phy_provider_register(dev,
+ rockchip_usb_phy_xlate);
+ return PTR_ERR_OR_ZERO(phy_provider);
+}
+
+static const struct of_device_id rockchip_usb_phy_dt_ids[] = {
+ { .compatible = "rockchip,rk3288-usb-phy" },
+ {}
+};
+
+MODULE_DEVICE_TABLE(of, rockchip_usb_phy_dt_ids);
+
+static struct platform_driver rockchip_usb_driver = {
+ .probe = rockchip_usb_phy_probe,
+ .driver = {
+ .name = "rockchip-usb-phy",
+ .owner = THIS_MODULE,
+ .of_match_table = rockchip_usb_phy_dt_ids,
+ },
+};
+
+module_platform_driver(rockchip_usb_driver);
+
+MODULE_AUTHOR("Yunzhi Li <[email protected]>");
+MODULE_DESCRIPTION("Rockchip USB 2.0 PHY driver");
+MODULE_LICENSE("GPL v2");
--
2.0.0
This patch adds a binding that describes the Rockchip usb PHYs
found on Rockchip SoCs usb interface.
Signed-off-by: Yunzhi Li <[email protected]>
---
Changes in v6: None
Changes in v5:
- Adjust entry order of example devicetree node in document.
Changes in v4:
- Updata description for phy device tree subnode.
Changes in v3: None
.../devicetree/bindings/phy/rockchip-usb-phy.txt | 32 ++++++++++++++++++++++
1 file changed, 32 insertions(+)
create mode 100644 Documentation/devicetree/bindings/phy/rockchip-usb-phy.txt
diff --git a/Documentation/devicetree/bindings/phy/rockchip-usb-phy.txt b/Documentation/devicetree/bindings/phy/rockchip-usb-phy.txt
new file mode 100644
index 0000000..e9500d9
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/rockchip-usb-phy.txt
@@ -0,0 +1,32 @@
+ROCKCHIP USB2 PHY
+
+Required properties:
+ - compatible: rockchip,rk3288-usb-phy
+ - rockchip,grf : phandle to the syscon managing the "general
+ register files"
+ - #phy-cells: should be 1
+ - #address-cells: should be 1
+ - #size-cells: should be 0
+
+Sub-nodes:
+Each PHY should be represented as a sub-node.
+
+Sub-nodes
+required properties:
+- reg: the PHY number
+ "0" - PHY connect to OTG controller
+ "1" - PHY connect to HOST0 controller
+ "2" - PHY connect to HOST1 controller
+
+Optional Properties:
+- clocks : phandle + clock specifier for the phy clocks
+
+Example:
+
+usbphy: phy {
+ compatible = "rockchip,rk3288-usb-phy";
+ rockchip,grf = <&grf>;
+ #phy-cells = <1>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+};
--
2.0.0
Get PHY parameters from devicetree and power off usb PHY during
system suspend.
Signed-off-by: Yunzhi Li <[email protected]>
Acked-by: Paul Zimmerman <[email protected]>
---
Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3:
- Fix coding style: both branches of the if() which only one
branch of the conditional statement is a single statement
should have braces.
- No need to test dwc2->phy for NULL before calling generic phy
APIs.
drivers/usb/dwc2/gadget.c | 33 ++++++++++++---------------------
drivers/usb/dwc2/platform.c | 36 ++++++++++++++++++++++++++++++++++--
2 files changed, 46 insertions(+), 23 deletions(-)
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 200168e..2601c61 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -3410,8 +3410,6 @@ int dwc2_gadget_init(struct dwc2_hsotg *hsotg, int irq)
{
struct device *dev = hsotg->dev;
struct s3c_hsotg_plat *plat = dev->platform_data;
- struct phy *phy;
- struct usb_phy *uphy;
struct s3c_hsotg_ep *eps;
int epnum;
int ret;
@@ -3421,30 +3419,23 @@ int dwc2_gadget_init(struct dwc2_hsotg *hsotg, int irq)
hsotg->phyif = GUSBCFG_PHYIF16;
/*
- * Attempt to find a generic PHY, then look for an old style
- * USB PHY, finally fall back to pdata
+ * If platform probe couldn't find a generic PHY or an old style
+ * USB PHY, fall back to pdata
*/
- phy = devm_phy_get(dev, "usb2-phy");
- if (IS_ERR(phy)) {
- uphy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2);
- if (IS_ERR(uphy)) {
- /* Fallback for pdata */
- plat = dev_get_platdata(dev);
- if (!plat) {
- dev_err(dev,
- "no platform data or transceiver defined\n");
- return -EPROBE_DEFER;
- }
- hsotg->plat = plat;
- } else
- hsotg->uphy = uphy;
- } else {
- hsotg->phy = phy;
+ if (IS_ERR_OR_NULL(hsotg->phy) && IS_ERR_OR_NULL(hsotg->uphy)) {
+ plat = dev_get_platdata(dev);
+ if (!plat) {
+ dev_err(dev,
+ "no platform data or transceiver defined\n");
+ return -EPROBE_DEFER;
+ }
+ hsotg->plat = plat;
+ } else if (hsotg->phy) {
/*
* If using the generic PHY framework, check if the PHY bus
* width is 8-bit and set the phyif appropriately.
*/
- if (phy_get_bus_width(phy) == 8)
+ if (phy_get_bus_width(hsotg->phy) == 8)
hsotg->phyif = GUSBCFG_PHYIF8;
}
diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
index 6a795aa..ae095f0 100644
--- a/drivers/usb/dwc2/platform.c
+++ b/drivers/usb/dwc2/platform.c
@@ -155,6 +155,8 @@ static int dwc2_driver_probe(struct platform_device *dev)
struct dwc2_core_params defparams;
struct dwc2_hsotg *hsotg;
struct resource *res;
+ struct phy *phy;
+ struct usb_phy *uphy;
int retval;
int irq;
@@ -212,6 +214,24 @@ static int dwc2_driver_probe(struct platform_device *dev)
hsotg->dr_mode = of_usb_get_dr_mode(dev->dev.of_node);
+ /*
+ * Attempt to find a generic PHY, then look for an old style
+ * USB PHY
+ */
+ phy = devm_phy_get(&dev->dev, "usb2-phy");
+ if (IS_ERR(phy)) {
+ hsotg->phy = NULL;
+ uphy = devm_usb_get_phy(&dev->dev, USB_PHY_TYPE_USB2);
+ if (IS_ERR(uphy))
+ hsotg->uphy = NULL;
+ else
+ hsotg->uphy = uphy;
+ } else {
+ hsotg->phy = phy;
+ phy_power_on(hsotg->phy);
+ phy_init(hsotg->phy);
+ }
+
spin_lock_init(&hsotg->lock);
mutex_init(&hsotg->init_mutex);
retval = dwc2_gadget_init(hsotg, irq);
@@ -231,8 +251,15 @@ static int __maybe_unused dwc2_suspend(struct device *dev)
struct dwc2_hsotg *dwc2 = dev_get_drvdata(dev);
int ret = 0;
- if (dwc2_is_device_mode(dwc2))
+ if (dwc2_is_device_mode(dwc2)) {
ret = s3c_hsotg_suspend(dwc2);
+ } else {
+ if (dwc2->lx_state == DWC2_L0)
+ return 0;
+ phy_exit(dwc2->phy);
+ phy_power_off(dwc2->phy);
+
+ }
return ret;
}
@@ -241,8 +268,13 @@ static int __maybe_unused dwc2_resume(struct device *dev)
struct dwc2_hsotg *dwc2 = dev_get_drvdata(dev);
int ret = 0;
- if (dwc2_is_device_mode(dwc2))
+ if (dwc2_is_device_mode(dwc2)) {
ret = s3c_hsotg_resume(dwc2);
+ } else {
+ phy_power_on(dwc2->phy);
+ phy_init(dwc2->phy);
+
+ }
return ret;
}
--
2.0.0
This patch adds a device_node for RK3288 SoC usb phy. It also
defines the phy to be used by three usb controllers: usb_host0/1
and usb_otg.
Signed-off-by: Yunzhi Li <[email protected]>
---
Changes in v6: None
Changes in v5:
- reorder the phy dt node to a correct position.
Changes in v4:
- Add phy subnodes.
Changes in v3: None
arch/arm/boot/dts/rk3288.dtsi | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi
index 874e66d..bd2a1e0 100644
--- a/arch/arm/boot/dts/rk3288.dtsi
+++ b/arch/arm/boot/dts/rk3288.dtsi
@@ -335,6 +335,8 @@
interrupts = <GIC_SPI 24 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&cru HCLK_USBHOST0>;
clock-names = "usbhost";
+ phys = <&usbphy 1>;
+ phy-names = "usb";
status = "disabled";
};
@@ -347,6 +349,8 @@
interrupts = <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&cru HCLK_USBHOST1>;
clock-names = "otg";
+ phys = <&usbphy 2>;
+ phy-names = "usb2-phy";
status = "disabled";
};
@@ -357,6 +361,8 @@
interrupts = <GIC_SPI 23 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&cru HCLK_OTG0>;
clock-names = "otg";
+ phys = <&usbphy 0>;
+ phy-names = "usb2-phy";
status = "disabled";
};
@@ -497,6 +503,27 @@
interrupts = <GIC_PPI 9 0xf04>;
};
+ usbphy: phy {
+ compatible = "rockchip,rk3288-usb-phy";
+ rockchip,grf = <&grf>;
+ #phy-cells = <1>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ status = "disabled";
+
+ usb-phy@0 {
+ reg = <0>;
+ };
+
+ usb-phy@1 {
+ reg = <1>;
+ };
+
+ usb-phy@2 {
+ reg = <2>;
+ };
+ };
+
pinctrl: pinctrl {
compatible = "rockchip,rk3288-pinctrl";
rockchip,grf = <&grf>;
--
2.0.0
Enable usb PHY for all usb ports on rk3288-evb.
Signed-off-by: Yunzhi Li <[email protected]>
---
Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3: None
arch/arm/boot/dts/rk3288-evb.dtsi | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/arm/boot/dts/rk3288-evb.dtsi b/arch/arm/boot/dts/rk3288-evb.dtsi
index cb83cea..992f323 100644
--- a/arch/arm/boot/dts/rk3288-evb.dtsi
+++ b/arch/arm/boot/dts/rk3288-evb.dtsi
@@ -174,6 +174,10 @@
};
};
+&usbphy {
+ status = "okay";
+};
+
&usb_host0_ehci {
status = "okay";
};
--
2.0.0
Hi,
On Thursday 11 December 2014 03:25 PM, Yunzhi Li wrote:
> This patch to add a generic PHY driver for ROCKCHIP usb PHYs,
> currently this driver can support RK3288. The RK3288 SoC have
> three independent USB PHY IPs which are all configured through a
> set of registers located in the GRF (general register files)
> module.
>
> Signed-off-by: Yunzhi Li <[email protected]>
>
> ---
>
> Changes in v6:
> - Rename SIDDQ_MSK to SIDDQ_WRITE_ENA.
> - Use phandle args to find a phy struct directly.
>
> Changes in v5: None
> Changes in v4:
> - Get number of PHYs from device tree.
> - Model each PHY as subnode of the phy provider node.
>
> Changes in v3:
> - Use BIT macro instead of bit shift ops.
> - Rename the config entry to PHY_ROCKCHIP_USB.
>
> drivers/phy/Kconfig | 7 ++
> drivers/phy/Makefile | 1 +
> drivers/phy/phy-rockchip-usb.c | 198 +++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 206 insertions(+)
> create mode 100644 drivers/phy/phy-rockchip-usb.c
>
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index ccad880..b24500a 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -239,6 +239,13 @@ config PHY_QCOM_IPQ806X_SATA
> depends on OF
> select GENERIC_PHY
>
> +config PHY_ROCKCHIP_USB
> + tristate "Rockchip USB2 PHY Driver"
> + depends on ARCH_ROCKCHIP && OF
> + select GENERIC_PHY
> + help
> + Enable this to support the Rockchip USB 2.0 PHY.
> +
> config PHY_ST_SPEAR1310_MIPHY
> tristate "ST SPEAR1310-MIPHY driver"
> select GENERIC_PHY
> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> index aa74f96..48bf5a1 100644
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -28,6 +28,7 @@ phy-exynos-usb2-$(CONFIG_PHY_EXYNOS5250_USB2) += phy-exynos5250-usb2.o
> phy-exynos-usb2-$(CONFIG_PHY_S5PV210_USB2) += phy-s5pv210-usb2.o
> obj-$(CONFIG_PHY_EXYNOS5_USBDRD) += phy-exynos5-usbdrd.o
> obj-$(CONFIG_PHY_QCOM_APQ8064_SATA) += phy-qcom-apq8064-sata.o
> +obj-$(CONFIG_PHY_ROCKCHIP_USB) += phy-rockchip-usb.o
> obj-$(CONFIG_PHY_QCOM_IPQ806X_SATA) += phy-qcom-ipq806x-sata.o
> obj-$(CONFIG_PHY_ST_SPEAR1310_MIPHY) += phy-spear1310-miphy.o
> obj-$(CONFIG_PHY_ST_SPEAR1340_MIPHY) += phy-spear1340-miphy.o
> diff --git a/drivers/phy/phy-rockchip-usb.c b/drivers/phy/phy-rockchip-usb.c
> new file mode 100644
> index 0000000..dad5194
> --- /dev/null
> +++ b/drivers/phy/phy-rockchip-usb.c
> @@ -0,0 +1,198 @@
> +/*
> + * Rockchip usb PHY driver
> + *
> + * Copyright (C) 2014 Yunzhi Li <[email protected]>
> + * Copyright (C) 2014 ROCKCHIP, Inc.
> + *
> + * 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.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/reset.h>
> +#include <linux/regmap.h>
> +#include <linux/mfd/syscon.h>
> +
> +#define ROCKCHIP_RK3288_UOC(n) (0x320 + n * 0x14)
> +
> +/*
> + * The higher 16-bit of this register is used for write protection
> + * only if BIT(13 + 16) set to 1 the BIT(13) can be written.
> + */
> +#define SIDDQ_WRITE_ENA BIT(29)
> +#define SIDDQ_ON BIT(13)
> +#define SIDDQ_OFF (0 << 13)
> +
> +struct rockchip_usb_phy {
> + struct regmap *reg_base;
> + unsigned int reg_offset;
> + struct clk *clk;
> + struct phy *phy;
> +};
> +
> +struct rockchip_usb_phy_priv {
> + struct rockchip_usb_phy *phys;
> + unsigned nphys;
> +};
> +
> +static int rockchip_usb_phy_power(struct rockchip_usb_phy *phy,
> + bool siddq)
> +{
> + return regmap_write(phy->reg_base, phy->reg_offset,
> + SIDDQ_WRITE_ENA | (siddq ? SIDDQ_ON : SIDDQ_OFF));
> +}
> +
> +static int rockchip_usb_phy_power_off(struct phy *_phy)
> +{
> + struct rockchip_usb_phy *phy = phy_get_drvdata(_phy);
> + int ret = 0;
> +
> + /* Power down usb phy analog blocks by set siddq 1 */
> + ret = rockchip_usb_phy_power(phy, 1);
> + if (ret)
> + return ret;
> +
> + clk_disable_unprepare(phy->clk);
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +
> +static int rockchip_usb_phy_power_on(struct phy *_phy)
> +{
> + struct rockchip_usb_phy *phy = phy_get_drvdata(_phy);
> + int ret = 0;
> +
> + ret = clk_prepare_enable(phy->clk);
> + if (ret)
> + return ret;
> +
> + /* Power up usb phy analog blocks by set siddq 0 */
> + ret = rockchip_usb_phy_power(phy, 0);
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +
> +static struct phy *rockchip_usb_phy_xlate(struct device *dev,
> + struct of_phandle_args *args)
> +{
> + struct rockchip_usb_phy_priv *priv = dev_get_drvdata(dev);
> + unsigned int phy_id = args->args[0];
> +
> + if (WARN_ON(phy_id < 0 || phy_id >= priv->nphys))
> + return ERR_PTR(-ENODEV);
> +
> + return priv->phys[phy_id].phy;
I didn't mean that. You can get rid of this entire xlate stuff if you use
something like below
phy@xxx {
compatible = "";
phy1:usb_phy {
}
phy2:usb_phy {
};
};
usb@xx {
compatible = "";
phys = <&phy1>; //doesn't need xlate
/* this needs xlate
phys = <&phy 1>;
*/
phy-names = "phy";
};
Thanks
Kishon
Hi Kishon:
On 2014/12/11 18:27, Kishon Vijay Abraham I wrote:
> Hi,
>
> On Thursday 11 December 2014 03:25 PM, Yunzhi Li wrote:
>> +
>> +static struct phy *rockchip_usb_phy_xlate(struct device *dev,
>> + struct of_phandle_args *args)
>> +{
>> + struct rockchip_usb_phy_priv *priv = dev_get_drvdata(dev);
>> + unsigned int phy_id = args->args[0];
>> +
>> + if (WARN_ON(phy_id < 0 || phy_id >= priv->nphys))
>> + return ERR_PTR(-ENODEV);
>> +
>> + return priv->phys[phy_id].phy;
> I didn't mean that. You can get rid of this entire xlate stuff if you use
> something like below
>
> phy@xxx {
> compatible = "";
> phy1:usb_phy {
> }
> phy2:usb_phy {
> };
> };
>
>
> usb@xx {
> compatible = "";
> phys = <&phy1>; //doesn't need xlate
> /* this needs xlate
> phys = <&phy 1>;
> */
> phy-names = "phy";
> };
Thank you so much for your suggestion, but still have a question:
I have to add the #phy-cells property in each phy sub-node, otherwise
devm_get_phy() will fail and I get log info like
"/usb@ff500000: could not get #phy-cells for /phy/usbp-phy1". So can the
#phy-cells property defines in patent node
also valid for it's child nodes like #address-cells ?
---
Yunzhi Li @ rockchip
Yunzhi,
On Thu, Dec 11, 2014 at 1:55 AM, Yunzhi Li <[email protected]> wrote:
> + rk_phy->clk = of_clk_get(child, 0);
> + if (IS_ERR(rk_phy->clk)) {
> + dev_warn(dev, "failed to get clock\n");
> + rk_phy->clk = NULL;
> + }
The device tree bindings don't specify a clock and the "dtsi" added to
rk3288 don't reference a clock. Take that code out and avoid a
warning in the logs at bootup.
...or should there be a clock?
> + rk_phy->phy = devm_phy_create(dev, NULL, &ops);
This has the wrong number of arguments. Even before the change that
added the 4th argument, this is still wrong because "ops" is supposed
to be the 2nd argument, not the 3rd.
...so I'm confused how this compiled for you. I think this ought to be:
rk_phy->phy = devm_phy_create(dev, child, &ops, NULL);
...but please correct me if I'm mistaken!
Kishon,
On Thu, Dec 11, 2014 at 2:27 AM, Kishon Vijay Abraham I <[email protected]> wrote:
> I didn't mean that. You can get rid of this entire xlate stuff if you use
> something like below
>
> phy@xxx {
> compatible = "";
> phy1:usb_phy {
> }
> phy2:usb_phy {
> };
> };
>
>
> usb@xx {
> compatible = "";
> phys = <&phy1>; //doesn't need xlate
> /* this needs xlate
> phys = <&phy 1>;
> */
> phy-names = "phy";
> };
Is the syntax you proposed really better? Are you saying that you
advocate never using "#phy-cells" other than 0 for new bindings? Is
that your own personal preference, or is there a discussion somewhere
where everyone agreed on this?
My vote is that since "phy-cells" exists and is part of the generic
phy bindings that it's meant to be used whenever you have a single PHY
driver that controls multiple PHYs.
-Doug
Yunzhi,
On Thu, Dec 11, 2014 at 1:55 AM, Yunzhi Li <[email protected]> wrote:
> This patch adds a binding that describes the Rockchip usb PHYs
> found on Rockchip SoCs usb interface.
Technically the bindings patch is supposed to come before the driver.
So this should be patch #1 and the driver patch #2.
> +Required properties:
> + - compatible: rockchip,rk3288-usb-phy
> + - rockchip,grf : phandle to the syscon managing the "general
> + register files"
> + - #phy-cells: should be 1
> + - #address-cells: should be 1
> + - #size-cells: should be 0
> +
> +Sub-nodes:
> +Each PHY should be represented as a sub-node.
> +
> +Sub-nodes
> +required properties:
> +- reg: the PHY number
> + "0" - PHY connect to OTG controller
> + "1" - PHY connect to HOST0 controller
> + "2" - PHY connect to HOST1 controller
You don't have any sub nodes and are using the phy-cells. Seems like
you should get rid of this? ...or I guess switch to using sub nodes
and set "phy-cells" to 0?
> +
> +Optional Properties:
> +- clocks : phandle + clock specifier for the phy clocks
As per earlier, you should get rid of clocks. If you really want a
clock here and it's optional:
* Back in the driver it shouldn't be a "warn". You don't warn when
optional things are missing.
* You really should specify a clock name. Right now this will pick
the first clock, which makes it hard to later add clocks.
Hi,
On Thu, Dec 11, 2014 at 10:46 AM, Doug Anderson <[email protected]> wrote:
> Yunzhi,
>
> On Thu, Dec 11, 2014 at 1:55 AM, Yunzhi Li <[email protected]> wrote:
>> This patch adds a binding that describes the Rockchip usb PHYs
>> found on Rockchip SoCs usb interface.
>
> Technically the bindings patch is supposed to come before the driver.
> So this should be patch #1 and the driver patch #2.
>
>
>> +Required properties:
>> + - compatible: rockchip,rk3288-usb-phy
>> + - rockchip,grf : phandle to the syscon managing the "general
>> + register files"
>> + - #phy-cells: should be 1
>> + - #address-cells: should be 1
>> + - #size-cells: should be 0
>> +
>> +Sub-nodes:
>> +Each PHY should be represented as a sub-node.
>> +
>> +Sub-nodes
>> +required properties:
>> +- reg: the PHY number
>> + "0" - PHY connect to OTG controller
>> + "1" - PHY connect to HOST0 controller
>> + "2" - PHY connect to HOST1 controller
>
> You don't have any sub nodes and are using the phy-cells. Seems like
> you should get rid of this? ...or I guess switch to using sub nodes
> and set "phy-cells" to 0?
Oh. You actually have them in the ".dtsi" patch, but not here in the
example. Hrm. I don't know enough about phys in general to say
whether they're needed (I will learn if someone else on this thread
doesn't just know), but if they are required then they ought to be in
the example...
Yunzhi,
On Thu, Dec 11, 2014 at 10:09 AM, Doug Anderson <[email protected]> wrote:
>> + rk_phy->phy = devm_phy_create(dev, NULL, &ops);
>
> This has the wrong number of arguments. Even before the change that
> added the 4th argument, this is still wrong because "ops" is supposed
> to be the 2nd argument, not the 3rd.
>
> ...so I'm confused how this compiled for you. I think this ought to be:
>
> rk_phy->phy = devm_phy_create(dev, child, &ops, NULL);
>
> ...but please correct me if I'm mistaken!
As you pointed out privately, I didn't have (dbc9863 phy: remove the
old lookup method). Sorry about the noise..
Hopefully my other comments are not quite as stupid...
-Doug
Hi Doug:
On 2014/12/12 2:09, Doug Anderson wrote:
> Yunzhi,
>
> On Thu, Dec 11, 2014 at 1:55 AM, Yunzhi Li <[email protected]> wrote:
>> + rk_phy->clk = of_clk_get(child, 0);
>> + if (IS_ERR(rk_phy->clk)) {
>> + dev_warn(dev, "failed to get clock\n");
>> + rk_phy->clk = NULL;
>> + }
> The device tree bindings don't specify a clock and the "dtsi" added to
> rk3288 don't reference a clock. Take that code out and avoid a
> warning in the logs at bootup.
>
> ...or should there be a clock?
Actually, there is a clk gating control bit in CRU for each usb phy and I
think we should manage these clocks by the usb phy driver, so I will add
clock property to usb PHYs nodes in next version of patche set.
>
>> + rk_phy->phy = devm_phy_create(dev, NULL, &ops);
> This has the wrong number of arguments. Even before the change that
> added the 4th argument, this is still wrong because "ops" is supposed
> to be the 2nd argument, not the 3rd.
>
> ...so I'm confused how this compiled for you. I think this ought to be:
>
> rk_phy->phy = devm_phy_create(dev, child, &ops, NULL);
>
> ...but please correct me if I'm mistaken!
>
>
>
Hi,
On Thursday 11 December 2014 07:15 PM, Yunzhi Li wrote:
> Hi Kishon:
>
> On 2014/12/11 18:27, Kishon Vijay Abraham I wrote:
>> Hi,
>>
>> On Thursday 11 December 2014 03:25 PM, Yunzhi Li wrote:
>>> +
>>> +static struct phy *rockchip_usb_phy_xlate(struct device *dev,
>>> + struct of_phandle_args *args)
>>> +{
>>> + struct rockchip_usb_phy_priv *priv = dev_get_drvdata(dev);
>>> + unsigned int phy_id = args->args[0];
>>> +
>>> + if (WARN_ON(phy_id < 0 || phy_id >= priv->nphys))
>>> + return ERR_PTR(-ENODEV);
>>> +
>>> + return priv->phys[phy_id].phy;
>> I didn't mean that. You can get rid of this entire xlate stuff if you use
>> something like below
>>
>> phy@xxx {
>> compatible = "";
>> phy1:usb_phy {
>> }
>> phy2:usb_phy {
>> };
>> };
>>
>>
>> usb@xx {
>> compatible = "";
>> phys = <&phy1>; //doesn't need xlate
>> /* this needs xlate
>> phys = <&phy 1>;
>> */
>> phy-names = "phy";
>> };
>
> Thank you so much for your suggestion, but still have a question:
> I have to add the #phy-cells property in each phy sub-node, otherwise
> devm_get_phy() will fail and I get log info like
> "/usb@ff500000: could not get #phy-cells for /phy/usbp-phy1". So can the
> #phy-cells property defines in patent node
> also valid for it's child nodes like #address-cells ?
No. You have to add #phy-cells property for every PHY node.
Thanks
Kishon
Hi,
On Friday 12 December 2014 12:11 AM, Doug Anderson wrote:
> Kishon,
>
> On Thu, Dec 11, 2014 at 2:27 AM, Kishon Vijay Abraham I <[email protected]> wrote:
>> I didn't mean that. You can get rid of this entire xlate stuff if you use
>> something like below
>>
>> phy@xxx {
>> compatible = "";
>> phy1:usb_phy {
>> }
>> phy2:usb_phy {
>> };
>> };
>>
>>
>> usb@xx {
>> compatible = "";
>> phys = <&phy1>; //doesn't need xlate
>> /* this needs xlate
>> phys = <&phy 1>;
>> */
>> phy-names = "phy";
>> };
>
> Is the syntax you proposed really better? Are you saying that you
> advocate never using "#phy-cells" other than 0 for new bindings? Is
No. It can still be used for configuring the PHY. For example, in the case of
PIPE3 PHY we configure it to USB PHY, SATA PHY or PCIE PHY depending on to
which controller the PHY is connected to.
I feel using phy-cells just for differentiating the PHY is pointless when you
have a separate node for each PHY.
Thanks
Kishon