Skip to contentSkip to conversation

Add support for Qualcomm MFD PMIC register layout

RFC PATCH Superseded Guru Das Srinagesh 2 participants 5 messages Oct 20, 2020 – Oct 21, 2020

Subsystems (5)

v1 of 5 Superseded Latest is v5 →

lore
Share & tools
lore
#0 lore raw

cover Add support for Qualcomm MFD PMIC register layout

(Re-sending to add devicetree mailing list)

This is a follow-up as promised [1] to the earlier attempts [2] [3] to upstream the driver that has been hitherto used to handle IRQs for Qualcomm's PMICs that have multiple on-board peripherals when they are interfaced over the I2C interface.

This series is a rewrite of that driver while making use of the regmap-irq framework, which needs some modifications to handle the register layout of Qualcomm's PMICs. This is an RFC because I would like to get feedback on my general approach before submitting as a patch per se.

Upon inspection of the regmap-irq framework, it was observed that the downstream driver was essentially replicating the framework's IRQ handling logic (such as adding an IRQ domain, and the interrupt handler thread that reads sub-irqs from a main status register). It was also observed that the framework could not be used as-is because:
- Qualcomm's PMIC peripheral register layout does not follow a fixed irq_reg_stride, and
- The "IRQ TYPE" configuration register takes one bit per interrupt, which when set configures that interrupt as Edge triggered, and when cleared sets it to Level triggered.
- There are two IRQ configuration registers in addition to "IRQ TYPE" that further configure the IRQ type as triggered by rising-edge/level high or alternatively, falling-edge/level low that have no support in the regmap-irq framework currently.

This patch series has been tested on an internal platform using PM8008 as a test MFD PMIC chip. PM8008 is a PMIC that contains 7 LDOs, 2 GPIOs, temperature monitoring, and can be interfaced over I2C.

Both the framework modifications as well as the chip driver
have been submitted here for review. Some details about the specific differences between the framework and QCOM PMICs' register layout are provided below using PM8008 as an example.

[PM8008 peripheral register layout]

Of all the peripherals in PM8008, only a few need IRQ support. They are laid out at the following base addresses (only four are added at the moment for simplicity):

	0x0900, 0x2400, 0xC000, 0xC100

Each peripheral is allocated a uniform size of 0x100 bytes and its IRQs are configured through a set of registers that are located at fixed offsets from the above base addresses, uniformly:

	Register name	       Addr	regmap-irq equivalent	Comment
	-----------------------------------------------------------------------
	INT_RT_STS_OFFSET      0x10	(no equivalent)		See #1 below
	INT_SET_TYPE_OFFSET    0x11	type_base 		See #2 below
	INT_POL_HIGH_OFFSET    0x12	(no equivalent)		See #3 below
	INT_POL_LOW_OFFSET     0x13	(no equivalent)		See #3 below
	INT_LATCHED_CLR_OFFSET 0x14	ack_base
	INT_EN_SET_OFFSET      0x15	unmask_base		See #4 below
	INT_EN_CLR_OFFSET      0x16	mask_base		See #4 below
	INT_LATCHED_STS_OFFSET 0x18	status_base

Comments (all registers are one bit per interrupt):
1. INT_RT_STS_OFFSET is not used by the regmap-irq, so it may be ignored.
2. INT_SET_TYPE_OFFSET: 1 for edge trigger, 0 for level trigger.
3. Support needs to be added for writing to INT_POL_HIGH_OFFSET and INT_POL_LOW_OFFSET correctly in the framework. Set to 1 or 0 to enable or disable rising-edge/level high or falling-edge/level low.
4. Even though INT_EN_SET_OFFSET and INT_EN_CLR_OFFSET map to unmask_base and mask_base in the regmap-irq framework conceptually, they are swapped in the chip driver because unmask_offset in the framework expects unmask_base to be larger than mask_base numerically. This has to be kept in mind while reviewing the "mfd: Add PM8008 driver" patch below.

[Summary of framework changes]

The main thrust of the changes is to introduce an array of peripheral offset values, which are to be added to the *_base addresses in order to arrive at the correct register addresses per peripheral. In order to get at the first peripheral's addresses, the first element of this array must be zero.

