Introduce QTI I2C PMIC controller

The Qualcomm Technologies, Inc. I2C PMIC Controller is used by multi-function PMIC devices which communicate over the I2C bus. The controller enumerates all child nodes as platform devices, and instantiates a regmap interface for them to communicate over the I2C bus.

The controller also controls interrupts for all of the children platform devices. The controller handles the summary interrupt by deciphering which peripheral triggered the interrupt, and which of the peripheral interrupts were triggered. Finally, it calls the interrupt handlers for each of the virtual interrupts that were registered.

Signed-off-by Guru Das Srinagesh <[email protected]>

---
Changes from v0:
- Fixed "FATAL ERROR: Unable to parse input tree" error thrown by make dt_binding_check.

 .../devicetree/bindings/mfd/qcom,i2c-pmic.yaml     | 86 ++++++++++++++++++++++
 1 file changed, 86 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/qcom,i2c-pmic.yaml

Documentation/devicetree/bindings/mfd/qcom,i2c-pmic.yaml new file +73 -0

new file mode 100644
index 0000000..42482af
@@ -0,0 +1,86 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/qcom,i2c-pmic.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm Technologies, Inc. I2C PMIC Interrupt Controller Platform Independent Bindings
+
+description: |
+  The I2C PMIC Controller is used by multi-function PMIC devices which
+  communicate over the I2C bus. An I2C PMIC controller node typically contains
+  one or more child nodes representing the device's peripherals. Each of the
+  peripherals typically has its own driver on the platform bus and will be
+  enumerated by this controller. The controller exposes a regmap to the
+  peripherals to communicate over the I2C bus.
+
+  The controller also controls interrupts for all of the peripherals on the
+  bus. The controller takes a summary interrupt, deciphers which peripheral
+  triggered the interrupt, and which of the peripheral's interrupts were
+  triggered. Finally, it calls the handlers for each of the virtual interrupts
+  that were registered.
+
+  This document describes the common platform independent bindings that apply
+  to all I2C PMIC interrupt controllers.
+
+maintainers:
+  - Guru Das Srinagesh <[email protected]>
+
+properties:
+  compatible:
+    const: qcom,i2c-pmic
+
+  reg:
+    description: 7-bit I2C address of the device.
+    maxItems: 1
+
+  interrupts:
+    description: Summary interrupt specifier.
+
+  interrupt-controller:
+    description: Flag indicating that this device is an interrupt controller.
+
+  "#interrupt-cells":
+    description: Number of cells to encode an interrupt source.
+
+  qcom,periph-map:
+    description: |
+      A contiguous list of u32 arrays where each element specifies the base
+      address of a single peripheral within the chip. This provides a mapping
+      between the summary status register bits and peripheral addresses as each
+      bit in the summary status register represents a peripheral.
+
+      The number of arrays should match the number of summary registers with up
+      to 8 elements each. Within each array, One element per bit of the summary
+      status register in order from the least sigificant bit to the most
+      significant bit.
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+
+  pinctrl-names:
+    const: default
+
+  pinctrl-0:
+    description: phandle of the pin configuration.
+    $ref: /schemas/types.yaml#/definitions/phandle
+
+required:
+  - compatible
+  - reg
+
+examples:
+  - |
+    qcom,smb138x@8 {
+      compatible = "qcom,i2c-pmic";

Documentation/devicetree/bindings/mfd/qcom,i2c-pmic.yaml new file +3 -0

+    };
+
+...

The Qualcomm Technologies, Inc. I2C PMIC Controller is used by multi-function PMIC devices which communicate over the I2C bus. The controller enumerates all child nodes as platform devices, and instantiates a regmap interface for them to communicate over the I2C bus.

The controller also controls interrupts for all of the children platform devices. The controller handles the summary interrupt by deciphering which peripheral triggered the interrupt, and which of the peripheral interrupts were triggered. Finally, it calls the interrupt handlers for each of the virtual interrupts that were registered.

Signed-off-by Guru Das Srinagesh <[email protected]>

---
Changes from v0:
- Fixed "FATAL ERROR: Unable to parse input tree" error thrown by make dt_binding_check.

 .../devicetree/bindings/mfd/qcom,i2c-pmic.yaml     | 86 ++++++++++++++++++++++
 1 file changed, 86 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/qcom,i2c-pmic.yaml

