All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH RFC v2 1/5] of: Add empty for_each_available_child_of_node() macro definition
  2013-08-22 20:18 ` Josh Cartwright
@ 2013-08-22 22:57   ` Josh Cartwright
  -1 siblings, 0 replies; 37+ messages in thread
From: Josh Cartwright @ 2012-12-10 19:41 UTC (permalink / raw)
  To: Grant Likely, Rob Herring, Greg Kroah-Hartman
  Cc: linux-kernel, linux-arm-kernel, linux-arm-msm, Sagar Dharia,
	Gilad Avidov, Michael Bohan, Sylwester Nawrocki, Kyungmin Park

From: Sylwester Nawrocki <s.nawrocki@samsung.com>

Add this empty macro definition so users can be compiled without
excluding this macro call with preprocessor directives when CONFIG_OF
is disabled.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
This patch was first introduced in December @
http://thread.gmane.org/gmane.linux.drivers.devicetree/26315

 include/linux/of.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/linux/of.h b/include/linux/of.h
index 1fd08ca..fefffb3 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -369,6 +369,9 @@ static inline bool of_have_populated_dt(void)
 #define for_each_child_of_node(parent, child) \
 	while (0)
 
+#define for_each_available_child_of_node(parent, child) \
+	while (0)
+
 static inline struct device_node *of_get_child_by_name(
 					const struct device_node *node,
 					const char *name)
-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

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

* [PATCH RFC v2 4/5] spmi: Add MSM PMIC Arbiter SPMI controller
  2013-08-22 20:18 ` Josh Cartwright
@ 2013-08-09 20:37   ` Josh Cartwright
  -1 siblings, 0 replies; 37+ messages in thread
From: Josh Cartwright @ 2013-08-09 20:37 UTC (permalink / raw)
  To: Grant Likely, Rob Herring, Greg Kroah-Hartman
  Cc: linux-kernel, linux-arm-kernel, linux-arm-msm, Sagar Dharia,
	Gilad Avidov, Michael Bohan

From: Kenneth Heitke <kheitke@codeaurora.org>

Qualcomm's PMIC Arbiter SPMI controller functions as a bus master and
is used to communication with one or more PMIC (slave) devices on the
SPMI bus.  The PMIC Arbiter is actually a hardware wrapper around the
SPMI controller that provides concurrent and autonomous PMIC access
to various entities that need to communicate with the PMIC.

The SPMI controller hardware handles all of the SPMI bus activity (bus
arbitration, sequence start condition, transmission of frames, etc).
This software driver uses the PMIC Arbiter register interface to
initiate command sequences on the SPMI bus.  The status register is
read to determine when the command sequence has completed and whether
or not it completed successfully.

Signed-off-by: Kenneth Heitke <kheitke@codeaurora.org>
Signed-off-by: Josh Cartwright <joshc@codeaurora.org>
---
 drivers/spmi/Kconfig         |  15 ++
 drivers/spmi/Makefile        |   2 +
 drivers/spmi/spmi-pmic-arb.c | 416 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 433 insertions(+)
 create mode 100644 drivers/spmi/spmi-pmic-arb.c

diff --git a/drivers/spmi/Kconfig b/drivers/spmi/Kconfig
index a03835f..056891d 100644
--- a/drivers/spmi/Kconfig
+++ b/drivers/spmi/Kconfig
@@ -7,3 +7,18 @@ menuconfig SPMI
 	  SPMI (System Power Management Interface) is a two-wire
 	  serial interface between baseband and application processors
 	  and Power Management Integrated Circuits (PMIC).
+
+if SPMI
+
+config SPMI_MSM_PMIC_ARB
+	tristate "Qualcomm MSM SPMI Controller (PMIC Arbiter)"
+	depends on ARCH_MSM
+	help
+	  If you say yes to this option, support will be included for the
+	  built-in SPMI PMIC Arbiter interface on Qualcomm MSM family
+	  processors.
+
+	  This is required for communicating with Qualcomm PMICs and
+	  other devices that have the SPMI interface.
+
+endif
diff --git a/drivers/spmi/Makefile b/drivers/spmi/Makefile
index 9d78ec9..e3cbd01 100644
--- a/drivers/spmi/Makefile
+++ b/drivers/spmi/Makefile
@@ -2,3 +2,5 @@
 # Makefile for kernel SPMI framework.
 #
 obj-$(CONFIG_SPMI)	+= spmi.o spmi-dbgfs.o
