linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/6] Qualcomm SSBI bus driver
@ 2013-03-07  0:29 David Brown
  2013-03-07  0:29 ` [PATCH 1/6] platform-drivers: msm: add single-wire serial bus interface (SSBI) driver David Brown
                   ` (7 more replies)
  0 siblings, 8 replies; 33+ messages in thread
From: David Brown @ 2013-03-07  0:29 UTC (permalink / raw)
  To: linux-arm-kernel

This small series adds the Qualcomm SSBI bus driver.  The original
driver was developed as part of Android.  Kenneth Heitke updated this,
and sent it out a while back.  This series updates this driver to use
DeviceTree, and fixes a few things that have changed since it was
originally sent out.

In addition, the 6th patch, the RFC, is a small test driver that reads
the version register off of the pm8058 device typically connected to
an MSM8660 or MSM8960.  This is primarily useful to anyone wanting to
work on these pmic drivers.

Updates to the PMIC drivers themselves to use this driver to come.

David

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

* [PATCH 1/6] platform-drivers: msm: add single-wire serial bus interface (SSBI) driver
  2013-03-07  0:29 [PATCH 0/6] Qualcomm SSBI bus driver David Brown
@ 2013-03-07  0:29 ` David Brown
  2013-03-07  1:30   ` Greg Kroah-Hartman
  2013-03-07  0:29 ` [PATCH 2/6] SSBI: Convert SSBI to device tree David Brown
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 33+ messages in thread
From: David Brown @ 2013-03-07  0:29 UTC (permalink / raw)
  To: linux-arm-kernel

From: Kenneth Heitke <kheitke@codeaurora.org>

SSBI is the Qualcomm single-wire serial bus interface used to connect
the MSM devices to the PMIC and other devices.

Since SSBI only supports a single slave, the driver gets the name of the
slave device passed in from the board file through the master device's
platform data.

SSBI registers pretty early (postcore), so that the PMIC can come up
before the board init. This is useful if the board init requires the
use of gpios that are connected through the PMIC.

Based on a patch by Dima Zavin <dima@android.com> that can be found at:
http://android.git.kernel.org/?p=kernel/msm.git;a=commitdiff;h=eb060bac4

This patch adds PMIC Arbiter support for the MSM8660. The PMIC Arbiter
is a hardware wrapper around the SSBI 2.0 controller that is designed to
overcome concurrency issues and security limitations.  A controller_type
field is added to the platform data to specify the type of the SSBI
controller (1.0, 2.0, or PMIC Arbiter).

[davidb at codeaurora.org:
 I've moved this driver into drivers/ssbi/ and added an include for
 linux/module.h so that it will compile]

Signed-off-by: Kenneth Heitke <kheitke@codeaurora.org>
Signed-off-by: David Brown <davidb@codeaurora.org>
---
 drivers/Kconfig          |   2 +
 drivers/Makefile         |   1 +
 drivers/ssbi/Kconfig     |  17 ++
 drivers/ssbi/Makefile    |   4 +
 drivers/ssbi/ssbi.c      | 397 +++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/msm_ssbi.h |  49 ++++++
 6 files changed, 470 insertions(+)
 create mode 100644 drivers/ssbi/Kconfig
 create mode 100644 drivers/ssbi/Makefile
 create mode 100644 drivers/ssbi/ssbi.c
 create mode 100644 include/linux/msm_ssbi.h

diff --git a/drivers/Kconfig b/drivers/Kconfig
index 202fa6d..78a956e 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -52,6 +52,8 @@ source "drivers/i2c/Kconfig"
 
 source "drivers/spi/Kconfig"
 
+source "drivers/ssbi/Kconfig"
+
 source "drivers/hsi/Kconfig"
 
 source "drivers/pps/Kconfig"
diff --git a/drivers/Makefile b/drivers/Makefile
index dce39a9..778821b 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -114,6 +114,7 @@ obj-y				+= firmware/
 obj-$(CONFIG_CRYPTO)		+= crypto/
 obj-$(CONFIG_SUPERH)		+= sh/
 obj-$(CONFIG_ARCH_SHMOBILE)	+= sh/
+obj-$(CONFIG_MSM_SSBI)		+= ssbi/
 ifndef CONFIG_ARCH_USES_GETTIMEOFFSET
 obj-y				+= clocksource/
 endif
diff --git a/drivers/ssbi/Kconfig b/drivers/ssbi/Kconfig
new file mode 100644
index 0000000..b667e62
--- /dev/null
+++ b/drivers/ssbi/Kconfig
@@ -0,0 +1,17 @@
+#
+# MSM SSBI bus support
+#
+
+menu "Qualcomm MSM SSBI bus support"
+	depends on ARCH_MSM
+
+config MSM_SSBI
+	bool "Qualcomm Single-wire Serial Bus Interface (SSBI)"
+	help
+	  If you say yes to this option, support will be included for the
+	  built-in SSBI interface on Qualcomm MSM family processors.
+
+	  This is required for communicating with Qualcomm PMICs and
+	  other devices that have the SSBI interface.
+
+endmenu
diff --git a/drivers/ssbi/Makefile b/drivers/ssbi/Makefile
new file mode 100644
index 0000000..ea8c1e4
--- /dev/null
+++ b/drivers/ssbi/Makefile
@@ -0,0 +1,4 @@
+#
+# Makefile for the MSM specific device drivers.
+#
+obj-$(CONFIG_MSM_SSBI) += ssbi.o
diff --git a/drivers/ssbi/ssbi.c b/drivers/ssbi/ssbi.c
new file mode 100644
index 0000000..8b0b10d
--- /dev/null
+++ b/drivers/ssbi/ssbi.c
@@ -0,0 +1,397 @@
+/* Copyright (c) 2009-2011, Code Aurora Forum. All rights reserved.
+ * Copyright (c) 2010, Google Inc.
+ *
+ * Original authors: Code Aurora Forum
+ *
+ * Author: Dima Zavin <dima@android.com>
+ *  - Largely rewritten from original to not be an i2c driver.
+ *
+ * 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.
+ */
+
+#define pr_fmt(fmt) "%s: " fmt, __func__
+
+#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/msm_ssbi.h>
+#include <linux/module.h>
+
+/* SSBI 2.0 controller registers */
+#define SSBI2_CMD			0x0008
+#define SSBI2_RD			0x0010
+#define SSBI2_STATUS			0x0014
+#define SSBI2_MODE2			0x001C
+
+/* SSBI_CMD fields */
+#define SSBI_CMD_RDWRN			(1 << 24)
+
+/* SSBI_STATUS fields */
+#define SSBI_STATUS_RD_READY		(1 << 2)
+#define SSBI_STATUS_READY		(1 << 1)
+#define SSBI_STATUS_MCHN_BUSY		(1 << 0)
+
+/* SSBI_MODE2 fields */
+#define SSBI_MODE2_REG_ADDR_15_8_SHFT	0x04
+#define SSBI_MODE2_REG_ADDR_15_8_MASK	(0x7f << SSBI_MODE2_REG_ADDR_15_8_SHFT)
+
+#define SET_SSBI_MODE2_REG_ADDR_15_8(MD, AD) \
+	(((MD) & 0x0F) | ((((AD) >> 8) << SSBI_MODE2_REG_ADDR_15_8_SHFT) & \
+	SSBI_MODE2_REG_ADDR_15_8_MASK))
+
+/* SSBI PMIC Arbiter command registers */
+#define SSBI_PA_CMD			0x0000
+#define SSBI_PA_RD_STATUS		0x0004
+
+/* SSBI_PA_CMD fields */
+#define SSBI_PA_CMD_RDWRN		(1 << 24)
+#define SSBI_PA_CMD_ADDR_MASK		0x7fff /* REG_ADDR_7_0, REG_ADDR_8_14*/
+
+/* SSBI_PA_RD_STATUS fields */
+#define SSBI_PA_RD_STATUS_TRANS_DONE	(1 << 27)
+#define SSBI_PA_RD_STATUS_TRANS_DENIED	(1 << 26)
+
+#define SSBI_TIMEOUT_US			100
+
+struct msm_ssbi {
+	struct device		*dev;
+	struct device		*slave;
+	void __iomem		*base;
+	spinlock_t		lock;
+	enum msm_ssbi_controller_type controller_type;
+	int (*read)(struct msm_ssbi *, u16 addr, u8 *buf, int len);
+	int (*write)(struct msm_ssbi *, u16 addr, u8 *buf, int len);
+};
+
+#define to_msm_ssbi(dev)	platform_get_drvdata(to_platform_device(dev))
+
+static inline u32 ssbi_readl(struct msm_ssbi *ssbi, u32 reg)
+{
+	return readl(ssbi->base + reg);
+}
+
+static inline void ssbi_writel(struct msm_ssbi *ssbi, u32 val, u32 reg)
+{
+	writel(val, ssbi->base + reg);
+}
+
+static int ssbi_wait_mask(struct msm_ssbi *ssbi, u32 set_mask, u32 clr_mask)
+{
+	u32 timeout = SSBI_TIMEOUT_US;
+	u32 val;
+
+	while (timeout--) {
+		val = ssbi_readl(ssbi, SSBI2_STATUS);
+		if (((val & set_mask) == set_mask) && ((val & clr_mask) == 0))
+			return 0;
+		udelay(1);
+	}
+
+	dev_err(ssbi->dev, "%s: timeout (status %x set_mask %x clr_mask %x)\n",
+		__func__, ssbi_readl(ssbi, SSBI2_STATUS), set_mask, clr_mask);
+	return -ETIMEDOUT;
+}
+
+static int
+msm_ssbi_read_bytes(struct msm_ssbi *ssbi, u16 addr, u8 *buf, int len)
+{
+	u32 cmd = SSBI_CMD_RDWRN | ((addr & 0xff) << 16);
+	int ret = 0;
+
+	if (ssbi->controller_type == MSM_SBI_CTRL_SSBI2) {
+		u32 mode2 = ssbi_readl(ssbi, SSBI2_MODE2);
+		mode2 = SET_SSBI_MODE2_REG_ADDR_15_8(mode2, addr);
+		ssbi_writel(ssbi, mode2, SSBI2_MODE2);
+	}
+
+	while (len) {
+		ret = ssbi_wait_mask(ssbi, SSBI_STATUS_READY, 0);
+		if (ret)
+			goto err;
+
+		ssbi_writel(ssbi, cmd, SSBI2_CMD);
+		ret = ssbi_wait_mask(ssbi, SSBI_STATUS_RD_READY, 0);
+		if (ret)
+			goto err;
+		*buf++ = ssbi_readl(ssbi, SSBI2_RD) & 0xff;
+		len--;
+	}
+
+err:
+	return ret;
+}
+
+static int
+msm_ssbi_write_bytes(struct msm_ssbi *ssbi, u16 addr, u8 *buf, int len)
+{
+	int ret = 0;
+
+	if (ssbi->controller_type == MSM_SBI_CTRL_SSBI2) {
+		u32 mode2 = ssbi_readl(ssbi, SSBI2_MODE2);
+		mode2 = SET_SSBI_MODE2_REG_ADDR_15_8(mode2, addr);
+		ssbi_writel(ssbi, mode2, SSBI2_MODE2);
+	}
+
+	while (len) {
+		ret = ssbi_wait_mask(ssbi, SSBI_STATUS_READY, 0);
+		if (ret)
+			goto err;
+
+		ssbi_writel(ssbi, ((addr & 0xff) << 16) | *buf, SSBI2_CMD);
+		ret = ssbi_wait_mask(ssbi, 0, SSBI_STATUS_MCHN_BUSY);
+		if (ret)
+			goto err;
+		buf++;
+		len--;
+	}
+
+err:
+	return ret;
+}
+
+static inline int
+msm_ssbi_pa_transfer(struct msm_ssbi *ssbi, u32 cmd, u8 *data)
+{
+	u32 timeout = SSBI_TIMEOUT_US;
+	u32 rd_status = 0;
+
+	ssbi_writel(ssbi, cmd, SSBI_PA_CMD);
+
+	while (timeout--) {
+		rd_status = ssbi_readl(ssbi, SSBI_PA_RD_STATUS);
+
+		if (rd_status & SSBI_PA_RD_STATUS_TRANS_DENIED) {
+			dev_err(ssbi->dev, "%s: transaction denied (0x%x)\n",
+					__func__, rd_status);
+			return -EPERM;
+		}
+
+		if (rd_status & SSBI_PA_RD_STATUS_TRANS_DONE) {
+			if (data)
+				*data = rd_status & 0xff;
+			return 0;
+		}
+		udelay(1);
+	}
+
+	dev_err(ssbi->dev, "%s: timeout, status 0x%x\n", __func__, rd_status);
+	return -ETIMEDOUT;
+}
+
+static int
+msm_ssbi_pa_read_bytes(struct msm_ssbi *ssbi, u16 addr, u8 *buf, int len)
+{
+	u32 cmd;
+	int ret = 0;
+
+	cmd = SSBI_PA_CMD_RDWRN | (addr & SSBI_PA_CMD_ADDR_MASK) << 8;
+
+	while (len) {
+		ret = msm_ssbi_pa_transfer(ssbi, cmd, buf);
+		if (ret)
+			goto err;
+		buf++;
+		len--;
+	}
+
+err:
+	return ret;
+}
+
+static int
+msm_ssbi_pa_write_bytes(struct msm_ssbi *ssbi, u16 addr, u8 *buf, int len)
+{
+	u32 cmd;
+	int ret = 0;
+
+	while (len) {
+		cmd = (addr & SSBI_PA_CMD_ADDR_MASK) << 8 | *buf;
+		ret = msm_ssbi_pa_transfer(ssbi, cmd, NULL);
+		if (ret)
+			goto err;
+		buf++;
+		len--;
+	}
+
+err:
+	return ret;
+}
+
+int msm_ssbi_read(struct device *dev, u16 addr, u8 *buf, int len)
+{
+	struct msm_ssbi *ssbi = to_msm_ssbi(dev);
+	unsigned long flags;
+	int ret;
+
+	if (ssbi->dev != dev)
+		return -ENXIO;
+
+	spin_lock_irqsave(&ssbi->lock, flags);
+	ret = ssbi->read(ssbi, addr, buf, len);
+	spin_unlock_irqrestore(&ssbi->lock, flags);
+
+	return ret;
+}
+EXPORT_SYMBOL(msm_ssbi_read);
+
+int msm_ssbi_write(struct device *dev, u16 addr, u8 *buf, int len)
+{
+	struct msm_ssbi *ssbi = to_msm_ssbi(dev);
+	unsigned long flags;
+	int ret;
+
+	if (ssbi->dev != dev)
+		return -ENXIO;
+
+	spin_lock_irqsave(&ssbi->lock, flags);
+	ret = ssbi->write(ssbi, addr, buf, len);
+	spin_unlock_irqrestore(&ssbi->lock, flags);
+
+	return ret;
+}
+EXPORT_SYMBOL(msm_ssbi_write);
+
+static int msm_ssbi_add_slave(struct msm_ssbi *ssbi,
+				const struct msm_ssbi_slave_info *slave)
+{
+	struct platform_device *slave_pdev;
+	int ret;
+
+	if (ssbi->slave) {
+		pr_err("slave already attached??\n");
+		return -EBUSY;
+	}
+
+	slave_pdev = platform_device_alloc(slave->name, -1);
+	if (!slave_pdev) {
+		pr_err("cannot allocate pdev for slave '%s'", slave->name);
+		ret = -ENOMEM;
+		goto err;
+	}
+
+	slave_pdev->dev.parent = ssbi->dev;
+	slave_pdev->dev.platform_data = slave->platform_data;
+
+	ret = platform_device_add(slave_pdev);
+	if (ret) {
+		pr_err("cannot add slave platform device for '%s'\n",
+				slave->name);
+		goto err;
+	}
+
+	ssbi->slave = &slave_pdev->dev;
+	return 0;
+
+err:
+	if (slave_pdev)
+		platform_device_put(slave_pdev);
+	return ret;
+}
+
+static int msm_ssbi_probe(struct platform_device *pdev)
+{
+	const struct msm_ssbi_platform_data *pdata = pdev->dev.platform_data;
+	struct resource *mem_res;
+	struct msm_ssbi *ssbi;
+	int ret = 0;
+
+	if (!pdata) {
+		pr_err("missing platform data\n");
+		return -EINVAL;
+	}
+
+	pr_debug("%s\n", pdata->slave.name);
+
+	ssbi = kzalloc(sizeof(struct msm_ssbi), GFP_KERNEL);
+	if (!ssbi) {
+		pr_err("can not allocate ssbi_data\n");
+		return -ENOMEM;
+	}
+
+	mem_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!mem_res) {
+		pr_err("missing mem resource\n");
+		ret = -EINVAL;
+		goto err_get_mem_res;
+	}
+
+	ssbi->base = ioremap(mem_res->start, resource_size(mem_res));
+	if (!ssbi->base) {
+		pr_err("ioremap of 0x%p failed\n", (void *)mem_res->start);
+		ret = -EINVAL;
+		goto err_ioremap;
+	}
+	ssbi->dev = &pdev->dev;
+	platform_set_drvdata(pdev, ssbi);
+
+	ssbi->controller_type = pdata->controller_type;
+	if (ssbi->controller_type == MSM_SBI_CTRL_PMIC_ARBITER) {
+		ssbi->read = msm_ssbi_pa_read_bytes;
+		ssbi->write = msm_ssbi_pa_write_bytes;
+	} else {
+		ssbi->read = msm_ssbi_read_bytes;
+		ssbi->write = msm_ssbi_write_bytes;
+	}
+
+	spin_lock_init(&ssbi->lock);
+
+	ret = msm_ssbi_add_slave(ssbi, &pdata->slave);
+	if (ret)
+		goto err_ssbi_add_slave;
+
+	return 0;
+
+err_ssbi_add_slave:
+	platform_set_drvdata(pdev, NULL);
+	iounmap(ssbi->base);
+err_ioremap:
+err_get_mem_res:
+	kfree(ssbi);
+	return ret;
+}
+
+static int msm_ssbi_remove(struct platform_device *pdev)
+{
+	struct msm_ssbi *ssbi = platform_get_drvdata(pdev);
+
+	platform_set_drvdata(pdev, NULL);
+	iounmap(ssbi->base);
+	kfree(ssbi);
+	return 0;
+}
+
+static struct platform_driver msm_ssbi_driver = {
+	.probe		= msm_ssbi_probe,
+	.remove		= __exit_p(msm_ssbi_remove),
+	.driver		= {
+		.name	= "msm_ssbi",
+		.owner	= THIS_MODULE,
+	},
+};
+
+static int __init msm_ssbi_init(void)
+{
+	return platform_driver_register(&msm_ssbi_driver);
+}
+postcore_initcall(msm_ssbi_init);
+
+static void __exit msm_ssbi_exit(void)
+{
+	platform_driver_unregister(&msm_ssbi_driver);
+}
+module_exit(msm_ssbi_exit)
+
+MODULE_LICENSE("GPL v2");
+MODULE_VERSION("1.0");
+MODULE_ALIAS("platform:msm_ssbi");
+MODULE_AUTHOR("Dima Zavin <dima@android.com>");
diff --git a/include/linux/msm_ssbi.h b/include/linux/msm_ssbi.h
new file mode 100644
index 0000000..cfa47df
--- /dev/null
+++ b/include/linux/msm_ssbi.h
@@ -0,0 +1,49 @@
+/* Copyright (C) 2010 Google, Inc.
+ * Copyright (c) 2011, Code Aurora Forum. All rights reserved.
+ * Author: Dima Zavin <dima@android.com>
+ *
+ * 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_MSM_SSBI_H
+#define _LINUX_MSM_SSBI_H
+
+#include <linux/types.h>
+
+struct msm_ssbi_slave_info {
+	const char	*name;
+	void		*platform_data;
+};
+
+enum msm_ssbi_controller_type {
+	MSM_SBI_CTRL_SSBI = 0,
+	MSM_SBI_CTRL_SSBI2,
+	MSM_SBI_CTRL_PMIC_ARBITER,
+};
+
+struct msm_ssbi_platform_data {
+	struct msm_ssbi_slave_info	slave;
+	enum msm_ssbi_controller_type controller_type;
+};
+
+#ifdef CONFIG_MSM_SSBI
+int msm_ssbi_write(struct device *dev, u16 addr, u8 *buf, int len);
+int msm_ssbi_read(struct device *dev, u16 addr, u8 *buf, int len);
+#else
+static inline int msm_ssbi_write(struct device *dev, u16 addr, u8 *buf, int len)
+{
+	return -ENXIO;
+}
+static inline int msm_ssbi_read(struct device *dev, u16 addr, u8 *buf, int len)
+{
+	return -ENXIO;
+}
+#endif
+#endif
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

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

* [PATCH 2/6] SSBI: Convert SSBI to device tree
  2013-03-07  0:29 [PATCH 0/6] Qualcomm SSBI bus driver David Brown
  2013-03-07  0:29 ` [PATCH 1/6] platform-drivers: msm: add single-wire serial bus interface (SSBI) driver David Brown