The Qualcomm Technologies, Inc. I2C PMIC Controller is used by multi-function PMIC devices which communicate over the I2C bus. The controller enumerates all child nodes as platform devices, and instantiates a regmap interface for them to communicate over the I2C bus.

The controller also controls interrupts for all of the children platform devices. The controller handles the summary interrupt by deciphering which peripheral triggered the interrupt, and which of the peripheral interrupts were triggered. Finally, it calls the interrupt handlers for each of the virtual interrupts that were registered.

Nicholas Troast is the original author of this driver.

Signed-off-by Guru Das Srinagesh <[email protected]>

---
 drivers/mfd/Kconfig         |  11 +
 drivers/mfd/Makefile        |   1 +
 drivers/mfd/qcom-i2c-pmic.c | 737 ++++++++++++++++++++++++++++++++++++++++++++

drivers/mfd/Kconfig +9 -0

+	depends on I2C && OF
+	select IRQ_DOMAIN
+	select REGMAP_I2C
+	help
+	  This enables support for controlling Qualcomm Technologies, Inc.
+	  PMICs over I2C. The driver controls interrupts, and provides register
+	  access for all of the device's peripherals.  Some QTI PMIC chips
+	  support communication over both I2C and SPMI.
+
 config MFD_QCOM_RPM
 	tristate "Qualcomm Resource Power Manager (RPM)"
 	depends on ARCH_QCOM && OF

drivers/mfd/Makefile +1 -0

index 7761f84..26f0b80 100644
@@ -199,6 +199,7 @@ obj-$(CONFIG_MFD_SI476X_CORE)	+= si476x-core.o
 obj-$(CONFIG_MFD_CS5535)	+= cs5535-mfd.o
 obj-$(CONFIG_MFD_OMAP_USB_HOST)	+= omap-usb-host.o omap-usb-tll.o
 obj-$(CONFIG_MFD_PM8XXX) 	+= qcom-pm8xxx.o ssbi.o
+obj-$(CONFIG_MFD_I2C_PMIC)     += qcom-i2c-pmic.o
 obj-$(CONFIG_MFD_QCOM_RPM)	+= qcom_rpm.o
 obj-$(CONFIG_MFD_SPMI_PMIC)	+= qcom-spmi-pmic.o
 obj-$(CONFIG_TPS65911_COMPARATOR)	+= tps65911-comparator.o

drivers/mfd/qcom-i2c-pmic.c new file +0 -0

new file mode 100644
index 0000000..d0f600a

drivers/mfd/qcom-i2c-pmic.c new file +21 -0

+#include <linux/bitops.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/irqdomain.h>
+#include <linux/module.h>
+#include <linux/of_platform.h>
+#include <linux/pinctrl/consumer.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+
+#define I2C_INTR_STATUS_BASE	0x0550
+#define INT_RT_STS_OFFSET	0x10
+#define INT_SET_TYPE_OFFSET	0x11
+#define INT_POL_HIGH_OFFSET	0x12
+#define INT_POL_LOW_OFFSET	0x13
+#define INT_LATCHED_CLR_OFFSET	0x14
+#define INT_EN_SET_OFFSET	0x15
+#define INT_EN_CLR_OFFSET	0x16
+#define INT_LATCHED_STS_OFFSET	0x18
+#define INT_PENDING_STS_OFFSET	0x19

Quoted 12 lines · Guru Das Srinagesh

The Qualcomm Technologies, Inc. I2C PMIC Controller is used by multi-function PMIC devices which communicate over the I2C bus. The controller enumerates all child nodes as platform devices, and instantiates a regmap interface for them to communicate over the I2C bus.

The controller also controls interrupts for all of the children platform devices. The controller handles the summary interrupt by deciphering which peripheral triggered the interrupt, and which of the peripheral interrupts were triggered. Finally, it calls the interrupt handlers for each of the virtual interrupts that were registered.

Quoted 7 lines

Nicholas Troast is the original author of this driver.

Signed-off-by Guru Das Srinagesh <[email protected]>

---
 drivers/mfd/Kconfig         |  11 +
 drivers/mfd/Makefile        |   1 +
 drivers/mfd/qcom-i2c-pmic.c | 737 ++++++++++++++++++++++++++++++++++++++++++++

