| Message ID | 20250215122409.162810-4-ivo.ivanov.ivanov1@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | phy: samsung: add Exynos2200 SNPS eUSB2 driver | expand |
On 15/02/2025 13:24, Ivaylo Ivanov wrote: > The Exynos2200 SoC uses Synopsis eUSB2 PHY for USB 2.0. Add a new > driver for it. > > eUSB2 on Exynos SoCs is usually paired alongside a USB PHY controller. > Currently the driver is modelled to take and enable/disable the usb phy > controller when needed. > > The driver is based on information from downstream drivers. > > Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com> > --- > drivers/phy/samsung/Kconfig | 13 + > drivers/phy/samsung/Makefile | 1 + > .../phy/samsung/phy-exynos2200-snps-eusb2.c | 351 ++++++++++++++++++ > 3 files changed, 365 insertions(+) > create mode 100644 drivers/phy/samsung/phy-exynos2200-snps-eusb2.c > > diff --git a/drivers/phy/samsung/Kconfig b/drivers/phy/samsung/Kconfig > index e2330b089..f62285254 100644 > --- a/drivers/phy/samsung/Kconfig > +++ b/drivers/phy/samsung/Kconfig > @@ -77,6 +77,19 @@ config PHY_S5PV210_USB2 > particular SoC is compiled in the driver. In case of S5PV210 two phys > are available - device and host. > > +config PHY_EXYNOS2200_SNPS_EUSB2 > + tristate "Exynos2200 eUSB 2.0 PHY driver" > + depends on (ARCH_EXYNOS && OF) || COMPILE_TEST > + depends on HAS_IOMEM > + depends on USB_DWC3_EXYNOS How does it depend? What are you using from DWC3? > + select GENERIC_PHY > + select MFD_SYSCON Where do you use it? > + default y > + help > + Enable USBCON PHY support for Exynos2200 SoC. > + This driver provides PHY interface for eUSB 2.0 controller > + present on Exynos5 SoC series. > + > config PHY_EXYNOS5_USBDRD > tristate "Exynos5 SoC series USB DRD PHY driver" > depends on (ARCH_EXYNOS && OF) || COMPILE_TEST > diff --git a/drivers/phy/samsung/Makefile b/drivers/phy/samsung/Makefile > index fea1f96d0..90b84c7fc 100644 > --- a/drivers/phy/samsung/Makefile > +++ b/drivers/phy/samsung/Makefile > @@ -14,5 +14,6 @@ phy-exynos-usb2-$(CONFIG_PHY_EXYNOS4210_USB2) += phy-exynos4210-usb2.o > phy-exynos-usb2-$(CONFIG_PHY_EXYNOS4X12_USB2) += phy-exynos4x12-usb2.o > phy-exynos-usb2-$(CONFIG_PHY_EXYNOS5250_USB2) += phy-exynos5250-usb2.o > phy-exynos-usb2-$(CONFIG_PHY_S5PV210_USB2) += phy-s5pv210-usb2.o > +obj-$(CONFIG_PHY_EXYNOS2200_SNPS_EUSB2) += phy-exynos2200-snps-eusb2.o Entire driver looks like repeating existing qcom-snps-eusb2. You need to integrate the changes, not create duplicated driver. ... > + > + ret = devm_clk_bulk_get(dev, drv_data->n_clks, > + phy->clks); > + if (ret) > + return dev_err_probe(dev, ret, > + "failed to get phy clock(s)\n"); > + > + for (int i = 0; i < phy->drv_data->n_clks; ++i) { > + if (!strcmp(phy->clks[i].id, "ref")) { > + phy->ref_clk = phy->clks[i].clk; > + break; > + } > + } > + > + phy->vregs = devm_kcalloc(dev, drv_data->n_regulators, > + sizeof(*phy->vregs), GFP_KERNEL); > + if (!phy->vregs) > + return -ENOMEM; > + regulator_bulk_set_supply_names(phy->vregs, > + drv_data->regulator_names, > + drv_data->n_regulators); > + ret = devm_regulator_bulk_get(dev, drv_data->n_regulators, > + phy->vregs); > + if (ret) > + return dev_err_probe(dev, ret, "failed to get regulators\n"); > + > + /* we treat the usblink controller phy as a separate phy */ > + phy->usbcon = devm_of_phy_get_by_index(dev, np, 0); > + if (IS_ERR(phy->usbcon)) > + return dev_err_probe(dev, PTR_ERR(phy->usbcon), > + "failed to get usbcon\n"); > + > + generic_phy = devm_phy_create(dev, NULL, &exynos2200_snps_eusb2_phy_ops); > + if (IS_ERR(generic_phy)) { > + dev_err(dev, "failed to create phy %d\n", ret); Syntax is return dev_err_probe > + return PTR_ERR(generic_phy); > + } > + > + dev_set_drvdata(dev, phy); > + phy_set_drvdata(generic_phy, phy); > + > + phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate); > + if (IS_ERR(phy_provider)) { > + dev_err(dev, "failed to register phy provider\n"); Syntax is return dev_err_probe > + return PTR_ERR(phy_provider); > + }; > + > + return 0; > +} Best regards, Krzysztof
On 2/16/25 11:26, Krzysztof Kozlowski wrote: > On 15/02/2025 13:24, Ivaylo Ivanov wrote: >> The Exynos2200 SoC uses Synopsis eUSB2 PHY for USB 2.0. Add a new >> driver for it. >> >> eUSB2 on Exynos SoCs is usually paired alongside a USB PHY controller. >> Currently the driver is modelled to take and enable/disable the usb phy >> controller when needed. >> >> The driver is based on information from downstream drivers. >> >> Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com> >> --- >> drivers/phy/samsung/Kconfig | 13 + >> drivers/phy/samsung/Makefile | 1 + >> .../phy/samsung/phy-exynos2200-snps-eusb2.c | 351 ++++++++++++++++++ >> 3 files changed, 365 insertions(+) >> create mode 100644 drivers/phy/samsung/phy-exynos2200-snps-eusb2.c >> >> diff --git a/drivers/phy/samsung/Kconfig b/drivers/phy/samsung/Kconfig >> index e2330b089..f62285254 100644 >> --- a/drivers/phy/samsung/Kconfig >> +++ b/drivers/phy/samsung/Kconfig >> @@ -77,6 +77,19 @@ config PHY_S5PV210_USB2 >> particular SoC is compiled in the driver. In case of S5PV210 two phys >> are available - device and host. >> >> +config PHY_EXYNOS2200_SNPS_EUSB2 >> + tristate "Exynos2200 eUSB 2.0 PHY driver" >> + depends on (ARCH_EXYNOS && OF) || COMPILE_TEST >> + depends on HAS_IOMEM >> + depends on USB_DWC3_EXYNOS > > How does it depend? What are you using from DWC3? Can drop, I guess. > >> + select GENERIC_PHY >> + select MFD_SYSCON > Where do you use it? Remained from USBCON driver. > >> + default y >> + help >> + Enable USBCON PHY support for Exynos2200 SoC. >> + This driver provides PHY interface for eUSB 2.0 controller >> + present on Exynos5 SoC series. >> + >> config PHY_EXYNOS5_USBDRD >> tristate "Exynos5 SoC series USB DRD PHY driver" >> depends on (ARCH_EXYNOS && OF) || COMPILE_TEST >> diff --git a/drivers/phy/samsung/Makefile b/drivers/phy/samsung/Makefile >> index fea1f96d0..90b84c7fc 100644 >> --- a/drivers/phy/samsung/Makefile >> +++ b/drivers/phy/samsung/Makefile >> @@ -14,5 +14,6 @@ phy-exynos-usb2-$(CONFIG_PHY_EXYNOS4210_USB2) += phy-exynos4210-usb2.o >> phy-exynos-usb2-$(CONFIG_PHY_EXYNOS4X12_USB2) += phy-exynos4x12-usb2.o >> phy-exynos-usb2-$(CONFIG_PHY_EXYNOS5250_USB2) += phy-exynos5250-usb2.o >> phy-exynos-usb2-$(CONFIG_PHY_S5PV210_USB2) += phy-s5pv210-usb2.o >> +obj-$(CONFIG_PHY_EXYNOS2200_SNPS_EUSB2) += phy-exynos2200-snps-eusb2.o > Entire driver looks like repeating existing qcom-snps-eusb2. It's the same IP, but implemented differently on a different platform. At the very least, the register layout is different. > You need to > integrate the changes, not create duplicated driver. I can do that, but it would be come a bit cluttered, won't it? Depends on if we want to follow the current oem-provided initialization sequence, or try and fully reuse what we have in there. Best regards, Ivaylo > > ... > >> + >> + ret = devm_clk_bulk_get(dev, drv_data->n_clks, >> + phy->clks); >> + if (ret) >> + return dev_err_probe(dev, ret, >> + "failed to get phy clock(s)\n"); >> + >> + for (int i = 0; i < phy->drv_data->n_clks; ++i) { >> + if (!strcmp(phy->clks[i].id, "ref")) { >> + phy->ref_clk = phy->clks[i].clk; >> + break; >> + } >> + } >> + >> + phy->vregs = devm_kcalloc(dev, drv_data->n_regulators, >> + sizeof(*phy->vregs), GFP_KERNEL); >> + if (!phy->vregs) >> + return -ENOMEM; >> + regulator_bulk_set_supply_names(phy->vregs, >> + drv_data->regulator_names, >> + drv_data->n_regulators); >> + ret = devm_regulator_bulk_get(dev, drv_data->n_regulators, >> + phy->vregs); >> + if (ret) >> + return dev_err_probe(dev, ret, "failed to get regulators\n"); >> + >> + /* we treat the usblink controller phy as a separate phy */ >> + phy->usbcon = devm_of_phy_get_by_index(dev, np, 0); >> + if (IS_ERR(phy->usbcon)) >> + return dev_err_probe(dev, PTR_ERR(phy->usbcon), >> + "failed to get usbcon\n"); >> + >> + generic_phy = devm_phy_create(dev, NULL, &exynos2200_snps_eusb2_phy_ops); >> + if (IS_ERR(generic_phy)) { >> + dev_err(dev, "failed to create phy %d\n", ret); > > Syntax is return dev_err_probe > >> + return PTR_ERR(generic_phy); >> + } >> + >> + dev_set_drvdata(dev, phy); >> + phy_set_drvdata(generic_phy, phy); >> + >> + phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate); >> + if (IS_ERR(phy_provider)) { >> + dev_err(dev, "failed to register phy provider\n"); > > Syntax is return dev_err_probe > >> + return PTR_ERR(phy_provider); >> + }; >> + >> + return 0; >> +} > Best regards, > Krzysztof
On 16/02/2025 10:41, Ivaylo Ivanov wrote: > On 2/16/25 11:26, Krzysztof Kozlowski wrote: >> On 15/02/2025 13:24, Ivaylo Ivanov wrote: >>> The Exynos2200 SoC uses Synopsis eUSB2 PHY for USB 2.0. Add a new >>> driver for it. >>> >>> eUSB2 on Exynos SoCs is usually paired alongside a USB PHY controller. >>> Currently the driver is modelled to take and enable/disable the usb phy >>> controller when needed. >>> >>> The driver is based on information from downstream drivers. >>> >>> Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com> >>> --- >>> drivers/phy/samsung/Kconfig | 13 + >>> drivers/phy/samsung/Makefile | 1 + >>> .../phy/samsung/phy-exynos2200-snps-eusb2.c | 351 ++++++++++++++++++ >>> 3 files changed, 365 insertions(+) >>> create mode 100644 drivers/phy/samsung/phy-exynos2200-snps-eusb2.c >>> >>> diff --git a/drivers/phy/samsung/Kconfig b/drivers/phy/samsung/Kconfig >>> index e2330b089..f62285254 100644 >>> --- a/drivers/phy/samsung/Kconfig >>> +++ b/drivers/phy/samsung/Kconfig >>> @@ -77,6 +77,19 @@ config PHY_S5PV210_USB2 >>> particular SoC is compiled in the driver. In case of S5PV210 two phys >>> are available - device and host. >>> >>> +config PHY_EXYNOS2200_SNPS_EUSB2 >>> + tristate "Exynos2200 eUSB 2.0 PHY driver" >>> + depends on (ARCH_EXYNOS && OF) || COMPILE_TEST >>> + depends on HAS_IOMEM >>> + depends on USB_DWC3_EXYNOS >> >> How does it depend? What are you using from DWC3? > > Can drop, I guess. > >> >>> + select GENERIC_PHY >>> + select MFD_SYSCON >> Where do you use it? > > Remained from USBCON driver. > >> >>> + default y >>> + help >>> + Enable USBCON PHY support for Exynos2200 SoC. >>> + This driver provides PHY interface for eUSB 2.0 controller >>> + present on Exynos5 SoC series. >>> + >>> config PHY_EXYNOS5_USBDRD >>> tristate "Exynos5 SoC series USB DRD PHY driver" >>> depends on (ARCH_EXYNOS && OF) || COMPILE_TEST >>> diff --git a/drivers/phy/samsung/Makefile b/drivers/phy/samsung/Makefile >>> index fea1f96d0..90b84c7fc 100644 >>> --- a/drivers/phy/samsung/Makefile >>> +++ b/drivers/phy/samsung/Makefile >>> @@ -14,5 +14,6 @@ phy-exynos-usb2-$(CONFIG_PHY_EXYNOS4210_USB2) += phy-exynos4210-usb2.o >>> phy-exynos-usb2-$(CONFIG_PHY_EXYNOS4X12_USB2) += phy-exynos4x12-usb2.o >>> phy-exynos-usb2-$(CONFIG_PHY_EXYNOS5250_USB2) += phy-exynos5250-usb2.o >>> phy-exynos-usb2-$(CONFIG_PHY_S5PV210_USB2) += phy-s5pv210-usb2.o >>> +obj-$(CONFIG_PHY_EXYNOS2200_SNPS_EUSB2) += phy-exynos2200-snps-eusb2.o >> Entire driver looks like repeating existing qcom-snps-eusb2. > > It's the same IP, but implemented differently on a different platform. At > the very least, the register layout is different. I checked few registers, looked very the same. Same blocks from synopsys have the common register layouts. > >> You need to >> integrate the changes, not create duplicated driver. > > I can do that, but it would be come a bit cluttered, won't it? Depends on > if we want to follow the current oem-provided initialization sequence, or > try and fully reuse what we have in there. I think it duplicates a lot, so it won't be clutter. We have many drivers having common code and per-variant ops. Best regards, Krzysztof
On 2/16/25 11:44, Krzysztof Kozlowski wrote: > On 16/02/2025 10:41, Ivaylo Ivanov wrote: >> On 2/16/25 11:26, Krzysztof Kozlowski wrote: >>> On 15/02/2025 13:24, Ivaylo Ivanov wrote: >>>> The Exynos2200 SoC uses Synopsis eUSB2 PHY for USB 2.0. Add a new >>>> driver for it. >>>> >>>> eUSB2 on Exynos SoCs is usually paired alongside a USB PHY controller. >>>> Currently the driver is modelled to take and enable/disable the usb phy >>>> controller when needed. >>>> >>>> The driver is based on information from downstream drivers. >>>> >>>> Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com> >>>> --- >>>> drivers/phy/samsung/Kconfig | 13 + >>>> drivers/phy/samsung/Makefile | 1 + >>>> .../phy/samsung/phy-exynos2200-snps-eusb2.c | 351 ++++++++++++++++++ >>>> 3 files changed, 365 insertions(+) >>>> create mode 100644 drivers/phy/samsung/phy-exynos2200-snps-eusb2.c >>>> >>>> diff --git a/drivers/phy/samsung/Kconfig b/drivers/phy/samsung/Kconfig >>>> index e2330b089..f62285254 100644 >>>> --- a/drivers/phy/samsung/Kconfig >>>> +++ b/drivers/phy/samsung/Kconfig >>>> @@ -77,6 +77,19 @@ config PHY_S5PV210_USB2 >>>> particular SoC is compiled in the driver. In case of S5PV210 two phys >>>> are available - device and host. >>>> >>>> +config PHY_EXYNOS2200_SNPS_EUSB2 >>>> + tristate "Exynos2200 eUSB 2.0 PHY driver" >>>> + depends on (ARCH_EXYNOS && OF) || COMPILE_TEST >>>> + depends on HAS_IOMEM >>>> + depends on USB_DWC3_EXYNOS >>> How does it depend? What are you using from DWC3? >> Can drop, I guess. >> >>>> + select GENERIC_PHY >>>> + select MFD_SYSCON >>> Where do you use it? >> Remained from USBCON driver. >> >>>> + default y >>>> + help >>>> + Enable USBCON PHY support for Exynos2200 SoC. >>>> + This driver provides PHY interface for eUSB 2.0 controller >>>> + present on Exynos5 SoC series. >>>> + >>>> config PHY_EXYNOS5_USBDRD >>>> tristate "Exynos5 SoC series USB DRD PHY driver" >>>> depends on (ARCH_EXYNOS && OF) || COMPILE_TEST >>>> diff --git a/drivers/phy/samsung/Makefile b/drivers/phy/samsung/Makefile >>>> index fea1f96d0..90b84c7fc 100644 >>>> --- a/drivers/phy/samsung/Makefile >>>> +++ b/drivers/phy/samsung/Makefile >>>> @@ -14,5 +14,6 @@ phy-exynos-usb2-$(CONFIG_PHY_EXYNOS4210_USB2) += phy-exynos4210-usb2.o >>>> phy-exynos-usb2-$(CONFIG_PHY_EXYNOS4X12_USB2) += phy-exynos4x12-usb2.o >>>> phy-exynos-usb2-$(CONFIG_PHY_EXYNOS5250_USB2) += phy-exynos5250-usb2.o >>>> phy-exynos-usb2-$(CONFIG_PHY_S5PV210_USB2) += phy-s5pv210-usb2.o >>>> +obj-$(CONFIG_PHY_EXYNOS2200_SNPS_EUSB2) += phy-exynos2200-snps-eusb2.o >>> Entire driver looks like repeating existing qcom-snps-eusb2. >> It's the same IP, but implemented differently on a different platform. At >> the very least, the register layout is different. > > I checked few registers, looked very the same. Same blocks from synopsys > have the common register layouts. I see. > >>> You need to >>> integrate the changes, not create duplicated driver. >> I can do that, but it would be come a bit cluttered, won't it? Depends on >> if we want to follow the current oem-provided initialization sequence, or >> try and fully reuse what we have in there. > > I think it duplicates a lot, so it won't be clutter. We have many > drivers having common code and per-variant ops. So the approach to take here is to make a common driver? What about the current modelling scheme, as-in taking the phandle to the usbcon phy and handling it? Best regards, Ivaylo > > Best regards, > Krzysztof
On 16/02/2025 10:51, Ivaylo Ivanov wrote: >> >>>> You need to >>>> integrate the changes, not create duplicated driver. >>> I can do that, but it would be come a bit cluttered, won't it? Depends on >>> if we want to follow the current oem-provided initialization sequence, or >>> try and fully reuse what we have in there. >> >> I think it duplicates a lot, so it won't be clutter. We have many >> drivers having common code and per-variant ops. > > So the approach to take here is to make a common driver? For example: one common module and two modules per each soc, because I assume some per-soc stuff might be needed. But maybe even these two modules are not necessary and everything could be in one driver. > > What about the current modelling scheme, as-in taking the phandle to > the usbcon phy and handling it? What about it? Did you look at the bindings of qcom snps eusb2? Are you saying you do not have here repeater? If so, then this phy phandle might not be correct. Best regards, Krzysztof
On 2/16/25 15:19, Krzysztof Kozlowski wrote: > On 16/02/2025 10:51, Ivaylo Ivanov wrote: >>>>> You need to >>>>> integrate the changes, not create duplicated driver. >>>> I can do that, but it would be come a bit cluttered, won't it? Depends on >>>> if we want to follow the current oem-provided initialization sequence, or >>>> try and fully reuse what we have in there. >>> I think it duplicates a lot, so it won't be clutter. We have many >>> drivers having common code and per-variant ops. >> So the approach to take here is to make a common driver? > For example: one common module and two modules per each soc, because I > assume some per-soc stuff might be needed. But maybe even these two > modules are not necessary and everything could be in one driver. The issue here is that, while both QCOM and SAMSUNG use that IP, a lot of the registers are not mapped for Exynoses. For example with QCOM: `` #define USB_PHY_UTMI_CTRL0 (0x3c) #define PHY_UTMI_CTRL5 (0x50) `` Exynoses: `` /* nothing before this */ #define EXYNOS2200_EUSB2_RST_CTRL 0x0 `` Here EXYNOS2200_EUSB2_RST_CTRL seems to be the same register as QCOM_PHY_UTMI_CTRL5. Another instance is: `` #define UTMI_PHY_CMN_CTRL0 (0x98) #define TESTBURNIN BIT(6) `` Exynoses: `` #define EXYNOS2200_EUSB2_TESTSE (0x20) #define EXYNOS2200_TEST_IDDQ BIT(6) `` But at the same time there are some.. inconsistencies between them. Looking at the register layout for the exynos implementation for TX tuning the following register offset and bits are described: `` /* * Offset : 0x0014 * Description: tune register of tx driver */ typedef union { u32 data; struct { // bit[0] : unsigned fsls_slew_rate : 1; // bit[2:1] : unsigned fsls_vref_tune : 2; // bit[3] : unsigned fsls_vreg_bypass : 1; // bit[6:4] unsigned hs_vref_tune : 3; // bit [8:7] unsigned hs_xv : 2; // bit [11:9] unsigned preemp : 3; // bit [13:12] unsigned res : 2; // bit [15:14] unsigned rise : 2; // bit[31:16] unsigned RSVD31_16 : 16; } b; } EUSBCON_REG_TXTUNE_o, *EUSBPHY_REG_TXTUNE_p; `` And for QCOM the latter functionality is split into two separate register offsets: `` #define USB_PHY_CFG_CTRL_8 (0x78) #define PHY_CFG_TX_FSLS_VREF_TUNE_MASK GENMASK(1, 0) #define PHY_CFG_TX_FSLS_VREG_BYPASS BIT(2) #define PHY_CFG_TX_HS_VREF_TUNE_MASK GENMASK(5, 3) #define PHY_CFG_TX_HS_XV_TUNE_MASK GENMASK(7, 6) #define USB_PHY_CFG_CTRL_9 (0x7c) #define PHY_CFG_TX_PREEMP_TUNE_MASK GENMASK(2, 0) #define PHY_CFG_TX_RES_TUNE_MASK GENMASK(4, 3) #define PHY_CFG_TX_RISE_TUNE_MASK GENMASK(6, 5) #define PHY_CFG_RCAL_BYPASS BIT(7) `` So, Exynos2200 has a much simpler eusb initialization sequence than what is present in mainline for QCOMs. I still don't really think the drivers should be merged, as we aren't really duplicating code per-say. I've already started working on merging them, and my current idea is to not redefine the registers once again for 2200, but rather make an enum that defines if the SoC is a QCOM or EXYNOS, and select the register offsets dynamically - similarly as how I did with USIv1. If a register offset is not present, it'd just not do the write. My guess is that this will make it work with the qualcomm init sequence as well, so it'd result in even less redundant code (apart from the eUSB tuning, which can be omitted for now). > >> What about the current modelling scheme, as-in taking the phandle to >> the usbcon phy and handling it? > What about it? As I said in the commit description, I'm passing the USBCON phy as a phandle to the eusb2 node and enabling/disabling it when needed. I'm not 100% sure it would be adequate to include that in a common snps EUSB driver, as it seems to more of a quirk with the exynoses. But then how can I model it so that it's correctly described according to how the hardware works (as-in usbcon "muxing" between child phys, in this case eUSB and snps USBDP combophy) Regarding repeaters, I still don't have the TI repeater implemented. Best regards, Ivaylo > Did you look at the bindings of qcom snps eusb2? Are you > saying you do not have here repeater? If so, then this phy phandle might > not be correct. > > > > Best regards, > Krzysztof
On 2/16/25 15:57, Ivaylo Ivanov wrote: > On 2/16/25 15:19, Krzysztof Kozlowski wrote: >> On 16/02/2025 10:51, Ivaylo Ivanov wrote: >>>>>> You need to >>>>>> integrate the changes, not create duplicated driver. >>>>> I can do that, but it would be come a bit cluttered, won't it? Depends on >>>>> if we want to follow the current oem-provided initialization sequence, or >>>>> try and fully reuse what we have in there. >>>> I think it duplicates a lot, so it won't be clutter. We have many >>>> drivers having common code and per-variant ops. >>> So the approach to take here is to make a common driver? >> For example: one common module and two modules per each soc, because I >> assume some per-soc stuff might be needed. But maybe even these two >> modules are not necessary and everything could be in one driver. ... > > So, Exynos2200 has a much simpler eusb initialization sequence than what > is present in mainline for QCOMs. I still don't really think the drivers > should be merged, as we aren't really duplicating code per-say. > > I've already started working on merging them, and my current idea is to > not redefine the registers once again for 2200, but rather make an enum > that defines if the SoC is a QCOM or EXYNOS, and select the register > offsets dynamically Never mind. That's a bad idea - after more digging way too much bits differ not just the register layout. I'll implement the init/exit sequence in the qcom driver separately. Sadly I can't reuse much code. Best regards, Ivaylo > - similarly as how I did with USIv1. If a register > offset is not present, it'd just not do the write. My guess is that this > will make it work with the qualcomm init sequence as well, so it'd result > in even less redundant code (apart from the eUSB tuning, which can be > omitted for now). > >>> What about the current modelling scheme, as-in taking the phandle to >>> the usbcon phy and handling it? >> What about it? > As I said in the commit description, I'm passing the USBCON phy as a > phandle to the eusb2 node and enabling/disabling it when needed. I'm > not 100% sure it would be adequate to include that in a common snps EUSB > driver, as it seems to more of a quirk with the exynoses. But then how > can I model it so that it's correctly described according to how the > hardware works (as-in usbcon "muxing" between child phys, in this case > eUSB and snps USBDP combophy) > > Regarding repeaters, I still don't have the TI repeater implemented. > > Best regards, > Ivaylo > >> Did you look at the bindings of qcom snps eusb2? Are you >> saying you do not have here repeater? If so, then this phy phandle might >> not be correct. >> >> >> >> Best regards, >> Krzysztof
On Sa, 2025-02-15 at 14:24 +0200, Ivaylo Ivanov wrote: > The Exynos2200 SoC uses Synopsis eUSB2 PHY for USB 2.0. Add a new > driver for it. > > eUSB2 on Exynos SoCs is usually paired alongside a USB PHY controller. > Currently the driver is modelled to take and enable/disable the usb phy > controller when needed. > > The driver is based on information from downstream drivers. > > Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com> > --- > drivers/phy/samsung/Kconfig | 13 + > drivers/phy/samsung/Makefile | 1 + > .../phy/samsung/phy-exynos2200-snps-eusb2.c | 351 ++++++++++++++++++ > 3 files changed, 365 insertions(+) > create mode 100644 drivers/phy/samsung/phy-exynos2200-snps-eusb2.c > [...] > diff --git a/drivers/phy/samsung/phy-exynos2200-snps-eusb2.c b/drivers/phy/samsung/phy-exynos2200-snps-eusb2.c > new file mode 100644 > index 000000000..ee6d96411 > --- /dev/null > +++ b/drivers/phy/samsung/phy-exynos2200-snps-eusb2.c > @@ -0,0 +1,351 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (c) 2025, Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com> > + */ > + > +#include <linux/bitfield.h> > +#include <linux/clk.h> > +#include <linux/delay.h> > +#include <linux/iopoll.h> > +#include <linux/mfd/syscon.h> > +#include <linux/mod_devicetable.h> > +#include <linux/phy/phy.h> > +#include <linux/platform_device.h> > +#include <linux/regulator/consumer.h> > +#include <linux/reset.h> Drop this ... [...] > +struct exynos2200_snps_eusb2_phy { > + struct phy *phy; > + void __iomem *base; > + > + struct clk *ref_clk; > + struct clk_bulk_data *clks; > + const struct exynos2200_snps_eusb2_phy_drvdata *drv_data; > + struct reset_control *phy_reset; ... and this. It's never used. regards Philipp
diff --git a/drivers/phy/samsung/Kconfig b/drivers/phy/samsung/Kconfig index e2330b089..f62285254 100644 --- a/drivers/phy/samsung/Kconfig +++ b/drivers/phy/samsung/Kconfig @@ -77,6 +77,19 @@ config PHY_S5PV210_USB2 particular SoC is compiled in the driver. In case of S5PV210 two phys are available - device and host. +config PHY_EXYNOS2200_SNPS_EUSB2 + tristate "Exynos2200 eUSB 2.0 PHY driver" + depends on (ARCH_EXYNOS && OF) || COMPILE_TEST + depends on HAS_IOMEM + depends on USB_DWC3_EXYNOS + select GENERIC_PHY + select MFD_SYSCON + default y + help + Enable USBCON PHY support for Exynos2200 SoC. + This driver provides PHY interface for eUSB 2.0 controller + present on Exynos5 SoC series. + config PHY_EXYNOS5_USBDRD tristate "Exynos5 SoC series USB DRD PHY driver" depends on (ARCH_EXYNOS && OF) || COMPILE_TEST diff --git a/drivers/phy/samsung/Makefile b/drivers/phy/samsung/Makefile index fea1f96d0..90b84c7fc 100644 --- a/drivers/phy/samsung/Makefile +++ b/drivers/phy/samsung/Makefile @@ -14,5 +14,6 @@ phy-exynos-usb2-$(CONFIG_PHY_EXYNOS4210_USB2) += phy-exynos4210-usb2.o phy-exynos-usb2-$(CONFIG_PHY_EXYNOS4X12_USB2) += phy-exynos4x12-usb2.o phy-exynos-usb2-$(CONFIG_PHY_EXYNOS5250_USB2) += phy-exynos5250-usb2.o phy-exynos-usb2-$(CONFIG_PHY_S5PV210_USB2) += phy-s5pv210-usb2.o +obj-$(CONFIG_PHY_EXYNOS2200_SNPS_EUSB2) += phy-exynos2200-snps-eusb2.o obj-$(CONFIG_PHY_EXYNOS5_USBDRD) += phy-exynos5-usbdrd.o obj-$(CONFIG_PHY_EXYNOS5250_SATA) += phy-exynos5250-sata.o diff --git a/drivers/phy/samsung/phy-exynos2200-snps-eusb2.c b/drivers/phy/samsung/phy-exynos2200-snps-eusb2.c new file mode 100644 index 000000000..ee6d96411 --- /dev/null +++ b/drivers/phy/samsung/phy-exynos2200-snps-eusb2.c @@ -0,0 +1,351 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (c) 2025, Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com> + */ + +#include <linux/bitfield.h> +#include <linux/clk.h> +#include <linux/delay.h> +#include <linux/iopoll.h> +#include <linux/mfd/syscon.h> +#include <linux/mod_devicetable.h> +#include <linux/phy/phy.h> +#include <linux/platform_device.h> +#include <linux/regulator/consumer.h> +#include <linux/reset.h> + +#define EXYNOS2200_EUSB2_RST_CTRL 0x0 +#define EXYNOS2200_UTMI_PORT_RESET_MASK GENMASK(5, 4) +#define EXYNOS2200_EUSB_PHY_RESET_MASK GENMASK(1, 0) + +#define EXYNOS2200_EUSB2_CMN_CTRL 0x4 +#define EXYNOS2200_PHY_CFG_RPTR_MODE BIT(10) +#define EXYNOS2200_REF_FREQ_SEL GENMASK(6, 4) +#define EXYNOS2200_PHY_ENABLE BIT(0) + +#define EXYNOS2200_EUSB2_PLLCFG0 0x8 +#define EXYNOS2200_PLL_FB_DIV_MASK GENMASK(19, 8) + +#define EXYNOS2200_EUSB2_PLLCFG1 0xc +#define EXYNOS2200_PLL_REF_DIV GENMASK(11, 8) + +#define EXYNOS2200_EUSB2_TESTSE 0x20 +#define EXYNOS2200_TEST_IDDQ BIT(6) + +struct exynos2200_snps_eusb2_phy_drvdata { + const char * const *clk_names; + int n_clks; + const char * const *regulator_names; + int n_regulators; +}; + +struct exynos2200_snps_eusb2_phy { + struct phy *phy; + void __iomem *base; + + struct clk *ref_clk; + struct clk_bulk_data *clks; + const struct exynos2200_snps_eusb2_phy_drvdata *drv_data; + struct reset_control *phy_reset; + + struct regulator_bulk_data *vregs; + + enum phy_mode mode; + + struct phy *repeater; + struct phy *usbcon; +}; + +static void exynos2200_snps_eusb2_phy_write_mask(void __iomem *base, u32 offset, + u32 mask, u32 val) +{ + u32 reg; + + reg = readl_relaxed(base + offset); + reg &= ~mask; + reg |= val & mask; + writel_relaxed(reg, base + offset); + + /* ensure above write is completed */ + readl_relaxed(base + offset); +} + +static int exynos2200_snps_eusb2_ref_clk_init(struct exynos2200_snps_eusb2_phy *phy) +{ + unsigned long ref_clk_freq = clk_get_rate(phy->ref_clk); + + switch (ref_clk_freq) { + case 19200000: + exynos2200_snps_eusb2_phy_write_mask(phy->base, EXYNOS2200_EUSB2_CMN_CTRL, + EXYNOS2200_REF_FREQ_SEL, + 0); + + exynos2200_snps_eusb2_phy_write_mask(phy->base, EXYNOS2200_EUSB2_PLLCFG0, + EXYNOS2200_PLL_FB_DIV_MASK, + FIELD_PREP(EXYNOS2200_PLL_FB_DIV_MASK, 368)); + + exynos2200_snps_eusb2_phy_write_mask(phy->base, EXYNOS2200_EUSB2_PLLCFG1, + EXYNOS2200_PLL_REF_DIV, + FIELD_PREP(EXYNOS2200_PLL_FB_DIV_MASK, 0)); + break; + + case 20000000: + exynos2200_snps_eusb2_phy_write_mask(phy->base, EXYNOS2200_EUSB2_CMN_CTRL, + EXYNOS2200_REF_FREQ_SEL, + FIELD_PREP(EXYNOS2200_REF_FREQ_SEL, 1)); + + exynos2200_snps_eusb2_phy_write_mask(phy->base, EXYNOS2200_EUSB2_PLLCFG0, + EXYNOS2200_PLL_FB_DIV_MASK, + FIELD_PREP(EXYNOS2200_PLL_FB_DIV_MASK, 352)); + + exynos2200_snps_eusb2_phy_write_mask(phy->base, EXYNOS2200_EUSB2_PLLCFG1, + EXYNOS2200_PLL_REF_DIV, + FIELD_PREP(EXYNOS2200_PLL_FB_DIV_MASK, 0)); + break; + + case 24000000: + exynos2200_snps_eusb2_phy_write_mask(phy->base, EXYNOS2200_EUSB2_CMN_CTRL, + EXYNOS2200_REF_FREQ_SEL, + FIELD_PREP(EXYNOS2200_REF_FREQ_SEL, 2)); + + exynos2200_snps_eusb2_phy_write_mask(phy->base, EXYNOS2200_EUSB2_PLLCFG0, + EXYNOS2200_PLL_FB_DIV_MASK, + FIELD_PREP(EXYNOS2200_PLL_FB_DIV_MASK, 288)); + + exynos2200_snps_eusb2_phy_write_mask(phy->base, EXYNOS2200_EUSB2_PLLCFG1, + EXYNOS2200_PLL_REF_DIV, + FIELD_PREP(EXYNOS2200_PLL_FB_DIV_MASK, 0)); + break; + + case 26000000: + exynos2200_snps_eusb2_phy_write_mask(phy->base, EXYNOS2200_EUSB2_CMN_CTRL, + EXYNOS2200_REF_FREQ_SEL, + FIELD_PREP(EXYNOS2200_REF_FREQ_SEL, 3)); + + exynos2200_snps_eusb2_phy_write_mask(phy->base, EXYNOS2200_EUSB2_PLLCFG0, + EXYNOS2200_PLL_FB_DIV_MASK, + FIELD_PREP(EXYNOS2200_PLL_FB_DIV_MASK, 263)); + + exynos2200_snps_eusb2_phy_write_mask(phy->base, EXYNOS2200_EUSB2_PLLCFG1, + EXYNOS2200_PLL_REF_DIV, + FIELD_PREP(EXYNOS2200_PLL_FB_DIV_MASK, 0)); + break; + + case 48000000: + exynos2200_snps_eusb2_phy_write_mask(phy->base, EXYNOS2200_EUSB2_CMN_CTRL, + EXYNOS2200_REF_FREQ_SEL, + FIELD_PREP(EXYNOS2200_REF_FREQ_SEL, 2)); + + exynos2200_snps_eusb2_phy_write_mask(phy->base, EXYNOS2200_EUSB2_PLLCFG0, + EXYNOS2200_PLL_FB_DIV_MASK, + FIELD_PREP(EXYNOS2200_PLL_FB_DIV_MASK, 288)); + + exynos2200_snps_eusb2_phy_write_mask(phy->base, EXYNOS2200_EUSB2_PLLCFG1, + EXYNOS2200_PLL_REF_DIV, + FIELD_PREP(EXYNOS2200_PLL_FB_DIV_MASK, 1)); + break; + + default: + dev_err(&phy->phy->dev, "unsupported ref_clk_freq:%lu\n", ref_clk_freq); + return -EINVAL; + } + + return 0; +} + +static int exynos2200_snps_eusb2_phy_init(struct phy *p) +{ + struct exynos2200_snps_eusb2_phy *phy = phy_get_drvdata(p); + int ret; + + ret = regulator_bulk_enable(phy->drv_data->n_regulators, phy->vregs); + if (ret) + return ret; + + ret = clk_bulk_prepare_enable(phy->drv_data->n_clks, phy->clks); + if (ret) { + dev_err(&p->dev, "failed to enable clocks, %d\n", ret); + goto disable_vreg; + } + + ret = phy_init(phy->usbcon); + if (ret) { + dev_err(&p->dev, "usbcon init failed. %d\n", ret); + goto disable_ref_clk; + } + + exynos2200_snps_eusb2_phy_write_mask(phy->base, EXYNOS2200_EUSB2_RST_CTRL, + EXYNOS2200_UTMI_PORT_RESET_MASK | + EXYNOS2200_EUSB_PHY_RESET_MASK, + FIELD_PREP(EXYNOS2200_UTMI_PORT_RESET_MASK, 0x1) | + FIELD_PREP(EXYNOS2200_EUSB_PHY_RESET_MASK, 0x1)); + fsleep(50); /* required after holding phy in reset */ + + exynos2200_snps_eusb2_phy_write_mask(phy->base, EXYNOS2200_EUSB2_CMN_CTRL, + EXYNOS2200_PHY_CFG_RPTR_MODE, + EXYNOS2200_PHY_CFG_RPTR_MODE); + + /* update ref_clk related registers */ + ret = exynos2200_snps_eusb2_ref_clk_init(phy); + if (ret) + return ret; + + exynos2200_snps_eusb2_phy_write_mask(phy->base, EXYNOS2200_EUSB2_TESTSE, + EXYNOS2200_TEST_IDDQ, + 0); + fsleep(10); /* required after releasing test_iddq */ + + exynos2200_snps_eusb2_phy_write_mask(phy->base, EXYNOS2200_EUSB2_RST_CTRL, + EXYNOS2200_EUSB_PHY_RESET_MASK, + 0); + + exynos2200_snps_eusb2_phy_write_mask(phy->base, EXYNOS2200_EUSB2_CMN_CTRL, + EXYNOS2200_PHY_ENABLE, + EXYNOS2200_PHY_ENABLE); + + exynos2200_snps_eusb2_phy_write_mask(phy->base, EXYNOS2200_EUSB2_RST_CTRL, + EXYNOS2200_UTMI_PORT_RESET_MASK, + 0); + return 0; + +disable_ref_clk: + clk_disable_unprepare(phy->ref_clk); + +disable_vreg: + regulator_bulk_disable(phy->drv_data->n_regulators, phy->vregs); + + return ret; +} + +static int exynos2200_snps_eusb2_phy_exit(struct phy *p) +{ + struct exynos2200_snps_eusb2_phy *phy = phy_get_drvdata(p); + + phy_exit(phy->usbcon); + + clk_disable_unprepare(phy->ref_clk); + regulator_bulk_disable(phy->drv_data->n_regulators, phy->vregs); + + return 0; +} + +static const struct phy_ops exynos2200_snps_eusb2_phy_ops = { + .init = exynos2200_snps_eusb2_phy_init, + .exit = exynos2200_snps_eusb2_phy_exit, + .owner = THIS_MODULE, +}; + +static int exynos2200_snps_eusb2_phy_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + struct device_node *np = dev->of_node; + struct exynos2200_snps_eusb2_phy *phy; + const struct exynos2200_snps_eusb2_phy_drvdata *drv_data; + struct phy_provider *phy_provider; + struct phy *generic_phy; + int ret; + + phy = devm_kzalloc(dev, sizeof(*phy), GFP_KERNEL); + if (!phy) + return -ENOMEM; + + drv_data = of_device_get_match_data(dev); + if (!drv_data) + return -EINVAL; + phy->drv_data = drv_data; + + phy->base = devm_platform_ioremap_resource(pdev, 0); + if (IS_ERR(phy->base)) + return PTR_ERR(phy->base); + + phy->clks = devm_kcalloc(dev, drv_data->n_clks, + sizeof(*phy->clks), GFP_KERNEL); + if (!phy->clks) + return -ENOMEM; + + for (int i = 0; i < drv_data->n_clks; ++i) + phy->clks[i].id = drv_data->clk_names[i]; + + ret = devm_clk_bulk_get(dev, drv_data->n_clks, + phy->clks); + if (ret) + return dev_err_probe(dev, ret, + "failed to get phy clock(s)\n"); + + for (int i = 0; i < phy->drv_data->n_clks; ++i) { + if (!strcmp(phy->clks[i].id, "ref")) { + phy->ref_clk = phy->clks[i].clk; + break; + } + } + + phy->vregs = devm_kcalloc(dev, drv_data->n_regulators, + sizeof(*phy->vregs), GFP_KERNEL); + if (!phy->vregs) + return -ENOMEM; + regulator_bulk_set_supply_names(phy->vregs, + drv_data->regulator_names, + drv_data->n_regulators); + ret = devm_regulator_bulk_get(dev, drv_data->n_regulators, + phy->vregs); + if (ret) + return dev_err_probe(dev, ret, "failed to get regulators\n"); + + /* we treat the usblink controller phy as a separate phy */ + phy->usbcon = devm_of_phy_get_by_index(dev, np, 0); + if (IS_ERR(phy->usbcon)) + return dev_err_probe(dev, PTR_ERR(phy->usbcon), + "failed to get usbcon\n"); + + generic_phy = devm_phy_create(dev, NULL, &exynos2200_snps_eusb2_phy_ops); + if (IS_ERR(generic_phy)) { + dev_err(dev, "failed to create phy %d\n", ret); + return PTR_ERR(generic_phy); + } + + dev_set_drvdata(dev, phy); + phy_set_drvdata(generic_phy, phy); + + phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate); + if (IS_ERR(phy_provider)) { + dev_err(dev, "failed to register phy provider\n"); + return PTR_ERR(phy_provider); + }; + + return 0; +} + +static const char * const exynos2200_clk_names[] = { + "ref", "apb", "ctrl", +}; + +static const char * const exynos2200_regulator_names[] = { + "vdd", "vdda12", +}; + +static const struct exynos2200_snps_eusb2_phy_drvdata exynos2200_snps_eusb2_phy = { + .clk_names = exynos2200_clk_names, + .n_clks = ARRAY_SIZE(exynos2200_clk_names), + .regulator_names = exynos2200_regulator_names, + .n_regulators = ARRAY_SIZE(exynos2200_regulator_names), +}; + +static const struct of_device_id exynos2200_snps_eusb2_phy_of_match_table[] = { + { + .compatible = "samsung,exynos2200-snps-eusb2-phy", + .data = &exynos2200_snps_eusb2_phy, + }, { }, +}; +MODULE_DEVICE_TABLE(of, exynos2200_snps_eusb2_phy_of_match_table); + +static struct platform_driver exynos2200_snps_eusb2_phy_driver = { + .probe = exynos2200_snps_eusb2_phy_probe, + .driver = { + .name = "exynos2200-snps-eusb2-hsphy", + .of_match_table = exynos2200_snps_eusb2_phy_of_match_table, + }, +}; + +module_platform_driver(exynos2200_snps_eusb2_phy_driver); +MODULE_DESCRIPTION("Exynos2200 SNPS eUSB2 PHY driver"); +MODULE_LICENSE("GPL");
The Exynos2200 SoC uses Synopsis eUSB2 PHY for USB 2.0. Add a new driver for it. eUSB2 on Exynos SoCs is usually paired alongside a USB PHY controller. Currently the driver is modelled to take and enable/disable the usb phy controller when needed. The driver is based on information from downstream drivers. Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com> --- drivers/phy/samsung/Kconfig | 13 + drivers/phy/samsung/Makefile | 1 + .../phy/samsung/phy-exynos2200-snps-eusb2.c | 351 ++++++++++++++++++ 3 files changed, 365 insertions(+) create mode 100644 drivers/phy/samsung/phy-exynos2200-snps-eusb2.c