| Message ID | 20230929084209.3033093-1-quic_ipkumar@quicinc.com |
|---|---|
| Headers | show |
| Series | Enable USB3 for Qualcomm IPQ5332 | expand |
On 29/09/2023 10:42, Praveenkumar I wrote: > Enable USB UNIPHY driver present in Qualcomm IPQ5332. > > Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com> > --- Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> --- This is an automated instruction, just in case, because many review tags are being ignored. If you know the process, you can skip it (please do not feel offended by me posting it here - no bad intentions intended). If you do not know the process, here is a short explanation: Please add Acked-by/Reviewed-by/Tested-by tags when posting new versions, under or above your Signed-off-by tag. Tag is "received", when provided in a message replied to you on the mailing list. Tools like b4 can help here. However, there's no need to repost patches *only* to add the tags. The upstream maintainer will do that for tags received on the version they apply. https://elixir.bootlin.com/linux/v6.5-rc3/source/Documentation/process/submitting-patches.rst#L577 Best regards, Krzysztof
On 9/30/2023 10:48 PM, Dmitry Baryshkov wrote: > 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. This UNIPHY is different from above mentioned ones. This a custom version for 22nm on Qualcomm IPQ5332. Can we name the driver as phy-qcom-uniphy-usb-ss-22ull.c / phy-qcom-usb-ss-22ull.c ? > >> + 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?) It is a USB PHY and the PHY name is UNIPHY. Added the usb in the file name to differentiate it. > >> @@ -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? Sure, will move this 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() Will update. > >> + 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. Sure, will add. > >> + >> + 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. Will do. > >> +{ >> + 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 Will do. > >> +{ >> + 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() Will change. > >> + 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(). Will change. > >> +} >> + >> +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? Will change it to devm_regulator_get(). > >> + 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. Sure, will change it. > >> + 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"); > -- Thanks, Praveenkumar
On Tue, 3 Oct 2023 at 17:22, Praveenkumar I <quic_ipkumar@quicinc.com> wrote: > > > > On 9/30/2023 10:48 PM, Dmitry Baryshkov wrote: > > 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. > This UNIPHY is different from above mentioned ones. This a custom > version for 22nm on Qualcomm IPQ5332. > Can we name the driver as phy-qcom-uniphy-usb-ss-22ull.c / > phy-qcom-usb-ss-22ull.c ? usb-ss-22ull sounds better. Or maybe usb-ipq-ss > > > >> + 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?) > It is a USB PHY and the PHY name is UNIPHY. Added the usb in the file > name to differentiate it. > >
On 10/3/2023 8:24 PM, Dmitry Baryshkov wrote: > On Tue, 3 Oct 2023 at 17:22, Praveenkumar I <quic_ipkumar@quicinc.com> wrote: >> >> >> On 9/30/2023 10:48 PM, Dmitry Baryshkov wrote: >>> 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. >> This UNIPHY is different from above mentioned ones. This a custom >> version for 22nm on Qualcomm IPQ5332. >> Can we name the driver as phy-qcom-uniphy-usb-ss-22ull.c / >> phy-qcom-usb-ss-22ull.c ? > usb-ss-22ull sounds better. Or maybe usb-ipq-ss Sure, will rename it to phy-qcom-usb-ss-22ull.c > >>>> + 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?) >> It is a USB PHY and the PHY name is UNIPHY. Added the usb in the file >> name to differentiate it. -- Thanks, Praveenkumar
On 3.10.2023 16:54, Dmitry Baryshkov wrote: > On Tue, 3 Oct 2023 at 17:22, Praveenkumar I <quic_ipkumar@quicinc.com> wrote: >> >> >> >> On 9/30/2023 10:48 PM, Dmitry Baryshkov wrote: >>> 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. >> This UNIPHY is different from above mentioned ones. This a custom >> version for 22nm on Qualcomm IPQ5332. >> Can we name the driver as phy-qcom-uniphy-usb-ss-22ull.c / >> phy-qcom-usb-ss-22ull.c ? > > usb-ss-22ull sounds better. Or maybe usb-ipq-ss usb-ipq-ss is as safe as usb-msm-ss We can not rely on the hardware never ever changing down the product line :D Konrad
On 10/7/2023 5:27 AM, Konrad Dybcio wrote: > On 3.10.2023 16:54, Dmitry Baryshkov wrote: >> On Tue, 3 Oct 2023 at 17:22, Praveenkumar I <quic_ipkumar@quicinc.com> wrote: >>> >>> >>> On 9/30/2023 10:48 PM, Dmitry Baryshkov wrote: >>>> 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. >>> This UNIPHY is different from above mentioned ones. This a custom >>> version for 22nm on Qualcomm IPQ5332. >>> Can we name the driver as phy-qcom-uniphy-usb-ss-22ull.c / >>> phy-qcom-usb-ss-22ull.c ? >> usb-ss-22ull sounds better. Or maybe usb-ipq-ss > usb-ipq-ss is as safe as usb-msm-ss > > We can not rely on the hardware never ever changing down the > product line :D Not all IPQ platforms having this UNIPHY. Only IPQ5332 and IPQ5018 are having this custom version of UNIPHY. Hence thought usb-ipq-ss might confuse. > > Konrad -- Thanks, Praveenkumar