The vast majority of this driver deals with IRQ handling. Why can't this be split out into its own IRQ Chip driver and moved to drivers/irqchip?

Quoted 12 lines

 3 files changed, 749 insertions(+)
 create mode 100644 drivers/mfd/qcom-i2c-pmic.c

drivers/mfd/Kconfig +0 -0

index 54b6aa4..bf112eb 100644
@@ -1002,6 +1002,17 @@ config MFD_PM8XXX
 	  Say M here if you want to include support for PM8xxx chips as a
 	  module. This will build a module called "pm8xxx-core".
 

drivers/mfd/Kconfig config MFD_PM8XXX · line 1005

Quoted 1 line

drivers/mfd/Kconfig +1 -0

+config MFD_I2C_PMIC

Too generic. This should identify the vendor too.

drivers/mfd/Kconfig config MFD_PM8XXX · line 1006

Quoted 2 lines

drivers/mfd/Kconfig +1 -0

+	tristate "QTI I2C PMIC support"

Why aren't you using QCOM?

Actually, this should be expanded here anyway.

Quoted 28 lines

drivers/mfd/Kconfig +9 -0

+	depends on I2C && OF
+	select IRQ_DOMAIN
+	select REGMAP_I2C
+	help
+	  This enables support for controlling Qualcomm Technologies, Inc.
+	  PMICs over I2C. The driver controls interrupts, and provides register
+	  access for all of the device's peripherals.  Some QTI PMIC chips
+	  support communication over both I2C and SPMI.
+
 config MFD_QCOM_RPM
 	tristate "Qualcomm Resource Power Manager (RPM)"
 	depends on ARCH_QCOM && OF

drivers/mfd/Makefile +1 -0

index 7761f84..26f0b80 100644
@@ -199,6 +199,7 @@ obj-$(CONFIG_MFD_SI476X_CORE)	+= si476x-core.o
 obj-$(CONFIG_MFD_CS5535)	+= cs5535-mfd.o
 obj-$(CONFIG_MFD_OMAP_USB_HOST)	+= omap-usb-host.o omap-usb-tll.o
 obj-$(CONFIG_MFD_PM8XXX) 	+= qcom-pm8xxx.o ssbi.o
+obj-$(CONFIG_MFD_I2C_PMIC)     += qcom-i2c-pmic.o
 obj-$(CONFIG_MFD_QCOM_RPM)	+= qcom_rpm.o
 obj-$(CONFIG_MFD_SPMI_PMIC)	+= qcom-spmi-pmic.o
 obj-$(CONFIG_TPS65911_COMPARATOR)	+= tps65911-comparator.o

drivers/mfd/qcom-i2c-pmic.c new file +0 -0

new file mode 100644
index 0000000..d0f600a

drivers/mfd/qcom-i2c-pmic.c line 3

Quoted 6 lines

drivers/mfd/qcom-i2c-pmic.c new file +3 -0

@@ -0,0 +1,737 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2016-2018, The Linux Foundation. All rights reserved.

This is very out of date.

drivers/mfd/qcom-i2c-pmic.c line 6

Quoted 4 lines

drivers/mfd/qcom-i2c-pmic.c new file +3 -0

+ */
+
+#define pr_fmt(fmt) "I2C PMIC: %s: " fmt, __func__

Please don't role your own debug helpers.

The ones the kernel provides are suitably proficient.

Quoted 22 lines

drivers/mfd/qcom-i2c-pmic.c new file +21 -0

+#include <linux/bitops.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/irqdomain.h>
+#include <linux/module.h>
+#include <linux/of_platform.h>
+#include <linux/pinctrl/consumer.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+
+#define I2C_INTR_STATUS_BASE	0x0550
+#define INT_RT_STS_OFFSET	0x10
+#define INT_SET_TYPE_OFFSET	0x11
+#define INT_POL_HIGH_OFFSET	0x12
+#define INT_POL_LOW_OFFSET	0x13
+#define INT_LATCHED_CLR_OFFSET	0x14
+#define INT_EN_SET_OFFSET	0x15
+#define INT_EN_CLR_OFFSET	0x16
+#define INT_LATCHED_STS_OFFSET	0x18
+#define INT_PENDING_STS_OFFSET	0x19

drivers/mfd/qcom-i2c-pmic.c line 38

