[PATCH v3 0/4] Support for Qualcomm QPNP PMIC's

[PATCH v3 0/4] Support for Qualcomm QPNP PMIC's

[Stanimir Varbanov]

Hello all,

Changes since v2:
 - 1/4 - added new line, signed-off-by / acked-by and module_authors.
 - 3/4 - the subject has been changed.

The previous v2 can be found at [1].

I'm still waiting Acks for:
 - 4/4 from Qualcomm folks.
 - 2/4 and 3/4 from DT folks.

The patchset is ready to merge version and also it can be treated as an
intermediate step until we find a solution for non-translatable peripheral
addresses.

regards,
Stan

[1] https://lkml.org/lkml/2014/7/17/877

--------------------------------------------------------------------------------

Hello everyone,

Here is the continuation of patch sets sent recently about Qualcomm
QPNP SPMI PMICs.

The previous version of the patch set can be found at [1].

Changes since v1:
 - removed completely custom *of* parser
 - renamed the mfd driver from qpnp-spmi to pm8xxx-spmi
 - now MFD_PM8XXX_SPMI Kconfig option depends on SPMI

Removing of the custom *of* parser leads to that that the *reg* devicetree
property cannot exist and therefore cannot be parsed to get PMIC peripheral
resources. I took this step aside because no one from mfd drivers does this
parsing. This will lead to inconvenience in the peripheral drivers to define
internally the SPMI base addresses depending on the compatible property
i.e. PMIC version. 

Comments are welcome!

[1] https://lkml.org/lkml/2014/7/8/428

Josh Cartwright (1):
  mfd: pm8xxx-spmi: add support for Qualcomm SPMI PMICs

Stanimir Varbanov (3):
  mfd: pm8xxx-spmi: document DT bindings for Qualcomm SPMI PMICs
  ARM: dts: qcom: add pm8941 and pm8841 PMICs device nodes
  mfd: pm8921: rename pm8921-core driver

 .../devicetree/bindings/mfd/qcom,pm8xxx-spmi.txt   |   49 +++++++++++++++
 arch/arm/boot/dts/qcom-msm8974.dtsi                |   37 +++++++++++
 drivers/mfd/Kconfig                                |   30 +++++++--
 drivers/mfd/Makefile                               |    3 +-
 drivers/mfd/pm8xxx-spmi.c                          |   65 ++++++++++++++++++++
 drivers/mfd/{pm8921-core.c => pm8xxx-ssbi.c}       |   38 ++++++------
 6 files changed, 195 insertions(+), 27 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mfd/qcom,pm8xxx-spmi.txt
 create mode 100644 drivers/mfd/pm8xxx-spmi.c
 rename drivers/mfd/{pm8921-core.c => pm8xxx-ssbi.c} (92%)

[PATCH v3 1/4] mfd: pm8xxx-spmi: add support for Qualcomm SPMI PMICs

[Stanimir Varbanov]

From: Josh Cartwright <joshc@codeaurora.org>

The Qualcomm SPMI PMIC chips are components used with the
Snapdragon 800 series SoC family.  This driver exists
largely as a glue mfd component, it exists to be an owner
of an SPMI regmap for children devices described in
device tree.

Signed-off-by: Josh Cartwright <joshc@codeaurora.org>
Signed-off-by: Stanimir Varbanov <svarbanov@mm-sol.com>
Acked-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mfd/Kconfig       |   16 +++++++++++
 drivers/mfd/Makefile      |    1 +
 drivers/mfd/pm8xxx-spmi.c |   65 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 82 insertions(+), 0 deletions(-)
 create mode 100644 drivers/mfd/pm8xxx-spmi.c

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 6cc4b6a..122e2d9 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -524,6 +524,22 @@ config MFD_PM8921_CORE
 	  Say M here if you want to include support for PM8921 chip as a module.
 	  This will build a module called "pm8921-core".
 
+config MFD_PM8XXX_SPMI
+	tristate "Qualcomm PM8XXX SPMI PMICs"
+	depends on ARCH_QCOM || COMPILE_TEST
+	depends on OF
+	depends on SPMI
+	select MFD_PM8XXX
+	select REGMAP_SPMI
+	help
+	  This enables support for the Qualcomm PM8XXX SPMI PMICs.
+	  These PMICs are currently used with the Snapdragon 800 series of
+	  SoCs.  Note, that this will only be useful paired with descriptions
+	  of the independent functions as children nodes in the device tree.
+
+	  Say M here if you want to include support for the PM8XXX SPMI PMIC
+	  series as a module.  The module will be called "pm8xxx-spmi".
+
 config MFD_RDC321X
 	tristate "RDC R-321x southbridge"
 	select MFD_CORE
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 8afedba..93d82cc 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -153,6 +153,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_PM8921_CORE) 	+= pm8921-core.o ssbi.o
+obj-$(CONFIG_MFD_PM8XXX_SPMI)	+= pm8xxx-spmi.o
 obj-$(CONFIG_TPS65911_COMPARATOR)	+= tps65911-comparator.o
 obj-$(CONFIG_MFD_TPS65090)	+= tps65090.o
 obj-$(CONFIG_MFD_AAT2870_CORE)	+= aat2870-core.o
