All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH 0/6] Support for QPNP PMIC's
@ 2014-06-20 12:21 Stanimir Varbanov
  2014-06-20 12:21 ` [RFC PATCH 1/6] dt: qpnp: add binding description of Qualcomm QPNP PMICs Stanimir Varbanov
                   ` (6 more replies)
  0 siblings, 7 replies; 14+ messages in thread
From: Stanimir Varbanov @ 2014-06-20 12:21 UTC (permalink / raw)
  To: linux-arm-msm; +Cc: Stanimir Varbanov

Hello to all,

The goal of this WIP set is to provide support for sub-functional
hw blocks inside Qualcomm QPNP PMIC chips by creating a new qpnp
bus (device-driver modeled). On SPMI interface we attach a so called
Slaves. For example one PMIC chip like pm8941 has two slave id's. On
every slaveid the PMIC chip has various peripherials. Every peripheral
driver should expose qpnp_driver structure and implement the .probe
and .remove callbacks. Every peripheral driver will reside on the
proper place in /drivers directory. 

The fisrt patch describes the devicetree binding of the slave devices
attached to the SPMI interface. The second patch adds implemetation of
the qpnp-bus and a layer connection with the SPMI interface. The third
patch adds spmi arbiter and underlying slaveid devicetree nodes for
msm8974 SoC. The other patches are example of rtc-qpnp driver and
binding documentation.

Since this set is a WIP, it will be used as a starting point for future
duscussions about implementing QPNP PMIC sub-function hw blocks.

Regards,
Stan

Ivan T. Ivanov (2):
  dt: qpnp: add binding description of Qualcomm QPNP PMICs
  dt: qcom: msm8974: add qpnp-spmi device nodes

Stanimir Varbanov (4):
  qpnp: add support for Qualcomm QPNP PMICs
  rtc: add qpnp rtc driver
  dt: msm8974: add qpnp rtc device node
  dt: rtc: add binding document for rtc-qpnp

 .../devicetree/bindings/qpnp/qcom,qpnp-spmi.txt    |   42 ++
 .../devicetree/bindings/rtc/qcom,rtc-qpnp.txt      |   21 +
 arch/arm/boot/dts/qcom-msm8974.dtsi                |   52 ++
 drivers/Kconfig                                    |    2 +
 drivers/Makefile                                   |    1 +
 drivers/qpnp/Kconfig                               |   12 +
 drivers/qpnp/Makefile                              |    1 +
 drivers/qpnp/qpnp-bus.c                            |  475 +++++++++++++++++++
 drivers/qpnp/qpnp-spmi.c                           |   63 +++
 drivers/rtc/Kconfig                                |    8 +
 drivers/rtc/Makefile                               |    1 +
 drivers/rtc/rtc-qpnp.c                             |  489 ++++++++++++++++++++
 include/linux/qpnp.h                               |   83 ++++
 13 files changed, 1250 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/qpnp/qcom,qpnp-spmi.txt
 create mode 100644 Documentation/devicetree/bindings/rtc/qcom,rtc-qpnp.txt
 create mode 100644 drivers/qpnp/Kconfig
 create mode 100644 drivers/qpnp/Makefile
 create mode 100644 drivers/qpnp/qpnp-bus.c
 create mode 100644 drivers/qpnp/qpnp-spmi.c
 create mode 100644 drivers/rtc/rtc-qpnp.c
 create mode 100644 include/linux/qpnp.h

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [RFC PATCH 1/6] dt: qpnp: add binding description of Qualcomm QPNP PMICs
  2014-06-20 12:21 [RFC PATCH 0/6] Support for QPNP PMIC's Stanimir Varbanov
@ 2014-06-20 12:21 ` Stanimir Varbanov
  2014-06-20 12:21 ` [RFC PATCH 2/6] qpnp: add support for " Stanimir Varbanov
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 14+ messages in thread
From: Stanimir Varbanov @ 2014-06-20 12:21 UTC (permalink / raw)
  To: linux-arm-msm; +Cc: Ivan T. Ivanov, Stanimir Varbanov

From: Ivan T. Ivanov <iivanov@mm-sol.com>

Document the bindings used to describe the Qualcomm QPNP PMICs.

Suggested-by: Josh Cartwright <joshc@codeaurora.org>
Signed-off-by: Ivan T. Ivanov <iivanov@mm-sol.com>
Signed-off-by: Stanimir Varbanov <svarbanov@mm-sol.com>
---
 .../devicetree/bindings/qpnp/qcom,qpnp-spmi.txt    |   42 ++++++++++++++++++++
 1 files changed, 42 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/qpnp/qcom,qpnp-spmi.txt