Quoted 10 lines

drivers/mfd/qcom-i2c-pmic.c new file +10 -0

+#define INT_MID_SEL_OFFSET	0x1A
+#define INT_MID_SEL_MASK	GENMASK(1, 0)
+#define INT_PRIORITY_OFFSET	0x1B
+#define INT_PRIORITY_BIT	BIT(0)
+
+enum {
+	IRQ_SET_TYPE = 0,
+	IRQ_POL_HIGH,
+	IRQ_POL_LOW,
+	IRQ_LATCHED_CLR, /* not needed but makes life easy */

"Not"

It doesn't matter if the value is not used.

I think you can drop the comment.

drivers/mfd/qcom-i2c-pmic.c line 41

Quoted 4 lines

drivers/mfd/qcom-i2c-pmic.c new file +3 -0

+	IRQ_EN_SET,
+	IRQ_MAX_REGS,
+};

Going to stop here for a second, as the vast majority of the remainder of the driver appears to surround IRQ management.

Quoted 6 lines · Lee Jones

Quoted 5 lines · Guru Das Srinagesh

The Qualcomm Technologies, Inc. I2C PMIC Controller is used by multi-function PMIC devices which communicate over the I2C bus. The controller enumerates all child nodes as platform devices, and instantiates a regmap interface for them to communicate over the I2C bus.

Quoted 1 line

drivers/mfd/qcom-i2c-pmic.c

Quoted 1 line

drivers/mfd/qcom-i2c-pmic.c +0 -0

Quoted 3 lines

Please don't role your own debug helpers.

The ones the kernel provides are suitably proficient.

Sure. Would this be acceptable instead, with the custom string replaced by a macro that the kernel provides?

	#define pr_fmt(fmt) "%s: %s: " fmt, KBUILD_MODNAME, __func__

Quoted 7 lines · Guru Das Srinagesh

Quoted 6 lines · Lee Jones

Quoted 5 lines · Guru Das Srinagesh

The Qualcomm Technologies, Inc. I2C PMIC Controller is used by multi-function PMIC devices which communicate over the I2C bus. The controller enumerates all child nodes as platform devices, and instantiates a regmap interface for them to communicate over the I2C bus.

[]

Quoted 1 line

Quoted 1 line

drivers/mfd/qcom-i2c-pmic.c

Quoted 1 line

drivers/mfd/qcom-i2c-pmic.c +0 -0

[]

Quoted 8 lines

Quoted 3 lines

Please don't role your own debug helpers.

The ones the kernel provides are suitably proficient.

Sure. Would this be acceptable instead, with the custom string replaced by a macro that the kernel provides?

	#define pr_fmt(fmt) "%s: %s: " fmt, KBUILD_MODNAME, __func__

trivia:

It's almost always smaller object code to use
the KBUILD_MODNAME as a fixed string instead of
as a printf argument.

	#define pr_fmt(fmt) KBUILD_MODNAME ": %s: " fmt, __func__

Quoted 22 lines · Lee Jones

Quoted 12 lines · Guru Das Srinagesh

The Qualcomm Technologies, Inc. I2C PMIC Controller is used by multi-function PMIC devices which communicate over the I2C bus. The controller enumerates all child nodes as platform devices, and instantiates a regmap interface for them to communicate over the I2C bus.

The controller also controls interrupts for all of the children platform devices. The controller handles the summary interrupt by deciphering which peripheral triggered the interrupt, and which of the peripheral interrupts were triggered. Finally, it calls the interrupt handlers for each of the virtual interrupts that were registered.

Quoted 7 lines

Nicholas Troast is the original author of this driver.

Signed-off-by Guru Das Srinagesh <[email protected]>

---
 drivers/mfd/Kconfig         |  11 +
 drivers/mfd/Makefile        |   1 +
 drivers/mfd/qcom-i2c-pmic.c | 737 ++++++++++++++++++++++++++++++++++++++++++++

Quoted 3 lines

The vast majority of this driver deals with IRQ handling. Why can't this be split out into its own IRQ Chip driver and moved to drivers/irqchip?

There appear to be quite a few in-tree MFD drivers that register IRQ controllers, like this driver does:

$ grep --exclude-dir=.git -rnE "irq_domain_(add|create).+\(" drivers/mfd | wc -l
23

