linux-kernel - Re: [PATCH v2 2/2] phy: exyons5-usbdrd: support HS phy for ExynosAutov920

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <a5c1a064-d760-4140-9e78-d74823b400a8@linaro.org>
Date: Tue, 20 May 2025 09:39:49 +0200
From: neil.armstrong@...aro.org
To: Pritam Manohar Sutar <pritam.sutar@...sung.com>, vkoul@...nel.org,
 kishon@...nel.org, robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org,
 alim.akhtar@...sung.com, andre.draszik@...aro.org, peter.griffin@...aro.org,
 kauschluss@...root.org, m.szyprowski@...sung.com, s.nawrocki@...sung.com
Cc: linux-phy@...ts.infradead.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
 linux-samsung-soc@...r.kernel.org, rosa.pila@...sung.com,
 dev.tailor@...sung.com, faraz.ata@...sung.com, muhammed.ali@...sung.com,
 selvarasu.g@...sung.com
Subject: Re: [PATCH v2 2/2] phy: exyons5-usbdrd: support HS phy for
 ExynosAutov920

On 16/05/2025 12:26, Pritam Manohar Sutar wrote:
> This SoC has a single USB 3.1 DRD combo phy and three USB2.0
> DRD HS phy controllers those only support the UTMI+ interface.
> 
> Support only UTMI+ for this SoC which is very similar to what
> the existing Exynos850 supports.
> 
> The combo phy supports both UTMI+ (HS) and PIPE3 (SS) and is
> out of scope of this commit.
> 
> Add required change in phy driver to support HS phy for this SoC.
> 
> Signed-off-by: Pritam Manohar Sutar <pritam.sutar@...sung.com>
> ---
>   drivers/phy/samsung/phy-exynos5-usbdrd.c | 85 ++++++++++++++++++++++++
>   1 file changed, 85 insertions(+)
> 
> diff --git a/drivers/phy/samsung/phy-exynos5-usbdrd.c b/drivers/phy/samsung/phy-exynos5-usbdrd.c
> index 634c4310c660..b440b56c6595 100644
> --- a/drivers/phy/samsung/phy-exynos5-usbdrd.c
> +++ b/drivers/phy/samsung/phy-exynos5-usbdrd.c
> @@ -177,6 +177,9 @@
>   #define HSPHYPLLTUNE_PLL_P_TUNE			GENMASK(3, 0)
>   
>   /* Exynos850: USB DRD PHY registers */
> +#define EXYNOSAUTOv920_DRD_CTRL_VER		0x00
> +#define CTRL_VER_MAJOR_VERSION			GENMASK(31, 24)
> +
>   #define EXYNOS850_DRD_LINKCTRL			0x04
>   #define LINKCTRL_FORCE_RXELECIDLE		BIT(18)
>   #define LINKCTRL_FORCE_PHYSTATUS		BIT(17)
> @@ -1772,6 +1775,10 @@ static const char * const exynos5_regulator_names[] = {
>   	"vbus", "vbus-boost",
>   };
>   
> +static const char * const exynosautov920_clk_names[] = {
> +	"ext_xtal",
> +};
> +
>   static const struct exynos5_usbdrd_phy_drvdata exynos5420_usbdrd_phy = {
>   	.phy_cfg		= phy_cfg_exynos5,
>   	.phy_ops		= &exynos5_usbdrd_phy_ops,
> @@ -1847,6 +1854,81 @@ static const struct exynos5_usbdrd_phy_drvdata exynos850_usbdrd_phy = {
>   	.n_regulators		= ARRAY_SIZE(exynos5_regulator_names),
>   };
>   
> +static void exynosautov920_usbdrd_utmi_init(struct exynos5_usbdrd_phy *phy_drd)
> +{
> +	u32 version;
> +
> +	version = readl(phy_drd->reg_phy + EXYNOSAUTOv920_DRD_CTRL_VER);
> +	dev_info(phy_drd->dev, "usbphy: version:0x%x\n", version);

Please do not add mode info to boot log, use dev_dbg instead.

> +
> +	if (FIELD_GET(CTRL_VER_MAJOR_VERSION, version) == 0x3)
> +		/* utmi init for exynosautov920 HS phy */
> +		exynos850_usbdrd_utmi_init(phy_drd);
> +}
> +
> +static int exynosautov920_usbdrd_phy_init(struct phy *phy)
> +{
> +	struct phy_usb_instance *inst = phy_get_drvdata(phy);
> +	struct exynos5_usbdrd_phy *phy_drd = to_usbdrd_phy(inst);
> +	int ret = 0;
> +
> +	ret = clk_bulk_prepare_enable(phy_drd->drv_data->n_clks, phy_drd->clks);
> +	if (ret)
> +		return ret;
> +
> +	/* UTMI or PIPE3 specific init */
> +	inst->phy_cfg->phy_init(phy_drd);
> +
> +	clk_bulk_disable_unprepare(phy_drd->drv_data->n_clks, phy_drd->clks);
> +
> +	return 0;
> +}
> +
> +static void exynosautov920_v3p1_phy_dis(struct phy *phy)
> +{
> +	struct phy_usb_instance *inst = phy_get_drvdata(phy);
> +	struct exynos5_usbdrd_phy *phy_drd = to_usbdrd_phy(inst);
> +	void __iomem *reg_phy = phy_drd->reg_phy;
> +	u32 version;
> +
> +	version = readl(reg_phy + EXYNOSAUTOv920_DRD_CTRL_VER);
> +
> +	if (FIELD_GET(CTRL_VER_MAJOR_VERSION, version) == 0x3)
> +		exynos850_usbdrd_phy_exit(phy);
> +}
> +
> +static int exynosautov920_usbdrd_phy_exit(struct phy *phy)
> +{
> +	struct phy_usb_instance *inst = phy_get_drvdata(phy);
> +
> +	if (inst->phy_cfg->id == EXYNOS5_DRDPHY_UTMI)
> +		exynosautov920_v3p1_phy_dis(phy);
> +
> +	return 0;
> +}
> +
> +static const struct phy_ops exynosautov920_usbdrd_phy_ops = {
> +	.init		= exynosautov920_usbdrd_phy_init,
> +	.exit		= exynosautov920_usbdrd_phy_exit,

<snip>

> +		.id		= EXYNOS5_DRDPHY_UTMI,
> +		.phy_init	= exynosautov920_usbdrd_utmi_init,

<snip>

> +	}, {
> +		.compatible = "samsung,exynosautov920-usb31drd-phy",
> +		.data = &exynosautov920_usb31drd_phy

All those new ops are only called when matching this compatible, it it really
necessary to check the version ? is there "samsung,exynosautov920-usb31drd-phy" PHYs
with version different from 3 in the wild ?

Neil

>   	},
>   	{ },
>   };