diff --git a/Documentation/devicetree/bindings/qpnp/qcom,qpnp-spmi.txt b/Documentation/devicetree/bindings/qpnp/qcom,qpnp-spmi.txt
new file mode 100644
index 0000000..f657587
--- /dev/null
+++ b/Documentation/devicetree/bindings/qpnp/qcom,qpnp-spmi.txt
@@ -0,0 +1,42 @@
+Qualcomm QPNP partitioned PMIC multi-function device bindings
+
+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 "qcom,qpnp-spmi"
+- reg:            Specifies the SPMI USID slave address for this device.
+                  For more info see bindings/spmi/spmi.txt
+- #address-cells: Defines address cells for peripheral nodes - should be 1
+- #size-cells:    Defines size cells for peripheral nodes - should be 0
+
+Required properties for peripheral devices:
+- compatible:     Should constain "qcom,qpnp-xxx"
+- reg:            One or more 8bits peripheral ID
+
+Each child node represents a function of the QPNP.  Each child 'reg' entry
+describes a QPNP peripheral ID (number) within the USID slave address where
+the region starts.
+
+Example:
+
+	pm8941@0 {
+		compatible = "qcom,qpnp-spmi";
+		reg = <0x0 SPMI_USID>;
+
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		rtc@60 {
+			compatible = "qcom,qpnp-rtc";
+			reg = <0x60>, <0x61>;
+		};
+	};
\ No newline at end of file
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [RFC PATCH 2/6] qpnp: add support for Qualcomm QPNP PMICs
  2014-06-20 12:21 [RFC PATCH 0/6] Support for QPNP PMIC's Stanimir Varbanov
  2014-06-20 12:21 ` [RFC PATCH 1/6] dt: qpnp: add binding description of Qualcomm QPNP PMICs Stanimir Varbanov
@ 2014-06-20 12:21 ` Stanimir Varbanov
  2014-06-20 12:21 ` [RFC PATCH 3/6] dt: qcom: msm8974: add qpnp-spmi device nodes Stanimir Varbanov
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 14+ messages in thread
From: Stanimir Varbanov @ 2014-06-20 12:21 UTC (permalink / raw)
  To: linux-arm-msm; +Cc: Stanimir Varbanov, Ivan T. Ivanov

The Qualcomm plug-and-play (QPNP) PMIC chips are components
used with the Snapdragon series SoC family. QPNP chips make
use of Qualcomm's SPMI register convention.

This driver exists largely as a glue component, it exists to
be an owner of an SPMI regmap for children devices described
in device tree.

Suggested-by: Josh Cartwright <joshc@codeaurora.org>
Signed-off-by: Ivan T. Ivanov <iivanov@mm-sol.com>
Signed-off-by: Stanimir Varbanov <svarbanov@mm-sol.com>
---
 drivers/Kconfig          |    2 +
 drivers/Makefile         |    1 +
 drivers/qpnp/Kconfig     |   12 ++
 drivers/qpnp/Makefile    |    1 +
 drivers/qpnp/qpnp-bus.c  |  475 ++++++++++++++++++++++++++++++++++++++++++++++
 drivers/qpnp/qpnp-spmi.c |   63 ++++++
 include/linux/qpnp.h     |   83 ++++++++
 7 files changed, 637 insertions(+), 0 deletions(-)
 create mode 100644 drivers/qpnp/Kconfig
 create mode 100644 drivers/qpnp/Makefile
 create mode 100644 drivers/qpnp/qpnp-bus.c
 create mode 100644 drivers/qpnp/qpnp-spmi.c
 create mode 100644 include/linux/qpnp.h

diff --git a/drivers/Kconfig b/drivers/Kconfig
index 0e87a34..412e6ae 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -176,4 +176,6 @@ source "drivers/powercap/Kconfig"
 
 source "drivers/mcb/Kconfig"
 
+source "drivers/qpnp/Kconfig"
+
 endmenu
diff --git a/drivers/Makefile b/drivers/Makefile
index f98b50d..bf2f3f3 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -158,3 +158,4 @@ obj-$(CONFIG_NTB)		+= ntb/
 obj-$(CONFIG_FMC)		+= fmc/
 obj-$(CONFIG_POWERCAP)		+= powercap/
 obj-$(CONFIG_MCB)		+= mcb/
+obj-y				+= qpnp/
diff --git a/drivers/qpnp/Kconfig b/drivers/qpnp/Kconfig
new file mode 100644
index 0000000..c86ab52
--- /dev/null
+++ b/drivers/qpnp/Kconfig
@@ -0,0 +1,12 @@
+config QPNP_SPMI
+	tristate "Qualcomm PMIC QPNP driver"
+	depends on ARCH_QCOM
+	select REGMAP_SPMI
+	help
+	  This enables basic support for the Qualcomm QPNP 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 PM8x41 series as a
+	  module.  The module will be called "qpnp-spmi".
diff --git a/drivers/qpnp/Makefile b/drivers/qpnp/Makefile
new file mode 100644
index 0000000..753bef8
--- /dev/null
+++ b/drivers/qpnp/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_QPNP_SPMI)	+= qpnp-bus.o qpnp-spmi.o
diff --git a/drivers/qpnp/qpnp-bus.c b/drivers/qpnp/qpnp-bus.c
new file mode 100644
index 0000000..70f72ad
--- /dev/null
+++ b/drivers/qpnp/qpnp-bus.c
@@ -0,0 +1,475 @@
+/* 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/errno.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/of_irq.h>
+#include <linux/qpnp.h>
+#include <linux/slab.h>
+
+#define QPNP_RESOURCE_SIZE	256
+
+static inline struct qpnp_driver *to_qpnp_driver(struct device_driver *dd)
+{
+	return container_of(dd, struct qpnp_driver, driver);
+}
+
+static void qpnp_device_release(struct device *dev)
+{
+	struct qpnp_device *qdev = to_qpnp_device(dev);
+
+	of_device_node_put(dev);
+	kfree(qdev->resource);
+	kfree(qdev);
+}
+
+static int qpnp_device_match(struct device *dev, struct device_driver *dd)
+{
+	if (of_driver_match_device(dev, dd))
+		return 1;
+
+	return 0;
+}
+
+static int qpnp_driver_probe(struct device *dev)
+{
+	const struct qpnp_driver *qdrv = to_qpnp_driver(dev->driver);
+	struct qpnp_device *qdev = to_qpnp_device(dev);
+
+	return qdrv->probe(qdev);
+}
+
+static int qpnp_driver_remove(struct device *dev)
+{
+	const struct qpnp_driver *qdrv = to_qpnp_driver(dev->driver);
+	struct qpnp_device *qdev = to_qpnp_device(dev);
+
+	qdrv->remove(qdev);
+	return 0;
+}
+
+static struct bus_type qpnp_bus_type = {
+	.name = "qpnp",
+	.match = qpnp_device_match,
+	.probe = qpnp_driver_probe,
+	.remove = qpnp_driver_remove,
+};
+
+/**
+ * qpnp_set_device_name - Use the device node data to assign a unique name
+ * @dev: pointer to device structure that is linked to a device tree node
+ *
+ * This routine will first try using the reg property value to derive a
+ * unique name.  As a last resort it will use the node name followed by
+ * a unique number.
+ */
+static void qpnp_set_device_name(struct device *dev, u8 sid)
+{
+	static atomic_t bus_no_reg_magic;
+	struct device_node *np = dev->of_node;
+	const __be32 *reg;
+	const __be32 *addrp = NULL;
+	u64 addr;
+
+	reg = of_get_property(np, "reg", NULL);
+	if (reg) {
+		addrp = of_get_address(np, 0, NULL, NULL);
+		if (addrp)
+			addr = of_read_number(addrp, 1);
+	}
+
+	if (!addrp)
+		/*
+		 * No BusID, use the np name and add a globally
+		 * incremented counter (and pray...)
+		 */
+		addr = atomic_add_return(1, &bus_no_reg_magic);
+
+	dev_set_name(dev, "%x.%02x.%s", sid, (u8) addr, np->name);
+}
+
+/**
+ * qpnp_device_alloc() - Allocate a new QPNP device
+ *
+ * Allocate a new QPNP device, fills QPNP device members and set device name.
+ */
+static struct qpnp_device *qpnp_device_alloc(struct device_node *np,
+					     struct device *parent, u8 sid,
+					     struct regmap *map)
+{
+	struct qpnp_device *qdev;
+
+	qdev = kzalloc(sizeof(*qdev), GFP_KERNEL);
+	if (!qdev)
+		return NULL;
+
+	qdev->sid = sid;
+	qdev->map = map;
+	qdev->dev.of_node = of_node_get(np);
+	qdev->dev.parent = parent;
+	qdev->dev.release = qpnp_device_release;
+	qdev->dev.bus = &qpnp_bus_type;
+
+	qpnp_set_device_name(&qdev->dev, sid);
+
+	return qdev;
+}
+
+/**
+ * qpnp_index_to_resource - Parse device tree address and return as resource
+ */
+static int
+qpnp_index_to_resource(struct device *dev, int index, struct resource *res)
+{
+	struct device_node *np = dev->of_node;
+	const __be32 *addrp;
+	u64 addr;
+	const char *name = NULL;
+
+	addrp = of_get_address(np, index, NULL, NULL);
+	if (addrp == NULL)
+		return -EINVAL;
+
+	addr = of_read_number(addrp, 1);
+
+	if (addr == OF_BAD_ADDR)
+		return -EINVAL;
+
+	/* Get optional "reg-names" property to add a name to a resource */
+	of_property_read_string_index(np, "reg-names", index, &name);
+
+	/* peripheral id is 8bits */
+	addr &= 0xff;
+	addr <<= 8;
+
+	res->start = addr;
+	res->end = addr + QPNP_RESOURCE_SIZE - 1;
+	res->flags = IORESOURCE_REG;
+	res->name = name ? name : np->full_name;
+
+	return 0;
+}
+
+/**
+ * qpnp_device_add_resources() - Allocate and add resources for QPNP device
+ * @qdev: qpnp device pointer
+ */
+static int qpnp_device_add_resources(struct qpnp_device *qdev)
+{
+	struct device_node *np = qdev->dev.of_node;
+	struct resource *res;
+	int ret, idx, num_resources = 0;
+
+	while (of_get_address(np, num_resources, NULL, NULL) != NULL)
+		num_resources++;
+
+	if (!num_resources)
+		return 0;
+
+	res = kcalloc(num_resources, sizeof(*res), GFP_KERNEL);
+	if (!res)
+		return -ENOMEM;
+
+	qdev->num_resources = num_resources;
+	qdev->resource = res;
+
+	/* Populate the resource table */
+	for (idx = 0; idx < num_resources; idx++, res++) {
+		ret = qpnp_index_to_resource(&qdev->dev, idx, res);
+		if (ret)
+			goto err;
+	}
+
+	return 0;
+err:
+	kfree(qdev->resource);
+	return ret;
+}
+
+/**
+ * qpnp_device_add - Alloc, initialize and register an device
+ * @np: pointer to node to create device for
+ * @parent: Linux device model parent device.
+ * @sid: Slave ID
+ *
+ * Returns pointer to created qpnp device, or NULL if a device was not
+ * registered.
+ */
+static struct qpnp_device *qpnp_device_add(struct device_node *np,
+					   struct device *parent, u8 sid,
+					   struct regmap *map)
+{
+	struct qpnp_device *qdev;
+	int ret;
+
+	qdev = qpnp_device_alloc(np, parent, sid, map);
+	if (!qdev)
+		return NULL;
+
+	ret = qpnp_device_add_resources(qdev);
+	if (ret)
+		goto fail;
+
+	ret = device_register(&qdev->dev);
+	if (ret)
+		goto fail;
+
+	return qdev;
+
+fail:
+	put_device(&qdev->dev);
+	return NULL;
+}
+
+static int qpnp_device_remove(struct device *dev, void *unused)
+{
+	device_unregister(dev);
+
+	return 0;
+}
+
+/**
+ * qpnp_node_create() - Create a device for a node and its children.
+ * @np: device np of the node to instantiate
+ * @parent: parent for new device
+ * @sid: Slave ID
+ *
+ * Creates a qpnp_device for the provided device_node, and
+ * recursively create devices for all the child nodes.
+ */
+static int qpnp_node_create(struct device_node *np, struct device *parent,
+			    u8 sid, struct regmap *map)
+{
+	struct device_node *child;
+	struct qpnp_device *qdev;
+	int ret = 0;
+
+	/* Make sure it has a compatible property */
+	if (!of_get_property(np, "compatible", NULL)) {
+		pr_debug("skipping %s, no compatible prop\n", np->full_name);
+		return 0;
+	}
+
+	qdev = qpnp_device_add(np, parent, sid, map);
+	if (!qdev)
+		return -ENOMEM;
+
+	for_each_available_child_of_node(np, child) {
+		pr_debug("   create child: %s\n", child->full_name);
+		ret = qpnp_node_create(child, &qdev->dev, sid, map);
+		if (ret) {
+			of_node_put(child);
+			break;
+		}
+	}
+
+	return ret;
+}
+
+/**
+ * qpnp_populate_devices() - Populate qpnp_devices from device tree data
+ * @root: parent of the first level to probe
+ * @parent: parent to hook devices from
+ * @sid: Slave ID
+ *
+ * This function walks the device tree and creates devices from nodes.
+ * It follows convention of requiring all device nodes to have a 'compatible'
+ * property, and it is suitable for creating devices which are children
+ * of the root.
+ *
+ * Returns 0 on success, < 0 on failure.
+ */
+int qpnp_populate_devices(struct device_node *root, struct device *parent,
+			  u8 sid, struct regmap *map)
+{
+	struct device_node *child;
+	int ret = 0;
+
+	root = of_node_get(root);
+	if (!root)
+		return -EINVAL;
+
+	for_each_available_child_of_node(root, child) {
+		ret = qpnp_node_create(child, parent, sid, map);
+		if (ret)
+			break;
+	}
+
+	of_node_put(root);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(qpnp_populate_devices);
+
+/**
+ * qpnp_remove_devices(): remove a QPNP devices
+ * @root: root to devices to be removed
+ */
+void qpnp_remove_devices(struct device *root)
+{
+	device_for_each_child(root, NULL, qpnp_device_remove);
+}
+EXPORT_SYMBOL_GPL(qpnp_remove_devices);
+
+/**
+ * __of_irq_get - Decode a node's IRQ and return it as a Linux irq number
+ * @dev: pointer to device tree node
+ * @index: zero-based index of the irq
+ *
+ * Returns Linux irq number on success, -EPROBE_DEFER if the irq domain
+ * is not yet created.
+ */
+static int __of_irq_get(struct device_node *dev, int index)
+{
+	struct of_phandle_args oirq;
+	struct irq_domain *domain;
+	int ret;
+
+	ret = of_irq_parse_one(dev, index, &oirq);
+	if (ret)
+		return ret;
+
+	domain = irq_find_host(oirq.np);
+	if (!domain)
+		return -EPROBE_DEFER;
+
+	return irq_create_of_mapping(&oirq);
+}
+
+/**
+ * qpnp_get_irq - get an IRQ for a device
+ */
+int qpnp_get_irq(struct qpnp_device *qdev, unsigned int num)
+{
+	return __of_irq_get(qdev->dev.of_node, num);
+}
+EXPORT_SYMBOL_GPL(qpnp_get_irq);
+
+/**
+ * qpnp_get_irq_byname - get an IRQ for a device by name
+ */
+int qpnp_get_irq_byname(struct qpnp_device *qdev, const char *name)
+{
+	struct device_node *np = qdev->dev.of_node;
+	int index;
+
+	if (!name)
+		return -EINVAL;
+
+	index = of_property_match_string(np, "interrupt-names", name);
+	if (index < 0)
+		return index;
+
+	return __of_irq_get(np, index);
+}
+EXPORT_SYMBOL_GPL(qpnp_get_irq_byname);
+
+/**
+ * qpnp_get_resource - get a resource for a device
+ * @dev: qpnp device
+ * @type: resource type
+ * @num: resource index
+ */
+struct resource *qpnp_get_resource(struct qpnp_device *dev,
+				   unsigned int type, unsigned int num)
+{
+	struct resource *res;
+	int idx;
+
+	if (type != IORESOURCE_REG)
+		return NULL;
+
+	for (idx = 0; idx < dev->num_resources; idx++) {
+		res = &dev->resource[idx];
+
+		if (type == resource_type(res) && num-- == 0)
+			return res;
+	}
+
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(qpnp_get_resource);
+
+/**
+ * qqpnp_get_region_byname - get a resource for a device by name
+ * @dev: qpnp device
+ * @type: resource type
+ * @name: resource name
+ */
+struct resource *qpnp_get_resource_byname(struct qpnp_device *dev,
+					  unsigned int type, const char *name)
+{
+	struct resource *res;
+	int idx;
+
+	if (type != IORESOURCE_REG)
+		return NULL;
+
+	for (idx = 0; idx < dev->num_resources; idx++) {
+		res = &dev->resource[idx];
+
+		if (!res->name)
+			continue;
+
+		if (type == resource_type(res) && !strcmp(res->name, name))
+			return res;
+	}
+
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(qpnp_get_resource_byname);
+
+/**
+ * qpnp_driver_register() - Register client driver with QPNP bus
+ * @qdrv: client driver to be associated with client-device.
+ *
+ * This API will register the client driver with the QPNP framework.
+ * It is typically called from the driver's module-init function.
+ */
+int qpnp_driver_register(struct qpnp_driver *qdrv)
+{
+	qdrv->driver.bus = &qpnp_bus_type;
+
+	return driver_register(&qdrv->driver);
+}
+EXPORT_SYMBOL_GPL(qpnp_driver_register);
+
+/**
+ * qpnp_driver_unregister() - unregister a QPNP client driver
+ * @qdrv: the driver to unregister
+ */
+void qpnp_driver_unregister(struct qpnp_driver *qdrv)
+{
+	if (qdrv)
+		driver_unregister(&qdrv->driver);
+}
+EXPORT_SYMBOL_GPL(qpnp_driver_unregister);
+
+static void __exit qpnp_exit(void)
+{
+	bus_unregister(&qpnp_bus_type);
+}
+
+module_exit(qpnp_exit);
+
+static int __init qpnp_init(void)
+{
+	return bus_register(&qpnp_bus_type);
+}
+
+postcore_initcall(qpnp_init);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("QPNP bus module");
+MODULE_ALIAS("qpnp");
diff --git a/drivers/qpnp/qpnp-spmi.c b/drivers/qpnp/qpnp-spmi.c
new file mode 100644
index 0000000..c3cb841
--- /dev/null
+++ b/drivers/qpnp/qpnp-spmi.c
@@ -0,0 +1,63 @@
+/* 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/qpnp.h>
+#include <linux/regmap.h>
+#include <linux/spmi.h>
+
+static const struct regmap_config qpnp_spmi_regmap_config = {
+	.reg_bits = 16,
+	.val_bits = 8,
+	.max_register = 0xffff,
+};
+
+static int qpnp_spmi_probe(struct spmi_device *sdev)
+{
+	struct regmap *regmap;
+
+	regmap = devm_regmap_init_spmi_ext(sdev, &qpnp_spmi_regmap_config);
+	if (IS_ERR(regmap)) {
+		dev_err(&sdev->dev, "regmap creation failed.\n");
+		return PTR_ERR(regmap);
+	}
+
+	return qpnp_populate_devices(sdev->dev.of_node, &sdev->dev,
+				     sdev->usid, regmap);
+}
+
+static void qpnp_spmi_remove(struct spmi_device *sdev)
+{
+	qpnp_remove_devices(&sdev->dev);
+}
+
+static const struct of_device_id qpnp_spmi_id_table[] = {
+	{ .compatible = "qcom,qpnp-spmi" },
+	{ },
+};
+
+MODULE_DEVICE_TABLE(of, qpnp_spmi_id_table);
+
+static struct spmi_driver qpnp_spmi_driver = {
+	.probe = qpnp_spmi_probe,
+	.remove = qpnp_spmi_remove,
+	.driver = {
+		   .name = "qpnp-spmi",
+		   .of_match_table = qpnp_spmi_id_table,
+	},
+};
+
+module_spmi_driver(qpnp_spmi_driver);
+
+MODULE_DESCRIPTION("Qualcomm PMIC QPNP driver");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("spmi:qpnp-spmi");
diff --git a/include/linux/qpnp.h b/include/linux/qpnp.h
new file mode 100644
index 0000000..788f886
--- /dev/null
+++ b/include/linux/qpnp.h
@@ -0,0 +1,83 @@
+/* Copyright (c) 2012-2013, 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.
+ */
+#ifndef _LINUX_QPNP_H
+#define _LINUX_QPNP_H
+
+#include <linux/types.h>
+#include <linux/device.h>
+#include <linux/regmap.h>
+
+/**
+ * struct qpnp_device - Basic representation of an QPNP device
+ * @map: regmap to be used by driver to access device registers
+ * @dev: driver model representation of the device
+ * @sid: this devices' Unique Slave IDentifier
+ * @num_resources: number of memory resources associated to device
+ * @resource: memory resources asocuated to device
+ */
+struct qpnp_device {
+	struct device		dev;
+	struct regmap		*map;
+	u8			sid;
+	u32			num_resources;
+	struct resource		*resource;
+};
+
+
+static inline struct qpnp_device *to_qpnp_device(struct device *d)
+{
+	return container_of(d, struct qpnp_device, dev);
+}
+
+static inline void *qpnp_get_drvdata(const struct qpnp_device *qdev)
+{
+	return dev_get_drvdata(&qdev->dev);
+}
+
+static inline void qpnp_set_drvdata(struct qpnp_device *qdev, void *data)
+{
+	dev_set_drvdata(&qdev->dev, data);
+}
+
+int qpnp_populate_devices(struct device_node *root, struct device *parent,
+			  u8 sid, struct regmap *map);
+void qpnp_remove_devices(struct device *root);
+
+struct resource *qpnp_get_resource(struct qpnp_device *dev, unsigned int type,
+				   unsigned int num);
+struct resource *qpnp_get_resource_byname(struct qpnp_device *dev,
+					  unsigned int type, const char *name);
+
+int qpnp_get_irq(struct qpnp_device *qdev, unsigned int num);
+int qpnp_get_irq_byname(struct qpnp_device *qdev, const char *name);
+
+/**
+ * struct qpnp_driver - QPNP slave device driver
+ * @driver: QPNP device drivers should initialize name and owner field of
+ *          this structure
+ * @probe: binds this driver to a QPNP device
+ * @remove: unbinds this driver from the QPNP device
+ */
+struct qpnp_driver {
+	struct device_driver driver;
+	int (*probe)(struct qpnp_device *qdev);
+	void (*remove)(struct qpnp_device *qdev);
+};
+
+int qpnp_driver_register(struct qpnp_driver *qdrv);
+void qpnp_driver_unregister(struct qpnp_driver *qdrv);
+
+#define module_qpnp_driver(__qpnp_driver) \
+	module_driver(__qpnp_driver, qpnp_driver_register, \
+			qpnp_driver_unregister)
+
+#endif
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [RFC PATCH 3/6] dt: qcom: msm8974: add qpnp-spmi device nodes
  2014-06-20 12:21 [RFC PATCH 0/6] Support for QPNP PMIC's Stanimir Varbanov
  2014-06-20 12:21 ` [RFC PATCH 1/6] dt: qpnp: add binding description of Qualcomm QPNP PMICs Stanimir Varbanov
  2014-06-20 12:21 ` [RFC PATCH 2/6] qpnp: add support for " Stanimir Varbanov
@ 2014-06-20 12:21 ` Stanimir Varbanov
  2014-06-20 12:21 ` [RFC PATCH 4/6] rtc: add qpnp rtc driver Stanimir Varbanov
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 14+ messages in thread
From: Stanimir Varbanov @ 2014-06-20 12:21 UTC (permalink / raw)
  To: linux-arm-msm; +Cc: Ivan T. Ivanov, Stanimir Varbanov

From: Ivan T. Ivanov <iivanov@mm-sol.com>

The qpnp-spmi device node are childrens of spmi pmic arbiter.
msm8974 SoC using two pmic chips pm8941 and pm8841. Every chip
has two spmi-qpnp bus id's.

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

diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi b/arch/arm/boot/dts/qcom-msm8974.dtsi
index 69dca2a..8149397 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,49 @@
 			#interrupt-cells = <2>;
 			interrupts = <0 208 0>;
 		};
+
+		spmi@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>;
+
+			usid_bus0: pm8941@0 {
+				compatible = "qcom,qpnp-spmi";
+				reg = <0x0 SPMI_USID>;
+				#address-cells = <1>;
+				#size-cells = <0>;
+			};
+
+			usid_bus1: pm8941@1 {
+				compatible = "qcom,qpnp-spmi";
+				reg = <0x1 SPMI_USID>;
+				#address-cells = <1>;
+				#size-cells = <0>;
+			};
+
+			usid_bus4: pm8841@4 {
+				compatible = "qcom,qpnp-spmi";
+				reg = <0x4 SPMI_USID>;
+				#address-cells = <1>;
+				#size-cells = <0>;
+			};
+
+			usid_bus5: pm8841@5 {
+				compatible = "qcom,qpnp-spmi";
+				reg = <0x5 SPMI_USID>;
+				#address-cells = <1>;
+				#size-cells = <0>;
+			};
+		};
 	};
 };
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [RFC PATCH 4/6] rtc: add qpnp rtc driver
  2014-06-20 12:21 [RFC PATCH 0/6] Support for QPNP PMIC's Stanimir Varbanov
                   ` (2 preceding siblings ...)
  2014-06-20 12:21 ` [RFC PATCH 3/6] dt: qcom: msm8974: add qpnp-spmi device nodes Stanimir Varbanov
@ 2014-06-20 12:21 ` Stanimir Varbanov
  2014-06-20 12:21 ` [RFC PATCH 5/6] dt: msm8974: add qpnp rtc device node Stanimir Varbanov
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 14+ messages in thread
From: Stanimir Varbanov @ 2014-06-20 12:21 UTC (permalink / raw)
  To: linux-arm-msm; +Cc: Stanimir Varbanov

A 32bits RTC is housed inside PMIC. The RTC driver uses QPNP
SPMI interface to communicate with the PMIC RTC module.

The RTC device is divided into two sub-peripherals:
 - RTC read-write peripheral having basic RTC registers
 - alarm peripheral for controlling alarm

These two RTC peripherals are childrens of QPNP SPMI bus. They
use regmap to read/write to its registers into PMIC.

Signed-off-by: Stanimir Varbanov <svarbanov@mm-sol.com>
---
 drivers/rtc/Kconfig    |    8 +
 drivers/rtc/Makefile   |    1 +
 drivers/rtc/rtc-qpnp.c |  489 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 498 insertions(+), 0 deletions(-)
 create mode 100644 drivers/rtc/rtc-qpnp.c

diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 0754f5c..b0a5f20 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -1365,6 +1365,14 @@ config RTC_DRV_XGENE
 	  This driver can also be built as a module, if so, the module
 	  will be called "rtc-xgene".
 
+config RTC_DRV_QPNP
+	tristate "Qualcomm QPNP PMIC RTC"
+	depends on QPNP_SPMI
+	help
+	  Say Y here if you want to support the Qualcomm QPNP PMIC RTC.
+	  To compile this driver as a module, choose M here: the
+	  module will be called rtc-qpnp.
+
 comment "HID Sensor RTC drivers"
 
 config RTC_DRV_HID_SENSOR_TIME
diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
index 70347d0..52488b5 100644
--- a/drivers/rtc/Makefile
+++ b/drivers/rtc/Makefile
@@ -141,3 +141,4 @@ obj-$(CONFIG_RTC_DRV_X1205)	+= rtc-x1205.o
 obj-$(CONFIG_RTC_DRV_XGENE)	+= rtc-xgene.o
 obj-$(CONFIG_RTC_DRV_SIRFSOC)	+= rtc-sirfsoc.o
 obj-$(CONFIG_RTC_DRV_MOXART)	+= rtc-moxart.o
+obj-$(CONFIG_RTC_DRV_QPNP)	+= rtc-qpnp.o
diff --git a/drivers/rtc/rtc-qpnp.c b/drivers/rtc/rtc-qpnp.c
new file mode 100644
index 0000000..611c695
--- /dev/null
+++ b/drivers/rtc/rtc-qpnp.c
@@ -0,0 +1,489 @@
+/* Copyright (c) 2012-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/module.h>
+#include <linux/rtc.h>
+#include <linux/pm.h>
+#include <linux/spinlock.h>
+#include <linux/mod_devicetable.h>
+#include <linux/qpnp.h>
+
+/* RTC/ALARM register offsets */
+#define REG_ALARM_RW			0x40
+#define REG_ALARM_CTRL1			0x46
+#define REG_ALARM_CTRL2			0x48
+#define REG_RTC_WRITE			0x40
+#define REG_RTC_CTRL			0x46
+#define REG_RTC_READ			0x48
+#define REG_PERP_SUBTYPE		0x05
+
+/* RTC_CTRL register bit fields */
+#define RTC_ENABLE			BIT(7)
+#define RTC_ALARM_ENABLE		BIT(7)
+#define RTC_ABORT_ENABLE		BIT(0)
+#define RTC_ALARM_CLEAR			BIT(0)
+
+/* RTC/ALARM peripheral subtype values */
+#define PERPH_SUBTYPE_RTC		0x1
+#define PERPH_SUBTYPE_ALARM		0x3
+#define NUM_8_BIT_RTC_REGS		0x4
+
+#define TO_SECS(arr)			\
+		(arr[0] | (arr[1] << 8) | (arr[2] << 16) | (arr[3] << 24))
+
+/* rtc driver internal structure */
+struct qpnp_rtc {
+	struct regmap *regmap;
+	struct device *dev;
+	struct rtc_device *rtc;
+	spinlock_t lock;	/* to protect RTC control register */
+	struct rtc_class_ops ops;
+	u8 rtc_ctrl_reg;
+	u8 alarm_ctrl_reg1;
+	u16 rtc_base;
+	u16 alarm_base;
+	int alarm_irq;
+};
+
+static int qpnp_rtc_read(struct qpnp_rtc *rtc, u8 *val, u16 off, int len)
+{
+	return regmap_bulk_read(rtc->regmap, rtc->rtc_base + off, val, len);
+}
+
+static int qpnp_rtc_write(struct qpnp_rtc *rtc, u8 *val, u16 off, int len)
+{
+	return regmap_bulk_write(rtc->regmap, rtc->rtc_base + off, val, len);
+}
+
+static int qpnp_alarm_read(struct qpnp_rtc *rtc, u8 *val, u16 off, int len)
+{
+	return regmap_bulk_read(rtc->regmap, rtc->alarm_base + off, val, len);
+}
+
+static int qpnp_alarm_write(struct qpnp_rtc *rtc, u8 *val, u16 off, int len)
+{
+	return regmap_bulk_write(rtc->regmap, rtc->alarm_base + off, val, len);
+}
+
+static int qpnp_rtc_set_time(struct device *dev, struct rtc_time *tm)
+{
+	struct qpnp_rtc *rtc = dev_get_drvdata(dev);
+	u8 value[4], reg = 0, alarm_enabled = 0, ctrl_reg;
+	u8 rtc_disabled = 0, rtc_ctrl_reg;
+	unsigned long secs, flags;
+	int ret;
+
+	rtc_tm_to_time(tm, &secs);
+
+	value[0] = (secs >>  0) & 0xff;
+	value[1] = (secs >>  8) & 0xff;
+	value[2] = (secs >> 16) & 0xff;
+	value[3] = (secs >> 24) & 0xff;
+
+	dev_dbg(dev, "Seconds value to be written to RTC = %lu\n", secs);
+
+	spin_lock_irqsave(&rtc->lock, flags);
+	ctrl_reg = rtc->alarm_ctrl_reg1;
+
+	if (ctrl_reg & RTC_ALARM_ENABLE) {
+		alarm_enabled = 1;
+		ctrl_reg &= ~RTC_ALARM_ENABLE;
+		ret = qpnp_alarm_write(rtc, &ctrl_reg, REG_ALARM_CTRL1, 1);
+		if (ret)
+			goto rtc_rw_fail;
+	} else {
+		spin_unlock_irqrestore(&rtc->lock, flags);
+	}
+
+	/*
+	 * 32 bit seconds value is coverted to four 8 bit values
+	 *	|<------  32 bit time value in seconds  ------>|
+	 *      <- 8 bit ->|<- 8 bit ->|<- 8 bit ->|<- 8 bit ->|
+	 *       ----------------------------------------------
+	 *      | BYTE[3]  |  BYTE[2]  |  BYTE[1]  |  BYTE[0]  |
+	 *       ----------------------------------------------
+	 *
+	 * RTC has four 8 bit registers for writting time in seconds:
+	 *             WDATA[3], WDATA[2], WDATA[1], WDATA[0]
+	 *
+	 * Write to the RTC registers should be done in following order
+	 * Clear WDATA[0] register
+	 *
+	 * Write BYTE[1], BYTE[2] and BYTE[3] of time to
+	 * RTC WDATA[3], WDATA[2], WDATA[1] registers
+	 *
+	 * Write BYTE[0] of time to RTC WDATA[0] register
+	 *
+	 * Clearing BYTE[0] and writting in the end will prevent any
+	 * unintentional overflow from WDATA[0] to higher bytes during the
+	 * write operation
+	 */
+
+	/* Disable RTC H/w before writing on RTC register*/
+	rtc_ctrl_reg = rtc->rtc_ctrl_reg;
+	if (rtc_ctrl_reg & RTC_ENABLE) {
+		rtc_disabled = 1;
+		rtc_ctrl_reg &= ~RTC_ENABLE;
+		ret = qpnp_rtc_write(rtc, &rtc_ctrl_reg, REG_RTC_CTRL, 1);
+		if (ret)
+			goto rtc_rw_fail;
+		rtc->rtc_ctrl_reg = rtc_ctrl_reg;
+	}
+
+	/* Clear WDATA[0] */
+	reg = 0x0;
+	ret = qpnp_rtc_write(rtc, &reg, REG_RTC_WRITE, 1);
+	if (ret)
+		goto rtc_rw_fail;
+
+	/* Write to WDATA[3], WDATA[2] and WDATA[1] */
+	ret = qpnp_rtc_write(rtc, &value[1], REG_RTC_WRITE + 1, 3);
+	if (ret)
+		goto rtc_rw_fail;
+
+	/* Write to WDATA[0] */
+	ret = qpnp_rtc_write(rtc, value, REG_RTC_WRITE, 1);
+	if (ret)
+		goto rtc_rw_fail;
+
+	/* Enable RTC after writing on RTC register */
+	if (rtc_disabled) {
+		rtc_ctrl_reg |= RTC_ENABLE;
+		ret = qpnp_rtc_write(rtc, &rtc_ctrl_reg, REG_RTC_CTRL, 1);
+		if (ret)
+			goto rtc_rw_fail;
+		rtc->rtc_ctrl_reg = rtc_ctrl_reg;
+	}
+
+	if (alarm_enabled) {
+		ctrl_reg |= RTC_ALARM_ENABLE;
+		ret = qpnp_alarm_write(rtc, &ctrl_reg, REG_ALARM_CTRL1, 1);
+		if (ret)
+			goto rtc_rw_fail;
+	}
+
+	rtc->alarm_ctrl_reg1 = ctrl_reg;
+
+rtc_rw_fail:
+	if (alarm_enabled)
+		spin_unlock_irqrestore(&rtc->lock, flags);
+
+	return ret;
+}
+
+static int qpnp_rtc_read_time(struct device *dev, struct rtc_time *tm)
+{
+	struct qpnp_rtc *rtc = dev_get_drvdata(dev);
+	u8 value[4], reg;
+	unsigned long secs;
+	int ret;
+
+	ret = qpnp_rtc_read(rtc, value, REG_RTC_READ, NUM_8_BIT_RTC_REGS);
+	if (ret)
+		return ret;
+
+	/*
+	 * Read the LSB again and check if there has been a carry over
+	 * If there is, redo the read operation
+	 */
+	ret = qpnp_rtc_read(rtc, &reg, REG_RTC_READ, 1);
+	if (ret)
+		return ret;
+
+	if (reg < value[0]) {
+		ret = qpnp_rtc_read(rtc, value, REG_RTC_READ,
+				    NUM_8_BIT_RTC_REGS);
+		if (ret)
+			return ret;
+	}
+
+	secs = TO_SECS(value);
+	rtc_time_to_tm(secs, tm);
+
+	ret = rtc_valid_tm(tm);
+	if (ret)
+		return ret;
+
+	dev_dbg(dev, "secs = %lu, h:m:s == %d:%d:%d, d/m/y = %d/%d/%d\n",
+		secs, tm->tm_hour, tm->tm_min, tm->tm_sec,
+		tm->tm_mday, tm->tm_mon, tm->tm_year);
+
+	return 0;
+}
+
+static int qpnp_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alarm)
+{
+	struct qpnp_rtc *rtc = dev_get_drvdata(dev);
+	unsigned long secs, secs_rtc, irq_flags;
+	struct rtc_time rtc_tm;
+	u8 value[4], ctrl_reg;
+	int ret;
+
+	rtc_tm_to_time(&alarm->time, &secs);
+
+	/*
+	 * Read the current RTC time and verify if the alarm time is in the
+	 * past. If yes, return invalid
+	 */
+	ret = qpnp_rtc_read_time(dev, &rtc_tm);
+	if (ret)
+		return -EINVAL;
+
+	rtc_tm_to_time(&rtc_tm, &secs_rtc);
+	if (secs < secs_rtc)
+		return -EINVAL;
+
+	value[0] = (secs >>  0) & 0xff;
+	value[1] = (secs >>  8) & 0xff;
+	value[2] = (secs >> 16) & 0xff;
+	value[3] = (secs >> 24) & 0xff;
+
+	spin_lock_irqsave(&rtc->lock, irq_flags);
+
+	ret = qpnp_alarm_write(rtc, value, REG_ALARM_RW, NUM_8_BIT_RTC_REGS);
+	if (ret)
+		goto rtc_rw_fail;
+
+	ctrl_reg = alarm->enabled ?
+			(rtc->alarm_ctrl_reg1 |  RTC_ALARM_ENABLE) :
+			(rtc->alarm_ctrl_reg1 & ~RTC_ALARM_ENABLE);
+
+	ret = qpnp_alarm_write(rtc, &ctrl_reg, REG_ALARM_CTRL1, 1);
+	if (ret)
+		goto rtc_rw_fail;
+
+	rtc->alarm_ctrl_reg1 = ctrl_reg;
+
+	dev_dbg(dev, "Alarm Set for h:r:s=%d:%d:%d, d/m/y=%d/%d/%d\n",
+			alarm->time.tm_hour, alarm->time.tm_min,
+			alarm->time.tm_sec, alarm->time.tm_mday,
+			alarm->time.tm_mon, alarm->time.tm_year);
+
+rtc_rw_fail:
+	spin_unlock_irqrestore(&rtc->lock, irq_flags);
+	return ret;
+}
+
+static int qpnp_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alarm)
+{
+	struct qpnp_rtc *rtc = dev_get_drvdata(dev);
+	unsigned long secs;
+	u8 value[4];
+	int ret;
+
+	ret = qpnp_alarm_read(rtc, value, REG_ALARM_RW, NUM_8_BIT_RTC_REGS);
+	if (ret)
+		return ret;
+
+	secs = TO_SECS(value);
+	rtc_time_to_tm(secs, &alarm->time);
+
+	ret = rtc_valid_tm(&alarm->time);
+	if (ret)
+		return ret;
+
+	dev_dbg(dev, "Alarm set for - h:r:s=%d:%d:%d, d/m/y=%d/%d/%d\n",
+		alarm->time.tm_hour, alarm->time.tm_min,
+				alarm->time.tm_sec, alarm->time.tm_mday,
+				alarm->time.tm_mon, alarm->time.tm_year);
+
+	return 0;
+}
+
+static int qpnp_rtc_alarm_irq_enable(struct device *dev, unsigned int enabled)
+{
+	struct qpnp_rtc *rtc = dev_get_drvdata(dev);
+	unsigned long flags;
+	u8 ctrl_reg;
+	u8 value[4] = {0};
+	int ret;
+
+	spin_lock_irqsave(&rtc->lock, flags);
+
+	ctrl_reg = rtc->alarm_ctrl_reg1;
+	ctrl_reg = enabled ?
+		(ctrl_reg | RTC_ALARM_ENABLE) : (ctrl_reg & ~RTC_ALARM_ENABLE);
+
+	ret = qpnp_alarm_write(rtc, &ctrl_reg, REG_ALARM_CTRL1, 1);
+	if (ret)
+		goto rtc_rw_fail;
+
+	rtc->alarm_ctrl_reg1 = ctrl_reg;
+
+	/* Clear Alarm register */
+	if (!enabled)
+		ret = qpnp_alarm_write(rtc, value, REG_ALARM_RW,
+				       NUM_8_BIT_RTC_REGS);
+
+rtc_rw_fail:
+	spin_unlock_irqrestore(&rtc->lock, flags);
+
+	return ret;
+}
+
+static irqreturn_t qpnp_alarm_trigger(int irq, void *dev_id)
+{
+	struct qpnp_rtc *rtc = dev_id;
+	unsigned long flags;
+	u8 ctrl_reg;
+	int ret;
+
+	rtc_update_irq(rtc->rtc, 1, RTC_IRQF | RTC_AF);
+
+	spin_lock_irqsave(&rtc->lock, flags);
+
+	/* Clear the alarm enable bit */
+	ctrl_reg = rtc->alarm_ctrl_reg1;
+	ctrl_reg &= ~RTC_ALARM_ENABLE;
+
+	ret = qpnp_alarm_write(rtc, &ctrl_reg, REG_ALARM_CTRL1, 1);
+	if (ret) {
+		spin_unlock_irqrestore(&rtc->lock, flags);
+		return IRQ_HANDLED;
+	}
+
+	rtc->alarm_ctrl_reg1 = ctrl_reg;
+	spin_unlock_irqrestore(&rtc->lock, flags);
+
+	/* Set ALARM_CLR bit */
+	ctrl_reg = 0x1;
+	ret = qpnp_alarm_write(rtc, &ctrl_reg, REG_ALARM_CTRL2, 1);
+	if (ret)
+		dev_dbg(rtc->dev, "Write to ALARM control reg failed\n");
+
+	return IRQ_HANDLED;
+}
+
+static int qpnp_rtc_probe(struct qpnp_device *qdev)
+{
+	struct device *dev = &qdev->dev;
+	struct qpnp_rtc *rtc;
+	struct resource *res;
+	u8 subtype;
+	int ret;
+
+	rtc = devm_kzalloc(dev, sizeof(*rtc), GFP_KERNEL);
+	if (!rtc)
+		return -ENOMEM;
+
+	/* Initialise spinlock to protect RTC control register */
+	spin_lock_init(&rtc->lock);
+
+	rtc->regmap = dev_get_regmap(dev->parent, NULL);
+	if (!rtc->regmap)
+		return -ENODEV;
+
+	rtc->dev = dev;
+
+	res = qpnp_get_resource(qdev, IORESOURCE_REG, 0);
+	if (!res)
+		return -ENODEV;
+
+	rtc->rtc_base = res->start;
+
+	res = qpnp_get_resource(qdev, IORESOURCE_REG, 1);
+	if (!res)
+		return -ENODEV;
+
+	rtc->alarm_base = res->start;
+
+	ret = qpnp_rtc_read(rtc, &subtype, REG_PERP_SUBTYPE, 1);
+	if (ret)
+		return ret;
+
+	if (subtype != PERPH_SUBTYPE_RTC)
+		return -ENODEV;
+
+	ret = qpnp_alarm_read(rtc, &subtype, REG_PERP_SUBTYPE, 1);
+	if (ret)
+		return ret;
+
+	if (subtype != PERPH_SUBTYPE_ALARM)
+		return -ENODEV;
+
+	rtc->alarm_irq = qpnp_get_irq(qdev, 0);
+	if (rtc->alarm_irq < 0)
+		return rtc->alarm_irq;
+
+	ret = qpnp_rtc_read(rtc, &rtc->rtc_ctrl_reg, REG_RTC_CTRL, 1);
+	if (ret)
+		return ret;
+
+	if (!(rtc->rtc_ctrl_reg & RTC_ENABLE)) {
+		dev_dbg(dev, "RTC h/w disabled, rtc not registered\n");
+		return -ENODEV;
+	}
+
+	ret = qpnp_alarm_read(rtc, &rtc->alarm_ctrl_reg1, REG_ALARM_CTRL1, 1);
+	if (ret)
+		return ret;
+
+	rtc->alarm_ctrl_reg1 |= RTC_ABORT_ENABLE;
+	ret = qpnp_alarm_write(rtc, &rtc->alarm_ctrl_reg1, REG_ALARM_CTRL1, 1);
+	if (ret)
+		return ret;
+
+	ret = devm_request_any_context_irq(dev, rtc->alarm_irq,
+					   qpnp_alarm_trigger,
+					   IRQF_TRIGGER_RISING,
+					   "rtc_qpnp_alarm", rtc);
+	if (ret < 0)
+		return ret;
+
+	device_init_wakeup(dev, 1);
+	enable_irq_wake(rtc->alarm_irq);
+
+	qpnp_set_drvdata(qdev, rtc);
+
+	rtc->ops.read_time = qpnp_rtc_read_time;
+	rtc->ops.set_alarm = qpnp_rtc_set_alarm;
+	rtc->ops.read_alarm = qpnp_rtc_read_alarm;
+	rtc->ops.alarm_irq_enable = qpnp_rtc_alarm_irq_enable;
+	rtc->ops.set_time = qpnp_rtc_set_time;
+
+	rtc->rtc = rtc_device_register("rtc-qpnp", dev, &rtc->ops, THIS_MODULE);
+	if (IS_ERR(rtc->rtc))
+		return PTR_ERR(rtc->rtc);
+
+	return 0;
+}
+
+static void qpnp_rtc_remove(struct qpnp_device *qdev)
+{
+	struct qpnp_rtc *rtc = qpnp_get_drvdata(qdev);
+
+	device_init_wakeup(rtc->dev, 0);
+	free_irq(rtc->alarm_irq, rtc);
+	rtc_device_unregister(rtc->rtc);
+}
+
+static const struct of_device_id rtc_qpnp_table[] = {
+	{ .compatible = "qcom,rtc-qpnp", },
+	{}
+};
+MODULE_DEVICE_TABLE(of, rtc_qpnp_table);
+
+static struct qpnp_driver qpnp_rtc_driver = {
+	.probe = qpnp_rtc_probe,
+	.remove = qpnp_rtc_remove,
+	.driver = {
+		.name = "qcom,rtc-qpnp",
+		.owner = THIS_MODULE,
+		.of_match_table = rtc_qpnp_table,
+	},
+};
+module_qpnp_driver(qpnp_rtc_driver);
+
+MODULE_ALIAS("platform:" KBUILD_MODNAME);
+MODULE_DESCRIPTION("SMPI PMIC RTC driver");
+MODULE_AUTHOR("The Linux Foundation");
+MODULE_LICENSE("GPL v2");
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [RFC PATCH 5/6] dt: msm8974: add qpnp rtc device node
  2014-06-20 12:21 [RFC PATCH 0/6] Support for QPNP PMIC's Stanimir Varbanov
                   ` (3 preceding siblings ...)
  2014-06-20 12:21 ` [RFC PATCH 4/6] rtc: add qpnp rtc driver Stanimir Varbanov
@ 2014-06-20 12:21 ` Stanimir Varbanov
  2014-06-20 12:21 ` [RFC PATCH 6/6] dt: rtc: add binding document for rtc-qpnp Stanimir Varbanov
  2014-06-24 22:36 ` [RFC PATCH 0/6] Support for QPNP PMIC's Courtney Cavin
  6 siblings, 0 replies; 14+ messages in thread
