It's based on a downstream driver from Sriharsha Allenki <[email protected]>
that uses USB phy framework, and gets rewrote to adpot generic phy
framework together with quite some cleanups.
Shawn Guo (1):
phy: qualcomm: Add Synopsys High-Speed USB PHY driver
Sriharsha Allenki (1):
dt-bindings: phy: Add Qualcomm Synopsys High-Speed USB PHY binding
.../phy/qcom,snps-28nm-usb-hs-phy.txt | 101 ++++
drivers/phy/qualcomm/Kconfig | 10 +
drivers/phy/qualcomm/Makefile | 1 +
.../phy/qualcomm/phy-qcom-usb-hs-snsp-28nm.c | 494 ++++++++++++++++++
4 files changed, 606 insertions(+)
create mode 100644 Documentation/devicetree/bindings/phy/qcom,snps-28nm-usb-hs-phy.txt
create mode 100644 drivers/phy/qualcomm/phy-qcom-usb-hs-snsp-28nm.c
--
2.18.0
From: Sriharsha Allenki <[email protected]>
It adds bindings for Synopsys 28nm femto phy controller that supports
LS/FS/HS usb connectivity on Qualcomm chipsets.
Signed-off-by: Sriharsha Allenki <[email protected]>
Signed-off-by: Anu Ramanathan <[email protected]>
Signed-off-by: Bjorn Andersson <[email protected]>
Signed-off-by: Shawn Guo <[email protected]>
---
.../phy/qcom,snps-28nm-usb-hs-phy.txt | 101 ++++++++++++++++++
1 file changed, 101 insertions(+)
create mode 100644 Documentation/devicetree/bindings/phy/qcom,snps-28nm-usb-hs-phy.txt
diff --git a/Documentation/devicetree/bindings/phy/qcom,snps-28nm-usb-hs-phy.txt b/Documentation/devicetree/bindings/phy/qcom,snps-28nm-usb-hs-phy.txt
new file mode 100644
index 000000000000..75e7a09dd558
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/qcom,snps-28nm-usb-hs-phy.txt
@@ -0,0 +1,101 @@
+Qualcomm Synopsys 28nm Femto phy controller
+===========================================
+
+Synopsys 28nm femto phy controller supports LS/FS/HS usb connectivity on
+Qualcomm chipsets.
+
+Required properties:
+
+- compatible:
+ Value type: <string>
+ Definition: Should contain "qcom,usb-snps-hsphy".
+
+- reg:
+ Value type: <prop-encoded-array>
+ Definition: USB PHY base address and length of the register map.
+
+- #phy-cells:
+ Value type: <u32>
+ Definition: Should be 0.
+
+- clocks:
+ Value type: <prop-encoded-array>
+ Definition: See clock-bindings.txt section "consumers". List of
+ three clock specifiers for reference, phy core and
+ sleep clocks.
+
+- clock-names:
+ Value type: <string>
+ Definition: Names of the clocks in 1-1 correspondence with the "clocks"
+ property. Must contain "ref", "phy" and "sleep".
+
+- resets:
+ Value type: <prop-encoded-array>
+ Definition: See reset.txt section "consumers". PHY reset specifiers
+ for phy core and POR resets.
+
+- reset-names:
+ Value type: <string>
+ Definition: Names of the resets in 1-1 correspondence with the "resets"
+ property. Must contain "phy" and "por".
+
+- vdd-supply:
+ Value type: <phandle>
+ Definition: phandle to the regulator VDD supply node.
+
+- vdda1p8-supply:
+ Value type: <phandle>
+ Definition: phandle to the regulator 1.8V supply node.
+
+- vdda3p3-supply:
+ Value type: <phandle>
+ Definition: phandle to the regulator 3.3V supply node.
+
+- qcom,vdd-voltage-level:
+ Value type: <prop-array>
+ Definition: This is a list of three integer values <no min max> where
+ each value corresponding to voltage corner in uV.
+
+Optional properties:
+
+- extcon:
+ Value type: <prop-encoded-array>
+ Definition: Should contain the vbus extcon.
+
+- qcom,init-seq:
+ Value type: <u32 array>
+ Definition: Should contain a sequence of <offset value delay> tuples to
+ program 'value' into phy register at 'offset' with 'delay'
+ in us afterwards.
+
+Example:
+
+ phy@7a000 {
+ compatible = "qcom,usb-snps-hsphy";
+ reg = <0x7a000 0x200>;
+ #phy-cells = <0>;
+ clocks = <&rpmcc RPM_SMD_LN_BB_CLK>,
+ <&gcc GCC_USB_HS_PHY_CFG_AHB_CLK>,
+ <&gcc GCC_USB2A_PHY_SLEEP_CLK>;
+ clock-names = "ref", "phy", "sleep";
+ resets = <&gcc GCC_USB_HS_PHY_CFG_AHB_BCR>,
+ <&gcc GCC_USB2A_PHY_BCR>;
+ reset-names = "phy", "por";
+ vdd-supply = <&vreg_l4_1p2>;
+ vdda1p8-supply = <&vreg_l5_1p8>;
+ vdda3p3-supply = <&vreg_l12_3p3>;
+ qcom,vdd-voltage-level = <0 1144000 1200000>;
+ qcom,init-seq = <0xc0 0x01 0>,
+ <0xe8 0x0d 0>,
+ <0x74 0x12 0>,
+ <0x98 0x63 0>,
+ <0x9c 0x03 0>,
+ <0xa0 0x1d 0>,
+ <0xa4 0x03 0>,
+ <0x8c 0x23 0>,
+ <0x78 0x08 0>,
+ <0x7c 0xdc 0>,
+ <0x90 0xe0 20>,
+ <0x74 0x10 0>,
+ <0x90 0x60 0>;
+ };
--
2.18.0
It adds Synopsys 28nm Femto High-Speed USB PHY driver support, which
is usually paired with Synopsys DWC3 USB controllers on Qualcomm SoCs.
Signed-off-by: Shawn Guo <[email protected]>
---
drivers/phy/qualcomm/Kconfig | 10 +
drivers/phy/qualcomm/Makefile | 1 +
.../phy/qualcomm/phy-qcom-usb-hs-snsp-28nm.c | 494 ++++++++++++++++++
3 files changed, 505 insertions(+)
create mode 100644 drivers/phy/qualcomm/phy-qcom-usb-hs-snsp-28nm.c
diff --git a/drivers/phy/qualcomm/Kconfig b/drivers/phy/qualcomm/Kconfig
index 32f7d34eb784..c7b5ee82895d 100644
--- a/drivers/phy/qualcomm/Kconfig
+++ b/drivers/phy/qualcomm/Kconfig
@@ -82,3 +82,13 @@ config PHY_QCOM_USB_HSIC
select GENERIC_PHY
help
Support for the USB HSIC ULPI compliant PHY on QCOM chipsets.
+
+config PHY_QCOM_USB_HS_SNPS_28NM
+ tristate "Qualcomm Synopsys 28nm USB HS PHY driver"
+ depends on ARCH_QCOM || COMPILE_TEST
+ depends on EXTCON || !EXTCON # if EXTCON=m, this cannot be built-in
+ select GENERIC_PHY
+ help
+ Enable this to support the Synopsys 28nm Femto USB PHY on Qualcomm
+ chips. This driver supports the high-speed PHY which is usually
+ paired with either the ChipIdea or Synopsys DWC3 USB IPs on MSM SOCs.
diff --git a/drivers/phy/qualcomm/Makefile b/drivers/phy/qualcomm/Makefile
index c56efd3af205..dc238d95b18c 100644
--- a/drivers/phy/qualcomm/Makefile
+++ b/drivers/phy/qualcomm/Makefile
@@ -9,3 +9,4 @@ obj-$(CONFIG_PHY_QCOM_UFS_14NM) += phy-qcom-ufs-qmp-14nm.o
obj-$(CONFIG_PHY_QCOM_UFS_20NM) += phy-qcom-ufs-qmp-20nm.o
obj-$(CONFIG_PHY_QCOM_USB_HS) += phy-qcom-usb-hs.o
obj-$(CONFIG_PHY_QCOM_USB_HSIC) += phy-qcom-usb-hsic.o
+obj-$(CONFIG_PHY_QCOM_USB_HS_SNPS_28NM) += phy-qcom-usb-hs-snsp-28nm.o
diff --git a/drivers/phy/qualcomm/phy-qcom-usb-hs-snsp-28nm.c b/drivers/phy/qualcomm/phy-qcom-usb-hs-snsp-28nm.c
new file mode 100644
index 000000000000..e3b23ccd4d37
--- /dev/null
+++ b/drivers/phy/qualcomm/phy-qcom-usb-hs-snsp-28nm.c
@@ -0,0 +1,494 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2009-2018, Linux Foundation. All rights reserved.
+ * Copyright (c) 2018, Linaro Limited
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/err.h>
+#include <linux/slab.h>
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/consumer.h>
+#include <linux/phy/phy.h>
+#include <linux/reset.h>
+#include <linux/extcon.h>
+#include <linux/notifier.h>
+
+/* PHY register and bit definitions */
+#define PHY_CTRL_COMMON0 0x078
+#define SIDDQ BIT(2)
+#define PHY_IRQ_CMD 0x0d0
+#define PHY_INTR_CLEAR0 0x0dc
+#define PHY_INTR_MASK0 0x0d4
+#define DPDM_MASK 0x1e
+#define DP_1_0 BIT(4)
+#define DP_0_1 BIT(3)
+#define DM_1_0 BIT(2)
+#define DM_0_1 BIT(1)
+
+enum hsphy_voltage {
+ VOL_NONE,
+ VOL_MIN,
+ VOL_MAX,
+ VOL_NUM,
+};
+
+enum hsphy_vreg {
+ VDD,
+ VDDA_1P8,
+ VDDA_3P3,
+ VREG_NUM,
+};
+
+struct hsphy_init_seq {
+ int offset;
+ int val;
+ int delay;
+};
+
+struct hsphy_priv {
+ void __iomem *base;
+ struct clk_bulk_data *clks;
+ int num_clks;
+ struct reset_control *phy_reset;
+ struct reset_control *por_reset;
+ struct regulator_bulk_data vregs[VREG_NUM];
+ unsigned int voltages[VREG_NUM][VOL_NUM];
+ struct hsphy_init_seq *init_seq;
+ bool cable_connected;
+ struct extcon_dev *vbus_edev;
+ struct notifier_block vbus_notify;
+ enum phy_mode mode;
+};
+
+static int qcom_snps_hsphy_config_regulators(struct hsphy_priv *priv, int high)
+{
+ int min, ret, i;
+
+ min = high ? 1 : 0; /* low or none? */
+
+ for (i = 0; i < VREG_NUM; i++) {
+ ret = regulator_set_voltage(priv->vregs[i].consumer,
+ priv->voltages[i][min],
+ priv->voltages[i][VOL_MAX]);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
+static int qcom_snps_hsphy_enable_regulators(struct hsphy_priv *priv)
+{
+ int ret;
+
+ ret = qcom_snps_hsphy_config_regulators(priv, 1);
+ if (ret)
+ return ret;
+
+ ret = regulator_set_load(priv->vregs[VDDA_1P8].consumer, 19000);
+ if (ret < 0)
+ goto unconfig_regulators;
+
+ ret = regulator_set_load(priv->vregs[VDDA_3P3].consumer, 16000);
+ if (ret < 0)
+ goto unset_1p8_load;
+
+ ret = regulator_bulk_enable(VREG_NUM, priv->vregs);
+ if (ret)
+ goto unset_3p3_load;
+
+ return 0;
+
+unset_3p3_load:
+ regulator_set_load(priv->vregs[VDDA_3P3].consumer, 0);
+unset_1p8_load:
+ regulator_set_load(priv->vregs[VDDA_1P8].consumer, 0);
+unconfig_regulators:
+ qcom_snps_hsphy_config_regulators(priv, 0);
+ return ret;
+}
+
+static void qcom_snps_hsphy_disable_regulators(struct hsphy_priv *priv)
+{
+ regulator_bulk_disable(VREG_NUM, priv->vregs);
+ regulator_set_load(priv->vregs[VDDA_1P8].consumer, 0);
+ regulator_set_load(priv->vregs[VDDA_3P3].consumer, 0);
+ qcom_snps_hsphy_config_regulators(priv, 0);
+}
+
+static int qcom_snps_hsphy_set_mode(struct phy *phy, enum phy_mode mode)
+{
+ struct hsphy_priv *priv = phy_get_drvdata(phy);
+
+ priv->mode = mode;
+
+ return 0;
+}
+
+static void qcom_snps_hsphy_enable_hv_interrupts(struct hsphy_priv *priv)
+{
+ u32 val;
+
+ /* Clear any existing interrupts before enabling the interrupts */
+ val = readb(priv->base + PHY_INTR_CLEAR0);
+ val |= DPDM_MASK;
+ writeb(val, priv->base + PHY_INTR_CLEAR0);
+
+ writeb(0x0, priv->base + PHY_IRQ_CMD);
+ usleep_range(200, 220);
+ writeb(0x1, priv->base + PHY_IRQ_CMD);
+
+ /* Make sure the interrupts are cleared */
+ usleep_range(200, 220);
+
+ val = readb(priv->base + PHY_INTR_MASK0);
+ switch (priv->mode) {
+ case PHY_MODE_USB_HOST_HS:
+ case PHY_MODE_USB_HOST_FS:
+ case PHY_MODE_USB_DEVICE_HS:
+ case PHY_MODE_USB_DEVICE_FS:
+ val |= DP_1_0 | DM_0_1;
+ break;
+ case PHY_MODE_USB_HOST_LS:
+ case PHY_MODE_USB_DEVICE_LS:
+ val |= DP_0_1 | DM_1_0;
+ break;
+ default:
+ /* No device connected */
+ val |= DP_0_1 | DM_0_1;
+ break;
+ }
+ writeb(val, priv->base + PHY_INTR_MASK0);
+}
+
+static void qcom_snps_hsphy_disable_hv_interrupts(struct hsphy_priv *priv)
+{
+ u32 val;
+
+ val = readb(priv->base + PHY_INTR_MASK0);
+ val &= ~DPDM_MASK;
+ writeb(val, priv->base + PHY_INTR_MASK0);
+
+ /* Clear any pending interrupts */
+ val = readb(priv->base + PHY_INTR_CLEAR0);
+ val |= DPDM_MASK;
+ writeb(val, priv->base + PHY_INTR_CLEAR0);
+
+ writeb(0x0, priv->base + PHY_IRQ_CMD);
+ usleep_range(200, 220);
+
+ writeb(0x1, priv->base + PHY_IRQ_CMD);
+ usleep_range(200, 220);
+}
+
+static void qcom_snps_hsphy_enter_retention(struct hsphy_priv *priv)
+{
+ u32 val;
+
+ val = readb(priv->base + PHY_CTRL_COMMON0);
+ val |= SIDDQ;
+ writeb(val, priv->base + PHY_CTRL_COMMON0);
+}
+
+static void qcom_snps_hsphy_exit_retention(struct hsphy_priv *priv)
+{
+ u32 val;
+
+ val = readb(priv->base + PHY_CTRL_COMMON0);
+ val &= ~SIDDQ;
+ writeb(val, priv->base + PHY_CTRL_COMMON0);
+}
+
+static int qcom_snps_hsphy_vbus_notifier(struct notifier_block *nb,
+ unsigned long event, void *ptr)
+{
+ struct hsphy_priv *priv = container_of(nb, struct hsphy_priv,
+ vbus_notify);
+ priv->cable_connected = !!event;
+ return 0;
+}
+
+static int qcom_snps_hsphy_power_on(struct phy *phy)
+{
+ struct hsphy_priv *priv = phy_get_drvdata(phy);
+ int ret;
+
+ if (priv->cable_connected) {
+ ret = clk_bulk_prepare_enable(priv->num_clks, priv->clks);
+ if (ret)
+ return ret;
+ qcom_snps_hsphy_disable_hv_interrupts(priv);
+ } else {
+ ret = qcom_snps_hsphy_enable_regulators(priv);
+ if (ret)
+ return ret;
+ ret = clk_bulk_prepare_enable(priv->num_clks, priv->clks);
+ if (ret)
+ return ret;
+ qcom_snps_hsphy_exit_retention(priv);
+ }
+
+ return 0;
+}
+
+static int qcom_snps_hsphy_power_off(struct phy *phy)
+{
+ struct hsphy_priv *priv = phy_get_drvdata(phy);
+
+ if (priv->cable_connected) {
+ qcom_snps_hsphy_enable_hv_interrupts(priv);
+ clk_bulk_disable_unprepare(priv->num_clks, priv->clks);
+ } else {
+ qcom_snps_hsphy_enter_retention(priv);
+ clk_bulk_disable_unprepare(priv->num_clks, priv->clks);
+ qcom_snps_hsphy_disable_regulators(priv);
+ }
+
+ return 0;
+}
+
+static int qcom_snps_hsphy_reset(struct hsphy_priv *priv)
+{
+ int ret;
+
+ ret = reset_control_assert(priv->phy_reset);
+ if (ret)
+ return ret;
+
+ usleep_range(10, 15);
+
+ ret = reset_control_deassert(priv->phy_reset);
+ if (ret)
+ return ret;
+
+ usleep_range(80, 100);
+
+ return 0;
+}
+
+static void qcom_snps_hsphy_init_sequence(struct hsphy_priv *priv)
+{
+ struct hsphy_init_seq *seq;
+
+ for (seq = priv->init_seq; seq->offset != -1; seq++) {
+ writeb(seq->val, priv->base + seq->offset);
+ if (seq->delay)
+ usleep_range(seq->delay, seq->delay + 10);
+ }
+
+ /* Ensure that the above parameter overrides is successful. */
+ mb();
+}
+
+static int qcom_snps_hsphy_por_reset(struct hsphy_priv *priv)
+{
+ int ret;
+
+ ret = reset_control_assert(priv->por_reset);
+ if (ret)
+ return ret;
+
+ /*
+ * The Femto PHY is POR reset in the following scenarios.
+ *
+ * 1. After overriding the parameter registers.
+ * 2. Low power mode exit from PHY retention.
+ *
+ * Ensure that SIDDQ is cleared before bringing the PHY
+ * out of reset.
+ */
+ qcom_snps_hsphy_exit_retention(priv);
+
+ /*
+ * As per databook, 10 usec delay is required between
+ * PHY POR assert and de-assert.
+ */
+ usleep_range(10, 20);
+ ret = reset_control_deassert(priv->por_reset);
+ if (ret)
+ return ret;
+
+ /*
+ * As per databook, it takes 75 usec for PHY to stabilize
+ * after the reset.
+ */
+ usleep_range(80, 100);
+
+ /* Ensure that RESET operation is completed. */
+ mb();
+
+ return 0;
+}
+
+static int qcom_snps_hsphy_init(struct phy *phy)
+{
+ struct hsphy_priv *priv = phy_get_drvdata(phy);
+ int state;
+ int ret;
+
+ ret = qcom_snps_hsphy_reset(priv);
+ if (ret)
+ return ret;
+
+ qcom_snps_hsphy_init_sequence(priv);
+
+ ret = qcom_snps_hsphy_por_reset(priv);
+ if (ret)
+ return ret;
+
+ /* Setup initial state */
+ if (priv->vbus_edev) {
+ state = extcon_get_state(priv->vbus_edev, EXTCON_USB);
+ ret = qcom_snps_hsphy_vbus_notifier(&priv->vbus_notify, state,
+ priv->vbus_edev);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
+static const struct phy_ops qcom_snps_hsphy_ops = {
+ .init = qcom_snps_hsphy_init,
+ .power_on = qcom_snps_hsphy_power_on,
+ .power_off = qcom_snps_hsphy_power_off,
+ .set_mode = qcom_snps_hsphy_set_mode,
+ .owner = THIS_MODULE,
+};
+
+static const char * const qcom_snps_hsphy_clks[] = {
+ "ref",
+ "phy",
+ "sleep",
+};
+
+static int qcom_snps_hsphy_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct phy_provider *provider;
+ struct hsphy_priv *priv;
+ struct resource *res;
+ struct phy *phy;
+ int size;
+ int ret;
+ int i;
+
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ priv->base = devm_ioremap_resource(dev, res);
+ if (IS_ERR(priv->base))
+ return PTR_ERR(priv->base);
+
+ priv->num_clks = ARRAY_SIZE(qcom_snps_hsphy_clks);
+ priv->clks = devm_kcalloc(dev, priv->num_clks, sizeof(*priv->clks),
+ GFP_KERNEL);
+ if (!priv->clks)
+ return -ENOMEM;
+
+ for (i = 0; i < priv->num_clks; i++)
+ priv->clks[i].id = qcom_snps_hsphy_clks[i];
+
+ ret = devm_clk_bulk_get(dev, priv->num_clks, priv->clks);
+ if (ret)
+ return ret;
+
+ priv->phy_reset = devm_reset_control_get(dev, "phy");
+ if (IS_ERR(priv->phy_reset))
+ return PTR_ERR(priv->phy_reset);
+
+ priv->por_reset = devm_reset_control_get(dev, "por");
+ if (IS_ERR(priv->por_reset))
+ return PTR_ERR(priv->por_reset);
+
+ priv->vregs[VDD].supply = "vdd";
+ priv->vregs[VDDA_1P8].supply = "vdda1p8";
+ priv->vregs[VDDA_3P3].supply = "vdda3p3";
+
+ ret = devm_regulator_bulk_get(dev, VREG_NUM, priv->vregs);
+ if (ret)
+ return ret;
+
+ priv->voltages[VDDA_1P8][VOL_NONE] = 0;
+ priv->voltages[VDDA_1P8][VOL_MIN] = 1800000;
+ priv->voltages[VDDA_1P8][VOL_MAX] = 1800000;
+
+ priv->voltages[VDDA_3P3][VOL_NONE] = 0;
+ priv->voltages[VDDA_3P3][VOL_MIN] = 3050000;
+ priv->voltages[VDDA_3P3][VOL_MAX] = 3300000;
+
+ ret = of_property_read_u32_array(dev->of_node, "qcom,vdd-voltage-level",
+ priv->voltages[VDD], VOL_NUM);
+ if (ret) {
+ dev_err(dev, "failed to read qcom,vdd-voltage-level\n");
+ return ret;
+ }
+
+ size = of_property_count_u32_elems(dev->of_node, "qcom,init-seq");
+ if (size < 0)
+ size = 0;
+
+ priv->init_seq = devm_kcalloc(dev, (size / 3) + 1,
+ sizeof(*priv->init_seq), GFP_KERNEL);
+ if (!priv->init_seq)
+ return -ENOMEM;
+
+ ret = of_property_read_u32_array(dev->of_node, "qcom,init-seq",
+ (u32 *) priv->init_seq, size);
+ if (ret && size)
+ return ret;
+
+ /* terminator */
+ priv->init_seq[size / 3].offset = -1;
+
+ phy = devm_phy_create(dev, dev->of_node, &qcom_snps_hsphy_ops);
+ if (IS_ERR(phy))
+ return PTR_ERR(phy);
+
+ priv->vbus_edev = extcon_get_edev_by_phandle(dev, 0);
+ if (IS_ERR(priv->vbus_edev)) {
+ if (PTR_ERR(priv->vbus_edev) != -ENODEV)
+ return PTR_ERR(priv->vbus_edev);
+ priv->vbus_edev = NULL;
+ }
+
+ if (priv->vbus_edev) {
+ priv->vbus_notify.notifier_call = qcom_snps_hsphy_vbus_notifier;
+ ret = devm_extcon_register_notifier(dev, priv->vbus_edev,
+ EXTCON_USB,
+ &priv->vbus_notify);
+ if (ret)
+ return ret;
+ }
+
+ phy_set_drvdata(phy, priv);
+
+ provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
+ return PTR_ERR_OR_ZERO(provider);
+}
+
+static const struct of_device_id qcom_snps_hsphy_match[] = {
+ { .compatible = "qcom,usb-snps-hsphy", },
+ { },
+};
+MODULE_DEVICE_TABLE(of, qcom_snps_hsphy_match);
+
+static struct platform_driver qcom_snps_hsphy_driver = {
+ .probe = qcom_snps_hsphy_probe,
+ .driver = {
+ .name = "qcom-usb-snps-hsphy",
+ .of_match_table = qcom_snps_hsphy_match,
+ },
+};
+module_platform_driver(qcom_snps_hsphy_driver);
+
+MODULE_DESCRIPTION("Qualcomm Synopsys 28nm USB High-Speed PHY driver");
+MODULE_LICENSE("GPL v2");
--
2.18.0
On 08-11-18, 15:04, Shawn Guo wrote:
> From: Sriharsha Allenki <[email protected]>
>
> It adds bindings for Synopsys 28nm femto phy controller that supports
> LS/FS/HS usb connectivity on Qualcomm chipsets.
>
> Signed-off-by: Sriharsha Allenki <[email protected]>
> Signed-off-by: Anu Ramanathan <[email protected]>
> Signed-off-by: Bjorn Andersson <[email protected]>
> Signed-off-by: Shawn Guo <[email protected]>
> ---
> .../phy/qcom,snps-28nm-usb-hs-phy.txt | 101 ++++++++++++++++++
> 1 file changed, 101 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/phy/qcom,snps-28nm-usb-hs-phy.txt
>
> diff --git a/Documentation/devicetree/bindings/phy/qcom,snps-28nm-usb-hs-phy.txt b/Documentation/devicetree/bindings/phy/qcom,snps-28nm-usb-hs-phy.txt
> new file mode 100644
> index 000000000000..75e7a09dd558
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/qcom,snps-28nm-usb-hs-phy.txt
> @@ -0,0 +1,101 @@
> +Qualcomm Synopsys 28nm Femto phy controller
> +===========================================
> +
> +Synopsys 28nm femto phy controller supports LS/FS/HS usb connectivity on
> +Qualcomm chipsets.
> +
> +Required properties:
> +
> +- compatible:
> + Value type: <string>
> + Definition: Should contain "qcom,usb-snps-hsphy".
> +
> +- reg:
> + Value type: <prop-encoded-array>
> + Definition: USB PHY base address and length of the register map.
> +
> +- #phy-cells:
> + Value type: <u32>
> + Definition: Should be 0.
I dont quite understand the definition that it should be 0, maybe you
mean allowed value is 0, if so why have this property?
--
~Vinod
On 08-11-18, 15:04, Shawn Guo wrote:
> +static int qcom_snps_hsphy_config_regulators(struct hsphy_priv *priv, int high)
> +{
> + int min, ret, i;
> +
> + min = high ? 1 : 0; /* low or none? */
> +
> + for (i = 0; i < VREG_NUM; i++) {
> + ret = regulator_set_voltage(priv->vregs[i].consumer,
> + priv->voltages[i][min],
> + priv->voltages[i][VOL_MAX]);
> + if (ret)
> + return ret;
should we not roll back the set voltages on error?
> +static int qcom_snps_hsphy_por_reset(struct hsphy_priv *priv)
> +{
> + int ret;
> +
> + ret = reset_control_assert(priv->por_reset);
> + if (ret)
> + return ret;
> +
> + /*
> + * The Femto PHY is POR reset in the following scenarios.
POR?
> +static int qcom_snps_hsphy_init(struct phy *phy)
> +{
> + struct hsphy_priv *priv = phy_get_drvdata(phy);
> + int state;
> + int ret;
perhaps they can be in a single line :)
> +static int qcom_snps_hsphy_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct phy_provider *provider;
> + struct hsphy_priv *priv;
> + struct resource *res;
> + struct phy *phy;
> + int size;
> + int ret;
> + int i;
> +
> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + priv->base = devm_ioremap_resource(dev, res);
> + if (IS_ERR(priv->base))
> + return PTR_ERR(priv->base);
> +
> + priv->num_clks = ARRAY_SIZE(qcom_snps_hsphy_clks);
> + priv->clks = devm_kcalloc(dev, priv->num_clks, sizeof(*priv->clks),
> + GFP_KERNEL);
> + if (!priv->clks)
> + return -ENOMEM;
> +
> + for (i = 0; i < priv->num_clks; i++)
> + priv->clks[i].id = qcom_snps_hsphy_clks[i];
> +
> + ret = devm_clk_bulk_get(dev, priv->num_clks, priv->clks);
> + if (ret)
> + return ret;
> +
> + priv->phy_reset = devm_reset_control_get(dev, "phy");
> + if (IS_ERR(priv->phy_reset))
> + return PTR_ERR(priv->phy_reset);
> +
> + priv->por_reset = devm_reset_control_get(dev, "por");
> + if (IS_ERR(priv->por_reset))
> + return PTR_ERR(priv->por_reset);
> +
> + priv->vregs[VDD].supply = "vdd";
> + priv->vregs[VDDA_1P8].supply = "vdda1p8";
> + priv->vregs[VDDA_3P3].supply = "vdda3p3";
> +
> + ret = devm_regulator_bulk_get(dev, VREG_NUM, priv->vregs);
> + if (ret)
> + return ret;
> +
> + priv->voltages[VDDA_1P8][VOL_NONE] = 0;
> + priv->voltages[VDDA_1P8][VOL_MIN] = 1800000;
> + priv->voltages[VDDA_1P8][VOL_MAX] = 1800000;
> +
> + priv->voltages[VDDA_3P3][VOL_NONE] = 0;
> + priv->voltages[VDDA_3P3][VOL_MIN] = 3050000;
> + priv->voltages[VDDA_3P3][VOL_MAX] = 3300000;
> +
> + ret = of_property_read_u32_array(dev->of_node, "qcom,vdd-voltage-level",
> + priv->voltages[VDD], VOL_NUM);
> + if (ret) {
> + dev_err(dev, "failed to read qcom,vdd-voltage-level\n");
> + return ret;
> + }
> +
> + size = of_property_count_u32_elems(dev->of_node, "qcom,init-seq");
> + if (size < 0)
> + size = 0;
> +
> + priv->init_seq = devm_kcalloc(dev, (size / 3) + 1,
size/3? I think it would be good to add a common explaining this
--
~Vinod
On Fri, Nov 09, 2018 at 10:38:19AM +0530, Vinod Koul wrote:
> On 08-11-18, 15:04, Shawn Guo wrote:
> > From: Sriharsha Allenki <[email protected]>
> >
> > It adds bindings for Synopsys 28nm femto phy controller that supports
> > LS/FS/HS usb connectivity on Qualcomm chipsets.
> >
> > Signed-off-by: Sriharsha Allenki <[email protected]>
> > Signed-off-by: Anu Ramanathan <[email protected]>
> > Signed-off-by: Bjorn Andersson <[email protected]>
> > Signed-off-by: Shawn Guo <[email protected]>
> > ---
> > .../phy/qcom,snps-28nm-usb-hs-phy.txt | 101 ++++++++++++++++++
> > 1 file changed, 101 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/phy/qcom,snps-28nm-usb-hs-phy.txt
> >
> > diff --git a/Documentation/devicetree/bindings/phy/qcom,snps-28nm-usb-hs-phy.txt b/Documentation/devicetree/bindings/phy/qcom,snps-28nm-usb-hs-phy.txt
> > new file mode 100644
> > index 000000000000..75e7a09dd558
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/phy/qcom,snps-28nm-usb-hs-phy.txt
> > @@ -0,0 +1,101 @@
> > +Qualcomm Synopsys 28nm Femto phy controller
> > +===========================================
> > +
> > +Synopsys 28nm femto phy controller supports LS/FS/HS usb connectivity on
> > +Qualcomm chipsets.
> > +
> > +Required properties:
> > +
> > +- compatible:
> > + Value type: <string>
> > + Definition: Should contain "qcom,usb-snps-hsphy".
> > +
> > +- reg:
> > + Value type: <prop-encoded-array>
> > + Definition: USB PHY base address and length of the register map.
> > +
> > +- #phy-cells:
> > + Value type: <u32>
> > + Definition: Should be 0.
>
> I dont quite understand the definition that it should be 0, maybe you
> mean allowed value is 0, if so why have this property?
The property is defined by generic phy bindings phy/phy-bindings.txt.
I can add a pointer to it if you think that's necessary. The property
should be 0 for our device, because there is zero number cell in phy
specifier from dwc3 node as shown in the example.
dwc3@78c0000 {
...
phys = <&usb2_phy_prim>;
phy-names = "usb2-phy";
}
And for that reason, we can use the generic .of_xlate implementation
of_phy_simple_xlate() provided by phy core. There are some comments
in kernel doc of of_phy_simple_xlate() which might be helpful.
Shawn
On Fri, Nov 09, 2018 at 10:52:17AM +0530, Vinod Koul wrote:
> On 08-11-18, 15:04, Shawn Guo wrote:
> > +static int qcom_snps_hsphy_config_regulators(struct hsphy_priv *priv, int high)
> > +{
> > + int min, ret, i;
> > +
> > + min = high ? 1 : 0; /* low or none? */
> > +
> > + for (i = 0; i < VREG_NUM; i++) {
> > + ret = regulator_set_voltage(priv->vregs[i].consumer,
> > + priv->voltages[i][min],
> > + priv->voltages[i][VOL_MAX]);
> > + if (ret)
> > + return ret;
>
> should we not roll back the set voltages on error?
Yes. I will get that handled in v2. Thanks.
>
> > +static int qcom_snps_hsphy_por_reset(struct hsphy_priv *priv)
> > +{
> > + int ret;
> > +
> > + ret = reset_control_assert(priv->por_reset);
> > + if (ret)
> > + return ret;
> > +
> > + /*
> > + * The Femto PHY is POR reset in the following scenarios.
>
> POR?
Hmm, I do not understand this comment. The POR is commonly used as the
abbrev of power-on-reset. What do you meat exactly?
>
> > +static int qcom_snps_hsphy_init(struct phy *phy)
> > +{
> > + struct hsphy_priv *priv = phy_get_drvdata(phy);
> > + int state;
> > + int ret;
>
> perhaps they can be in a single line :)
I prefer to keep them on separate line, as that makes the addition and
removal of one of them relatively easier.
>
> > +static int qcom_snps_hsphy_probe(struct platform_device *pdev)
> > +{
> > + struct device *dev = &pdev->dev;
> > + struct phy_provider *provider;
> > + struct hsphy_priv *priv;
> > + struct resource *res;
> > + struct phy *phy;
> > + int size;
> > + int ret;
> > + int i;
> > +
> > + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> > + if (!priv)
> > + return -ENOMEM;
> > +
> > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > + priv->base = devm_ioremap_resource(dev, res);
> > + if (IS_ERR(priv->base))
> > + return PTR_ERR(priv->base);
> > +
> > + priv->num_clks = ARRAY_SIZE(qcom_snps_hsphy_clks);
> > + priv->clks = devm_kcalloc(dev, priv->num_clks, sizeof(*priv->clks),
> > + GFP_KERNEL);
> > + if (!priv->clks)
> > + return -ENOMEM;
> > +
> > + for (i = 0; i < priv->num_clks; i++)
> > + priv->clks[i].id = qcom_snps_hsphy_clks[i];
> > +
> > + ret = devm_clk_bulk_get(dev, priv->num_clks, priv->clks);
> > + if (ret)
> > + return ret;
> > +
> > + priv->phy_reset = devm_reset_control_get(dev, "phy");
> > + if (IS_ERR(priv->phy_reset))
> > + return PTR_ERR(priv->phy_reset);
> > +
> > + priv->por_reset = devm_reset_control_get(dev, "por");
> > + if (IS_ERR(priv->por_reset))
> > + return PTR_ERR(priv->por_reset);
> > +
> > + priv->vregs[VDD].supply = "vdd";
> > + priv->vregs[VDDA_1P8].supply = "vdda1p8";
> > + priv->vregs[VDDA_3P3].supply = "vdda3p3";
> > +
> > + ret = devm_regulator_bulk_get(dev, VREG_NUM, priv->vregs);
> > + if (ret)
> > + return ret;
> > +
> > + priv->voltages[VDDA_1P8][VOL_NONE] = 0;
> > + priv->voltages[VDDA_1P8][VOL_MIN] = 1800000;
> > + priv->voltages[VDDA_1P8][VOL_MAX] = 1800000;
> > +
> > + priv->voltages[VDDA_3P3][VOL_NONE] = 0;
> > + priv->voltages[VDDA_3P3][VOL_MIN] = 3050000;
> > + priv->voltages[VDDA_3P3][VOL_MAX] = 3300000;
> > +
> > + ret = of_property_read_u32_array(dev->of_node, "qcom,vdd-voltage-level",
> > + priv->voltages[VDD], VOL_NUM);
> > + if (ret) {
> > + dev_err(dev, "failed to read qcom,vdd-voltage-level\n");
> > + return ret;
> > + }
> > +
> > + size = of_property_count_u32_elems(dev->of_node, "qcom,init-seq");
> > + if (size < 0)
> > + size = 0;
> > +
> > + priv->init_seq = devm_kcalloc(dev, (size / 3) + 1,
>
> size/3? I think it would be good to add a common explaining this
The property is a sequence of <offset value delay> tuples, and we are
figuring out how many tuples are there. Yep, I will add a comment in
there for v2.
Shawn
On 09-11-18, 14:31, Shawn Guo wrote:
> On Fri, Nov 09, 2018 at 10:38:19AM +0530, Vinod Koul wrote:
> > On 08-11-18, 15:04, Shawn Guo wrote:
> > > +
> > > +- #phy-cells:
> > > + Value type: <u32>
> > > + Definition: Should be 0.
> >
> > I dont quite understand the definition that it should be 0, maybe you
> > mean allowed value is 0, if so why have this property?
>
> The property is defined by generic phy bindings phy/phy-bindings.txt.
> I can add a pointer to it if you think that's necessary. The property
> should be 0 for our device, because there is zero number cell in phy
> specifier from dwc3 node as shown in the example.
That makes sense, also does it make sense it mention the properties in
phy/phy-bindings.txt, why not refer that here for the properties we use
and vlaues.
>
> dwc3@78c0000 {
> ...
> phys = <&usb2_phy_prim>;
> phy-names = "usb2-phy";
> }
>
> And for that reason, we can use the generic .of_xlate implementation
> of_phy_simple_xlate() provided by phy core. There are some comments
> in kernel doc of of_phy_simple_xlate() which might be helpful.
>
> Shawn
--
~Vinod
On 09-11-18, 14:52, Shawn Guo wrote:
> On Fri, Nov 09, 2018 at 10:52:17AM +0530, Vinod Koul wrote:
> > On 08-11-18, 15:04, Shawn Guo wrote:
> > > +static int qcom_snps_hsphy_config_regulators(struct hsphy_priv *priv, int high)
> > > +{
> > > + int min, ret, i;
> > > +
> > > + min = high ? 1 : 0; /* low or none? */
> > > +
> > > + for (i = 0; i < VREG_NUM; i++) {
> > > + ret = regulator_set_voltage(priv->vregs[i].consumer,
> > > + priv->voltages[i][min],
> > > + priv->voltages[i][VOL_MAX]);
> > > + if (ret)
> > > + return ret;
> >
> > should we not roll back the set voltages on error?
>
> Yes. I will get that handled in v2. Thanks.
>
> >
> > > +static int qcom_snps_hsphy_por_reset(struct hsphy_priv *priv)
> > > +{
> > > + int ret;
> > > +
> > > + ret = reset_control_assert(priv->por_reset);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + /*
> > > + * The Femto PHY is POR reset in the following scenarios.
> >
> > POR?
>
> Hmm, I do not understand this comment. The POR is commonly used as the
> abbrev of power-on-reset. What do you meat exactly?
I wasnt aware that POR refers to power-on-reset :) I dont know if it is
a very generic term :D
--
~Vinod
Hi Rob,
On Mon, Nov 12, 2018 at 01:24:51PM -0600, Rob Herring wrote:
> On Thu, Nov 08, 2018 at 03:04:48PM +0800, Shawn Guo wrote:
> > From: Sriharsha Allenki <[email protected]>
> >
> > It adds bindings for Synopsys 28nm femto phy controller that supports
> > LS/FS/HS usb connectivity on Qualcomm chipsets.
> >
> > Signed-off-by: Sriharsha Allenki <[email protected]>
> > Signed-off-by: Anu Ramanathan <[email protected]>
> > Signed-off-by: Bjorn Andersson <[email protected]>
> > Signed-off-by: Shawn Guo <[email protected]>
> > ---
> > .../phy/qcom,snps-28nm-usb-hs-phy.txt | 101 ++++++++++++++++++
> > 1 file changed, 101 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/phy/qcom,snps-28nm-usb-hs-phy.txt
> >
> > diff --git a/Documentation/devicetree/bindings/phy/qcom,snps-28nm-usb-hs-phy.txt b/Documentation/devicetree/bindings/phy/qcom,snps-28nm-usb-hs-phy.txt
> > new file mode 100644
> > index 000000000000..75e7a09dd558
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/phy/qcom,snps-28nm-usb-hs-phy.txt
> > @@ -0,0 +1,101 @@
> > +Qualcomm Synopsys 28nm Femto phy controller
> > +===========================================
> > +
> > +Synopsys 28nm femto phy controller supports LS/FS/HS usb connectivity on
> > +Qualcomm chipsets.
> > +
> > +Required properties:
> > +
> > +- compatible:
> > + Value type: <string>
> > + Definition: Should contain "qcom,usb-snps-hsphy".
>
> SoC specific compatible?
Agreed. A SoC prefixed compatible would be more specific and scalable
for handling different programming model of the same IP. I will use
"qcom,qcs404-usb-hsphy" in v3.
>
> > +
> > +- reg:
> > + Value type: <prop-encoded-array>
> > + Definition: USB PHY base address and length of the register map.
> > +
> > +- #phy-cells:
> > + Value type: <u32>
> > + Definition: Should be 0.
> > +
> > +- clocks:
> > + Value type: <prop-encoded-array>
> > + Definition: See clock-bindings.txt section "consumers". List of
> > + three clock specifiers for reference, phy core and
> > + sleep clocks.
> > +
> > +- clock-names:
> > + Value type: <string>
> > + Definition: Names of the clocks in 1-1 correspondence with the "clocks"
> > + property. Must contain "ref", "phy" and "sleep".
> > +
> > +- resets:
> > + Value type: <prop-encoded-array>
> > + Definition: See reset.txt section "consumers". PHY reset specifiers
> > + for phy core and POR resets.
> > +
> > +- reset-names:
> > + Value type: <string>
> > + Definition: Names of the resets in 1-1 correspondence with the "resets"
> > + property. Must contain "phy" and "por".
> > +
> > +- vdd-supply:
> > + Value type: <phandle>
> > + Definition: phandle to the regulator VDD supply node.
> > +
> > +- vdda1p8-supply:
> > + Value type: <phandle>
> > + Definition: phandle to the regulator 1.8V supply node.
> > +
> > +- vdda3p3-supply:
> > + Value type: <phandle>
> > + Definition: phandle to the regulator 3.3V supply node.
> > +
> > +- qcom,vdd-voltage-level:
> > + Value type: <prop-array>
> > + Definition: This is a list of three integer values <no min max> where
> > + each value corresponding to voltage corner in uV.
> > +
> > +Optional properties:
> > +
> > +- extcon:
> > + Value type: <prop-encoded-array>
> > + Definition: Should contain the vbus extcon.
>
> Don't use extcon for new bindings. Use usb-connector binding.
Okay, I just did a bit of research and found that 'extcon' is becoming
a deprecated DT property recently and we should OF graph bindings to
specify the connector. Will do in v3.
>
> > +
> > +- qcom,init-seq:
> > + Value type: <u32 array>
> > + Definition: Should contain a sequence of <offset value delay> tuples to
> > + program 'value' into phy register at 'offset' with 'delay'
> > + in us afterwards.
>
> If we wanted this type of thing in DT, we'd have a generic binding (or
> forth).
Right now, this is a qualcomm usb phy specific bindings - first used in
qcom,usb-hs-phy.txt and I extended it a bit for my phy. As this is not
a so good hardware description, I'm a little hesitated to make it
generic for other platforms to use in general. What about we put off it
a little bit until we see more platforms need the same thing?
Shawn
> This should probably be split between SoC specific settings in
> the driver and board properties in DT.
Hi Sriharsha,
On Tue, Nov 13, 2018 at 11:42 AM Shawn Guo <[email protected]> wrote:
<snip>
> > > +- qcom,init-seq:
> > > + Value type: <u32 array>
> > > + Definition: Should contain a sequence of <offset value delay> tuples to
> > > + program 'value' into phy register at 'offset' with 'delay'
> > > + in us afterwards.
> >
> > If we wanted this type of thing in DT, we'd have a generic binding (or
> > forth).
>
> Right now, this is a qualcomm usb phy specific bindings - first used in
> qcom,usb-hs-phy.txt and I extended it a bit for my phy. As this is not
> a so good hardware description, I'm a little hesitated to make it
> generic for other platforms to use in general. What about we put off it
> a little bit until we see more platforms need the same thing?
Are those register write sequences really required here? At least,
from the test I do, it still works with this property dropped.
Shawn
On Mon, Nov 12, 2018 at 9:42 PM Shawn Guo <[email protected]> wrote:
>
> Hi Rob,
>
> On Mon, Nov 12, 2018 at 01:24:51PM -0600, Rob Herring wrote:
> > On Thu, Nov 08, 2018 at 03:04:48PM +0800, Shawn Guo wrote:
> > > From: Sriharsha Allenki <[email protected]>
> > >
> > > It adds bindings for Synopsys 28nm femto phy controller that supports
> > > LS/FS/HS usb connectivity on Qualcomm chipsets.
> > >
> > > Signed-off-by: Sriharsha Allenki <[email protected]>
> > > Signed-off-by: Anu Ramanathan <[email protected]>
> > > Signed-off-by: Bjorn Andersson <[email protected]>
> > > Signed-off-by: Shawn Guo <[email protected]>
> > > ---
> > > .../phy/qcom,snps-28nm-usb-hs-phy.txt | 101 ++++++++++++++++++
> > > 1 file changed, 101 insertions(+)
> > > create mode 100644 Documentation/devicetree/bindings/phy/qcom,snps-28nm-usb-hs-phy.txt
> > >
> > > diff --git a/Documentation/devicetree/bindings/phy/qcom,snps-28nm-usb-hs-phy.txt b/Documentation/devicetree/bindings/phy/qcom,snps-28nm-usb-hs-phy.txt
> > > new file mode 100644
> > > index 000000000000..75e7a09dd558
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/phy/qcom,snps-28nm-usb-hs-phy.txt
> > > @@ -0,0 +1,101 @@
> > > +Qualcomm Synopsys 28nm Femto phy controller
> > > +===========================================
> > > +
> > > +Synopsys 28nm femto phy controller supports LS/FS/HS usb connectivity on
> > > +Qualcomm chipsets.
> > > +
> > > +Required properties:
> > > +
> > > +- compatible:
> > > + Value type: <string>
> > > + Definition: Should contain "qcom,usb-snps-hsphy".
> >
> > SoC specific compatible?
>
> Agreed. A SoC prefixed compatible would be more specific and scalable
> for handling different programming model of the same IP. I will use
> "qcom,qcs404-usb-hsphy" in v3.
>
> >
> > > +
> > > +- reg:
> > > + Value type: <prop-encoded-array>
> > > + Definition: USB PHY base address and length of the register map.
> > > +
> > > +- #phy-cells:
> > > + Value type: <u32>
> > > + Definition: Should be 0.
> > > +
> > > +- clocks:
> > > + Value type: <prop-encoded-array>
> > > + Definition: See clock-bindings.txt section "consumers". List of
> > > + three clock specifiers for reference, phy core and
> > > + sleep clocks.
> > > +
> > > +- clock-names:
> > > + Value type: <string>
> > > + Definition: Names of the clocks in 1-1 correspondence with the "clocks"
> > > + property. Must contain "ref", "phy" and "sleep".
> > > +
> > > +- resets:
> > > + Value type: <prop-encoded-array>
> > > + Definition: See reset.txt section "consumers". PHY reset specifiers
> > > + for phy core and POR resets.
> > > +
> > > +- reset-names:
> > > + Value type: <string>
> > > + Definition: Names of the resets in 1-1 correspondence with the "resets"
> > > + property. Must contain "phy" and "por".
> > > +
> > > +- vdd-supply:
> > > + Value type: <phandle>
> > > + Definition: phandle to the regulator VDD supply node.
> > > +
> > > +- vdda1p8-supply:
> > > + Value type: <phandle>
> > > + Definition: phandle to the regulator 1.8V supply node.
> > > +
> > > +- vdda3p3-supply:
> > > + Value type: <phandle>
> > > + Definition: phandle to the regulator 3.3V supply node.
> > > +
> > > +- qcom,vdd-voltage-level:
> > > + Value type: <prop-array>
> > > + Definition: This is a list of three integer values <no min max> where
> > > + each value corresponding to voltage corner in uV.
> > > +
> > > +Optional properties:
> > > +
> > > +- extcon:
> > > + Value type: <prop-encoded-array>
> > > + Definition: Should contain the vbus extcon.
> >
> > Don't use extcon for new bindings. Use usb-connector binding.
>
> Okay, I just did a bit of research and found that 'extcon' is becoming
> a deprecated DT property recently and we should OF graph bindings to
> specify the connector. Will do in v3.
>
> >
> > > +
> > > +- qcom,init-seq:
> > > + Value type: <u32 array>
> > > + Definition: Should contain a sequence of <offset value delay> tuples to
> > > + program 'value' into phy register at 'offset' with 'delay'
> > > + in us afterwards.
> >
> > If we wanted this type of thing in DT, we'd have a generic binding (or
> > forth).
>
> Right now, this is a qualcomm usb phy specific bindings - first used in
> qcom,usb-hs-phy.txt and I extended it a bit for my phy. As this is not
> a so good hardware description, I'm a little hesitated to make it
> generic for other platforms to use in general. What about we put off it
> a little bit until we see more platforms need the same thing?
I'm not saying I want it generic. Quite the opposite. I don't think we
should have it either generically or vendor specific. The main thing I
have a problem with is the timing information because then we're more
that just data. Without that we're talking about a bunch of properties
for register fields or just raw register values in DT. That becomes
more of a judgement call. There's not too much value in making a
driver translate a bunch of properties just to stuff them into
registers on init. But then just allowing any raw register value to be
in DT could be easily abused.
Rob
On Sat, Nov 17, 2018 at 09:13:38AM -0600, Rob Herring wrote:
<snip>
> > > > +- qcom,init-seq:
> > > > + Value type: <u32 array>
> > > > + Definition: Should contain a sequence of <offset value delay> tuples to
> > > > + program 'value' into phy register at 'offset' with 'delay'
> > > > + in us afterwards.
> > >
> > > If we wanted this type of thing in DT, we'd have a generic binding (or
> > > forth).
> >
> > Right now, this is a qualcomm usb phy specific bindings - first used in
> > qcom,usb-hs-phy.txt and I extended it a bit for my phy. As this is not
> > a so good hardware description, I'm a little hesitated to make it
> > generic for other platforms to use in general. What about we put off it
> > a little bit until we see more platforms need the same thing?
>
> I'm not saying I want it generic. Quite the opposite. I don't think we
> should have it either generically or vendor specific. The main thing I
> have a problem with is the timing information because then we're more
> that just data. Without that we're talking about a bunch of properties
> for register fields or just raw register values in DT. That becomes
> more of a judgement call. There's not too much value in making a
> driver translate a bunch of properties just to stuff them into
> registers on init. But then just allowing any raw register value to be
> in DT could be easily abused.
Rob,
I agree with your comments. Honestly, I'm not comfortable with this
'qcom,init-seq' thing in the first impression. The similar existence in
mainline qcom,usb-hs-phy.txt makes me think it might be acceptable with
the timing data added. Okay, I know your position on this now.
@Sriharsha,
Seeing that 'qcom,init-seq' is being configured with the exactly same
values for both HS phys in SoC level dts file (qcs404.dtsi), I think
such settings can be moved into driver code as SoC specific data.
Unless you have a different view on this, I will do it with v4.
Shawn
On Mon, Nov 19, 2018 at 12:29 PM Shawn Guo <[email protected]> wrote:
>
> On Sat, Nov 17, 2018 at 09:13:38AM -0600, Rob Herring wrote:
> <snip>
> > > > > +- qcom,init-seq:
> > > > > + Value type: <u32 array>
> > > > > + Definition: Should contain a sequence of <offset value delay> tuples to
> > > > > + program 'value' into phy register at 'offset' with 'delay'
> > > > > + in us afterwards.
> > > >
> > > > If we wanted this type of thing in DT, we'd have a generic binding (or
> > > > forth).
> > >
> > > Right now, this is a qualcomm usb phy specific bindings - first used in
> > > qcom,usb-hs-phy.txt and I extended it a bit for my phy. As this is not
> > > a so good hardware description, I'm a little hesitated to make it
> > > generic for other platforms to use in general. What about we put off it
> > > a little bit until we see more platforms need the same thing?
> >
> > I'm not saying I want it generic. Quite the opposite. I don't think we
> > should have it either generically or vendor specific. The main thing I
> > have a problem with is the timing information because then we're more
> > that just data. Without that we're talking about a bunch of properties
> > for register fields or just raw register values in DT. That becomes
> > more of a judgement call. There's not too much value in making a
> > driver translate a bunch of properties just to stuff them into
> > registers on init. But then just allowing any raw register value to be
> > in DT could be easily abused.
>
> Rob,
>
> I agree with your comments. Honestly, I'm not comfortable with this
> 'qcom,init-seq' thing in the first impression. The similar existence in
> mainline qcom,usb-hs-phy.txt makes me think it might be acceptable with
> the timing data added. Okay, I know your position on this now.
>
> @Sriharsha,
>
> Seeing that 'qcom,init-seq' is being configured with the exactly same
> values for both HS phys in SoC level dts file (qcs404.dtsi), I think
> such settings can be moved into driver code as SoC specific data.
> Unless you have a different view on this, I will do it with v4.
phy-qcom-qmp and phy-qcom-qusb2 have been maintaining such SoC specific
init sequences in the drivers if you would like to have pointers from them.
Thanks
Vivek
>
> Shawn
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
On Mon, Nov 19, 2018 at 12:40:59PM +0530, Vivek Gautam wrote:
> On Mon, Nov 19, 2018 at 12:29 PM Shawn Guo <[email protected]> wrote:
> >
> > On Sat, Nov 17, 2018 at 09:13:38AM -0600, Rob Herring wrote:
> > <snip>
> > > > > > +- qcom,init-seq:
> > > > > > + Value type: <u32 array>
> > > > > > + Definition: Should contain a sequence of <offset value delay> tuples to
> > > > > > + program 'value' into phy register at 'offset' with 'delay'
> > > > > > + in us afterwards.
> > > > >
> > > > > If we wanted this type of thing in DT, we'd have a generic binding (or
> > > > > forth).
> > > >
> > > > Right now, this is a qualcomm usb phy specific bindings - first used in
> > > > qcom,usb-hs-phy.txt and I extended it a bit for my phy. As this is not
> > > > a so good hardware description, I'm a little hesitated to make it
> > > > generic for other platforms to use in general. What about we put off it
> > > > a little bit until we see more platforms need the same thing?
> > >
> > > I'm not saying I want it generic. Quite the opposite. I don't think we
> > > should have it either generically or vendor specific. The main thing I
> > > have a problem with is the timing information because then we're more
> > > that just data. Without that we're talking about a bunch of properties
> > > for register fields or just raw register values in DT. That becomes
> > > more of a judgement call. There's not too much value in making a
> > > driver translate a bunch of properties just to stuff them into
> > > registers on init. But then just allowing any raw register value to be
> > > in DT could be easily abused.
> >
> > Rob,
> >
> > I agree with your comments. Honestly, I'm not comfortable with this
> > 'qcom,init-seq' thing in the first impression. The similar existence in
> > mainline qcom,usb-hs-phy.txt makes me think it might be acceptable with
> > the timing data added. Okay, I know your position on this now.
> >
> > @Sriharsha,
> >
> > Seeing that 'qcom,init-seq' is being configured with the exactly same
> > values for both HS phys in SoC level dts file (qcs404.dtsi), I think
> > such settings can be moved into driver code as SoC specific data.
> > Unless you have a different view on this, I will do it with v4.
>
> phy-qcom-qmp and phy-qcom-qusb2 have been maintaining such SoC specific
> init sequences in the drivers if you would like to have pointers from them.
Thanks for the pointer, Vivek. That helps.
Shawn