2011-02-24 22:19:35

by Kenneth Heitke

[permalink] [raw]
Subject: [PATCH v2 2/2] msm: add single-wire serial bus interface (SSBI) driver

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 <[email protected]> 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).

Signed-off-by: Kenneth Heitke <[email protected]>
---
drivers/Kconfig | 2 +
drivers/Makefile | 1 +
drivers/msm/Kconfig | 13 ++
drivers/msm/Makefile | 4 +
drivers/msm/ssbi.c | 376 ++++++++++++++++++++++++++++++++++++++++++++++
include/linux/msm_ssbi.h | 49 ++++++
6 files changed, 445 insertions(+), 0 deletions(-)
create mode 100644 drivers/msm/Kconfig
create mode 100644 drivers/msm/Makefile
create mode 100644 drivers/msm/ssbi.c
create mode 100644 include/linux/msm_ssbi.h

diff --git a/drivers/Kconfig b/drivers/Kconfig
index 9bfb71f..fceeb20 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -117,4 +117,6 @@ source "drivers/staging/Kconfig"
source "drivers/platform/Kconfig"

source "drivers/clk/Kconfig"
+
+source "drivers/msm/Kconfig"
endmenu
diff --git a/drivers/Makefile b/drivers/Makefile
index b423bb1..14cb94b 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -117,3 +117,4 @@ obj-y += platform/
obj-y += ieee802154/
#common clk code
obj-y += clk/
+obj-y += msm/
diff --git a/drivers/msm/Kconfig b/drivers/msm/Kconfig
new file mode 100644
index 0000000..413bb65
--- /dev/null
+++ b/drivers/msm/Kconfig
@@ -0,0 +1,13 @@
+menu "Qualcomm MSM specific device drivers"
+ 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/msm/Makefile b/drivers/msm/Makefile
new file mode 100644
index 0000000..ea8c1e4
--- /dev/null
+++ b/drivers/msm/Makefile
@@ -0,0 +1,4 @@
+#
+# Makefile for the MSM specific device drivers.
+#
+obj-$(CONFIG_MSM_SSBI) += ssbi.o
diff --git a/drivers/msm/ssbi.c b/drivers/msm/ssbi.c
new file mode 100644
index 0000000..1fae443
--- /dev/null
+++ b/drivers/msm/ssbi.c
@@ -0,0 +1,376 @@
+/* Copyright (c) 2009-2011, Code Aurora Forum. All rights reserved.
+ * Copyright (c) 2010, Google Inc.
+ *
+ * Original authors: Code Aurura Forum
+ *
+ * Author: Dima Zavin <[email protected]>
+ * - 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.
+ */
+
+#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>
+
+/* 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)
+
+/* 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 & 0x3fff) << 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 & 0x3fff) << 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;
+
+ BUG_ON(ssbi->dev != dev);
+
+ 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;
+
+ BUG_ON(ssbi->dev != dev);
+
+ 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 __devinit 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("%s: slave already attached??\n", __func__);
+ return -EBUSY;
+ }
+
+ slave_pdev = platform_device_alloc(slave->name, -1);
+ if (!slave_pdev) {
+ pr_err("%s: cannot allocate pdev for slave '%s'", __func__,
+ 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("%s: cannot add slave platform device for '%s'\n",
+ __func__, slave->name);
+ goto err;
+ }
+
+ ssbi->slave = &slave_pdev->dev;
+ return 0;
+
+err:
+ if (slave_pdev)
+ platform_device_put(slave_pdev);
+ return ret;
+}
+
+static int __devinit 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;
+
+ printk(KERN_INFO "%s: %s\n", __func__, pdata->slave.name);
+
+ if (!pdata) {
+ pr_err("%s: missing platform data\n", __func__);
+ return -EINVAL;
+ }
+
+ ssbi = kzalloc(sizeof(struct msm_ssbi), GFP_KERNEL);
+ if (!ssbi) {
+ pr_err("%s: cannot allocate ssbi_data\n", __func__);
+ return -ENOMEM;
+ }
+
+ mem_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!mem_res) {
+ pr_err("%s: missing mem resource\n", __func__);
+ ret = -EINVAL;
+ goto err_get_mem_res;
+ }
+
+ ssbi->base = ioremap(mem_res->start, resource_size(mem_res));
+ if (!ssbi->base) {
+ pr_err("%s: ioremap of 0x%p failed\n", __func__,
+ (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 struct platform_driver msm_ssbi_driver = {
+ .probe = msm_ssbi_probe,
+ .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);
+
+MODULE_LICENSE("GPL v2");
+MODULE_VERSION("1.0");
+MODULE_ALIAS("platform:msm_ssbi");
+MODULE_AUTHOR("Dima Zavin <[email protected]>");
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 <[email protected]>
+ *
+ * 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
--
1.7.3.3

Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum


2011-02-25 02:23:53

by Daniel Walker

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] msm: add single-wire serial bus interface (SSBI) driver


You have some submission problems. one is that the subject should be
"drivers: msm: ..." , this patch also shouldn't go to David.

On Thu, 2011-02-24 at 15:19 -0700, Kenneth Heitke wrote:
> 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 <[email protected]> that can be found at:
> http://android.git.kernel.org/?p=kernel/msm.git;a=commitdiff;h=eb060bac4

I don't really like links in the commit text, cause as soon as Google
deletes this tree the link is worthless.

> 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).
>
> Signed-off-by: Kenneth Heitke <[email protected]>
> ---
> drivers/Kconfig | 2 +
> drivers/Makefile | 1 +
> drivers/msm/Kconfig | 13 ++
> drivers/msm/Makefile | 4 +
> drivers/msm/ssbi.c | 376 ++++++++++++++++++++++++++++++++++++++++++++++
> include/linux/msm_ssbi.h | 49 ++++++
> 6 files changed, 445 insertions(+), 0 deletions(-)
> create mode 100644 drivers/msm/Kconfig
> create mode 100644 drivers/msm/Makefile
> create mode 100644 drivers/msm/ssbi.c
> create mode 100644 include/linux/msm_ssbi.h
>
> diff --git a/drivers/Kconfig b/drivers/Kconfig
> index 9bfb71f..fceeb20 100644
> --- a/drivers/Kconfig
> +++ b/drivers/Kconfig
> @@ -117,4 +117,6 @@ source "drivers/staging/Kconfig"
> source "drivers/platform/Kconfig"
>
> source "drivers/clk/Kconfig"
> +
> +source "drivers/msm/Kconfig"
> endmenu
> diff --git a/drivers/Makefile b/drivers/Makefile
> index b423bb1..14cb94b 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -117,3 +117,4 @@ obj-y += platform/
> obj-y += ieee802154/
> #common clk code
> obj-y += clk/
> +obj-y += msm/
> diff --git a/drivers/msm/Kconfig b/drivers/msm/Kconfig
> new file mode 100644
> index 0000000..413bb65
> --- /dev/null
> +++ b/drivers/msm/Kconfig
> @@ -0,0 +1,13 @@
> +menu "Qualcomm MSM specific device drivers"
> + 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/msm/Makefile b/drivers/msm/Makefile
> new file mode 100644
> index 0000000..ea8c1e4
> --- /dev/null
> +++ b/drivers/msm/Makefile
> @@ -0,0 +1,4 @@
> +#
> +# Makefile for the MSM specific device drivers.
> +#
> +obj-$(CONFIG_MSM_SSBI) += ssbi.o
> diff --git a/drivers/msm/ssbi.c b/drivers/msm/ssbi.c
> new file mode 100644
> index 0000000..1fae443
> --- /dev/null
> +++ b/drivers/msm/ssbi.c
> @@ -0,0 +1,376 @@
> +/* Copyright (c) 2009-2011, Code Aurora Forum. All rights reserved.
> + * Copyright (c) 2010, Google Inc.
> + *
> + * Original authors: Code Aurura Forum

Spelling problem.

> + * Author: Dima Zavin <[email protected]>
> + * - 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.
> + */
> +
> +#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>
> +
> +/* 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)
> +
> +/* 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 & 0x3fff) << 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 & 0x3fff) << 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;
> +
> + BUG_ON(ssbi->dev != dev);

Are you sure you don't just want to bail out here, cause this hangs the
system when it triggers?

> + 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;
> +
> + BUG_ON(ssbi->dev != dev);
> +
> + 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 __devinit 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("%s: slave already attached??\n", __func__);
> + return -EBUSY;

You can use a pr_fmt line to add the __func__ part, look in mach-msm for
examples.

> + }
> +
> + slave_pdev = platform_device_alloc(slave->name, -1);
> + if (!slave_pdev) {
> + pr_err("%s: cannot allocate pdev for slave '%s'", __func__,
> + 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("%s: cannot add slave platform device for '%s'\n",
> + __func__, slave->name);
> + goto err;
> + }
> +
> + ssbi->slave = &slave_pdev->dev;
> + return 0;
> +
> +err:
> + if (slave_pdev)
> + platform_device_put(slave_pdev);
> + return ret;
> +}
> +
> +static int __devinit 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;
> +
> + printk(KERN_INFO "%s: %s\n", __func__, pdata->slave.name);

pr_info() ?

Daniel

2011-02-25 02:31:40

by David Brown

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] msm: add single-wire serial bus interface (SSBI) driver

On Thu, Feb 24 2011, Daniel Walker wrote:

> You have some submission problems. one is that the subject should be
> "drivers: msm: ..." , this patch also shouldn't go to David.

Subject needs to be fixed. Patch to add drivers/msm to the maintainers
file has been Acked, so this should be sent to the MSM maintainers.

David

2011-02-25 02:35:55

by Daniel Walker

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] msm: add single-wire serial bus interface (SSBI) driver

On Thu, 2011-02-24 at 18:31 -0800, David Brown wrote:
> On Thu, Feb 24 2011, Daniel Walker wrote:
>
> > You have some submission problems. one is that the subject should be
> > "drivers: msm: ..." , this patch also shouldn't go to David.
>
> Subject needs to be fixed. Patch to add drivers/msm to the maintainers
> file has been Acked, so this should be sent to the MSM maintainers.

It was only acked by Nico, he's just the one who suggested it. I think
Greg needs to be involved at some level ..

Daniel

2011-02-25 02:39:55

by Dima Zavin

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] msm: add single-wire serial bus interface (SSBI) driver

On that note, should we have a "drivers/arm/msm" instead of just
"drivers/msm"? Other non-msm arm sub-arches probably have the same
problem and polluting drivers/ top-level for every arm-subarch maybe
undesirable.

--Dima


On Thu, Feb 24, 2011 at 6:35 PM, Daniel Walker <[email protected]> wrote:
> On Thu, 2011-02-24 at 18:31 -0800, David Brown wrote:
>> On Thu, Feb 24 2011, Daniel Walker wrote:
>>
>> > You have some submission problems. one is that the subject should be
>> > "drivers: msm: ..." , this patch also shouldn't go to David.
>>
>> Subject needs to be fixed. ?Patch to add drivers/msm to the maintainers
>> file has been Acked, so this should be sent to the MSM maintainers.
>
> It was only acked by Nico, he's just the one who suggested it. I think
> Greg needs to be involved at some level ..
>
> Daniel
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at ?http://www.tux.org/lkml/
>

2011-02-25 03:20:48

by David Brown

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] msm: add single-wire serial bus interface (SSBI) driver

On Thu, Feb 24 2011, Dima Zavin wrote:

> On that note, should we have a "drivers/arm/msm" instead of just
> "drivers/msm"? Other non-msm arm sub-arches probably have the same
> problem and polluting drivers/ top-level for every arm-subarch maybe
> undesirable.

I'm moving linux-kernel to the 'to' field, and adding Greg KH to CC.

This is about a new driver (ssbi) for the MSM chips, and where the
driver for this should go. The driver is specific to MSM chips (as far
as I know, the bus is only supported there). There are likely to be
future msm-specific drivers, so the suggestion was to put this under
drivers/msm, or possibly drivers/arm/msm.

Anyone other suggestions, or preferences for the location?

Thanks,
David Brown

2011-02-25 03:30:56

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] msm: add single-wire serial bus interface (SSBI) driver

On Thu, Feb 24, 2011 at 07:20:45PM -0800, David Brown wrote:
> On Thu, Feb 24 2011, Dima Zavin wrote:
>
> > On that note, should we have a "drivers/arm/msm" instead of just
> > "drivers/msm"? Other non-msm arm sub-arches probably have the same
> > problem and polluting drivers/ top-level for every arm-subarch maybe
> > undesirable.
>
> I'm moving linux-kernel to the 'to' field, and adding Greg KH to CC.
>
> This is about a new driver (ssbi) for the MSM chips, and where the
> driver for this should go. The driver is specific to MSM chips (as far
> as I know, the bus is only supported there). There are likely to be
> future msm-specific drivers, so the suggestion was to put this under
> drivers/msm, or possibly drivers/arm/msm.
>
> Anyone other suggestions, or preferences for the location?

drivers/ssbi? What's keeping this from later moving off of the msm
chips to run on others? USB started out only on one processor, as did a
lot of other bus-specific drivers, before the hardware became present on
other architectures. So no need to bury it under a msm specific
location.

Hope this helps,

greg k-h

2011-02-25 03:54:24

by David Brown

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] msm: add single-wire serial bus interface (SSBI) driver

On Thu, Feb 24 2011, Greg KH wrote:

> drivers/ssbi? What's keeping this from later moving off of the msm
> chips to run on others? USB started out only on one processor, as did a
> lot of other bus-specific drivers, before the hardware became present on
> other architectures. So no need to bury it under a msm specific
> location.

Not sure if anything is stopping it from moving off of MSM chips, but it
doesn't seem very likely. There are too many similar interfaces that
are standard that others would be likely to use. For the most part, it
is the bus to communicate between the MSM and it's specialized
peripherals.

We can still put it there, but it will likely be the only driver ever in
the directory.

David

--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

2011-02-25 03:58:08

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] msm: add single-wire serial bus interface (SSBI) driver

On Thu, Feb 24, 2011 at 07:54:21PM -0800, David Brown wrote:
> On Thu, Feb 24 2011, Greg KH wrote:
>
> > drivers/ssbi? What's keeping this from later moving off of the msm
> > chips to run on others? USB started out only on one processor, as did a
> > lot of other bus-specific drivers, before the hardware became present on
> > other architectures. So no need to bury it under a msm specific
> > location.
>
> Not sure if anything is stopping it from moving off of MSM chips, but it
> doesn't seem very likely. There are too many similar interfaces that
> are standard that others would be likely to use. For the most part, it
> is the bus to communicate between the MSM and it's specialized
> peripherals.
>
> We can still put it there, but it will likely be the only driver ever in
> the directory.

Ah, so it's just one driver? Then how about drivers/platform/msm/ ?

thanks,

greg k-h

2011-02-25 04:06:16

by David Brown

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] msm: add single-wire serial bus interface (SSBI) driver

On Thu, Feb 24 2011, Greg KH wrote:

> On Thu, Feb 24, 2011 at 07:54:21PM -0800, David Brown wrote:
>> On Thu, Feb 24 2011, Greg KH wrote:
>>
>> > drivers/ssbi? What's keeping this from later moving off of the msm
>> > chips to run on others? USB started out only on one processor, as did a
>> > lot of other bus-specific drivers, before the hardware became present on
>> > other architectures. So no need to bury it under a msm specific
>> > location.
>>
>> Not sure if anything is stopping it from moving off of MSM chips, but it
>> doesn't seem very likely. There are too many similar interfaces that
>> are standard that others would be likely to use. For the most part, it
>> is the bus to communicate between the MSM and it's specialized
>> peripherals.
>>
>> We can still put it there, but it will likely be the only driver ever in
>> the directory.
>
> Ah, so it's just one driver? Then how about drivers/platform/msm/ ?

I like it. It keeps drivers from getting cluttered. Plus x86 is
getting lonely in there.

Ken, in addition to addressing the comments made so far and any others
that come in, let's put this under drivers/platform/msm.

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.

2011-02-25 19:00:30

by David Brown

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] msm: add single-wire serial bus interface (SSBI) driver

On Thu, Feb 24 2011, Daniel Walker wrote:

>> Based on a patch by Dima Zavin <[email protected]> that can be found at:
>> http://android.git.kernel.org/?p=kernel/msm.git;a=commitdiff;h=eb060bac4
>
> I don't really like links in the commit text, cause as soon as Google
> deletes this tree the link is worthless.

I'm not sure how to better convey the information. The tree is at least
on kernel.org, and there's good useful information there that works
now. Links in commit text seem fairly common.

I think it is helpful to demonstrate that Ken is trying to be diligent
about indicating where the code comes from.

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.

2011-02-25 20:22:05

by Brian Swetland

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] msm: add single-wire serial bus interface (SSBI) driver

On Fri, Feb 25, 2011 at 11:00 AM, David Brown <[email protected]> wrote:
> On Thu, Feb 24 2011, Daniel Walker wrote:
>
>>> Based on a patch by Dima Zavin <[email protected]> that can be found at:
>>> http://android.git.kernel.org/?p=kernel/msm.git;a=commitdiff;h=eb060bac4
>>
>> I don't really like links in the commit text, cause as soon as Google
>> deletes this tree the link is worthless.
>
> I'm not sure how to better convey the information.  The tree is at least
> on kernel.org, and there's good useful information there that works
> now.  Links in commit text seem fairly common.
>
> I think it is helpful to demonstrate that Ken is trying to be diligent
> about indicating where the code comes from.

We do also keep older branches around, since this is how we publish
kernel sources when we ship devices. So while I can't be sure these
links will work *forever*, barring mishap they should exist for quite
a while.

Brian