From: Stanimir Varbanov @ 2014-06-20 12:21 UTC (permalink / raw)
  To: linux-arm-msm; +Cc: Stanimir Varbanov

Add QPNP RTC device tree node. The RTC device resides into pm8941
and is attached on busid0.

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

diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi b/arch/arm/boot/dts/qcom-msm8974.dtsi
index 8149397..a7cd1f4 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/interrupt-controller/irq.h>
 #include <dt-bindings/spmi/spmi.h>
 
 / {
@@ -258,6 +259,12 @@
 				reg = <0x0 SPMI_USID>;
 				#address-cells = <1>;
 				#size-cells = <0>;
+
+				pm8941_rtc {
+					compatible = "qcom,rtc-qpnp";
+					reg = <0x60>, <0x61>;
+					interrupts = <0x0 0x61 0x1 IRQ_TYPE_EDGE_RISING>;
+				};
 			};
 
 			usid_bus1: pm8941@1 {
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [RFC PATCH 6/6] dt: rtc: add binding document for rtc-qpnp
  2014-06-20 12:21 [RFC PATCH 0/6] Support for QPNP PMIC's Stanimir Varbanov
                   ` (4 preceding siblings ...)
  2014-06-20 12:21 ` [RFC PATCH 5/6] dt: msm8974: add qpnp rtc device node Stanimir Varbanov
@ 2014-06-20 12:21 ` Stanimir Varbanov
  2014-06-24 22:36 ` [RFC PATCH 0/6] Support for QPNP PMIC's Courtney Cavin
  6 siblings, 0 replies; 14+ messages in thread
From: Stanimir Varbanov @ 2014-06-20 12:21 UTC (permalink / raw)
  To: linux-arm-msm; +Cc: Stanimir Varbanov

Add devicetree binding document which describes the rtc-qpnp.

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

diff --git a/Documentation/devicetree/bindings/rtc/qcom,rtc-qpnp.txt b/Documentation/devicetree/bindings/rtc/qcom,rtc-qpnp.txt
new file mode 100644
index 0000000..609603f
--- /dev/null
+++ b/Documentation/devicetree/bindings/rtc/qcom,rtc-qpnp.txt
@@ -0,0 +1,21 @@
+Qualcomm RTC QPNP
+
+The RTC device supports 32bit RTC module housed inside QPNP PMIC's.
+The driver utilizes SPMI interface to communicate with the RTC module.
+RTC device is divided into two sub-peripherals one which controls
+basic RTC and other for controlling alarm.
+
+Required properties :
+ - compatible:  Should be "qcom,rtc-qpnp".
+ - reg:         Specify the peripheral id for device. Currently there
+                are two sub-peripherals - rtc base and alarm base.
+ - interrupts:  Specifies alarm interrupt, only for alarm sub-peripheral.
+                For more info about interrupt cells see
+                Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.txt
+
+Example:
+	rtc@60 {
+		compatible = "qcom,rtc-qpnp";
+		reg = <0x60>, <0x61>;
+		interrupts = <0x0 0x61 0x1 IRQ_TYPE_EDGE_RISING>;
+	};
\ No newline at end of file
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH 0/6] Support for QPNP PMIC's
  2014-06-20 12:21 [RFC PATCH 0/6] Support for QPNP PMIC's Stanimir Varbanov
                   ` (5 preceding siblings ...)
  2014-06-20 12:21 ` [RFC PATCH 6/6] dt: rtc: add binding document for rtc-qpnp Stanimir Varbanov
@ 2014-06-24 22:36 ` Courtney Cavin
  2014-06-25 11:07   ` Stanimir Varbanov
  6 siblings, 1 reply; 14+ messages in thread
From: Courtney Cavin @ 2014-06-24 22:36 UTC (permalink / raw)
  To: Stanimir Varbanov
  Cc: linux-arm-msm@vger.kernel.org, Ivan T. Ivanov, Josh Cartwright,
	Stephen Boyd

On Fri, Jun 20, 2014 at 02:21:19PM +0200, Stanimir Varbanov wrote:
> Hello to all,
> 
> The goal of this WIP set is to provide support for sub-functional
> hw blocks inside Qualcomm QPNP PMIC chips by creating a new qpnp
> bus (device-driver modeled). On SPMI interface we attach a so called
> Slaves. For example one PMIC chip like pm8941 has two slave id's. On
> every slaveid the PMIC chip has various peripherials. Every peripheral
> driver should expose qpnp_driver structure and implement the .probe
> and .remove callbacks. Every peripheral driver will reside on the
> proper place in /drivers directory. 
> 
> The fisrt patch describes the devicetree binding of the slave devices
> attached to the SPMI interface. The second patch adds implemetation of
> the qpnp-bus and a layer connection with the SPMI interface. The third
> patch adds spmi arbiter and underlying slaveid devicetree nodes for
> msm8974 SoC. The other patches are example of rtc-qpnp driver and
> binding documentation.
> 
> Since this set is a WIP, it will be used as a starting point for future
> duscussions about implementing QPNP PMIC sub-function hw blocks.