Since there are two new registers (INT_POL_HIGH_OFFSET and INT_POL_LOW_OFFSET), add support for storing the per-peripheral values and also writing to them. These will be used only if peripheral offsets are specified.

[1] https://lore.kernel.org/lkml/[email protected]/
[2] https://lore.kernel.org/lkml/[email protected]/
[3] https://lore.kernel.org/lkml/[email protected]/

Guru Das Srinagesh (3):
regmap-irq: Add support for peripheral offsets
dt-bindings: mfd: Add QCOM PM8008 MFD bindings
mfd: Add PM8008 driver

 .../bindings/mfd/qcom,pm8008-irqchip.yaml          | 103 +++++++++++
 drivers/base/regmap/regmap-irq.c                   | 191 ++++++++++++++++----
 drivers/mfd/Kconfig                                |  14 ++
 drivers/mfd/Makefile                               |   1 +
 drivers/mfd/qcom-pm8008.c                          | 197 +++++++++++++++++++++
 include/linux/regmap.h                             |   6 +
 6 files changed, 478 insertions(+), 34 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mfd/qcom,pm8008-irqchip.yaml
 create mode 100644 drivers/mfd/qcom-pm8008.c
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
#1 lore raw

RFC PATCH 1/3 regmap-irq: Add support for peripheral offsets

Some MFD chips do not have the register space for their peripherals mapped out with a fixed stride. Add peripheral address offsets to the framework to support such address spaces.

In this new scheme, the regmap-irq client registering with the framework shall have to define the *_base registers (e.g. status_base, mask_base, type_base, etc.) as those of the very first peripheral in the chip, and then specify address offsets of each subsequent peripheral so that their corresponding *_base registers may be calculated by the framework. The first element of the periph_offs array must be zero so that the first peripherals' addresses may be accessed.

Some MFD chips define two registers in addition to the IRQ type
registers: POLARITY_HI and POLARITY_LO, so add support to manage their data as well as write to them.

---
 drivers/base/regmap/regmap-irq.c | 191 ++++++++++++++++++++++++++++++++-------
 include/linux/regmap.h           |   6 ++
 2 files changed, 163 insertions(+), 34 deletions(-)
