lkml.org 
[lkml]   [2023]   [Sep]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 2/8] phy: qcom: Introduce Super-Speed USB UNIPHY driver
On 29/09/2023 11:42, Praveenkumar I wrote:
> Adds Qualcomm 22ull Super-Speed USB UNIPHY driver support which
> is present in Qualcomm IPQ5332 SoC. This PHY is interfaced with
> SNPS DWC3 USB and SNPS DWC PCIe. Either one of the interface
> can use the it and selection is done via mux present in TCSR
> register. This driver selects the PHY for DWC3 USB and handles
> the reset, clocks and regulator.
>
> Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
> ---
> drivers/phy/qualcomm/Kconfig | 11 +
> drivers/phy/qualcomm/Makefile | 1 +
> drivers/phy/qualcomm/phy-qcom-uniphy-usb.c | 322 +++++++++++++++++++++
> 3 files changed, 334 insertions(+)
> create mode 100644 drivers/phy/qualcomm/phy-qcom-uniphy-usb.c
>
> diff --git a/drivers/phy/qualcomm/Kconfig b/drivers/phy/qualcomm/Kconfig
> index d891058b7c39..7257c8455c53 100644
> --- a/drivers/phy/qualcomm/Kconfig
> +++ b/drivers/phy/qualcomm/Kconfig
> @@ -154,6 +154,17 @@ config PHY_QCOM_M31_USB
> management. This driver is required even for peripheral only or
> host only mode configurations.
>
> +config PHY_QCOM_UNIPHY_USB
> + tristate "Qualcomm USB Super-Speed UNIPHY driver"

Can we please have more specific driver name? As I wrote earlier, there
are two other (different) kinds of Qualcomm UNI PHY devices:
- DSI / HDMI UNIPHY on apq8064 / msm8974 / msm8960 (?)
- USB QMP UNI PHY drivers

Adding a driver called UNIPHY, which is not related to those two kinds
sounds pretty confusing to me.

> + depends on USB && (ARCH_QCOM || COMPILE_TEST)
> + select GENERIC_PHY
> + help
> + Enable this to support the Qualcomm USB Super-Speed UNIPHY transceiver
> + with DWC3 USB core. It handles PHY initialization, clock
> + management required after resetting the hardware and power
> + management. This driver is required even for peripheral only or
> + host only mode configurations.
> +
> config PHY_QCOM_USB_HS
> tristate "Qualcomm USB HS PHY module"
> depends on USB_ULPI_BUS
> diff --git a/drivers/phy/qualcomm/Makefile b/drivers/phy/qualcomm/Makefile
> index ffd609ac6233..c3e0112a7a70 100644
> --- a/drivers/phy/qualcomm/Makefile
> +++ b/drivers/phy/qualcomm/Makefile
> @@ -17,6 +17,7 @@ obj-$(CONFIG_PHY_QCOM_QMP_USB_LEGACY) += phy-qcom-qmp-usb-legacy.o
> obj-$(CONFIG_PHY_QCOM_QUSB2) += phy-qcom-qusb2.o
> obj-$(CONFIG_PHY_QCOM_SNPS_EUSB2) += phy-qcom-snps-eusb2.o
> obj-$(CONFIG_PHY_QCOM_EUSB2_REPEATER) += phy-qcom-eusb2-repeater.o
> +obj-$(CONFIG_PHY_QCOM_UNIPHY_USB) += phy-qcom-uniphy-usb.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_28NM) += phy-qcom-usb-hs-28nm.o
> diff --git a/drivers/phy/qualcomm/phy-qcom-uniphy-usb.c b/drivers/phy/qualcomm/phy-qcom-uniphy-usb.c
> new file mode 100644
> index 000000000000..fdfc9c225995
> --- /dev/null
> +++ b/drivers/phy/qualcomm/phy-qcom-uniphy-usb.c

So, is it a USB PHY or UNI PHY (where I would expect that it handles USB
and PCIe?)

> @@ -0,0 +1,322 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (c) 2023, Qualcomm Innovation Center, Inc. All rights reserved.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/clk-provider.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/reset.h>
> +
> +#define PCIE_USB_COMBO_PHY_CFG_MISC1 0x214
> +#define PCIE_USB_COMBO_PHY_CFG_RX_AFE_2 0x7C4
> +#define PCIE_USB_COMBO_PHY_CFG_RX_DLF_DEMUX_2 0x7E8
> +
> +/* TCSR_USB_MUX_SEL regiter bits */
> +#define TCSR_USB_MUX_SEL BIT(0)
> +
> +struct phy_init_tbl {
> + unsigned int offset;
> + unsigned int val;
> +};
> +
> +#define PHY_INIT_CFG(o, v) \
> + { \
> + .offset = o, \
> + .val = v, \
> + }
> +
> +static const struct phy_init_tbl ipq5332_usb_uniphy_init_tbl[] = {
> + PHY_INIT_CFG(PCIE_USB_COMBO_PHY_CFG_RX_AFE_2, 0x1076),
> + PHY_INIT_CFG(PCIE_USB_COMBO_PHY_CFG_RX_DLF_DEMUX_2, 0x3142),
> + PHY_INIT_CFG(PCIE_USB_COMBO_PHY_CFG_MISC1, 0x3),
> +};

We already have this issue in QMP drivers. Could you please move data
definitions to come after all struct definitions?