Ok.  I have a few critical problems with this approach to implementing
support for the PMICs:
  - It's *way* more code than needed if done with of_platform_populate()
  - AFAICT, the only direct relation between this and 'QPNP' is the 256
    byte block size, which I would argue can be clearly expressed in DT
    instead with #size-cells == 1, and #address-cells == 1
  - The resource code from drivers/{base,of}/platform.c is reimplemented
    here, without any added benefit

What am I missing?

-Courtney

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH 0/6] Support for QPNP PMIC's
  2014-06-24 22:36 ` [RFC PATCH 0/6] Support for QPNP PMIC's Courtney Cavin
@ 2014-06-25 11:07   ` Stanimir Varbanov
  2014-06-25 18:04     ` Courtney Cavin
  0 siblings, 1 reply; 14+ messages in thread
From: Stanimir Varbanov @ 2014-06-25 11:07 UTC (permalink / raw)
  To: Courtney Cavin
  Cc: linux-arm-msm@vger.kernel.org, Ivan T. Ivanov, Josh Cartwright,
	Stephen Boyd

Hi Courtney,

Thanks for the comments.

On 06/25/2014 01:36 AM, Courtney Cavin wrote:
> On Fri, Jun 20, 2014 at 02:21:19PM +0200, Stanimir Varbanov wrote:
>> Hello to all,
>>
>> The goal of this WIP set is to provide support for sub-functional
>> hw blocks inside Qualcomm QPNP PMIC chips by creating a new qpnp
>> bus (device-driver modeled). On SPMI interface we attach a so called
>> Slaves. For example one PMIC chip like pm8941 has two slave id's. On
>> every slaveid the PMIC chip has various peripherials. Every peripheral
>> driver should expose qpnp_driver structure and implement the .probe
>> and .remove callbacks. Every peripheral driver will reside on the
>> proper place in /drivers directory. 
>>
>> The fisrt patch describes the devicetree binding of the slave devices
>> attached to the SPMI interface. The second patch adds implemetation of
>> the qpnp-bus and a layer connection with the SPMI interface. The third
>> patch adds spmi arbiter and underlying slaveid devicetree nodes for
>> msm8974 SoC. The other patches are example of rtc-qpnp driver and
>> binding documentation.
>>
>> Since this set is a WIP, it will be used as a starting point for future
>> duscussions about implementing QPNP PMIC sub-function hw blocks.
> 
> Ok.  I have a few critical problems with this approach to implementing
> support for the PMICs:
>   - It's *way* more code than needed if done with of_platform_populate()

Agreed, but this will be the price we must pay to keep architectural
correctness. The PMIC peripherals are not directly accessible by the CPU
bus and thus they shouldn't be treated as platform devices. That fact
gives as an opportunity to create a qpnp bus on which we will attach the
whole bunch of PMIC peripherals and using DT we can describe register
regions (256B each) belonging to it.

>   - AFAICT, the only direct relation between this and 'QPNP' is the 256
>     byte block size, which I would argue can be clearly expressed in DT
>     instead with #size-cells == 1, and #address-cells == 1

The QPNP schema is a private case of Qualcomm PMIC's, so generalising
will over-engineering IMO.

>   - The resource code from drivers/{base,of}/platform.c is reimplemented
>     here, without any added benefit

The resource code could be simplified to satisfy our needs.

-- 
regards,
Stan

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH 0/6] Support for QPNP PMIC's
  2014-06-25 11:07   ` Stanimir Varbanov
