Re: [PATCH v2 7/8] phy: phy-snps-eusb2: add support for exynos2200

Ivaylo Ivanov
2025-02-24T07:31:00Z

On 2/24/25 01:51, Dmitry Baryshkov wrote:
> On Sun, Feb 23, 2025 at 02:22:26PM +0200, Ivaylo Ivanov wrote:
>> The Exynos2200 SoC reuses the Synopsis eUSB2 PHY IP, alongside an
>> external repeater, for USB 2.0. Add support for it to the existing
>> driver.
>>
>> Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
>> ---
>> drivers/phy/Kconfig | 2 +-
>> drivers/phy/phy-snps-eusb2.c | 172 +++++++++++++++++++++++++++++++++++
>> 2 files changed, 173 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
>> index 11c166204..58c911e1b 100644
>> --- a/drivers/phy/Kconfig
>> +++ b/drivers/phy/Kconfig
>> @@ -45,7 +45,7 @@ config PHY_PISTACHIO_USB
>>
>> config PHY_SNPS_EUSB2
>> tristate "SNPS eUSB2 PHY Driver"
>> - depends on OF && (ARCH_QCOM || COMPILE_TEST)
>> + depends on OF && (ARCH_EXYNOS || ARCH_QCOM || COMPILE_TEST)
>> select GENERIC_PHY
>> help
>> Enable support for the USB high-speed SNPS eUSB2 phy on select
>> diff --git a/drivers/phy/phy-snps-eusb2.c b/drivers/phy/phy-snps-eusb2.c
>> index 7a242fe32..67a19d671 100644
>> --- a/drivers/phy/phy-snps-eusb2.c
>> +++ b/drivers/phy/phy-snps-eusb2.c
>> @@ -13,6 +13,39 @@
>> #include <linux/regulator/consumer.h>
>> #include <linux/reset.h>
>>
>> +#define EXYNOS_USB_PHY_HS_PHY_CTRL_RST (0x0)
>> +#define USB_PHY_RST_MASK GENMASK(1, 0)
>> +#define UTMI_PORT_RST_MASK GENMASK(5, 4)
>> +
>> +#define EXYNOS_USB_PHY_HS_PHY_CTRL_COMMON (0x4)
>> +#define RPTR_MODE BIT(10)
>> +#define FSEL_20_MHZ_VAL (0x1)
>> +#define FSEL_24_MHZ_VAL (0x2)
>> +#define FSEL_26_MHZ_VAL (0x3)
>> +#define FSEL_48_MHZ_VAL (0x2)
>> +
>> +#define EXYNOS_USB_PHY_CFG_PLLCFG0 (0x8)
>> +#define PHY_CFG_PLL_FB_DIV_19_8_MASK GENMASK(19, 8)
>> +#define DIV_19_8_19_2_MHZ_VAL (0x170)
>> +#define DIV_19_8_20_MHZ_VAL (0x160)
>> +#define DIV_19_8_24_MHZ_VAL (0x120)
>> +#define DIV_19_8_26_MHZ_VAL (0x107)
>> +#define DIV_19_8_48_MHZ_VAL (0x120)
>> +
>> +#define EXYNOS_USB_PHY_CFG_PLLCFG1 (0xc)
>> +#define EXYNOS_PHY_CFG_PLL_FB_DIV_11_8_MASK GENMASK(11, 8)
>> +#define EXYNOS_DIV_11_8_19_2_MHZ_VAL (0x0)
>> +#define EXYNOS_DIV_11_8_20_MHZ_VAL (0x0)
>> +#define EXYNOS_DIV_11_8_24_MHZ_VAL (0x0)
>> +#define EXYNOS_DIV_11_8_26_MHZ_VAL (0x0)
>> +#define EXYNOS_DIV_11_8_48_MHZ_VAL (0x1)
>> +
>> +#define EXYNOS_PHY_CFG_TX (0x14)
>> +#define EXYNOS_PHY_CFG_TX_FSLS_VREF_TUNE_MASK GENMASK(2, 1)
>> +
>> +#define EXYNOS_USB_PHY_UTMI_TESTSE (0x20)
>> +#define TEST_IDDQ BIT(6)
>> +
>> #define QCOM_USB_PHY_UTMI_CTRL0 (0x3c)
>> #define SLEEPM BIT(0)
>> #define OPMODE_MASK GENMASK(4, 3)
>> @@ -196,6 +229,93 @@ static void qcom_eusb2_default_parameters(struct snps_eusb2_hsphy *phy)
>> FIELD_PREP(PHY_CFG_TX_HS_XV_TUNE_MASK, 0x0));
>> }
>>
>> +static int exynos_eusb2_ref_clk_init(struct snps_eusb2_hsphy *phy)
>> +{
>> + unsigned long ref_clk_freq = clk_get_rate(phy->ref_clk);
>> +
>> + switch (ref_clk_freq) {
>> + case 19200000:
>> + snps_eusb2_hsphy_write_mask(phy->base, EXYNOS_USB_PHY_HS_PHY_CTRL_COMMON,
>> + FSEL_MASK,
>> + FIELD_PREP(FSEL_MASK, FSEL_19_2_MHZ_VAL));
>> +
> Could you please unify the switchcase? assign the values to temp
> variables, then program them from a single code stream. Or maybe even
> replace switch-case with a table-based lookup.
>
> (we probably should implement the similar change for qcom part. Maybe
> you can refactor it too?)

Alright. I'll do it for the Qualcomm part too in a separate commit.

Thanks for the feedback!

Best regards,
Ivaylo

> Other than that LGTM.
>
>> + snps_eusb2_hsphy_write_mask(phy->base, EXYNOS_USB_PHY_CFG_PLLCFG0,
>> + PHY_CFG_PLL_FB_DIV_19_8_MASK,
>> + FIELD_PREP(PHY_CFG_PLL_FB_DIV_19_8_MASK,
>> + DIV_19_8_19_2_MHZ_VAL));
>> +
>> + snps_eusb2_hsphy_write_mask(phy->base, EXYNOS_USB_PHY_CFG_PLLCFG1,
>> + EXYNOS_PHY_CFG_PLL_FB_DIV_11_8_MASK,
>> + EXYNOS_DIV_11_8_19_2_MHZ_VAL);
>> + break;
>> +
>> + case 20000000:
>> + snps_eusb2_hsphy_write_mask(phy->base, EXYNOS_USB_PHY_HS_PHY_CTRL_COMMON,
>> + FSEL_MASK,
>> + FIELD_PREP(FSEL_MASK, FSEL_20_MHZ_VAL));
>> +
>> + snps_eusb2_hsphy_write_mask(phy->base, EXYNOS_USB_PHY_CFG_PLLCFG0,
>> + PHY_CFG_PLL_FB_DIV_19_8_MASK,
>> + FIELD_PREP(PHY_CFG_PLL_FB_DIV_19_8_MASK,
>> + DIV_19_8_20_MHZ_VAL));
>> +
>> + snps_eusb2_hsphy_write_mask(phy->base, EXYNOS_USB_PHY_CFG_PLLCFG1,
>> + EXYNOS_PHY_CFG_PLL_FB_DIV_11_8_MASK,
>> + EXYNOS_DIV_11_8_20_MHZ_VAL);
>> + break;
>> +