* [PATCH v3 1/3] scsi: ufs: dt-bindings: starfive: Add UFS Host Controller for JHB100 soc
2026-09-11 10:23 [PATCH v3 0/3] Add StarFive JHB100 soc UFS platform driver Minda Chen
@ 2026-09-11 10:23 ` Minda Chen
2026-09-12 10:25 ` Conor Dooley
2026-09-11 10:23 ` [PATCH v3 2/3] scsi: ufs: dwc: Rename amd-versal2 read/write PHY API and move to dwc common file Minda Chen
2026-09-11 10:23 ` [PATCH v3 3/3] scsi: ufs: starfive: Add UFS support for StarFive JHB100 SoC Minda Chen
2 siblings, 1 reply; 7+ messages in thread
From: Minda Chen @ 2026-09-11 10:23 UTC (permalink / raw)
To: Alim Akhtar, Avri Altman, Bart Van Assche, Sai Krishna Potthuri,
Ajay Neeli, James E . J . Bottomley, Martin K . Petersen,
Arnd Bergmann, AngeloGioacchino Del Regno, Conor Dooley,
Rob Herring, Krzysztof Kozlowski, linux-scsi
Cc: linux-kernel, devicetree, Minda Chen
Add devicetree document for UFS Host Controller StarFive JHB100 SoC.
The UFS controller is based on the Synopsys DesignWare UFS controller.
Signed-off-by: Minda Chen <minda.chen@starfivetech.com>
---
.../bindings/ufs/starfive,jhb100-ufs.yaml | 71 +++++++++++++++++++
MAINTAINERS | 5 ++
2 files changed, 76 insertions(+)
create mode 100644 Documentation/devicetree/bindings/ufs/starfive,jhb100-ufs.yaml
diff --git a/Documentation/devicetree/bindings/ufs/starfive,jhb100-ufs.yaml b/Documentation/devicetree/bindings/ufs/starfive,jhb100-ufs.yaml
new file mode 100644
index 000000000000..a65cc9455a85
--- /dev/null
+++ b/Documentation/devicetree/bindings/ufs/starfive,jhb100-ufs.yaml
@@ -0,0 +1,71 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/ufs/starfive,jhb100-ufs.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Starfive Universal Flash Storage (UFS) Controller
+
+maintainers:
+ - Minda Chen <minda.chen@starfivetech.com>
+
+allOf:
+ - $ref: ufs-common.yaml
+
+properties:
+ compatible:
+ const: starfive,jhb100-ufs
+
+ reg:
+ maxItems: 1
+
+ clocks:
+ maxItems: 1
+
+ clock-names:
+ items:
+ - const: main
+
+ resets:
+ items:
+ - description: UFS main reset
+ - description: UFS PHY reset
+
+ reset-names:
+ items:
+ - const: main
+ - const: phy
+
+ interrupts:
+ maxItems: 1
+
+ starfive,syscon:
+ $ref: /schemas/types.yaml#/definitions/phandle
+ description:
+ The phandle to System Register Controller syscon node.
+
+required:
+ - compatible
+ - reg
+ - clocks
+ - clock-names
+ - resets
+ - reset-names
+ - interrupts
+ - starfive,syscon
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ ufs@11b10000 {
+ compatible = "starfive,jhb100-ufs";
+ reg = <0x11b10000 0x20000>;
+ interrupts = <105>;
+ clocks = <&syscrg 4>;
+ clock-names = "main";
+ resets = <&syscrg 10>,
+ <&syscrg 7>;
+ reset-names = "main", "phy";
+ starfive,syscon = <&syscon>;
+ };
diff --git a/MAINTAINERS b/MAINTAINERS
index fc320b373656..10b0d0839ca4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -28095,6 +28095,11 @@ L: linux-scsi@vger.kernel.org
S: Maintained
F: drivers/ufs/host/ufs-renesas.c
+UNIVERSAL FLASH STORAGE HOST CONTROLLER DRIVER STARFIVE
+M: Minda Chen <minda.chen@starfivetech.com>
+S: Maintained
+F: Documentation/devicetree/bindings/ufs/starfive,jhb100-ufs.yaml
+
UNIWILL LAPTOP DRIVER
M: Armin Wolf <W_Armin@gmx.de>
L: platform-driver-x86@vger.kernel.org
--
2.17.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH v3 1/3] scsi: ufs: dt-bindings: starfive: Add UFS Host Controller for JHB100 soc
2026-09-11 10:23 ` [PATCH v3 1/3] scsi: ufs: dt-bindings: starfive: Add UFS Host Controller for JHB100 soc Minda Chen
@ 2026-09-12 10:25 ` Conor Dooley
0 siblings, 0 replies; 7+ messages in thread
From: Conor Dooley @ 2026-09-12 10:25 UTC (permalink / raw)
To: Minda Chen
Cc: Alim Akhtar, Avri Altman, Bart Van Assche, Sai Krishna Potthuri,
Ajay Neeli, James E . J . Bottomley, Martin K . Petersen,
Arnd Bergmann, AngeloGioacchino Del Regno, Rob Herring,
Krzysztof Kozlowski, linux-scsi, linux-kernel, devicetree
[-- Attachment #1: Type: text/plain, Size: 3465 bytes --]
On Fri, Sep 11, 2026 at 06:23:57PM +0800, Minda Chen wrote:
> Add devicetree document for UFS Host Controller StarFive JHB100 SoC.
> The UFS controller is based on the Synopsys DesignWare UFS controller.
>
> Signed-off-by: Minda Chen <minda.chen@starfivetech.com>
> ---
> .../bindings/ufs/starfive,jhb100-ufs.yaml | 71 +++++++++++++++++++
> MAINTAINERS | 5 ++
> 2 files changed, 76 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/ufs/starfive,jhb100-ufs.yaml
>
> diff --git a/Documentation/devicetree/bindings/ufs/starfive,jhb100-ufs.yaml b/Documentation/devicetree/bindings/ufs/starfive,jhb100-ufs.yaml
> new file mode 100644
> index 000000000000..a65cc9455a85
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/ufs/starfive,jhb100-ufs.yaml
> @@ -0,0 +1,71 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/ufs/starfive,jhb100-ufs.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Starfive Universal Flash Storage (UFS) Controller
> +
> +maintainers:
> + - Minda Chen <minda.chen@starfivetech.com>
> +
> +allOf:
> + - $ref: ufs-common.yaml
> +
> +properties:
> + compatible:
> + const: starfive,jhb100-ufs
> +
> + reg:
> + maxItems: 1
> +
> + clocks:
> + maxItems: 1
> +
> + clock-names:
> + items:
> + - const: main
> +
> + resets:
> + items:
> + - description: UFS main reset
> + - description: UFS PHY reset
> +
> + reset-names:
> + items:
> + - const: main
> + - const: phy
> +
> + interrupts:
> + maxItems: 1
> +
> + starfive,syscon:
> + $ref: /schemas/types.yaml#/definitions/phandle
> + description:
> + The phandle to System Register Controller syscon node.
This should probably say "that contains xyz" where xyz is the thing that
you need up access in your driver. The jhb100 has a large number of
syscons, so knowing what to look for here is essential! (although
realistically you'll set this in jhb100.dtsi and noone will have to look
it up themselves)
Otherwise,
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
pw-bot: changes-requested
Thanks,
Conor.
> +
> +required:
> + - compatible
> + - reg
> + - clocks
> + - clock-names
> + - resets
> + - reset-names
> + - interrupts
> + - starfive,syscon
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + ufs@11b10000 {
> + compatible = "starfive,jhb100-ufs";
> + reg = <0x11b10000 0x20000>;
> + interrupts = <105>;
> + clocks = <&syscrg 4>;
> + clock-names = "main";
> + resets = <&syscrg 10>,
> + <&syscrg 7>;
> + reset-names = "main", "phy";
> + starfive,syscon = <&syscon>;
> + };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index fc320b373656..10b0d0839ca4 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -28095,6 +28095,11 @@ L: linux-scsi@vger.kernel.org
> S: Maintained
> F: drivers/ufs/host/ufs-renesas.c
>
> +UNIVERSAL FLASH STORAGE HOST CONTROLLER DRIVER STARFIVE
> +M: Minda Chen <minda.chen@starfivetech.com>
> +S: Maintained
> +F: Documentation/devicetree/bindings/ufs/starfive,jhb100-ufs.yaml
> +
> UNIWILL LAPTOP DRIVER
> M: Armin Wolf <W_Armin@gmx.de>
> L: platform-driver-x86@vger.kernel.org
> --
> 2.17.1
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v3 2/3] scsi: ufs: dwc: Rename amd-versal2 read/write PHY API and move to dwc common file
2026-09-11 10:23 [PATCH v3 0/3] Add StarFive JHB100 soc UFS platform driver Minda Chen
2026-09-11 10:23 ` [PATCH v3 1/3] scsi: ufs: dt-bindings: starfive: Add UFS Host Controller for JHB100 soc Minda Chen
@ 2026-09-11 10:23 ` Minda Chen
2026-09-11 10:32 ` sashiko-bot
2026-09-11 10:23 ` [PATCH v3 3/3] scsi: ufs: starfive: Add UFS support for StarFive JHB100 SoC Minda Chen
2 siblings, 1 reply; 7+ messages in thread
From: Minda Chen @ 2026-09-11 10:23 UTC (permalink / raw)
To: Alim Akhtar, Avri Altman, Bart Van Assche, Sai Krishna Potthuri,
Ajay Neeli, James E . J . Bottomley, Martin K . Petersen,
Arnd Bergmann, AngeloGioacchino Del Regno, Conor Dooley,
Rob Herring, Krzysztof Kozlowski, linux-scsi
Cc: linux-kernel, devicetree, Minda Chen
AMD versal2 UFS using designware ufs mipi PHY. The read/write PHY
register API are common functions for designware ufs PHY. For other
vendors reuse the code, move to common ufshcd-dwc.c file.
Signed-off-by: Minda Chen <minda.chen@starfivetech.com>
---
drivers/ufs/host/ufs-amd-versal2.c | 85 ++++++------------------------
drivers/ufs/host/ufshcd-dwc.c | 53 +++++++++++++++++++
drivers/ufs/host/ufshcd-dwc.h | 2 +
3 files changed, 72 insertions(+), 68 deletions(-)
diff --git a/drivers/ufs/host/ufs-amd-versal2.c b/drivers/ufs/host/ufs-amd-versal2.c
index 2154d6286817..2e671881e7ae 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 = ufs_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 = ufs_dwc_phy_reg_write(hba, FAST_FLAGS(0), reg);
if (ret)
return ret;
- ret = ufs_versal2_phy_reg_read(hba, FAST_FLAGS(1), ®);
+ ret = ufs_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 = ufs_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 = ufs_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 = ufs_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 = ufs_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 = ufs_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 = ufs_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 = ufs_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 = ufs_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 ufs_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 = ufs_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 = ufs_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 = ufs_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 = ufs_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 = ufs_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..b057a78e151c 100644
--- a/drivers/ufs/host/ufshcd-dwc.c
+++ b/drivers/ufs/host/ufshcd-dwc.c
@@ -15,6 +15,59 @@
#include "ufshcd-dwc.h"
#include "ufshci-dwc.h"
+int ufs_dwc_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));
+}
+EXPORT_SYMBOL(ufs_dwc_phy_reg_write);
+
+int ufs_dwc_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;
+}
+EXPORT_SYMBOL(ufs_dwc_phy_reg_read);
+
int ufshcd_dwc_dme_set_attrs(struct ufs_hba *hba,
const struct ufshcd_dme_attr_val *v, int n)
{
diff --git a/drivers/ufs/host/ufshcd-dwc.h b/drivers/ufs/host/ufshcd-dwc.h
index c618bb914904..8091f186a9b3 100644
--- a/drivers/ufs/host/ufshcd-dwc.h
+++ b/drivers/ufs/host/ufshcd-dwc.h
@@ -68,4 +68,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 ufs_dwc_phy_reg_write(struct ufs_hba *hba, u32 addr, u32 val);
+int ufs_dwc_phy_reg_read(struct ufs_hba *hba, u32 addr, u32 *val);
#endif /* End of Header */
--
2.17.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH v3 2/3] scsi: ufs: dwc: Rename amd-versal2 read/write PHY API and move to dwc common file
2026-09-11 10:23 ` [PATCH v3 2/3] scsi: ufs: dwc: Rename amd-versal2 read/write PHY API and move to dwc common file Minda Chen
@ 2026-09-11 10:32 ` sashiko-bot
0 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-09-11 10:32 UTC (permalink / raw)
To: Minda Chen; +Cc: robh, conor+dt, linux-scsi, devicetree
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Data race in `ufs_dwc_phy_reg_write` and `ufs_dwc_phy_reg_read` due to shared mutable `static` arrays
--
commit 0483c36fe85ab166d063a9b9c880720ee52b5d3b
Author: Minda Chen <minda.chen@starfivetech.com>
scsi: ufs: dwc: Rename amd-versal2 read/write PHY API and move to dwc common file
This commit renames the AMD Versal2 UFS read/write PHY register APIs and moves
them to the common ufshcd-dwc.c file. This allows other designware UFS PHY
vendors to reuse the PHY register read/write functions.
> diff --git a/drivers/ufs/host/ufshcd-dwc.c b/drivers/ufs/host/ufshcd-dwc.c
> index 21b1cf912dcc6..b057a78e151c4 100644
> --- a/drivers/ufs/host/ufshcd-dwc.c
> +++ b/drivers/ufs/host/ufshcd-dwc.c
> @@ -15,6 +15,59 @@
> #include "ufshcd-dwc.h"
> #include "ufshci-dwc.h"
>
> +int ufs_dwc_phy_reg_write(struct ufs_hba *hba, u32 addr, u32 val)
> +{
> + static struct ufshcd_dme_attr_val phy_write_attrs[] = {
[Severity: High]
This isn't a bug introduced by this patch, but moving it from the specific
versal2 driver to the generic ufshcd-dwc.c library makes it affect all DWC
UFS controllers.
Since the phy_write_attrs array in ufs_dwc_phy_reg_write() is declared static
but is mutated on every function call below, could this cause a data race?
If multiple UFS host controllers in a system execute PHY configuration or
power state changes concurrently (e.g., during asynchronous device probing),
they could overwrite each other's attribute values in this globally shared
array. This might result in writing incorrect values to the UFS PHY
registers, potentially leading to link instability or storage failures.
> + { 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(ufs_dwc_phy_reg_write);
> +
> +int ufs_dwc_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[] = {
[Severity: High]
This is a pre-existing issue, but similar to the above, the phy_read_attrs
array in ufs_dwc_phy_reg_read() is mutated on every function call.
Could multiple controllers executing concurrently overwrite each other's
values here, leading to incorrect UFS PHY register reads?
> + { 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);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260911102359.83288-1-minda.chen@starfivetech.com?part=2
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v3 3/3] scsi: ufs: starfive: Add UFS support for StarFive JHB100 SoC
2026-09-11 10:23 [PATCH v3 0/3] Add StarFive JHB100 soc UFS platform driver Minda Chen
2026-09-11 10:23 ` [PATCH v3 1/3] scsi: ufs: dt-bindings: starfive: Add UFS Host Controller for JHB100 soc Minda Chen
2026-09-11 10:23 ` [PATCH v3 2/3] scsi: ufs: dwc: Rename amd-versal2 read/write PHY API and move to dwc common file Minda Chen
@ 2026-09-11 10:23 ` Minda Chen
2026-09-11 10:37 ` sashiko-bot
2 siblings, 1 reply; 7+ messages in thread
From: Minda Chen @ 2026-09-11 10:23 UTC (permalink / raw)
To: Alim Akhtar, Avri Altman, Bart Van Assche, Sai Krishna Potthuri,
Ajay Neeli, James E . J . Bottomley, Martin K . Petersen,
Arnd Bergmann, AngeloGioacchino Del Regno, Conor Dooley,
Rob Herring, Krzysztof Kozlowski, linux-scsi
Cc: linux-kernel, devicetree, Minda Chen
Add support for the UFS host controller on JHB100 SoC, built on
the Synopsys DWC UFS controller and using UFSHCD platform driver.
This controller requires specific configurations like
M-PHY/RMMI/UniPro
Signed-off-by: Minda Chen <minda.chen@starfivetech.com>
---
MAINTAINERS | 1 +
drivers/ufs/host/Kconfig | 13 ++
drivers/ufs/host/Makefile | 1 +
drivers/ufs/host/ufs-starfive.c | 297 ++++++++++++++++++++++++++++++++
drivers/ufs/host/ufshcd-dwc.h | 18 ++
5 files changed, 330 insertions(+)
create mode 100644 drivers/ufs/host/ufs-starfive.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 10b0d0839ca4..de81bd15f3bc 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -28099,6 +28099,7 @@ UNIVERSAL FLASH STORAGE HOST CONTROLLER DRIVER STARFIVE
M: Minda Chen <minda.chen@starfivetech.com>
S: Maintained
F: Documentation/devicetree/bindings/ufs/starfive,jhb100-ufs.yaml
+F: drivers/ufs/host/ufs-starfive.c
UNIWILL LAPTOP DRIVER
M: Armin Wolf <W_Armin@gmx.de>
diff --git a/drivers/ufs/host/Kconfig b/drivers/ufs/host/Kconfig
index ff170c0b6da0..9d96f5bb92ef 100644
--- a/drivers/ufs/host/Kconfig
+++ b/drivers/ufs/host/Kconfig
@@ -168,3 +168,16 @@ config SCSI_UFS_AMD_VERSAL2
Select this if you have UFS controller on AMD Versal Gen 2 SoC.
If unsure, say N.
+
+config SCSI_UFS_STARFIVE
+ tristate "Starfive UFS controller platform driver"
+ depends on OF && SCSI_UFSHCD_PLATFORM
+ depends on ARCH_STARFIVE || COMPILE_TEST
+ help
+ This selects the StarFive specific additions to UFSHCD platform driver.
+ UFS host on StarFive needs some vendor specific configuration before
+ accessing the hardware which includes PHY configuration and vendor
+ specific registers.
+
+ Select this if you have UFS controller on StarFive chipset.
+ If unsure, say N.
diff --git a/drivers/ufs/host/Makefile b/drivers/ufs/host/Makefile
index 7d8db67eb23c..9c6c2897692d 100644
--- a/drivers/ufs/host/Makefile
+++ b/drivers/ufs/host/Makefile
@@ -16,3 +16,4 @@ obj-$(CONFIG_SCSI_UFS_ROCKCHIP) += ufs-rockchip.o
obj-$(CONFIG_SCSI_UFS_SPRD) += ufs-sprd.o
obj-$(CONFIG_SCSI_UFS_TI_J721E) += ti-j721e-ufs.o
obj-$(CONFIG_SCSI_UFS_AMD_VERSAL2) += ufs-amd-versal2.o ufshcd-dwc.o
+obj-$(CONFIG_SCSI_UFS_STARFIVE) += ufs-starfive.o ufshcd-dwc.o
diff --git a/drivers/ufs/host/ufs-starfive.c b/drivers/ufs/host/ufs-starfive.c
new file mode 100644
index 000000000000..fab65da0507e
--- /dev/null
+++ b/drivers/ufs/host/ufs-starfive.c
@@ -0,0 +1,297 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Starfive UFS host platform driver
+ *
+ * Copyright (C) 2026 Starfive, Inc.
+ *
+ * Authors: Minda Chen <minda.chen@starfivetech.com>
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/kernel.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/reset.h>
+#include <ufs/unipro.h>
+
+#include "ufshcd-pltfrm.h"
+#include "ufshcd-dwc.h"
+#include "ufshci-dwc.h"
+
+struct ufs_starfive_host {
+ struct ufs_hba *hba;
+ struct regmap *syscon;
+ struct reset_control *core_reset;
+ struct reset_control *phy_reset;
+ struct clk *ufs_clk;
+ bool phy_init;
+};
+
+#define SRAM_STATUS 0x38
+#define SRAM_EXT_LD_DONE BIT(1)
+#define SRAM_INIT_DONE BIT(2)
+#define UFS_REFCLK 0x3c
+#define REFCLK_OEN BIT(8)
+#define RESET_I BIT(9)
+#define RESET_OEN BIT(10)
+
+#define MPHY_POLL_INTERVAL_US 100
+#define MPHY_POLL_TIMEOUT_US 10000
+
+static int ufs_starfive_phy_config(struct ufs_hba *hba, struct ufs_starfive_host *host)
+{
+ static struct ufs_dwc_phy_pair_data phy_data[] = {
+ { RAWAONLANEN_DIG_MPLLA_COARSE_TUNE, 0x51},
+ { MPLL_SKIPCAL_COARSE_TUNE, 0x51},
+ { RX_AFE_ATT_IDAC(0), 0x8a},
+ { RX_AFE_ATT_IDAC(1), 0xc2},
+ { RX_AFE_CTLE_IDAC(0), 0x8e},
+ { RX_AFE_CTLE_IDAC(1), 0x8b},
+ { FAST_FLAGS(0), 0x0004 },
+ { FAST_FLAGS(1), 0x0004 },
+ { RX_ADAPT_DFE(0), 0xa00},
+ { RX_ADAPT_DFE(1), 0xa00},
+ };
+ struct ufs_dwc_phy_pair_data *data;
+ int ret, i;
+
+ for (i = 0; i < ARRAY_SIZE(phy_data); i++) {
+ data = &phy_data[i];
+ ret = ufs_dwc_phy_reg_write(hba, data->addr, data->value);
+ if (ret)
+ return ret;
+ }
+
+ ret = ufshcd_dme_set(hba, UIC_ARG_MIB(VS_MPHYDISABLE), 0);
+ if (ret)
+ return ret;
+
+ ret = ufshcd_dme_set(hba, UIC_ARG_MIB(VS_MPHYCFGUPDT), 1);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+static int ufs_starfive_phy_init(struct ufs_hba *hba)
+{
+ struct ufs_starfive_host *host = ufshcd_get_variant(hba);
+ static struct ufshcd_dme_attr_val rmmi_config[] = {
+ { UIC_ARG_MIB(CBRATESEL), 0x1,
+ DME_LOCAL },
+ { UIC_ARG_MIB(CBREFCLKCTRL2), CBREFREFCLK_GATE_OVR_EN,
+ DME_LOCAL },
+ { UIC_ARG_MIB_SEL(RXSQCONTROL, SELIND_LN0_RX), 0x01,
+ DME_LOCAL },
+ { UIC_ARG_MIB_SEL(RXRHOLDCTRLOPT, SELIND_LN0_RX), 0x02,
+ DME_LOCAL },
+ { UIC_ARG_MIB_SEL(RXSQCONTROL, SELIND_LN1_RX), 0x01,
+ DME_LOCAL },
+ { UIC_ARG_MIB_SEL(RXRHOLDCTRLOPT, SELIND_LN1_RX), 0x02,
+ DME_LOCAL },
+ { UIC_ARG_MIB(EXT_COARSE_TUNE_RATEA), 0x25,
+ DME_LOCAL },
+ { UIC_ARG_MIB(EXT_COARSE_TUNE_RATEB), 0x51,
+ DME_LOCAL },
+ { UIC_ARG_MIB(CBCRCTRL), 0x01, DME_LOCAL },
+ { UIC_ARG_MIB(VS_MPHYCFGUPDT), 0x1,
+ DME_LOCAL },
+ };
+ int ret, val;
+
+ if (host->phy_init)
+ return 0;
+
+ ret = ufshcd_dwc_dme_set_attrs(hba, rmmi_config,
+ ARRAY_SIZE(rmmi_config));
+ if (ret) {
+ dev_err(hba->dev, "set rmmi config failed\n");
+ return ret;
+ }
+
+ ret = reset_control_deassert(host->phy_reset);
+ if (ret) {
+ dev_err(hba->dev, "Failed to reset phy\n");
+ return ret;
+ }
+
+ ret = regmap_read_poll_timeout(host->syscon,
+ SRAM_STATUS, val,
+ (val & SRAM_INIT_DONE),
+ MPHY_POLL_INTERVAL_US,
+ MPHY_POLL_TIMEOUT_US);
+ if (ret) {
+ dev_err(hba->dev, "wait sram init done timeout\n");
+ return ret;
+ }
+
+ regmap_update_bits(host->syscon, SRAM_STATUS,
+ SRAM_EXT_LD_DONE, SRAM_EXT_LD_DONE);
+
+ ret = ufs_starfive_phy_config(hba, host);
+ if (ret) {
+ dev_err(hba->dev, "configure phy failed\n");
+ return ret;
+ }
+
+ host->phy_init = 1;
+
+ return 0;
+}
+
+static int ufs_starfive_init(struct ufs_hba *hba)
+{
+ struct ufs_starfive_host *host;
+ struct device *dev = hba->dev;
+ struct platform_device *pdev;
+ int ret;
+
+ pdev = container_of(dev, struct platform_device, dev);
+ host = devm_kzalloc(dev, sizeof(*host), GFP_KERNEL);
+ if (!host)
+ return dev_err_probe(dev, -ENOMEM,
+ "no memory for starfive ufs host\n");
+
+ host->syscon = syscon_regmap_lookup_by_phandle(dev->of_node,
+ "starfive,syscon");
+
+ if (IS_ERR(host->syscon))
+ return dev_err_probe(dev, PTR_ERR(host->syscon), "getting the regmap failed\n");
+
+ host->core_reset = devm_reset_control_get_exclusive(hba->dev, "main");
+ if (IS_ERR(host->core_reset))
+ return dev_err_probe(dev, PTR_ERR(host->core_reset),
+ "Failed to get core clock resets");
+
+ host->phy_reset = devm_reset_control_get_exclusive(hba->dev, "phy");
+ if (IS_ERR(host->phy_reset))
+ return dev_err_probe(dev, PTR_ERR(host->phy_reset),
+ "Failed to get phy clk reset\n");
+
+ host->ufs_clk = devm_clk_get_enabled(&pdev->dev, "main");
+ if (IS_ERR(host->ufs_clk))
+ return dev_err_probe(dev, PTR_ERR(host->ufs_clk),
+ "Failed to get ufs clock\n");
+
+ regmap_update_bits(host->syscon, UFS_REFCLK,
+ REFCLK_OEN | RESET_OEN, 0);
+ udelay(10);
+ regmap_update_bits(host->syscon, UFS_REFCLK, RESET_I, RESET_I);
+
+ ret = reset_control_deassert(host->core_reset);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "Failed to reset core clock");
+
+ host->hba = hba;
+ ufshcd_set_variant(hba, host);
+ hba->caps |= UFSHCD_CAP_WB_EN;
+
+ return 0;
+}
+
+static void ufs_starfive_exit(struct ufs_hba *hba)
+{
+ struct ufs_starfive_host *host = ufshcd_get_variant(hba);
+
+ if (host->phy_init) {
+ reset_control_assert(host->phy_reset);
+ host->phy_init = 0;
+ }
+ clk_disable_unprepare(host->ufs_clk);
+
+ reset_control_assert(host->core_reset);
+}
+
+static int ufs_starfive_link_startup_notify(struct ufs_hba *hba,
+ enum ufs_notify_change_status status)
+{
+ int ret;
+
+ if (status == PRE_CHANGE) {
+ ret = ufshcd_vops_phy_initialization(hba);
+ if (ret) {
+ dev_err(hba->dev, "Phy setup failed (%d)\n", ret);
+ return ret;
+ }
+ } else { /* POST_CHANGE */
+ return ufshcd_dwc_link_startup_notify(hba, status);
+ }
+
+ return 0;
+}
+
+static int ufs_starfive_hce_enable_notify(struct ufs_hba *hba,
+ enum ufs_notify_change_status status)
+{
+ u32 val;
+
+ if (status != POST_CHANGE)
+ return 0;
+
+ /* Disable Gating clock. Auto hibernation quirk */
+ val = ufshcd_readl(hba, REG_BUSTHRTL);
+ val &= ~(LP_AH8_POWER_GATING_EN
+ | LP_POWER_GATING_EN
+ | CLK_GATING_EN);
+ ufshcd_writel(hba, val, REG_BUSTHRTL);
+
+ return 0;
+}
+
+static struct ufs_hba_variant_ops ufs_hba_vops = {
+ .name = "ufs_starfive_platform",
+ .init = ufs_starfive_init,
+ .exit = ufs_starfive_exit,
+ .link_startup_notify = ufs_starfive_link_startup_notify,
+ .phy_initialization = ufs_starfive_phy_init,
+ .hce_enable_notify = ufs_starfive_hce_enable_notify,
+};
+
+static int ufs_starfive_probe(struct platform_device *pdev)
+{
+ int err;
+
+ /* Perform generic probe */
+ err = ufshcd_pltfrm_init(pdev, &ufs_hba_vops);
+ if (err)
+ dev_err(&pdev->dev, "ufshcd_pltfrm_init() failed %d\n", err);
+
+ return err;
+}
+
+static void ufs_starfive_remove(struct platform_device *pdev)
+{
+ ufshcd_pltfrm_remove(pdev);
+}
+
+static const struct dev_pm_ops ufs_starfive_pm_ops = {
+ SET_SYSTEM_SLEEP_PM_OPS(ufshcd_system_suspend, ufshcd_system_resume)
+ SET_RUNTIME_PM_OPS(ufshcd_runtime_suspend, ufshcd_runtime_resume, NULL)
+};
+
+static const struct of_device_id ufs_starfive_pltfm_match[] = {
+ { .compatible = "starfive,jhb100-ufs", },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, ufs_starfive_pltfm_match);
+
+static struct platform_driver ufs_starfive_driver = {
+ .probe = ufs_starfive_probe,
+ .remove = ufs_starfive_remove,
+ .driver = {
+ .name = "ufs-starfive",
+ .pm = pm_ptr(&ufs_starfive_pm_ops),
+ .of_match_table = of_match_ptr(ufs_starfive_pltfm_match),
+ },
+};
+
+module_platform_driver(ufs_starfive_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:ufs-starfive");
+MODULE_DESCRIPTION("Starfive UFS host platform glue driver");
diff --git a/drivers/ufs/host/ufshcd-dwc.h b/drivers/ufs/host/ufshcd-dwc.h
index 8091f186a9b3..71a60ede6805 100644
--- a/drivers/ufs/host/ufshcd-dwc.h
+++ b/drivers/ufs/host/ufshcd-dwc.h
@@ -12,7 +12,15 @@
#include <ufs/ufshcd.h>
+/* ufshcd vendor specific register */
+#define REG_BUSTHRTL 0xc0
+#define LP_AH8_POWER_GATING_EN BIT(17)
+#define LP_POWER_GATING_EN BIT(16)
+#define CLK_GATING_EN BIT(12)
+
/* RMMI Attributes */
+#define RXSQCONTROL 0x8009
+#define RXRHOLDCTRLOPT 0x8013
#define CBREFCLKCTRL2 0x8132
#define CBCRCTRL 0x811F
#define CBC10DIRECTCONF2 0x810E
@@ -24,6 +32,8 @@
#define CBCREGRDLSB 0x811A
#define CBCREGRDMSB 0x811B
#define CBCREGRDWRSEL 0x811C
+#define EXT_COARSE_TUNE_RATEA 0x814D
+#define EXT_COARSE_TUNE_RATEB 0x814E
#define CBREFREFCLK_GATE_OVR_EN BIT(7)
@@ -32,12 +42,15 @@
#define MRX_FSM_STATE 0xC1
/* M-PHY registers */
+#define MPLL_SKIPCAL_COARSE_TUNE 0x28
#define RX_OVRD_IN_1(n) (0x3006 + ((n) * 0x100))
#define RX_PCS_OUT(n) (0x300F + ((n) * 0x100))
#define FAST_FLAGS(n) (0x401C + ((n) * 0x100))
+#define RX_ADAPT_DFE(n) (0x401E + ((n) * 0x100))
#define RX_AFE_ATT_IDAC(n) (0x4000 + ((n) * 0x100))
#define RX_AFE_CTLE_IDAC(n) (0x4001 + ((n) * 0x100))
#define FW_CALIB_CCFG(n) (0x404D + ((n) * 0x100))
+#define RAWAONLANEN_DIG_MPLLA_COARSE_TUNE 0x7014
/* Tx/Rx FSM state */
enum rx_fsm_state {
@@ -64,6 +77,11 @@ struct ufshcd_dme_attr_val {
u8 peer;
};
+struct ufs_dwc_phy_pair_data {
+ u32 addr;
+ u32 value;
+};
+
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,
--
2.17.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH v3 3/3] scsi: ufs: starfive: Add UFS support for StarFive JHB100 SoC
2026-09-11 10:23 ` [PATCH v3 3/3] scsi: ufs: starfive: Add UFS support for StarFive JHB100 SoC Minda Chen
@ 2026-09-11 10:37 ` sashiko-bot
0 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-09-11 10:37 UTC (permalink / raw)
To: Minda Chen; +Cc: linux-scsi, robh, conor+dt, devicetree
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [High] Double disable and unprepare of the UFS clock (`ufs_clk`) during driver teardown.
- [High] Resource leak of the `phy_reset` line if PHY initialization fails.
Pre-existing issues:
- [High] Data race in `ufs_dwc_phy_reg_write` due to the use of an unprotected static array.
--
commit 7e44e99d3adeb04d28f12e6d3a13f795f6d5cf86
Author: Minda Chen <minda.chen@starfivetech.com>
scsi: ufs: starfive: Add UFS support for StarFive JHB100 SoC
Add support for the UFS host controller on JHB100 SoC, built on
the Synopsys DWC UFS controller and using UFSHCD platform driver.
This controller requires specific configurations like
M-PHY/RMMI/UniPro
> diff --git a/drivers/ufs/host/ufs-starfive.c b/drivers/ufs/host/ufs-starfive.c
> new file mode 100644
> index 0000000000000..fab65da0507e8
> --- /dev/null
> +++ b/drivers/ufs/host/ufs-starfive.c
> @@ -0,0 +1,297 @@
[ ... ]
> +static int ufs_starfive_phy_config(struct ufs_hba *hba, struct ufs_starfive_host *host)
> +{
> + static struct ufs_dwc_phy_pair_data phy_data[] = {
> + { RAWAONLANEN_DIG_MPLLA_COARSE_TUNE, 0x51},
> + { MPLL_SKIPCAL_COARSE_TUNE, 0x51},
> + { RX_AFE_ATT_IDAC(0), 0x8a},
> + { RX_AFE_ATT_IDAC(1), 0xc2},
> + { RX_AFE_CTLE_IDAC(0), 0x8e},
> + { RX_AFE_CTLE_IDAC(1), 0x8b},
> + { FAST_FLAGS(0), 0x0004 },
> + { FAST_FLAGS(1), 0x0004 },
> + { RX_ADAPT_DFE(0), 0xa00},
> + { RX_ADAPT_DFE(1), 0xa00},
> + };
> + struct ufs_dwc_phy_pair_data *data;
> + int ret, i;
> +
> + for (i = 0; i < ARRAY_SIZE(phy_data); i++) {
> + data = &phy_data[i];
> + ret = ufs_dwc_phy_reg_write(hba, data->addr, data->value);
[Severity: High]
This is a pre-existing issue, but calling ufs_dwc_phy_reg_write() here
appears to expose a data race if multiple Synopsys DWC UFS host controllers
are probed concurrently.
Looking at ufs_dwc_phy_reg_write() in drivers/ufs/host/ufshcd-dwc.c:
int ufs_dwc_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 },
...
phy_write_attrs[0].mib_val = (u8)addr;
Because phy_write_attrs is declared as static and modified before being
passed to ufshcd_dwc_dme_set_attrs(), concurrent invocations will clobber
each other's configuration payloads.
Can this cause undefined device behavior or link startup failures if multiple
controllers execute PHY initialization simultaneously?
> + if (ret)
> + return ret;
> + }
[ ... ]
> +static int ufs_starfive_phy_init(struct ufs_hba *hba)
> +{
[ ... ]
> + ret = reset_control_deassert(host->phy_reset);
> + if (ret) {
> + dev_err(hba->dev, "Failed to reset phy\n");
> + return ret;
> + }
> +
> + ret = regmap_read_poll_timeout(host->syscon,
> + SRAM_STATUS, val,
> + (val & SRAM_INIT_DONE),
> + MPHY_POLL_INTERVAL_US,
> + MPHY_POLL_TIMEOUT_US);
> + if (ret) {
> + dev_err(hba->dev, "wait sram init done timeout\n");
> + return ret;
> + }
[ ... ]
> + host->phy_init = 1;
> +
> + return 0;
> +}
[ ... ]
> +static int ufs_starfive_init(struct ufs_hba *hba)
> +{
[ ... ]
> + host->ufs_clk = devm_clk_get_enabled(&pdev->dev, "main");
> + if (IS_ERR(host->ufs_clk))
> + return dev_err_probe(dev, PTR_ERR(host->ufs_clk),
> + "Failed to get ufs clock\n");
[ ... ]
> +static void ufs_starfive_exit(struct ufs_hba *hba)
> +{
> + struct ufs_starfive_host *host = ufshcd_get_variant(hba);
> +
> + if (host->phy_init) {
> + reset_control_assert(host->phy_reset);
> + host->phy_init = 0;
> + }
[Severity: High]
Could this leak the phy_reset line in a deasserted state if initialization
fails partway through?
If ufs_starfive_phy_init() successfully deasserts host->phy_reset, but a
subsequent operation (like regmap_read_poll_timeout()) fails, it returns
early and leaves host->phy_init as 0.
When the probe rollback path subsequently calls ufs_starfive_exit(), it sees
host->phy_init == 0 and skips asserting the reset line.
Is it possible this leaves the hardware in an invalid state that prevents
subsequent driver probe retries from succeeding?
> + clk_disable_unprepare(host->ufs_clk);
[Severity: High]
Will this result in an unbalanced clock disable during driver teardown?
In ufs_starfive_init(), host->ufs_clk is acquired using
devm_clk_get_enabled(). This API automatically registers a devres action that
calls clk_disable_unprepare() when the driver is unbound.
By explicitly calling clk_disable_unprepare(host->ufs_clk) here in
ufs_starfive_exit(), it seems this might disable the clock a second time when
the devres framework fires its cleanup action, triggering a kernel warning.
> +
> + reset_control_assert(host->core_reset);
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260911102359.83288-1-minda.chen@starfivetech.com?part=3
^ permalink raw reply [flat|nested] 7+ messages in thread