* [PATCH v2 08/12] phy: phy-mtk-dp: Add support for digital and analog calibration
From: AngeloGioacchino Del Regno @ 2026-07-07 14:19 UTC (permalink / raw)
To: chunfeng.yun
Cc: vkoul, neil.armstrong, robh, krzk+dt, conor+dt, matthias.bgg,
angelogioacchino.delregno, chunkuang.hu, p.zabel, justin.yeh,
linux-arm-kernel, linux-mediatek, linux-phy, devicetree,
linux-kernel, dri-devel, kernel
In-Reply-To: <20260707141931.191172-1-angelogioacchino.delregno@collabora.com>
Add support for reading the calibration values from eFuse: if
present, write those - otherwise, rely on the defaults from
SoC-specific data.
This also adds support for writing the calibration values for
the analog part of the PHY.
Note that before this change, only default hardcoded calibration
values were supported for the digital driving parameters.
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
drivers/phy/mediatek/phy-mtk-dp.c | 296 ++++++++++++++++++++++++++++--
1 file changed, 277 insertions(+), 19 deletions(-)
diff --git a/drivers/phy/mediatek/phy-mtk-dp.c b/drivers/phy/mediatek/phy-mtk-dp.c
index 270825f130a9..d0ef8e8f6670 100644
--- a/drivers/phy/mediatek/phy-mtk-dp.c
+++ b/drivers/phy/mediatek/phy-mtk-dp.c
@@ -13,6 +13,7 @@
#include <linux/delay.h>
#include <linux/io.h>
#include <linux/mfd/syscon.h>
+#include <linux/nvmem-consumer.h>
#include <linux/of.h>
#include <linux/phy/phy.h>
#include <linux/platform_device.h>
@@ -20,6 +21,19 @@
#define MTK_DP_PHY_MAX_LANES 4
+/* DP_PHYA_GLB_BIAS_GEN_0 (PHYA - Analog) */
+#define XTP_GLB_BIAS_INT_R_CTRL GENMASK(20, 16)
+
+/* DP_PHYA_GLB_FORCE_CTRL_1 */
+#define CKM_CKTX0_EN_FORCE_MODE BIT(10)
+
+/* DP_PHYA_GLB_DPAUX_TX */
+#define CKM_PT0_CKTX_IMPSEL GENMASK(23, 20)
+
+/* DP_PHYA_LAN_LANE_TX_0 */
+#define XTP_LN_TX_IMPSEL_PMOS GENMASK(15, 12)
+#define XTP_LN_TX_IMPSEL_NMOS GENMASK(19, 16)
+
/* DP_PHYA_GLB_FORCE_CTRL_1 */
#define CKM_CKTX0_EN_FORCE_MODE BIT(10)
@@ -53,11 +67,29 @@
#define PHYD_DP_TX_FORCE_VOLT_SWING_VAL GENMASK(2, 1)
#define PHYD_DP_TX_FORCE_PRE_EMPH_VAL GENMASK(4, 3)
+/*
+ * DRIVING_PARAM_X (PHYD - Digital)
+ *
+ * Driving param registers are split in three sets, all containing settings
+ * for Voltage Swing and Pre-Emphasis for each lane's differential pair.
+ *
+ * All three sets share the same layout, but for different physical signals;
+ * In particular:
+ * [0-2]: LC TX CM (Minus / Negative Edge)
+ * [3-5]: LC TX C (Logic State Change Point)
+ * [6-8]: LC TX CP (Plus / Positive Edge)
+ *
+ * And they contain values for:
+ * [0,3,6]: Swing 0 Pre[0-3]
+ * [1,4,7]: Swing 1 Pre[0-2] and Swing 2 Pre0
+ * [2,5,8]: Swing 2 Pre1 and Swing 3 Pre0
+ */
+#define PHYD_DIG_NUM_DRV_PARA_REGS 9
#define XTP_LN_TX_LCTXC0_SW0_PRE0_DEFAULT BIT(4)
#define XTP_LN_TX_LCTXC0_SW0_PRE1_DEFAULT (BIT(10) | BIT(12))
#define XTP_LN_TX_LCTXC0_SW0_PRE2_DEFAULT GENMASK(20, 19)
#define XTP_LN_TX_LCTXC0_SW0_PRE3_DEFAULT GENMASK(29, 29)
-#define DRIVING_PARAM_3_DEFAULT (XTP_LN_TX_LCTXC0_SW0_PRE0_DEFAULT | \
+#define MT8195_DRIVING_PARAM_3_DEFAULT (XTP_LN_TX_LCTXC0_SW0_PRE0_DEFAULT | \
XTP_LN_TX_LCTXC0_SW0_PRE1_DEFAULT | \
XTP_LN_TX_LCTXC0_SW0_PRE2_DEFAULT | \
XTP_LN_TX_LCTXC0_SW0_PRE3_DEFAULT)
@@ -66,21 +98,21 @@
#define XTP_LN_TX_LCTXC0_SW1_PRE1_DEFAULT GENMASK(12, 9)
#define XTP_LN_TX_LCTXC0_SW1_PRE2_DEFAULT (BIT(18) | BIT(21))
#define XTP_LN_TX_LCTXC0_SW2_PRE0_DEFAULT GENMASK(29, 29)
-#define DRIVING_PARAM_4_DEFAULT (XTP_LN_TX_LCTXC0_SW1_PRE0_DEFAULT | \
+#define MT8195_DRIVING_PARAM_4_DEFAULT (XTP_LN_TX_LCTXC0_SW1_PRE0_DEFAULT | \
XTP_LN_TX_LCTXC0_SW1_PRE1_DEFAULT | \
XTP_LN_TX_LCTXC0_SW1_PRE2_DEFAULT | \
XTP_LN_TX_LCTXC0_SW2_PRE0_DEFAULT)
#define XTP_LN_TX_LCTXC0_SW2_PRE1_DEFAULT (BIT(3) | BIT(5))
#define XTP_LN_TX_LCTXC0_SW3_PRE0_DEFAULT GENMASK(13, 12)
-#define DRIVING_PARAM_5_DEFAULT (XTP_LN_TX_LCTXC0_SW2_PRE1_DEFAULT | \
+#define MT8195_DRIVING_PARAM_5_DEFAULT (XTP_LN_TX_LCTXC0_SW2_PRE1_DEFAULT | \
XTP_LN_TX_LCTXC0_SW3_PRE0_DEFAULT)
#define XTP_LN_TX_LCTXCP1_SW0_PRE0_DEFAULT 0
#define XTP_LN_TX_LCTXCP1_SW0_PRE1_DEFAULT GENMASK(10, 10)
#define XTP_LN_TX_LCTXCP1_SW0_PRE2_DEFAULT GENMASK(19, 19)
#define XTP_LN_TX_LCTXCP1_SW0_PRE3_DEFAULT GENMASK(28, 28)
-#define DRIVING_PARAM_6_DEFAULT (XTP_LN_TX_LCTXCP1_SW0_PRE0_DEFAULT | \
+#define MT8195_DRIVING_PARAM_6_DEFAULT (XTP_LN_TX_LCTXCP1_SW0_PRE0_DEFAULT | \
XTP_LN_TX_LCTXCP1_SW0_PRE1_DEFAULT | \
XTP_LN_TX_LCTXCP1_SW0_PRE2_DEFAULT | \
XTP_LN_TX_LCTXCP1_SW0_PRE3_DEFAULT)
@@ -89,22 +121,30 @@
#define XTP_LN_TX_LCTXCP1_SW1_PRE1_DEFAULT GENMASK(10, 9)
#define XTP_LN_TX_LCTXCP1_SW1_PRE2_DEFAULT GENMASK(19, 18)
#define XTP_LN_TX_LCTXCP1_SW2_PRE0_DEFAULT 0
-#define DRIVING_PARAM_7_DEFAULT (XTP_LN_TX_LCTXCP1_SW1_PRE0_DEFAULT | \
+#define MT8195_DRIVING_PARAM_7_DEFAULT (XTP_LN_TX_LCTXCP1_SW1_PRE0_DEFAULT | \
XTP_LN_TX_LCTXCP1_SW1_PRE1_DEFAULT | \
XTP_LN_TX_LCTXCP1_SW1_PRE2_DEFAULT | \
XTP_LN_TX_LCTXCP1_SW2_PRE0_DEFAULT)
#define XTP_LN_TX_LCTXCP1_SW2_PRE1_DEFAULT GENMASK(3, 3)
#define XTP_LN_TX_LCTXCP1_SW3_PRE0_DEFAULT 0
-#define DRIVING_PARAM_8_DEFAULT (XTP_LN_TX_LCTXCP1_SW2_PRE1_DEFAULT | \
+#define MT8195_DRIVING_PARAM_8_DEFAULT (XTP_LN_TX_LCTXCP1_SW2_PRE1_DEFAULT | \
XTP_LN_TX_LCTXCP1_SW3_PRE0_DEFAULT)
enum mtk_dp_phya_ana_glb_regidx {
+ DP_PHYA_GLB_BIAS_GEN_0,
+ DP_PHYA_GLB_BIAS_GEN_1,
+ DP_PHYA_GLB_DPAUX_TX,
DP_PHYA_GLB_FORCE_CTRL_0,
DP_PHYA_GLB_FORCE_CTRL_1,
DP_PHYA_GLOBAL_MAX
};
+enum mtk_dp_phya_ana_lane_regidx {
+ DP_PHYA_LAN_LANE_TX_0,
+ DP_PHYA_LAN_MAX
+};
+
enum mtk_dp_phyd_dig_lane_regidx {
DP_PHYD_LAN_DRIVING_FORCE,
DP_PHYD_LAN_DRIVING_PARAM_0,
@@ -122,10 +162,17 @@ enum mtk_dp_phyd_dig_glb_regidx {
};
static const u8 mt8195_phy_ana_glb_regs[DP_PHYA_GLOBAL_MAX] = {
+ [DP_PHYA_GLB_BIAS_GEN_0] = 0x0,
+ [DP_PHYA_GLB_BIAS_GEN_1] = 0x4,
+ [DP_PHYA_GLB_DPAUX_TX] = 0x8,
[DP_PHYA_GLB_FORCE_CTRL_0] = 0x30,
[DP_PHYA_GLB_FORCE_CTRL_1] = 0x34,
};
+static const u8 mt8195_phy_ana_lane_regs[DP_PHYA_LAN_MAX] = {
+ [DP_PHYA_LAN_LANE_TX_0] = 0x4,
+};
+
static const u8 mt8195_phy_dig_lane_regs[DP_PHYD_LAN_MAX] = {
[DP_PHYD_LAN_DRIVING_FORCE] = 0x18,
[DP_PHYD_LAN_DRIVING_PARAM_0] = 0x2c,
@@ -140,46 +187,101 @@ static const u8 mt8195_phy_dig_glb_regs[DP_PHYD_GLOBAL_MAX] = {
[DP_PHYD_TX_CTL_0] = 0x44,
};
+/**
+ * struct mtk_dp_phya_imp_sel - Per-Lane Impedance Selection
+ * @pmos: Impedance selection for P-Channel MOSFET
+ * @nmos: Impedance selection for N-Channel MOSFET
+ */
+struct mtk_dp_phya_imp_sel {
+ u8 pmos : 4;
+ u8 nmos : 4;
+};
+
/**
* struct mtk_dp_phy_pdata - Platform data and defaults for MediaTek DP/eDP PHY
* @off_ana_glb: Base offset for dptx_phyd_sifslv_ana_glb
+ * @off_ana_lane: Base offsets for dptx_phyd_sifslv_ana_lan (for each lane)
* @off_dig_glb: Base offset for dptx_phyd_sifslv_dig_glb
* @off_dig_lane: Base offsets for dptx_phyd_sifslv_dig_lan (for each lane)
* @regs_ana_glb: Register (layout) offsets for ana_glb
+ * @regs_ana_lane: Register (layout) offsets for ana_lan
* @regs_dig_glb: Register (layout) offsets for dig_glb
* @regs_dig_lane: Register (layout) offsets for dig_lan
+ * @ana_bias_r: Internal resistance "R" Selection Settings (global)
+ * @ana_cktx_imp: TX Clock Impedance Selection Settings (global)
+ * @ana_lanes_imp: TX Impedance Selection Settings (for all lanes)
+ * @driving_params: Voltage Swing and Pre-Emphasis settings (for all lanes)
*/
struct mtk_dp_phy_pdata {
/* Register offsets */
u16 off_ana_glb;
+ u16 off_ana_lane[MTK_DP_PHY_MAX_LANES];
u16 off_dig_glb;
u16 off_dig_lane[MTK_DP_PHY_MAX_LANES];
/* Register maps */
const u8 *regs_ana_glb;
+ const u8 *regs_ana_lane;
const u8 *regs_dig_glb;
const u8 *regs_dig_lane;
+
+ /* Calibration defaults */
+ u8 ana_bias_r;
+ u8 ana_cktx_imp;
+ struct mtk_dp_phya_imp_sel ana_lanes_imp;
+ u32 driving_params[PHYD_DIG_NUM_DRV_PARA_REGS];
};
struct mtk_dp_phy {
struct device *dev;
struct regmap *regmap;
const struct mtk_dp_phy_pdata *pdata;
+
+ u8 ana_bias_r;
+ u8 ana_cktx_imp;
+ struct mtk_dp_phya_imp_sel ana_impsel[MTK_DP_PHY_MAX_LANES];
};
-static int mtk_dp_phy_init(struct phy *phy)
+static int mtk_dp_phy_set_analog_calibration_params(struct mtk_dp_phy *dp_phy)
+{
+ const struct mtk_dp_phy_pdata *pdata = dp_phy->pdata;
+ const u8 *regs_ana_glb = pdata->regs_ana_glb;
+ const u8 *regs_ana_lane = pdata->regs_ana_lane;
+ int i, ret;
+
+ ret = regmap_update_bits(dp_phy->regmap,
+ pdata->off_ana_glb + regs_ana_glb[DP_PHYA_GLB_BIAS_GEN_0],
+ XTP_GLB_BIAS_INT_R_CTRL,
+ FIELD_PREP(XTP_GLB_BIAS_INT_R_CTRL, dp_phy->ana_bias_r));
+ if (ret)
+ return ret;
+
+ ret = regmap_update_bits(dp_phy->regmap,
+ pdata->off_ana_glb + regs_ana_glb[DP_PHYA_GLB_DPAUX_TX],
+ CKM_PT0_CKTX_IMPSEL,
+ FIELD_PREP(CKM_PT0_CKTX_IMPSEL, dp_phy->ana_cktx_imp));
+ if (ret)
+ return ret;
+
+ for (i = 0; i < MTK_DP_PHY_MAX_LANES; i++) {
+ struct mtk_dp_phya_imp_sel *ana_imp = &dp_phy->ana_impsel[i];
+ u32 val = FIELD_PREP(XTP_LN_TX_IMPSEL_PMOS, ana_imp->pmos) |
+ FIELD_PREP(XTP_LN_TX_IMPSEL_NMOS, ana_imp->nmos);
+ u32 off_ana_lane = pdata->off_ana_lane[i];
+
+ ret = regmap_update_bits(dp_phy->regmap,
+ off_ana_lane + regs_ana_lane[DP_PHYA_LAN_LANE_TX_0],
+ XTP_LN_TX_IMPSEL_PMOS | XTP_LN_TX_IMPSEL_NMOS, val);
+ if (ret)
+ return ret;
+ }
+ return 0;
+}
+
+static int mtk_dp_phy_set_digital_drv_params(struct mtk_dp_phy *dp_phy)
{
- struct mtk_dp_phy *dp_phy = phy_get_drvdata(phy);
const struct mtk_dp_phy_pdata *pdata = dp_phy->pdata;
const u32 reg = pdata->regs_dig_lane[DP_PHYD_LAN_DRIVING_PARAM_0];
- static const u32 driving_params[] = {
- DRIVING_PARAM_3_DEFAULT,
- DRIVING_PARAM_4_DEFAULT,
- DRIVING_PARAM_5_DEFAULT,
- DRIVING_PARAM_6_DEFAULT,
- DRIVING_PARAM_7_DEFAULT,
- DRIVING_PARAM_8_DEFAULT
- };
int i, ret;
/*
@@ -190,11 +292,31 @@ static int mtk_dp_phy_init(struct phy *phy)
for (i = 0; i < MTK_DP_PHY_MAX_LANES; i++) {
ret = regmap_bulk_write(dp_phy->regmap,
pdata->off_dig_lane[i] + reg,
- driving_params,
- ARRAY_SIZE(driving_params));
+ pdata->driving_params,
+ ARRAY_SIZE(pdata->driving_params));
if (ret)
return ret;
};
+ return 0;
+}
+
+static int mtk_dp_phy_init(struct phy *phy)
+{
+ struct mtk_dp_phy *dp_phy = phy_get_drvdata(phy);
+ struct device *dev = &phy->dev;
+ int ret;
+
+ ret = mtk_dp_phy_set_digital_drv_params(dp_phy);
+ if (ret) {
+ dev_err(dev, "Cannot set driving params\n");
+ return ret;
+ }
+
+ ret = mtk_dp_phy_set_analog_calibration_params(dp_phy);
+ if (ret) {
+ dev_err(dev, "Cannot set analog calibration\n");
+ return ret;
+ }
return 0;
}
@@ -407,6 +529,111 @@ static const struct phy_ops mtk_dp_phy_dev_ops = {
.owner = THIS_MODULE,
};
+static void mtk_dp_phy_get_default_cal_data(struct mtk_dp_phy *dp_phy)
+{
+ const struct mtk_dp_phy_pdata *pdata = dp_phy->pdata;
+ int i;
+
+ dp_phy->ana_bias_r = pdata->ana_bias_r;
+ dp_phy->ana_cktx_imp = pdata->ana_cktx_imp;
+
+ /* Copy the default lane impedance settings to all lanes */
+ for (i = 0; i < MTK_DP_PHY_MAX_LANES; i++)
+ memcpy(&dp_phy->ana_impsel[i], &pdata->ana_lanes_imp,
+ sizeof(dp_phy->ana_impsel[0]));
+
+ return;
+}
+
+static int mtk_dp_phy_get_one_cal_para(struct device *dev, const char *name, u8 max_val)
+{
+ u8 buf_byte;
+ u16 buf;
+ int ret;
+
+ /*
+ * All of the calibrations are always max 8 bits long, but some may
+ * be split between two different 8-bits cells: handle this corner
+ * case by retrying reading as u16.
+ */
+ ret = nvmem_cell_read_u8(dev, name, &buf_byte);
+ if (ret)
+ ret = nvmem_cell_read_u16(dev, name, &buf);
+ else
+ buf = buf_byte;
+
+ if (ret) {
+ dev_err(dev, "Cannot get calibration data for %s: %d\n", name, ret);
+ return ret;
+ };
+
+ if (buf == 0) {
+ dev_warn(dev, "No calibration for %s. Using defaults\n", name);
+ return -ENOENT;
+ }
+
+ if (buf > max_val) {
+ dev_err(dev, "Bad value %u retrieved for %s. Returning.\n", buf, name);
+ return -ERANGE;
+ };
+
+ return buf;
+}
+
+static int mtk_dp_phy_get_calibration_data(struct mtk_dp_phy *dp_phy)
+{
+ char mtk_dp_cal_lane_imp_name[] = "impedance-laneXM";
+ struct device *dev = dp_phy->dev;
+ int i, ret;
+
+ ret = mtk_dp_phy_get_one_cal_para(dev, "rbias-trim", FIELD_MAX(XTP_GLB_BIAS_INT_R_CTRL));
+ if (ret < 0)
+ goto end;
+ dp_phy->ana_bias_r = ret;
+
+ ret = mtk_dp_phy_get_one_cal_para(dev, "impedance-txclk", FIELD_MAX(CKM_PT0_CKTX_IMPSEL));
+ if (ret < 0)
+ goto end;
+ dp_phy->ana_cktx_imp = ret;
+
+ /* Get impedance params for each lane */
+ for (i = 0; i < MTK_DP_PHY_MAX_LANES; i++) {
+ /* P-MOSFET first */
+ snprintf(mtk_dp_cal_lane_imp_name, ARRAY_SIZE(mtk_dp_cal_lane_imp_name),
+ "impedance-lane%dp", i);
+ ret = mtk_dp_phy_get_one_cal_para(dev, mtk_dp_cal_lane_imp_name,
+ FIELD_MAX(XTP_LN_TX_IMPSEL_PMOS));
+ if (ret < 0)
+ goto end;
+ dp_phy->ana_impsel[i].pmos = ret;
+
+ /* ...and then N-MOSFET too */
+ snprintf(mtk_dp_cal_lane_imp_name, ARRAY_SIZE(mtk_dp_cal_lane_imp_name),
+ "impedance-lane%dn", i);
+ ret = mtk_dp_phy_get_one_cal_para(dev, mtk_dp_cal_lane_imp_name,
+ FIELD_MAX(XTP_LN_TX_IMPSEL_NMOS));
+ if (ret < 0)
+ goto end;
+ dp_phy->ana_impsel[i].nmos = ret;
+ }
+end:
+ if (ret < 0) {
+ /*
+ * If any of the calibration values is missing, or if there
+ * is no calibration at all in the eFuses, copy the default
+ * one entirely (as partial values shall not be mixed!)
+ */
+ if (ret == -ENOENT) {
+ dev_info(dev, "Using calibration default values\n");
+ mtk_dp_phy_get_default_cal_data(dp_phy);
+ return 0;
+ }
+ return ret;
+ };
+
+ return 0;
+}
+
static void mtk_dp_phy_legacy_remove_lookup(void *data)
{
struct phy *phy = data;
@@ -444,6 +671,8 @@ static int mtk_dp_phy_legacy_probe(struct platform_device *pdev, struct mtk_dp_p
if (ret)
return ret;
+ mtk_dp_phy_get_default_cal_data(dp_phy);
+
return 0;
}
@@ -461,6 +690,7 @@ static int mtk_dp_phy_probe(struct platform_device *pdev)
struct mtk_dp_phy *dp_phy;
void __iomem *base;
struct phy *phy;
+ int ret;
dp_phy = devm_kzalloc(dev, sizeof(*dp_phy), GFP_KERNEL);
if (!dp_phy)
@@ -482,6 +712,15 @@ static int mtk_dp_phy_probe(struct platform_device *pdev)
dp_phy->pdata = device_get_match_data(dev);
+ if (IS_REACHABLE(CONFIG_NVMEM)) {
+ ret = mtk_dp_phy_get_calibration_data(dp_phy);
+ if (ret)
+ return ret;
+ } else {
+ /* Use default calibration data */
+ mtk_dp_phy_get_default_cal_data(dp_phy);
+ }
+
phy = devm_phy_create(dev, NULL, &mtk_dp_phy_dev_ops);
if (IS_ERR(phy))
return dev_err_probe(dev, PTR_ERR(phy),
@@ -501,12 +740,31 @@ static int mtk_dp_phy_probe(struct platform_device *pdev)
}
static const struct mtk_dp_phy_pdata mt8195_dp_phy_data = {
- .off_ana_glb = 0x0,
+ .off_ana_glb = 0,
+ .off_ana_lane = (const u16[]) { 0x100, 0x200, 0x300, 0x400 },
.off_dig_glb = 0x1000,
.off_dig_lane = (const u16[]) { 0x1100, 0x1200, 0x1300, 0x1400 },
.regs_ana_glb = mt8195_phy_ana_glb_regs,
+ .regs_ana_lane = mt8195_phy_ana_lane_regs,
.regs_dig_glb = mt8195_phy_dig_glb_regs,
.regs_dig_lane = mt8195_phy_dig_lane_regs,
+ .ana_bias_r = 15,
+ .ana_cktx_imp = 8,
+ .ana_lanes_imp = {
+ .pmos = 8,
+ .nmos = 8,
+ },
+ .driving_params = (const u32[]) {
+ [0] = 0,
+ [1] = 0,
+ [2] = 0,
+ [3] = MT8195_DRIVING_PARAM_3_DEFAULT,
+ [4] = MT8195_DRIVING_PARAM_4_DEFAULT,
+ [5] = MT8195_DRIVING_PARAM_5_DEFAULT,
+ [6] = MT8195_DRIVING_PARAM_6_DEFAULT,
+ [7] = MT8195_DRIVING_PARAM_7_DEFAULT,
+ [8] = MT8195_DRIVING_PARAM_8_DEFAULT
+ },
};
static const struct of_device_id mtk_dp_phy_of_match[] = {
--
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 v2 04/12] phy: phy-mtk-dp: Migrate register offsets to SoC specific pdata
From: AngeloGioacchino Del Regno @ 2026-07-07 14:19 UTC (permalink / raw)
To: chunfeng.yun
Cc: vkoul, neil.armstrong, robh, krzk+dt, conor+dt, matthias.bgg,
angelogioacchino.delregno, chunkuang.hu, p.zabel, justin.yeh,
linux-arm-kernel, linux-mediatek, linux-phy, devicetree,
linux-kernel, dri-devel, kernel
In-Reply-To: <20260707141931.191172-1-angelogioacchino.delregno@collabora.com>
In preparation for adding support for newer SoCs and for adding
more capabilities to this driver in an efficient manner, migrate
all of the hardcoded register offsets to SoC specific pdata and
assign that for both DT and platform probing.
While at it also cleanup writing the driving parameters to the PHY
by iterating through all lanes with a loop instead.
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
drivers/phy/mediatek/phy-mtk-dp.c | 138 +++++++++++++++++++++++-------
1 file changed, 109 insertions(+), 29 deletions(-)
diff --git a/drivers/phy/mediatek/phy-mtk-dp.c b/drivers/phy/mediatek/phy-mtk-dp.c
index b042ab67ebaf..915f148325c4 100644
--- a/drivers/phy/mediatek/phy-mtk-dp.c
+++ b/drivers/phy/mediatek/phy-mtk-dp.c
@@ -4,6 +4,10 @@
*
* Copyright (c) 2022, BayLibre Inc.
* Copyright (c) 2022, MediaTek Inc.
+ *
+ * Major refactoring
+ * Copyright (c) 2026, Collabora Ltd.
+ * AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
*/
#include <linux/delay.h>
@@ -14,24 +18,25 @@
#include <linux/platform_device.h>
#include <linux/regmap.h>
-#define PHY_OFFSET 0x1000
+#define MTK_DP_PHY_MAX_LANES 4
-#define MTK_DP_PHY_DIG_PLL_CTL_1 (PHY_OFFSET + 0x14)
+/* DP_PHYD_PLL_CTL_1 */
#define TPLL_SSC_EN BIT(3)
-#define MTK_DP_PHY_DIG_BIT_RATE (PHY_OFFSET + 0x3C)
-#define BIT_RATE_RBR 0
-#define BIT_RATE_HBR 1
-#define BIT_RATE_HBR2 2
-#define BIT_RATE_HBR3 3
+/* DP_PHYD_BIT_RATE */
+#define PHYD_DIG_RG_BIT_RATE GENMASK(1, 0)
+# define BIT_RATE_RBR 0
+# define BIT_RATE_HBR 1
+# define BIT_RATE_HBR2 2
+# define BIT_RATE_HBR3 3
-#define MTK_DP_PHY_DIG_SW_RST (PHY_OFFSET + 0x38)
-#define DP_GLB_SW_RST_PHYD BIT(0)
+/* DP_PHYD_SW_RST */
+#define PHYD_DIG_GLB_SW_RST_B GENMASK(7, 0)
+# define DP_GLB_SW_RST_PHYD BIT(0)
+# define DP_GLB_SW_RST_TFIFO_ANA BIT(1)
+# define DP_GLB_SW_RST_XTAL_CLK BIT(2)
+# define DP_GLB_SW_RST_MAIN_LINK BIT(3)
-#define MTK_DP_LANE0_DRIVING_PARAM_3 (PHY_OFFSET + 0x138)
-#define MTK_DP_LANE1_DRIVING_PARAM_3 (PHY_OFFSET + 0x238)
-#define MTK_DP_LANE2_DRIVING_PARAM_3 (PHY_OFFSET + 0x338)
-#define MTK_DP_LANE3_DRIVING_PARAM_3 (PHY_OFFSET + 0x438)
#define XTP_LN_TX_LCTXC0_SW0_PRE0_DEFAULT BIT(4)
#define XTP_LN_TX_LCTXC0_SW0_PRE1_DEFAULT (BIT(10) | BIT(12))
#define XTP_LN_TX_LCTXC0_SW0_PRE2_DEFAULT GENMASK(20, 19)
@@ -78,14 +83,58 @@
#define DRIVING_PARAM_8_DEFAULT (XTP_LN_TX_LCTXCP1_SW2_PRE1_DEFAULT | \
XTP_LN_TX_LCTXCP1_SW3_PRE0_DEFAULT)
+enum mtk_dp_phyd_dig_lane_regidx {
+ DP_PHYD_LAN_DRIVING_PARAM_0,
+ DP_PHYD_LAN_MAX
+};
+
+enum mtk_dp_phyd_dig_glb_regidx {
+ DP_PHYD_PLL_CTL_0,
+ DP_PHYD_PLL_CTL_1,
+ DP_PHYD_SW_RST,
+ DP_PHYD_BIT_RATE,
+ DP_PHYD_GLOBAL_MAX
+};
+
+static const u8 mt8195_phy_dig_lane_regs[DP_PHYD_LAN_MAX] = {
+ [DP_PHYD_LAN_DRIVING_PARAM_0] = 0x2c,
+};
+
+static const u8 mt8195_phy_dig_glb_regs[DP_PHYD_GLOBAL_MAX] = {
+ [DP_PHYD_PLL_CTL_0] = 0x10,
+ [DP_PHYD_PLL_CTL_1] = 0x14,
+ [DP_PHYD_SW_RST] = 0x38,
+ [DP_PHYD_BIT_RATE] = 0x3c,
+};
+
+/**
+ * struct mtk_dp_phy_pdata - Platform data and defaults for MediaTek DP/eDP PHY
+ * @off_dig_glb: Base offset for dptx_phyd_sifslv_dig_glb
+ * @off_dig_lane: Base offsets for dptx_phyd_sifslv_dig_lan (for each lane)
+ * @regs_dig_glb: Register (layout) offsets for dig_glb
+ * @regs_dig_lane: Register (layout) offsets for dig_lan
+ */
+struct mtk_dp_phy_pdata {
+ /* Register offsets */
+ u16 off_dig_glb;
+ u16 off_dig_lane[MTK_DP_PHY_MAX_LANES];
+
+ /* Register maps */
+ const u8 *regs_dig_glb;
+ const u8 *regs_dig_lane;
+};
+
struct mtk_dp_phy {
struct device *dev;
struct regmap *regmap;
+ const struct mtk_dp_phy_pdata *pdata;
};
static int mtk_dp_phy_init(struct phy *phy)
{
struct mtk_dp_phy *dp_phy = phy_get_drvdata(phy);
+ const struct mtk_dp_phy_pdata *pdata = dp_phy->pdata;
+ const u32 reg = pdata->regs_dig_lane[DP_PHYD_LAN_DRIVING_PARAM_0];
static const u32 driving_params[] = {
DRIVING_PARAM_3_DEFAULT,
DRIVING_PARAM_4_DEFAULT,
@@ -94,15 +143,21 @@ static int mtk_dp_phy_init(struct phy *phy)
DRIVING_PARAM_7_DEFAULT,
DRIVING_PARAM_8_DEFAULT
};
-
- regmap_bulk_write(dp_phy->regmap, MTK_DP_LANE0_DRIVING_PARAM_3,
- driving_params, ARRAY_SIZE(driving_params));
- regmap_bulk_write(dp_phy->regmap, MTK_DP_LANE1_DRIVING_PARAM_3,
- driving_params, ARRAY_SIZE(driving_params));
- regmap_bulk_write(dp_phy->regmap, MTK_DP_LANE2_DRIVING_PARAM_3,
- driving_params, ARRAY_SIZE(driving_params));
- regmap_bulk_write(dp_phy->regmap, MTK_DP_LANE3_DRIVING_PARAM_3,
- driving_params, ARRAY_SIZE(driving_params));
+ int i, ret;
+
+ /*
+ * Assume that all lanes need the same driving parameters: this
+ * will bulk write from DRIVING_PARAM_0 to DRIVING_PARAM_8 on
+ * all lanes (a grand total of [9 * num_lanes] 32-bit writes)
+ */
+ for (i = 0; i < MTK_DP_PHY_MAX_LANES; i++) {
+ ret = regmap_bulk_write(dp_phy->regmap,
+ pdata->off_dig_lane[i] + reg,
+ driving_params,
+ ARRAY_SIZE(driving_params));
+ if (ret)
+ return ret;
+ };
return 0;
}
@@ -110,9 +165,12 @@ static int mtk_dp_phy_init(struct phy *phy)
static int mtk_dp_phy_configure(struct phy *phy, union phy_configure_opts *opts)
{
struct mtk_dp_phy *dp_phy = phy_get_drvdata(phy);
+ const struct mtk_dp_phy_pdata *pdata = dp_phy->pdata;
u32 val;
if (opts->dp.set_rate) {
+ const u32 reg_bit_rate = pdata->regs_dig_glb[DP_PHYD_BIT_RATE];
+
switch (opts->dp.link_rate) {
default:
dev_err(&phy->dev,
@@ -132,10 +190,11 @@ static int mtk_dp_phy_configure(struct phy *phy, union phy_configure_opts *opts)
val = BIT_RATE_HBR3;
break;
}
- regmap_write(dp_phy->regmap, MTK_DP_PHY_DIG_BIT_RATE, val);
+ regmap_write(dp_phy->regmap, pdata->off_dig_glb + reg_bit_rate, val);
}
- regmap_update_bits(dp_phy->regmap, MTK_DP_PHY_DIG_PLL_CTL_1,
+ regmap_update_bits(dp_phy->regmap,
+ pdata->off_dig_glb + pdata->regs_dig_glb[DP_PHYD_PLL_CTL_1],
TPLL_SSC_EN, opts->dp.ssc ? TPLL_SSC_EN : 0);
return 0;
@@ -144,12 +203,17 @@ static int mtk_dp_phy_configure(struct phy *phy, union phy_configure_opts *opts)
static int mtk_dp_phy_reset(struct phy *phy)
{
struct mtk_dp_phy *dp_phy = phy_get_drvdata(phy);
+ const struct mtk_dp_phy_pdata *pdata = dp_phy->pdata;
+ const u32 reg_rst = pdata->regs_dig_glb[DP_PHYD_SW_RST];
+
+ /* Clearing bits sets reset state */
+ regmap_clear_bits(dp_phy->regmap, pdata->off_dig_glb + reg_rst, DP_GLB_SW_RST_PHYD);
- regmap_update_bits(dp_phy->regmap, MTK_DP_PHY_DIG_SW_RST,
- DP_GLB_SW_RST_PHYD, 0);
+ /* PHYD needs 50uS to guarantee reset done */
usleep_range(50, 200);
- regmap_update_bits(dp_phy->regmap, MTK_DP_PHY_DIG_SW_RST,
- DP_GLB_SW_RST_PHYD, 1);
+
+ /* Setting bits means go out of reset */
+ regmap_set_bits(dp_phy->regmap, pdata->off_dig_glb + reg_rst, DP_GLB_SW_RST_PHYD);
return 0;
}
@@ -169,12 +233,19 @@ static void mtk_dp_phy_legacy_remove_lookup(void *data)
phy_remove_lookup(phy, "dp", dev_name(dp_phy->dev));
}
+static const struct mtk_dp_phy_pdata mt8195_dp_phy_data;
+
static int mtk_dp_phy_legacy_probe(struct platform_device *pdev, struct mtk_dp_phy *dp_phy)
{
struct device *dev = &pdev->dev;
struct phy *phy;
int ret;
+ /*
+ * If legacy platform driver probe, assume this is MT8195 or compatible
+ * with a devicetree that was not migrated to the new, proper bindings.
+ */
+ dp_phy->pdata = &mt8195_dp_phy_data;
dp_phy->regmap = *(struct regmap **)dev->platform_data;
if (!dp_phy->regmap)
return dev_err_probe(dev, -EINVAL, "No platform data available\n");
@@ -227,6 +298,8 @@ static int mtk_dp_phy_probe(struct platform_device *pdev)
if (IS_ERR(dp_phy->regmap))
return PTR_ERR(dp_phy->regmap);
+ dp_phy->pdata = device_get_match_data(dev);
+
phy = devm_phy_create(dev, NULL, &mtk_dp_phy_dev_ops);
if (IS_ERR(phy))
return dev_err_probe(dev, PTR_ERR(phy),
@@ -245,8 +318,15 @@ static int mtk_dp_phy_probe(struct platform_device *pdev)
return 0;
}
+static const struct mtk_dp_phy_pdata mt8195_dp_phy_data = {
+ .off_dig_glb = 0x1000,
+ .off_dig_lane = (const u16[]) { 0x1100, 0x1200, 0x1300, 0x1400 },
+ .regs_dig_glb = mt8195_phy_dig_glb_regs,
+ .regs_dig_lane = mt8195_phy_dig_lane_regs,
+};
+
static const struct of_device_id mtk_dp_phy_of_match[] = {
- { .compatible = "mediatek,mt8195-dp-phy" },
+ { .compatible = "mediatek,mt8195-dp-phy", .data = &mt8195_dp_phy_data },
{ /* sentinel */ }
};
MODULE_DEVICE_TABLE(of, mtk_dp_phy_of_match);
--
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 v2 05/12] phy: phy-mtk-dp: Implement power_on and power_off PHY callbacks
From: AngeloGioacchino Del Regno @ 2026-07-07 14:19 UTC (permalink / raw)
To: chunfeng.yun
Cc: vkoul, neil.armstrong, robh, krzk+dt, conor+dt, matthias.bgg,
angelogioacchino.delregno, chunkuang.hu, p.zabel, justin.yeh,
linux-arm-kernel, linux-mediatek, linux-phy, devicetree,
linux-kernel, dri-devel, kernel
In-Reply-To: <20260707141931.191172-1-angelogioacchino.delregno@collabora.com>
Add .power_on() and .power_off() callbacks to mtk_dp_phy_dev_ops
to be able to call those with phy_power_on() and phy_power_off()
API in the DisplayPort driver to be able to stop using all of
those hardcoded register writes in that external driver.
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
drivers/phy/mediatek/phy-mtk-dp.c | 82 +++++++++++++++++++++++++++++++
1 file changed, 82 insertions(+)
diff --git a/drivers/phy/mediatek/phy-mtk-dp.c b/drivers/phy/mediatek/phy-mtk-dp.c
index 915f148325c4..0c3f06791613 100644
--- a/drivers/phy/mediatek/phy-mtk-dp.c
+++ b/drivers/phy/mediatek/phy-mtk-dp.c
@@ -20,6 +20,9 @@
#define MTK_DP_PHY_MAX_LANES 4
+/* DP_PHYA_GLB_FORCE_CTRL_1 */
+#define CKM_CKTX0_EN_FORCE_MODE BIT(10)
+
/* DP_PHYD_PLL_CTL_1 */
#define TPLL_SSC_EN BIT(3)
@@ -37,6 +40,11 @@
# define DP_GLB_SW_RST_XTAL_CLK BIT(2)
# define DP_GLB_SW_RST_MAIN_LINK BIT(3)
+/* DP_PHYD_AUX_RX_CTL */
+#define PHYD_DIG_DPAUX_RX_EN BIT(0)
+#define PHYD_DIG_XTP_GLB_CKDET_EN BIT(1)
+#define PHYD_DIG_DPAUX_RX_DEGLITCH_EN BIT(2)
+
#define XTP_LN_TX_LCTXC0_SW0_PRE0_DEFAULT BIT(4)
#define XTP_LN_TX_LCTXC0_SW0_PRE1_DEFAULT (BIT(10) | BIT(12))
#define XTP_LN_TX_LCTXC0_SW0_PRE2_DEFAULT GENMASK(20, 19)
@@ -83,6 +91,12 @@
#define DRIVING_PARAM_8_DEFAULT (XTP_LN_TX_LCTXCP1_SW2_PRE1_DEFAULT | \
XTP_LN_TX_LCTXCP1_SW3_PRE0_DEFAULT)
+enum mtk_dp_phya_ana_glb_regidx {
+ DP_PHYA_GLB_FORCE_CTRL_0,
+ DP_PHYA_GLB_FORCE_CTRL_1,
+ DP_PHYA_GLOBAL_MAX
+};
+
enum mtk_dp_phyd_dig_lane_regidx {
DP_PHYD_LAN_DRIVING_PARAM_0,
DP_PHYD_LAN_MAX
@@ -93,9 +107,15 @@ enum mtk_dp_phyd_dig_glb_regidx {
DP_PHYD_PLL_CTL_1,
DP_PHYD_SW_RST,
DP_PHYD_BIT_RATE,
+ DP_PHYD_AUX_RX_CTL,
DP_PHYD_GLOBAL_MAX
};
+static const u8 mt8195_phy_ana_glb_regs[DP_PHYA_GLOBAL_MAX] = {
+ [DP_PHYA_GLB_FORCE_CTRL_0] = 0x30,
+ [DP_PHYA_GLB_FORCE_CTRL_1] = 0x34,
+};
+
static const u8 mt8195_phy_dig_lane_regs[DP_PHYD_LAN_MAX] = {
[DP_PHYD_LAN_DRIVING_PARAM_0] = 0x2c,
};
@@ -105,21 +125,26 @@ static const u8 mt8195_phy_dig_glb_regs[DP_PHYD_GLOBAL_MAX] = {
[DP_PHYD_PLL_CTL_1] = 0x14,
[DP_PHYD_SW_RST] = 0x38,
[DP_PHYD_BIT_RATE] = 0x3c,
+ [DP_PHYD_AUX_RX_CTL] = 0x40,
};
/**
* struct mtk_dp_phy_pdata - Platform data and defaults for MediaTek DP/eDP PHY
+ * @off_ana_glb: Base offset for dptx_phyd_sifslv_ana_glb
* @off_dig_glb: Base offset for dptx_phyd_sifslv_dig_glb
* @off_dig_lane: Base offsets for dptx_phyd_sifslv_dig_lan (for each lane)
+ * @regs_ana_glb: Register (layout) offsets for ana_glb
* @regs_dig_glb: Register (layout) offsets for dig_glb
* @regs_dig_lane: Register (layout) offsets for dig_lan
*/
struct mtk_dp_phy_pdata {
/* Register offsets */
+ u16 off_ana_glb;
u16 off_dig_glb;
u16 off_dig_lane[MTK_DP_PHY_MAX_LANES];
/* Register maps */
+ const u8 *regs_ana_glb;
const u8 *regs_dig_glb;
const u8 *regs_dig_lane;
};
@@ -200,6 +225,59 @@ static int mtk_dp_phy_configure(struct phy *phy, union phy_configure_opts *opts)
return 0;
}
+static int mtk_dp_phy_power_on(struct phy *phy)
+{
+ struct mtk_dp_phy *dp_phy = phy_get_drvdata(phy);
+ const struct mtk_dp_phy_pdata *pdata = dp_phy->pdata;
+ const u8 *regs_dig = pdata->regs_dig_glb;
+ const u8 *regs_ana = pdata->regs_ana_glb;
+ int ret;
+
+ /* Enable AUX Channel with RX De-Glitch and input clock detection */
+ ret = regmap_write(dp_phy->regmap,
+ pdata->off_dig_glb + regs_dig[DP_PHYD_AUX_RX_CTL],
+ PHYD_DIG_DPAUX_RX_EN |
+ PHYD_DIG_XTP_GLB_CKDET_EN |
+ PHYD_DIG_DPAUX_RX_DEGLITCH_EN);
+ if (ret)
+ return ret;
+
+ ret = regmap_clear_bits(dp_phy->regmap,
+ pdata->off_ana_glb + regs_ana[DP_PHYA_GLB_FORCE_CTRL_1],
+ CKM_CKTX0_EN_FORCE_MODE);
+ if (ret) {
+ /* Shut down AUX channel */
+ regmap_write(dp_phy->regmap,
+ pdata->off_dig_glb + regs_dig[DP_PHYD_AUX_RX_CTL], 0);
+ return ret;
+ }
+
+ return 0;
+}
+
+static int mtk_dp_phy_power_off(struct phy *phy)
+{
+ struct mtk_dp_phy *dp_phy = phy_get_drvdata(phy);
+ const struct mtk_dp_phy_pdata *pdata = dp_phy->pdata;
+ const u8 *regs_dig = pdata->regs_dig_glb;
+ const u8 *regs_ana = pdata->regs_ana_glb;
+ int ret;
+
+ ret = regmap_set_bits(dp_phy->regmap,
+ pdata->off_ana_glb + regs_ana[DP_PHYA_GLB_FORCE_CTRL_1],
+ CKM_CKTX0_EN_FORCE_MODE);
+ if (ret)
+ return ret;
+
+ /* Disable RX */
+ ret = regmap_write(dp_phy->regmap,
+ pdata->off_dig_glb + regs_dig[DP_PHYD_AUX_RX_CTL], 0);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
static int mtk_dp_phy_reset(struct phy *phy)
{
struct mtk_dp_phy *dp_phy = phy_get_drvdata(phy);
@@ -220,6 +298,8 @@ static int mtk_dp_phy_reset(struct phy *phy)
static const struct phy_ops mtk_dp_phy_dev_ops = {
.init = mtk_dp_phy_init,
+ .power_on = mtk_dp_phy_power_on,
+ .power_off = mtk_dp_phy_power_off,
.configure = mtk_dp_phy_configure,
.reset = mtk_dp_phy_reset,
.owner = THIS_MODULE,
@@ -319,8 +399,10 @@ static int mtk_dp_phy_probe(struct platform_device *pdev)
}
static const struct mtk_dp_phy_pdata mt8195_dp_phy_data = {
+ .off_ana_glb = 0x0,
.off_dig_glb = 0x1000,
.off_dig_lane = (const u16[]) { 0x1100, 0x1200, 0x1300, 0x1400 },
+ .regs_ana_glb = mt8195_phy_ana_glb_regs,
.regs_dig_glb = mt8195_phy_dig_glb_regs,
.regs_dig_lane = mt8195_phy_dig_lane_regs,
};
--
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 v2 06/12] phy: phy-mtk-dp: Support set_lanes in configure and properly cleanup
From: AngeloGioacchino Del Regno @ 2026-07-07 14:19 UTC (permalink / raw)
To: chunfeng.yun
Cc: vkoul, neil.armstrong, robh, krzk+dt, conor+dt, matthias.bgg,
angelogioacchino.delregno, chunkuang.hu, p.zabel, justin.yeh,
linux-arm-kernel, linux-mediatek, linux-phy, devicetree,
linux-kernel, dri-devel, kernel
In-Reply-To: <20260707141931.191172-1-angelogioacchino.delregno@collabora.com>
Add support for enabling a specifically requested number of lanes
in the .configure() callback and disable all lanes in power off
and reset callbacks for proper hardware cleanup.
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
drivers/phy/mediatek/phy-mtk-dp.c | 60 +++++++++++++++++++++++++++++++
1 file changed, 60 insertions(+)
diff --git a/drivers/phy/mediatek/phy-mtk-dp.c b/drivers/phy/mediatek/phy-mtk-dp.c
index 0c3f06791613..2f3ec3fa384d 100644
--- a/drivers/phy/mediatek/phy-mtk-dp.c
+++ b/drivers/phy/mediatek/phy-mtk-dp.c
@@ -45,6 +45,9 @@
#define PHYD_DIG_XTP_GLB_CKDET_EN BIT(1)
#define PHYD_DIG_DPAUX_RX_DEGLITCH_EN BIT(2)
+/* DP_PHYD_TX_CTL_0 */
+#define PHYD_TX_LN_EN GENMASK(7, 4)
+
#define XTP_LN_TX_LCTXC0_SW0_PRE0_DEFAULT BIT(4)
#define XTP_LN_TX_LCTXC0_SW0_PRE1_DEFAULT (BIT(10) | BIT(12))
#define XTP_LN_TX_LCTXC0_SW0_PRE2_DEFAULT GENMASK(20, 19)
@@ -108,6 +111,7 @@ enum mtk_dp_phyd_dig_glb_regidx {
DP_PHYD_SW_RST,
DP_PHYD_BIT_RATE,
DP_PHYD_AUX_RX_CTL,
+ DP_PHYD_TX_CTL_0,
DP_PHYD_GLOBAL_MAX
};
@@ -126,6 +130,7 @@ static const u8 mt8195_phy_dig_glb_regs[DP_PHYD_GLOBAL_MAX] = {
[DP_PHYD_SW_RST] = 0x38,
[DP_PHYD_BIT_RATE] = 0x3c,
[DP_PHYD_AUX_RX_CTL] = 0x40,
+ [DP_PHYD_TX_CTL_0] = 0x44,
};
/**
@@ -192,6 +197,7 @@ static int mtk_dp_phy_configure(struct phy *phy, union phy_configure_opts *opts)
struct mtk_dp_phy *dp_phy = phy_get_drvdata(phy);
const struct mtk_dp_phy_pdata *pdata = dp_phy->pdata;
u32 val;
+ int i;
if (opts->dp.set_rate) {
const u32 reg_bit_rate = pdata->regs_dig_glb[DP_PHYD_BIT_RATE];
@@ -218,6 +224,17 @@ static int mtk_dp_phy_configure(struct phy *phy, union phy_configure_opts *opts)
regmap_write(dp_phy->regmap, pdata->off_dig_glb + reg_bit_rate, val);
}
+ if (opts->dp.set_lanes) {
+ const u32 reg_dig_tx_ctl = pdata->regs_dig_glb[DP_PHYD_TX_CTL_0];
+
+ val = 0;
+ for (i = 0; i < opts->dp.lanes; i++)
+ val |= FIELD_PREP(PHYD_TX_LN_EN, BIT(i));
+
+ regmap_update_bits(dp_phy->regmap, pdata->off_dig_glb + reg_dig_tx_ctl,
+ PHYD_TX_LN_EN, val);
+ }
+
regmap_update_bits(dp_phy->regmap,
pdata->off_dig_glb + pdata->regs_dig_glb[DP_PHYD_PLL_CTL_1],
TPLL_SSC_EN, opts->dp.ssc ? TPLL_SSC_EN : 0);
@@ -255,6 +272,37 @@ static int mtk_dp_phy_power_on(struct phy *phy)
return 0;
}
+static int mtk_dp_phy_disable_all_lanes(struct mtk_dp_phy *dp_phy)
+{
+ const struct mtk_dp_phy_pdata *pdata = dp_phy->pdata;
+ const u8 *regs = pdata->regs_dig_glb;
+ int ret;
+ u32 val;
+
+ ret = regmap_read(dp_phy->regmap, pdata->off_dig_glb + regs[DP_PHYD_TX_CTL_0], &val);
+ if (ret)
+ return ret;
+
+ /* Get mask of currently enabled lane */
+ val = FIELD_GET(PHYD_TX_LN_EN, val);
+ if (val == 0)
+ return 0;
+
+ /* Disable all lanes (needs to be done one by one, from last to first) */
+ do {
+ u32 lane_num = fls(val) - 1;
+ val &= ~BIT(lane_num);
+
+ ret = regmap_clear_bits(dp_phy->regmap,
+ pdata->off_dig_glb + regs[DP_PHYD_TX_CTL_0],
+ FIELD_PREP(PHYD_TX_LN_EN, BIT(lane_num)));
+ if (ret)
+ return ret;
+ } while (val);
+
+ return 0;
+}
+
static int mtk_dp_phy_power_off(struct phy *phy)
{
struct mtk_dp_phy *dp_phy = phy_get_drvdata(phy);
@@ -275,6 +323,12 @@ static int mtk_dp_phy_power_off(struct phy *phy)
if (ret)
return ret;
+ ret = mtk_dp_phy_disable_all_lanes(dp_phy);
+ if (ret) {
+ dev_err(dp_phy->dev, "Could not disable lanes for poweroff!\n");
+ return ret;
+ }
+
return 0;
}
@@ -283,6 +337,7 @@ static int mtk_dp_phy_reset(struct phy *phy)
struct mtk_dp_phy *dp_phy = phy_get_drvdata(phy);
const struct mtk_dp_phy_pdata *pdata = dp_phy->pdata;
const u32 reg_rst = pdata->regs_dig_glb[DP_PHYD_SW_RST];
+ int ret;
/* Clearing bits sets reset state */
regmap_clear_bits(dp_phy->regmap, pdata->off_dig_glb + reg_rst, DP_GLB_SW_RST_PHYD);
@@ -293,6 +348,11 @@ static int mtk_dp_phy_reset(struct phy *phy)
/* Setting bits means go out of reset */
regmap_set_bits(dp_phy->regmap, pdata->off_dig_glb + reg_rst, DP_GLB_SW_RST_PHYD);
+ /* Disable all lanes and continue reset even if this fails, but notify */
+ ret = mtk_dp_phy_disable_all_lanes(dp_phy);
+ if (ret)
+ dev_err(dp_phy->dev, "Could not disable lanes during reset!\n");
+
return 0;
}
--
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 v2 01/12] dt-bindings: phy: Document MT8195 and MT8196 DisplayPort PHYs
From: AngeloGioacchino Del Regno @ 2026-07-07 14:19 UTC (permalink / raw)
To: chunfeng.yun
Cc: vkoul, neil.armstrong, robh, krzk+dt, conor+dt, matthias.bgg,
angelogioacchino.delregno, chunkuang.hu, p.zabel, justin.yeh,
linux-arm-kernel, linux-mediatek, linux-phy, devicetree,
linux-kernel, dri-devel, kernel
In-Reply-To: <20260707141931.191172-1-angelogioacchino.delregno@collabora.com>
This adds bindings for the DisplayPort and Embedded DisplayPort
PHYs found in the MediaTek MT8195 SoC (and variants of) and for
the Embedded DisplayPort found in the MT8196 SoC (and variants).
This PHY supports varying impedance calibrations for the various
signals to reach an optimal EYE signal pattern for any specific
board(s), especially useful for very high bitrates such as HBR3
and higher, depending on board design.
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
.../bindings/phy/mediatek,mt8195-dp-phy.yaml | 77 +++++++++++++++++++
1 file changed, 77 insertions(+)
create mode 100644 Documentation/devicetree/bindings/phy/mediatek,mt8195-dp-phy.yaml
diff --git a/Documentation/devicetree/bindings/phy/mediatek,mt8195-dp-phy.yaml b/Documentation/devicetree/bindings/phy/mediatek,mt8195-dp-phy.yaml
new file mode 100644
index 000000000000..1c2dc4f03cd6
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/mediatek,mt8195-dp-phy.yaml
@@ -0,0 +1,77 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/mediatek,mt8195-dp-phy.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MediaTek SoC DisplayPort Transmitter PHY
+
+maintainers:
+ - AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
+
+properties:
+ compatible:
+ enum:
+ - mediatek,mt8195-dp-phy
+ - mediatek,mt8196-edp-phy
+
+ reg:
+ maxItems: 1
+
+ "#phy-cells":
+ const: 0
+
+ nvmem-cells:
+ description: PHY calibrations from eFuse for optimal EYE signal pattern
+ items:
+ - description: PHY-Global Reference Bias trim
+ - description: PHY-Global AUX Transmitter clock impedance adjustment
+ - description: Lane 0 Transmitter impedance selection (P-MOSFET)
+ - description: Lane 0 Transmitter impedance selection (N-MOSFET)
+ - description: Lane 1 Transmitter impedance selection (P-MOSFET)
+ - description: Lane 1 Transmitter impedance selection (N-MOSFET)
+ - description: Lane 2 Transmitter impedance selection (P-MOSFET)
+ - description: Lane 2 Transmitter impedance selection (N-MOSFET)
+ - description: Lane 3 Transmitter impedance selection (P-MOSFET)
+ - description: Lane 3 Transmitter impedance selection (N-MOSFET)
+
+ nvmem-cell-names:
+ items:
+ - const: rbias-trim
+ - const: impedance-txclk
+ - const: impedance-lane0p
+ - const: impedance-lane0n
+ - const: impedance-lane1p
+ - const: impedance-lane1n
+ - const: impedance-lane2p
+ - const: impedance-lane2n
+ - const: impedance-lane3p
+ - const: impedance-lane3n
+
+ power-domains:
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+ - "#phy-cells"
+
+additionalProperties: false
+
+examples:
+ - |
+ phy@1c500000 {
+ compatible = "mediatek,mt8195-dp-phy";
+ reg = <0x1c500000 0x2000>;
+ #phy-cells = <0>;
+ nvmem-cells = <&edp_glb_bias_trim>, <&edp_clktx_impsel>,
+ <&edp_imp_ln0_pmos>, <&edp_imp_ln0_nmos>,
+ <&edp_imp_ln1_pmos>, <&edp_imp_ln1_nmos>,
+ <&edp_imp_ln2_pmos>, <&edp_imp_ln2_nmos>,
+ <&edp_imp_ln3_pmos>, <&edp_imp_ln3_nmos>;
+ nvmem-cell-names = "rbias-trim", "impedance-txclk",
+ "impedance-lane0p", "impedance-lane0n",
+ "impedance-lane1p", "impedance-lane1n",
+ "impedance-lane2p", "impedance-lane2n",
+ "impedance-lane3p", "impedance-lane3n";
+ };
--
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 v2 00/12] PHY: MediaTek DP PHY refactor and MT8196 eDP
From: AngeloGioacchino Del Regno @ 2026-07-07 14:19 UTC (permalink / raw)
To: chunfeng.yun
Cc: vkoul, neil.armstrong, robh, krzk+dt, conor+dt, matthias.bgg,
angelogioacchino.delregno, chunkuang.hu, p.zabel, justin.yeh,
linux-arm-kernel, linux-mediatek, linux-phy, devicetree,
linux-kernel, dri-devel, kernel
Changes in v2:
- Fixed register offsets for ANA phy in MT8196
- Set/clear FORCE_VOLT_SWING_EN bit when configuring and resetting
the voltage swing registers
- Fixed endianness issue in call to nvmem_read_u16 (Sashiko)
- Added early exit during lanes disablement if none are enabled to
avoid out-of-bounds bitwise shift (Sashiko)
- Removed useless call to pm_runtime_get_sync() in probe(), which
also fixes issues pointed out by Sashiko
- Fixed wrong dig vs ana register array usage in power_on/off (Sashiko)
This series performs major refactoring on the MediaTek DisplayPort PHY
driver, makes it probe with devicetree instead of getting registered
by the mtk_dp DRM driver, adds power_on/off() callbacks, and honors
the phy configure opts' set_lanes and set_voltages for, respectively,
varying the number of lanes and setting the voltage pre-emphasis and
swing on the PHY, for each lane.
This driver now also properly gets the PHY (EYE) Calibration Data from
NVMEM (eFuse array) if provided, instead of getting it (improperly)
injected by the mtk_dp driver.
Additionally, all of the driving parameters calculations and most of
the other register definitions were refactored to greatly enhance the
human readability of this code.
As a last step, this also transfers the register offsets for both the
digital and analog phy registers in arrays assigned to soc specific
data, in an effort to both introduce support for new minor revisions
of the MediaTek DisplayPort PHY and to have a clearer view of the
register related differences between those (for example, it is easily
understandable that the analog part remained exactly the same between
MT8195 and MT8196, but the digital part gets a slight update).
Speaking of which, as a last step, this also adds support for the
MT8196 SoC (and its derivatives), which uses this PHY only for its
Embedded DisplayPort (eDP) IP (spoiler: the DP one seems to be way
too different and requiring an entirely new PHY driver).
In this state, this driver can also easily support the MT8189 SoC
with a few lines of code: even though I do have clean code to add
support for this one, I was not (*yet*) able to test it on upstream
based kernels, and for this reason I decided to leave that one out
for now (but it's coming later for sure).
NOTE!
Despite all the apparently breaking changes in the refactoring, full
compatibility with older MTK_DP driver and with old devicetrees was
retained and carefully tested on multiple platforms!
P.S.: I am aware of the BUILD_DRIVING_PARAM_0( 0, 2, 4, 7) checkpatch
warning and I didn't fix it in bigger favor of human readability.
AngeloGioacchino Del Regno (12):
dt-bindings: phy: Document MT8195 and MT8196 DisplayPort PHYs
phy: phy-mtk-dp: Rename regs to regmap in struct mtk_dp_phy
phy: phy-mtk-dp: Allow probing with devicetree match
phy: phy-mtk-dp: Migrate register offsets to SoC specific pdata
phy: phy-mtk-dp: Implement power_on and power_off PHY callbacks
phy: phy-mtk-dp: Support set_lanes in configure and properly cleanup
phy: phy-mtk-dp: Support setting volt swing and preemphasis values
phy: phy-mtk-dp: Add support for digital and analog calibration
phy: phy-mtk-dp: Rewrite and document default driving param macros
phy: phy-mtk-dp: Add bitrate register val definitions to SoC data
phy: phy-mtk-dp: Add PHYD Lane EN register mask to SoC data
phy: phy-mtk-dp: Add support for MT8196 eDP PHY
.../bindings/phy/mediatek,mt8195-dp-phy.yaml | 77 ++
drivers/phy/mediatek/phy-mtk-dp.c | 863 ++++++++++++++++--
2 files changed, 843 insertions(+), 97 deletions(-)
create mode 100644 Documentation/devicetree/bindings/phy/mediatek,mt8195-dp-phy.yaml
--
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 1/2] phy: mediatek: phy-mtk-hdmi-mt8195: Fix PLL calc divisor overflow
From: AngeloGioacchino Del Regno @ 2026-07-07 14:14 UTC (permalink / raw)
To: sashiko-reviews; +Cc: dri-devel, olteanv, vkoul, linux-phy, neil.armstrong
In-Reply-To: <20260701123106.1B5801F00A3E@smtp.kernel.org>
On 7/1/26 14:31, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] Calculations using `14835 * MEGA`, `5 * GIGA`, and `12 * GIGA` silently overflow 32-bit `unsigned long` types on 32-bit architectures, completely breaking clock tiering and boundary checks.
> --
>
> commit 8179a32cf1dac8f1d080e57bbce79246661b50eb
> Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>
> phy: mediatek: phy-mtk-hdmi-mt8195: Fix PLL calc divisor overflow
>
> This commit fixes an issue where calculating a PLL rate for high display
> resolutions results in a pixel clock exceeding 32 bits, but the digital
> clock divider calculation uses div_u64(), which expects a 32-bit divisor.
> It resolves this by using div64_u64() instead to prevent overflow.
>
>> diff --git a/drivers/phy/mediatek/phy-mtk-hdmi-mt8195.c b/drivers/phy/mediatek/phy-mtk-hdmi-mt8195.c
>> index 1426a2db984d5..e6ee8e0800222 100644
>> --- a/drivers/phy/mediatek/phy-mtk-hdmi-mt8195.c
>> +++ b/drivers/phy/mediatek/phy-mtk-hdmi-mt8195.c
>> @@ -290,7 +290,7 @@ static int mtk_hdmi_pll_calc(struct mtk_hdmi_phy *hdmi_phy, struct clk_hw *hw,
>
> [Severity: High]
> This is a pre-existing issue, but do the constants in this function silently
> overflow on 32-bit architectures?
>
There is no 32-bit architecture compatible with this driver. This will run only
on AArch64, and it's very unlikely to see a 32-bits kernel running on MT8195.
This can be eventually fixed with a later series, but shall not block the proposed
fixes from being picked, both because it's unlikely to happen, and because those
are pre-existing issues.
Besides, the series *is* actually fixing some other overflow issues and was already
tested good.
> Earlier in mtk_hdmi_pll_calc(), there are tier threshold calculations like:
>
> else if ((tmds_clk * 100) >= 14835 * MEGA && (tmds_clk * 10) < 2967 * MEGA)
>
> and later boundary checks:
>
> if (ns_hdmipll_ck >= 5 * GIGA &&
> ns_hdmipll_ck <= 12 * GIGA)
>
> If MEGA and GIGA evaluate as 32-bit types on 32-bit systems, a multiplication
> like 14835 * MEGA evaluates to 14,835,000,000, which wraps around modulo 2^32
> to 1,950,098,112 before being promoted to u64 for the comparison against
> tmds_clk * 100.
>
> Similarly, 5 * GIGA and 12 * GIGA would overflow, potentially rendering the
> valid PLL clock range mathematically unreachable and forcing the function to
> always return -EINVAL on 32-bit builds.
>
> Should these constants be explicitly typed or cast to u64 to ensure the
> comparisons work correctly on 32-bit kernels?
>
>> posdiv2 = 1;
>>
>> /* Digital clk divider, max /32 */
>> - digital_div = div_u64(ns_hdmipll_ck, posdiv1 * posdiv2 * pixel_clk);
>> + digital_div = div64_u64(ns_hdmipll_ck, posdiv1 * posdiv2 * pixel_clk);
>> if (!(digital_div <= 32 && digital_div >= 1))
>> return -EINVAL;
>>
>
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v12 00/12] SPMI: Implement sub-devices and migrate drivers
From: Andy Shevchenko @ 2026-07-07 13:18 UTC (permalink / raw)
To: AngeloGioacchino Del Regno
Cc: jic23, sboyd, dlechner, nuno.sa, andy, arnd, gregkh, srini, vkoul,
neil.armstrong, sre, 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
In-Reply-To: <20260707111829.180158-1-angelogioacchino.delregno@collabora.com>
On Tue, Jul 07, 2026 at 01:18:17PM +0200, AngeloGioacchino Del Regno wrote:
> Changes in v12:
> - Removed call to of_node_put() for failure as it's being already
> done in spmi_dev_release(), as pointed out by Sashiko
> - Fixed usage of the new helper in all users (it's too hot today, sorry)
You have send two versions in a row and effectively ignored my comment.
What is going on? Perhaps you need to slow down, find a cool place and
start over?
--
With Best Regards,
Andy Shevchenko
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v12 05/12] spmi: Implement spmi_subdevice_alloc_and_add() and devm variant
From: AngeloGioacchino Del Regno @ 2026-07-07 11:59 UTC (permalink / raw)
To: sashiko-reviews; +Cc: neil.armstrong, vkoul, olteanv, linux-phy
In-Reply-To: <20260707113101.6CEB81F000E9@smtp.kernel.org>
On 7/7/26 13:31, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Combining device allocation and addition into a single `alloc_and_add` function breaks device model configuration semantics for `spmi_bus_type` devices.
> --
>
> --- Patch [5]: [PATCH v12 05/12] spmi: Implement spmi_subdevice_alloc_and_add() and devm variant ---
> commit 07c884decc8feb83970dc7a2fe54a4fbf80be610
> Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>
> spmi: Implement spmi_subdevice_alloc_and_add() and devm variant
>
> This patch adds new functions to manage sub-devices of an SPMI device,
> including spmi_subdevice_alloc_and_add() and spmi_subdevice_remove().
> These are intended to support complex PMICs that feature multiple
> sub-devices addressable via SPMI register ranges.
>
>> diff --git a/drivers/spmi/spmi.c b/drivers/spmi/spmi.c
>> index 4050f3614dab8..e218c81405458 100644
>> --- a/drivers/spmi/spmi.c
>> +++ b/drivers/spmi/spmi.c
>
> [ ... ]
>
>> +struct spmi_subdevice *spmi_subdevice_alloc_and_add(struct spmi_device *sparent)
>> +{
>> + struct spmi_subdevice *sub_sdev;
>> + struct spmi_device *sdev;
>> + int 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.%u.auto",
>> + sdev->ctrl->nr, sdev->usid, sub_sdev->devid);
>> + if (ret)
>> + goto err_put_dev;
>> +
>> + ret = device_add(&sdev->dev);
>
> [Severity: High]
> Does combining allocation and registration into a single function prevent
> callers from properly configuring device properties?
>
> Since device_add() is called immediately here, the driver core executes bus
> matching and broadcasts the KOBJ_ADD uevent before the caller receives the
> device pointer. This leaves no window to set properties like of_node or
> fwnode.
It's done for simplicity of API usage, and no properties need to be set during
registration of the SPMI subdevice.
>
> Will this cause the uevent to be sent without a MODALIAS, breaking userspace
> module autoloading and automatic driver probing for these subdevices?
>
No, and I'm not changing any more single bit of this series, because this series
now is completely fine.
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v12 06/12] spmi: Add helper to get a parent SPMI device
From: AngeloGioacchino Del Regno @ 2026-07-07 11:57 UTC (permalink / raw)
To: sashiko-reviews; +Cc: neil.armstrong, olteanv, vkoul, linux-phy
In-Reply-To: <20260707112756.DEAAB1F000E9@smtp.kernel.org>
On 7/7/26 13:27, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Time-of-check to time-of-use (TOCTOU) race on `dev->parent` can lead to a NULL pointer dereference.
> --
>
> --- Patch [6]: [PATCH v12 06/12] spmi: Add helper to get a parent SPMI device ---
> commit fb3218d6b528eabdc83ac90ed869685f4b9bd99e
> Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>
> spmi: Add helper to get a parent SPMI device
>
> This commit adds a new helper function spmi_get_parent_spmi_device()
> which takes a struct device and returns a pointer to its parent SPMI
> device if one exists, without modifying the reference count.
>
>> diff --git a/drivers/spmi/spmi.c b/drivers/spmi/spmi.c
>> index e218c81405458..a0a6ea9b1d2db 100644
>> --- a/drivers/spmi/spmi.c
>> +++ b/drivers/spmi/spmi.c
>
> [ ... ]
>
>> @@ -433,6 +433,29 @@ struct spmi_device *spmi_find_device_by_of_node(struct device_node *np)
>> }
>> EXPORT_SYMBOL_GPL(spmi_find_device_by_of_node);
>>
>> +/**
>> + * spmi_get_parent_spmi_device() - get the parent SPMI device from current dev
>> + * @dev: pointer to a subdevice on SPMI bus
>> + *
>> + * Checks if the passed device is a child of an SPMI device and returns a
>> + * handle to the parent SPMI device without incrementing any refcount.
>> + *
>> + * Return: Handle to the parent SPMI device or NULL
>> + */
>> +struct spmi_device *spmi_get_parent_spmi_device(struct device *dev)
>> +{
>> + struct device *parent;
>> +
>> + if (dev && dev->parent) {
>> + parent = dev->parent;
>> +
>> + if (parent->type == &spmi_dev_type)
>
> [Severity: High]
> Can a concurrent device topology change (like device_move()) cause a NULL
> pointer dereference here?
>
That's not happening.
> If dev->parent is modified to NULL between the check in the if condition
> and the assignment to the local parent variable, parent will become NULL
> and this parent->type check will panic.
>
> Since the compiler is permitted to emit two separate memory loads for
> dev->parent in this sequence, would it be safer to read it exactly once
> into the local variable before checking its validity?
>
Yes, but it's overcautious, and purely a nitpick. Not severe at all, and I'm not
fixing that as it's still safe.
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v12 05/12] spmi: Implement spmi_subdevice_alloc_and_add() and devm variant
From: sashiko-bot @ 2026-07-07 11:31 UTC (permalink / raw)
To: AngeloGioacchino Del Regno; +Cc: neil.armstrong, vkoul, olteanv, linux-phy
In-Reply-To: <20260707111829.180158-6-angelogioacchino.delregno@collabora.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Combining device allocation and addition into a single `alloc_and_add` function breaks device model configuration semantics for `spmi_bus_type` devices.
--
--- Patch [5]: [PATCH v12 05/12] spmi: Implement spmi_subdevice_alloc_and_add() and devm variant ---
commit 07c884decc8feb83970dc7a2fe54a4fbf80be610
Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
spmi: Implement spmi_subdevice_alloc_and_add() and devm variant
This patch adds new functions to manage sub-devices of an SPMI device,
including spmi_subdevice_alloc_and_add() and spmi_subdevice_remove().
These are intended to support complex PMICs that feature multiple
sub-devices addressable via SPMI register ranges.
> diff --git a/drivers/spmi/spmi.c b/drivers/spmi/spmi.c
> index 4050f3614dab8..e218c81405458 100644
> --- a/drivers/spmi/spmi.c
> +++ b/drivers/spmi/spmi.c
[ ... ]
> +struct spmi_subdevice *spmi_subdevice_alloc_and_add(struct spmi_device *sparent)
> +{
> + struct spmi_subdevice *sub_sdev;
> + struct spmi_device *sdev;
> + int 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.%u.auto",
> + sdev->ctrl->nr, sdev->usid, sub_sdev->devid);
> + if (ret)
> + goto err_put_dev;
> +
> + ret = device_add(&sdev->dev);
[Severity: High]
Does combining allocation and registration into a single function prevent
callers from properly configuring device properties?
Since device_add() is called immediately here, the driver core executes bus
matching and broadcasts the KOBJ_ADD uevent before the caller receives the
device pointer. This leaves no window to set properties like of_node or
fwnode.
Will this cause the uevent to be sent without a MODALIAS, breaking userspace
module autoloading and automatic driver probing for these subdevices?
> + 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);
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260707111829.180158-1-angelogioacchino.delregno@collabora.com?part=5
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v12 06/12] spmi: Add helper to get a parent SPMI device
From: sashiko-bot @ 2026-07-07 11:27 UTC (permalink / raw)
To: AngeloGioacchino Del Regno; +Cc: neil.armstrong, olteanv, vkoul, linux-phy
In-Reply-To: <20260707111829.180158-7-angelogioacchino.delregno@collabora.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Time-of-check to time-of-use (TOCTOU) race on `dev->parent` can lead to a NULL pointer dereference.
--
--- Patch [6]: [PATCH v12 06/12] spmi: Add helper to get a parent SPMI device ---
commit fb3218d6b528eabdc83ac90ed869685f4b9bd99e
Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
spmi: Add helper to get a parent SPMI device
This commit adds a new helper function spmi_get_parent_spmi_device()
which takes a struct device and returns a pointer to its parent SPMI
device if one exists, without modifying the reference count.
> diff --git a/drivers/spmi/spmi.c b/drivers/spmi/spmi.c
> index e218c81405458..a0a6ea9b1d2db 100644
> --- a/drivers/spmi/spmi.c
> +++ b/drivers/spmi/spmi.c
[ ... ]
> @@ -433,6 +433,29 @@ struct spmi_device *spmi_find_device_by_of_node(struct device_node *np)
> }
> EXPORT_SYMBOL_GPL(spmi_find_device_by_of_node);
>
> +/**
> + * spmi_get_parent_spmi_device() - get the parent SPMI device from current dev
> + * @dev: pointer to a subdevice on SPMI bus
> + *
> + * Checks if the passed device is a child of an SPMI device and returns a
> + * handle to the parent SPMI device without incrementing any refcount.
> + *
> + * Return: Handle to the parent SPMI device or NULL
> + */
> +struct spmi_device *spmi_get_parent_spmi_device(struct device *dev)
> +{
> + struct device *parent;
> +
> + if (dev && dev->parent) {
> + parent = dev->parent;
> +
> + if (parent->type == &spmi_dev_type)
[Severity: High]
Can a concurrent device topology change (like device_move()) cause a NULL
pointer dereference here?
If dev->parent is modified to NULL between the check in the if condition
and the assignment to the local parent variable, parent will become NULL
and this parent->type check will panic.
Since the compiler is permitted to emit two separate memory loads for
dev->parent in this sequence, would it be safer to read it exactly once
into the local variable before checking its validity?
> + return to_spmi_device(parent);
> + }
> + return NULL;
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260707111829.180158-1-angelogioacchino.delregno@collabora.com?part=6
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v11 11/12] iio: adc: qcom-spmi-iadc: Migrate to devm_spmi_subdevice_alloc_and_add()
From: sashiko-bot @ 2026-07-07 11:24 UTC (permalink / raw)
To: AngeloGioacchino Del Regno; +Cc: olteanv, linux-phy, neil.armstrong, vkoul
In-Reply-To: <20260707101821.173319-12-angelogioacchino.delregno@collabora.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unconditional probe failure due to an incorrect argument passed to `spmi_get_parent_spmi_device()`.
--
--- Patch [11]: [PATCH v11 11/12] iio: adc: qcom-spmi-iadc: Migrate to devm_spmi_subdevice_alloc_and_add() ---
commit a65138c323454925e722f4d8782149a45b65f227
Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
iio: adc: qcom-spmi-iadc: Migrate to devm_spmi_subdevice_alloc_and_add()
This commit migrates the Qualcomm SPMI IADC driver to use
devm_spmi_subdevice_alloc_and_add() to register an SPMI sub-device.
Instead of treating the parent PMIC as a syscon, it initializes its own
regmap with the sub-device's specific base address, allowing the regmap
API to handle register base offsets.
> diff --git a/drivers/iio/adc/qcom-spmi-iadc.c b/drivers/iio/adc/qcom-spmi-iadc.c
> index 0ec3a0c4b1de0..71049cdc27653 100644
> --- a/drivers/iio/adc/qcom-spmi-iadc.c
> +++ b/drivers/iio/adc/qcom-spmi-iadc.c
[ ... ]
> @@ -502,18 +508,24 @@ 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)
> + sparent = spmi_get_parent_spmi_device(dev->parent);
[Severity: High]
Does this code inadvertently pass the wrong argument to
spmi_get_parent_spmi_device()?
Looking at the implementation of spmi_get_parent_spmi_device():
struct spmi_device *spmi_get_parent_spmi_device(struct device *dev)
{
struct device *parent;
if (dev && dev->parent) {
parent = dev->parent;
if (parent->type == &spmi_dev_type)
return to_spmi_device(parent);
}
return NULL;
}
If we pass dev->parent, the helper will evaluate the grandparent's type
instead of the parent's. Since the grandparent is the SPMI controller
(&spmi_ctrl_type), this condition will fail, causing sparent to be NULL
and leading to an unconditional -ENODEV probe failure.
Should this be passing dev instead?
> + if (!sparent)
> return -ENODEV;
>
> + sub_sdev = devm_spmi_subdevice_alloc_and_add(dev, sparent);
> + if (IS_ERR(sub_sdev))
> + return PTR_ERR(sub_sdev);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260707101821.173319-1-angelogioacchino.delregno@collabora.com?part=11
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v11 10/12] misc: qcom-coincell: Migrate to devm_spmi_subdevice_alloc_and_add()
From: sashiko-bot @ 2026-07-07 11:20 UTC (permalink / raw)
To: AngeloGioacchino Del Regno; +Cc: vkoul, neil.armstrong, olteanv, linux-phy
In-Reply-To: <20260707101821.173319-11-angelogioacchino.delregno@collabora.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The probe function incorrectly passes `dev->parent` to `spmi_get_parent_spmi_device()`, causing an unconditional probe failure.
--
--- Patch [10]: [PATCH v11 10/12] misc: qcom-coincell: Migrate to devm_spmi_subdevice_alloc_and_add() ---
commit eab936776e34776b8e8c7c97d0effbd869a38db0
Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
misc: qcom-coincell: Migrate to devm_spmi_subdevice_alloc_and_add()
This commit migrates the coincell charger driver to use a new SPMI
sub-device instead of using the parent PMIC as a syscon. It initializes
a separate regmap with the sub-device's specific base address retrieved
from the devicetree.
> diff --git a/drivers/misc/qcom-coincell.c b/drivers/misc/qcom-coincell.c
> --- a/drivers/misc/qcom-coincell.c
> +++ b/drivers/misc/qcom-coincell.c
[ ... ]
> @@ -92,16 +101,25 @@ static int qcom_coincell_probe(struct platform_device *pdev)
>
> chgr.dev = &pdev->dev;
>
> - chgr.regmap = dev_get_regmap(pdev->dev.parent, NULL);
> - if (!chgr.regmap) {
> - dev_err(chgr.dev, "Unable to get regmap\n");
> - return -EINVAL;
> - }
> -
> - 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 = spmi_get_parent_spmi_device(dev->parent);
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[Severity: High]
Does this cause an unconditional probe failure?
If dev is the platform device, dev->parent is the SPMI device (the PMIC).
The spmi_get_parent_spmi_device() helper already inspects the parent of the
device passed to it. By passing dev->parent here, it evaluates
dev->parent->parent (the SPMI bus controller).
Since the bus controller's device type is &spmi_ctrl_type rather than
&spmi_dev_type, the helper's internal type check will fail and return NULL,
causing qcom_coincell_probe() to always return -ENODEV.
Should this pass dev instead of dev->parent?
> + if (!sparent)
> + return -ENODEV;
> +
> + sub_sdev = devm_spmi_subdevice_alloc_and_add(dev, sparent);
> + if (IS_ERR(sub_sdev))
> + return PTR_ERR(sub_sdev);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260707101821.173319-1-angelogioacchino.delregno@collabora.com?part=10
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* [PATCH v12 12/12] iio: adc: qcom-spmi-iadc: Remove regmap R/W wrapper functions
From: AngeloGioacchino Del Regno @ 2026-07-07 11:18 UTC (permalink / raw)
To: jic23, sboyd
Cc: dlechner, nuno.sa, andy, arnd, gregkh, srini, vkoul,
neil.armstrong, sre, 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: <20260707111829.180158-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 21e35df82193..196525ba0b4b 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 v12 11/12] iio: adc: qcom-spmi-iadc: Migrate to devm_spmi_subdevice_alloc_and_add()
From: AngeloGioacchino Del Regno @ 2026-07-07 11:18 UTC (permalink / raw)
To: jic23, sboyd
Cc: dlechner, nuno.sa, andy, arnd, gregkh, srini, vkoul,
neil.armstrong, sre, 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: <20260707111829.180158-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 | 33 ++++++++++++++++++++++----------
1 file changed, 23 insertions(+), 10 deletions(-)
diff --git a/drivers/iio/adc/qcom-spmi-iadc.c b/drivers/iio/adc/qcom-spmi-iadc.c
index 0ec3a0c4b1de..21e35df82193 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,19 @@ 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;
indio_dev = devm_iio_device_alloc(dev, sizeof(*iadc));
if (!indio_dev)
@@ -502,18 +508,24 @@ 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)
+ sparent = spmi_get_parent_spmi_device(dev);
+ if (!sparent)
return -ENODEV;
+ 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 v12 10/12] misc: qcom-coincell: Migrate to devm_spmi_subdevice_alloc_and_add()
From: AngeloGioacchino Del Regno @ 2026-07-07 11:18 UTC (permalink / raw)
To: jic23, sboyd
Cc: dlechner, nuno.sa, andy, arnd, gregkh, srini, vkoul,
neil.armstrong, sre, 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: <20260707111829.180158-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..28f48f60ecb7 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,19 +71,28 @@ 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;
@@ -92,16 +101,25 @@ static int qcom_coincell_probe(struct platform_device *pdev)
chgr.dev = &pdev->dev;
- chgr.regmap = dev_get_regmap(pdev->dev.parent, NULL);
- if (!chgr.regmap) {
- dev_err(chgr.dev, "Unable to get regmap\n");
- return -EINVAL;
- }
-
- 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 = spmi_get_parent_spmi_device(dev);
+ if (!sparent)
+ return -ENODEV;
+
+ 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 v12 09/12] phy: qualcomm: eusb2-repeater: Migrate to devm_spmi_subdevice_alloc_and_add()
From: AngeloGioacchino Del Regno @ 2026-07-07 11:18 UTC (permalink / raw)
To: jic23, sboyd
Cc: dlechner, nuno.sa, andy, arnd, gregkh, srini, vkoul,
neil.armstrong, sre, 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: <20260707111829.180158-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 | 52 ++++++++++++-------
2 files changed, 34 insertions(+), 20 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..89bc1c89fc85 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,11 +240,18 @@ 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;
rptr = devm_kzalloc(dev, sizeof(*rptr), GFP_KERNEL);
@@ -260,15 +265,21 @@ 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)
+ sparent = spmi_get_parent_spmi_device(dev);
+ if (!sparent)
return -ENODEV;
- ret = of_property_read_u32(np, "reg", &res);
+ sub_sdev = devm_spmi_subdevice_alloc_and_add(dev, sparent);
+ if (IS_ERR(sub_sdev))
+ return PTR_ERR(sub_sdev);
+
+ 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 v12 08/12] power: reset: qcom-pon: Migrate to devm_spmi_subdevice_alloc_and_add()
From: AngeloGioacchino Del Regno @ 2026-07-07 11:18 UTC (permalink / raw)
To: jic23, sboyd
Cc: dlechner, nuno.sa, andy, arnd, gregkh, srini, vkoul,
neil.armstrong, sre, 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: <20260707111829.180158-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 | 31 +++++++++++++++++++++++--------
1 file changed, 23 insertions(+), 8 deletions(-)
diff --git a/drivers/power/reset/qcom-pon.c b/drivers/power/reset/qcom-pon.c
index 7e108982a582..cd7ba680b841 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,6 +46,15 @@ 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;
@@ -56,17 +65,22 @@ static int qcom_pon_probe(struct platform_device *pdev)
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");
+ sparent = spmi_get_parent_spmi_device(dev);
+ if (!sparent)
return -ENODEV;
- }
- error = of_property_read_u32(pdev->dev.of_node, "reg",
- &pon->baseaddr);
+ sub_sdev = devm_spmi_subdevice_alloc_and_add(dev, sparent);
+ if (IS_ERR(sub_sdev))
+ return PTR_ERR(sub_sdev);
+
+ 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 v12 07/12] nvmem: qcom-spmi-sdam: Migrate to devm_spmi_subdevice_alloc_and_add()
From: AngeloGioacchino Del Regno @ 2026-07-07 11:18 UTC (permalink / raw)
To: jic23, sboyd
Cc: dlechner, nuno.sa, andy, arnd, gregkh, srini, vkoul,
neil.armstrong, sre, 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: <20260707111829.180158-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 | 41 ++++++++++++++++++++++++----------
2 files changed, 30 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..2aec95dff05d 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,26 @@ 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 = spmi_get_parent_spmi_device(dev);
+ if (!sparent)
+ return -ENODEV;
- rc = of_property_read_u32(pdev->dev.of_node, "reg", &sdam->base);
+ sub_sdev = devm_spmi_subdevice_alloc_and_add(dev, sparent);
+ if (IS_ERR(sub_sdev))
+ return PTR_ERR(sub_sdev);
+
+ 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 +175,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 +197,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 v12 06/12] spmi: Add helper to get a parent SPMI device
From: AngeloGioacchino Del Regno @ 2026-07-07 11:18 UTC (permalink / raw)
To: jic23, sboyd
Cc: dlechner, nuno.sa, andy, arnd, gregkh, srini, vkoul,
neil.armstrong, sre, 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
In-Reply-To: <20260707111829.180158-1-angelogioacchino.delregno@collabora.com>
Drivers that register a SPMI sub-device as a platform_driver are
not guaranteed to have a SPMI device as their parent: add a new
helper `spmi_get_parent_spmi_device()` that takes a struct device
and checks if there's a parent, and if that parent is actually a
SPMI device by checking if its device type matches with SPMI: if
so, returns a pointer to the relative struct spmi_device without
incrementing any refcount.
As a note, in the specific case of using this helper to retrieve
a SPMI subdevice's parent, the spmi_subdevice_alloc_and_add()
function does actually call device_add(), which will already take
care of raising the refcount of the associated parent device.
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
drivers/spmi/spmi.c | 23 +++++++++++++++++++++++
include/linux/spmi.h | 1 +
2 files changed, 24 insertions(+)
diff --git a/drivers/spmi/spmi.c b/drivers/spmi/spmi.c
index e218c8140545..a0a6ea9b1d2d 100644
--- a/drivers/spmi/spmi.c
+++ b/drivers/spmi/spmi.c
@@ -433,6 +433,29 @@ struct spmi_device *spmi_find_device_by_of_node(struct device_node *np)
}
EXPORT_SYMBOL_GPL(spmi_find_device_by_of_node);
+/**
+ * spmi_get_parent_spmi_device() - get the parent SPMI device from current dev
+ * @dev: pointer to a subdevice on SPMI bus
+ *
+ * Checks if the passed device is a child of an SPMI device and returns a
+ * handle to the parent SPMI device without incrementing any refcount.
+ *
+ * Return: Handle to the parent SPMI device or NULL
+ */
+struct spmi_device *spmi_get_parent_spmi_device(struct device *dev)
+{
+ struct device *parent;
+
+ if (dev && dev->parent) {
+ parent = dev->parent;
+
+ if (parent->type == &spmi_dev_type)
+ return to_spmi_device(parent);
+ }
+ return NULL;
+}
+EXPORT_SYMBOL_NS_GPL(spmi_get_parent_spmi_device, "SPMI");
+
/**
* spmi_device_alloc() - Allocate a new SPMI device
* @ctrl: associated controller
diff --git a/include/linux/spmi.h b/include/linux/spmi.h
index a78a8924b2ac..4daebc980501 100644
--- a/include/linux/spmi.h
+++ b/include/linux/spmi.h
@@ -189,6 +189,7 @@ static inline void spmi_driver_unregister(struct spmi_driver *sdrv)
struct device_node;
struct spmi_device *spmi_find_device_by_of_node(struct device_node *np);
+struct spmi_device *spmi_get_parent_spmi_device(struct device *dev);
int spmi_register_read(struct spmi_device *sdev, u8 addr, u8 *buf);
int spmi_ext_register_read(struct spmi_device *sdev, u8 addr, u8 *buf,
size_t len);
--
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 v12 05/12] spmi: Implement spmi_subdevice_alloc_and_add() and devm variant
From: AngeloGioacchino Del Regno @ 2026-07-07 11:18 UTC (permalink / raw)
To: jic23, sboyd
Cc: dlechner, nuno.sa, andy, arnd, gregkh, srini, vkoul,
neil.armstrong, sre, 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: <20260707111829.180158-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 4050f3614dab..e218c8140545 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)
{
@@ -34,6 +35,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);
@@ -90,6 +104,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)
{
@@ -431,6 +457,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_obj(*sub_sdev);
+ 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.%u.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 v12 04/12] spmi: Remove unneeded goto in spmi_device_add() error path
From: AngeloGioacchino Del Regno @ 2026-07-07 11:18 UTC (permalink / raw)
To: jic23, sboyd
Cc: dlechner, nuno.sa, andy, arnd, gregkh, srini, vkoul,
neil.armstrong, sre, 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: <20260707111829.180158-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 244bd116f970..4050f3614dab 100644
--- a/drivers/spmi/spmi.c
+++ b/drivers/spmi/spmi.c
@@ -72,13 +72,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 v12 03/12] spmi: Print error status with %pe format
From: AngeloGioacchino Del Regno @ 2026-07-07 11:18 UTC (permalink / raw)
To: jic23, sboyd
Cc: dlechner, nuno.sa, andy, arnd, gregkh, srini, vkoul,
neil.armstrong, sre, 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: <20260707111829.180158-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 de019f3f1e27..244bd116f970 100644
--- a/drivers/spmi/spmi.c
+++ b/drivers/spmi/spmi.c
@@ -71,7 +71,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;
}
@@ -496,8 +496,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;
}
@@ -525,7 +525,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 v12 02/12] spmi: Remove redundant dev_name() print in spmi_device_add()
From: AngeloGioacchino Del Regno @ 2026-07-07 11:18 UTC (permalink / raw)
To: jic23, sboyd
Cc: dlechner, nuno.sa, andy, arnd, gregkh, srini, vkoul,
neil.armstrong, sre, 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: <20260707111829.180158-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 2e4ed06fab42..de019f3f1e27 100644
--- a/drivers/spmi/spmi.c
+++ b/drivers/spmi/spmi.c
@@ -71,12 +71,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
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