+
+obj-$(CONFIG_SPMI_MSM_PMIC_ARB)	+= spmi-pmic-arb.o
diff --git a/drivers/spmi/spmi-pmic-arb.c b/drivers/spmi/spmi-pmic-arb.c
new file mode 100644
index 0000000..155348e
--- /dev/null
+++ b/drivers/spmi/spmi-pmic-arb.c
@@ -0,0 +1,416 @@
+/* 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.
+ */
+#include <linux/debugfs.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/spmi.h>
+#include <linux/of.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/seq_file.h>
+
+#define SPMI_PMIC_ARB_NAME		"spmi_pmic_arb"
+
+/* PMIC Arbiter configuration registers */
+#define PMIC_ARB_VERSION		0x0000
+#define PMIC_ARB_INT_EN			0x0004
+
+/* PMIC Arbiter channel registers */
+#define PMIC_ARB_CMD(N)			(0x0800 + (0x80 * (N)))
+#define PMIC_ARB_CONFIG(N)		(0x0804 + (0x80 * (N)))
+#define PMIC_ARB_STATUS(N)		(0x0808 + (0x80 * (N)))
+#define PMIC_ARB_WDATA0(N)		(0x0810 + (0x80 * (N)))
+#define PMIC_ARB_WDATA1(N)		(0x0814 + (0x80 * (N)))
+#define PMIC_ARB_RDATA0(N)		(0x0818 + (0x80 * (N)))
+#define PMIC_ARB_RDATA1(N)		(0x081C + (0x80 * (N)))
+
+/* Interrupt Controller */
+#define SPMI_PIC_OWNER_ACC_STATUS(M, N)	(0x0000 + ((32 * (M)) + (4 * (N))))
+#define SPMI_PIC_ACC_ENABLE(N)		(0x0200 + (4 * (N)))
+#define SPMI_PIC_IRQ_STATUS(N)		(0x0600 + (4 * (N)))
+#define SPMI_PIC_IRQ_CLEAR(N)		(0x0A00 + (4 * (N)))
+
+/* Mapping Table */
+#define SPMI_MAPPING_TABLE_REG(N)	(0x0B00 + (4 * (N)))
+#define SPMI_MAPPING_BIT_INDEX(X)	(((X) >> 18) & 0xF)
+#define SPMI_MAPPING_BIT_IS_0_FLAG(X)	(((X) >> 17) & 0x1)
+#define SPMI_MAPPING_BIT_IS_0_RESULT(X)	(((X) >> 9) & 0xFF)
+#define SPMI_MAPPING_BIT_IS_1_FLAG(X)	(((X) >> 8) & 0x1)
+#define SPMI_MAPPING_BIT_IS_1_RESULT(X)	(((X) >> 0) & 0xFF)
+
+#define SPMI_MAPPING_TABLE_LEN		255
+#define SPMI_MAPPING_TABLE_TREE_DEPTH	16	/* Maximum of 16-bits */
+
+/* Ownership Table */
+#define SPMI_OWNERSHIP_TABLE_REG(N)	(0x0700 + (4 * (N)))
+#define SPMI_OWNERSHIP_PERIPH2OWNER(X)	((X) & 0x7)
+
+/* Channel Status fields */
+enum pmic_arb_chnl_status {
+	PMIC_ARB_STATUS_DONE	= (1 << 0),
+	PMIC_ARB_STATUS_FAILURE	= (1 << 1),
+	PMIC_ARB_STATUS_DENIED	= (1 << 2),
+	PMIC_ARB_STATUS_DROPPED	= (1 << 3),
+};
+
+/* Command register fields */
+#define PMIC_ARB_CMD_MAX_BYTE_COUNT	8
+
+/* Command Opcodes */
+enum pmic_arb_cmd_op_code {
+	PMIC_ARB_OP_EXT_WRITEL = 0,
+	PMIC_ARB_OP_EXT_READL = 1,
+	PMIC_ARB_OP_EXT_WRITE = 2,
+	PMIC_ARB_OP_RESET = 3,
+	PMIC_ARB_OP_SLEEP = 4,
+	PMIC_ARB_OP_SHUTDOWN = 5,
+	PMIC_ARB_OP_WAKEUP = 6,
+	PMIC_ARB_OP_AUTHENTICATE = 7,
+	PMIC_ARB_OP_MSTR_READ = 8,
+	PMIC_ARB_OP_MSTR_WRITE = 9,
+	PMIC_ARB_OP_EXT_READ = 13,
+	PMIC_ARB_OP_WRITE = 14,
+	PMIC_ARB_OP_READ = 15,
+	PMIC_ARB_OP_ZERO_WRITE = 16,
+};
+
+/* Maximum number of support PMIC peripherals */
+#define PMIC_ARB_MAX_PERIPHS		256
+#define PMIC_ARB_PERIPH_ID_VALID	(1 << 15)
+#define PMIC_ARB_TIMEOUT_US		100
+#define PMIC_ARB_MAX_TRANS_BYTES	(8)
+
+#define PMIC_ARB_APID_MASK		0xFF
+#define PMIC_ARB_PPID_MASK		0xFFF
+
+/* interrupt enable bit */
+#define SPMI_PIC_ACC_ENABLE_BIT		BIT(0)
+
+/**
+ * base - base address of the PMIC Arbiter core registers
+ * intr - base address of the SPMI interrupt control registers
+ * cnfg - base address of the PMIC Arbiter configuration registers
+ */
+struct spmi_pmic_arb_dev {
+	void __iomem		*base;
+	void __iomem		*intr;
+	void __iomem		*cnfg;
+	bool			allow_wakeup;
+	spinlock_t		lock;
+	u8			channel;
+	u8			min_apid;
+	u8			max_apid;
+	u16			periph_id_map[PMIC_ARB_MAX_PERIPHS];
+	u32			mapping_table[SPMI_MAPPING_TABLE_LEN];
+};
+
+static u32 pmic_arb_read(struct spmi_pmic_arb_dev *dev, u32 offset)
+{
+	return readl_relaxed(dev->base + offset);
+}
+
+static void pmic_arb_write(struct spmi_pmic_arb_dev *dev, u32 offset, u32 val)
+{
+	writel_relaxed(val, dev->base + offset);
+}
+
+static int pmic_arb_wait_for_done(struct spmi_controller *ctrl)
+{
+	struct spmi_pmic_arb_dev *dev = spmi_controller_get_drvdata(ctrl);
+	u32 status = 0;
+	u32 timeout = PMIC_ARB_TIMEOUT_US;
+	u32 offset = PMIC_ARB_STATUS(dev->channel);
+
+	while (timeout--) {
+		status = pmic_arb_read(dev, offset);
+
+		if (status & PMIC_ARB_STATUS_DONE) {
+			if (status & PMIC_ARB_STATUS_DENIED) {
+				dev_err(&ctrl->dev,
+					"%s: transaction denied (0x%x)\n",
+					__func__, status);
+				return -EPERM;
+			}
+
+			if (status & PMIC_ARB_STATUS_FAILURE) {
+				dev_err(&ctrl->dev,
+					"%s: transaction failed (0x%x)\n",
+					__func__, status);
+				return -EIO;
+			}
+
+			if (status & PMIC_ARB_STATUS_DROPPED) {
+				dev_err(&ctrl->dev,
+					"%s: transaction dropped (0x%x)\n",
+					__func__, status);
+				return -EIO;
+			}
+
+			return 0;
+		}
+		udelay(1);
+	}
+
+	dev_err(&ctrl->dev,
+		"%s: timeout, status 0x%x\n",
+		__func__, status);
+	return -ETIMEDOUT;
+}
+
+/**
+ * pa_read_data: reads pmic-arb's register and copy 1..4 bytes to buf
+ * @bc byte count -1. range: 0..3
+ * @reg register's address
+ * @buf output parameter, length must be bc+1
+ */
+static void pa_read_data(struct spmi_pmic_arb_dev *dev, u8 *buf, u32 reg, u8 bc)
+{
+	u32 data = pmic_arb_read(dev, reg);
+	memcpy(buf, &data, (bc & 3) + 1);
+}
+
+/**
+ * pa_write_data: write 1..4 bytes from buf to pmic-arb's register
+ * @bc byte-count -1. range: 0..3
+ * @reg register's address
+ * @buf buffer to write. length must be bc+1
+ */
+static void
+pa_write_data(struct spmi_pmic_arb_dev *dev, u8 *buf, u32 reg, u8 bc)
+{
+	u32 data = 0;
+	memcpy(&data, buf, (bc & 3) + 1);
+	pmic_arb_write(dev, reg, data);
+}
+
+/* Non-data command */
+static int pmic_arb_cmd(struct spmi_controller *ctrl, u8 opc, u8 sid)
+{
+	struct spmi_pmic_arb_dev *pmic_arb = spmi_controller_get_drvdata(ctrl);
+	unsigned long flags;
+	u32 cmd;
+	int rc;
+
+	dev_dbg(&ctrl->dev, "op:0x%x sid:%d\n", opc, sid);
+
+	/* Check for valid non-data command */
+	if (opc < SPMI_CMD_RESET || opc > SPMI_CMD_WAKEUP)
+		return -EINVAL;
+
+	cmd = ((opc | 0x40) << 27) | ((sid & 0xf) << 20);
+
+	spin_lock_irqsave(&pmic_arb->lock, flags);
+	pmic_arb_write(pmic_arb, PMIC_ARB_CMD(pmic_arb->channel), cmd);
+	rc = pmic_arb_wait_for_done(ctrl);
+	spin_unlock_irqrestore(&pmic_arb->lock, flags);
+
+	return rc;
+}
+
+static int pmic_arb_read_cmd(struct spmi_controller *ctrl,
+			     u8 opc, u8 sid, u16 addr, u8 bc, u8 *buf)
+{
+	struct spmi_pmic_arb_dev *pmic_arb = spmi_controller_get_drvdata(ctrl);
+	unsigned long flags;
+	u32 cmd;
+	int rc;
+
+	if (bc >= PMIC_ARB_MAX_TRANS_BYTES) {
+		dev_err(&ctrl->dev,
+			"pmic-arb supports 1..%d bytes per trans, but:%d requested",
+			PMIC_ARB_MAX_TRANS_BYTES, bc+1);
+		return  -EINVAL;
+	}
+	dev_dbg(&ctrl->dev,
+		"op:0x%x sid:%d bc:%d addr:0x%x\n", opc, sid, bc, addr);
+
+	/* Check the opcode */
+	if (opc >= 0x60 && opc <= 0x7F)
+		opc = PMIC_ARB_OP_READ;
+	else if (opc >= 0x20 && opc <= 0x2F)
+		opc = PMIC_ARB_OP_EXT_READ;
+	else if (opc >= 0x38 && opc <= 0x3F)
+		opc = PMIC_ARB_OP_EXT_READL;
+	else
+		return -EINVAL;
+
+	cmd = (opc << 27) | ((sid & 0xf) << 20) | (addr << 4) | (bc & 0x7);
+
+	spin_lock_irqsave(&pmic_arb->lock, flags);
+	pmic_arb_write(pmic_arb, PMIC_ARB_CMD(pmic_arb->channel), cmd);
+	rc = pmic_arb_wait_for_done(ctrl);
+	if (rc)
+		goto done;
+
+	/* Read from FIFO, note 'bc' is actually number of bytes minus 1 */
+	pa_read_data(pmic_arb, buf, PMIC_ARB_RDATA0(pmic_arb->channel)
+							, min_t(u8, bc, 3));
+
+	if (bc > 3)
+		pa_read_data(pmic_arb, buf + 4,
+				PMIC_ARB_RDATA1(pmic_arb->channel), bc - 4);
+
+done:
+	spin_unlock_irqrestore(&pmic_arb->lock, flags);
+	return rc;
+}
+
+static int pmic_arb_write_cmd(struct spmi_controller *ctrl,
+			      u8 opc, u8 sid, u16 addr, u8 bc, u8 *buf)
+{
+	struct spmi_pmic_arb_dev *pmic_arb = spmi_controller_get_drvdata(ctrl);
+	unsigned long flags;
+	u32 cmd;
+	int rc;
+
+	if (bc >= PMIC_ARB_MAX_TRANS_BYTES) {
+		dev_err(&ctrl->dev,
+			"pmic-arb supports 1..%d bytes per trans, but:%d requested",
+			PMIC_ARB_MAX_TRANS_BYTES, bc+1);
+		return  -EINVAL;
+	}
+	dev_dbg(&ctrl->dev,
+		"op:0x%x sid:%d bc:%d addr:0x%x\n", opc, sid, bc, addr);
+
+	/* Check the opcode */
+	if (opc >= 0x40 && opc <= 0x5F)
+		opc = PMIC_ARB_OP_WRITE;
+	else if (opc >= 0x00 && opc <= 0x0F)
+		opc = PMIC_ARB_OP_EXT_WRITE;
+	else if (opc >= 0x30 && opc <= 0x37)
+		opc = PMIC_ARB_OP_EXT_WRITEL;
+	else if (opc >= 0x80 && opc <= 0xFF)
+		opc = PMIC_ARB_OP_ZERO_WRITE;
+	else
+		return -EINVAL;
+
+	cmd = (opc << 27) | ((sid & 0xf) << 20) | (addr << 4) | (bc & 0x7);
+
+	/* Write data to FIFOs */
+	spin_lock_irqsave(&pmic_arb->lock, flags);
+	pa_write_data(pmic_arb, buf, PMIC_ARB_WDATA0(pmic_arb->channel)
+							, min_t(u8, bc, 3));
+	if (bc > 3)
+		pa_write_data(pmic_arb, buf + 4,
+				PMIC_ARB_WDATA1(pmic_arb->channel), bc - 4);
+
+	/* Start the transaction */
+	pmic_arb_write(pmic_arb, PMIC_ARB_CMD(pmic_arb->channel), cmd);
+	rc = pmic_arb_wait_for_done(ctrl);
+	spin_unlock_irqrestore(&pmic_arb->lock, flags);
+
+	return rc;
+}
+
+/* APID to PPID */
+static inline u16 get_peripheral_id(struct spmi_pmic_arb_dev *pmic_arb, u8 apid)
+{
+	return pmic_arb->periph_id_map[apid] & PMIC_ARB_PPID_MASK;
+}
+
+/* APID to PPID, returns valid flag */
+static inline int is_apid_valid(struct spmi_pmic_arb_dev *pmic_arb, u8 apid)
+{
+	return pmic_arb->periph_id_map[apid] & PMIC_ARB_PERIPH_ID_VALID;
+}
+
+static int spmi_pmic_arb_probe(struct platform_device *pdev)
+{
+	struct spmi_pmic_arb_dev *pa;
+	struct spmi_controller *ctrl;
+	struct resource *res;
+	int err;
+	int i;
+
+	ctrl = spmi_controller_alloc(&pdev->dev, sizeof(*pa));
+	if (!ctrl)
+		return -ENOMEM;
+
+	pa = spmi_controller_get_drvdata(ctrl);
+
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "core");
+	pa->base = devm_ioremap_resource(&ctrl->dev, res);
+	if (IS_ERR(pa->base)) {
+		err = PTR_ERR(pa->base);
+		goto err_put_ctrl;
+	}
+
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "intr");
+	pa->intr = devm_ioremap_resource(&ctrl->dev, res);
+	if (IS_ERR(pa->intr)) {
+		err = PTR_ERR(pa->intr);
+		goto err_put_ctrl;
+	}
+
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "cnfg");
+	pa->cnfg = devm_ioremap_resource(&ctrl->dev, res);
+	if (IS_ERR(pa->cnfg)) {
+		err = PTR_ERR(pa->cnfg);
+		goto err_put_ctrl;
+	}
+
+	for (i = 0; i < ARRAY_SIZE(pa->mapping_table); ++i)
+		pa->mapping_table[i] = readl_relaxed(
+				pa->cnfg + SPMI_MAPPING_TABLE_REG(i));
+
+	platform_set_drvdata(pdev, ctrl);
+	spin_lock_init(&pa->lock);
+
+	pa->channel = 0;
+	pa->max_apid = 0;
+	pa->min_apid = PMIC_ARB_MAX_PERIPHS - 1;
+
+	ctrl->cmd = pmic_arb_cmd;
+	ctrl->read_cmd = pmic_arb_read_cmd;
+	ctrl->write_cmd =  pmic_arb_write_cmd;
+
+	err = spmi_controller_add(ctrl);
+	if (err)
+		goto err_put_ctrl;
+
+	dev_dbg(&ctrl->dev, "PMIC Arb Version 0x%x\n",
+		pmic_arb_read(pa, PMIC_ARB_VERSION));
+
+	return 0;
+
+err_put_ctrl:
+	spmi_controller_put(ctrl);
+	return err;
+}
+
+static int __exit spmi_pmic_arb_remove(struct platform_device *pdev)
+{
+	struct spmi_controller *ctrl = platform_get_drvdata(pdev);
+	spmi_controller_remove(ctrl);
+	return 0;
+}
+
+static struct of_device_id spmi_pmic_arb_match_table[] = {
+	{	.compatible = "qcom,spmi-pmic-arb", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, spmi_pmic_arb_match_table);
+
+static struct platform_driver spmi_pmic_arb_driver = {
+	.probe		= spmi_pmic_arb_probe,
+	.remove		= __exit_p(spmi_pmic_arb_remove),
+	.driver		= {
+		.name	= SPMI_PMIC_ARB_NAME,
+		.owner	= THIS_MODULE,
+		.of_match_table = spmi_pmic_arb_match_table,
+	},
+};
+module_platform_driver(spmi_pmic_arb_driver);
-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

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

* [PATCH RFC v2 4/5] spmi: Add MSM PMIC Arbiter SPMI controller
@ 2013-08-09 20:37   ` Josh Cartwright
  0 siblings, 0 replies; 37+ messages in thread
From: Josh Cartwright @ 2013-08-09 20:37 UTC (permalink / raw)
  To: linux-arm-kernel

From: Kenneth Heitke <kheitke@codeaurora.org>

Qualcomm's PMIC Arbiter SPMI controller functions as a bus master and
is used to communication with one or more PMIC (slave) devices on the
SPMI bus.  The PMIC Arbiter is actually a hardware wrapper around the
SPMI controller that provides concurrent and autonomous PMIC access
to various entities that need to communicate with the PMIC.

The SPMI controller hardware handles all of the SPMI bus activity (bus
arbitration, sequence start condition, transmission of frames, etc).
This software driver uses the PMIC Arbiter register interface to
initiate command sequences on the SPMI bus.  The status register is
read to determine when the command sequence has completed and whether
or not it completed successfully.

Signed-off-by: Kenneth Heitke <kheitke@codeaurora.org>
Signed-off-by: Josh Cartwright <joshc@codeaurora.org>
---
 drivers/spmi/Kconfig         |  15 ++
 drivers/spmi/Makefile        |   2 +
 drivers/spmi/spmi-pmic-arb.c | 416 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 433 insertions(+)
 create mode 100644 drivers/spmi/spmi-pmic-arb.c

diff --git a/drivers/spmi/Kconfig b/drivers/spmi/Kconfig
index a03835f..056891d 100644
--- a/drivers/spmi/Kconfig
+++ b/drivers/spmi/Kconfig
@@ -7,3 +7,18 @@ menuconfig SPMI
 	  SPMI (System Power Management Interface) is a two-wire
 	  serial interface between baseband and application processors
 	  and Power Management Integrated Circuits (PMIC).
+
+if SPMI
+
+config SPMI_MSM_PMIC_ARB
+	tristate "Qualcomm MSM SPMI Controller (PMIC Arbiter)"
+	depends on ARCH_MSM
+	help
+	  If you say yes to this option, support will be included for the
+	  built-in SPMI PMIC Arbiter interface on Qualcomm MSM family
+	  processors.
+
+	  This is required for communicating with Qualcomm PMICs and
+	  other devices that have the SPMI interface.
+
+endif
diff --git a/drivers/spmi/Makefile b/drivers/spmi/Makefile
index 9d78ec9..e3cbd01 100644
--- a/drivers/spmi/Makefile
+++ b/drivers/spmi/Makefile
@@ -2,3 +2,5 @@
 # Makefile for kernel SPMI framework.
 #
 obj-$(CONFIG_SPMI)	+= spmi.o spmi-dbgfs.o
+
+obj-$(CONFIG_SPMI_MSM_PMIC_ARB)	+= spmi-pmic-arb.o
diff --git a/drivers/spmi/spmi-pmic-arb.c b/drivers/spmi/spmi-pmic-arb.c
new file mode 100644
index 0000000..155348e
--- /dev/null
+++ b/drivers/spmi/spmi-pmic-arb.c
@@ -0,0 +1,416 @@
+/* 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.
+ */
+#include <linux/debugfs.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/spmi.h>
+#include <linux/of.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/seq_file.h>
+
+#define SPMI_PMIC_ARB_NAME		"spmi_pmic_arb"
+
+/* PMIC Arbiter configuration registers */
+#define PMIC_ARB_VERSION		0x0000
+#define PMIC_ARB_INT_EN			0x0004
+
+/* PMIC Arbiter channel registers */
+#define PMIC_ARB_CMD(N)			(0x0800 + (0x80 * (N)))
+#define PMIC_ARB_CONFIG(N)		(0x0804 + (0x80 * (N)))
+#define PMIC_ARB_STATUS(N)		(0x0808 + (0x80 * (N)))
+#define PMIC_ARB_WDATA0(N)		(0x0810 + (0x80 * (N)))
+#define PMIC_ARB_WDATA1(N)		(0x0814 + (0x80 * (N)))
+#define PMIC_ARB_RDATA0(N)		(0x0818 + (0x80 * (N)))
+#define PMIC_ARB_RDATA1(N)		(0x081C + (0x80 * (N)))
+
+/* Interrupt Controller */
+#define SPMI_PIC_OWNER_ACC_STATUS(M, N)	(0x0000 + ((32 * (M)) + (4 * (N))))
+#define SPMI_PIC_ACC_ENABLE(N)		(0x0200 + (4 * (N)))
+#define SPMI_PIC_IRQ_STATUS(N)		(0x0600 + (4 * (N)))
+#define SPMI_PIC_IRQ_CLEAR(N)		(0x0A00 + (4 * (N)))
+
+/* Mapping Table */
+#define SPMI_MAPPING_TABLE_REG(N)	(0x0B00 + (4 * (N)))
+#define SPMI_MAPPING_BIT_INDEX(X)	(((X) >> 18) & 0xF)
+#define SPMI_MAPPING_BIT_IS_0_FLAG(X)	(((X) >> 17) & 0x1)
+#define SPMI_MAPPING_BIT_IS_0_RESULT(X)	(((X) >> 9) & 0xFF)
+#define SPMI_MAPPING_BIT_IS_1_FLAG(X)	(((X) >> 8) & 0x1)
+#define SPMI_MAPPING_BIT_IS_1_RESULT(X)	(((X) >> 0) & 0xFF)
+
+#define SPMI_MAPPING_TABLE_LEN		255
+#define SPMI_MAPPING_TABLE_TREE_DEPTH	16	/* Maximum of 16-bits */
+
+/* Ownership Table */
+#define SPMI_OWNERSHIP_TABLE_REG(N)	(0x0700 + (4 * (N)))
+#define SPMI_OWNERSHIP_PERIPH2OWNER(X)	((X) & 0x7)
+
+/* Channel Status fields */
+enum pmic_arb_chnl_status {
+	PMIC_ARB_STATUS_DONE	= (1 << 0),
+	PMIC_ARB_STATUS_FAILURE	= (1 << 1),
+	PMIC_ARB_STATUS_DENIED	= (1 << 2),
+	PMIC_ARB_STATUS_DROPPED	= (1 << 3),
+};
+
+/* Command register fields */
+#define PMIC_ARB_CMD_MAX_BYTE_COUNT	8
+
+/* Command Opcodes */
+enum pmic_arb_cmd_op_code {
+	PMIC_ARB_OP_EXT_WRITEL = 0,
+	PMIC_ARB_OP_EXT_READL = 1,
+	PMIC_ARB_OP_EXT_WRITE = 2,
+	PMIC_ARB_OP_RESET = 3,
+	PMIC_ARB_OP_SLEEP = 4,
+	PMIC_ARB_OP_SHUTDOWN = 5,
+	PMIC_ARB_OP_WAKEUP = 6,
+	PMIC_ARB_OP_AUTHENTICATE = 7,
+	PMIC_ARB_OP_MSTR_READ = 8,
+	PMIC_ARB_OP_MSTR_WRITE = 9,
+	PMIC_ARB_OP_EXT_READ = 13,
+	PMIC_ARB_OP_WRITE = 14,
+	PMIC_ARB_OP_READ = 15,
+	PMIC_ARB_OP_ZERO_WRITE = 16,
+};
+
+/* Maximum number of support PMIC peripherals */
+#define PMIC_ARB_MAX_PERIPHS		256
+#define PMIC_ARB_PERIPH_ID_VALID	(1 << 15)
+#define PMIC_ARB_TIMEOUT_US		100
+#define PMIC_ARB_MAX_TRANS_BYTES	(8)
+
+#define PMIC_ARB_APID_MASK		0xFF
+#define PMIC_ARB_PPID_MASK		0xFFF
+
+/* interrupt enable bit */
+#define SPMI_PIC_ACC_ENABLE_BIT		BIT(0)
+
+/**
+ * base - base address of the PMIC Arbiter core registers
+ * intr - base address of the SPMI interrupt control registers
+ * cnfg - base address of the PMIC Arbiter configuration registers
+ */
+struct spmi_pmic_arb_dev {
+	void __iomem		*base;
+	void __iomem		*intr;
+	void __iomem		*cnfg;
+	bool			allow_wakeup;
+	spinlock_t		lock;
+	u8			channel;
+	u8			min_apid;
+	u8			max_apid;
+	u16			periph_id_map[PMIC_ARB_MAX_PERIPHS];
+	u32			mapping_table[SPMI_MAPPING_TABLE_LEN];
+};
+
+static u32 pmic_arb_read(struct spmi_pmic_arb_dev *dev, u32 offset)
+{
+	return readl_relaxed(dev->base + offset);
+}
+
+static void pmic_arb_write(struct spmi_pmic_arb_dev *dev, u32 offset, u32 val)
+{
+	writel_relaxed(val, dev->base + offset);
+}
+
+static int pmic_arb_wait_for_done(struct spmi_controller *ctrl)
+{
+	struct spmi_pmic_arb_dev *dev = spmi_controller_get_drvdata(ctrl);
+	u32 status = 0;
+	u32 timeout = PMIC_ARB_TIMEOUT_US;
+	u32 offset = PMIC_ARB_STATUS(dev->channel);
+
+	while (timeout--) {
+		status = pmic_arb_read(dev, offset);
+
+		if (status & PMIC_ARB_STATUS_DONE) {
+			if (status & PMIC_ARB_STATUS_DENIED) {
+				dev_err(&ctrl->dev,
+					"%s: transaction denied (0x%x)\n",
+					__func__, status);
+				return -EPERM;
+			}
+
+			if (status & PMIC_ARB_STATUS_FAILURE) {
+				dev_err(&ctrl->dev,
+					"%s: transaction failed (0x%x)\n",
+					__func__, status);
+				return -EIO;
+			}
+
+			if (status & PMIC_ARB_STATUS_DROPPED) {
+				dev_err(&ctrl->dev,
+					"%s: transaction dropped (0x%x)\n",
+					__func__, status);
+				return -EIO;
+			}
+
+			return 0;
+		}
+		udelay(1);
+	}
+
+	dev_err(&ctrl->dev,
+		"%s: timeout, status 0x%x\n",
+		__func__, status);
+	return -ETIMEDOUT;
+}
+
+/**
+ * pa_read_data: reads pmic-arb's register and copy 1..4 bytes to buf
+ * @bc byte count -1. range: 0..3
+ * @reg register's address
+ * @buf output parameter, length must be bc+1
+ */
+static void pa_read_data(struct spmi_pmic_arb_dev *dev, u8 *buf, u32 reg, u8 bc)
+{
+	u32 data = pmic_arb_read(dev, reg);
+	memcpy(buf, &data, (bc & 3) + 1);
+}
+
+/**
+ * pa_write_data: write 1..4 bytes from buf to pmic-arb's register
+ * @bc byte-count -1. range: 0..3
+ * @reg register's address
+ * @buf buffer to write. length must be bc+1
+ */
+static void
+pa_write_data(struct spmi_pmic_arb_dev *dev, u8 *buf, u32 reg, u8 bc)
+{
+	u32 data = 0;
+	memcpy(&data, buf, (bc & 3) + 1);
+	pmic_arb_write(dev, reg, data);
+}
+
+/* Non-data command */
+static int pmic_arb_cmd(struct spmi_controller *ctrl, u8 opc, u8 sid)
+{
+	struct spmi_pmic_arb_dev *pmic_arb = spmi_controller_get_drvdata(ctrl);
+	unsigned long flags;
+	u32 cmd;
+	int rc;
+
+	dev_dbg(&ctrl->dev, "op:0x%x sid:%d\n", opc, sid);
+
+	/* Check for valid non-data command */
+	if (opc < SPMI_CMD_RESET || opc > SPMI_CMD_WAKEUP)
+		return -EINVAL;
+
+	cmd = ((opc | 0x40) << 27) | ((sid & 0xf) << 20);
+
+	spin_lock_irqsave(&pmic_arb->lock, flags);
+	pmic_arb_write(pmic_arb, PMIC_ARB_CMD(pmic_arb->channel), cmd);
+	rc = pmic_arb_wait_for_done(ctrl);
+	spin_unlock_irqrestore(&pmic_arb->lock, flags);
+
+	return rc;
+}
+
+static int pmic_arb_read_cmd(struct spmi_controller *ctrl,
+			     u8 opc, u8 sid, u16 addr, u8 bc, u8 *buf)
+{
+	struct spmi_pmic_arb_dev *pmic_arb = spmi_controller_get_drvdata(ctrl);
+	unsigned long flags;
+	u32 cmd;
+	int rc;
+
+	if (bc >= PMIC_ARB_MAX_TRANS_BYTES) {
+		dev_err(&ctrl->dev,
+			"pmic-arb supports 1..%d bytes per trans, but:%d requested",
+			PMIC_ARB_MAX_TRANS_BYTES, bc+1);
+		return  -EINVAL;
+	}
+	dev_dbg(&ctrl->dev,
+		"op:0x%x sid:%d bc:%d addr:0x%x\n", opc, sid, bc, addr);
+
+	/* Check the opcode */
+	if (opc >= 0x60 && opc <= 0x7F)
+		opc = PMIC_ARB_OP_READ;
+	else if (opc >= 0x20 && opc <= 0x2F)
+		opc = PMIC_ARB_OP_EXT_READ;
+	else if (opc >= 0x38 && opc <= 0x3F)
+		opc = PMIC_ARB_OP_EXT_READL;
+	else
+		return -EINVAL;
+
+	cmd = (opc << 27) | ((sid & 0xf) << 20) | (addr << 4) | (bc & 0x7);
+
+	spin_lock_irqsave(&pmic_arb->lock, flags);
+	pmic_arb_write(pmic_arb, PMIC_ARB_CMD(pmic_arb->channel), cmd);
+	rc = pmic_arb_wait_for_done(ctrl);
+	if (rc)
+		goto done;
+
+	/* Read from FIFO, note 'bc' is actually number of bytes minus 1 */
+	pa_read_data(pmic_arb, buf, PMIC_ARB_RDATA0(pmic_arb->channel)
+							, min_t(u8, bc, 3));
+
+	if (bc > 3)
+		pa_read_data(pmic_arb, buf + 4,
+				PMIC_ARB_RDATA1(pmic_arb->channel), bc - 4);
+
+done:
+	spin_unlock_irqrestore(&pmic_arb->lock, flags);
+	return rc;
+}
+
+static int pmic_arb_write_cmd(struct spmi_controller *ctrl,
+			      u8 opc, u8 sid, u16 addr, u8 bc, u8 *buf)
+{
+	struct spmi_pmic_arb_dev *pmic_arb = spmi_controller_get_drvdata(ctrl);
+	unsigned long flags;
+	u32 cmd;
+	int rc;
+
+	if (bc >= PMIC_ARB_MAX_TRANS_BYTES) {
+		dev_err(&ctrl->dev,
+			"pmic-arb supports 1..%d bytes per trans, but:%d requested",
+			PMIC_ARB_MAX_TRANS_BYTES, bc+1);
+		return  -EINVAL;
+	}
+	dev_dbg(&ctrl->dev,
+		"op:0x%x sid:%d bc:%d addr:0x%x\n", opc, sid, bc, addr);
+
+	/* Check the opcode */
+	if (opc >= 0x40 && opc <= 0x5F)
+		opc = PMIC_ARB_OP_WRITE;
+	else if (opc >= 0x00 && opc <= 0x0F)
+		opc = PMIC_ARB_OP_EXT_WRITE;
+	else if (opc >= 0x30 && opc <= 0x37)
+		opc = PMIC_ARB_OP_EXT_WRITEL;
+	else if (opc >= 0x80 && opc <= 0xFF)
+		opc = PMIC_ARB_OP_ZERO_WRITE;
+	else
+		return -EINVAL;
+
+	cmd = (opc << 27) | ((sid & 0xf) << 20) | (addr << 4) | (bc & 0x7);
+
+	/* Write data to FIFOs */
+	spin_lock_irqsave(&pmic_arb->lock, flags);
+	pa_write_data(pmic_arb, buf, PMIC_ARB_WDATA0(pmic_arb->channel)
+							, min_t(u8, bc, 3));
+	if (bc > 3)
+		pa_write_data(pmic_arb, buf + 4,
+				PMIC_ARB_WDATA1(pmic_arb->channel), bc - 4);
+
+	/* Start the transaction */
+	pmic_arb_write(pmic_arb, PMIC_ARB_CMD(pmic_arb->channel), cmd);
+	rc = pmic_arb_wait_for_done(ctrl);
+	spin_unlock_irqrestore(&pmic_arb->lock, flags);
+
+	return rc;
+}
+
+/* APID to PPID */
+static inline u16 get_peripheral_id(struct spmi_pmic_arb_dev *pmic_arb, u8 apid)
+{
+	return pmic_arb->periph_id_map[apid] & PMIC_ARB_PPID_MASK;
+}
+
+/* APID to PPID, returns valid flag */
+static inline int is_apid_valid(struct spmi_pmic_arb_dev *pmic_arb, u8 apid)
+{
+	return pmic_arb->periph_id_map[apid] & PMIC_ARB_PERIPH_ID_VALID;
+}
+
+static int spmi_pmic_arb_probe(struct platform_device *pdev)
+{
+	struct spmi_pmic_arb_dev *pa;
+	struct spmi_controller *ctrl;
+	struct resource *res;
+	int err;
+	int i;
+
+	ctrl = spmi_controller_alloc(&pdev->dev, sizeof(*pa));
+	if (!ctrl)
+		return -ENOMEM;
+
+	pa = spmi_controller_get_drvdata(ctrl);
+
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "core");
+	pa->base = devm_ioremap_resource(&ctrl->dev, res);
+	if (IS_ERR(pa->base)) {
+		err = PTR_ERR(pa->base);
+		goto err_put_ctrl;
+	}
+
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "intr");
+	pa->intr = devm_ioremap_resource(&ctrl->dev, res);
+	if (IS_ERR(pa->intr)) {
+		err = PTR_ERR(pa->intr);
+		goto err_put_ctrl;
+	}
+
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "cnfg");
+	pa->cnfg = devm_ioremap_resource(&ctrl->dev, res);
+	if (IS_ERR(pa->cnfg)) {
+		err = PTR_ERR(pa->cnfg);
+		goto err_put_ctrl;
+	}
+
+	for (i = 0; i < ARRAY_SIZE(pa->mapping_table); ++i)
+		pa->mapping_table[i] = readl_relaxed(
+				pa->cnfg + SPMI_MAPPING_TABLE_REG(i));
+
+	platform_set_drvdata(pdev, ctrl);
+	spin_lock_init(&pa->lock);
+
+	pa->channel = 0;
+	pa->max_apid = 0;
+	pa->min_apid = PMIC_ARB_MAX_PERIPHS - 1;
+
+	ctrl->cmd = pmic_arb_cmd;
+	ctrl->read_cmd = pmic_arb_read_cmd;
+	ctrl->write_cmd =  pmic_arb_write_cmd;
+
+	err = spmi_controller_add(ctrl);
+	if (err)
+		goto err_put_ctrl;
+
+	dev_dbg(&ctrl->dev, "PMIC Arb Version 0x%x\n",
+		pmic_arb_read(pa, PMIC_ARB_VERSION));
+
+	return 0;
+
+err_put_ctrl:
+	spmi_controller_put(ctrl);
+	return err;
+}
+
+static int __exit spmi_pmic_arb_remove(struct platform_device *pdev)
+{
+	struct spmi_controller *ctrl = platform_get_drvdata(pdev);
+	spmi_controller_remove(ctrl);
+	return 0;
+}
+
+static struct of_device_id spmi_pmic_arb_match_table[] = {
+	{	.compatible = "qcom,spmi-pmic-arb", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, spmi_pmic_arb_match_table);
+
+static struct platform_driver spmi_pmic_arb_driver = {
+	.probe		= spmi_pmic_arb_probe,
+	.remove		= __exit_p(spmi_pmic_arb_remove),
+	.driver		= {
+		.name	= SPMI_PMIC_ARB_NAME,
+		.owner	= THIS_MODULE,
+		.of_match_table = spmi_pmic_arb_match_table,
+	},
+};
+module_platform_driver(spmi_pmic_arb_driver);
-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

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

* [PATCH RFC v2 5/5] spmi: document the PMIC arbiter SPMI bindings
  2013-08-22 20:18 ` Josh Cartwright
@ 2013-08-09 20:37   ` Josh Cartwright
  -1 siblings, 0 replies; 37+ messages in thread
From: Josh Cartwright @ 2013-08-09 20:37 UTC (permalink / raw)
  To: Grant Likely, Rob Herring, Greg Kroah-Hartman, Pawel Moll,
	Mark Rutland, Stephen Warren, Ian Campbell, Kumar Gala
  Cc: linux-kernel, linux-arm-kernel, linux-arm-msm, Sagar Dharia,
	Gilad Avidov, Michael Bohan, devicetree

Signed-off-by: Josh Cartwright <joshc@codeaurora.org>
---
 .../bindings/spmi/qcom,spmi-pmic-arb.txt           | 36 ++++++++++++++++++++++
 1 file changed, 36 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.txt

diff --git a/Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.txt b/Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.txt
new file mode 100644
index 0000000..3965236
--- /dev/null
+++ b/Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.txt
@@ -0,0 +1,36 @@
+Qualcomm SPMI Controller (PMIC Arbiter)
+
+The SPMI PMIC Arbiter is found on the Snapdragon 800 Series.  It is an SPMI
+controller with wrapping arbitration logic to allow for multiple on-chip
+devices to control a single SPMI master.
+
+See spmi.txt for the generic SPMI controller bindings.
+
+Required properties:
+- compatible : should be "qcom,spmi-pmic-arb".
+- reg-names  : should be "core", "intr", "cnfg"
+- reg : offset and length of the PMIC Arbiter Core register map.
+- reg : offset and length of the PMIC Arbiter Interrupt controller register map.
+- reg : offset and length of the PMIC Arbiter Configuration register map.
+- #address-cells : must be set to 1
+- #size-cells : must be set to 0
+
+Child nodes:
+
+Zero or more child nodes must be specified as per the spmi.txt document.
+
+Example:
+
+	qcom,spmi@fc4c0000 {
+		compatible = "qcom,spmi-pmic-arb";
+		reg-names = "core", "intr", "cnfg";
+		reg = <0xfc4cf000 0x1000>,
+		      <0Xfc4cb000 0x1000>,
+		      <0Xfc4ca000 0x1000>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		qcom,pm8841@4 {
+			reg = <0x4>;
+		};
+	};
-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

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

* [PATCH RFC v2 5/5] spmi: document the PMIC arbiter SPMI bindings
@ 2013-08-09 20:37   ` Josh Cartwright
  0 siblings, 0 replies; 37+ messages in thread
From: Josh Cartwright @ 2013-08-09 20:37 UTC (permalink / raw)
  To: linux-arm-kernel

Signed-off-by: Josh Cartwright <joshc@codeaurora.org>
---
 .../bindings/spmi/qcom,spmi-pmic-arb.txt           | 36 ++++++++++++++++++++++
 1 file changed, 36 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.txt

diff --git a/Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.txt b/Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.txt
new file mode 100644
index 0000000..3965236
--- /dev/null
+++ b/Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.txt
@@ -0,0 +1,36 @@
+Qualcomm SPMI Controller (PMIC Arbiter)
+
+The SPMI PMIC Arbiter is found on the Snapdragon 800 Series.  It is an SPMI
+controller with wrapping arbitration logic to allow for multiple on-chip
+devices to control a single SPMI master.
+
+See spmi.txt for the generic SPMI controller bindings.
+
+Required properties:
+- compatible : should be "qcom,spmi-pmic-arb".
+- reg-names  : should be "core", "intr", "cnfg"
+- reg : offset and length of the PMIC Arbiter Core register map.
+- reg : offset and length of the PMIC Arbiter Interrupt controller register map.
+- reg : offset and length of the PMIC Arbiter Configuration register map.
+- #address-cells : must be set to 1
+- #size-cells : must be set to 0
+
+Child nodes:
+
+Zero or more child nodes must be specified as per the spmi.txt document.
+
+Example:
+
+	qcom,spmi at fc4c0000 {
+		compatible = "qcom,spmi-pmic-arb";
+		reg-names = "core", "intr", "cnfg";
+		reg = <0xfc4cf000 0x1000>,
+		      <0Xfc4cb000 0x1000>,
+		      <0Xfc4ca000 0x1000>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		qcom,pm8841 at 4 {
+			reg = <0x4>;
+		};
+	};
-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

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

* [PATCH RFC v2 2/5] spmi: Linux driver framework for SPMI
  2013-08-22 20:18 ` Josh Cartwright
@ 2013-08-09 20:37   ` Josh Cartwright
  -1 siblings, 0 replies; 37+ messages in thread
From: Josh Cartwright @ 2013-08-09 20:37 UTC (permalink / raw)
  To: Grant Likely, Rob Herring, Greg Kroah-Hartman
  Cc: linux-kernel, linux-arm-kernel, linux-arm-msm, Sagar Dharia,
	Gilad Avidov, Michael Bohan

From: Kenneth Heitke <kheitke@codeaurora.org>

System Power Management Interface (SPMI) is a specification
developed by the MIPI (Mobile Industry Process Interface) Alliance
optimized for the real time control of Power Management ICs (PMIC).

SPMI is a two-wire serial interface that supports up to 4 master
devices and up to 16 logical slaves.

The framework supports message APIs, multiple busses (1 controller
per bus) and multiple clients/slave devices per controller.

Signed-off-by: Kenneth Heitke <kheitke@codeaurora.org>
Signed-off-by: Michael Bohan <mbohan@codeaurora.org>
Signed-off-by: Josh Cartwright <joshc@codeaurora.org>
---
 drivers/Kconfig                 |   2 +
 drivers/Makefile                |   1 +
 drivers/spmi/Kconfig            |   9 +
 drivers/spmi/Makefile           |   4 +
 drivers/spmi/spmi-dbgfs.c       | 580 ++++++++++++++++++++++++++++++++++++++++
 drivers/spmi/spmi-dbgfs.h       |  25 ++
 drivers/spmi/spmi.c             | 491 ++++++++++++++++++++++++++++++++++
 include/linux/mod_devicetable.h |   8 +
 include/linux/spmi.h            | 355 ++++++++++++++++++++++++
 9 files changed, 1475 insertions(+)
 create mode 100644 drivers/spmi/Kconfig
 create mode 100644 drivers/spmi/Makefile
 create mode 100644 drivers/spmi/spmi-dbgfs.c
 create mode 100644 drivers/spmi/spmi-dbgfs.h
 create mode 100644 drivers/spmi/spmi.c
 create mode 100644 include/linux/spmi.h

diff --git a/drivers/Kconfig b/drivers/Kconfig
index aa43b91..2b35420 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -52,6 +52,8 @@ source "drivers/i2c/Kconfig"
 
 source "drivers/spi/Kconfig"
 
+source "drivers/spmi/Kconfig"
+
 source "drivers/hsi/Kconfig"
 
 source "drivers/pps/Kconfig"
diff --git a/drivers/Makefile b/drivers/Makefile
index ab93de8..5c250df 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -64,6 +64,7 @@ obj-$(CONFIG_ATA)		+= ata/
 obj-$(CONFIG_TARGET_CORE)	+= target/
 obj-$(CONFIG_MTD)		+= mtd/
 obj-$(CONFIG_SPI)		+= spi/
+obj-$(CONFIG_SPMI)		+= spmi/
 obj-y				+= hsi/
 obj-y				+= net/
 obj-$(CONFIG_ATM)		+= atm/
diff --git a/drivers/spmi/Kconfig b/drivers/spmi/Kconfig
new file mode 100644
index 0000000..a03835f
--- /dev/null
+++ b/drivers/spmi/Kconfig
@@ -0,0 +1,9 @@
+#
+# SPMI driver configuration
+#
+menuconfig SPMI
+	bool "SPMI support"
+	help
+	  SPMI (System Power Management Interface) is a two-wire
+	  serial interface between baseband and application processors
+	  and Power Management Integrated Circuits (PMIC).
diff --git a/drivers/spmi/Makefile b/drivers/spmi/Makefile
new file mode 100644
index 0000000..9d78ec9
--- /dev/null
+++ b/drivers/spmi/Makefile
@@ -0,0 +1,4 @@
+#
+# Makefile for kernel SPMI framework.
+#
+obj-$(CONFIG_SPMI)	+= spmi.o spmi-dbgfs.o
diff --git a/drivers/spmi/spmi-dbgfs.c b/drivers/spmi/spmi-dbgfs.c
new file mode 100644
index 0000000..8b6f0e3
--- /dev/null
+++ b/drivers/spmi/spmi-dbgfs.c
@@ -0,0 +1,580 @@
+/* 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.
+ */
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/sizes.h>
+#include <linux/slab.h>
+#include <linux/uaccess.h>
+#include <linux/spinlock.h>
+#include <linux/string.h>
+#include <linux/debugfs.h>
+#include <linux/spmi.h>
+#include <linux/ctype.h>
+#include "spmi-dbgfs.h"
+
+#define ADDR_LEN	 6	/* 5 byte address + 1 space character */
+#define CHARS_PER_ITEM	 3	/* Format is 'XX ' */
+#define ITEMS_PER_LINE	16	/* 16 data items per line */
+#define MAX_LINE_LENGTH	(ADDR_LEN + (ITEMS_PER_LINE * CHARS_PER_ITEM) + 1)
+
+#define MAX_REG_PER_TRANSACTION	(8)
+
+static const mode_t DFS_MODE = S_IRUSR | S_IWUSR;
+
+/* Log buffer */
+struct spmi_log_buffer {
+	u32 rpos;	/* Current 'read' position in buffer */
+	u32 wpos;	/* Current 'write' position in buffer */
+	u32 len;	/* Length of the buffer */
+	char data[0];	/* Log buffer */
+};
+
+/* SPMI transaction parameters */
+struct spmi_trans {
+	u32 cnt;	/* Number of bytes to read */
+	u32 addr;	/* 20-bit address: SID + PID + Register offset */
+	u32 offset;	/* Offset of last read data */
+	bool raw_data;	/* Set to true for raw data dump */
+	struct spmi_device *sdev;
+	struct spmi_log_buffer *log; /* log buffer */
+};
+
+static char dbgfs_help[] =
+	"SPMI Debug-FS support\n"
+	"\n"
+	"Hierarchy schema:\n"
+	"/sys/kernel/debug/spmi\n"
+	"       /help                -- Static help text\n"
+	"       /spmi-0              -- Directory for SPMI bus 0\n"
+	"       /spmi-0/0-1          -- Directory for SPMI device '0-1'\n"
+	"       /spmi-0/0-1/address  -- Starting register for reads or writes\n"
+	"       /spmi-0/0-1/count    -- Number of registers to read (only used for reads)\n"
+	"       /spmi-0/0-1/data     -- Initiates the SPMI read (formatted output)\n"
+	"       /spmi-0/0-1/data_raw -- Initiates the SPMI raw read or write\n"
+	"       /spmi-n              -- Directory for SPMI bus n\n"
+	"\n"
+	"To perform SPMI read or write transactions, you need to first write the\n"
+	"address of the slave device register to the 'address' file.  For read\n"
+	"transactions, the number of bytes to be read needs to be written to the\n"
+	"'count' file.\n"
+	"\n"
+	"The 'address' file specifies the 20-bit address of a slave device register.\n"
+	"The upper 4 bits 'address[19..16]' specify the slave identifier (SID) for\n"
+	"the slave device.  The lower 16 bits specify the slave register address.\n"
+	"\n"
+	"Reading from the 'data' file will initiate a SPMI read transaction starting\n"
+	"from slave register 'address' for 'count' number of bytes.\n"
+	"\n"
+	"Writing to the 'data' file will initiate a SPMI write transaction starting\n"
+	"from slave register 'address'.  The number of registers written to will\n"
+	"match the number of bytes written to the 'data' file.\n"
+	"\n"
+	"Example: Read 4 bytes starting at register address 0x1234 for SID 2\n"
+	"\n"
+	"echo 0x21234 > address\n"
+	"echo 4 > count\n"
+	"cat data\n"
+	"\n"
+	"Example: Write 3 bytes starting at register address 0x1008 for SID 1\n"
+	"\n"
+	"echo 0x11008 > address\n"
+	"echo 0x01 0x02 0x03 > data\n"
+	"\n"
+	"Note that the count file is not used for writes.  Since 3 bytes are\n"
+	"written to the 'data' file, then 3 bytes will be written across the\n"
+	"SPMI bus.\n\n";
+
+static struct debugfs_blob_wrapper spmi_debug_help = {
+	.data	= dbgfs_help,
+	.size	= sizeof(dbgfs_help),
+};
+
+static struct dentry *spmi_debug_root;
+
+static int spmi_device_dfs_open(struct spmi_device *sdev, struct file *file)
+{
+	struct spmi_log_buffer *log;
+	struct spmi_trans *trans;
+
+	size_t logbufsize = SZ_4K;
+
+	/* Per file "transaction" data */
+	trans = kzalloc(sizeof(*trans), GFP_KERNEL);
+	if (!trans)
+		return -ENOMEM;
+
+	log = kzalloc(logbufsize, GFP_KERNEL);
+	if (!log) {
+		kfree(trans);
+		return -ENOMEM;
+	}
+
+	log->rpos = 0;
+	log->wpos = 0;
+	log->len = logbufsize - sizeof(*log);
+
+	trans->log = log;
+	trans->cnt = sdev->dfs_cnt;
+	trans->addr = sdev->dfs_addr;
+	trans->sdev = sdev;
+	trans->offset = trans->addr;
+
+	file->private_data = trans;
+	return 0;
+}
+
+static int spmi_device_dfs_data_open(struct inode *inode, struct file *file)
+{
+	struct spmi_device *sdev = inode->i_private;
+	return spmi_device_dfs_open(sdev, file);
+}
+
+static int spmi_device_dfs_raw_data_open(struct inode *inode, struct file *file)
+{
+	struct spmi_device *sdev = inode->i_private;
+	struct spmi_trans *trans;
+	int rc;
+
+	rc = spmi_device_dfs_open(sdev, file);
+	trans = file->private_data;
+	trans->raw_data = true;
+	return rc;
+}
+
+static int spmi_device_dfs_close(struct inode *inode, struct file *file)
+{
+	struct spmi_trans *trans = file->private_data;
+	kfree(trans->log);
+	kfree(trans);
+	return 0;
+}
+
+/**
+ * spmi_read_data: reads data across the SPMI bus
+ * @ctrl: The SPMI controller
+ * @buf: buffer to store the data read.
+ * @offset: SPMI address offset to start reading from.
+ * @cnt: The number of bytes to read.
+ *
+ * Returns 0 on success, otherwise returns error code from SPMI driver.
+ */
+static int
+spmi_read_data(struct spmi_device *sdev, uint8_t *buf, int offset, int cnt)
+{
+	int ret = 0;
+	int len;
+	uint16_t addr;
+
+	while (cnt > 0) {
+		addr = offset & 0xFFFF;
+		len = min(cnt, MAX_REG_PER_TRANSACTION);
+
+		ret = spmi_ext_register_readl(sdev, addr, buf, len);
+		if (ret < 0) {
+			dev_err(&sdev->dev,
+				"SPMI read failed, err = %d\n", ret);
+			goto done;
+		}
+
+		cnt -= len;
+		buf += len;
+		offset += len;
+	}
+
+done:
+	return ret;
+}
+
+/**
+ * spmi_write_data: writes data across the SPMI bus
+ * @ctrl: The SPMI controller
+ * @buf: data to be written.
+ * @offset: SPMI address offset to start writing to.
+ * @cnt: The number of bytes to write.
+ *
+ * Returns 0 on success, otherwise returns error code from SPMI driver.
+ */
+static int
+spmi_write_data(struct spmi_device *sdev, uint8_t *buf, int offset, int cnt)
+{
+	int ret = 0;
+	int len;
+	uint16_t addr;
+
+	while (cnt > 0) {
+		addr = offset & 0xFFFF;
+		len = min(cnt, MAX_REG_PER_TRANSACTION);
+
+		ret = spmi_ext_register_writel(sdev, addr, buf, len);
+		if (ret < 0) {
+			dev_err(&sdev->dev,
+				"SPMI write failed, err = %d\n", ret);
+			goto done;
+		}
+
+		cnt -= len;
+		buf += len;
+		offset += len;
+	}
+
+done:
+	return ret;
+}
+
+/**
+ * print_to_log: format a string and place into the log buffer
+ * @log: The log buffer to place the result into.
+ * @fmt: The format string to use.
+ * @...: The arguments for the format string.
+ *
+ * The return value is the number of characters written to @log buffer
+ * not including the trailing '\0'.
+ */
+static int print_to_log(struct spmi_log_buffer *log, const char *fmt, ...)
+{
+	va_list args;
+	int cnt;
+	char *buf = &log->data[log->wpos];
+	size_t size = log->len - log->wpos;
+
+	va_start(args, fmt);
+	cnt = vscnprintf(buf, size, fmt, args);
+	va_end(args);
+
+	log->wpos += cnt;
+	return cnt;
+}
+
+/**
+ * write_next_line_to_log: Writes a single "line" of data into the log buffer
+ * @trans: Pointer to SPMI transaction data.
+ * @offset: SPMI address offset to start reading from.
+ * @pcnt: Pointer to 'cnt' variable.  Indicates the number of bytes to read.
+ *
+ * The 'offset' is a 20-bits SPMI address which includes a 4-bit slave id (SID),
+ * an 8-bit peripheral id (PID), and an 8-bit peripheral register address.
+ *
+ * On a successful read, the pcnt is decremented by the number of data
+ * bytes read across the SPMI bus.  When the cnt reaches 0, all requested
+ * bytes have been read.
+ */
+static int
+write_next_line_to_log(struct spmi_trans *trans, int offset, size_t *pcnt)
+{
+	int i, j;
+	u8  data[ITEMS_PER_LINE];
+	struct spmi_log_buffer *log = trans->log;
+
+	int cnt = 0;
+	int padding = offset % ITEMS_PER_LINE;
+	int items_to_read = min(ARRAY_SIZE(data) - padding, *pcnt);
+	int items_to_log = min(ITEMS_PER_LINE, padding + items_to_read);
+
+	/* Buffer needs enough space for an entire line */
+	if ((log->len - log->wpos) < MAX_LINE_LENGTH)
+		goto done;
+
+	/* Read the desired number of "items" */
+	if (spmi_read_data(trans->sdev, data, offset, items_to_read))
+		goto done;
+
+	*pcnt -= items_to_read;
+
+	/* Each line starts with the aligned offset (20-bit address) */
+	cnt = print_to_log(log, "%5.5X ", offset & 0xffff0);
+	if (cnt == 0)
+		goto done;
+
+	/* If the offset is unaligned, add padding to right justify items */
+	for (i = 0; i < padding; ++i) {
+		cnt = print_to_log(log, "-- ");
+		if (cnt == 0)
+			goto done;
+	}
+
+	/* Log the data items */
+	for (j = 0; i < items_to_log; ++i, ++j) {
+		cnt = print_to_log(log, "%2.2X ", data[j]);
+		if (cnt == 0)
+			goto done;
+	}
+
+	/* If the last character was a space, then replace it with a newline */
+	if (log->wpos > 0 && log->data[log->wpos - 1] == ' ')
+		log->data[log->wpos - 1] = '\n';
+
+done:
+	return cnt;
+}
+
+/**
+ * write_raw_data_to_log: Writes a single "line" of data into the log buffer
+ * @trans: Pointer to SPMI transaction data.
+ * @offset: SPMI address offset to start reading from.
+ * @pcnt: Pointer to 'cnt' variable.  Indicates the number of bytes to read.
+ *
+ * The 'offset' is a 20-bits SPMI address which includes a 4-bit slave id (SID),
+ * an 8-bit peripheral id (PID), and an 8-bit peripheral register address.
+ *
+ * On a successful read, the pcnt is decremented by the number of data
+ * bytes read across the SPMI bus.  When the cnt reaches 0, all requested
+ * bytes have been read.
+ */
+static int
+write_raw_data_to_log(struct spmi_trans *trans, int offset, size_t *pcnt)
+{
+	u8  data[16];
+	struct spmi_log_buffer *log = trans->log;
+
+	int i;
+	int cnt = 0;
+	int items_to_read = min(ARRAY_SIZE(data), *pcnt);
+
+	/* Buffer needs enough space for an entire line */
+	if ((log->len - log->wpos) < 80)
+		goto done;
+
+	/* Read the desired number of "items" */
+	if (spmi_read_data(trans->sdev, data, offset, items_to_read))
+		goto done;
+
+	*pcnt -= items_to_read;
+
+	/* Log the data items */
+	for (i = 0; i < items_to_read; ++i) {
+		cnt = print_to_log(log, "0x%2.2X ", data[i]);
+		if (cnt == 0)
+			goto done;
+	}
+
+	/* If the last character was a space, then replace it with a newline */
+	if (log->wpos > 0 && log->data[log->wpos - 1] == ' ')
+		log->data[log->wpos - 1] = '\n';
+
+done:
+	return cnt;
+}
+
+/**
+ * get_log_data - reads data across the SPMI bus and saves to the log buffer
+ * @trans: Pointer to SPMI transaction data.
+ *
+ * Returns the number of "items" read or SPMI error code for read failures.
+ */
+static int get_log_data(struct spmi_trans *trans)
+{
+	int cnt;
+	int last_cnt;
+	int items_read;
+	int total_items_read = 0;
+	u32 offset = trans->offset;
+	size_t item_cnt = trans->cnt;
+	struct spmi_log_buffer *log = trans->log;
+	int (*write_to_log)(struct spmi_trans *, int, size_t *);
+
+	if (item_cnt == 0)
+		return 0;
+
+	if (trans->raw_data)
+		write_to_log = write_raw_data_to_log;
+	else
+		write_to_log = write_next_line_to_log;
+
+	/* Reset the log buffer 'pointers' */
+	log->wpos = log->rpos = 0;
+
+	/* Keep reading data until the log is full */
+	do {
+		last_cnt = item_cnt;
+		cnt = write_to_log(trans, offset, &item_cnt);
+		items_read = last_cnt - item_cnt;
+		offset += items_read;
+		total_items_read += items_read;
+	} while (cnt && item_cnt > 0);
+
+	/* Adjust the transaction offset and count */
+	trans->cnt = item_cnt;
+	trans->offset += total_items_read;
+
+	return total_items_read;
+}
+
+static ssize_t spmi_device_dfs_reg_write(struct file *file,
+					 const char __user *buf,
+					 size_t count, loff_t *ppos)
+{
+	int bytes_read;
+	int data;
+	int pos = 0;
+	int cnt = 0;
+	u8  *values;
+	size_t ret = 0;
+
+	struct spmi_trans *trans = file->private_data;
+	struct spmi_device *sdev = trans->sdev;
+	u32 offset = trans->offset;
+
+	/* Make a copy of the user data */
+	char *kbuf = kmalloc(count + 1, GFP_KERNEL);
+	if (!kbuf)
+		return -ENOMEM;
+
+	if (copy_from_user(kbuf, buf, count)) {
+		ret = -EFAULT;
+		goto free_buf;
+	}
+
+	*ppos += count;
+	kbuf[count] = '\0';
+
+	/* Override the text buffer with the raw data */
+	values = kbuf;
+
+	/* Parse the data in the buffer.  It should be a string of numbers */
+	while (sscanf(kbuf + pos, "%i%n", &data, &bytes_read) == 1) {
+		pos += bytes_read;
+		values[cnt++] = data & 0xff;
+	}
+
+	if (!cnt)
+		goto free_buf;
+
+	/* Perform the SPMI write(s) */
+	ret = spmi_write_data(trans->sdev, values, offset, cnt);
+
+	if (ret) {
+		dev_err(&sdev->dev, "SPMI write failed, err = %zu\n", ret);
+	} else {
+		ret = count;
+		trans->offset += cnt;
+	}
+
+free_buf:
+	kfree(kbuf);
+	return ret;
+}
+
+static ssize_t spmi_device_dfs_reg_read(struct file *file, char __user *buf,
+					size_t count, loff_t *ppos)
+{
+	struct spmi_trans *trans = file->private_data;
+	struct spmi_device *sdev = trans->sdev;
+	struct spmi_log_buffer *log = trans->log;
+	size_t len;
+
+	/* Is the the log buffer empty */
+	if (log->rpos >= log->wpos) {
+		if (get_log_data(trans) <= 0)
+			return 0;
+	}
+
+	len = min(count, (size_t) log->wpos - log->rpos);
+
+	if (copy_to_user(buf, &log->data[log->rpos], len))
+		return -EFAULT;
+
+	*ppos += len;
+	log->rpos += len;
+	return len;
+}
+
+static const struct file_operations spmi_dfs_reg_fops = {
+	.open		= spmi_device_dfs_data_open,
+	.release	= spmi_device_dfs_close,
+	.read		= spmi_device_dfs_reg_read,
+	.write		= spmi_device_dfs_reg_write,
+};
+
+static const struct file_operations spmi_dfs_raw_data_fops = {
+	.open		= spmi_device_dfs_raw_data_open,
+	.release	= spmi_device_dfs_close,
+	.read		= spmi_device_dfs_reg_read,
+	.write		= spmi_device_dfs_reg_write,
+};
+
+void spmi_dfs_controller_add(struct spmi_controller *ctrl)
+{
+	ctrl->dfs_dir = debugfs_create_dir(dev_name(&ctrl->dev),
+					   spmi_debug_root);
+	WARN_ON(!ctrl->dfs_dir);
+
+	dev_dbg(&ctrl->dev, "adding debug entries for spmi controller\n");
+}
+
+void spmi_dfs_controller_remove(struct spmi_controller *ctrl)
+{
+	debugfs_remove_recursive(ctrl->dfs_dir);
+}
+
+void spmi_dfs_device_add(struct spmi_device *sdev)
+{
+	struct dentry *file;
+
+	dev_dbg(&sdev->dev, "adding debugfs entries for spmi device\n");
+
+	sdev->dfs_dir = debugfs_create_dir(dev_name(&sdev->dev),
+					   sdev->ctrl->dfs_dir);
+	if (WARN_ON(!sdev->dfs_dir))
+		return;
+
+	sdev->dfs_cnt  = 1;
+
+	file = debugfs_create_u32("count", DFS_MODE, sdev->dfs_dir,
+				  &sdev->dfs_cnt);
+	if (WARN_ON(!file))
+		goto err_remove_fs;
+
+	file = debugfs_create_x32("address", DFS_MODE, sdev->dfs_dir,
+				  &sdev->dfs_addr);
+	if (WARN_ON(!file))
+		goto err_remove_fs;
+
+	file = debugfs_create_file("data", DFS_MODE, sdev->dfs_dir, sdev,
+				   &spmi_dfs_reg_fops);
+	if (WARN_ON(!file))
+		goto err_remove_fs;
+
+	file = debugfs_create_file("data_raw", DFS_MODE, sdev->dfs_dir,
+				   sdev, &spmi_dfs_raw_data_fops);
+	if (WARN_ON(!file))
+		goto err_remove_fs;
+
+	return;
+
+err_remove_fs:
+	debugfs_remove_recursive(sdev->dfs_dir);
+}
+
+void spmi_dfs_device_remove(struct spmi_device *sdev)
+{
+	dev_dbg(&sdev->dev, "Deleting device\n");
+	debugfs_remove_recursive(sdev->dfs_dir);
+}
+
+void __exit spmi_dfs_exit(void)
+{
+	debugfs_remove_recursive(spmi_debug_root);
+}
+
+void __init spmi_dfs_init(void)
+{
+	struct dentry *help;
+
+	spmi_debug_root = debugfs_create_dir("spmi", NULL);
+
+	help = debugfs_create_blob("help", S_IRUGO, spmi_debug_root,
+				   &spmi_debug_help);
+
+	WARN_ON(!spmi_debug_root || !help);
+}
+
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/spmi/spmi-dbgfs.h b/drivers/spmi/spmi-dbgfs.h
new file mode 100644
index 0000000..d3162fb
--- /dev/null
+++ b/drivers/spmi/spmi-dbgfs.h
@@ -0,0 +1,25 @@
+/* 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 _SPMI_DBGFS_H
+#define _SPMI_DBGFS_H
+
+struct spmi_controller;
+struct spmi_device;
+
+void __init spmi_dfs_init(void);
+void __exit spmi_dfs_exit(void);
+void spmi_dfs_controller_add(struct spmi_controller *ctrl);
+void spmi_dfs_controller_remove(struct spmi_controller *ctrl);
+void spmi_dfs_device_add(struct spmi_device *sdev);
+void spmi_dfs_device_remove(struct spmi_device *sdev);
+
+#endif /* _SPMI_DBGFS_H */
diff --git a/drivers/spmi/spmi.c b/drivers/spmi/spmi.c
new file mode 100644
index 0000000..c3da65e
--- /dev/null
+++ b/drivers/spmi/spmi.c
@@ -0,0 +1,491 @@
+/* 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.
+ */
+#include <linux/kernel.h>
+#include <linux/errno.h>
+#include <linux/idr.h>
+#include <linux/slab.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/spmi.h>
+#include <linux/module.h>
+#include <linux/pm_runtime.h>
+
+#include "spmi-dbgfs.h"
+
+static DEFINE_MUTEX(ctrl_idr_lock);
+static DEFINE_IDR(ctrl_idr);
+
+static void spmi_dev_release(struct device *dev)
+{
+	struct spmi_device *sdev = to_spmi_device(dev);
+	spmi_dfs_device_remove(sdev);
+	kfree(sdev);
+}
+
+static struct device_type spmi_dev_type = {
+	.release	= spmi_dev_release,
+};
+
+static void spmi_ctrl_release(struct device *dev)
+{
+	struct spmi_controller *ctrl = to_spmi_controller(dev);
+	spmi_dfs_controller_remove(ctrl);
+	mutex_lock(&ctrl_idr_lock);
+	idr_remove(&ctrl_idr, ctrl->nr);
+	mutex_unlock(&ctrl_idr_lock);
+	kfree(ctrl);
+}
+
+static struct device_type spmi_ctrl_type = {
+	.release	= spmi_ctrl_release,
+};
+
+#ifdef CONFIG_PM_SLEEP
+static int spmi_pm_suspend(struct device *dev)
+{
+	const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
+
+	if (pm)
+		return pm_generic_suspend(dev);
+	else
+		return 0;
+}
+
+static int spmi_pm_resume(struct device *dev)
+{
+	const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
+
+	if (pm)
+		return pm_generic_resume(dev);
+	else
+		return 0;
+}
+#else
+#define spmi_pm_suspend		NULL
+#define spmi_pm_resume		NULL
+#endif
+
+static const struct dev_pm_ops spmi_pm_ops = {
+	.suspend	= spmi_pm_suspend,
+	.resume		= spmi_pm_resume,
+	SET_RUNTIME_PM_OPS(
+		pm_generic_suspend,
+		pm_generic_resume,
+		pm_generic_runtime_idle
+	)
+};
+
+static int spmi_device_match(struct device *dev, struct device_driver *drv)
+{
+	if (of_driver_match_device(dev, drv))
+		return 1;
+
+	if (drv->name)
+		return strncmp(dev_name(dev), drv->name,
+			       SPMI_NAME_SIZE) == 0;
+
+	return 0;
+}
+
+int spmi_device_add(struct spmi_device *sdev)
+{
+	struct spmi_controller *ctrl = sdev->ctrl;
+	int err;
+
+	dev_set_name(&sdev->dev, "%d-%02x", ctrl->nr, sdev->usid);
+
+	err = device_add(&sdev->dev);
+	if (err < 0) {
+		dev_err(&sdev->dev, "Can't add %s, status %d\n",
+			dev_name(&sdev->dev), err);
+		goto err_device_add;
+	}
+
+	spmi_dfs_device_add(sdev);
+
+	dev_dbg(&sdev->dev, "device %s registered\n", dev_name(&sdev->dev));
+
+err_device_add:
+	return err;
+}
+EXPORT_SYMBOL_GPL(spmi_device_add);
+
+void spmi_device_remove(struct spmi_device *sdev)
+{
+	device_unregister(&sdev->dev);
+}
+EXPORT_SYMBOL_GPL(spmi_device_remove);
+
+static inline int
+spmi_cmd(struct spmi_controller *ctrl, u8 opcode, u8 sid)
+{
+	if (!ctrl || !ctrl->cmd || ctrl->dev.type != &spmi_ctrl_type)
+		return -EINVAL;
+
+	return ctrl->cmd(ctrl, opcode, sid);
+}
+
+static inline int spmi_read_cmd(struct spmi_controller *ctrl,
+				u8 opcode, u8 sid, u16 addr, u8 bc, u8 *buf)
+{
+	if (!ctrl || !ctrl->read_cmd || ctrl->dev.type != &spmi_ctrl_type)
+		return -EINVAL;
+
+	return ctrl->read_cmd(ctrl, opcode, sid, addr, bc, buf);
+}
+
+static inline int spmi_write_cmd(struct spmi_controller *ctrl,
+				 u8 opcode, u8 sid, u16 addr, u8 bc, u8 *buf)
+{
+	if (!ctrl || !ctrl->write_cmd || ctrl->dev.type != &spmi_ctrl_type)
+		return -EINVAL;
+
+	return ctrl->write_cmd(ctrl, opcode, sid, addr, bc, buf);
+}
+
+/*
+ * register read/write: 5-bit address, 1 byte of data
+ * extended register read/write: 8-bit address, up to 16 bytes of data
+ * extended register read/write long: 16-bit address, up to 8 bytes of data
+ */
+
+int spmi_register_read(struct spmi_device *sdev, u8 addr, u8 *buf)
+{
+	/* 5-bit register address */
+	if (addr > 0x1F)
+		return -EINVAL;
+
+	return spmi_read_cmd(sdev->ctrl, SPMI_CMD_READ, sdev->usid, addr, 0,
+			     buf);
+}
+EXPORT_SYMBOL_GPL(spmi_register_read);
+
+int spmi_ext_register_read(struct spmi_device *sdev, u8 addr, u8 *buf,
+			   size_t len)
+{
+	/* 8-bit register address, up to 16 bytes */
+	if (len == 0 || len > 16)
+		return -EINVAL;
+
+	return spmi_read_cmd(sdev->ctrl, SPMI_CMD_EXT_READ, sdev->usid, addr,
+			     len - 1, buf);
+}
+EXPORT_SYMBOL_GPL(spmi_ext_register_read);
+
+int spmi_ext_register_readl(struct spmi_device *sdev, u16 addr, u8 *buf,
+			    size_t len)
+{
+	/* 16-bit register address, up to 8 bytes */
+	if (len == 0 || len > 8)
+		return -EINVAL;
+
+	return spmi_read_cmd(sdev->ctrl, SPMI_CMD_EXT_READL, sdev->usid, addr,
+			     len - 1, buf);
+}
+EXPORT_SYMBOL_GPL(spmi_ext_register_readl);
+
+int spmi_register_write(struct spmi_device *sdev, u8 addr, u8 *buf)
+{
+	/* 5-bit register address */
+	if (addr > 0x1F)
+		return -EINVAL;
+
+	return spmi_write_cmd(sdev->ctrl, SPMI_CMD_WRITE, sdev->usid, addr, 0,
+			      buf);
+}
+EXPORT_SYMBOL_GPL(spmi_register_write);
+
+int spmi_register_zero_write(struct spmi_device *sdev, u8 data)
+{
+	return spmi_write_cmd(sdev->ctrl, SPMI_CMD_ZERO_WRITE, sdev->usid, 0,
+			      0, &data);
+}
+EXPORT_SYMBOL_GPL(spmi_register_zero_write);
+
+int spmi_ext_register_write(struct spmi_device *sdev, u8 addr, u8 *buf,
+			    size_t len)
+{
+	/* 8-bit register address, up to 16 bytes */
+	if (len == 0 || len > 16)
+		return -EINVAL;
+
+	return spmi_write_cmd(sdev->ctrl, SPMI_CMD_EXT_WRITE, sdev->usid, addr,
+			      len - 1, buf);
+}
+EXPORT_SYMBOL_GPL(spmi_ext_register_write);
+
+int spmi_ext_register_writel(struct spmi_device *sdev, u16 addr, u8 *buf,
+			     size_t len)
+{
+	/* 4-bit Slave Identifier, 16-bit register address, up to 8 bytes */
+	if (len == 0 || len > 8)
+		return -EINVAL;
+
+	return spmi_write_cmd(sdev->ctrl, SPMI_CMD_EXT_WRITEL, sdev->usid,
+			      addr, len - 1, buf);
+}
+EXPORT_SYMBOL_GPL(spmi_ext_register_writel);
+
+int spmi_command_reset(struct spmi_device *sdev)
+{
+	return spmi_cmd(sdev->ctrl, SPMI_CMD_RESET, sdev->usid);
+}
+EXPORT_SYMBOL_GPL(spmi_command_reset);
+
+int spmi_command_sleep(struct spmi_device *sdev)
+{
+	return spmi_cmd(sdev->ctrl, SPMI_CMD_SLEEP, sdev->usid);
+}
+EXPORT_SYMBOL_GPL(spmi_command_sleep);
+
+int spmi_command_wakeup(struct spmi_device *sdev)
+{
+	return spmi_cmd(sdev->ctrl, SPMI_CMD_WAKEUP, sdev->usid);
+}
+EXPORT_SYMBOL_GPL(spmi_command_wakeup);
+
+int spmi_command_shutdown(struct spmi_device *sdev)
+{
+	return spmi_cmd(sdev->ctrl, SPMI_CMD_SHUTDOWN, sdev->usid);
+}
+EXPORT_SYMBOL_GPL(spmi_command_shutdown);
+
+static int spmi_drv_probe(struct device *dev)
+{
+	const struct spmi_driver *sdrv = to_spmi_driver(dev->driver);
+	int err = 0;
+
+	if (sdrv->probe)
+		err = sdrv->probe(to_spmi_device(dev));
+
+	return err;
+}
+
+static int spmi_drv_remove(struct device *dev)
+{
+	const struct spmi_driver *sdrv = to_spmi_driver(dev->driver);
+	int err = 0;
+
+	if (sdrv->remove)
+		err = sdrv->remove(to_spmi_device(dev));
+
+	return err;
+}
+
+static void spmi_drv_shutdown(struct device *dev)
+{
+	const struct spmi_driver *sdrv = to_spmi_driver(dev->driver);
+
+	if (sdrv->shutdown)
+		sdrv->shutdown(to_spmi_device(dev));
+}
+
+static struct bus_type spmi_bus_type = {
+	.name		= "spmi",
+	.match		= spmi_device_match,
+	.pm		= &spmi_pm_ops,
+	.probe		= spmi_drv_probe,
+	.remove		= spmi_drv_remove,
+	.shutdown	= spmi_drv_shutdown,
+};
+
+struct spmi_device *spmi_device_alloc(struct spmi_controller *ctrl)
+{
+	struct spmi_device *sdev;
+
+	sdev = kzalloc(sizeof(*sdev), GFP_KERNEL);
+	if (!sdev)
+		return NULL;
+
+	sdev->ctrl = ctrl;
+	device_initialize(&sdev->dev);
+	sdev->dev.parent = &ctrl->dev;
+	sdev->dev.bus = &spmi_bus_type;
+	sdev->dev.type = &spmi_dev_type;
+	return sdev;
+}
+EXPORT_SYMBOL_GPL(spmi_device_alloc);
+
+struct spmi_controller *spmi_controller_alloc(struct device *parent,
+					      size_t size)
+{
+	struct spmi_controller *ctrl;
+	int id;
+
+	if (WARN_ON(!parent))
+		return NULL;
+
+	ctrl = kzalloc(sizeof(*ctrl) + size, GFP_KERNEL);
+	if (!ctrl)
+		return NULL;
+
+	device_initialize(&ctrl->dev);
+	ctrl->dev.type = &spmi_ctrl_type;
+	ctrl->dev.bus = &spmi_bus_type;
+	ctrl->dev.parent = parent;
+	ctrl->dev.of_node = parent->of_node;
+	spmi_controller_set_drvdata(ctrl, &ctrl[1]);
+
+	idr_preload(GFP_KERNEL);
+	mutex_lock(&ctrl_idr_lock);
+	id = idr_alloc(&ctrl_idr, ctrl, 0, 0, GFP_NOWAIT);
+	mutex_unlock(&ctrl_idr_lock);
+	idr_preload_end();
+
+	if (id < 0) {
+		dev_err(parent,
+			"unable to allocate SPMI controller identifier.\n");
+		spmi_controller_put(ctrl);
+		return NULL;
+	}
+
+	ctrl->nr = id;
+	dev_set_name(&ctrl->dev, "spmi-%d", id);
+
+	dev_dbg(&ctrl->dev, "allocated controller 0x%p id %d\n", ctrl, id);
+	return ctrl;
+}
+EXPORT_SYMBOL_GPL(spmi_controller_alloc);
+
+static int of_spmi_register_devices(struct spmi_controller *ctrl)
+{
+	struct device_node *node;
+	int err;
+
+	dev_dbg(&ctrl->dev, "of_spmi_register_devices\n");
+
+	if (!ctrl->dev.of_node)
+		return -ENODEV;
+
+	dev_dbg(&ctrl->dev, "looping through children\n");
+
+	for_each_available_child_of_node(ctrl->dev.of_node, node) {
+		struct spmi_device *sdev;
+		u32 usid;
+
+		dev_dbg(&ctrl->dev, "adding child %s\n", node->full_name);
+
+		err = of_property_read_u32(node, "reg", &usid);
+		if (err) {
+			dev_err(&ctrl->dev,
+				"node %s does not have 'reg' property\n",
+				node->full_name);
+			continue;
+		}
+
+		if (usid > 0xFF) {
+			dev_err(&ctrl->dev,
+				"invalid usid on node %s\n",
+				node->full_name);
+			continue;
+		}
+
+		dev_dbg(&ctrl->dev, "read usid %02x\n", usid);
+
+		sdev = spmi_device_alloc(ctrl);
+		if (!sdev)
+			continue;
+
+		sdev->dev.of_node = node;
+		sdev->usid = (u8) usid;
+
+		err = spmi_device_add(sdev);
+		if (err) {
+			dev_err(&sdev->dev,
+				"failure adding device. status %d\n", err);
+			spmi_device_put(sdev);
+			continue;
+		}
+	}
+
+	return 0;
+}
+
+int spmi_controller_add(struct spmi_controller *ctrl)
+{
+	int ret;
+
+	/* Can't register until after driver model init */
+	if (WARN_ON(!spmi_bus_type.p))
+		return -EAGAIN;
+
+	ret = device_add(&ctrl->dev);
+	if (ret)
+		return ret;
+
+	spmi_dfs_controller_add(ctrl);
+
+	if (IS_ENABLED(CONFIG_OF))
+		of_spmi_register_devices(ctrl);
+
+	dev_dbg(&ctrl->dev, "spmi-%d registered: dev:%p\n",
+		ctrl->nr, &ctrl->dev);
+
+	return 0;
+};
+EXPORT_SYMBOL_GPL(spmi_controller_add);
+
+/* Remove a device associated with a controller */
+static int spmi_ctrl_remove_device(struct device *dev, void *data)
+{
+	struct spmi_device *spmidev = to_spmi_device(dev);
+	if (dev->type == &spmi_dev_type)
+		spmi_device_remove(spmidev);
+	return 0;
+}
+
+/**
+ * spmi_controller_remove: Controller tear-down.
+ * @ctrl: controller to be removed.
+ *
+ * Controller added with the above API is torn down using this API.
+ */
+int spmi_controller_remove(struct spmi_controller *ctrl)
+{
+	int dummy;
+
+	if (!ctrl)
+		return -EINVAL;
+
+	dummy = device_for_each_child(&ctrl->dev, NULL,
+				      spmi_ctrl_remove_device);
+	device_unregister(&ctrl->dev);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(spmi_controller_remove);
+
+int spmi_driver_register(struct spmi_driver *drv)
+{
+	drv->driver.bus = &spmi_bus_type;
+	return driver_register(&drv->driver);
+}
+EXPORT_SYMBOL_GPL(spmi_driver_register);
+
+static void __exit spmi_exit(void)
+{
+	bus_unregister(&spmi_bus_type);
+	spmi_dfs_exit();
+}
+module_exit(spmi_exit);
+
+static int __init spmi_init(void)
+{
+	spmi_dfs_init();
+	return bus_register(&spmi_bus_type);
+}
+postcore_initcall(spmi_init);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("SPMI module");
+MODULE_ALIAS("platform:spmi");
diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
index 45e9214..677e474 100644
--- a/include/linux/mod_devicetable.h
+++ b/include/linux/mod_devicetable.h
@@ -432,6 +432,14 @@ struct spi_device_id {
 	kernel_ulong_t driver_data;	/* Data private to the driver */
 };
 
+#define SPMI_NAME_SIZE	32
+#define SPMI_MODULE_PREFIX "spmi:"
+
+struct spmi_device_id {
+	char name[SPMI_NAME_SIZE];
+	kernel_ulong_t driver_data;	/* Data private to the driver */
+};
+
 /* dmi */
 enum dmi_field {
 	DMI_NONE,
diff --git a/include/linux/spmi.h b/include/linux/spmi.h
new file mode 100644
index 0000000..8b9c232
--- /dev/null
+++ b/include/linux/spmi.h
@@ -0,0 +1,355 @@
+/* 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_SPMI_H
+#define _LINUX_SPMI_H
+
+#include <linux/types.h>
+#include <linux/device.h>
+#include <linux/mod_devicetable.h>
+
+/* Maximum slave identifier */
+#define SPMI_MAX_SLAVE_ID		16
+
+/* SPMI Commands */
+#define SPMI_CMD_EXT_WRITE		0x00
+#define SPMI_CMD_RESET			0x10
+#define SPMI_CMD_SLEEP			0x11
+#define SPMI_CMD_SHUTDOWN		0x12
+#define SPMI_CMD_WAKEUP			0x13
+#define SPMI_CMD_AUTHENTICATE		0x14
+#define SPMI_CMD_MSTR_READ		0x15
+#define SPMI_CMD_MSTR_WRITE		0x16
+#define SPMI_CMD_TRANSFER_BUS_OWNERSHIP	0x1A
+#define SPMI_CMD_DDB_MASTER_READ	0x1B
+#define SPMI_CMD_DDB_SLAVE_READ		0x1C
+#define SPMI_CMD_EXT_READ		0x20
+#define SPMI_CMD_EXT_WRITEL		0x30
+#define SPMI_CMD_EXT_READL		0x38
+#define SPMI_CMD_WRITE			0x40
+#define SPMI_CMD_READ			0x60
+#define SPMI_CMD_ZERO_WRITE		0x80
+
+/**
+ * Client/device handle (struct spmi_device):
+ *  This is the client/device handle returned when a SPMI device
+ *  is registered with a controller.
+ *  Pointer to this structure is used by client-driver as a handle.
+ *  @dev: Driver model representation of the device.
+ *  @ctrl: SPMI controller managing the bus hosting this device.
+ *  @usid: Unique Slave IDentifier.
+ *  @dfs_dir: dentry for this device in debugfs
+ *  @dfs_cnt: number of bytes to access for a debugfs operation
+ *  @dfs_addr: address to access for a debugfs operation
+ */
+struct spmi_device {
+	struct device		dev;
+	struct spmi_controller	*ctrl;
+	u8			usid;
+
+#if CONFIG_DEBUG_FS
+	struct dentry		*dfs_dir;
+	u32			dfs_cnt;
+	u32			dfs_addr;
+#endif
+};
+#define to_spmi_device(d) container_of(d, struct spmi_device, dev)
+
+static inline void *spmi_device_get_drvdata(const struct spmi_device *sdev)
+{
+	return dev_get_drvdata(&sdev->dev);
+}
+
+static inline void spmi_device_set_drvdata(struct spmi_device *sdev, void *data)
+{
+	dev_set_drvdata(&sdev->dev, data);
+}
+
+/**
+ * spmi_controller_alloc: Allocate a new SPMI controller
+ * @ctrl: associated controller
+ *
+ * Caller is responsible for either calling spmi_device_add() to add the
+ * newly allocated controller, or calling spmi_device_put() to discard it.
+ */
+struct spmi_device *spmi_device_alloc(struct spmi_controller *ctrl);
+
+static inline void spmi_device_put(struct spmi_device *sdev)
+{
+	if (sdev)
+		put_device(&sdev->dev);
+}
+
+/**
+ * spmi_device_add: add a device previously constructed via spmi_device_alloc()
+ * @sdev: spmi_device to be added
+ */
+int spmi_device_add(struct spmi_device *sdev);
+
+/**
+ * spmi_device_remove: remove a device
+ * @sdev: spmi_device to be removed
+ */
+void spmi_device_remove(struct spmi_device *sdev);
+
+/**
+ * struct spmi_controller: interface to the SPMI master controller
+ * @dev: Driver model representation of the device.
+ * @nr: board-specific number identifier for this controller/bus
+ * @cmd: sends a non-data command sequence on the SPMI bus.
+ * @read_cmd: sends a register read command sequence on the SPMI bus.
+ * @write_cmd: sends a register write command sequence on the SPMI bus.
+ * @dfs_dir: dentry for controller directory in debugfs
+ */
+struct spmi_controller {
+	struct device		dev;
+	unsigned int		nr;
+	int	(*cmd)(struct spmi_controller *ctrl, u8 opcode, u8 sid);
+	int	(*read_cmd)(struct spmi_controller *ctrl, u8 opcode,
+			    u8 sid, u16 addr, u8 bc, u8 *buf);
+	int	(*write_cmd)(struct spmi_controller *ctrl, u8 opcode,
+			     u8 sid, u16 addr, u8 bc, u8 *buf);
+#if CONFIG_DEBUG_FS
+	struct dentry		*dfs_dir;
+#endif
+};
+#define to_spmi_controller(d) container_of(d, struct spmi_controller, dev)
+
+static inline
+void *spmi_controller_get_drvdata(const struct spmi_controller *ctrl)
+{
+	return dev_get_drvdata(&ctrl->dev);
+}
+
+static inline void spmi_controller_set_drvdata(struct spmi_controller *ctrl,
+					       void *data)
+{
+	dev_set_drvdata(&ctrl->dev, data);
+}
+
+/**
+ * spmi_controller_alloc: Allocate a new SPMI controller
+ * @parent: parent device
+ * @size: size of private data
+ *
+ * Caller is responsible for either calling spmi_controller_add() to add the
+ * newly allocated controller, or calling spmi_controller_put() to discard it.
+ */
+struct spmi_controller *spmi_controller_alloc(struct device *parent,
+					      size_t size);
+
+static inline void spmi_controller_put(struct spmi_controller *ctrl)
+{
+	if (ctrl)
+		put_device(&ctrl->dev);
+}
+
+/**
+ * spmi_controller_add: Controller bring-up.
+ * @ctrl: controller to be registered.
+ *
+ * Register a controller previously allocated via spmi_controller_alloc() with
+ * the SPMI core
+ */
+int spmi_controller_add(struct spmi_controller *ctrl);
+
+/**
+ * spmi_controller_remove: Controller tear-down.
+ *
+ * Remove a SPMI controller.
+ */
+int spmi_controller_remove(struct spmi_controller *ctrl);
+
+/**
+ * struct spmi_driver: Manage SPMI generic/slave device driver
+ * @driver: SPMI device drivers should initialize name and owner field of
+ *	    this structure
+ * @probe: binds this driver to a SPMI device.
+ * @remove: unbinds this driver from the SPMI device.
+ * @shutdown: standard shutdown callback used during powerdown/halt.
+ * @suspend: standard suspend callback used during system suspend
+ * @resume: standard resume callback used during system resume
+ */
+struct spmi_driver {
+	struct device_driver driver;
+	int	(*probe)(struct spmi_device *sdev);
+	int	(*remove)(struct spmi_device *sdev);
+	void	(*shutdown)(struct spmi_device *sdev);
+	int	(*suspend)(struct spmi_device *sdev, pm_message_t pmesg);
+	int	(*resume)(struct spmi_device *sdev);
+};
+#define to_spmi_driver(d) container_of(d, struct spmi_driver, driver)
+
+/**
+ * spmi_driver_register: Client driver registration with SPMI framework.
+ * @sdrv: client driver to be associated with client-device.
+ *
+ * This API will register the client driver with the SPMI framework.
+ * It is called from the driver's module-init function.
+ */
+int spmi_driver_register(struct spmi_driver *sdrv);
+
+/**
+ * spmi_driver_unregister - reverse effect of spmi_driver_register
+ * @sdrv: the driver to unregister
+ * Context: can sleep
+ */
+static inline void spmi_driver_unregister(struct spmi_driver *sdrv)
+{
+	if (sdrv)
+		driver_unregister(&sdrv->driver);
+}
+
+#define module_spmi_driver(__spmi_driver) \
+	module_driver(__spmi_driver, spmi_driver_register, \
+			spmi_driver_unregister)
+
+/**
+ * spmi_register_read() - register read
+ * @sdev: SPMI device
+ * @addr: slave register address (5-bit address).
+ * @buf: buffer to be populated with data from the Slave.
+ *
+ * Reads 1 byte of data from a Slave device register.
+ */
+int spmi_register_read(struct spmi_device *sdev, u8 addr, u8 *buf);
+
+/**
+ * spmi_ext_register_read() - extended register read
+ * @sdev: SPMI device
+ * @addr: slave register address (8-bit address).
+ * @buf: buffer to be populated with data from the Slave.
+ * @len: the request number of bytes to read (up to 16 bytes).
+ *
+ * Reads up to 16 bytes of data from the extended register space on a
+ * Slave device.
+ */
+int spmi_ext_register_read(struct spmi_device *sdev, u8 addr, u8 *buf,
+			   size_t len);
+
+/**
+ * spmi_ext_register_readl() - extended register read long
+ * @sdev: SPMI device
+ * @addr: slave register address (16-bit address).
+ * @buf: buffer to be populated with data from the Slave.
+ * @len: the request number of bytes to read (up to 8 bytes).
+ *
+ * Reads up to 8 bytes of data from the extended register space on a
+ * Slave device using 16-bit address.
+ */
+int spmi_ext_register_readl(struct spmi_device *sdev, u16 addr, u8 *buf,
+			    size_t len);
+
+/**
+ * spmi_register_write() - register write
+ * @sdev: SPMI device
+ * @addr: slave register address (5-bit address).
+ * @buf: buffer containing the data to be transferred to the Slave.
+ *
+ * Writes 1 byte of data to a Slave device register.
+ */
+int spmi_register_write(struct spmi_device *sdev, u8 addr, u8 *buf);
+
+/**
+ * spmi_register_zero_write() - register zero write
+ * @sdev: SPMI device
+ * @data: the data to be written to register 0 (7-bits).
+ *
+ * Writes data to register 0 of the Slave device.
+ */
+int spmi_register_zero_write(struct spmi_device *sdev, u8 data);
+
+/**
+ * spmi_ext_register_write() - extended register write
+ * @sdev: SPMI device
+ * @addr: slave register address (8-bit address).
+ * @buf: buffer containing the data to be transferred to the Slave.
+ * @len: the request number of bytes to read (up to 16 bytes).
+ *
+ * Writes up to 16 bytes of data to the extended register space of a
+ * Slave device.
+ */
+int spmi_ext_register_write(struct spmi_device *sdev, u8 addr, u8 *buf,
+			    size_t len);
+
+/**
+ * spmi_ext_register_writel() - extended register write long
+ * @sdev: SPMI device
+ * @addr: slave register address (16-bit address).
+ * @buf: buffer containing the data to be transferred to the Slave.
+ * @len: the request number of bytes to read (up to 8 bytes).
+ *
+ * Writes up to 8 bytes of data to the extended register space of a
+ * Slave device using 16-bit address.
+ */
+int spmi_ext_register_writel(struct spmi_device *sdev, u16 addr,
+			     u8 *buf, size_t len);
+
+/**
+ * spmi_command_reset() - sends RESET command to the specified slave
+ * @sdev: SPMI device
+ *
+ * The Reset command initializes the Slave and forces all registers to
+ * their reset values. The Slave shall enter the STARTUP state after
+ * receiving a Reset command.
+ *
+ * Returns
+ * -EINVAL for invalid slave identifier.
+ * -EPERM if the SPMI transaction is denied due to permission issues.
+ * -EIO if the SPMI transaction fails (parity errors, etc).
+ * -ETIMEDOUT if the SPMI transaction times out.
+ */
+int spmi_command_reset(struct spmi_device *sdev);
+
+/**
+ * spmi_command_sleep() - sends SLEEP command to the specified slave
+ * @sdev: SPMI device
+ *
+ * The Sleep command causes the Slave to enter the user defined SLEEP state.
+ *
+ * Returns
+ * -EINVAL for invalid slave identifier.
+ * -EPERM if the SPMI transaction is denied due to permission issues.
+ * -EIO if the SPMI transaction fails (parity errors, etc).
+ * -ETIMEDOUT if the SPMI transaction times out.
+ */
+int spmi_command_sleep(struct spmi_device *sdev);
+
+/**
+ * spmi_command_wakeup() - sends WAKEUP command to the specified slave
+ * @sdev: SPMI device
+ *
+ * The Wakeup command causes the Slave to move from the SLEEP state to
+ * the ACTIVE state.
+ *
+ * Returns
+ * -EINVAL for invalid slave identifier.
+ * -EPERM if the SPMI transaction is denied due to permission issues.
+ * -EIO if the SPMI transaction fails (parity errors, etc).
+ * -ETIMEDOUT if the SPMI transaction times out.
+ */
+int spmi_command_wakeup(struct spmi_device *sdev);
+
+/**
+ * spmi_command_shutdown() - sends SHUTDOWN command to the specified slave
+ * @sdev: SPMI device
+ *
+ * The Shutdown command causes the Slave to enter the SHUTDOWN state.
+ *
+ * Returns
+ * -EINVAL for invalid slave identifier.
+ * -EPERM if the SPMI transaction is denied due to permission issues.
+ * -EIO if the SPMI transaction fails (parity errors, etc).
+ * -ETIMEDOUT if the SPMI transaction times out.
+ */
+int spmi_command_shutdown(struct spmi_device *sdev);
+
+#endif
-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

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

* [PATCH RFC v2 2/5] spmi: Linux driver framework for SPMI
@ 2013-08-09 20:37   ` Josh Cartwright
  0 siblings, 0 replies; 37+ messages in thread
From: Josh Cartwright @ 2013-08-09 20:37 UTC (permalink / raw)
  To: linux-arm-kernel

From: Kenneth Heitke <kheitke@codeaurora.org>

System Power Management Interface (SPMI) is a specification
developed by the MIPI (Mobile Industry Process Interface) Alliance
optimized for the real time control of Power Management ICs (PMIC).

SPMI is a two-wire serial interface that supports up to 4 master
devices and up to 16 logical slaves.

The framework supports message APIs, multiple busses (1 controller
per bus) and multiple clients/slave devices per controller.

Signed-off-by: Kenneth Heitke <kheitke@codeaurora.org>
Signed-off-by: Michael Bohan <mbohan@codeaurora.org>
Signed-off-by: Josh Cartwright <joshc@codeaurora.org>
---
 drivers/Kconfig                 |   2 +
 drivers/Makefile                |   1 +
 drivers/spmi/Kconfig            |   9 +
 drivers/spmi/Makefile           |   4 +
 drivers/spmi/spmi-dbgfs.c       | 580 ++++++++++++++++++++++++++++++++++++++++
 drivers/spmi/spmi-dbgfs.h       |  25 ++
 drivers/spmi/spmi.c             | 491 ++++++++++++++++++++++++++++++++++
 include/linux/mod_devicetable.h |   8 +
 include/linux/spmi.h            | 355 ++++++++++++++++++++++++
 9 files changed, 1475 insertions(+)
 create mode 100644 drivers/spmi/Kconfig
 create mode 100644 drivers/spmi/Makefile
 create mode 100644 drivers/spmi/spmi-dbgfs.c
 create mode 100644 drivers/spmi/spmi-dbgfs.h
 create mode 100644 drivers/spmi/spmi.c
 create mode 100644 include/linux/spmi.h

diff --git a/drivers/Kconfig b/drivers/Kconfig
index aa43b91..2b35420 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -52,6 +52,8 @@ source "drivers/i2c/Kconfig"
 
 source "drivers/spi/Kconfig"
 
+source "drivers/spmi/Kconfig"
+
 source "drivers/hsi/Kconfig"
 
 source "drivers/pps/Kconfig"
diff --git a/drivers/Makefile b/drivers/Makefile
index ab93de8..5c250df 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -64,6 +64,7 @@ obj-$(CONFIG_ATA)		+= ata/
 obj-$(CONFIG_TARGET_CORE)	+= target/
 obj-$(CONFIG_MTD)		+= mtd/
 obj-$(CONFIG_SPI)		+= spi/
+obj-$(CONFIG_SPMI)		+= spmi/
 obj-y				+= hsi/
 obj-y				+= net/
 obj-$(CONFIG_ATM)		+= atm/
diff --git a/drivers/spmi/Kconfig b/drivers/spmi/Kconfig
new file mode 100644
index 0000000..a03835f
--- /dev/null
+++ b/drivers/spmi/Kconfig
@@ -0,0 +1,9 @@
+#
+# SPMI driver configuration
+#
+menuconfig SPMI
+	bool "SPMI support"
+	help
+	  SPMI (System Power Management Interface) is a two-wire
+	  serial interface between baseband and application processors
+	  and Power Management Integrated Circuits (PMIC).
diff --git a/drivers/spmi/Makefile b/drivers/spmi/Makefile
new file mode 100644
index 0000000..9d78ec9
--- /dev/null
+++ b/drivers/spmi/Makefile
@@ -0,0 +1,4 @@
+#
+# Makefile for kernel SPMI framework.
+#
+obj-$(CONFIG_SPMI)	+= spmi.o spmi-dbgfs.o
diff --git a/drivers/spmi/spmi-dbgfs.c b/drivers/spmi/spmi-dbgfs.c
new file mode 100644
index 0000000..8b6f0e3
--- /dev/null
+++ b/drivers/spmi/spmi-dbgfs.c
@@ -0,0 +1,580 @@
+/* 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.
+ */
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/sizes.h>
+#include <linux/slab.h>
+#include <linux/uaccess.h>
+#include <linux/spinlock.h>
+#include <linux/string.h>
+#include <linux/debugfs.h>
+#include <linux/spmi.h>
+#include <linux/ctype.h>
+#include "spmi-dbgfs.h"
+
+#define ADDR_LEN	 6	/* 5 byte address + 1 space character */
+#define CHARS_PER_ITEM	 3	/* Format is 'XX ' */
+#define ITEMS_PER_LINE	16	/* 16 data items per line */
+#define MAX_LINE_LENGTH	(ADDR_LEN + (ITEMS_PER_LINE * CHARS_PER_ITEM) + 1)
+
+#define MAX_REG_PER_TRANSACTION	(8)
+
+static const mode_t DFS_MODE = S_IRUSR | S_IWUSR;
+
+/* Log buffer */
+struct spmi_log_buffer {
+	u32 rpos;	/* Current 'read' position in buffer */
+	u32 wpos;	/* Current 'write' position in buffer */
+	u32 len;	/* Length of the buffer */
+	char data[0];	/* Log buffer */
+};
+
+/* SPMI transaction parameters */
+struct spmi_trans {
+	u32 cnt;	/* Number of bytes to read */
+	u32 addr;	/* 20-bit address: SID + PID + Register offset */
+	u32 offset;	/* Offset of last read data */
+	bool raw_data;	/* Set to true for raw data dump */
+	struct spmi_device *sdev;
+	struct spmi_log_buffer *log; /* log buffer */
+};
+
+static char dbgfs_help[] =
+	"SPMI Debug-FS support\n"
+	"\n"
+	"Hierarchy schema:\n"
+	"/sys/kernel/debug/spmi\n"
+	"       /help                -- Static help text\n"
+	"       /spmi-0              -- Directory for SPMI bus 0\n"
+	"       /spmi-0/0-1          -- Directory for SPMI device '0-1'\n"
+	"       /spmi-0/0-1/address  -- Starting register for reads or writes\n"
+	"       /spmi-0/0-1/count    -- Number of registers to read (only used for reads)\n"
+	"       /spmi-0/0-1/data     -- Initiates the SPMI read (formatted output)\n"
+	"       /spmi-0/0-1/data_raw -- Initiates the SPMI raw read or write\n"
+	"       /spmi-n              -- Directory for SPMI bus n\n"
+	"\n"
+	"To perform SPMI read or write transactions, you need to first write the\n"
+	"address of the slave device register to the 'address' file.  For read\n"
+	"transactions, the number of bytes to be read needs to be written to the\n"
+	"'count' file.\n"
+	"\n"
+	"The 'address' file specifies the 20-bit address of a slave device register.\n"
+	"The upper 4 bits 'address[19..16]' specify the slave identifier (SID) for\n"
+	"the slave device.  The lower 16 bits specify the slave register address.\n"
+	"\n"
+	"Reading from the 'data' file will initiate a SPMI read transaction starting\n"
+	"from slave register 'address' for 'count' number of bytes.\n"
+	"\n"
+	"Writing to the 'data' file will initiate a SPMI write transaction starting\n"
+	"from slave register 'address'.  The number of registers written to will\n"
+	"match the number of bytes written to the 'data' file.\n"
+	"\n"
+	"Example: Read 4 bytes starting at register address 0x1234 for SID 2\n"
+	"\n"
+	"echo 0x21234 > address\n"
+	"echo 4 > count\n"
+	"cat data\n"
+	"\n"
+	"Example: Write 3 bytes starting at register address 0x1008 for SID 1\n"
+	"\n"
+	"echo 0x11008 > address\n"
+	"echo 0x01 0x02 0x03 > data\n"
+	"\n"
+	"Note that the count file is not used for writes.  Since 3 bytes are\n"
+	"written to the 'data' file, then 3 bytes will be written across the\n"
+	"SPMI bus.\n\n";
+
+static struct debugfs_blob_wrapper spmi_debug_help = {
+	.data	= dbgfs_help,
+	.size	= sizeof(dbgfs_help),
+};
+
+static struct dentry *spmi_debug_root;
+
+static int spmi_device_dfs_open(struct spmi_device *sdev, struct file *file)
+{
+	struct spmi_log_buffer *log;
+	struct spmi_trans *trans;
+
+	size_t logbufsize = SZ_4K;
+
+	/* Per file "transaction" data */
+	trans = kzalloc(sizeof(*trans), GFP_KERNEL);
+	if (!trans)
+		return -ENOMEM;
+
+	log = kzalloc(logbufsize, GFP_KERNEL);
+	if (!log) {
+		kfree(trans);
+		return -ENOMEM;
+	}
+
+	log->rpos = 0;
+	log->wpos = 0;
+	log->len = logbufsize - sizeof(*log);
+
+	trans->log = log;
+	trans->cnt = sdev->dfs_cnt;
+	trans->addr = sdev->dfs_addr;
+	trans->sdev = sdev;
+	trans->offset = trans->addr;
+
+	file->private_data = trans;
+	return 0;
+}
+
+static int spmi_device_dfs_data_open(struct inode *inode, struct file *file)
+{
+	struct spmi_device *sdev = inode->i_private;
+	return spmi_device_dfs_open(sdev, file);
+}
+
+static int spmi_device_dfs_raw_data_open(struct inode *inode, struct file *file)
+{
+	struct spmi_device *sdev = inode->i_private;
+	struct spmi_trans *trans;
+	int rc;
+
+	rc = spmi_device_dfs_open(sdev, file);
+	trans = file->private_data;
+	trans->raw_data = true;
+	return rc;
+}
+
+static int spmi_device_dfs_close(struct inode *inode, struct file *file)
+{
+	struct spmi_trans *trans = file->private_data;
+	kfree(trans->log);
+	kfree(trans);
+	return 0;
+}
+
+/**
+ * spmi_read_data: reads data across the SPMI bus
+ * @ctrl: The SPMI controller
+ * @buf: buffer to store the data read.
+ * @offset: SPMI address offset to start reading from.
+ * @cnt: The number of bytes to read.
+ *
+ * Returns 0 on success, otherwise returns error code from SPMI driver.
+ */
+static int
+spmi_read_data(struct spmi_device *sdev, uint8_t *buf, int offset, int cnt)
+{
+	int ret = 0;
+	int len;
+	uint16_t addr;
+
+	while (cnt > 0) {
+		addr = offset & 0xFFFF;
+		len = min(cnt, MAX_REG_PER_TRANSACTION);
+
+		ret = spmi_ext_register_readl(sdev, addr, buf, len);
+		if (ret < 0) {
+			dev_err(&sdev->dev,
+				"SPMI read failed, err = %d\n", ret);
+			goto done;
+		}
+
+		cnt -= len;
+		buf += len;
+		offset += len;
+	}
+
+done:
+	return ret;
+}
+
+/**
+ * spmi_write_data: writes data across the SPMI bus
+ * @ctrl: The SPMI controller
+ * @buf: data to be written.
+ * @offset: SPMI address offset to start writing to.
+ * @cnt: The number of bytes to write.
+ *
+ * Returns 0 on success, otherwise returns error code from SPMI driver.
+ */
+static int
+spmi_write_data(struct spmi_device *sdev, uint8_t *buf, int offset, int cnt)
+{
+	int ret = 0;
+	int len;
+	uint16_t addr;
+
+	while (cnt > 0) {
+		addr = offset & 0xFFFF;
+		len = min(cnt, MAX_REG_PER_TRANSACTION);
+
+		ret = spmi_ext_register_writel(sdev, addr, buf, len);
+		if (ret < 0) {
+			dev_err(&sdev->dev,
+				"SPMI write failed, err = %d\n", ret);
+			goto done;
+		}
+
+		cnt -= len;
+		buf += len;
+		offset += len;
+	}
+
+done:
+	return ret;
+}
+
+/**
+ * print_to_log: format a string and place into the log buffer
+ * @log: The log buffer to place the result into.
+ * @fmt: The format string to use.
+ * @...: The arguments for the format string.
+ *
+ * The return value is the number of characters written to @log buffer
+ * not including the trailing '\0'.
+ */
+static int print_to_log(struct spmi_log_buffer *log, const char *fmt, ...)
+{
+	va_list args;
+	int cnt;
+	char *buf = &log->data[log->wpos];
+	size_t size = log->len - log->wpos;
+
+	va_start(args, fmt);
+	cnt = vscnprintf(buf, size, fmt, args);
+	va_end(args);
+
+	log->wpos += cnt;
+	return cnt;
+}
+
+/**
+ * write_next_line_to_log: Writes a single "line" of data into the log buffer
+ * @trans: Pointer to SPMI transaction data.
+ * @offset: SPMI address offset to start reading from.
+ * @pcnt: Pointer to 'cnt' variable.  Indicates the number of bytes to read.
+ *
+ * The 'offset' is a 20-bits SPMI address which includes a 4-bit slave id (SID),
+ * an 8-bit peripheral id (PID), and an 8-bit peripheral register address.
+ *
+ * On a successful read, the pcnt is decremented by the number of data
+ * bytes read across the SPMI bus.  When the cnt reaches 0, all requested
+ * bytes have been read.
+ */
+static int
+write_next_line_to_log(struct spmi_trans *trans, int offset, size_t *pcnt)
+{
+	int i, j;
+	u8  data[ITEMS_PER_LINE];
+	struct spmi_log_buffer *log = trans->log;
+
+	int cnt = 0;
+	int padding = offset % ITEMS_PER_LINE;
+	int items_to_read = min(ARRAY_SIZE(data) - padding, *pcnt);
+	int items_to_log = min(ITEMS_PER_LINE, padding + items_to_read);
+
+	/* Buffer needs enough space for an entire line */
+	if ((log->len - log->wpos) < MAX_LINE_LENGTH)
+		goto done;
+
+	/* Read the desired number of "items" */
+	if (spmi_read_data(trans->sdev, data, offset, items_to_read))
+		goto done;
+
+	*pcnt -= items_to_read;
+
+	/* Each line starts with the aligned offset (20-bit address) */
+	cnt = print_to_log(log, "%5.5X ", offset & 0xffff0);
+	if (cnt == 0)
+		goto done;
+
+	/* If the offset is unaligned, add padding to right justify items */
+	for (i = 0; i < padding; ++i) {
+		cnt = print_to_log(log, "-- ");
+		if (cnt == 0)
+			goto done;
+	}
+
+	/* Log the data items */
+	for (j = 0; i < items_to_log; ++i, ++j) {
+		cnt = print_to_log(log, "%2.2X ", data[j]);
+		if (cnt == 0)
+			goto done;
+	}
+
+	/* If the last character was a space, then replace it with a newline */
+	if (log->wpos > 0 && log->data[log->wpos - 1] == ' ')
+		log->data[log->wpos - 1] = '\n';
+
+done:
+	return cnt;
+}
+
+/**
+ * write_raw_data_to_log: Writes a single "line" of data into the log buffer
+ * @trans: Pointer to SPMI transaction data.
+ * @offset: SPMI address offset to start reading from.
+ * @pcnt: Pointer to 'cnt' variable.  Indicates the number of bytes to read.
+ *
+ * The 'offset' is a 20-bits SPMI address which includes a 4-bit slave id (SID),
+ * an 8-bit peripheral id (PID), and an 8-bit peripheral register address.
+ *
+ * On a successful read, the pcnt is decremented by the number of data
+ * bytes read across the SPMI bus.  When the cnt reaches 0, all requested
+ * bytes have been read.
+ */
+static int
+write_raw_data_to_log(struct spmi_trans *trans, int offset, size_t *pcnt)
+{
+	u8  data[16];
+	struct spmi_log_buffer *log = trans->log;
+
+	int i;
+	int cnt = 0;
+	int items_to_read = min(ARRAY_SIZE(data), *pcnt);
+
+	/* Buffer needs enough space for an entire line */
+	if ((log->len - log->wpos) < 80)
+		goto done;
+
+	/* Read the desired number of "items" */
+	if (spmi_read_data(trans->sdev, data, offset, items_to_read))
+		goto done;
+
+	*pcnt -= items_to_read;
+
+	/* Log the data items */
+	for (i = 0; i < items_to_read; ++i) {
+		cnt = print_to_log(log, "0x%2.2X ", data[i]);
+		if (cnt == 0)
+			goto done;
+	}
+
+	/* If the last character was a space, then replace it with a newline */
+	if (log->wpos > 0 && log->data[log->wpos - 1] == ' ')
+		log->data[log->wpos - 1] = '\n';
+
+done:
+	return cnt;
+}
+
+/**
+ * get_log_data - reads data across the SPMI bus and saves to the log buffer
+ * @trans: Pointer to SPMI transaction data.
+ *
+ * Returns the number of "items" read or SPMI error code for read failures.
+ */
+static int get_log_data(struct spmi_trans *trans)
+{
+	int cnt;
+	int last_cnt;
+	int items_read;
+	int total_items_read = 0;
+	u32 offset = trans->offset;
+	size_t item_cnt = trans->cnt;
+	struct spmi_log_buffer *log = trans->log;
+	int (*write_to_log)(struct spmi_trans *, int, size_t *);
+
+	if (item_cnt == 0)
+		return 0;
+
+	if (trans->raw_data)
+		write_to_log = write_raw_data_to_log;
+	else
+		write_to_log = write_next_line_to_log;
+
+	/* Reset the log buffer 'pointers' */
+	log->wpos = log->rpos = 0;
+
+	/* Keep reading data until the log is full */
+	do {
+		last_cnt = item_cnt;
+		cnt = write_to_log(trans, offset, &item_cnt);
+		items_read = last_cnt - item_cnt;
+		offset += items_read;
+		total_items_read += items_read;
+	} while (cnt && item_cnt > 0);
+
+	/* Adjust the transaction offset and count */
+	trans->cnt = item_cnt;
+	trans->offset += total_items_read;
+
+	return total_items_read;
+}
+
+static ssize_t spmi_device_dfs_reg_write(struct file *file,
+					 const char __user *buf,
+					 size_t count, loff_t *ppos)
+{
+	int bytes_read;
+	int data;
+	int pos = 0;
+	int cnt = 0;
+	u8  *values;
+	size_t ret = 0;
+
+	struct spmi_trans *trans = file->private_data;
+	struct spmi_device *sdev = trans->sdev;
+	u32 offset = trans->offset;
+
+	/* Make a copy of the user data */
+	char *kbuf = kmalloc(count + 1, GFP_KERNEL);
+	if (!kbuf)
+		return -ENOMEM;
+
+	if (copy_from_user(kbuf, buf, count)) {
+		ret = -EFAULT;
+		goto free_buf;
+	}
+
+	*ppos += count;
+	kbuf[count] = '\0';
+
+	/* Override the text buffer with the raw data */
+	values = kbuf;
+
+	/* Parse the data in the buffer.  It should be a string of numbers */
+	while (sscanf(kbuf + pos, "%i%n", &data, &bytes_read) == 1) {
+		pos += bytes_read;
+		values[cnt++] = data & 0xff;
+	}
+
+	if (!cnt)
+		goto free_buf;
+
+	/* Perform the SPMI write(s) */
+	ret = spmi_write_data(trans->sdev, values, offset, cnt);
+
+	if (ret) {
+		dev_err(&sdev->dev, "SPMI write failed, err = %zu\n", ret);
+	} else {
+		ret = count;
+		trans->offset += cnt;
+	}
+
+free_buf:
+	kfree(kbuf);
+	return ret;
+}
+
+static ssize_t spmi_device_dfs_reg_read(struct file *file, char __user *buf,
+					size_t count, loff_t *ppos)
+{
+	struct spmi_trans *trans = file->private_data;
+	struct spmi_device *sdev = trans->sdev;
+	struct spmi_log_buffer *log = trans->log;
+	size_t len;
+
+	/* Is the the log buffer empty */
+	if (log->rpos >= log->wpos) {
+		if (get_log_data(trans) <= 0)
+			return 0;
+	}
+
+	len = min(count, (size_t) log->wpos - log->rpos);
+
+	if (copy_to_user(buf, &log->data[log->rpos], len))
+		return -EFAULT;
+
+	*ppos += len;
+	log->rpos += len;
+	return len;
+}
+
+static const struct file_operations spmi_dfs_reg_fops = {
+	.open		= spmi_device_dfs_data_open,
+	.release	= spmi_device_dfs_close,
+	.read		= spmi_device_dfs_reg_read,
+	.write		= spmi_device_dfs_reg_write,
+};
+
+static const struct file_operations spmi_dfs_raw_data_fops = {
+	.open		= spmi_device_dfs_raw_data_open,
+	.release	= spmi_device_dfs_close,
+	.read		= spmi_device_dfs_reg_read,
+	.write		= spmi_device_dfs_reg_write,
+};
+
+void spmi_dfs_controller_add(struct spmi_controller *ctrl)
+{
+	ctrl->dfs_dir = debugfs_create_dir(dev_name(&ctrl->dev),
+					   spmi_debug_root);
+	WARN_ON(!ctrl->dfs_dir);
+
+	dev_dbg(&ctrl->dev, "adding debug entries for spmi controller\n");
+}
+
+void spmi_dfs_controller_remove(struct spmi_controller *ctrl)
+{
+	debugfs_remove_recursive(ctrl->dfs_dir);
+}
+
+void spmi_dfs_device_add(struct spmi_device *sdev)
+{
+	struct dentry *file;
+
+	dev_dbg(&sdev->dev, "adding debugfs entries for spmi device\n");
+
+	sdev->dfs_dir = debugfs_create_dir(dev_name(&sdev->dev),
+					   sdev->ctrl->dfs_dir);
+	if (WARN_ON(!sdev->dfs_dir))
+		return;
+
+	sdev->dfs_cnt  = 1;
+
+	file = debugfs_create_u32("count", DFS_MODE, sdev->dfs_dir,
+				  &sdev->dfs_cnt);
+	if (WARN_ON(!file))
+		goto err_remove_fs;
+
+	file = debugfs_create_x32("address", DFS_MODE, sdev->dfs_dir,
+				  &sdev->dfs_addr);
+	if (WARN_ON(!file))
+		goto err_remove_fs;
+
+	file = debugfs_create_file("data", DFS_MODE, sdev->dfs_dir, sdev,
+				   &spmi_dfs_reg_fops);
+	if (WARN_ON(!file))
+		goto err_remove_fs;
+
+	file = debugfs_create_file("data_raw", DFS_MODE, sdev->dfs_dir,
+				   sdev, &spmi_dfs_raw_data_fops);
+	if (WARN_ON(!file))
+		goto err_remove_fs;
+
+	return;
+
+err_remove_fs:
+	debugfs_remove_recursive(sdev->dfs_dir);
+}
+
+void spmi_dfs_device_remove(struct spmi_device *sdev)
+{
+	dev_dbg(&sdev->dev, "Deleting device\n");
+	debugfs_remove_recursive(sdev->dfs_dir);
+}
+
+void __exit spmi_dfs_exit(void)
+{
+	debugfs_remove_recursive(spmi_debug_root);
+}
+
+void __init spmi_dfs_init(void)
+{
+	struct dentry *help;
+
+	spmi_debug_root = debugfs_create_dir("spmi", NULL);
+
+	help = debugfs_create_blob("help", S_IRUGO, spmi_debug_root,
+				   &spmi_debug_help);
+
+	WARN_ON(!spmi_debug_root || !help);
+}
+
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/spmi/spmi-dbgfs.h b/drivers/spmi/spmi-dbgfs.h
new file mode 100644
index 0000000..d3162fb
--- /dev/null
+++ b/drivers/spmi/spmi-dbgfs.h
@@ -0,0 +1,25 @@
+/* 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 _SPMI_DBGFS_H
+#define _SPMI_DBGFS_H
+
+struct spmi_controller;
+struct spmi_device;
+
+void __init spmi_dfs_init(void);
+void __exit spmi_dfs_exit(void);
+void spmi_dfs_controller_add(struct spmi_controller *ctrl);
+void spmi_dfs_controller_remove(struct spmi_controller *ctrl);
+void spmi_dfs_device_add(struct spmi_device *sdev);
+void spmi_dfs_device_remove(struct spmi_device *sdev);
+
+#endif /* _SPMI_DBGFS_H */
diff --git a/drivers/spmi/spmi.c b/drivers/spmi/spmi.c
new file mode 100644
index 0000000..c3da65e
--- /dev/null
+++ b/drivers/spmi/spmi.c
@@ -0,0 +1,491 @@
+/* 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.
+ */
+#include <linux/kernel.h>
+#include <linux/errno.h>
+#include <linux/idr.h>
+#include <linux/slab.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/spmi.h>
+#include <linux/module.h>
+#include <linux/pm_runtime.h>
+
+#include "spmi-dbgfs.h"
+
+static DEFINE_MUTEX(ctrl_idr_lock);
+static DEFINE_IDR(ctrl_idr);
+
+static void spmi_dev_release(struct device *dev)
+{
+	struct spmi_device *sdev = to_spmi_device(dev);
+	spmi_dfs_device_remove(sdev);
+	kfree(sdev);
+}
+
+static struct device_type spmi_dev_type = {
+	.release	= spmi_dev_release,
+};
+
+static void spmi_ctrl_release(struct device *dev)
+{
+	struct spmi_controller *ctrl = to_spmi_controller(dev);
+	spmi_dfs_controller_remove(ctrl);
+	mutex_lock(&ctrl_idr_lock);
+	idr_remove(&ctrl_idr, ctrl->nr);
+	mutex_unlock(&ctrl_idr_lock);
+	kfree(ctrl);
+}
+
+static struct device_type spmi_ctrl_type = {
+	.release	= spmi_ctrl_release,
+};
+
+#ifdef CONFIG_PM_SLEEP
+static int spmi_pm_suspend(struct device *dev)
+{
+	const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
+
+	if (pm)
+		return pm_generic_suspend(dev);
+	else
+		return 0;
+}
+
+static int spmi_pm_resume(struct device *dev)
+{
+	const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
+
+	if (pm)
+		return pm_generic_resume(dev);
+	else
+		return 0;
+}
+#else
+#define spmi_pm_suspend		NULL
+#define spmi_pm_resume		NULL
+#endif
+
+static const struct dev_pm_ops spmi_pm_ops = {
+	.suspend	= spmi_pm_suspend,
+	.resume		= spmi_pm_resume,
+	SET_RUNTIME_PM_OPS(
+		pm_generic_suspend,
+		pm_generic_resume,
+		pm_generic_runtime_idle
+	)
+};
+
+static int spmi_device_match(struct device *dev, struct device_driver *drv)
+{
+	if (of_driver_match_device(dev, drv))
+		return 1;
+
+	if (drv->name)
+		return strncmp(dev_name(dev), drv->name,
+			       SPMI_NAME_SIZE) == 0;
+
+	return 0;
+}
+
+int spmi_device_add(struct spmi_device *sdev)
+{
+	struct spmi_controller *ctrl = sdev->ctrl;
+	int err;
+
+	dev_set_name(&sdev->dev, "%d-%02x", ctrl->nr, sdev->usid);
+
+	err = device_add(&sdev->dev);
+	if (err < 0) {
+		dev_err(&sdev->dev, "Can't add %s, status %d\n",
+			dev_name(&sdev->dev), err);
+		goto err_device_add;
+	}
+
+	spmi_dfs_device_add(sdev);
+
+	dev_dbg(&sdev->dev, "device %s registered\n", dev_name(&sdev->dev));
+
+err_device_add:
+	return err;
+}
+EXPORT_SYMBOL_GPL(spmi_device_add);
+
+void spmi_device_remove(struct spmi_device *sdev)
+{
+	device_unregister(&sdev->dev);
+}
+EXPORT_SYMBOL_GPL(spmi_device_remove);
+
+static inline int
+spmi_cmd(struct spmi_controller *ctrl, u8 opcode, u8 sid)
+{
+	if (!ctrl || !ctrl->cmd || ctrl->dev.type != &spmi_ctrl_type)
+		return -EINVAL;
+
+	return ctrl->cmd(ctrl, opcode, sid);
+}
+
+static inline int spmi_read_cmd(struct spmi_controller *ctrl,
+				u8 opcode, u8 sid, u16 addr, u8 bc, u8 *buf)
+{
+	if (!ctrl || !ctrl->read_cmd || ctrl->dev.type != &spmi_ctrl_type)
+		return -EINVAL;
+
+	return ctrl->read_cmd(ctrl, opcode, sid, addr, bc, buf);
+}
+
+static inline int spmi_write_cmd(struct spmi_controller *ctrl,
+				 u8 opcode, u8 sid, u16 addr, u8 bc, u8 *buf)
+{
+	if (!ctrl || !ctrl->write_cmd || ctrl->dev.type != &spmi_ctrl_type)
+		return -EINVAL;
+
+	return ctrl->write_cmd(ctrl, opcode, sid, addr, bc, buf);
+}
+
+/*
+ * register read/write: 5-bit address, 1 byte of data
+ * extended register read/write: 8-bit address, up to 16 bytes of data
+ * extended register read/write long: 16-bit address, up to 8 bytes of data
+ */
+
+int spmi_register_read(struct spmi_device *sdev, u8 addr, u8 *buf)
+{
+	/* 5-bit register address */
+	if (addr > 0x1F)
+		return -EINVAL;
+
+	return spmi_read_cmd(sdev->ctrl, SPMI_CMD_READ, sdev->usid, addr, 0,
+			     buf);
+}
+EXPORT_SYMBOL_GPL(spmi_register_read);
+
+int spmi_ext_register_read(struct spmi_device *sdev, u8 addr, u8 *buf,
+			   size_t len)
+{
+	/* 8-bit register address, up to 16 bytes */
+	if (len == 0 || len > 16)
+		return -EINVAL;
+
+	return spmi_read_cmd(sdev->ctrl, SPMI_CMD_EXT_READ, sdev->usid, addr,
+			     len - 1, buf);
+}
+EXPORT_SYMBOL_GPL(spmi_ext_register_read);
+
+int spmi_ext_register_readl(struct spmi_device *sdev, u16 addr, u8 *buf,
+			    size_t len)
+{
+	/* 16-bit register address, up to 8 bytes */
+	if (len == 0 || len > 8)
+		return -EINVAL;
+
+	return spmi_read_cmd(sdev->ctrl, SPMI_CMD_EXT_READL, sdev->usid, addr,
+			     len - 1, buf);
+}
+EXPORT_SYMBOL_GPL(spmi_ext_register_readl);
+
+int spmi_register_write(struct spmi_device *sdev, u8 addr, u8 *buf)
+{
+	/* 5-bit register address */
+	if (addr > 0x1F)
+		return -EINVAL;
+
+	return spmi_write_cmd(sdev->ctrl, SPMI_CMD_WRITE, sdev->usid, addr, 0,
+			      buf);
+}
+EXPORT_SYMBOL_GPL(spmi_register_write);
+
+int spmi_register_zero_write(struct spmi_device *sdev, u8 data)
+{
+	return spmi_write_cmd(sdev->ctrl, SPMI_CMD_ZERO_WRITE, sdev->usid, 0,
+			      0, &data);
+}
+EXPORT_SYMBOL_GPL(spmi_register_zero_write);
+
+int spmi_ext_register_write(struct spmi_device *sdev, u8 addr, u8 *buf,
+			    size_t len)
+{
+	/* 8-bit register address, up to 16 bytes */
+	if (len == 0 || len > 16)
+		return -EINVAL;
+
+	return spmi_write_cmd(sdev->ctrl, SPMI_CMD_EXT_WRITE, sdev->usid, addr,
+			      len - 1, buf);
+}
+EXPORT_SYMBOL_GPL(spmi_ext_register_write);
+
+int spmi_ext_register_writel(struct spmi_device *sdev, u16 addr, u8 *buf,
+			     size_t len)
+{
+	/* 4-bit Slave Identifier, 16-bit register address, up to 8 bytes */
+	if (len == 0 || len > 8)
+		return -EINVAL;
+
+	return spmi_write_cmd(sdev->ctrl, SPMI_CMD_EXT_WRITEL, sdev->usid,
+			      addr, len - 1, buf);
+}
+EXPORT_SYMBOL_GPL(spmi_ext_register_writel);
+
+int spmi_command_reset(struct spmi_device *sdev)
+{
+	return spmi_cmd(sdev->ctrl, SPMI_CMD_RESET, sdev->usid);
+}
+EXPORT_SYMBOL_GPL(spmi_command_reset);
+
+int spmi_command_sleep(struct spmi_device *sdev)
+{
+	return spmi_cmd(sdev->ctrl, SPMI_CMD_SLEEP, sdev->usid);
+}
+EXPORT_SYMBOL_GPL(spmi_command_sleep);
+
+int spmi_command_wakeup(struct spmi_device *sdev)
+{
+	return spmi_cmd(sdev->ctrl, SPMI_CMD_WAKEUP, sdev->usid);
+}
+EXPORT_SYMBOL_GPL(spmi_command_wakeup);
+
+int spmi_command_shutdown(struct spmi_device *sdev)
+{
+	return spmi_cmd(sdev->ctrl, SPMI_CMD_SHUTDOWN, sdev->usid);
+}
+EXPORT_SYMBOL_GPL(spmi_command_shutdown);
+
+static int spmi_drv_probe(struct device *dev)
+{
+	const struct spmi_driver *sdrv = to_spmi_driver(dev->driver);
+	int err = 0;
+
+	if (sdrv->probe)
+		err = sdrv->probe(to_spmi_device(dev));
+
+	return err;
+}
+
+static int spmi_drv_remove(struct device *dev)
+{
+	const struct spmi_driver *sdrv = to_spmi_driver(dev->driver);
+	int err = 0;
+
+	if (sdrv->remove)
+		err = sdrv->remove(to_spmi_device(dev));
+
+	return err;
+}
+
+static void spmi_drv_shutdown(struct device *dev)
+{
+	const struct spmi_driver *sdrv = to_spmi_driver(dev->driver);
+
+	if (sdrv->shutdown)
+		sdrv->shutdown(to_spmi_device(dev));
+}
+
+static struct bus_type spmi_bus_type = {
+	.name		= "spmi",
+	.match		= spmi_device_match,
+	.pm		= &spmi_pm_ops,
+	.probe		= spmi_drv_probe,
+	.remove		= spmi_drv_remove,
+	.shutdown	= spmi_drv_shutdown,
+};
+
+struct spmi_device *spmi_device_alloc(struct spmi_controller *ctrl)
+{
+	struct spmi_device *sdev;
+
+	sdev = kzalloc(sizeof(*sdev), GFP_KERNEL);
+	if (!sdev)
+		return NULL;
+
+	sdev->ctrl = ctrl;
+	device_initialize(&sdev->dev);
+	sdev->dev.parent = &ctrl->dev;
+	sdev->dev.bus = &spmi_bus_type;
+	sdev->dev.type = &spmi_dev_type;
+	return sdev;
+}
+EXPORT_SYMBOL_GPL(spmi_device_alloc);
+
+struct spmi_controller *spmi_controller_alloc(struct device *parent,
+					      size_t size)
+{
+	struct spmi_controller *ctrl;
+	int id;
+
+	if (WARN_ON(!parent))
+		return NULL;
+
+	ctrl = kzalloc(sizeof(*ctrl) + size, GFP_KERNEL);
+	if (!ctrl)
+		return NULL;
+
+	device_initialize(&ctrl->dev);
+	ctrl->dev.type = &spmi_ctrl_type;
+	ctrl->dev.bus = &spmi_bus_type;
+	ctrl->dev.parent = parent;
+	ctrl->dev.of_node = parent->of_node;
+	spmi_controller_set_drvdata(ctrl, &ctrl[1]);
+
+	idr_preload(GFP_KERNEL);
+	mutex_lock(&ctrl_idr_lock);
+	id = idr_alloc(&ctrl_idr, ctrl, 0, 0, GFP_NOWAIT);
+	mutex_unlock(&ctrl_idr_lock);
+	idr_preload_end();
+
+	if (id < 0) {
+		dev_err(parent,
+			"unable to allocate SPMI controller identifier.\n");
+		spmi_controller_put(ctrl);
+		return NULL;
+	}
+
+	ctrl->nr = id;
+	dev_set_name(&ctrl->dev, "spmi-%d", id);
+
+	dev_dbg(&ctrl->dev, "allocated controller 0x%p id %d\n", ctrl, id);
+	return ctrl;
+}
+EXPORT_SYMBOL_GPL(spmi_controller_alloc);
+
+static int of_spmi_register_devices(struct spmi_controller *ctrl)
+{
+	struct device_node *node;
+	int err;
+
+	dev_dbg(&ctrl->dev, "of_spmi_register_devices\n");
+
+	if (!ctrl->dev.of_node)
+		return -ENODEV;
+
+	dev_dbg(&ctrl->dev, "looping through children\n");
+
+	for_each_available_child_of_node(ctrl->dev.of_node, node) {
+		struct spmi_device *sdev;
+		u32 usid;
+
+		dev_dbg(&ctrl->dev, "adding child %s\n", node->full_name);
+
+		err = of_property_read_u32(node, "reg", &usid);
+		if (err) {
+			dev_err(&ctrl->dev,
+				"node %s does not have 'reg' property\n",
+				node->full_name);
+			continue;
+		}
+
+		if (usid > 0xFF) {
+			dev_err(&ctrl->dev,
+				"invalid usid on node %s\n",
+				node->full_name);
+			continue;
+		}
+
+		dev_dbg(&ctrl->dev, "read usid %02x\n", usid);
+
+		sdev = spmi_device_alloc(ctrl);
+		if (!sdev)
+			continue;
+
+		sdev->dev.of_node = node;
+		sdev->usid = (u8) usid;
+
+		err = spmi_device_add(sdev);
+		if (err) {
+			dev_err(&sdev->dev,
+				"failure adding device. status %d\n", err);
+			spmi_device_put(sdev);
+			continue;
+		}
+	}
+
+	return 0;
+}
+
+int spmi_controller_add(struct spmi_controller *ctrl)
+{
+	int ret;
+
+	/* Can't register until after driver model init */
+	if (WARN_ON(!spmi_bus_type.p))
+		return -EAGAIN;
+
+	ret = device_add(&ctrl->dev);
+	if (ret)
+		return ret;
+
+	spmi_dfs_controller_add(ctrl);
+
+	if (IS_ENABLED(CONFIG_OF))
+		of_spmi_register_devices(ctrl);
+
+	dev_dbg(&ctrl->dev, "spmi-%d registered: dev:%p\n",
+		ctrl->nr, &ctrl->dev);
+
+	return 0;
+};
+EXPORT_SYMBOL_GPL(spmi_controller_add);
+
+/* Remove a device associated with a controller */
+static int spmi_ctrl_remove_device(struct device *dev, void *data)
+{
+	struct spmi_device *spmidev = to_spmi_device(dev);
+	if (dev->type == &spmi_dev_type)
+		spmi_device_remove(spmidev);
+	return 0;
+}
+
+/**
+ * spmi_controller_remove: Controller tear-down.
+ * @ctrl: controller to be removed.
+ *
+ * Controller added with the above API is torn down using this API.
+ */
+int spmi_controller_remove(struct spmi_controller *ctrl)
+{
+	int dummy;
+
+	if (!ctrl)
+		return -EINVAL;
+
+	dummy = device_for_each_child(&ctrl->dev, NULL,
+				      spmi_ctrl_remove_device);
+	device_unregister(&ctrl->dev);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(spmi_controller_remove);
+
+int spmi_driver_register(struct spmi_driver *drv)
+{
+	drv->driver.bus = &spmi_bus_type;
+	return driver_register(&drv->driver);
+}
+EXPORT_SYMBOL_GPL(spmi_driver_register);
+
+static void __exit spmi_exit(void)
+{
+	bus_unregister(&spmi_bus_type);
+	spmi_dfs_exit();
+}
+module_exit(spmi_exit);
+
+static int __init spmi_init(void)
+{
+	spmi_dfs_init();
+	return bus_register(&spmi_bus_type);
+}
+postcore_initcall(spmi_init);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("SPMI module");
+MODULE_ALIAS("platform:spmi");
diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
index 45e9214..677e474 100644
--- a/include/linux/mod_devicetable.h
+++ b/include/linux/mod_devicetable.h
@@ -432,6 +432,14 @@ struct spi_device_id {
 	kernel_ulong_t driver_data;	/* Data private to the driver */
 };
 
+#define SPMI_NAME_SIZE	32
+#define SPMI_MODULE_PREFIX "spmi:"
+
+struct spmi_device_id {
+	char name[SPMI_NAME_SIZE];
+	kernel_ulong_t driver_data;	/* Data private to the driver */
+};
+
 /* dmi */
 enum dmi_field {
 	DMI_NONE,
diff --git a/include/linux/spmi.h b/include/linux/spmi.h
new file mode 100644
index 0000000..8b9c232
--- /dev/null
+++ b/include/linux/spmi.h
@@ -0,0 +1,355 @@
+/* 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_SPMI_H
+#define _LINUX_SPMI_H
+
+#include <linux/types.h>
+#include <linux/device.h>
+#include <linux/mod_devicetable.h>
+
+/* Maximum slave identifier */
+#define SPMI_MAX_SLAVE_ID		16
+
+/* SPMI Commands */
+#define SPMI_CMD_EXT_WRITE		0x00
+#define SPMI_CMD_RESET			0x10
+#define SPMI_CMD_SLEEP			0x11
+#define SPMI_CMD_SHUTDOWN		0x12
+#define SPMI_CMD_WAKEUP			0x13
+#define SPMI_CMD_AUTHENTICATE		0x14
+#define SPMI_CMD_MSTR_READ		0x15
+#define SPMI_CMD_MSTR_WRITE		0x16
+#define SPMI_CMD_TRANSFER_BUS_OWNERSHIP	0x1A
+#define SPMI_CMD_DDB_MASTER_READ	0x1B
+#define SPMI_CMD_DDB_SLAVE_READ		0x1C
+#define SPMI_CMD_EXT_READ		0x20
+#define SPMI_CMD_EXT_WRITEL		0x30
+#define SPMI_CMD_EXT_READL		0x38
+#define SPMI_CMD_WRITE			0x40
+#define SPMI_CMD_READ			0x60
+#define SPMI_CMD_ZERO_WRITE		0x80
+
+/**
+ * Client/device handle (struct spmi_device):
+ *  This is the client/device handle returned when a SPMI device
+ *  is registered with a controller.
+ *  Pointer to this structure is used by client-driver as a handle.
+ *  @dev: Driver model representation of the device.
+ *  @ctrl: SPMI controller managing the bus hosting this device.
+ *  @usid: Unique Slave IDentifier.
+ *  @dfs_dir: dentry for this device in debugfs
+ *  @dfs_cnt: number of bytes to access for a debugfs operation
+ *  @dfs_addr: address to access for a debugfs operation
+ */
+struct spmi_device {
+	struct device		dev;
+	struct spmi_controller	*ctrl;
+	u8			usid;
+
+#if CONFIG_DEBUG_FS
+	struct dentry		*dfs_dir;
+	u32			dfs_cnt;
+	u32			dfs_addr;
+#endif
+};
+#define to_spmi_device(d) container_of(d, struct spmi_device, dev)
+
+static inline void *spmi_device_get_drvdata(const struct spmi_device *sdev)
+{
+	return dev_get_drvdata(&sdev->dev);
+}
+
+static inline void spmi_device_set_drvdata(struct spmi_device *sdev, void *data)
+{
+	dev_set_drvdata(&sdev->dev, data);
+}
+
+/**
+ * spmi_controller_alloc: Allocate a new SPMI controller
+ * @ctrl: associated controller
+ *
+ * Caller is responsible for either calling spmi_device_add() to add the
+ * newly allocated controller, or calling spmi_device_put() to discard it.
+ */
+struct spmi_device *spmi_device_alloc(struct spmi_controller *ctrl);
+
+static inline void spmi_device_put(struct spmi_device *sdev)
+{
+	if (sdev)
+		put_device(&sdev->dev);
+}
+
+/**
+ * spmi_device_add: add a device previously constructed via spmi_device_alloc()
+ * @sdev: spmi_device to be added
+ */
+int spmi_device_add(struct spmi_device *sdev);
+
+/**
+ * spmi_device_remove: remove a device
+ * @sdev: spmi_device to be removed
+ */
+void spmi_device_remove(struct spmi_device *sdev);
+
+/**
+ * struct spmi_controller: interface to the SPMI master controller
+ * @dev: Driver model representation of the device.
+ * @nr: board-specific number identifier for this controller/bus
+ * @cmd: sends a non-data command sequence on the SPMI bus.
+ * @read_cmd: sends a register read command sequence on the SPMI bus.
+ * @write_cmd: sends a register write command sequence on the SPMI bus.
+ * @dfs_dir: dentry for controller directory in debugfs
+ */
+struct spmi_controller {
+	struct device		dev;
+	unsigned int		nr;
+	int	(*cmd)(struct spmi_controller *ctrl, u8 opcode, u8 sid);
+	int	(*read_cmd)(struct spmi_controller *ctrl, u8 opcode,
+			    u8 sid, u16 addr, u8 bc, u8 *buf);
+	int	(*write_cmd)(struct spmi_controller *ctrl, u8 opcode,
+			     u8 sid, u16 addr, u8 bc, u8 *buf);
+#if CONFIG_DEBUG_FS
+	struct dentry		*dfs_dir;
+#endif
+};
+#define to_spmi_controller(d) container_of(d, struct spmi_controller, dev)
+
+static inline
+void *spmi_controller_get_drvdata(const struct spmi_controller *ctrl)
+{
+	return dev_get_drvdata(&ctrl->dev);
+}
+
+static inline void spmi_controller_set_drvdata(struct spmi_controller *ctrl,
+					       void *data)
+{
+	dev_set_drvdata(&ctrl->dev, data);
+}
+
+/**
+ * spmi_controller_alloc: Allocate a new SPMI controller
+ * @parent: parent device
+ * @size: size of private data
+ *
+ * Caller is responsible for either calling spmi_controller_add() to add the
+ * newly allocated controller, or calling spmi_controller_put() to discard it.
+ */
+struct spmi_controller *spmi_controller_alloc(struct device *parent,
+					      size_t size);
+
+static inline void spmi_controller_put(struct spmi_controller *ctrl)
+{
+	if (ctrl)
+		put_device(&ctrl->dev);
+}
+
+/**
+ * spmi_controller_add: Controller bring-up.
+ * @ctrl: controller to be registered.
+ *
+ * Register a controller previously allocated via spmi_controller_alloc() with
+ * the SPMI core
+ */
+int spmi_controller_add(struct spmi_controller *ctrl);
+
+/**
+ * spmi_controller_remove: Controller tear-down.
+ *
+ * Remove a SPMI controller.
+ */
+int spmi_controller_remove(struct spmi_controller *ctrl);
+
+/**
+ * struct spmi_driver: Manage SPMI generic/slave device driver
+ * @driver: SPMI device drivers should initialize name and owner field of
+ *	    this structure
+ * @probe: binds this driver to a SPMI device.
+ * @remove: unbinds this driver from the SPMI device.
+ * @shutdown: standard shutdown callback used during powerdown/halt.
+ * @suspend: standard suspend callback used during system suspend
+ * @resume: standard resume callback used during system resume
+ */
+struct spmi_driver {
+	struct device_driver driver;
+	int	(*probe)(struct spmi_device *sdev);
+	int	(*remove)(struct spmi_device *sdev);
+	void	(*shutdown)(struct spmi_device *sdev);
+	int	(*suspend)(struct spmi_device *sdev, pm_message_t pmesg);
+	int	(*resume)(struct spmi_device *sdev);
+};
+#define to_spmi_driver(d) container_of(d, struct spmi_driver, driver)
+
+/**
+ * spmi_driver_register: Client driver registration with SPMI framework.
+ * @sdrv: client driver to be associated with client-device.
+ *
+ * This API will register the client driver with the SPMI framework.
+ * It is called from the driver's module-init function.
+ */
+int spmi_driver_register(struct spmi_driver *sdrv);
+
+/**
+ * spmi_driver_unregister - reverse effect of spmi_driver_register
+ * @sdrv: the driver to unregister
+ * Context: can sleep
+ */
+static inline void spmi_driver_unregister(struct spmi_driver *sdrv)
+{
+	if (sdrv)
+		driver_unregister(&sdrv->driver);
+}
+
+#define module_spmi_driver(__spmi_driver) \
+	module_driver(__spmi_driver, spmi_driver_register, \
+			spmi_driver_unregister)
+
+/**
+ * spmi_register_read() - register read
+ * @sdev: SPMI device
+ * @addr: slave register address (5-bit address).
+ * @buf: buffer to be populated with data from the Slave.
+ *
+ * Reads 1 byte of data from a Slave device register.
+ */
+int spmi_register_read(struct spmi_device *sdev, u8 addr, u8 *buf);
+
+/**
+ * spmi_ext_register_read() - extended register read
+ * @sdev: SPMI device
+ * @addr: slave register address (8-bit address).
+ * @buf: buffer to be populated with data from the Slave.
+ * @len: the request number of bytes to read (up to 16 bytes).
+ *
+ * Reads up to 16 bytes of data from the extended register space on a
+ * Slave device.
+ */
+int spmi_ext_register_read(struct spmi_device *sdev, u8 addr, u8 *buf,
+			   size_t len);
+
+/**
+ * spmi_ext_register_readl() - extended register read long
+ * @sdev: SPMI device
+ * @addr: slave register address (16-bit address).
+ * @buf: buffer to be populated with data from the Slave.
+ * @len: the request number of bytes to read (up to 8 bytes).
+ *
+ * Reads up to 8 bytes of data from the extended register space on a
+ * Slave device using 16-bit address.
+ */
+int spmi_ext_register_readl(struct spmi_device *sdev, u16 addr, u8 *buf,
+			    size_t len);
+
+/**
+ * spmi_register_write() - register write
+ * @sdev: SPMI device
+ * @addr: slave register address (5-bit address).
+ * @buf: buffer containing the data to be transferred to the Slave.
+ *
+ * Writes 1 byte of data to a Slave device register.
+ */
+int spmi_register_write(struct spmi_device *sdev, u8 addr, u8 *buf);
+
+/**
+ * spmi_register_zero_write() - register zero write
+ * @sdev: SPMI device
+ * @data: the data to be written to register 0 (7-bits).
+ *
+ * Writes data to register 0 of the Slave device.
+ */
+int spmi_register_zero_write(struct spmi_device *sdev, u8 data);
+
+/**
+ * spmi_ext_register_write() - extended register write
+ * @sdev: SPMI device
+ * @addr: slave register address (8-bit address).
+ * @buf: buffer containing the data to be transferred to the Slave.
+ * @len: the request number of bytes to read (up to 16 bytes).
+ *
+ * Writes up to 16 bytes of data to the extended register space of a
+ * Slave device.
+ */
+int spmi_ext_register_write(struct spmi_device *sdev, u8 addr, u8 *buf,
+			    size_t len);
+
+/**
+ * spmi_ext_register_writel() - extended register write long
+ * @sdev: SPMI device
+ * @addr: slave register address (16-bit address).
+ * @buf: buffer containing the data to be transferred to the Slave.
+ * @len: the request number of bytes to read (up to 8 bytes).
+ *
+ * Writes up to 8 bytes of data to the extended register space of a
+ * Slave device using 16-bit address.
+ */
+int spmi_ext_register_writel(struct spmi_device *sdev, u16 addr,
+			     u8 *buf, size_t len);
+
+/**
+ * spmi_command_reset() - sends RESET command to the specified slave
+ * @sdev: SPMI device
+ *
+ * The Reset command initializes the Slave and forces all registers to
+ * their reset values. The Slave shall enter the STARTUP state after
+ * receiving a Reset command.
+ *
+ * Returns
+ * -EINVAL for invalid slave identifier.
+ * -EPERM if the SPMI transaction is denied due to permission issues.
+ * -EIO if the SPMI transaction fails (parity errors, etc).
+ * -ETIMEDOUT if the SPMI transaction times out.
+ */
+int spmi_command_reset(struct spmi_device *sdev);
+
+/**
+ * spmi_command_sleep() - sends SLEEP command to the specified slave
+ * @sdev: SPMI device
+ *
+ * The Sleep command causes the Slave to enter the user defined SLEEP state.
+ *
+ * Returns
+ * -EINVAL for invalid slave identifier.
+ * -EPERM if the SPMI transaction is denied due to permission issues.
+ * -EIO if the SPMI transaction fails (parity errors, etc).
+ * -ETIMEDOUT if the SPMI transaction times out.
+ */
+int spmi_command_sleep(struct spmi_device *sdev);
+
+/**
+ * spmi_command_wakeup() - sends WAKEUP command to the specified slave
+ * @sdev: SPMI device
+ *
+ * The Wakeup command causes the Slave to move from the SLEEP state to
+ * the ACTIVE state.
+ *
+ * Returns
+ * -EINVAL for invalid slave identifier.
+ * -EPERM if the SPMI transaction is denied due to permission issues.
+ * -EIO if the SPMI transaction fails (parity errors, etc).
+ * -ETIMEDOUT if the SPMI transaction times out.
+ */
+int spmi_command_wakeup(struct spmi_device *sdev);
+
+/**
+ * spmi_command_shutdown() - sends SHUTDOWN command to the specified slave
+ * @sdev: SPMI device
+ *
+ * The Shutdown command causes the Slave to enter the SHUTDOWN state.
+ *
+ * Returns
+ * -EINVAL for invalid slave identifier.
+ * -EPERM if the SPMI transaction is denied due to permission issues.
+ * -EIO if the SPMI transaction fails (parity errors, etc).
+ * -ETIMEDOUT if the SPMI transaction times out.
+ */
+int spmi_command_shutdown(struct spmi_device *sdev);
+
+#endif
-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

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

* [PATCH RFC v2 3/5] spmi: add generic SPMI controller binding documentation
  2013-08-22 20:18 ` Josh Cartwright
@ 2013-08-22 19:59   ` Josh Cartwright
  -1 siblings, 0 replies; 37+ messages in thread
From: Josh Cartwright @ 2013-08-22 19:59 UTC (permalink / raw)
  To: Grant Likely, Rob Herring, Greg Kroah-Hartman, Pawel Moll,
	Mark Rutland, Stephen Warren, Ian Campbell, Kumar Gala
  Cc: linux-kernel, linux-arm-kernel, linux-arm-msm, Sagar Dharia,
	Gilad Avidov, Michael Bohan, devicetree

Signed-off-by: Josh Cartwright <joshc@codeaurora.org>
---
I'm introducing this as an RFC, because there are set of assumptions
made in this binding spec, that currently hold true for the supported
controller/addressing scheme for the Snapdragon 800 series, but don't
necessarily hold true for SPMI in general.

  1. No use of Group Slave Identifiers (GSIDs)
     (SPMI allows for a slave to belong to zero or more groups specified
     by GSID, however this feature isn't currently implemented)

  2. No specification of Master Identifier (MID)
     (A "system integrator" allocates to each master a 2-bit MID, this
     currently isn't being specified, as it isn't needed by software for
     the PMIC Arb; not sure if this is of use to other SPMI controllers)

  3. Single SPMI master per controller

Effectively, only a subset of possible SPMI configurations are specified
in this document.

So, if it's considered necessary to provide a generic SPMI binding
specification, is it acceptable to only define a subset at this time,
expanding only when necessary, or shall I expand the definition to at
least address 1 & 2, even though they are of no use in the current
implementation?

 Documentation/devicetree/bindings/spmi/spmi.txt | 36 +++++++++++++++++++++++++
 1 file changed, 36 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/spmi/spmi.txt

diff --git a/Documentation/devicetree/bindings/spmi/spmi.txt b/Documentation/devicetree/bindings/spmi/spmi.txt
new file mode 100644
index 0000000..a01b064
--- /dev/null
+++ b/Documentation/devicetree/bindings/spmi/spmi.txt
@@ -0,0 +1,36 @@
+System Power Management Interface (SPMI) Controller
+
+This document defines a generic set of bindings for use by SPMI controllers.  A
+controller is modelled in device tree as a node with zero or more child nodes,
+each representing a unique slave on the bus.
+
+Required properties:
+- #address-cells : must be set to 1
+- #size-cells : must be set to 0
+
+Child nodes:
+
+An SPMI controller node can contain zero or more children.  Each child must
+have a reg property defining its 4-bit Unique Slave Identifier (USID) on the
+SPMI bus.  This is the ID that has been "statically assigned by the system
+integrator", as per the SPMI spec.
+
+Each child node represents a slave device on the bus.
+
+	controller@.. {
+		compatible = "...";
+		reg = <...>;
+
+		#address-cells = <1>;
+		#size-cells <0>;
+
+		child@0 {
+			compatible = "...";
+			reg = <0>;
+		};
+
+		child@7 {
+			compatible = "...";
+			reg = <7>;
+		};
+	};
-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

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

* [PATCH RFC v2 3/5] spmi: add generic SPMI controller binding documentation
@ 2013-08-22 19:59   ` Josh Cartwright
  0 siblings, 0 replies; 37+ messages in thread
From: Josh Cartwright @ 2013-08-22 19:59 UTC (permalink / raw)
  To: linux-arm-kernel

Signed-off-by: Josh Cartwright <joshc@codeaurora.org>
---
I'm introducing this as an RFC, because there are set of assumptions
made in this binding spec, that currently hold true for the supported
controller/addressing scheme for the Snapdragon 800 series, but don't
necessarily hold true for SPMI in general.

  1. No use of Group Slave Identifiers (GSIDs)
     (SPMI allows for a slave to belong to zero or more groups specified
     by GSID, however this feature isn't currently implemented)

  2. No specification of Master Identifier (MID)
     (A "system integrator" allocates to each master a 2-bit MID, this
     currently isn't being specified, as it isn't needed by software for
     the PMIC Arb; not sure if this is of use to other SPMI controllers)

  3. Single SPMI master per controller

Effectively, only a subset of possible SPMI configurations are specified
in this document.

So, if it's considered necessary to provide a generic SPMI binding
specification, is it acceptable to only define a subset at this time,
expanding only when necessary, or shall I expand the definition to at
least address 1 & 2, even though they are of no use in the current
implementation?

 Documentation/devicetree/bindings/spmi/spmi.txt | 36 +++++++++++++++++++++++++
 1 file changed, 36 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/spmi/spmi.txt

diff --git a/Documentation/devicetree/bindings/spmi/spmi.txt b/Documentation/devicetree/bindings/spmi/spmi.txt
new file mode 100644
index 0000000..a01b064
--- /dev/null
+++ b/Documentation/devicetree/bindings/spmi/spmi.txt
@@ -0,0 +1,36 @@
+System Power Management Interface (SPMI) Controller
+
+This document defines a generic set of bindings for use by SPMI controllers.  A
+controller is modelled in device tree as a node with zero or more child nodes,
+each representing a unique slave on the bus.
+
+Required properties:
+- #address-cells : must be set to 1
+- #size-cells : must be set to 0
+
+Child nodes:
+
+An SPMI controller node can contain zero or more children.  Each child must
+have a reg property defining its 4-bit Unique Slave Identifier (USID) on the
+SPMI bus.  This is the ID that has been "statically assigned by the system
+integrator", as per the SPMI spec.
+
+Each child node represents a slave device on the bus.
+
+	controller at .. {
+		compatible = "...";
+		reg = <...>;
+
+		#address-cells = <1>;
+		#size-cells <0>;
+
+		child at 0 {
+			compatible = "...";
+			reg = <0>;
+		};
+
+		child at 7 {
+			compatible = "...";
+			reg = <7>;
+		};
+	};
-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

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

* [PATCH RFC v2 0/3] Add support for the System Power Management Interface (SPMI)
@ 2013-08-22 20:18 ` Josh Cartwright
  0 siblings, 0 replies; 37+ messages in thread
From: Josh Cartwright @ 2013-08-22 20:18 UTC (permalink / raw)
  To: Grant Likely, Rob Herring, Greg Kroah-Hartman
  Cc: linux-kernel, linux-arm-kernel, linux-arm-msm, Sagar Dharia,
	Gilad Avidov, Michael Bohan

The System Power Management Interface (SPMI) is a high-speed,
low-latency, bi-directional, two-wire serial bus suitable for real-time
control of voltage and frequency scaled multi-core application
processors and its power management of auxiliary components. SPMI
obsoletes a number of legacy, custom point-to-point interfaces and
provides a low pin count, high-speed control bus for up to 4 Master and
16 Slave devices.

SPMI is specified by the MIPI (Mobile Industry Process Interface)
Alliance [1].

Introduced in this patchset are the core SPMI bus components, as well an
implementation of an SPMI controller, the "PMIC arbiter", which is used
on the Qualcomm Snapdragon 800 series of SoCs to communicate with
off-chip Power Management ICs (PMICs).  Notably missing from this
patchset is an implementation of an SPMI device/slave driver, which is
still forthcoming.  With this patchset in it's current state, it is
possible to perform register accesses to a slave via debugfs.

Changes from v1[2]:
  - Adopted patch (1/5) to #define for_each_available_node() shim
    in the !CONFIG_OF case
  - Moved device tree logic out of drivers/of and into spmi.c core (this
    mirrors what SPI is doing, and what i2c will soon be doing)
  - Move of_spmi_add_devices() call into spmi_device_add(), so drivers don't
    have to call it explicitly
  - Unconditionally build in debugfs code (rely on the underlying
    CONFIG_DEBUG_FS switch to throw unused code away)
  - Change pr_* print functions to their dev_* equivalents
  - Fix copy_{to,from}_user error handling
  - Renamed "board_lock" to "ctrl_idr_lock" to better describe it's purpose
  - Rework device object lifetime management
  - Rename PMIC arb binding document, add description of PMIC arb
  - Add generic SPMI device tree bindings

[1]: http://www.mipi.org/specifications/system-power-management-interface
[2]: http://thread.gmane.org/gmane.linux.ports.arm.msm/4886

Josh Cartwright (2):
  spmi: add generic SPMI controller binding documentation
  spmi: document the PMIC arbiter SPMI bindings

Kenneth Heitke (2):
  spmi: Linux driver framework for SPMI
  spmi: Add MSM PMIC Arbiter SPMI controller

Sylwester Nawrocki (1):
  of: Add empty for_each_available_child_of_node() macro definition

 .../bindings/spmi/qcom,spmi-pmic-arb.txt           |  36 ++
 Documentation/devicetree/bindings/spmi/spmi.txt    |  36 ++
 drivers/Kconfig                                    |   2 +
 drivers/Makefile                                   |   1 +
 drivers/spmi/Kconfig                               |  24 +
 drivers/spmi/Makefile                              |   6 +
 drivers/spmi/spmi-dbgfs.c                          | 580 +++++++++++++++++++++
 drivers/spmi/spmi-dbgfs.h                          |  25 +
 drivers/spmi/spmi-pmic-arb.c                       | 416 +++++++++++++++
 drivers/spmi/spmi.c                                | 491 +++++++++++++++++
 include/linux/mod_devicetable.h                    |   8 +
 include/linux/of.h                                 |   3 +
 include/linux/spmi.h                               | 355 +++++++++++++
 13 files changed, 1983 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.txt
 create mode 100644 Documentation/devicetree/bindings/spmi/spmi.txt
 create mode 100644 drivers/spmi/Kconfig
 create mode 100644 drivers/spmi/Makefile
 create mode 100644 drivers/spmi/spmi-dbgfs.c
 create mode 100644 drivers/spmi/spmi-dbgfs.h
 create mode 100644 drivers/spmi/spmi-pmic-arb.c
 create mode 100644 drivers/spmi/spmi.c
 create mode 100644 include/linux/spmi.h

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

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

* [PATCH RFC v2 0/3] Add support for the System Power Management Interface (SPMI)
@ 2013-08-22 20:18 ` Josh Cartwright
  0 siblings, 0 replies; 37+ messages in thread
From: Josh Cartwright @ 2013-08-22 20:18 UTC (permalink / raw)
  To: linux-arm-kernel

The System Power Management Interface (SPMI) is a high-speed,
low-latency, bi-directional, two-wire serial bus suitable for real-time
control of voltage and frequency scaled multi-core application
processors and its power management of auxiliary components. SPMI
obsoletes a number of legacy, custom point-to-point interfaces and
provides a low pin count, high-speed control bus for up to 4 Master and
16 Slave devices.

SPMI is specified by the MIPI (Mobile Industry Process Interface)
Alliance [1].

Introduced in this patchset are the core SPMI bus components, as well an
implementation of an SPMI controller, the "PMIC arbiter", which is used
on the Qualcomm Snapdragon 800 series of SoCs to communicate with
off-chip Power Management ICs (PMICs).  Notably missing from this
patchset is an implementation of an SPMI device/slave driver, which is
still forthcoming.  With this patchset in it's current state, it is
possible to perform register accesses to a slave via debugfs.

Changes from v1[2]:
  - Adopted patch (1/5) to #define for_each_available_node() shim
    in the !CONFIG_OF case
  - Moved device tree logic out of drivers/of and into spmi.c core (this
    mirrors what SPI is doing, and what i2c will soon be doing)
  - Move of_spmi_add_devices() call into spmi_device_add(), so drivers don't
    have to call it explicitly
  - Unconditionally build in debugfs code (rely on the underlying
    CONFIG_DEBUG_FS switch to throw unused code away)
  - Change pr_* print functions to their dev_* equivalents
  - Fix copy_{to,from}_user error handling
  - Renamed "board_lock" to "ctrl_idr_lock" to better describe it's purpose
  - Rework device object lifetime management
  - Rename PMIC arb binding document, add description of PMIC arb
  - Add generic SPMI device tree bindings

[1]: http://www.mipi.org/specifications/system-power-management-interface
[2]: http://thread.gmane.org/gmane.linux.ports.arm.msm/4886

Josh Cartwright (2):
  spmi: add generic SPMI controller binding documentation
  spmi: document the PMIC arbiter SPMI bindings

Kenneth Heitke (2):
  spmi: Linux driver framework for SPMI
  spmi: Add MSM PMIC Arbiter SPMI controller

Sylwester Nawrocki (1):
  of: Add empty for_each_available_child_of_node() macro definition

 .../bindings/spmi/qcom,spmi-pmic-arb.txt           |  36 ++
 Documentation/devicetree/bindings/spmi/spmi.txt    |  36 ++
 drivers/Kconfig                                    |   2 +
 drivers/Makefile                                   |   1 +
 drivers/spmi/Kconfig                               |  24 +
 drivers/spmi/Makefile                              |   6 +
 drivers/spmi/spmi-dbgfs.c                          | 580 +++++++++++++++++++++
 drivers/spmi/spmi-dbgfs.h                          |  25 +
 drivers/spmi/spmi-pmic-arb.c                       | 416 +++++++++++++++
 drivers/spmi/spmi.c                                | 491 +++++++++++++++++
 include/linux/mod_devicetable.h                    |   8 +
 include/linux/of.h                                 |   3 +
 include/linux/spmi.h                               | 355 +++++++++++++
 13 files changed, 1983 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.txt
 create mode 100644 Documentation/devicetree/bindings/spmi/spmi.txt
 create mode 100644 drivers/spmi/Kconfig
 create mode 100644 drivers/spmi/Makefile
 create mode 100644 drivers/spmi/spmi-dbgfs.c
 create mode 100644 drivers/spmi/spmi-dbgfs.h
 create mode 100644 drivers/spmi/spmi-pmic-arb.c
 create mode 100644 drivers/spmi/spmi.c
 create mode 100644 include/linux/spmi.h

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

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

* [PATCH RFC v2 1/5] of: Add empty for_each_available_child_of_node() macro definition
@ 2013-08-22 22:57   ` Josh Cartwright
  0 siblings, 0 replies; 37+ messages in thread
From: Josh Cartwright @ 2013-08-22 22:57 UTC (permalink / raw)
  To: linux-arm-kernel

From: Sylwester Nawrocki <s.nawrocki@samsung.com>

Add this empty macro definition so users can be compiled without
excluding this macro call with preprocessor directives when CONFIG_OF
is disabled.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
This patch was first introduced in December @
http://thread.gmane.org/gmane.linux.drivers.devicetree/26315

 include/linux/of.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/linux/of.h b/include/linux/of.h
index 1fd08ca..fefffb3 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -369,6 +369,9 @@ static inline bool of_have_populated_dt(void)
 #define for_each_child_of_node(parent, child) \
 	while (0)
 
+#define for_each_available_child_of_node(parent, child) \
+	while (0)
+
 static inline struct device_node *of_get_child_by_name(
 					const struct device_node *node,
 					const char *name)
-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

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

* Re: [PATCH RFC v2 2/5] spmi: Linux driver framework for SPMI
  2013-08-09 20:37   ` Josh Cartwright
@ 2013-08-22 23:10     ` Greg Kroah-Hartman
  -1 siblings, 0 replies; 37+ messages in thread
From: Greg Kroah-Hartman @ 2013-08-22 23:10 UTC (permalink / raw)
  To: Josh Cartwright
  Cc: Grant Likely, Rob Herring, linux-kernel, linux-arm-kernel,
	linux-arm-msm, Sagar Dharia, Gilad Avidov, Michael Bohan

On Fri, Aug 09, 2013 at 01:37:09PM -0700, Josh Cartwright wrote:
> +static char dbgfs_help[] =
> +	"SPMI Debug-FS support\n"
> +	"\n"
> +	"Hierarchy schema:\n"
> +	"/sys/kernel/debug/spmi\n"
> +	"       /help                -- Static help text\n"
> +	"       /spmi-0              -- Directory for SPMI bus 0\n"
> +	"       /spmi-0/0-1          -- Directory for SPMI device '0-1'\n"
> +	"       /spmi-0/0-1/address  -- Starting register for reads or writes\n"
> +	"       /spmi-0/0-1/count    -- Number of registers to read (only used for reads)\n"
> +	"       /spmi-0/0-1/data     -- Initiates the SPMI read (formatted output)\n"
> +	"       /spmi-0/0-1/data_raw -- Initiates the SPMI raw read or write\n"
> +	"       /spmi-n              -- Directory for SPMI bus n\n"
> +	"\n"
> +	"To perform SPMI read or write transactions, you need to first write the\n"
> +	"address of the slave device register to the 'address' file.  For read\n"
> +	"transactions, the number of bytes to be read needs to be written to the\n"
> +	"'count' file.\n"
> +	"\n"
> +	"The 'address' file specifies the 20-bit address of a slave device register.\n"
> +	"The upper 4 bits 'address[19..16]' specify the slave identifier (SID) for\n"
> +	"the slave device.  The lower 16 bits specify the slave register address.\n"
> +	"\n"
> +	"Reading from the 'data' file will initiate a SPMI read transaction starting\n"
> +	"from slave register 'address' for 'count' number of bytes.\n"
> +	"\n"
> +	"Writing to the 'data' file will initiate a SPMI write transaction starting\n"
> +	"from slave register 'address'.  The number of registers written to will\n"
> +	"match the number of bytes written to the 'data' file.\n"
> +	"\n"
> +	"Example: Read 4 bytes starting at register address 0x1234 for SID 2\n"
> +	"\n"
> +	"echo 0x21234 > address\n"
> +	"echo 4 > count\n"
> +	"cat data\n"
> +	"\n"
> +	"Example: Write 3 bytes starting at register address 0x1008 for SID 1\n"
> +	"\n"
> +	"echo 0x11008 > address\n"
> +	"echo 0x01 0x02 0x03 > data\n"
> +	"\n"
> +	"Note that the count file is not used for writes.  Since 3 bytes are\n"
> +	"written to the 'data' file, then 3 bytes will be written across the\n"
> +	"SPMI bus.\n\n";

The help file within the kernel is a nice touch :)

I do know the only rule for debugfs is "There are no rules", but this
looks like you are going to have the way to interact to this bus and
devices as debugfs, is that correct?

Or is this only for "debugging"?  If so, please document it as such.

> +void spmi_dfs_controller_add(struct spmi_controller *ctrl)
> +{
> +	ctrl->dfs_dir = debugfs_create_dir(dev_name(&ctrl->dev),
> +					   spmi_debug_root);
> +	WARN_ON(!ctrl->dfs_dir);

Why?  What is a user going to be able to do with something like this?
You do this in a number of places, please provide "valid" error messages
instead of just kernel stack tracebacks, failing to show the device for
which the error occured (hint, use dev_err()).

Again, never use WARN_ON() as error handling, it's lazy, and wrong.

thanks,

greg k-h

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

* [PATCH RFC v2 2/5] spmi: Linux driver framework for SPMI
@ 2013-08-22 23:10     ` Greg Kroah-Hartman
  0 siblings, 0 replies; 37+ messages in thread
From: Greg Kroah-Hartman @ 2013-08-22 23:10 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Aug 09, 2013 at 01:37:09PM -0700, Josh Cartwright wrote:
> +static char dbgfs_help[] =
> +	"SPMI Debug-FS support\n"
> +	"\n"
> +	"Hierarchy schema:\n"
> +	"/sys/kernel/debug/spmi\n"
> +	"       /help                -- Static help text\n"
> +	"       /spmi-0              -- Directory for SPMI bus 0\n"
> +	"       /spmi-0/0-1          -- Directory for SPMI device '0-1'\n"
> +	"       /spmi-0/0-1/address  -- Starting register for reads or writes\n"
> +	"       /spmi-0/0-1/count    -- Number of registers to read (only used for reads)\n"
> +	"       /spmi-0/0-1/data     -- Initiates the SPMI read (formatted output)\n"
> +	"       /spmi-0/0-1/data_raw -- Initiates the SPMI raw read or write\n"
> +	"       /spmi-n              -- Directory for SPMI bus n\n"
> +	"\n"
> +	"To perform SPMI read or write transactions, you need to first write the\n"
> +	"address of the slave device register to the 'address' file.  For read\n"
> +	"transactions, the number of bytes to be read needs to be written to the\n"
> +	"'count' file.\n"
> +	"\n"
> +	"The 'address' file specifies the 20-bit address of a slave device register.\n"
> +	"The upper 4 bits 'address[19..16]' specify the slave identifier (SID) for\n"
> +	"the slave device.  The lower 16 bits specify the slave register address.\n"
> +	"\n"
> +	"Reading from the 'data' file will initiate a SPMI read transaction starting\n"
> +	"from slave register 'address' for 'count' number of bytes.\n"
> +	"\n"
> +	"Writing to the 'data' file will initiate a SPMI write transaction starting\n"
> +	"from slave register 'address'.  The number of registers written to will\n"
> +	"match the number of bytes written to the 'data' file.\n"
> +	"\n"
> +	"Example: Read 4 bytes starting at register address 0x1234 for SID 2\n"
> +	"\n"
> +	"echo 0x21234 > address\n"
> +	"echo 4 > count\n"
> +	"cat data\n"
> +	"\n"
> +	"Example: Write 3 bytes starting at register address 0x1008 for SID 1\n"
> +	"\n"
> +	"echo 0x11008 > address\n"
> +	"echo 0x01 0x02 0x03 > data\n"
> +	"\n"
> +	"Note that the count file is not used for writes.  Since 3 bytes are\n"
> +	"written to the 'data' file, then 3 bytes will be written across the\n"
> +	"SPMI bus.\n\n";

The help file within the kernel is a nice touch :)

I do know the only rule for debugfs is "There are no rules", but this
looks like you are going to have the way to interact to this bus and
devices as debugfs, is that correct?

Or is this only for "debugging"?  If so, please document it as such.

> +void spmi_dfs_controller_add(struct spmi_controller *ctrl)
> +{
> +	ctrl->dfs_dir = debugfs_create_dir(dev_name(&ctrl->dev),
> +					   spmi_debug_root);
> +	WARN_ON(!ctrl->dfs_dir);

Why?  What is a user going to be able to do with something like this?
You do this in a number of places, please provide "valid" error messages
instead of just kernel stack tracebacks, failing to show the device for
which the error occured (hint, use dev_err()).

Again, never use WARN_ON() as error handling, it's lazy, and wrong.

thanks,

greg k-h

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

* Re: [PATCH RFC v2 2/5] spmi: Linux driver framework for SPMI
  2013-08-22 23:10     ` Greg Kroah-Hartman
@ 2013-08-23 16:06       ` Josh Cartwright
  -1 siblings, 0 replies; 37+ messages in thread
From: Josh Cartwright @ 2013-08-23 16:06 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Grant Likely, Rob Herring, linux-kernel, linux-arm-kernel,
	linux-arm-msm, Sagar Dharia, Gilad Avidov, Michael Bohan

On Thu, Aug 22, 2013 at 04:10:54PM -0700, Greg Kroah-Hartman wrote:
> On Fri, Aug 09, 2013 at 01:37:09PM -0700, Josh Cartwright wrote:
> > +static char dbgfs_help[] =
> > +	"SPMI Debug-FS support\n"
> > +	"\n"
> > +	"Hierarchy schema:\n"
> > +	"/sys/kernel/debug/spmi\n"
> > +	"       /help                -- Static help text\n"
> > +	"       /spmi-0              -- Directory for SPMI bus 0\n"
> > +	"       /spmi-0/0-1          -- Directory for SPMI device '0-1'\n"
> > +	"       /spmi-0/0-1/address  -- Starting register for reads or writes\n"
> > +	"       /spmi-0/0-1/count    -- Number of registers to read (only used for reads)\n"
> > +	"       /spmi-0/0-1/data     -- Initiates the SPMI read (formatted output)\n"
> > +	"       /spmi-0/0-1/data_raw -- Initiates the SPMI raw read or write\n"
> > +	"       /spmi-n              -- Directory for SPMI bus n\n"
> > +	"\n"
> > +	"To perform SPMI read or write transactions, you need to first write the\n"
> > +	"address of the slave device register to the 'address' file.  For read\n"
> > +	"transactions, the number of bytes to be read needs to be written to the\n"
> > +	"'count' file.\n"
> > +	"\n"
> > +	"The 'address' file specifies the 20-bit address of a slave device register.\n"
> > +	"The upper 4 bits 'address[19..16]' specify the slave identifier (SID) for\n"
> > +	"the slave device.  The lower 16 bits specify the slave register address.\n"
> > +	"\n"
> > +	"Reading from the 'data' file will initiate a SPMI read transaction starting\n"
> > +	"from slave register 'address' for 'count' number of bytes.\n"
> > +	"\n"
> > +	"Writing to the 'data' file will initiate a SPMI write transaction starting\n"
> > +	"from slave register 'address'.  The number of registers written to will\n"
> > +	"match the number of bytes written to the 'data' file.\n"
> > +	"\n"
> > +	"Example: Read 4 bytes starting at register address 0x1234 for SID 2\n"
> > +	"\n"
> > +	"echo 0x21234 > address\n"
> > +	"echo 4 > count\n"
> > +	"cat data\n"
> > +	"\n"
> > +	"Example: Write 3 bytes starting at register address 0x1008 for SID 1\n"
> > +	"\n"
> > +	"echo 0x11008 > address\n"
> > +	"echo 0x01 0x02 0x03 > data\n"
> > +	"\n"
> > +	"Note that the count file is not used for writes.  Since 3 bytes are\n"
> > +	"written to the 'data' file, then 3 bytes will be written across the\n"
> > +	"SPMI bus.\n\n";
> 
> The help file within the kernel is a nice touch :)
> 
> I do know the only rule for debugfs is "There are no rules", but this
> looks like you are going to have the way to interact to this bus and
> devices as debugfs, is that correct?

Using debugfs is _a_ way to interact with the controller/slaves, however
it is not _the_ way to do so.  The primary interface is the in-kernel
spmi_{read,write,...}_* functions called within the context of a proper
slave driver.

> Or is this only for "debugging"?  If so, please document it as such.

It's there because it provides a useful interface for debugging of the
controller code, and for simple peek/poke of the slave registers without
having a full driver in place.  Will document this.

> > +void spmi_dfs_controller_add(struct spmi_controller *ctrl)
> > +{
> > +	ctrl->dfs_dir = debugfs_create_dir(dev_name(&ctrl->dev),
> > +					   spmi_debug_root);
> > +	WARN_ON(!ctrl->dfs_dir);
> 
> Why?  What is a user going to be able to do with something like this?
> You do this in a number of places, please provide "valid" error messages
> instead of just kernel stack tracebacks, failing to show the device for
> which the error occured (hint, use dev_err()).

Will do.  Thanks.

> Again, never use WARN_ON() as error handling, it's lazy, and wrong.

To be fair to the original author of this code, this was one of the
'cleanups' I implemented.  So, I'll take full responsibility for the
laziness. :)

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

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

* [PATCH RFC v2 2/5] spmi: Linux driver framework for SPMI
@ 2013-08-23 16:06       ` Josh Cartwright
  0 siblings, 0 replies; 37+ messages in thread
From: Josh Cartwright @ 2013-08-23 16:06 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Aug 22, 2013 at 04:10:54PM -0700, Greg Kroah-Hartman wrote:
> On Fri, Aug 09, 2013 at 01:37:09PM -0700, Josh Cartwright wrote:
> > +static char dbgfs_help[] =
> > +	"SPMI Debug-FS support\n"
> > +	"\n"
> > +	"Hierarchy schema:\n"
> > +	"/sys/kernel/debug/spmi\n"
> > +	"       /help                -- Static help text\n"
> > +	"       /spmi-0              -- Directory for SPMI bus 0\n"
> > +	"       /spmi-0/0-1          -- Directory for SPMI device '0-1'\n"
> > +	"       /spmi-0/0-1/address  -- Starting register for reads or writes\n"
> > +	"       /spmi-0/0-1/count    -- Number of registers to read (only used for reads)\n"
> > +	"       /spmi-0/0-1/data     -- Initiates the SPMI read (formatted output)\n"
> > +	"       /spmi-0/0-1/data_raw -- Initiates the SPMI raw read or write\n"
> > +	"       /spmi-n              -- Directory for SPMI bus n\n"
> > +	"\n"
> > +	"To perform SPMI read or write transactions, you need to first write the\n"
> > +	"address of the slave device register to the 'address' file.  For read\n"
> > +	"transactions, the number of bytes to be read needs to be written to the\n"
> > +	"'count' file.\n"
> > +	"\n"
> > +	"The 'address' file specifies the 20-bit address of a slave device register.\n"
> > +	"The upper 4 bits 'address[19..16]' specify the slave identifier (SID) for\n"
> > +	"the slave device.  The lower 16 bits specify the slave register address.\n"
> > +	"\n"
> > +	"Reading from the 'data' file will initiate a SPMI read transaction starting\n"
> > +	"from slave register 'address' for 'count' number of bytes.\n"
> > +	"\n"
> > +	"Writing to the 'data' file will initiate a SPMI write transaction starting\n"
> > +	"from slave register 'address'.  The number of registers written to will\n"
> > +	"match the number of bytes written to the 'data' file.\n"
> > +	"\n"
> > +	"Example: Read 4 bytes starting at register address 0x1234 for SID 2\n"
> > +	"\n"
> > +	"echo 0x21234 > address\n"
> > +	"echo 4 > count\n"
> > +	"cat data\n"
> > +	"\n"
> > +	"Example: Write 3 bytes starting at register address 0x1008 for SID 1\n"
> > +	"\n"
> > +	"echo 0x11008 > address\n"
> > +	"echo 0x01 0x02 0x03 > data\n"
> > +	"\n"
> > +	"Note that the count file is not used for writes.  Since 3 bytes are\n"
> > +	"written to the 'data' file, then 3 bytes will be written across the\n"
> > +	"SPMI bus.\n\n";
> 
> The help file within the kernel is a nice touch :)
> 
> I do know the only rule for debugfs is "There are no rules", but this
> looks like you are going to have the way to interact to this bus and
> devices as debugfs, is that correct?

Using debugfs is _a_ way to interact with the controller/slaves, however
it is not _the_ way to do so.  The primary interface is the in-kernel
spmi_{read,write,...}_* functions called within the context of a proper
slave driver.

> Or is this only for "debugging"?  If so, please document it as such.

It's there because it provides a useful interface for debugging of the
controller code, and for simple peek/poke of the slave registers without
having a full driver in place.  Will document this.

> > +void spmi_dfs_controller_add(struct spmi_controller *ctrl)
> > +{
> > +	ctrl->dfs_dir = debugfs_create_dir(dev_name(&ctrl->dev),
> > +					   spmi_debug_root);
> > +	WARN_ON(!ctrl->dfs_dir);
> 
> Why?  What is a user going to be able to do with something like this?
> You do this in a number of places, please provide "valid" error messages
> instead of just kernel stack tracebacks, failing to show the device for
> which the error occured (hint, use dev_err()).

Will do.  Thanks.

> Again, never use WARN_ON() as error handling, it's lazy, and wrong.

To be fair to the original author of this code, this was one of the
'cleanups' I implemented.  So, I'll take full responsibility for the
laziness. :)

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

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

* Re: [PATCH RFC v2 5/5] spmi: document the PMIC arbiter SPMI bindings
  2013-08-09 20:37   ` Josh Cartwright
@ 2013-08-23 21:55     ` Stephen Warren
  -1 siblings, 0 replies; 37+ messages in thread
From: Stephen Warren @ 2013-08-23 21:55 UTC (permalink / raw)
  To: Josh Cartwright
  Cc: Grant Likely, Rob Herring, Greg Kroah-Hartman, Pawel Moll,
	Mark Rutland, Ian Campbell, Kumar Gala, linux-kernel,
	linux-arm-kernel, linux-arm-msm, Sagar Dharia, Gilad Avidov,
	Michael Bohan, devicetree

On 08/09/2013 02:37 PM, Josh Cartwright wrote:

Patch description?

> diff --git a/Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.txt b/Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.txt

> +Required properties:
> +- compatible : should be "qcom,spmi-pmic-arb".
> +- reg-names  : should be "core", "intr", "cnfg"

> +- reg : offset and length of the PMIC Arbiter Core register map.
> +- reg : offset and length of the PMIC Arbiter Interrupt controller register map.
> +- reg : offset and length of the PMIC Arbiter Configuration register map.

This seems like it's defining the "reg" property 3 times each with a
different meaning. It'd be better to say something like:

reg : register specifier. Must contain 3 entries, in the following
order: core registers, interrupt register, configuration registers.

> +	qcom,spmi@fc4c0000 {
...
> +		qcom,pm8841@4 {

Node names typically don't include a vendor prefix. For the first
instance above, I think just "spmi@fc4c0000" or even just "spmi" would
be appropriate here; the latter being best in the case where there's
only 1 SPMI controller and hence no need to include the unit address for
uniqueness.

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

* [PATCH RFC v2 5/5] spmi: document the PMIC arbiter SPMI bindings
@ 2013-08-23 21:55     ` Stephen Warren
  0 siblings, 0 replies; 37+ messages in thread
From: Stephen Warren @ 2013-08-23 21:55 UTC (permalink / raw)
  To: linux-arm-kernel

On 08/09/2013 02:37 PM, Josh Cartwright wrote:

Patch description?

> diff --git a/Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.txt b/Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.txt

> +Required properties:
> +- compatible : should be "qcom,spmi-pmic-arb".
> +- reg-names  : should be "core", "intr", "cnfg"

> +- reg : offset and length of the PMIC Arbiter Core register map.
> +- reg : offset and length of the PMIC Arbiter Interrupt controller register map.
> +- reg : offset and length of the PMIC Arbiter Configuration register map.

This seems like it's defining the "reg" property 3 times each with a
different meaning. It'd be better to say something like:

reg : register specifier. Must contain 3 entries, in the following
order: core registers, interrupt register, configuration registers.

> +	qcom,spmi at fc4c0000 {
...
> +		qcom,pm8841 at 4 {

Node names typically don't include a vendor prefix. For the first
instance above, I think just "spmi at fc4c0000" or even just "spmi" would
be appropriate here; the latter being best in the case where there's
only 1 SPMI controller and hence no need to include the unit address for
uniqueness.

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

* Re: [PATCH RFC v2 3/5] spmi: add generic SPMI controller binding documentation
  2013-08-22 19:59   ` Josh Cartwright
@ 2013-08-23 21:58     ` Stephen Warren
  -1 siblings, 0 replies; 37+ messages in thread
From: Stephen Warren @ 2013-08-23 21:58 UTC (permalink / raw)
  To: Josh Cartwright
  Cc: Grant Likely, Rob Herring, Greg Kroah-Hartman, Pawel Moll,
	Mark Rutland, Ian Campbell, Kumar Gala, linux-kernel,
	linux-arm-kernel, linux-arm-msm, Sagar Dharia, Gilad Avidov,
	Michael Bohan, devicetree

On 08/22/2013 01:59 PM, Josh Cartwright wrote:
> Signed-off-by: Josh Cartwright <joshc@codeaurora.org>
> ---
> I'm introducing this as an RFC, because there are set of assumptions
> made in this binding spec, that currently hold true for the supported
> controller/addressing scheme for the Snapdragon 800 series, but don't
> necessarily hold true for SPMI in general.
> 
>   1. No use of Group Slave Identifiers (GSIDs)
>      (SPMI allows for a slave to belong to zero or more groups specified
>      by GSID, however this feature isn't currently implemented)
> 
>   2. No specification of Master Identifier (MID)
>      (A "system integrator" allocates to each master a 2-bit MID, this
>      currently isn't being specified, as it isn't needed by software for
>      the PMIC Arb; not sure if this is of use to other SPMI controllers)
> 
>   3. Single SPMI master per controller
> 
> Effectively, only a subset of possible SPMI configurations are specified
> in this document.
> 
> So, if it's considered necessary to provide a generic SPMI binding
> specification, is it acceptable to only define a subset at this time,
> expanding only when necessary, or shall I expand the definition to at
> least address 1 & 2, even though they are of no use in the current
> implementation?

It's best to define the whole thing from the start if possible. It's
easier to ensure the whole binding is consistent, and nothing has been
left out.

However, it's probably OK to define a subset binding initially and then
expand it later, as long as some thought it put into how it can be
expanded in a way that is 100% compatible: old DTs will still operate
with new kernels and perhaps even new DTs will still operate with old
kernels.

That said, if the thought is put in to ensure that's possible, it's
probably just as easy to define the whole binding from the start.

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

* [PATCH RFC v2 3/5] spmi: add generic SPMI controller binding documentation
@ 2013-08-23 21:58     ` Stephen Warren
  0 siblings, 0 replies; 37+ messages in thread
From: Stephen Warren @ 2013-08-23 21:58 UTC (permalink / raw)
  To: linux-arm-kernel

On 08/22/2013 01:59 PM, Josh Cartwright wrote:
> Signed-off-by: Josh Cartwright <joshc@codeaurora.org>
> ---
> I'm introducing this as an RFC, because there are set of assumptions
> made in this binding spec, that currently hold true for the supported
> controller/addressing scheme for the Snapdragon 800 series, but don't
> necessarily hold true for SPMI in general.
> 
>   1. No use of Group Slave Identifiers (GSIDs)
>      (SPMI allows for a slave to belong to zero or more groups specified
>      by GSID, however this feature isn't currently implemented)
> 
>   2. No specification of Master Identifier (MID)
>      (A "system integrator" allocates to each master a 2-bit MID, this
>      currently isn't being specified, as it isn't needed by software for
>      the PMIC Arb; not sure if this is of use to other SPMI controllers)
> 
>   3. Single SPMI master per controller
> 
> Effectively, only a subset of possible SPMI configurations are specified
> in this document.
> 
> So, if it's considered necessary to provide a generic SPMI binding
> specification, is it acceptable to only define a subset at this time,
> expanding only when necessary, or shall I expand the definition to at
> least address 1 & 2, even though they are of no use in the current
> implementation?

It's best to define the whole thing from the start if possible. It's
easier to ensure the whole binding is consistent, and nothing has been
left out.

However, it's probably OK to define a subset binding initially and then
expand it later, as long as some thought it put into how it can be
expanded in a way that is 100% compatible: old DTs will still operate
with new kernels and perhaps even new DTs will still operate with old
kernels.

That said, if the thought is put in to ensure that's possible, it's
probably just as easy to define the whole binding from the start.

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

* Re: [PATCH RFC v2 3/5] spmi: add generic SPMI controller binding documentation
  2013-08-23 21:58     ` Stephen Warren
@ 2013-08-27 17:01       ` Josh Cartwright
  -1 siblings, 0 replies; 37+ messages in thread
From: Josh Cartwright @ 2013-08-27 17:01 UTC (permalink / raw)
  To: Stephen Warren
  Cc: Josh Cartwright, Grant Likely, Rob Herring, Greg Kroah-Hartman,
	Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, linux-kernel,
	linux-arm-kernel, linux-arm-msm, Sagar Dharia, Gilad Avidov,
	Michael Bohan, devicetree

Hey Stephen-

Thanks for the comments.

On Fri, Aug 23, 2013 at 03:58:36PM -0600, Stephen Warren wrote:
> On 08/22/2013 01:59 PM, Josh Cartwright wrote:
> > Signed-off-by: Josh Cartwright <joshc@codeaurora.org>
> > ---
> > I'm introducing this as an RFC, because there are set of assumptions
> > made in this binding spec, that currently hold true for the supported
> > controller/addressing scheme for the Snapdragon 800 series, but don't
> > necessarily hold true for SPMI in general.
> > 
> >   1. No use of Group Slave Identifiers (GSIDs)
> >      (SPMI allows for a slave to belong to zero or more groups specified
> >      by GSID, however this feature isn't currently implemented)
> > 
> >   2. No specification of Master Identifier (MID)
> >      (A "system integrator" allocates to each master a 2-bit MID, this
> >      currently isn't being specified, as it isn't needed by software for
> >      the PMIC Arb; not sure if this is of use to other SPMI controllers)
> > 
> >   3. Single SPMI master per controller
> > 
> > Effectively, only a subset of possible SPMI configurations are specified
> > in this document.
> > 
> > So, if it's considered necessary to provide a generic SPMI binding
> > specification, is it acceptable to only define a subset at this time,
> > expanding only when necessary, or shall I expand the definition to at
> > least address 1 & 2, even though they are of no use in the current
> > implementation?
> 
> It's best to define the whole thing from the start if possible. It's
> easier to ensure the whole binding is consistent, and nothing has been
> left out.

That makes sense.  I think I'll go down this route for v3 of this
patchset:

For #1 above, extend the 'reg' property of a slave node to include the
group slave ID's in which the slave is a member.  The first 'reg' entry
will remain the slave's Unique Slave Identifier (USID) as before.

For #2, add additional required 'spmi-mid' property in the
controller/master node that describes the 2-bit Master Identifier (MID).

For #3, rename the SPMI API's s/controller/master/.  The current
controller/master terminology difference is confusing and unnecessary.

> However, it's probably OK to define a subset binding initially and then
> expand it later, as long as some thought it put into how it can be
> expanded in a way that is 100% compatible: old DTs will still operate
> with new kernels and perhaps even new DTs will still operate with old
> kernels.
>
> That said, if the thought is put in to ensure that's possible, it's
> probably just as easy to define the whole binding from the start.

That all makes sense.

If we want to ensure for the generic bindings that we are fulling
characterizing/describing the SPMI bus, then we'll additionally need to
tackle an additional identified assumption:

  4. One master per SPMI bus.  (The SPMI spec allows for up to 4
     masters)

On the Snapdragon 800 series, there exists only one software-controlled
master, but it is conceivably possible to have a setup with two
software-controlled masters on the same SPMI bus.

This necessarily means that the description of the slaves and the
masters will need to be decoupled; I'm imagining a generic binding
supporting multiple masters would look something like this:

	master0: master@0 {
		compatible = "...";
		#spmi-master-cells = <0>;
		spmi-mid = <0>;

		...
	};

	master2: master@2 {
		compatible = "...";
		#spmi-master-cells = <0>;
		spmi-mid = <2>;

		...
	};

	spmi_bus {
		compatible = "...";

		spmi-masters = <&master0 &master2>;

		foo@0 {
			compatible = "...";
			reg = <0 ...>;
		};

		foo@8 {
			compatible = "...";
			reg = <8 ...>;
		};
	};

(This will also necessitate a change in the underlying SPMI driver
model, in the current implementation, a SPMI master 'owns' a particular
device.  This is not a valid assumption to make.)

Would this property-containing-phandle-vector be considered the
canonical way of representing nodes with multiple parents in the device
tree?

Thanks,
  Josh

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

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

* [PATCH RFC v2 3/5] spmi: add generic SPMI controller binding documentation
@ 2013-08-27 17:01       ` Josh Cartwright
  0 siblings, 0 replies; 37+ messages in thread
From: Josh Cartwright @ 2013-08-27 17:01 UTC (permalink / raw)
  To: linux-arm-kernel

Hey Stephen-

Thanks for the comments.

On Fri, Aug 23, 2013 at 03:58:36PM -0600, Stephen Warren wrote:
> On 08/22/2013 01:59 PM, Josh Cartwright wrote:
> > Signed-off-by: Josh Cartwright <joshc@codeaurora.org>
> > ---
> > I'm introducing this as an RFC, because there are set of assumptions
> > made in this binding spec, that currently hold true for the supported
> > controller/addressing scheme for the Snapdragon 800 series, but don't
> > necessarily hold true for SPMI in general.
> > 
> >   1. No use of Group Slave Identifiers (GSIDs)
> >      (SPMI allows for a slave to belong to zero or more groups specified
> >      by GSID, however this feature isn't currently implemented)
> > 
> >   2. No specification of Master Identifier (MID)
> >      (A "system integrator" allocates to each master a 2-bit MID, this
> >      currently isn't being specified, as it isn't needed by software for
> >      the PMIC Arb; not sure if this is of use to other SPMI controllers)
> > 
> >   3. Single SPMI master per controller
> > 
> > Effectively, only a subset of possible SPMI configurations are specified
> > in this document.
> > 
> > So, if it's considered necessary to provide a generic SPMI binding
> > specification, is it acceptable to only define a subset at this time,
> > expanding only when necessary, or shall I expand the definition to at
> > least address 1 & 2, even though they are of no use in the current
> > implementation?
> 
> It's best to define the whole thing from the start if possible. It's
> easier to ensure the whole binding is consistent, and nothing has been
> left out.

That makes sense.  I think I'll go down this route for v3 of this
patchset:

For #1 above, extend the 'reg' property of a slave node to include the
group slave ID's in which the slave is a member.  The first 'reg' entry
will remain the slave's Unique Slave Identifier (USID) as before.

For #2, add additional required 'spmi-mid' property in the
controller/master node that describes the 2-bit Master Identifier (MID).

For #3, rename the SPMI API's s/controller/master/.  The current
controller/master terminology difference is confusing and unnecessary.

> However, it's probably OK to define a subset binding initially and then
> expand it later, as long as some thought it put into how it can be
> expanded in a way that is 100% compatible: old DTs will still operate
> with new kernels and perhaps even new DTs will still operate with old
> kernels.
>
> That said, if the thought is put in to ensure that's possible, it's
> probably just as easy to define the whole binding from the start.

That all makes sense.

If we want to ensure for the generic bindings that we are fulling
characterizing/describing the SPMI bus, then we'll additionally need to
tackle an additional identified assumption:

  4. One master per SPMI bus.  (The SPMI spec allows for up to 4
     masters)

On the Snapdragon 800 series, there exists only one software-controlled
master, but it is conceivably possible to have a setup with two
software-controlled masters on the same SPMI bus.

This necessarily means that the description of the slaves and the
masters will need to be decoupled; I'm imagining a generic binding
supporting multiple masters would look something like this:

	master0: master at 0 {
		compatible = "...";
		#spmi-master-cells = <0>;
		spmi-mid = <0>;

		...
	};

	master2: master at 2 {
		compatible = "...";
		#spmi-master-cells = <0>;
		spmi-mid = <2>;

		...
	};

	spmi_bus {
		compatible = "...";

		spmi-masters = <&master0 &master2>;

		foo at 0 {
			compatible = "...";
			reg = <0 ...>;
		};

		foo at 8 {
			compatible = "...";
			reg = <8 ...>;
		};
	};

(This will also necessitate a change in the underlying SPMI driver
model, in the current implementation, a SPMI master 'owns' a particular
device.  This is not a valid assumption to make.)

Would this property-containing-phandle-vector be considered the
canonical way of representing nodes with multiple parents in the device
tree?

Thanks,
  Josh

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

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

* Re: [PATCH RFC v2 3/5] spmi: add generic SPMI controller binding documentation
  2013-08-27 17:01       ` Josh Cartwright
@ 2013-08-27 21:55         ` Stephen Warren
  -1 siblings, 0 replies; 37+ messages in thread
From: Stephen Warren @ 2013-08-27 21:55 UTC (permalink / raw)
  To: Josh Cartwright
  Cc: Grant Likely, Rob Herring, Greg Kroah-Hartman, Pawel Moll,
	Mark Rutland, Ian Campbell, Kumar Gala, linux-kernel,
	linux-arm-kernel, linux-arm-msm, Sagar Dharia, Gilad Avidov,
	Michael Bohan, devicetree

On 08/27/2013 11:01 AM, Josh Cartwright wrote:
...
> If we want to ensure for the generic bindings that we are fulling
> characterizing/describing the SPMI bus, then we'll additionally need to
> tackle an additional identified assumption:
> 
>   4. One master per SPMI bus.  (The SPMI spec allows for up to 4
>      masters)
> 
> On the Snapdragon 800 series, there exists only one software-controlled
> master, but it is conceivably possible to have a setup with two
> software-controlled masters on the same SPMI bus.
> 
> This necessarily means that the description of the slaves and the
> masters will need to be decoupled; I'm imagining a generic binding
> supporting multiple masters would look something like this:

Is there a need to represent the other masters in the DT? Sure they're
there in HW, but if there's no specific way for the
CPU-to-which-the-DT-applies to actually interact with those other
masters (except perhaps by experiencing some arbitration delays) then
presumably there's no need to represent the other masters in DT?

> 	master0: master@0 {
> 		compatible = "...";
> 		#spmi-master-cells = <0>;
> 		spmi-mid = <0>;
> 
> 		...
> 	};
> 
> 	master2: master@2 {
> 		compatible = "...";
> 		#spmi-master-cells = <0>;
> 		spmi-mid = <2>;
> 
> 		...
> 	};
> 
> 	spmi_bus {
> 		compatible = "...";
> 
> 		spmi-masters = <&master0 &master2>;
> 
> 		foo@0 {
> 			compatible = "...";
> 			reg = <0 ...>;
> 		};
> 
> 		foo@8 {
> 			compatible = "...";
> 			reg = <8 ...>;
> 		};
> 	};
> 
> (This will also necessitate a change in the underlying SPMI driver
> model, in the current implementation, a SPMI master 'owns' a particular
> device.  This is not a valid assumption to make.)
> 
> Would this property-containing-phandle-vector be considered the
> canonical way of representing nodes with multiple parents in the device
> tree?

I don't think I've seen anything like this before, although that
in-and-of-itself doesn't make it wrong.

Another approach might be to encode master-vs-slave into a cell in the
reg property? Something like:

cell 0 - address type (0: master, 1: unique ID, 2: group ID, ...)
cell 1 - address value

I haven't thought much about that; perhaps there are disadvantages doing
that.

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

* [PATCH RFC v2 3/5] spmi: add generic SPMI controller binding documentation
@ 2013-08-27 21:55         ` Stephen Warren
  0 siblings, 0 replies; 37+ messages in thread
From: Stephen Warren @ 2013-08-27 21:55 UTC (permalink / raw)
  To: linux-arm-kernel

On 08/27/2013 11:01 AM, Josh Cartwright wrote:
...
> If we want to ensure for the generic bindings that we are fulling
> characterizing/describing the SPMI bus, then we'll additionally need to
> tackle an additional identified assumption:
> 
>   4. One master per SPMI bus.  (The SPMI spec allows for up to 4
>      masters)
> 
> On the Snapdragon 800 series, there exists only one software-controlled
> master, but it is conceivably possible to have a setup with two
> software-controlled masters on the same SPMI bus.
> 
> This necessarily means that the description of the slaves and the
> masters will need to be decoupled; I'm imagining a generic binding
> supporting multiple masters would look something like this:

Is there a need to represent the other masters in the DT? Sure they're
there in HW, but if there's no specific way for the
CPU-to-which-the-DT-applies to actually interact with those other
masters (except perhaps by experiencing some arbitration delays) then
presumably there's no need to represent the other masters in DT?

> 	master0: master at 0 {
> 		compatible = "...";
> 		#spmi-master-cells = <0>;
> 		spmi-mid = <0>;
> 
> 		...
> 	};
> 
> 	master2: master at 2 {
> 		compatible = "...";
> 		#spmi-master-cells = <0>;
> 		spmi-mid = <2>;
> 
> 		...
> 	};
> 
> 	spmi_bus {
> 		compatible = "...";
> 
> 		spmi-masters = <&master0 &master2>;
> 
> 		foo at 0 {
> 			compatible = "...";
> 			reg = <0 ...>;
> 		};
> 
> 		foo at 8 {
> 			compatible = "...";
> 			reg = <8 ...>;
> 		};
> 	};
> 
> (This will also necessitate a change in the underlying SPMI driver
> model, in the current implementation, a SPMI master 'owns' a particular
> device.  This is not a valid assumption to make.)
> 
> Would this property-containing-phandle-vector be considered the
> canonical way of representing nodes with multiple parents in the device
> tree?

I don't think I've seen anything like this before, although that
in-and-of-itself doesn't make it wrong.

Another approach might be to encode master-vs-slave into a cell in the
reg property? Something like:

cell 0 - address type (0: master, 1: unique ID, 2: group ID, ...)
cell 1 - address value

I haven't thought much about that; perhaps there are disadvantages doing
that.

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

* Re: [PATCH RFC v2 3/5] spmi: add generic SPMI controller binding documentation
  2013-08-27 21:55         ` Stephen Warren
@ 2013-08-28 18:00           ` Josh Cartwright
  -1 siblings, 0 replies; 37+ messages in thread
From: Josh Cartwright @ 2013-08-28 18:00 UTC (permalink / raw)
  To: Stephen Warren
  Cc: Grant Likely, Rob Herring, Greg Kroah-Hartman, Pawel Moll,
	Mark Rutland, Ian Campbell, Kumar Gala, linux-kernel,
	linux-arm-kernel, linux-arm-msm, Sagar Dharia, Gilad Avidov,
	Michael Bohan, devicetree, Wolfram Sang

On Tue, Aug 27, 2013 at 03:55:19PM -0600, Stephen Warren wrote:
> On 08/27/2013 11:01 AM, Josh Cartwright wrote:
> ...
> > If we want to ensure for the generic bindings that we are fulling
> > characterizing/describing the SPMI bus, then we'll additionally need to
> > tackle an additional identified assumption:
> >
> >   4. One master per SPMI bus.  (The SPMI spec allows for up to 4
> >      masters)
> >
> > On the Snapdragon 800 series, there exists only one software-controlled
> > master, but it is conceivably possible to have a setup with two
> > software-controlled masters on the same SPMI bus.
> >
> > This necessarily means that the description of the slaves and the
> > masters will need to be decoupled; I'm imagining a generic binding
> > supporting multiple masters would look something like this:
>
> Is there a need to represent the other masters in the DT? Sure they're
> there in HW, but if there's no specific way for the
> CPU-to-which-the-DT-applies to actually interact with those other
> masters (except perhaps by experiencing some arbitration delays) then
> presumably there's no need to represent the other masters in DT?

My example is contrived, but there is nothing in the SPMI spec
preventing two masters from being controllable by the same
CPU-to-which-the-DT-applies, sharing the same underlying bus.

I would also expect this configuration to be uncommon, I'm checking with
some folks with more SPMI experience to make sure they agree.

Interestingly, i2c as far as I can tell has also made the same
assumption.  There doesn't appear to be any way to express a
multi-master i2c setup where both masters are controlled by the same OS.

> > 	master0: master@0 {
> > 		compatible = "...";
> > 		#spmi-master-cells = <0>;
> > 		spmi-mid = <0>;
> > 
> > 		...
> > 	};
> > 
> > 	master2: master@2 {
> > 		compatible = "...";
> > 		#spmi-master-cells = <0>;
> > 		spmi-mid = <2>;
> > 
> > 		...
> > 	};
> > 
> > 	spmi_bus {
> > 		compatible = "...";
> > 
> > 		spmi-masters = <&master0 &master2>;
> > 
> > 		foo@0 {
> > 			compatible = "...";
> > 			reg = <0 ...>;
> > 		};
> > 
> > 		foo@8 {
> > 			compatible = "...";
> > 			reg = <8 ...>;
> > 		};
> > 	};
> > 
> > (This will also necessitate a change in the underlying SPMI driver
> > model, in the current implementation, a SPMI master 'owns' a particular
> > device.  This is not a valid assumption to make.)
> > 
> > Would this property-containing-phandle-vector be considered the
> > canonical way of representing nodes with multiple parents in the device
> > tree?
> 
> I don't think I've seen anything like this before, although that
> in-and-of-itself doesn't make it wrong.
> 
> Another approach might be to encode master-vs-slave into a cell in the
> reg property? Something like:
> 
> cell 0 - address type (0: master, 1: unique ID, 2: group ID, ...)
> cell 1 - address value
> 
> I haven't thought much about that; perhaps there are disadvantages doing
> that.

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

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

* [PATCH RFC v2 3/5] spmi: add generic SPMI controller binding documentation
@ 2013-08-28 18:00           ` Josh Cartwright
  0 siblings, 0 replies; 37+ messages in thread
From: Josh Cartwright @ 2013-08-28 18:00 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Aug 27, 2013 at 03:55:19PM -0600, Stephen Warren wrote:
> On 08/27/2013 11:01 AM, Josh Cartwright wrote:
> ...
> > If we want to ensure for the generic bindings that we are fulling
> > characterizing/describing the SPMI bus, then we'll additionally need to
> > tackle an additional identified assumption:
> >
> >   4. One master per SPMI bus.  (The SPMI spec allows for up to 4
> >      masters)
> >
> > On the Snapdragon 800 series, there exists only one software-controlled
> > master, but it is conceivably possible to have a setup with two
> > software-controlled masters on the same SPMI bus.
> >
> > This necessarily means that the description of the slaves and the
> > masters will need to be decoupled; I'm imagining a generic binding
> > supporting multiple masters would look something like this:
>
> Is there a need to represent the other masters in the DT? Sure they're
> there in HW, but if there's no specific way for the
> CPU-to-which-the-DT-applies to actually interact with those other
> masters (except perhaps by experiencing some arbitration delays) then
> presumably there's no need to represent the other masters in DT?

My example is contrived, but there is nothing in the SPMI spec
preventing two masters from being controllable by the same
CPU-to-which-the-DT-applies, sharing the same underlying bus.

I would also expect this configuration to be uncommon, I'm checking with
some folks with more SPMI experience to make sure they agree.

Interestingly, i2c as far as I can tell has also made the same
assumption.  There doesn't appear to be any way to express a
multi-master i2c setup where both masters are controlled by the same OS.

> > 	master0: master at 0 {
> > 		compatible = "...";
> > 		#spmi-master-cells = <0>;
> > 		spmi-mid = <0>;
> > 
> > 		...
> > 	};
> > 
> > 	master2: master at 2 {
> > 		compatible = "...";
> > 		#spmi-master-cells = <0>;
> > 		spmi-mid = <2>;
> > 
> > 		...
> > 	};
> > 
> > 	spmi_bus {
> > 		compatible = "...";
> > 
> > 		spmi-masters = <&master0 &master2>;
> > 
> > 		foo at 0 {
> > 			compatible = "...";
> > 			reg = <0 ...>;
> > 		};
> > 
> > 		foo at 8 {
> > 			compatible = "...";
> > 			reg = <8 ...>;
> > 		};
> > 	};
> > 
> > (This will also necessitate a change in the underlying SPMI driver
> > model, in the current implementation, a SPMI master 'owns' a particular
> > device.  This is not a valid assumption to make.)
> > 
> > Would this property-containing-phandle-vector be considered the
> > canonical way of representing nodes with multiple parents in the device
> > tree?
> 
> I don't think I've seen anything like this before, although that
> in-and-of-itself doesn't make it wrong.
> 
> Another approach might be to encode master-vs-slave into a cell in the
> reg property? Something like:
> 
> cell 0 - address type (0: master, 1: unique ID, 2: group ID, ...)
> cell 1 - address value
> 
> I haven't thought much about that; perhaps there are disadvantages doing
> that.

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

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

* Re: [PATCH RFC v2 3/5] spmi: add generic SPMI controller binding documentation
  2013-08-28 18:00           ` Josh Cartwright
@ 2013-08-28 18:32             ` Stephen Warren
  -1 siblings, 0 replies; 37+ messages in thread
From: Stephen Warren @ 2013-08-28 18:32 UTC (permalink / raw)
  To: Josh Cartwright
  Cc: Grant Likely, Rob Herring, Greg Kroah-Hartman, Pawel Moll,
	Mark Rutland, Ian Campbell, Kumar Gala, linux-kernel,
	linux-arm-kernel, linux-arm-msm, Sagar Dharia, Gilad Avidov,
	Michael Bohan, devicetree, Wolfram Sang

On 08/28/2013 12:00 PM, Josh Cartwright wrote:
> On Tue, Aug 27, 2013 at 03:55:19PM -0600, Stephen Warren wrote:
>> On 08/27/2013 11:01 AM, Josh Cartwright wrote:
>> ...
>>> If we want to ensure for the generic bindings that we are fulling
>>> characterizing/describing the SPMI bus, then we'll additionally need to
>>> tackle an additional identified assumption:
>>>
>>>   4. One master per SPMI bus.  (The SPMI spec allows for up to 4
>>>      masters)
>>>
>>> On the Snapdragon 800 series, there exists only one software-controlled
>>> master, but it is conceivably possible to have a setup with two
>>> software-controlled masters on the same SPMI bus.
>>>
>>> This necessarily means that the description of the slaves and the
>>> masters will need to be decoupled; I'm imagining a generic binding
>>> supporting multiple masters would look something like this:
>>
>> Is there a need to represent the other masters in the DT? Sure they're
>> there in HW, but if there's no specific way for the
>> CPU-to-which-the-DT-applies to actually interact with those other
>> masters (except perhaps by experiencing some arbitration delays) then
>> presumably there's no need to represent the other masters in DT?
> 
> My example is contrived, but there is nothing in the SPMI spec
> preventing two masters from being controllable by the same
> CPU-to-which-the-DT-applies, sharing the same underlying bus.

That's true.

> I would also expect this configuration to be uncommon, I'm checking with
> some folks with more SPMI experience to make sure they agree.
> 
> Interestingly, i2c as far as I can tell has also made the same
> assumption.  There doesn't appear to be any way to express a
> multi-master i2c setup where both masters are controlled by the same OS.

Yes, I think it's a fair assumption that we don't need to represent
that; I immediately thought about the I2C counter-example after reading
your first paragraph.

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

* [PATCH RFC v2 3/5] spmi: add generic SPMI controller binding documentation
@ 2013-08-28 18:32             ` Stephen Warren
  0 siblings, 0 replies; 37+ messages in thread
From: Stephen Warren @ 2013-08-28 18:32 UTC (permalink / raw)
  To: linux-arm-kernel

On 08/28/2013 12:00 PM, Josh Cartwright wrote:
> On Tue, Aug 27, 2013 at 03:55:19PM -0600, Stephen Warren wrote:
>> On 08/27/2013 11:01 AM, Josh Cartwright wrote:
>> ...
>>> If we want to ensure for the generic bindings that we are fulling
>>> characterizing/describing the SPMI bus, then we'll additionally need to
>>> tackle an additional identified assumption:
>>>
>>>   4. One master per SPMI bus.  (The SPMI spec allows for up to 4
>>>      masters)
>>>
>>> On the Snapdragon 800 series, there exists only one software-controlled
>>> master, but it is conceivably possible to have a setup with two
>>> software-controlled masters on the same SPMI bus.
>>>
>>> This necessarily means that the description of the slaves and the
>>> masters will need to be decoupled; I'm imagining a generic binding
>>> supporting multiple masters would look something like this:
>>
>> Is there a need to represent the other masters in the DT? Sure they're
>> there in HW, but if there's no specific way for the
>> CPU-to-which-the-DT-applies to actually interact with those other
>> masters (except perhaps by experiencing some arbitration delays) then
>> presumably there's no need to represent the other masters in DT?
> 
> My example is contrived, but there is nothing in the SPMI spec
> preventing two masters from being controllable by the same
> CPU-to-which-the-DT-applies, sharing the same underlying bus.

That's true.

> I would also expect this configuration to be uncommon, I'm checking with
> some folks with more SPMI experience to make sure they agree.
> 
> Interestingly, i2c as far as I can tell has also made the same
> assumption.  There doesn't appear to be any way to express a
> multi-master i2c setup where both masters are controlled by the same OS.

Yes, I think it's a fair assumption that we don't need to represent
that; I immediately thought about the I2C counter-example after reading
your first paragraph.

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

* Re: [PATCH RFC v2 2/5] spmi: Linux driver framework for SPMI
  2013-08-23 16:06       ` Josh Cartwright
@ 2013-09-09 15:52         ` Mark Brown
  -1 siblings, 0 replies; 37+ messages in thread
From: Mark Brown @ 2013-09-09 15:52 UTC (permalink / raw)
  To: Josh Cartwright
  Cc: Greg Kroah-Hartman, Grant Likely, Rob Herring, linux-kernel,
	linux-arm-kernel, linux-arm-msm, Sagar Dharia, Gilad Avidov,
	Michael Bohan

[-- Attachment #1: Type: text/plain, Size: 1394 bytes --]

On Fri, Aug 23, 2013 at 11:06:25AM -0500, Josh Cartwright wrote:
> On Thu, Aug 22, 2013 at 04:10:54PM -0700, Greg Kroah-Hartman wrote:
> > On Fri, Aug 09, 2013 at 01:37:09PM -0700, Josh Cartwright wrote:

> > > +	"Example: Read 4 bytes starting at register address 0x1234 for SID 2\n"
> > > +	"\n"
> > > +	"echo 0x21234 > address\n"
> > > +	"echo 4 > count\n"
> > > +	"cat data\n"
> > > +	"\n"
> > > +	"Example: Write 3 bytes starting at register address 0x1008 for SID 1\n"
> > > +	"\n"
> > > +	"echo 0x11008 > address\n"
> > > +	"echo 0x01 0x02 0x03 > data\n"
> > > +	"\n"
> > > +	"Note that the count file is not used for writes.  Since 3 bytes are\n"
> > > +	"written to the 'data' file, then 3 bytes will be written across the\n"
> > > +	"SPMI bus.\n\n";

> > The help file within the kernel is a nice touch :)

> > Or is this only for "debugging"?  If so, please document it as such.

> It's there because it provides a useful interface for debugging of the
> controller code, and for simple peek/poke of the slave registers without
> having a full driver in place.  Will document this.

This looks awfully like a version of the debugfs interfaces that regmap
provides, and indeed the entire bus sounds like something that could
idiomatically be supported via regmap.  Have you considered doing that?
This would give access to standard tracepoints as well, plus the cache
infrastructure.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

* [PATCH RFC v2 2/5] spmi: Linux driver framework for SPMI
@ 2013-09-09 15:52         ` Mark Brown
  0 siblings, 0 replies; 37+ messages in thread
From: Mark Brown @ 2013-09-09 15:52 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Aug 23, 2013 at 11:06:25AM -0500, Josh Cartwright wrote:
> On Thu, Aug 22, 2013 at 04:10:54PM -0700, Greg Kroah-Hartman wrote:
> > On Fri, Aug 09, 2013 at 01:37:09PM -0700, Josh Cartwright wrote:

> > > +	"Example: Read 4 bytes starting at register address 0x1234 for SID 2\n"
> > > +	"\n"
> > > +	"echo 0x21234 > address\n"
> > > +	"echo 4 > count\n"
> > > +	"cat data\n"
> > > +	"\n"
> > > +	"Example: Write 3 bytes starting at register address 0x1008 for SID 1\n"
> > > +	"\n"
> > > +	"echo 0x11008 > address\n"
> > > +	"echo 0x01 0x02 0x03 > data\n"
> > > +	"\n"
> > > +	"Note that the count file is not used for writes.  Since 3 bytes are\n"
> > > +	"written to the 'data' file, then 3 bytes will be written across the\n"
> > > +	"SPMI bus.\n\n";

> > The help file within the kernel is a nice touch :)

> > Or is this only for "debugging"?  If so, please document it as such.

> It's there because it provides a useful interface for debugging of the
> controller code, and for simple peek/poke of the slave registers without
> having a full driver in place.  Will document this.

This looks awfully like a version of the debugfs interfaces that regmap
provides, and indeed the entire bus sounds like something that could
idiomatically be supported via regmap.  Have you considered doing that?
This would give access to standard tracepoints as well, plus the cache
infrastructure.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130909/f9e563f8/attachment-0001.sig>

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

* Re: [PATCH RFC v2 2/5] spmi: Linux driver framework for SPMI
  2013-09-09 15:52         ` Mark Brown
@ 2013-09-09 16:56           ` Josh Cartwright
  -1 siblings, 0 replies; 37+ messages in thread
From: Josh Cartwright @ 2013-09-09 16:56 UTC (permalink / raw)
  To: Mark Brown
  Cc: Greg Kroah-Hartman, Grant Likely, Rob Herring, linux-kernel,
	linux-arm-kernel, linux-arm-msm, Sagar Dharia, Gilad Avidov,
	Michael Bohan

Hey Mark-

Thanks for the comments.

On Mon, Sep 09, 2013 at 04:52:24PM +0100, Mark Brown wrote:
> On Fri, Aug 23, 2013 at 11:06:25AM -0500, Josh Cartwright wrote:
> > On Thu, Aug 22, 2013 at 04:10:54PM -0700, Greg Kroah-Hartman wrote:
> > > On Fri, Aug 09, 2013 at 01:37:09PM -0700, Josh Cartwright wrote:
> 
> > > > +	"Example: Read 4 bytes starting at register address 0x1234 for SID 2\n"
> > > > +	"\n"
> > > > +	"echo 0x21234 > address\n"
> > > > +	"echo 4 > count\n"
> > > > +	"cat data\n"
> > > > +	"\n"
> > > > +	"Example: Write 3 bytes starting at register address 0x1008 for SID 1\n"
> > > > +	"\n"
> > > > +	"echo 0x11008 > address\n"
> > > > +	"echo 0x01 0x02 0x03 > data\n"
> > > > +	"\n"
> > > > +	"Note that the count file is not used for writes.  Since 3 bytes are\n"
> > > > +	"written to the 'data' file, then 3 bytes will be written across the\n"
> > > > +	"SPMI bus.\n\n";
> 
> > > The help file within the kernel is a nice touch :)
> 
> > > Or is this only for "debugging"?  If so, please document it as such.
> 
> > It's there because it provides a useful interface for debugging of the
> > controller code, and for simple peek/poke of the slave registers without
> > having a full driver in place.  Will document this.
> 
> This looks awfully like a version of the debugfs interfaces that regmap
> provides, and indeed the entire bus sounds like something that could
> idiomatically be supported via regmap.  Have you considered doing that?
> This would give access to standard tracepoints as well, plus the cache
> infrastructure.

It does indeed look like regmap might work out nicely for SPMI.  I
hadn't seriously considered it just due to a lack of familiarity. I'll
give it a shot and see if I run into any problems.

Thanks,
  Josh

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

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

* [PATCH RFC v2 2/5] spmi: Linux driver framework for SPMI
@ 2013-09-09 16:56           ` Josh Cartwright
  0 siblings, 0 replies; 37+ messages in thread
From: Josh Cartwright @ 2013-09-09 16:56 UTC (permalink / raw)
  To: linux-arm-kernel

Hey Mark-

Thanks for the comments.

On Mon, Sep 09, 2013 at 04:52:24PM +0100, Mark Brown wrote:
> On Fri, Aug 23, 2013 at 11:06:25AM -0500, Josh Cartwright wrote:
> > On Thu, Aug 22, 2013 at 04:10:54PM -0700, Greg Kroah-Hartman wrote:
> > > On Fri, Aug 09, 2013 at 01:37:09PM -0700, Josh Cartwright wrote:
> 
> > > > +	"Example: Read 4 bytes starting at register address 0x1234 for SID 2\n"
> > > > +	"\n"
> > > > +	"echo 0x21234 > address\n"
> > > > +	"echo 4 > count\n"
> > > > +	"cat data\n"
> > > > +	"\n"
> > > > +	"Example: Write 3 bytes starting at register address 0x1008 for SID 1\n"
> > > > +	"\n"
> > > > +	"echo 0x11008 > address\n"
> > > > +	"echo 0x01 0x02 0x03 > data\n"
> > > > +	"\n"
> > > > +	"Note that the count file is not used for writes.  Since 3 bytes are\n"
> > > > +	"written to the 'data' file, then 3 bytes will be written across the\n"
> > > > +	"SPMI bus.\n\n";
> 
> > > The help file within the kernel is a nice touch :)
> 
> > > Or is this only for "debugging"?  If so, please document it as such.
> 
> > It's there because it provides a useful interface for debugging of the
> > controller code, and for simple peek/poke of the slave registers without
> > having a full driver in place.  Will document this.
> 
> This looks awfully like a version of the debugfs interfaces that regmap
> provides, and indeed the entire bus sounds like something that could
> idiomatically be supported via regmap.  Have you considered doing that?
> This would give access to standard tracepoints as well, plus the cache
> infrastructure.

It does indeed look like regmap might work out nicely for SPMI.  I
hadn't seriously considered it just due to a lack of familiarity. I'll
give it a shot and see if I run into any problems.

Thanks,
  Josh

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

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

* Re: [PATCH RFC v2 3/5] spmi: add generic SPMI controller binding documentation
  2013-08-27 21:55         ` Stephen Warren
@ 2013-10-06  6:11           ` Bjorn Andersson
  -1 siblings, 0 replies; 37+ messages in thread
From: Bjorn Andersson @ 2013-10-06  6:11 UTC (permalink / raw)
  To: Stephen Warren
  Cc: Josh Cartwright, Grant Likely, Rob Herring, Greg Kroah-Hartman,
	Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, linux-kernel,
	linux-arm-kernel, linux-arm-msm, Sagar Dharia, Gilad Avidov,
	Michael Bohan, devicetree

On Tue, Aug 27, 2013 at 2:55 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 08/27/2013 11:01 AM, Josh Cartwright wrote:
> ...
> cell 0 - address type (0: master, 1: unique ID, 2: group ID, ...)

I think it would make sense to have the master id as a property of the
bus, as you could consider this to indicate different buses and then
usid, gsid and base being part of the reg.

> cell 1 - address value

I did hack up Josh patchset to read a reg touple of <usid, base>
instead of just usid. I stored the second value in the spmi_device
struct for easy access, but maybe it should be done like on
codeaurora; in a resource?
I believe this looks nice, but as I haven't read the mipi spec I
wonder, will there be a case where you don't have an offset/base?
Should it just be made optional?

Can we make the address <usid, [base]> and have the code populate a
resource based on a reg-names property? That way it would be possible
to extend it to support gsid in case we want to (would require
reg-names though).


With the hack to Josh's patchset I quickly ported qpnp-revision and
qpnp-vibrator, and it seems to work quite nicely.

Regards,
Bjorn

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

* [PATCH RFC v2 3/5] spmi: add generic SPMI controller binding documentation
@ 2013-10-06  6:11           ` Bjorn Andersson
  0 siblings, 0 replies; 37+ messages in thread
From: Bjorn Andersson @ 2013-10-06  6:11 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Aug 27, 2013 at 2:55 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 08/27/2013 11:01 AM, Josh Cartwright wrote:
> ...
> cell 0 - address type (0: master, 1: unique ID, 2: group ID, ...)

I think it would make sense to have the master id as a property of the
bus, as you could consider this to indicate different buses and then
usid, gsid and base being part of the reg.

> cell 1 - address value

I did hack up Josh patchset to read a reg touple of <usid, base>
instead of just usid. I stored the second value in the spmi_device
struct for easy access, but maybe it should be done like on
codeaurora; in a resource?
I believe this looks nice, but as I haven't read the mipi spec I
wonder, will there be a case where you don't have an offset/base?
Should it just be made optional?

Can we make the address <usid, [base]> and have the code populate a
resource based on a reg-names property? That way it would be possible
to extend it to support gsid in case we want to (would require
reg-names though).


With the hack to Josh's patchset I quickly ported qpnp-revision and
qpnp-vibrator, and it seems to work quite nicely.

Regards,
Bjorn

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

* Re: [PATCH RFC v2 3/5] spmi: add generic SPMI controller binding documentation
  2013-10-06  6:11           ` Bjorn Andersson
  (?)
@ 2013-10-07 21:17               ` Josh Cartwright
  -1 siblings, 0 replies; 37+ messages in thread
From: Josh Cartwright @ 2013-10-07 21:17 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Stephen Warren, Grant Likely, Rob Herring, Greg Kroah-Hartman,
	Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, Sagar Dharia, Gilad Avidov,
	Michael Bohan, devicetree-u79uwXL29TY76Z2rM5mHXA

Hey Bjorn-

On Sat, Oct 05, 2013 at 11:11:36PM -0700, Bjorn Andersson wrote:
> On Tue, Aug 27, 2013 at 2:55 PM, Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> wrote:
> > On 08/27/2013 11:01 AM, Josh Cartwright wrote:
> > ...
> > cell 0 - address type (0: master, 1: unique ID, 2: group ID, ...)
>
> I think it would make sense to have the master id as a property of the
> bus,

Except that SPMI supports bus configurations with multiple masters.
Unless by 'bus' you meant 'bus controller' here?

> as you could consider this to indicate different buses and then
> usid, gsid and base being part of the reg.
>
> > cell 1 - address value
>
> I did hack up Josh patchset to read a reg touple of <usid, base>
> instead of just usid. I stored the second value in the spmi_device
> struct for easy access, but maybe it should be done like on
> codeaurora; in a resource?
> I believe this looks nice, but as I haven't read the mipi spec I
> wonder, will there be a case where you don't have an offset/base?
> Should it just be made optional?

The SPMI spec says nothing about partitioning up the slave address to
support multiple functions.  AFAICT, this is a Qualcomm-created
construct (QPNP) for the 8x41 PMICs.  It's difficult to tell at this
point whether or not other vendors might implement a similarly
partitioned scheme.

I suspect the intent is that implementations make use of logical slave
ID for each function in a multi-function device.

> Can we make the address <usid, [base]> and have the code populate a
> resource based on a reg-names property? That way it would be possible
> to extend it to support gsid in case we want to (would require
> reg-names though).

It is certainly possible, and, as you've seen, is how the current
codeaurora.org tree implements SPMI.  But, I'm actually actively trying
to avoid doing so, as it conflates Qualcomm-implementation details and
what is actually in the spec (and not just the address space
partitioning, but also the of_spmi.c[2] parsing must know about
interrupts, which are _also_ completely outside the SPMI spec).

Instead what I hope to do for v3 is either:
  A) Make QPNP its own bus type (for which I have a prototyped
     implementation).  A PMIC driver sits on the SPMI bus and registers
     itself as a QPNP controller.  QPNP controllers have very
     simple 8-bit register read/write operations used by QPNP devices.
  B) Effectively the same as A, but gets rid of a special QPNP bus type
     and uses mfd/platform devices, similar to other in-tree PMIC drivers
     (currently working on prototyping this approach)

> With the hack to Josh's patchset I quickly ported qpnp-revision and
> qpnp-vibrator, and it seems to work quite nicely.

Great! Thanks for testing.

  Josh

1: https://www.codeaurora.org/cgit/quic/la/kernel/msm-3.10/tree/drivers/spmi
2: https://www.codeaurora.org/cgit/quic/la/kernel/msm-3.10/tree/drivers/of/of_spmi.c

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH RFC v2 3/5] spmi: add generic SPMI controller binding documentation
@ 2013-10-07 21:17               ` Josh Cartwright
  0 siblings, 0 replies; 37+ messages in thread
From: Josh Cartwright @ 2013-10-07 21:17 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Stephen Warren, Grant Likely, Rob Herring, Greg Kroah-Hartman,
	Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, linux-kernel,
	linux-arm-kernel, linux-arm-msm, Sagar Dharia, Gilad Avidov,
	Michael Bohan, devicetree

Hey Bjorn-

On Sat, Oct 05, 2013 at 11:11:36PM -0700, Bjorn Andersson wrote:
> On Tue, Aug 27, 2013 at 2:55 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> > On 08/27/2013 11:01 AM, Josh Cartwright wrote:
> > ...
> > cell 0 - address type (0: master, 1: unique ID, 2: group ID, ...)
>
> I think it would make sense to have the master id as a property of the
> bus,

Except that SPMI supports bus configurations with multiple masters.
Unless by 'bus' you meant 'bus controller' here?

> as you could consider this to indicate different buses and then
> usid, gsid and base being part of the reg.
>
> > cell 1 - address value
>
> I did hack up Josh patchset to read a reg touple of <usid, base>
> instead of just usid. I stored the second value in the spmi_device
> struct for easy access, but maybe it should be done like on
> codeaurora; in a resource?
> I believe this looks nice, but as I haven't read the mipi spec I
> wonder, will there be a case where you don't have an offset/base?
> Should it just be made optional?

The SPMI spec says nothing about partitioning up the slave address to
support multiple functions.  AFAICT, this is a Qualcomm-created
construct (QPNP) for the 8x41 PMICs.  It's difficult to tell at this
point whether or not other vendors might implement a similarly
partitioned scheme.

I suspect the intent is that implementations make use of logical slave
ID for each function in a multi-function device.

> Can we make the address <usid, [base]> and have the code populate a
> resource based on a reg-names property? That way it would be possible
> to extend it to support gsid in case we want to (would require
> reg-names though).

It is certainly possible, and, as you've seen, is how the current
codeaurora.org tree implements SPMI.  But, I'm actually actively trying
to avoid doing so, as it conflates Qualcomm-implementation details and
what is actually in the spec (and not just the address space
partitioning, but also the of_spmi.c[2] parsing must know about
interrupts, which are _also_ completely outside the SPMI spec).

Instead what I hope to do for v3 is either:
  A) Make QPNP its own bus type (for which I have a prototyped
     implementation).  A PMIC driver sits on the SPMI bus and registers
     itself as a QPNP controller.  QPNP controllers have very
     simple 8-bit register read/write operations used by QPNP devices.
  B) Effectively the same as A, but gets rid of a special QPNP bus type
     and uses mfd/platform devices, similar to other in-tree PMIC drivers
     (currently working on prototyping this approach)

> With the hack to Josh's patchset I quickly ported qpnp-revision and
> qpnp-vibrator, and it seems to work quite nicely.

Great! Thanks for testing.

  Josh

1: https://www.codeaurora.org/cgit/quic/la/kernel/msm-3.10/tree/drivers/spmi
2: https://www.codeaurora.org/cgit/quic/la/kernel/msm-3.10/tree/drivers/of/of_spmi.c

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

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

* [PATCH RFC v2 3/5] spmi: add generic SPMI controller binding documentation
@ 2013-10-07 21:17               ` Josh Cartwright
  0 siblings, 0 replies; 37+ messages in thread
From: Josh Cartwright @ 2013-10-07 21:17 UTC (permalink / raw)
  To: linux-arm-kernel

Hey Bjorn-

On Sat, Oct 05, 2013 at 11:11:36PM -0700, Bjorn Andersson wrote:
> On Tue, Aug 27, 2013 at 2:55 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> > On 08/27/2013 11:01 AM, Josh Cartwright wrote:
> > ...
> > cell 0 - address type (0: master, 1: unique ID, 2: group ID, ...)
>
> I think it would make sense to have the master id as a property of the
> bus,

Except that SPMI supports bus configurations with multiple masters.
Unless by 'bus' you meant 'bus controller' here?

> as you could consider this to indicate different buses and then
> usid, gsid and base being part of the reg.
>
> > cell 1 - address value
>
> I did hack up Josh patchset to read a reg touple of <usid, base>
> instead of just usid. I stored the second value in the spmi_device
> struct for easy access, but maybe it should be done like on
> codeaurora; in a resource?
> I believe this looks nice, but as I haven't read the mipi spec I
> wonder, will there be a case where you don't have an offset/base?
> Should it just be made optional?

The SPMI spec says nothing about partitioning up the slave address to
support multiple functions.  AFAICT, this is a Qualcomm-created
construct (QPNP) for the 8x41 PMICs.  It's difficult to tell at this
point whether or not other vendors might implement a similarly
partitioned scheme.

I suspect the intent is that implementations make use of logical slave
ID for each function in a multi-function device.

> Can we make the address <usid, [base]> and have the code populate a
> resource based on a reg-names property? That way it would be possible
> to extend it to support gsid in case we want to (would require
> reg-names though).

It is certainly possible, and, as you've seen, is how the current
codeaurora.org tree implements SPMI.  But, I'm actually actively trying
to avoid doing so, as it conflates Qualcomm-implementation details and
what is actually in the spec (and not just the address space
partitioning, but also the of_spmi.c[2] parsing must know about
interrupts, which are _also_ completely outside the SPMI spec).

Instead what I hope to do for v3 is either:
  A) Make QPNP its own bus type (for which I have a prototyped
     implementation).  A PMIC driver sits on the SPMI bus and registers
     itself as a QPNP controller.  QPNP controllers have very
     simple 8-bit register read/write operations used by QPNP devices.
  B) Effectively the same as A, but gets rid of a special QPNP bus type
     and uses mfd/platform devices, similar to other in-tree PMIC drivers
     (currently working on prototyping this approach)