@ 2013-03-07  0:29 ` David Brown
  2013-03-07  0:29 ` [PATCH 3/6] ssbi: Fix exit mismatch in remove function David Brown
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 33+ messages in thread
From: David Brown @ 2013-03-07  0:29 UTC (permalink / raw)
  To: linux-arm-kernel

The SSBI bus is exclusive to the Qualcomm MSM targets, and all SoCs
using it will be using device tree.  Convert this driver to indentify
with device tree.

This makes the bus probing a good bit simpler, since the attaching of
child nodes can be represented directly in the devicetree, rather than
having to be inferred by name.

Signed-off-by: David Brown <davidb@codeaurora.org>
---
 Documentation/devicetree/bindings/arm/msm/ssbi.txt | 18 +++++
 arch/arm/boot/dts/msm8660-surf.dts                 |  6 ++
 arch/arm/boot/dts/msm8960-cdp.dts                  |  6 ++
 drivers/ssbi/ssbi.c                                | 81 +++++++++-------------
 4 files changed, 62 insertions(+), 49 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/msm/ssbi.txt

diff --git a/Documentation/devicetree/bindings/arm/msm/ssbi.txt b/Documentation/devicetree/bindings/arm/msm/ssbi.txt
new file mode 100644
index 0000000..54fd5ce
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/msm/ssbi.txt
@@ -0,0 +1,18 @@
+* Qualcomm SSBI
+
+Some Qualcomm MSM devices contain a point-to-point serial bus used to
+communicate with a limited range of devices (mostly power management
+chips).
+
+These require the following properties:
+
+- compatible: "qcom,ssbi"
+
+- qcom,controller-type
+  indicates the SSBI bus variant the controller should use to talk
+  with the slave device.  This should be one of "ssbi", "ssbi2", or
+  "pmic-arbiter".  The type chosen is determined by the attached
+  slave.
+
+The slave device should be the single child node of the ssbi device
+with a compatible field.
diff --git a/arch/arm/boot/dts/msm8660-surf.dts b/arch/arm/boot/dts/msm8660-surf.dts
index 31f2157..67f8670 100644
--- a/arch/arm/boot/dts/msm8660-surf.dts
+++ b/arch/arm/boot/dts/msm8660-surf.dts
@@ -38,4 +38,10 @@
 		      <0x19c00000 0x1000>;
 		interrupts = <0 195 0x0>;
 	};
+
+	qcom,ssbi at 500000 {
+		compatible = "qcom,ssbi";
+		reg = <0x500000 0x1000>;
+		qcom,controller-type = "pmic-arbiter";
+	};
 };
diff --git a/arch/arm/boot/dts/msm8960-cdp.dts b/arch/arm/boot/dts/msm8960-cdp.dts
index 9e621b5..c9b09a8 100644
--- a/arch/arm/boot/dts/msm8960-cdp.dts
+++ b/arch/arm/boot/dts/msm8960-cdp.dts
@@ -38,4 +38,10 @@
 		      <0x16400000 0x1000>;
 		interrupts = <0 154 0x0>;
 	};
+
+	qcom,ssbi at 500000 {
+		compatible = "qcom,ssbi";
+		reg = <0x500000 0x1000>;
+		qcom,controller-type = "pmic-arbiter";
+	};
 };
diff --git a/drivers/ssbi/ssbi.c b/drivers/ssbi/ssbi.c
index 8b0b10d..86d8416 100644
--- a/drivers/ssbi/ssbi.c
+++ b/drivers/ssbi/ssbi.c
@@ -26,6 +26,8 @@
 #include <linux/slab.h>
 #include <linux/msm_ssbi.h>
 #include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
 
 /* SSBI 2.0 controller registers */
 #define SSBI2_CMD			0x0008
@@ -261,56 +263,13 @@ int msm_ssbi_write(struct device *dev, u16 addr, u8 *buf, int len)
 }
 EXPORT_SYMBOL(msm_ssbi_write);
 
-static int msm_ssbi_add_slave(struct msm_ssbi *ssbi,
-				const struct msm_ssbi_slave_info *slave)
-{
-	struct platform_device *slave_pdev;
-	int ret;
-
-	if (ssbi->slave) {
-		pr_err("slave already attached??\n");
-		return -EBUSY;
-	}
-
-	slave_pdev = platform_device_alloc(slave->name, -1);
-	if (!slave_pdev) {
-		pr_err("cannot allocate pdev for slave '%s'", slave->name);
-		ret = -ENOMEM;
-		goto err;
-	}
-
-	slave_pdev->dev.parent = ssbi->dev;
-	slave_pdev->dev.platform_data = slave->platform_data;
-
-	ret = platform_device_add(slave_pdev);
-	if (ret) {
-		pr_err("cannot add slave platform device for '%s'\n",
-				slave->name);
-		goto err;
-	}
-
-	ssbi->slave = &slave_pdev->dev;
-	return 0;
-
-err:
-	if (slave_pdev)
-		platform_device_put(slave_pdev);
-	return ret;
-}
-
 static int msm_ssbi_probe(struct platform_device *pdev)
 {
-	const struct msm_ssbi_platform_data *pdata = pdev->dev.platform_data;
+	struct device_node *np = pdev->dev.of_node;
 	struct resource *mem_res;
 	struct msm_ssbi *ssbi;
 	int ret = 0;
-
-	if (!pdata) {
-		pr_err("missing platform data\n");
-		return -EINVAL;
-	}
-
-	pr_debug("%s\n", pdata->slave.name);
+	const char *type;
 
 	ssbi = kzalloc(sizeof(struct msm_ssbi), GFP_KERNEL);
 	if (!ssbi) {
@@ -334,7 +293,25 @@ static int msm_ssbi_probe(struct platform_device *pdev)
 	ssbi->dev = &pdev->dev;
 	platform_set_drvdata(pdev, ssbi);
 
-	ssbi->controller_type = pdata->controller_type;
+	type = of_get_property(np, "qcom,controller-type", NULL);
+	if (type == NULL) {
+		pr_err("Missing qcom,controller-type property\n");
+		ret = -EINVAL;
+		goto err_ssbi_controller;
+	}
+	dev_info(&pdev->dev, "SSBI controller type: '%s'\n", type);
+	if (strcmp(type, "ssbi") == 0)
+		ssbi->controller_type = MSM_SBI_CTRL_SSBI;
+	else if (strcmp(type, "ssbi2") == 0)
+		ssbi->controller_type = MSM_SBI_CTRL_SSBI2;
+	else if (strcmp(type, "pmic-arbiter") == 0)
+		ssbi->controller_type = MSM_SBI_CTRL_PMIC_ARBITER;
+	else {
+		pr_err("Unknown qcom,controller-type\n");
+		ret = -EINVAL;
+		goto err_ssbi_controller;
+	}
+
 	if (ssbi->controller_type == MSM_SBI_CTRL_PMIC_ARBITER) {
 		ssbi->read = msm_ssbi_pa_read_bytes;
 		ssbi->write = msm_ssbi_pa_write_bytes;
@@ -345,13 +322,13 @@ static int msm_ssbi_probe(struct platform_device *pdev)
 
 	spin_lock_init(&ssbi->lock);
 
-	ret = msm_ssbi_add_slave(ssbi, &pdata->slave);
+	ret = of_platform_populate(np, NULL, NULL, &pdev->dev);
 	if (ret)
-		goto err_ssbi_add_slave;
+		goto err_ssbi_controller;
 
 	return 0;
 
-err_ssbi_add_slave:
+err_ssbi_controller:
 	platform_set_drvdata(pdev, NULL);
 	iounmap(ssbi->base);
 err_ioremap:
@@ -370,12 +347,18 @@ static int msm_ssbi_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static struct of_device_id ssbi_match_table[] = {
+	{ .compatible = "qcom,ssbi" },
+	{}
+};
+
 static struct platform_driver msm_ssbi_driver = {
 	.probe		= msm_ssbi_probe,
 	.remove		= __exit_p(msm_ssbi_remove),
 	.driver		= {
 		.name	= "msm_ssbi",
 		.owner	= THIS_MODULE,
+		.of_match_table = ssbi_match_table,
 	},
 };
 
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

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

* [PATCH 3/6] ssbi: Fix exit mismatch in remove function
  2013-03-07  0:29 [PATCH 0/6] Qualcomm SSBI bus driver David Brown
  2013-03-07  0:29 ` [PATCH 1/6] platform-drivers: msm: add single-wire serial bus interface (SSBI) driver David Brown
  2013-03-07  0:29 ` [PATCH 2/6] SSBI: Convert SSBI to device tree David Brown
@ 2013-03-07  0:29 ` David Brown
  2013-03-07  1:30   ` Greg Kroah-Hartman
  2013-03-07  0:29 ` [PATCH 4/6] ssbi: Use regular init level David Brown
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 33+ messages in thread
From: David Brown @ 2013-03-07  0:29 UTC (permalink / raw)
  To: linux-arm-kernel

msm_ssbi_remove is referenced with __exit_p, but not declared with
__exit.  This causes a warning when the driver is not built as a
module:

drivers/ssbi/ssbi.c:341:23: warning: 'msm_ssbi_remove' defined but not used [-Wunused-function]

Fix by adding the __exit declaration to the function.

Signed-off-by: David Brown <davidb@codeaurora.org>
---
 drivers/ssbi/ssbi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/ssbi/ssbi.c b/drivers/ssbi/ssbi.c
index 86d8416..4d503da 100644
--- a/drivers/ssbi/ssbi.c
+++ b/drivers/ssbi/ssbi.c
@@ -337,7 +337,7 @@ err_get_mem_res:
 	return ret;
 }
 
-static int msm_ssbi_remove(struct platform_device *pdev)
+static int __exit msm_ssbi_remove(struct platform_device *pdev)
 {
 	struct msm_ssbi *ssbi = platform_get_drvdata(pdev);
 
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

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

* [PATCH 4/6] ssbi: Use regular init level
  2013-03-07  0:29 [PATCH 0/6] Qualcomm SSBI bus driver David Brown
                   ` (2 preceding siblings ...)
  2013-03-07  0:29 ` [PATCH 3/6] ssbi: Fix exit mismatch in remove function David Brown
@ 2013-03-07  0:29 ` David Brown
  2013-03-07  0:29 ` [PATCH 5/6] ARM: msm: enable SSBI driver in defconfig David Brown
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 33+ messages in thread
From: David Brown @ 2013-03-07  0:29 UTC (permalink / raw)
  To: linux-arm-kernel

With device tree, and deferred probe, it is no longer necessary to
make sure that the ssbi bus driver is initialized very early.  Restore
to a regular module_init().

Signed-off-by: David Brown <davidb@codeaurora.org>
---
 drivers/ssbi/ssbi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/ssbi/ssbi.c b/drivers/ssbi/ssbi.c
index 4d503da..4c02421 100644
--- a/drivers/ssbi/ssbi.c
+++ b/drivers/ssbi/ssbi.c
@@ -366,7 +366,7 @@ static int __init msm_ssbi_init(void)
 {
 	return platform_driver_register(&msm_ssbi_driver);
 }
-postcore_initcall(msm_ssbi_init);
+module_init(msm_ssbi_init);
 
 static void __exit msm_ssbi_exit(void)
 {
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

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

* [PATCH 5/6] ARM: msm: enable SSBI driver in defconfig
  2013-03-07  0:29 [PATCH 0/6] Qualcomm SSBI bus driver David Brown
                   ` (3 preceding siblings ...)
  2013-03-07  0:29 ` [PATCH 4/6] ssbi: Use regular init level David Brown
@ 2013-03-07  0:29 ` David Brown
  2013-03-07  0:29 ` [PATCH 6/6] RFC: SSBI: Simple pm8058 test driver David Brown
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 33+ messages in thread
From: David Brown @ 2013-03-07  0:29 UTC (permalink / raw)
  To: linux-arm-kernel

Signed-off-by: David Brown <davidb@codeaurora.org>
---
 arch/arm/configs/msm_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/configs/msm_defconfig b/arch/arm/configs/msm_defconfig
index 3a5e519..e6aadbd 100644
--- a/arch/arm/configs/msm_defconfig
+++ b/arch/arm/configs/msm_defconfig
@@ -84,6 +84,7 @@ CONFIG_HW_RANDOM=y
 CONFIG_I2C=y
 CONFIG_I2C_CHARDEV=y
 CONFIG_SPI=y
+CONFIG_MSM_SSBI=y
 CONFIG_DEBUG_GPIO=y
 CONFIG_GPIO_SYSFS=y
 CONFIG_POWER_SUPPLY=y
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

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

* [PATCH 6/6] RFC: SSBI: Simple pm8058 test driver
  2013-03-07  0:29 [PATCH 0/6] Qualcomm SSBI bus driver David Brown
                   ` (4 preceding siblings ...)
  2013-03-07  0:29 ` [PATCH 5/6] ARM: msm: enable SSBI driver in defconfig David Brown
@ 2013-03-07  0:29 ` David Brown
  2013-03-12 18:41 ` [PATCH v2 0/11] Qualcomm SSBI bus driver David Brown
  2013-03-12 20:12 ` [PATCH v2 11/11] RFC: SSBI: Simple pm8058 test driver David Brown
  7 siblings, 0 replies; 33+ messages in thread
From: David Brown @ 2013-03-07  0:29 UTC (permalink / raw)
  To: linux-arm-kernel

A very small ssbi device driver that reads the pm8058 version register
;and prints it out.

This isn't really a useful driver, hence the RFC, but it is useful for
someone wanting to test the SSBI driver itself.  Once the PMIC drivers
are finished, this isn't useful at all.

Signed-off-by: David Brown <davidb@codeaurora.org>
---
 arch/arm/boot/dts/msm8660-surf.dts |  4 +++
 arch/arm/boot/dts/msm8960-cdp.dts  |  4 +++
 drivers/ssbi/Makefile              |  1 +
 drivers/ssbi/ssbi-test.c           | 61 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 70 insertions(+)
 create mode 100644 drivers/ssbi/ssbi-test.c

diff --git a/arch/arm/boot/dts/msm8660-surf.dts b/arch/arm/boot/dts/msm8660-surf.dts
index 67f8670..68a0c7c4 100644
--- a/arch/arm/boot/dts/msm8660-surf.dts
+++ b/arch/arm/boot/dts/msm8660-surf.dts
@@ -43,5 +43,9 @@
 		compatible = "qcom,ssbi";
 		reg = <0x500000 0x1000>;
 		qcom,controller-type = "pmic-arbiter";
+
+		qcom,ssbi-test {
+			compatible = "qcom,ssbi-test";
+		};
 	};
 };
diff --git a/arch/arm/boot/dts/msm8960-cdp.dts b/arch/arm/boot/dts/msm8960-cdp.dts
index c9b09a8..60804d7 100644
--- a/arch/arm/boot/dts/msm8960-cdp.dts
+++ b/arch/arm/boot/dts/msm8960-cdp.dts
@@ -43,5 +43,9 @@
 		compatible = "qcom,ssbi";
 		reg = <0x500000 0x1000>;
 		qcom,controller-type = "pmic-arbiter";
+
+		qcom,ssbi-test {
+			compatible = "qcom,ssbi-test";
+		};
 	};
 };
diff --git a/drivers/ssbi/Makefile b/drivers/ssbi/Makefile
index ea8c1e4..3561280 100644
--- a/drivers/ssbi/Makefile
+++ b/drivers/ssbi/Makefile
@@ -2,3 +2,4 @@
 # Makefile for the MSM specific device drivers.
 #
 obj-$(CONFIG_MSM_SSBI) += ssbi.o
+obj-$(CONFIG_MSM_SSBI) += ssbi-test.o
diff --git a/drivers/ssbi/ssbi-test.c b/drivers/ssbi/ssbi-test.c
new file mode 100644
index 0000000..dea4dd0
--- /dev/null
+++ b/drivers/ssbi/ssbi-test.c
@@ -0,0 +1,61 @@
+/* A simple ssbi test device. */
+
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/msm_ssbi.h>
+
+static int __init ssbi_test_probe(struct platform_device *pdev)
+{
+	int ret;
+	char version;
+
+	dev_info(&pdev->dev, "probe, me = %p, parent = %p\n",
+		 pdev, pdev->dev.parent);
+
+	/* Let's try reading. */
+	ret = msm_ssbi_read(pdev->dev.parent, 0x02, &version, 1);
+	if (ret != 0)
+		return ret;
+
+	dev_info(&pdev->dev, "Version = %02x\n", version);
+
+	/* Should already be hooked in. */
+	return 0;
+}
+
+static int ssbi_test_remove(struct platform_device *pdev)
+{
+	return 0;
+}
+
+static struct of_device_id ssbi_test_match_table[] = {
+	{ .compatible = "qcom,ssbi-test" },
+	{}
+};
+
+static struct platform_driver ssbi_test_driver = {
+	.remove = ssbi_test_remove,
+	.driver = {
+		.name = "sbbi_test",
+		.owner = THIS_MODULE,
+		.of_match_table = ssbi_test_match_table,
+	},
+};
+
+static int __init ssbi_test_init(void)
+{
+	int ret;
+
+	ret = platform_driver_probe(&ssbi_test_driver, ssbi_test_probe);
+	return ret;
+}
+
+static void __exit ssbi_test_exit(void)
+{
+}
+
+module_init(ssbi_test_init);
+module_exit(ssbi_test_exit);
+
+MODULE_LICENSE("GPL");
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

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

* [PATCH 1/6] platform-drivers: msm: add single-wire serial bus interface (SSBI) driver
  2013-03-07  0:29 ` [PATCH 1/6] platform-drivers: msm: add single-wire serial bus interface (SSBI) driver David Brown
@ 2013-03-07  1:30   ` Greg Kroah-Hartman
  2013-03-07  5:20     ` David Brown
  2013-03-12  6:51     ` David Brown
  0 siblings, 2 replies; 33+ messages in thread
From: Greg Kroah-Hartman @ 2013-03-07  1:30 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Mar 06, 2013 at 04:29:42PM -0800, David Brown wrote:
> +menu "Qualcomm MSM SSBI bus support"
> +	depends on ARCH_MSM

Why?

> +config MSM_SSBI
> +	bool "Qualcomm Single-wire Serial Bus Interface (SSBI)"

Why can't this be a module?

The multi-platform (or whatever it's called), requires that things be
modules and not built in.

> +	help
> +	  If you say yes to this option, support will be included for the
> +	  built-in SSBI interface on Qualcomm MSM family processors.
> +
> +	  This is required for communicating with Qualcomm PMICs and
> +	  other devices that have the SSBI interface.
> +
> +endmenu
> diff --git a/drivers/ssbi/Makefile b/drivers/ssbi/Makefile
> new file mode 100644
> index 0000000..ea8c1e4
> --- /dev/null
> +++ b/drivers/ssbi/Makefile
> @@ -0,0 +1,4 @@
> +#
> +# Makefile for the MSM specific device drivers.

MSM?

No comment needed at all in this file :)

> --- /dev/null
> +++ b/drivers/ssbi/ssbi.c
> @@ -0,0 +1,397 @@
> +/* Copyright (c) 2009-2011, Code Aurora Forum. All rights reserved.
> + * Copyright (c) 2010, Google Inc.
> + *
> + * Original authors: Code Aurora Forum
> + *
> + * Author: Dima Zavin <dima@android.com>
> + *  - Largely rewritten from original to not be an i2c driver.
> + *
> + * 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.
> + */
> +
> +#define pr_fmt(fmt) "%s: " fmt, __func__
> +
> +#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/msm_ssbi.h>
> +#include <linux/module.h>
> +
> +/* SSBI 2.0 controller registers */
> +#define SSBI2_CMD			0x0008
> +#define SSBI2_RD			0x0010
> +#define SSBI2_STATUS			0x0014
> +#define SSBI2_MODE2			0x001C
> +
> +/* SSBI_CMD fields */
> +#define SSBI_CMD_RDWRN			(1 << 24)
> +
> +/* SSBI_STATUS fields */
> +#define SSBI_STATUS_RD_READY		(1 << 2)
> +#define SSBI_STATUS_READY		(1 << 1)
> +#define SSBI_STATUS_MCHN_BUSY		(1 << 0)
> +
> +/* SSBI_MODE2 fields */
> +#define SSBI_MODE2_REG_ADDR_15_8_SHFT	0x04
> +#define SSBI_MODE2_REG_ADDR_15_8_MASK	(0x7f << SSBI_MODE2_REG_ADDR_15_8_SHFT)
> +
> +#define SET_SSBI_MODE2_REG_ADDR_15_8(MD, AD) \
> +	(((MD) & 0x0F) | ((((AD) >> 8) << SSBI_MODE2_REG_ADDR_15_8_SHFT) & \
> +	SSBI_MODE2_REG_ADDR_15_8_MASK))
> +
> +/* SSBI PMIC Arbiter command registers */
> +#define SSBI_PA_CMD			0x0000
> +#define SSBI_PA_RD_STATUS		0x0004
> +
> +/* SSBI_PA_CMD fields */
> +#define SSBI_PA_CMD_RDWRN		(1 << 24)
> +#define SSBI_PA_CMD_ADDR_MASK		0x7fff /* REG_ADDR_7_0, REG_ADDR_8_14*/
> +
> +/* SSBI_PA_RD_STATUS fields */
> +#define SSBI_PA_RD_STATUS_TRANS_DONE	(1 << 27)
> +#define SSBI_PA_RD_STATUS_TRANS_DENIED	(1 << 26)
> +
> +#define SSBI_TIMEOUT_US			100
> +
> +struct msm_ssbi {
> +	struct device		*dev;
> +	struct device		*slave;

Really?  Two pointers to devices?  Why?  Shouldn't this have a struct
device embedded in it instead of the dev member being a pointer?

> +	void __iomem		*base;
> +	spinlock_t		lock;
> +	enum msm_ssbi_controller_type controller_type;
> +	int (*read)(struct msm_ssbi *, u16 addr, u8 *buf, int len);
> +	int (*write)(struct msm_ssbi *, u16 addr, u8 *buf, int len);
> +};
> +
> +#define to_msm_ssbi(dev)	platform_get_drvdata(to_platform_device(dev))

That doesn't work for the above structure, right?

> +static inline u32 ssbi_readl(struct msm_ssbi *ssbi, u32 reg)
> +{
> +	return readl(ssbi->base + reg);
> +}
> +
> +static inline void ssbi_writel(struct msm_ssbi *ssbi, u32 val, u32 reg)
> +{
> +	writel(val, ssbi->base + reg);
> +}

Did you run sparse on this file?

> +static int ssbi_wait_mask(struct msm_ssbi *ssbi, u32 set_mask, u32 clr_mask)
> +{
> +	u32 timeout = SSBI_TIMEOUT_US;
> +	u32 val;
> +
> +	while (timeout--) {
> +		val = ssbi_readl(ssbi, SSBI2_STATUS);
> +		if (((val & set_mask) == set_mask) && ((val & clr_mask) == 0))
> +			return 0;
> +		udelay(1);

Busy loop?  Really?

> +	}
> +
> +	dev_err(ssbi->dev, "%s: timeout (status %x set_mask %x clr_mask %x)\n",
> +		__func__, ssbi_readl(ssbi, SSBI2_STATUS), set_mask, clr_mask);

Why polute the log with this?  What can a user do with it?


> +	return -ETIMEDOUT;
> +}
> +
> +static int
> +msm_ssbi_read_bytes(struct msm_ssbi *ssbi, u16 addr, u8 *buf, int len)
> +{
> +	u32 cmd = SSBI_CMD_RDWRN | ((addr & 0xff) << 16);
> +	int ret = 0;
> +
> +	if (ssbi->controller_type == MSM_SBI_CTRL_SSBI2) {
> +		u32 mode2 = ssbi_readl(ssbi, SSBI2_MODE2);
> +		mode2 = SET_SSBI_MODE2_REG_ADDR_15_8(mode2, addr);
> +		ssbi_writel(ssbi, mode2, SSBI2_MODE2);
> +	}
> +
> +	while (len) {
> +		ret = ssbi_wait_mask(ssbi, SSBI_STATUS_READY, 0);
> +		if (ret)
> +			goto err;
> +
> +		ssbi_writel(ssbi, cmd, SSBI2_CMD);
> +		ret = ssbi_wait_mask(ssbi, SSBI_STATUS_RD_READY, 0);
> +		if (ret)
> +			goto err;
> +		*buf++ = ssbi_readl(ssbi, SSBI2_RD) & 0xff;
> +		len--;
> +	}
> +
> +err:
> +	return ret;
> +}
> +
> +static int
> +msm_ssbi_write_bytes(struct msm_ssbi *ssbi, u16 addr, u8 *buf, int len)
> +{
> +	int ret = 0;
> +
> +	if (ssbi->controller_type == MSM_SBI_CTRL_SSBI2) {
> +		u32 mode2 = ssbi_readl(ssbi, SSBI2_MODE2);
> +		mode2 = SET_SSBI_MODE2_REG_ADDR_15_8(mode2, addr);
> +		ssbi_writel(ssbi, mode2, SSBI2_MODE2);
> +	}

Perhaps have a controller type write function that can handle this type
of stuff instead of putting it in the "generic" write path?

> +	while (len) {
> +		ret = ssbi_wait_mask(ssbi, SSBI_STATUS_READY, 0);
> +		if (ret)
> +			goto err;
> +
> +		ssbi_writel(ssbi, ((addr & 0xff) << 16) | *buf, SSBI2_CMD);
> +		ret = ssbi_wait_mask(ssbi, 0, SSBI_STATUS_MCHN_BUSY);
> +		if (ret)
> +			goto err;
> +		buf++;
> +		len--;
> +	}
> +
> +err:
> +	return ret;
> +}
> +
> +static inline int
> +msm_ssbi_pa_transfer(struct msm_ssbi *ssbi, u32 cmd, u8 *data)
> +{
> +	u32 timeout = SSBI_TIMEOUT_US;
> +	u32 rd_status = 0;
> +
> +	ssbi_writel(ssbi, cmd, SSBI_PA_CMD);
> +
> +	while (timeout--) {
> +		rd_status = ssbi_readl(ssbi, SSBI_PA_RD_STATUS);
> +
> +		if (rd_status & SSBI_PA_RD_STATUS_TRANS_DENIED) {
> +			dev_err(ssbi->dev, "%s: transaction denied (0x%x)\n",
> +					__func__, rd_status);
> +			return -EPERM;
> +		}
> +
> +		if (rd_status & SSBI_PA_RD_STATUS_TRANS_DONE) {
> +			if (data)
> +				*data = rd_status & 0xff;
> +			return 0;
> +		}
> +		udelay(1);

Busy loop again?

> +	}
> +
> +	dev_err(ssbi->dev, "%s: timeout, status 0x%x\n", __func__, rd_status);

Don't polute the logs unless you want the user to do something with the
information.


> +	return -ETIMEDOUT;
> +}
> +
> +static int
> +msm_ssbi_pa_read_bytes(struct msm_ssbi *ssbi, u16 addr, u8 *buf, int len)
> +{
> +	u32 cmd;
> +	int ret = 0;
> +
> +	cmd = SSBI_PA_CMD_RDWRN | (addr & SSBI_PA_CMD_ADDR_MASK) << 8;
> +
> +	while (len) {
> +		ret = msm_ssbi_pa_transfer(ssbi, cmd, buf);
> +		if (ret)
> +			goto err;
> +		buf++;
> +		len--;
> +	}
> +
> +err:
> +	return ret;
> +}
> +
> +static int
> +msm_ssbi_pa_write_bytes(struct msm_ssbi *ssbi, u16 addr, u8 *buf, int len)
> +{
> +	u32 cmd;
> +	int ret = 0;
> +
> +	while (len) {
> +		cmd = (addr & SSBI_PA_CMD_ADDR_MASK) << 8 | *buf;
> +		ret = msm_ssbi_pa_transfer(ssbi, cmd, NULL);
> +		if (ret)
> +			goto err;
> +		buf++;
> +		len--;
> +	}
> +
> +err:
> +	return ret;
> +}
> +
> +int msm_ssbi_read(struct device *dev, u16 addr, u8 *buf, int len)
> +{
> +	struct msm_ssbi *ssbi = to_msm_ssbi(dev);
> +	unsigned long flags;
> +	int ret;
> +
> +	if (ssbi->dev != dev)
> +		return -ENXIO;
> +
> +	spin_lock_irqsave(&ssbi->lock, flags);
> +	ret = ssbi->read(ssbi, addr, buf, len);
> +	spin_unlock_irqrestore(&ssbi->lock, flags);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL(msm_ssbi_read);

msm_*?  Why not just ssbi_*?

EXPORT_SYMBOL_GPL()?

> +static struct platform_driver msm_ssbi_driver = {
> +	.probe		= msm_ssbi_probe,
> +	.remove		= __exit_p(msm_ssbi_remove),

You just oopsed if someone unbound your device from the driver from
userspace.  Not good.

thanks,

greg k-h

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

* [PATCH 3/6] ssbi: Fix exit mismatch in remove function
  2013-03-07  0:29 ` [PATCH 3/6] ssbi: Fix exit mismatch in remove function David Brown
@ 2013-03-07  1:30   ` Greg Kroah-Hartman
  2013-03-07  5:21     ` David Brown
  0 siblings, 1 reply; 33+ messages in thread
From: Greg Kroah-Hartman @ 2013-03-07  1:30 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Mar 06, 2013 at 04:29:44PM -0800, David Brown wrote:
> msm_ssbi_remove is referenced with __exit_p, but not declared with
> __exit.  This causes a warning when the driver is not built as a
> module:
> 
> drivers/ssbi/ssbi.c:341:23: warning: 'msm_ssbi_remove' defined but not used [-Wunused-function]
> 
> Fix by adding the __exit declaration to the function.
> 
> Signed-off-by: David Brown <davidb@codeaurora.org>
> ---
>  drivers/ssbi/ssbi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/ssbi/ssbi.c b/drivers/ssbi/ssbi.c
> index 86d8416..4d503da 100644
> --- a/drivers/ssbi/ssbi.c
> +++ b/drivers/ssbi/ssbi.c
> @@ -337,7 +337,7 @@ err_get_mem_res:
>  	return ret;
>  }
>  
> -static int msm_ssbi_remove(struct platform_device *pdev)
> +static int __exit msm_ssbi_remove(struct platform_device *pdev)

No, remove the __exit_p marking instead, unless you want your kernel to
be oopsed :)

thanks,

greg k-h

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

* [PATCH 1/6] platform-drivers: msm: add single-wire serial bus interface (SSBI) driver
  2013-03-07  1:30   ` Greg Kroah-Hartman
@ 2013-03-07  5:20     ` David Brown
  2013-03-07  6:01       ` Greg Kroah-Hartman
  2013-03-12  6:51     ` David Brown
  1 sibling, 1 reply; 33+ messages in thread
From: David Brown @ 2013-03-07  5:20 UTC (permalink / raw)
  To: linux-arm-kernel

Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:

> On Wed, Mar 06, 2013 at 04:29:42PM -0800, David Brown wrote:
>> +menu "Qualcomm MSM SSBI bus support"
>> +	depends on ARCH_MSM
>
> Why?

In the sense that ARCH_MSM are the only devices that ever were, or ever
will be made with this device.  It doesn't strictly depend on it, but do
we want to clutter the config for everything else.

>> +config MSM_SSBI
>> +	bool "Qualcomm Single-wire Serial Bus Interface (SSBI)"
>
> Why can't this be a module?

Without this device, the PMIC drivers won't work, regulators can't be
turned on, and most of the other devices won't work.  I can try if it
can still be made a module, and it might be good at exercising the
deferred probe mechanism.

So, a deeper question.  I've resent this driver with minimal change.
I've also made some other changes as patches afterwards.  Do we want
these squashed into a single patch, or the initial one (not authored by
me) followed by updates?

>> +#
>> +# Makefile for the MSM specific device drivers.
>
> MSM?
>
> No comment needed at all in this file :)

Thanks, missed this.  I think the original patch was using platform/msm,
and the minimalist comment almost made sense in that context.

>> +struct msm_ssbi {
>> +	struct device		*dev;
>> +	struct device		*slave;
>
> Really?  Two pointers to devices?  Why?  Shouldn't this have a struct
> device embedded in it instead of the dev member being a pointer?

I'll go through this more thoroughly and organize things better.

>> +static int ssbi_wait_mask(struct msm_ssbi *ssbi, u32 set_mask, u32 clr_mask)
>> +{
>> +	u32 timeout = SSBI_TIMEOUT_US;
>> +	u32 val;
>> +
>> +	while (timeout--) {
>> +		val = ssbi_readl(ssbi, SSBI2_STATUS);
>> +		if (((val & set_mask) == set_mask) && ((val & clr_mask) == 0))
>> +			return 0;
>> +		udelay(1);
>
> Busy loop?  Really?

I'm not sure what else to do here.  The controller is only slightly more
than a bit-banged bus.  I think the reason for the longer possibly delay
is because there is an arbiter (the PMIC is shared with the radio CPU).
I'll investigate further.

>> +	}
>> +
>> +	dev_err(ssbi->dev, "%s: timeout (status %x set_mask %x clr_mask %x)\n",
>> +		__func__, ssbi_readl(ssbi, SSBI2_STATUS), set_mask, clr_mask);
>
> Why polute the log with this?  What can a user do with it?

Nothing in fact, other than possibly learn that things, indeed aren't
working.  I'll take it and the others out.

>> +static int
>> +msm_ssbi_write_bytes(struct msm_ssbi *ssbi, u16 addr, u8 *buf, int len)
>> +{
>> +	int ret = 0;
>> +
>> +	if (ssbi->controller_type == MSM_SBI_CTRL_SSBI2) {
>> +		u32 mode2 = ssbi_readl(ssbi, SSBI2_MODE2);
>> +		mode2 = SET_SSBI_MODE2_REG_ADDR_15_8(mode2, addr);
>> +		ssbi_writel(ssbi, mode2, SSBI2_MODE2);
>> +	}
>
> Perhaps have a controller type write function that can handle this type
> of stuff instead of putting it in the "generic" write path?

Yes, that's a better approach.

>> +EXPORT_SYMBOL(msm_ssbi_read);
>
> msm_*?  Why not just ssbi_*?

I'm fine with ssbi.  It is very MSM specific, though.

> EXPORT_SYMBOL_GPL()?

Yes, it's definitely a kernel internal API.

>> +static struct platform_driver msm_ssbi_driver = {
>> +	.probe		= msm_ssbi_probe,
>> +	.remove		= __exit_p(msm_ssbi_remove),
>
> You just oopsed if someone unbound your device from the driver from
> userspace.  Not good.

I did catch this one in a later patch :-)  I can clean it up in the
driver, though, since it looks like some more work needs to go into
this.

Thanks,
David

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

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

* [PATCH 3/6] ssbi: Fix exit mismatch in remove function
  2013-03-07  1:30   ` Greg Kroah-Hartman
@ 2013-03-07  5:21     ` David Brown
  0 siblings, 0 replies; 33+ messages in thread
From: David Brown @ 2013-03-07  5:21 UTC (permalink / raw)
  To: linux-arm-kernel

Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:

> On Wed, Mar 06, 2013 at 04:29:44PM -0800, David Brown wrote:
>> msm_ssbi_remove is referenced with __exit_p, but not declared with
>> __exit.  This causes a warning when the driver is not built as a
>> module:
>> 
>> drivers/ssbi/ssbi.c:341:23: warning: 'msm_ssbi_remove' defined but not used [-Wunused-function]
>> 
>> Fix by adding the __exit declaration to the function.
>> 
>> Signed-off-by: David Brown <davidb@codeaurora.org>
>> ---
>>  drivers/ssbi/ssbi.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/drivers/ssbi/ssbi.c b/drivers/ssbi/ssbi.c
>> index 86d8416..4d503da 100644
>> --- a/drivers/ssbi/ssbi.c
>> +++ b/drivers/ssbi/ssbi.c
>> @@ -337,7 +337,7 @@ err_get_mem_res:
>>  	return ret;
>>  }
>>  
>> -static int msm_ssbi_remove(struct platform_device *pdev)
>> +static int __exit msm_ssbi_remove(struct platform_device *pdev)
>
> No, remove the __exit_p marking instead, unless you want your kernel to
> be oopsed :)

Thanks.  Oopsing is not fun.

David

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

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

* [PATCH 1/6] platform-drivers: msm: add single-wire serial bus interface (SSBI) driver
  2013-03-07  5:20     ` David Brown
@ 2013-03-07  6:01       ` Greg Kroah-Hartman
  2013-03-07 10:05         ` Sekhar Nori
  2013-03-07 18:50         ` David Brown
  0 siblings, 2 replies; 33+ messages in thread
From: Greg Kroah-Hartman @ 2013-03-07  6:01 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Mar 06, 2013 at 09:20:45PM -0800, David Brown wrote:
> Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> 
> > On Wed, Mar 06, 2013 at 04:29:42PM -0800, David Brown wrote:
> >> +menu "Qualcomm MSM SSBI bus support"
> >> +	depends on ARCH_MSM
> >
> > Why?
> 
> In the sense that ARCH_MSM are the only devices that ever were, or ever
> will be made with this device.  It doesn't strictly depend on it, but do
> we want to clutter the config for everything else.

It's not "clutter".  You want me to build this on other platforms, to
catch api and other types of build breakages.  This is the way for
almost all Linux drivers.

> >> +config MSM_SSBI
> >> +	bool "Qualcomm Single-wire Serial Bus Interface (SSBI)"
> >
> > Why can't this be a module?
> 
> Without this device, the PMIC drivers won't work, regulators can't be
> turned on, and most of the other devices won't work.  I can try if it
> can still be made a module, and it might be good at exercising the
> deferred probe mechanism.

If the PMIC drivers are dependant on the symbols in this module, there
should not be any problems, right?

> So, a deeper question.  I've resent this driver with minimal change.
> I've also made some other changes as patches afterwards.  Do we want
> these squashed into a single patch, or the initial one (not authored by
> me) followed by updates?

To preserve the authorship, you might want to fix this stuff with
follow-on patches.  As long as the original patch can build properly.

> >> +static struct platform_driver msm_ssbi_driver = {
> >> +	.probe		= msm_ssbi_probe,
> >> +	.remove		= __exit_p(msm_ssbi_remove),
> >
> > You just oopsed if someone unbound your device from the driver from
> > userspace.  Not good.
> 
> I did catch this one in a later patch :-)  I can clean it up in the
> driver, though, since it looks like some more work needs to go into
> this.

Yes, but it's very close, fix this all up and resend your series and all
should be fine.

Nice job,

greg k-h

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

* [PATCH 1/6] platform-drivers: msm: add single-wire serial bus interface (SSBI) driver
  2013-03-07  6:01       ` Greg Kroah-Hartman
@ 2013-03-07 10:05         ` Sekhar Nori
  2013-03-07 18:45           ` David Brown
  2013-03-07 18:50         ` David Brown
  1 sibling, 1 reply; 33+ messages in thread
From: Sekhar Nori @ 2013-03-07 10:05 UTC (permalink / raw)
  To: linux-arm-kernel

On 3/7/2013 11:31 AM, Greg Kroah-Hartman wrote:
> On Wed, Mar 06, 2013 at 09:20:45PM -0800, David Brown wrote:
>> Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
>>
>>> On Wed, Mar 06, 2013 at 04:29:42PM -0800, David Brown wrote:
>>>> +menu "Qualcomm MSM SSBI bus support"
>>>> +	depends on ARCH_MSM
>>>
>>> Why?
>>
>> In the sense that ARCH_MSM are the only devices that ever were, or ever
>> will be made with this device.  It doesn't strictly depend on it, but do
>> we want to clutter the config for everything else.
> 
> It's not "clutter".  You want me to build this on other platforms, to
> catch api and other types of build breakages.  This is the way for
> almost all Linux drivers.

Not having a depends on helps build coverage, but I have seen
objections to showing up irrelevant configurations to users (of x86 for
example). See one here from Linus for OMAP_OCP2SCP

http://lkml.indiana.edu/hypermail/linux/kernel/1210.0/00785.html

If this case is different, I am not sure why.

Thanks,
Sekhar

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

* [PATCH 1/6] platform-drivers: msm: add single-wire serial bus interface (SSBI) driver
  2013-03-07 10:05         ` Sekhar Nori
@ 2013-03-07 18:45           ` David Brown
  0 siblings, 0 replies; 33+ messages in thread
From: David Brown @ 2013-03-07 18:45 UTC (permalink / raw)
  To: linux-arm-kernel

Sekhar Nori <nsekhar@ti.com> writes:

> On 3/7/2013 11:31 AM, Greg Kroah-Hartman wrote:
>> On Wed, Mar 06, 2013 at 09:20:45PM -0800, David Brown wrote:
>>> Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
>>>
>>>> On Wed, Mar 06, 2013 at 04:29:42PM -0800, David Brown wrote:
>>>>> +menu "Qualcomm MSM SSBI bus support"
>>>>> +	depends on ARCH_MSM
>>>>
>>>> Why?
>>>
>>> In the sense that ARCH_MSM are the only devices that ever were, or ever
>>> will be made with this device.  It doesn't strictly depend on it, but do
>>> we want to clutter the config for everything else.
>> 
>> It's not "clutter".  You want me to build this on other platforms, to
>> catch api and other types of build breakages.  This is the way for
>> almost all Linux drivers.
>
> Not having a depends on helps build coverage, but I have seen
> objections to showing up irrelevant configurations to users (of x86 for
> example). See one here from Linus for OMAP_OCP2SCP
>
> http://lkml.indiana.edu/hypermail/linux/kernel/1210.0/00785.html
>
> If this case is different, I am not sure why.

This was, in fact, the reason I put the dependency there.  I found it a
little annoying being asked about a bunch of OMAP devices when building
the x86 kernel.  At least they weren't cancer curing options (default
y).

David

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

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

* [PATCH 1/6] platform-drivers: msm: add single-wire serial bus interface (SSBI) driver
  2013-03-07  6:01       ` Greg Kroah-Hartman
  2013-03-07 10:05         ` Sekhar Nori
@ 2013-03-07 18:50         ` David Brown
  2013-03-07 23:29           ` Greg Kroah-Hartman
  1 sibling, 1 reply; 33+ messages in thread
From: David Brown @ 2013-03-07 18:50 UTC (permalink / raw)
  To: linux-arm-kernel

Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:

>> Without this device, the PMIC drivers won't work, regulators can't be
>> turned on, and most of the other devices won't work.  I can try if it
>> can still be made a module, and it might be good at exercising the
>> deferred probe mechanism.
>
> If the PMIC drivers are dependant on the symbols in this module, there
> should not be any problems, right?

The PMIC drivers directly will be, but the users of gpios/irqs and
regulators coming from the PMIC will only depend on the generic APIs for
this.  In theory, the deferred probe should handle this, and it might
be useful for testing.

I can allow it to be a module, although I don't picture that really
being a useful configuration.

David

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

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

* [PATCH 1/6] platform-drivers: msm: add single-wire serial bus interface (SSBI) driver
  2013-03-07 18:50         ` David Brown
@ 2013-03-07 23:29           ` Greg Kroah-Hartman
  0 siblings, 0 replies; 33+ messages in thread
From: Greg Kroah-Hartman @ 2013-03-07 23:29 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Mar 07, 2013 at 10:50:40AM -0800, David Brown wrote:
> Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> 
> >> Without this device, the PMIC drivers won't work, regulators can't be
> >> turned on, and most of the other devices won't work.  I can try if it
> >> can still be made a module, and it might be good at exercising the
> >> deferred probe mechanism.
> >
> > If the PMIC drivers are dependant on the symbols in this module, there
> > should not be any problems, right?
> 
> The PMIC drivers directly will be, but the users of gpios/irqs and
> regulators coming from the PMIC will only depend on the generic APIs for
> this.  In theory, the deferred probe should handle this, and it might
> be useful for testing.
> 
> I can allow it to be a module, although I don't picture that really
> being a useful configuration.

It will allow this to get multi-arch build testing at the very least, a
very valuable thing as time goes on.  You can always specify this as 'Y'
in your defconfig for your board.

thanks,

greg k-h

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

* [PATCH 1/6] platform-drivers: msm: add single-wire serial bus interface (SSBI) driver
  2013-03-07  1:30   ` Greg Kroah-Hartman
  2013-03-07  5:20     ` David Brown
@ 2013-03-12  6:51     ` David Brown
  2013-03-12 13:27       ` Greg Kroah-Hartman
  1 sibling, 1 reply; 33+ messages in thread
From: David Brown @ 2013-03-12  6:51 UTC (permalink / raw)
  To: linux-arm-kernel

Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:

>> +static int ssbi_wait_mask(struct msm_ssbi *ssbi, u32 set_mask, u32 clr_mask)
>> +{
>> +	u32 timeout = SSBI_TIMEOUT_US;
>> +	u32 val;
>> +
>> +	while (timeout--) {
>> +		val = ssbi_readl(ssbi, SSBI2_STATUS);
>> +		if (((val & set_mask) == set_mask) && ((val & clr_mask) == 0))
>> +			return 0;
>> +		udelay(1);
>
> Busy loop?  Really?

Finally was able to dig up some of the reason for this.  The
transactions typically take about 5us.  In the case of contention with
another CPU, it could take as much as 20us.

Would it be sufficient to just explain this in a comment?

David

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

* [PATCH 1/6] platform-drivers: msm: add single-wire serial bus interface (SSBI) driver
  2013-03-12  6:51     ` David Brown
@ 2013-03-12 13:27       ` Greg Kroah-Hartman
  0 siblings, 0 replies; 33+ messages in thread
From: Greg Kroah-Hartman @ 2013-03-12 13:27 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Mar 11, 2013 at 11:51:08PM -0700, David Brown wrote:
> Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> 
> >> +static int ssbi_wait_mask(struct msm_ssbi *ssbi, u32 set_mask, u32 clr_mask)
> >> +{
> >> +	u32 timeout = SSBI_TIMEOUT_US;
> >> +	u32 val;
> >> +
> >> +	while (timeout--) {
> >> +		val = ssbi_readl(ssbi, SSBI2_STATUS);
> >> +		if (((val & set_mask) == set_mask) && ((val & clr_mask) == 0))
> >> +			return 0;
> >> +		udelay(1);
> >
> > Busy loop?  Really?
> 
> Finally was able to dig up some of the reason for this.  The
> transactions typically take about 5us.  In the case of contention with
> another CPU, it could take as much as 20us.
> 
> Would it be sufficient to just explain this in a comment?

That would be good to do, especially if it turns out to be a longer
delay and people start to wonder why their system load is increasing for
no noticeable reason.

thanks,

greg k-h

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

* [PATCH v2 0/11] Qualcomm SSBI bus driver
  2013-03-07  0:29 [PATCH 0/6] Qualcomm SSBI bus driver David Brown
                   ` (5 preceding siblings ...)
  2013-03-07  0:29 ` [PATCH 6/6] RFC: SSBI: Simple pm8058 test driver David Brown
@ 2013-03-12 18:41 ` David Brown
  2013-03-12 18:41   ` [PATCH v2 01/11] platform-drivers: msm: add single-wire serial bus interface (SSBI) driver David Brown
                     ` (10 more replies)
  2013-03-12 20:12 ` [PATCH v2 11/11] RFC: SSBI: Simple pm8058 test driver David Brown
  7 siblings, 11 replies; 33+ messages in thread
From: David Brown @ 2013-03-12 18:41 UTC (permalink / raw)
  To: linux-arm-kernel

This small series adds the Qualcomm SSBI bus driver.  The original
driver was developed as part of Android.  Kenneth Heitke updated this,
and sent it out a while back.  This series updates this driver to use
DeviceTree, and fixes a few things that have changed since it was
originally sent out.

In addition, the 11th patch, the RFC, is a small test driver that reads
the version register off of the pm8058 device typically connected to
an MSM8660 or MSM8960.  This is primarily useful to anyone wanting to
work on these pmic drivers.

Updates to the PMIC drivers themselves to use this driver to come.

Patch version:
  v2: Updates from mailing list feedback:
     - Use EXPORT_SYMBOL_GPL
     - Proper fix for driver remove section mismatch
     - Allow compilation as a module
     - Remove extraneous dev_err() messages
     - Comment use of busywait loop
     - Update MAINTAINERS file

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

* [PATCH v2 01/11] platform-drivers: msm: add single-wire serial bus interface (SSBI) driver
  2013-03-12 18:41 ` [PATCH v2 0/11] Qualcomm SSBI bus driver David Brown
@ 2013-03-12 18:41   ` David Brown
  2013-03-12 18:41   ` [PATCH v2 02/11] fix: Use EXPORT_SYMBOL_GPL David Brown
                     ` (9 subsequent siblings)
  10 siblings, 0 replies; 33+ messages in thread
From: David Brown @ 2013-03-12 18:41 UTC (permalink / raw)
  To: linux-arm-kernel

From: Kenneth Heitke <kheitke@codeaurora.org>

SSBI is the Qualcomm single-wire serial bus interface used to connect
the MSM devices to the PMIC and other devices.

Since SSBI only supports a single slave, the driver gets the name of the
slave device passed in from the board file through the master device's
platform data.

SSBI registers pretty early (postcore), so that the PMIC can come up
before the board init. This is useful if the board init requires the
use of gpios that are connected through the PMIC.

Based on a patch by Dima Zavin <dima@android.com> that can be found at:
http://android.git.kernel.org/?p=kernel/msm.git;a=commitdiff;h=eb060bac4

This patch adds PMIC Arbiter support for the MSM8660. The PMIC Arbiter
is a hardware wrapper around the SSBI 2.0 controller that is designed to
overcome concurrency issues and security limitations.  A controller_type
field is added to the platform data to specify the type of the SSBI
controller (1.0, 2.0, or PMIC Arbiter).

[davidb at codeaurora.org:
 I've moved this driver into drivers/ssbi/ and added an include for
 linux/module.h so that it will compile]

Signed-off-by: Kenneth Heitke <kheitke@codeaurora.org>
Signed-off-by: David Brown <davidb@codeaurora.org>
---
 drivers/Kconfig          |   2 +
 drivers/Makefile         |   1 +
 drivers/ssbi/Kconfig     |  16 ++
 drivers/ssbi/Makefile    |   1 +
 drivers/ssbi/ssbi.c      | 397 +++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/msm_ssbi.h |  49 ++++++
 6 files changed, 466 insertions(+)
 create mode 100644 drivers/ssbi/Kconfig
 create mode 100644 drivers/ssbi/Makefile
 create mode 100644 drivers/ssbi/ssbi.c
 create mode 100644 include/linux/msm_ssbi.h

diff --git a/drivers/Kconfig b/drivers/Kconfig
index 202fa6d..78a956e 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -52,6 +52,8 @@ source "drivers/i2c/Kconfig"
 
 source "drivers/spi/Kconfig"
 
+source "drivers/ssbi/Kconfig"
+
 source "drivers/hsi/Kconfig"
 
 source "drivers/pps/Kconfig"
diff --git a/drivers/Makefile b/drivers/Makefile
index dce39a9..778821b 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -114,6 +114,7 @@ obj-y				+= firmware/
 obj-$(CONFIG_CRYPTO)		+= crypto/
 obj-$(CONFIG_SUPERH)		+= sh/
 obj-$(CONFIG_ARCH_SHMOBILE)	+= sh/
+obj-$(CONFIG_MSM_SSBI)		+= ssbi/
 ifndef CONFIG_ARCH_USES_GETTIMEOFFSET
 obj-y				+= clocksource/
 endif
diff --git a/drivers/ssbi/Kconfig b/drivers/ssbi/Kconfig
new file mode 100644
index 0000000..b57c41b
--- /dev/null
+++ b/drivers/ssbi/Kconfig
@@ -0,0 +1,16 @@
+#
+# MSM SSBI bus support
+#
+
+menu "Qualcomm MSM SSBI bus support"
+
+config MSM_SSBI
+	bool "Qualcomm Single-wire Serial Bus Interface (SSBI)"
+	help
+	  If you say yes to this option, support will be included for the
+	  built-in SSBI interface on Qualcomm MSM family processors.
+
+	  This is required for communicating with Qualcomm PMICs and
+	  other devices that have the SSBI interface.
+
+endmenu
diff --git a/drivers/ssbi/Makefile b/drivers/ssbi/Makefile
new file mode 100644
index 0000000..22e408f
--- /dev/null
+++ b/drivers/ssbi/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_MSM_SSBI) += ssbi.o
diff --git a/drivers/ssbi/ssbi.c b/drivers/ssbi/ssbi.c
new file mode 100644
index 0000000..8b0b10d
--- /dev/null
+++ b/drivers/ssbi/ssbi.c
@@ -0,0 +1,397 @@
+/* Copyright (c) 2009-2011, Code Aurora Forum. All rights reserved.
+ * Copyright (c) 2010, Google Inc.
+ *
+ * Original authors: Code Aurora Forum
+ *
+ * Author: Dima Zavin <dima@android.com>
+ *  - Largely rewritten from original to not be an i2c driver.
+ *
+ * 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.
+ */
+
+#define pr_fmt(fmt) "%s: " fmt, __func__
+
+#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/msm_ssbi.h>
+#include <linux/module.h>
+
+/* SSBI 2.0 controller registers */
+#define SSBI2_CMD			0x0008
+#define SSBI2_RD			0x0010
+#define SSBI2_STATUS			0x0014
+#define SSBI2_MODE2			0x001C
+
+/* SSBI_CMD fields */
+#define SSBI_CMD_RDWRN			(1 << 24)
+
+/* SSBI_STATUS fields */
+#define SSBI_STATUS_RD_READY		(1 << 2)
+#define SSBI_STATUS_READY		(1 << 1)
+#define SSBI_STATUS_MCHN_BUSY		(1 << 0)
+
+/* SSBI_MODE2 fields */
+#define SSBI_MODE2_REG_ADDR_15_8_SHFT	0x04
+#define SSBI_MODE2_REG_ADDR_15_8_MASK	(0x7f << SSBI_MODE2_REG_ADDR_15_8_SHFT)
+
+#define SET_SSBI_MODE2_REG_ADDR_15_8(MD, AD) \
+	(((MD) & 0x0F) | ((((AD) >> 8) << SSBI_MODE2_REG_ADDR_15_8_SHFT) & \
+	SSBI_MODE2_REG_ADDR_15_8_MASK))
+
+/* SSBI PMIC Arbiter command registers */
+#define SSBI_PA_CMD			0x0000
+#define SSBI_PA_RD_STATUS		0x0004
+
+/* SSBI_PA_CMD fields */
+#define SSBI_PA_CMD_RDWRN		(1 << 24)
+#define SSBI_PA_CMD_ADDR_MASK		0x7fff /* REG_ADDR_7_0, REG_ADDR_8_14*/
+
+/* SSBI_PA_RD_STATUS fields */
+#define SSBI_PA_RD_STATUS_TRANS_DONE	(1 << 27)
+#define SSBI_PA_RD_STATUS_TRANS_DENIED	(1 << 26)
+
+#define SSBI_TIMEOUT_US			100
+
+struct msm_ssbi {
+	struct device		*dev;
+	struct device		*slave;
+	void __iomem		*base;
+	spinlock_t		lock;
+	enum msm_ssbi_controller_type controller_type;
+	int (*read)(struct msm_ssbi *, u16 addr, u8 *buf, int len);
+	int (*write)(struct msm_ssbi *, u16 addr, u8 *buf, int len);
+};
+
+#define to_msm_ssbi(dev)	platform_get_drvdata(to_platform_device(dev))
+
+static inline u32 ssbi_readl(struct msm_ssbi *ssbi, u32 reg)
+{
+	return readl(ssbi->base + reg);
+}
+
+static inline void ssbi_writel(struct msm_ssbi *ssbi, u32 val, u32 reg)
+{
+	writel(val, ssbi->base + reg);
+}
+
+static int ssbi_wait_mask(struct msm_ssbi *ssbi, u32 set_mask, u32 clr_mask)
+{
+	u32 timeout = SSBI_TIMEOUT_US;
+	u32 val;
+
+	while (timeout--) {
+		val = ssbi_readl(ssbi, SSBI2_STATUS);
+		if (((val & set_mask) == set_mask) && ((val & clr_mask) == 0))
+			return 0;
+		udelay(1);
+	}
+
+	dev_err(ssbi->dev, "%s: timeout (status %x set_mask %x clr_mask %x)\n",
+		__func__, ssbi_readl(ssbi, SSBI2_STATUS), set_mask, clr_mask);
+	return -ETIMEDOUT;
+}
+
+static int
+msm_ssbi_read_bytes(struct msm_ssbi *ssbi, u16 addr, u8 *buf, int len)
+{
+	u32 cmd = SSBI_CMD_RDWRN | ((addr & 0xff) << 16);
+	int ret = 0;
+
+	if (ssbi->controller_type == MSM_SBI_CTRL_SSBI2) {
+		u32 mode2 = ssbi_readl(ssbi, SSBI2_MODE2);
+		mode2 = SET_SSBI_MODE2_REG_ADDR_15_8(mode2, addr);
+		ssbi_writel(ssbi, mode2, SSBI2_MODE2);
+	}
+
+	while (len) {
+		ret = ssbi_wait_mask(ssbi, SSBI_STATUS_READY, 0);
+		if (ret)
+			goto err;
+
+		ssbi_writel(ssbi, cmd, SSBI2_CMD);
+		ret = ssbi_wait_mask(ssbi, SSBI_STATUS_RD_READY, 0);
+		if (ret)
+			goto err;
+		*buf++ = ssbi_readl(ssbi, SSBI2_RD) & 0xff;
+		len--;
+	}
+
+err:
+	return ret;
+}
+
+static int
+msm_ssbi_write_bytes(struct msm_ssbi *ssbi, u16 addr, u8 *buf, int len)
+{
+	int ret = 0;
+
+	if (ssbi->controller_type == MSM_SBI_CTRL_SSBI2) {
+		u32 mode2 = ssbi_readl(ssbi, SSBI2_MODE2);
+		mode2 = SET_SSBI_MODE2_REG_ADDR_15_8(mode2, addr);
+		ssbi_writel(ssbi, mode2, SSBI2_MODE2);
+	}
+
+	while (len) {
+		ret = ssbi_wait_mask(ssbi, SSBI_STATUS_READY, 0);
+		if (ret)
+			goto err;
+
+		ssbi_writel(ssbi, ((addr & 0xff) << 16) | *buf, SSBI2_CMD);
+		ret = ssbi_wait_mask(ssbi, 0, SSBI_STATUS_MCHN_BUSY);
+		if (ret)
+			goto err;
+		buf++;
+		len--;
+	}
+
+err:
+	return ret;
+}
+
+static inline int
+msm_ssbi_pa_transfer(struct msm_ssbi *ssbi, u32 cmd, u8 *data)
+{
+	u32 timeout = SSBI_TIMEOUT_US;
+	u32 rd_status = 0;
+
+	ssbi_writel(ssbi, cmd, SSBI_PA_CMD);
+
+	while (timeout--) {
+		rd_status = ssbi_readl(ssbi, SSBI_PA_RD_STATUS);
+
+		if (rd_status & SSBI_PA_RD_STATUS_TRANS_DENIED) {
+			dev_err(ssbi->dev, "%s: transaction denied (0x%x)\n",
+					__func__, rd_status);
+			return -EPERM;
+		}
+
+		if (rd_status & SSBI_PA_RD_STATUS_TRANS_DONE) {
+			if (data)
+				*data = rd_status & 0xff;
+			return 0;
+		}
+		udelay(1);
+	}
+
+	dev_err(ssbi->dev, "%s: timeout, status 0x%x\n", __func__, rd_status);
+	return -ETIMEDOUT;
+}
+
+static int
+msm_ssbi_pa_read_bytes(struct msm_ssbi *ssbi, u16 addr, u8 *buf, int len)
+{
+	u32 cmd;
+	int ret = 0;
+
+	cmd = SSBI_PA_CMD_RDWRN | (addr & SSBI_PA_CMD_ADDR_MASK) << 8;
+
+	while (len) {
+		ret = msm_ssbi_pa_transfer(ssbi, cmd, buf);
+		if (ret)
+			goto err;
+		buf++;
+		len--;
+	}
+
+err:
+	return ret;
+}
+
+static int
+msm_ssbi_pa_write_bytes(struct msm_ssbi *ssbi, u16 addr, u8 *buf, int len)
+{
+	u32 cmd;
+	int ret = 0;
+
+	while (len) {
+		cmd = (addr & SSBI_PA_CMD_ADDR_MASK) << 8 | *buf;
+		ret = msm_ssbi_pa_transfer(ssbi, cmd, NULL);
+		if (ret)
+			goto err;
+		buf++;
+		len--;
+	}
+
+err:
+	return ret;
+}
+
+int msm_ssbi_read(struct device *dev, u16 addr, u8 *buf, int len)
+{
+	struct msm_ssbi *ssbi = to_msm_ssbi(dev);
+	unsigned long flags;
+	int ret;
+
+	if (ssbi->dev != dev)
+		return -ENXIO;
+
+	spin_lock_irqsave(&ssbi->lock, flags);
+	ret = ssbi->read(ssbi, addr, buf, len);
+	spin_unlock_irqrestore(&ssbi->lock, flags);
+
+	return ret;
+}
+EXPORT_SYMBOL(msm_ssbi_read);
+
+int msm_ssbi_write(struct device *dev, u16 addr, u8 *buf, int len)
+{
+	struct msm_ssbi *ssbi = to_msm_ssbi(dev);
+	unsigned long flags;
+	int ret;
+
+	if (ssbi->dev != dev)
+		return -ENXIO;
+
+	spin_lock_irqsave(&ssbi->lock, flags);
+	ret = ssbi->write(ssbi, addr, buf, len);
+	spin_unlock_irqrestore(&ssbi->lock, flags);
+
+	return ret;
+}
+EXPORT_SYMBOL(msm_ssbi_write);
+
+static int msm_ssbi_add_slave(struct msm_ssbi *ssbi,
+				const struct msm_ssbi_slave_info *slave)
+{
+	struct platform_device *slave_pdev;
+	int ret;
+
+	if (ssbi->slave) {
+		pr_err("slave already attached??\n");
+		return -EBUSY;
+	}
+
+	slave_pdev = platform_device_alloc(slave->name, -1);
+	if (!slave_pdev) {
+		pr_err("cannot allocate pdev for slave '%s'", slave->name);
+		ret = -ENOMEM;
+		goto err;
+	}
+
+	slave_pdev->dev.parent = ssbi->dev;
+	slave_pdev->dev.platform_data = slave->platform_data;
+
+	ret = platform_device_add(slave_pdev);
+	if (ret) {
+		pr_err("cannot add slave platform device for '%s'\n",
+				slave->name);
+		goto err;
+	}
+
+	ssbi->slave = &slave_pdev->dev;
+	return 0;
+
+err:
+	if (slave_pdev)
+		platform_device_put(slave_pdev);
+	return ret;
+}
+
+static int msm_ssbi_probe(struct platform_device *pdev)
+{
+	const struct msm_ssbi_platform_data *pdata = pdev->dev.platform_data;
+	struct resource *mem_res;
+	struct msm_ssbi *ssbi;
+	int ret = 0;
+
+	if (!pdata) {
+		pr_err("missing platform data\n");
+		return -EINVAL;
+	}
+
+	pr_debug("%s\n", pdata->slave.name);
+
+	ssbi = kzalloc(sizeof(struct msm_ssbi), GFP_KERNEL);
+	if (!ssbi) {
+		pr_err("can not allocate ssbi_data\n");
+		return -ENOMEM;
+	}
+
+	mem_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!mem_res) {
+		pr_err("missing mem resource\n");
+		ret = -EINVAL;
+		goto err_get_mem_res;
+	}
+
+	ssbi->base = ioremap(mem_res->start, resource_size(mem_res));
+	if (!ssbi->base) {
+		pr_err("ioremap of 0x%p failed\n", (void *)mem_res->start);
+		ret = -EINVAL;
+		goto err_ioremap;
+	}
+	ssbi->dev = &pdev->dev;
+	platform_set_drvdata(pdev, ssbi);
+
+	ssbi->controller_type = pdata->controller_type;
+	if (ssbi->controller_type == MSM_SBI_CTRL_PMIC_ARBITER) {
+		ssbi->read = msm_ssbi_pa_read_bytes;
+		ssbi->write = msm_ssbi_pa_write_bytes;
+	} else {
+		ssbi->read = msm_ssbi_read_bytes;
+		ssbi->write = msm_ssbi_write_bytes;
+	}
+
+	spin_lock_init(&ssbi->lock);
+
+	ret = msm_ssbi_add_slave(ssbi, &pdata->slave);
+	if (ret)
+		goto err_ssbi_add_slave;
+
+	return 0;
+
+err_ssbi_add_slave:
+	platform_set_drvdata(pdev, NULL);
+	iounmap(ssbi->base);
+err_ioremap:
+err_get_mem_res:
+	kfree(ssbi);
+	return ret;
+}
+
+static int msm_ssbi_remove(struct platform_device *pdev)
+{
+	struct msm_ssbi *ssbi = platform_get_drvdata(pdev);
+
+	platform_set_drvdata(pdev, NULL);
+	iounmap(ssbi->base);
+	kfree(ssbi);
+	return 0;
+}
+
+static struct platform_driver msm_ssbi_driver = {
+	.probe		= msm_ssbi_probe,
+	.remove		= __exit_p(msm_ssbi_remove),
+	.driver		= {
+		.name	= "msm_ssbi",
+		.owner	= THIS_MODULE,
+	},
+};
+
+static int __init msm_ssbi_init(void)
+{
+	return platform_driver_register(&msm_ssbi_driver);
+}
+postcore_initcall(msm_ssbi_init);
+
+static void __exit msm_ssbi_exit(void)
+{
+	platform_driver_unregister(&msm_ssbi_driver);
+}
+module_exit(msm_ssbi_exit)
+
+MODULE_LICENSE("GPL v2");
+MODULE_VERSION("1.0");
+MODULE_ALIAS("platform:msm_ssbi");
+MODULE_AUTHOR("Dima Zavin <dima@android.com>");
diff --git a/include/linux/msm_ssbi.h b/include/linux/msm_ssbi.h
new file mode 100644
index 0000000..cfa47df
--- /dev/null
+++ b/include/linux/msm_ssbi.h
@@ -0,0 +1,49 @@
+/* Copyright (C) 2010 Google, Inc.
+ * Copyright (c) 2011, Code Aurora Forum. All rights reserved.
+ * Author: Dima Zavin <dima@android.com>
+ *
+ * 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_MSM_SSBI_H
+#define _LINUX_MSM_SSBI_H
+
+#include <linux/types.h>
+
+struct msm_ssbi_slave_info {
+	const char	*name;
+	void		*platform_data;
+};
+
+enum msm_ssbi_controller_type {
+	MSM_SBI_CTRL_SSBI = 0,
+	MSM_SBI_CTRL_SSBI2,
+	MSM_SBI_CTRL_PMIC_ARBITER,
+};
+
+struct msm_ssbi_platform_data {
+	struct msm_ssbi_slave_info	slave;
+	enum msm_ssbi_controller_type controller_type;
+};
+
+#ifdef CONFIG_MSM_SSBI
+int msm_ssbi_write(struct device *dev, u16 addr, u8 *buf, int len);
+int msm_ssbi_read(struct device *dev, u16 addr, u8 *buf, int len);
+#else
+static inline int msm_ssbi_write(struct device *dev, u16 addr, u8 *buf, int len)
+{
+	return -ENXIO;
+}
+static inline int msm_ssbi_read(struct device *dev, u16 addr, u8 *buf, int len)
+{
+	return -ENXIO;
+}
+#endif
+#endif
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

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

* [PATCH v2 02/11] fix: Use EXPORT_SYMBOL_GPL
  2013-03-12 18:41 ` [PATCH v2 0/11] Qualcomm SSBI bus driver David Brown
  2013-03-12 18:41   ` [PATCH v2 01/11] platform-drivers: msm: add single-wire serial bus interface (SSBI) driver David Brown
@ 2013-03-12 18:41   ` David Brown
  2013-03-12 18:41   ` [PATCH v2 03/11] ssbi: Fix exit mismatch in remove function David Brown
                     ` (8 subsequent siblings)
  10 siblings, 0 replies; 33+ messages in thread
From: David Brown @ 2013-03-12 18:41 UTC (permalink / raw)
  To: linux-arm-kernel

Signed-off-by: David Brown <davidb@codeaurora.org>
---
 drivers/ssbi/ssbi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/ssbi/ssbi.c b/drivers/ssbi/ssbi.c
index 8b0b10d..c08a7b8 100644
--- a/drivers/ssbi/ssbi.c
+++ b/drivers/ssbi/ssbi.c
@@ -242,7 +242,7 @@ int msm_ssbi_read(struct device *dev, u16 addr, u8 *buf, int len)
 
 	return ret;
 }
-EXPORT_SYMBOL(msm_ssbi_read);
+EXPORT_SYMBOL_GPL(msm_ssbi_read);
 
 int msm_ssbi_write(struct device *dev, u16 addr, u8 *buf, int len)
 {
@@ -259,7 +259,7 @@ int msm_ssbi_write(struct device *dev, u16 addr, u8 *buf, int len)
 
 	return ret;
 }
-EXPORT_SYMBOL(msm_ssbi_write);
+EXPORT_SYMBOL_GPL(msm_ssbi_write);
 
 static int msm_ssbi_add_slave(struct msm_ssbi *ssbi,
 				const struct msm_ssbi_slave_info *slave)
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

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

* [PATCH v2 03/11] ssbi: Fix exit mismatch in remove function
  2013-03-12 18:41 ` [PATCH v2 0/11] Qualcomm SSBI bus driver David Brown
  2013-03-12 18:41   ` [PATCH v2 01/11] platform-drivers: msm: add single-wire serial bus interface (SSBI) driver David Brown
  2013-03-12 18:41   ` [PATCH v2 02/11] fix: Use EXPORT_SYMBOL_GPL David Brown
@ 2013-03-12 18:41   ` David Brown
  2013-03-12 18:41   ` [PATCH v2 04/11] ssbi: Allow compilation as a module David Brown
                     ` (7 subsequent siblings)
  10 siblings, 0 replies; 33+ messages in thread
From: David Brown @ 2013-03-12 18:41 UTC (permalink / raw)
  To: linux-arm-kernel

msm_ssbi_remove is referenced with __exit_p, but not declared with
__exit.  This causes a warning when the driver is not built as a
module:

drivers/ssbi/ssbi.c:341:23: warning: 'msm_ssbi_remove' defined but not used [-Wunused-function]

The remove is needed for unbinding to work, even if not compiled as a
module, so just remove it.

Signed-off-by: David Brown <davidb@codeaurora.org>
---
 drivers/ssbi/ssbi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/ssbi/ssbi.c b/drivers/ssbi/ssbi.c
index c08a7b8..da086d4 100644
--- a/drivers/ssbi/ssbi.c
+++ b/drivers/ssbi/ssbi.c
@@ -372,7 +372,7 @@ static int msm_ssbi_remove(struct platform_device *pdev)
 
 static struct platform_driver msm_ssbi_driver = {
 	.probe		= msm_ssbi_probe,
-	.remove		= __exit_p(msm_ssbi_remove),
+	.remove		= msm_ssbi_remove,
 	.driver		= {
 		.name	= "msm_ssbi",
 		.owner	= THIS_MODULE,
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

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

* [PATCH v2 04/11] ssbi: Allow compilation as a module
  2013-03-12 18:41 ` [PATCH v2 0/11] Qualcomm SSBI bus driver David Brown
                     ` (2 preceding siblings ...)
  2013-03-12 18:41   ` [PATCH v2 03/11] ssbi: Fix exit mismatch in remove function David Brown
@ 2013-03-12 18:41   ` David Brown
  2013-03-12 18:41   ` [PATCH v2 05/11] SSBI: Convert SSBI to device tree David Brown
                     ` (6 subsequent siblings)
  10 siblings, 0 replies; 33+ messages in thread
From: David Brown @ 2013-03-12 18:41 UTC (permalink / raw)
  To: linux-arm-kernel

The ssbi driver's read/write entry points are protected with wrappers
in the case when the driver isn't enabled.  These wrappers don't make
any sense, since a client of the SSBI bus won't work without it.  Make
these just regular functions, so that the SSBI driver can be built as
a module.

Signed-off-by: David Brown <davidb@codeaurora.org>
---
 drivers/ssbi/Kconfig     |  2 +-
 include/linux/msm_ssbi.h | 11 -----------
 2 files changed, 1 insertion(+), 12 deletions(-)

diff --git a/drivers/ssbi/Kconfig b/drivers/ssbi/Kconfig
index b57c41b..c7bc534 100644
--- a/drivers/ssbi/Kconfig
+++ b/drivers/ssbi/Kconfig
@@ -5,7 +5,7 @@
 menu "Qualcomm MSM SSBI bus support"
 
 config MSM_SSBI
-	bool "Qualcomm Single-wire Serial Bus Interface (SSBI)"
+	tristate "Qualcomm Single-wire Serial Bus Interface (SSBI)"
 	help
 	  If you say yes to this option, support will be included for the
 	  built-in SSBI interface on Qualcomm MSM family processors.
diff --git a/include/linux/msm_ssbi.h b/include/linux/msm_ssbi.h
index cfa47df..0fe245b 100644
--- a/include/linux/msm_ssbi.h
+++ b/include/linux/msm_ssbi.h
@@ -33,17 +33,6 @@ struct msm_ssbi_platform_data {
 	enum msm_ssbi_controller_type controller_type;
 };
 
-#ifdef CONFIG_MSM_SSBI
 int msm_ssbi_write(struct device *dev, u16 addr, u8 *buf, int len);
 int msm_ssbi_read(struct device *dev, u16 addr, u8 *buf, int len);
-#else
-static inline int msm_ssbi_write(struct device *dev, u16 addr, u8 *buf, int len)
-{
-	return -ENXIO;
-}
-static inline int msm_ssbi_read(struct device *dev, u16 addr, u8 *buf, int len)
-{
-	return -ENXIO;
-}
-#endif
 #endif
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

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

* [PATCH v2 05/11] SSBI: Convert SSBI to device tree
  2013-03-12 18:41 ` [PATCH v2 0/11] Qualcomm SSBI bus driver David Brown
                     ` (3 preceding siblings ...)
  2013-03-12 18:41   ` [PATCH v2 04/11] ssbi: Allow compilation as a module David Brown
@ 2013-03-12 18:41   ` David Brown
  2013-03-12 20:46     ` Stephen Boyd
  2013-03-12 18:41   ` [PATCH v2 06/11] ssbi: Comment the use of udelay() David Brown
                     ` (5 subsequent siblings)
  10 siblings, 1 reply; 33+ messages in thread
From: David Brown @ 2013-03-12 18:41 UTC (permalink / raw)
  To: linux-arm-kernel

The SSBI bus is exclusive to the Qualcomm MSM targets, and all SoCs
using it will be using device tree.  Convert this driver to indentify
with device tree.

This makes the bus probing a good bit simpler, since the attaching of
child nodes can be represented directly in the devicetree, rather than
having to be inferred by name.

Signed-off-by: David Brown <davidb@codeaurora.org>
---
 Documentation/devicetree/bindings/arm/msm/ssbi.txt | 18 +++++
 arch/arm/boot/dts/msm8660-surf.dts                 |  6 ++
 arch/arm/boot/dts/msm8960-cdp.dts                  |  6 ++
 drivers/ssbi/ssbi.c                                | 81 +++++++++-------------
 4 files changed, 62 insertions(+), 49 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/msm/ssbi.txt

diff --git a/Documentation/devicetree/bindings/arm/msm/ssbi.txt b/Documentation/devicetree/bindings/arm/msm/ssbi.txt
new file mode 100644
index 0000000..54fd5ce
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/msm/ssbi.txt
@@ -0,0 +1,18 @@
+* Qualcomm SSBI
+
+Some Qualcomm MSM devices contain a point-to-point serial bus used to
+communicate with a limited range of devices (mostly power management
+chips).
+
+These require the following properties:
+
+- compatible: "qcom,ssbi"
+
+- qcom,controller-type
+  indicates the SSBI bus variant the controller should use to talk
+  with the slave device.  This should be one of "ssbi", "ssbi2", or
+  "pmic-arbiter".  The type chosen is determined by the attached
+  slave.
+
+The slave device should be the single child node of the ssbi device
+with a compatible field.
diff --git a/arch/arm/boot/dts/msm8660-surf.dts b/arch/arm/boot/dts/msm8660-surf.dts
index 31f2157..67f8670 100644
--- a/arch/arm/boot/dts/msm8660-surf.dts
+++ b/arch/arm/boot/dts/msm8660-surf.dts
@@ -38,4 +38,10 @@
 		      <0x19c00000 0x1000>;
 		interrupts = <0 195 0x0>;
 	};
+
+	qcom,ssbi at 500000 {
+		compatible = "qcom,ssbi";
+		reg = <0x500000 0x1000>;
+		qcom,controller-type = "pmic-arbiter";
+	};
 };
diff --git a/arch/arm/boot/dts/msm8960-cdp.dts b/arch/arm/boot/dts/msm8960-cdp.dts
index 9e621b5..c9b09a8 100644
--- a/arch/arm/boot/dts/msm8960-cdp.dts
+++ b/arch/arm/boot/dts/msm8960-cdp.dts
@@ -38,4 +38,10 @@
 		      <0x16400000 0x1000>;
 		interrupts = <0 154 0x0>;
 	};
+
+	qcom,ssbi at 500000 {
+		compatible = "qcom,ssbi";
+		reg = <0x500000 0x1000>;
+		qcom,controller-type = "pmic-arbiter";
+	};
 };
diff --git a/drivers/ssbi/ssbi.c b/drivers/ssbi/ssbi.c
index da086d4..6fbcb25 100644
--- a/drivers/ssbi/ssbi.c
+++ b/drivers/ssbi/ssbi.c
@@ -26,6 +26,8 @@
 #include <linux/slab.h>
 #include <linux/msm_ssbi.h>
 #include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
 
 /* SSBI 2.0 controller registers */
 #define SSBI2_CMD			0x0008
@@ -261,56 +263,13 @@ int msm_ssbi_write(struct device *dev, u16 addr, u8 *buf, int len)
 }
 EXPORT_SYMBOL_GPL(msm_ssbi_write);
 
-static int msm_ssbi_add_slave(struct msm_ssbi *ssbi,
-				const struct msm_ssbi_slave_info *slave)
-{
-	struct platform_device *slave_pdev;
-	int ret;
-
-	if (ssbi->slave) {
-		pr_err("slave already attached??\n");
-		return -EBUSY;
-	}
-
-	slave_pdev = platform_device_alloc(slave->name, -1);
-	if (!slave_pdev) {
-		pr_err("cannot allocate pdev for slave '%s'", slave->name);
-		ret = -ENOMEM;
-		goto err;
-	}
-
-	slave_pdev->dev.parent = ssbi->dev;
-	slave_pdev->dev.platform_data = slave->platform_data;
-
-	ret = platform_device_add(slave_pdev);
-	if (ret) {
-		pr_err("cannot add slave platform device for '%s'\n",
-				slave->name);
-		goto err;
-	}
-
-	ssbi->slave = &slave_pdev->dev;
-	return 0;
-
-err:
-	if (slave_pdev)
-		platform_device_put(slave_pdev);
-	return ret;
-}
-
 static int msm_ssbi_probe(struct platform_device *pdev)
 {
-	const struct msm_ssbi_platform_data *pdata = pdev->dev.platform_data;
+	struct device_node *np = pdev->dev.of_node;
 	struct resource *mem_res;
 	struct msm_ssbi *ssbi;
 	int ret = 0;
-
-	if (!pdata) {
-		pr_err("missing platform data\n");
-		return -EINVAL;
-	}
-
-	pr_debug("%s\n", pdata->slave.name);
+	const char *type;
 
 	ssbi = kzalloc(sizeof(struct msm_ssbi), GFP_KERNEL);
 	if (!ssbi) {
@@ -334,7 +293,25 @@ static int msm_ssbi_probe(struct platform_device *pdev)
 	ssbi->dev = &pdev->dev;
 	platform_set_drvdata(pdev, ssbi);
 
-	ssbi->controller_type = pdata->controller_type;
+	type = of_get_property(np, "qcom,controller-type", NULL);
+	if (type == NULL) {
+		pr_err("Missing qcom,controller-type property\n");
+		ret = -EINVAL;
+		goto err_ssbi_controller;
+	}
+	dev_info(&pdev->dev, "SSBI controller type: '%s'\n", type);
+	if (strcmp(type, "ssbi") == 0)
+		ssbi->controller_type = MSM_SBI_CTRL_SSBI;
+	else if (strcmp(type, "ssbi2") == 0)
+		ssbi->controller_type = MSM_SBI_CTRL_SSBI2;
+	else if (strcmp(type, "pmic-arbiter") == 0)
+		ssbi->controller_type = MSM_SBI_CTRL_PMIC_ARBITER;
+	else {
+		pr_err("Unknown qcom,controller-type\n");
+		ret = -EINVAL;
+		goto err_ssbi_controller;
+	}
+
 	if (ssbi->controller_type == MSM_SBI_CTRL_PMIC_ARBITER) {
 		ssbi->read = msm_ssbi_pa_read_bytes;
 		ssbi->write = msm_ssbi_pa_write_bytes;
@@ -345,13 +322,13 @@ static int msm_ssbi_probe(struct platform_device *pdev)
 
 	spin_lock_init(&ssbi->lock);
 
-	ret = msm_ssbi_add_slave(ssbi, &pdata->slave);
+	ret = of_platform_populate(np, NULL, NULL, &pdev->dev);
 	if (ret)
-		goto err_ssbi_add_slave;
+		goto err_ssbi_controller;
 
 	return 0;
 
-err_ssbi_add_slave:
+err_ssbi_controller:
 	platform_set_drvdata(pdev, NULL);
 	iounmap(ssbi->base);
 err_ioremap:
@@ -370,12 +347,18 @@ static int msm_ssbi_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static struct of_device_id ssbi_match_table[] = {
+	{ .compatible = "qcom,ssbi" },
+	{}
+};
+
 static struct platform_driver msm_ssbi_driver = {
 	.probe		= msm_ssbi_probe,
 	.remove		= msm_ssbi_remove,
 	.driver		= {
 		.name	= "msm_ssbi",
 		.owner	= THIS_MODULE,
+		.of_match_table = ssbi_match_table,
 	},
 };
 
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

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

* [PATCH v2 06/11] ssbi: Comment the use of udelay()
  2013-03-12 18:41 ` [PATCH v2 0/11] Qualcomm SSBI bus driver David Brown
                     ` (4 preceding siblings ...)
  2013-03-12 18:41   ` [PATCH v2 05/11] SSBI: Convert SSBI to device tree David Brown
@ 2013-03-12 18:41   ` David Brown
  2013-03-12 18:41   ` [PATCH v2 07/11] ssbi: Use regular init level David Brown
                     ` (4 subsequent siblings)
  10 siblings, 0 replies; 33+ messages in thread
From: David Brown @ 2013-03-12 18:41 UTC (permalink / raw)
  To: linux-arm-kernel

The ssbi driver uses a busywait loop to read its status register.  Add
a comment explaining the timing of the device itself so that future
developers can better understand this delay, and possibly diagnose any
problems.

Signed-off-by: David Brown <davidb@codeaurora.org>
---
 drivers/ssbi/ssbi.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/ssbi/ssbi.c b/drivers/ssbi/ssbi.c
index 6fbcb25..7ae8925 100644
--- a/drivers/ssbi/ssbi.c
+++ b/drivers/ssbi/ssbi.c
@@ -87,6 +87,15 @@ static inline void ssbi_writel(struct msm_ssbi *ssbi, u32 val, u32 reg)
 	writel(val, ssbi->base + reg);
 }
 
+/*
+ * Via private exchange with one of the original authors, the hardware
+ * should generally finish a transaction in about 5us.  The worst
+ * case, is when using the arbiter and both other CPUs have just
+ * started trying to use the SSBI bus will result in a time of about
+ * 20us.  It should never take longer than this.
+ *
+ * As such, this wait merely spins, with a udelay.
+ */
 static int ssbi_wait_mask(struct msm_ssbi *ssbi, u32 set_mask, u32 clr_mask)
 {
 	u32 timeout = SSBI_TIMEOUT_US;
@@ -161,6 +170,10 @@ err:
 	return ret;
 }
 
+/*
+ * See ssbi_wait_mask for an explanation of the time and the
+ * busywait.
+ */
 static inline int
 msm_ssbi_pa_transfer(struct msm_ssbi *ssbi, u32 cmd, u8 *data)
 {
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

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

* [PATCH v2 07/11] ssbi: Use regular init level
  2013-03-12 18:41 ` [PATCH v2 0/11] Qualcomm SSBI bus driver David Brown
                     ` (5 preceding siblings ...)
  2013-03-12 18:41   ` [PATCH v2 06/11] ssbi: Comment the use of udelay() David Brown
@ 2013-03-12 18:41   ` David Brown
  2013-03-12 20:26     ` Stephen Boyd
  2013-03-12 18:41   ` [PATCH v2 08/11] ssbi: Remove extraneous logging David Brown
                     ` (3 subsequent siblings)
  10 siblings, 1 reply; 33+ messages in thread
From: David Brown @ 2013-03-12 18:41 UTC (permalink / raw)
  To: linux-arm-kernel

With device tree, and deferred probe, it is no longer necessary to
make sure that the ssbi bus driver is initialized very early.  Restore
to a regular module_init().

Signed-off-by: David Brown <davidb@codeaurora.org>
---
 drivers/ssbi/ssbi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/ssbi/ssbi.c b/drivers/ssbi/ssbi.c
index 7ae8925..6878e55 100644
--- a/drivers/ssbi/ssbi.c
+++ b/drivers/ssbi/ssbi.c
@@ -379,7 +379,7 @@ static int __init msm_ssbi_init(void)
 {
 	return platform_driver_register(&msm_ssbi_driver);
 }
-postcore_initcall(msm_ssbi_init);
+module_init(msm_ssbi_init);
 
 static void __exit msm_ssbi_exit(void)
 {
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

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

* [PATCH v2 08/11] ssbi: Remove extraneous logging
  2013-03-12 18:41 ` [PATCH v2 0/11] Qualcomm SSBI bus driver David Brown
                     ` (6 preceding siblings ...)
  2013-03-12 18:41   ` [PATCH v2 07/11] ssbi: Use regular init level David Brown
@ 2013-03-12 18:41   ` David Brown
  2013-03-12 18:41   ` [PATCH v2 09/11] SSBI: Remove MSM_ prefix from SSBI drivers David Brown
                     ` (2 subsequent siblings)
  10 siblings, 0 replies; 33+ messages in thread
From: David Brown @ 2013-03-12 18:41 UTC (permalink / raw)
  To: linux-arm-kernel

Remove some unhelpful error logs.  This also removes the necessity of
having a pointer back to the struct device within the ssbi-specific
structure

Signed-off-by: David Brown <davidb@codeaurora.org>
---
 drivers/ssbi/ssbi.c | 16 +---------------
 1 file changed, 1 insertion(+), 15 deletions(-)

diff --git a/drivers/ssbi/ssbi.c b/drivers/ssbi/ssbi.c
index 6878e55..b056a07 100644
--- a/drivers/ssbi/ssbi.c
+++ b/drivers/ssbi/ssbi.c
@@ -66,7 +66,6 @@
 #define SSBI_TIMEOUT_US			100
 
 struct msm_ssbi {
-	struct device		*dev;
 	struct device		*slave;
 	void __iomem		*base;
 	spinlock_t		lock;
@@ -108,8 +107,6 @@ static int ssbi_wait_mask(struct msm_ssbi *ssbi, u32 set_mask, u32 clr_mask)
 		udelay(1);
 	}
 
-	dev_err(ssbi->dev, "%s: timeout (status %x set_mask %x clr_mask %x)\n",
-		__func__, ssbi_readl(ssbi, SSBI2_STATUS), set_mask, clr_mask);
 	return -ETIMEDOUT;
 }
 
@@ -185,11 +182,8 @@ msm_ssbi_pa_transfer(struct msm_ssbi *ssbi, u32 cmd, u8 *data)
 	while (timeout--) {
 		rd_status = ssbi_readl(ssbi, SSBI_PA_RD_STATUS);
 
-		if (rd_status & SSBI_PA_RD_STATUS_TRANS_DENIED) {
-			dev_err(ssbi->dev, "%s: transaction denied (0x%x)\n",
-					__func__, rd_status);
+		if (rd_status & SSBI_PA_RD_STATUS_TRANS_DENIED)
 			return -EPERM;
-		}
 
 		if (rd_status & SSBI_PA_RD_STATUS_TRANS_DONE) {
 			if (data)
@@ -199,7 +193,6 @@ msm_ssbi_pa_transfer(struct msm_ssbi *ssbi, u32 cmd, u8 *data)
 		udelay(1);
 	}
 
-	dev_err(ssbi->dev, "%s: timeout, status 0x%x\n", __func__, rd_status);
 	return -ETIMEDOUT;
 }
 
@@ -248,9 +241,6 @@ int msm_ssbi_read(struct device *dev, u16 addr, u8 *buf, int len)
 	unsigned long flags;
 	int ret;
 
-	if (ssbi->dev != dev)
-		return -ENXIO;
-
 	spin_lock_irqsave(&ssbi->lock, flags);
 	ret = ssbi->read(ssbi, addr, buf, len);
 	spin_unlock_irqrestore(&ssbi->lock, flags);
@@ -265,9 +255,6 @@ int msm_ssbi_write(struct device *dev, u16 addr, u8 *buf, int len)
 	unsigned long flags;
 	int ret;
 
-	if (ssbi->dev != dev)
-		return -ENXIO;
-
 	spin_lock_irqsave(&ssbi->lock, flags);
 	ret = ssbi->write(ssbi, addr, buf, len);
 	spin_unlock_irqrestore(&ssbi->lock, flags);
@@ -303,7 +290,6 @@ static int msm_ssbi_probe(struct platform_device *pdev)
 		ret = -EINVAL;
 		goto err_ioremap;
 	}
-	ssbi->dev = &pdev->dev;
 	platform_set_drvdata(pdev, ssbi);
 
 	type = of_get_property(np, "qcom,controller-type", NULL);
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

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

* [PATCH v2 09/11] SSBI: Remove MSM_ prefix from SSBI drivers
  2013-03-12 18:41 ` [PATCH v2 0/11] Qualcomm SSBI bus driver David Brown
                     ` (7 preceding siblings ...)
  2013-03-12 18:41   ` [PATCH v2 08/11] ssbi: Remove extraneous logging David Brown
@ 2013-03-12 18:41   ` David Brown
  2013-03-12 18:41   ` [PATCH v2 10/11] MAINTAINERS: add ssbi David Brown
  2013-03-25 17:40   ` [PATCH v2 0/11] Qualcomm SSBI bus driver Greg Kroah-Hartman
  10 siblings, 0 replies; 33+ messages in thread
From: David Brown @ 2013-03-12 18:41 UTC (permalink / raw)
  To: linux-arm-kernel

Although the SSBI sub is currently only used on MSM SoCs, it is still
a bus in its own right.  Remove this msm_ prefix from the driver and
it's symbols.  Clients can now refer directly to ssbi_write() and
ssbi_read().

Signed-off-by: David Brown <davidb@codeaurora.org>
---
 drivers/Makefile                     |  2 +-
 drivers/mfd/Kconfig                  |  2 +-
 drivers/mfd/pm8921-core.c            | 14 +++---
 drivers/ssbi/Kconfig                 |  4 +-
 drivers/ssbi/Makefile                |  2 +-
 drivers/ssbi/ssbi.c                  | 86 ++++++++++++++++++------------------
 include/linux/{msm_ssbi.h => ssbi.h} | 18 ++++----
 7 files changed, 64 insertions(+), 64 deletions(-)
 rename include/linux/{msm_ssbi.h => ssbi.h} (67%)

diff --git a/drivers/Makefile b/drivers/Makefile
index 778821b..4865ed2 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -114,7 +114,7 @@ obj-y				+= firmware/
 obj-$(CONFIG_CRYPTO)		+= crypto/
 obj-$(CONFIG_SUPERH)		+= sh/
 obj-$(CONFIG_ARCH_SHMOBILE)	+= sh/
-obj-$(CONFIG_MSM_SSBI)		+= ssbi/
+obj-$(CONFIG_SSBI)		+= ssbi/
 ifndef CONFIG_ARCH_USES_GETTIMEOFFSET
 obj-y				+= clocksource/
 endif
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 671f5b1..5bfa7bb 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -990,7 +990,7 @@ config MFD_PM8XXX
 
 config MFD_PM8921_CORE
 	tristate "Qualcomm PM8921 PMIC chip"
-	depends on MSM_SSBI
+	depends on SSBI
 	select MFD_CORE
 	select MFD_PM8XXX
 	help
diff --git a/drivers/mfd/pm8921-core.c b/drivers/mfd/pm8921-core.c
index d4b297c..ecc137f 100644
--- a/drivers/mfd/pm8921-core.c
+++ b/drivers/mfd/pm8921-core.c
@@ -17,7 +17,7 @@
 #include <linux/platform_device.h>
 #include <linux/slab.h>
 #include <linux/err.h>
-#include <linux/msm_ssbi.h>
+#include <linux/ssbi.h>
 #include <linux/mfd/core.h>
 #include <linux/mfd/pm8xxx/pm8921.h>
 #include <linux/mfd/pm8xxx/core.h>
@@ -35,7 +35,7 @@ static int pm8921_readb(const struct device *dev, u16 addr, u8 *val)
 	const struct pm8xxx_drvdata *pm8921_drvdata = dev_get_drvdata(dev);
 	const struct pm8921 *pmic = pm8921_drvdata->pm_chip_data;
 
-	return msm_ssbi_read(pmic->dev->parent, addr, val, 1);
+	return ssbi_read(pmic->dev->parent, addr, val, 1);
 }
 
 static int pm8921_writeb(const struct device *dev, u16 addr, u8 val)
@@ -43,7 +43,7 @@ static int pm8921_writeb(const struct device *dev, u16 addr, u8 val)
 	const struct pm8xxx_drvdata *pm8921_drvdata = dev_get_drvdata(dev);
 	const struct pm8921 *pmic = pm8921_drvdata->pm_chip_data;
 
-	return msm_ssbi_write(pmic->dev->parent, addr, &val, 1);
+	return ssbi_write(pmic->dev->parent, addr, &val, 1);
 }
 
 static int pm8921_read_buf(const struct device *dev, u16 addr, u8 *buf,
@@ -52,7 +52,7 @@ static int pm8921_read_buf(const struct device *dev, u16 addr, u8 *buf,
 	const struct pm8xxx_drvdata *pm8921_drvdata = dev_get_drvdata(dev);
 	const struct pm8921 *pmic = pm8921_drvdata->pm_chip_data;
 
-	return msm_ssbi_read(pmic->dev->parent, addr, buf, cnt);
+	return ssbi_read(pmic->dev->parent, addr, buf, cnt);
 }
 
 static int pm8921_write_buf(const struct device *dev, u16 addr, u8 *buf,
@@ -61,7 +61,7 @@ static int pm8921_write_buf(const struct device *dev, u16 addr, u8 *buf,
 	const struct pm8xxx_drvdata *pm8921_drvdata = dev_get_drvdata(dev);
 	const struct pm8921 *pmic = pm8921_drvdata->pm_chip_data;
 
-	return msm_ssbi_write(pmic->dev->parent, addr, buf, cnt);
+	return ssbi_write(pmic->dev->parent, addr, buf, cnt);
 }
 
 static int pm8921_read_irq_stat(const struct device *dev, int irq)
@@ -124,7 +124,7 @@ static int pm8921_probe(struct platform_device *pdev)
 	}
 
 	/* Read PMIC chip revision */
-	rc = msm_ssbi_read(pdev->dev.parent, REG_HWREV, &val, sizeof(val));
+	rc = ssbi_read(pdev->dev.parent, REG_HWREV, &val, sizeof(val));
 	if (rc) {
 		pr_err("Failed to read hw rev reg %d:rc=%d\n", REG_HWREV, rc);
 		goto err_read_rev;
@@ -133,7 +133,7 @@ static int pm8921_probe(struct platform_device *pdev)
 	rev = val;
 
 	/* Read PMIC chip revision 2 */
-	rc = msm_ssbi_read(pdev->dev.parent, REG_HWREV_2, &val, sizeof(val));
+	rc = ssbi_read(pdev->dev.parent, REG_HWREV_2, &val, sizeof(val));
 	if (rc) {
 		pr_err("Failed to read hw rev 2 reg %d:rc=%d\n",
 			REG_HWREV_2, rc);
diff --git a/drivers/ssbi/Kconfig b/drivers/ssbi/Kconfig
index c7bc534..1ae4040 100644
--- a/drivers/ssbi/Kconfig
+++ b/drivers/ssbi/Kconfig
@@ -1,10 +1,10 @@
 #
-# MSM SSBI bus support
+# SSBI bus support
 #
 
 menu "Qualcomm MSM SSBI bus support"
 
-config MSM_SSBI
+config SSBI
 	tristate "Qualcomm Single-wire Serial Bus Interface (SSBI)"
 	help
 	  If you say yes to this option, support will be included for the
diff --git a/drivers/ssbi/Makefile b/drivers/ssbi/Makefile
index 22e408f..38fb70c 100644
--- a/drivers/ssbi/Makefile
+++ b/drivers/ssbi/Makefile
@@ -1 +1 @@
-obj-$(CONFIG_MSM_SSBI) += ssbi.o
+obj-$(CONFIG_SSBI) += ssbi.o
diff --git a/drivers/ssbi/ssbi.c b/drivers/ssbi/ssbi.c
index b056a07..f32da02 100644
--- a/drivers/ssbi/ssbi.c
+++ b/drivers/ssbi/ssbi.c
@@ -1,4 +1,4 @@
-/* Copyright (c) 2009-2011, Code Aurora Forum. All rights reserved.
+/* Copyright (c) 2009-2013, The Linux Foundation. All rights reserved.
  * Copyright (c) 2010, Google Inc.
  *
  * Original authors: Code Aurora Forum
@@ -24,7 +24,7 @@
 #include <linux/kernel.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
-#include <linux/msm_ssbi.h>
+#include <linux/ssbi.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
@@ -65,23 +65,23 @@
 
 #define SSBI_TIMEOUT_US			100
 
-struct msm_ssbi {
+struct ssbi {
 	struct device		*slave;
 	void __iomem		*base;
 	spinlock_t		lock;
-	enum msm_ssbi_controller_type controller_type;
-	int (*read)(struct msm_ssbi *, u16 addr, u8 *buf, int len);
-	int (*write)(struct msm_ssbi *, u16 addr, u8 *buf, int len);
+	enum ssbi_controller_type controller_type;
+	int (*read)(struct ssbi *, u16 addr, u8 *buf, int len);
+	int (*write)(struct ssbi *, u16 addr, u8 *buf, int len);
 };
 
-#define to_msm_ssbi(dev)	platform_get_drvdata(to_platform_device(dev))
+#define to_ssbi(dev)	platform_get_drvdata(to_platform_device(dev))
 
-static inline u32 ssbi_readl(struct msm_ssbi *ssbi, u32 reg)
+static inline u32 ssbi_readl(struct ssbi *ssbi, u32 reg)
 {
 	return readl(ssbi->base + reg);
 }
 
-static inline void ssbi_writel(struct msm_ssbi *ssbi, u32 val, u32 reg)
+static inline void ssbi_writel(struct ssbi *ssbi, u32 val, u32 reg)
 {
 	writel(val, ssbi->base + reg);
 }
@@ -95,7 +95,7 @@ static inline void ssbi_writel(struct msm_ssbi *ssbi, u32 val, u32 reg)
  *
  * As such, this wait merely spins, with a udelay.
  */
-static int ssbi_wait_mask(struct msm_ssbi *ssbi, u32 set_mask, u32 clr_mask)
+static int ssbi_wait_mask(struct ssbi *ssbi, u32 set_mask, u32 clr_mask)
 {
 	u32 timeout = SSBI_TIMEOUT_US;
 	u32 val;
@@ -111,7 +111,7 @@ static int ssbi_wait_mask(struct msm_ssbi *ssbi, u32 set_mask, u32 clr_mask)
 }
 
 static int
-msm_ssbi_read_bytes(struct msm_ssbi *ssbi, u16 addr, u8 *buf, int len)
+ssbi_read_bytes(struct ssbi *ssbi, u16 addr, u8 *buf, int len)
 {
 	u32 cmd = SSBI_CMD_RDWRN | ((addr & 0xff) << 16);
 	int ret = 0;
@@ -140,7 +140,7 @@ err:
 }
 
 static int
-msm_ssbi_write_bytes(struct msm_ssbi *ssbi, u16 addr, u8 *buf, int len)
+ssbi_write_bytes(struct ssbi *ssbi, u16 addr, u8 *buf, int len)
 {
 	int ret = 0;
 
@@ -172,7 +172,7 @@ err:
  * busywait.
  */
 static inline int
-msm_ssbi_pa_transfer(struct msm_ssbi *ssbi, u32 cmd, u8 *data)
+ssbi_pa_transfer(struct ssbi *ssbi, u32 cmd, u8 *data)
 {
 	u32 timeout = SSBI_TIMEOUT_US;
 	u32 rd_status = 0;
@@ -197,7 +197,7 @@ msm_ssbi_pa_transfer(struct msm_ssbi *ssbi, u32 cmd, u8 *data)
 }
 
 static int
-msm_ssbi_pa_read_bytes(struct msm_ssbi *ssbi, u16 addr, u8 *buf, int len)
+ssbi_pa_read_bytes(struct ssbi *ssbi, u16 addr, u8 *buf, int len)
 {
 	u32 cmd;
 	int ret = 0;
@@ -205,7 +205,7 @@ msm_ssbi_pa_read_bytes(struct msm_ssbi *ssbi, u16 addr, u8 *buf, int len)
 	cmd = SSBI_PA_CMD_RDWRN | (addr & SSBI_PA_CMD_ADDR_MASK) << 8;
 
 	while (len) {
-		ret = msm_ssbi_pa_transfer(ssbi, cmd, buf);
+		ret = ssbi_pa_transfer(ssbi, cmd, buf);
 		if (ret)
 			goto err;
 		buf++;
@@ -217,14 +217,14 @@ err:
 }
 
 static int
-msm_ssbi_pa_write_bytes(struct msm_ssbi *ssbi, u16 addr, u8 *buf, int len)
+ssbi_pa_write_bytes(struct ssbi *ssbi, u16 addr, u8 *buf, int len)
 {
 	u32 cmd;
 	int ret = 0;
 
 	while (len) {
 		cmd = (addr & SSBI_PA_CMD_ADDR_MASK) << 8 | *buf;
-		ret = msm_ssbi_pa_transfer(ssbi, cmd, NULL);
+		ret = ssbi_pa_transfer(ssbi, cmd, NULL);
 		if (ret)
 			goto err;
 		buf++;
@@ -235,9 +235,9 @@ err:
 	return ret;
 }
 
-int msm_ssbi_read(struct device *dev, u16 addr, u8 *buf, int len)
+int ssbi_read(struct device *dev, u16 addr, u8 *buf, int len)
 {
-	struct msm_ssbi *ssbi = to_msm_ssbi(dev);
+	struct ssbi *ssbi = to_ssbi(dev);
 	unsigned long flags;
 	int ret;
 
@@ -247,11 +247,11 @@ int msm_ssbi_read(struct device *dev, u16 addr, u8 *buf, int len)
 
 	return ret;
 }
-EXPORT_SYMBOL_GPL(msm_ssbi_read);
+EXPORT_SYMBOL_GPL(ssbi_read);
 
-int msm_ssbi_write(struct device *dev, u16 addr, u8 *buf, int len)
+int ssbi_write(struct device *dev, u16 addr, u8 *buf, int len)
 {
-	struct msm_ssbi *ssbi = to_msm_ssbi(dev);
+	struct ssbi *ssbi = to_ssbi(dev);
 	unsigned long flags;
 	int ret;
 
@@ -261,17 +261,17 @@ int msm_ssbi_write(struct device *dev, u16 addr, u8 *buf, int len)
 
 	return ret;
 }
-EXPORT_SYMBOL_GPL(msm_ssbi_write);
+EXPORT_SYMBOL_GPL(ssbi_write);
 
-static int msm_ssbi_probe(struct platform_device *pdev)
+static int ssbi_probe(struct platform_device *pdev)
 {
 	struct device_node *np = pdev->dev.of_node;
 	struct resource *mem_res;
-	struct msm_ssbi *ssbi;
+	struct ssbi *ssbi;
 	int ret = 0;
 	const char *type;
 
-	ssbi = kzalloc(sizeof(struct msm_ssbi), GFP_KERNEL);
+	ssbi = kzalloc(sizeof(struct ssbi), GFP_KERNEL);
 	if (!ssbi) {
 		pr_err("can not allocate ssbi_data\n");
 		return -ENOMEM;
@@ -312,11 +312,11 @@ static int msm_ssbi_probe(struct platform_device *pdev)
 	}
 
 	if (ssbi->controller_type == MSM_SBI_CTRL_PMIC_ARBITER) {
-		ssbi->read = msm_ssbi_pa_read_bytes;
-		ssbi->write = msm_ssbi_pa_write_bytes;
+		ssbi->read = ssbi_pa_read_bytes;
+		ssbi->write = ssbi_pa_write_bytes;
 	} else {
-		ssbi->read = msm_ssbi_read_bytes;
-		ssbi->write = msm_ssbi_write_bytes;
+		ssbi->read = ssbi_read_bytes;
+		ssbi->write = ssbi_write_bytes;
 	}
 
 	spin_lock_init(&ssbi->lock);
@@ -336,9 +336,9 @@ err_get_mem_res:
 	return ret;
 }
 
-static int msm_ssbi_remove(struct platform_device *pdev)
+static int ssbi_remove(struct platform_device *pdev)
 {
-	struct msm_ssbi *ssbi = platform_get_drvdata(pdev);
+	struct ssbi *ssbi = platform_get_drvdata(pdev);
 
 	platform_set_drvdata(pdev, NULL);
 	iounmap(ssbi->base);
@@ -351,29 +351,29 @@ static struct of_device_id ssbi_match_table[] = {
 	{}
 };
 
-static struct platform_driver msm_ssbi_driver = {
-	.probe		= msm_ssbi_probe,
-	.remove		= msm_ssbi_remove,
+static struct platform_driver ssbi_driver = {
+	.probe		= ssbi_probe,
+	.remove		= ssbi_remove,
 	.driver		= {
-		.name	= "msm_ssbi",
+		.name	= "ssbi",
 		.owner	= THIS_MODULE,
 		.of_match_table = ssbi_match_table,
 	},
 };
 
-static int __init msm_ssbi_init(void)
+static int __init ssbi_init(void)
 {
-	return platform_driver_register(&msm_ssbi_driver);
+	return platform_driver_register(&ssbi_driver);
 }
-module_init(msm_ssbi_init);
+module_init(ssbi_init);
 
-static void __exit msm_ssbi_exit(void)
+static void __exit ssbi_exit(void)
 {
-	platform_driver_unregister(&msm_ssbi_driver);
+	platform_driver_unregister(&ssbi_driver);
 }
-module_exit(msm_ssbi_exit)
+module_exit(ssbi_exit)
 
 MODULE_LICENSE("GPL v2");
 MODULE_VERSION("1.0");
-MODULE_ALIAS("platform:msm_ssbi");
+MODULE_ALIAS("platform:ssbi");
 MODULE_AUTHOR("Dima Zavin <dima@android.com>");
diff --git a/include/linux/msm_ssbi.h b/include/linux/ssbi.h
similarity index 67%
rename from include/linux/msm_ssbi.h
rename to include/linux/ssbi.h
index 0fe245b..44ef5da 100644
--- a/include/linux/msm_ssbi.h
+++ b/include/linux/ssbi.h
@@ -12,27 +12,27 @@
  * GNU General Public License for more details.
  */
 
-#ifndef _LINUX_MSM_SSBI_H
-#define _LINUX_MSM_SSBI_H
+#ifndef _LINUX_SSBI_H
+#define _LINUX_SSBI_H
 
 #include <linux/types.h>
 
-struct msm_ssbi_slave_info {
+struct ssbi_slave_info {
 	const char	*name;
 	void		*platform_data;
 };
 
-enum msm_ssbi_controller_type {
+enum ssbi_controller_type {
 	MSM_SBI_CTRL_SSBI = 0,
 	MSM_SBI_CTRL_SSBI2,
 	MSM_SBI_CTRL_PMIC_ARBITER,
 };
 
-struct msm_ssbi_platform_data {
-	struct msm_ssbi_slave_info	slave;
-	enum msm_ssbi_controller_type controller_type;
+struct ssbi_platform_data {
+	struct ssbi_slave_info	slave;
+	enum ssbi_controller_type controller_type;
 };
 
-int msm_ssbi_write(struct device *dev, u16 addr, u8 *buf, int len);
-int msm_ssbi_read(struct device *dev, u16 addr, u8 *buf, int len);
+int ssbi_write(struct device *dev, u16 addr, u8 *buf, int len);
+int ssbi_read(struct device *dev, u16 addr, u8 *buf, int len);
 #endif
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

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

* [PATCH v2 10/11] MAINTAINERS: add ssbi
  2013-03-12 18:41 ` [PATCH v2 0/11] Qualcomm SSBI bus driver David Brown
                     ` (8 preceding siblings ...)
  2013-03-12 18:41   ` [PATCH v2 09/11] SSBI: Remove MSM_ prefix from SSBI drivers David Brown
@ 2013-03-12 18:41   ` David Brown
  2013-03-25 17:40   ` [PATCH v2 0/11] Qualcomm SSBI bus driver Greg Kroah-Hartman
  10 siblings, 0 replies; 33+ messages in thread
From: David Brown @ 2013-03-12 18:41 UTC (permalink / raw)
  To: linux-arm-kernel

The ssbi device is specific to the Qualcomm MSM SoCs.

Signed-off-by: David Brown <davidb@codeaurora.org>
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 9561658..f8fdec5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1031,6 +1031,7 @@ F:	drivers/mmc/host/msm_sdcc.h
 F:	drivers/tty/serial/msm_serial.h
 F:	drivers/tty/serial/msm_serial.c
 F:	drivers/*/pm8???-*
+F:	drivers/ssbi/
 F:	include/linux/mfd/pm8xxx/
 T:	git git://git.kernel.org/pub/scm/linux/kernel/git/davidb/linux-msm.git
 S:	Maintained
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

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

* [PATCH v2 11/11] RFC: SSBI: Simple pm8058 test driver
  2013-03-07  0:29 [PATCH 0/6] Qualcomm SSBI bus driver David Brown
                   ` (6 preceding siblings ...)
  2013-03-12 18:41 ` [PATCH v2 0/11] Qualcomm SSBI bus driver David Brown
@ 2013-03-12 20:12 ` David Brown
  7 siblings, 0 replies; 33+ messages in thread
From: David Brown @ 2013-03-12 20:12 UTC (permalink / raw)
  To: linux-arm-kernel

A very small ssbi device driver that reads the pm8058 version register
and prints it out.

Signed-off-by: David Brown <davidb@codeaurora.org>
---
Resending due to address error in header

 arch/arm/boot/dts/msm8660-surf.dts |  4 +++
 arch/arm/boot/dts/msm8960-cdp.dts  |  4 +++
 drivers/ssbi/Makefile              |  1 +
 drivers/ssbi/ssbi-test.c           | 61 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 70 insertions(+)
 create mode 100644 drivers/ssbi/ssbi-test.c

diff --git a/arch/arm/boot/dts/msm8660-surf.dts b/arch/arm/boot/dts/msm8660-surf.dts
index 67f8670..68a0c7c4 100644
--- a/arch/arm/boot/dts/msm8660-surf.dts
+++ b/arch/arm/boot/dts/msm8660-surf.dts
@@ -43,5 +43,9 @@
 		compatible = "qcom,ssbi";
 		reg = <0x500000 0x1000>;
 		qcom,controller-type = "pmic-arbiter";
+
+		qcom,ssbi-test {
+			compatible = "qcom,ssbi-test";
+		};
 	};
 };
diff --git a/arch/arm/boot/dts/msm8960-cdp.dts b/arch/arm/boot/dts/msm8960-cdp.dts
index c9b09a8..60804d7 100644
--- a/arch/arm/boot/dts/msm8960-cdp.dts
+++ b/arch/arm/boot/dts/msm8960-cdp.dts
@@ -43,5 +43,9 @@
 		compatible = "qcom,ssbi";
 		reg = <0x500000 0x1000>;
 		qcom,controller-type = "pmic-arbiter";
+
+		qcom,ssbi-test {
+			compatible = "qcom,ssbi-test";
+		};
 	};
 };
diff --git a/drivers/ssbi/Makefile b/drivers/ssbi/Makefile
index 38fb70c..dea13c1 100644
--- a/drivers/ssbi/Makefile
+++ b/drivers/ssbi/Makefile
@@ -1 +1,2 @@
 obj-$(CONFIG_SSBI) += ssbi.o
+obj-$(CONFIG_SSBI) += ssbi-test.o
diff --git a/drivers/ssbi/ssbi-test.c b/drivers/ssbi/ssbi-test.c
new file mode 100644
index 0000000..7804cfe
--- /dev/null
+++ b/drivers/ssbi/ssbi-test.c
@@ -0,0 +1,61 @@
+/* A simple ssbi test device. */
+
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/ssbi.h>
+
+static int __init ssbi_test_probe(struct platform_device *pdev)
+{
+	int ret;
+	char version;
+
+	dev_info(&pdev->dev, "probe, me = %p, parent = %p\n",
+		 pdev, pdev->dev.parent);
+
+	/* Let's try reading. */
+	ret = ssbi_read(pdev->dev.parent, 0x02, &version, 1);
+	if (ret != 0)
+		return ret;
+
+	dev_info(&pdev->dev, "Version = %02x\n", version);
+
+	/* Should already be hooked in. */
+	return 0;
+}
+
+static int ssbi_test_remove(struct platform_device *pdev)
+{
+	return 0;
+}
+
+static struct of_device_id ssbi_test_match_table[] = {
+	{ .compatible = "qcom,ssbi-test" },
+	{}
+};
+
+static struct platform_driver ssbi_test_driver = {
+	.remove = ssbi_test_remove,
+	.driver = {
+		.name = "sbbi_test",
+		.owner = THIS_MODULE,
+		.of_match_table = ssbi_test_match_table,
+	},
+};
+
+static int __init ssbi_test_init(void)
+{
+	int ret;
+
+	ret = platform_driver_probe(&ssbi_test_driver, ssbi_test_probe);
+	return ret;
+}
+
+static void __exit ssbi_test_exit(void)
+{
+}
+
+module_init(ssbi_test_init);
+module_exit(ssbi_test_exit);
+
+MODULE_LICENSE("GPL");
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

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

* [PATCH v2 07/11] ssbi: Use regular init level
  2013-03-12 18:41   ` [PATCH v2 07/11] ssbi: Use regular init level David Brown
@ 2013-03-12 20:26     ` Stephen Boyd
  0 siblings, 0 replies; 33+ messages in thread
From: Stephen Boyd @ 2013-03-12 20:26 UTC (permalink / raw)
  To: linux-arm-kernel

On 03/12/13 11:41, David Brown wrote:
> With device tree, and deferred probe, it is no longer necessary to
> make sure that the ssbi bus driver is initialized very early.  Restore
> to a regular module_init().
>
> Signed-off-by: David Brown <davidb@codeaurora.org>
> ---
>  drivers/ssbi/ssbi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/ssbi/ssbi.c b/drivers/ssbi/ssbi.c
> index 7ae8925..6878e55 100644
> --- a/drivers/ssbi/ssbi.c
> +++ b/drivers/ssbi/ssbi.c
> @@ -379,7 +379,7 @@ static int __init msm_ssbi_init(void)
>  {
>  	return platform_driver_register(&msm_ssbi_driver);
>  }
> -postcore_initcall(msm_ssbi_init);
> +module_init(msm_ssbi_init);
>  
>  static void __exit msm_ssbi_exit(void)
>  {

With this change we can use module_platform_driver() too.

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

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

* [PATCH v2 05/11] SSBI: Convert SSBI to device tree
  2013-03-12 18:41   ` [PATCH v2 05/11] SSBI: Convert SSBI to device tree David Brown
@ 2013-03-12 20:46     ` Stephen Boyd
  0 siblings, 0 replies; 33+ messages in thread
From: Stephen Boyd @ 2013-03-12 20:46 UTC (permalink / raw)
  To: linux-arm-kernel

On 03/12/13 11:41, David Brown wrote:
> @@ -261,56 +263,13 @@ int msm_ssbi_write(struct device *dev, u16 addr, u8 *buf, int len)
>  }
>  EXPORT_SYMBOL_GPL(msm_ssbi_write);
>  
> -static int msm_ssbi_add_slave(struct msm_ssbi *ssbi,
> -				const struct msm_ssbi_slave_info *slave)
> -{
> -	struct platform_device *slave_pdev;
> -	int ret;
> -
> -	if (ssbi->slave) {
> -		pr_err("slave already attached??\n");
> -		return -EBUSY;
> -	}
> -
> -	slave_pdev = platform_device_alloc(slave->name, -1);
> -	if (!slave_pdev) {
> -		pr_err("cannot allocate pdev for slave '%s'", slave->name);
> -		ret = -ENOMEM;
> -		goto err;
> -	}
> -
> -	slave_pdev->dev.parent = ssbi->dev;
> -	slave_pdev->dev.platform_data = slave->platform_data;
> -
> -	ret = platform_device_add(slave_pdev);
> -	if (ret) {
> -		pr_err("cannot add slave platform device for '%s'\n",
> -				slave->name);
> -		goto err;
> -	}
> -
> -	ssbi->slave = &slave_pdev->dev;
> -	return 0;
> -
> -err:
> -	if (slave_pdev)
> -		platform_device_put(slave_pdev);
> -	return ret;
> -}
> -
>  static int msm_ssbi_probe(struct platform_device *pdev)
>  {
> -	const struct msm_ssbi_platform_data *pdata = pdev->dev.platform_data;

Now that all this code is gone do we have a user of the
msm_ssbi_platform_data struct? Why not remove it from the header file?
If you remove it from the header you can probably move the enum for
controller_type into this file too.

> @@ -334,7 +293,25 @@ static int msm_ssbi_probe(struct platform_device *pdev)
>  	ssbi->dev = &pdev->dev;
>  	platform_set_drvdata(pdev, ssbi);
>  
> -	ssbi->controller_type = pdata->controller_type;
> +	type = of_get_property(np, "qcom,controller-type", NULL);
> +	if (type == NULL) {
> +		pr_err("Missing qcom,controller-type property\n");

This could be dev_err() considering you use dev_info() below.
> +		ret = -EINVAL;
> +		goto err_ssbi_controller;
> +	}
> +	dev_info(&pdev->dev, "SSBI controller type: '%s'\n", type);
> +	if (strcmp(type, "ssbi") == 0)
> +		ssbi->controller_type = MSM_SBI_CTRL_SSBI;
> +	else if (strcmp(type, "ssbi2") == 0)
> +		ssbi->controller_type = MSM_SBI_CTRL_SSBI2;
> +	else if (strcmp(type, "pmic-arbiter") == 0)
> +		ssbi->controller_type = MSM_SBI_CTRL_PMIC_ARBITER;
> +	else {
> +		pr_err("Unknown qcom,controller-type\n");

dev_err()?

> @@ -370,12 +347,18 @@ static int msm_ssbi_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> +static struct of_device_id ssbi_match_table[] = {

const?

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

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

* [PATCH v2 0/11] Qualcomm SSBI bus driver
  2013-03-12 18:41 ` [PATCH v2 0/11] Qualcomm SSBI bus driver David Brown
                     ` (9 preceding siblings ...)
  2013-03-12 18:41   ` [PATCH v2 10/11] MAINTAINERS: add ssbi David Brown
@ 2013-03-25 17:40   ` Greg Kroah-Hartman
  10 siblings, 0 replies; 33+ messages in thread
From: Greg Kroah-Hartman @ 2013-03-25 17:40 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Mar 12, 2013 at 11:41:45AM -0700, David Brown wrote:
> This small series adds the Qualcomm SSBI bus driver.  The original
> driver was developed as part of Android.  Kenneth Heitke updated this,
> and sent it out a while back.  This series updates this driver to use
> DeviceTree, and fixes a few things that have changed since it was
> originally sent out.
> 
> In addition, the 11th patch, the RFC, is a small test driver that reads
> the version register off of the pm8058 device typically connected to
> an MSM8660 or MSM8960.  This is primarily useful to anyone wanting to
> work on these pmic drivers.
> 
> Updates to the PMIC drivers themselves to use this driver to come.

The first 10 now applied to my char/misc tree, thanks.

greg k-h

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

end of thread, other threads:[~2013-03-25 17:40 UTC | newest]

Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-03-07  0:29 [PATCH 0/6] Qualcomm SSBI bus driver David Brown
2013-03-07  0:29 ` [PATCH 1/6] platform-drivers: msm: add single-wire serial bus interface (SSBI) driver David Brown
2013-03-07  1:30   ` Greg Kroah-Hartman
2013-03-07  5:20     ` David Brown
2013-03-07  6:01       ` Greg Kroah-Hartman
2013-03-07 10:05         ` Sekhar Nori
2013-03-07 18:45           ` David Brown
2013-03-07 18:50         ` David Brown
2013-03-07 23:29           ` Greg Kroah-Hartman
2013-03-12  6:51     ` David Brown
2013-03-12 13:27       ` Greg Kroah-Hartman
2013-03-07  0:29 ` [PATCH 2/6] SSBI: Convert SSBI to device tree David Brown
2013-03-07  0:29 ` [PATCH 3/6] ssbi: Fix exit mismatch in remove function David Brown
2013-03-07  1:30   ` Greg Kroah-Hartman
2013-03-07  5:21     ` David Brown
2013-03-07  0:29 ` [PATCH 4/6] ssbi: Use regular init level David Brown
2013-03-07  0:29 ` [PATCH 5/6] ARM: msm: enable SSBI driver in defconfig David Brown
2013-03-07  0:29 ` [PATCH 6/6] RFC: SSBI: Simple pm8058 test driver David Brown
2013-03-12 18:41 ` [PATCH v2 0/11] Qualcomm SSBI bus driver David Brown
2013-03-12 18:41   ` [PATCH v2 01/11] platform-drivers: msm: add single-wire serial bus interface (SSBI) driver David Brown
2013-03-12 18:41   ` [PATCH v2 02/11] fix: Use EXPORT_SYMBOL_GPL David Brown
2013-03-12 18:41   ` [PATCH v2 03/11] ssbi: Fix exit mismatch in remove function David Brown
2013-03-12 18:41   ` [PATCH v2 04/11] ssbi: Allow compilation as a module David Brown
2013-03-12 18:41   ` [PATCH v2 05/11] SSBI: Convert SSBI to device tree David Brown
2013-03-12 20:46     ` Stephen Boyd
2013-03-12 18:41   ` [PATCH v2 06/11] ssbi: Comment the use of udelay() David Brown
2013-03-12 18:41   ` [PATCH v2 07/11] ssbi: Use regular init level David Brown
2013-03-12 20:26     ` Stephen Boyd
2013-03-12 18:41   ` [PATCH v2 08/11] ssbi: Remove extraneous logging David Brown
2013-03-12 18:41   ` [PATCH v2 09/11] SSBI: Remove MSM_ prefix from SSBI drivers David Brown
2013-03-12 18:41   ` [PATCH v2 10/11] MAINTAINERS: add ssbi David Brown
2013-03-25 17:40   ` [PATCH v2 0/11] Qualcomm SSBI bus driver Greg Kroah-Hartman
2013-03-12 20:12 ` [PATCH v2 11/11] RFC: SSBI: Simple pm8058 test driver David Brown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).