As a further example, drivers/mfd/stpmic1.c closely resembles this driver in that it uses both devm_regmap_add_irq_chip() as well as devm_of_platform_populate().

As such, it seems like this driver is in line with some of the architectural choices that have been accepted in already-merged drivers. Could you please elaborate on your concerns?

Quoted 37 lines · Guru Das Srinagesh

Quoted 22 lines · Lee Jones

Quoted 12 lines · Guru Das Srinagesh

The Qualcomm Technologies, Inc. I2C PMIC Controller is used by multi-function PMIC devices which communicate over the I2C bus. The controller enumerates all child nodes as platform devices, and instantiates a regmap interface for them to communicate over the I2C bus.

The controller also controls interrupts for all of the children platform devices. The controller handles the summary interrupt by deciphering which peripheral triggered the interrupt, and which of the peripheral interrupts were triggered. Finally, it calls the interrupt handlers for each of the virtual interrupts that were registered.

Quoted 7 lines

Nicholas Troast is the original author of this driver.

Signed-off-by Guru Das Srinagesh <[email protected]>

---
 drivers/mfd/Kconfig         |  11 +
 drivers/mfd/Makefile        |   1 +
 drivers/mfd/qcom-i2c-pmic.c | 737 ++++++++++++++++++++++++++++++++++++++++++++

Quoted 3 lines

The vast majority of this driver deals with IRQ handling. Why can't this be split out into its own IRQ Chip driver and moved to drivers/irqchip?

There appear to be quite a few in-tree MFD drivers that register IRQ controllers, like this driver does:

$ grep --exclude-dir=.git -rnE "irq_domain_(add|create).+\(" drivers/mfd | wc -l
23

As a further example, drivers/mfd/stpmic1.c closely resembles this driver in that it uses both devm_regmap_add_irq_chip() as well as devm_of_platform_populate().

Quoted 3 lines

As such, it seems like this driver is in line with some of the architectural choices that have been accepted in already-merged drivers. Could you please elaborate on your concerns?

It is true that *basic* IRQ domain support has been added to these drivers in the past. However, IMHO the support added to this driver goes beyond those realms such that it would justify a driver of its own.

Quoted 39 lines · Lee Jones

Quoted 37 lines · Guru Das Srinagesh

Quoted 22 lines · Lee Jones

Quoted 12 lines · Guru Das Srinagesh

The Qualcomm Technologies, Inc. I2C PMIC Controller is used by multi-function PMIC devices which communicate over the I2C bus. The controller enumerates all child nodes as platform devices, and instantiates a regmap interface for them to communicate over the I2C bus.

The controller also controls interrupts for all of the children platform devices. The controller handles the summary interrupt by deciphering which peripheral triggered the interrupt, and which of the peripheral interrupts were triggered. Finally, it calls the interrupt handlers for each of the virtual interrupts that were registered.

Quoted 7 lines

Nicholas Troast is the original author of this driver.

Signed-off-by Guru Das Srinagesh <[email protected]>

---
 drivers/mfd/Kconfig         |  11 +
 drivers/mfd/Makefile        |   1 +
 drivers/mfd/qcom-i2c-pmic.c | 737 ++++++++++++++++++++++++++++++++++++++++++++

Quoted 3 lines

The vast majority of this driver deals with IRQ handling. Why can't this be split out into its own IRQ Chip driver and moved to drivers/irqchip?

There appear to be quite a few in-tree MFD drivers that register IRQ controllers, like this driver does:

$ grep --exclude-dir=.git -rnE "irq_domain_(add|create).+\(" drivers/mfd | wc -l
23

As a further example, drivers/mfd/stpmic1.c closely resembles this driver in that it uses both devm_regmap_add_irq_chip() as well as devm_of_platform_populate().

Quoted 8 lines

Quoted 3 lines

As such, it seems like this driver is in line with some of the architectural choices that have been accepted in already-merged drivers. Could you please elaborate on your concerns?

It is true that *basic* IRQ domain support has been added to these drivers in the past. However, IMHO the support added to this driver goes beyond those realms such that it would justify a driver of its own.

I am exploring an option to see if the regmap-irq APIs may be used in this driver, similar to stpmic1.c. Just to let you know, it might be a few days before I am able to post my next patchset as I'll have to make the necessary changes and test them out first.