@ 2014-06-25 18:04     ` Courtney Cavin
  2014-06-25 21:18       ` Rob Herring
  2014-06-26 15:48       ` Stanimir Varbanov
  0 siblings, 2 replies; 14+ messages in thread
From: Courtney Cavin @ 2014-06-25 18:04 UTC (permalink / raw)
  To: Stanimir Varbanov, Grant Likely, Rob Herring, Greg Kroah-Hartman
  Cc: linux-arm-msm@vger.kernel.org, Ivan T. Ivanov, Josh Cartwright,
	Stephen Boyd

On Wed, Jun 25, 2014 at 01:07:13PM +0200, Stanimir Varbanov wrote:
> Hi Courtney,
> 
> Thanks for the comments.
> 
> On 06/25/2014 01:36 AM, Courtney Cavin wrote:
> > On Fri, Jun 20, 2014 at 02:21:19PM +0200, Stanimir Varbanov wrote:
> >> Hello to all,
> >>
> >> The goal of this WIP set is to provide support for sub-functional
> >> hw blocks inside Qualcomm QPNP PMIC chips by creating a new qpnp
> >> bus (device-driver modeled). On SPMI interface we attach a so called
> >> Slaves. For example one PMIC chip like pm8941 has two slave id's. On
> >> every slaveid the PMIC chip has various peripherials. Every peripheral
> >> driver should expose qpnp_driver structure and implement the .probe
> >> and .remove callbacks. Every peripheral driver will reside on the
> >> proper place in /drivers directory. 
> >>
> >> The fisrt patch describes the devicetree binding of the slave devices
> >> attached to the SPMI interface. The second patch adds implemetation of
> >> the qpnp-bus and a layer connection with the SPMI interface. The third
> >> patch adds spmi arbiter and underlying slaveid devicetree nodes for
> >> msm8974 SoC. The other patches are example of rtc-qpnp driver and
> >> binding documentation.
> >>
> >> Since this set is a WIP, it will be used as a starting point for future
> >> duscussions about implementing QPNP PMIC sub-function hw blocks.
> > 
> > Ok.  I have a few critical problems with this approach to implementing
> > support for the PMICs:
> >   - It's *way* more code than needed if done with of_platform_populate()
> 
> Agreed, but this will be the price we must pay to keep architectural
> correctness. The PMIC peripherals are not directly accessible by the CPU
> bus and thus they shouldn't be treated as platform devices. That fact
> gives as an opportunity to create a qpnp bus on which we will attach the
> whole bunch of PMIC peripherals and using DT we can describe register
> regions (256B each) belonging to it.
> 

So the price we must pay for *your* vision of architectural correctness
is to duplicate code without any actual benefit?

According to Documentation/driver-model/platform.txt your view is
correct:

  Platform devices are devices that typically appear as autonomous
  entities in the system. This includes legacy port-based devices and
  host bridges to peripheral buses, and most controllers integrated
  into system-on-chip platforms.  What they usually have in common
  is direct addressing from a CPU bus.  Rarely, a platform_device will
  be connected through a segment of some other kind of bus; but its
  registers will still be directly addressable.

However, it is clear that with recent changes to the platform code, this
view is getting distorted somewhere.  I haven't specifically heard any
decisions on what to do here though.

Greg, Grant, Rob?  What's the law?

> >   - AFAICT, the only direct relation between this and 'QPNP' is the 256
> >     byte block size, which I would argue can be clearly expressed in DT
> >     instead with #size-cells == 1, and #address-cells == 1
> 
> The QPNP schema is a private case of Qualcomm PMIC's, so generalising
> will over-engineering IMO.
> 

I made a similar attempt to write something like this, but did it
entirely based on regmap "partitions", abstracting out the need for
knowing anything in the client drivers but regmap....  It was about the
same amount of code/complexity as this, but I tossed it out for the same
reasons I mentioned here.  Over/under-engineering has little to do with
it.

Using 'QPNP' as an excuse to have a bus in the kernel that Qualcomm can
define may not be your goal, but it does appear that way.  From what I
know of QPNP (which is only what Josh has described on the list) the
entirety of what matters regarding the QPNP spec is already implemented
in drivers/spmi/spmi-pmic-arb.c

> >   - The resource code from drivers/{base,of}/platform.c is reimplemented
> >     here, without any added benefit
> 
> The resource code could be simplified to satisfy our needs.

Because we would then have our own playground on which to play?

On a related note, it would probably be a good idea to move much of the
platform resource stuff out of the platform code... so we don't
re-implement it over-and-over again.

-Courtney

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH 0/6] Support for QPNP PMIC's
  2014-06-25 18:04     ` Courtney Cavin