> +
> +struct uniphy_cfg {
> + const struct phy_init_tbl *init_seq;
> + int num_init_seq;
> +};
> +
> +struct uniphy_usb {
> + struct device *dev;
> + const struct uniphy_cfg *cfg;
> + struct phy *phy;
> + void __iomem *base;
> + struct clk_bulk_data *clks;
> + unsigned int num_clks;
> + struct reset_control *reset;
> + struct regulator *vreg;
> + struct clk_fixed_rate pipe_clk_fixed;
> + struct regmap *tcsr;
> + unsigned int usb_mux_offset;
> +};
> +
> +static const struct uniphy_cfg ipq5332_usb_uniphy_cfg = {
> + .init_seq = ipq5332_usb_uniphy_init_tbl,
> + .num_init_seq = ARRAY_SIZE(ipq5332_usb_uniphy_init_tbl),
> +};
> +
> +static int uniphy_usb_mux_enable(struct uniphy_usb *uniphy, bool enable)
> +{
> + struct device *dev = uniphy->dev;
> + unsigned int val;
> + int ret;
> +
> + if (!uniphy->tcsr)
> + return -EINVAL;
> +
> + ret = regmap_read(uniphy->tcsr, uniphy->usb_mux_offset, &val);
> + if (ret) {
> + dev_err(dev, "Mux read failed: %d\n", ret);
> + return ret;
> + }
> +
> + val = enable ? (val | TCSR_USB_MUX_SEL) : (val & ~TCSR_USB_MUX_SEL);
> + ret = regmap_write(uniphy->tcsr, uniphy->usb_mux_offset, val);

regmap_update_bits()

> + if (ret) {
> + dev_err(dev, "Mux write failed: %d\n", ret);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int uniphy_usb_init(struct phy *phy)
> +{
> + struct uniphy_usb *uniphy = phy_get_drvdata(phy);
> + const struct uniphy_cfg *cfg = uniphy->cfg;
> + const struct phy_init_tbl *tbl = cfg->init_seq;
> + void __iomem *base = uniphy->base;
> + struct device *dev = uniphy->dev;
> + int i, ret;
> +
> + ret = regulator_enable(uniphy->vreg);
> + if (ret) {
> + dev_err(dev, "failed to enable regulator, %d\n", ret);
> + return ret;
> + }
> +
> + /* Perform phy reset */
> + reset_control_assert(uniphy->reset);
> + usleep_range(1, 5);
> + reset_control_deassert(uniphy->reset);

Error checkig, please.

> +
> + ret = uniphy_usb_mux_enable(uniphy, true);
> + if (ret < 0)
> + goto err_assert_reset;
> +
> + ret = clk_bulk_prepare_enable(uniphy->num_clks, uniphy->clks);
> + if (ret) {
> + dev_err(dev, "failed to enable clocks: %d\n", ret);
> + goto err_assert_reset;
> + }
> +
> + /* phy autoload delay */
> + usleep_range(35, 40);
> +
> + for (i = 0; i < cfg->num_init_seq; i++)
> + writel(tbl[i].val, base + tbl[i].offset);
> +
> + return 0;
> +
> +err_assert_reset:
> + /* Assert phy reset */
> + reset_control_assert(uniphy->reset);
> +
> + return ret;
> +}
> +
> +static int uniphy_usb_shutdown(struct phy *phy)
> +{
> + struct uniphy_usb *uniphy = phy_get_drvdata(phy);
> +
> + clk_bulk_disable_unprepare(uniphy->num_clks, uniphy->clks);
> +
> + uniphy_usb_mux_enable(uniphy, false);
> +
> + /* Assert phy reset */
> + reset_control_assert(uniphy->reset);
> +
> + regulator_disable(uniphy->vreg);
> +
> + return 0;
> +}
> +
> +static const struct phy_ops uniphy_usb_ops = {
> + .power_on = uniphy_usb_init,
> + .power_off = uniphy_usb_shutdown,
> + .owner = THIS_MODULE,
> +};
> +
> +static int qcom_uniphy_usb_mux_init(struct uniphy_usb *uniphy)

Inline this function please.

> +{
> + struct device *dev = uniphy->dev;
> + int ret;
> +
> + uniphy->tcsr = syscon_regmap_lookup_by_phandle_args(dev->of_node, "qcom,phy-usb-mux-sel",
> + 1, &uniphy->usb_mux_offset);
> + if (IS_ERR(uniphy->tcsr)) {
> + ret = PTR_ERR(uniphy->tcsr);
> + uniphy->tcsr = NULL;
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int qcom_uniphy_usb_clk_init(struct uniphy_usb *uniphy)

Inline

> +{
> + struct device *dev = uniphy->dev;
> + int ret;
> +
> + ret = devm_clk_bulk_get_all(dev, &uniphy->clks);
> + if (ret < 0)
> + return ret;
> +
> + uniphy->num_clks = ret;
> +
> + return 0;
> +}
> +
> +static void phy_clk_release_provider(void *res)
> +{
> + of_clk_del_provider(res);
> +}
> +
> +/*
> + * Register a fixed rate pipe clock.
> + *
> + * The <s>_pipe_clksrc generated by PHY goes to the GCC that gate
> + * controls it. The <s>_pipe_clk coming out of the GCC is requested
> + * by the PHY driver for its operations.
> + * We register the <s>_pipe_clksrc here. The gcc driver takes care
> + * of assigning this <s>_pipe_clksrc as parent to <s>_pipe_clk.
> + * Below picture shows this relationship.
> + *
> + * +---------------+
> + * | PHY block |<<---------------------------------------+
> + * | | |
> + * | +-------+ | +-----+ |
> + * I/P---^-->| PLL |---^--->pipe_clksrc--->| GCC |--->pipe_clk---+
> + * clk | +-------+ | +-----+
> + * +---------------+
> + */
> +static int phy_pipe_clk_register(struct uniphy_usb *uniphy, struct device_node *np)
> +{
> + struct clk_fixed_rate *fixed = &uniphy->pipe_clk_fixed;
> + struct device *dev = uniphy->dev;
> + struct clk_init_data init = { };
> + int ret;
> +
> + ret = of_property_read_string(np, "clock-output-names", &init.name);
> + if (ret) {
> + dev_err(dev, "%pOFn: No clock-output-names\n", np);
> + return ret;
> + }
> +
> + init.ops = &clk_fixed_rate_ops;
> +
> + fixed->fixed_rate = 250000000;
> + fixed->hw.init = &init;
> +
> + ret = devm_clk_hw_register(dev, &fixed->hw);

devm_clk_hw_register_fixed_rate()

> + if (ret)
> + return ret;
> +
> + ret = of_clk_add_hw_provider(np, of_clk_hw_simple_get, &fixed->hw);
> + if (ret)
> + return ret;
> +
> + return devm_add_action_or_reset(dev, phy_clk_release_provider, np);

devm_of_clk_add_hw_provider().

> +}
> +
> +static int qcom_uniphy_usb_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct phy_provider *phy_provider;
> + struct uniphy_usb *uniphy;
> + struct device_node *np;
> + int ret;
> +
> + uniphy = devm_kzalloc(dev, sizeof(*uniphy), GFP_KERNEL);
> + if (!uniphy)
> + return -ENOMEM;
> +
> + uniphy->dev = dev;
> +
> + uniphy->cfg = of_device_get_match_data(dev);
> + if (!uniphy->cfg)
> + return -EINVAL;
> +
> + uniphy->base = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(uniphy->base))
> + return PTR_ERR(uniphy->base);
> +
> + ret = qcom_uniphy_usb_clk_init(uniphy);
> + if (ret)
> + return dev_err_probe(dev, ret, "failed to get clock\n");
> +
> + ret = qcom_uniphy_usb_mux_init(uniphy);
> + if (ret)
> + return dev_err_probe(dev, ret, "failed to get USB mux\n");
> +
> + uniphy->reset = devm_reset_control_get_exclusive_by_index(dev, 0);
> + if (IS_ERR(uniphy->reset))
> + return dev_err_probe(dev, PTR_ERR(uniphy->reset), "failed to get reset\n");
> +
> + uniphy->vreg = devm_regulator_get_exclusive(dev, "vdd");

Why do you need the exclusive control here?

> + if (IS_ERR(uniphy->vreg))
> + return dev_err_probe(dev, PTR_ERR(uniphy->phy), "failed to get vreg\n");
> +
> + np = of_node_get(dev->of_node);

No need to get/put it, just use dev->of_node directly.

> + ret = phy_pipe_clk_register(uniphy, np);
> + if (ret) {
> + dev_err_probe(dev, ret, "failed to register pipe clk\n");
> + goto err;
> + }
> +
> + uniphy->phy = devm_phy_create(dev, NULL, &uniphy_usb_ops);
> + if (IS_ERR(uniphy->phy)) {
> + ret = PTR_ERR(uniphy->phy);
> + dev_err_probe(dev, ret, "failed to create PHY\n");
> + goto err;
> + }
> +
> + phy_set_drvdata(uniphy->phy, uniphy);
> +
> + phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
> +
> + ret = PTR_ERR_OR_ZERO(phy_provider);
> +
> +err:
> + of_node_put(np);
> + return ret;
> +}
> +
> +static const struct of_device_id qcom_uniphy_usb_of_match[] = {
> + { .compatible = "qcom,ipq5332-usb-uniphy", .data = &ipq5332_usb_uniphy_cfg},
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, qcom_uniphy_usb_of_match);
> +
> +static struct platform_driver qcom_uniphy_usb_driver = {
> + .probe = qcom_uniphy_usb_probe,
> + .driver = {
> + .of_match_table = qcom_uniphy_usb_of_match,
> + .name = "qcom,uniphy-usb",
> + }
> +};
> +module_platform_driver(qcom_uniphy_usb_driver);
> +
> +MODULE_DESCRIPTION("Qualcomm Super-Speed USB UNIPHY driver");
> +MODULE_LICENSE("GPL");

--
With best wishes
Dmitry

\
 
 \ /
  Last update: 2023-09-30 19:18    [W:0.244 / U:0.236 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site