diff --git a/drivers/mfd/pm8xxx-spmi.c b/drivers/mfd/pm8xxx-spmi.c
new file mode 100644
index 0000000..16335a1
--- /dev/null
+++ b/drivers/mfd/pm8xxx-spmi.c
@@ -0,0 +1,65 @@
+/*
+ * Copyright (c) 2014, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/spmi.h>
+#include <linux/regmap.h>
+#include <linux/of_platform.h>
+
+static const struct regmap_config pm8xxx_regmap_config = {
+	.reg_bits	= 16,
+	.val_bits	= 8,
+	.max_register	= 0xffff,
+};
+
+static int pm8xxx_probe(struct spmi_device *sdev)
+{
+	struct device_node *root = sdev->dev.of_node;
+	struct regmap *regmap;
+
+	regmap = devm_regmap_init_spmi_ext(sdev, &pm8xxx_regmap_config);
+	if (IS_ERR(regmap))
+		return PTR_ERR(regmap);
+
+	return of_platform_populate(root, NULL, NULL, &sdev->dev);
+}
+
+static void pm8xxx_remove(struct spmi_device *sdev)
+{
+	of_platform_depopulate(&sdev->dev);
+}
+
+static const struct of_device_id pm8xxx_id_table[] = {
+	{ .compatible = "qcom,pm8941" },
+	{ .compatible = "qcom,pm8841" },
+	{ .compatible = "qcom,pma8084" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, pm8xxx_id_table);
+
+static struct spmi_driver pm8xxx_driver = {
+	.probe = pm8xxx_probe,
+	.remove = pm8xxx_remove,
+	.driver = {
+		.name = "pm8xxx-spmi",
+		.of_match_table = pm8xxx_id_table,
+	},
+};
+module_spmi_driver(pm8xxx_driver);
+
+MODULE_DESCRIPTION("PM8XXX SPMI PMIC driver");
+MODULE_ALIAS("spmi:pm8xxx-spmi");
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Josh Cartwright <joshc@codeaurora.org>");
+MODULE_AUTHOR("Stanimir Varbanov <svarbanov@mm-sol.com>");
-- 
1.7.0.4

[PATCH v3 2/4] mfd: pm8xxx-spmi: document DT bindings for Qualcomm SPMI PMICs

[Stanimir Varbanov]

Document DT bindings used to describe the Qualcomm SPMI PMICs.
Currently the SPMI PMICs supported are pm8941, pm8841 and pma8084.

Signed-off-by: Stanimir Varbanov <svarbanov@mm-sol.com>
---
 .../devicetree/bindings/mfd/qcom,pm8xxx-spmi.txt   |   49 ++++++++++++++++++++
 1 files changed, 49 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mfd/qcom,pm8xxx-spmi.txt

diff --git a/Documentation/devicetree/bindings/mfd/qcom,pm8xxx-spmi.txt b/Documentation/devicetree/bindings/mfd/qcom,pm8xxx-spmi.txt
new file mode 100644
index 0000000..2b7f5b6
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/qcom,pm8xxx-spmi.txt
@@ -0,0 +1,49 @@
+          Qualcomm PM8XXX SPMI PMICs multi-function device bindings
+
+The Qualcomm PM8XXX series presently includes PM8941, PM8841 and PMA8084
+PMICs.  These PMICs use a QPNP scheme through SPMI interface.
+QPNP is effectively a partitioning scheme for dividing the SPMI extended
+register space up into logical pieces, and set of fixed register
+locations/definitions within these regions, with some of these regions
+specifically used for interrupt handling.
+
+The QPNP PMICs are used with the Qualcomm Snapdragon series SoCs, and are
+interfaced to the chip via the SPMI (System Power Management Interface) bus.
+Support for multiple independent functions are implemented by splitting the
+16-bit SPMI slave address space into 256 smaller fixed-size regions, 256 bytes
+each. A function can consume one or more of these fixed-size register regions.
+
+Required properties:
+- compatible:      Should contain one of:
+                     "qcom,pm8941"
+                     "qcom,pm8841"
+                     "qcom,pma8084"
+- reg:             Specifies the SPMI USID slave address for this device.
+                   For more information see:
+                   Documentation/devicetree/bindings/spmi/spmi.txt
+
+Required properties for peripheral child nodes:
+- compatible:      Should contain "qcom,pm8xxx-xxx", where "xxx" is
+                   peripheral name. The "pm8xxx" can be any of supported PMICs,
+                   see example below.
+
+Optional properties for peripheral child nodes:
+- interrupts:      Interrupts are specified as a 4-tuple. For more information
+                   see:
+                   Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.txt
+- interrupt-names: Corresponding interrupt name to the interrupts property
+
+Each child node represents a function of the PMIC.
+
+Example:
+
+	pm8941 at 0 {
+		compatible = "qcom,pm8941";
+		reg = <0x0 SPMI_USID>;
+
+		rtc {
+			compatible = "qcom,pm8941-rtc";
+			interrupts = <0x0 0x61 0x1 IRQ_TYPE_EDGE_RISING>;
+			interrupt-names = "alarm";
+		};
+	};
-- 
1.7.0.4

[PATCH v3 3/4] ARM: dts: qcom: add pm8941 and pm8841 PMICs device nodes

[Stanimir Varbanov]

The pm8941 and pm8841 spmi devicetree nodes are childrens of
spmi pmic arbiter. The msm8974 SoC uses two PMIC chips
pm8941 and pm8841. Every PMIC chip has two spmi bus slave id's.

Signed-off-by: Stanimir Varbanov <svarbanov@mm-sol.com>
---
 arch/arm/boot/dts/qcom-msm8974.dtsi |   37 +++++++++++++++++++++++++++++++++++
 1 files changed, 37 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi b/arch/arm/boot/dts/qcom-msm8974.dtsi
index 69dca2a..5e08d43 100644
--- a/arch/arm/boot/dts/qcom-msm8974.dtsi
+++ b/arch/arm/boot/dts/qcom-msm8974.dtsi
@@ -3,6 +3,7 @@
 #include "skeleton.dtsi"
 
 #include <dt-bindings/clock/qcom,gcc-msm8974.h>
+#include <dt-bindings/spmi/spmi.h>
 
 / {
 	model = "Qualcomm MSM8974";
@@ -236,5 +237,41 @@
 			#interrupt-cells = <2>;
 			interrupts = <0 208 0>;
 		};
+
+		spmi at fc4cf000 {
+			compatible = "qcom,spmi-pmic-arb";
+			reg-names = "core", "intr", "cnfg";
+			reg = <0xfc4cf000 0x1000>,
+			      <0xfc4cb000 0x1000>,
+			      <0xfc4ca000 0x1000>;
+			interrupt-names = "periph_irq";
+			interrupts = <0 190 0>;
+			qcom,ee = <0>;
+			qcom,channel = <0>;
+			#address-cells = <2>;
+			#size-cells = <0>;
+			interrupt-controller;
+			#interrupt-cells = <4>;
+
+			pm8941 at 0 {
+				compatible = "qcom,pm8941";
+				reg = <0x0 SPMI_USID>;
+			};
+
+			pm8941 at 1 {
+				compatible = "qcom,pm8941";
+				reg = <0x1 SPMI_USID>;
+			};
+
+			pm8841 at 4 {
+				compatible = "qcom,pm8841";
+				reg = <0x4 SPMI_USID>;
+			};
+
+			pm8841 at 5 {
+				compatible = "qcom,pm8841";
+				reg = <0x5 SPMI_USID>;
+			};
+		};
 	};
 };
-- 
1.7.0.4

[PATCH v3 4/4] mfd: pm8921: rename pm8921-core driver

[Stanimir Varbanov]

The pm8921-core driver presently supports pm8921 and pm8058
Qualcomm PMICs.  To avoid confusion with new generation PMICs
(like pm8941) rename the pm8921-core driver to more
appropriate name pm8xxx-ssbi, which reflects better that
those chips use SSBI interface.

Signed-off-by: Stanimir Varbanov <svarbanov@mm-sol.com>
Acked-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mfd/Kconfig                          |   14 +++++-----
 drivers/mfd/Makefile                         |    2 +-
 drivers/mfd/{pm8921-core.c => pm8xxx-ssbi.c} |   38 +++++++++++++-------------
 3 files changed, 27 insertions(+), 27 deletions(-)
 rename drivers/mfd/{pm8921-core.c => pm8xxx-ssbi.c} (92%)

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 122e2d9..884bc32 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -507,8 +507,8 @@ config UCB1400_CORE
 config MFD_PM8XXX
 	tristate
 
-config MFD_PM8921_CORE
-	tristate "Qualcomm PM8921 PMIC chip"
+config MFD_PM8XXX_SSBI
+	tristate "Qualcomm PM8XXX SSBI PMICs"
 	depends on (ARM || HEXAGON)
 	select IRQ_DOMAIN
 	select MFD_CORE
@@ -516,13 +516,13 @@ config MFD_PM8921_CORE
 	select REGMAP
 	help
 	  If you say yes to this option, support will be included for the
-	  built-in PM8921 PMIC chip.
+	  built-in PM8921 and PM8058 PMIC chips.
 
-	  This is required if your board has a PM8921 and uses its features,
-	  such as: MPPs, GPIOs, regulators, interrupts, and PWM.
+	  This is required if your board has a PM8921 or PM8058 and uses
+	  its features, such as: MPPs, GPIOs, regulators, interrupts, and PWM.
 
-	  Say M here if you want to include support for PM8921 chip as a module.
-	  This will build a module called "pm8921-core".
+	  Say M here if you want to include support for PM8921 or PM8058 chip
+	  as a module. This will build a module called "pm8xxx-ssbi".
 
 config MFD_PM8XXX_SPMI
 	tristate "Qualcomm PM8XXX SPMI PMICs"
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 93d82cc..d332085 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -152,7 +152,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_PM8921_CORE) 	+= pm8921-core.o ssbi.o
+obj-$(CONFIG_MFD_PM8XXX_SSBI) 	+= pm8xxx-ssbi.o ssbi.o
 obj-$(CONFIG_MFD_PM8XXX_SPMI)	+= pm8xxx-spmi.o
 obj-$(CONFIG_TPS65911_COMPARATOR)	+= tps65911-comparator.o
 obj-$(CONFIG_MFD_TPS65090)	+= tps65090.o
diff --git a/drivers/mfd/pm8921-core.c b/drivers/mfd/pm8xxx-ssbi.c
similarity index 92%
rename from drivers/mfd/pm8921-core.c
rename to drivers/mfd/pm8xxx-ssbi.c
index 9595138..102d7fb 100644
--- a/drivers/mfd/pm8921-core.c
+++ b/drivers/mfd/pm8xxx-ssbi.c
@@ -277,14 +277,14 @@ static const struct regmap_config ssbi_regmap_config = {
 	.reg_write = ssbi_reg_write
 };
 
-static const struct of_device_id pm8921_id_table[] = {
+static const struct of_device_id pm8xxx_id_table[] = {
 	{ .compatible = "qcom,pm8058", },
 	{ .compatible = "qcom,pm8921", },
 	{ }
 };
-MODULE_DEVICE_TABLE(of, pm8921_id_table);
+MODULE_DEVICE_TABLE(of, pm8xxx_id_table);
 
-static int pm8921_probe(struct platform_device *pdev)
+static int pm8xxx_probe(struct platform_device *pdev)
 {
 	struct regmap *regmap;
 	int irq, rc;
@@ -354,18 +354,18 @@ static int pm8921_probe(struct platform_device *pdev)
 	return rc;
 }
 
-static int pm8921_remove_child(struct device *dev, void *unused)
+static int pm8xxx_remove_child(struct device *dev, void *unused)
 {
 	platform_device_unregister(to_platform_device(dev));
 	return 0;
 }
 
-static int pm8921_remove(struct platform_device *pdev)
+static int pm8xxx_remove(struct platform_device *pdev)
 {
 	int irq = platform_get_irq(pdev, 0);
 	struct pm_irq_chip *chip = platform_get_drvdata(pdev);
 
-	device_for_each_child(&pdev->dev, NULL, pm8921_remove_child);
+	device_for_each_child(&pdev->dev, NULL, pm8xxx_remove_child);
 	irq_set_chained_handler(irq, NULL);
 	irq_set_handler_data(irq, NULL);
 	irq_domain_remove(chip->irqdomain);
@@ -373,29 +373,29 @@ static int pm8921_remove(struct platform_device *pdev)
 	return 0;
 }
 
-static struct platform_driver pm8921_driver = {
-	.probe		= pm8921_probe,
-	.remove		= pm8921_remove,
+static struct platform_driver pm8xxx_driver = {
+	.probe		= pm8xxx_probe,
+	.remove		= pm8xxx_remove,
 	.driver		= {
-		.name	= "pm8921-core",
+		.name	= "pm8xxx-ssbi",
 		.owner	= THIS_MODULE,
-		.of_match_table = pm8921_id_table,
+		.of_match_table = pm8xxx_id_table,
 	},
 };
 
-static int __init pm8921_init(void)
+static int __init pm8xxx_init(void)
 {
-	return platform_driver_register(&pm8921_driver);
+	return platform_driver_register(&pm8xxx_driver);
 }
-subsys_initcall(pm8921_init);
+subsys_initcall(pm8xxx_init);
 
-static void __exit pm8921_exit(void)
+static void __exit pm8xxx_exit(void)
 {
-	platform_driver_unregister(&pm8921_driver);
+	platform_driver_unregister(&pm8xxx_driver);
 }
-module_exit(pm8921_exit);
+module_exit(pm8xxx_exit);
 
 MODULE_LICENSE("GPL v2");
-MODULE_DESCRIPTION("PMIC 8921 core driver");
+MODULE_DESCRIPTION("PMIC PM8XXX SSBI driver");
 MODULE_VERSION("1.0");
-MODULE_ALIAS("platform:pm8921-core");
+MODULE_ALIAS("platform:pm8xxx-ssbi");
-- 
1.7.0.4

[PATCH v3 0/4] Support for Qualcomm QPNP PMIC's

[Stanimir Varbanov]

Hi all,

On 07/24/2014 03:45 PM, Stanimir Varbanov wrote:
> Hello all,
> 
> Changes since v2:
>  - 1/4 - added new line, signed-off-by / acked-by and module_authors.
>  - 3/4 - the subject has been changed.
> 
> The previous v2 can be found at [1].
> 
> I'm still waiting Acks for:
>  - 4/4 from Qualcomm folks.
>  - 2/4 and 3/4 from DT folks.

Can you look at these patches, please.

-- 
regards,
Stan

[PATCH v3 1/4] mfd: pm8xxx-spmi: add support for Qualcomm SPMI PMICs

[David Collins]

On 07/24/2014 05:45 AM, Stanimir Varbanov wrote:
> From: Josh Cartwright <joshc@codeaurora.org>
> 
> The Qualcomm SPMI PMIC chips are components used with the
> Snapdragon 800 series SoC family.  This driver exists
> largely as a glue mfd component, it exists to be an owner
> of an SPMI regmap for children devices described in
> device tree.
> 
> Signed-off-by: Josh Cartwright <joshc@codeaurora.org>
> Signed-off-by: Stanimir Varbanov <svarbanov@mm-sol.com>
> Acked-by: Lee Jones <lee.jones@linaro.org>
> ---
>  drivers/mfd/Kconfig       |   16 +++++++++++
>  drivers/mfd/Makefile      |    1 +
>  drivers/mfd/pm8xxx-spmi.c |   65 +++++++++++++++++++++++++++++++++++++++++++++

Would it be possible to rename this driver: qcom-spmi-pmic.c?  The driver
will be supporting several PMICs that do not fit the pm8xxx naming scheme.
 One of which is even specified in the compatible list of this driver
(pma8084).  There is presently downstream support for the following PMICs:
PM8019, PM8110, PM8226, PM8841, PM8916, PM8941, PM8994, PMA8084, PMD9635,
PMI8962, and PMI8994 [1].  Four of these do not fit the "PM8XXX" template.

If we can agree on changing the file name for the driver, then all other
instances of "pm8xxx" would need to be renamed in this patch series (e.g.
config options, function names, struct names, DT binding documentation, etc.)

It would probably be good to rename pm8xxx-ssbi.c to qcom-ssbi-pmic.c as
well in order to maintain consistency.

(...)
> +static const struct regmap_config pm8xxx_regmap_config = {
> +	.reg_bits	= 16,
> +	.val_bits	= 8,
> +	.max_register	= 0xffff,

Can you please add the following line here?

	.fast_io	= true;

This will cause a spinlock to be held during SPMI transactions instead of
a mutex lock.  This is needed because several downstream peripheral
drivers need to make SPMI read and write calls from atomic context.  I
have commented on this point in a previous thread with specific examples [2].

> +};

(...)
> +static const struct of_device_id pm8xxx_id_table[] = {
> +	{ .compatible = "qcom,pm8941" },
> +	{ .compatible = "qcom,pm8841" },
> +	{ .compatible = "qcom,pma8084" },

Would it be possible to add a generic compatible string as well?  Perhaps
something like "qcom,spmi-pmic" could be used.  This driver is not doing
anything with the PMIC specific compatible strings.  The generic
compatible string could be specified in device tree in conjunction a PMIC
specific string that is not present in this list.  That way, this driver
file would not need to be touched as new PMIC chips are introduced unless
some weird workaround is needed.  In that case, the PMIC specific
compatible string could be added to the list along with whatever special
function is needed to handle it.

Take care,
David Collins

[1]:
https://www.codeaurora.org/cgit/quic/la/kernel/msm-3.10/tree/arch/arm/boot/dts/qcom?h=msm-3.10

[2]: https://lkml.org/lkml/2014/4/25/720

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

[PATCH v3 2/4] mfd: pm8xxx-spmi: document DT bindings for Qualcomm SPMI PMICs

[David Collins]

On 07/24/2014 05:45 AM, Stanimir Varbanov wrote:
> Document DT bindings used to describe the Qualcomm SPMI PMICs.
> Currently the SPMI PMICs supported are pm8941, pm8841 and pma8084.
> 
> Signed-off-by: Stanimir Varbanov <svarbanov@mm-sol.com>

(...)
> +Required properties for peripheral child nodes:
> +- compatible:      Should contain "qcom,pm8xxx-xxx", where "xxx" is
> +                   peripheral name. The "pm8xxx" can be any of supported PMICs,
> +                   see example below.

I don't think that this binding document should be imposing any formatting
restrictions on the compatible strings for QPNP peripheral drivers.  The
QPNP peripheral drivers in the downstream msm-3.10 tree [1] do not specify
per-PMIC compatible strings.  This is because ideally, a given QPNP
peripheral represents a hardware block that is identical in interface and
operation between PMICs.

These peripheral drivers determine the base address for a given device
instance via device tree reg and reg-names properties.  In order for this
to continue to work with the pm8xxx-spmi driver, some mechanism will need
to be introduced which creates resource structs for the
non-memory-mappable SPMI base addresses.  One possible solution is
currently being discussed in another thread [2].  This document will need
to be updated to show the child node reg property scheme once a solution
is reached.

(...)
> +Example:
> +
> +	pm8941 at 0 {
> +		compatible = "qcom,pm8941";
> +		reg = <0x0 SPMI_USID>;
> +
> +		rtc {
> +			compatible = "qcom,pm8941-rtc";
> +			interrupts = <0x0 0x61 0x1 IRQ_TYPE_EDGE_RISING>;
> +			interrupt-names = "alarm";
> +		};
> +	};

Can you please expand your example to include the second SID for the
PM8941 chip?  That way, it will be clear that each PMIC needs two DT
nodes; one for each SID.

Thanks,
David Collins

[1]: https://www.codeaurora.org/cgit/quic/la/kernel/msm-3.10/tree/?h=msm-3.10

[2]: https://lkml.org/lkml/2014/7/29/252

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

[PATCH v3 1/4] mfd: pm8xxx-spmi: add support for Qualcomm SPMI PMICs

[Stanimir Varbanov]

Hi David,

Thanks for the comments!

On 07/30/2014 12:54 AM, David Collins wrote:
> On 07/24/2014 05:45 AM, Stanimir Varbanov wrote:
>> From: Josh Cartwright <joshc@codeaurora.org>
>>
>> The Qualcomm SPMI PMIC chips are components used with the
>> Snapdragon 800 series SoC family.  This driver exists
>> largely as a glue mfd component, it exists to be an owner
>> of an SPMI regmap for children devices described in
>> device tree.
>>
>> Signed-off-by: Josh Cartwright <joshc@codeaurora.org>
>> Signed-off-by: Stanimir Varbanov <svarbanov@mm-sol.com>
>> Acked-by: Lee Jones <lee.jones@linaro.org>
>> ---
>>  drivers/mfd/Kconfig       |   16 +++++++++++
>>  drivers/mfd/Makefile      |    1 +
>>  drivers/mfd/pm8xxx-spmi.c |   65 +++++++++++++++++++++++++++++++++++++++++++++
> 
> Would it be possible to rename this driver: qcom-spmi-pmic.c?  The driver
> will be supporting several PMICs that do not fit the pm8xxx naming scheme.
>  One of which is even specified in the compatible list of this driver
> (pma8084).  There is presently downstream support for the following PMICs:
> PM8019, PM8110, PM8226, PM8841, PM8916, PM8941, PM8994, PMA8084, PMD9635,
> PMI8962, and PMI8994 [1].  Four of these do not fit the "PM8XXX" template.

I haven't strong opinion on the file names. The qcom prefix is the one
which annoying me. If you look at /drivers/mfd the company name prefixes
are very few.

The *compatible* strings are the important thing here. So If MFD
maintainer is fine with this name I'm fine too.

> 
> If we can agree on changing the file name for the driver, then all other
> instances of "pm8xxx" would need to be renamed in this patch series (e.g.
> config options, function names, struct names, DT binding documentation, etc.)
> 
> It would probably be good to rename pm8xxx-ssbi.c to qcom-ssbi-pmic.c as
> well in order to maintain consistency.
> 
> (...)
>> +static const struct regmap_config pm8xxx_regmap_config = {
>> +	.reg_bits	= 16,
>> +	.val_bits	= 8,
>> +	.max_register	= 0xffff,
> 
> Can you please add the following line here?
> 
> 	.fast_io	= true;
> 
> This will cause a spinlock to be held during SPMI transactions instead of
> a mutex lock.  This is needed because several downstream peripheral
> drivers need to make SPMI read and write calls from atomic context.  I
> have commented on this point in a previous thread with specific examples [2].

OK, I understand the need of atomic context, but pmic_arb_read_cmd() and
pmic_arb_write_cmd() functions use raw_spin_lock_irqsave already. Isn't
those locks enough?

> 
>> +};
> 
> (...)
>> +static const struct of_device_id pm8xxx_id_table[] = {
>> +	{ .compatible = "qcom,pm8941" },
>> +	{ .compatible = "qcom,pm8841" },
>> +	{ .compatible = "qcom,pma8084" },
> 
> Would it be possible to add a generic compatible string as well?  Perhaps
> something like "qcom,spmi-pmic" could be used.  This driver is not doing
> anything with the PMIC specific compatible strings.  The generic
> compatible string could be specified in device tree in conjunction a PMIC
> specific string that is not present in this list.  That way, this driver
> file would not need to be touched as new PMIC chips are introduced unless
> some weird workaround is needed.  In that case, the PMIC specific
> compatible string could be added to the list along with whatever special
> function is needed to handle it.

OK, I'm fine with this suggestion.

-- 
regards,
Stan

[PATCH v3 2/4] mfd: pm8xxx-spmi: document DT bindings for Qualcomm SPMI PMICs

[Stanimir Varbanov]

On 07/30/2014 01:23 AM, David Collins wrote:
> On 07/24/2014 05:45 AM, Stanimir Varbanov wrote:
>> Document DT bindings used to describe the Qualcomm SPMI PMICs.
>> Currently the SPMI PMICs supported are pm8941, pm8841 and pma8084.
>>
>> Signed-off-by: Stanimir Varbanov <svarbanov@mm-sol.com>
> 
> (...)
>> +Required properties for peripheral child nodes:
>> +- compatible:      Should contain "qcom,pm8xxx-xxx", where "xxx" is
>> +                   peripheral name. The "pm8xxx" can be any of supported PMICs,
>> +                   see example below.
> 
> I don't think that this binding document should be imposing any formatting
> restrictions on the compatible strings for QPNP peripheral drivers.  The
> QPNP peripheral drivers in the downstream msm-3.10 tree [1] do not specify
> per-PMIC compatible strings.  This is because ideally, a given QPNP
> peripheral represents a hardware block that is identical in interface and
> operation between PMICs.
> 

Isn't "hardware block that is identical in interface and operation
between PMICs" exactly the meaning of *compatible* property?

No *compatible* property, no platform device. We must have this property
for every peripheral driver.

> These peripheral drivers determine the base address for a given device
> instance via device tree reg and reg-names properties.  In order for this
> to continue to work with the pm8xxx-spmi driver, some mechanism will need
> to be introduced which creates resource structs for the
> non-memory-mappable SPMI base addresses.  One possible solution is
> currently being discussed in another thread [2].  This document will need
> to be updated to show the child node reg property scheme once a solution
> is reached.
> 

That's correct. If we reach the "reg" solution this binding document
must be changed.

> (...)
>> +Example:
>> +
>> +	pm8941 at 0 {
>> +		compatible = "qcom,pm8941";
>> +		reg = <0x0 SPMI_USID>;
>> +
>> +		rtc {
>> +			compatible = "qcom,pm8941-rtc";
>> +			interrupts = <0x0 0x61 0x1 IRQ_TYPE_EDGE_RISING>;
>> +			interrupt-names = "alarm";
>> +		};
>> +	};
> 
> Can you please expand your example to include the second SID for the
> PM8941 chip?  That way, it will be clear that each PMIC needs two DT
> nodes; one for each SID.

Sure, will do.

-- 
regards,
Stan

[PATCH v3 1/4] mfd: pm8xxx-spmi: add support for Qualcomm SPMI PMICs

[David Collins]

On 07/31/2014 01:48 AM, Stanimir Varbanov wrote:
> Hi David,
> 
> Thanks for the comments!
> 
> On 07/30/2014 12:54 AM, David Collins wrote:
>> On 07/24/2014 05:45 AM, Stanimir Varbanov wrote:
>>> From: Josh Cartwright <joshc@codeaurora.org>
>>>
>>> The Qualcomm SPMI PMIC chips are components used with the
>>> Snapdragon 800 series SoC family.  This driver exists
>>> largely as a glue mfd component, it exists to be an owner
>>> of an SPMI regmap for children devices described in
>>> device tree.

(...)
>>> +static const struct regmap_config pm8xxx_regmap_config = {
>>> +	.reg_bits	= 16,
>>> +	.val_bits	= 8,
>>> +	.max_register	= 0xffff,
>>
>> Can you please add the following line here?
>>
>> 	.fast_io	= true;
>>
>> This will cause a spinlock to be held during SPMI transactions instead of
>> a mutex lock.  This is needed because several downstream peripheral
>> drivers need to make SPMI read and write calls from atomic context.  I
>> have commented on this point in a previous thread with specific examples [2].
> 
> OK, I understand the need of atomic context, but pmic_arb_read_cmd() and
> pmic_arb_write_cmd() functions use raw_spin_lock_irqsave already. Isn't
> those locks enough?

No, that isn't sufficient.  The problem is that the peripheral driver
would already be in atomic context at the time that it needs to perform an
SPMI read or write via regmap_read() or regmap_write() respectively.
These regmap calls would take a mutex lock if fast_io == false.  This is
not allowed since calling a sleepable function in atomic context can lead
to deadlock.

Take care,
David

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

[PATCH v3 2/4] mfd: pm8xxx-spmi: document DT bindings for Qualcomm SPMI PMICs

[David Collins]

On 07/31/2014 01:58 AM, Stanimir Varbanov wrote:
> On 07/30/2014 01:23 AM, David Collins wrote:
>> On 07/24/2014 05:45 AM, Stanimir Varbanov wrote:
>>> Document DT bindings used to describe the Qualcomm SPMI PMICs.
>>> Currently the SPMI PMICs supported are pm8941, pm8841 and pma8084.
>>>
>>> Signed-off-by: Stanimir Varbanov <svarbanov@mm-sol.com>
>>
>> (...)
>>> +Required properties for peripheral child nodes:
>>> +- compatible:      Should contain "qcom,pm8xxx-xxx", where "xxx" is
>>> +                   peripheral name. The "pm8xxx" can be any of supported PMICs,
>>> +                   see example below.
>>
>> I don't think that this binding document should be imposing any formatting
>> restrictions on the compatible strings for QPNP peripheral drivers.  The
>> QPNP peripheral drivers in the downstream msm-3.10 tree [1] do not specify
>> per-PMIC compatible strings.  This is because ideally, a given QPNP
>> peripheral represents a hardware block that is identical in interface and
>> operation between PMICs.
>>
> 
> Isn't "hardware block that is identical in interface and operation
> between PMICs" exactly the meaning of *compatible* property?
> 
> No *compatible* property, no platform device. We must have this property
> for every peripheral driver.

I am not suggesting that we remove the compatible property for peripheral
device nodes.  I agree that these nodes need to have a compatible
property.  My concern is that this binding document should not require the
peripheral nodes to have compatible property values that are PMIC specific.

For example, in the downstream msm-3.10 branch the power-on peripheral
uses the same compatible string regardless of what PMIC the peripheral is
found on: "qcom,qpnp-power-on" [1].  There is no reason for the driver to
care about what PMIC the peripheral is found on.  Therefore, it does not
make sense to force it to support compatible strings like
"qcom,pm8941-power-on", "qcom,pm8841-power-on", "qcom,pma8084-power-on", etc.

Every QPNP PMIC peripheral has its own set of type and version registers
that allow software to auto-detect its capabilities at runtime.

Take care,
David

[1]:
https://www.codeaurora.org/cgit/quic/la/kernel/msm-3.10/tree/Documentation/devicetree/bindings/platform/msm/qpnp-power-on.txt?h=msm-3.10

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

[PATCH v3 1/4] mfd: pm8xxx-spmi: add support for Qualcomm SPMI PMICs

[Stanimir Varbanov]

On 07/31/2014 11:48 AM, Stanimir Varbanov wrote:
> Hi David,
> 
> Thanks for the comments!
> 
> On 07/30/2014 12:54 AM, David Collins wrote:
>> On 07/24/2014 05:45 AM, Stanimir Varbanov wrote:
>>> From: Josh Cartwright <joshc@codeaurora.org>
>>>
>>> The Qualcomm SPMI PMIC chips are components used with the
>>> Snapdragon 800 series SoC family.  This driver exists
>>> largely as a glue mfd component, it exists to be an owner
>>> of an SPMI regmap for children devices described in
>>> device tree.
>>>
>>> Signed-off-by: Josh Cartwright <joshc@codeaurora.org>
>>> Signed-off-by: Stanimir Varbanov <svarbanov@mm-sol.com>
>>> Acked-by: Lee Jones <lee.jones@linaro.org>
>>> ---
>>>  drivers/mfd/Kconfig       |   16 +++++++++++
>>>  drivers/mfd/Makefile      |    1 +
>>>  drivers/mfd/pm8xxx-spmi.c |   65 +++++++++++++++++++++++++++++++++++++++++++++
>>
>> Would it be possible to rename this driver: qcom-spmi-pmic.c?  The driver
>> will be supporting several PMICs that do not fit the pm8xxx naming scheme.
>>  One of which is even specified in the compatible list of this driver
>> (pma8084).  There is presently downstream support for the following PMICs:
>> PM8019, PM8110, PM8226, PM8841, PM8916, PM8941, PM8994, PMA8084, PMD9635,
>> PMI8962, and PMI8994 [1].  Four of these do not fit the "PM8XXX" template.
> 
> I haven't strong opinion on the file names. The qcom prefix is the one
> which annoying me. If you look at /drivers/mfd the company name prefixes
> are very few.
> 
> The *compatible* strings are the important thing here. So If MFD
> maintainer is fine with this name I'm fine too.

Lee, are you OK with suggested names qcom-spmi-pmic and qcom-ssbi-pmic?

-- 
regards,
Stan

[PATCH v3 1/4] mfd: pm8xxx-spmi: add support for Qualcomm SPMI PMICs

[Lee Jones]

On Fri, 01 Aug 2014, Stanimir Varbanov wrote:
> On 07/31/2014 11:48 AM, Stanimir Varbanov wrote:
> > On 07/30/2014 12:54 AM, David Collins wrote:
> >> On 07/24/2014 05:45 AM, Stanimir Varbanov wrote:
> >>> From: Josh Cartwright <joshc@codeaurora.org>
> >>>
> >>> The Qualcomm SPMI PMIC chips are components used with the
> >>> Snapdragon 800 series SoC family.  This driver exists
> >>> largely as a glue mfd component, it exists to be an owner
> >>> of an SPMI regmap for children devices described in
> >>> device tree.
> >>>
> >>> Signed-off-by: Josh Cartwright <joshc@codeaurora.org>
> >>> Signed-off-by: Stanimir Varbanov <svarbanov@mm-sol.com>
> >>> Acked-by: Lee Jones <lee.jones@linaro.org>
> >>> ---
> >>>  drivers/mfd/Kconfig       |   16 +++++++++++
> >>>  drivers/mfd/Makefile      |    1 +
> >>>  drivers/mfd/pm8xxx-spmi.c |   65 +++++++++++++++++++++++++++++++++++++++++++++
> >>
> >> Would it be possible to rename this driver: qcom-spmi-pmic.c?  The driver
> >> will be supporting several PMICs that do not fit the pm8xxx naming scheme.
> >>  One of which is even specified in the compatible list of this driver
> >> (pma8084).  There is presently downstream support for the following PMICs:
> >> PM8019, PM8110, PM8226, PM8841, PM8916, PM8941, PM8994, PMA8084, PMD9635,
> >> PMI8962, and PMI8994 [1].  Four of these do not fit the "PM8XXX" template.
> > 
> > I haven't strong opinion on the file names. The qcom prefix is the one
> > which annoying me. If you look at /drivers/mfd the company name prefixes
> > are very few.
> > 
> > The *compatible* strings are the important thing here. So If MFD
> > maintainer is fine with this name I'm fine too.
> 
> Lee, are you OK with suggested names qcom-spmi-pmic and qcom-ssbi-pmic?

Sounds fine to me.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog