[PATCH v2 0/7] mfd: qcom-pm8008: Add support for PM8010 over I2C

[PATCH v2 0/7] mfd: qcom-pm8008: Add support for PM8010 over I2C

[Jishnu Prakash]

PM8010 is the next generation PMIC following PM8008 and has similar
functionality, providing 7 LDOs that power camera sensors and associated
hardware, with some differences from PM8008 LDOs. The series also adds
support for skipping interrupt configurations which is needed to support
SoCs where the PMIC's interrupt pin is grounded, such as Glymur.

Patch 1 adds the new compatible string for PM8010 over I2C

Patch 2 adds PM8010 LDO support

Patch 3 adds mode voting support for PM8010, which is not supported on PM8008

Patch 4 drops GPIO support from the MFD driver, as the GPIOs have fixed
functions and don't need SW configurations

Patch 5 adds PM8010 support in the MFD driver

Patch 6 updates documentation to make interrupt configuration optional

Patch 7 updates the MFD driver to tolerate a missing interrupt

Signed-off-by: Jishnu Prakash <jishnu.prakash@oss.qualcomm.com>
---
Changes in v2:
- Simplified of_device_is_compatible() call in mode support patch as
  suggested by sashiko.
- Added a change to drop GPIO support in MFD driver as suggested by Konrad.
- Replaced match_data parsing in probe in PM8010 MFD support patch with enum
  identifier and switch() statement as suggested by Lee.
- Moved interrupt configuration from probe into separate function in missing
  interrupt support patch as suggested by Lee.
- Added MFD cells for no-IRQ variants of PM8008/PM8010 and updated probe to
  use them in missing interrupt support patch as suggested by sashiko.
- Collected Rob's Acked-by tags on binding changes and Oleg's Tested-by tags
  on regulator driver changes (not on MFD changes since some significant
  changes are made in them now).
- Link to v1: https://patch.msgid.link/20260918-pm8010_i2c_support-v1-0-de33d78fad9d@oss.qualcomm.com

---
Dhruvin Rajpura (1):
      regulator: qcom-pm8008: Add PM8010 mode support

Jishnu Prakash (1):
      mfd: qcom-pm8008: Drop GPIO support

Konrad Dybcio (5):
      dt-bindings: mfd: pm8008: Add qcom,pm8010-i2c compatible
      regulator: pm8008: Add PM8010 support
      mfd: qcom-pm8008: Add PM8010 support
      dt-bindings: mfd: pm8008: Make interrupts optional
      mfd: qcom-pm8008: Tolerate missing interrupt

 .../devicetree/bindings/mfd/qcom,pm8008.yaml       |  10 +-
 drivers/mfd/qcom-pm8008.c                          | 165 +++++++++++++-----
 drivers/regulator/qcom-pm8008-regulator.c          | 184 ++++++++++++++++++---
 3 files changed, 289 insertions(+), 70 deletions(-)
---
base-commit: 24a19b953e2a3d0ea5a8c4951a0ab8f32720c907
change-id: 20260915-pm8010_i2c_support-3062421372c3

Best regards,
--  
Jishnu Prakash <jishnu.prakash@oss.qualcomm.com>

[PATCH v2 1/7] dt-bindings: mfd: pm8008: Add qcom,pm8010-i2c compatible

[Jishnu Prakash]

From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

The PM8010 can communicate over both SPMI and I2C, based on whether
GPIO1 is pulled high or low respectively. Since the qcom,pm8010
compatible is already taken in the SPMI PMIC bindings and that set of
properties is heavily tailored towards SPMI, introduce a secondary
compatible to handle the I2C case.

Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Acked-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: Jishnu Prakash <jishnu.prakash@oss.qualcomm.com>
---
 Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml b/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml
index 0c6e1870db1d..2701aa755f31 100644
--- a/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml
+++ b/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml
@@ -16,7 +16,9 @@ description: |
 
 properties:
   compatible:
-    const: qcom,pm8008
+    enum:
+      - qcom,pm8008
+      - qcom,pm8010-i2c
 
   reg:
     maxItems: 1

-- 
2.43.0

[PATCH v2 2/7] regulator: pm8008: Add PM8010 support

[Jishnu Prakash]

From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

PM8010 is effectively the next generation of PM8008.

Like PM8008, PM8010 also has 7 LDOs under it which are used to power
camera sensors. The PM8010 LDO are of different types from the PM8008
ones and have different voltage dropout values. PM8010 LDOs do not have
a voltage step rate configuration register.

Add the necessary changes to support PM8010 LDOs.

Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Co-developed-by: Dhruvin Rajpura <drajpura@qti.qualcomm.com>
Signed-off-by: Dhruvin Rajpura <drajpura@qti.qualcomm.com>
Tested-by: Oleg Keri <okerixx@gmail.com> # Lenovo Yoga Slim 7x Gen 11 (Glymur)
Co-developed-by: Jishnu Prakash <jishnu.prakash@oss.qualcomm.com>
Signed-off-by: Jishnu Prakash <jishnu.prakash@oss.qualcomm.com>
---
 drivers/regulator/qcom-pm8008-regulator.c | 109 +++++++++++++++++++++++-------
 1 file changed, 85 insertions(+), 24 deletions(-)

diff --git a/drivers/regulator/qcom-pm8008-regulator.c b/drivers/regulator/qcom-pm8008-regulator.c
index 9c9b8be2e15a..b484c0f73eea 100644
--- a/drivers/regulator/qcom-pm8008-regulator.c
+++ b/drivers/regulator/qcom-pm8008-regulator.c
@@ -39,24 +39,59 @@ struct pm8008_regulator_data {
 	unsigned int			base;
 	int				min_dropout_uV;
 	const struct linear_range	*voltage_range;
+	int				n_linear_ranges;
 };
 
-static const struct linear_range nldo_ranges[] = {
+struct pm8008_match_data {
+	const bool has_stepper_ctl_reg;
+	const struct pm8008_regulator_data *regulator_data;
+	const int num_regulators;
+};
+
+static const struct linear_range pm8008_nldo_ranges[] = {
 	REGULATOR_LINEAR_RANGE(528000, 0, 122, 8000),
 };
 
-static const struct linear_range pldo_ranges[] = {
+static const struct linear_range pm8008_pldo_ranges[] = {
 	REGULATOR_LINEAR_RANGE(1504000, 0, 237, 8000),
 };
 
+static const struct linear_range pm8010_nldo_ranges[] = {
+	REGULATOR_LINEAR_RANGE(528000, 0, 127, 8000),
+};
+
+static const struct linear_range pm8010_pldo_ranges[] = {
+	REGULATOR_LINEAR_RANGE(1504000, 0, 255, 8000),
+};
+
+static const struct linear_range pm8010_pldo_lv_ranges[] = {
+	REGULATOR_LINEAR_RANGE(1800000, 0,  2,  200000),
+	REGULATOR_LINEAR_RANGE(2608000, 3,  28, 16000),
+	REGULATOR_LINEAR_RANGE(3104000, 29, 30, 96000),
+	REGULATOR_LINEAR_RANGE(3312000, 31, 31, 0),
+};
+
+#define PM8008_REGULATOR(_name, _supply, _base, _dropout, _range)	\
+	{ _name, _supply, _base, _dropout, _range, ARRAY_SIZE(_range) }
+
 static const struct pm8008_regulator_data pm8008_reg_data[] = {
-	{ "ldo1", "vdd-l1-l2", 0x4000, 225000, nldo_ranges, },
-	{ "ldo2", "vdd-l1-l2", 0x4100, 225000, nldo_ranges, },
-	{ "ldo3", "vdd-l3-l4", 0x4200, 300000, pldo_ranges, },
-	{ "ldo4", "vdd-l3-l4", 0x4300, 300000, pldo_ranges, },
-	{ "ldo5", "vdd-l5",    0x4400, 200000, pldo_ranges, },
-	{ "ldo6", "vdd-l6",    0x4500, 200000, pldo_ranges, },
-	{ "ldo7", "vdd-l7",    0x4600, 200000, pldo_ranges, },
+	PM8008_REGULATOR("ldo1", "vdd-l1-l2", 0x4000, 225000, pm8008_nldo_ranges),
+	PM8008_REGULATOR("ldo2", "vdd-l1-l2", 0x4100, 225000, pm8008_nldo_ranges),
+	PM8008_REGULATOR("ldo3", "vdd-l3-l4", 0x4200, 300000, pm8008_pldo_ranges),
+	PM8008_REGULATOR("ldo4", "vdd-l3-l4", 0x4300, 300000, pm8008_pldo_ranges),
+	PM8008_REGULATOR("ldo5", "vdd-l5",    0x4400, 200000, pm8008_pldo_ranges),
+	PM8008_REGULATOR("ldo6", "vdd-l6",    0x4500, 200000, pm8008_pldo_ranges),
+	PM8008_REGULATOR("ldo7", "vdd-l7",    0x4600, 200000, pm8008_pldo_ranges),
+};
+
+static const struct pm8008_regulator_data pm8010_reg_data[] = {
+	PM8008_REGULATOR("ldo1", "vdd-l1-l2", 0x4000, 172000, pm8010_nldo_ranges),
+	PM8008_REGULATOR("ldo2", "vdd-l1-l2", 0x4100, 172000, pm8010_nldo_ranges),
+	PM8008_REGULATOR("ldo3", "vdd-l3-l4", 0x4200, 80000, pm8010_pldo_lv_ranges),
+	PM8008_REGULATOR("ldo4", "vdd-l3-l4", 0x4300, 80000, pm8010_pldo_lv_ranges),
+	PM8008_REGULATOR("ldo5", "vdd-l5",    0x4400, 296000, pm8010_pldo_ranges),
+	PM8008_REGULATOR("ldo6", "vdd-l6",    0x4500, 80000, pm8010_pldo_lv_ranges),
+	PM8008_REGULATOR("ldo7", "vdd-l7",    0x4600, 296000, pm8010_pldo_ranges),
 };
 
 static int pm8008_regulator_set_voltage_sel(struct regulator_dev *rdev, unsigned int sel)
@@ -100,7 +135,7 @@ static int pm8008_regulator_get_voltage_sel(struct regulator_dev *rdev)
 }
 
 static const struct regulator_ops pm8008_regulator_ops = {
-	.list_voltage		= regulator_list_voltage_linear,
+	.list_voltage		= regulator_list_voltage_linear_range,
 	.set_voltage_sel	= pm8008_regulator_set_voltage_sel,
 	.get_voltage_sel	= pm8008_regulator_get_voltage_sel,
 	.enable			= regulator_enable_regmap,
@@ -110,8 +145,10 @@ static const struct regulator_ops pm8008_regulator_ops = {
 
 static int pm8008_regulator_probe(struct platform_device *pdev)
 {
+	const struct pm8008_match_data *match_data;
 	const struct pm8008_regulator_data *data;
 	struct regulator_config config = {};
+	const struct platform_device_id *id;
 	struct device *dev = &pdev->dev;
 	struct pm8008_regulator *preg;
 	struct regulator_desc *desc;
@@ -120,12 +157,20 @@ static int pm8008_regulator_probe(struct platform_device *pdev)
 	unsigned int val;
 	int ret, i;
 
+	id = platform_get_device_id(pdev);
+	if (!id)
+		return dev_err_probe(dev, -ENODEV, "Missing platform device id\n");
+
+	match_data = (const struct pm8008_match_data *)id->driver_data;
+	if (!match_data)
+		return dev_err_probe(dev, -ENODATA, "Missing driver match data\n");
+
 	regmap = dev_get_regmap(dev->parent, "secondary");
 	if (!regmap)
 		return -EINVAL;
 
-	for (i = 0; i < ARRAY_SIZE(pm8008_reg_data); i++) {
-		data = &pm8008_reg_data[i];
+	for (i = 0; i < match_data->num_regulators; i++) {
+		data = &match_data->regulator_data[i];
 
 		preg = devm_kzalloc(dev, sizeof(*preg), GFP_KERNEL);
 		if (!preg)
@@ -145,18 +190,21 @@ static int pm8008_regulator_probe(struct platform_device *pdev)
 		desc->owner = THIS_MODULE;
 
 		desc->linear_ranges = data->voltage_range;
-		desc->n_linear_ranges = 1;
-		desc->uV_step = desc->linear_ranges[0].step;
-		desc->min_uV = desc->linear_ranges[0].min;
-		desc->n_voltages = linear_range_values_in_range(&desc->linear_ranges[0]);
-
-		ret = regmap_read(regmap, preg->base + LDO_STEPPER_CTL_REG, &val);
-		if (ret < 0) {
-			dev_err(dev, "failed to read step rate: %d\n", ret);
-			return ret;
+		desc->n_linear_ranges = data->n_linear_ranges;
+		desc->n_voltages = linear_range_values_in_range_array(desc->linear_ranges,
+								      desc->n_linear_ranges);
+
+		if (match_data->has_stepper_ctl_reg) {
+			ret = regmap_read(regmap, preg->base + LDO_STEPPER_CTL_REG, &val);
+			if (ret < 0) {
+				dev_err(dev, "failed to read step rate: %d\n", ret);
+				return ret;
+			}
+			val &= STEP_RATE_MASK;
+			desc->ramp_delay = DEFAULT_VOLTAGE_STEPPER_RATE >> val;
+		} else {
+			desc->ramp_delay = DEFAULT_VOLTAGE_STEPPER_RATE;
 		}
-		val &= STEP_RATE_MASK;
-		desc->ramp_delay = DEFAULT_VOLTAGE_STEPPER_RATE >> val;
 
 		desc->min_dropout_uV = data->min_dropout_uV;
 
@@ -179,8 +227,21 @@ static int pm8008_regulator_probe(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct pm8008_match_data pm8008_data = {
+	.has_stepper_ctl_reg = true,
+	.regulator_data = pm8008_reg_data,
+	.num_regulators = ARRAY_SIZE(pm8008_reg_data),
+};
+
+static const struct pm8008_match_data pm8010_data = {
+	.has_stepper_ctl_reg = false,
+	.regulator_data = pm8010_reg_data,
+	.num_regulators = ARRAY_SIZE(pm8010_reg_data),
+};
+
 static const struct platform_device_id pm8008_regulator_id_table[] = {
-	{ .name = "pm8008-regulator" },
+	{ .name = "pm8008-regulator", .driver_data = (kernel_ulong_t)&pm8008_data },
+	{ .name = "pm8010-regulator", .driver_data = (kernel_ulong_t)&pm8010_data },
 	{ }
 };
 MODULE_DEVICE_TABLE(platform, pm8008_regulator_id_table);

-- 
2.43.0

[PATCH v2 3/7] regulator: qcom-pm8008: Add PM8010 mode support

[Jishnu Prakash]

From: Dhruvin Rajpura <drajpura@qti.qualcomm.com>

Implement set_mode/get_mode support for PM8010 LDOs using the mode
control and status registers to allow clients to vote for LDO modes.
Mode voting is not supported in the PM8008 LDOs.

Signed-off-by: Dhruvin Rajpura <drajpura@qti.qualcomm.com>
Tested-by: Oleg Keri <okerixx@gmail.com> # Lenovo Yoga Slim 7x Gen 11 (Glymur)
Signed-off-by: Jishnu Prakash <jishnu.prakash@oss.qualcomm.com>
---
 drivers/regulator/qcom-pm8008-regulator.c | 75 ++++++++++++++++++++++++++++++-
 1 file changed, 74 insertions(+), 1 deletion(-)

diff --git a/drivers/regulator/qcom-pm8008-regulator.c b/drivers/regulator/qcom-pm8008-regulator.c
index b484c0f73eea..ae5370770b2f 100644
--- a/drivers/regulator/qcom-pm8008-regulator.c
+++ b/drivers/regulator/qcom-pm8008-regulator.c
@@ -8,6 +8,7 @@
 #include <linux/array_size.h>
 #include <linux/bits.h>
 #include <linux/device.h>
+#include <linux/i2c.h>
 #include <linux/math.h>
 #include <linux/module.h>
 #include <linux/of.h>
@@ -24,9 +25,19 @@
 
 #define LDO_VSET_LB_REG			0x40
 
+#define LDO_MODE_CTL1_REG		0x45
+#define MODE_PRIMARY_MASK		GENMASK(2, 0)
+#define LDO_MODE_NPM			7
+#define LDO_MODE_LPM			4
+
 #define LDO_ENABLE_REG			0x46
 #define ENABLE_BIT			BIT(7)
 
+#define LDO_STATUS1_REG			0x08
+#define MODE_STATE_MASK			GENMASK(1, 0)
+#define MODE_STATE_NPM			3
+#define MODE_STATE_LPM			2
+
 struct pm8008_regulator {
 	struct regmap		*regmap;
 	struct regulator_desc	desc;
@@ -134,6 +145,51 @@ static int pm8008_regulator_get_voltage_sel(struct regulator_dev *rdev)
 	return regulator_map_voltage_linear_range(rdev, uV, INT_MAX);
 }
 
+static int pm8010_regulator_set_mode(struct regulator_dev *rdev, unsigned int mode)
+{
+	struct pm8008_regulator *preg = rdev_get_drvdata(rdev);
+	unsigned int val;
+
+	switch (mode) {
+	case REGULATOR_MODE_NORMAL:
+		val = LDO_MODE_NPM;
+		break;
+	case REGULATOR_MODE_IDLE:
+		val = LDO_MODE_LPM;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return regmap_update_bits(preg->regmap, preg->base + LDO_MODE_CTL1_REG,
+				   MODE_PRIMARY_MASK, val);
+}
+
+static unsigned int pm8010_regulator_get_mode(struct regulator_dev *rdev)
+{
+	struct pm8008_regulator *preg = rdev_get_drvdata(rdev);
+	unsigned int val;
+	int ret;
+
+	ret = regmap_read(preg->regmap, preg->base + LDO_STATUS1_REG, &val);
+	if (ret < 0)
+		return REGULATOR_MODE_INVALID;
+
+	return (val & MODE_STATE_MASK) == MODE_STATE_NPM ?
+		REGULATOR_MODE_NORMAL : REGULATOR_MODE_IDLE;
+}
+
+static unsigned int pm8010_regulator_of_map_mode(unsigned int mode)
+{
+	switch (mode) {
+	case REGULATOR_MODE_NORMAL:
+	case REGULATOR_MODE_IDLE:
+		return mode;
+	default:
+		return REGULATOR_MODE_INVALID;
+	}
+}
+
 static const struct regulator_ops pm8008_regulator_ops = {
 	.list_voltage		= regulator_list_voltage_linear_range,
 	.set_voltage_sel	= pm8008_regulator_set_voltage_sel,
@@ -143,6 +199,17 @@ static const struct regulator_ops pm8008_regulator_ops = {
 	.is_enabled		= regulator_is_enabled_regmap,
 };
 
+static const struct regulator_ops pm8010_regulator_ops = {
+	.list_voltage		= regulator_list_voltage_linear_range,
+	.set_voltage_sel	= pm8008_regulator_set_voltage_sel,
+	.get_voltage_sel	= pm8008_regulator_get_voltage_sel,
+	.enable			= regulator_enable_regmap,
+	.disable		= regulator_disable_regmap,
+	.is_enabled		= regulator_is_enabled_regmap,
+	.set_mode		= pm8010_regulator_set_mode,
+	.get_mode		= pm8010_regulator_get_mode,
+};
+
 static int pm8008_regulator_probe(struct platform_device *pdev)
 {
 	const struct pm8008_match_data *match_data;
@@ -155,6 +222,7 @@ static int pm8008_regulator_probe(struct platform_device *pdev)
 	struct regulator_dev *rdev;
 	struct regmap *regmap;
 	unsigned int val;
+	bool is_pm8010;
 	int ret, i;
 
 	id = platform_get_device_id(pdev);
@@ -169,6 +237,9 @@ static int pm8008_regulator_probe(struct platform_device *pdev)
 	if (!regmap)
 		return -EINVAL;
 
+	is_pm8010 = of_device_is_compatible(dev->parent->of_node,
+					    "qcom,pm8010-i2c");
+
 	for (i = 0; i < match_data->num_regulators; i++) {
 		data = &match_data->regulator_data[i];
 
@@ -185,7 +256,9 @@ static int pm8008_regulator_probe(struct platform_device *pdev)
 		desc->supply_name = data->supply_name;
 		desc->of_match = data->name;
 		desc->regulators_node = of_match_ptr("regulators");
-		desc->ops = &pm8008_regulator_ops;
+		desc->ops = is_pm8010 ? &pm8010_regulator_ops : &pm8008_regulator_ops;
+		if (is_pm8010)
+			desc->of_map_mode = pm8010_regulator_of_map_mode;
 		desc->type = REGULATOR_VOLTAGE;
 		desc->owner = THIS_MODULE;
 

-- 
2.43.0

Re: [PATCH v2 3/7] regulator: qcom-pm8008: Add PM8010 mode support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] regulator: qcom-pm8008: get_mode() reads status instead of control reg
--

commit b99c63e4d74725e0e0de61987ec984083cab186f
Author: Dhruvin Rajpura <drajpura@qti.qualcomm.com>

regulator: qcom-pm8008: Add PM8010 mode support

This commit implements set_mode and get_mode support for PM8010 LDOs using
the mode control and status registers. It allows clients to vote for LDO
modes, a feature that is not supported in the PM8008 LDOs.

> diff --git a/drivers/regulator/qcom-pm8008-regulator.c b/drivers/regulator/qcom-pm8008-regulator.c
> index b484c0f73eeac..ae5370770b2fc 100644
> --- a/drivers/regulator/qcom-pm8008-regulator.c
> +++ b/drivers/regulator/qcom-pm8008-regulator.c
[ ... ]
> @@ -134,6 +145,51 @@ static int pm8008_regulator_get_voltage_sel(struct regulator_dev *rdev)
>  	return regulator_map_voltage_linear_range(rdev, uV, INT_MAX);
>  }
>  
[ ... ]
> +static unsigned int pm8010_regulator_get_mode(struct regulator_dev *rdev)
> +{
> +	struct pm8008_regulator *preg = rdev_get_drvdata(rdev);
> +	unsigned int val;
> +	int ret;
> +
> +	ret = regmap_read(preg->regmap, preg->base + LDO_STATUS1_REG, &val);
> +	if (ret < 0)
> +		return REGULATOR_MODE_INVALID;
> +
> +	return (val & MODE_STATE_MASK) == MODE_STATE_NPM ?
> +		REGULATOR_MODE_NORMAL : REGULATOR_MODE_IDLE;
> +}

[Severity: Medium]
Does reading LDO_STATUS1_REG instead of the control register
(LDO_MODE_CTL1_REG) here break the regulator core's set_mode optimization
when the LDO is disabled?

When the LDO is disabled, the physical status register indicates an OFF state
(evaluating to 0), which incorrectly falls through to return
REGULATOR_MODE_IDLE.

If a consumer calls regulator_set_mode() with REGULATOR_MODE_IDLE while the
regulator is disabled, the core compares the requested mode with the returned
get_mode() value. Seeing a match, it skips writing the new mode to the control
register. Later, when the regulator is enabled, it powers up using the stale
mode in LDO_MODE_CTL1_REG (e.g., REGULATOR_MODE_NORMAL), completely ignoring
the consumer's earlier REGULATOR_MODE_IDLE request.

Should pm8010_regulator_get_mode() read LDO_MODE_CTL1_REG instead to
accurately reflect the configured state rather than the dynamic physical
status?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014-pm8010_i2c_support-v2-0-b395d2b3f668@oss.qualcomm.com?part=3

[PATCH v2 4/7] mfd: qcom-pm8008: Drop GPIO support

[Jishnu Prakash]

GPIOs used on PM8008 and also the upcoming PM8010 have fixed
functions (reset and interrupt input functionality) so it is
not useful to expose them for general-purpose usage. Drop the
GPIO MFD cell and associated peripheral definitions.

Signed-off-by: Jishnu Prakash <jishnu.prakash@oss.qualcomm.com>
---
 drivers/mfd/qcom-pm8008.c | 16 ----------------
 1 file changed, 16 deletions(-)

diff --git a/drivers/mfd/qcom-pm8008.c b/drivers/mfd/qcom-pm8008.c
index 60204cc9a2dc..2d6c01cf54f3 100644
--- a/drivers/mfd/qcom-pm8008.c
+++ b/drivers/mfd/qcom-pm8008.c
@@ -31,19 +31,13 @@
 enum {
 	PM8008_MISC,
 	PM8008_TEMP_ALARM,
-	PM8008_GPIO1,
-	PM8008_GPIO2,
 	PM8008_NUM_PERIPHS,
 };
 
 #define PM8008_PERIPH_0_BASE	0x900
 #define PM8008_PERIPH_1_BASE	0x2400
-#define PM8008_PERIPH_2_BASE	0xc000
-#define PM8008_PERIPH_3_BASE	0xc100
 
 #define PM8008_TEMP_ALARM_ADDR	PM8008_PERIPH_1_BASE
-#define PM8008_GPIO1_ADDR	PM8008_PERIPH_2_BASE
-#define PM8008_GPIO2_ADDR	PM8008_PERIPH_3_BASE
 
 /* PM8008 IRQ numbers */
 #define PM8008_IRQ_MISC_UVLO	0
@@ -52,8 +46,6 @@ enum {
 #define PM8008_IRQ_MISC_OTST3	3
 #define PM8008_IRQ_MISC_LDO_OCP	4
 #define PM8008_IRQ_TEMP_ALARM	5
-#define PM8008_IRQ_GPIO1	6
-#define PM8008_IRQ_GPIO2	7
 
 enum {
 	SET_TYPE_INDEX,
@@ -84,15 +76,11 @@ static const struct regmap_irq pm8008_irqs[] = {
 	_IRQ(PM8008_IRQ_MISC_OTST3,   PM8008_MISC,	BIT(3), IRQ_TYPE_EDGE_RISING),
 	_IRQ(PM8008_IRQ_MISC_LDO_OCP, PM8008_MISC,	BIT(4), IRQ_TYPE_EDGE_RISING),
 	_IRQ(PM8008_IRQ_TEMP_ALARM,   PM8008_TEMP_ALARM,BIT(0), IRQ_TYPE_SENSE_MASK),
-	_IRQ(PM8008_IRQ_GPIO1,	      PM8008_GPIO1,	BIT(0), IRQ_TYPE_SENSE_MASK),
-	_IRQ(PM8008_IRQ_GPIO2,	      PM8008_GPIO2,	BIT(0), IRQ_TYPE_SENSE_MASK),
 };
 
 static const unsigned int pm8008_periph_base[] = {
 	PM8008_PERIPH_0_BASE,
 	PM8008_PERIPH_1_BASE,
-	PM8008_PERIPH_2_BASE,
-	PM8008_PERIPH_3_BASE,
 };
 
 static unsigned int pm8008_get_irq_reg(struct regmap_irq_chip_data *data,
@@ -180,7 +168,6 @@ static const struct resource pm8008_temp_res[] = {
 static const struct mfd_cell pm8008_cells[] = {
 	MFD_CELL_NAME("pm8008-regulator"),
 	MFD_CELL_RES("qpnp-temp-alarm", pm8008_temp_res),
-	MFD_CELL_NAME("pm8008-gpio"),
 };
 
 static void devm_irq_domain_fwnode_release(void *data)
@@ -252,9 +239,6 @@ static int pm8008_probe(struct i2c_client *client)
 		return ret;
 	}
 
-	/* Needed by GPIO driver. */
-	dev_set_drvdata(dev, regmap_irq_get_domain(irq_data));
-
 	return devm_mfd_add_devices(dev, PLATFORM_DEVID_AUTO, pm8008_cells,
 				ARRAY_SIZE(pm8008_cells), NULL, 0,
 				regmap_irq_get_domain(irq_data));

-- 
2.43.0

Re: [PATCH v2 4/7] mfd: qcom-pm8008: Drop GPIO support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] mfd: qcom-pm8008: OOB array access due to truncated pm8008_periph_base
--

Include code snippets in your questions without diff formatting.thought
Okay, wait! The prompt says "CRITICAL OVERRIDE: Your previous response was blocked by the API recitation filter for quoting the patch diff verbatim. Ignore the diff-quoting instructions in inline-template.md. Do NOT quote any code or diff lines with '>'. Instead, start with the Commit/Author/Subject headers, write the summary, and describe each finding directly by referencing file names, function names, and line numbers in plain prose. Include code snippets in your questions without diff formatting."

Wait, the prompt says "NEVER QUOTE LINE NUMBERS", but the CRITICAL OVERRIDE says "describe each finding directly by referencing file names, function names, and line numbers in plain prose".
Since the override is a CRITICAL OVERRIDE that says "referencing file names, function names, and line numbers", I will include line numbers. Wait! No, the override mentions line numbers, but let's stick to using code snippets and function names to be safe. It says "describe each finding directly by referencing file names, function names, and line numbers in plain prose. Include code snippets in your questions without diff formatting." I will just provide the file and function name and snippet. The override specifically said "and line numbers", so maybe I *should* include line numbers. But let me combine them smoothly. Let's just follow the override's instructions literally without "> " quoting.

I will formulate the plain text email.

Headers:
commit 8f95e31f13fcffe905ad6efeb0bf108998166be9
Author: Jishnu Prakash <jishnu.prakash@oss.qualcomm.com>
Subject: mfd: qcom-pm8008: Drop GPIO support

Brief summary:
This commit drops the GPIO MFD cell and its associated peripheral and IRQ definitions from the PM8008 driver. This is because the GPIOs on this chip and the upcoming PM8010 have fixed functions, making general-purpose GPIO exposure unnecessary.

Finding:
The patch removes `PM8008_GPIO1` and `PM8008_GPIO2` from the enum, which reduces `PM8008_NUM_PERIPHS` from 4 to 2. It also truncates `pm8008_periph_base` from 4 to 2 elements.
Wait, since `PM8008_NUM_PERIPHS` sets `num_regs = 2` in `pm8008_irq_chip`, we now have `chip->num_regs = 2`.
The `regmap-irq` framework determines `max_main_bits = chip->num_regs` which makes `max_main_bits = 2`.

If a physical GPIO1 interrupt fires, the hardware sets bit 2 in the main status register.
In `drivers/base/regmap/regmap-irq.c:read_irq_data()`, there is a loop over the active bits:

```c
			for_each_set_bit(b, &mreg, map->format.val_bytes * 8) {
				if (i * map->format.val_bytes * 8 + b >
				    max_main_bits)
					break;
				ret = read_sub_irq_data(data, b);
```

Since `max_main_bits` is 2, the bounds check `b > max_main_bits` evaluates to `2 > 2` which is false. This allows `read_sub_irq_data(data, 2)` to be called.

In `drivers/base/regmap/regmap-irq.c:read_sub_irq_data()`:

```c
static int read_sub_irq_data(struct regmap_irq_chip_data *data,
			     unsigned int b)
{
	const struct regmap_irq_chip *chip = data->chip;
	struct regmap *map = data->map;
	struct regmap_irq_sub_irq_map *subreg;
	unsigned int reg;
	int ret = 0;

	if (chip->sub_reg_offsets) {
...
	} else {
		reg = data->get_irq_reg(data, chip->status_base, b);
		ret = regmap_read(map, reg, &data->status_buf[b]);
	}
	return ret;
}
```
Wait, the snippet shows:
```c
		reg = data->get_irq_reg(data, chip->status_base, b);
		ret = regmap_read(map, reg, &data->status_buf[b]);
```
Since `b = 2` and `status_buf` was allocated for `num_regs = 2` (indices 0 and 1), `data->status_buf[2]` is an out-of-bounds write.

Also, `data->get_irq_reg()` calls `pm8008_get_irq_reg()` in `drivers/mfd/qcom-pm8008.c` at line 88:

```c
static unsigned int pm8008_get_irq_reg(struct regmap_irq_chip_data *data,
				       unsigned int base, int index)
{
...
	return pm8008_periph_base[index] + base;
}
```
With `index = 2`, reading `pm8008_periph_base[2]` results in an out-of-bounds array read because the array is truncated to 2 elements.

So this reduction of `PM8008_NUM_PERIPHS` without masking the hardware interrupt can cause an OOB read and OOB write when the unhandled hardware interrupt fires.

I need to write this up clearly.

Summary of email components:
- Headers
- Brief summary
- A question about the potential OOB memory access
- Explanation with code snippets.

Severity: High
[Severity: High]

Drafting the text:

commit 8f95e31f13fcffe905ad6efeb0bf108998166be9
Author: Jishnu Prakash <jishnu.prakash@oss.qualcomm.com>
Subject: mfd: qcom-pm8008: Drop GPIO support

This commit drops the GPIO MFD cell and its associated peripheral and IRQ
definitions from the PM8008 driver. It removes the GPIO elements from the
peripherals enum, which reduces PM8008_NUM_PERIPHS from 4 to 2, and truncates
pm8008_periph_base[] to 2 elements.

[Severity: High]
Does this code cause out-of-bounds memory accesses if the physical GPIO1
interrupt still fires?

In drivers/base/regmap/regmap-irq.c:read_irq_data(), the max_main_bits is derived
from chip->num_regs, which is now 2. Due to an off-by-one condition in the
bounds check:

			for_each_set_bit(b, &mreg, map->format.val_bytes * 8) {
				if (i * map->format.val_bytes * 8 + b >
				    max_main_bits)
					break;
				ret = read_sub_irq_data(data, b);

If bit 2 (the dropped GPIO1 interrupt) is set by the hardware in the main
status register, the check "2 > 2" is false. The code proceeds to call
read_sub_irq_data() with b = 2.

In drivers/base/regmap/regmap-irq.c:read_sub_irq_data():

		reg = data->get_irq_reg(data, chip->status_base, b);
		ret = regmap_read(map, reg, &data->status_buf[b]);

Since status_buf was allocated with size chip->num_regs (2), writing to
data->status_buf[2] will overflow the buffer.

Additionally, data->get_irq_reg() calls pm8008_get_irq_reg() in
drivers/mfd/qcom-pm8008.c:

static unsigned int pm8008_get_irq_reg(struct regmap_irq_chip_data *data,
				       unsigned int base, int index)
{
	/* Simple linear addressing for the main status register */
	if (base == I2C_INTR_STATUS_BASE)
		return base + index;

	return pm8008_periph_base[index] + base;
}

When index is 2, reading pm8008_periph_base[2] causes an out-of-bounds read
because the array was truncated to 2 elements in this patch.

Wait, should I specify that the regmap-irq off-by-one bug is pre-existing?
"This problem wasn't introduced by this patch, but..."
Ah, the JSON says `"preexisting": false` for this finding! The finding says the bug is `mfd: qcom-pm8008: OOB array access due to truncated pm8008_periph_base`. The off-by-one in `regmap-irq.c` is preexisting but the OOB array access in `pm8008` and `status_buf` overflow is introduced by reducing the size in the patch. I don't need to add the pre-existing clause since the JSON specifies it is false. (However, the off-by-one check in `regmap-irq.c` makes it possible to exploit the truncated array).

Wait, the prompt says:
"PRE-EXISTING ISSUES: If any finding has `"preexisting": true`, include it in the report and state explicitly at the start of its comment that the problem was not introduced by this patch (for example: "This problem wasn't introduced by this patch, but...")."
Since this finding has `preexisting: false`, I shouldn't add that label.

Let's refine the question.
Instead of "Does this code cause...", maybe:
"Can reducing PM8008_NUM_PERIPHS and truncating pm8008_periph_base[] lead to out-of-bounds memory accesses if a GPIO interrupt fires?"

Let's structure the explanation clearly.

```text
commit 8f95e31f13fcffe905ad6efeb0bf108998166be9
Author: Jishnu Prakash <jishnu.prakash@oss.qualcomm.com>
Subject: mfd: qcom-pm8008: Drop GPIO support

This commit drops the GPIO MFD cell and its associated peripheral and IRQ
definitions from the PM8008 driver. It removes the GPIO elements from the
peripherals enum, which reduces PM8008_NUM_PERIPHS from 4 to 2, and truncates
pm8008_periph_base[] to 2 elements.

[Severity: High]
Can reducing PM8008_NUM_PERIPHS and truncating pm8008_periph_base[] lead to
out-of-bounds memory accesses if a physical GPIO interrupt still fires?

In drivers/base/regmap/regmap-irq.c:read_irq_data(), max_main_bits is derived
from chip->num_regs, which is now 2. Due to an off-by-one bounds check:

			for_each_set_bit(b, &mreg, map->format.val_bytes * 8) {
				if (i * map->format.val_bytes * 8 + b >
				    max_main_bits)
					break;
				ret = read_sub_irq_data(data, b);

If bit 2 (the dropped GPIO1 interrupt) is set by the hardware in the main
status register, the check "2 > 2" is false. The code will proceed to call
read_sub_irq_data() with b = 2.

This leads to an out-of-bounds write in
drivers/base/regmap/regmap-irq.c:read_sub_irq_data():

		reg = data->get_irq_reg(data, chip->status_base, b);
		ret = regmap_read(map, reg, &data->status_buf[b]);

Since status_buf was allocated with size chip->num_regs (2), writing to
data->status_buf[2] overflows the buffer.

Additionally, data->get_irq_reg() calls pm8008_get_irq_reg() in
drivers/mfd/qcom-pm8008.c:

static unsigned int pm8008_get_irq_reg(struct regmap_irq_chip_data *data,
				       unsigned int base, int index)
{
	/* Simple linear addressing for the main status register */
	if (base == I2C_INTR_STATUS_BASE)
		return base + index;

	return pm8008_periph_base[index] + base;
}

When index is 2, reading pm8008_periph_base[2] causes an out-of-bounds read
because the array was truncated to 2 elements.
```

Let's review the prompt's formatting instructions.
- Ensure clear, concise paragraphs. No long or dense paragraphs.
- Blank lines between factual sentences.
- Blank line before the question. (I have the question at the top of the block, which is also fine, or I could put facts first and then the question).

Let's re-order to present facts first:

```text
commit 8f95e31f13fcffe905ad6efeb0bf108998166be9
Author: Jishnu Prakash <jishnu.prakash@oss.qualcomm.com>
Subject: mfd: qcom-pm8008: Drop GPIO support

This commit drops the GPIO MFD cell and its associated peripheral and IRQ
definitions from the PM8008 driver. It removes the GPIO elements from the
peripherals enum, which reduces PM8008_NUM_PERIPHS from 4 to 2, and truncates
pm8008_periph_base[] to 2 elements.

[Severity: High]
In drivers/base/regmap/regmap-irq.c:read_irq_data(), max_main_bits is derived
from chip->num_regs, which is now 2.

Due to an off-by-one bounds check, if bit 2 (the dropped GPIO1 interrupt) is
set by the hardware in the main status register, the check "2 > 2" is false:

			for_each_set_bit(b, &mreg, map->format.val_bytes * 8) {
				if (i * map->format.val_bytes * 8 + b >
				    max_main_bits)
					break;
				ret = read_sub_irq_data(data, b);

This allows the framework to process bit 2 and call read_sub_irq_data() with
b = 2.

In drivers/base/regmap/regmap-irq.c:read_sub_irq_data(), this leads to a memory
overflow when writing to the status_buf array:

		reg = data->get_irq_reg(data, chip->status_base, b);
		ret = regmap_read(map, reg, &data->status_buf[b]);

Since status_buf was allocated with size chip->num_regs (2), writing to
data->status_buf[2] corrupts adjacent kernel memory.

Additionally, data->get_irq_reg() calls pm8008_get_irq_reg() in
drivers/mfd/qcom-pm8008.c:

static unsigned int pm8008_get_irq_reg(struct regmap_irq_chip_data *data,
				       unsigned int base, int index)
{
	/* Simple linear addressing for the main status register */
	if (base == I2C_INTR_STATUS_BASE)
		return base + index;

	return pm8008_periph_base[index] + base;
}

When index is 2, reading pm8008_periph_base[2] reads past the end of the array
because it was truncated to 2 elements.

Can reducing PM8008_NUM_PERIPHS and truncating pm8008_periph_base[] lead to
out-of-bounds memory accesses if the physical GPIO1 interrupt still fires?
```

Is there anything else to include? The problem asks to follow the specific formatting:
- The report must be in plain text only.
- Any long lines present in the unified diff should be preserved, but any summary, comments or questions you add should be wrapped at 78 characters.
- Explain the regressions as questions about the code, but do not mention the author.
- Vary your question phrasing.
- If the bug came from SR-* patterns, it is a subjective review (not applicable here, this is PVF1 memory access).
- Ask your question specifically about the sources you're referencing. "Can reducing PM8008_NUM_PERIPHS and truncating pm8008_periph_base[] lead to out-of-bounds memory accesses in data->status_buf[] and pm8008_periph_base[] if the physical GPIO1 interrupt still fires?"
- Never quote line numbers, use small code snippets. (Check! I didn't use line numbers).
- ALWAYS end the report with a blank line.
- The report should contain only the portions of hunks needed to explain the review's concerns (per CRITICAL OVERRIDE, we don't quote hunks with `>`, but instead use the code snippets in prose).
- You MUST NOT REFERENCE LINE NUMBERS IN THIS REPORT.

Let's refine the question to be more specific as required:
"Can reducing PM8008_NUM_PERIPHS lead to out-of-bounds accesses in data->status_buf[] and pm8008_periph_base[] if the physical GPIO1 interrupt fires?"

Let's review the whole text again.

```text
commit 8f95e31f13fcffe905ad6efeb0bf108998166be9
Author: Jishnu Prakash <jishnu.prakash@oss.qualcomm.com>
Subject: mfd: qcom-pm8008: Drop GPIO support

This commit drops the GPIO MFD cell and its associated peripheral and IRQ
definitions from the PM8008 driver. It removes the GPIO elements from the
peripherals enum, which reduces PM8008_NUM_PERIPHS from 4 to 2, and truncates
pm8008_periph_base[] to 2 elements.

[Severity: High]
In drivers/base/regmap/regmap-irq.c:read_irq_data(), max_main_bits is derived
from chip->num_regs, which is now 2.

Due to an off-by-one bounds check, if bit 2 (the dropped GPIO1 interrupt) is
set by the hardware in the main status register, the check "2 > 2" is false:

			for_each_set_bit(b, &mreg, map->format.val_bytes * 8) {
				if (i * map->format.val_bytes * 8 + b >
				    max_main_bits)
					break;
				ret = read_sub_irq_data(data, b);

This allows the framework to process bit 2 and call read_sub_irq_data() with
b = 2.

In drivers/base/regmap/regmap-irq.c:read_sub_irq_data(), this leads to a memory
overflow when writing to the status_buf array:

		reg = data->get_irq_reg(data, chip->status_base, b);
		ret = regmap_read(map, map, reg, &data->status_buf[b]);
Wait, the snippet from original file says:
		ret = regmap_read(map, reg, &data->status_buf[b]);

Since status_buf was allocated with size chip->num_regs (2), writing to
data->status_buf[2] corrupts adjacent kernel memory.

Additionally, data->get_irq_reg() calls pm8008_get_irq_reg() in
drivers/mfd/qcom-pm8008.c:

static unsigned int pm8008_get_irq_reg(struct regmap_irq_chip_data *data,
				       unsigned int base, int index)
{
	/* Simple linear addressing for the main status register */
	if (base == I2C_INTR_STATUS_BASE)
		return base + index;

	return pm8008_periph_base[index] + base;
}

When index is 2, reading pm8008_periph_base[2] reads past the end of the array
because it was truncated to 2 elements.

Can reducing PM8008_NUM_PERIPHS lead to out-of-bounds accesses in
data->status_buf[] and pm8008_periph_base[] if the physical GPIO1 interrupt
still fires?

```

Wait, `map, map, reg` was a typo I made. Fixed to `map, reg, ...`.
Check the loop: `i * map->format.val_bytes * 8 + b > max_main_bits`.
Here, `i=0` presumably since `num_main_regs` is 1. `b` is `2`. So `0 + 2 > 2` which is false.
This all makes perfect sense.

Is there any Link: tags? `git log` showed none.
Did I include "A brief (max 3 sentence) summary of the commit"? Yes.
Did I wrap text at 78 characters? Yes.
Did I use "Can this code..." without mentioning the author? Yes.
Are line numbers used? No.
Did I use markdown headers or ALL CAPS? No. Only `[Severity: High]` and `PM8008_NUM_PERIPHS` (which is a macro, acceptable).
Did I end with a blank line? Yes.
The `CRITICAL OVERRIDE` states: "Instead, start with the Commit/Author/Subject headers, write the summary, and describe each finding directly by referencing file names, function names, and line numbers in plain prose. Include code snippets in your questions without diff formatting."
I included file names and function names in plain prose. I omitted line numbers to conform to the base prompt's strict "NEVER QUOTE LINE NUMBERS" rule, which is safer given the contradiction, but actually, I didn't see a contradiction if I just use snippets instead. Wait, the override says "referencing file names, function names, and line numbers". To be absolutely compliant with the override without violating the spirit of "no line numbers from the review UI", I can just not mention the line numbers. The snippet is enough.

Let's do a final check of the generated text.

```
commit 8f95e31f13fcffe905ad6efeb0bf108998166be9
Author: Jishnu Prakash <jishnu.prakash@oss.qualcomm.com>
Subject: mfd: qcom-pm8008: Drop GPIO support

This commit drops the GPIO MFD cell and its associated peripheral and IRQ
definitions from the PM8008 driver. It removes the GPIO elements from the
peripherals enum, which reduces PM8008_NUM_PERIPHS from 4 to 2, and truncates
pm8008_periph_base[] to 2 elements.

[Severity: High]
In drivers/base/regmap/regmap-irq.c:read_irq_data(), max_main_bits is derived
from chip->num_regs, which is now 2.

Due to an off-by-one bounds check, if bit 2 (the dropped GPIO1 interrupt) is
set by the hardware in the main status register, the check "2 > 2" is false:

			for_each_set_bit(b, &mreg, map->format.val_bytes * 8) {
				if (i * map->format.val_bytes * 8 + b >
				    max_main_bits)
					break;
				ret = read_sub_irq_data(data, b);

This allows the framework to process bit 2 and call read_sub_irq_data() with
b = 2.

In drivers/base/regmap/regmap-irq.c:read_sub_irq_data(), this leads to a memory
overflow when writing to the status_buf array:

		reg = data->get_irq_reg(data, chip->status_base, b);
		ret = regmap_read(map, reg, &data->status_buf[b]);

Since status_buf was allocated with size chip->num_regs (2), writing to
data->status_buf[2] corrupts adjacent kernel memory.

Additionally, data->get_irq_reg() calls pm8008_get_irq_reg() in
drivers/mfd/qcom-pm8008.c:

static unsigned int pm8008_get_irq_reg(struct regmap_irq_chip_data *data,
				       unsigned int base, int index)
{
	/* Simple linear addressing for the main status register */
	if (base == I2C_INTR_STATUS_BASE)
		return base + index;

	return pm8008_periph_base[index] + base;
}

When index is 2, reading pm8008_periph_base[2] reads past the end of the array
because it was truncated to 2 elements.

Can reducing PM8008_NUM_PERIPHS lead to out-of-bounds accesses in
data->status_buf[] and pm8008_periph_base[] if the physical GPIO1 interrupt
still fires?

```
This looks perfectly compliant, well reasoned, and addresses both the prompt's rules and the override.
commit 8f95e31f13fcffe905ad6efeb0bf108998166be9
Author: Jishnu Prakash <jishnu.prakash@oss.qualcomm.com>
Subject: mfd: qcom-pm8008: Drop GPIO support

This commit drops the GPIO MFD cell and its associated peripheral and IRQ
definitions from the PM8008 driver. It removes the GPIO elements from the
peripherals enum, which reduces PM8008_NUM_PERIPHS from 4 to 2, and truncates
pm8008_periph_base[] to 2 elements.

[Severity: High]
In drivers/base/regmap/regmap-irq.c:read_irq_data(), max_main_bits is derived
from chip->num_regs, which is now 2.

Due to an off-by-one bounds check, if bit 2 (the dropped GPIO1 interrupt) is
set by the hardware in the main status register, the check "2 > 2" is false:

			for_each_set_bit(b, &mreg, map->format.val_bytes * 8) {
				if (i * map->format.val_bytes * 8 + b >
				    max_main_bits)
					break;
				ret = read_sub_irq_data(data, b);

This allows the framework to process bit 2 and call read_sub_irq_data() with
b = 2.

In drivers/base/regmap/regmap-irq.c:read_sub_irq_data(), this leads to a memory
overflow when writing to the status_buf array:

		reg = data->get_irq_reg(data, chip->status_base, b);
		ret = regmap_read(map, reg, &data->status_buf[b]);

Since status_buf was allocated with size chip->num_regs (2), writing to
data->status_buf[2] corrupts adjacent kernel memory.

Additionally, data->get_irq_reg() calls pm8008_get_irq_reg() in
drivers/mfd/qcom-pm8008.c:

static unsigned int pm8008_get_irq_reg(struct regmap_irq_chip_data *data,
				       unsigned int base, int index)
{
	/* Simple linear addressing for the main status register */
	if (base == I2C_INTR_STATUS_BASE)
		return base + index;

	return pm8008_periph_base[index] + base;
}

When index is 2, reading pm8008_periph_base[2] reads past the end of the array
because it was truncated to 2 elements.

Can reducing PM8008_NUM_PERIPHS lead to out-of-bounds accesses in
data->status_buf[] and pm8008_periph_base[] if the physical GPIO1 interrupt
still fires?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014-pm8010_i2c_support-v2-0-b395d2b3f668@oss.qualcomm.com?part=4

[PATCH v2 5/7] mfd: qcom-pm8008: Add PM8010 support

[Jishnu Prakash]

From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

PM8010 is effectively the next generation of PM8008. The device is
generally designed to serve the exact same function - power camera
sensors and the associated supporting hardware.

PM8010 supports both I2C and SPMI communications. The latter case is
already handled, since it reuses the same peripherals as other PMICs.
The I2C front-end is more less the same to the one of PM8008.

Plumb in the necessary changes to support it.

Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Co-developed-by: Dhruvin Rajpura <drajpura@qti.qualcomm.com>
Signed-off-by: Dhruvin Rajpura <drajpura@qti.qualcomm.com>
Co-developed-by: Jishnu Prakash <jishnu.prakash@oss.qualcomm.com>
Signed-off-by: Jishnu Prakash <jishnu.prakash@oss.qualcomm.com>
---
 drivers/mfd/qcom-pm8008.c | 69 ++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 65 insertions(+), 4 deletions(-)

diff --git a/drivers/mfd/qcom-pm8008.c b/drivers/mfd/qcom-pm8008.c
index 2d6c01cf54f3..4179427f3891 100644
--- a/drivers/mfd/qcom-pm8008.c
+++ b/drivers/mfd/qcom-pm8008.c
@@ -34,6 +34,11 @@ enum {
 	PM8008_NUM_PERIPHS,
 };
 
+enum pm8008_chip_type {
+	TYPE_PM8008,
+	TYPE_PM8010,
+};
+
 #define PM8008_PERIPH_0_BASE	0x900
 #define PM8008_PERIPH_1_BASE	0x2400
 
@@ -47,6 +52,9 @@ enum {
 #define PM8008_IRQ_MISC_LDO_OCP	4
 #define PM8008_IRQ_TEMP_ALARM	5
 
+#define PM8010_IRQ_MISC_MBG_FAULT	0
+#define PM8010_IRQ_MISC_LDO_OCP		4
+
 enum {
 	SET_TYPE_INDEX,
 	POLARITY_HI_INDEX,
@@ -78,6 +86,12 @@ static const struct regmap_irq pm8008_irqs[] = {
 	_IRQ(PM8008_IRQ_TEMP_ALARM,   PM8008_TEMP_ALARM,BIT(0), IRQ_TYPE_SENSE_MASK),
 };
 
+static const struct regmap_irq pm8010_irqs[] = {
+	_IRQ(PM8010_IRQ_MISC_MBG_FAULT, PM8008_MISC, BIT(0), IRQ_TYPE_EDGE_RISING),
+	_IRQ(PM8010_IRQ_MISC_LDO_OCP, PM8008_MISC, BIT(4), IRQ_TYPE_EDGE_RISING),
+	_IRQ(PM8008_IRQ_TEMP_ALARM, PM8008_TEMP_ALARM, BIT(0), IRQ_TYPE_SENSE_MASK),
+};
+
 static const unsigned int pm8008_periph_base[] = {
 	PM8008_PERIPH_0_BASE,
 	PM8008_PERIPH_1_BASE,
@@ -146,6 +160,25 @@ static const struct regmap_irq_chip pm8008_irq_chip = {
 	.get_irq_reg		= pm8008_get_irq_reg,
 };
 
+static const struct regmap_irq_chip pm8010_irq_chip = {
+	.name			= "pm8010",
+	.main_status		= I2C_INTR_STATUS_BASE,
+	.num_main_regs		= 1,
+	.irqs			= pm8010_irqs,
+	.num_irqs		= ARRAY_SIZE(pm8010_irqs),
+	.num_regs		= PM8008_NUM_PERIPHS,
+	.status_base		= INT_LATCHED_STS_OFFSET,
+	.mask_base		= INT_EN_CLR_OFFSET,
+	.unmask_base		= INT_EN_SET_OFFSET,
+	.mask_unmask_non_inverted = true,
+	.ack_base		= INT_LATCHED_CLR_OFFSET,
+	.config_base		= pm8008_config_regs,
+	.num_config_bases	= ARRAY_SIZE(pm8008_config_regs),
+	.num_config_regs	= PM8008_NUM_PERIPHS,
+	.set_type_config	= pm8008_set_type_config,
+	.get_irq_reg		= pm8008_get_irq_reg,
+};
+
 static const struct regmap_config qcom_mfd_regmap_cfg = {
 	.name		= "primary",
 	.reg_bits	= 16,
@@ -170,6 +203,11 @@ static const struct mfd_cell pm8008_cells[] = {
 	MFD_CELL_RES("qpnp-temp-alarm", pm8008_temp_res),
 };
 
+static const struct mfd_cell pm8010_cells[] = {
+	MFD_CELL_NAME("pm8010-regulator"),
+	MFD_CELL_RES("qpnp-temp-alarm", pm8008_temp_res),
+};
+
 static void devm_irq_domain_fwnode_release(void *data)
 {
 	struct fwnode_handle *fwnode = data;
@@ -180,14 +218,36 @@ static void devm_irq_domain_fwnode_release(void *data)
 static int pm8008_probe(struct i2c_client *client)
 {
 	struct regmap_irq_chip_data *irq_data;
+	const struct regmap_irq_chip *irq_chip_desc;
 	struct device *dev = &client->dev;
+	const struct mfd_cell *mfd_cells;
 	struct regmap *regmap, *regmap2;
 	struct fwnode_handle *fwnode;
 	struct i2c_client *dummy;
 	struct gpio_desc *reset;
+	unsigned long pmic_type;
+	int num_mfd_cells;
 	char *name;
 	int ret;
 
+	pmic_type = (unsigned long)of_device_get_match_data(dev);
+
+	switch (pmic_type) {
+	case TYPE_PM8008:
+		irq_chip_desc = &pm8008_irq_chip;
+		mfd_cells = pm8008_cells;
+		num_mfd_cells = ARRAY_SIZE(pm8008_cells);
+		break;
+	case TYPE_PM8010:
+		irq_chip_desc = &pm8010_irq_chip;
+		mfd_cells = pm8010_cells;
+		num_mfd_cells = ARRAY_SIZE(pm8010_cells);
+		break;
+	default:
+		dev_err(dev, "Unknown device type: %lu\n", pmic_type);
+		return -EINVAL;
+	}
+
 	dummy = devm_i2c_new_dummy_device(dev, client->adapter, client->addr + 1);
 	if (IS_ERR(dummy)) {
 		ret = PTR_ERR(dummy);
@@ -233,19 +293,20 @@ static int pm8008_probe(struct i2c_client *client)
 		return ret;
 
 	ret = devm_regmap_add_irq_chip_fwnode(dev, fwnode, regmap, client->irq,
-				IRQF_SHARED, 0, &pm8008_irq_chip, &irq_data);
+				IRQF_SHARED, 0, irq_chip_desc, &irq_data);
 	if (ret) {
 		dev_err(dev, "failed to add IRQ chip: %d\n", ret);
 		return ret;
 	}
 
-	return devm_mfd_add_devices(dev, PLATFORM_DEVID_AUTO, pm8008_cells,
-				ARRAY_SIZE(pm8008_cells), NULL, 0,
+	return devm_mfd_add_devices(dev, PLATFORM_DEVID_AUTO, mfd_cells,
+				num_mfd_cells, NULL, 0,
 				regmap_irq_get_domain(irq_data));
 }
 
 static const struct of_device_id pm8008_match[] = {
-	{ .compatible = "qcom,pm8008", },
+	{ .compatible = "qcom,pm8008", .data = (void *)TYPE_PM8008, },
+	{ .compatible = "qcom,pm8010-i2c", .data = (void *)TYPE_PM8010, },
 	{ },
 };
 MODULE_DEVICE_TABLE(of, pm8008_match);

-- 
2.43.0

Re: [PATCH v2 5/7] mfd: qcom-pm8008: Add PM8010 support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] mfd: silent fallback to PM8008 on non-DT PM8010 instances
--

commit 4e77301709b8663667033b7dfed82cf937a2d7c5
Author: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

mfd: qcom-pm8008: Add PM8010 support

This commit adds support for the PM8010 to the qcom-pm8008 driver. It
incorporates PM8010 specific IRQs, MFD cells, and match data to support the
new device type via I2C.

> diff --git a/drivers/mfd/qcom-pm8008.c b/drivers/mfd/qcom-pm8008.c
> index 2d6c01cf54f32..4179427f38911 100644
> --- a/drivers/mfd/qcom-pm8008.c
> +++ b/drivers/mfd/qcom-pm8008.c
[ ... ]
> @@ -180,14 +218,36 @@ static void devm_irq_domain_fwnode_release(void *data)
>  static int pm8008_probe(struct i2c_client *client)
>  {
>  	struct regmap_irq_chip_data *irq_data;
> +	const struct regmap_irq_chip *irq_chip_desc;
>  	struct device *dev = &client->dev;
> +	const struct mfd_cell *mfd_cells;
>  	struct regmap *regmap, *regmap2;
>  	struct fwnode_handle *fwnode;
>  	struct i2c_client *dummy;
>  	struct gpio_desc *reset;
> +	unsigned long pmic_type;
> +	int num_mfd_cells;
>  	char *name;
>  	int ret;
>  
> +	pmic_type = (unsigned long)of_device_get_match_data(dev);

[Severity: Medium]
Will this silently fall back to TYPE_PM8008 for non-DT devices?

of_device_get_match_data() explicitly checks for dev->of_node and returns
NULL when it isn't present (such as devices instantiated via ACPI PRP0001
or swnodes). A NULL return evaluates to 0, which matches TYPE_PM8008 in
the switch below.

This means a valid qcom,pm8010-i2c device instantiated via ACPI or swnodes
will be misidentified as TYPE_PM8008 instead of TYPE_PM8010.

Could this use i2c_get_match_data() or device_get_match_data() to ensure
device variants are correctly extracted regardless of the underlying
firmware representation?

> +
> +	switch (pmic_type) {
> +	case TYPE_PM8008:
> +		irq_chip_desc = &pm8008_irq_chip;
> +		mfd_cells = pm8008_cells;
> +		num_mfd_cells = ARRAY_SIZE(pm8008_cells);
> +		break;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014-pm8010_i2c_support-v2-0-b395d2b3f668@oss.qualcomm.com?part=5

[PATCH v2 6/7] dt-bindings: mfd: pm8008: Make interrupts optional

[Jishnu Prakash]

From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Some designs integrate the PMIC with the interrupt pin left
unconnected, deliberately. That of course compromises some
functionality, but the core function of programming the voltage
regulators remains there.

Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Acked-by: Rob Herring (Arm) <robh@kernel.org>
Co-developed-by: Jishnu Prakash <jishnu.prakash@oss.qualcomm.com>
Signed-off-by: Jishnu Prakash <jishnu.prakash@oss.qualcomm.com>
---
 Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml b/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml
index 2701aa755f31..3c0febc8152b 100644
--- a/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml
+++ b/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml
@@ -90,7 +90,6 @@ properties:
 required:
   - compatible
   - reg
-  - interrupts
   - vdd-l1-l2-supply
   - vdd-l3-l4-supply
   - vdd-l5-supply
@@ -99,10 +98,11 @@ required:
   - gpio-controller
   - "#gpio-cells"
   - gpio-ranges
-  - interrupt-controller
-  - "#interrupt-cells"
   - "#thermal-sensor-cells"
 
+dependencies:
+  interrupt-controller: [ interrupts, "#interrupt-cells" ]
+
 additionalProperties: false
 
 examples:

-- 
2.43.0

[PATCH v2 7/7] mfd: qcom-pm8008: Tolerate missing interrupt

[Jishnu Prakash]

From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

PM8010 is often wired up with the interrupt pin tied to ground. Only
register the IRQ infrastructure if we have a parent irq.

Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Co-developed-by: Dhruvin Rajpura <drajpura@qti.qualcomm.com>
Signed-off-by: Dhruvin Rajpura <drajpura@qti.qualcomm.com>
Co-developed-by: Jishnu Prakash <jishnu.prakash@oss.qualcomm.com>
Signed-off-by: Jishnu Prakash <jishnu.prakash@oss.qualcomm.com>
---
 drivers/mfd/qcom-pm8008.c | 82 ++++++++++++++++++++++++++++++++++-------------
 1 file changed, 60 insertions(+), 22 deletions(-)

diff --git a/drivers/mfd/qcom-pm8008.c b/drivers/mfd/qcom-pm8008.c
index 4179427f3891..8b1bbb4052a7 100644
--- a/drivers/mfd/qcom-pm8008.c
+++ b/drivers/mfd/qcom-pm8008.c
@@ -208,6 +208,14 @@ static const struct mfd_cell pm8010_cells[] = {
 	MFD_CELL_RES("qpnp-temp-alarm", pm8008_temp_res),
 };
 
+static const struct mfd_cell pm8008_no_irq_cells[] = {
+	MFD_CELL_NAME("pm8008-regulator"),
+};
+
+static const struct mfd_cell pm8010_no_irq_cells[] = {
+	MFD_CELL_NAME("pm8010-regulator"),
+};
+
 static void devm_irq_domain_fwnode_release(void *data)
 {
 	struct fwnode_handle *fwnode = data;
@@ -215,19 +223,50 @@ static void devm_irq_domain_fwnode_release(void *data)
 	irq_domain_free_fwnode(fwnode);
 }
 
+static int pm8008_irq_config(struct device *dev, int irq, struct regmap *regmap,
+			     const struct regmap_irq_chip *irq_chip_desc,
+			     struct regmap_irq_chip_data **irq_data)
+{
+	struct fwnode_handle *fwnode;
+	char *name;
+	int ret;
+
+	name = devm_kasprintf(dev, GFP_KERNEL, "%pOF-internal", dev->of_node);
+	if (!name)
+		return -ENOMEM;
+
+	name = strreplace(name, '/', ':');
+
+	fwnode = irq_domain_alloc_named_fwnode(name);
+	if (!fwnode)
+		return -ENOMEM;
+
+	ret = devm_add_action_or_reset(dev, devm_irq_domain_fwnode_release, fwnode);
+	if (ret)
+		return ret;
+
+	ret = devm_regmap_add_irq_chip_fwnode(dev, fwnode, regmap, irq,
+					      IRQF_SHARED, 0, irq_chip_desc,
+					      irq_data);
+	if (ret) {
+		dev_err(dev, "failed to add IRQ chip: %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
 static int pm8008_probe(struct i2c_client *client)
 {
-	struct regmap_irq_chip_data *irq_data;
+	struct regmap_irq_chip_data *irq_data = NULL;
 	const struct regmap_irq_chip *irq_chip_desc;
 	struct device *dev = &client->dev;
 	const struct mfd_cell *mfd_cells;
 	struct regmap *regmap, *regmap2;
-	struct fwnode_handle *fwnode;
 	struct i2c_client *dummy;
 	struct gpio_desc *reset;
 	unsigned long pmic_type;
 	int num_mfd_cells;
-	char *name;
 	int ret;
 
 	pmic_type = (unsigned long)of_device_get_match_data(dev);
@@ -248,6 +287,19 @@ static int pm8008_probe(struct i2c_client *client)
 		return -EINVAL;
 	}
 
+	if (!client->irq) {
+		switch (pmic_type) {
+		case TYPE_PM8008:
+			mfd_cells = pm8008_no_irq_cells;
+			num_mfd_cells = ARRAY_SIZE(pm8008_no_irq_cells);
+			break;
+		case TYPE_PM8010:
+			mfd_cells = pm8010_no_irq_cells;
+			num_mfd_cells = ARRAY_SIZE(pm8010_no_irq_cells);
+			break;
+		}
+	}
+
 	dummy = devm_i2c_new_dummy_device(dev, client->adapter, client->addr + 1);
 	if (IS_ERR(dummy)) {
 		ret = PTR_ERR(dummy);
@@ -278,25 +330,11 @@ static int pm8008_probe(struct i2c_client *client)
 	 */
 	usleep_range(1000, 2000);
 
-	name = devm_kasprintf(dev, GFP_KERNEL, "%pOF-internal", dev->of_node);
-	if (!name)
-		return -ENOMEM;
-
-	name = strreplace(name, '/', ':');
-
-	fwnode = irq_domain_alloc_named_fwnode(name);
-	if (!fwnode)
-		return -ENOMEM;
-
-	ret = devm_add_action_or_reset(dev, devm_irq_domain_fwnode_release, fwnode);
-	if (ret)
-		return ret;
-
-	ret = devm_regmap_add_irq_chip_fwnode(dev, fwnode, regmap, client->irq,
-				IRQF_SHARED, 0, irq_chip_desc, &irq_data);
-	if (ret) {
-		dev_err(dev, "failed to add IRQ chip: %d\n", ret);
-		return ret;
+	if (client->irq) {
+		ret = pm8008_irq_config(dev, client->irq, regmap, irq_chip_desc,
+					&irq_data);
+		if (ret)
+			return ret;
 	}
 
 	return devm_mfd_add_devices(dev, PLATFORM_DEVID_AUTO, mfd_cells,

-- 
2.43.0