From: John Crispin <[email protected]>
Add a driver to setup the USB phy on Qualcom Dakota SoCs.
The driver sets up HS and SS phys.
Signed-off-by: John Crispin <[email protected]>
Signed-off-by: Robert Marko <[email protected]>
Cc: Luka Perkov <[email protected]>
---
Changes from v2 to v3:
* Remove magic writes as they are not needed
* Correct commit message
drivers/phy/qualcomm/Kconfig | 7 +
drivers/phy/qualcomm/Makefile | 1 +
drivers/phy/qualcomm/phy-qcom-ipq4019-usb.c | 152 ++++++++++++++++++++
3 files changed, 160 insertions(+)
create mode 100644 drivers/phy/qualcomm/phy-qcom-ipq4019-usb.c
diff --git a/drivers/phy/qualcomm/Kconfig b/drivers/phy/qualcomm/Kconfig
index e46824da29f6..964bd5d784d2 100644
--- a/drivers/phy/qualcomm/Kconfig
+++ b/drivers/phy/qualcomm/Kconfig
@@ -18,6 +18,13 @@ config PHY_QCOM_APQ8064_SATA
depends on OF
select GENERIC_PHY
+config PHY_QCOM_IPQ4019_USB
+ tristate "Qualcomm IPQ4019 USB PHY module"
+ depends on OF && ARCH_QCOM
+ select GENERIC_PHY
+ help
+ Support for the USB PHY on QCOM IPQ4019/Dakota chipsets.
+
config PHY_QCOM_IPQ806X_SATA
tristate "Qualcomm IPQ806x SATA SerDes/PHY driver"
depends on ARCH_QCOM
diff --git a/drivers/phy/qualcomm/Makefile b/drivers/phy/qualcomm/Makefile
index 283251d6a5d9..8afe6c4f5178 100644
--- a/drivers/phy/qualcomm/Makefile
+++ b/drivers/phy/qualcomm/Makefile
@@ -1,6 +1,7 @@
# SPDX-License-Identifier: GPL-2.0
obj-$(CONFIG_PHY_ATH79_USB) += phy-ath79-usb.o
obj-$(CONFIG_PHY_QCOM_APQ8064_SATA) += phy-qcom-apq8064-sata.o
+obj-$(CONFIG_PHY_QCOM_IPQ4019_USB) += phy-qcom-ipq4019-usb.o
obj-$(CONFIG_PHY_QCOM_IPQ806X_SATA) += phy-qcom-ipq806x-sata.o
obj-$(CONFIG_PHY_QCOM_PCIE2) += phy-qcom-pcie2.o
obj-$(CONFIG_PHY_QCOM_QMP) += phy-qcom-qmp.o
diff --git a/drivers/phy/qualcomm/phy-qcom-ipq4019-usb.c b/drivers/phy/qualcomm/phy-qcom-ipq4019-usb.c
new file mode 100644
index 000000000000..7efebae6b6fd
--- /dev/null
+++ b/drivers/phy/qualcomm/phy-qcom-ipq4019-usb.c
@@ -0,0 +1,152 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2018 John Crispin <[email protected]>
+ *
+ * Based on code from
+ * Allwinner Technology Co., Ltd. <http://www.allwinnertech.com>
+ *
+ */
+
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of_platform.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+#include <linux/reset.h>
+
+struct ipq4019_usb_phy {
+ struct device *dev;
+ struct phy *phy;
+ void __iomem *base;
+ struct reset_control *por_rst;
+ struct reset_control *srif_rst;
+};
+
+static int ipq4019_ss_phy_power_off(struct phy *_phy)
+{
+ struct ipq4019_usb_phy *phy = phy_get_drvdata(_phy);
+
+ reset_control_assert(phy->por_rst);
+ msleep(10);
+
+ return 0;
+}
+
+static int ipq4019_ss_phy_power_on(struct phy *_phy)
+{
+ struct ipq4019_usb_phy *phy = phy_get_drvdata(_phy);
+
+ ipq4019_ss_phy_power_off(_phy);
+
+ reset_control_deassert(phy->por_rst);
+
+ return 0;
+}
+
+static struct phy_ops ipq4019_usb_ss_phy_ops = {
+ .power_on = ipq4019_ss_phy_power_on,
+ .power_off = ipq4019_ss_phy_power_off,
+};
+
+static int ipq4019_hs_phy_power_off(struct phy *_phy)
+{
+ struct ipq4019_usb_phy *phy = phy_get_drvdata(_phy);
+
+ reset_control_assert(phy->por_rst);
+ msleep(10);
+
+ reset_control_assert(phy->srif_rst);
+ msleep(10);
+
+ return 0;
+}
+
+static int ipq4019_hs_phy_power_on(struct phy *_phy)
+{
+ struct ipq4019_usb_phy *phy = phy_get_drvdata(_phy);
+
+ ipq4019_hs_phy_power_off(_phy);
+
+ reset_control_deassert(phy->srif_rst);
+ msleep(10);
+
+ reset_control_deassert(phy->por_rst);
+
+ return 0;
+}
+
+static struct phy_ops ipq4019_usb_hs_phy_ops = {
+ .power_on = ipq4019_hs_phy_power_on,
+ .power_off = ipq4019_hs_phy_power_off,
+};
+
+static const struct of_device_id ipq4019_usb_phy_of_match[] = {
+ { .compatible = "qcom,usb-hs-ipq4019-phy", .data = &ipq4019_usb_hs_phy_ops},
+ { .compatible = "qcom,usb-ss-ipq4019-phy", .data = &ipq4019_usb_ss_phy_ops},
+ { },
+};
+MODULE_DEVICE_TABLE(of, ipq4019_usb_phy_of_match);
+
+static int ipq4019_usb_phy_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct resource *res;
+ struct phy_provider *phy_provider;
+ struct ipq4019_usb_phy *phy;
+ const struct of_device_id *match;
+
+ match = of_match_device(ipq4019_usb_phy_of_match, &pdev->dev);
+ if (!match)
+ return -ENODEV;
+
+ phy = devm_kzalloc(dev, sizeof(*phy), GFP_KERNEL);
+ if (!phy)
+ return -ENOMEM;
+
+ phy->dev = &pdev->dev;
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ phy->base = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(phy->base)) {
+ dev_err(dev, "failed to remap register memory\n");
+ return PTR_ERR(phy->base);
+ }
+
+ phy->por_rst = devm_reset_control_get(phy->dev, "por_rst");
+ if (IS_ERR(phy->por_rst)) {
+ if (PTR_ERR(phy->por_rst) != -EPROBE_DEFER)
+ dev_err(dev, "POR reset is missing\n");
+ return PTR_ERR(phy->por_rst);
+ }
+
+ phy->srif_rst = devm_reset_control_get_optional(phy->dev, "srif_rst");
+ if (IS_ERR(phy->srif_rst))
+ return PTR_ERR(phy->srif_rst);
+
+ phy->phy = devm_phy_create(dev, NULL, match->data);
+ if (IS_ERR(phy->phy)) {
+ dev_err(dev, "failed to create PHY\n");
+ return PTR_ERR(phy->phy);
+ }
+ phy_set_drvdata(phy->phy, phy);
+
+ phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
+
+ return PTR_ERR_OR_ZERO(phy_provider);
+}
+
+static struct platform_driver ipq4019_usb_phy_driver = {
+ .probe = ipq4019_usb_phy_probe,
+ .driver = {
+ .of_match_table = ipq4019_usb_phy_of_match,
+ .name = "ipq4019-usb-phy",
+ }
+};
+module_platform_driver(ipq4019_usb_phy_driver);
+
+MODULE_DESCRIPTION("QCOM/IPQ4019 USB phy driver");
+MODULE_AUTHOR("John Crispin <[email protected]>");
+MODULE_LICENSE("GPL v2");
--
2.26.0
Kishon, can you pick this up for 5.8?
Thanks
On Wed, Apr 1, 2020 at 6:36 PM Robert Marko <[email protected]> wrote:
>
> From: John Crispin <[email protected]>
>
> Add a driver to setup the USB phy on Qualcom Dakota SoCs.
> The driver sets up HS and SS phys.
>
> Signed-off-by: John Crispin <[email protected]>
> Signed-off-by: Robert Marko <[email protected]>
> Cc: Luka Perkov <[email protected]>
> ---
> Changes from v2 to v3:
> * Remove magic writes as they are not needed
> * Correct commit message
>
> drivers/phy/qualcomm/Kconfig | 7 +
> drivers/phy/qualcomm/Makefile | 1 +
> drivers/phy/qualcomm/phy-qcom-ipq4019-usb.c | 152 ++++++++++++++++++++
> 3 files changed, 160 insertions(+)
> create mode 100644 drivers/phy/qualcomm/phy-qcom-ipq4019-usb.c
>
> diff --git a/drivers/phy/qualcomm/Kconfig b/drivers/phy/qualcomm/Kconfig
> index e46824da29f6..964bd5d784d2 100644
> --- a/drivers/phy/qualcomm/Kconfig
> +++ b/drivers/phy/qualcomm/Kconfig
> @@ -18,6 +18,13 @@ config PHY_QCOM_APQ8064_SATA
> depends on OF
> select GENERIC_PHY
>
> +config PHY_QCOM_IPQ4019_USB
> + tristate "Qualcomm IPQ4019 USB PHY module"
> + depends on OF && ARCH_QCOM
> + select GENERIC_PHY
> + help
> + Support for the USB PHY on QCOM IPQ4019/Dakota chipsets.
> +
> config PHY_QCOM_IPQ806X_SATA
> tristate "Qualcomm IPQ806x SATA SerDes/PHY driver"
> depends on ARCH_QCOM
> diff --git a/drivers/phy/qualcomm/Makefile b/drivers/phy/qualcomm/Makefile
> index 283251d6a5d9..8afe6c4f5178 100644
> --- a/drivers/phy/qualcomm/Makefile
> +++ b/drivers/phy/qualcomm/Makefile
> @@ -1,6 +1,7 @@
> # SPDX-License-Identifier: GPL-2.0
> obj-$(CONFIG_PHY_ATH79_USB) += phy-ath79-usb.o
> obj-$(CONFIG_PHY_QCOM_APQ8064_SATA) += phy-qcom-apq8064-sata.o
> +obj-$(CONFIG_PHY_QCOM_IPQ4019_USB) += phy-qcom-ipq4019-usb.o
> obj-$(CONFIG_PHY_QCOM_IPQ806X_SATA) += phy-qcom-ipq806x-sata.o
> obj-$(CONFIG_PHY_QCOM_PCIE2) += phy-qcom-pcie2.o
> obj-$(CONFIG_PHY_QCOM_QMP) += phy-qcom-qmp.o
> diff --git a/drivers/phy/qualcomm/phy-qcom-ipq4019-usb.c b/drivers/phy/qualcomm/phy-qcom-ipq4019-usb.c
> new file mode 100644
> index 000000000000..7efebae6b6fd
> --- /dev/null
> +++ b/drivers/phy/qualcomm/phy-qcom-ipq4019-usb.c
> @@ -0,0 +1,152 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) 2018 John Crispin <[email protected]>
> + *
> + * Based on code from
> + * Allwinner Technology Co., Ltd. <http://www.allwinnertech.com>
> + *
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/of_platform.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/reset.h>
> +
> +struct ipq4019_usb_phy {
> + struct device *dev;
> + struct phy *phy;
> + void __iomem *base;
> + struct reset_control *por_rst;
> + struct reset_control *srif_rst;
> +};
> +
> +static int ipq4019_ss_phy_power_off(struct phy *_phy)
> +{
> + struct ipq4019_usb_phy *phy = phy_get_drvdata(_phy);
> +
> + reset_control_assert(phy->por_rst);
> + msleep(10);
> +
> + return 0;
> +}
> +
> +static int ipq4019_ss_phy_power_on(struct phy *_phy)
> +{
> + struct ipq4019_usb_phy *phy = phy_get_drvdata(_phy);
> +
> + ipq4019_ss_phy_power_off(_phy);
> +
> + reset_control_deassert(phy->por_rst);
> +
> + return 0;
> +}
> +
> +static struct phy_ops ipq4019_usb_ss_phy_ops = {
> + .power_on = ipq4019_ss_phy_power_on,
> + .power_off = ipq4019_ss_phy_power_off,
> +};
> +
> +static int ipq4019_hs_phy_power_off(struct phy *_phy)
> +{
> + struct ipq4019_usb_phy *phy = phy_get_drvdata(_phy);
> +
> + reset_control_assert(phy->por_rst);
> + msleep(10);
> +
> + reset_control_assert(phy->srif_rst);
> + msleep(10);
> +
> + return 0;
> +}
> +
> +static int ipq4019_hs_phy_power_on(struct phy *_phy)
> +{
> + struct ipq4019_usb_phy *phy = phy_get_drvdata(_phy);
> +
> + ipq4019_hs_phy_power_off(_phy);
> +
> + reset_control_deassert(phy->srif_rst);
> + msleep(10);
> +
> + reset_control_deassert(phy->por_rst);
> +
> + return 0;
> +}
> +
> +static struct phy_ops ipq4019_usb_hs_phy_ops = {
> + .power_on = ipq4019_hs_phy_power_on,
> + .power_off = ipq4019_hs_phy_power_off,
> +};
> +
> +static const struct of_device_id ipq4019_usb_phy_of_match[] = {
> + { .compatible = "qcom,usb-hs-ipq4019-phy", .data = &ipq4019_usb_hs_phy_ops},
> + { .compatible = "qcom,usb-ss-ipq4019-phy", .data = &ipq4019_usb_ss_phy_ops},
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, ipq4019_usb_phy_of_match);
> +
> +static int ipq4019_usb_phy_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct resource *res;
> + struct phy_provider *phy_provider;
> + struct ipq4019_usb_phy *phy;
> + const struct of_device_id *match;
> +
> + match = of_match_device(ipq4019_usb_phy_of_match, &pdev->dev);
> + if (!match)
> + return -ENODEV;
> +
> + phy = devm_kzalloc(dev, sizeof(*phy), GFP_KERNEL);
> + if (!phy)
> + return -ENOMEM;
> +
> + phy->dev = &pdev->dev;
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + phy->base = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(phy->base)) {
> + dev_err(dev, "failed to remap register memory\n");
> + return PTR_ERR(phy->base);
> + }
> +
> + phy->por_rst = devm_reset_control_get(phy->dev, "por_rst");
> + if (IS_ERR(phy->por_rst)) {
> + if (PTR_ERR(phy->por_rst) != -EPROBE_DEFER)
> + dev_err(dev, "POR reset is missing\n");
> + return PTR_ERR(phy->por_rst);
> + }
> +
> + phy->srif_rst = devm_reset_control_get_optional(phy->dev, "srif_rst");
> + if (IS_ERR(phy->srif_rst))
> + return PTR_ERR(phy->srif_rst);
> +
> + phy->phy = devm_phy_create(dev, NULL, match->data);
> + if (IS_ERR(phy->phy)) {
> + dev_err(dev, "failed to create PHY\n");
> + return PTR_ERR(phy->phy);
> + }
> + phy_set_drvdata(phy->phy, phy);
> +
> + phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
> +
> + return PTR_ERR_OR_ZERO(phy_provider);
> +}
> +
> +static struct platform_driver ipq4019_usb_phy_driver = {
> + .probe = ipq4019_usb_phy_probe,
> + .driver = {
> + .of_match_table = ipq4019_usb_phy_of_match,
> + .name = "ipq4019-usb-phy",
> + }
> +};
> +module_platform_driver(ipq4019_usb_phy_driver);
> +
> +MODULE_DESCRIPTION("QCOM/IPQ4019 USB phy driver");
> +MODULE_AUTHOR("John Crispin <[email protected]>");
> +MODULE_LICENSE("GPL v2");
> --
> 2.26.0
>
Hello Robert,
On 01-04-20, 18:35, Robert Marko wrote:
> +static int ipq4019_ss_phy_power_on(struct phy *_phy)
> +{
> + struct ipq4019_usb_phy *phy = phy_get_drvdata(_phy);
> +
> + ipq4019_ss_phy_power_off(_phy);
> +
> + reset_control_deassert(phy->por_rst);
> +
> + return 0;
> +}
> +
> +static struct phy_ops ipq4019_usb_ss_phy_ops = {
> + .power_on = ipq4019_ss_phy_power_on,
> + .power_off = ipq4019_ss_phy_power_off,
> +};
> +
> +static int ipq4019_hs_phy_power_off(struct phy *_phy)
> +{
> + struct ipq4019_usb_phy *phy = phy_get_drvdata(_phy);
> +
> + reset_control_assert(phy->por_rst);
> + msleep(10);
why not call ipq4019_ss_phy_power_off() here as well?
> +
> + reset_control_assert(phy->srif_rst);
> + msleep(10);
> +
> + return 0;
> +}
> +
> +static int ipq4019_hs_phy_power_on(struct phy *_phy)
> +{
> + struct ipq4019_usb_phy *phy = phy_get_drvdata(_phy);
> +
> + ipq4019_hs_phy_power_off(_phy);
> +
> + reset_control_deassert(phy->srif_rst);
> + msleep(10);
> +
> + reset_control_deassert(phy->por_rst);
> +
> + return 0;
> +}
> +
> +static struct phy_ops ipq4019_usb_hs_phy_ops = {
> + .power_on = ipq4019_hs_phy_power_on,
> + .power_off = ipq4019_hs_phy_power_off,
> +};
So this is fiddling with resets, what about phy configuration and
calibration, who take care of that?
> +static int ipq4019_usb_phy_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct resource *res;
> + struct phy_provider *phy_provider;
> + struct ipq4019_usb_phy *phy;
> + const struct of_device_id *match;
> +
> + match = of_match_device(ipq4019_usb_phy_of_match, &pdev->dev);
> + if (!match)
> + return -ENODEV;
you are using this to get match-data few lines below, why not use
of_device_get_match_data() and get the match->data which you are
interested in?
--
~Vinod
On Mon, Apr 27, 2020 at 6:45 PM Vinod Koul <[email protected]> wrote:
>
> Hello Robert,
>
> On 01-04-20, 18:35, Robert Marko wrote:
>
> > +static int ipq4019_ss_phy_power_on(struct phy *_phy)
> > +{
> > + struct ipq4019_usb_phy *phy = phy_get_drvdata(_phy);
> > +
> > + ipq4019_ss_phy_power_off(_phy);
> > +
> > + reset_control_deassert(phy->por_rst);
> > +
> > + return 0;
> > +}
> > +
> > +static struct phy_ops ipq4019_usb_ss_phy_ops = {
> > + .power_on = ipq4019_ss_phy_power_on,
> > + .power_off = ipq4019_ss_phy_power_off,
> > +};
> > +
> > +static int ipq4019_hs_phy_power_off(struct phy *_phy)
> > +{
> > + struct ipq4019_usb_phy *phy = phy_get_drvdata(_phy);
> > +
> > + reset_control_assert(phy->por_rst);
> > + msleep(10);
>
> why not call ipq4019_ss_phy_power_off() here as well?
Its not necessary, SS and HS PHY-s are separated but share
the same register space.
So when HS PHY is controlled, SS PHY can remain working.
>
> > +
> > + reset_control_assert(phy->srif_rst);
> > + msleep(10);
> > +
> > + return 0;
> > +}
> > +
> > +static int ipq4019_hs_phy_power_on(struct phy *_phy)
> > +{
> > + struct ipq4019_usb_phy *phy = phy_get_drvdata(_phy);
> > +
> > + ipq4019_hs_phy_power_off(_phy);
> > +
> > + reset_control_deassert(phy->srif_rst);
> > + msleep(10);
> > +
> > + reset_control_deassert(phy->por_rst);
> > +
> > + return 0;
> > +}
> > +
> > +static struct phy_ops ipq4019_usb_hs_phy_ops = {
> > + .power_on = ipq4019_hs_phy_power_on,
> > + .power_off = ipq4019_hs_phy_power_off,
> > +};
>
> So this is fiddling with resets, what about phy configuration and
> calibration, who take care of that?
As as I understand, since I don't have documentation access is that no
calibration and configuration except to properly reset them are needed.
Development hardware required some magic register values to be
written but in the previous revisions of this driver it was
discovered that they were leftovers from the development HW.
>
> > +static int ipq4019_usb_phy_probe(struct platform_device *pdev)
> > +{
> > + struct device *dev = &pdev->dev;
> > + struct resource *res;
> > + struct phy_provider *phy_provider;
> > + struct ipq4019_usb_phy *phy;
> > + const struct of_device_id *match;
> > +
> > + match = of_match_device(ipq4019_usb_phy_of_match, &pdev->dev);
> > + if (!match)
> > + return -ENODEV;
>
> you are using this to get match-data few lines below, why not use
> of_device_get_match_data() and get the match->data which you are
> interested in?
Thanks, I will look into it.
>
> --
> ~Vinod