> With the hack to Josh's patchset I quickly ported qpnp-revision and
> qpnp-vibrator, and it seems to work quite nicely.

Great! Thanks for testing.

  Josh

1: https://www.codeaurora.org/cgit/quic/la/kernel/msm-3.10/tree/drivers/spmi
2: https://www.codeaurora.org/cgit/quic/la/kernel/msm-3.10/tree/drivers/of/of_spmi.c

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

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

end of thread, other threads:[~2013-10-07 21:18 UTC | newest]

Thread overview: 37+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-08-22 20:18 [PATCH RFC v2 0/3] Add support for the System Power Management Interface (SPMI) Josh Cartwright
2013-08-22 20:18 ` Josh Cartwright
2012-12-10 19:41 ` [PATCH RFC v2 1/5] of: Add empty for_each_available_child_of_node() macro definition Josh Cartwright
2013-08-22 22:57   ` Josh Cartwright
2013-08-09 20:37 ` [PATCH RFC v2 4/5] spmi: Add MSM PMIC Arbiter SPMI controller Josh Cartwright
2013-08-09 20:37   ` Josh Cartwright
2013-08-09 20:37 ` [PATCH RFC v2 5/5] spmi: document the PMIC arbiter SPMI bindings Josh Cartwright
2013-08-09 20:37   ` Josh Cartwright
2013-08-23 21:55   ` Stephen Warren
2013-08-23 21:55     ` Stephen Warren
2013-08-09 20:37 ` [PATCH RFC v2 2/5] spmi: Linux driver framework for SPMI Josh Cartwright
2013-08-09 20:37   ` Josh Cartwright
2013-08-22 23:10   ` Greg Kroah-Hartman
2013-08-22 23:10     ` Greg Kroah-Hartman
2013-08-23 16:06     ` Josh Cartwright
2013-08-23 16:06       ` Josh Cartwright
2013-09-09 15:52       ` Mark Brown
2013-09-09 15:52         ` Mark Brown
2013-09-09 16:56         ` Josh Cartwright
2013-09-09 16:56           ` Josh Cartwright
2013-08-22 19:59 ` [PATCH RFC v2 3/5] spmi: add generic SPMI controller binding documentation Josh Cartwright
2013-08-22 19:59   ` Josh Cartwright
2013-08-23 21:58   ` Stephen Warren
2013-08-23 21:58     ` Stephen Warren
2013-08-27 17:01     ` Josh Cartwright
2013-08-27 17:01       ` Josh Cartwright
2013-08-27 21:55       ` Stephen Warren
2013-08-27 21:55         ` Stephen Warren
2013-08-28 18:00         ` Josh Cartwright
2013-08-28 18:00           ` Josh Cartwright
2013-08-28 18:32           ` Stephen Warren
2013-08-28 18:32             ` Stephen Warren
2013-10-06  6:11         ` Bjorn Andersson
2013-10-06  6:11           ` Bjorn Andersson
     [not found]           ` <CAJAp7Oi-bPytsLtsppdanOi_p0Y5vfBriGB-B5by7w5Z7SGU-Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2013-10-07 21:17             ` Josh Cartwright
2013-10-07 21:17               ` Josh Cartwright
2013-10-07 21:17               ` Josh Cartwright

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.