drivers/base/regmap/regmap-irq.c +157 -34
index ad5c2de..dbf2c86 100644
@@ -38,6 +38,8 @@ struct regmap_irq_chip_data {
 	unsigned int *wake_buf;
 	unsigned int *type_buf;
 	unsigned int *type_buf_def;
+	unsigned int *polarity_hi_buf;
+	unsigned int *polarity_lo_buf;
 
 	unsigned int irq_reg_stride;
 	unsigned int type_reg_stride;
@@ -87,8 +89,13 @@ static void regmap_irq_sync_unlock(struct irq_data *data)
 
 	if (d->clear_status) {
 		for (i = 0; i < d->chip->num_regs; i++) {
-			reg = d->chip->status_base +
-				(i * map->reg_stride * d->irq_reg_stride);
+			if (d->chip->periph_offs)
+				reg = d->chip->status_base +
+					d->chip->periph_offs[i];
+			else
+				reg = d->chip->status_base +
+					(i * map->reg_stride *
+					 d->irq_reg_stride);
 
 			ret = regmap_read(map, reg, &val);
 			if (ret)
@@ -108,8 +115,13 @@ static void regmap_irq_sync_unlock(struct irq_data *data)
 		if (!d->chip->mask_base)
 			continue;
 
-		reg = d->chip->mask_base +
-			(i * map->reg_stride * d->irq_reg_stride);
+		if (d->chip->periph_offs)
+			reg = d->chip->mask_base +
+				d->chip->periph_offs[i];
+		else
+			reg = d->chip->mask_base +
+				(i * map->reg_stride * d->irq_reg_stride);
+
 		if (d->chip->mask_invert) {
 			ret = regmap_irq_update_bits(d, reg,
 					 d->mask_buf_def[i], ~d->mask_buf[i]);
@@ -136,8 +148,13 @@ static void regmap_irq_sync_unlock(struct irq_data *data)
 			dev_err(d->map->dev, "Failed to sync masks in %x\n",
 				reg);
 
-		reg = d->chip->wake_base +
-			(i * map->reg_stride * d->irq_reg_stride);
+		if (d->chip->periph_offs)
+			reg = d->chip->wake_base +
+				d->chip->periph_offs[i];
+		else
+			reg = d->chip->wake_base +
+				(i * map->reg_stride * d->irq_reg_stride);
+
 		if (d->wake_buf) {
 			if (d->chip->wake_invert)
 				ret = regmap_irq_update_bits(d, reg,
@@ -161,8 +178,14 @@ static void regmap_irq_sync_unlock(struct irq_data *data)
 		 * it'll be ignored in irq handler, then may introduce irq storm
 		 */
 		if (d->mask_buf[i] && (d->chip->ack_base || d->chip->use_ack)) {
-			reg = d->chip->ack_base +
-				(i * map->reg_stride * d->irq_reg_stride);
+			if (d->chip->periph_offs)
+				reg = d->chip->ack_base +
+					d->chip->periph_offs[i];
+			else
+				reg = d->chip->ack_base +
+					(i * map->reg_stride *
+					 d->irq_reg_stride);
+
 			/* some chips ack by write 0 */
 			if (d->chip->ack_invert)
 				ret = regmap_write(map, reg, ~d->mask_buf[i]);
@@ -187,8 +210,14 @@ static void regmap_irq_sync_unlock(struct irq_data *data)
 		for (i = 0; i < d->chip->num_type_reg; i++) {
 			if (!d->type_buf_def[i])
 				continue;
-			reg = d->chip->type_base +
-				(i * map->reg_stride * d->type_reg_stride);
+			if (d->chip->periph_offs)
+				reg = d->chip->type_base +
+					d->chip->periph_offs[i];
+			else
+				reg = d->chip->type_base +
+					(i * map->reg_stride *
+					 d->type_reg_stride);
+
 			if (d->chip->type_invert)
 				ret = regmap_irq_update_bits(d, reg,
 					d->type_buf_def[i], ~d->type_buf[i]);
@@ -198,6 +227,25 @@ static void regmap_irq_sync_unlock(struct irq_data *data)
 			if (ret != 0)
 				dev_err(d->map->dev, "Failed to sync type in %x\n",
 					reg);
+
+			if (!d->chip->periph_offs ||
+			    !d->chip->polarity_hi_base ||
+			    !d->chip->polarity_lo_base)
+				continue;
+
+			reg = d->chip->polarity_hi_base +
+				d->chip->periph_offs[i];
+			ret = regmap_write(map, reg, d->polarity_hi_buf[i]);
+			if (ret != 0)
+				dev_err(d->map->dev, "Failed to sync polarity hi in %x\n",
+					reg);
+
+			reg = d->chip->polarity_lo_base +
+				d->chip->periph_offs[i];
+			ret = regmap_write(map, reg, d->polarity_lo_buf[i]);
+			if (ret != 0)
+				dev_err(d->map->dev, "Failed to sync polarity lo in %x\n",
+					reg);
 		}
 	}
 
@@ -280,23 +328,49 @@ static int regmap_irq_set_type(struct irq_data *data, unsigned int type)
 	switch (type) {
 	case IRQ_TYPE_EDGE_FALLING:
 		d->type_buf[reg] |= t->type_falling_val;
+		if (d->chip->periph_offs) {
+			d->polarity_hi_buf[reg] &= ~t->type_falling_val;
+			d->polarity_lo_buf[reg] |= t->type_falling_val;
+		}
 		break;
 
 	case IRQ_TYPE_EDGE_RISING:
 		d->type_buf[reg] |= t->type_rising_val;
+		if (d->chip->periph_offs) {
+			d->polarity_hi_buf[reg] |= t->type_rising_val;
+			d->polarity_lo_buf[reg] &= ~t->type_rising_val;
+		}
 		break;
 
 	case IRQ_TYPE_EDGE_BOTH:
 		d->type_buf[reg] |= (t->type_falling_val |
 					t->type_rising_val);
+		if (d->chip->periph_offs) {
+			d->polarity_hi_buf[reg] |= (t->type_falling_val |
+					t->type_rising_val);
+			d->polarity_lo_buf[reg] |= (t->type_falling_val |
+					t->type_rising_val);
+		}
 		break;
 
 	case IRQ_TYPE_LEVEL_HIGH:
-		d->type_buf[reg] |= t->type_level_high_val;
+		if (!d->chip->periph_offs) {
+			d->type_buf[reg] |= t->type_level_high_val;
+		} else {
+			d->type_buf[reg] &= ~t->type_level_high_val;
+			d->polarity_hi_buf[reg] |= t->type_level_high_val;
+			d->polarity_lo_buf[reg] &= ~t->type_level_high_val;
+		}
 		break;
 
 	case IRQ_TYPE_LEVEL_LOW:
-		d->type_buf[reg] |= t->type_level_low_val;
+		if (!d->chip->periph_offs) {
+			d->type_buf[reg] |= t->type_level_low_val;
+		} else {
+			d->type_buf[reg] &= ~t->type_level_low_val;
+			d->polarity_hi_buf[reg] &= ~t->type_level_low_val;
+			d->polarity_lo_buf[reg] |= t->type_level_low_val;
+		}
 		break;
 	default:
 		return -EINVAL;
@@ -342,12 +416,10 @@ static inline int read_sub_irq_data(struct regmap_irq_chip_data *data,
 	struct regmap_irq_sub_irq_map *subreg;
 	int i, ret = 0;
 
-	if (!chip->sub_reg_offsets) {
-		/* Assume linear mapping */
-		ret = regmap_read(map, chip->status_base +
-				  (b * map->reg_stride * data->irq_reg_stride),
-				   &data->status_buf[b]);
-	} else {
+	if (chip->periph_offs) {
+		ret = regmap_read(map, chip->status_base + chip->periph_offs[b],
+				&data->status_buf[b]);
+	} else if (chip->sub_reg_offsets) {
 		subreg = &chip->sub_reg_offsets[b];
 		for (i = 0; i < subreg->num_regs; i++) {
 			unsigned int offset = subreg->offset[i];
@@ -357,6 +429,11 @@ static inline int read_sub_irq_data(struct regmap_irq_chip_data *data,
 			if (ret)
 				break;
 		}
+	} else {
+		/* Assume linear mapping */
+		ret = regmap_read(map, chip->status_base +
+				  (b * map->reg_stride * data->irq_reg_stride),
+				   &data->status_buf[b]);
 	}
 	return ret;
 }
@@ -474,10 +551,14 @@ static irqreturn_t regmap_irq_thread(int irq, void *d)
 
 	} else {
 		for (i = 0; i < data->chip->num_regs; i++) {
-			ret = regmap_read(map, chip->status_base +
-					  (i * map->reg_stride
-					   * data->irq_reg_stride),
-					  &data->status_buf[i]);
+			if (chip->periph_offs)
+				reg = chip->status_base + chip->periph_offs[i];
+			else
+				reg = chip->status_base +
+					(i * map->reg_stride *
+					 data->irq_reg_stride);
+
+			ret = regmap_read(map, reg, &data->status_buf[i]);
 
 			if (ret != 0) {
 				dev_err(map->dev,
@@ -499,8 +580,13 @@ static irqreturn_t regmap_irq_thread(int irq, void *d)
 		data->status_buf[i] &= ~data->mask_buf[i];
 
 		if (data->status_buf[i] && (chip->ack_base || chip->use_ack)) {
-			reg = chip->ack_base +
-				(i * map->reg_stride * data->irq_reg_stride);
+			if (chip->periph_offs)
+				reg = chip->ack_base + chip->periph_offs[i];
+			else
+				reg = chip->ack_base +
+					(i * map->reg_stride *
+					 data->irq_reg_stride);
+
 			if (chip->ack_invert)
 				ret = regmap_write(map, reg,
 						~data->status_buf[i]);
@@ -662,6 +748,18 @@ int regmap_add_irq_chip_fwnode(struct fwnode_handle *fwnode,
 			goto err_alloc;
 	}
 
+	if (chip->periph_offs) {
+		d->polarity_hi_buf = kcalloc(chip->num_regs,
+					     sizeof(unsigned int), GFP_KERNEL);
+		if (!d->polarity_hi_buf)
+			goto err_alloc;
+
+		d->polarity_lo_buf = kcalloc(chip->num_regs,
+					     sizeof(unsigned int), GFP_KERNEL);
+		if (!d->polarity_lo_buf)
+			goto err_alloc;
+	}
+
 	d->irq_chip = regmap_irq_chip;
 	d->irq_chip.name = chip->name;
 	d->irq = irq;
@@ -700,8 +798,12 @@ int regmap_add_irq_chip_fwnode(struct fwnode_handle *fwnode,
 		if (!chip->mask_base)
 			continue;
 
-		reg = chip->mask_base +
-			(i * map->reg_stride * d->irq_reg_stride);
+		if (chip->periph_offs)
+			reg = chip->mask_base + chip->periph_offs[i];
+		else
+			reg = chip->mask_base +
+				(i * map->reg_stride * d->irq_reg_stride);
+
 		if (chip->mask_invert)
 			ret = regmap_irq_update_bits(d, reg,
 					 d->mask_buf[i], ~d->mask_buf[i]);
@@ -725,8 +827,12 @@ int regmap_add_irq_chip_fwnode(struct fwnode_handle *fwnode,
 			continue;
 
 		/* Ack masked but set interrupts */
-		reg = chip->status_base +
-			(i * map->reg_stride * d->irq_reg_stride);
+		if (chip->periph_offs)
+			reg = chip->status_base + chip->periph_offs[i];
+		else
+			reg = chip->status_base +
+				(i * map->reg_stride * d->irq_reg_stride);
+
 		ret = regmap_read(map, reg, &d->status_buf[i]);
 		if (ret != 0) {
 			dev_err(map->dev, "Failed to read IRQ status: %d\n",
@@ -735,8 +841,13 @@ int regmap_add_irq_chip_fwnode(struct fwnode_handle *fwnode,
 		}
 
 		if (d->status_buf[i] && (chip->ack_base || chip->use_ack)) {
-			reg = chip->ack_base +
-				(i * map->reg_stride * d->irq_reg_stride);
+			if (chip->periph_offs)
+				reg = chip->ack_base + chip->periph_offs[i];
+			else
+				reg = chip->ack_base +
+					(i * map->reg_stride *
+					 d->irq_reg_stride);
+
 			if (chip->ack_invert)
 				ret = regmap_write(map, reg,
 					~(d->status_buf[i] & d->mask_buf[i]));
@@ -765,8 +876,12 @@ int regmap_add_irq_chip_fwnode(struct fwnode_handle *fwnode,
 	if (d->wake_buf) {
 		for (i = 0; i < chip->num_regs; i++) {
 			d->wake_buf[i] = d->mask_buf_def[i];
-			reg = chip->wake_base +
-				(i * map->reg_stride * d->irq_reg_stride);
+			if (chip->periph_offs)
+				reg = chip->wake_base + chip->periph_offs[i];
+			else
+				reg = chip->wake_base +
+					(i * map->reg_stride *
+					 d->irq_reg_stride);
 
 			if (chip->wake_invert)
 				ret = regmap_irq_update_bits(d, reg,
@@ -786,8 +901,12 @@ int regmap_add_irq_chip_fwnode(struct fwnode_handle *fwnode,
 
 	if (chip->num_type_reg && !chip->type_in_mask) {
 		for (i = 0; i < chip->num_type_reg; ++i) {
-			reg = chip->type_base +
-				(i * map->reg_stride * d->type_reg_stride);
+			if (chip->periph_offs)
+				reg = chip->type_base + chip->periph_offs[i];
+			else
+				reg = chip->type_base +
+					(i * map->reg_stride *
+					 d->type_reg_stride);
 
 			ret = regmap_read(map, reg, &d->type_buf_def[i]);
 
@@ -833,6 +952,8 @@ int regmap_add_irq_chip_fwnode(struct fwnode_handle *fwnode,
 	/* Should really dispose of the domain but... */
 err_alloc:
 	kfree(d->type_buf);
+	kfree(d->polarity_hi_buf);
+	kfree(d->polarity_lo_buf);
 	kfree(d->type_buf_def);
 	kfree(d->wake_buf);
 	kfree(d->mask_buf_def);
@@ -903,6 +1024,8 @@ void regmap_del_irq_chip(int irq, struct regmap_irq_chip_data *d)
 
 	irq_domain_remove(d->domain);
 	kfree(d->type_buf);
+	kfree(d->polarity_hi_buf);
+	kfree(d->polarity_lo_buf);
 	kfree(d->type_buf_def);
 	kfree(d->wake_buf);
 	kfree(d->mask_buf_def);
include/linux/regmap.h +6 -0
index e7834d9..6fb1090 100644
@@ -1338,6 +1338,7 @@ struct regmap_irq_sub_irq_map {
  *		     status_base. Should contain num_regs arrays.
  *		     Can be provided for chips with more complex mapping than
  *		     1.st bit to 1.st sub-reg, 2.nd bit to 2.nd sub-reg, ...
+ * @periph_offs:    Array of addresses of peripherals, should be num_reg long.
  * @num_main_regs: Number of 'main status' irq registers for chips which have
  *		   main_status set.
  *
@@ -1350,6 +1351,8 @@ struct regmap_irq_sub_irq_map {
  *               Using zero value is possible with @use_ack bit.
  * @wake_base:   Base address for wake enables.  If zero unsupported.
  * @type_base:   Base address for irq type.  If zero unsupported.
+ * @polarity_hi_base:   Base address for polarity high when periph_offs is used.
+ * @polarity_lo_base:   Base address for polarity low when periph_offs is used.
  * @irq_reg_stride:  Stride to use for chips where registers are not contiguous.
  * @init_ack_masked: Ack all masked interrupts once during initalization.
  * @mask_invert: Inverted mask register: cleared bits are masked out.
@@ -1390,6 +1393,7 @@ struct regmap_irq_chip {
 	unsigned int main_status;
 	unsigned int num_main_status_bits;
 	struct regmap_irq_sub_irq_map *sub_reg_offsets;
+	unsigned int *periph_offs;
 	int num_main_regs;
 
 	unsigned int status_base;
@@ -1398,6 +1402,8 @@ struct regmap_irq_chip {
 	unsigned int ack_base;
 	unsigned int wake_base;
 	unsigned int type_base;
+	unsigned int polarity_hi_base;
+	unsigned int polarity_lo_base;
 	unsigned int irq_reg_stride;
 	bool mask_writeonly:1;
 	bool init_ack_masked:1;
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
#2 lore raw

RFC PATCH 2/3 dt-bindings: mfd: Add QCOM PM8008 MFD bindings

In reply to Guru Das Srinagesh

Add device tree bindings for the driver for Qualcomm Technology Inc.'s PM8008 MFD PMIC.

---
 .../bindings/mfd/qcom,pm8008-irqchip.yaml          | 103 +++++++++++++++++++++
 1 file changed, 103 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/qcom,pm8008-irqchip.yaml
Documentation/devicetree/bindings/mfd/qcom,pm8008-irqchip.yaml new file +103 -0
new file mode 100644
index 0000000..296ff85
@@ -0,0 +1,103 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/qcom/qcom,pm8008-irqchip.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm Technologies, Inc. PM8008 Multi-Function Device PMIC
+
+maintainers:
+  - Guru Das Srinagesh <[email protected]>
+
+description: |
+  PM8008 is a PMIC that contains 7 LDOs, 2 GPIOs, temperature monitoring, and
+  can be interfaced over I2C.
+
+properties:
+  compatible:
+    const: qcom,pm8008-irqchip
+
+  reg:
+    maxItems: 1
+
+  interrupt-names:
+    const: pm8008
+
+  interrupts:
+    maxItems: 1
+
+  interrupt-controller:
+    type: boolean
+
+  "#address-cells":
+    const: 1
+    description:
+      Must be specified if child nodes are specified.
+
+  "#size-cells":
+    const: 0
+    description:
+      Must be specified if child nodes are specified.
+
+  "#interrupt-cells":
+    const: 2
+    description:
+      The first cell is the IRQ number, the second cell is the IRQ trigger flag.
+
+patternProperties:
+  "^.*[a-zA-Z][a-zA-Z0-9,+\\-._]{0,63}@[0-9a-f]+$":
+    type: object
+    # Each peripheral in PM8008 must be represented as a child node with an
+    # optional label for referencing as phandle elsewhere. This is optional.
+    properties:
+      compatible:
+        description:
+          The compatible string for the peripheral's driver.
+
+      reg:
+        maxItems: 1
+
+      interrupts:
+        maxItems: 1
+
+    required:
+      - compatible
+      - reg
+      - interrupts
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - "#interrupt-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    &qupv3_se13_i2c {
+            #address-cells = <1>;
+            #size-cells = <0>;
+    
+            pm8008i@8 {
+                    compatible = "qcom,pm8008-irqchip";
+                    reg = <0x8>;
+                    #address-cells = <1>;
+                    #size-cells = <0>;
+                    interrupt-controller;
+                    #interrupt-cells = <2>;
+    
+                    interrupt-names = "pm8008";
+                    interrupt-parent = <&tlmm>;
+                    interrupts = <32 IRQ_TYPE_EDGE_RISING>;
+    
+                    pm8008_tz: qcom,temp-alarm@2400 {
+                            compatible = "qcom,spmi-temp-alarm";
+                            reg = <0x2400>;
+                            interrupts = <0x5 IRQ_TYPE_EDGE_BOTH>;
+                            #thermal-sensor-cells = <0>;
+                    };
+    	    };
+   };
+
+...
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
#4 lore raw

RFC PATCH 3/3 mfd: Add PM8008 driver

In reply to Guru Das Srinagesh

PM8008 is a PMIC that contains 7 LDOs, 2 GPIOs, temperature monitoring, and can be interfaced over I2C. This driver uses the regmap-irq framework to handle interrupts, creates a regmap and uses it to instantiate all the child nodes under it in the device tree.

Only four peripherals have been added at the moment.

Every peripheral that has its TYPE register's hw default value as zero must have it set to an all-ones mask (to cover all interrupts it supports) in pm8008_init(). This is as per commit 84267d1b18ab ("regmap:
regmap-irq: Remove default irq type setting from core").

---
 drivers/mfd/Kconfig       |  14 ++++
 drivers/mfd/Makefile      |   1 +
 drivers/mfd/qcom-pm8008.c | 197 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 212 insertions(+)
 create mode 100644 drivers/mfd/qcom-pm8008.c
drivers/mfd/Kconfig +14 -0
index 33df083..3d4e989 100644
@@ -1960,6 +1960,20 @@ config MFD_ROHM_BD70528
 	  10 bits SAR ADC for battery temperature monitor and 1S battery
 	  charger.
 
+config MFD_QCOM_PM8008
+	tristate "QCOM PM8008 Power Management IC"
+	depends on I2C && OF
+	select REGMAP_I2C
+	select REGMAP_IRQ
+	help
+	  Select this option to get support for the PM8008 PMIC chip. PM8008 is
+	  a low-cost PMIC that contains 7 LDOs, 2 GPIOs, temperature
+	  monitoring, and can be interfaced over I2C. This driver provides
+	  common support for accessing the device by instantiating all the
+	  child nodes under it in the device tree and, therefore, additional
+	  drivers must be enabled in order to use the functionality of the
+	  device.
+
 config MFD_ROHM_BD71828
 	tristate "ROHM BD71828 Power Management IC"
 	depends on I2C=y
drivers/mfd/Makefile +1 -0
index a60e5f8..e316064 100644
@@ -260,6 +260,7 @@ obj-$(CONFIG_RAVE_SP_CORE)	+= rave-sp.o
 obj-$(CONFIG_MFD_ROHM_BD70528)	+= rohm-bd70528.o
 obj-$(CONFIG_MFD_ROHM_BD71828)	+= rohm-bd71828.o
 obj-$(CONFIG_MFD_ROHM_BD718XX)	+= rohm-bd718x7.o
+obj-$(CONFIG_MFD_QCOM_PM8008)	+= qcom-pm8008.o
 obj-$(CONFIG_MFD_STMFX) 	+= stmfx.o
 obj-$(CONFIG_MFD_KHADAS_MCU) 	+= khadas-mcu.o
 
drivers/mfd/qcom-pm8008.c new file +197 -0

197 lines not on this page · raw message

Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
maintainer
#3 lore raw

Re: [RFC PATCH RESEND v1 2/3] dt-bindings: mfd: Add QCOM PM8008 MFD bindings

In reply to Guru Das Srinagesh
Quoted 9 lines · Guru Das Srinagesh

Add device tree bindings for the driver for Qualcomm Technology Inc.'s PM8008 MFD PMIC.

---
 .../bindings/mfd/qcom,pm8008-irqchip.yaml          | 103 +++++++++++++++++++++
 1 file changed, 103 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/qcom,pm8008-irqchip.yaml

My bot found errors running 'make dt_binding_check' on your patch:

Traceback (most recent call last):
File "/usr/local/bin/dt-extract-example", line 45, in <module>
binding = yaml.load(open(args.yamlfile, encoding='utf-8').read())
File "/usr/local/lib/python3.8/dist-packages/ruamel/yaml/main.py", line 343, in load
return constructor.get_single_data()
File "/usr/local/lib/python3.8/dist-packages/ruamel/yaml/constructor.py", line 111, in get_single_data
node = self.composer.get_single_node()
File "_ruamel_yaml.pyx", line 706, in _ruamel_yaml.CParser.get_single_node
File "_ruamel_yaml.pyx", line 724, in _ruamel_yaml.CParser._compose_document
File "_ruamel_yaml.pyx", line 775, in _ruamel_yaml.CParser._compose_node
File "_ruamel_yaml.pyx", line 889, in _ruamel_yaml.CParser._compose_mapping_node
File "_ruamel_yaml.pyx", line 773, in _ruamel_yaml.CParser._compose_node
File "_ruamel_yaml.pyx", line 852, in _ruamel_yaml.CParser._compose_sequence_node
File "_ruamel_yaml.pyx", line 904, in _ruamel_yaml.CParser._parse_next_event
ruamel.yaml.parser.ParserError: while parsing a block collection
in "<unicode string>", line 77, column 3
did not find expected '-' indicator
in "<unicode string>", line 101, column 4
make[1]: *** [Documentation/devicetree/bindings/Makefile:20: Documentation/devicetree/bindings/mfd/qcom,pm8008-irqchip.example.dts] Error 1 make[1]: *** Deleting file 'Documentation/devicetree/bindings/mfd/qcom,pm8008-irqchip.example.dts'
make[1]: *** Waiting for unfinished jobs....
Traceback (most recent call last):
File "/usr/bin/yamllint", line 11, in <module>

    load_entry_point('yamllint==1.20.0', 'console_scripts', 'yamllint')()

File "/usr/lib/python3/dist-packages/yamllint/cli.py", line 184, in run
prob_level = show_problems(problems, file, args_format=args.format,
File "/usr/lib/python3/dist-packages/yamllint/cli.py", line 91, in show_problems
for problem in problems:
File "/usr/lib/python3/dist-packages/yamllint/linter.py", line 200, in _run
for problem in get_cosmetic_problems(buffer, conf, filepath):
File "/usr/lib/python3/dist-packages/yamllint/linter.py", line 137, in get_cosmetic_problems
for problem in rule.check(rule_conf,
File "/usr/lib/python3/dist-packages/yamllint/rules/indentation.py", line 570, in check
for problem in _check(conf, token, prev, next, nextnext, context):
File "/usr/lib/python3/dist-packages/yamllint/rules/indentation.py", line 336, in _check
'wrong indentation: expected %d but found %d' %
TypeError: %d format: a number is required, not NoneType
make[1]: *** [Documentation/devicetree/bindings/Makefile:59: Documentation/devicetree/bindings/processed-schema-examples.json] Error 123
make: *** [Makefile:1366: dt_binding_check] Error 2

See https://patchwork.ozlabs.org/patch/1385253

The base for the patch is generally the last rc1. Any dependencies should be noted.

If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date:

pip3 install dtschema --upgrade

Please check and re-submit.