@ 2014-06-25 21:18       ` Rob Herring
  2014-06-25 21:28         ` Courtney Cavin
  2014-06-26 15:48       ` Stanimir Varbanov
  1 sibling, 1 reply; 14+ messages in thread
From: Rob Herring @ 2014-06-25 21:18 UTC (permalink / raw)
  To: Courtney Cavin
  Cc: Stanimir Varbanov, Grant Likely, Greg Kroah-Hartman,
	linux-arm-msm@vger.kernel.org, Ivan T. Ivanov, Josh Cartwright,
	Stephen Boyd

On Wed, Jun 25, 2014 at 1:04 PM, Courtney Cavin
<courtney.cavin@sonymobile.com> wrote:
> On Wed, Jun 25, 2014 at 01:07:13PM +0200, Stanimir Varbanov wrote:
>> Hi Courtney,
>>
>> Thanks for the comments.
>>
>> On 06/25/2014 01:36 AM, Courtney Cavin wrote:
>> > On Fri, Jun 20, 2014 at 02:21:19PM +0200, Stanimir Varbanov wrote:
>> >> Hello to all,
>> >>
>> >> The goal of this WIP set is to provide support for sub-functional
>> >> hw blocks inside Qualcomm QPNP PMIC chips by creating a new qpnp
>> >> bus (device-driver modeled). On SPMI interface we attach a so called
>> >> Slaves. For example one PMIC chip like pm8941 has two slave id's. On
>> >> every slaveid the PMIC chip has various peripherials. Every peripheral
>> >> driver should expose qpnp_driver structure and implement the .probe
>> >> and .remove callbacks. Every peripheral driver will reside on the
>> >> proper place in /drivers directory.
>> >>
>> >> The fisrt patch describes the devicetree binding of the slave devices
>> >> attached to the SPMI interface. The second patch adds implemetation of
>> >> the qpnp-bus and a layer connection with the SPMI interface. The third
>> >> patch adds spmi arbiter and underlying slaveid devicetree nodes for
>> >> msm8974 SoC. The other patches are example of rtc-qpnp driver and
>> >> binding documentation.
>> >>
>> >> Since this set is a WIP, it will be used as a starting point for future
>> >> duscussions about implementing QPNP PMIC sub-function hw blocks.
>> >
>> > Ok.  I have a few critical problems with this approach to implementing
>> > support for the PMICs:
>> >   - It's *way* more code than needed if done with of_platform_populate()
>>
>> Agreed, but this will be the price we must pay to keep architectural
>> correctness. The PMIC peripherals are not directly accessible by the CPU
>> bus and thus they shouldn't be treated as platform devices. That fact
>> gives as an opportunity to create a qpnp bus on which we will attach the
>> whole bunch of PMIC peripherals and using DT we can describe register
>> regions (256B each) belonging to it.
>>
>
> So the price we must pay for *your* vision of architectural correctness
> is to duplicate code without any actual benefit?
>
> According to Documentation/driver-model/platform.txt your view is
> correct:
>
>   Platform devices are devices that typically appear as autonomous
>   entities in the system. This includes legacy port-based devices and
>   host bridges to peripheral buses, and most controllers integrated
>   into system-on-chip platforms.  What they usually have in common
>   is direct addressing from a CPU bus.  Rarely, a platform_device will
>   be connected through a segment of some other kind of bus; but its
>   registers will still be directly addressable.
>
> However, it is clear that with recent changes to the platform code, this
> view is getting distorted somewhere.  I haven't specifically heard any
> decisions on what to do here though.
>
> Greg, Grant, Rob?  What's the law?

Generally sub-blocks of a device are handled as platform devices. If
there is a good enough reason then creating a new device type may be
okay, but we certainly wouldn't want every PMIC or MFD driver to go
off and define their own bus. Probably not each vendor doing a bus
either.

>> >   - AFAICT, the only direct relation between this and 'QPNP' is the 256
>> >     byte block size, which I would argue can be clearly expressed in DT
>> >     instead with #size-cells == 1, and #address-cells == 1

I agree that I don't see anything unique here.

>> The QPNP schema is a private case of Qualcomm PMIC's, so generalising
>> will over-engineering IMO.

But it is already generalized for you with platform devices.

> I made a similar attempt to write something like this, but did it
> entirely based on regmap "partitions", abstracting out the need for
> knowing anything in the client drivers but regmap....  It was about the
> same amount of code/complexity as this, but I tossed it out for the same
> reasons I mentioned here.  Over/under-engineering has little to do with
> it.
>
> Using 'QPNP' as an excuse to have a bus in the kernel that Qualcomm can
> define may not be your goal, but it does appear that way.  From what I
> know of QPNP (which is only what Josh has described on the list) the
> entirety of what matters regarding the QPNP spec is already implemented
> in drivers/spmi/spmi-pmic-arb.c
>
>> >   - The resource code from drivers/{base,of}/platform.c is reimplemented
>> >     here, without any added benefit
>>
>> The resource code could be simplified to satisfy our needs.
>
> Because we would then have our own playground on which to play?
>
> On a related note, it would probably be a good idea to move much of the
> platform resource stuff out of the platform code... so we don't
> re-implement it over-and-over again.

Which part is implemented over-and-over?

Rob

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH 0/6] Support for QPNP PMIC's
  2014-06-25 21:18       ` Rob Herring
