* Re: [PATCH v2 2/2] dt-bindings: phy: qcom,qmp-usb: Add ipq5210 USB3 PHY
From: Krzysztof Kozlowski @ 2026-06-08 20:06 UTC (permalink / raw)
To: Varadarajan Narayanan
Cc: vkoul, neil.armstrong, robh, krzk+dt, conor+dt, quic_wcheng,
linux-arm-msm, linux-phy, devicetree, linux-kernel
In-Reply-To: <20260608103344.2740174-3-varadarajan.narayanan@oss.qualcomm.com>
On Mon, Jun 08, 2026 at 04:03:44PM +0530, Varadarajan Narayanan wrote:
> Add dt-bindings for the USB3 QMP PHY found on the Qualcomm IPQ5210 SoC. The
> IPQ5210 PHY is compatible with the IPQ9574 PHY, so add it as a fallback-
> compatible entry using a oneOf construct rather than a plain enum entry.
>
> Signed-off-by: Varadarajan Narayanan <varadarajan.narayanan@oss.qualcomm.com>
> ---
> .../phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml | 46 +++++++++++--------
> 1 file changed, 26 insertions(+), 20 deletions(-)
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Best regards,
Krzysztof
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v2 1/2] dt-bindings: phy: qcom,qusb2: Document IPQ5210 compatible
From: Krzysztof Kozlowski @ 2026-06-08 20:04 UTC (permalink / raw)
To: Varadarajan Narayanan
Cc: vkoul, neil.armstrong, robh, krzk+dt, conor+dt, quic_wcheng,
linux-arm-msm, linux-phy, devicetree, linux-kernel
In-Reply-To: <20260608103344.2740174-2-varadarajan.narayanan@oss.qualcomm.com>
On Mon, Jun 08, 2026 at 04:03:43PM +0530, Varadarajan Narayanan wrote:
> Document the QUSB2 PHY compatible for the IPQ5210 SoC. The IPQ5210 PHY is
> compatible with the IPQ6018 QUSB2 PHY, so allow it to use
> qcom,ipq6018-qusb2-phy as the fallback compatible.
>
> Signed-off-by: Varadarajan Narayanan <varadarajan.narayanan@oss.qualcomm.com>
> ---
> Documentation/devicetree/bindings/phy/qcom,qusb2-phy.yaml | 4 ++++
> 1 file changed, 4 insertions(+)
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Best regards,
Krzysztof
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v3 3/3] phy: qcom-qmp-ufs: Add UFS PHY support on Hawi
From: Dmitry Baryshkov @ 2026-06-08 18:37 UTC (permalink / raw)
To: palash.kambar
Cc: vkoul, neil.armstrong, robh, krzk+dt, conor+dt, mani, alim.akhtar,
bvanassche, andersson, abel.vesa, luca.weiss, linux-arm-msm,
linux-phy, devicetree, linux-kernel, linux-scsi, nitin.rawat
In-Reply-To: <20260526090956.2340262-4-palash.kambar@oss.qualcomm.com>
On Tue, May 26, 2026 at 02:39:56PM +0530, palash.kambar@oss.qualcomm.com wrote:
> From: Palash Kambar <palash.kambar@oss.qualcomm.com>
>
> Add the init sequence tables and config for the UFS QMP phy found in
> the Hawi SoC.
>
> Signed-off-by: Palash Kambar <palash.kambar@oss.qualcomm.com>
> ---
> .../phy/qualcomm/phy-qcom-qmp-pcs-ufs-v7.h | 24 +++
> .../phy-qcom-qmp-qserdes-txrx-ufs-v8.h | 37 +++++
> drivers/phy/qualcomm/phy-qcom-qmp-ufs.c | 139 ++++++++++++++++++
> 3 files changed, 200 insertions(+)
> create mode 100644 drivers/phy/qualcomm/phy-qcom-qmp-pcs-ufs-v7.h
> create mode 100644 drivers/phy/qualcomm/phy-qcom-qmp-qserdes-txrx-ufs-v8.h
>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
--
With best wishes
Dmitry
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v3 3/3] phy: qcom-qmp-ufs: Add UFS PHY support on Hawi
From: Palash Kambar @ 2026-06-08 15:15 UTC (permalink / raw)
To: vkoul, neil.armstrong, robh, krzk+dt, conor+dt, mani, alim.akhtar,
bvanassche, andersson, dmitry.baryshkov, abel.vesa, luca.weiss
Cc: linux-arm-msm, linux-phy, devicetree, linux-kernel, linux-scsi,
nitin.rawat
In-Reply-To: <20260526090956.2340262-4-palash.kambar@oss.qualcomm.com>
On 5/26/2026 2:39 PM, palash.kambar@oss.qualcomm.com wrote:
> From: Palash Kambar <palash.kambar@oss.qualcomm.com>
>
> Add the init sequence tables and config for the UFS QMP phy found in
> the Hawi SoC.
>
> Signed-off-by: Palash Kambar <palash.kambar@oss.qualcomm.com>
> ---
> .../phy/qualcomm/phy-qcom-qmp-pcs-ufs-v7.h | 24 +++
> .../phy-qcom-qmp-qserdes-txrx-ufs-v8.h | 37 +++++
> drivers/phy/qualcomm/phy-qcom-qmp-ufs.c | 139 ++++++++++++++++++
> 3 files changed, 200 insertions(+)
> create mode 100644 drivers/phy/qualcomm/phy-qcom-qmp-pcs-ufs-v7.h
> create mode 100644 drivers/phy/qualcomm/phy-qcom-qmp-qserdes-txrx-ufs-v8.h
>
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcs-ufs-v7.h b/drivers/phy/qualcomm/phy-qcom-qmp-pcs-ufs-v7.h
> new file mode 100644
> index 000000000000..e80d3dd6a190
> --- /dev/null
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcs-ufs-v7.h
> @@ -0,0 +1,24 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2026, The Linux Foundation. All rights reserved.
> + */
> +
> +#ifndef QCOM_PHY_QMP_PCS_UFS_V7_H_
> +#define QCOM_PHY_QMP_PCS_UFS_V7_H_
> +
> +/* Only for QMP V7 PHY - UFS PCS registers */
> +#define QPHY_V7_PCS_UFS_PHY_START 0x000
> +#define QPHY_V7_PCS_UFS_POWER_DOWN_CONTROL 0x004
> +#define QPHY_V7_PCS_UFS_SW_RESET 0x008
> +#define QPHY_V7_PCS_UFS_PCS_CTRL1 0x01C
> +#define QPHY_V7_PCS_UFS_PLL_CNTL 0x028
> +#define QPHY_V7_PCS_UFS_TX_LARGE_AMP_DRV_LVL 0x02C
> +#define QPHY_V7_PCS_UFS_TX_HSGEAR_CAPABILITY 0x060
> +#define QPHY_V7_PCS_UFS_RX_HSGEAR_CAPABILITY 0x094
> +#define QPHY_V7_PCS_UFS_LINECFG_DISABLE 0x140
> +#define QPHY_V7_PCS_UFS_RX_SIGDET_CTRL2 0x150
> +#define QPHY_V7_PCS_UFS_READY_STATUS 0x16c
> +#define QPHY_V7_PCS_UFS_TX_MID_TERM_CTRL1 0x1b8
> +#define QPHY_V7_PCS_UFS_MULTI_LANE_CTRL1 0x1c0
> +
> +#endif
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-qserdes-txrx-ufs-v8.h b/drivers/phy/qualcomm/phy-qcom-qmp-qserdes-txrx-ufs-v8.h
> new file mode 100644
> index 000000000000..5f923c3e64ec
> --- /dev/null
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-qserdes-txrx-ufs-v8.h
> @@ -0,0 +1,37 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2026, The Linux Foundation. All rights reserved.
> + */
> +
> +#ifndef QCOM_PHY_QMP_QSERDES_TXRX_UFS_V8_H_
> +#define QCOM_PHY_QMP_QSERDES_TXRX_UFS_V8_H_
> +
> +#define QSERDES_UFS_V8_TX_RES_CODE_LANE_OFFSET_TX (0x34)
> +#define QSERDES_UFS_V8_TX_RES_CODE_LANE_OFFSET_RX (0x38)
> +#define QSERDES_UFS_V8_TX_LANE_MODE_1 (0x80)
> +#define QSERDES_UFS_V8_RX_UCDR_FO_GAIN_RATE2 (0x1BC)
> +#define QSERDES_UFS_V8_RX_UCDR_FO_GAIN_RATE4 (0x1C4)
> +#define QSERDES_UFS_V8_RX_UCDR_SO_GAIN_RATE4 (0x1DC)
> +#define QSERDES_UFS_V8_RX_EQ_OFFSET_ADAPTOR_CNTRL1 (0x2C8)
> +#define QSERDES_UFS_V8_RX_UCDR_PI_CONTROLS (0x1E4)
> +#define QSERDES_UFS_V8_RX_OFFSET_ADAPTOR_CNTRL3 (0x2D0)
> +#define QSERDES_UFS_V8_RX_UCDR_FASTLOCK_COUNT_HIGH_RATE4 (0x120)
> +#define QSERDES_UFS_V8_RX_UCDR_FASTLOCK_FO_GAIN_RATE4 (0xD4)
> +#define QSERDES_UFS_V8_RX_UCDR_FASTLOCK_SO_GAIN_RATE4 (0xEC)
> +#define QSERDES_UFS_V8_RX_VGA_CAL_MAN_VAL (0x288)
> +#define QSERDES_UFS_V8_RX_EQU_ADAPTOR_CNTRL4 (0x2B0)
> +#define QSERDES_UFS_V8_RX_MODE_RATE_0_1_B4 (0x324)
> +#define QSERDES_UFS_V8_RX_MODE_RATE4_SA_B7 (0x3B4)
> +#define QSERDES_UFS_V8_RX_MODE_RATE4_SA_B9 (0x3BC)
> +#define QSERDES_UFS_V8_RX_MODE_RATE4_SB_B7 (0x3E0)
> +#define QSERDES_UFS_V8_RX_MODE_RATE4_SB_B9 (0x3E8)
> +#define QSERDES_UFS_V8_RX_MODE_RATE5_SA_B7 (0x40C)
> +#define QSERDES_UFS_V8_RX_MODE_RATE5_SA_B9 (0x414)
> +#define QSERDES_UFS_V8_RX_MODE_RATE5_SB_B7 (0x438)
> +#define QSERDES_UFS_V8_RX_MODE_RATE5_SB_B9 (0x440)
> +#define QSERDES_UFS_V8_RX_UCDR_SO_SATURATION (0xF4)
> +#define QSERDES_UFS_V8_RX_TERM_BW_CTRL0 (0x1AC)
> +#define QSERDES_UFS_V8_RX_DLL0_FTUNE_CTRL (0x498)
> +#define QSERDES_UFS_V8_RX_SIGDET_CAL_TRIM (0x4d0)
> +
> +#endif
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c b/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c
> index 771bc7c2ab50..2fac3a7eb820 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c
> @@ -29,9 +29,11 @@
> #include "phy-qcom-qmp-pcs-ufs-v4.h"
> #include "phy-qcom-qmp-pcs-ufs-v5.h"
> #include "phy-qcom-qmp-pcs-ufs-v6.h"
> +#include "phy-qcom-qmp-pcs-ufs-v7.h"
>
> #include "phy-qcom-qmp-qserdes-txrx-ufs-v6.h"
> #include "phy-qcom-qmp-qserdes-txrx-ufs-v7.h"
> +#include "phy-qcom-qmp-qserdes-txrx-ufs-v8.h"
>
> /* QPHY_PCS_READY_STATUS bit */
> #define PCS_READY BIT(0)
> @@ -84,6 +86,13 @@ static const unsigned int ufsphy_v6_regs_layout[QPHY_LAYOUT_SIZE] = {
> [QPHY_PCS_POWER_DOWN_CONTROL] = QPHY_V6_PCS_UFS_POWER_DOWN_CONTROL,
> };
>
> +static const unsigned int ufsphy_v7_regs_layout[QPHY_LAYOUT_SIZE] = {
> + [QPHY_START_CTRL] = QPHY_V7_PCS_UFS_PHY_START,
> + [QPHY_PCS_READY_STATUS] = QPHY_V7_PCS_UFS_READY_STATUS,
> + [QPHY_SW_RESET] = QPHY_V7_PCS_UFS_SW_RESET,
> + [QPHY_PCS_POWER_DOWN_CONTROL] = QPHY_V7_PCS_UFS_POWER_DOWN_CONTROL,
> +};
> +
> static const struct qmp_phy_init_tbl milos_ufsphy_serdes[] = {
> QMP_PHY_INIT_CFG(QSERDES_V6_COM_SYSCLK_EN_SEL, 0xd9),
> QMP_PHY_INIT_CFG(QSERDES_V6_COM_CMN_CONFIG_1, 0x16),
> @@ -1306,6 +1315,11 @@ static const struct regulator_bulk_data sm8750_ufsphy_vreg_l[] = {
> { .supply = "vdda-pll", .init_load_uA = 18300 },
> };
>
> +static const struct regulator_bulk_data hawi_ufsphy_vreg_l[] = {
> + { .supply = "vdda-phy", .init_load_uA = 324000 },
> + { .supply = "vdda-pll", .init_load_uA = 27000 },
> +};
> +
> static const struct qmp_ufs_offsets qmp_ufs_offsets = {
> .serdes = 0,
> .pcs = 0xc00,
> @@ -1324,6 +1338,15 @@ static const struct qmp_ufs_offsets qmp_ufs_offsets_v6 = {
> .rx2 = 0x1a00,
> };
>
> +static const struct qmp_ufs_offsets qmp_ufs_offsets_v7 = {
> + .serdes = 0,
> + .pcs = 0x0400,
> + .tx = 0x2000,
> + .rx = 0x2000,
> + .tx2 = 0x3000,
> + .rx2 = 0x3000,
> +};
> +
> static const struct qmp_phy_cfg milos_ufsphy_cfg = {
> .lanes = 2,
>
> @@ -1844,6 +1867,119 @@ static const struct qmp_phy_cfg sm8750_ufsphy_cfg = {
>
> };
>
> +static const struct qmp_phy_init_tbl hawi_ufsphy_serdes[] = {
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_SYSCLK_EN_SEL, 0xd9),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_CMN_CONFIG_1, 0x16),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_HSCLK_SEL_1, 0x11),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_HSCLK_HS_SWITCH_SEL_1, 0x00),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_LOCK_CMP_EN, 0x01),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_LOCK_CMP_CFG, 0x60),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_PLL_IVCO, 0x1f),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_PLL_IVCO_MODE1, 0x1f),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_CMN_IETRIM, 0x07),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_CMN_IPTRIM, 0x20),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_VCO_TUNE_MAP, 0x04),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_VCO_TUNE_CTRL, 0x40),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_ADAPTIVE_ANALOG_CONFIG, 0x06),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_DEC_START_MODE0, 0x41),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_CP_CTRL_MODE0, 0x06),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_PLL_RCTRL_MODE0, 0x18),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_PLL_CCTRL_MODE0, 0x14),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_CP_CTRL_ADAPTIVE_MODE0, 0x06),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_PLL_RCCTRL_ADAPTIVE_MODE0, 0x18),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_PLL_CCTRL_ADAPTIVE_MODE0, 0x14),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_LOCK_CMP1_MODE0, 0x7f),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_LOCK_CMP2_MODE0, 0x06),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_BIN_VCOCAL_CMP_CODE1_MODE0, 0x92),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_BIN_VCOCAL_CMP_CODE2_MODE0, 0x1e),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_DEC_START_MODE1, 0x4c),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_CP_CTRL_MODE1, 0x06),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_PLL_RCTRL_MODE1, 0x18),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_PLL_CCTRL_MODE1, 0x14),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_CP_CTRL_ADAPTIVE_MODE1, 0x06),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_PLL_RCCTRL_ADAPTIVE_MODE1, 0x18),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_PLL_CCTRL_ADAPTIVE_MODE1, 0x14),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_LOCK_CMP1_MODE1, 0x99),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_LOCK_CMP2_MODE1, 0x07),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_BIN_VCOCAL_CMP_CODE1_MODE1, 0xbe),
> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_BIN_VCOCAL_CMP_CODE2_MODE1, 0x23),
> +};
> +
> +static const struct qmp_phy_init_tbl hawi_ufsphy_tx[] = {
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_TX_LANE_MODE_1, 0x0c),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_TX_RES_CODE_LANE_OFFSET_TX, 0x07),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_TX_RES_CODE_LANE_OFFSET_RX, 0x17),
> +};
> +
> +static const struct qmp_phy_init_tbl hawi_ufsphy_rx[] = {
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_UCDR_FO_GAIN_RATE2, 0x0c),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_UCDR_FO_GAIN_RATE4, 0x0c),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_UCDR_SO_GAIN_RATE4, 0x04),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_EQ_OFFSET_ADAPTOR_CNTRL1, 0x14),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_UCDR_PI_CONTROLS, 0x07),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_OFFSET_ADAPTOR_CNTRL3, 0x0e),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_UCDR_FASTLOCK_COUNT_HIGH_RATE4, 0x02),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_UCDR_FASTLOCK_FO_GAIN_RATE4, 0x1c),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_UCDR_FASTLOCK_SO_GAIN_RATE4, 0x06),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_VGA_CAL_MAN_VAL, 0x8e),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_EQU_ADAPTOR_CNTRL4, 0x0f),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_MODE_RATE_0_1_B4, 0xb8),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_MODE_RATE4_SA_B7, 0x66),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_MODE_RATE4_SA_B9, 0x1f),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_MODE_RATE4_SB_B7, 0x66),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_MODE_RATE4_SB_B9, 0x1f),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_MODE_RATE5_SA_B7, 0x66),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_MODE_RATE5_SA_B9, 0x1f),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_MODE_RATE5_SB_B7, 0x66),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_MODE_RATE5_SB_B9, 0x1f),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_UCDR_SO_SATURATION, 0x1f),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_TERM_BW_CTRL0, 0xfa),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_DLL0_FTUNE_CTRL, 0x30),
> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_SIGDET_CAL_TRIM, 0x77),
> +};
> +
> +static const struct qmp_phy_init_tbl hawi_ufsphy_pcs[] = {
> + QMP_PHY_INIT_CFG(QPHY_V7_PCS_UFS_TX_MID_TERM_CTRL1, 0x43),
> + QMP_PHY_INIT_CFG(QPHY_V7_PCS_UFS_PCS_CTRL1, 0x42),
> + QMP_PHY_INIT_CFG(QPHY_V7_PCS_UFS_TX_LARGE_AMP_DRV_LVL, 0x0f),
> + QMP_PHY_INIT_CFG(QPHY_V7_PCS_UFS_RX_SIGDET_CTRL2, 0x68),
> + QMP_PHY_INIT_CFG(QPHY_V7_PCS_UFS_MULTI_LANE_CTRL1, 0x02),
> +};
> +
> +static const struct qmp_phy_init_tbl hawi_ufsphy_g5_pcs[] = {
> + QMP_PHY_INIT_CFG(QPHY_V7_PCS_UFS_PLL_CNTL, 0x3b),
> + QMP_PHY_INIT_CFG(QPHY_V7_PCS_UFS_TX_HSGEAR_CAPABILITY, 0x06),
> + QMP_PHY_INIT_CFG(QPHY_V7_PCS_UFS_RX_HSGEAR_CAPABILITY, 0x06),
> +};
> +
> +static const struct qmp_phy_cfg hawi_ufsphy_cfg = {
> + .lanes = 2,
> +
> + .offsets = &qmp_ufs_offsets_v7,
> + .max_supported_gear = UFS_HS_G5,
> +
> + .tbls = {
> + .serdes = hawi_ufsphy_serdes,
> + .serdes_num = ARRAY_SIZE(hawi_ufsphy_serdes),
> + .tx = hawi_ufsphy_tx,
> + .tx_num = ARRAY_SIZE(hawi_ufsphy_tx),
> + .rx = hawi_ufsphy_rx,
> + .rx_num = ARRAY_SIZE(hawi_ufsphy_rx),
> + .pcs = hawi_ufsphy_pcs,
> + .pcs_num = ARRAY_SIZE(hawi_ufsphy_pcs),
> + },
> +
> + .tbls_hs_overlay[0] = {
> + .pcs = hawi_ufsphy_g5_pcs,
> + .pcs_num = ARRAY_SIZE(hawi_ufsphy_g5_pcs),
> + .max_gear = UFS_HS_G5,
> + },
> +
> + .vreg_list = hawi_ufsphy_vreg_l,
> + .num_vregs = ARRAY_SIZE(hawi_ufsphy_vreg_l),
> + .regs = ufsphy_v7_regs_layout,
> +};
> +
> static void qmp_ufs_serdes_init(struct qmp_ufs *qmp, const struct qmp_phy_cfg_tbls *tbls)
> {
> void __iomem *serdes = qmp->serdes;
> @@ -2258,6 +2394,9 @@ static int qmp_ufs_probe(struct platform_device *pdev)
>
> static const struct of_device_id qmp_ufs_of_match_table[] = {
> {
> + .compatible = "qcom,hawi-qmp-ufs-phy",
> + .data = &hawi_ufsphy_cfg,
> + }, {
> .compatible = "qcom,milos-qmp-ufs-phy",
> .data = &milos_ufsphy_cfg,
> }, {
Hi Dmitry,
I have addressed your previous review comments, please let me know if there are any other comments from your side.
Thanks.
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v3 phy-next 14/16] dt-bindings: phy: lynx-10g: initial document
From: Vladimir Oltean @ 2026-06-08 12:03 UTC (permalink / raw)
To: Rob Herring
Cc: Conor Dooley, sashiko-reviews, vkoul, neil.armstrong, conor+dt,
linux-phy, devicetree, olteanv
In-Reply-To: <20260605202458.GA6304-robh@kernel.org>
On Fri, Jun 05, 2026 at 03:24:58PM -0500, Rob Herring wrote:
> On Fri, Jun 05, 2026 at 11:37:37AM +0300, Vladimir Oltean wrote:
> > On Wed, Jun 03, 2026 at 04:50:39PM +0100, Conor Dooley wrote:
> > > On Wed, Jun 03, 2026 at 04:38:25PM +0300, Vladimir Oltean wrote:
> > > > On Wed, Jun 03, 2026 at 01:33:50PM +0000, sashiko-bot@kernel.org wrote:
> > > > > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> > > > > - [Medium] The `if` condition for enforcing the `big-endian` property incorrectly matches the `compatible` string array directly against an `enum`, which will always evaluate to false and cause the requirement to silently fail.
> > > > > --
> > > > >
> > > > > commit 8bd3982a89722b20a3686e42d4ab3b9f0711ae11
> > > > > Author: Vladimir Oltean <vladimir.oltean@nxp.com>
> > > > >
> > > > > dt-bindings: phy: lynx-10g: initial document
> > > > >
> > > > > This commit adds the schema for the 10G Lynx SerDes used on most QorIQ and
> > > > > Layerscape SoCs. It defines the valid compatible strings, PHY cell requirements,
> > > > > and enforces SoC-specific constraints such as lane counts and endianness.
> > > > >
> > > > > > diff --git a/Documentation/devicetree/bindings/phy/fsl,lynx-10g.yaml b/Documentation/devicetree/bindings/phy/fsl,lynx-10g.yaml
> > > > > > new file mode 100644
> > > > > > index 0000000000000..8cf6027a75b91
> > > > > > --- /dev/null
> > > > > > +++ b/Documentation/devicetree/bindings/phy/fsl,lynx-10g.yaml
> > > > > > + - if:
> > > > > > + properties:
> > > > > > + compatible:
> > > > > > + enum:
> > > > > > + - fsl,ls1046a-serdes1
> > > > > > + - fsl,ls1046a-serdes2
> > > > >
> > > > > [Severity: Medium]
> > > > > Does this conditional correctly enforce the big-endian property?
> > > > >
> > > > > Since the compatible property is a string array in device trees, evaluating it
> > > > > directly against an enum always returns false. Should this use "contains:"
> > > > > with the enum, similar to the preceding if block, so dt_binding_check
> > > > > correctly enforces the requirement?
> > > >
> > > > The tests I did suggest that it does enforce the requirement.
> > >
> > > Yeah, sashiko just doesn't understand how this works without "contains".
> > > "contains" is only actually required for validation purposes if there's
> > > fallback compatibles and you're trying to get a partial match.
>
> Not exactly. The missing part is the schema is fixed up so the above is
> converted to:
>
> compatible:
> minItems: 1
> maxItems: 1
> items:
> - enum:
> - fsl,ls1046a-serdes1
> - fsl,ls1046a-serdes2
>
> If we didn't do that, the above is not what we want here, but 'contains'
> is as that's the loosest condition and works regardless of how many
> entries compatible has. That's what you should do here.
>
> This fixup used to be a lot more widespread when we used the DT YAML
> format and everything was decoded to an an array or matrix. The fixups
> saved a lot of boilerplate. That's less true now, so maybe getting rid
> of more fixups like this case would be better.
Thanks for the explanation, it makes sense now.
I ran "dt-mk-schema -j Documentation/devicetree/bindings/phy/fsl,lynx-10g.yaml"
and I can see the following, which seems to check out:
"http://devicetree.org/schemas/phy/fsl,lynx-10g.yaml": {
"$filename": "/opt/linux/net-phy-upstreaming/Documentation/devicetree/bindings/phy/fsl,lynx-10g.yaml",
"$id": "http://devicetree.org/schemas/phy/fsl,lynx-10g.yaml#",
"$schema": "http://devicetree.org/meta-schemas/core.yaml#",
"additionalProperties": false,
"allOf": [
{
{
"if": {
"properties": {
"compatible": {
"items": [
{
"enum": [
"fsl,ls1046a-serdes1",
"fsl,ls1046a-serdes2"
]
}
],
"maxItems": 1,
"minItems": 1,
"type": "array"
}
}
},
"then": {
"required": [
"big-endian"
]
}
}
}
]
}
I can send a v4 replacing the constraint with:
allOf:
- if:
properties:
compatible:
contains:
enum:
- fsl,ls1028a-serdes
- fsl,ls1046a-serdes1
- fsl,ls1046a-serdes2
- fsl,ls1088a-serdes1
- fsl,ls1088a-serdes2
then:
patternProperties:
"^phy@[0-7]$":
properties:
reg:
minimum: 0
maximum: 3
- if:
properties:
compatible:
contains:
enum:
- fsl,ls1046a-serdes1
- fsl,ls1046a-serdes2
then:
properties:
big-endian: true
required:
- big-endian
since you've said that we shouldn't rely on this fixup anymore. I see
you gave me a Reviewed-by and I deliberately am not adding a fallback
compatible, but I can do so anyway.
> > Yeah, I have no clear understanding of the json-schema syntax either, I
> > just copied from another place where it was clear that the intention was
> > to have multiple matches on nodes having a single compatible string each.
> >
> > But maybe it would be good from DT bindings maintainers to teach LLMs
> > where they get things wrong in this repo?
> > https://github.com/masoncl/review-prompts/blob/main/kernel/subsystem/dt-bindings.md
>
> Yes, there's a few things it consistently gets wrong. I've looked at
> this and I can go write more instructions (and fix some things that seem
> wrong), but how do I know if it really works? First, I'd be testing with
> a different LLM as that's what I have access to. Second, how do I know
> if no warning is just the indeterminate nature of LLMs?
If you have a lot of time on your hands, the question "was this concern
considered and dismissed, or not considered?" should be able to be
answered.
In the sashiko web UI for the patch set, there is a "View Raw Log" link
for each patch:
https://sashiko.dev/#/patchset/20260603131914.503053-1-vladimir.oltean%40nxp.com
This gives you its reasoning process and its internal findings (which it
tries to dismiss again in a later stage, before reporting them):
https://sashiko.dev/#/log/67856
```json
{
"findings": [
{
"problem": "The `if` condition for enforcing the `big-endian` property incorrectly matches the `compatible` string array directly against an `enum`, which will always evaluate to false and cause the requirement to silently fail.",
"severity": "Medium",
"severity_explanation": "In Device Tree schema validation, the `compatible` property is processed as a JSON string array. Inside an `if` block, standard JSON schema rules apply. Using `properties: compatible: enum:` evaluates whether the array itself exactly equals one of the scalar strings in the enum, which is structurally impossible. As a result, the condition will always evaluate to false, and the `big-endian` requirement for `fsl,ls1046a-serdes1` and `fsl,ls1046a-serdes2` will never be enforced by `dt_binding_check`. The check must be updated to use `properties: compatible: contains: enum:` to correctly evaluate the elements within the array, similar to the preceding `if` block.",
"preexisting": false,
"locations": [
{
"file": "Documentation/devicetree/bindings/phy/fsl,lynx-10g.yaml",
"function_or_symbol": null,
"line": 91,
"code_snippet": " - if:\n properties:\n compatible:\n enum:\n - fsl,ls1046a-serdes1\n - fsl,ls1046a-serdes2\n then:\n required:\n - big-endian",
"why_this_location_matters": "This conditional block incorrectly omits `contains:`, rendering the entire schema validation rule for the `big-endian` property ineffective."
}
]
}
]
}
```
> What's really needed is for sashiko to incorporate feedback like any
> other developer. Otherwise, it's going to be like some certain
> reviewers we've banned.
I don't think that process is in place right now - the learning process
from mailing list discussions is human driven and not autonomous.
Nor would it be immediately obvious how to automate it, if you consider
the risk of pollution from certain reviewers, which you've also pointed
out. But as a reviewer and maintainer, I think you are in a good
position to write your experience down in the dt-schema skill and see
how it goes from there.
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v9 05/10] nvmem: qcom-spmi-sdam: Migrate to devm_spmi_subdevice_alloc_and_add()
From: AngeloGioacchino Del Regno @ 2026-06-08 11:32 UTC (permalink / raw)
To: sashiko-reviews; +Cc: linux-phy, vkoul, neil.armstrong, olteanv
In-Reply-To: <20260608105406.C9E051F00893@smtp.kernel.org>
On 6/8/26 12:54, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] The driver blindly casts `dev->parent` to a `struct spmi_device *` without validating the parent device type, leading to a wild pointer dereference on malformed device trees.
> --
>
> commit b6b479ab20e3f2d7a7247657ca4bb0a5a70fc75a
> Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>
> nvmem: qcom-spmi-sdam: Migrate to devm_spmi_subdevice_alloc_and_add()
>
> This commit migrates the SDAM driver to use devm_spmi_subdevice_alloc_and_add().
> It registers a new SPMI sub-device for SDAM and initializes a regmap with its
> specific base address retrieved from the devicetree.
>
>> diff --git a/drivers/nvmem/qcom-spmi-sdam.c b/drivers/nvmem/qcom-spmi-sdam.c
>> index 4f1cca6eab71e..4974105dd9632 100644
>> --- a/drivers/nvmem/qcom-spmi-sdam.c
>> +++ b/drivers/nvmem/qcom-spmi-sdam.c
> [ ... ]
>> @@ -110,8 +110,17 @@ static int sdam_write(void *priv, unsigned int offset, void *val,
>>
>> static int sdam_probe(struct platform_device *pdev)
>> {
> [ ... ]
>> struct sdam_chip *sdam;
>> struct nvmem_device *nvmem;
>> + struct spmi_device *sparent;
>> + struct spmi_subdevice *sub_sdev;
>> + struct device *dev = &pdev->dev;
>> unsigned int val;
>> int rc;
>>
>> @@ -119,19 +128,23 @@ static int sdam_probe(struct platform_device *pdev)
>> if (!sdam)
>> return -ENOMEM;
>>
>> - sdam->regmap = dev_get_regmap(pdev->dev.parent, NULL);
>> - if (!sdam->regmap) {
>> - dev_err(&pdev->dev, "Failed to get regmap handle\n");
>> - return -ENXIO;
>> - }
>> + sparent = to_spmi_device(dev->parent);
>> + sub_sdev = devm_spmi_subdevice_alloc_and_add(dev, sparent);
>
> [Severity: High]
> Does this code assume that the parent device is always a valid SPMI device?
>
Yes - and that's ok, because this device cannot exist outside of SPMI and will
never be registered on its own without a SPMI parent.
Overcautious coding bloats drivers for no reason.
> If the device tree is malformed and this node is placed under a simple-bus
> instead of an SPMI PMIC, the parent device will not be an spmi_device.
> Because to_spmi_device() uses container_of(), this will result in a bogus
> pointer.
>
> When that bogus pointer is passed into devm_spmi_subdevice_alloc_and_add(),
> does it cause a wild pointer dereference when accessing sparent->ctrl or
> sparent->usid?
>
> Prior to this patch, dev_get_regmap(pdev->dev.parent, NULL) would gracefully
> return NULL for malformed hierarchies, but this change appears to introduce
> a potential kernel panic.
>
>> + if (IS_ERR(sub_sdev))
>> + return PTR_ERR(sub_sdev);
>
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v9 08/10] misc: qcom-coincell: Migrate to devm_spmi_subdevice_alloc_and_add()
From: AngeloGioacchino Del Regno @ 2026-06-08 11:32 UTC (permalink / raw)
To: sashiko-reviews; +Cc: neil.armstrong, vkoul, olteanv, linux-phy
In-Reply-To: <20260608110945.608BD1F00893@smtp.kernel.org>
On 6/8/26 13:09, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Unsafe downcast of `dev->parent` to `struct spmi_device` causing out-of-bounds read and kernel panic
> --
>
> commit eb0fe105cd38bff6269804925b90232e1c422522
> Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>
> misc: qcom-coincell: Migrate to devm_spmi_subdevice_alloc_and_add()
>
> This commit updates the qcom-coincell driver to use a dedicated SPMI
> sub-device instead of fetching the regmap from its parent. It creates the
> sub-device using the parent device and initializes a specific regmap to
> handle the base address offsets automatically.
>
>> diff --git a/drivers/misc/qcom-coincell.c b/drivers/misc/qcom-coincell.c
>> index 3c57f74291476..b0904f441300c 100644
>> --- a/drivers/misc/qcom-coincell.c
>> +++ b/drivers/misc/qcom-coincell.c
>> @@ -71,37 +71,55 @@ static int qcom_coincell_probe(struct platform_device *pdev)
> [ ... ]
>> - rc = of_property_read_u32(node, "reg", &chgr.base_addr);
>> + rc = device_property_read_u32(dev, "reg", &qcom_coincell_regmap_config.reg_base);
>> if (rc)
>> return rc;
>>
>> + sparent = to_spmi_device(dev->parent);
>> + sub_sdev = devm_spmi_subdevice_alloc_and_add(dev, sparent);
>
> [Severity: High]
> Could this lead to a panic if the parent device is not actually an SPMI
> device?
>
> If this driver is bound to a device whose parent is a regular platform
> device (for example, via a malformed device tree or a manual sysfs bind),
> dev->parent will not point to a valid spmi_device.
>
This device cannot exist outside of SPMI and will never be registered on its
own without a SPMI parent.
Overcautious coding bloats drivers for no reason, so this is fine.
> Because to_spmi_device() is a container_of cast, it will blindly cast the
> parent device. When passed to devm_spmi_subdevice_alloc_and_add(), the
> underlying spmi_subdevice_alloc_and_add() function reads sparent->ctrl:
>
> drivers/spmi/spmi.c:spmi_subdevice_alloc_and_add() {
> ...
> sdev->ctrl = sparent->ctrl;
> ...
> }
>
> Since the parent is actually a platform_device, this ends up reading the
> memory offset corresponding to platform_dma_mask instead of a valid ctrl
> pointer. Later regmap initialization will dereference this invalid pointer,
> causing an out-of-bounds read.
>
> The previous dev_get_regmap() approach avoided this by safely searching the
> parent's devres list without relying on a structural cast. Is there a way to
> verify the parent device type before casting?
>
>> + if (IS_ERR(sub_sdev))
>> + return PTR_ERR(sub_sdev);
>
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v9 02/10] spmi: Print error status with %pe format
From: AngeloGioacchino Del Regno @ 2026-06-08 11:29 UTC (permalink / raw)
To: sashiko-reviews; +Cc: neil.armstrong, linux-phy, vkoul, olteanv
In-Reply-To: <20260608103702.411271F00893@smtp.kernel.org>
On 6/8/26 12:37, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] The OF device node assigned to the newly allocated SPMI device does not have its reference count incremented, creating a potential use-after-free condition when the iterator macro automatically drops the reference.
> --
>
> commit eb2ded032315a7ba1f8c878f6e0a10db80cf0346
> Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>
> spmi: Print error status with %pe format
>
> This commit updates error messages in the SPMI driver to use the %pe format
> specifier for printing error statuses instead of printing integer error codes.
>
>> diff --git a/drivers/spmi/spmi.c b/drivers/spmi/spmi.c
>> index e93fc2c957590..95db19a0a44ec 100644
>> --- a/drivers/spmi/spmi.c
>> +++ b/drivers/spmi/spmi.c
> [ ... ]
>> @@ -522,7 +522,7 @@ static void of_spmi_register_devices(struct spmi_controller *ctrl)
>> err = spmi_device_add(sdev);
>> if (err) {
>> dev_err(&sdev->dev,
>> - "failure adding device. status %d\n", err);
>> + "failure adding device. status %pe\n", ERR_PTR(err));
>
> [Severity: High]
> This is a pre-existing issue, but I noticed a potential use-after-free
> condition with the OF node reference counting.
>
> In drivers/spmi/spmi.c:of_spmi_register_devices(), the
> for_each_available_child_of_node() loop iterates through child nodes,
> automatically calling of_node_put() on the previous node when advancing.
>
> Inside the loop, the node is assigned to the SPMI device:
>
> device_set_node(&sdev->dev, of_fwnode_handle(node));
>
> Does this assignment need to invoke of_node_get() to increment the node's
> reference count? Without it, when the loop advances, the node's reference
> is dropped, leaving sdev->dev.of_node as a dangling pointer.
>
> If the device tree node is dynamically managed (e.g., via DT overlays),
> unloading the overlay will free the node, causing a use-after-free whenever
> the active SPMI device later accesses its of_node.
>
> Additionally, drivers/spmi/spmi.c:spmi_dev_release() lacks an of_node_put()
> to balance the required reference:
>
> static void spmi_dev_release(struct device *dev)
> {
> struct spmi_device *sdev = to_spmi_device(dev);
>
> kfree(sdev);
> }
>
> Should spmi_dev_release() be updated to safely release the reference once
> it is correctly obtained?
>
>> spmi_device_put(sdev);
>> }
>> }
>
Although very unlikely to happen at this moment in time, this is a valid concern.
It's out of scope for this series - so I will send a fix separately.
This should not block this series.
Cheers,
Angelo
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v4 5/5] phy: fsl-imx8mq-usb: keep PHY power domain runtime always-on for i.MX8MP
From: Xu Yang @ 2026-06-08 11:27 UTC (permalink / raw)
To: Frank Li
Cc: Vinod Koul, Neil Armstrong, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, Jun Li, linux-phy, imx, linux-arm-kernel,
linux-kernel, Xu Yang
In-Reply-To: <aiLrhzEdQE6zAauv@lizhi-Precision-Tower-5810>
On Fri, Jun 05, 2026 at 11:30:15AM -0400, Frank Li wrote:
> On Fri, Jun 05, 2026 at 07:13:06PM +0800, Xu Yang wrote:
> > From: Xu Yang <xu.yang_2@nxp.com>
> >
> > On i.MX8MP, the USB PHY has a dedicated power domain that was previously
> > never powered off at runtime. With the introduction of runtime PM support,
> > the power domain will be powered off if the device is runtime suspended,
> > which breaks USB wakeup functionality.
>
> Does it need depend on if device enable wakeup?
No. The wakeup is for system suspend. We handle the runtime case here.
Thanks,
Xu Yang
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v4 4/5] phy: fsl-imx8mq-usb: add control register regmap
From: Xu Yang @ 2026-06-08 11:26 UTC (permalink / raw)
To: Frank Li
Cc: Vinod Koul, Neil Armstrong, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, Jun Li, linux-phy, imx, linux-arm-kernel,
linux-kernel, Xu Yang
In-Reply-To: <aiLrMs7AMECq7U66@lizhi-Precision-Tower-5810>
On Fri, Jun 05, 2026 at 11:28:50AM -0400, Frank Li wrote:
> On Fri, Jun 05, 2026 at 07:13:05PM +0800, Xu Yang wrote:
> > From: Xu Yang <xu.yang_2@nxp.com>
> >
> > The CR port is a simple 16-bit data/address parallel port that is
> > accessed through 32-bit MMIO registers for on-chip access to the
> > control registers inside the USB 3.0 femtoPHY. Add control register
> > regmap and export these registers by debugfs to help PHY's diagnostic.
> >
> > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> >
>
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
>
> > ---
> > Changes in v4:
> > - improve commit message as Haibo's suggestion
> > Changes in v3:
> > - drop Frank's tag because it includes other changes
> > - new patch
> > ---
> > drivers/phy/freescale/phy-fsl-imx8mq-usb.c | 27 ++++++++++++++++++++++++++-
> > 1 file changed, 26 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> > index 27aa696f5dd4..e24f46d7924b 100644
> > --- a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> > +++ b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> > @@ -1,5 +1,5 @@
> > // SPDX-License-Identifier: GPL-2.0+
> > -/* Copyright (c) 2017 NXP. */
> > +/* Copyright 2017-2026 NXP. */
> >
> > #include <linux/bitfield.h>
> > #include <linux/clk.h>
> > @@ -11,6 +11,7 @@
> > #include <linux/platform_device.h>
> > #include <linux/pm_runtime.h>
> > #include <linux/regulator/consumer.h>
> > +#include <linux/regmap.h>
> > #include <linux/usb/typec_mux.h>
> >
> > #define PHY_CTRL0 0x0
> > @@ -56,6 +57,8 @@
> > #define PHY_CTRL6_ALT_CLK_EN BIT(1)
> > #define PHY_CTRL6_ALT_CLK_SEL BIT(0)
> >
> > +#define PHY_CRCTL 0x30
> > +
> > #define PHY_TUNE_DEFAULT 0xffffffff
> >
> > #define TCA_CLK_RST 0x00
> > @@ -119,6 +122,7 @@ struct imx8mq_usb_phy {
> > void __iomem *base;
> > struct regulator *vbus;
> > struct tca_blk *tca;
> > + struct regmap *cr_regmap;
> > u32 pcs_tx_swing_full;
> > u32 pcs_tx_deemph_3p5db;
> > u32 tx_vref_tune;
> > @@ -667,6 +671,14 @@ static const struct of_device_id imx8mq_usb_phy_of_match[] = {
> > };
> > MODULE_DEVICE_TABLE(of, imx8mq_usb_phy_of_match);
> >
> > +static const struct regmap_config imx_cr_regmap_config = {
> > + .name = "cr",
> > + .reg_bits = 32,
> > + .val_bits = 32,
> > + .reg_stride = 4,
> > + .max_register = 0x7,
> > +};
> > +
> > static int imx8mq_usb_phy_probe(struct platform_device *pdev)
> > {
> > struct phy_provider *phy_provider;
> > @@ -696,6 +708,13 @@ static int imx8mq_usb_phy_probe(struct platform_device *pdev)
> > if (IS_ERR(imx_phy->base))
> > return PTR_ERR(imx_phy->base);
> >
> > + imx_phy->cr_regmap = devm_regmap_init_mmio(dev, imx_phy->base + PHY_CRCTL,
> > + &imx_cr_regmap_config);
> > + if (IS_ERR(imx_phy->cr_regmap)) {
> > + dev_warn(dev, "Fail to init debug register regmap\n");
> > + imx_phy->cr_regmap = NULL;
> > + }
> > +
> > ret = devm_pm_runtime_set_active_enabled(dev);
> > if (ret)
> > return dev_err_probe(dev, ret, "Failed to enable runtime PM\n");
> > @@ -731,6 +750,9 @@ static int imx8mq_usb_phy_runtime_suspend(struct device *dev)
> > {
> > struct imx8mq_usb_phy *imx_phy = dev_get_drvdata(dev);
> >
> > + if (imx_phy->cr_regmap)
> > + regcache_cache_only(imx_phy->cr_regmap, true);
> > +
>
> I think this common problem, is possible to change
> regmap_read_debugfs(), let it call runtime_pm_get(), there are already
> have runtime_pm in regmap field.
Yes, it should be a common issue when access debugfs on a suspended device.
Not sure why regmap common driver doesn't add it. We can do it in future if
more such cases appear.
>
> So you debug fs always to get update value, instead cached value?
The default cache type is REGCACHE_NONE in imx_cr_regmap_config. So no
cache at all. The main purpose is to block register access here. Once
cache_only is disabled, it will read HW register value.
Thanks,
Xu Yang
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v9 04/10] spmi: Implement spmi_subdevice_alloc_and_add() and devm variant
From: AngeloGioacchino Del Regno @ 2026-06-08 11:27 UTC (permalink / raw)
To: Stephen Boyd, Jonathan Cameron
Cc: linux-phy, vkoul, olteanv, neil.armstrong, sashiko-reviews
In-Reply-To: <20260608104323.3B15D1F00893@smtp.kernel.org>
On 6/8/26 12:43, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] dev_set_name() uses the '%d' format specifier for a variable of type 'unsigned int', which will cause a -Wformat compiler warning.
> --
>
> commit 35ac69140b9b2d8a3b0e2aa01fe5f5ac3ffe4d02
> Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>
> spmi: Implement spmi_subdevice_alloc_and_add() and devm variant
>
> This commit introduces functions for managing sub-devices of an SPMI
> device, including spmi_subdevice_alloc_and_add() and its devm
> variant. These allow complex PMICs with multiple sub-devices to be
> properly modeled and managed via the SPMI bus.
>
>> diff --git a/drivers/spmi/spmi.c b/drivers/spmi/spmi.c
>> index 91a40ea8031ad..7945cacf6d655 100644
>> --- a/drivers/spmi/spmi.c
>> +++ b/drivers/spmi/spmi.c
>
> [ ... ]
>
>> @@ -428,6 +454,58 @@ struct spmi_device *spmi_device_alloc(struct spmi_controller *ctrl)
>> }
>> EXPORT_SYMBOL_GPL(spmi_device_alloc);
>>
>> +/**
>> + * spmi_subdevice_alloc_and_add(): Allocate and add a new SPMI sub-device
>> + * @sparent: SPMI parent device with previously registered SPMI controller
>> + *
>> + * Returns:
>> + * Pointer to newly allocated SPMI sub-device for success or error pointer.
>> + */
>> +struct spmi_subdevice *spmi_subdevice_alloc_and_add(struct spmi_device *sparent)
>> +{
>> + struct spmi_subdevice *sub_sdev;
>> + struct spmi_device *sdev;
>> + int ret;
>> +
>> + sub_sdev = kzalloc(sizeof(*sub_sdev), GFP_KERNEL);
>> + if (!sub_sdev)
>> + return ERR_PTR(-ENOMEM);
>> +
>> + sdev = &sub_sdev->sdev;
>> + sdev->ctrl = sparent->ctrl;
>> + sdev->usid = sparent->usid;
>> +
>> + ret = ida_alloc(&spmi_subdevice_ida, GFP_KERNEL);
>> + if (ret < 0) {
>> + kfree(sub_sdev);
>> + return ERR_PTR(ret);
>> + }
>> + sub_sdev->devid = ret;
>> +
>> + device_initialize(&sdev->dev);
>> + sdev->dev.parent = &sparent->dev;
>> + sdev->dev.bus = &spmi_bus_type;
>> + sdev->dev.type = &spmi_subdev_type;
>> +
>> + ret = dev_set_name(&sdev->dev, "%u-%02x.%d.auto",
>> + sdev->ctrl->nr, sdev->usid, sub_sdev->devid);
>
> [Severity: Low]
> This isn't a bug, but should this use %u instead of %d for the format
> specifier? The devid field in struct spmi_subdevice is defined as an
> unsigned int, which can cause a -Wformat compiler warning here.
>
I wonder if this can be fixed while applying please?
Thanks,
Angelo
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v4 3/5] phy: fsl-imx8mq-usb: add runtime PM support
From: Xu Yang @ 2026-06-08 11:14 UTC (permalink / raw)
To: Frank Li
Cc: Vinod Koul, Neil Armstrong, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, Jun Li, linux-phy, imx, linux-arm-kernel,
linux-kernel, Xu Yang
In-Reply-To: <aiLn81lg32BPDZVp@lizhi-Precision-Tower-5810>
On Fri, Jun 05, 2026 at 11:14:59AM -0400, Frank Li wrote:
> On Fri, Jun 05, 2026 at 07:13:04PM +0800, Xu Yang wrote:
> > From: Xu Yang <xu.yang_2@nxp.com>
> >
> > Add runtime PM to ensure the PHY is properly powered and clocked during
> > register access, preventing potential system hangs.
> >
> > It guards register access in the following scenarios:
> > - PHY operations: init() and power_on/off() callbacks are guarded by
> > phy core
> > - Type-C orientation switching when PHY/Controller are suspended which
> > needs explicitly care
> > - Future PHY control port register regmap debugfs access
> >
> > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> >
> > ---
> > Changes in v4:
> > - replace guard() with PM_RUNTIME_ACQUIRE()
> > Changes in v3:
> > - new patch
> > ---
> > drivers/phy/freescale/phy-fsl-imx8mq-usb.c | 62 +++++++++++++++++++++---------
> > 1 file changed, 43 insertions(+), 19 deletions(-)
> >
> > diff --git a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> > index 591ddf346061..27aa696f5dd4 100644
> > --- a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> > +++ b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> > @@ -9,6 +9,7 @@
> > #include <linux/of.h>
> > #include <linux/phy/phy.h>
> > #include <linux/platform_device.h>
> > +#include <linux/pm_runtime.h>
> > #include <linux/regulator/consumer.h>
> > #include <linux/usb/typec_mux.h>
> >
> > @@ -136,17 +137,15 @@ static int tca_blk_typec_switch_set(struct typec_switch_dev *sw,
> > {
> > struct imx8mq_usb_phy *imx_phy = typec_switch_get_drvdata(sw);
> > struct tca_blk *tca = imx_phy->tca;
> > - int ret;
> >
> > if (tca->orientation == orientation)
> > return 0;
> >
> > - ret = clk_prepare_enable(imx_phy->clk);
> > - if (ret)
> > - return ret;
> > + PM_RUNTIME_ACQUIRE(&imx_phy->phy->dev, pm);
> > + if (PM_RUNTIME_ACQUIRE_ERR(&pm))
> > + return -ENXIO;
> >
> > tca_blk_orientation_set(tca, orientation);
> > - clk_disable_unprepare(imx_phy->clk);
> >
> > return 0;
> > }
> > @@ -620,16 +619,6 @@ static int imx8mq_phy_power_on(struct phy *phy)
> > if (ret)
> > return ret;
> >
> > - ret = clk_prepare_enable(imx_phy->clk);
> > - if (ret)
> > - return ret;
> > -
> > - ret = clk_prepare_enable(imx_phy->alt_clk);
> > - if (ret) {
> > - clk_disable_unprepare(imx_phy->clk);
> > - return ret;
> > - }
> > -
> > /* Disable rx term override */
> > value = readl(imx_phy->base + PHY_CTRL6);
> > value &= ~PHY_CTRL6_RXTERM_OVERRIDE_SEL;
> > @@ -648,8 +637,6 @@ static int imx8mq_phy_power_off(struct phy *phy)
> > value |= PHY_CTRL6_RXTERM_OVERRIDE_SEL;
> > writel(value, imx_phy->base + PHY_CTRL6);
> >
> > - clk_disable_unprepare(imx_phy->alt_clk);
> > - clk_disable_unprepare(imx_phy->clk);
> > regulator_disable(imx_phy->vbus);
> >
> > return 0;
> > @@ -686,6 +673,7 @@ static int imx8mq_usb_phy_probe(struct platform_device *pdev)
> > struct device *dev = &pdev->dev;
> > struct imx8mq_usb_phy *imx_phy;
> > const struct phy_ops *phy_ops;
> > + int ret;
> >
> > imx_phy = devm_kzalloc(dev, sizeof(*imx_phy), GFP_KERNEL);
> > if (!imx_phy)
> > @@ -693,13 +681,13 @@ static int imx8mq_usb_phy_probe(struct platform_device *pdev)
> >
> > platform_set_drvdata(pdev, imx_phy);
> >
> > - imx_phy->clk = devm_clk_get(dev, "phy");
> > + imx_phy->clk = devm_clk_get_enabled(dev, "phy");
> > if (IS_ERR(imx_phy->clk)) {
> > dev_err(dev, "failed to get imx8mq usb phy clock\n");
> > return PTR_ERR(imx_phy->clk);
> > }
> >
> > - imx_phy->alt_clk = devm_clk_get_optional(dev, "alt");
> > + imx_phy->alt_clk = devm_clk_get_optional_enabled(dev, "alt");
>
> when driver remove, devm will disable clock, which may cause refcound
> wrong if device already suspend by runtime pm
Right, so it's improper to use both devm for clk and runtime PM. I will
remove devm_pm_runtime_set_active_enabled() and use non-devm callbacks
to avoid such issue.
>
> > if (IS_ERR(imx_phy->alt_clk))
> > return dev_err_probe(dev, PTR_ERR(imx_phy->alt_clk),
> > "Failed to get alt clk\n");
> > @@ -708,6 +696,10 @@ static int imx8mq_usb_phy_probe(struct platform_device *pdev)
> > if (IS_ERR(imx_phy->base))
> > return PTR_ERR(imx_phy->base);
> >
> > + ret = devm_pm_runtime_set_active_enabled(dev);
> > + if (ret)
> > + return dev_err_probe(dev, ret, "Failed to enable runtime PM\n");
> > +
>
> You have not set auto suspend, so runtime_suspend will never entry.
At the end of __driver_probe_device(), it will call pm_request_idle() for
the device. So runtime_suspend() will run. And the test result align with
this logic.
Thanks,
Xu Yang
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v9 08/10] misc: qcom-coincell: Migrate to devm_spmi_subdevice_alloc_and_add()
From: sashiko-bot @ 2026-06-08 11:09 UTC (permalink / raw)
To: AngeloGioacchino Del Regno; +Cc: neil.armstrong, vkoul, olteanv, linux-phy
In-Reply-To: <20260608100949.36309-9-angelogioacchino.delregno@collabora.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unsafe downcast of `dev->parent` to `struct spmi_device` causing out-of-bounds read and kernel panic
--
commit eb0fe105cd38bff6269804925b90232e1c422522
Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
misc: qcom-coincell: Migrate to devm_spmi_subdevice_alloc_and_add()
This commit updates the qcom-coincell driver to use a dedicated SPMI
sub-device instead of fetching the regmap from its parent. It creates the
sub-device using the parent device and initializes a specific regmap to
handle the base address offsets automatically.
> diff --git a/drivers/misc/qcom-coincell.c b/drivers/misc/qcom-coincell.c
> index 3c57f74291476..b0904f441300c 100644
> --- a/drivers/misc/qcom-coincell.c
> +++ b/drivers/misc/qcom-coincell.c
> @@ -71,37 +71,55 @@ static int qcom_coincell_probe(struct platform_device *pdev)
[ ... ]
> - rc = of_property_read_u32(node, "reg", &chgr.base_addr);
> + rc = device_property_read_u32(dev, "reg", &qcom_coincell_regmap_config.reg_base);
> if (rc)
> return rc;
>
> + sparent = to_spmi_device(dev->parent);
> + sub_sdev = devm_spmi_subdevice_alloc_and_add(dev, sparent);
[Severity: High]
Could this lead to a panic if the parent device is not actually an SPMI
device?
If this driver is bound to a device whose parent is a regular platform
device (for example, via a malformed device tree or a manual sysfs bind),
dev->parent will not point to a valid spmi_device.
Because to_spmi_device() is a container_of cast, it will blindly cast the
parent device. When passed to devm_spmi_subdevice_alloc_and_add(), the
underlying spmi_subdevice_alloc_and_add() function reads sparent->ctrl:
drivers/spmi/spmi.c:spmi_subdevice_alloc_and_add() {
...
sdev->ctrl = sparent->ctrl;
...
}
Since the parent is actually a platform_device, this ends up reading the
memory offset corresponding to platform_dma_mask instead of a valid ctrl
pointer. Later regmap initialization will dereference this invalid pointer,
causing an out-of-bounds read.
The previous dev_get_regmap() approach avoided this by safely searching the
parent's devres list without relying on a structural cast. Is there a way to
verify the parent device type before casting?
> + if (IS_ERR(sub_sdev))
> + return PTR_ERR(sub_sdev);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260608100949.36309-1-angelogioacchino.delregno@collabora.com?part=8
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v9 05/10] nvmem: qcom-spmi-sdam: Migrate to devm_spmi_subdevice_alloc_and_add()
From: sashiko-bot @ 2026-06-08 10:54 UTC (permalink / raw)
To: AngeloGioacchino Del Regno; +Cc: linux-phy, vkoul, neil.armstrong, olteanv
In-Reply-To: <20260608100949.36309-6-angelogioacchino.delregno@collabora.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The driver blindly casts `dev->parent` to a `struct spmi_device *` without validating the parent device type, leading to a wild pointer dereference on malformed device trees.
--
commit b6b479ab20e3f2d7a7247657ca4bb0a5a70fc75a
Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
nvmem: qcom-spmi-sdam: Migrate to devm_spmi_subdevice_alloc_and_add()
This commit migrates the SDAM driver to use devm_spmi_subdevice_alloc_and_add().
It registers a new SPMI sub-device for SDAM and initializes a regmap with its
specific base address retrieved from the devicetree.
> diff --git a/drivers/nvmem/qcom-spmi-sdam.c b/drivers/nvmem/qcom-spmi-sdam.c
> index 4f1cca6eab71e..4974105dd9632 100644
> --- a/drivers/nvmem/qcom-spmi-sdam.c
> +++ b/drivers/nvmem/qcom-spmi-sdam.c
[ ... ]
> @@ -110,8 +110,17 @@ static int sdam_write(void *priv, unsigned int offset, void *val,
>
> static int sdam_probe(struct platform_device *pdev)
> {
[ ... ]
> struct sdam_chip *sdam;
> struct nvmem_device *nvmem;
> + struct spmi_device *sparent;
> + struct spmi_subdevice *sub_sdev;
> + struct device *dev = &pdev->dev;
> unsigned int val;
> int rc;
>
> @@ -119,19 +128,23 @@ static int sdam_probe(struct platform_device *pdev)
> if (!sdam)
> return -ENOMEM;
>
> - sdam->regmap = dev_get_regmap(pdev->dev.parent, NULL);
> - if (!sdam->regmap) {
> - dev_err(&pdev->dev, "Failed to get regmap handle\n");
> - return -ENXIO;
> - }
> + sparent = to_spmi_device(dev->parent);
> + sub_sdev = devm_spmi_subdevice_alloc_and_add(dev, sparent);
[Severity: High]
Does this code assume that the parent device is always a valid SPMI device?
If the device tree is malformed and this node is placed under a simple-bus
instead of an SPMI PMIC, the parent device will not be an spmi_device.
Because to_spmi_device() uses container_of(), this will result in a bogus
pointer.
When that bogus pointer is passed into devm_spmi_subdevice_alloc_and_add(),
does it cause a wild pointer dereference when accessing sparent->ctrl or
sparent->usid?
Prior to this patch, dev_get_regmap(pdev->dev.parent, NULL) would gracefully
return NULL for malformed hierarchies, but this change appears to introduce
a potential kernel panic.
> + if (IS_ERR(sub_sdev))
> + return PTR_ERR(sub_sdev);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260608100949.36309-1-angelogioacchino.delregno@collabora.com?part=5
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: phy: nuvoton: Add MA35D1 USB2 OTG PHY binding
From: Krzysztof Kozlowski @ 2026-06-08 10:45 UTC (permalink / raw)
To: Joey Lu
Cc: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Jacky Huang, Shan-Chun Hung, linux-phy, devicetree,
linux-arm-kernel, linux-kernel
In-Reply-To: <20260604101220.1092822-2-a0987203069@gmail.com>
On Thu, Jun 04, 2026 at 06:12:19PM +0800, Joey Lu wrote:
A nit, subject: drop second/last, redundant "binding". The
"dt-bindings" prefix is already stating that these are bindings.
See also:
https://elixir.bootlin.com/linux/v6.17-rc3/source/Documentation/devicetree/bindings/submitting-patches.rst#L18
> +properties:
> + compatible:
> + const: nuvoton,ma35d1-usb2-phy-otg
> +
Where is reg? MMIO is expressed with reg, not via custom phandle.
> + clocks:
> + maxItems: 1
> +
> + nuvoton,sys:
> + $ref: /schemas/types.yaml#/definitions/phandle-array
> + items:
> + - items:
> + - description: phandle to the system management syscon.
> + - description: PHY instance index.
> + enum:
> + - 0 # USB0, OTG port (shared with DWC2 gadget controller)
> + - 1 # USB1, host-only port
> + description:
> + A phandle to the syscon node covering the SYS register block, with
> + one argument selecting the PHY instance. Index 0 selects the OTG
> + port PHY (USB0) and index 1 selects the host-only PHY (USB1).
> +
> + "#phy-cells":
> + const: 0
> +
> + nuvoton,rcalcode:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + minimum: 0
> + maximum: 15
> + description:
> + Resistor calibration trim code written to the RCALCODE field in
> + USBPMISCR. The 4-bit value adjusts the PHY's internal termination
> + resistance. When absent the hardware reset default is used.
> +
> + nuvoton,oc-active-high:
> + type: boolean
> + description:
> + When present, the over-current detect input from the VBUS power
> + switch is treated as active-high. The default (property absent) is
> + active-low. This setting is shared by both USB host ports.
> +
> +required:
> + - compatible
> + - clocks
> + - nuvoton,sys
> + - "#phy-cells"
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/clock/nuvoton,ma35d1-clk.h>
> +
> + usb_hphy0: usb-host-phy {
usb-phy
And drop unused label.
> + compatible = "nuvoton,ma35d1-usb2-phy-otg";
> + clocks = <&clk HUSBH0_GATE>;
> + nuvoton,sys = <&sys 0>;
> + #phy-cells = <0>;
> + };
> --
> 2.43.0
>
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v3 3/5] arm64: dts: qcom: Add Shikra CQ2390M SoM platform
From: Konrad Dybcio @ 2026-06-08 10:44 UTC (permalink / raw)
To: Komal Bajaj, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Vinod Koul, Neil Armstrong,
Wesley Cheng, Ulf Hansson
Cc: linux-arm-msm, devicetree, linux-kernel, linux-phy, linux-mmc,
monish.chunara, Rakesh Kota
In-Reply-To: <3758be2d-4553-40b3-8ae6-bc3e502d0531@oss.qualcomm.com>
On 5/27/26 3:32 PM, Komal Bajaj wrote:
> On 5/25/2026 6:46 PM, Konrad Dybcio wrote:
>> On 5/22/26 8:02 PM, Komal Bajaj wrote:
>>> Add device tree include for the CQ2390M variant of the Shikra
>>> System-on-Module, a compact compute module integrating the Shikra SoC
>>> and PMIC for IoT applications, designed to mount on carrier boards.
>>>
>>> - shikra-cqm-som.dtsi: Retail SoM with modem (PM4125 and PM8005 PMIC)
>>>
>>> The DTSI includes the common shikra.dtsi, adds PM4125 and PM8005 PMIC
>>> regulator definitions specific to this variant.
>>>
>>> Co-developed-by: Rakesh Kota <rakesh.kota@oss.qualcomm.com>
>>> Signed-off-by: Rakesh Kota <rakesh.kota@oss.qualcomm.com>
>>> Signed-off-by: Komal Bajaj <komal.bajaj@oss.qualcomm.com>
>>> ---
[...]
>>> +&pm8005_regulators {
>>> + status = "disabled";
>>> +};
>> ?
>
> Already there's an ongoing discussion over the thread [1].
> PM8005 regulators are controlled by rpmpd so it's not expected to be enabled here.
>
> [1]: https://lore.kernel.org/all/20260525093453.edmqryggp5kcqxxy@hu-kotarake-hyd.qualcomm.com/
Right. Downstream, MSM8998 (the other user) binds it to the GFX CPR,
which probably has the benefit lower latency than if it were to go
through RPM (although I was sorta hoping this would be an instance
of cargo cult that we could just rip out..).
If you add /omit-if-no-ref/ above the definition of the pm8005_regulators
node in pm8005.dtsi, does it get correctly skipped (i.e. if you do
dtc -I dtb shikra.dtb -O dts | grep qcom,pm8005-regulators, is is there)?
Konrad
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v9 04/10] spmi: Implement spmi_subdevice_alloc_and_add() and devm variant
From: sashiko-bot @ 2026-06-08 10:43 UTC (permalink / raw)
To: AngeloGioacchino Del Regno; +Cc: linux-phy, vkoul, olteanv, neil.armstrong
In-Reply-To: <20260608100949.36309-5-angelogioacchino.delregno@collabora.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] dev_set_name() uses the '%d' format specifier for a variable of type 'unsigned int', which will cause a -Wformat compiler warning.
--
commit 35ac69140b9b2d8a3b0e2aa01fe5f5ac3ffe4d02
Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
spmi: Implement spmi_subdevice_alloc_and_add() and devm variant
This commit introduces functions for managing sub-devices of an SPMI
device, including spmi_subdevice_alloc_and_add() and its devm
variant. These allow complex PMICs with multiple sub-devices to be
properly modeled and managed via the SPMI bus.
> diff --git a/drivers/spmi/spmi.c b/drivers/spmi/spmi.c
> index 91a40ea8031ad..7945cacf6d655 100644
> --- a/drivers/spmi/spmi.c
> +++ b/drivers/spmi/spmi.c
[ ... ]
> @@ -428,6 +454,58 @@ struct spmi_device *spmi_device_alloc(struct spmi_controller *ctrl)
> }
> EXPORT_SYMBOL_GPL(spmi_device_alloc);
>
> +/**
> + * spmi_subdevice_alloc_and_add(): Allocate and add a new SPMI sub-device
> + * @sparent: SPMI parent device with previously registered SPMI controller
> + *
> + * Returns:
> + * Pointer to newly allocated SPMI sub-device for success or error pointer.
> + */
> +struct spmi_subdevice *spmi_subdevice_alloc_and_add(struct spmi_device *sparent)
> +{
> + struct spmi_subdevice *sub_sdev;
> + struct spmi_device *sdev;
> + int ret;
> +
> + sub_sdev = kzalloc(sizeof(*sub_sdev), GFP_KERNEL);
> + if (!sub_sdev)
> + return ERR_PTR(-ENOMEM);
> +
> + sdev = &sub_sdev->sdev;
> + sdev->ctrl = sparent->ctrl;
> + sdev->usid = sparent->usid;
> +
> + ret = ida_alloc(&spmi_subdevice_ida, GFP_KERNEL);
> + if (ret < 0) {
> + kfree(sub_sdev);
> + return ERR_PTR(ret);
> + }
> + sub_sdev->devid = ret;
> +
> + device_initialize(&sdev->dev);
> + sdev->dev.parent = &sparent->dev;
> + sdev->dev.bus = &spmi_bus_type;
> + sdev->dev.type = &spmi_subdev_type;
> +
> + ret = dev_set_name(&sdev->dev, "%u-%02x.%d.auto",
> + sdev->ctrl->nr, sdev->usid, sub_sdev->devid);
[Severity: Low]
This isn't a bug, but should this use %u instead of %d for the format
specifier? The devid field in struct spmi_subdevice is defined as an
unsigned int, which can cause a -Wformat compiler warning here.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260608100949.36309-1-angelogioacchino.delregno@collabora.com?part=4
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v9 02/10] spmi: Print error status with %pe format
From: sashiko-bot @ 2026-06-08 10:37 UTC (permalink / raw)
To: AngeloGioacchino Del Regno; +Cc: neil.armstrong, linux-phy, vkoul, olteanv
In-Reply-To: <20260608100949.36309-3-angelogioacchino.delregno@collabora.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] The OF device node assigned to the newly allocated SPMI device does not have its reference count incremented, creating a potential use-after-free condition when the iterator macro automatically drops the reference.
--
commit eb2ded032315a7ba1f8c878f6e0a10db80cf0346
Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
spmi: Print error status with %pe format
This commit updates error messages in the SPMI driver to use the %pe format
specifier for printing error statuses instead of printing integer error codes.
> diff --git a/drivers/spmi/spmi.c b/drivers/spmi/spmi.c
> index e93fc2c957590..95db19a0a44ec 100644
> --- a/drivers/spmi/spmi.c
> +++ b/drivers/spmi/spmi.c
[ ... ]
> @@ -522,7 +522,7 @@ static void of_spmi_register_devices(struct spmi_controller *ctrl)
> err = spmi_device_add(sdev);
> if (err) {
> dev_err(&sdev->dev,
> - "failure adding device. status %d\n", err);
> + "failure adding device. status %pe\n", ERR_PTR(err));
[Severity: High]
This is a pre-existing issue, but I noticed a potential use-after-free
condition with the OF node reference counting.
In drivers/spmi/spmi.c:of_spmi_register_devices(), the
for_each_available_child_of_node() loop iterates through child nodes,
automatically calling of_node_put() on the previous node when advancing.
Inside the loop, the node is assigned to the SPMI device:
device_set_node(&sdev->dev, of_fwnode_handle(node));
Does this assignment need to invoke of_node_get() to increment the node's
reference count? Without it, when the loop advances, the node's reference
is dropped, leaving sdev->dev.of_node as a dangling pointer.
If the device tree node is dynamically managed (e.g., via DT overlays),
unloading the overlay will free the node, causing a use-after-free whenever
the active SPMI device later accesses its of_node.
Additionally, drivers/spmi/spmi.c:spmi_dev_release() lacks an of_node_put()
to balance the required reference:
static void spmi_dev_release(struct device *dev)
{
struct spmi_device *sdev = to_spmi_device(dev);
kfree(sdev);
}
Should spmi_dev_release() be updated to safely release the reference once
it is correctly obtained?
> spmi_device_put(sdev);
> }
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260608100949.36309-1-angelogioacchino.delregno@collabora.com?part=2
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* [PATCH v2 2/2] dt-bindings: phy: qcom,qmp-usb: Add ipq5210 USB3 PHY
From: Varadarajan Narayanan @ 2026-06-08 10:33 UTC (permalink / raw)
To: vkoul, neil.armstrong, robh, krzk+dt, conor+dt, quic_wcheng,
linux-arm-msm, linux-phy, devicetree, linux-kernel
Cc: Varadarajan Narayanan
In-Reply-To: <20260608103344.2740174-1-varadarajan.narayanan@oss.qualcomm.com>
Add dt-bindings for the USB3 QMP PHY found on the Qualcomm IPQ5210 SoC. The
IPQ5210 PHY is compatible with the IPQ9574 PHY, so add it as a fallback-
compatible entry using a oneOf construct rather than a plain enum entry.
Signed-off-by: Varadarajan Narayanan <varadarajan.narayanan@oss.qualcomm.com>
---
.../phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml | 46 +++++++++++--------
1 file changed, 26 insertions(+), 20 deletions(-)
diff --git a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml
index 623c2f8c7d22..01342823e57f 100644
--- a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml
@@ -15,26 +15,32 @@ description:
properties:
compatible:
- enum:
- - qcom,glymur-qmp-usb3-uni-phy
- - qcom,ipq5424-qmp-usb3-phy
- - qcom,ipq6018-qmp-usb3-phy
- - qcom,ipq8074-qmp-usb3-phy
- - qcom,ipq9574-qmp-usb3-phy
- - qcom,msm8996-qmp-usb3-phy
- - qcom,qcs8300-qmp-usb3-uni-phy
- - qcom,qdu1000-qmp-usb3-uni-phy
- - qcom,sa8775p-qmp-usb3-uni-phy
- - qcom,sc8180x-qmp-usb3-uni-phy
- - qcom,sc8280xp-qmp-usb3-uni-phy
- - qcom,sdm845-qmp-usb3-uni-phy
- - qcom,sdx55-qmp-usb3-uni-phy
- - qcom,sdx65-qmp-usb3-uni-phy
- - qcom,sdx75-qmp-usb3-uni-phy
- - qcom,sm8150-qmp-usb3-uni-phy
- - qcom,sm8250-qmp-usb3-uni-phy
- - qcom,sm8350-qmp-usb3-uni-phy
- - qcom,x1e80100-qmp-usb3-uni-phy
+ oneOf:
+ - items:
+ - enum:
+ - qcom,glymur-qmp-usb3-uni-phy
+ - qcom,ipq5424-qmp-usb3-phy
+ - qcom,ipq6018-qmp-usb3-phy
+ - qcom,ipq8074-qmp-usb3-phy
+ - qcom,ipq9574-qmp-usb3-phy
+ - qcom,msm8996-qmp-usb3-phy
+ - qcom,qcs8300-qmp-usb3-uni-phy
+ - qcom,qdu1000-qmp-usb3-uni-phy
+ - qcom,sa8775p-qmp-usb3-uni-phy
+ - qcom,sc8180x-qmp-usb3-uni-phy
+ - qcom,sc8280xp-qmp-usb3-uni-phy
+ - qcom,sdm845-qmp-usb3-uni-phy
+ - qcom,sdx55-qmp-usb3-uni-phy
+ - qcom,sdx65-qmp-usb3-uni-phy
+ - qcom,sdx75-qmp-usb3-uni-phy
+ - qcom,sm8150-qmp-usb3-uni-phy
+ - qcom,sm8250-qmp-usb3-uni-phy
+ - qcom,sm8350-qmp-usb3-uni-phy
+ - qcom,x1e80100-qmp-usb3-uni-phy
+ - items:
+ - enum:
+ - qcom,ipq5210-qmp-usb3-phy
+ - const: qcom,ipq9574-qmp-usb3-phy
reg:
maxItems: 1
--
2.34.1
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* [PATCH v2 1/2] dt-bindings: phy: qcom,qusb2: Document IPQ5210 compatible
From: Varadarajan Narayanan @ 2026-06-08 10:33 UTC (permalink / raw)
To: vkoul, neil.armstrong, robh, krzk+dt, conor+dt, quic_wcheng,
linux-arm-msm, linux-phy, devicetree, linux-kernel
Cc: Varadarajan Narayanan
In-Reply-To: <20260608103344.2740174-1-varadarajan.narayanan@oss.qualcomm.com>
Document the QUSB2 PHY compatible for the IPQ5210 SoC. The IPQ5210 PHY is
compatible with the IPQ6018 QUSB2 PHY, so allow it to use
qcom,ipq6018-qusb2-phy as the fallback compatible.
Signed-off-by: Varadarajan Narayanan <varadarajan.narayanan@oss.qualcomm.com>
---
Documentation/devicetree/bindings/phy/qcom,qusb2-phy.yaml | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/Documentation/devicetree/bindings/phy/qcom,qusb2-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,qusb2-phy.yaml
index 39851ba9de43..449c2a7e5fec 100644
--- a/Documentation/devicetree/bindings/phy/qcom,qusb2-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/qcom,qusb2-phy.yaml
@@ -30,6 +30,10 @@ properties:
- qcom,sdm660-qusb2-phy
- qcom,sm4250-qusb2-phy
- qcom,sm6115-qusb2-phy
+ - items:
+ - enum:
+ - qcom,ipq5210-qusb2-phy
+ - const: qcom,ipq6018-qusb2-phy
- items:
- enum:
- qcom,sc7180-qusb2-phy
--
2.34.1
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* [PATCH v2 0/2] Add ipq5210 USB phy support
From: Varadarajan Narayanan @ 2026-06-08 10:33 UTC (permalink / raw)
To: vkoul, neil.armstrong, robh, krzk+dt, conor+dt, quic_wcheng,
linux-arm-msm, linux-phy, devicetree, linux-kernel
Cc: Varadarajan Narayanan
The ipq5210 SoC has both USB2.0 and USB3.0 controllers. The USB3.0
can connect to either of USB2.0 or USB3.0 phy and operate in the
respective mode.
v2: Use ipq6018 and ipq9574 as fallback compatibles for qusb2 and qmp
phys respectively instead of introducing ipq5210 as a new compatible.
Dropped driver changes as moved to fallback compatible.
Separate the controller and phy patches
v1: https://lore.kernel.org/linux-arm-msm/20260515-usb2phy-v1-0-5f8338d466bf@oss.qualcomm.com/
Varadarajan Narayanan (2):
dt-bindings: phy: qcom,qusb2: Document IPQ5210 compatible
dt-bindings: phy: qcom,qmp-usb: Add ipq5210 USB3 PHY
.../bindings/phy/qcom,qusb2-phy.yaml | 4 ++
.../phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml | 46 +++++++++++--------
2 files changed, 30 insertions(+), 20 deletions(-)
--
2.34.1
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: phy: qcom,ipq8074-qmp-pcie: document IPQ9650 QMP PCIe PHYs
From: Krzysztof Kozlowski @ 2026-06-08 10:12 UTC (permalink / raw)
To: Kathiravan Thirumoorthy
Cc: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, linux-arm-msm, linux-phy, devicetree, linux-kernel
In-Reply-To: <20260602-ipq9650_pcie_phy-v1-1-d8c32a36dbd9@oss.qualcomm.com>
On Tue, Jun 02, 2026 at 02:40:17PM +0530, Kathiravan Thirumoorthy wrote:
> Document the single-lane and dual-lane QMP PCIe PHYs found on the
> IPQ9650 SoC.
>
> Unlike the PHYs in the other supported IPQ SoCs, the IPQ9650 PHYs require
> the on-chip refgen supply to power up. Add the refgen-supply property
> and require it only for the IPQ9650 compatibles.
>
> Signed-off-by: Kathiravan Thirumoorthy <kathiravan.thirumoorthy@oss.qualcomm.com>
> ---
> .../bindings/phy/qcom,ipq8074-qmp-pcie-phy.yaml | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Best regards,
Krzysztof
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* [PATCH v9 10/10] iio: adc: qcom-spmi-iadc: Remove regmap R/W wrapper functions
From: AngeloGioacchino Del Regno @ 2026-06-08 10:09 UTC (permalink / raw)
To: jic23
Cc: dlechner, nuno.sa, andy, arnd, gregkh, srini, vkoul,
neil.armstrong, sre, sboyd, angelogioacchino.delregno, krzk,
dmitry.baryshkov, quic_wcheng, melody.olvera, quic_nsekar,
ivo.ivanov.ivanov1, abelvesa, luca.weiss, konrad.dybcio,
mitltlatltl, krishna.kurapati, linux-arm-msm, linux-iio,
linux-kernel, linux-phy, linux-pm, kernel, Jonathan Cameron,
Andy Shevchenko
In-Reply-To: <20260608100949.36309-1-angelogioacchino.delregno@collabora.com>
This driver doesn't need to add any register base address to any
regmap call anymore since it was migrated to register as a SPMI
subdevice with its own regmap reg_base, which makes the regmap
API to automatically add such base address internally.
Since the iadc_{read,write,read_result}() functions now only do
call regmap_{read,write,bulk_read}() and nothing else, simplify
the driver by removing them and by calling regmap APIs directly.
Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
Tested-by: Neil Armstrong <neil.armstrong@linaro.org> # on SM8650-QRD
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
drivers/iio/adc/qcom-spmi-iadc.c | 89 ++++++++++++--------------------
1 file changed, 33 insertions(+), 56 deletions(-)
diff --git a/drivers/iio/adc/qcom-spmi-iadc.c b/drivers/iio/adc/qcom-spmi-iadc.c
index ad66bb92fe19..29e71e6ac374 100644
--- a/drivers/iio/adc/qcom-spmi-iadc.c
+++ b/drivers/iio/adc/qcom-spmi-iadc.c
@@ -113,77 +113,59 @@ struct iadc_chip {
struct completion complete;
};
-static int iadc_read(struct iadc_chip *iadc, u16 offset, u8 *data)
-{
- unsigned int val;
- int ret;
-
- ret = regmap_read(iadc->regmap, offset, &val);
- if (ret < 0)
- return ret;
-
- *data = val;
- return 0;
-}
-
-static int iadc_write(struct iadc_chip *iadc, u16 offset, u8 data)
-{
- return regmap_write(iadc->regmap, offset, data);
-}
-
static int iadc_reset(struct iadc_chip *iadc)
{
- u8 data;
+ u32 data;
int ret;
- ret = iadc_write(iadc, IADC_SEC_ACCESS, IADC_SEC_ACCESS_DATA);
+ ret = regmap_write(iadc->regmap, IADC_SEC_ACCESS, IADC_SEC_ACCESS_DATA);
if (ret < 0)
return ret;
- ret = iadc_read(iadc, IADC_PERH_RESET_CTL3, &data);
+ ret = regmap_read(iadc->regmap, IADC_PERH_RESET_CTL3, &data);
if (ret < 0)
return ret;
- ret = iadc_write(iadc, IADC_SEC_ACCESS, IADC_SEC_ACCESS_DATA);
+ ret = regmap_write(iadc->regmap, IADC_SEC_ACCESS, IADC_SEC_ACCESS_DATA);
if (ret < 0)
return ret;
data |= IADC_FOLLOW_WARM_RB;
- return iadc_write(iadc, IADC_PERH_RESET_CTL3, data);
+ return regmap_write(iadc->regmap, IADC_PERH_RESET_CTL3, data);
}
static int iadc_set_state(struct iadc_chip *iadc, bool state)
{
- return iadc_write(iadc, IADC_EN_CTL1, state ? IADC_EN_CTL1_SET : 0);
+ return regmap_write(iadc->regmap, IADC_EN_CTL1, state ? IADC_EN_CTL1_SET : 0);
}
static void iadc_status_show(struct iadc_chip *iadc)
{
- u8 mode, sta1, chan, dig, en, req;
+ u32 mode, sta1, chan, dig, en, req;
int ret;
- ret = iadc_read(iadc, IADC_MODE_CTL, &mode);
+ ret = regmap_read(iadc->regmap, IADC_MODE_CTL, &mode);
if (ret < 0)
return;
- ret = iadc_read(iadc, IADC_DIG_PARAM, &dig);
+ ret = regmap_read(iadc->regmap, IADC_DIG_PARAM, &dig);
if (ret < 0)
return;
- ret = iadc_read(iadc, IADC_CH_SEL_CTL, &chan);
+ ret = regmap_read(iadc->regmap, IADC_CH_SEL_CTL, &chan);
if (ret < 0)
return;
- ret = iadc_read(iadc, IADC_CONV_REQ, &req);
+ ret = regmap_read(iadc->regmap, IADC_CONV_REQ, &req);
if (ret < 0)
return;
- ret = iadc_read(iadc, IADC_STATUS1, &sta1);
+ ret = regmap_read(iadc->regmap, IADC_STATUS1, &sta1);
if (ret < 0)
return;
- ret = iadc_read(iadc, IADC_EN_CTL1, &en);
+ ret = regmap_read(iadc->regmap, IADC_EN_CTL1, &en);
if (ret < 0)
return;
@@ -199,34 +181,34 @@ static int iadc_configure(struct iadc_chip *iadc, int channel)
/* Mode selection */
mode = (IADC_OP_MODE_NORMAL << IADC_OP_MODE_SHIFT) | IADC_TRIM_EN;
- ret = iadc_write(iadc, IADC_MODE_CTL, mode);
+ ret = regmap_write(iadc->regmap, IADC_MODE_CTL, mode);
if (ret < 0)
return ret;
/* Channel selection */
- ret = iadc_write(iadc, IADC_CH_SEL_CTL, channel);
+ ret = regmap_write(iadc->regmap, IADC_CH_SEL_CTL, channel);
if (ret < 0)
return ret;
/* Digital parameter setup */
decim = IADC_DEF_DECIMATION << IADC_DIG_DEC_RATIO_SEL_SHIFT;
- ret = iadc_write(iadc, IADC_DIG_PARAM, decim);
+ ret = regmap_write(iadc->regmap, IADC_DIG_PARAM, decim);
if (ret < 0)
return ret;
/* HW settle time delay */
- ret = iadc_write(iadc, IADC_HW_SETTLE_DELAY, IADC_DEF_HW_SETTLE_TIME);
+ ret = regmap_write(iadc->regmap, IADC_HW_SETTLE_DELAY, IADC_DEF_HW_SETTLE_TIME);
if (ret < 0)
return ret;
- ret = iadc_write(iadc, IADC_FAST_AVG_CTL, IADC_DEF_AVG_SAMPLES);
+ ret = regmap_write(iadc->regmap, IADC_FAST_AVG_CTL, IADC_DEF_AVG_SAMPLES);
if (ret < 0)
return ret;
if (IADC_DEF_AVG_SAMPLES)
- ret = iadc_write(iadc, IADC_FAST_AVG_EN, IADC_FAST_AVG_EN_SET);
+ ret = regmap_write(iadc->regmap, IADC_FAST_AVG_EN, IADC_FAST_AVG_EN_SET);
else
- ret = iadc_write(iadc, IADC_FAST_AVG_EN, 0);
+ ret = regmap_write(iadc->regmap, IADC_FAST_AVG_EN, 0);
if (ret < 0)
return ret;
@@ -239,19 +221,19 @@ static int iadc_configure(struct iadc_chip *iadc, int channel)
return ret;
/* Request conversion */
- return iadc_write(iadc, IADC_CONV_REQ, IADC_CONV_REQ_SET);
+ return regmap_write(iadc->regmap, IADC_CONV_REQ, IADC_CONV_REQ_SET);
}
static int iadc_poll_wait_eoc(struct iadc_chip *iadc, unsigned int interval_us)
{
unsigned int count, retry;
+ u32 sta1;
int ret;
- u8 sta1;
retry = interval_us / IADC_CONV_TIME_MIN_US;
for (count = 0; count < retry; count++) {
- ret = iadc_read(iadc, IADC_STATUS1, &sta1);
+ ret = regmap_read(iadc->regmap, IADC_STATUS1, &sta1);
if (ret < 0)
return ret;
@@ -267,11 +249,6 @@ static int iadc_poll_wait_eoc(struct iadc_chip *iadc, unsigned int interval_us)
return -ETIMEDOUT;
}
-static int iadc_read_result(struct iadc_chip *iadc, u16 *data)
-{
- return regmap_bulk_read(iadc->regmap, IADC_DATA, data, 2);
-}
-
static int iadc_do_conversion(struct iadc_chip *iadc, int chan, u16 *data)
{
unsigned int wait;
@@ -296,7 +273,7 @@ static int iadc_do_conversion(struct iadc_chip *iadc, int chan, u16 *data)
}
if (!ret)
- ret = iadc_read_result(iadc, data);
+ ret = regmap_bulk_read(iadc->regmap, IADC_DATA, data, sizeof(*data));
exit:
iadc_set_state(iadc, false);
if (ret < 0)
@@ -392,33 +369,33 @@ static int iadc_update_offset(struct iadc_chip *iadc)
static int iadc_version_check(struct iadc_chip *iadc)
{
- u8 val;
+ u32 val;
int ret;
- ret = iadc_read(iadc, IADC_PERPH_TYPE, &val);
+ ret = regmap_read(iadc->regmap, IADC_PERPH_TYPE, &val);
if (ret < 0)
return ret;
if (val < IADC_PERPH_TYPE_ADC) {
- dev_err(iadc->dev, "%d is not ADC\n", val);
+ dev_err(iadc->dev, "%u is not ADC\n", val);
return -EINVAL;
}
- ret = iadc_read(iadc, IADC_PERPH_SUBTYPE, &val);
+ ret = regmap_read(iadc->regmap, IADC_PERPH_SUBTYPE, &val);
if (ret < 0)
return ret;
if (val < IADC_PERPH_SUBTYPE_IADC) {
- dev_err(iadc->dev, "%d is not IADC\n", val);
+ dev_err(iadc->dev, "%u is not IADC\n", val);
return -EINVAL;
}
- ret = iadc_read(iadc, IADC_REVISION2, &val);
+ ret = regmap_read(iadc->regmap, IADC_REVISION2, &val);
if (ret < 0)
return ret;
if (val < IADC_REVISION2_SUPPORTED_IADC) {
- dev_err(iadc->dev, "revision %d not supported\n", val);
+ dev_err(iadc->dev, "revision %u not supported\n", val);
return -EINVAL;
}
@@ -428,7 +405,7 @@ static int iadc_version_check(struct iadc_chip *iadc)
static int iadc_rsense_read(struct iadc_chip *iadc, struct device_node *node)
{
int ret, sign, int_sense;
- u8 deviation;
+ u32 deviation;
ret = of_property_read_u32(node, "qcom,external-resistor-micro-ohms",
&iadc->rsense[IADC_EXT_RSENSE]);
@@ -440,7 +417,7 @@ static int iadc_rsense_read(struct iadc_chip *iadc, struct device_node *node)
return -EINVAL;
}
- ret = iadc_read(iadc, IADC_NOMINAL_RSENSE, &deviation);
+ ret = regmap_read(iadc->regmap, IADC_NOMINAL_RSENSE, &deviation);
if (ret < 0)
return ret;
--
2.54.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* [PATCH v9 09/10] iio: adc: qcom-spmi-iadc: Migrate to devm_spmi_subdevice_alloc_and_add()
From: AngeloGioacchino Del Regno @ 2026-06-08 10:09 UTC (permalink / raw)
To: jic23
Cc: dlechner, nuno.sa, andy, arnd, gregkh, srini, vkoul,
neil.armstrong, sre, sboyd, angelogioacchino.delregno, krzk,
dmitry.baryshkov, quic_wcheng, melody.olvera, quic_nsekar,
ivo.ivanov.ivanov1, abelvesa, luca.weiss, konrad.dybcio,
mitltlatltl, krishna.kurapati, linux-arm-msm, linux-iio,
linux-kernel, linux-phy, linux-pm, kernel, Jonathan Cameron,
Andy Shevchenko
In-Reply-To: <20260608100949.36309-1-angelogioacchino.delregno@collabora.com>
Some Qualcomm PMICs integrate an Current ADC device, reachable
in a specific address range over SPMI.
Instead of using the parent SPMI device (the main PMIC) as a kind
of syscon in this driver, register a new SPMI sub-device and
initialize its own regmap with this sub-device's specific base
address, retrieved from the devicetree.
This allows to stop manually adding the register base address to
every R/W call in this driver, as this can be, and is now, handled
by the regmap API instead.
Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
Tested-by: Neil Armstrong <neil.armstrong@linaro.org> # on SM8650-QRD
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
drivers/iio/adc/qcom-spmi-iadc.c | 35 ++++++++++++++++++++++----------
1 file changed, 24 insertions(+), 11 deletions(-)
diff --git a/drivers/iio/adc/qcom-spmi-iadc.c b/drivers/iio/adc/qcom-spmi-iadc.c
index 0ec3a0c4b1de..ad66bb92fe19 100644
--- a/drivers/iio/adc/qcom-spmi-iadc.c
+++ b/drivers/iio/adc/qcom-spmi-iadc.c
@@ -16,6 +16,7 @@
#include <linux/platform_device.h>
#include <linux/regmap.h>
#include <linux/slab.h>
+#include <linux/spmi.h>
/* IADC register and bit definition */
#define IADC_REVISION2 0x1
@@ -94,7 +95,6 @@
* struct iadc_chip - IADC Current ADC device structure.
* @regmap: regmap for register read/write.
* @dev: This device pointer.
- * @base: base offset for the ADC peripheral.
* @rsense: Values of the internal and external sense resister in micro Ohms.
* @poll_eoc: Poll for end of conversion instead of waiting for IRQ.
* @offset: Raw offset values for the internal and external channels.
@@ -105,7 +105,6 @@
struct iadc_chip {
struct regmap *regmap;
struct device *dev;
- u16 base;
bool poll_eoc;
u32 rsense[2];
u16 offset[2];
@@ -119,7 +118,7 @@ static int iadc_read(struct iadc_chip *iadc, u16 offset, u8 *data)
unsigned int val;
int ret;
- ret = regmap_read(iadc->regmap, iadc->base + offset, &val);
+ ret = regmap_read(iadc->regmap, offset, &val);
if (ret < 0)
return ret;
@@ -129,7 +128,7 @@ static int iadc_read(struct iadc_chip *iadc, u16 offset, u8 *data)
static int iadc_write(struct iadc_chip *iadc, u16 offset, u8 data)
{
- return regmap_write(iadc->regmap, iadc->base + offset, data);
+ return regmap_write(iadc->regmap, offset, data);
}
static int iadc_reset(struct iadc_chip *iadc)
@@ -270,7 +269,7 @@ static int iadc_poll_wait_eoc(struct iadc_chip *iadc, unsigned int interval_us)
static int iadc_read_result(struct iadc_chip *iadc, u16 *data)
{
- return regmap_bulk_read(iadc->regmap, iadc->base + IADC_DATA, data, 2);
+ return regmap_bulk_read(iadc->regmap, IADC_DATA, data, 2);
}
static int iadc_do_conversion(struct iadc_chip *iadc, int chan, u16 *data)
@@ -488,12 +487,22 @@ static void iadc_disable_irq_wake(void *data)
static int iadc_probe(struct platform_device *pdev)
{
+ struct regmap_config iadc_regmap_config = {
+ .reg_bits = 16,
+ .val_bits = 8,
+ .max_register = 0xff,
+ .fast_io = true,
+ };
struct device_node *node = pdev->dev.of_node;
struct device *dev = &pdev->dev;
+ struct spmi_subdevice *sub_sdev;
+ struct spmi_device *sparent;
struct iio_dev *indio_dev;
struct iadc_chip *iadc;
int ret, irq_eoc;
- u32 res;
+
+ if (!dev->parent)
+ return -ENODEV;
indio_dev = devm_iio_device_alloc(dev, sizeof(*iadc));
if (!indio_dev)
@@ -502,18 +511,21 @@ static int iadc_probe(struct platform_device *pdev)
iadc = iio_priv(indio_dev);
iadc->dev = dev;
- iadc->regmap = dev_get_regmap(dev->parent, NULL);
- if (!iadc->regmap)
- return -ENODEV;
+ sparent = to_spmi_device(dev->parent);
+ sub_sdev = devm_spmi_subdevice_alloc_and_add(dev, sparent);
+ if (IS_ERR(sub_sdev))
+ return PTR_ERR(sub_sdev);
init_completion(&iadc->complete);
mutex_init(&iadc->lock);
- ret = of_property_read_u32(node, "reg", &res);
+ ret = device_property_read_u32(dev, "reg", &iadc_regmap_config.reg_base);
if (ret < 0)
return -ENODEV;
- iadc->base = res;
+ iadc->regmap = devm_regmap_init_spmi_ext(&sub_sdev->sdev, &iadc_regmap_config);
+ if (IS_ERR(iadc->regmap))
+ return PTR_ERR(iadc->regmap);
ret = iadc_version_check(iadc);
if (ret < 0)
@@ -596,3 +608,4 @@ MODULE_ALIAS("platform:qcom-spmi-iadc");
MODULE_DESCRIPTION("Qualcomm SPMI PMIC current ADC driver");
MODULE_LICENSE("GPL v2");
MODULE_AUTHOR("Ivan T. Ivanov <iivanov@mm-sol.com>");
+MODULE_IMPORT_NS("SPMI");
--
2.54.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* [PATCH v9 08/10] misc: qcom-coincell: Migrate to devm_spmi_subdevice_alloc_and_add()
From: AngeloGioacchino Del Regno @ 2026-06-08 10:09 UTC (permalink / raw)
To: jic23
Cc: dlechner, nuno.sa, andy, arnd, gregkh, srini, vkoul,
neil.armstrong, sre, sboyd, angelogioacchino.delregno, krzk,
dmitry.baryshkov, quic_wcheng, melody.olvera, quic_nsekar,
ivo.ivanov.ivanov1, abelvesa, luca.weiss, konrad.dybcio,
mitltlatltl, krishna.kurapati, linux-arm-msm, linux-iio,
linux-kernel, linux-phy, linux-pm, kernel, Andy Shevchenko
In-Reply-To: <20260608100949.36309-1-angelogioacchino.delregno@collabora.com>
Some Qualcomm PMICs integrate a charger for coincells, usually
powering an RTC when external (or main battery) power is missing.
Instead of using the parent SPMI device (the main PMIC) as a kind
of syscon in this driver, register a new SPMI sub-device and
initialize its own regmap with this sub-device's specific base
address, retrieved from the devicetree.
This allows to stop manually adding the register base address to
every R/W call in this driver, as this can be, and is now, handled
by the regmap API instead.
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Tested-by: Neil Armstrong <neil.armstrong@linaro.org> # on SM8650-QRD
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
drivers/misc/Kconfig | 2 ++
drivers/misc/qcom-coincell.c | 45 +++++++++++++++++++++++++-----------
2 files changed, 34 insertions(+), 13 deletions(-)
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 390256ed91f4..90947c015179 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -291,6 +291,8 @@ config HP_ILO
config QCOM_COINCELL
tristate "Qualcomm coincell charger support"
depends on MFD_SPMI_PMIC || COMPILE_TEST
+ depends on SPMI
+ select REGMAP_SPMI
help
This driver supports the coincell block found inside of
Qualcomm PMICs. The coincell charger provides a means to
diff --git a/drivers/misc/qcom-coincell.c b/drivers/misc/qcom-coincell.c
index 3c57f7429147..b0904f441300 100644
--- a/drivers/misc/qcom-coincell.c
+++ b/drivers/misc/qcom-coincell.c
@@ -9,11 +9,11 @@
#include <linux/of.h>
#include <linux/regmap.h>
#include <linux/platform_device.h>
+#include <linux/spmi.h>
struct qcom_coincell {
struct device *dev;
struct regmap *regmap;
- u32 base_addr;
};
#define QCOM_COINCELL_REG_RSET 0x44
@@ -35,7 +35,7 @@ static int qcom_coincell_chgr_config(struct qcom_coincell *chgr, int rset,
/* if disabling, just do that and skip other operations */
if (!enable)
return regmap_write(chgr->regmap,
- chgr->base_addr + QCOM_COINCELL_REG_ENABLE, 0);
+ QCOM_COINCELL_REG_ENABLE, 0);
/* find index for current-limiting resistor */
for (i = 0; i < ARRAY_SIZE(qcom_rset_map); i++)
@@ -58,7 +58,7 @@ static int qcom_coincell_chgr_config(struct qcom_coincell *chgr, int rset,
}
rc = regmap_write(chgr->regmap,
- chgr->base_addr + QCOM_COINCELL_REG_RSET, i);
+ QCOM_COINCELL_REG_RSET, i);
if (rc) {
/*
* This is mainly to flag a bad base_addr (reg) from dts.
@@ -71,37 +71,55 @@ static int qcom_coincell_chgr_config(struct qcom_coincell *chgr, int rset,
}
rc = regmap_write(chgr->regmap,
- chgr->base_addr + QCOM_COINCELL_REG_VSET, j);
+ QCOM_COINCELL_REG_VSET, j);
if (rc)
return rc;
/* set 'enable' register */
return regmap_write(chgr->regmap,
- chgr->base_addr + QCOM_COINCELL_REG_ENABLE,
+ QCOM_COINCELL_REG_ENABLE,
QCOM_COINCELL_ENABLE);
}
static int qcom_coincell_probe(struct platform_device *pdev)
{
- struct device_node *node = pdev->dev.of_node;
+ struct regmap_config qcom_coincell_regmap_config = {
+ .reg_bits = 16,
+ .val_bits = 8,
+ .max_register = 0xff,
+ .fast_io = true,
+ };
+ struct device *dev = &pdev->dev;
+ struct device_node *node = dev->of_node;
+ struct spmi_subdevice *sub_sdev;
+ struct spmi_device *sparent;
struct qcom_coincell chgr;
u32 rset = 0;
u32 vset = 0;
bool enable;
int rc;
- chgr.dev = &pdev->dev;
+ if (!dev->parent)
+ return -ENODEV;
- chgr.regmap = dev_get_regmap(pdev->dev.parent, NULL);
- if (!chgr.regmap) {
- dev_err(chgr.dev, "Unable to get regmap\n");
- return -EINVAL;
- }
+ chgr.dev = &pdev->dev;
- rc = of_property_read_u32(node, "reg", &chgr.base_addr);
+ rc = device_property_read_u32(dev, "reg", &qcom_coincell_regmap_config.reg_base);
if (rc)
return rc;
+ sparent = to_spmi_device(dev->parent);
+ sub_sdev = devm_spmi_subdevice_alloc_and_add(dev, sparent);
+ if (IS_ERR(sub_sdev))
+ return PTR_ERR(sub_sdev);
+
+ chgr.regmap = devm_regmap_init_spmi_ext(&sub_sdev->sdev,
+ &qcom_coincell_regmap_config);
+ if (IS_ERR(chgr.regmap)) {
+ dev_err(chgr.dev, "Unable to get regmap\n");
+ return PTR_ERR(chgr.regmap);
+ }
+
enable = !of_property_read_bool(node, "qcom,charger-disable");
if (enable) {
@@ -142,3 +160,4 @@ module_platform_driver(qcom_coincell_driver);
MODULE_DESCRIPTION("Qualcomm PMIC coincell charger driver");
MODULE_LICENSE("GPL v2");
+MODULE_IMPORT_NS("SPMI");
--
2.54.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox