* [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
* [PATCH v9 07/10] phy: qualcomm: eusb2-repeater: 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, Abel Vesa,
Andy Shevchenko
In-Reply-To: <20260608100949.36309-1-angelogioacchino.delregno@collabora.com>
Some Qualcomm PMICs integrate an USB Repeater device, used to
convert between eUSB2 and USB 2.0 signaling levels, 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 for EUSB2
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.
Tested-by: Neil Armstrong <neil.armstrong@linaro.org> # on SM8650-QRD
Reviewed-by: Abel Vesa <abel.vesa@linaro.org>
Acked-by: Vinod Koul <vkoul@kernel.org>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
drivers/phy/qualcomm/Kconfig | 2 +
.../phy/qualcomm/phy-qcom-eusb2-repeater.c | 54 +++++++++++--------
2 files changed, 35 insertions(+), 21 deletions(-)
diff --git a/drivers/phy/qualcomm/Kconfig b/drivers/phy/qualcomm/Kconfig
index 60a0ead127fa..902a788f35f1 100644
--- a/drivers/phy/qualcomm/Kconfig
+++ b/drivers/phy/qualcomm/Kconfig
@@ -128,7 +128,9 @@ config PHY_QCOM_QUSB2
config PHY_QCOM_EUSB2_REPEATER
tristate "Qualcomm PMIC eUSB2 Repeater Driver"
depends on OF && (ARCH_QCOM || COMPILE_TEST)
+ depends on SPMI
select GENERIC_PHY
+ select REGMAP_SPMI
help
Enable support for the USB high-speed eUSB2 repeater on Qualcomm
PMICs. The repeater is paired with a Synopsys or M31 eUSB2 Phy
diff --git a/drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c b/drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c
index efeec4709a15..878ae03f70af 100644
--- a/drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c
+++ b/drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c
@@ -9,6 +9,7 @@
#include <linux/regmap.h>
#include <linux/of.h>
#include <linux/phy/phy.h>
+#include <linux/spmi.h>
/* eUSB2 status registers */
#define EUSB2_RPTR_STATUS 0x08
@@ -66,7 +67,6 @@ struct eusb2_repeater {
struct phy *phy;
struct regulator_bulk_data *vregs;
const struct eusb2_repeater_cfg *cfg;
- u32 base;
enum phy_mode mode;
};
@@ -143,7 +143,6 @@ static int eusb2_repeater_init(struct phy *phy)
struct eusb2_repeater *rptr = phy_get_drvdata(phy);
struct device_node *np = rptr->dev->of_node;
struct regmap *regmap = rptr->regmap;
- u32 base = rptr->base;
u32 poll_val;
s32 dt_val;
int ret;
@@ -154,37 +153,37 @@ static int eusb2_repeater_init(struct phy *phy)
if (ret)
return ret;
- regmap_write(regmap, base + EUSB2_EN_CTL1, EUSB2_RPTR_EN);
+ regmap_write(regmap, EUSB2_EN_CTL1, EUSB2_RPTR_EN);
/* Write registers from init table */
for (int i = 0; i < rptr->cfg->init_tbl_num; i++)
- regmap_write(regmap, base + rptr->cfg->init_tbl[i].reg,
+ regmap_write(regmap, rptr->cfg->init_tbl[i].reg,
rptr->cfg->init_tbl[i].value);
/* Override registers from devicetree values */
if (!of_property_read_u8(np, "qcom,tune-usb2-preem", &val))
- regmap_write(regmap, base + EUSB2_TUNE_USB2_PREEM, val);
+ regmap_write(regmap, EUSB2_TUNE_USB2_PREEM, val);
if (!of_property_read_u8(np, "qcom,tune-usb2-disc-thres", &val))
- regmap_write(regmap, base + EUSB2_TUNE_HSDISC, val);
+ regmap_write(regmap, EUSB2_TUNE_HSDISC, val);
if (!of_property_read_u8(np, "qcom,tune-usb2-amplitude", &val))
- regmap_write(regmap, base + EUSB2_TUNE_IUSB2, val);
+ regmap_write(regmap, EUSB2_TUNE_IUSB2, val);
if (!of_property_read_u8(np, "qcom,tune-res-fsdif", &val))
- regmap_write(regmap, base + EUSB2_TUNE_RES_FSDIF, val);
+ regmap_write(regmap, EUSB2_TUNE_RES_FSDIF, val);
if (!of_property_read_s32(np, "qcom,squelch-detector-bp", &dt_val)) {
for (i = 0; i < ARRAY_SIZE(squelch_detector); i++) {
if (squelch_detector[i] == dt_val) {
- regmap_write(regmap, base + EUSB2_TUNE_SQUELCH_U, i);
+ regmap_write(regmap, EUSB2_TUNE_SQUELCH_U, i);
break;
}
}
}
/* Wait for status OK */
- ret = regmap_read_poll_timeout(regmap, base + EUSB2_RPTR_STATUS, poll_val,
+ ret = regmap_read_poll_timeout(regmap, EUSB2_RPTR_STATUS, poll_val,
poll_val & RPTR_OK, 10, 5);
if (ret)
dev_err(rptr->dev, "initialization timed-out\n");
@@ -197,7 +196,6 @@ static int eusb2_repeater_set_mode(struct phy *phy,
{
struct eusb2_repeater *rptr = phy_get_drvdata(phy);
struct regmap *regmap = rptr->regmap;
- u32 base = rptr->base;
switch (mode) {
case PHY_MODE_USB_HOST:
@@ -206,8 +204,8 @@ static int eusb2_repeater_set_mode(struct phy *phy,
* per eUSB 1.2 Spec. Below implement software workaround until
* PHY and controller is fixing seen observation.
*/
- regmap_write(regmap, base + EUSB2_FORCE_EN_5, F_CLK_19P2M_EN);
- regmap_write(regmap, base + EUSB2_FORCE_VAL_5, V_CLK_19P2M_EN);
+ regmap_write(regmap, EUSB2_FORCE_EN_5, F_CLK_19P2M_EN);
+ regmap_write(regmap, EUSB2_FORCE_VAL_5, V_CLK_19P2M_EN);
break;
case PHY_MODE_USB_DEVICE:
/*
@@ -216,8 +214,8 @@ static int eusb2_repeater_set_mode(struct phy *phy,
* repeater doesn't clear previous value due to shared
* regulators (say host <-> device mode switch).
*/
- regmap_write(regmap, base + EUSB2_FORCE_EN_5, 0);
- regmap_write(regmap, base + EUSB2_FORCE_VAL_5, 0);
+ regmap_write(regmap, EUSB2_FORCE_EN_5, 0);
+ regmap_write(regmap, EUSB2_FORCE_VAL_5, 0);
break;
default:
return -EINVAL;
@@ -242,13 +240,23 @@ static const struct phy_ops eusb2_repeater_ops = {
static int eusb2_repeater_probe(struct platform_device *pdev)
{
+ struct regmap_config eusb2_regmap_config = {
+ .reg_bits = 16,
+ .val_bits = 8,
+ .max_register = 0xff,
+ .fast_io = true,
+ };
+ struct spmi_device *sparent;
struct eusb2_repeater *rptr;
+ struct spmi_subdevice *sub_sdev;
struct device *dev = &pdev->dev;
struct phy_provider *phy_provider;
struct device_node *np = dev->of_node;
- u32 res;
int ret;
+ if (!dev->parent)
+ return -ENODEV;
+
rptr = devm_kzalloc(dev, sizeof(*rptr), GFP_KERNEL);
if (!rptr)
return -ENOMEM;
@@ -260,15 +268,18 @@ static int eusb2_repeater_probe(struct platform_device *pdev)
if (!rptr->cfg)
return -EINVAL;
- rptr->regmap = dev_get_regmap(dev->parent, NULL);
- if (!rptr->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);
- ret = of_property_read_u32(np, "reg", &res);
+ ret = device_property_read_u32(dev, "reg", &eusb2_regmap_config.reg_base);
if (ret < 0)
return ret;
- rptr->base = res;
+ rptr->regmap = devm_regmap_init_spmi_ext(&sub_sdev->sdev, &eusb2_regmap_config);
+ if (IS_ERR(rptr->regmap))
+ return PTR_ERR(rptr->regmap);
ret = eusb2_repeater_init_vregs(rptr);
if (ret < 0) {
@@ -335,3 +346,4 @@ module_platform_driver(eusb2_repeater_driver);
MODULE_DESCRIPTION("Qualcomm PMIC eUSB2 Repeater driver");
MODULE_LICENSE("GPL");
+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 05/10] nvmem: qcom-spmi-sdam: 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 SDAM device, internally located in
a specific address range reachable through SPMI communication.
Instead of using the parent SPMI device (the main PMIC) as a kind
of syscon in this driver, register a new SPMI sub-device for SDAM
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: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Tested-by: Neil Armstrong <neil.armstrong@linaro.org> # on SM8650-QRD
Acked-by: Srinivas Kandagatla <srini@kernel.org>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
drivers/nvmem/Kconfig | 1 +
drivers/nvmem/qcom-spmi-sdam.c | 38 +++++++++++++++++++++++-----------
2 files changed, 27 insertions(+), 12 deletions(-)
diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
index e10f7ff725ff..cc032d60b8c1 100644
--- a/drivers/nvmem/Kconfig
+++ b/drivers/nvmem/Kconfig
@@ -380,6 +380,7 @@ config NVMEM_SNVS_LPGPR
config NVMEM_SPMI_SDAM
tristate "SPMI SDAM Support"
depends on SPMI
+ select REGMAP_SPMI
help
This driver supports the Shared Direct Access Memory Module on
Qualcomm Technologies, Inc. PMICs. It provides the clients
diff --git a/drivers/nvmem/qcom-spmi-sdam.c b/drivers/nvmem/qcom-spmi-sdam.c
index 4f1cca6eab71..4974105dd963 100644
--- a/drivers/nvmem/qcom-spmi-sdam.c
+++ b/drivers/nvmem/qcom-spmi-sdam.c
@@ -9,6 +9,7 @@
#include <linux/nvmem-provider.h>
#include <linux/platform_device.h>
#include <linux/regmap.h>
+#include <linux/spmi.h>
#define SDAM_MEM_START 0x40
#define REGISTER_MAP_ID 0x40
@@ -20,7 +21,6 @@
struct sdam_chip {
struct regmap *regmap;
struct nvmem_config sdam_config;
- unsigned int base;
unsigned int size;
};
@@ -73,7 +73,7 @@ static int sdam_read(void *priv, unsigned int offset, void *val,
return -EINVAL;
}
- rc = regmap_bulk_read(sdam->regmap, sdam->base + offset, val, bytes);
+ rc = regmap_bulk_read(sdam->regmap, offset, val, bytes);
if (rc < 0)
dev_err(dev, "Failed to read SDAM offset %#x len=%zd, rc=%d\n",
offset, bytes, rc);
@@ -100,7 +100,7 @@ static int sdam_write(void *priv, unsigned int offset, void *val,
return -EINVAL;
}
- rc = regmap_bulk_write(sdam->regmap, sdam->base + offset, val, bytes);
+ rc = regmap_bulk_write(sdam->regmap, offset, val, bytes);
if (rc < 0)
dev_err(dev, "Failed to write SDAM offset %#x len=%zd, rc=%d\n",
offset, bytes, rc);
@@ -110,8 +110,17 @@ static int sdam_write(void *priv, unsigned int offset, void *val,
static int sdam_probe(struct platform_device *pdev)
{
+ struct regmap_config sdam_regmap_config = {
+ .reg_bits = 16,
+ .val_bits = 8,
+ .max_register = 0xff,
+ .fast_io = true,
+ };
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);
+ if (IS_ERR(sub_sdev))
+ return PTR_ERR(sub_sdev);
- rc = of_property_read_u32(pdev->dev.of_node, "reg", &sdam->base);
+ rc = device_property_read_u32(dev, "reg", &sdam_regmap_config.reg_base);
if (rc < 0) {
- dev_err(&pdev->dev, "Failed to get SDAM base, rc=%d\n", rc);
+ dev_err(dev, "Failed to get SDAM base, rc=%d\n", rc);
return -EINVAL;
}
- rc = regmap_read(sdam->regmap, sdam->base + SDAM_SIZE, &val);
+ sdam->regmap = devm_regmap_init_spmi_ext(&sub_sdev->sdev, &sdam_regmap_config);
+ if (IS_ERR(sdam->regmap))
+ return dev_err_probe(dev, PTR_ERR(sdam->regmap),
+ "Failed to get regmap handle\n");
+
+ rc = regmap_read(sdam->regmap, SDAM_SIZE, &val);
if (rc < 0) {
dev_err(&pdev->dev, "Failed to read SDAM_SIZE rc=%d\n", rc);
return -EINVAL;
@@ -159,7 +172,7 @@ static int sdam_probe(struct platform_device *pdev)
}
dev_dbg(&pdev->dev,
"SDAM base=%#x size=%u registered successfully\n",
- sdam->base, sdam->size);
+ sdam_regmap_config.reg_base, sdam->size);
return 0;
}
@@ -181,3 +194,4 @@ module_platform_driver(sdam_driver);
MODULE_DESCRIPTION("QCOM SPMI SDAM 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
* [PATCH v9 06/10] power: reset: qcom-pon: 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, Sebastian Reichel,
Andy Shevchenko
In-Reply-To: <20260608100949.36309-1-angelogioacchino.delregno@collabora.com>
Some Qualcomm PMICs integrates a Power On device supporting pwrkey
and resin along with the Android reboot reason action identifier.
Instead of using the parent SPMI device (the main PMIC) as a kind
of syscon in this driver, register a new SPMI sub-device for PON
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: Sebastian Reichel <sebastian.reichel@collabora.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/power/reset/qcom-pon.c | 33 ++++++++++++++++++++++++---------
1 file changed, 24 insertions(+), 9 deletions(-)
diff --git a/drivers/power/reset/qcom-pon.c b/drivers/power/reset/qcom-pon.c
index 7e108982a582..182af188c9be 100644
--- a/drivers/power/reset/qcom-pon.c
+++ b/drivers/power/reset/qcom-pon.c
@@ -11,6 +11,7 @@
#include <linux/reboot.h>
#include <linux/reboot-mode.h>
#include <linux/regmap.h>
+#include <linux/spmi.h>
#define PON_SOFT_RB_SPARE 0x8f
@@ -22,7 +23,6 @@
struct qcom_pon {
struct device *dev;
struct regmap *regmap;
- u32 baseaddr;
struct reboot_mode_driver reboot_mode;
long reason_shift;
};
@@ -35,7 +35,7 @@ static int qcom_pon_reboot_mode_write(struct reboot_mode_driver *reboot,
int ret;
ret = regmap_update_bits(pon->regmap,
- pon->baseaddr + PON_SOFT_RB_SPARE,
+ PON_SOFT_RB_SPARE,
GENMASK(7, pon->reason_shift),
magic << pon->reason_shift);
if (ret < 0)
@@ -46,27 +46,41 @@ static int qcom_pon_reboot_mode_write(struct reboot_mode_driver *reboot,
static int qcom_pon_probe(struct platform_device *pdev)
{
+ struct regmap_config qcom_pon_regmap_config = {
+ .reg_bits = 16,
+ .val_bits = 8,
+ .max_register = 0xff,
+ .fast_io = true,
+ };
+ struct device *dev = &pdev->dev;
+ struct spmi_subdevice *sub_sdev;
+ struct spmi_device *sparent;
struct qcom_pon *pon;
long reason_shift;
int error;
+ if (!dev->parent)
+ return -ENODEV;
+
pon = devm_kzalloc(&pdev->dev, sizeof(*pon), GFP_KERNEL);
if (!pon)
return -ENOMEM;
pon->dev = &pdev->dev;
- pon->regmap = dev_get_regmap(pdev->dev.parent, NULL);
- if (!pon->regmap) {
- dev_err(&pdev->dev, "failed to locate regmap\n");
- 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);
- error = of_property_read_u32(pdev->dev.of_node, "reg",
- &pon->baseaddr);
+ error = device_property_read_u32(dev, "reg", &qcom_pon_regmap_config.reg_base);
if (error)
return error;
+ pon->regmap = devm_regmap_init_spmi_ext(&sub_sdev->sdev, &qcom_pon_regmap_config);
+ if (IS_ERR(pon->regmap))
+ return PTR_ERR(pon->regmap);
+
reason_shift = (long)of_device_get_match_data(&pdev->dev);
if (reason_shift != NO_REASON_SHIFT) {
@@ -106,3 +120,4 @@ module_platform_driver(qcom_pon_driver);
MODULE_DESCRIPTION("Qualcomm Power On 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
* [PATCH v9 04/10] spmi: Implement spmi_subdevice_alloc_and_add() and devm variant
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 devices connected over the SPMI bus may be big, in the sense
that those may be a complex of devices managed by a single chip
over the SPMI bus, reachable through a single SID.
Add new functions aimed at managing sub-devices of a SPMI device
spmi_subdevice_alloc_and_add() and a spmi_subdevice_remove() for
adding a new subdevice and removing it respectively, and also
add their devm_* variants.
The need for such functions comes from the existence of those
complex Power Management ICs (PMICs), which feature one or many
sub-devices, in some cases with these being even addressable on
the chip in form of SPMI register ranges.
Examples of those devices can be found in both Qualcomm platforms
with their PMICs having PON, RTC, SDAM, GPIO controller, and other
sub-devices, and in newer MediaTek platforms showing similar HW
features and a similar layout with those also having many subdevs.
Also, instead of generally exporting symbols, export them with a
new "SPMI" namespace: all users will have to import this namespace
to make use of the newly introduced exports.
Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
Tested-by: Neil Armstrong <neil.armstrong@linaro.org> # on SM8650-QRD
Acked-by: Stephen Boyd <sboyd@kernel.org>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
drivers/spmi/spmi-devres.c | 24 ++++++++++++
drivers/spmi/spmi.c | 78 ++++++++++++++++++++++++++++++++++++++
include/linux/spmi.h | 16 ++++++++
3 files changed, 118 insertions(+)
diff --git a/drivers/spmi/spmi-devres.c b/drivers/spmi/spmi-devres.c
index 62c4b3f24d06..c3e889fe1b6e 100644
--- a/drivers/spmi/spmi-devres.c
+++ b/drivers/spmi/spmi-devres.c
@@ -60,5 +60,29 @@ int devm_spmi_controller_add(struct device *parent, struct spmi_controller *ctrl
}
EXPORT_SYMBOL_GPL(devm_spmi_controller_add);
+static void devm_spmi_subdevice_remove(void *sub_sdev)
+{
+ spmi_subdevice_remove(sub_sdev);
+}
+
+struct spmi_subdevice *devm_spmi_subdevice_alloc_and_add(struct device *dev,
+ struct spmi_device *sparent)
+{
+ struct spmi_subdevice *sub_sdev;
+ int ret;
+
+ sub_sdev = spmi_subdevice_alloc_and_add(sparent);
+ if (IS_ERR(sub_sdev))
+ return sub_sdev;
+
+ ret = devm_add_action_or_reset(dev, devm_spmi_subdevice_remove, sub_sdev);
+ if (ret)
+ return ERR_PTR(ret);
+
+ return sub_sdev;
+}
+EXPORT_SYMBOL_NS_GPL(devm_spmi_subdevice_alloc_and_add, "SPMI");
+
MODULE_LICENSE("GPL");
MODULE_DESCRIPTION("SPMI devres helpers");
+MODULE_IMPORT_NS("SPMI");
diff --git a/drivers/spmi/spmi.c b/drivers/spmi/spmi.c
index 0f32f099b0ce..0d587248665e 100644
--- a/drivers/spmi/spmi.c
+++ b/drivers/spmi/spmi.c
@@ -19,6 +19,7 @@
static bool is_registered;
static DEFINE_IDA(ctrl_ida);
+static DEFINE_IDA(spmi_subdevice_ida);
static void spmi_dev_release(struct device *dev)
{
@@ -31,6 +32,19 @@ static const struct device_type spmi_dev_type = {
.release = spmi_dev_release,
};
+static void spmi_subdev_release(struct device *dev)
+{
+ struct spmi_device *sdev = to_spmi_device(dev);
+ struct spmi_subdevice *sub_sdev = container_of(sdev, struct spmi_subdevice, sdev);
+
+ ida_free(&spmi_subdevice_ida, sub_sdev->devid);
+ kfree(sub_sdev);
+}
+
+static const struct device_type spmi_subdev_type = {
+ .release = spmi_subdev_release,
+};
+
static void spmi_ctrl_release(struct device *dev)
{
struct spmi_controller *ctrl = to_spmi_controller(dev);
@@ -87,6 +101,18 @@ void spmi_device_remove(struct spmi_device *sdev)
}
EXPORT_SYMBOL_GPL(spmi_device_remove);
+/**
+ * spmi_subdevice_remove() - Remove an SPMI subdevice
+ * @sub_sdev: spmi_device to be removed
+ */
+void spmi_subdevice_remove(struct spmi_subdevice *sub_sdev)
+{
+ struct spmi_device *sdev = &sub_sdev->sdev;
+
+ device_unregister(&sdev->dev);
+}
+EXPORT_SYMBOL_NS_GPL(spmi_subdevice_remove, "SPMI");
+
static inline int
spmi_cmd(struct spmi_controller *ctrl, u8 opcode, u8 sid)
{
@@ -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);
+ if (ret)
+ goto err_put_dev;
+
+ ret = device_add(&sdev->dev);
+ if (ret) {
+ dev_err(&sdev->dev, "Can't add device, status %pe\n", ERR_PTR(ret));
+ goto err_put_dev;
+ }
+
+ return sub_sdev;
+
+err_put_dev:
+ put_device(&sdev->dev);
+ return ERR_PTR(ret);
+}
+EXPORT_SYMBOL_NS_GPL(spmi_subdevice_alloc_and_add, "SPMI");
+
/**
* spmi_controller_alloc() - Allocate a new SPMI controller
* @parent: parent device
diff --git a/include/linux/spmi.h b/include/linux/spmi.h
index 4eb9564a7fb3..a78a8924b2ac 100644
--- a/include/linux/spmi.h
+++ b/include/linux/spmi.h
@@ -69,6 +69,22 @@ int spmi_device_add(struct spmi_device *sdev);
void spmi_device_remove(struct spmi_device *sdev);
+/**
+ * struct spmi_subdevice - Basic representation of an SPMI sub-device
+ * @sdev: Sub-device representation of an SPMI device
+ * @devid: Platform Device ID of an SPMI sub-device
+ */
+struct spmi_subdevice {
+ struct spmi_device sdev;
+ unsigned int devid;
+};
+
+struct spmi_subdevice *spmi_subdevice_alloc_and_add(struct spmi_device *sparent);
+void spmi_subdevice_remove(struct spmi_subdevice *sdev);
+
+struct spmi_subdevice *devm_spmi_subdevice_alloc_and_add(struct device *dev,
+ struct spmi_device *sparent);
+
/**
* struct spmi_controller - interface to the SPMI master controller
* @dev: Driver model representation of the device.
--
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 03/10] spmi: Remove unneeded goto in spmi_device_add() error path
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>
If any error happens during device_add() just return inside of the
conditional, as the goto path doesn't do anything else if not just
returning.
While at it, to improve readability, also change this function to
explicitly return 0 (for success) at the end.
Acked-by: Stephen Boyd <sboyd@kernel.org>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
drivers/spmi/spmi.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/spmi/spmi.c b/drivers/spmi/spmi.c
index b0017bd61d95..0f32f099b0ce 100644
--- a/drivers/spmi/spmi.c
+++ b/drivers/spmi/spmi.c
@@ -69,13 +69,11 @@ int spmi_device_add(struct spmi_device *sdev)
err = device_add(&sdev->dev);
if (err < 0) {
dev_err(&sdev->dev, "Can't add device, status %pe\n", ERR_PTR(err));
- goto err_device_add;
+ return err;
}
dev_dbg(&sdev->dev, "device registered\n");
-
-err_device_add:
- return err;
+ return 0;
}
EXPORT_SYMBOL_GPL(spmi_device_add);
--
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 02/10] spmi: Print error status with %pe format
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>
Instead of printing just a number, use the %pe format for error
status, increasing readability of error prints.
Acked-by: Stephen Boyd <sboyd@kernel.org>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
drivers/spmi/spmi.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/spmi/spmi.c b/drivers/spmi/spmi.c
index 152787ae82ca..b0017bd61d95 100644
--- a/drivers/spmi/spmi.c
+++ b/drivers/spmi/spmi.c
@@ -68,7 +68,7 @@ int spmi_device_add(struct spmi_device *sdev)
err = device_add(&sdev->dev);
if (err < 0) {
- dev_err(&sdev->dev, "Can't add device, status %d\n", err);
+ dev_err(&sdev->dev, "Can't add device, status %pe\n", ERR_PTR(err));
goto err_device_add;
}
@@ -493,8 +493,8 @@ static void of_spmi_register_devices(struct spmi_controller *ctrl)
err = of_property_read_u32_array(node, "reg", reg, 2);
if (err) {
dev_err(&ctrl->dev,
- "node %pOF err (%d) does not have 'reg' property\n",
- node, err);
+ "node %pOF err (%pe) does not have 'reg' property\n",
+ node, ERR_PTR(err));
continue;
}
@@ -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));
spmi_device_put(sdev);
}
}
--
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 01/10] spmi: Remove redundant dev_name() print in spmi_device_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>
Function spmi_device_add() uses dev_{dbg,err}() for respectively
debug and error prints, and passes the same device pointer as both
the dev_{dbg,err}() parameters and to a dev_name() that is part of
the actual message.
This means that the device name gets printed twice!
Remove the redundant dev_name() from the messages.
Acked-by: Stephen Boyd <sboyd@kernel.org>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
drivers/spmi/spmi.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/spmi/spmi.c b/drivers/spmi/spmi.c
index 57b7c0cb4240..152787ae82ca 100644
--- a/drivers/spmi/spmi.c
+++ b/drivers/spmi/spmi.c
@@ -68,12 +68,11 @@ int spmi_device_add(struct spmi_device *sdev)
err = device_add(&sdev->dev);
if (err < 0) {
- dev_err(&sdev->dev, "Can't add %s, status %d\n",
- dev_name(&sdev->dev), err);
+ dev_err(&sdev->dev, "Can't add device, status %d\n", err);
goto err_device_add;
}
- dev_dbg(&sdev->dev, "device %s registered\n", dev_name(&sdev->dev));
+ dev_dbg(&sdev->dev, "device registered\n");
err_device_add:
return err;
--
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 00/10] SPMI: Implement sub-devices and migrate drivers
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
Changes in v9:
- Added check for dev->parent where missing (Sashiko)
- Changed %d to %u in dev_set_name() call as arg is unsigned (Sashiko)
- Propagating error code from devm_regmap_init_spmi_ext() instead of
returning -ENODEV in phy-qcom-eusb2-repeater.c (Sashiko)
Changes in v8:
- Renamed *res to *sub_sdev in devm_spmi_subdevice_remove() (Andy)
- Changed kerneldoc wording to "error pointer" for function
spmi_subdevice_alloc_and_add() (Andy)
- Shuffled around some assignments in spmi_subdevice_alloc_and_add() (Andy)
- Used device_property_read_u32() instead of of_property_read_u32()
in all of the migrated drivers (Andy)
- Changed .max_register field in all of the migrated drivers from
0x100 to 0xff (Andy)
- Kept `sta1` declaration in reversed xmas tree order in function
iadc_poll_wait_eoc() of qcom-spmi-iadc.c (Andy)
Changes in v7:
- Added commit to cleanup redundant dev_name() in the pre-existing
spmi_device_add() function
- Added commit removing unneeded goto and improving spmi_device_add()
readability by returning error in error path, and explicitly zero
for success at the end.
Changes in v6:
- Added commit to convert spmi.c to %pe error format and used
%pe error format in spmi_subdevice code as wanted by Uwe Kleine-Konig
Changes in v5:
- Changed dev_err to dev_err_probe in qcom-spmi-sdam (and done
that even though I disagree - because I wanted this series to
*exclusively* introduce the minimum required changes to
migrate to the new API, but okay, whatever....!);
- Added missing REGMAP dependency in Kconfig for qcom-spmi-sdam,
phy-qcom-eusb2-repeater and qcom-coincell to resolve build
issues when the already allowed COMPILE_TEST is enabled
as pointed out by the test robot's randconfig builds.
Changes in v4:
- Added selection of REGMAP_SPMI in Kconfig for qcom-coincell and
for phy-qcom-eusb2-repeater to resolve undefined references when
compiled with some randconfig
Changes in v3:
- Fixed importing "SPMI" namespace in spmi-devres.c
- Removed all instances of defensive programming, as pointed out by
jic23 and Sebastian
- Removed explicit casting as pointed out by jic23
- Moved ida_free call to spmi_subdev_release() and simplified error
handling in spmi_subdevice_alloc_and_add() as pointed out by jic23
Changes in v2:
- Fixed missing `sparent` initialization in phy-qcom-eusb2-repeater
- Changed val_bits to 8 in all Qualcomm drivers to ensure
compatibility as suggested by Casey
- Added struct device pointer in all conversion commits as suggested
by Andy
- Exported newly introduced functions with a new "SPMI" namespace
and imported the same in all converted drivers as suggested by Andy
- Added missing error checking for dev_set_name() call in spmi.c
as suggested by Andy
- Added comma to last entry of regmap_config as suggested by Andy
While adding support for newer MediaTek platforms, featuring complex
SPMI PMICs, I've seen that those SPMI-connected chips are internally
divided in various IP blocks, reachable in specific contiguous address
ranges... more or less like a MMIO, but over a slow SPMI bus instead.
I recalled that Qualcomm had something similar... and upon checking a
couple of devicetrees, yeah - indeed it's the same over there.
What I've seen then is a common pattern of reading the "reg" property
from devicetree in a struct member and then either
A. Wrapping regmap_{read/write/etc}() calls in a function that adds
the register base with "base + ..register", like it's done with
writel()/readl() calls; or
B. Doing the same as A. but without wrapper functions.
Even though that works just fine, in my opinion it's wrong.
The regmap API is way more complex than MMIO-only readl()/writel()
functions for multiple reasons (including supporting multiple busses
like SPMI, of course) - but everyone seemed to forget that regmap
can manage register base offsets transparently and automatically in
its API functions by simply adding a `reg_base` to the regmap_config
structure, which is used for initializing a `struct regmap`.
So, here we go: this series implements the software concept of an SPMI
Sub-Device (which, well, also reflects how Qualcomm and MediaTek's
actual hardware is laid out anyway).
SPMI Controller
| ______
| / Sub-Device 1
V /
SPMI Device (PMIC) ----------- Sub-Device 2
\
\______ Sub-Device 3
As per this implementation, an SPMI Sub-Device can be allocated/created
and added in any driver that implements a... well.. subdevice (!) with
an SPMI "main" device as its parent: this allows to create and finally
to correctly configure a regmap that is specific to the sub-device,
operating on its specific address range and reading, and writing, to
its registers with the regmap API taking care of adding the base address
of a sub-device's registers as per regmap API design.
All of the SPMI Sub-Devices are therefore added as children of the SPMI
Device (usually a PMIC), as communication depends on the PMIC's SPMI bus
to be available (and the PMIC to be up and running, of course).
Summarizing the dependency chain (which is obvious to whoever knows what
is going on with Qualcomm and/or MediaTek SPMI PMICs):
"SPMI Sub-Device x...N" are children "SPMI Device"
"SPMI Device" is a child of "SPMI Controller"
(that was just another way to say the same thing as the graph above anyway).
Along with the new SPMI Sub-Device registration functions, I have also
performed a conversion of some Qualcomm SPMI drivers and only where the
actual conversion was trivial.
I haven't included any conversion of more complex Qualcomm SPMI drivers
because I don't have the required bandwidth to do so (and besides, I think,
but haven't exactly verified, that some of those require SoCs that I don't
have for testing anyway).
AngeloGioacchino Del Regno (10):
spmi: Remove redundant dev_name() print in spmi_device_add()
spmi: Print error status with %pe format
spmi: Remove unneeded goto in spmi_device_add() error path
spmi: Implement spmi_subdevice_alloc_and_add() and devm variant
nvmem: qcom-spmi-sdam: Migrate to devm_spmi_subdevice_alloc_and_add()
power: reset: qcom-pon: Migrate to devm_spmi_subdevice_alloc_and_add()
phy: qualcomm: eusb2-repeater: Migrate to
devm_spmi_subdevice_alloc_and_add()
misc: qcom-coincell: Migrate to devm_spmi_subdevice_alloc_and_add()
iio: adc: qcom-spmi-iadc: Migrate to
devm_spmi_subdevice_alloc_and_add()
iio: adc: qcom-spmi-iadc: Remove regmap R/W wrapper functions
drivers/iio/adc/qcom-spmi-iadc.c | 118 ++++++++----------
drivers/misc/Kconfig | 2 +
drivers/misc/qcom-coincell.c | 45 +++++--
drivers/nvmem/Kconfig | 1 +
drivers/nvmem/qcom-spmi-sdam.c | 38 ++++--
drivers/phy/qualcomm/Kconfig | 2 +
.../phy/qualcomm/phy-qcom-eusb2-repeater.c | 54 ++++----
drivers/power/reset/qcom-pon.c | 33 +++--
drivers/spmi/spmi-devres.c | 24 ++++
drivers/spmi/spmi.c | 95 ++++++++++++--
include/linux/spmi.h | 16 +++
11 files changed, 299 insertions(+), 129 deletions(-)
--
2.54.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v3 2/3] dt-bindings: PCI: qcom,pcie-sm8550: Add Eliza compatible
From: sashiko-bot @ 2026-06-08 8:56 UTC (permalink / raw)
To: Krishna Chaitanya Chundru
Cc: linux-phy, olteanv, vkoul, neil.armstrong, linux-pci, conor+dt,
robh, devicetree
In-Reply-To: <20260608-eliza-v3-2-9bdeb7434b28@oss.qualcomm.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The schema uses four separate `if` conditional blocks to apply constraints for the exact same `qcom,eliza-pcie` compatible string, instead of consolidating them into a single block.
--
commit 4a8af2f6d98b918d7b48060ac70342492b7abb27
Author: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
dt-bindings: PCI: qcom,pcie-sm8550: Add Eliza compatible
This commit adds the qcom,eliza-pcie compatible string to the SM8550 PCIe
bindings. It introduces specific property constraints for the Eliza SoC,
defining the required number of registers, clocks, interrupts, and resets.
> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie-sm8550.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie-sm8550.yaml
> --- a/Documentation/devicetree/bindings/pci/qcom,pcie-sm8550.yaml
> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie-sm8550.yaml
[ ... ]
> @@ -91,6 +92,55 @@ required:
>
> allOf:
> - $ref: qcom,pcie-common.yaml#
> + - if:
> + properties:
> + compatible:
> + contains:
> + const: qcom,eliza-pcie
> + then:
> + properties:
> + reg:
> + minItems: 6
> + reg-names:
> + minItems: 6
> +
> + - if:
> + properties:
> + compatible:
> + contains:
> + const: qcom,eliza-pcie
[Severity: Low]
Is there a reason to use four separate conditional blocks for the exact same
compatible string? Could the properties for reg, clocks, interrupts, and
resets be consolidated under the then clause of a single if block to reduce
verbosity and schema complexity?
> + then:
> + properties:
> + clocks:
> + minItems: 8
> + maxItems: 8
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260608-eliza-v3-0-9bdeb7434b28@oss.qualcomm.com?part=2
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* [PATCH v3 3/3] phy: qcom: qmp-pcie: Add QMP PCIe PHY support for Eliza
From: Krishna Chaitanya Chundru @ 2026-06-08 8:48 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Lorenzo Pieralisi, Krzysztof Wilczyński,
Manivannan Sadhasivam, Bjorn Helgaas, Bjorn Andersson
Cc: linux-arm-msm, linux-phy, devicetree, linux-kernel, linux-pci,
Krishna Chaitanya Chundru, Dmitry Baryshkov
In-Reply-To: <20260608-eliza-v3-0-9bdeb7434b28@oss.qualcomm.com>
Add QMP PCIe PHY support for the Eliza SoC. Introduce a new Gen3x1 PHY
configuration with Eliza-specific initialization tables, and reuse the
existing sm8550 Gen3x2 configuration for the Gen3x2 PHY instance.
Also add the missing QPHY_PCIE_V6_PCS_PCIE_INT_AUX_CLK_CONFIG1 register
definition to the PCIe V6 PCS header.
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 139 ++++++++++++++++++++++++
drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v6.h | 1 +
2 files changed, 140 insertions(+)
diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
index fed2fc9bb311..257b4df965c3 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
@@ -198,6 +198,112 @@ static const struct qmp_phy_init_tbl msm8998_pcie_pcs_tbl[] = {
QMP_PHY_INIT_CFG(QPHY_V3_PCS_SIGDET_CNTRL, 0x03),
};
+static const struct qmp_phy_init_tbl eliza_qmp_gen3x1_pcie_serdes_tbl[] = {
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_SSC_STEP_SIZE1_MODE1, 0x93),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_SSC_STEP_SIZE2_MODE1, 0x01),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_CP_CTRL_MODE1, 0x02),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_PLL_RCTRL_MODE1, 0x16),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_PLL_CCTRL_MODE1, 0x36),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_CORECLK_DIV_MODE1, 0x04),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_LOCK_CMP1_MODE1, 0x0a),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_LOCK_CMP2_MODE1, 0x1a),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_DEC_START_MODE1, 0x34),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_DIV_FRAC_START1_MODE1, 0x55),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_DIV_FRAC_START2_MODE1, 0x55),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_DIV_FRAC_START3_MODE1, 0x01),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_HSCLK_SEL_1, 0x01),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_VCO_TUNE1_MODE1, 0xb4),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_VCO_TUNE2_MODE1, 0x03),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_SSC_STEP_SIZE1_MODE0, 0xf8),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_SSC_STEP_SIZE2_MODE0, 0x01),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_CP_CTRL_MODE0, 0x02),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_PLL_RCTRL_MODE0, 0x16),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_PLL_CCTRL_MODE0, 0x36),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_LOCK_CMP1_MODE0, 0x04),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_LOCK_CMP2_MODE0, 0x0d),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_DEC_START_MODE0, 0x41),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_DIV_FRAC_START1_MODE0, 0xab),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_DIV_FRAC_START2_MODE0, 0xaa),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_DIV_FRAC_START3_MODE0, 0x01),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_VCO_TUNE1_MODE0, 0x24),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_BG_TIMER, 0x0a),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_SSC_EN_CENTER, 0x01),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_SSC_PER1, 0x62),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_SSC_PER2, 0x02),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_CLK_ENABLE1, 0x90),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_SYS_CLK_CTRL, 0x82),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_PLL_IVCO, 0x07),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_SYSCLK_EN_SEL, 0x08),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_LOCK_CMP_EN, 0x42),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_VCO_TUNE_MAP, 0x14),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_CLK_SELECT, 0x34),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_CORE_CLK_EN, 0xa0),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_CMN_CONFIG_1, 0x16),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_ADDITIONAL_MISC_3, 0x0f),
+};
+
+static const struct qmp_phy_init_tbl eliza_qmp_gen3x1_pcie_pcs_tbl[] = {
+ QMP_PHY_INIT_CFG(QPHY_V6_PCS_REFGEN_REQ_CONFIG1, 0x05),
+ QMP_PHY_INIT_CFG(QPHY_V6_PCS_G3S2_PRE_GAIN, 0x2e),
+ QMP_PHY_INIT_CFG(QPHY_V6_PCS_RX_SIGDET_LVL, 0x77),
+ QMP_PHY_INIT_CFG(QPHY_V6_PCS_RATE_SLEW_CNTRL1, 0x0b),
+ QMP_PHY_INIT_CFG(QPHY_V6_PCS_PCS_TX_RX_CONFIG, 0x0c),
+ QMP_PHY_INIT_CFG(QPHY_V6_PCS_EQ_CONFIG2, 0x0f),
+};
+
+static const struct qmp_phy_init_tbl eliza_qmp_gen3x1_pcie_misc_pcs_tbl[] = {
+ QMP_PHY_INIT_CFG(QPHY_PCIE_V6_PCS_PCIE_POWER_STATE_CONFIG2, 0x1d),
+ QMP_PHY_INIT_CFG(QPHY_PCIE_V6_PCS_PCIE_ENDPOINT_REFCLK_DRIVE, 0xc1),
+ QMP_PHY_INIT_CFG(QPHY_PCIE_V6_PCS_PCIE_INT_AUX_CLK_CONFIG1, 0x00),
+ QMP_PHY_INIT_CFG(QPHY_PCIE_V6_PCS_PCIE_OSC_DTCT_ACTIONS, 0x00),
+ QMP_PHY_INIT_CFG(QPHY_PCIE_V6_PCS_PCIE_RXEQEVAL_TIME, 0x27),
+};
+
+static const struct qmp_phy_init_tbl eliza_qmp_gen3x1_pcie_tx_tbl[] = {
+ QMP_PHY_INIT_CFG(QSERDES_V6_TX_RES_CODE_LANE_OFFSET_TX, 0x17),
+ QMP_PHY_INIT_CFG(QSERDES_V6_TX_RES_CODE_LANE_OFFSET_RX, 0x06),
+ QMP_PHY_INIT_CFG(QSERDES_V6_TX_LANE_MODE_1, 0x15),
+ QMP_PHY_INIT_CFG(QSERDES_V6_TX_LANE_MODE_4, 0x3f),
+ QMP_PHY_INIT_CFG(QSERDES_V6_TX_RCV_DETECT_LVL_2, 0x12),
+ QMP_PHY_INIT_CFG(QSERDES_V6_TX_PI_QEC_CTRL, 0x02),
+};
+
+static const struct qmp_phy_init_tbl eliza_qmp_gen3x1_pcie_rx_tbl[] = {
+ QMP_PHY_INIT_CFG(QSERDES_V6_RX_UCDR_FO_GAIN, 0x09),
+ QMP_PHY_INIT_CFG(QSERDES_V6_RX_UCDR_SO_GAIN, 0x05),
+ QMP_PHY_INIT_CFG(QSERDES_V6_RX_UCDR_SO_SATURATION_AND_ENABLE, 0x7f),
+ QMP_PHY_INIT_CFG(QSERDES_V6_RX_UCDR_PI_CONTROLS, 0xf0),
+ QMP_PHY_INIT_CFG(QSERDES_V6_RX_UCDR_SB2_THRESH1, 0x08),
+ QMP_PHY_INIT_CFG(QSERDES_V6_RX_UCDR_SB2_THRESH2, 0x08),
+ QMP_PHY_INIT_CFG(QSERDES_V6_RX_AUX_DATA_TCOARSE_TFINE, 0x30),
+ QMP_PHY_INIT_CFG(QSERDES_V6_RX_TX_ADAPT_POST_THRESH, 0xf0),
+ QMP_PHY_INIT_CFG(QSERDES_V6_RX_VGA_CAL_CNTRL1, 0x04),
+ QMP_PHY_INIT_CFG(QSERDES_V6_RX_VGA_CAL_CNTRL2, 0x0f),
+ QMP_PHY_INIT_CFG(QSERDES_V6_RX_GM_CAL, 0x0d),
+ QMP_PHY_INIT_CFG(QSERDES_V6_RX_RX_EQU_ADAPTOR_CNTRL2, 0x0e),
+ QMP_PHY_INIT_CFG(QSERDES_V6_RX_RX_EQU_ADAPTOR_CNTRL3, 0x4a),
+ QMP_PHY_INIT_CFG(QSERDES_V6_RX_RX_EQU_ADAPTOR_CNTRL4, 0x0a),
+ QMP_PHY_INIT_CFG(QSERDES_V6_RX_RX_IDAC_TSETTLE_LOW, 0x07),
+ QMP_PHY_INIT_CFG(QSERDES_V6_RX_RX_EQ_OFFSET_ADAPTOR_CNTRL1, 0x14),
+ QMP_PHY_INIT_CFG(QSERDES_V6_RX_SIDGET_ENABLES, 0x0c),
+ QMP_PHY_INIT_CFG(QSERDES_V6_RX_RX_MODE_00_LOW, 0x3f),
+ QMP_PHY_INIT_CFG(QSERDES_V6_RX_RX_MODE_00_HIGH, 0xbf),
+ QMP_PHY_INIT_CFG(QSERDES_V6_RX_RX_MODE_00_HIGH2, 0xbf),
+ QMP_PHY_INIT_CFG(QSERDES_V6_RX_RX_MODE_00_HIGH3, 0xb7),
+ QMP_PHY_INIT_CFG(QSERDES_V6_RX_RX_MODE_00_HIGH4, 0xea),
+ QMP_PHY_INIT_CFG(QSERDES_V6_RX_RX_MODE_01_LOW, 0xdc),
+ QMP_PHY_INIT_CFG(QSERDES_V6_RX_RX_MODE_01_HIGH, 0x5c),
+ QMP_PHY_INIT_CFG(QSERDES_V6_RX_RX_MODE_01_HIGH2, 0x9c),
+ QMP_PHY_INIT_CFG(QSERDES_V6_RX_RX_MODE_01_HIGH3, 0x1a),
+ QMP_PHY_INIT_CFG(QSERDES_V6_RX_RX_MODE_01_HIGH4, 0x89),
+ QMP_PHY_INIT_CFG(QSERDES_V6_RX_RX_MODE_10_HIGH, 0x94),
+ QMP_PHY_INIT_CFG(QSERDES_V6_RX_RX_MODE_10_HIGH2, 0x5b),
+ QMP_PHY_INIT_CFG(QSERDES_V6_RX_RX_MODE_10_HIGH3, 0x1a),
+ QMP_PHY_INIT_CFG(QSERDES_V6_RX_RX_MODE_10_HIGH4, 0x89),
+ QMP_PHY_INIT_CFG(QSERDES_V6_RX_DFE_CTLE_POST_CAL_OFFSET, 0x38),
+ QMP_PHY_INIT_CFG(QSERDES_V6_RX_SIGDET_CAL_TRIM, 0x08),
+};
+
static const struct qmp_phy_init_tbl ipq6018_pcie_serdes_tbl[] = {
QMP_PHY_INIT_CFG(QSERDES_PLL_SSC_PER1, 0x7d),
QMP_PHY_INIT_CFG(QSERDES_PLL_SSC_PER2, 0x01),
@@ -3532,6 +3638,33 @@ static const struct qmp_pcie_offsets qmp_pcie_offsets_v8_50 = {
.txrxz = 0xd000,
};
+static const struct qmp_phy_cfg eliza_qmp_gen3x1_pciephy_cfg = {
+ .lanes = 1,
+
+ .offsets = &qmp_pcie_offsets_v5,
+
+ .tbls = {
+ .serdes = eliza_qmp_gen3x1_pcie_serdes_tbl,
+ .serdes_num = ARRAY_SIZE(eliza_qmp_gen3x1_pcie_serdes_tbl),
+ .tx = eliza_qmp_gen3x1_pcie_tx_tbl,
+ .tx_num = ARRAY_SIZE(eliza_qmp_gen3x1_pcie_tx_tbl),
+ .rx = eliza_qmp_gen3x1_pcie_rx_tbl,
+ .rx_num = ARRAY_SIZE(eliza_qmp_gen3x1_pcie_rx_tbl),
+ .pcs = eliza_qmp_gen3x1_pcie_pcs_tbl,
+ .pcs_num = ARRAY_SIZE(eliza_qmp_gen3x1_pcie_pcs_tbl),
+ .pcs_misc = eliza_qmp_gen3x1_pcie_misc_pcs_tbl,
+ .pcs_misc_num = ARRAY_SIZE(eliza_qmp_gen3x1_pcie_misc_pcs_tbl),
+ },
+ .reset_list = sdm845_pciephy_reset_l,
+ .num_resets = ARRAY_SIZE(sdm845_pciephy_reset_l),
+ .vreg_list = qmp_phy_vreg_l,
+ .num_vregs = ARRAY_SIZE(qmp_phy_vreg_l),
+ .regs = pciephy_v6_regs_layout,
+
+ .pwrdn_ctrl = SW_PWRDN | REFCLK_DRV_DSBL,
+ .phy_status = PHYSTATUS,
+};
+
static const struct qmp_phy_cfg ipq8074_pciephy_cfg = {
.lanes = 1,
@@ -5399,6 +5532,12 @@ static int qmp_pcie_probe(struct platform_device *pdev)
static const struct of_device_id qmp_pcie_of_match_table[] = {
{
+ .compatible = "qcom,eliza-qmp-gen3x1-pcie-phy",
+ .data = &eliza_qmp_gen3x1_pciephy_cfg,
+ }, {
+ .compatible = "qcom,eliza-qmp-gen3x2-pcie-phy",
+ .data = &sm8550_qmp_gen3x2_pciephy_cfg,
+ }, {
.compatible = "qcom,glymur-qmp-gen4x2-pcie-phy",
.data = &glymur_qmp_gen4x2_pciephy_cfg,
}, {
diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v6.h b/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v6.h
index 45397cb3c0c6..17a0f9d18acf 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v6.h
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v6.h
@@ -12,6 +12,7 @@
#define QPHY_PCIE_V6_PCS_PCIE_POWER_STATE_CONFIG2 0x0c
#define QPHY_PCIE_V6_PCS_PCIE_POWER_STATE_CONFIG4 0x14
#define QPHY_PCIE_V6_PCS_PCIE_ENDPOINT_REFCLK_DRIVE 0x20
+#define QPHY_PCIE_V6_PCS_PCIE_INT_AUX_CLK_CONFIG1 0x54
#define QPHY_PCIE_V6_PCS_PCIE_OSC_DTCT_ACTIONS 0x94
#define QPHY_PCIE_V6_PCS_LANE1_INSIG_SW_CTRL2 0x024
--
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 v3 2/3] dt-bindings: PCI: qcom,pcie-sm8550: Add Eliza compatible
From: Krishna Chaitanya Chundru @ 2026-06-08 8:48 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Lorenzo Pieralisi, Krzysztof Wilczyński,
Manivannan Sadhasivam, Bjorn Helgaas, Bjorn Andersson
Cc: linux-arm-msm, linux-phy, devicetree, linux-kernel, linux-pci,
Krishna Chaitanya Chundru
In-Reply-To: <20260608-eliza-v3-0-9bdeb7434b28@oss.qualcomm.com>
PCIe controller present in Eliza SoC is backwards compatible with the
controller present in SM8550 SoC. Hence, add the compatible with SM8550
fallback.
Eliza requires 6 reg entries, 8 clocks and 9 interrupts, so add the
corresponding allOf constraints.
Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
.../devicetree/bindings/pci/qcom,pcie-sm8550.yaml | 50 ++++++++++++++++++++++
1 file changed, 50 insertions(+)
diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie-sm8550.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie-sm8550.yaml
index 3a94a9c1bb15..fb706b1397a3 100644
--- a/Documentation/devicetree/bindings/pci/qcom,pcie-sm8550.yaml
+++ b/Documentation/devicetree/bindings/pci/qcom,pcie-sm8550.yaml
@@ -20,6 +20,7 @@ properties:
- const: qcom,pcie-sm8550
- items:
- enum:
+ - qcom,eliza-pcie
- qcom,kaanapali-pcie
- qcom,sar2130p-pcie
- qcom,pcie-sm8650
@@ -91,6 +92,55 @@ required:
allOf:
- $ref: qcom,pcie-common.yaml#
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: qcom,eliza-pcie
+ then:
+ properties:
+ reg:
+ minItems: 6
+ reg-names:
+ minItems: 6
+
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: qcom,eliza-pcie
+ then:
+ properties:
+ clocks:
+ minItems: 8
+ maxItems: 8
+ clock-names:
+ minItems: 8
+ maxItems: 8
+
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: qcom,eliza-pcie
+ then:
+ properties:
+ interrupts:
+ minItems: 9
+ interrupt-names:
+ minItems: 9
+
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: qcom,eliza-pcie
+ then:
+ properties:
+ resets:
+ minItems: 2
+ reset-names:
+ minItems: 2
unevaluatedProperties: false
--
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 v3 1/3] dt-bindings: phy: sc8280xp-qmp-pcie: Document Eliza PCIe phy
From: Krishna Chaitanya Chundru @ 2026-06-08 8:48 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Lorenzo Pieralisi, Krzysztof Wilczyński,
Manivannan Sadhasivam, Bjorn Helgaas, Bjorn Andersson
Cc: linux-arm-msm, linux-phy, devicetree, linux-kernel, linux-pci,
Krishna Chaitanya Chundru, Krzysztof Kozlowski
In-Reply-To: <20260608-eliza-v3-0-9bdeb7434b28@oss.qualcomm.com>
Add compatibles for the Eliza PCIe QMP PHY's, which supports Gen3x1 and
Gen3x2 configurations.
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
.../devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml
index 3a35120a77ec..be4bbc327982 100644
--- a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml
@@ -16,6 +16,8 @@ description:
properties:
compatible:
enum:
+ - qcom,eliza-qmp-gen3x1-pcie-phy
+ - qcom,eliza-qmp-gen3x2-pcie-phy
- qcom,glymur-qmp-gen4x2-pcie-phy
- qcom,glymur-qmp-gen5x4-pcie-phy
- qcom,kaanapali-qmp-gen3x2-pcie-phy
@@ -181,6 +183,8 @@ allOf:
compatible:
contains:
enum:
+ - qcom,eliza-qmp-gen3x1-pcie-phy
+ - qcom,eliza-qmp-gen3x2-pcie-phy
- qcom,glymur-qmp-gen4x2-pcie-phy
- qcom,glymur-qmp-gen5x4-pcie-phy
- qcom,qcs8300-qmp-gen4x2-pcie-phy
@@ -206,6 +210,8 @@ allOf:
compatible:
contains:
enum:
+ - qcom,eliza-qmp-gen3x1-pcie-phy
+ - qcom,eliza-qmp-gen3x2-pcie-phy
- qcom,glymur-qmp-gen4x2-pcie-phy
- qcom,glymur-qmp-gen5x4-pcie-phy
- qcom,kaanapali-qmp-gen3x2-pcie-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 v3 0/3] PCI: qcom: Add support for Eliza
From: Krishna Chaitanya Chundru @ 2026-06-08 8:48 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Lorenzo Pieralisi, Krzysztof Wilczyński,
Manivannan Sadhasivam, Bjorn Helgaas, Bjorn Andersson
Cc: linux-arm-msm, linux-phy, devicetree, linux-kernel, linux-pci,
Krishna Chaitanya Chundru, Krzysztof Kozlowski, Dmitry Baryshkov
This series adds PCIe support for the Qualcomm Eliza SoC. Eliza includes
two PCIe root complex controllers capable of 8GT/s x1 and 8GT/s x2.
The QMP PCIe PHY support adds a new Gen3x1 PHY configuration with
Eliza-specific initialization tables, and reuses the existing SM8550
Gen3x2 configuration for the x2 PHY instance.
The series consists of:
- dt-bindings for the Eliza PCIe QMP PHY
- dt-bindings schema for the Eliza PCIe controller
- QMP PCIe PHY initialization tables and configuration for Eliza
Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
Changes in v3:
- Add allOf constraints for eliza (Krzysztof)
- Link to v2: https://patch.msgid.link/20260601-eliza-v2-0-6b44c9c23d5e@oss.qualcomm.com
Changes in v2:
- Remove the driver patch and instead use compatible of sm8550 just like
kaanapali (Krzysztof)
- Move all the phy settings to lowercase (Dimitry).
- Link to v1: https://patch.msgid.link/20260521-eliza-v1-0-97cdbe88389d@oss.qualcomm.com
---
Krishna Chaitanya Chundru (3):
dt-bindings: phy: sc8280xp-qmp-pcie: Document Eliza PCIe phy
dt-bindings: PCI: qcom,pcie-sm8550: Add Eliza compatible
phy: qcom: qmp-pcie: Add QMP PCIe PHY support for Eliza
.../devicetree/bindings/pci/qcom,pcie-sm8550.yaml | 50 ++++++++
.../bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml | 6 +
drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 139 +++++++++++++++++++++
drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v6.h | 1 +
4 files changed, 196 insertions(+)
---
base-commit: 254f49634ee16a731174d2ae34bc50bd5f45e731
change-id: 20260427-eliza-e53155ae8821
Best regards,
--
Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH 3/5] phy: rockchip: phy-rockchip-typec: Add DRM AUX bridge
From: sashiko-bot @ 2026-06-08 7:24 UTC (permalink / raw)
To: Chaoyi Chen; +Cc: neil.armstrong, dri-devel, linux-phy, vkoul, olteanv
In-Reply-To: <20260608070805.88-4-kernel@airkyi.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Double-free of device node `child_np` due to missing `of_node_get()`
- [Medium] Logic error and potential error masking due to incorrect check ordering
--
commit 35100033d8cc196dcdf5880de55b8c3780fffd11
Author: Chaoyi Chen <chaoyi.chen@rock-chips.com>
Subject: phy: rockchip: phy-rockchip-typec: Add DRM AUX bridge
This commit updates the Rockchip Type-C PHY driver to use the DRM_AUX_BRIDGE
helper. It registers a transparent DRM bridge device for the DP port defined
in the device tree.
> diff --git a/drivers/phy/rockchip/phy-rockchip-typec.c b/drivers/phy/rockchip/phy-rockchip-typec.c
> --- a/drivers/phy/rockchip/phy-rockchip-typec.c
> +++ b/drivers/phy/rockchip/phy-rockchip-typec.c
[ ... ]
> @@ -1162,16 +1163,24 @@ static int rockchip_typec_phy_probe(struct platform_device *pdev)
>
> for_each_available_child_of_node(np, child_np) {
> struct phy *phy;
> + ret = 0;
>
> - if (of_node_name_eq(child_np, "dp-port"))
> + if (of_node_name_eq(child_np, "dp-port")) {
> phy = devm_phy_create(dev, child_np,
> &rockchip_dp_phy_ops);
> - else if (of_node_name_eq(child_np, "usb3-port"))
> + ret = drm_aux_bridge_register_from_node(dev, child_np);
[Severity: High]
Does this cause a double-free of the child_np device node?
The loop macro for_each_available_child_of_node() provides a temporary
reference to child_np that is automatically released when the loop advances.
However, drm_aux_bridge_register_from_node() passes this node to
device_set_node() which assigns the node without incrementing its reference
count.
When the auxiliary bridge is later unregistered, drm_aux_bridge_release()
unconditionally calls of_node_put() on this node:
drivers/gpu/drm/bridge/aux-bridge.c:drm_aux_bridge_release() {
...
of_node_put(dev->of_node);
...
}
Because an additional reference was never acquired, this can cause a reference
count underflow and a use-after-free of the device node structure.
Should we call of_node_get(child_np) before passing it to
drm_aux_bridge_register_from_node()?
> + } else if (of_node_name_eq(child_np, "usb3-port"))
> phy = devm_phy_create(dev, child_np,
> &rockchip_usb3_phy_ops);
> else
> continue;
>
> + if (ret) {
> + pm_runtime_disable(dev);
> + of_node_put(child_np);
> + return ret;
> + }
> +
[Severity: Medium]
If devm_phy_create() fails, will this error be masked if
drm_aux_bridge_register_from_node() also fails?
If devm_phy_create() returns an ERR_PTR, but drm_aux_bridge_register_from_node()
fails and returns a non-zero value, the error check here captures
the bridge error instead of the PHY error.
This would return an incorrect error code and skip the "failed to create phy"
debug log entirely. Also, if the PHY fails but the bridge succeeds, the code
registers an unnecessary auxiliary device only for devres to immediately tear
it down.
Would it be better to check IS_ERR(phy) immediately after devm_phy_create()
and before registering the bridge?
> if (IS_ERR(phy)) {
> dev_err(dev, "failed to create phy: %pOFn\n",
> child_np);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260608070805.88-1-kernel@airkyi.com?part=3
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH 4/5] drm/rockchip: cdn-dp: Support handle lane info without extcon
From: sashiko-bot @ 2026-06-08 7:22 UTC (permalink / raw)
To: Chaoyi Chen; +Cc: olteanv, neil.armstrong, linux-phy, vkoul, dri-devel
In-Reply-To: <20260608070805.88-5-kernel@airkyi.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Incorrect extcon pointer validation logic skips valid extcon devices
--
commit a9e1ff8879a685d4cf4ae0348e11004c03d1dc46
Author: Chaoyi Chen <chaoyi.chen@rock-chips.com>
drm/rockchip: cdn-dp: Support handle lane info without extcon
This commit adds support for getting PHY lane info without the help of extcon
when the Type-C controller is present. It updates multiple functions to verify
the extcon device pointer before using it.
> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.c b/drivers/gpu/drm/rockchip/cdn-dp-core.c
> index 177e30445ee83..9068118859e21 100644
> --- a/drivers/gpu/drm/rockchip/cdn-dp-core.c
> +++ b/drivers/gpu/drm/rockchip/cdn-dp-core.c
[ ... ]
> @@ -1121,14 +1130,14 @@ static int cdn_dp_probe(struct platform_device *pdev)
> PTR_ERR(phy) == -EPROBE_DEFER)
> return -EPROBE_DEFER;
>
> - if (IS_ERR(extcon) || IS_ERR(phy))
> + if (IS_ERR(phy) || PTR_ERR(extcon) != -ENODEV)
> continue;
[Severity: High]
Does this logic skip valid extcon devices?
When extcon_get_edev_by_phandle() returns a valid pointer, PTR_ERR(extcon)
evaluates to the pointer's memory address, which will not equal -ENODEV.
This causes the condition to evaluate to true, executing the continue
statement and bypassing initialization for boards that rely on a valid
extcon device.
>
> port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
> if (!port)
> return -ENOMEM;
>
> - port->extcon = extcon;
> + port->extcon = IS_ERR(extcon) ? NULL : extcon;
> port->phy = phy;
> port->dp = dp;
> port->id = i;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260608070805.88-1-kernel@airkyi.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 5/5] drm/rockchip: cdn-dp: Add multiple bridges to support PHY port selection
From: sashiko-bot @ 2026-06-08 7:22 UTC (permalink / raw)
To: Chaoyi Chen; +Cc: linux-phy, dri-devel, olteanv, vkoul, neil.armstrong
In-Reply-To: <20260608070805.88-6-kernel@airkyi.com>
Thank you for your contribution! Sashiko AI review found 11 potential issue(s) to consider:
New issues:
- [High] `cdn_dp_connected_port()` unconditionally returns the first port on extcon systems, breaking hotplug detection.
- [High] Dangling pointers in `dp->next_bridge_list` cause double-free and use-after-free on bind retry or unbind.
- [High] Mismatch between `dp->ports` and `dp->bridge_count` causes uninitialized stack reads and NULL pointer dereferences.
- [High] Buffer overflow when parsing DT endpoints in `cdn_dp_parse_next_bridge_dt()`.
- [High] Memory and resource leaks due to devres (`devm_`) allocations in the component bind callback.
- [High] Hardware multiplexing logic error allows multiple encoders to be enabled simultaneously, corrupting hardware state.
- [High] Double-free / UAF of cached EDIDs upon unbind/bind cycles.
- [High] `cdn_dp_switch_port()` unconditionally attempts video configuration during an EDID probe, aborting the port switch.
- [Medium] Data race on multiple bridge connection state variables.
Pre-existing issues:
- [High] Notifier chain corruption via `devm_extcon_register_notifier()` inside the component bind callback.
- [High] Use-After-Free via uncancelled extcon notifiers after component unbind.
--
commit 8f73288a10a1e2cd66218daa8be78785997dc0e3
Author: Chaoyi Chen <chaoyi.chen@rock-chips.com>
Subject: drm/rockchip: cdn-dp: Add multiple bridges to support PHY port selection
This commit adds support for multiple bridges to allow flexible selection
of the output port on RK3399 CDN-DP controllers. It registers a separate
encoder and bridge for each PHY port and parses DT connections to map
them properly.
> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.c b/drivers/gpu/drm/rockchip/cdn-dp-core.c
> index 9068118859e21..f2eaad61ba440 100644
> --- a/drivers/gpu/drm/rockchip/cdn-dp-core.c
> +++ b/drivers/gpu/drm/rockchip/cdn-dp-core.c
[ ... ]
> @@ -192,14 +193,27 @@ static int cdn_dp_get_sink_count(struct cdn_dp_device *dp, u8 *sink_count)
> static struct cdn_dp_port *cdn_dp_connected_port(struct cdn_dp_device *dp)
> {
> struct cdn_dp_port *port;
> - int i, lanes;
> + int i, lanes[MAX_PHY];
>
> for (i = 0; i < dp->ports; i++) {
> port = dp->port[i];
> - lanes = cdn_dp_get_port_lanes(port);
> - if (lanes)
> + lanes[i] = cdn_dp_get_port_lanes(port);
> + if (!dp->next_bridge_valid)
> return port;
[Severity: High]
Does this unconditionally return the first port on extcon systems?
In cdn_dp_connected_port(), the loop evaluates if (!dp->next_bridge_valid)
and returns the port immediately on the first iteration. It completely
ignores the lanes connection check that was present in the old code. For
systems relying on extcon for hotplug detection, this might cause the
driver to always report the first port as connected to userspace, even
when nothing is plugged in.
> }
> +
> + if (dp->next_bridge_valid) {
> + /* If more than one port is available, pick the last active port */
> + if (dp->active_port >= 0 && lanes[dp->active_port])
> + return dp->port[dp->active_port];
> +
> + /* If the last active port is not available, pick an available port in order */
> + for (i = 0; i < dp->bridge_count; i++) {
> + if (lanes[i])
> + return dp->port[i];
> + }
> + }
> +
> return NULL;
> }
[Severity: High]
Can this loop read uninitialized memory from the lanes array?
The lanes array is only populated up to dp->ports, but this loop iterates
up to dp->bridge_count. If a device tree defines more endpoints than there
are successfully probed PHYs, dp->bridge_count will exceed dp->ports,
causing out of bounds reads on the stack memory.
> @@ -254,12 +268,45 @@ static const struct drm_edid *
> cdn_dp_bridge_edid_read(struct drm_bridge *bridge, struct drm_connector *connector)
> {
> struct cdn_dp_device *dp = bridge_to_dp(bridge);
> - const struct drm_edid *drm_edid;
> + struct cdn_dp_bridge *dp_bridge = bridge_to_dp_bridge(bridge);
> + struct cdn_dp_port *port = dp->port[dp_bridge->id];
[Severity: High]
Could this dereference a NULL pointer?
If dp->bridge_count exceeds dp->ports due to a PHY probe failure,
dp_bridge->id can be greater than or equal to dp->ports. Indexing dp->port
with dp_bridge->id would return NULL, leading to a kernel panic when
port->phy_enabled is evaluated below.
> + struct cdn_dp_port *prev_port = NULL;
> + const struct drm_edid *drm_edid = NULL;
> + int i, ret;
>
> mutex_lock(&dp->lock);
> +
> + /* More than one port is available */
> + if (dp->bridge_count > 1 && !port->phy_enabled) {
> + for (i = 0; i < dp->bridge_count; i++) {
> + /* Another port already enable */
> + if (dp->bridge_list[i] != dp_bridge && dp->bridge_list[i]->enabled)
> + goto get_cache;
> + /* Find already enabled port */
> + if (dp->port[i]->phy_enabled)
> + prev_port = dp->port[i];
> + }
> +
> + /* Switch to current port */
> + if (prev_port) {
> + ret = cdn_dp_switch_port(dp, prev_port, port);
> + if (ret)
> + goto get_cache;
> + }
> + }
> +
> drm_edid = drm_edid_read_custom(connector, cdn_dp_get_edid_block, dp);
> + /* replace edid cache */
> + if (dp->edid_cache[dp_bridge->id])
> + drm_edid_free(dp->edid_cache[dp_bridge->id]);
> + dp->edid_cache[dp_bridge->id] = drm_edid_dup(drm_edid);
[Severity: High]
Can this cause a double-free on the cached EDID?
In cdn_dp_unbind(), the cached EDIDs are freed but the array pointers
are not set to NULL. Since the dp context structure outlives the unbind and
bind cycle, the next time the component is bound and this EDID read
function is called, it might attempt to free the dangling pointer again.
[ ... ]
> @@ -551,6 +599,50 @@ static bool cdn_dp_check_link_status(struct cdn_dp_device *dp)
> return drm_dp_channel_eq_ok(link_status, min(port->lanes, sink_lanes));
> }
>
> +static int cdn_dp_switch_port(struct cdn_dp_device *dp, struct cdn_dp_port *prev_port,
> + struct cdn_dp_port *port)
> +{
[ ... ]
> + if (!cdn_dp_check_link_status(dp)) {
> + dev_info(dp->dev, "Connected with sink; re-train link\n");
> +
> + ret = cdn_dp_train_link(dp);
> + if (ret) {
> + dev_err(dp->dev, "Training link failed: %d\n", ret);
> + goto out;
> + }
> +
> + ret = cdn_dp_set_video_status(dp, CONTROL_VIDEO_IDLE);
> + if (ret) {
> + dev_err(dp->dev, "Failed to idle video %d\n", ret);
> + goto out;
> + }
> +
> + ret = cdn_dp_config_video(dp);
> + if (ret)
> + dev_err(dp->dev, "Failed to configure video: %d\n", ret);
> + }
> +
> +out:
> + return ret;
> +}
[Severity: High]
Will this video configuration fail and abort the port switch during EDID probing?
When reading an EDID on an inactive port, cdn_dp_bridge_edid_read() calls
cdn_dp_switch_port(). However, there is no display mode assigned yet during
early EDID probing, so dp->mode.clock is 0. Calling cdn_dp_config_video()
here calculates a symbol value of 0, which triggers an arithmetic
validation loop failure returning -EINVAL. This might abort the port switch
entirely.
[ ... ]
> @@ -600,6 +693,9 @@ static void cdn_dp_bridge_atomic_enable(struct drm_bridge *bridge, struct drm_at
>
> mutex_lock(&dp->lock);
>
> + if (dp->next_bridge_valid)
> + dp->active_port = dp_bridge->id;
> +
> ret = cdn_dp_enable(dp);
[Severity: High]
Does this logic allow multiple encoders to be enabled simultaneously and
corrupt hardware state?
If a second encoder is enabled, cdn_dp_bridge_atomic_enable() sets
dp->active_port, but cdn_dp_enable() will exit early because dp->active is
already true. The hardware port switch is never executed. Without an
atomic_check to prevent concurrent commits, the shared hardware controller
might be placed into an invalid state.
[ ... ]
> @@ -639,9 +736,11 @@ static void cdn_dp_bridge_atomic_enable(struct drm_bridge *bridge, struct drm_at
> static void cdn_dp_bridge_atomic_disable(struct drm_bridge *bridge, struct drm_atomic_state *state)
> {
> struct cdn_dp_device *dp = bridge_to_dp(bridge);
> + struct cdn_dp_bridge *dp_bridge = bridge_to_dp_bridge(bridge);
> int ret;
>
> mutex_lock(&dp->lock);
> + dp_bridge->enabled = false;
>
> if (dp->active) {
> ret = cdn_dp_disable(dp);
[Severity: High]
Will this unconditionally disable the shared hardware regardless of other
active encoders?
Because the DP controller is shared, if one encoder is disabled while
another is still active, cdn_dp_disable() turns off the shared hardware
controller. This would instantly crash the stream for the other active
encoder.
[ ... ]
> @@ -828,6 +927,17 @@ static int cdn_dp_audio_mute_stream(struct drm_bridge *bridge,
> return ret;
> }
>
> +static void cdn_dp_bridge_hpd_notify(struct drm_bridge *bridge,
> + struct drm_connector *connector,
> + enum drm_connector_status status)
> +{
> + struct cdn_dp_bridge *dp_bridge = bridge_to_dp_bridge(bridge);
> + struct cdn_dp_device *dp = bridge_to_dp(bridge);
> +
> + dp->bridge_list[dp_bridge->id]->connected = status == connector_status_connected;
> + schedule_work(&dp->event_work);
> +}
[Severity: Medium]
Is this updating connection state without holding the appropriate lock?
The connection flag is modified here without acquiring dp->lock. At the
same time, cdn_dp_pd_event_work() reads these connection states iteratively
while explicitly dropping dp->lock. This might cause a data race when
evaluating compound connection flags for reporting to userspace.
[ ... ]
> @@ -1003,26 +1106,35 @@ static int cdn_dp_bind(struct device *dev, struct device *master, void *data)
[ ... ]
> +static int cdn_dp_parse_next_bridge_dt(struct cdn_dp_device *dp)
> +{
> + struct device_node *np = dp->dev->of_node;
> + struct device_node *port __free(device_node) = of_graph_get_port_by_id(np, 1);
> + struct drm_bridge *bridge;
> + int count = 0;
> + int ret = 0;
> + int i;
[ ... ]
> + /* One endpoint may correspond to one next bridge. */
> + for_each_of_graph_port_endpoint(port, dp_ep) {
> + struct device_node *next_bridge_node __free(device_node) =
> + of_graph_get_remote_port_parent(dp_ep);
> +
> + bridge = of_drm_find_bridge(next_bridge_node);
> + if (!bridge) {
> + ret = -EPROBE_DEFER;
> + goto out;
> + }
> +
> + dp->next_bridge_valid = true;
> + dp->next_bridge_list[count] = drm_bridge_get(bridge);
> + count++;
> + }
[Severity: High]
Can this overflow the next_bridge_list array?
The for_each_of_graph_port_endpoint loop iterates over all endpoints defined
in the device tree and stores each resolved bridge in
dp->next_bridge_list[count]. Since next_bridge_list is a fixed-size array
of MAX_PHY (2), a device tree defining 3 or more endpoints would cause a
buffer overflow, corrupting adjacent driver memory.
[ ... ]
> + for (i = 0; i < dp->bridge_count; i++) {
> + dp_bridge = devm_drm_bridge_alloc(dev, struct cdn_dp_bridge, bridge,
> + &cdn_dp_bridge_funcs);
> + if (IS_ERR(dp_bridge)) {
> + ret = PTR_ERR(dp_bridge);
> + goto out;
> + }
> + dp_bridge->id = i;
> + dp_bridge->parent = dp;
> + if (!dp->next_bridge_valid)
> + dp_bridge->connected = true;
> + dp->bridge_list[i] = dp_bridge;
> + }
[Severity: High]
Does allocating devm resources inside the component bind callback cause memory leaks?
The component framework requires resources allocated in bind() to be freed
in unbind(). The driver uses devm_drm_bridge_alloc() and then calls
devm_drm_bridge_add() inside cdn_bridge_add(). These resources are tied to
the lifetime of the component device, not the bind cycle. Every time the
component is bound, such as after an -EPROBE_DEFER or unbind/rebind, new
bridge memory is allocated and new bridges are registered, leaking memory
and piling up duplicate DRM bridges.
[ ... ]
> for (i = 0; i < dp->ports; i++) {
> port = dp->port[i];
>
> @@ -1045,7 +1252,7 @@ static int cdn_dp_bind(struct device *dev, struct device *master, void *data)
> if (ret) {
> DRM_DEV_ERROR(dev,
> "register EXTCON_DISP_DP notifier err\n");
> - return ret;
> + goto out;
> }
> }
[Severity: High]
This is a pre-existing issue, but does using devm_extcon_register_notifier
in the bind callback corrupt the notifier chain?
Because devm_extcon_register_notifier is managed by device removal, it does
not unregister the notifier in cdn_dp_unbind(). When the component is bound
a second time, the exact same event_nb object is re-registered to the
extcon notifier chain. Adding an already-linked node to a list creates an
infinite cycle, leading to a hard lockup the next time the notifier fires.
> @@ -1054,15 +1261,29 @@ static int cdn_dp_bind(struct device *dev, struct device *master, void *data)
> schedule_work(&dp->event_work);
>
> return 0;
> +
> +out:
> + for (i = 0; i < MAX_PHY; i++)
> + drm_bridge_put(dp->next_bridge_list[i]);
> +
> + return ret;
> }
>
> static void cdn_dp_unbind(struct device *dev, struct device *master, void *data)
> {
> struct cdn_dp_device *dp = dev_get_drvdata(dev);
> - struct drm_encoder *encoder = &dp->encoder.encoder;
> + struct drm_encoder *encoder;
> + int i;
>
> cancel_work_sync(&dp->event_work);
> - encoder->funcs->destroy(encoder);
> + for (i = 0; i < dp->bridge_count; i++) {
> + encoder = &dp->bridge_list[i]->encoder.encoder;
> + encoder->funcs->destroy(encoder);
> + drm_bridge_put(dp->next_bridge_list[i]);
> + }
[Severity: High]
Could this leave dangling pointers in dp->next_bridge_list causing a double-free?
If cdn_dp_bind() encounters an error and jumps to the out block, or when
cdn_dp_unbind() is called, the driver calls drm_bridge_put() on the pointers
but fails to set them to NULL. Since the dp context structure is allocated
in probe() and survives component bind retries, a subsequent bind failure or
unbind will call drm_bridge_put() on the exact same dangling pointers.
[Severity: High]
This isn't a bug introduced by this patch, but does failing to unregister
the notifiers during unbind cause a use-after-free?
The driver correctly calls cancel_work_sync(&dp->event_work), but fails to
unregister the event sources because devm_extcon_register_notifier() is
managed by device removal. If a hotplug event fires between unbind() and
remove(), or if the work executes concurrently, it will call
schedule_work(&dp->event_work) on memory that may have already been freed.
> +
> + for (i = 0; i < MAX_PHY; i++)
> + drm_edid_free(dp->edid_cache[i]);
[Severity: High]
Are these pointers left dangling after being freed?
The cached EDIDs are freed via drm_edid_free() here, but the array indices
are not reset to NULL. The dp context outlives the unbind/bind cycle,
meaning cdn_dp_bridge_edid_read() might later attempt to free these dangling
pointers again.
> pm_runtime_disable(dev);
> if (dp->fw_loaded)
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260608070805.88-1-kernel@airkyi.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/5] drm/bridge: Implement generic USB Type-C DP HPD bridge
From: sashiko-bot @ 2026-06-08 7:22 UTC (permalink / raw)
To: Chaoyi Chen; +Cc: olteanv, vkoul, dri-devel, linux-phy, neil.armstrong
In-Reply-To: <20260608070805.88-2-kernel@airkyi.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Critical] Unbounded memory leak and devres accumulation due to missing BUS_NOTIFY_DEL_DEVICE handler.
- [High] Missing initialization for pre-existing Type-C devices.
- [High] Double bridge registration conflicts with explicit controller driver bridges.
--
commit 8b46d64e255268bcd7c9f7656649f0dfbcb9e277
Author: Chaoyi Chen <chaoyi.chen@rock-chips.com>
Subject: drm/bridge: Implement generic USB Type-C DP HPD bridge
This patch implements a generic DP HPD bridge that monitors Type-C bus
events. When a Type-C port device containing the DP SVID is registered,
it automatically creates an HPD bridge for it without requiring the USB-C
controller driver to implement the feature.
> diff --git a/drivers/gpu/drm/bridge/aux-hpd-typec-dp-bridge.c b/drivers/gpu/drm/bridge/aux-hpd-typec-dp-bridge.c
> new file mode 100644
> index 0000000000000..c09579ff7ab91
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/aux-hpd-typec-dp-bridge.c
> @@ -0,0 +1,54 @@
> +// SPDX-License-Identifier: GPL-2.0+
[ ... ]
> +static int drm_typec_bus_event(struct notifier_block *nb,
> + unsigned long action, void *data)
> +{
> + struct device *dev = (struct device *)data;
> + struct typec_altmode *alt = to_typec_altmode(dev);
> +
> + if (action != BUS_NOTIFY_ADD_DEVICE)
> + return NOTIFY_OK;
[Severity: Critical]
Since this only handles BUS_NOTIFY_ADD_DEVICE, will this leak memory when a
cable is unplugged?
Type-C altmode devices are frequently registered and unregistered during normal
operation. Because there is no BUS_NOTIFY_DEL_DEVICE handler to clean up these
bridges, and the registration uses a devm allocation attached to the parent
controller, won't every plug event leak a new bridge allocation and IDA entry?
> +
> + /*
> + * alt->dev.parent->parent : USB-C controller device
> + * alt->dev.parent : USB-C connector device
> + */
> + if (is_typec_port_altmode(&alt->dev) && alt->svid == USB_TYPEC_DP_SID)
> + drm_dp_hpd_bridge_register(alt->dev.parent->parent,
> + to_of_node(alt->dev.parent->fwnode));
[Severity: High]
Does this unconditionally allocate a new bridge even if the controller driver
already allocated one?
Several Type-C controller drivers already explicitly allocate a DP HPD bridge
for their device nodes during probe. If this notifier registers a second bridge
for the exact same device node, won't this create duplicate bridges?
This could break of_drm_find_bridge(), causing it to return the bridge created
by this generic notifier while the controller driver continues sending events
to its explicitly created bridge, leading to missed HPD events.
> +
> + return NOTIFY_OK;
> +}
> +
> +static struct notifier_block drm_typec_event_nb = {
> + .notifier_call = drm_typec_bus_event,
> +};
> +
> +static void drm_aux_hpd_typec_dp_bridge_module_exit(void)
> +{
> + bus_unregister_notifier(&typec_bus, &drm_typec_event_nb);
> +}
[Severity: Critical]
Does this module exit function leave dangling devres actions?
Unregistering the notifier prevents new events, but without a cleanup routine
here or a device removal handler, all previously accumulated devm actions from
plug events will remain attached to the controller devices indefinitely.
> +
> +static int __init drm_aux_hpd_typec_dp_bridge_module_init(void)
> +{
> + bus_register_notifier(&typec_bus, &drm_typec_event_nb);
> +
> + return 0;
> +}
[Severity: High]
If this module is loaded after the Type-C subsystem has already probed, will
it miss all existing devices?
bus_register_notifier() only invokes the callback for events that occur after
the notifier is registered. Do we need to manually iterate over existing
devices on typec_bus to ensure bridges are created for pre-existing DP
altmodes?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260608070805.88-1-kernel@airkyi.com?part=1
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH 2/5] drm/bridge: aux: Add drm_aux_bridge_register_from_node()
From: sashiko-bot @ 2026-06-08 7:17 UTC (permalink / raw)
To: Chaoyi Chen; +Cc: vkoul, neil.armstrong, linux-phy, dri-devel, olteanv
In-Reply-To: <20260608070805.88-3-kernel@airkyi.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing `of_node_get()` on `np` causes an OF node reference count underflow when the device is unregistered or fails initialization.
--
commit cb36f8cc044a8e4dcecbd3dd967a5ee3d94e60aa
Author: Chaoyi Chen <chaoyi.chen@rock-chips.com>
drm/bridge: aux: Add drm_aux_bridge_register_from_node()
This patch introduces drm_aux_bridge_register_from_node() to allow
registering an aux bridge with a specific device node pointer. It refactors
drm_aux_bridge_register() to call this new function.
> diff --git a/drivers/gpu/drm/bridge/aux-bridge.c b/drivers/gpu/drm/bridge/aux-bridge.c
> --- a/drivers/gpu/drm/bridge/aux-bridge.c
> +++ b/drivers/gpu/drm/bridge/aux-bridge.c
[ ... ]
> @@ -62,7 +63,10 @@ int drm_aux_bridge_register(struct device *parent)
> adev->dev.parent = parent;
> adev->dev.release = drm_aux_bridge_release;
>
> - device_set_of_node_from_dev(&adev->dev, parent);
> + if (np)
> + device_set_node(&adev->dev, of_fwnode_handle(np));
> + else
> + device_set_of_node_from_dev(&adev->dev, parent);
>
> ret = auxiliary_device_init(adev);
> if (ret) {
> of_node_put(adev->dev.of_node);
[Severity: High]
Does this code introduce an OF node refcount underflow when np is provided?
When np is not NULL, device_set_node() is called, which simply assigns the
pointer without acquiring a reference. However, the else branch calls
device_set_of_node_from_dev(), which explicitly calls of_node_get().
If auxiliary_device_init() fails, the error path shown above unconditionally
calls of_node_put(adev->dev.of_node).
Similarly, when the device is destroyed, drm_aux_bridge_release() performs:
drivers/gpu/drm/bridge/aux-bridge.c:drm_aux_bridge_release() {
struct auxiliary_device *adev = to_auxiliary_dev(dev);
of_node_put(dev->of_node);
...
}
Will this lead to an unbalanced of_node_put() dropping a reference that was
never acquired when a specific device node pointer is passed?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260608070805.88-1-kernel@airkyi.com?part=2
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
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