@ 2014-06-25 21:28         ` Courtney Cavin
  2014-06-26 15:12           ` Stanimir Varbanov
  0 siblings, 1 reply; 14+ messages in thread
From: Courtney Cavin @ 2014-06-25 21:28 UTC (permalink / raw)
  To: Rob Herring
  Cc: Stanimir Varbanov, Grant Likely, Greg Kroah-Hartman,
	linux-arm-msm@vger.kernel.org, Ivan T. Ivanov, Josh Cartwright,
	Stephen Boyd

On Wed, Jun 25, 2014 at 11:18:57PM +0200, Rob Herring wrote:
> On Wed, Jun 25, 2014 at 1:04 PM, Courtney Cavin
> <courtney.cavin@sonymobile.com> wrote:
> > On Wed, Jun 25, 2014 at 01:07:13PM +0200, Stanimir Varbanov wrote:
> >> On 06/25/2014 01:36 AM, Courtney Cavin wrote:
> > Greg, Grant, Rob?  What's the law?
> 
> Generally sub-blocks of a device are handled as platform devices. If
> there is a good enough reason then creating a new device type may be
> okay, but we certainly wouldn't want every PMIC or MFD driver to go
> off and define their own bus. Probably not each vendor doing a bus
> either.

Thanks for the clarification!

> > On a related note, it would probably be a good idea to move much of the
> > platform resource stuff out of the platform code... so we don't
> > re-implement it over-and-over again.
> 
> Which part is implemented over-and-over?

These two, and all the convenient wrapper/population functions:
  struct resource *resource;
  u32 num_resources;

-Courtney

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH 0/6] Support for QPNP PMIC's
  2014-06-25 21:28         ` Courtney Cavin
@ 2014-06-26 15:12           ` Stanimir Varbanov
  0 siblings, 0 replies; 14+ messages in thread
From: Stanimir Varbanov @ 2014-06-26 15:12 UTC (permalink / raw)
  To: Courtney Cavin
  Cc: Rob Herring, Grant Likely, Greg Kroah-Hartman,
	linux-arm-msm@vger.kernel.org, Ivan T. Ivanov, Josh Cartwright,
	Stephen Boyd

On 06/26/2014 12:28 AM, Courtney Cavin wrote:
> On Wed, Jun 25, 2014 at 11:18:57PM +0200, Rob Herring wrote:
>> On Wed, Jun 25, 2014 at 1:04 PM, Courtney Cavin
>> <courtney.cavin@sonymobile.com> wrote:
>>> On Wed, Jun 25, 2014 at 01:07:13PM +0200, Stanimir Varbanov wrote:
>>>> On 06/25/2014 01:36 AM, Courtney Cavin wrote:
>>> Greg, Grant, Rob?  What's the law?
>>
>> Generally sub-blocks of a device are handled as platform devices. If
>> there is a good enough reason then creating a new device type may be
>> okay, but we certainly wouldn't want every PMIC or MFD driver to go
>> off and define their own bus. Probably not each vendor doing a bus
>> either.
> 
> Thanks for the clarification!

OK, I tend to agree that creating a new bus only to define new device
type is pointless. But using platform devices for sub-blocks attached to
spmi-bus seems wrong too.

Courtney, the other option could be to extend
spmi.c::of_spmi_register_devices to create spmi_device for each
sub-function per each slave. Currently the function creates devices only
for every spmi slave.

Thus every sub-function driver will be spmi_driver (something like
i2c_driver for example). Sub-function resources could be embedded in
spmi_device structure.

I'm not sure how much code/efforts will be needed but it seems it is
worth to do.

Any thoughts?


-- 
regards,
Stan

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH 0/6] Support for QPNP PMIC's
  2014-06-25 18:04     ` Courtney Cavin
  2014-06-25 21:18       ` Rob Herring
@ 2014-06-26 15:48       ` Stanimir Varbanov
  1 sibling, 0 replies; 14+ messages in thread
From: Stanimir Varbanov @ 2014-06-26 15:48 UTC (permalink / raw)
  To: Courtney Cavin
  Cc: Grant Likely, Rob Herring, Greg Kroah-Hartman,
	linux-arm-msm@vger.kernel.org, Ivan T. Ivanov, Josh Cartwright,
	Stephen Boyd

On 06/25/2014 09:04 PM, Courtney Cavin wrote:
> On Wed, Jun 25, 2014 at 01:07:13PM +0200, Stanimir Varbanov wrote:
>> Hi Courtney,
>>
>> Thanks for the comments.
>>
>> On 06/25/2014 01:36 AM, Courtney Cavin wrote:
>>> On Fri, Jun 20, 2014 at 02:21:19PM +0200, Stanimir Varbanov wrote:
>>>> Hello to all,
>>>>
>>>> The goal of this WIP set is to provide support for sub-functional
>>>> hw blocks inside Qualcomm QPNP PMIC chips by creating a new qpnp
>>>> bus (device-driver modeled). On SPMI interface we attach a so called
>>>> Slaves. For example one PMIC chip like pm8941 has two slave id's. On
>>>> every slaveid the PMIC chip has various peripherials. Every peripheral
>>>> driver should expose qpnp_driver structure and implement the .probe
>>>> and .remove callbacks. Every peripheral driver will reside on the
>>>> proper place in /drivers directory. 
>>>>
>>>> The fisrt patch describes the devicetree binding of the slave devices
>>>> attached to the SPMI interface. The second patch adds implemetation of
>>>> the qpnp-bus and a layer connection with the SPMI interface. The third
>>>> patch adds spmi arbiter and underlying slaveid devicetree nodes for
>>>> msm8974 SoC. The other patches are example of rtc-qpnp driver and
>>>> binding documentation.
>>>>
>>>> Since this set is a WIP, it will be used as a starting point for future
>>>> duscussions about implementing QPNP PMIC sub-function hw blocks.
>>>
>>> Ok.  I have a few critical problems with this approach to implementing
>>> support for the PMICs:
>>>   - It's *way* more code than needed if done with of_platform_populate()
>>
>> Agreed, but this will be the price we must pay to keep architectural
>> correctness. The PMIC peripherals are not directly accessible by the CPU
>> bus and thus they shouldn't be treated as platform devices. That fact
>> gives as an opportunity to create a qpnp bus on which we will attach the
>> whole bunch of PMIC peripherals and using DT we can describe register
>> regions (256B each) belonging to it.
>>
> 
> So the price we must pay for *your* vision of architectural correctness
> is to duplicate code without any actual benefit?
> 
> According to Documentation/driver-model/platform.txt your view is
> correct:
> 
>   Platform devices are devices that typically appear as autonomous
>   entities in the system. This includes legacy port-based devices and
>   host bridges to peripheral buses, and most controllers integrated
>   into system-on-chip platforms.  What they usually have in common
>   is direct addressing from a CPU bus.  Rarely, a platform_device will
>   be connected through a segment of some other kind of bus; but its
>   registers will still be directly addressable.
> 
> However, it is clear that with recent changes to the platform code, this
> view is getting distorted somewhere.  I haven't specifically heard any
> decisions on what to do here though.
> 

Could you point me to the discussion?

> Greg, Grant, Rob?  What's the law?
> 
>>>   - AFAICT, the only direct relation between this and 'QPNP' is the 256
>>>     byte block size, which I would argue can be clearly expressed in DT
>>>     instead with #size-cells == 1, and #address-cells == 1
>>
>> The QPNP schema is a private case of Qualcomm PMIC's, so generalising
>> will over-engineering IMO.
>>
> 
> I made a similar attempt to write something like this, but did it
> entirely based on regmap "partitions", abstracting out the need for
> knowing anything in the client drivers but regmap....  It was about the
> same amount of code/complexity as this, but I tossed it out for the same
> reasons I mentioned here.  Over/under-engineering has little to do with
> it.

Hm, I missed it.

-- 
regards,
Stan

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2014-06-26 15:48 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-06-20 12:21 [RFC PATCH 0/6] Support for QPNP PMIC's Stanimir Varbanov
2014-06-20 12:21 ` [RFC PATCH 1/6] dt: qpnp: add binding description of Qualcomm QPNP PMICs Stanimir Varbanov
2014-06-20 12:21 ` [RFC PATCH 2/6] qpnp: add support for " Stanimir Varbanov
2014-06-20 12:21 ` [RFC PATCH 3/6] dt: qcom: msm8974: add qpnp-spmi device nodes Stanimir Varbanov
2014-06-20 12:21 ` [RFC PATCH 4/6] rtc: add qpnp rtc driver Stanimir Varbanov
2014-06-20 12:21 ` [RFC PATCH 5/6] dt: msm8974: add qpnp rtc device node Stanimir Varbanov
2014-06-20 12:21 ` [RFC PATCH 6/6] dt: rtc: add binding document for rtc-qpnp Stanimir Varbanov
2014-06-24 22:36 ` [RFC PATCH 0/6] Support for QPNP PMIC's Courtney Cavin
2014-06-25 11:07   ` Stanimir Varbanov
2014-06-25 18:04     ` Courtney Cavin
2014-06-25 21:18       ` Rob Herring
2014-06-25 21:28         ` Courtney Cavin
2014-06-26 15:12           ` Stanimir Varbanov
2014-06-26 15:48       ` Stanimir Varbanov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.