From: Larisa Ileana Grigore <larisa.grigore@oss.nxp.com>
To: Frank Li <Frank.li@oss.nxp.com>
Cc: NXP S32 Linux Team <s32@nxp.com>,
Alim Akhtar <alim.akhtar@samsung.com>,
Avri Altman <avri.altman@sandisk.com>,
Bart Van Assche <bvanassche@acm.org>,
Rob Herring <robh@kernel.org>,
Krzysztof Kozlowski <krzk+dt@kernel.org>,
Conor Dooley <conor+dt@kernel.org>, Frank Li <Frank.Li@nxp.com>,
Sascha Hauer <s.hauer@pengutronix.de>,
Pengutronix Kernel Team <kernel@pengutronix.de>,
Fabio Estevam <festevam@gmail.com>,
"James E . J . Bottomley" <James.Bottomley@hansenpartnership.com>,
"Martin K . Petersen" <martin.petersen@oracle.com>,
Sai Krishna Potthuri <sai.krishna.potthuri@amd.com>,
Ajay Neeli <ajay.neeli@amd.com>,
Manivannan Sadhasivam <mani@kernel.org>,
Pedro Sousa <pedrom.sousa@synopsys.com>,
clizzi@redhat.com, aruizrui@redhat.com, eballetb@redhat.com,
echanude@redhat.com, linux-scsi@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
imx@lists.linux.dev, linux-arm-kernel@lists.infradead.org,
linux-arm-msm@vger.kernel.org
Subject: Re: [PATCH 04/10] scsi: ufs: Move Versal2 M-PHY CREG access helpers into ufshcd-dwc
Date: Thu, 27 Aug 2026 17:11:02 +0300 [thread overview]
Message-ID: <85c580d7-dcc9-429d-a2ff-e286d4ac0fb5@oss.nxp.com> (raw)
In-Reply-To: <ao8ERPmAsNEtKHUn@SMW015318>
On 8/26/2026 6:20 PM, Frank Li wrote:
> On Wed, Aug 26, 2026 at 03:40:42PM +0200, Larisa Grigore wrote:
>>
>> The AMD Versal Gen 2 UFS platform driver carries local helpers to access
>> M-PHY control registers through the UniPro CREG interface.
>>
>> Move these helpers into the common Synopsys DWC layer as
>> ufshcd_dwc_phy_reg_read() and ufshcd_dwc_phy_reg_write(). This avoids code
>> duplication in DWC-based glue drivers and provides a single implementation
>> for future reuse.
>
> Nit: add empty line between paragraph
>
Thank you! I’ll address it in the next revision.
>> Dropped `static` from `phy_write_attrs` and `phy_read_attrs` since there
>> is no benefit in keeping them persistent.
>>
>> Update the AMD Versal2 glue driver to use the new ufshcd-dwc helpers.
>>
>> Signed-off-by: Larisa Grigore <larisa.grigore@oss.nxp.com>
>> ---
>
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
>
>> drivers/ufs/host/ufs-amd-versal2.c | 85 ++++++------------------------
>> drivers/ufs/host/ufshcd-dwc.c | 77 +++++++++++++++++++++++++++
>> drivers/ufs/host/ufshcd-dwc.h | 2 +
>> 3 files changed, 96 insertions(+), 68 deletions(-)
>>
>> diff --git a/drivers/ufs/host/ufs-amd-versal2.c b/drivers/ufs/host/ufs-amd-versal2.c
>> index 0d4d5e98ad65..c7da95999c85 100644
>> --- a/drivers/ufs/host/ufs-amd-versal2.c
>> +++ b/drivers/ufs/host/ufs-amd-versal2.c
>> @@ -43,57 +43,6 @@ struct ufs_versal2_host {
>> u8 ctlecompval1;
>> };
>>
>> -static int ufs_versal2_phy_reg_write(struct ufs_hba *hba, u32 addr, u32 val)
>> -{
>> - static struct ufshcd_dme_attr_val phy_write_attrs[] = {
>> - { UIC_ARG_MIB(CBCREGADDRLSB), 0, DME_LOCAL },
>> - { UIC_ARG_MIB(CBCREGADDRMSB), 0, DME_LOCAL },
>> - { UIC_ARG_MIB(CBCREGWRLSB), 0, DME_LOCAL },
>> - { UIC_ARG_MIB(CBCREGWRMSB), 0, DME_LOCAL },
>> - { UIC_ARG_MIB(CBCREGRDWRSEL), 1, DME_LOCAL },
>> - { UIC_ARG_MIB(VS_MPHYCFGUPDT), 1, DME_LOCAL }
>> - };
>> -
>> - phy_write_attrs[0].mib_val = (u8)addr;
>> - phy_write_attrs[1].mib_val = (u8)(addr >> 8);
>> - phy_write_attrs[2].mib_val = (u8)val;
>> - phy_write_attrs[3].mib_val = (u8)(val >> 8);
>> -
>> - return ufshcd_dwc_dme_set_attrs(hba, phy_write_attrs, ARRAY_SIZE(phy_write_attrs));
>> -}
>> -
>> -static int ufs_versal2_phy_reg_read(struct ufs_hba *hba, u32 addr, u32 *val)
>> -{
>> - u32 mib_val;
>> - int ret;
>> - static struct ufshcd_dme_attr_val phy_read_attrs[] = {
>> - { UIC_ARG_MIB(CBCREGADDRLSB), 0, DME_LOCAL },
>> - { UIC_ARG_MIB(CBCREGADDRMSB), 0, DME_LOCAL },
>> - { UIC_ARG_MIB(CBCREGRDWRSEL), 0, DME_LOCAL },
>> - { UIC_ARG_MIB(VS_MPHYCFGUPDT), 1, DME_LOCAL }
>> - };
>> -
>> - phy_read_attrs[0].mib_val = (u8)addr;
>> - phy_read_attrs[1].mib_val = (u8)(addr >> 8);
>> -
>> - ret = ufshcd_dwc_dme_set_attrs(hba, phy_read_attrs, ARRAY_SIZE(phy_read_attrs));
>> - if (ret)
>> - return ret;
>> -
>> - ret = ufshcd_dme_get(hba, UIC_ARG_MIB(CBCREGRDLSB), &mib_val);
>> - if (ret)
>> - return ret;
>> -
>> - *val = mib_val;
>> - ret = ufshcd_dme_get(hba, UIC_ARG_MIB(CBCREGRDMSB), &mib_val);
>> - if (ret)
>> - return ret;
>> -
>> - *val |= (mib_val << 8);
>> -
>> - return 0;
>> -}
>> -
>> static int ufs_versal2_enable_phy(struct ufs_hba *hba)
>> {
>> u32 offset, reg;
>> @@ -162,64 +111,64 @@ static int ufs_versal2_setup_phy(struct ufs_hba *hba)
>> u32 reg;
>>
>> /* Bypass RX-AFE offset calibrations (ATT/CTLE) */
>> - ret = ufs_versal2_phy_reg_read(hba, FAST_FLAGS(0), ®);
>> + ret = ufshcd_dwc_phy_reg_read(hba, FAST_FLAGS(0), ®);
>> if (ret)
>> return ret;
>>
>> reg |= MPHY_FAST_RX_AFE_CAL;
>> - ret = ufs_versal2_phy_reg_write(hba, FAST_FLAGS(0), reg);
>> + ret = ufshcd_dwc_phy_reg_write(hba, FAST_FLAGS(0), reg);
>> if (ret)
>> return ret;
>>
>> - ret = ufs_versal2_phy_reg_read(hba, FAST_FLAGS(1), ®);
>> + ret = ufshcd_dwc_phy_reg_read(hba, FAST_FLAGS(1), ®);
>> if (ret)
>> return ret;
>>
>> reg |= MPHY_FAST_RX_AFE_CAL;
>> - ret = ufs_versal2_phy_reg_write(hba, FAST_FLAGS(1), reg);
>> + ret = ufshcd_dwc_phy_reg_write(hba, FAST_FLAGS(1), reg);
>> if (ret)
>> return ret;
>>
>> /* Program ATT and CTLE compensation values */
>> if (host->attcompval0) {
>> - ret = ufs_versal2_phy_reg_write(hba, RX_AFE_ATT_IDAC(0), host->attcompval0);
>> + ret = ufshcd_dwc_phy_reg_write(hba, RX_AFE_ATT_IDAC(0), host->attcompval0);
>> if (ret)
>> return ret;
>> }
>>
>> if (host->attcompval1) {
>> - ret = ufs_versal2_phy_reg_write(hba, RX_AFE_ATT_IDAC(1), host->attcompval1);
>> + ret = ufshcd_dwc_phy_reg_write(hba, RX_AFE_ATT_IDAC(1), host->attcompval1);
>> if (ret)
>> return ret;
>> }
>>
>> if (host->ctlecompval0) {
>> - ret = ufs_versal2_phy_reg_write(hba, RX_AFE_CTLE_IDAC(0), host->ctlecompval0);
>> + ret = ufshcd_dwc_phy_reg_write(hba, RX_AFE_CTLE_IDAC(0), host->ctlecompval0);
>> if (ret)
>> return ret;
>> }
>>
>> if (host->ctlecompval1) {
>> - ret = ufs_versal2_phy_reg_write(hba, RX_AFE_CTLE_IDAC(1), host->ctlecompval1);
>> + ret = ufshcd_dwc_phy_reg_write(hba, RX_AFE_CTLE_IDAC(1), host->ctlecompval1);
>> if (ret)
>> return ret;
>> }
>>
>> - ret = ufs_versal2_phy_reg_read(hba, FW_CALIB_CCFG(0), ®);
>> + ret = ufshcd_dwc_phy_reg_read(hba, FW_CALIB_CCFG(0), ®);
>> if (ret)
>> return ret;
>>
>> reg |= MPHY_FW_CALIB_CFG_VAL;
>> - ret = ufs_versal2_phy_reg_write(hba, FW_CALIB_CCFG(0), reg);
>> + ret = ufshcd_dwc_phy_reg_write(hba, FW_CALIB_CCFG(0), reg);
>> if (ret)
>> return ret;
>>
>> - ret = ufs_versal2_phy_reg_read(hba, FW_CALIB_CCFG(1), ®);
>> + ret = ufshcd_dwc_phy_reg_read(hba, FW_CALIB_CCFG(1), ®);
>> if (ret)
>> return ret;
>>
>> reg |= MPHY_FW_CALIB_CFG_VAL;
>> - return ufs_versal2_phy_reg_write(hba, FW_CALIB_CCFG(1), reg);
>> + return ufshcd_dwc_phy_reg_write(hba, FW_CALIB_CCFG(1), reg);
>> }
>>
>> static int ufs_versal2_phy_init(struct ufs_hba *hba)
>> @@ -406,7 +355,7 @@ static int ufs_versal2_phy_ratesel(struct ufs_hba *hba, u32 activelanes, u32 rx_
>>
>> for (lane = 0; lane < activelanes; lane++) {
>> time_left = TIMEOUT_MICROSEC;
>> - ret = ufs_versal2_phy_reg_read(hba, RX_OVRD_IN_1(lane), ®);
>> + ret = ufshcd_dwc_phy_reg_read(hba, RX_OVRD_IN_1(lane), ®);
>> if (ret)
>> return ret;
>>
>> @@ -416,12 +365,12 @@ static int ufs_versal2_phy_ratesel(struct ufs_hba *hba, u32 activelanes, u32 rx_
>> else
>> reg &= ~MPHY_RX_OVRD_VAL;
>>
>> - ret = ufs_versal2_phy_reg_write(hba, RX_OVRD_IN_1(lane), reg);
>> + ret = ufshcd_dwc_phy_reg_write(hba, RX_OVRD_IN_1(lane), reg);
>> if (ret)
>> return ret;
>>
>> do {
>> - ret = ufs_versal2_phy_reg_read(hba, RX_PCS_OUT(lane), ®);
>> + ret = ufshcd_dwc_phy_reg_read(hba, RX_PCS_OUT(lane), ®);
>> if (ret)
>> return ret;
>>
>> @@ -483,12 +432,12 @@ static int ufs_versal2_pwr_change_notify(struct ufs_hba *hba, enum ufs_notify_ch
>>
>> /* Remove rx_req override */
>> for (lane = 0; lane < dev_req_params->lane_tx; lane++) {
>> - ret = ufs_versal2_phy_reg_read(hba, RX_OVRD_IN_1(lane), ®);
>> + ret = ufshcd_dwc_phy_reg_read(hba, RX_OVRD_IN_1(lane), ®);
>> if (ret)
>> return ret;
>>
>> reg &= ~MPHY_RX_OVRD_EN;
>> - ret = ufs_versal2_phy_reg_write(hba, RX_OVRD_IN_1(lane), reg);
>> + ret = ufshcd_dwc_phy_reg_write(hba, RX_OVRD_IN_1(lane), reg);
>> if (ret)
>> return ret;
>> }
>> diff --git a/drivers/ufs/host/ufshcd-dwc.c b/drivers/ufs/host/ufshcd-dwc.c
>> index 21b1cf912dcc..45920793d1a8 100644
>> --- a/drivers/ufs/host/ufshcd-dwc.c
>> +++ b/drivers/ufs/host/ufshcd-dwc.c
>> @@ -141,6 +141,83 @@ int ufshcd_dwc_link_startup_notify(struct ufs_hba *hba,
>> }
>> EXPORT_SYMBOL(ufshcd_dwc_link_startup_notify);
>>
>> +/**
>> + * ufshcd_dwc_phy_reg_write - Write a DWC M-PHY CREG register
>> + * @hba: private structure pointer
>> + * @addr: M-PHY CREG register address
>> + * @val: value to write
>> + *
>> + * Write a 16-bit M-PHY CREG register through the Synopsys DesignWare
>> + * UniPro indirect register access interface.
>> + *
>> + * Return: 0 on success, non-zero value on failure.
>> + */
>> +int ufshcd_dwc_phy_reg_write(struct ufs_hba *hba, u32 addr, u32 val)
>> +{
>> + struct ufshcd_dme_attr_val phy_write_attrs[] = {
>> + { UIC_ARG_MIB(CBCREGADDRLSB), 0, DME_LOCAL },
>> + { UIC_ARG_MIB(CBCREGADDRMSB), 0, DME_LOCAL },
>> + { UIC_ARG_MIB(CBCREGWRLSB), 0, DME_LOCAL },
>> + { UIC_ARG_MIB(CBCREGWRMSB), 0, DME_LOCAL },
>> + { UIC_ARG_MIB(CBCREGRDWRSEL), 1, DME_LOCAL },
>> + { UIC_ARG_MIB(VS_MPHYCFGUPDT), 1, DME_LOCAL }
>> + };
>> +
>> + phy_write_attrs[0].mib_val = (u8)addr;
>> + phy_write_attrs[1].mib_val = (u8)(addr >> 8);
>> + phy_write_attrs[2].mib_val = (u8)val;
>> + phy_write_attrs[3].mib_val = (u8)(val >> 8);
>> +
>> + return ufshcd_dwc_dme_set_attrs(hba, phy_write_attrs,
>> + ARRAY_SIZE(phy_write_attrs));
>> +}
>> +EXPORT_SYMBOL(ufshcd_dwc_phy_reg_write);
>> +
>> +/**
>> + * ufshcd_dwc_phy_reg_read - Read a DWC M-PHY CREG register
>> + * @hba: private structure pointer
>> + * @addr: M-PHY CREG register address
>> + * @val: pointer where the read value is stored
>> + *
>> + * Read a 16-bit M-PHY CREG register through the Synopsys DesignWare
>> + * UniPro indirect register access interface.
>> + *
>> + * Return: 0 on success, non-zero value on failure.
>> + */
>> +int ufshcd_dwc_phy_reg_read(struct ufs_hba *hba, u32 addr, u32 *val)
>> +{
>> + struct ufshcd_dme_attr_val phy_read_attrs[] = {
>> + { UIC_ARG_MIB(CBCREGADDRLSB), 0, DME_LOCAL },
>> + { UIC_ARG_MIB(CBCREGADDRMSB), 0, DME_LOCAL },
>> + { UIC_ARG_MIB(CBCREGRDWRSEL), 0, DME_LOCAL },
>> + { UIC_ARG_MIB(VS_MPHYCFGUPDT), 1, DME_LOCAL }
>> + };
>> + u32 mib_val;
>> + int ret;
>> +
>> + phy_read_attrs[0].mib_val = (u8)addr;
>> + phy_read_attrs[1].mib_val = (u8)(addr >> 8);
>> +
>> + ret = ufshcd_dwc_dme_set_attrs(hba, phy_read_attrs,
>> + ARRAY_SIZE(phy_read_attrs));
>> + if (ret)
>> + return ret;
>> +
>> + ret = ufshcd_dme_get(hba, UIC_ARG_MIB(CBCREGRDLSB), &mib_val);
>> + if (ret)
>> + return ret;
>> +
>> + *val = mib_val;
>> + ret = ufshcd_dme_get(hba, UIC_ARG_MIB(CBCREGRDMSB), &mib_val);
>> + if (ret)
>> + return ret;
>> +
>> + *val |= (mib_val << 8);
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL(ufshcd_dwc_phy_reg_read);
>> +
>> MODULE_AUTHOR("Joao Pinto <Joao.Pinto@synopsys.com>");
>> MODULE_DESCRIPTION("UFS Host driver for Synopsys Designware Core");
>> MODULE_LICENSE("Dual BSD/GPL");
>> diff --git a/drivers/ufs/host/ufshcd-dwc.h b/drivers/ufs/host/ufshcd-dwc.h
>> index 0406f2bb576b..d8673cd54d4c 100644
>> --- a/drivers/ufs/host/ufshcd-dwc.h
>> +++ b/drivers/ufs/host/ufshcd-dwc.h
>> @@ -45,4 +45,6 @@ int ufshcd_dwc_link_startup_notify(struct ufs_hba *hba,
>> enum ufs_notify_change_status status);
>> int ufshcd_dwc_dme_set_attrs(struct ufs_hba *hba,
>> const struct ufshcd_dme_attr_val *v, int n);
>> +int ufshcd_dwc_phy_reg_write(struct ufs_hba *hba, u32 addr, u32 val);
>> +int ufshcd_dwc_phy_reg_read(struct ufs_hba *hba, u32 addr, u32 *val);
>> #endif /* End of Header */
>> --
>> 2.43.0
>>
>>
next prev parent reply other threads:[~2026-08-27 14:11 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-26 13:40 [PATCH 00/10] Add S32N79RDB UFS support Larisa Grigore
2026-08-26 13:40 ` [PATCH 01/10] ufs: unipro: Move Tx/Rx FSM state definitions to unipro.h Larisa Grigore
2026-08-26 15:05 ` Frank Li
2026-08-27 13:59 ` Larisa Ileana Grigore
2026-08-26 13:40 ` [PATCH 02/10] ufs: unipro: Add TX/RX FSM state attributes Larisa Grigore
2026-08-26 15:09 ` Frank Li
2026-08-26 13:40 ` [PATCH 03/10] ufs: host: Add common Hibern8 TX FSM polling helper Larisa Grigore
2026-08-26 15:17 ` Frank Li
2026-08-26 15:33 ` Bart Van Assche
2026-08-27 14:03 ` Larisa Ileana Grigore
2026-08-27 9:14 ` Bough Chen
2026-08-27 14:10 ` Larisa Ileana Grigore
2026-08-27 15:00 ` Stanley Jhu
2026-08-26 13:40 ` [PATCH 04/10] scsi: ufs: Move Versal2 M-PHY CREG access helpers into ufshcd-dwc Larisa Grigore
2026-08-26 15:20 ` Frank Li
2026-08-27 14:11 ` Larisa Ileana Grigore [this message]
2026-08-26 15:36 ` Bart Van Assche
2026-08-27 14:13 ` Larisa Ileana Grigore
2026-08-27 10:47 ` Bough Chen
2026-08-26 13:40 ` [PATCH 05/10] scsi: ufs: dwc: Export common clock divider and link status helpers Larisa Grigore
2026-08-26 15:22 ` Frank Li
2026-08-26 13:40 ` [PATCH 06/10] dt-bindings: ufs: Add compatibles for S32N79 Host Controller platform driver Larisa Grigore
2026-08-26 14:35 ` Frank Li
2026-08-26 15:57 ` Conor Dooley
2026-08-27 14:37 ` Larisa Ileana Grigore
2026-08-27 14:36 ` Larisa Ileana Grigore
2026-08-27 14:45 ` Frank Li
2026-08-27 15:09 ` Larisa Ileana Grigore
2026-08-26 13:40 ` [PATCH 07/10] scsi: ufs: Add NXP S32N79 UFS host controller driver Larisa Grigore
2026-08-26 14:01 ` sashiko-bot
2026-08-26 15:49 ` Frank Li
2026-08-27 14:44 ` Larisa Ileana Grigore
2026-08-26 13:40 ` [PATCH 08/10] arm64: dts: freescale: s32n79: Add UFS host controller Larisa Grigore
2026-08-26 13:40 ` [PATCH 09/10] arm64: dts: freescale: s32n79-rdb: Enable UFS Larisa Grigore
2026-08-26 13:40 ` [PATCH 10/10] MAINTAINERS: Add NXP S32N7 UFS host controller entry Larisa Grigore
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=85c580d7-dcc9-429d-a2ff-e286d4ac0fb5@oss.nxp.com \
--to=larisa.grigore@oss.nxp.com \
--cc=Frank.Li@nxp.com \
--cc=Frank.li@oss.nxp.com \
--cc=James.Bottomley@hansenpartnership.com \
--cc=ajay.neeli@amd.com \
--cc=alim.akhtar@samsung.com \
--cc=aruizrui@redhat.com \
--cc=avri.altman@sandisk.com \
--cc=bvanassche@acm.org \
--cc=clizzi@redhat.com \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=eballetb@redhat.com \
--cc=echanude@redhat.com \
--cc=festevam@gmail.com \
--cc=imx@lists.linux.dev \
--cc=kernel@pengutronix.de \
--cc=krzk+dt@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-scsi@vger.kernel.org \
--cc=mani@kernel.org \
--cc=martin.petersen@oracle.com \
--cc=pedrom.sousa@synopsys.com \
--cc=robh@kernel.org \
--cc=s.hauer@pengutronix.de \
--cc=s32@nxp.com \
--cc=sai.krishna.potthuri@amd.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox