All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/8] Modernize pm8921 with irqdomains + regmap
@ 2013-12-10 23:35 ` Stephen Boyd
  0 siblings, 0 replies; 77+ messages in thread
From: Stephen Boyd @ 2013-12-10 23:35 UTC (permalink / raw)
  To: Samuel Ortiz, Lee Jones
  Cc: linux-kernel, linux-arm-msm, linux-arm-kernel, Mark Brown

These patches lay the groundwork for converting the pm8921 code to 
devicetree as well as simplify the API by migrating the core code
to use the regmap API instead of the custom pm8xxx read/write wrapper.

Please note, I put the generic regmap change in this set of patches.
Hopefully Mark and Samuel/Lee can sort out how these patches should
merge.

Srinivas Ramana (1):
  regmap: Add support for using regmap over ssbi

Stephen Boyd (7):
  mfd: ssbi: Remove platform data structs and hide ssbi type enum
  mfd: ssbi: Constify buffer in ssbi_write
  mfd: ssbi: Mark match table const
  mfd: Move pm8xxx-irq.c contents into only driver that uses it
  mfd: pm8921: Update for genirq changes
  mfd: pm8921: Migrate to irqdomains
  mfd: pm8921: Use ssbi regmap

 drivers/base/regmap/Kconfig       |   5 +-
 drivers/base/regmap/Makefile      |   1 +
 drivers/base/regmap/regmap-ssbi.c | 103 ++++++++++
 drivers/mfd/Kconfig               |  12 +-
 drivers/mfd/pm8921-core.c         | 416 +++++++++++++++++++++++++++++++++-----
 drivers/mfd/pm8xxx-irq.c          | 371 ---------------------------------
 drivers/mfd/ssbi.c                |  16 +-
 include/linux/mfd/pm8xxx/irq.h    |  59 ------
 include/linux/mfd/pm8xxx/pm8921.h |  30 ---
 include/linux/regmap.h            |   4 +
 include/linux/ssbi.h              |  19 +-
 11 files changed, 486 insertions(+), 550 deletions(-)
 create mode 100644 drivers/base/regmap/regmap-ssbi.c
 delete mode 100644 drivers/mfd/pm8xxx-irq.c
 delete mode 100644 include/linux/mfd/pm8xxx/irq.h
 delete mode 100644 include/linux/mfd/pm8xxx/pm8921.h

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

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

* [PATCH 0/8] Modernize pm8921 with irqdomains + regmap
@ 2013-12-10 23:35 ` Stephen Boyd
  0 siblings, 0 replies; 77+ messages in thread
From: Stephen Boyd @ 2013-12-10 23:35 UTC (permalink / raw)
  To: linux-arm-kernel

These patches lay the groundwork for converting the pm8921 code to 
devicetree as well as simplify the API by migrating the core code
to use the regmap API instead of the custom pm8xxx read/write wrapper.

Please note, I put the generic regmap change in this set of patches.
Hopefully Mark and Samuel/Lee can sort out how these patches should
merge.

Srinivas Ramana (1):
  regmap: Add support for using regmap over ssbi

Stephen Boyd (7):
  mfd: ssbi: Remove platform data structs and hide ssbi type enum
  mfd: ssbi: Constify buffer in ssbi_write
  mfd: ssbi: Mark match table const
  mfd: Move pm8xxx-irq.c contents into only driver that uses it
  mfd: pm8921: Update for genirq changes
  mfd: pm8921: Migrate to irqdomains
  mfd: pm8921: Use ssbi regmap

 drivers/base/regmap/Kconfig       |   5 +-
 drivers/base/regmap/Makefile      |   1 +
 drivers/base/regmap/regmap-ssbi.c | 103 ++++++++++
 drivers/mfd/Kconfig               |  12 +-
 drivers/mfd/pm8921-core.c         | 416 +++++++++++++++++++++++++++++++++-----
 drivers/mfd/pm8xxx-irq.c          | 371 ---------------------------------
 drivers/mfd/ssbi.c                |  16 +-
 include/linux/mfd/pm8xxx/irq.h    |  59 ------
 include/linux/mfd/pm8xxx/pm8921.h |  30 ---
 include/linux/regmap.h            |   4 +
 include/linux/ssbi.h              |  19 +-
 11 files changed, 486 insertions(+), 550 deletions(-)
 create mode 100644 drivers/base/regmap/regmap-ssbi.c
 delete mode 100644 drivers/mfd/pm8xxx-irq.c
 delete mode 100644 include/linux/mfd/pm8xxx/irq.h
 delete mode 100644 include/linux/mfd/pm8xxx/pm8921.h

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

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

* [PATCH 1/8] mfd: ssbi: Remove platform data structs and hide ssbi type enum
  2013-12-10 23:35 ` Stephen Boyd
@ 2013-12-10 23:35   ` Stephen Boyd
  -1 siblings, 0 replies; 77+ messages in thread
From: Stephen Boyd @ 2013-12-10 23:35 UTC (permalink / raw)
  To: Samuel Ortiz, Lee Jones; +Cc: linux-kernel, linux-arm-msm, linux-arm-kernel

The ssbi driver assumes that the device is DT based. Remove the
platform data structs that will never be used and hide the enum
in the only C file that uses it.

Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 drivers/mfd/ssbi.c   |  6 ++++++
 include/linux/ssbi.h | 17 +----------------
 2 files changed, 7 insertions(+), 16 deletions(-)

diff --git a/drivers/mfd/ssbi.c b/drivers/mfd/ssbi.c
index 102a2284..435c6f7 100644
--- a/drivers/mfd/ssbi.c
+++ b/drivers/mfd/ssbi.c
@@ -65,6 +65,12 @@
 
 #define SSBI_TIMEOUT_US			100
 
+enum ssbi_controller_type {
+	MSM_SBI_CTRL_SSBI = 0,
+	MSM_SBI_CTRL_SSBI2,
+	MSM_SBI_CTRL_PMIC_ARBITER,
+};
+
 struct ssbi {
 	struct device		*slave;
 	void __iomem		*base;
diff --git a/include/linux/ssbi.h b/include/linux/ssbi.h
index 44ef5da..a92561a 100644
--- a/include/linux/ssbi.h
+++ b/include/linux/ssbi.h
@@ -17,22 +17,7 @@
 
 #include <linux/types.h>
 
-struct ssbi_slave_info {
-	const char	*name;
-	void		*platform_data;
-};
-
-enum ssbi_controller_type {
-	MSM_SBI_CTRL_SSBI = 0,
-	MSM_SBI_CTRL_SSBI2,
-	MSM_SBI_CTRL_PMIC_ARBITER,
-};
-
-struct ssbi_platform_data {
-	struct ssbi_slave_info	slave;
-	enum ssbi_controller_type controller_type;
-};
-
 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] 77+ messages in thread

* [PATCH 1/8] mfd: ssbi: Remove platform data structs and hide ssbi type enum
@ 2013-12-10 23:35   ` Stephen Boyd
  0 siblings, 0 replies; 77+ messages in thread
From: Stephen Boyd @ 2013-12-10 23:35 UTC (permalink / raw)
  To: linux-arm-kernel

The ssbi driver assumes that the device is DT based. Remove the
platform data structs that will never be used and hide the enum
in the only C file that uses it.

Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 drivers/mfd/ssbi.c   |  6 ++++++
 include/linux/ssbi.h | 17 +----------------
 2 files changed, 7 insertions(+), 16 deletions(-)

diff --git a/drivers/mfd/ssbi.c b/drivers/mfd/ssbi.c
index 102a2284..435c6f7 100644
--- a/drivers/mfd/ssbi.c
+++ b/drivers/mfd/ssbi.c
@@ -65,6 +65,12 @@
 
 #define SSBI_TIMEOUT_US			100
 
+enum ssbi_controller_type {
+	MSM_SBI_CTRL_SSBI = 0,
+	MSM_SBI_CTRL_SSBI2,
+	MSM_SBI_CTRL_PMIC_ARBITER,
+};
+
 struct ssbi {
 	struct device		*slave;
 	void __iomem		*base;
diff --git a/include/linux/ssbi.h b/include/linux/ssbi.h
index 44ef5da..a92561a 100644
--- a/include/linux/ssbi.h
+++ b/include/linux/ssbi.h
@@ -17,22 +17,7 @@
 
 #include <linux/types.h>
 
-struct ssbi_slave_info {
-	const char	*name;
-	void		*platform_data;
-};
-
-enum ssbi_controller_type {
-	MSM_SBI_CTRL_SSBI = 0,
-	MSM_SBI_CTRL_SSBI2,
-	MSM_SBI_CTRL_PMIC_ARBITER,
-};
-
-struct ssbi_platform_data {
-	struct ssbi_slave_info	slave;
-	enum ssbi_controller_type controller_type;
-};
-
 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] 77+ messages in thread

* [PATCH 2/8] mfd: ssbi: Constify buffer in ssbi_write
  2013-12-10 23:35 ` Stephen Boyd
@ 2013-12-10 23:35   ` Stephen Boyd
  -1 siblings, 0 replies; 77+ messages in thread
From: Stephen Boyd @ 2013-12-10 23:35 UTC (permalink / raw)
  To: Samuel Ortiz, Lee Jones; +Cc: linux-kernel, linux-arm-msm, linux-arm-kernel

In preparation for passing a const pointer directly to
ssbi_write() from the regmap APIs.

Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 drivers/mfd/ssbi.c   | 8 ++++----
 include/linux/ssbi.h | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/mfd/ssbi.c b/drivers/mfd/ssbi.c
index 435c6f7..dd1d28f 100644
--- a/drivers/mfd/ssbi.c
+++ b/drivers/mfd/ssbi.c
@@ -77,7 +77,7 @@ struct ssbi {
 	spinlock_t		lock;
 	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);
+	int (*write)(struct ssbi *, u16 addr, const u8 *buf, int len);
 };
 
 #define to_ssbi(dev)	platform_get_drvdata(to_platform_device(dev))
@@ -146,7 +146,7 @@ err:
 }
 
 static int
-ssbi_write_bytes(struct ssbi *ssbi, u16 addr, u8 *buf, int len)
+ssbi_write_bytes(struct ssbi *ssbi, u16 addr, const u8 *buf, int len)
 {
 	int ret = 0;
 
@@ -223,7 +223,7 @@ err:
 }
 
 static int
-ssbi_pa_write_bytes(struct ssbi *ssbi, u16 addr, u8 *buf, int len)
+ssbi_pa_write_bytes(struct ssbi *ssbi, u16 addr, const u8 *buf, int len)
 {
 	u32 cmd;
 	int ret = 0;
@@ -255,7 +255,7 @@ int ssbi_read(struct device *dev, u16 addr, u8 *buf, int len)
 }
 EXPORT_SYMBOL_GPL(ssbi_read);
 
-int ssbi_write(struct device *dev, u16 addr, u8 *buf, int len)
+int ssbi_write(struct device *dev, u16 addr, const u8 *buf, int len)
 {
 	struct ssbi *ssbi = to_ssbi(dev);
 	unsigned long flags;
diff --git a/include/linux/ssbi.h b/include/linux/ssbi.h
index a92561a..bcbb642 100644
--- a/include/linux/ssbi.h
+++ b/include/linux/ssbi.h
@@ -17,7 +17,7 @@
 
 #include <linux/types.h>
 
-int ssbi_write(struct device *dev, u16 addr, u8 *buf, int len);
+int ssbi_write(struct device *dev, u16 addr, const 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] 77+ messages in thread

* [PATCH 2/8] mfd: ssbi: Constify buffer in ssbi_write
@ 2013-12-10 23:35   ` Stephen Boyd
  0 siblings, 0 replies; 77+ messages in thread
From: Stephen Boyd @ 2013-12-10 23:35 UTC (permalink / raw)
  To: linux-arm-kernel

In preparation for passing a const pointer directly to
ssbi_write() from the regmap APIs.

Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 drivers/mfd/ssbi.c   | 8 ++++----
 include/linux/ssbi.h | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/mfd/ssbi.c b/drivers/mfd/ssbi.c
index 435c6f7..dd1d28f 100644
--- a/drivers/mfd/ssbi.c
+++ b/drivers/mfd/ssbi.c
@@ -77,7 +77,7 @@ struct ssbi {
 	spinlock_t		lock;
 	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);
+	int (*write)(struct ssbi *, u16 addr, const u8 *buf, int len);
 };
 
 #define to_ssbi(dev)	platform_get_drvdata(to_platform_device(dev))
@@ -146,7 +146,7 @@ err:
 }
 
 static int
-ssbi_write_bytes(struct ssbi *ssbi, u16 addr, u8 *buf, int len)
+ssbi_write_bytes(struct ssbi *ssbi, u16 addr, const u8 *buf, int len)
 {
 	int ret = 0;
 
@@ -223,7 +223,7 @@ err:
 }
 
 static int
-ssbi_pa_write_bytes(struct ssbi *ssbi, u16 addr, u8 *buf, int len)
+ssbi_pa_write_bytes(struct ssbi *ssbi, u16 addr, const u8 *buf, int len)
 {
 	u32 cmd;
 	int ret = 0;
@@ -255,7 +255,7 @@ int ssbi_read(struct device *dev, u16 addr, u8 *buf, int len)
 }
 EXPORT_SYMBOL_GPL(ssbi_read);
 
-int ssbi_write(struct device *dev, u16 addr, u8 *buf, int len)
+int ssbi_write(struct device *dev, u16 addr, const u8 *buf, int len)
 {
 	struct ssbi *ssbi = to_ssbi(dev);
 	unsigned long flags;
diff --git a/include/linux/ssbi.h b/include/linux/ssbi.h
index a92561a..bcbb642 100644
--- a/include/linux/ssbi.h
+++ b/include/linux/ssbi.h
@@ -17,7 +17,7 @@
 
 #include <linux/types.h>
 
-int ssbi_write(struct device *dev, u16 addr, u8 *buf, int len);
+int ssbi_write(struct device *dev, u16 addr, const 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] 77+ messages in thread

* [PATCH 3/8] regmap: Add support for using regmap over ssbi
  2013-12-10 23:35 ` Stephen Boyd
@ 2013-12-10 23:35   ` Stephen Boyd
  -1 siblings, 0 replies; 77+ messages in thread
From: Stephen Boyd @ 2013-12-10 23:35 UTC (permalink / raw)
  To: Samuel Ortiz, Lee Jones
  Cc: Srinivas Ramana, linux-kernel, linux-arm-msm, linux-arm-kernel,
	Mark Brown

From: Srinivas Ramana <sramana@codeaurora.org>

This patch will add support for using regmap API over SSBI.
Many of the Qualcomm PMIC chips(ex: PM8038, 8921) uses SSBI interface.
This patch adds only regmap-ssbi suport. Individual PMIC drivers will
need to register with their config to use the regmap API.
regmap-ssbi uses the SSBI driver interface.

Cc: Mark Brown <broonie@kernel.org>
Signed-off-by: Srinivas Ramana <sramana@codeaurora.org>
Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 drivers/base/regmap/Kconfig       |   5 +-
 drivers/base/regmap/Makefile      |   1 +
 drivers/base/regmap/regmap-ssbi.c | 103 ++++++++++++++++++++++++++++++++++++++
 include/linux/regmap.h            |   4 ++
 4 files changed, 112 insertions(+), 1 deletion(-)
 create mode 100644 drivers/base/regmap/regmap-ssbi.c

diff --git a/drivers/base/regmap/Kconfig b/drivers/base/regmap/Kconfig
index 4251570..fd0e264 100644
--- a/drivers/base/regmap/Kconfig
+++ b/drivers/base/regmap/Kconfig
@@ -3,7 +3,7 @@
 # subsystems should select the appropriate symbols.
 
 config REGMAP
-	default y if (REGMAP_I2C || REGMAP_SPI || REGMAP_SPMI || REGMAP_MMIO || REGMAP_IRQ)
+	default y if (REGMAP_I2C || REGMAP_SPI || REGMAP_SPMI || REGMAP_MMIO || REGMAP_IRQ || REGMAP_SSBI)
 	select LZO_COMPRESS
 	select LZO_DECOMPRESS
 	select IRQ_DOMAIN if REGMAP_IRQ
@@ -23,3 +23,6 @@ config REGMAP_MMIO
 
 config REGMAP_IRQ
 	bool
+
+config REGMAP_SSBI
+	tristate
diff --git a/drivers/base/regmap/Makefile b/drivers/base/regmap/Makefile
index a7c670b..3f53763 100644
--- a/drivers/base/regmap/Makefile
+++ b/drivers/base/regmap/Makefile
@@ -6,3 +6,4 @@ obj-$(CONFIG_REGMAP_SPI) += regmap-spi.o
 obj-$(CONFIG_REGMAP_SPMI) += regmap-spmi.o
 obj-$(CONFIG_REGMAP_MMIO) += regmap-mmio.o
 obj-$(CONFIG_REGMAP_IRQ) += regmap-irq.o
+obj-$(CONFIG_REGMAP_SSBI) += regmap-ssbi.o
diff --git a/drivers/base/regmap/regmap-ssbi.c b/drivers/base/regmap/regmap-ssbi.c
new file mode 100644
index 0000000..71bce4d
--- /dev/null
+++ b/drivers/base/regmap/regmap-ssbi.c
@@ -0,0 +1,103 @@
+/*
+ * Register map access API - SSBI support
+ *
+ * Copyright (c) 2013, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/device.h>
+#include <linux/regmap.h>
+#include <linux/module.h>
+#include <linux/ssbi.h>
+
+static int regmap_ssbi_read(void *context,
+			    const void *reg, size_t reg_size,
+			    void *val, size_t val_size)
+{
+	int ret;
+
+	BUG_ON(reg_size != 2);
+
+	while (val_size) {
+		ret = ssbi_read(context, *(u16 *)reg, val, 1);
+		if (ret)
+			return ret;
+		reg += sizeof(u16);
+		val += sizeof(u8);
+		val_size -= sizeof(u8);
+	}
+
+	return 0;
+}
+
+static int regmap_ssbi_gather_write(void *context,
+				   const void *reg, size_t reg_size,
+				   const void *val, size_t val_size)
+{
+	int ret;
+
+	BUG_ON(reg_size != 2);
+
+	while (val_size) {
+		ret = ssbi_write(context, *(u16 *)reg, val, 1);
+		if (ret)
+			return ret;
+		reg += sizeof(u16);
+		val += sizeof(u8);
+		val_size -= sizeof(u8);
+	}
+
+	return 0;
+}
+
+static int regmap_ssbi_write(void *context, const void *data, size_t count)
+{
+	BUG_ON(count < 2);
+	return regmap_ssbi_gather_write(context, data, 2, data + 2, count - 2);
+}
+
+static const struct regmap_bus regmap_ssbi = {
+	.read				= regmap_ssbi_read,
+	.write				= regmap_ssbi_write,
+	.gather_write			= regmap_ssbi_gather_write,
+	.reg_format_endian_default	= REGMAP_ENDIAN_NATIVE,
+	.val_format_endian_default	= REGMAP_ENDIAN_NATIVE,
+};
+
+/**
+ * regmap_init_ssbi(): Initialise register map
+ *
+ * @dev: Device that will be interacted with
+ * @config: Configuration for register map
+ *
+ * The return value will be an ERR_PTR() on error or a valid pointer to
+ * a struct regmap.
+ */
+struct regmap *regmap_init_ssbi(struct device *dev,
+			       const struct regmap_config *config)
+{
+	return regmap_init(dev, &regmap_ssbi, dev->parent, config);
+}
+EXPORT_SYMBOL_GPL(regmap_init_ssbi);
+
+/**
+ * devm_regmap_init_ssbi(): Initialise managed register map
+ *
+ * @dev: Device that will be interacted with
+ * @config: Configuration for register map
+ *
+ * The return value will be an ERR_PTR() on error or a valid pointer
+ * to a struct regmap.  The regmap will be automatically freed by the
+ * device management code.
+ */
+struct regmap *devm_regmap_init_ssbi(struct device *dev,
+				    const struct regmap_config *config)
+{
+	return devm_regmap_init(dev, &regmap_ssbi, dev->parent, config);
+}
+EXPORT_SYMBOL_GPL(devm_regmap_init_ssbi);
+
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index e559078..528ee99 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -326,6 +326,8 @@ struct regmap *regmap_init_spmi(struct spmi_device *dev,
 struct regmap *regmap_init_mmio_clk(struct device *dev, const char *clk_id,
 				    void __iomem *regs,
 				    const struct regmap_config *config);
+struct regmap *regmap_init_ssbi(struct device *dev,
+				const struct regmap_config *config);
 
 struct regmap *devm_regmap_init(struct device *dev,
 				const struct regmap_bus *bus,
@@ -340,6 +342,8 @@ struct regmap *devm_regmap_init_spmi(struct spmi_device *dev,
 struct regmap *devm_regmap_init_mmio_clk(struct device *dev, const char *clk_id,
 					 void __iomem *regs,
 					 const struct regmap_config *config);
+struct regmap *devm_regmap_init_ssbi(struct device *dev,
+					const struct regmap_config *config);
 
 /**
  * regmap_init_mmio(): Initialise register map
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

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

* [PATCH 3/8] regmap: Add support for using regmap over ssbi
@ 2013-12-10 23:35   ` Stephen Boyd
  0 siblings, 0 replies; 77+ messages in thread
From: Stephen Boyd @ 2013-12-10 23:35 UTC (permalink / raw)
  To: linux-arm-kernel

From: Srinivas Ramana <sramana@codeaurora.org>

This patch will add support for using regmap API over SSBI.
Many of the Qualcomm PMIC chips(ex: PM8038, 8921) uses SSBI interface.
This patch adds only regmap-ssbi suport. Individual PMIC drivers will
need to register with their config to use the regmap API.
regmap-ssbi uses the SSBI driver interface.

Cc: Mark Brown <broonie@kernel.org>
Signed-off-by: Srinivas Ramana <sramana@codeaurora.org>
Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 drivers/base/regmap/Kconfig       |   5 +-
 drivers/base/regmap/Makefile      |   1 +
 drivers/base/regmap/regmap-ssbi.c | 103 ++++++++++++++++++++++++++++++++++++++
 include/linux/regmap.h            |   4 ++
 4 files changed, 112 insertions(+), 1 deletion(-)
 create mode 100644 drivers/base/regmap/regmap-ssbi.c

diff --git a/drivers/base/regmap/Kconfig b/drivers/base/regmap/Kconfig
index 4251570..fd0e264 100644
--- a/drivers/base/regmap/Kconfig
+++ b/drivers/base/regmap/Kconfig
@@ -3,7 +3,7 @@
 # subsystems should select the appropriate symbols.
 
 config REGMAP
-	default y if (REGMAP_I2C || REGMAP_SPI || REGMAP_SPMI || REGMAP_MMIO || REGMAP_IRQ)
+	default y if (REGMAP_I2C || REGMAP_SPI || REGMAP_SPMI || REGMAP_MMIO || REGMAP_IRQ || REGMAP_SSBI)
 	select LZO_COMPRESS
 	select LZO_DECOMPRESS
 	select IRQ_DOMAIN if REGMAP_IRQ
@@ -23,3 +23,6 @@ config REGMAP_MMIO
 
 config REGMAP_IRQ
 	bool
+
+config REGMAP_SSBI
+	tristate
diff --git a/drivers/base/regmap/Makefile b/drivers/base/regmap/Makefile
index a7c670b..3f53763 100644
--- a/drivers/base/regmap/Makefile
+++ b/drivers/base/regmap/Makefile
@@ -6,3 +6,4 @@ obj-$(CONFIG_REGMAP_SPI) += regmap-spi.o
 obj-$(CONFIG_REGMAP_SPMI) += regmap-spmi.o
 obj-$(CONFIG_REGMAP_MMIO) += regmap-mmio.o
 obj-$(CONFIG_REGMAP_IRQ) += regmap-irq.o
+obj-$(CONFIG_REGMAP_SSBI) += regmap-ssbi.o
diff --git a/drivers/base/regmap/regmap-ssbi.c b/drivers/base/regmap/regmap-ssbi.c
new file mode 100644
index 0000000..71bce4d
--- /dev/null
+++ b/drivers/base/regmap/regmap-ssbi.c
@@ -0,0 +1,103 @@
+/*
+ * Register map access API - SSBI support
+ *
+ * Copyright (c) 2013, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/device.h>
+#include <linux/regmap.h>
+#include <linux/module.h>
+#include <linux/ssbi.h>
+
+static int regmap_ssbi_read(void *context,
+			    const void *reg, size_t reg_size,
+			    void *val, size_t val_size)
+{
+	int ret;
+
+	BUG_ON(reg_size != 2);
+
+	while (val_size) {
+		ret = ssbi_read(context, *(u16 *)reg, val, 1);
+		if (ret)
+			return ret;
+		reg += sizeof(u16);
+		val += sizeof(u8);
+		val_size -= sizeof(u8);
+	}
+
+	return 0;
+}
+
+static int regmap_ssbi_gather_write(void *context,
+				   const void *reg, size_t reg_size,
+				   const void *val, size_t val_size)
+{
+	int ret;
+
+	BUG_ON(reg_size != 2);
+
+	while (val_size) {
+		ret = ssbi_write(context, *(u16 *)reg, val, 1);
+		if (ret)
+			return ret;
+		reg += sizeof(u16);
+		val += sizeof(u8);
+		val_size -= sizeof(u8);
+	}
+
+	return 0;
+}
+
+static int regmap_ssbi_write(void *context, const void *data, size_t count)
+{
+	BUG_ON(count < 2);
+	return regmap_ssbi_gather_write(context, data, 2, data + 2, count - 2);
+}
+
+static const struct regmap_bus regmap_ssbi = {
+	.read				= regmap_ssbi_read,
+	.write				= regmap_ssbi_write,
+	.gather_write			= regmap_ssbi_gather_write,
+	.reg_format_endian_default	= REGMAP_ENDIAN_NATIVE,
+	.val_format_endian_default	= REGMAP_ENDIAN_NATIVE,
+};
+
+/**
+ * regmap_init_ssbi(): Initialise register map
+ *
+ * @dev: Device that will be interacted with
+ * @config: Configuration for register map
+ *
+ * The return value will be an ERR_PTR() on error or a valid pointer to
+ * a struct regmap.
+ */
+struct regmap *regmap_init_ssbi(struct device *dev,
+			       const struct regmap_config *config)
+{
+	return regmap_init(dev, &regmap_ssbi, dev->parent, config);
+}
+EXPORT_SYMBOL_GPL(regmap_init_ssbi);
+
+/**
+ * devm_regmap_init_ssbi(): Initialise managed register map
+ *
+ * @dev: Device that will be interacted with
+ * @config: Configuration for register map
+ *
+ * The return value will be an ERR_PTR() on error or a valid pointer
+ * to a struct regmap.  The regmap will be automatically freed by the
+ * device management code.
+ */
+struct regmap *devm_regmap_init_ssbi(struct device *dev,
+				    const struct regmap_config *config)
+{
+	return devm_regmap_init(dev, &regmap_ssbi, dev->parent, config);
+}
+EXPORT_SYMBOL_GPL(devm_regmap_init_ssbi);
+
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index e559078..528ee99 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -326,6 +326,8 @@ struct regmap *regmap_init_spmi(struct spmi_device *dev,
 struct regmap *regmap_init_mmio_clk(struct device *dev, const char *clk_id,
 				    void __iomem *regs,
 				    const struct regmap_config *config);
+struct regmap *regmap_init_ssbi(struct device *dev,
+				const struct regmap_config *config);
 
 struct regmap *devm_regmap_init(struct device *dev,
 				const struct regmap_bus *bus,
@@ -340,6 +342,8 @@ struct regmap *devm_regmap_init_spmi(struct spmi_device *dev,
 struct regmap *devm_regmap_init_mmio_clk(struct device *dev, const char *clk_id,
 					 void __iomem *regs,
 					 const struct regmap_config *config);
+struct regmap *devm_regmap_init_ssbi(struct device *dev,
+					const struct regmap_config *config);
 
 /**
  * regmap_init_mmio(): Initialise register map
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

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

* [PATCH 4/8] mfd: ssbi: Mark match table const
  2013-12-10 23:35 ` Stephen Boyd
@ 2013-12-10 23:35   ` Stephen Boyd
  -1 siblings, 0 replies; 77+ messages in thread
From: Stephen Boyd @ 2013-12-10 23:35 UTC (permalink / raw)
  To: Samuel Ortiz, Lee Jones; +Cc: linux-kernel, linux-arm-msm, linux-arm-kernel

This is a read-only data structure.

Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 drivers/mfd/ssbi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mfd/ssbi.c b/drivers/mfd/ssbi.c
index dd1d28f..b78942e 100644
--- a/drivers/mfd/ssbi.c
+++ b/drivers/mfd/ssbi.c
@@ -317,7 +317,7 @@ static int ssbi_probe(struct platform_device *pdev)
 	return of_platform_populate(np, NULL, NULL, &pdev->dev);
 }
 
-static struct of_device_id ssbi_match_table[] = {
+static const struct of_device_id ssbi_match_table[] = {
 	{ .compatible = "qcom,ssbi" },
 	{}
 };
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

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

* [PATCH 4/8] mfd: ssbi: Mark match table const
@ 2013-12-10 23:35   ` Stephen Boyd
  0 siblings, 0 replies; 77+ messages in thread
From: Stephen Boyd @ 2013-12-10 23:35 UTC (permalink / raw)
  To: linux-arm-kernel

This is a read-only data structure.

Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 drivers/mfd/ssbi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mfd/ssbi.c b/drivers/mfd/ssbi.c
index dd1d28f..b78942e 100644
--- a/drivers/mfd/ssbi.c
+++ b/drivers/mfd/ssbi.c
@@ -317,7 +317,7 @@ static int ssbi_probe(struct platform_device *pdev)
 	return of_platform_populate(np, NULL, NULL, &pdev->dev);
 }
 
-static struct of_device_id ssbi_match_table[] = {
+static const struct of_device_id ssbi_match_table[] = {
 	{ .compatible = "qcom,ssbi" },
 	{}
 };
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

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

* [PATCH 5/8] mfd: Move pm8xxx-irq.c contents into only driver that uses it
  2013-12-10 23:35 ` Stephen Boyd
@ 2013-12-10 23:35   ` Stephen Boyd
  -1 siblings, 0 replies; 77+ messages in thread
From: Stephen Boyd @ 2013-12-10 23:35 UTC (permalink / raw)
  To: Samuel Ortiz, Lee Jones; +Cc: linux-kernel, linux-arm-msm, linux-arm-kernel

The pm8xxx-irq.c code is practically mandatory given that the
pm8921-core driver will WARN about it missing and the Kconfig
marks it as default y when a PM8xxx chips is enabled. The only
reason the file was split out was because we planned to support
other pm8xxx chips with different pm8xxx-core.c files. Now that
we have DT on ARM this isn't necessary because we should be able
to support all the ssbi based PM8xxx chips in one driver and one
file with no data bloat. Let's move this code into the only
driver that uses it right now (pm8921) so that it's always compiled when
needed. In the future we can rename pm8921-core.c to something
more generic.

Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 drivers/mfd/Kconfig       |  10 --
 drivers/mfd/pm8921-core.c | 349 +++++++++++++++++++++++++++++++++++++++++++
 drivers/mfd/pm8xxx-irq.c  | 371 ----------------------------------------------
 3 files changed, 349 insertions(+), 381 deletions(-)
 delete mode 100644 drivers/mfd/pm8xxx-irq.c

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 914c3d1..9ee4ce6 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -466,16 +466,6 @@ config MFD_PM8921_CORE
 	  Say M here if you want to include support for PM8921 chip as a module.
 	  This will build a module called "pm8921-core".
 
-config MFD_PM8XXX_IRQ
-	bool "Qualcomm PM8xxx IRQ features"
-	depends on MFD_PM8XXX
-	default y if MFD_PM8XXX
-	help
-	  This is the IRQ driver for Qualcomm PM 8xxx PMIC chips.
-
-	  This is required to use certain other PM 8xxx features, such as GPIO
-	  and MPP.
-
 config MFD_RDC321X
 	tristate "RDC R-321x southbridge"
 	select MFD_CORE
diff --git a/drivers/mfd/pm8921-core.c b/drivers/mfd/pm8921-core.c
index a6841f7..143f59d 100644
--- a/drivers/mfd/pm8921-core.c
+++ b/drivers/mfd/pm8921-core.c
@@ -14,6 +14,8 @@
 #define pr_fmt(fmt) "%s: " fmt, __func__
 
 #include <linux/kernel.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
@@ -22,6 +24,30 @@
 #include <linux/mfd/core.h>
 #include <linux/mfd/pm8xxx/pm8921.h>
 #include <linux/mfd/pm8xxx/core.h>
+#include <linux/mfd/pm8xxx/irq.h>
+
+#define	SSBI_REG_ADDR_IRQ_BASE		0x1BB
+
+#define	SSBI_REG_ADDR_IRQ_ROOT		(SSBI_REG_ADDR_IRQ_BASE + 0)
+#define	SSBI_REG_ADDR_IRQ_M_STATUS1	(SSBI_REG_ADDR_IRQ_BASE + 1)
+#define	SSBI_REG_ADDR_IRQ_M_STATUS2	(SSBI_REG_ADDR_IRQ_BASE + 2)
+#define	SSBI_REG_ADDR_IRQ_M_STATUS3	(SSBI_REG_ADDR_IRQ_BASE + 3)
+#define	SSBI_REG_ADDR_IRQ_M_STATUS4	(SSBI_REG_ADDR_IRQ_BASE + 4)
+#define	SSBI_REG_ADDR_IRQ_BLK_SEL	(SSBI_REG_ADDR_IRQ_BASE + 5)
+#define	SSBI_REG_ADDR_IRQ_IT_STATUS	(SSBI_REG_ADDR_IRQ_BASE + 6)
+#define	SSBI_REG_ADDR_IRQ_CONFIG	(SSBI_REG_ADDR_IRQ_BASE + 7)
+#define	SSBI_REG_ADDR_IRQ_RT_STATUS	(SSBI_REG_ADDR_IRQ_BASE + 8)
+
+#define	PM_IRQF_LVL_SEL			0x01	/* level select */
+#define	PM_IRQF_MASK_FE			0x02	/* mask falling edge */
+#define	PM_IRQF_MASK_RE			0x04	/* mask rising edge */
+#define	PM_IRQF_CLR			0x08	/* clear interrupt */
+#define	PM_IRQF_BITS_MASK		0x70
+#define	PM_IRQF_BITS_SHIFT		4
+#define	PM_IRQF_WRITE			0x80
+
+#define	PM_IRQF_MASK_ALL		(PM_IRQF_MASK_FE | \
+					PM_IRQF_MASK_RE)
 
 #define REG_HWREV		0x002  /* PMIC4 revision */
 #define REG_HWREV_2		0x0E8  /* PMIC4 revision 2 */
@@ -31,6 +57,329 @@ struct pm8921 {
 	struct pm_irq_chip		*irq_chip;
 };
 
+struct pm_irq_chip {
+	struct device		*dev;
+	spinlock_t		pm_irq_lock;
+	unsigned int		devirq;
+	unsigned int		irq_base;
+	unsigned int		num_irqs;
+	unsigned int		num_blocks;
+	unsigned int		num_masters;
+	u8			config[0];
+};
+
+static int pm8xxx_read_root_irq(const struct pm_irq_chip *chip, u8 *rp)
+{
+	return pm8xxx_readb(chip->dev, SSBI_REG_ADDR_IRQ_ROOT, rp);
+}
+
+static int pm8xxx_read_master_irq(const struct pm_irq_chip *chip, u8 m, u8 *bp)
+{
+	return pm8xxx_readb(chip->dev,
+			SSBI_REG_ADDR_IRQ_M_STATUS1 + m, bp);
+}
+
+static int pm8xxx_read_block_irq(struct pm_irq_chip *chip, u8 bp, u8 *ip)
+{
+	int	rc;
+
+	spin_lock(&chip->pm_irq_lock);
+	rc = pm8xxx_writeb(chip->dev, SSBI_REG_ADDR_IRQ_BLK_SEL, bp);
+	if (rc) {
+		pr_err("Failed Selecting Block %d rc=%d\n", bp, rc);
+		goto bail;
+	}
+
+	rc = pm8xxx_readb(chip->dev, SSBI_REG_ADDR_IRQ_IT_STATUS, ip);
+	if (rc)
+		pr_err("Failed Reading Status rc=%d\n", rc);
+bail:
+	spin_unlock(&chip->pm_irq_lock);
+	return rc;
+}
+
+static int pm8xxx_config_irq(struct pm_irq_chip *chip, u8 bp, u8 cp)
+{
+	int	rc;
+
+	spin_lock(&chip->pm_irq_lock);
+	rc = pm8xxx_writeb(chip->dev, SSBI_REG_ADDR_IRQ_BLK_SEL, bp);
+	if (rc) {
+		pr_err("Failed Selecting Block %d rc=%d\n", bp, rc);
+		goto bail;
+	}
+
+	cp |= PM_IRQF_WRITE;
+	rc = pm8xxx_writeb(chip->dev, SSBI_REG_ADDR_IRQ_CONFIG, cp);
+	if (rc)
+		pr_err("Failed Configuring IRQ rc=%d\n", rc);
+bail:
+	spin_unlock(&chip->pm_irq_lock);
+	return rc;
+}
+
+static int pm8xxx_irq_block_handler(struct pm_irq_chip *chip, int block)
+{
+	int pmirq, irq, i, ret = 0;
+	u8 bits;
+
+	ret = pm8xxx_read_block_irq(chip, block, &bits);
+	if (ret) {
+		pr_err("Failed reading %d block ret=%d", block, ret);
+		return ret;
+	}
+	if (!bits) {
+		pr_err("block bit set in master but no irqs: %d", block);
+		return 0;
+	}
+
+	/* Check IRQ bits */
+	for (i = 0; i < 8; i++) {
+		if (bits & (1 << i)) {
+			pmirq = block * 8 + i;
+			irq = pmirq + chip->irq_base;
+			generic_handle_irq(irq);
+		}
+	}
+	return 0;
+}
+
+static int pm8xxx_irq_master_handler(struct pm_irq_chip *chip, int master)
+{
+	u8 blockbits;
+	int block_number, i, ret = 0;
+
+	ret = pm8xxx_read_master_irq(chip, master, &blockbits);
+	if (ret) {
+		pr_err("Failed to read master %d ret=%d\n", master, ret);
+		return ret;
+	}
+	if (!blockbits) {
+		pr_err("master bit set in root but no blocks: %d", master);
+		return 0;
+	}
+
+	for (i = 0; i < 8; i++)
+		if (blockbits & (1 << i)) {
+			block_number = master * 8 + i;	/* block # */
+			ret |= pm8xxx_irq_block_handler(chip, block_number);
+		}
+	return ret;
+}
+
+static void pm8xxx_irq_handler(unsigned int irq, struct irq_desc *desc)
+{
+	struct pm_irq_chip *chip = irq_desc_get_handler_data(desc);
+	struct irq_chip *irq_chip = irq_desc_get_chip(desc);
+	u8	root;
+	int	i, ret, masters = 0;
+
+	ret = pm8xxx_read_root_irq(chip, &root);
+	if (ret) {
+		pr_err("Can't read root status ret=%d\n", ret);
+		return;
+	}
+
+	/* on pm8xxx series masters start from bit 1 of the root */
+	masters = root >> 1;
+
+	/* Read allowed masters for blocks. */
+	for (i = 0; i < chip->num_masters; i++)
+		if (masters & (1 << i))
+			pm8xxx_irq_master_handler(chip, i);
+
+	irq_chip->irq_ack(&desc->irq_data);
+}
+
+static void pm8xxx_irq_mask_ack(struct irq_data *d)
+{
+	struct pm_irq_chip *chip = irq_data_get_irq_chip_data(d);
+	unsigned int pmirq = d->irq - chip->irq_base;
+	int	master, irq_bit;
+	u8	block, config;
+
+	block = pmirq / 8;
+	master = block / 8;
+	irq_bit = pmirq % 8;
+
+	config = chip->config[pmirq] | PM_IRQF_MASK_ALL | PM_IRQF_CLR;
+	pm8xxx_config_irq(chip, block, config);
+}
+
+static void pm8xxx_irq_unmask(struct irq_data *d)
+{
+	struct pm_irq_chip *chip = irq_data_get_irq_chip_data(d);
+	unsigned int pmirq = d->irq - chip->irq_base;
+	int	master, irq_bit;
+	u8	block, config;
+
+	block = pmirq / 8;
+	master = block / 8;
+	irq_bit = pmirq % 8;
+
+	config = chip->config[pmirq];
+	pm8xxx_config_irq(chip, block, config);
+}
+
+static int pm8xxx_irq_set_type(struct irq_data *d, unsigned int flow_type)
+{
+	struct pm_irq_chip *chip = irq_data_get_irq_chip_data(d);
+	unsigned int pmirq = d->irq - chip->irq_base;
+	int master, irq_bit;
+	u8 block, config;
+
+	block = pmirq / 8;
+	master = block / 8;
+	irq_bit  = pmirq % 8;
+
+	chip->config[pmirq] = (irq_bit << PM_IRQF_BITS_SHIFT)
+							| PM_IRQF_MASK_ALL;
+	if (flow_type & (IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING)) {
+		if (flow_type & IRQF_TRIGGER_RISING)
+			chip->config[pmirq] &= ~PM_IRQF_MASK_RE;
+		if (flow_type & IRQF_TRIGGER_FALLING)
+			chip->config[pmirq] &= ~PM_IRQF_MASK_FE;
+	} else {
+		chip->config[pmirq] |= PM_IRQF_LVL_SEL;
+
+		if (flow_type & IRQF_TRIGGER_HIGH)
+			chip->config[pmirq] &= ~PM_IRQF_MASK_RE;
+		else
+			chip->config[pmirq] &= ~PM_IRQF_MASK_FE;
+	}
+
+	config = chip->config[pmirq] | PM_IRQF_CLR;
+	return pm8xxx_config_irq(chip, block, config);
+}
+
+static int pm8xxx_irq_set_wake(struct irq_data *d, unsigned int on)
+{
+	return 0;
+}
+
+static struct irq_chip pm8xxx_irq_chip = {
+	.name		= "pm8xxx",
+	.irq_mask_ack	= pm8xxx_irq_mask_ack,
+	.irq_unmask	= pm8xxx_irq_unmask,
+	.irq_set_type	= pm8xxx_irq_set_type,
+	.irq_set_wake	= pm8xxx_irq_set_wake,
+	.flags		= IRQCHIP_MASK_ON_SUSPEND,
+};
+
+/**
+ * pm8xxx_get_irq_stat - get the status of the irq line
+ * @chip: pointer to identify a pmic irq controller
+ * @irq: the irq number
+ *
+ * The pm8xxx gpio and mpp rely on the interrupt block to read
+ * the values on their pins. This function is to facilitate reading
+ * the status of a gpio or an mpp line. The caller has to convert the
+ * gpio number to irq number.
+ *
+ * RETURNS:
+ * an int indicating the value read on that line
+ */
+int pm8xxx_get_irq_stat(struct pm_irq_chip *chip, int irq)
+{
+	int pmirq, rc;
+	u8  block, bits, bit;
+	unsigned long flags;
+
+	if (chip == NULL || irq < chip->irq_base ||
+			irq >= chip->irq_base + chip->num_irqs)
+		return -EINVAL;
+
+	pmirq = irq - chip->irq_base;
+
+	block = pmirq / 8;
+	bit = pmirq % 8;
+
+	spin_lock_irqsave(&chip->pm_irq_lock, flags);
+
+	rc = pm8xxx_writeb(chip->dev, SSBI_REG_ADDR_IRQ_BLK_SEL, block);
+	if (rc) {
+		pr_err("Failed Selecting block irq=%d pmirq=%d blk=%d rc=%d\n",
+			irq, pmirq, block, rc);
+		goto bail_out;
+	}
+
+	rc = pm8xxx_readb(chip->dev, SSBI_REG_ADDR_IRQ_RT_STATUS, &bits);
+	if (rc) {
+		pr_err("Failed Configuring irq=%d pmirq=%d blk=%d rc=%d\n",
+			irq, pmirq, block, rc);
+		goto bail_out;
+	}
+
+	rc = (bits & (1 << bit)) ? 1 : 0;
+
+bail_out:
+	spin_unlock_irqrestore(&chip->pm_irq_lock, flags);
+
+	return rc;
+}
+EXPORT_SYMBOL_GPL(pm8xxx_get_irq_stat);
+
+struct pm_irq_chip *  pm8xxx_irq_init(struct device *dev,
+				const struct pm8xxx_irq_platform_data *pdata)
+{
+	struct pm_irq_chip  *chip;
+	int devirq, rc;
+	unsigned int pmirq;
+
+	if (!pdata) {
+		pr_err("No platform data\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	devirq = pdata->devirq;
+	if (devirq < 0) {
+		pr_err("missing devirq\n");
+		rc = devirq;
+		return ERR_PTR(-EINVAL);
+	}
+
+	chip = kzalloc(sizeof(struct pm_irq_chip)
+			+ sizeof(u8) * pdata->irq_cdata.nirqs, GFP_KERNEL);
+	if (!chip) {
+		pr_err("Cannot alloc pm_irq_chip struct\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	chip->dev = dev;
+	chip->devirq = devirq;
+	chip->irq_base = pdata->irq_base;
+	chip->num_irqs = pdata->irq_cdata.nirqs;
+	chip->num_blocks = DIV_ROUND_UP(chip->num_irqs, 8);
+	chip->num_masters = DIV_ROUND_UP(chip->num_blocks, 8);
+	spin_lock_init(&chip->pm_irq_lock);
+
+	for (pmirq = 0; pmirq < chip->num_irqs; pmirq++) {
+		irq_set_chip_and_handler(chip->irq_base + pmirq,
+				&pm8xxx_irq_chip,
+				handle_level_irq);
+		irq_set_chip_data(chip->irq_base + pmirq, chip);
+#ifdef CONFIG_ARM
+		set_irq_flags(chip->irq_base + pmirq, IRQF_VALID);
+#else
+		irq_set_noprobe(chip->irq_base + pmirq);
+#endif
+	}
+
+	irq_set_irq_type(devirq, pdata->irq_trigger_flag);
+	irq_set_handler_data(devirq, chip);
+	irq_set_chained_handler(devirq, pm8xxx_irq_handler);
+	set_irq_wake(devirq, 1);
+
+	return chip;
+}
+
+int pm8xxx_irq_exit(struct pm_irq_chip *chip)
+{
+	irq_set_chained_handler(chip->devirq, NULL);
+	kfree(chip);
+	return 0;
+}
+
 static int pm8921_readb(const struct device *dev, u16 addr, u8 *val)
 {
 	const struct pm8xxx_drvdata *pm8921_drvdata = dev_get_drvdata(dev);
diff --git a/drivers/mfd/pm8xxx-irq.c b/drivers/mfd/pm8xxx-irq.c
deleted file mode 100644
index 1360e20..0000000
--- a/drivers/mfd/pm8xxx-irq.c
+++ /dev/null
@@ -1,371 +0,0 @@
-/*
- * Copyright (c) 2011, Code Aurora Forum. All rights reserved.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 and
- * only version 2 as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- */
-
-#define pr_fmt(fmt)	"%s: " fmt, __func__
-
-#include <linux/err.h>
-#include <linux/interrupt.h>
-#include <linux/irq.h>
-#include <linux/kernel.h>
-#include <linux/mfd/pm8xxx/core.h>
-#include <linux/mfd/pm8xxx/irq.h>
-#include <linux/platform_device.h>
-#include <linux/slab.h>
-
-/* PMIC8xxx IRQ */
-
-#define	SSBI_REG_ADDR_IRQ_BASE		0x1BB
-
-#define	SSBI_REG_ADDR_IRQ_ROOT		(SSBI_REG_ADDR_IRQ_BASE + 0)
-#define	SSBI_REG_ADDR_IRQ_M_STATUS1	(SSBI_REG_ADDR_IRQ_BASE + 1)
-#define	SSBI_REG_ADDR_IRQ_M_STATUS2	(SSBI_REG_ADDR_IRQ_BASE + 2)
-#define	SSBI_REG_ADDR_IRQ_M_STATUS3	(SSBI_REG_ADDR_IRQ_BASE + 3)
-#define	SSBI_REG_ADDR_IRQ_M_STATUS4	(SSBI_REG_ADDR_IRQ_BASE + 4)
-#define	SSBI_REG_ADDR_IRQ_BLK_SEL	(SSBI_REG_ADDR_IRQ_BASE + 5)
-#define	SSBI_REG_ADDR_IRQ_IT_STATUS	(SSBI_REG_ADDR_IRQ_BASE + 6)
-#define	SSBI_REG_ADDR_IRQ_CONFIG	(SSBI_REG_ADDR_IRQ_BASE + 7)
-#define	SSBI_REG_ADDR_IRQ_RT_STATUS	(SSBI_REG_ADDR_IRQ_BASE + 8)
-
-#define	PM_IRQF_LVL_SEL			0x01	/* level select */
-#define	PM_IRQF_MASK_FE			0x02	/* mask falling edge */
-#define	PM_IRQF_MASK_RE			0x04	/* mask rising edge */
-#define	PM_IRQF_CLR			0x08	/* clear interrupt */
-#define	PM_IRQF_BITS_MASK		0x70
-#define	PM_IRQF_BITS_SHIFT		4
-#define	PM_IRQF_WRITE			0x80
-
-#define	PM_IRQF_MASK_ALL		(PM_IRQF_MASK_FE | \
-					PM_IRQF_MASK_RE)
-
-struct pm_irq_chip {
-	struct device		*dev;
-	spinlock_t		pm_irq_lock;
-	unsigned int		devirq;
-	unsigned int		irq_base;
-	unsigned int		num_irqs;
-	unsigned int		num_blocks;
-	unsigned int		num_masters;
-	u8			config[0];
-};
-
-static int pm8xxx_read_root_irq(const struct pm_irq_chip *chip, u8 *rp)
-{
-	return pm8xxx_readb(chip->dev, SSBI_REG_ADDR_IRQ_ROOT, rp);
-}
-
-static int pm8xxx_read_master_irq(const struct pm_irq_chip *chip, u8 m, u8 *bp)
-{
-	return pm8xxx_readb(chip->dev,
-			SSBI_REG_ADDR_IRQ_M_STATUS1 + m, bp);
-}
-
-static int pm8xxx_read_block_irq(struct pm_irq_chip *chip, u8 bp, u8 *ip)
-{
-	int	rc;
-
-	spin_lock(&chip->pm_irq_lock);
-	rc = pm8xxx_writeb(chip->dev, SSBI_REG_ADDR_IRQ_BLK_SEL, bp);
-	if (rc) {
-		pr_err("Failed Selecting Block %d rc=%d\n", bp, rc);
-		goto bail;
-	}
-
-	rc = pm8xxx_readb(chip->dev, SSBI_REG_ADDR_IRQ_IT_STATUS, ip);
-	if (rc)
-		pr_err("Failed Reading Status rc=%d\n", rc);
-bail:
-	spin_unlock(&chip->pm_irq_lock);
-	return rc;
-}
-
-static int pm8xxx_config_irq(struct pm_irq_chip *chip, u8 bp, u8 cp)
-{
-	int	rc;
-
-	spin_lock(&chip->pm_irq_lock);
-	rc = pm8xxx_writeb(chip->dev, SSBI_REG_ADDR_IRQ_BLK_SEL, bp);
-	if (rc) {
-		pr_err("Failed Selecting Block %d rc=%d\n", bp, rc);
-		goto bail;
-	}
-
-	cp |= PM_IRQF_WRITE;
-	rc = pm8xxx_writeb(chip->dev, SSBI_REG_ADDR_IRQ_CONFIG, cp);
-	if (rc)
-		pr_err("Failed Configuring IRQ rc=%d\n", rc);
-bail:
-	spin_unlock(&chip->pm_irq_lock);
-	return rc;
-}
-
-static int pm8xxx_irq_block_handler(struct pm_irq_chip *chip, int block)
-{
-	int pmirq, irq, i, ret = 0;
-	u8 bits;
-
-	ret = pm8xxx_read_block_irq(chip, block, &bits);
-	if (ret) {
-		pr_err("Failed reading %d block ret=%d", block, ret);
-		return ret;
-	}
-	if (!bits) {
-		pr_err("block bit set in master but no irqs: %d", block);
-		return 0;
-	}
-
-	/* Check IRQ bits */
-	for (i = 0; i < 8; i++) {
-		if (bits & (1 << i)) {
-			pmirq = block * 8 + i;
-			irq = pmirq + chip->irq_base;
-			generic_handle_irq(irq);
-		}
-	}
-	return 0;
-}
-
-static int pm8xxx_irq_master_handler(struct pm_irq_chip *chip, int master)
-{
-	u8 blockbits;
-	int block_number, i, ret = 0;
-
-	ret = pm8xxx_read_master_irq(chip, master, &blockbits);
-	if (ret) {
-		pr_err("Failed to read master %d ret=%d\n", master, ret);
-		return ret;
-	}
-	if (!blockbits) {
-		pr_err("master bit set in root but no blocks: %d", master);
-		return 0;
-	}
-
-	for (i = 0; i < 8; i++)
-		if (blockbits & (1 << i)) {
-			block_number = master * 8 + i;	/* block # */
-			ret |= pm8xxx_irq_block_handler(chip, block_number);
-		}
-	return ret;
-}
-
-static void pm8xxx_irq_handler(unsigned int irq, struct irq_desc *desc)
-{
-	struct pm_irq_chip *chip = irq_desc_get_handler_data(desc);
-	struct irq_chip *irq_chip = irq_desc_get_chip(desc);
-	u8	root;
-	int	i, ret, masters = 0;
-
-	ret = pm8xxx_read_root_irq(chip, &root);
-	if (ret) {
-		pr_err("Can't read root status ret=%d\n", ret);
-		return;
-	}
-
-	/* on pm8xxx series masters start from bit 1 of the root */
-	masters = root >> 1;
-
-	/* Read allowed masters for blocks. */
-	for (i = 0; i < chip->num_masters; i++)
-		if (masters & (1 << i))
-			pm8xxx_irq_master_handler(chip, i);
-
-	irq_chip->irq_ack(&desc->irq_data);
-}
-
-static void pm8xxx_irq_mask_ack(struct irq_data *d)
-{
-	struct pm_irq_chip *chip = irq_data_get_irq_chip_data(d);
-	unsigned int pmirq = d->irq - chip->irq_base;
-	int	master, irq_bit;
-	u8	block, config;
-
-	block = pmirq / 8;
-	master = block / 8;
-	irq_bit = pmirq % 8;
-
-	config = chip->config[pmirq] | PM_IRQF_MASK_ALL | PM_IRQF_CLR;
-	pm8xxx_config_irq(chip, block, config);
-}
-
-static void pm8xxx_irq_unmask(struct irq_data *d)
-{
-	struct pm_irq_chip *chip = irq_data_get_irq_chip_data(d);
-	unsigned int pmirq = d->irq - chip->irq_base;
-	int	master, irq_bit;
-	u8	block, config;
-
-	block = pmirq / 8;
-	master = block / 8;
-	irq_bit = pmirq % 8;
-
-	config = chip->config[pmirq];
-	pm8xxx_config_irq(chip, block, config);
-}
-
-static int pm8xxx_irq_set_type(struct irq_data *d, unsigned int flow_type)
-{
-	struct pm_irq_chip *chip = irq_data_get_irq_chip_data(d);
-	unsigned int pmirq = d->irq - chip->irq_base;
-	int master, irq_bit;
-	u8 block, config;
-
-	block = pmirq / 8;
-	master = block / 8;
-	irq_bit  = pmirq % 8;
-
-	chip->config[pmirq] = (irq_bit << PM_IRQF_BITS_SHIFT)
-							| PM_IRQF_MASK_ALL;
-	if (flow_type & (IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING)) {
-		if (flow_type & IRQF_TRIGGER_RISING)
-			chip->config[pmirq] &= ~PM_IRQF_MASK_RE;
-		if (flow_type & IRQF_TRIGGER_FALLING)
-			chip->config[pmirq] &= ~PM_IRQF_MASK_FE;
-	} else {
-		chip->config[pmirq] |= PM_IRQF_LVL_SEL;
-
-		if (flow_type & IRQF_TRIGGER_HIGH)
-			chip->config[pmirq] &= ~PM_IRQF_MASK_RE;
-		else
-			chip->config[pmirq] &= ~PM_IRQF_MASK_FE;
-	}
-
-	config = chip->config[pmirq] | PM_IRQF_CLR;
-	return pm8xxx_config_irq(chip, block, config);
-}
-
-static int pm8xxx_irq_set_wake(struct irq_data *d, unsigned int on)
-{
-	return 0;
-}
-
-static struct irq_chip pm8xxx_irq_chip = {
-	.name		= "pm8xxx",
-	.irq_mask_ack	= pm8xxx_irq_mask_ack,
-	.irq_unmask	= pm8xxx_irq_unmask,
-	.irq_set_type	= pm8xxx_irq_set_type,
-	.irq_set_wake	= pm8xxx_irq_set_wake,
-	.flags		= IRQCHIP_MASK_ON_SUSPEND,
-};
-
-/**
- * pm8xxx_get_irq_stat - get the status of the irq line
- * @chip: pointer to identify a pmic irq controller
- * @irq: the irq number
- *
- * The pm8xxx gpio and mpp rely on the interrupt block to read
- * the values on their pins. This function is to facilitate reading
- * the status of a gpio or an mpp line. The caller has to convert the
- * gpio number to irq number.
- *
- * RETURNS:
- * an int indicating the value read on that line
- */
-int pm8xxx_get_irq_stat(struct pm_irq_chip *chip, int irq)
-{
-	int pmirq, rc;
-	u8  block, bits, bit;
-	unsigned long flags;
-
-	if (chip == NULL || irq < chip->irq_base ||
-			irq >= chip->irq_base + chip->num_irqs)
-		return -EINVAL;
-
-	pmirq = irq - chip->irq_base;
-
-	block = pmirq / 8;
-	bit = pmirq % 8;
-
-	spin_lock_irqsave(&chip->pm_irq_lock, flags);
-
-	rc = pm8xxx_writeb(chip->dev, SSBI_REG_ADDR_IRQ_BLK_SEL, block);
-	if (rc) {
-		pr_err("Failed Selecting block irq=%d pmirq=%d blk=%d rc=%d\n",
-			irq, pmirq, block, rc);
-		goto bail_out;
-	}
-
-	rc = pm8xxx_readb(chip->dev, SSBI_REG_ADDR_IRQ_RT_STATUS, &bits);
-	if (rc) {
-		pr_err("Failed Configuring irq=%d pmirq=%d blk=%d rc=%d\n",
-			irq, pmirq, block, rc);
-		goto bail_out;
-	}
-
-	rc = (bits & (1 << bit)) ? 1 : 0;
-
-bail_out:
-	spin_unlock_irqrestore(&chip->pm_irq_lock, flags);
-
-	return rc;
-}
-EXPORT_SYMBOL_GPL(pm8xxx_get_irq_stat);
-
-struct pm_irq_chip *  pm8xxx_irq_init(struct device *dev,
-				const struct pm8xxx_irq_platform_data *pdata)
-{
-	struct pm_irq_chip  *chip;
-	int devirq, rc;
-	unsigned int pmirq;
-
-	if (!pdata) {
-		pr_err("No platform data\n");
-		return ERR_PTR(-EINVAL);
-	}
-
-	devirq = pdata->devirq;
-	if (devirq < 0) {
-		pr_err("missing devirq\n");
-		rc = devirq;
-		return ERR_PTR(-EINVAL);
-	}
-
-	chip = kzalloc(sizeof(struct pm_irq_chip)
-			+ sizeof(u8) * pdata->irq_cdata.nirqs, GFP_KERNEL);
-	if (!chip) {
-		pr_err("Cannot alloc pm_irq_chip struct\n");
-		return ERR_PTR(-EINVAL);
-	}
-
-	chip->dev = dev;
-	chip->devirq = devirq;
-	chip->irq_base = pdata->irq_base;
-	chip->num_irqs = pdata->irq_cdata.nirqs;
-	chip->num_blocks = DIV_ROUND_UP(chip->num_irqs, 8);
-	chip->num_masters = DIV_ROUND_UP(chip->num_blocks, 8);
-	spin_lock_init(&chip->pm_irq_lock);
-
-	for (pmirq = 0; pmirq < chip->num_irqs; pmirq++) {
-		irq_set_chip_and_handler(chip->irq_base + pmirq,
-				&pm8xxx_irq_chip,
-				handle_level_irq);
-		irq_set_chip_data(chip->irq_base + pmirq, chip);
-#ifdef CONFIG_ARM
-		set_irq_flags(chip->irq_base + pmirq, IRQF_VALID);
-#else
-		irq_set_noprobe(chip->irq_base + pmirq);
-#endif
-	}
-
-	irq_set_irq_type(devirq, pdata->irq_trigger_flag);
-	irq_set_handler_data(devirq, chip);
-	irq_set_chained_handler(devirq, pm8xxx_irq_handler);
-	set_irq_wake(devirq, 1);
-
-	return chip;
-}
-
-int pm8xxx_irq_exit(struct pm_irq_chip *chip)
-{
-	irq_set_chained_handler(chip->devirq, NULL);
-	kfree(chip);
-	return 0;
-}
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

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

* [PATCH 5/8] mfd: Move pm8xxx-irq.c contents into only driver that uses it
@ 2013-12-10 23:35   ` Stephen Boyd
  0 siblings, 0 replies; 77+ messages in thread
From: Stephen Boyd @ 2013-12-10 23:35 UTC (permalink / raw)
  To: linux-arm-kernel

The pm8xxx-irq.c code is practically mandatory given that the
pm8921-core driver will WARN about it missing and the Kconfig
marks it as default y when a PM8xxx chips is enabled. The only
reason the file was split out was because we planned to support
other pm8xxx chips with different pm8xxx-core.c files. Now that
we have DT on ARM this isn't necessary because we should be able
to support all the ssbi based PM8xxx chips in one driver and one
file with no data bloat. Let's move this code into the only
driver that uses it right now (pm8921) so that it's always compiled when
needed. In the future we can rename pm8921-core.c to something
more generic.

Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 drivers/mfd/Kconfig       |  10 --
 drivers/mfd/pm8921-core.c | 349 +++++++++++++++++++++++++++++++++++++++++++
 drivers/mfd/pm8xxx-irq.c  | 371 ----------------------------------------------
 3 files changed, 349 insertions(+), 381 deletions(-)
 delete mode 100644 drivers/mfd/pm8xxx-irq.c

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 914c3d1..9ee4ce6 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -466,16 +466,6 @@ config MFD_PM8921_CORE
 	  Say M here if you want to include support for PM8921 chip as a module.
 	  This will build a module called "pm8921-core".
 
-config MFD_PM8XXX_IRQ
-	bool "Qualcomm PM8xxx IRQ features"
-	depends on MFD_PM8XXX
-	default y if MFD_PM8XXX
-	help
-	  This is the IRQ driver for Qualcomm PM 8xxx PMIC chips.
-
-	  This is required to use certain other PM 8xxx features, such as GPIO
-	  and MPP.
-
 config MFD_RDC321X
 	tristate "RDC R-321x southbridge"
 	select MFD_CORE
diff --git a/drivers/mfd/pm8921-core.c b/drivers/mfd/pm8921-core.c
index a6841f7..143f59d 100644
--- a/drivers/mfd/pm8921-core.c
+++ b/drivers/mfd/pm8921-core.c
@@ -14,6 +14,8 @@
 #define pr_fmt(fmt) "%s: " fmt, __func__
 
 #include <linux/kernel.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
@@ -22,6 +24,30 @@
 #include <linux/mfd/core.h>
 #include <linux/mfd/pm8xxx/pm8921.h>
 #include <linux/mfd/pm8xxx/core.h>
+#include <linux/mfd/pm8xxx/irq.h>
+
+#define	SSBI_REG_ADDR_IRQ_BASE		0x1BB
+
+#define	SSBI_REG_ADDR_IRQ_ROOT		(SSBI_REG_ADDR_IRQ_BASE + 0)
+#define	SSBI_REG_ADDR_IRQ_M_STATUS1	(SSBI_REG_ADDR_IRQ_BASE + 1)
+#define	SSBI_REG_ADDR_IRQ_M_STATUS2	(SSBI_REG_ADDR_IRQ_BASE + 2)
+#define	SSBI_REG_ADDR_IRQ_M_STATUS3	(SSBI_REG_ADDR_IRQ_BASE + 3)
+#define	SSBI_REG_ADDR_IRQ_M_STATUS4	(SSBI_REG_ADDR_IRQ_BASE + 4)
+#define	SSBI_REG_ADDR_IRQ_BLK_SEL	(SSBI_REG_ADDR_IRQ_BASE + 5)
+#define	SSBI_REG_ADDR_IRQ_IT_STATUS	(SSBI_REG_ADDR_IRQ_BASE + 6)
+#define	SSBI_REG_ADDR_IRQ_CONFIG	(SSBI_REG_ADDR_IRQ_BASE + 7)
+#define	SSBI_REG_ADDR_IRQ_RT_STATUS	(SSBI_REG_ADDR_IRQ_BASE + 8)
+
+#define	PM_IRQF_LVL_SEL			0x01	/* level select */
+#define	PM_IRQF_MASK_FE			0x02	/* mask falling edge */
+#define	PM_IRQF_MASK_RE			0x04	/* mask rising edge */
+#define	PM_IRQF_CLR			0x08	/* clear interrupt */
+#define	PM_IRQF_BITS_MASK		0x70
+#define	PM_IRQF_BITS_SHIFT		4
+#define	PM_IRQF_WRITE			0x80
+
+#define	PM_IRQF_MASK_ALL		(PM_IRQF_MASK_FE | \
+					PM_IRQF_MASK_RE)
 
 #define REG_HWREV		0x002  /* PMIC4 revision */
 #define REG_HWREV_2		0x0E8  /* PMIC4 revision 2 */
@@ -31,6 +57,329 @@ struct pm8921 {
 	struct pm_irq_chip		*irq_chip;
 };
 
+struct pm_irq_chip {
+	struct device		*dev;
+	spinlock_t		pm_irq_lock;
+	unsigned int		devirq;
+	unsigned int		irq_base;
+	unsigned int		num_irqs;
+	unsigned int		num_blocks;
+	unsigned int		num_masters;
+	u8			config[0];
+};
+
+static int pm8xxx_read_root_irq(const struct pm_irq_chip *chip, u8 *rp)
+{
+	return pm8xxx_readb(chip->dev, SSBI_REG_ADDR_IRQ_ROOT, rp);
+}
+
+static int pm8xxx_read_master_irq(const struct pm_irq_chip *chip, u8 m, u8 *bp)
+{
+	return pm8xxx_readb(chip->dev,
+			SSBI_REG_ADDR_IRQ_M_STATUS1 + m, bp);
+}
+
+static int pm8xxx_read_block_irq(struct pm_irq_chip *chip, u8 bp, u8 *ip)
+{
+	int	rc;
+
+	spin_lock(&chip->pm_irq_lock);
+	rc = pm8xxx_writeb(chip->dev, SSBI_REG_ADDR_IRQ_BLK_SEL, bp);
+	if (rc) {
+		pr_err("Failed Selecting Block %d rc=%d\n", bp, rc);
+		goto bail;
+	}
+
+	rc = pm8xxx_readb(chip->dev, SSBI_REG_ADDR_IRQ_IT_STATUS, ip);
+	if (rc)
+		pr_err("Failed Reading Status rc=%d\n", rc);
+bail:
+	spin_unlock(&chip->pm_irq_lock);
+	return rc;
+}
+
+static int pm8xxx_config_irq(struct pm_irq_chip *chip, u8 bp, u8 cp)
+{
+	int	rc;
+
+	spin_lock(&chip->pm_irq_lock);
+	rc = pm8xxx_writeb(chip->dev, SSBI_REG_ADDR_IRQ_BLK_SEL, bp);
+	if (rc) {
+		pr_err("Failed Selecting Block %d rc=%d\n", bp, rc);
+		goto bail;
+	}
+
+	cp |= PM_IRQF_WRITE;
+	rc = pm8xxx_writeb(chip->dev, SSBI_REG_ADDR_IRQ_CONFIG, cp);
+	if (rc)
+		pr_err("Failed Configuring IRQ rc=%d\n", rc);
+bail:
+	spin_unlock(&chip->pm_irq_lock);
+	return rc;
+}
+
+static int pm8xxx_irq_block_handler(struct pm_irq_chip *chip, int block)
+{
+	int pmirq, irq, i, ret = 0;
+	u8 bits;
+
+	ret = pm8xxx_read_block_irq(chip, block, &bits);
+	if (ret) {
+		pr_err("Failed reading %d block ret=%d", block, ret);
+		return ret;
+	}
+	if (!bits) {
+		pr_err("block bit set in master but no irqs: %d", block);
+		return 0;
+	}
+
+	/* Check IRQ bits */
+	for (i = 0; i < 8; i++) {
+		if (bits & (1 << i)) {
+			pmirq = block * 8 + i;
+			irq = pmirq + chip->irq_base;
+			generic_handle_irq(irq);
+		}
+	}
+	return 0;
+}
+
+static int pm8xxx_irq_master_handler(struct pm_irq_chip *chip, int master)
+{
+	u8 blockbits;
+	int block_number, i, ret = 0;
+
+	ret = pm8xxx_read_master_irq(chip, master, &blockbits);
+	if (ret) {
+		pr_err("Failed to read master %d ret=%d\n", master, ret);
+		return ret;
+	}
+	if (!blockbits) {
+		pr_err("master bit set in root but no blocks: %d", master);
+		return 0;
+	}
+
+	for (i = 0; i < 8; i++)
+		if (blockbits & (1 << i)) {
+			block_number = master * 8 + i;	/* block # */
+			ret |= pm8xxx_irq_block_handler(chip, block_number);
+		}
+	return ret;
+}
+
+static void pm8xxx_irq_handler(unsigned int irq, struct irq_desc *desc)
+{
+	struct pm_irq_chip *chip = irq_desc_get_handler_data(desc);
+	struct irq_chip *irq_chip = irq_desc_get_chip(desc);
+	u8	root;
+	int	i, ret, masters = 0;
+
+	ret = pm8xxx_read_root_irq(chip, &root);
+	if (ret) {
+		pr_err("Can't read root status ret=%d\n", ret);
+		return;
+	}
+
+	/* on pm8xxx series masters start from bit 1 of the root */
+	masters = root >> 1;
+
+	/* Read allowed masters for blocks. */
+	for (i = 0; i < chip->num_masters; i++)
+		if (masters & (1 << i))
+			pm8xxx_irq_master_handler(chip, i);
+
+	irq_chip->irq_ack(&desc->irq_data);
+}
+
+static void pm8xxx_irq_mask_ack(struct irq_data *d)
+{
+	struct pm_irq_chip *chip = irq_data_get_irq_chip_data(d);
+	unsigned int pmirq = d->irq - chip->irq_base;
+	int	master, irq_bit;
+	u8	block, config;
+
+	block = pmirq / 8;
+	master = block / 8;
+	irq_bit = pmirq % 8;
+
+	config = chip->config[pmirq] | PM_IRQF_MASK_ALL | PM_IRQF_CLR;
+	pm8xxx_config_irq(chip, block, config);
+}
+
+static void pm8xxx_irq_unmask(struct irq_data *d)
+{
+	struct pm_irq_chip *chip = irq_data_get_irq_chip_data(d);
+	unsigned int pmirq = d->irq - chip->irq_base;
+	int	master, irq_bit;
+	u8	block, config;
+
+	block = pmirq / 8;
+	master = block / 8;
+	irq_bit = pmirq % 8;
+
+	config = chip->config[pmirq];
+	pm8xxx_config_irq(chip, block, config);
+}
+
+static int pm8xxx_irq_set_type(struct irq_data *d, unsigned int flow_type)
+{
+	struct pm_irq_chip *chip = irq_data_get_irq_chip_data(d);
+	unsigned int pmirq = d->irq - chip->irq_base;
+	int master, irq_bit;
+	u8 block, config;
+
+	block = pmirq / 8;
+	master = block / 8;
+	irq_bit  = pmirq % 8;
+
+	chip->config[pmirq] = (irq_bit << PM_IRQF_BITS_SHIFT)
+							| PM_IRQF_MASK_ALL;
+	if (flow_type & (IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING)) {
+		if (flow_type & IRQF_TRIGGER_RISING)
+			chip->config[pmirq] &= ~PM_IRQF_MASK_RE;
+		if (flow_type & IRQF_TRIGGER_FALLING)
+			chip->config[pmirq] &= ~PM_IRQF_MASK_FE;
+	} else {
+		chip->config[pmirq] |= PM_IRQF_LVL_SEL;
+
+		if (flow_type & IRQF_TRIGGER_HIGH)
+			chip->config[pmirq] &= ~PM_IRQF_MASK_RE;
+		else
+			chip->config[pmirq] &= ~PM_IRQF_MASK_FE;
+	}
+
+	config = chip->config[pmirq] | PM_IRQF_CLR;
+	return pm8xxx_config_irq(chip, block, config);
+}
+
+static int pm8xxx_irq_set_wake(struct irq_data *d, unsigned int on)
+{
+	return 0;
+}
+
+static struct irq_chip pm8xxx_irq_chip = {
+	.name		= "pm8xxx",
+	.irq_mask_ack	= pm8xxx_irq_mask_ack,
+	.irq_unmask	= pm8xxx_irq_unmask,
+	.irq_set_type	= pm8xxx_irq_set_type,
+	.irq_set_wake	= pm8xxx_irq_set_wake,
+	.flags		= IRQCHIP_MASK_ON_SUSPEND,
+};
+
+/**
+ * pm8xxx_get_irq_stat - get the status of the irq line
+ * @chip: pointer to identify a pmic irq controller
+ * @irq: the irq number
+ *
+ * The pm8xxx gpio and mpp rely on the interrupt block to read
+ * the values on their pins. This function is to facilitate reading
+ * the status of a gpio or an mpp line. The caller has to convert the
+ * gpio number to irq number.
+ *
+ * RETURNS:
+ * an int indicating the value read on that line
+ */
+int pm8xxx_get_irq_stat(struct pm_irq_chip *chip, int irq)
+{
+	int pmirq, rc;
+	u8  block, bits, bit;
+	unsigned long flags;
+
+	if (chip == NULL || irq < chip->irq_base ||
+			irq >= chip->irq_base + chip->num_irqs)
+		return -EINVAL;
+
+	pmirq = irq - chip->irq_base;
+
+	block = pmirq / 8;
+	bit = pmirq % 8;
+
+	spin_lock_irqsave(&chip->pm_irq_lock, flags);
+
+	rc = pm8xxx_writeb(chip->dev, SSBI_REG_ADDR_IRQ_BLK_SEL, block);
+	if (rc) {
+		pr_err("Failed Selecting block irq=%d pmirq=%d blk=%d rc=%d\n",
+			irq, pmirq, block, rc);
+		goto bail_out;
+	}
+
+	rc = pm8xxx_readb(chip->dev, SSBI_REG_ADDR_IRQ_RT_STATUS, &bits);
+	if (rc) {
+		pr_err("Failed Configuring irq=%d pmirq=%d blk=%d rc=%d\n",
+			irq, pmirq, block, rc);
+		goto bail_out;
+	}
+
+	rc = (bits & (1 << bit)) ? 1 : 0;
+
+bail_out:
+	spin_unlock_irqrestore(&chip->pm_irq_lock, flags);
+
+	return rc;
+}
+EXPORT_SYMBOL_GPL(pm8xxx_get_irq_stat);
+
+struct pm_irq_chip *  pm8xxx_irq_init(struct device *dev,
+				const struct pm8xxx_irq_platform_data *pdata)
+{
+	struct pm_irq_chip  *chip;
+	int devirq, rc;
+	unsigned int pmirq;
+
+	if (!pdata) {
+		pr_err("No platform data\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	devirq = pdata->devirq;
+	if (devirq < 0) {
+		pr_err("missing devirq\n");
+		rc = devirq;
+		return ERR_PTR(-EINVAL);
+	}
+
+	chip = kzalloc(sizeof(struct pm_irq_chip)
+			+ sizeof(u8) * pdata->irq_cdata.nirqs, GFP_KERNEL);
+	if (!chip) {
+		pr_err("Cannot alloc pm_irq_chip struct\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	chip->dev = dev;
+	chip->devirq = devirq;
+	chip->irq_base = pdata->irq_base;
+	chip->num_irqs = pdata->irq_cdata.nirqs;
+	chip->num_blocks = DIV_ROUND_UP(chip->num_irqs, 8);
+	chip->num_masters = DIV_ROUND_UP(chip->num_blocks, 8);
+	spin_lock_init(&chip->pm_irq_lock);
+
+	for (pmirq = 0; pmirq < chip->num_irqs; pmirq++) {
+		irq_set_chip_and_handler(chip->irq_base + pmirq,
+				&pm8xxx_irq_chip,
+				handle_level_irq);
+		irq_set_chip_data(chip->irq_base + pmirq, chip);
+#ifdef CONFIG_ARM
+		set_irq_flags(chip->irq_base + pmirq, IRQF_VALID);
+#else
+		irq_set_noprobe(chip->irq_base + pmirq);
+#endif
+	}
+
+	irq_set_irq_type(devirq, pdata->irq_trigger_flag);
+	irq_set_handler_data(devirq, chip);
+	irq_set_chained_handler(devirq, pm8xxx_irq_handler);
+	set_irq_wake(devirq, 1);
+
+	return chip;
+}
+
+int pm8xxx_irq_exit(struct pm_irq_chip *chip)
+{
+	irq_set_chained_handler(chip->devirq, NULL);
+	kfree(chip);
+	return 0;
+}
+
 static int pm8921_readb(const struct device *dev, u16 addr, u8 *val)
 {
 	const struct pm8xxx_drvdata *pm8921_drvdata = dev_get_drvdata(dev);
diff --git a/drivers/mfd/pm8xxx-irq.c b/drivers/mfd/pm8xxx-irq.c
deleted file mode 100644
index 1360e20..0000000
--- a/drivers/mfd/pm8xxx-irq.c
+++ /dev/null
@@ -1,371 +0,0 @@
-/*
- * Copyright (c) 2011, Code Aurora Forum. All rights reserved.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 and
- * only version 2 as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- */
-
-#define pr_fmt(fmt)	"%s: " fmt, __func__
-
-#include <linux/err.h>
-#include <linux/interrupt.h>
-#include <linux/irq.h>
-#include <linux/kernel.h>
-#include <linux/mfd/pm8xxx/core.h>
-#include <linux/mfd/pm8xxx/irq.h>
-#include <linux/platform_device.h>
-#include <linux/slab.h>
-
-/* PMIC8xxx IRQ */
-
-#define	SSBI_REG_ADDR_IRQ_BASE		0x1BB
-
-#define	SSBI_REG_ADDR_IRQ_ROOT		(SSBI_REG_ADDR_IRQ_BASE + 0)
-#define	SSBI_REG_ADDR_IRQ_M_STATUS1	(SSBI_REG_ADDR_IRQ_BASE + 1)
-#define	SSBI_REG_ADDR_IRQ_M_STATUS2	(SSBI_REG_ADDR_IRQ_BASE + 2)
-#define	SSBI_REG_ADDR_IRQ_M_STATUS3	(SSBI_REG_ADDR_IRQ_BASE + 3)
-#define	SSBI_REG_ADDR_IRQ_M_STATUS4	(SSBI_REG_ADDR_IRQ_BASE + 4)
-#define	SSBI_REG_ADDR_IRQ_BLK_SEL	(SSBI_REG_ADDR_IRQ_BASE + 5)
-#define	SSBI_REG_ADDR_IRQ_IT_STATUS	(SSBI_REG_ADDR_IRQ_BASE + 6)
-#define	SSBI_REG_ADDR_IRQ_CONFIG	(SSBI_REG_ADDR_IRQ_BASE + 7)
-#define	SSBI_REG_ADDR_IRQ_RT_STATUS	(SSBI_REG_ADDR_IRQ_BASE + 8)
-
-#define	PM_IRQF_LVL_SEL			0x01	/* level select */
-#define	PM_IRQF_MASK_FE			0x02	/* mask falling edge */
-#define	PM_IRQF_MASK_RE			0x04	/* mask rising edge */
-#define	PM_IRQF_CLR			0x08	/* clear interrupt */
-#define	PM_IRQF_BITS_MASK		0x70
-#define	PM_IRQF_BITS_SHIFT		4
-#define	PM_IRQF_WRITE			0x80
-
-#define	PM_IRQF_MASK_ALL		(PM_IRQF_MASK_FE | \
-					PM_IRQF_MASK_RE)
-
-struct pm_irq_chip {
-	struct device		*dev;
-	spinlock_t		pm_irq_lock;
-	unsigned int		devirq;
-	unsigned int		irq_base;
-	unsigned int		num_irqs;
-	unsigned int		num_blocks;
-	unsigned int		num_masters;
-	u8			config[0];
-};
-
-static int pm8xxx_read_root_irq(const struct pm_irq_chip *chip, u8 *rp)
-{
-	return pm8xxx_readb(chip->dev, SSBI_REG_ADDR_IRQ_ROOT, rp);
-}
-
-static int pm8xxx_read_master_irq(const struct pm_irq_chip *chip, u8 m, u8 *bp)
-{
-	return pm8xxx_readb(chip->dev,
-			SSBI_REG_ADDR_IRQ_M_STATUS1 + m, bp);
-}
-
-static int pm8xxx_read_block_irq(struct pm_irq_chip *chip, u8 bp, u8 *ip)
-{
-	int	rc;
-
-	spin_lock(&chip->pm_irq_lock);
-	rc = pm8xxx_writeb(chip->dev, SSBI_REG_ADDR_IRQ_BLK_SEL, bp);
-	if (rc) {
-		pr_err("Failed Selecting Block %d rc=%d\n", bp, rc);
-		goto bail;
-	}
-
-	rc = pm8xxx_readb(chip->dev, SSBI_REG_ADDR_IRQ_IT_STATUS, ip);
-	if (rc)
-		pr_err("Failed Reading Status rc=%d\n", rc);
-bail:
-	spin_unlock(&chip->pm_irq_lock);
-	return rc;
-}
-
-static int pm8xxx_config_irq(struct pm_irq_chip *chip, u8 bp, u8 cp)
-{
-	int	rc;
-
-	spin_lock(&chip->pm_irq_lock);
-	rc = pm8xxx_writeb(chip->dev, SSBI_REG_ADDR_IRQ_BLK_SEL, bp);
-	if (rc) {
-		pr_err("Failed Selecting Block %d rc=%d\n", bp, rc);
-		goto bail;
-	}
-
-	cp |= PM_IRQF_WRITE;
-	rc = pm8xxx_writeb(chip->dev, SSBI_REG_ADDR_IRQ_CONFIG, cp);
-	if (rc)
-		pr_err("Failed Configuring IRQ rc=%d\n", rc);
-bail:
-	spin_unlock(&chip->pm_irq_lock);
-	return rc;
-}
-
-static int pm8xxx_irq_block_handler(struct pm_irq_chip *chip, int block)
-{
-	int pmirq, irq, i, ret = 0;
-	u8 bits;
-
-	ret = pm8xxx_read_block_irq(chip, block, &bits);
-	if (ret) {
-		pr_err("Failed reading %d block ret=%d", block, ret);
-		return ret;
-	}
-	if (!bits) {
-		pr_err("block bit set in master but no irqs: %d", block);
-		return 0;
-	}
-
-	/* Check IRQ bits */
-	for (i = 0; i < 8; i++) {
-		if (bits & (1 << i)) {
-			pmirq = block * 8 + i;
-			irq = pmirq + chip->irq_base;
-			generic_handle_irq(irq);
-		}
-	}
-	return 0;
-}
-
-static int pm8xxx_irq_master_handler(struct pm_irq_chip *chip, int master)
-{
-	u8 blockbits;
-	int block_number, i, ret = 0;
-
-	ret = pm8xxx_read_master_irq(chip, master, &blockbits);
-	if (ret) {
-		pr_err("Failed to read master %d ret=%d\n", master, ret);
-		return ret;
-	}
-	if (!blockbits) {
-		pr_err("master bit set in root but no blocks: %d", master);
-		return 0;
-	}
-
-	for (i = 0; i < 8; i++)
-		if (blockbits & (1 << i)) {
-			block_number = master * 8 + i;	/* block # */
-			ret |= pm8xxx_irq_block_handler(chip, block_number);
-		}
-	return ret;
-}
-
-static void pm8xxx_irq_handler(unsigned int irq, struct irq_desc *desc)
-{
-	struct pm_irq_chip *chip = irq_desc_get_handler_data(desc);
-	struct irq_chip *irq_chip = irq_desc_get_chip(desc);
-	u8	root;
-	int	i, ret, masters = 0;
-
-	ret = pm8xxx_read_root_irq(chip, &root);
-	if (ret) {
-		pr_err("Can't read root status ret=%d\n", ret);
-		return;
-	}
-
-	/* on pm8xxx series masters start from bit 1 of the root */
-	masters = root >> 1;
-
-	/* Read allowed masters for blocks. */
-	for (i = 0; i < chip->num_masters; i++)
-		if (masters & (1 << i))
-			pm8xxx_irq_master_handler(chip, i);
-
-	irq_chip->irq_ack(&desc->irq_data);
-}
-
-static void pm8xxx_irq_mask_ack(struct irq_data *d)
-{
-	struct pm_irq_chip *chip = irq_data_get_irq_chip_data(d);
-	unsigned int pmirq = d->irq - chip->irq_base;
-	int	master, irq_bit;
-	u8	block, config;
-
-	block = pmirq / 8;
-	master = block / 8;
-	irq_bit = pmirq % 8;
-
-	config = chip->config[pmirq] | PM_IRQF_MASK_ALL | PM_IRQF_CLR;
-	pm8xxx_config_irq(chip, block, config);
-}
-
-static void pm8xxx_irq_unmask(struct irq_data *d)
-{
-	struct pm_irq_chip *chip = irq_data_get_irq_chip_data(d);
-	unsigned int pmirq = d->irq - chip->irq_base;
-	int	master, irq_bit;
-	u8	block, config;
-
-	block = pmirq / 8;
-	master = block / 8;
-	irq_bit = pmirq % 8;
-
-	config = chip->config[pmirq];
-	pm8xxx_config_irq(chip, block, config);
-}
-
-static int pm8xxx_irq_set_type(struct irq_data *d, unsigned int flow_type)
-{
-	struct pm_irq_chip *chip = irq_data_get_irq_chip_data(d);
-	unsigned int pmirq = d->irq - chip->irq_base;
-	int master, irq_bit;
-	u8 block, config;
-
-	block = pmirq / 8;
-	master = block / 8;
-	irq_bit  = pmirq % 8;
-
-	chip->config[pmirq] = (irq_bit << PM_IRQF_BITS_SHIFT)
-							| PM_IRQF_MASK_ALL;
-	if (flow_type & (IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING)) {
-		if (flow_type & IRQF_TRIGGER_RISING)
-			chip->config[pmirq] &= ~PM_IRQF_MASK_RE;
-		if (flow_type & IRQF_TRIGGER_FALLING)
-			chip->config[pmirq] &= ~PM_IRQF_MASK_FE;
-	} else {
-		chip->config[pmirq] |= PM_IRQF_LVL_SEL;
-
-		if (flow_type & IRQF_TRIGGER_HIGH)
-			chip->config[pmirq] &= ~PM_IRQF_MASK_RE;
-		else
-			chip->config[pmirq] &= ~PM_IRQF_MASK_FE;
-	}
-
-	config = chip->config[pmirq] | PM_IRQF_CLR;
-	return pm8xxx_config_irq(chip, block, config);
-}
-
-static int pm8xxx_irq_set_wake(struct irq_data *d, unsigned int on)
-{
-	return 0;
-}
-
-static struct irq_chip pm8xxx_irq_chip = {
-	.name		= "pm8xxx",
-	.irq_mask_ack	= pm8xxx_irq_mask_ack,
-	.irq_unmask	= pm8xxx_irq_unmask,
-	.irq_set_type	= pm8xxx_irq_set_type,
-	.irq_set_wake	= pm8xxx_irq_set_wake,
-	.flags		= IRQCHIP_MASK_ON_SUSPEND,
-};
-
-/**
- * pm8xxx_get_irq_stat - get the status of the irq line
- * @chip: pointer to identify a pmic irq controller
- * @irq: the irq number
- *
- * The pm8xxx gpio and mpp rely on the interrupt block to read
- * the values on their pins. This function is to facilitate reading
- * the status of a gpio or an mpp line. The caller has to convert the
- * gpio number to irq number.
- *
- * RETURNS:
- * an int indicating the value read on that line
- */
-int pm8xxx_get_irq_stat(struct pm_irq_chip *chip, int irq)
-{
-	int pmirq, rc;
-	u8  block, bits, bit;
-	unsigned long flags;
-
-	if (chip == NULL || irq < chip->irq_base ||
-			irq >= chip->irq_base + chip->num_irqs)
-		return -EINVAL;
-
-	pmirq = irq - chip->irq_base;
-
-	block = pmirq / 8;
-	bit = pmirq % 8;
-
-	spin_lock_irqsave(&chip->pm_irq_lock, flags);
-
-	rc = pm8xxx_writeb(chip->dev, SSBI_REG_ADDR_IRQ_BLK_SEL, block);
-	if (rc) {
-		pr_err("Failed Selecting block irq=%d pmirq=%d blk=%d rc=%d\n",
-			irq, pmirq, block, rc);
-		goto bail_out;
-	}
-
-	rc = pm8xxx_readb(chip->dev, SSBI_REG_ADDR_IRQ_RT_STATUS, &bits);
-	if (rc) {
-		pr_err("Failed Configuring irq=%d pmirq=%d blk=%d rc=%d\n",
-			irq, pmirq, block, rc);
-		goto bail_out;
-	}
-
-	rc = (bits & (1 << bit)) ? 1 : 0;
-
-bail_out:
-	spin_unlock_irqrestore(&chip->pm_irq_lock, flags);
-
-	return rc;
-}
-EXPORT_SYMBOL_GPL(pm8xxx_get_irq_stat);
-
-struct pm_irq_chip *  pm8xxx_irq_init(struct device *dev,
-				const struct pm8xxx_irq_platform_data *pdata)
-{
-	struct pm_irq_chip  *chip;
-	int devirq, rc;
-	unsigned int pmirq;
-
-	if (!pdata) {
-		pr_err("No platform data\n");
-		return ERR_PTR(-EINVAL);
-	}
-
-	devirq = pdata->devirq;
-	if (devirq < 0) {
-		pr_err("missing devirq\n");
-		rc = devirq;
-		return ERR_PTR(-EINVAL);
-	}
-
-	chip = kzalloc(sizeof(struct pm_irq_chip)
-			+ sizeof(u8) * pdata->irq_cdata.nirqs, GFP_KERNEL);
-	if (!chip) {
-		pr_err("Cannot alloc pm_irq_chip struct\n");
-		return ERR_PTR(-EINVAL);
-	}
-
-	chip->dev = dev;
-	chip->devirq = devirq;
-	chip->irq_base = pdata->irq_base;
-	chip->num_irqs = pdata->irq_cdata.nirqs;
-	chip->num_blocks = DIV_ROUND_UP(chip->num_irqs, 8);
-	chip->num_masters = DIV_ROUND_UP(chip->num_blocks, 8);
-	spin_lock_init(&chip->pm_irq_lock);
-
-	for (pmirq = 0; pmirq < chip->num_irqs; pmirq++) {
-		irq_set_chip_and_handler(chip->irq_base + pmirq,
-				&pm8xxx_irq_chip,
-				handle_level_irq);
-		irq_set_chip_data(chip->irq_base + pmirq, chip);
-#ifdef CONFIG_ARM
-		set_irq_flags(chip->irq_base + pmirq, IRQF_VALID);
-#else
-		irq_set_noprobe(chip->irq_base + pmirq);
-#endif
-	}
-
-	irq_set_irq_type(devirq, pdata->irq_trigger_flag);
-	irq_set_handler_data(devirq, chip);
-	irq_set_chained_handler(devirq, pm8xxx_irq_handler);
-	set_irq_wake(devirq, 1);
-
-	return chip;
-}
-
-int pm8xxx_irq_exit(struct pm_irq_chip *chip)
-{
-	irq_set_chained_handler(chip->devirq, NULL);
-	kfree(chip);
-	return 0;
-}
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

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

* [PATCH 6/8] mfd: pm8921: Update for genirq changes
  2013-12-10 23:35 ` Stephen Boyd
@ 2013-12-10 23:35   ` Stephen Boyd
  -1 siblings, 0 replies; 77+ messages in thread
From: Stephen Boyd @ 2013-12-10 23:35 UTC (permalink / raw)
  To: Samuel Ortiz, Lee Jones; +Cc: linux-kernel, linux-arm-msm, linux-arm-kernel

Since this code has been marked broken for some time a few genirq
tree wide changes weren't made. set_irq_wake() was renamed to
irq_set_irq_wake() in commit a0cd9ca2b (genirq: Namespace
cleanup, 2011-02-10) and commit 10a8c383 (irq: introduce entry
and exit functions for chained handlers) introduced the chained
irq functions but this driver wasn't updated to use them. Fix
these problems and remove the BROKEN marking on this driver.

Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 drivers/mfd/Kconfig       | 1 -
 drivers/mfd/pm8921-core.c | 7 +++++--
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 9ee4ce6..35007ed 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -453,7 +453,6 @@ config MFD_PM8XXX
 config MFD_PM8921_CORE
 	tristate "Qualcomm PM8921 PMIC chip"
 	depends on (ARCH_MSM || HEXAGON)
-	depends on BROKEN
 	select MFD_CORE
 	select MFD_PM8XXX
 	help
diff --git a/drivers/mfd/pm8921-core.c b/drivers/mfd/pm8921-core.c
index 143f59d..083fab2 100644
--- a/drivers/mfd/pm8921-core.c
+++ b/drivers/mfd/pm8921-core.c
@@ -15,6 +15,7 @@
 
 #include <linux/kernel.h>
 #include <linux/interrupt.h>
+#include <linux/irqchip/chained_irq.h>
 #include <linux/irq.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
@@ -174,6 +175,8 @@ static void pm8xxx_irq_handler(unsigned int irq, struct irq_desc *desc)
 	u8	root;
 	int	i, ret, masters = 0;
 
+	chained_irq_enter(irq_chip, desc);
+
 	ret = pm8xxx_read_root_irq(chip, &root);
 	if (ret) {
 		pr_err("Can't read root status ret=%d\n", ret);
@@ -188,7 +191,7 @@ static void pm8xxx_irq_handler(unsigned int irq, struct irq_desc *desc)
 		if (masters & (1 << i))
 			pm8xxx_irq_master_handler(chip, i);
 
-	irq_chip->irq_ack(&desc->irq_data);
+	chained_irq_exit(irq_chip, desc);
 }
 
 static void pm8xxx_irq_mask_ack(struct irq_data *d)
@@ -368,7 +371,7 @@ struct pm_irq_chip *  pm8xxx_irq_init(struct device *dev,
 	irq_set_irq_type(devirq, pdata->irq_trigger_flag);
 	irq_set_handler_data(devirq, chip);
 	irq_set_chained_handler(devirq, pm8xxx_irq_handler);
-	set_irq_wake(devirq, 1);
+	irq_set_irq_wake(devirq, 1);
 
 	return chip;
 }
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

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

* [PATCH 6/8] mfd: pm8921: Update for genirq changes
@ 2013-12-10 23:35   ` Stephen Boyd
  0 siblings, 0 replies; 77+ messages in thread
From: Stephen Boyd @ 2013-12-10 23:35 UTC (permalink / raw)
  To: linux-arm-kernel

Since this code has been marked broken for some time a few genirq
tree wide changes weren't made. set_irq_wake() was renamed to
irq_set_irq_wake() in commit a0cd9ca2b (genirq: Namespace
cleanup, 2011-02-10) and commit 10a8c383 (irq: introduce entry
and exit functions for chained handlers) introduced the chained
irq functions but this driver wasn't updated to use them. Fix
these problems and remove the BROKEN marking on this driver.

Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 drivers/mfd/Kconfig       | 1 -
 drivers/mfd/pm8921-core.c | 7 +++++--
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 9ee4ce6..35007ed 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -453,7 +453,6 @@ config MFD_PM8XXX
 config MFD_PM8921_CORE
 	tristate "Qualcomm PM8921 PMIC chip"
 	depends on (ARCH_MSM || HEXAGON)
-	depends on BROKEN
 	select MFD_CORE
 	select MFD_PM8XXX
 	help
diff --git a/drivers/mfd/pm8921-core.c b/drivers/mfd/pm8921-core.c
index 143f59d..083fab2 100644
--- a/drivers/mfd/pm8921-core.c
+++ b/drivers/mfd/pm8921-core.c
@@ -15,6 +15,7 @@
 
 #include <linux/kernel.h>
 #include <linux/interrupt.h>
+#include <linux/irqchip/chained_irq.h>
 #include <linux/irq.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
@@ -174,6 +175,8 @@ static void pm8xxx_irq_handler(unsigned int irq, struct irq_desc *desc)
 	u8	root;
 	int	i, ret, masters = 0;
 
+	chained_irq_enter(irq_chip, desc);
+
 	ret = pm8xxx_read_root_irq(chip, &root);
 	if (ret) {
 		pr_err("Can't read root status ret=%d\n", ret);
@@ -188,7 +191,7 @@ static void pm8xxx_irq_handler(unsigned int irq, struct irq_desc *desc)
 		if (masters & (1 << i))
 			pm8xxx_irq_master_handler(chip, i);
 
-	irq_chip->irq_ack(&desc->irq_data);
+	chained_irq_exit(irq_chip, desc);
 }
 
 static void pm8xxx_irq_mask_ack(struct irq_data *d)
@@ -368,7 +371,7 @@ struct pm_irq_chip *  pm8xxx_irq_init(struct device *dev,
 	irq_set_irq_type(devirq, pdata->irq_trigger_flag);
 	irq_set_handler_data(devirq, chip);
 	irq_set_chained_handler(devirq, pm8xxx_irq_handler);
-	set_irq_wake(devirq, 1);
+	irq_set_irq_wake(devirq, 1);
 
 	return chip;
 }
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

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

* [PATCH 7/8] mfd: pm8921: Migrate to irqdomains
  2013-12-10 23:35 ` Stephen Boyd
@ 2013-12-10 23:35   ` Stephen Boyd
  -1 siblings, 0 replies; 77+ messages in thread
From: Stephen Boyd @ 2013-12-10 23:35 UTC (permalink / raw)
  To: Samuel Ortiz, Lee Jones; +Cc: linux-kernel, linux-arm-msm, linux-arm-kernel

Convert this driver to use irqdomains so that the PMIC's child
devices can be converted to devicetree.

Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 drivers/mfd/pm8921-core.c         | 186 ++++++++++++++------------------------
 include/linux/mfd/pm8xxx/irq.h    |  59 ------------
 include/linux/mfd/pm8xxx/pm8921.h |  30 ------
 3 files changed, 66 insertions(+), 209 deletions(-)
 delete mode 100644 include/linux/mfd/pm8xxx/irq.h
 delete mode 100644 include/linux/mfd/pm8xxx/pm8921.h

diff --git a/drivers/mfd/pm8921-core.c b/drivers/mfd/pm8921-core.c
index 083fab2..7ea6c7a 100644
--- a/drivers/mfd/pm8921-core.c
+++ b/drivers/mfd/pm8921-core.c
@@ -17,15 +17,15 @@
 #include <linux/interrupt.h>
 #include <linux/irqchip/chained_irq.h>
 #include <linux/irq.h>
+#include <linux/irqdomain.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
 #include <linux/err.h>
 #include <linux/ssbi.h>
+#include <linux/of_platform.h>
 #include <linux/mfd/core.h>
-#include <linux/mfd/pm8xxx/pm8921.h>
 #include <linux/mfd/pm8xxx/core.h>
-#include <linux/mfd/pm8xxx/irq.h>
 
 #define	SSBI_REG_ADDR_IRQ_BASE		0x1BB
 
@@ -61,8 +61,7 @@ struct pm8921 {
 struct pm_irq_chip {
 	struct device		*dev;
 	spinlock_t		pm_irq_lock;
-	unsigned int		devirq;
-	unsigned int		irq_base;
+	struct irq_domain	*domain;
 	unsigned int		num_irqs;
 	unsigned int		num_blocks;
 	unsigned int		num_masters;
@@ -138,7 +137,7 @@ static int pm8xxx_irq_block_handler(struct pm_irq_chip *chip, int block)
 	for (i = 0; i < 8; i++) {
 		if (bits & (1 << i)) {
 			pmirq = block * 8 + i;
-			irq = pmirq + chip->irq_base;
+			irq = irq_find_mapping(chip->domain, pmirq);
 			generic_handle_irq(irq);
 		}
 	}
@@ -197,12 +196,11 @@ static void pm8xxx_irq_handler(unsigned int irq, struct irq_desc *desc)
 static void pm8xxx_irq_mask_ack(struct irq_data *d)
 {
 	struct pm_irq_chip *chip = irq_data_get_irq_chip_data(d);
-	unsigned int pmirq = d->irq - chip->irq_base;
-	int	master, irq_bit;
+	unsigned int pmirq = irqd_to_hwirq(d);
+	int	irq_bit;
 	u8	block, config;
 
 	block = pmirq / 8;
-	master = block / 8;
 	irq_bit = pmirq % 8;
 
 	config = chip->config[pmirq] | PM_IRQF_MASK_ALL | PM_IRQF_CLR;
@@ -212,12 +210,11 @@ static void pm8xxx_irq_mask_ack(struct irq_data *d)
 static void pm8xxx_irq_unmask(struct irq_data *d)
 {
 	struct pm_irq_chip *chip = irq_data_get_irq_chip_data(d);
-	unsigned int pmirq = d->irq - chip->irq_base;
-	int	master, irq_bit;
+	unsigned int pmirq = irqd_to_hwirq(d);
+	int	irq_bit;
 	u8	block, config;
 
 	block = pmirq / 8;
-	master = block / 8;
 	irq_bit = pmirq % 8;
 
 	config = chip->config[pmirq];
@@ -227,12 +224,11 @@ static void pm8xxx_irq_unmask(struct irq_data *d)
 static int pm8xxx_irq_set_type(struct irq_data *d, unsigned int flow_type)
 {
 	struct pm_irq_chip *chip = irq_data_get_irq_chip_data(d);
-	unsigned int pmirq = d->irq - chip->irq_base;
-	int master, irq_bit;
+	unsigned int pmirq = irqd_to_hwirq(d);
+	int irq_bit;
 	u8 block, config;
 
 	block = pmirq / 8;
-	master = block / 8;
 	irq_bit  = pmirq % 8;
 
 	chip->config[pmirq] = (irq_bit << PM_IRQF_BITS_SHIFT)
@@ -282,17 +278,14 @@ static struct irq_chip pm8xxx_irq_chip = {
  * RETURNS:
  * an int indicating the value read on that line
  */
-int pm8xxx_get_irq_stat(struct pm_irq_chip *chip, int irq)
+static int pm8xxx_get_irq_stat(struct pm_irq_chip *chip, int irq)
 {
 	int pmirq, rc;
 	u8  block, bits, bit;
 	unsigned long flags;
+	struct irq_data *irq_data = irq_get_irq_data(irq);
 
-	if (chip == NULL || irq < chip->irq_base ||
-			irq >= chip->irq_base + chip->num_irqs)
-		return -EINVAL;
-
-	pmirq = irq - chip->irq_base;
+	pmirq = irq_data->hwirq;
 
 	block = pmirq / 8;
 	bit = pmirq % 8;
@@ -322,64 +315,55 @@ bail_out:
 }
 EXPORT_SYMBOL_GPL(pm8xxx_get_irq_stat);
 
-struct pm_irq_chip *  pm8xxx_irq_init(struct device *dev,
-				const struct pm8xxx_irq_platform_data *pdata)
+static struct lock_class_key pm8xxx_irq_lock_class;
+
+static int pm8xxx_irq_domain_map(struct irq_domain *d, unsigned int irq,
+				   irq_hw_number_t hwirq)
 {
-	struct pm_irq_chip  *chip;
-	int devirq, rc;
-	unsigned int pmirq;
+	struct pm_irq_chip *chip = d->host_data;
 
-	if (!pdata) {
-		pr_err("No platform data\n");
-		return ERR_PTR(-EINVAL);
-	}
+	irq_set_lockdep_class(irq, &pm8xxx_irq_lock_class);
+	irq_set_chip_and_handler(irq, &pm8xxx_irq_chip, handle_level_irq);
+	irq_set_chip_data(irq, chip);
+#ifdef CONFIG_ARM
+	set_irq_flags(irq, IRQF_VALID);
+#else
+	irq_set_noprobe(irq);
+#endif
+	return 0;
+}
 
-	devirq = pdata->devirq;
-	if (devirq < 0) {
-		pr_err("missing devirq\n");
-		rc = devirq;
-		return ERR_PTR(-EINVAL);
-	}
+static const struct irq_domain_ops pm8xxx_irq_domain_ops = {
+	.xlate = irq_domain_xlate_twocell,
+	.map = pm8xxx_irq_domain_map,
+};
 
-	chip = kzalloc(sizeof(struct pm_irq_chip)
-			+ sizeof(u8) * pdata->irq_cdata.nirqs, GFP_KERNEL);
-	if (!chip) {
-		pr_err("Cannot alloc pm_irq_chip struct\n");
-		return ERR_PTR(-EINVAL);
-	}
+static int pm8xxx_irq_init(struct platform_device *pdev, unsigned int irq,
+			   unsigned int nirqs)
+{
+	struct pm_irq_chip  *chip;
+
+	chip = devm_kzalloc(&pdev->dev, sizeof(*chip) + sizeof(u8) * nirqs,
+			    GFP_KERNEL);
+	if (!chip)
+		return -ENOMEM;
 
-	chip->dev = dev;
-	chip->devirq = devirq;
-	chip->irq_base = pdata->irq_base;
-	chip->num_irqs = pdata->irq_cdata.nirqs;
+	chip->dev = &pdev->dev;
+	chip->num_irqs = nirqs;
 	chip->num_blocks = DIV_ROUND_UP(chip->num_irqs, 8);
 	chip->num_masters = DIV_ROUND_UP(chip->num_blocks, 8);
 	spin_lock_init(&chip->pm_irq_lock);
 
-	for (pmirq = 0; pmirq < chip->num_irqs; pmirq++) {
-		irq_set_chip_and_handler(chip->irq_base + pmirq,
-				&pm8xxx_irq_chip,
-				handle_level_irq);
-		irq_set_chip_data(chip->irq_base + pmirq, chip);
-#ifdef CONFIG_ARM
-		set_irq_flags(chip->irq_base + pmirq, IRQF_VALID);
-#else
-		irq_set_noprobe(chip->irq_base + pmirq);
-#endif
-	}
+	chip->domain = irq_domain_add_linear(pdev->dev.of_node, nirqs,
+						&pm8xxx_irq_domain_ops,
+						chip);
+	if (!chip->domain)
+		return -ENODEV;
 
-	irq_set_irq_type(devirq, pdata->irq_trigger_flag);
-	irq_set_handler_data(devirq, chip);
-	irq_set_chained_handler(devirq, pm8xxx_irq_handler);
-	irq_set_irq_wake(devirq, 1);
+	irq_set_handler_data(irq, chip);
+	irq_set_chained_handler(irq, pm8xxx_irq_handler);
+	irq_set_irq_wake(irq, 1);
 
-	return chip;
-}
-
-int pm8xxx_irq_exit(struct pm_irq_chip *chip)
-{
-	irq_set_chained_handler(chip->devirq, NULL);
-	kfree(chip);
 	return 0;
 }
 
@@ -433,42 +417,18 @@ static struct pm8xxx_drvdata pm8921_drvdata = {
 	.pmic_read_irq_stat	= pm8921_read_irq_stat,
 };
 
-static int pm8921_add_subdevices(const struct pm8921_platform_data
-					   *pdata,
-					   struct pm8921 *pmic,
-					   u32 rev)
-{
-	int ret = 0, irq_base = 0;
-	struct pm_irq_chip *irq_chip;
-
-	if (pdata->irq_pdata) {
-		pdata->irq_pdata->irq_cdata.nirqs = PM8921_NR_IRQS;
-		pdata->irq_pdata->irq_cdata.rev = rev;
-		irq_base = pdata->irq_pdata->irq_base;
-		irq_chip = pm8xxx_irq_init(pmic->dev, pdata->irq_pdata);
-
-		if (IS_ERR(irq_chip)) {
-			pr_err("Failed to init interrupts ret=%ld\n",
-					PTR_ERR(irq_chip));
-			return PTR_ERR(irq_chip);
-		}
-		pmic->irq_chip = irq_chip;
-	}
-	return ret;
-}
-
 static int pm8921_probe(struct platform_device *pdev)
 {
-	const struct pm8921_platform_data *pdata = dev_get_platdata(&pdev->dev);
 	struct pm8921 *pmic;
 	int rc;
 	u8 val;
+	unsigned int irq;
 	u32 rev;
 
-	if (!pdata) {
-		pr_err("missing platform data\n");
-		return -EINVAL;
-	}
+
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0)
+		return irq;
 
 	pmic = devm_kzalloc(&pdev->dev, sizeof(struct pm8921), GFP_KERNEL);
 	if (!pmic) {
@@ -499,37 +459,23 @@ static int pm8921_probe(struct platform_device *pdev)
 	pm8921_drvdata.pm_chip_data = pmic;
 	platform_set_drvdata(pdev, &pm8921_drvdata);
 
-	rc = pm8921_add_subdevices(pdata, pmic, rev);
-	if (rc) {
-		pr_err("Cannot add subdevices rc=%d\n", rc);
-		goto err;
-	}
+	rc = pm8xxx_irq_init(pdev, irq, 256);
+	if (rc)
+		return rc;
 
-	/* gpio might not work if no irq device is found */
-	WARN_ON(pmic->irq_chip == NULL);
+	return of_platform_populate(pdev->dev.of_node, NULL, NULL, &pdev->dev);
+}
 
+static int pm8921_remove_child(struct device *dev, void *unused)
+{
+	platform_device_unregister(to_platform_device(dev));
 	return 0;
-
-err:
-	mfd_remove_devices(pmic->dev);
-	return rc;
 }
 
 static int pm8921_remove(struct platform_device *pdev)
 {
-	struct pm8xxx_drvdata *drvdata;
-	struct pm8921 *pmic = NULL;
-
-	drvdata = platform_get_drvdata(pdev);
-	if (drvdata)
-		pmic = drvdata->pm_chip_data;
-	if (pmic)
-		mfd_remove_devices(pmic->dev);
-	if (pmic->irq_chip) {
-		pm8xxx_irq_exit(pmic->irq_chip);
-		pmic->irq_chip = NULL;
-	}
-
+	device_for_each_child(&pdev->dev, NULL, pm8921_remove_child);
+	irq_set_chained_handler(platform_get_irq(pdev, 0), NULL);
 	return 0;
 }
 
diff --git a/include/linux/mfd/pm8xxx/irq.h b/include/linux/mfd/pm8xxx/irq.h
deleted file mode 100644
index f83d6b4..0000000
--- a/include/linux/mfd/pm8xxx/irq.h
+++ /dev/null
@@ -1,59 +0,0 @@
-/*
- * Copyright (c) 2011, Code Aurora Forum. All rights reserved.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 and
- * only version 2 as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- */
-/*
- * Qualcomm PMIC irq 8xxx driver header file
- *
- */
-
-#ifndef __MFD_PM8XXX_IRQ_H
-#define __MFD_PM8XXX_IRQ_H
-
-#include <linux/errno.h>
-#include <linux/err.h>
-
-struct pm8xxx_irq_core_data {
-	u32		rev;
-	int		nirqs;
-};
-
-struct pm8xxx_irq_platform_data {
-	int				irq_base;
-	struct pm8xxx_irq_core_data	irq_cdata;
-	int				devirq;
-	int				irq_trigger_flag;
-};
-
-struct pm_irq_chip;
-
-#ifdef CONFIG_MFD_PM8XXX_IRQ
-int pm8xxx_get_irq_stat(struct pm_irq_chip *chip, int irq);
-struct pm_irq_chip *pm8xxx_irq_init(struct device *dev,
-				const struct pm8xxx_irq_platform_data *pdata);
-int pm8xxx_irq_exit(struct pm_irq_chip *chip);
-#else
-static inline int pm8xxx_get_irq_stat(struct pm_irq_chip *chip, int irq)
-{
-	return -ENXIO;
-}
-static inline struct pm_irq_chip *pm8xxx_irq_init(
-				const struct device *dev,
-				const struct pm8xxx_irq_platform_data *pdata)
-{
-	return ERR_PTR(-ENXIO);
-}
-static inline int pm8xxx_irq_exit(struct pm_irq_chip *chip)
-{
-	return -ENXIO;
-}
-#endif /* CONFIG_MFD_PM8XXX_IRQ */
-#endif /* __MFD_PM8XXX_IRQ_H */
diff --git a/include/linux/mfd/pm8xxx/pm8921.h b/include/linux/mfd/pm8xxx/pm8921.h
deleted file mode 100644
index 00fa3de..0000000
--- a/include/linux/mfd/pm8xxx/pm8921.h
+++ /dev/null
@@ -1,30 +0,0 @@
-/*
- * Copyright (c) 2011, Code Aurora Forum. All rights reserved.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 and
- * only version 2 as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- */
-/*
- * Qualcomm PMIC 8921 driver header file
- *
- */
-
-#ifndef __MFD_PM8921_H
-#define __MFD_PM8921_H
-
-#include <linux/mfd/pm8xxx/irq.h>
-
-#define PM8921_NR_IRQS		256
-
-struct pm8921_platform_data {
-	int					irq_base;
-	struct pm8xxx_irq_platform_data		*irq_pdata;
-};
-
-#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] 77+ messages in thread

* [PATCH 7/8] mfd: pm8921: Migrate to irqdomains
@ 2013-12-10 23:35   ` Stephen Boyd
  0 siblings, 0 replies; 77+ messages in thread
From: Stephen Boyd @ 2013-12-10 23:35 UTC (permalink / raw)
  To: linux-arm-kernel

Convert this driver to use irqdomains so that the PMIC's child
devices can be converted to devicetree.

Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 drivers/mfd/pm8921-core.c         | 186 ++++++++++++++------------------------
 include/linux/mfd/pm8xxx/irq.h    |  59 ------------
 include/linux/mfd/pm8xxx/pm8921.h |  30 ------
 3 files changed, 66 insertions(+), 209 deletions(-)
 delete mode 100644 include/linux/mfd/pm8xxx/irq.h
 delete mode 100644 include/linux/mfd/pm8xxx/pm8921.h

diff --git a/drivers/mfd/pm8921-core.c b/drivers/mfd/pm8921-core.c
index 083fab2..7ea6c7a 100644
--- a/drivers/mfd/pm8921-core.c
+++ b/drivers/mfd/pm8921-core.c
@@ -17,15 +17,15 @@
 #include <linux/interrupt.h>
 #include <linux/irqchip/chained_irq.h>
 #include <linux/irq.h>
+#include <linux/irqdomain.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
 #include <linux/err.h>
 #include <linux/ssbi.h>
+#include <linux/of_platform.h>
 #include <linux/mfd/core.h>
-#include <linux/mfd/pm8xxx/pm8921.h>
 #include <linux/mfd/pm8xxx/core.h>
-#include <linux/mfd/pm8xxx/irq.h>
 
 #define	SSBI_REG_ADDR_IRQ_BASE		0x1BB
 
@@ -61,8 +61,7 @@ struct pm8921 {
 struct pm_irq_chip {
 	struct device		*dev;
 	spinlock_t		pm_irq_lock;
-	unsigned int		devirq;
-	unsigned int		irq_base;
+	struct irq_domain	*domain;
 	unsigned int		num_irqs;
 	unsigned int		num_blocks;
 	unsigned int		num_masters;
@@ -138,7 +137,7 @@ static int pm8xxx_irq_block_handler(struct pm_irq_chip *chip, int block)
 	for (i = 0; i < 8; i++) {
 		if (bits & (1 << i)) {
 			pmirq = block * 8 + i;
-			irq = pmirq + chip->irq_base;
+			irq = irq_find_mapping(chip->domain, pmirq);
 			generic_handle_irq(irq);
 		}
 	}
@@ -197,12 +196,11 @@ static void pm8xxx_irq_handler(unsigned int irq, struct irq_desc *desc)
 static void pm8xxx_irq_mask_ack(struct irq_data *d)
 {
 	struct pm_irq_chip *chip = irq_data_get_irq_chip_data(d);
-	unsigned int pmirq = d->irq - chip->irq_base;
-	int	master, irq_bit;
+	unsigned int pmirq = irqd_to_hwirq(d);
+	int	irq_bit;
 	u8	block, config;
 
 	block = pmirq / 8;
-	master = block / 8;
 	irq_bit = pmirq % 8;
 
 	config = chip->config[pmirq] | PM_IRQF_MASK_ALL | PM_IRQF_CLR;
@@ -212,12 +210,11 @@ static void pm8xxx_irq_mask_ack(struct irq_data *d)
 static void pm8xxx_irq_unmask(struct irq_data *d)
 {
 	struct pm_irq_chip *chip = irq_data_get_irq_chip_data(d);
-	unsigned int pmirq = d->irq - chip->irq_base;
-	int	master, irq_bit;
+	unsigned int pmirq = irqd_to_hwirq(d);
+	int	irq_bit;
 	u8	block, config;
 
 	block = pmirq / 8;
-	master = block / 8;
 	irq_bit = pmirq % 8;
 
 	config = chip->config[pmirq];
@@ -227,12 +224,11 @@ static void pm8xxx_irq_unmask(struct irq_data *d)
 static int pm8xxx_irq_set_type(struct irq_data *d, unsigned int flow_type)
 {
 	struct pm_irq_chip *chip = irq_data_get_irq_chip_data(d);
-	unsigned int pmirq = d->irq - chip->irq_base;
-	int master, irq_bit;
+	unsigned int pmirq = irqd_to_hwirq(d);
+	int irq_bit;
 	u8 block, config;
 
 	block = pmirq / 8;
-	master = block / 8;
 	irq_bit  = pmirq % 8;
 
 	chip->config[pmirq] = (irq_bit << PM_IRQF_BITS_SHIFT)
@@ -282,17 +278,14 @@ static struct irq_chip pm8xxx_irq_chip = {
  * RETURNS:
  * an int indicating the value read on that line
  */
-int pm8xxx_get_irq_stat(struct pm_irq_chip *chip, int irq)
+static int pm8xxx_get_irq_stat(struct pm_irq_chip *chip, int irq)
 {
 	int pmirq, rc;
 	u8  block, bits, bit;
 	unsigned long flags;
+	struct irq_data *irq_data = irq_get_irq_data(irq);
 
-	if (chip == NULL || irq < chip->irq_base ||
-			irq >= chip->irq_base + chip->num_irqs)
-		return -EINVAL;
-
-	pmirq = irq - chip->irq_base;
+	pmirq = irq_data->hwirq;
 
 	block = pmirq / 8;
 	bit = pmirq % 8;
@@ -322,64 +315,55 @@ bail_out:
 }
 EXPORT_SYMBOL_GPL(pm8xxx_get_irq_stat);
 
-struct pm_irq_chip *  pm8xxx_irq_init(struct device *dev,
-				const struct pm8xxx_irq_platform_data *pdata)
+static struct lock_class_key pm8xxx_irq_lock_class;
+
+static int pm8xxx_irq_domain_map(struct irq_domain *d, unsigned int irq,
+				   irq_hw_number_t hwirq)
 {
-	struct pm_irq_chip  *chip;
-	int devirq, rc;
-	unsigned int pmirq;
+	struct pm_irq_chip *chip = d->host_data;
 
-	if (!pdata) {
-		pr_err("No platform data\n");
-		return ERR_PTR(-EINVAL);
-	}
+	irq_set_lockdep_class(irq, &pm8xxx_irq_lock_class);
+	irq_set_chip_and_handler(irq, &pm8xxx_irq_chip, handle_level_irq);
+	irq_set_chip_data(irq, chip);
+#ifdef CONFIG_ARM
+	set_irq_flags(irq, IRQF_VALID);
+#else
+	irq_set_noprobe(irq);
+#endif
+	return 0;
+}
 
-	devirq = pdata->devirq;
-	if (devirq < 0) {
-		pr_err("missing devirq\n");
-		rc = devirq;
-		return ERR_PTR(-EINVAL);
-	}
+static const struct irq_domain_ops pm8xxx_irq_domain_ops = {
+	.xlate = irq_domain_xlate_twocell,
+	.map = pm8xxx_irq_domain_map,
+};
 
-	chip = kzalloc(sizeof(struct pm_irq_chip)
-			+ sizeof(u8) * pdata->irq_cdata.nirqs, GFP_KERNEL);
-	if (!chip) {
-		pr_err("Cannot alloc pm_irq_chip struct\n");
-		return ERR_PTR(-EINVAL);
-	}
+static int pm8xxx_irq_init(struct platform_device *pdev, unsigned int irq,
+			   unsigned int nirqs)
+{
+	struct pm_irq_chip  *chip;
+
+	chip = devm_kzalloc(&pdev->dev, sizeof(*chip) + sizeof(u8) * nirqs,
+			    GFP_KERNEL);
+	if (!chip)
+		return -ENOMEM;
 
-	chip->dev = dev;
-	chip->devirq = devirq;
-	chip->irq_base = pdata->irq_base;
-	chip->num_irqs = pdata->irq_cdata.nirqs;
+	chip->dev = &pdev->dev;
+	chip->num_irqs = nirqs;
 	chip->num_blocks = DIV_ROUND_UP(chip->num_irqs, 8);
 	chip->num_masters = DIV_ROUND_UP(chip->num_blocks, 8);
 	spin_lock_init(&chip->pm_irq_lock);
 
-	for (pmirq = 0; pmirq < chip->num_irqs; pmirq++) {
-		irq_set_chip_and_handler(chip->irq_base + pmirq,
-				&pm8xxx_irq_chip,
-				handle_level_irq);
-		irq_set_chip_data(chip->irq_base + pmirq, chip);
-#ifdef CONFIG_ARM
-		set_irq_flags(chip->irq_base + pmirq, IRQF_VALID);
-#else
-		irq_set_noprobe(chip->irq_base + pmirq);
-#endif
-	}
+	chip->domain = irq_domain_add_linear(pdev->dev.of_node, nirqs,
+						&pm8xxx_irq_domain_ops,
+						chip);
+	if (!chip->domain)
+		return -ENODEV;
 
-	irq_set_irq_type(devirq, pdata->irq_trigger_flag);
-	irq_set_handler_data(devirq, chip);
-	irq_set_chained_handler(devirq, pm8xxx_irq_handler);
-	irq_set_irq_wake(devirq, 1);
+	irq_set_handler_data(irq, chip);
+	irq_set_chained_handler(irq, pm8xxx_irq_handler);
+	irq_set_irq_wake(irq, 1);
 
-	return chip;
-}
-
-int pm8xxx_irq_exit(struct pm_irq_chip *chip)
-{
-	irq_set_chained_handler(chip->devirq, NULL);
-	kfree(chip);
 	return 0;
 }
 
@@ -433,42 +417,18 @@ static struct pm8xxx_drvdata pm8921_drvdata = {
 	.pmic_read_irq_stat	= pm8921_read_irq_stat,
 };
 
-static int pm8921_add_subdevices(const struct pm8921_platform_data
-					   *pdata,
-					   struct pm8921 *pmic,
-					   u32 rev)
-{
-	int ret = 0, irq_base = 0;
-	struct pm_irq_chip *irq_chip;
-
-	if (pdata->irq_pdata) {
-		pdata->irq_pdata->irq_cdata.nirqs = PM8921_NR_IRQS;
-		pdata->irq_pdata->irq_cdata.rev = rev;
-		irq_base = pdata->irq_pdata->irq_base;
-		irq_chip = pm8xxx_irq_init(pmic->dev, pdata->irq_pdata);
-
-		if (IS_ERR(irq_chip)) {
-			pr_err("Failed to init interrupts ret=%ld\n",
-					PTR_ERR(irq_chip));
-			return PTR_ERR(irq_chip);
-		}
-		pmic->irq_chip = irq_chip;
-	}
-	return ret;
-}
-
 static int pm8921_probe(struct platform_device *pdev)
 {
-	const struct pm8921_platform_data *pdata = dev_get_platdata(&pdev->dev);
 	struct pm8921 *pmic;
 	int rc;
 	u8 val;
+	unsigned int irq;
 	u32 rev;
 
-	if (!pdata) {
-		pr_err("missing platform data\n");
-		return -EINVAL;
-	}
+
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0)
+		return irq;
 
 	pmic = devm_kzalloc(&pdev->dev, sizeof(struct pm8921), GFP_KERNEL);
 	if (!pmic) {
@@ -499,37 +459,23 @@ static int pm8921_probe(struct platform_device *pdev)
 	pm8921_drvdata.pm_chip_data = pmic;
 	platform_set_drvdata(pdev, &pm8921_drvdata);
 
-	rc = pm8921_add_subdevices(pdata, pmic, rev);
-	if (rc) {
-		pr_err("Cannot add subdevices rc=%d\n", rc);
-		goto err;
-	}
+	rc = pm8xxx_irq_init(pdev, irq, 256);
+	if (rc)
+		return rc;
 
-	/* gpio might not work if no irq device is found */
-	WARN_ON(pmic->irq_chip == NULL);
+	return of_platform_populate(pdev->dev.of_node, NULL, NULL, &pdev->dev);
+}
 
+static int pm8921_remove_child(struct device *dev, void *unused)
+{
+	platform_device_unregister(to_platform_device(dev));
 	return 0;
-
-err:
-	mfd_remove_devices(pmic->dev);
-	return rc;
 }
 
 static int pm8921_remove(struct platform_device *pdev)
 {
-	struct pm8xxx_drvdata *drvdata;
-	struct pm8921 *pmic = NULL;
-
-	drvdata = platform_get_drvdata(pdev);
-	if (drvdata)
-		pmic = drvdata->pm_chip_data;
-	if (pmic)
-		mfd_remove_devices(pmic->dev);
-	if (pmic->irq_chip) {
-		pm8xxx_irq_exit(pmic->irq_chip);
-		pmic->irq_chip = NULL;
-	}
-
+	device_for_each_child(&pdev->dev, NULL, pm8921_remove_child);
+	irq_set_chained_handler(platform_get_irq(pdev, 0), NULL);
 	return 0;
 }
 
diff --git a/include/linux/mfd/pm8xxx/irq.h b/include/linux/mfd/pm8xxx/irq.h
deleted file mode 100644
index f83d6b4..0000000
--- a/include/linux/mfd/pm8xxx/irq.h
+++ /dev/null
@@ -1,59 +0,0 @@
-/*
- * Copyright (c) 2011, Code Aurora Forum. All rights reserved.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 and
- * only version 2 as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- */
-/*
- * Qualcomm PMIC irq 8xxx driver header file
- *
- */
-
-#ifndef __MFD_PM8XXX_IRQ_H
-#define __MFD_PM8XXX_IRQ_H
-
-#include <linux/errno.h>
-#include <linux/err.h>
-
-struct pm8xxx_irq_core_data {
-	u32		rev;
-	int		nirqs;
-};
-
-struct pm8xxx_irq_platform_data {
-	int				irq_base;
-	struct pm8xxx_irq_core_data	irq_cdata;
-	int				devirq;
-	int				irq_trigger_flag;
-};
-
-struct pm_irq_chip;
-
-#ifdef CONFIG_MFD_PM8XXX_IRQ
-int pm8xxx_get_irq_stat(struct pm_irq_chip *chip, int irq);
-struct pm_irq_chip *pm8xxx_irq_init(struct device *dev,
-				const struct pm8xxx_irq_platform_data *pdata);
-int pm8xxx_irq_exit(struct pm_irq_chip *chip);
-#else
-static inline int pm8xxx_get_irq_stat(struct pm_irq_chip *chip, int irq)
-{
-	return -ENXIO;
-}
-static inline struct pm_irq_chip *pm8xxx_irq_init(
-				const struct device *dev,
-				const struct pm8xxx_irq_platform_data *pdata)
-{
-	return ERR_PTR(-ENXIO);
-}
-static inline int pm8xxx_irq_exit(struct pm_irq_chip *chip)
-{
-	return -ENXIO;
-}
-#endif /* CONFIG_MFD_PM8XXX_IRQ */
-#endif /* __MFD_PM8XXX_IRQ_H */
diff --git a/include/linux/mfd/pm8xxx/pm8921.h b/include/linux/mfd/pm8xxx/pm8921.h
deleted file mode 100644
index 00fa3de..0000000
--- a/include/linux/mfd/pm8xxx/pm8921.h
+++ /dev/null
@@ -1,30 +0,0 @@
-/*
- * Copyright (c) 2011, Code Aurora Forum. All rights reserved.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 and
- * only version 2 as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- */
-/*
- * Qualcomm PMIC 8921 driver header file
- *
- */
-
-#ifndef __MFD_PM8921_H
-#define __MFD_PM8921_H
-
-#include <linux/mfd/pm8xxx/irq.h>
-
-#define PM8921_NR_IRQS		256
-
-struct pm8921_platform_data {
-	int					irq_base;
-	struct pm8xxx_irq_platform_data		*irq_pdata;
-};
-
-#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] 77+ messages in thread

* [PATCH 8/8] mfd: pm8921: Use ssbi regmap
  2013-12-10 23:35 ` Stephen Boyd
@ 2013-12-10 23:35   ` Stephen Boyd
  -1 siblings, 0 replies; 77+ messages in thread
From: Stephen Boyd @ 2013-12-10 23:35 UTC (permalink / raw)
  To: Samuel Ortiz, Lee Jones; +Cc: linux-kernel, linux-arm-msm, linux-arm-kernel

Use a regmap so that the pm8xxx read/write APIs can be removed
once all consumer drivers are converted.

Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 drivers/mfd/Kconfig       |  1 +
 drivers/mfd/pm8921-core.c | 70 +++++++++++++++++++++++++----------------------
 2 files changed, 39 insertions(+), 32 deletions(-)

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 35007ed..87d8e1b 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -455,6 +455,7 @@ config MFD_PM8921_CORE
 	depends on (ARCH_MSM || HEXAGON)
 	select MFD_CORE
 	select MFD_PM8XXX
+	select REGMAP_SSBI
 	help
 	  If you say yes to this option, support will be included for the
 	  built-in PM8921 PMIC chip.
diff --git a/drivers/mfd/pm8921-core.c b/drivers/mfd/pm8921-core.c
index 7ea6c7a..d1d996e 100644
--- a/drivers/mfd/pm8921-core.c
+++ b/drivers/mfd/pm8921-core.c
@@ -23,6 +23,7 @@
 #include <linux/slab.h>
 #include <linux/err.h>
 #include <linux/ssbi.h>
+#include <linux/regmap.h>
 #include <linux/of_platform.h>
 #include <linux/mfd/core.h>
 #include <linux/mfd/pm8xxx/core.h>
@@ -60,6 +61,7 @@ struct pm8921 {
 
 struct pm_irq_chip {
 	struct device		*dev;
+	struct regmap		*regmap;
 	spinlock_t		pm_irq_lock;
 	struct irq_domain	*domain;
 	unsigned int		num_irqs;
@@ -68,29 +70,19 @@ struct pm_irq_chip {
 	u8			config[0];
 };
 
-static int pm8xxx_read_root_irq(const struct pm_irq_chip *chip, u8 *rp)
-{
-	return pm8xxx_readb(chip->dev, SSBI_REG_ADDR_IRQ_ROOT, rp);
-}
-
-static int pm8xxx_read_master_irq(const struct pm_irq_chip *chip, u8 m, u8 *bp)
-{
-	return pm8xxx_readb(chip->dev,
-			SSBI_REG_ADDR_IRQ_M_STATUS1 + m, bp);
-}
-
-static int pm8xxx_read_block_irq(struct pm_irq_chip *chip, u8 bp, u8 *ip)
+static int pm8xxx_read_block_irq(struct pm_irq_chip *chip, unsigned int bp,
+				 unsigned int *ip)
 {
 	int	rc;
 
 	spin_lock(&chip->pm_irq_lock);
-	rc = pm8xxx_writeb(chip->dev, SSBI_REG_ADDR_IRQ_BLK_SEL, bp);
+	rc = regmap_write(chip->regmap, SSBI_REG_ADDR_IRQ_BLK_SEL, bp);
 	if (rc) {
 		pr_err("Failed Selecting Block %d rc=%d\n", bp, rc);
 		goto bail;
 	}
 
-	rc = pm8xxx_readb(chip->dev, SSBI_REG_ADDR_IRQ_IT_STATUS, ip);
+	rc = regmap_read(chip->regmap, SSBI_REG_ADDR_IRQ_IT_STATUS, ip);
 	if (rc)
 		pr_err("Failed Reading Status rc=%d\n", rc);
 bail:
@@ -98,19 +90,20 @@ bail:
 	return rc;
 }
 
-static int pm8xxx_config_irq(struct pm_irq_chip *chip, u8 bp, u8 cp)
+static int
+pm8xxx_config_irq(struct pm_irq_chip *chip, unsigned int bp, unsigned int cp)
 {
 	int	rc;
 
 	spin_lock(&chip->pm_irq_lock);
-	rc = pm8xxx_writeb(chip->dev, SSBI_REG_ADDR_IRQ_BLK_SEL, bp);
+	rc = regmap_write(chip->regmap, SSBI_REG_ADDR_IRQ_BLK_SEL, bp);
 	if (rc) {
 		pr_err("Failed Selecting Block %d rc=%d\n", bp, rc);
 		goto bail;
 	}
 
 	cp |= PM_IRQF_WRITE;
-	rc = pm8xxx_writeb(chip->dev, SSBI_REG_ADDR_IRQ_CONFIG, cp);
+	rc = regmap_write(chip->regmap, SSBI_REG_ADDR_IRQ_CONFIG, cp);
 	if (rc)
 		pr_err("Failed Configuring IRQ rc=%d\n", rc);
 bail:
@@ -121,7 +114,7 @@ bail:
 static int pm8xxx_irq_block_handler(struct pm_irq_chip *chip, int block)
 {
 	int pmirq, irq, i, ret = 0;
-	u8 bits;
+	unsigned int bits;
 
 	ret = pm8xxx_read_block_irq(chip, block, &bits);
 	if (ret) {
@@ -146,10 +139,11 @@ static int pm8xxx_irq_block_handler(struct pm_irq_chip *chip, int block)
 
 static int pm8xxx_irq_master_handler(struct pm_irq_chip *chip, int master)
 {
-	u8 blockbits;
+	unsigned int blockbits;
 	int block_number, i, ret = 0;
 
-	ret = pm8xxx_read_master_irq(chip, master, &blockbits);
+	ret = regmap_read(chip->regmap, SSBI_REG_ADDR_IRQ_M_STATUS1 + master,
+			  &blockbits);
 	if (ret) {
 		pr_err("Failed to read master %d ret=%d\n", master, ret);
 		return ret;
@@ -171,12 +165,12 @@ static void pm8xxx_irq_handler(unsigned int irq, struct irq_desc *desc)
 {
 	struct pm_irq_chip *chip = irq_desc_get_handler_data(desc);
 	struct irq_chip *irq_chip = irq_desc_get_chip(desc);
-	u8	root;
+	unsigned int root;
 	int	i, ret, masters = 0;
 
 	chained_irq_enter(irq_chip, desc);
 
-	ret = pm8xxx_read_root_irq(chip, &root);
+	ret = regmap_read(chip->regmap, SSBI_REG_ADDR_IRQ_ROOT, &root);
 	if (ret) {
 		pr_err("Can't read root status ret=%d\n", ret);
 		return;
@@ -281,7 +275,7 @@ static struct irq_chip pm8xxx_irq_chip = {
 static int pm8xxx_get_irq_stat(struct pm_irq_chip *chip, int irq)
 {
 	int pmirq, rc;
-	u8  block, bits, bit;
+	unsigned int  block, bits, bit;
 	unsigned long flags;
 	struct irq_data *irq_data = irq_get_irq_data(irq);
 
@@ -292,14 +286,14 @@ static int pm8xxx_get_irq_stat(struct pm_irq_chip *chip, int irq)
 
 	spin_lock_irqsave(&chip->pm_irq_lock, flags);
 
-	rc = pm8xxx_writeb(chip->dev, SSBI_REG_ADDR_IRQ_BLK_SEL, block);
+	rc = regmap_write(chip->regmap, SSBI_REG_ADDR_IRQ_BLK_SEL, block);
 	if (rc) {
 		pr_err("Failed Selecting block irq=%d pmirq=%d blk=%d rc=%d\n",
 			irq, pmirq, block, rc);
 		goto bail_out;
 	}
 
-	rc = pm8xxx_readb(chip->dev, SSBI_REG_ADDR_IRQ_RT_STATUS, &bits);
+	rc = regmap_read(chip->regmap, SSBI_REG_ADDR_IRQ_RT_STATUS, &bits);
 	if (rc) {
 		pr_err("Failed Configuring irq=%d pmirq=%d blk=%d rc=%d\n",
 			irq, pmirq, block, rc);
@@ -338,8 +332,8 @@ static const struct irq_domain_ops pm8xxx_irq_domain_ops = {
 	.map = pm8xxx_irq_domain_map,
 };
 
-static int pm8xxx_irq_init(struct platform_device *pdev, unsigned int irq,
-			   unsigned int nirqs)
+static int pm8xxx_irq_init(struct platform_device *pdev, struct regmap *regmap,
+			   unsigned int irq, unsigned int nirqs)
 {
 	struct pm_irq_chip  *chip;
 
@@ -348,7 +342,7 @@ static int pm8xxx_irq_init(struct platform_device *pdev, unsigned int irq,
 	if (!chip)
 		return -ENOMEM;
 
-	chip->dev = &pdev->dev;
+	chip->regmap = regmap;
 	chip->num_irqs = nirqs;
 	chip->num_blocks = DIV_ROUND_UP(chip->num_irqs, 8);
 	chip->num_masters = DIV_ROUND_UP(chip->num_blocks, 8);
@@ -417,11 +411,19 @@ static struct pm8xxx_drvdata pm8921_drvdata = {
 	.pmic_read_irq_stat	= pm8921_read_irq_stat,
 };
 
+static const struct regmap_config ssbi_regmap_config = {
+	.reg_bits = 16,
+	.val_bits = 8,
+	.max_register = 0x3ff,
+	.fast_io = true,
+};
+
 static int pm8921_probe(struct platform_device *pdev)
 {
 	struct pm8921 *pmic;
+	struct regmap *regmap;
 	int rc;
-	u8 val;
+	unsigned int val;
 	unsigned int irq;
 	u32 rev;
 
@@ -436,8 +438,12 @@ static int pm8921_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
+	regmap = devm_regmap_init_ssbi(&pdev->dev, &ssbi_regmap_config);
+	if (IS_ERR(regmap))
+		return PTR_ERR(regmap);
+
 	/* Read PMIC chip revision */
-	rc = ssbi_read(pdev->dev.parent, REG_HWREV, &val, sizeof(val));
+	rc = regmap_read(regmap, REG_HWREV, &val);
 	if (rc) {
 		pr_err("Failed to read hw rev reg %d:rc=%d\n", REG_HWREV, rc);
 		return rc;
@@ -446,7 +452,7 @@ static int pm8921_probe(struct platform_device *pdev)
 	rev = val;
 
 	/* Read PMIC chip revision 2 */
-	rc = ssbi_read(pdev->dev.parent, REG_HWREV_2, &val, sizeof(val));
+	rc = regmap_read(regmap, REG_HWREV_2, &val);
 	if (rc) {
 		pr_err("Failed to read hw rev 2 reg %d:rc=%d\n",
 			REG_HWREV_2, rc);
@@ -459,7 +465,7 @@ static int pm8921_probe(struct platform_device *pdev)
 	pm8921_drvdata.pm_chip_data = pmic;
 	platform_set_drvdata(pdev, &pm8921_drvdata);
 
-	rc = pm8xxx_irq_init(pdev, irq, 256);
+	rc = pm8xxx_irq_init(pdev, regmap, irq, 256);
 	if (rc)
 		return rc;
 
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

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

* [PATCH 8/8] mfd: pm8921: Use ssbi regmap
@ 2013-12-10 23:35   ` Stephen Boyd
  0 siblings, 0 replies; 77+ messages in thread
From: Stephen Boyd @ 2013-12-10 23:35 UTC (permalink / raw)
  To: linux-arm-kernel

Use a regmap so that the pm8xxx read/write APIs can be removed
once all consumer drivers are converted.

Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 drivers/mfd/Kconfig       |  1 +
 drivers/mfd/pm8921-core.c | 70 +++++++++++++++++++++++++----------------------
 2 files changed, 39 insertions(+), 32 deletions(-)

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 35007ed..87d8e1b 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -455,6 +455,7 @@ config MFD_PM8921_CORE
 	depends on (ARCH_MSM || HEXAGON)
 	select MFD_CORE
 	select MFD_PM8XXX
+	select REGMAP_SSBI
 	help
 	  If you say yes to this option, support will be included for the
 	  built-in PM8921 PMIC chip.
diff --git a/drivers/mfd/pm8921-core.c b/drivers/mfd/pm8921-core.c
index 7ea6c7a..d1d996e 100644
--- a/drivers/mfd/pm8921-core.c
+++ b/drivers/mfd/pm8921-core.c
@@ -23,6 +23,7 @@
 #include <linux/slab.h>
 #include <linux/err.h>
 #include <linux/ssbi.h>
+#include <linux/regmap.h>
 #include <linux/of_platform.h>
 #include <linux/mfd/core.h>
 #include <linux/mfd/pm8xxx/core.h>
@@ -60,6 +61,7 @@ struct pm8921 {
 
 struct pm_irq_chip {
 	struct device		*dev;
+	struct regmap		*regmap;
 	spinlock_t		pm_irq_lock;
 	struct irq_domain	*domain;
 	unsigned int		num_irqs;
@@ -68,29 +70,19 @@ struct pm_irq_chip {
 	u8			config[0];
 };
 
-static int pm8xxx_read_root_irq(const struct pm_irq_chip *chip, u8 *rp)
-{
-	return pm8xxx_readb(chip->dev, SSBI_REG_ADDR_IRQ_ROOT, rp);
-}
-
-static int pm8xxx_read_master_irq(const struct pm_irq_chip *chip, u8 m, u8 *bp)
-{
-	return pm8xxx_readb(chip->dev,
-			SSBI_REG_ADDR_IRQ_M_STATUS1 + m, bp);
-}
-
-static int pm8xxx_read_block_irq(struct pm_irq_chip *chip, u8 bp, u8 *ip)
+static int pm8xxx_read_block_irq(struct pm_irq_chip *chip, unsigned int bp,
+				 unsigned int *ip)
 {
 	int	rc;
 
 	spin_lock(&chip->pm_irq_lock);
-	rc = pm8xxx_writeb(chip->dev, SSBI_REG_ADDR_IRQ_BLK_SEL, bp);
+	rc = regmap_write(chip->regmap, SSBI_REG_ADDR_IRQ_BLK_SEL, bp);
 	if (rc) {
 		pr_err("Failed Selecting Block %d rc=%d\n", bp, rc);
 		goto bail;
 	}
 
-	rc = pm8xxx_readb(chip->dev, SSBI_REG_ADDR_IRQ_IT_STATUS, ip);
+	rc = regmap_read(chip->regmap, SSBI_REG_ADDR_IRQ_IT_STATUS, ip);
 	if (rc)
 		pr_err("Failed Reading Status rc=%d\n", rc);
 bail:
@@ -98,19 +90,20 @@ bail:
 	return rc;
 }
 
-static int pm8xxx_config_irq(struct pm_irq_chip *chip, u8 bp, u8 cp)
+static int
+pm8xxx_config_irq(struct pm_irq_chip *chip, unsigned int bp, unsigned int cp)
 {
 	int	rc;
 
 	spin_lock(&chip->pm_irq_lock);
-	rc = pm8xxx_writeb(chip->dev, SSBI_REG_ADDR_IRQ_BLK_SEL, bp);
+	rc = regmap_write(chip->regmap, SSBI_REG_ADDR_IRQ_BLK_SEL, bp);
 	if (rc) {
 		pr_err("Failed Selecting Block %d rc=%d\n", bp, rc);
 		goto bail;
 	}
 
 	cp |= PM_IRQF_WRITE;
-	rc = pm8xxx_writeb(chip->dev, SSBI_REG_ADDR_IRQ_CONFIG, cp);
+	rc = regmap_write(chip->regmap, SSBI_REG_ADDR_IRQ_CONFIG, cp);
 	if (rc)
 		pr_err("Failed Configuring IRQ rc=%d\n", rc);
 bail:
@@ -121,7 +114,7 @@ bail:
 static int pm8xxx_irq_block_handler(struct pm_irq_chip *chip, int block)
 {
 	int pmirq, irq, i, ret = 0;
-	u8 bits;
+	unsigned int bits;
 
 	ret = pm8xxx_read_block_irq(chip, block, &bits);
 	if (ret) {
@@ -146,10 +139,11 @@ static int pm8xxx_irq_block_handler(struct pm_irq_chip *chip, int block)
 
 static int pm8xxx_irq_master_handler(struct pm_irq_chip *chip, int master)
 {
-	u8 blockbits;
+	unsigned int blockbits;
 	int block_number, i, ret = 0;
 
-	ret = pm8xxx_read_master_irq(chip, master, &blockbits);
+	ret = regmap_read(chip->regmap, SSBI_REG_ADDR_IRQ_M_STATUS1 + master,
+			  &blockbits);
 	if (ret) {
 		pr_err("Failed to read master %d ret=%d\n", master, ret);
 		return ret;
@@ -171,12 +165,12 @@ static void pm8xxx_irq_handler(unsigned int irq, struct irq_desc *desc)
 {
 	struct pm_irq_chip *chip = irq_desc_get_handler_data(desc);
 	struct irq_chip *irq_chip = irq_desc_get_chip(desc);
-	u8	root;
+	unsigned int root;
 	int	i, ret, masters = 0;
 
 	chained_irq_enter(irq_chip, desc);
 
-	ret = pm8xxx_read_root_irq(chip, &root);
+	ret = regmap_read(chip->regmap, SSBI_REG_ADDR_IRQ_ROOT, &root);
 	if (ret) {
 		pr_err("Can't read root status ret=%d\n", ret);
 		return;
@@ -281,7 +275,7 @@ static struct irq_chip pm8xxx_irq_chip = {
 static int pm8xxx_get_irq_stat(struct pm_irq_chip *chip, int irq)
 {
 	int pmirq, rc;
-	u8  block, bits, bit;
+	unsigned int  block, bits, bit;
 	unsigned long flags;
 	struct irq_data *irq_data = irq_get_irq_data(irq);
 
@@ -292,14 +286,14 @@ static int pm8xxx_get_irq_stat(struct pm_irq_chip *chip, int irq)
 
 	spin_lock_irqsave(&chip->pm_irq_lock, flags);
 
-	rc = pm8xxx_writeb(chip->dev, SSBI_REG_ADDR_IRQ_BLK_SEL, block);
+	rc = regmap_write(chip->regmap, SSBI_REG_ADDR_IRQ_BLK_SEL, block);
 	if (rc) {
 		pr_err("Failed Selecting block irq=%d pmirq=%d blk=%d rc=%d\n",
 			irq, pmirq, block, rc);
 		goto bail_out;
 	}
 
-	rc = pm8xxx_readb(chip->dev, SSBI_REG_ADDR_IRQ_RT_STATUS, &bits);
+	rc = regmap_read(chip->regmap, SSBI_REG_ADDR_IRQ_RT_STATUS, &bits);
 	if (rc) {
 		pr_err("Failed Configuring irq=%d pmirq=%d blk=%d rc=%d\n",
 			irq, pmirq, block, rc);
@@ -338,8 +332,8 @@ static const struct irq_domain_ops pm8xxx_irq_domain_ops = {
 	.map = pm8xxx_irq_domain_map,
 };
 
-static int pm8xxx_irq_init(struct platform_device *pdev, unsigned int irq,
-			   unsigned int nirqs)
+static int pm8xxx_irq_init(struct platform_device *pdev, struct regmap *regmap,
+			   unsigned int irq, unsigned int nirqs)
 {
 	struct pm_irq_chip  *chip;
 
@@ -348,7 +342,7 @@ static int pm8xxx_irq_init(struct platform_device *pdev, unsigned int irq,
 	if (!chip)
 		return -ENOMEM;
 
-	chip->dev = &pdev->dev;
+	chip->regmap = regmap;
 	chip->num_irqs = nirqs;
 	chip->num_blocks = DIV_ROUND_UP(chip->num_irqs, 8);
 	chip->num_masters = DIV_ROUND_UP(chip->num_blocks, 8);
@@ -417,11 +411,19 @@ static struct pm8xxx_drvdata pm8921_drvdata = {
 	.pmic_read_irq_stat	= pm8921_read_irq_stat,
 };
 
+static const struct regmap_config ssbi_regmap_config = {
+	.reg_bits = 16,
+	.val_bits = 8,
+	.max_register = 0x3ff,
+	.fast_io = true,
+};
+
 static int pm8921_probe(struct platform_device *pdev)
 {
 	struct pm8921 *pmic;
+	struct regmap *regmap;
 	int rc;
-	u8 val;
+	unsigned int val;
 	unsigned int irq;
 	u32 rev;
 
@@ -436,8 +438,12 @@ static int pm8921_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
+	regmap = devm_regmap_init_ssbi(&pdev->dev, &ssbi_regmap_config);
+	if (IS_ERR(regmap))
+		return PTR_ERR(regmap);
+
 	/* Read PMIC chip revision */
-	rc = ssbi_read(pdev->dev.parent, REG_HWREV, &val, sizeof(val));
+	rc = regmap_read(regmap, REG_HWREV, &val);
 	if (rc) {
 		pr_err("Failed to read hw rev reg %d:rc=%d\n", REG_HWREV, rc);
 		return rc;
@@ -446,7 +452,7 @@ static int pm8921_probe(struct platform_device *pdev)
 	rev = val;
 
 	/* Read PMIC chip revision 2 */
-	rc = ssbi_read(pdev->dev.parent, REG_HWREV_2, &val, sizeof(val));
+	rc = regmap_read(regmap, REG_HWREV_2, &val);
 	if (rc) {
 		pr_err("Failed to read hw rev 2 reg %d:rc=%d\n",
 			REG_HWREV_2, rc);
@@ -459,7 +465,7 @@ static int pm8921_probe(struct platform_device *pdev)
 	pm8921_drvdata.pm_chip_data = pmic;
 	platform_set_drvdata(pdev, &pm8921_drvdata);
 
-	rc = pm8xxx_irq_init(pdev, irq, 256);
+	rc = pm8xxx_irq_init(pdev, regmap, irq, 256);
 	if (rc)
 		return rc;
 
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

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

* Re: [PATCH 3/8] regmap: Add support for using regmap over ssbi
  2013-12-10 23:35   ` Stephen Boyd
@ 2013-12-10 23:50     ` Mark Brown
  -1 siblings, 0 replies; 77+ messages in thread
From: Mark Brown @ 2013-12-10 23:50 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Samuel Ortiz, Lee Jones, Srinivas Ramana, linux-kernel,
	linux-arm-msm, linux-arm-kernel

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

On Tue, Dec 10, 2013 at 03:35:18PM -0800, Stephen Boyd wrote:

> +	while (val_size) {
> +		ret = ssbi_read(context, *(u16 *)reg, val, 1);
> +		if (ret)
> +			return ret;
> +		reg += sizeof(u16);
> +		val += sizeof(u8);
> +		val_size -= sizeof(u8);

I'd expect this to generate out of bounds accesses and bad interactions
on the bus if we go through the loop more than once since it appears to
incrementing the address of reg for every register.  I'm also having a
hard time understanding why this is doing a read per byte, ssbi_read()
seems to map fairly directly onto the interface of the operation so
there doesn't seem to be any reason for this loop to exist in the first
place.

Has this been tested?  

It'd be helpful to CC the entire series, or at least the patches this
builds on...

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

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

* [PATCH 3/8] regmap: Add support for using regmap over ssbi
@ 2013-12-10 23:50     ` Mark Brown
  0 siblings, 0 replies; 77+ messages in thread
From: Mark Brown @ 2013-12-10 23:50 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Dec 10, 2013 at 03:35:18PM -0800, Stephen Boyd wrote:

> +	while (val_size) {
> +		ret = ssbi_read(context, *(u16 *)reg, val, 1);
> +		if (ret)
> +			return ret;
> +		reg += sizeof(u16);
> +		val += sizeof(u8);
> +		val_size -= sizeof(u8);

I'd expect this to generate out of bounds accesses and bad interactions
on the bus if we go through the loop more than once since it appears to
incrementing the address of reg for every register.  I'm also having a
hard time understanding why this is doing a read per byte, ssbi_read()
seems to map fairly directly onto the interface of the operation so
there doesn't seem to be any reason for this loop to exist in the first
place.

Has this been tested?  

It'd be helpful to CC the entire series, or at least the patches this
builds on...
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131210/1f9dde51/attachment.sig>

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

* Re: [PATCH 3/8] regmap: Add support for using regmap over ssbi
  2013-12-10 23:50     ` Mark Brown
@ 2013-12-11  0:13       ` Stephen Boyd
  -1 siblings, 0 replies; 77+ messages in thread
From: Stephen Boyd @ 2013-12-11  0:13 UTC (permalink / raw)
  To: Mark Brown
  Cc: Samuel Ortiz, Lee Jones, Srinivas Ramana, linux-kernel,
	linux-arm-msm, linux-arm-kernel

On 12/10/13 15:50, Mark Brown wrote:
> On Tue, Dec 10, 2013 at 03:35:18PM -0800, Stephen Boyd wrote:
>
>> +	while (val_size) {
>> +		ret = ssbi_read(context, *(u16 *)reg, val, 1);
>> +		if (ret)
>> +			return ret;
>> +		reg += sizeof(u16);
>> +		val += sizeof(u8);
>> +		val_size -= sizeof(u8);
> I'd expect this to generate out of bounds accesses and bad interactions
> on the bus if we go through the loop more than once since it appears to
> incrementing the address of reg for every register.  I'm also having a
> hard time understanding why this is doing a read per byte, ssbi_read()
> seems to map fairly directly onto the interface of the operation so
> there doesn't seem to be any reason for this loop to exist in the first
> place.

ssbi_read() just reads the same register x number of times and doesn't
do any sort of incrementing of address. My understanding was that
regmap_bulk_read() will read incrementing addresses and then call down
into this code with the sequential addresses formatted into the reg
buffer. That was the flaw. Instead we need to take reg and then
increment reg by 1 every time through this loop. Or should we just have
use_single_rw == true?

>
> Has this been tested?  

Yes. But so far they've all been single register reads.

>
> It'd be helpful to CC the entire series, or at least the patches this
> builds on...

Sure, will do next time.

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

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

* [PATCH 3/8] regmap: Add support for using regmap over ssbi
@ 2013-12-11  0:13       ` Stephen Boyd
  0 siblings, 0 replies; 77+ messages in thread
From: Stephen Boyd @ 2013-12-11  0:13 UTC (permalink / raw)
  To: linux-arm-kernel

On 12/10/13 15:50, Mark Brown wrote:
> On Tue, Dec 10, 2013 at 03:35:18PM -0800, Stephen Boyd wrote:
>
>> +	while (val_size) {
>> +		ret = ssbi_read(context, *(u16 *)reg, val, 1);
>> +		if (ret)
>> +			return ret;
>> +		reg += sizeof(u16);
>> +		val += sizeof(u8);
>> +		val_size -= sizeof(u8);
> I'd expect this to generate out of bounds accesses and bad interactions
> on the bus if we go through the loop more than once since it appears to
> incrementing the address of reg for every register.  I'm also having a
> hard time understanding why this is doing a read per byte, ssbi_read()
> seems to map fairly directly onto the interface of the operation so
> there doesn't seem to be any reason for this loop to exist in the first
> place.

ssbi_read() just reads the same register x number of times and doesn't
do any sort of incrementing of address. My understanding was that
regmap_bulk_read() will read incrementing addresses and then call down
into this code with the sequential addresses formatted into the reg
buffer. That was the flaw. Instead we need to take reg and then
increment reg by 1 every time through this loop. Or should we just have
use_single_rw == true?

>
> Has this been tested?  

Yes. But so far they've all been single register reads.

>
> It'd be helpful to CC the entire series, or at least the patches this
> builds on...

Sure, will do next time.

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

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

* Re: [PATCH 3/8] regmap: Add support for using regmap over ssbi
  2013-12-11  0:13       ` Stephen Boyd
@ 2013-12-11  0:51         ` Mark Brown
  -1 siblings, 0 replies; 77+ messages in thread
From: Mark Brown @ 2013-12-11  0:51 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Samuel Ortiz, Lee Jones, Srinivas Ramana, linux-kernel,
	linux-arm-msm, linux-arm-kernel

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

On Tue, Dec 10, 2013 at 04:13:15PM -0800, Stephen Boyd wrote:
> On 12/10/13 15:50, Mark Brown wrote:
> > On Tue, Dec 10, 2013 at 03:35:18PM -0800, Stephen Boyd wrote:

> >> +		reg += sizeof(u16);

> > I'd expect this to generate out of bounds accesses and bad interactions
> > on the bus if we go through the loop more than once since it appears to
> > incrementing the address of reg for every register.  I'm also having a

> ssbi_read() just reads the same register x number of times and doesn't
> do any sort of incrementing of address. My understanding was that
> regmap_bulk_read() will read incrementing addresses and then call down
> into this code with the sequential addresses formatted into the reg
> buffer. That was the flaw. Instead we need to take reg and then

That's possibly not the ideal decision for the underlying API, if
nothing else it's confusing naming given what a read function would
normally do.

> increment reg by 1 every time through this loop. Or should we just have
> use_single_rw == true?

No, it doesn't - it increments the address of reg by the size of a
register value each time.  Using use_single_rw might make sense, or if
you can't do bulk I/O at all then hooking in via reg_read() and
reg_write() in the config rather than trying to parse out the buffers
might be even better (you can still make helpers to set that up).

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

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

* [PATCH 3/8] regmap: Add support for using regmap over ssbi
@ 2013-12-11  0:51         ` Mark Brown
  0 siblings, 0 replies; 77+ messages in thread
From: Mark Brown @ 2013-12-11  0:51 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Dec 10, 2013 at 04:13:15PM -0800, Stephen Boyd wrote:
> On 12/10/13 15:50, Mark Brown wrote:
> > On Tue, Dec 10, 2013 at 03:35:18PM -0800, Stephen Boyd wrote:

> >> +		reg += sizeof(u16);

> > I'd expect this to generate out of bounds accesses and bad interactions
> > on the bus if we go through the loop more than once since it appears to
> > incrementing the address of reg for every register.  I'm also having a

> ssbi_read() just reads the same register x number of times and doesn't
> do any sort of incrementing of address. My understanding was that
> regmap_bulk_read() will read incrementing addresses and then call down
> into this code with the sequential addresses formatted into the reg
> buffer. That was the flaw. Instead we need to take reg and then

That's possibly not the ideal decision for the underlying API, if
nothing else it's confusing naming given what a read function would
normally do.

> increment reg by 1 every time through this loop. Or should we just have
> use_single_rw == true?

No, it doesn't - it increments the address of reg by the size of a
register value each time.  Using use_single_rw might make sense, or if
you can't do bulk I/O at all then hooking in via reg_read() and
reg_write() in the config rather than trying to parse out the buffers
might be even better (you can still make helpers to set that up).
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131211/f07b25f5/attachment.sig>

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

* Re: [PATCH 3/8] regmap: Add support for using regmap over ssbi
  2013-12-11  0:51         ` Mark Brown
@ 2013-12-11  1:32           ` Stephen Boyd
  -1 siblings, 0 replies; 77+ messages in thread
From: Stephen Boyd @ 2013-12-11  1:32 UTC (permalink / raw)
  To: Mark Brown
  Cc: Samuel Ortiz, Lee Jones, Srinivas Ramana, linux-kernel,
	linux-arm-msm, linux-arm-kernel

On 12/10/13 16:51, Mark Brown wrote:
> On Tue, Dec 10, 2013 at 04:13:15PM -0800, Stephen Boyd wrote:
>> increment reg by 1 every time through this loop. Or should we just have
>> use_single_rw == true?
> No, it doesn't - it increments the address of reg by the size of a
> register value each time.  Using use_single_rw might make sense, or if
> you can't do bulk I/O at all then hooking in via reg_read() and
> reg_write() in the config rather than trying to parse out the buffers
> might be even better (you can still make helpers to set that up).


Are you suggesting we implement the reg_read/reg_write as global helpers
that the config points to and then call regmap_init()? At a quick glance
it looks like we lose out on regmap_bulk_read() if we do that. There is
one driver that will use regmap_bulk_read(), but I suppose we can just
loop on regmap_read() and do our own increment? If we use use_single_rw
everything works and we can simplify this code to just pass the reg and
val buffers directly to ssbi_read/write.

This is what I have now.

static int regmap_ssbi_read(void *context,
                            const void *regp, size_t reg_size,
                            void *val, size_t val_size)
{
        int ret;
        u16 reg;

        BUG_ON(reg_size != 2);

        reg = *(u16 *)regp;
        while (val_size) {
                ret = ssbi_read(context, reg, val, 1);
                if (ret)
                        return ret;
                reg++;
                val += sizeof(u8);
                val_size -= sizeof(u8);
        }

        return 0;
}

With use_single_rw I think it can be this.

static int regmap_ssbi_read(void *context,
                            const void *reg, size_t reg_size,
                            void *val, size_t val_size)
{
        BUG_ON(reg_size != 2);  
        return ssbi_read(context, *(u16 *)reg, val, 1);
}

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

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

* [PATCH 3/8] regmap: Add support for using regmap over ssbi
@ 2013-12-11  1:32           ` Stephen Boyd
  0 siblings, 0 replies; 77+ messages in thread
From: Stephen Boyd @ 2013-12-11  1:32 UTC (permalink / raw)
  To: linux-arm-kernel

On 12/10/13 16:51, Mark Brown wrote:
> On Tue, Dec 10, 2013 at 04:13:15PM -0800, Stephen Boyd wrote:
>> increment reg by 1 every time through this loop. Or should we just have
>> use_single_rw == true?
> No, it doesn't - it increments the address of reg by the size of a
> register value each time.  Using use_single_rw might make sense, or if
> you can't do bulk I/O at all then hooking in via reg_read() and
> reg_write() in the config rather than trying to parse out the buffers
> might be even better (you can still make helpers to set that up).


Are you suggesting we implement the reg_read/reg_write as global helpers
that the config points to and then call regmap_init()? At a quick glance
it looks like we lose out on regmap_bulk_read() if we do that. There is
one driver that will use regmap_bulk_read(), but I suppose we can just
loop on regmap_read() and do our own increment? If we use use_single_rw
everything works and we can simplify this code to just pass the reg and
val buffers directly to ssbi_read/write.

This is what I have now.

static int regmap_ssbi_read(void *context,
                            const void *regp, size_t reg_size,
                            void *val, size_t val_size)
{
        int ret;
        u16 reg;

        BUG_ON(reg_size != 2);

        reg = *(u16 *)regp;
        while (val_size) {
                ret = ssbi_read(context, reg, val, 1);
                if (ret)
                        return ret;
                reg++;
                val += sizeof(u8);
                val_size -= sizeof(u8);
        }

        return 0;
}

With use_single_rw I think it can be this.

static int regmap_ssbi_read(void *context,
                            const void *reg, size_t reg_size,
                            void *val, size_t val_size)
{
        BUG_ON(reg_size != 2);  
        return ssbi_read(context, *(u16 *)reg, val, 1);
}

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

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

* Re: [PATCH 1/8] mfd: ssbi: Remove platform data structs and hide ssbi type enum
  2013-12-10 23:35   ` Stephen Boyd
@ 2013-12-11  9:27     ` Lee Jones
  -1 siblings, 0 replies; 77+ messages in thread
From: Lee Jones @ 2013-12-11  9:27 UTC (permalink / raw)
  To: Stephen Boyd; +Cc: Samuel Ortiz, linux-kernel, linux-arm-msm, linux-arm-kernel

On Tue, 10 Dec 2013, Stephen Boyd wrote:

> The ssbi driver assumes that the device is DT based. Remove the
> platform data structs that will never be used and hide the enum
> in the only C file that uses it.
> 
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> ---
>  drivers/mfd/ssbi.c   |  6 ++++++
>  include/linux/ssbi.h | 17 +----------------
>  2 files changed, 7 insertions(+), 16 deletions(-)

I don't see any uses of the aforementioned struct.

Patch applied, thanks.

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

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

* [PATCH 1/8] mfd: ssbi: Remove platform data structs and hide ssbi type enum
@ 2013-12-11  9:27     ` Lee Jones
  0 siblings, 0 replies; 77+ messages in thread
From: Lee Jones @ 2013-12-11  9:27 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, 10 Dec 2013, Stephen Boyd wrote:

> The ssbi driver assumes that the device is DT based. Remove the
> platform data structs that will never be used and hide the enum
> in the only C file that uses it.
> 
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> ---
>  drivers/mfd/ssbi.c   |  6 ++++++
>  include/linux/ssbi.h | 17 +----------------
>  2 files changed, 7 insertions(+), 16 deletions(-)

I don't see any uses of the aforementioned struct.

Patch applied, thanks.

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

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

* Re: [PATCH 2/8] mfd: ssbi: Constify buffer in ssbi_write
  2013-12-10 23:35   ` Stephen Boyd
@ 2013-12-11  9:28     ` Lee Jones
  -1 siblings, 0 replies; 77+ messages in thread
From: Lee Jones @ 2013-12-11  9:28 UTC (permalink / raw)
  To: Stephen Boyd; +Cc: Samuel Ortiz, linux-kernel, linux-arm-msm, linux-arm-kernel

On Tue, 10 Dec 2013, Stephen Boyd wrote:

> In preparation for passing a const pointer directly to
> ssbi_write() from the regmap APIs.
> 
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> ---
>  drivers/mfd/ssbi.c   | 8 ++++----
>  include/linux/ssbi.h | 2 +-
>  2 files changed, 5 insertions(+), 5 deletions(-)

Applied, thanks.

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

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

* [PATCH 2/8] mfd: ssbi: Constify buffer in ssbi_write
@ 2013-12-11  9:28     ` Lee Jones
  0 siblings, 0 replies; 77+ messages in thread
From: Lee Jones @ 2013-12-11  9:28 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, 10 Dec 2013, Stephen Boyd wrote:

> In preparation for passing a const pointer directly to
> ssbi_write() from the regmap APIs.
> 
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> ---
>  drivers/mfd/ssbi.c   | 8 ++++----
>  include/linux/ssbi.h | 2 +-
>  2 files changed, 5 insertions(+), 5 deletions(-)

Applied, thanks.

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

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

* Re: [PATCH 4/8] mfd: ssbi: Mark match table const
  2013-12-10 23:35   ` Stephen Boyd
@ 2013-12-11  9:29     ` Lee Jones
  -1 siblings, 0 replies; 77+ messages in thread
From: Lee Jones @ 2013-12-11  9:29 UTC (permalink / raw)
  To: Stephen Boyd; +Cc: Samuel Ortiz, linux-kernel, linux-arm-msm, linux-arm-kernel

On Tue, 10 Dec 2013, Stephen Boyd wrote:

> This is a read-only data structure.
> 
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> ---
>  drivers/mfd/ssbi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

It certainly is.

Applied, thanks.

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

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

* [PATCH 4/8] mfd: ssbi: Mark match table const
@ 2013-12-11  9:29     ` Lee Jones
  0 siblings, 0 replies; 77+ messages in thread
From: Lee Jones @ 2013-12-11  9:29 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, 10 Dec 2013, Stephen Boyd wrote:

> This is a read-only data structure.
> 
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> ---
>  drivers/mfd/ssbi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

It certainly is.

Applied, thanks.

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

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

* Re: [PATCH 5/8] mfd: Move pm8xxx-irq.c contents into only driver that uses it
  2013-12-10 23:35   ` Stephen Boyd
@ 2013-12-11  9:35     ` Lee Jones
  -1 siblings, 0 replies; 77+ messages in thread
From: Lee Jones @ 2013-12-11  9:35 UTC (permalink / raw)
  To: Stephen Boyd; +Cc: Samuel Ortiz, linux-kernel, linux-arm-msm, linux-arm-kernel

On Tue, 10 Dec 2013, Stephen Boyd wrote:

> The pm8xxx-irq.c code is practically mandatory given that the
> pm8921-core driver will WARN about it missing and the Kconfig
> marks it as default y when a PM8xxx chips is enabled. The only
> reason the file was split out was because we planned to support
> other pm8xxx chips with different pm8xxx-core.c files. Now that
> we have DT on ARM this isn't necessary because we should be able
> to support all the ssbi based PM8xxx chips in one driver and one
> file with no data bloat. Let's move this code into the only
> driver that uses it right now (pm8921) so that it's always compiled when
> needed. In the future we can rename pm8921-core.c to something
> more generic.
> 
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> ---
>  drivers/mfd/Kconfig       |  10 --
>  drivers/mfd/pm8921-core.c | 349 +++++++++++++++++++++++++++++++++++++++++++
>  drivers/mfd/pm8xxx-irq.c  | 371 ----------------------------------------------

If you're going to take this approach (which I'm fine with unless
there are likely to be anymore incarnations of pm8xxx chips?), I think
you should also clean-up the header in this patch.

  include/linux/mfd/pm8xxx/irq.h

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

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

* [PATCH 5/8] mfd: Move pm8xxx-irq.c contents into only driver that uses it
@ 2013-12-11  9:35     ` Lee Jones
  0 siblings, 0 replies; 77+ messages in thread
From: Lee Jones @ 2013-12-11  9:35 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, 10 Dec 2013, Stephen Boyd wrote:

> The pm8xxx-irq.c code is practically mandatory given that the
> pm8921-core driver will WARN about it missing and the Kconfig
> marks it as default y when a PM8xxx chips is enabled. The only
> reason the file was split out was because we planned to support
> other pm8xxx chips with different pm8xxx-core.c files. Now that
> we have DT on ARM this isn't necessary because we should be able
> to support all the ssbi based PM8xxx chips in one driver and one
> file with no data bloat. Let's move this code into the only
> driver that uses it right now (pm8921) so that it's always compiled when
> needed. In the future we can rename pm8921-core.c to something
> more generic.
> 
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> ---
>  drivers/mfd/Kconfig       |  10 --
>  drivers/mfd/pm8921-core.c | 349 +++++++++++++++++++++++++++++++++++++++++++
>  drivers/mfd/pm8xxx-irq.c  | 371 ----------------------------------------------

If you're going to take this approach (which I'm fine with unless
there are likely to be anymore incarnations of pm8xxx chips?), I think
you should also clean-up the header in this patch.

  include/linux/mfd/pm8xxx/irq.h

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

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

* Re: [PATCH 6/8] mfd: pm8921: Update for genirq changes
  2013-12-10 23:35   ` Stephen Boyd
@ 2013-12-11  9:48     ` Lee Jones
  -1 siblings, 0 replies; 77+ messages in thread
From: Lee Jones @ 2013-12-11  9:48 UTC (permalink / raw)
  To: Stephen Boyd; +Cc: Samuel Ortiz, linux-kernel, linux-arm-msm, linux-arm-kernel

> Since this code has been marked broken for some time a few genirq
> tree wide changes weren't made. set_irq_wake() was renamed to
> irq_set_irq_wake() in commit a0cd9ca2b (genirq: Namespace
> cleanup, 2011-02-10) and commit 10a8c383 (irq: introduce entry
> and exit functions for chained handlers) introduced the chained
> irq functions but this driver wasn't updated to use them. Fix
> these problems and remove the BROKEN marking on this driver.
> 
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> ---
>  drivers/mfd/Kconfig       | 1 -
>  drivers/mfd/pm8921-core.c | 7 +++++--
>  2 files changed, 5 insertions(+), 3 deletions(-)

This driver was marked as BROKEN due to compilation failures only, so
as long as it now compiles cleanly (which I'm sure you've tested) I'm
happy with this patch.

It doesn't apply, without your 5th patch, so for now I'll just Ack
it. Please remember to apply my Ack when you resubmit, as it will act
as a reminder and I won't have to re-review.

Acked-by: Lee Jones <lee.jones@linaro.org>

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

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

* [PATCH 6/8] mfd: pm8921: Update for genirq changes
@ 2013-12-11  9:48     ` Lee Jones
  0 siblings, 0 replies; 77+ messages in thread
From: Lee Jones @ 2013-12-11  9:48 UTC (permalink / raw)
  To: linux-arm-kernel

> Since this code has been marked broken for some time a few genirq
> tree wide changes weren't made. set_irq_wake() was renamed to
> irq_set_irq_wake() in commit a0cd9ca2b (genirq: Namespace
> cleanup, 2011-02-10) and commit 10a8c383 (irq: introduce entry
> and exit functions for chained handlers) introduced the chained
> irq functions but this driver wasn't updated to use them. Fix
> these problems and remove the BROKEN marking on this driver.
> 
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> ---
>  drivers/mfd/Kconfig       | 1 -
>  drivers/mfd/pm8921-core.c | 7 +++++--
>  2 files changed, 5 insertions(+), 3 deletions(-)

This driver was marked as BROKEN due to compilation failures only, so
as long as it now compiles cleanly (which I'm sure you've tested) I'm
happy with this patch.

It doesn't apply, without your 5th patch, so for now I'll just Ack
it. Please remember to apply my Ack when you resubmit, as it will act
as a reminder and I won't have to re-review.

Acked-by: Lee Jones <lee.jones@linaro.org>

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

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

* Re: [PATCH 7/8] mfd: pm8921: Migrate to irqdomains
  2013-12-10 23:35   ` Stephen Boyd
@ 2013-12-11  9:53     ` Lee Jones
  -1 siblings, 0 replies; 77+ messages in thread
From: Lee Jones @ 2013-12-11  9:53 UTC (permalink / raw)
  To: Stephen Boyd; +Cc: Samuel Ortiz, linux-kernel, linux-arm-msm, linux-arm-kernel

> Convert this driver to use irqdomains so that the PMIC's child
> devices can be converted to devicetree.
> 
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> ---
>  drivers/mfd/pm8921-core.c         | 186 ++++++++++++++------------------------
>  include/linux/mfd/pm8xxx/irq.h    |  59 ------------
>  include/linux/mfd/pm8xxx/pm8921.h |  30 ------
>  3 files changed, 66 insertions(+), 209 deletions(-)
>  delete mode 100644 include/linux/mfd/pm8xxx/irq.h
>  delete mode 100644 include/linux/mfd/pm8xxx/pm8921.h

I'd like to review this patch by applying it, which I can't do until
patch 5 has been fixed. Please resubmit once it's been rectified.

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

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

* [PATCH 7/8] mfd: pm8921: Migrate to irqdomains
@ 2013-12-11  9:53     ` Lee Jones
  0 siblings, 0 replies; 77+ messages in thread
From: Lee Jones @ 2013-12-11  9:53 UTC (permalink / raw)
  To: linux-arm-kernel

> Convert this driver to use irqdomains so that the PMIC's child
> devices can be converted to devicetree.
> 
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> ---
>  drivers/mfd/pm8921-core.c         | 186 ++++++++++++++------------------------
>  include/linux/mfd/pm8xxx/irq.h    |  59 ------------
>  include/linux/mfd/pm8xxx/pm8921.h |  30 ------
>  3 files changed, 66 insertions(+), 209 deletions(-)
>  delete mode 100644 include/linux/mfd/pm8xxx/irq.h
>  delete mode 100644 include/linux/mfd/pm8xxx/pm8921.h

I'd like to review this patch by applying it, which I can't do until
patch 5 has been fixed. Please resubmit once it's been rectified.

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

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

* Re: [PATCH 8/8] mfd: pm8921: Use ssbi regmap
  2013-12-10 23:35   ` Stephen Boyd
@ 2013-12-11  9:55     ` Lee Jones
  -1 siblings, 0 replies; 77+ messages in thread
From: Lee Jones @ 2013-12-11  9:55 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Samuel Ortiz, linux-kernel, linux-arm-msm, linux-arm-kernel, broonie

> Use a regmap so that the pm8xxx read/write APIs can be removed
> once all consumer drivers are converted.
> 
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> ---
>  drivers/mfd/Kconfig       |  1 +
>  drivers/mfd/pm8921-core.c | 70 +++++++++++++++++++++++++----------------------
>  2 files changed, 39 insertions(+), 32 deletions(-)

As this is regmap related, don't forget to CC Mark Brown when you
resubmit this patchset.

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

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

* [PATCH 8/8] mfd: pm8921: Use ssbi regmap
@ 2013-12-11  9:55     ` Lee Jones
  0 siblings, 0 replies; 77+ messages in thread
From: Lee Jones @ 2013-12-11  9:55 UTC (permalink / raw)
  To: linux-arm-kernel

> Use a regmap so that the pm8xxx read/write APIs can be removed
> once all consumer drivers are converted.
> 
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> ---
>  drivers/mfd/Kconfig       |  1 +
>  drivers/mfd/pm8921-core.c | 70 +++++++++++++++++++++++++----------------------
>  2 files changed, 39 insertions(+), 32 deletions(-)

As this is regmap related, don't forget to CC Mark Brown when you
resubmit this patchset.

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

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

* Re: [PATCH 3/8] regmap: Add support for using regmap over ssbi
  2013-12-11  1:32           ` Stephen Boyd
@ 2013-12-11 13:27             ` Mark Brown
  -1 siblings, 0 replies; 77+ messages in thread
From: Mark Brown @ 2013-12-11 13:27 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Samuel Ortiz, Lee Jones, Srinivas Ramana, linux-kernel,
	linux-arm-msm, linux-arm-kernel

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

On Tue, Dec 10, 2013 at 05:32:51PM -0800, Stephen Boyd wrote:

> Are you suggesting we implement the reg_read/reg_write as global helpers
> that the config points to and then call regmap_init()? At a quick glance

Yes, assuming your bus really is limited in the way it seems from the
code.

> it looks like we lose out on regmap_bulk_read() if we do that. There is
> one driver that will use regmap_bulk_read(), but I suppose we can just

bulk_read() should decay to individual reads if there isn't a block
operaton and it's not like the hardware actually supports bulk reads
anyway.

> With use_single_rw I think it can be this.

> static int regmap_ssbi_read(void *context,
>                             const void *reg, size_t reg_size,
>                             void *val, size_t val_size)
> {
>         BUG_ON(reg_size != 2);  
>         return ssbi_read(context, *(u16 *)reg, val, 1);
> }

Right, which is a lot simpler.  Though I'm still not sure the
ssbi_read() interface is the best.

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

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

* [PATCH 3/8] regmap: Add support for using regmap over ssbi
@ 2013-12-11 13:27             ` Mark Brown
  0 siblings, 0 replies; 77+ messages in thread
From: Mark Brown @ 2013-12-11 13:27 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Dec 10, 2013 at 05:32:51PM -0800, Stephen Boyd wrote:

> Are you suggesting we implement the reg_read/reg_write as global helpers
> that the config points to and then call regmap_init()? At a quick glance

Yes, assuming your bus really is limited in the way it seems from the
code.

> it looks like we lose out on regmap_bulk_read() if we do that. There is
> one driver that will use regmap_bulk_read(), but I suppose we can just

bulk_read() should decay to individual reads if there isn't a block
operaton and it's not like the hardware actually supports bulk reads
anyway.

> With use_single_rw I think it can be this.

> static int regmap_ssbi_read(void *context,
>                             const void *reg, size_t reg_size,
>                             void *val, size_t val_size)
> {
>         BUG_ON(reg_size != 2);  
>         return ssbi_read(context, *(u16 *)reg, val, 1);
> }

Right, which is a lot simpler.  Though I'm still not sure the
ssbi_read() interface is the best.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131211/440aae0b/attachment.sig>

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

* Re: [PATCH 7/8] mfd: pm8921: Migrate to irqdomains
  2013-12-10 23:35   ` Stephen Boyd
  (?)
@ 2013-12-11 21:30     ` Courtney Cavin
  -1 siblings, 0 replies; 77+ messages in thread
From: Courtney Cavin @ 2013-12-11 21:30 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Samuel Ortiz, Lee Jones, linux-kernel, linux-arm-msm, linux-arm-kernel

On Wed, Dec 11, 2013 at 12:35:22AM +0100, Stephen Boyd wrote:
> Convert this driver to use irqdomains so that the PMIC's child
> devices can be converted to devicetree.
> 
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> ---
>  drivers/mfd/pm8921-core.c         | 186 ++++++++++++++------------------------
>  include/linux/mfd/pm8xxx/irq.h    |  59 ------------
>  include/linux/mfd/pm8xxx/pm8921.h |  30 ------
>  3 files changed, 66 insertions(+), 209 deletions(-)
>  delete mode 100644 include/linux/mfd/pm8xxx/irq.h
>  delete mode 100644 include/linux/mfd/pm8xxx/pm8921.h
> 
[...]
> @@ -282,17 +278,14 @@ static struct irq_chip pm8xxx_irq_chip = {
>   * RETURNS:
>   * an int indicating the value read on that line
>   */
> -int pm8xxx_get_irq_stat(struct pm_irq_chip *chip, int irq)
> +static int pm8xxx_get_irq_stat(struct pm_irq_chip *chip, int irq)
>  {
>         int pmirq, rc;
>         u8  block, bits, bit;
>         unsigned long flags;
> +       struct irq_data *irq_data = irq_get_irq_data(irq);
> 
> -       if (chip == NULL || irq < chip->irq_base ||
> -                       irq >= chip->irq_base + chip->num_irqs)
> -               return -EINVAL;
> -
> -       pmirq = irq - chip->irq_base;
> +       pmirq = irq_data->hwirq;
> 
>         block = pmirq / 8;
>         bit = pmirq % 8;
> @@ -322,64 +315,55 @@ bail_out:
>  }
>  EXPORT_SYMBOL_GPL(pm8xxx_get_irq_stat);

Surely this isn't needed anymore, since the function is now static.

[...]
> +static int pm8xxx_irq_init(struct platform_device *pdev, unsigned int irq,
> +                          unsigned int nirqs)

'nirqs' seems to always be 256.  Is there a benefit to keeping this
dynamic?

> +{
> +       struct pm_irq_chip  *chip;
> +
> +       chip = devm_kzalloc(&pdev->dev, sizeof(*chip) + sizeof(u8) * nirqs,
> +                           GFP_KERNEL);
> +       if (!chip)
> +               return -ENOMEM;
> 
> -       chip->dev = dev;
> -       chip->devirq = devirq;
> -       chip->irq_base = pdata->irq_base;
> -       chip->num_irqs = pdata->irq_cdata.nirqs;
> +       chip->dev = &pdev->dev;
> +       chip->num_irqs = nirqs;
>         chip->num_blocks = DIV_ROUND_UP(chip->num_irqs, 8);
>         chip->num_masters = DIV_ROUND_UP(chip->num_blocks, 8);
>         spin_lock_init(&chip->pm_irq_lock);
> 
> -       for (pmirq = 0; pmirq < chip->num_irqs; pmirq++) {

-Courtney

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

* Re: [PATCH 7/8] mfd: pm8921: Migrate to irqdomains
@ 2013-12-11 21:30     ` Courtney Cavin
  0 siblings, 0 replies; 77+ messages in thread
From: Courtney Cavin @ 2013-12-11 21:30 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Samuel Ortiz, Lee Jones, linux-kernel, linux-arm-msm, linux-arm-kernel

On Wed, Dec 11, 2013 at 12:35:22AM +0100, Stephen Boyd wrote:
> Convert this driver to use irqdomains so that the PMIC's child
> devices can be converted to devicetree.
> 
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> ---
>  drivers/mfd/pm8921-core.c         | 186 ++++++++++++++------------------------
>  include/linux/mfd/pm8xxx/irq.h    |  59 ------------
>  include/linux/mfd/pm8xxx/pm8921.h |  30 ------
>  3 files changed, 66 insertions(+), 209 deletions(-)
>  delete mode 100644 include/linux/mfd/pm8xxx/irq.h
>  delete mode 100644 include/linux/mfd/pm8xxx/pm8921.h
> 
[...]
> @@ -282,17 +278,14 @@ static struct irq_chip pm8xxx_irq_chip = {
>   * RETURNS:
>   * an int indicating the value read on that line
>   */
> -int pm8xxx_get_irq_stat(struct pm_irq_chip *chip, int irq)
> +static int pm8xxx_get_irq_stat(struct pm_irq_chip *chip, int irq)
>  {
>         int pmirq, rc;
>         u8  block, bits, bit;
>         unsigned long flags;
> +       struct irq_data *irq_data = irq_get_irq_data(irq);
> 
> -       if (chip == NULL || irq < chip->irq_base ||
> -                       irq >= chip->irq_base + chip->num_irqs)
> -               return -EINVAL;
> -
> -       pmirq = irq - chip->irq_base;
> +       pmirq = irq_data->hwirq;
> 
>         block = pmirq / 8;
>         bit = pmirq % 8;
> @@ -322,64 +315,55 @@ bail_out:
>  }
>  EXPORT_SYMBOL_GPL(pm8xxx_get_irq_stat);

Surely this isn't needed anymore, since the function is now static.

[...]
> +static int pm8xxx_irq_init(struct platform_device *pdev, unsigned int irq,
> +                          unsigned int nirqs)

'nirqs' seems to always be 256.  Is there a benefit to keeping this
dynamic?

> +{
> +       struct pm_irq_chip  *chip;
> +
> +       chip = devm_kzalloc(&pdev->dev, sizeof(*chip) + sizeof(u8) * nirqs,
> +                           GFP_KERNEL);
> +       if (!chip)
> +               return -ENOMEM;
> 
> -       chip->dev = dev;
> -       chip->devirq = devirq;
> -       chip->irq_base = pdata->irq_base;
> -       chip->num_irqs = pdata->irq_cdata.nirqs;
> +       chip->dev = &pdev->dev;
> +       chip->num_irqs = nirqs;
>         chip->num_blocks = DIV_ROUND_UP(chip->num_irqs, 8);
>         chip->num_masters = DIV_ROUND_UP(chip->num_blocks, 8);
>         spin_lock_init(&chip->pm_irq_lock);
> 
> -       for (pmirq = 0; pmirq < chip->num_irqs; pmirq++) {

-Courtney

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

* [PATCH 7/8] mfd: pm8921: Migrate to irqdomains
@ 2013-12-11 21:30     ` Courtney Cavin
  0 siblings, 0 replies; 77+ messages in thread
From: Courtney Cavin @ 2013-12-11 21:30 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Dec 11, 2013 at 12:35:22AM +0100, Stephen Boyd wrote:
> Convert this driver to use irqdomains so that the PMIC's child
> devices can be converted to devicetree.
> 
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> ---
>  drivers/mfd/pm8921-core.c         | 186 ++++++++++++++------------------------
>  include/linux/mfd/pm8xxx/irq.h    |  59 ------------
>  include/linux/mfd/pm8xxx/pm8921.h |  30 ------
>  3 files changed, 66 insertions(+), 209 deletions(-)
>  delete mode 100644 include/linux/mfd/pm8xxx/irq.h
>  delete mode 100644 include/linux/mfd/pm8xxx/pm8921.h
> 
[...]
> @@ -282,17 +278,14 @@ static struct irq_chip pm8xxx_irq_chip = {
>   * RETURNS:
>   * an int indicating the value read on that line
>   */
> -int pm8xxx_get_irq_stat(struct pm_irq_chip *chip, int irq)
> +static int pm8xxx_get_irq_stat(struct pm_irq_chip *chip, int irq)
>  {
>         int pmirq, rc;
>         u8  block, bits, bit;
>         unsigned long flags;
> +       struct irq_data *irq_data = irq_get_irq_data(irq);
> 
> -       if (chip == NULL || irq < chip->irq_base ||
> -                       irq >= chip->irq_base + chip->num_irqs)
> -               return -EINVAL;
> -
> -       pmirq = irq - chip->irq_base;
> +       pmirq = irq_data->hwirq;
> 
>         block = pmirq / 8;
>         bit = pmirq % 8;
> @@ -322,64 +315,55 @@ bail_out:
>  }
>  EXPORT_SYMBOL_GPL(pm8xxx_get_irq_stat);

Surely this isn't needed anymore, since the function is now static.

[...]
> +static int pm8xxx_irq_init(struct platform_device *pdev, unsigned int irq,
> +                          unsigned int nirqs)

'nirqs' seems to always be 256.  Is there a benefit to keeping this
dynamic?

> +{
> +       struct pm_irq_chip  *chip;
> +
> +       chip = devm_kzalloc(&pdev->dev, sizeof(*chip) + sizeof(u8) * nirqs,
> +                           GFP_KERNEL);
> +       if (!chip)
> +               return -ENOMEM;
> 
> -       chip->dev = dev;
> -       chip->devirq = devirq;
> -       chip->irq_base = pdata->irq_base;
> -       chip->num_irqs = pdata->irq_cdata.nirqs;
> +       chip->dev = &pdev->dev;
> +       chip->num_irqs = nirqs;
>         chip->num_blocks = DIV_ROUND_UP(chip->num_irqs, 8);
>         chip->num_masters = DIV_ROUND_UP(chip->num_blocks, 8);
>         spin_lock_init(&chip->pm_irq_lock);
> 
> -       for (pmirq = 0; pmirq < chip->num_irqs; pmirq++) {

-Courtney

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

* Re: [PATCH 7/8] mfd: pm8921: Migrate to irqdomains
  2013-12-11 21:30     ` Courtney Cavin
  (?)
@ 2013-12-12 19:05       ` Stephen Boyd
  -1 siblings, 0 replies; 77+ messages in thread
From: Stephen Boyd @ 2013-12-12 19:05 UTC (permalink / raw)
  To: Courtney Cavin
  Cc: Samuel Ortiz, Lee Jones, linux-kernel, linux-arm-msm, linux-arm-kernel

On 12/11/13 13:30, Courtney Cavin wrote:
> On Wed, Dec 11, 2013 at 12:35:22AM +0100, Stephen Boyd wrote:
>> @@ -282,17 +278,14 @@ static struct irq_chip pm8xxx_irq_chip = {
>>   * RETURNS:
>>   * an int indicating the value read on that line
>>   */
>> -int pm8xxx_get_irq_stat(struct pm_irq_chip *chip, int irq)
>> +static int pm8xxx_get_irq_stat(struct pm_irq_chip *chip, int irq)
>>  {
>>         int pmirq, rc;
>>         u8  block, bits, bit;
>>         unsigned long flags;
>> +       struct irq_data *irq_data = irq_get_irq_data(irq);
>>
>> -       if (chip == NULL || irq < chip->irq_base ||
>> -                       irq >= chip->irq_base + chip->num_irqs)
>> -               return -EINVAL;
>> -
>> -       pmirq = irq - chip->irq_base;
>> +       pmirq = irq_data->hwirq;
>>
>>         block = pmirq / 8;
>>         bit = pmirq % 8;
>> @@ -322,64 +315,55 @@ bail_out:
>>  }
>>  EXPORT_SYMBOL_GPL(pm8xxx_get_irq_stat);
> Surely this isn't needed anymore, since the function is now static.

Sure. Deleted.

>
> [...]
>> +static int pm8xxx_irq_init(struct platform_device *pdev, unsigned int irq,
>> +                          unsigned int nirqs)
> 'nirqs' seems to always be 256.  Is there a benefit to keeping this
> dynamic?

I was future coding. Some pmics have less than 256 interrupts. I'll just
hardcode it for now and if/when we support the other pmics we can make
it a parameter.

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

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

* Re: [PATCH 7/8] mfd: pm8921: Migrate to irqdomains
@ 2013-12-12 19:05       ` Stephen Boyd
  0 siblings, 0 replies; 77+ messages in thread
From: Stephen Boyd @ 2013-12-12 19:05 UTC (permalink / raw)
  To: Courtney Cavin
  Cc: Samuel Ortiz, Lee Jones, linux-kernel, linux-arm-msm, linux-arm-kernel

On 12/11/13 13:30, Courtney Cavin wrote:
> On Wed, Dec 11, 2013 at 12:35:22AM +0100, Stephen Boyd wrote:
>> @@ -282,17 +278,14 @@ static struct irq_chip pm8xxx_irq_chip = {
>>   * RETURNS:
>>   * an int indicating the value read on that line
>>   */
>> -int pm8xxx_get_irq_stat(struct pm_irq_chip *chip, int irq)
>> +static int pm8xxx_get_irq_stat(struct pm_irq_chip *chip, int irq)
>>  {
>>         int pmirq, rc;
>>         u8  block, bits, bit;
>>         unsigned long flags;
>> +       struct irq_data *irq_data = irq_get_irq_data(irq);
>>
>> -       if (chip == NULL || irq < chip->irq_base ||
>> -                       irq >= chip->irq_base + chip->num_irqs)
>> -               return -EINVAL;
>> -
>> -       pmirq = irq - chip->irq_base;
>> +       pmirq = irq_data->hwirq;
>>
>>         block = pmirq / 8;
>>         bit = pmirq % 8;
>> @@ -322,64 +315,55 @@ bail_out:
>>  }
>>  EXPORT_SYMBOL_GPL(pm8xxx_get_irq_stat);
> Surely this isn't needed anymore, since the function is now static.

Sure. Deleted.

>
> [...]
>> +static int pm8xxx_irq_init(struct platform_device *pdev, unsigned int irq,
>> +                          unsigned int nirqs)
> 'nirqs' seems to always be 256.  Is there a benefit to keeping this
> dynamic?

I was future coding. Some pmics have less than 256 interrupts. I'll just
hardcode it for now and if/when we support the other pmics we can make
it a parameter.

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


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

* [PATCH 7/8] mfd: pm8921: Migrate to irqdomains
@ 2013-12-12 19:05       ` Stephen Boyd
  0 siblings, 0 replies; 77+ messages in thread
From: Stephen Boyd @ 2013-12-12 19:05 UTC (permalink / raw)
  To: linux-arm-kernel

On 12/11/13 13:30, Courtney Cavin wrote:
> On Wed, Dec 11, 2013 at 12:35:22AM +0100, Stephen Boyd wrote:
>> @@ -282,17 +278,14 @@ static struct irq_chip pm8xxx_irq_chip = {
>>   * RETURNS:
>>   * an int indicating the value read on that line
>>   */
>> -int pm8xxx_get_irq_stat(struct pm_irq_chip *chip, int irq)
>> +static int pm8xxx_get_irq_stat(struct pm_irq_chip *chip, int irq)
>>  {
>>         int pmirq, rc;
>>         u8  block, bits, bit;
>>         unsigned long flags;
>> +       struct irq_data *irq_data = irq_get_irq_data(irq);
>>
>> -       if (chip == NULL || irq < chip->irq_base ||
>> -                       irq >= chip->irq_base + chip->num_irqs)
>> -               return -EINVAL;
>> -
>> -       pmirq = irq - chip->irq_base;
>> +       pmirq = irq_data->hwirq;
>>
>>         block = pmirq / 8;
>>         bit = pmirq % 8;
>> @@ -322,64 +315,55 @@ bail_out:
>>  }
>>  EXPORT_SYMBOL_GPL(pm8xxx_get_irq_stat);
> Surely this isn't needed anymore, since the function is now static.

Sure. Deleted.

>
> [...]
>> +static int pm8xxx_irq_init(struct platform_device *pdev, unsigned int irq,
>> +                          unsigned int nirqs)
> 'nirqs' seems to always be 256.  Is there a benefit to keeping this
> dynamic?

I was future coding. Some pmics have less than 256 interrupts. I'll just
hardcode it for now and if/when we support the other pmics we can make
it a parameter.

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

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

* Re: [PATCH 5/8] mfd: Move pm8xxx-irq.c contents into only driver that uses it
  2013-12-11  9:35     ` Lee Jones
@ 2013-12-12 19:06       ` Stephen Boyd
  -1 siblings, 0 replies; 77+ messages in thread
From: Stephen Boyd @ 2013-12-12 19:06 UTC (permalink / raw)
  To: Lee Jones; +Cc: Samuel Ortiz, linux-kernel, linux-arm-msm, linux-arm-kernel

On 12/11/13 01:35, Lee Jones wrote:
> On Tue, 10 Dec 2013, Stephen Boyd wrote:
>
>> The pm8xxx-irq.c code is practically mandatory given that the
>> pm8921-core driver will WARN about it missing and the Kconfig
>> marks it as default y when a PM8xxx chips is enabled. The only
>> reason the file was split out was because we planned to support
>> other pm8xxx chips with different pm8xxx-core.c files. Now that
>> we have DT on ARM this isn't necessary because we should be able
>> to support all the ssbi based PM8xxx chips in one driver and one
>> file with no data bloat. Let's move this code into the only
>> driver that uses it right now (pm8921) so that it's always compiled when
>> needed. In the future we can rename pm8921-core.c to something
>> more generic.
>>
>> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
>> ---
>>  drivers/mfd/Kconfig       |  10 --
>>  drivers/mfd/pm8921-core.c | 349 +++++++++++++++++++++++++++++++++++++++++++
>>  drivers/mfd/pm8xxx-irq.c  | 371 ----------------------------------------------
> If you're going to take this approach (which I'm fine with unless
> there are likely to be anymore incarnations of pm8xxx chips?), I think
> you should also clean-up the header in this patch.
>
>   include/linux/mfd/pm8xxx/irq.h
>

Thanks. I'll remove the header and resend.

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

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

* [PATCH 5/8] mfd: Move pm8xxx-irq.c contents into only driver that uses it
@ 2013-12-12 19:06       ` Stephen Boyd
  0 siblings, 0 replies; 77+ messages in thread
From: Stephen Boyd @ 2013-12-12 19:06 UTC (permalink / raw)
  To: linux-arm-kernel

On 12/11/13 01:35, Lee Jones wrote:
> On Tue, 10 Dec 2013, Stephen Boyd wrote:
>
>> The pm8xxx-irq.c code is practically mandatory given that the
>> pm8921-core driver will WARN about it missing and the Kconfig
>> marks it as default y when a PM8xxx chips is enabled. The only
>> reason the file was split out was because we planned to support
>> other pm8xxx chips with different pm8xxx-core.c files. Now that
>> we have DT on ARM this isn't necessary because we should be able
>> to support all the ssbi based PM8xxx chips in one driver and one
>> file with no data bloat. Let's move this code into the only
>> driver that uses it right now (pm8921) so that it's always compiled when
>> needed. In the future we can rename pm8921-core.c to something
>> more generic.
>>
>> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
>> ---
>>  drivers/mfd/Kconfig       |  10 --
>>  drivers/mfd/pm8921-core.c | 349 +++++++++++++++++++++++++++++++++++++++++++
>>  drivers/mfd/pm8xxx-irq.c  | 371 ----------------------------------------------
> If you're going to take this approach (which I'm fine with unless
> there are likely to be anymore incarnations of pm8xxx chips?), I think
> you should also clean-up the header in this patch.
>
>   include/linux/mfd/pm8xxx/irq.h
>

Thanks. I'll remove the header and resend.

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

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

* Re: [PATCH 3/8] regmap: Add support for using regmap over ssbi
  2013-12-11 13:27             ` Mark Brown
@ 2013-12-12 23:13               ` Stephen Boyd
  -1 siblings, 0 replies; 77+ messages in thread
From: Stephen Boyd @ 2013-12-12 23:13 UTC (permalink / raw)
  To: Mark Brown
  Cc: Samuel Ortiz, Lee Jones, Srinivas Ramana, linux-kernel,
	linux-arm-msm, linux-arm-kernel

On 12/11/13 05:27, Mark Brown wrote:
> On Tue, Dec 10, 2013 at 05:32:51PM -0800, Stephen Boyd wrote:
>
>> Are you suggesting we implement the reg_read/reg_write as global helpers
>> that the config points to and then call regmap_init()? At a quick glance
> Yes, assuming your bus really is limited in the way it seems from the
> code.

Ok, I'm happy to go the reg_read/reg_write way if we can have the
bulk_read and bulk_write operations work. I was trying to mimic the
regmap_mmio code because it seems to do the same things with respect to
bulk operations. Or perhaps this code should live in drivers/mfd/ssbi.c
and then we could return an error from the regmap_init call if
use_single_rw is false?

>
>> it looks like we lose out on regmap_bulk_read() if we do that. There is
>> one driver that will use regmap_bulk_read(), but I suppose we can just
> bulk_read() should decay to individual reads if there isn't a block
> operaton and it's not like the hardware actually supports bulk reads
> anyway.

So regmap_bulk_read() should work if I don't have a map->bus? To make it
work with reg_read/write I had to do this. I'm not sure how to make
bulk_write work.

diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index 9c021d9..1ccd61b 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -1897,14 +1897,10 @@ int regmap_bulk_read(struct regmap *map, unsigned int reg, void *val,
        size_t val_bytes = map->format.val_bytes;
        bool vol = regmap_volatile_range(map, reg, val_count);
 
-       if (!map->bus)
-               return -EINVAL;
-       if (!map->format.parse_inplace)
-               return -EINVAL;
        if (reg % map->reg_stride)
                return -EINVAL;
 
-       if (vol || map->cache_type == REGCACHE_NONE) {
+       if (map->bus && map->format.parse_inplace && (vol || map->cache_type == REGCACHE_NONE)) {
                /*
                 * Some devices does not support bulk read, for
                 * them we have a series of single read operations.

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

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

* [PATCH 3/8] regmap: Add support for using regmap over ssbi
@ 2013-12-12 23:13               ` Stephen Boyd
  0 siblings, 0 replies; 77+ messages in thread
From: Stephen Boyd @ 2013-12-12 23:13 UTC (permalink / raw)
  To: linux-arm-kernel

On 12/11/13 05:27, Mark Brown wrote:
> On Tue, Dec 10, 2013 at 05:32:51PM -0800, Stephen Boyd wrote:
>
>> Are you suggesting we implement the reg_read/reg_write as global helpers
>> that the config points to and then call regmap_init()? At a quick glance
> Yes, assuming your bus really is limited in the way it seems from the
> code.

Ok, I'm happy to go the reg_read/reg_write way if we can have the
bulk_read and bulk_write operations work. I was trying to mimic the
regmap_mmio code because it seems to do the same things with respect to
bulk operations. Or perhaps this code should live in drivers/mfd/ssbi.c
and then we could return an error from the regmap_init call if
use_single_rw is false?

>
>> it looks like we lose out on regmap_bulk_read() if we do that. There is
>> one driver that will use regmap_bulk_read(), but I suppose we can just
> bulk_read() should decay to individual reads if there isn't a block
> operaton and it's not like the hardware actually supports bulk reads
> anyway.

So regmap_bulk_read() should work if I don't have a map->bus? To make it
work with reg_read/write I had to do this. I'm not sure how to make
bulk_write work.

diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index 9c021d9..1ccd61b 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -1897,14 +1897,10 @@ int regmap_bulk_read(struct regmap *map, unsigned int reg, void *val,
        size_t val_bytes = map->format.val_bytes;
        bool vol = regmap_volatile_range(map, reg, val_count);
 
-       if (!map->bus)
-               return -EINVAL;
-       if (!map->format.parse_inplace)
-               return -EINVAL;
        if (reg % map->reg_stride)
                return -EINVAL;
 
-       if (vol || map->cache_type == REGCACHE_NONE) {
+       if (map->bus && map->format.parse_inplace && (vol || map->cache_type == REGCACHE_NONE)) {
                /*
                 * Some devices does not support bulk read, for
                 * them we have a series of single read operations.

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

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

* Re: [PATCH 3/8] regmap: Add support for using regmap over ssbi
  2013-12-12 23:13               ` Stephen Boyd
@ 2013-12-13 10:41                 ` Mark Brown
  -1 siblings, 0 replies; 77+ messages in thread
From: Mark Brown @ 2013-12-13 10:41 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Samuel Ortiz, Lee Jones, Srinivas Ramana, linux-kernel,
	linux-arm-msm, linux-arm-kernel

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

On Thu, Dec 12, 2013 at 03:13:01PM -0800, Stephen Boyd wrote:

> > bulk_read() should decay to individual reads if there isn't a block
> > operaton and it's not like the hardware actually supports bulk reads
> > anyway.

> So regmap_bulk_read() should work if I don't have a map->bus? To make it
> work with reg_read/write I had to do this. I'm not sure how to make
> bulk_write work.

Yes, I'd expect the operation to work.  Your changes below are mostly
fine (we should add an additional check for values that aren't integer
numbers of bytes, I can add that) - can you send as a signed off patch
please and I'll apply?

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

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

* [PATCH 3/8] regmap: Add support for using regmap over ssbi
@ 2013-12-13 10:41                 ` Mark Brown
  0 siblings, 0 replies; 77+ messages in thread
From: Mark Brown @ 2013-12-13 10:41 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Dec 12, 2013 at 03:13:01PM -0800, Stephen Boyd wrote:

> > bulk_read() should decay to individual reads if there isn't a block
> > operaton and it's not like the hardware actually supports bulk reads
> > anyway.

> So regmap_bulk_read() should work if I don't have a map->bus? To make it
> work with reg_read/write I had to do this. I'm not sure how to make
> bulk_write work.

Yes, I'd expect the operation to work.  Your changes below are mostly
fine (we should add an additional check for values that aren't integer
numbers of bytes, I can add that) - can you send as a signed off patch
please and I'll apply?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131213/89b5bcdc/attachment.sig>

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

* [PATCH] regmap: Allow regmap_bulk_read() to work for "no-bus" regmaps
  2013-12-13 10:41                 ` Mark Brown
  (?)
@ 2013-12-13 17:14                   ` Stephen Boyd
  -1 siblings, 0 replies; 77+ messages in thread
From: Stephen Boyd @ 2013-12-13 17:14 UTC (permalink / raw)
  To: Mark Brown
  Cc: Samuel Ortiz, Srinivas Ramana, linux-arm-msm, linux-kernel,
	Lee Jones, linux-arm-kernel

regmap_bulk_read() should decay to performing individual reads if
we're using a "no-bus" regmap. Unfortunately, it returns an
error because there is no map->bus pointer. Fix it.

Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---

On 12/13/13 02:41, Mark Brown wrote:
> On Thu, Dec 12, 2013 at 03:13:01PM -0800, Stephen Boyd wrote:
>
>>> bulk_read() should decay to individual reads if there isn't a block
>>> operaton and it's not like the hardware actually supports bulk reads
>>> anyway.
>
>> So regmap_bulk_read() should work if I don't have a map->bus? To make it
>> work with reg_read/write I had to do this. I'm not sure how to make
>> bulk_write work.
>
> Yes, I'd expect the operation to work.  Your changes below are mostly
> fine (we should add an additional check for values that aren't integer
> numbers of bytes, I can add that) - can you send as a signed off patch
> please and I'll apply?

Here you go. Do you have any suggestions on how to make regmap_bulk_write()
work?

 drivers/base/regmap/regmap.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index 9c021d9..1ccd61b 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -1897,14 +1897,10 @@ int regmap_bulk_read(struct regmap *map, unsigned int reg, void *val,
 	size_t val_bytes = map->format.val_bytes;
 	bool vol = regmap_volatile_range(map, reg, val_count);
 
-	if (!map->bus)
-		return -EINVAL;
-	if (!map->format.parse_inplace)
-		return -EINVAL;
 	if (reg % map->reg_stride)
 		return -EINVAL;
 
-	if (vol || map->cache_type == REGCACHE_NONE) {
+	if (map->bus && map->format.parse_inplace && (vol || map->cache_type == REGCACHE_NONE)) {
 		/*
 		 * Some devices does not support bulk read, for
 		 * them we have a series of single read operations.
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

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

* [PATCH] regmap: Allow regmap_bulk_read() to work for "no-bus" regmaps
@ 2013-12-13 17:14                   ` Stephen Boyd
  0 siblings, 0 replies; 77+ messages in thread
From: Stephen Boyd @ 2013-12-13 17:14 UTC (permalink / raw)
  To: Mark Brown
  Cc: Samuel Ortiz, Lee Jones, Srinivas Ramana, linux-kernel,
	linux-arm-msm, linux-arm-kernel

regmap_bulk_read() should decay to performing individual reads if
we're using a "no-bus" regmap. Unfortunately, it returns an
error because there is no map->bus pointer. Fix it.

Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---

On 12/13/13 02:41, Mark Brown wrote:
> On Thu, Dec 12, 2013 at 03:13:01PM -0800, Stephen Boyd wrote:
>
>>> bulk_read() should decay to individual reads if there isn't a block
>>> operaton and it's not like the hardware actually supports bulk reads
>>> anyway.
>
>> So regmap_bulk_read() should work if I don't have a map->bus? To make it
>> work with reg_read/write I had to do this. I'm not sure how to make
>> bulk_write work.
>
> Yes, I'd expect the operation to work.  Your changes below are mostly
> fine (we should add an additional check for values that aren't integer
> numbers of bytes, I can add that) - can you send as a signed off patch
> please and I'll apply?

Here you go. Do you have any suggestions on how to make regmap_bulk_write()
work?

 drivers/base/regmap/regmap.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index 9c021d9..1ccd61b 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -1897,14 +1897,10 @@ int regmap_bulk_read(struct regmap *map, unsigned int reg, void *val,
 	size_t val_bytes = map->format.val_bytes;
 	bool vol = regmap_volatile_range(map, reg, val_count);
 
-	if (!map->bus)
-		return -EINVAL;
-	if (!map->format.parse_inplace)
-		return -EINVAL;
 	if (reg % map->reg_stride)
 		return -EINVAL;
 
-	if (vol || map->cache_type == REGCACHE_NONE) {
+	if (map->bus && map->format.parse_inplace && (vol || map->cache_type == REGCACHE_NONE)) {
 		/*
 		 * Some devices does not support bulk read, for
 		 * them we have a series of single read operations.
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation


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

* [PATCH] regmap: Allow regmap_bulk_read() to work for "no-bus" regmaps
@ 2013-12-13 17:14                   ` Stephen Boyd
  0 siblings, 0 replies; 77+ messages in thread
From: Stephen Boyd @ 2013-12-13 17:14 UTC (permalink / raw)
  To: linux-arm-kernel

regmap_bulk_read() should decay to performing individual reads if
we're using a "no-bus" regmap. Unfortunately, it returns an
error because there is no map->bus pointer. Fix it.

Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---

On 12/13/13 02:41, Mark Brown wrote:
> On Thu, Dec 12, 2013 at 03:13:01PM -0800, Stephen Boyd wrote:
>
>>> bulk_read() should decay to individual reads if there isn't a block
>>> operaton and it's not like the hardware actually supports bulk reads
>>> anyway.
>
>> So regmap_bulk_read() should work if I don't have a map->bus? To make it
>> work with reg_read/write I had to do this. I'm not sure how to make
>> bulk_write work.
>
> Yes, I'd expect the operation to work.  Your changes below are mostly
> fine (we should add an additional check for values that aren't integer
> numbers of bytes, I can add that) - can you send as a signed off patch
> please and I'll apply?

Here you go. Do you have any suggestions on how to make regmap_bulk_write()
work?

 drivers/base/regmap/regmap.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index 9c021d9..1ccd61b 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -1897,14 +1897,10 @@ int regmap_bulk_read(struct regmap *map, unsigned int reg, void *val,
 	size_t val_bytes = map->format.val_bytes;
 	bool vol = regmap_volatile_range(map, reg, val_count);
 
-	if (!map->bus)
-		return -EINVAL;
-	if (!map->format.parse_inplace)
-		return -EINVAL;
 	if (reg % map->reg_stride)
 		return -EINVAL;
 
-	if (vol || map->cache_type == REGCACHE_NONE) {
+	if (map->bus && map->format.parse_inplace && (vol || map->cache_type == REGCACHE_NONE)) {
 		/*
 		 * Some devices does not support bulk read, for
 		 * them we have a series of single read operations.
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

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

* Re: [PATCH 3/8] regmap: Add support for using regmap over ssbi
  2013-12-13 10:41                 ` Mark Brown
@ 2013-12-13 21:37                   ` Stephen Boyd
  -1 siblings, 0 replies; 77+ messages in thread
From: Stephen Boyd @ 2013-12-13 21:37 UTC (permalink / raw)
  To: Mark Brown
  Cc: Samuel Ortiz, Lee Jones, Srinivas Ramana, linux-kernel,
	linux-arm-msm, linux-arm-kernel

On 12/13, Mark Brown wrote:
> On Thu, Dec 12, 2013 at 03:13:01PM -0800, Stephen Boyd wrote:
> 
> > > bulk_read() should decay to individual reads if there isn't a block
> > > operaton and it's not like the hardware actually supports bulk reads
> > > anyway.
> 
> > So regmap_bulk_read() should work if I don't have a map->bus? To make it
> > work with reg_read/write I had to do this. I'm not sure how to make
> > bulk_write work.

I came up with this (possibly ugly) patch. It works for my
specific case but I'm not sure if unpacking the val bits into an
unsigned int and passing that to _regmap_write() is sane. What do
you think?

---8<----
diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index 1ccd61b..333c059 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -1514,11 +1514,9 @@ int regmap_bulk_write(struct regmap *map, unsigned int reg, const void *val,
 {
 	int ret = 0, i;
 	size_t val_bytes = map->format.val_bytes;
-	void *wval;
+	void *wval = NULL;
 
-	if (!map->bus)
-		return -EINVAL;
-	if (!map->format.parse_inplace)
+	if (map->bus && !map->format.parse_inplace)
 		return -EINVAL;
 	if (reg % map->reg_stride)
 		return -EINVAL;
@@ -1528,7 +1526,7 @@ int regmap_bulk_write(struct regmap *map, unsigned int reg, const void *val,
 	/* No formatting is require if val_byte is 1 */
 	if (val_bytes == 1) {
 		wval = (void *)val;
-	} else {
+	} else if (map->bus) {
 		wval = kmemdup(val, val_count * val_bytes, GFP_KERNEL);
 		if (!wval) {
 			ret = -ENOMEM;
@@ -1538,24 +1536,37 @@ int regmap_bulk_write(struct regmap *map, unsigned int reg, const void *val,
 		for (i = 0; i < val_count * val_bytes; i += val_bytes)
 			map->format.parse_inplace(wval + i);
 	}
-	/*
-	 * Some devices does not support bulk write, for
-	 * them we have a series of single write operations.
-	 */
-	if (map->use_single_rw) {
-		for (i = 0; i < val_count; i++) {
-			ret = _regmap_raw_write(map,
+
+	if (map->bus) {
+		/*
+		 * Some devices does not support bulk write, for
+		 * them we have a series of single write operations.
+		 */
+		if (map->use_single_rw) {
+			for (i = 0; i < val_count; i++) {
+				ret = _regmap_raw_write(map,
 						reg + (i * map->reg_stride),
 						val + (i * val_bytes),
 						val_bytes);
-			if (ret != 0)
-				return ret;
+				if (ret != 0)
+					return ret;
+			}
+		} else {
+			ret = _regmap_raw_write(map, reg, wval,
+						val_bytes * val_count);
 		}
 	} else {
-		ret = _regmap_raw_write(map, reg, wval, val_bytes * val_count);
+		for (i = 0; i < val_count; i++) {
+			unsigned int ival;
+			ival = *(unsigned int *)(val + (i * val_bytes));
+			ret = _regmap_write(map, reg + (i * map->reg_stride),
+					ival);
+			if (ret != 0)
+				goto out;
+		}
 	}
 
-	if (val_bytes != 1)
+	if (val_bytes != 1 && map->bus)
 		kfree(wval);
 
 out:

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

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

* [PATCH 3/8] regmap: Add support for using regmap over ssbi
@ 2013-12-13 21:37                   ` Stephen Boyd
  0 siblings, 0 replies; 77+ messages in thread
From: Stephen Boyd @ 2013-12-13 21:37 UTC (permalink / raw)
  To: linux-arm-kernel

On 12/13, Mark Brown wrote:
> On Thu, Dec 12, 2013 at 03:13:01PM -0800, Stephen Boyd wrote:
> 
> > > bulk_read() should decay to individual reads if there isn't a block
> > > operaton and it's not like the hardware actually supports bulk reads
> > > anyway.
> 
> > So regmap_bulk_read() should work if I don't have a map->bus? To make it
> > work with reg_read/write I had to do this. I'm not sure how to make
> > bulk_write work.

I came up with this (possibly ugly) patch. It works for my
specific case but I'm not sure if unpacking the val bits into an
unsigned int and passing that to _regmap_write() is sane. What do
you think?

---8<----
diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index 1ccd61b..333c059 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -1514,11 +1514,9 @@ int regmap_bulk_write(struct regmap *map, unsigned int reg, const void *val,
 {
 	int ret = 0, i;
 	size_t val_bytes = map->format.val_bytes;
-	void *wval;
+	void *wval = NULL;
 
-	if (!map->bus)
-		return -EINVAL;
-	if (!map->format.parse_inplace)
+	if (map->bus && !map->format.parse_inplace)
 		return -EINVAL;
 	if (reg % map->reg_stride)
 		return -EINVAL;
@@ -1528,7 +1526,7 @@ int regmap_bulk_write(struct regmap *map, unsigned int reg, const void *val,
 	/* No formatting is require if val_byte is 1 */
 	if (val_bytes == 1) {
 		wval = (void *)val;
-	} else {
+	} else if (map->bus) {
 		wval = kmemdup(val, val_count * val_bytes, GFP_KERNEL);
 		if (!wval) {
 			ret = -ENOMEM;
@@ -1538,24 +1536,37 @@ int regmap_bulk_write(struct regmap *map, unsigned int reg, const void *val,
 		for (i = 0; i < val_count * val_bytes; i += val_bytes)
 			map->format.parse_inplace(wval + i);
 	}
-	/*
-	 * Some devices does not support bulk write, for
-	 * them we have a series of single write operations.
-	 */
-	if (map->use_single_rw) {
-		for (i = 0; i < val_count; i++) {
-			ret = _regmap_raw_write(map,
+
+	if (map->bus) {
+		/*
+		 * Some devices does not support bulk write, for
+		 * them we have a series of single write operations.
+		 */
+		if (map->use_single_rw) {
+			for (i = 0; i < val_count; i++) {
+				ret = _regmap_raw_write(map,
 						reg + (i * map->reg_stride),
 						val + (i * val_bytes),
 						val_bytes);
-			if (ret != 0)
-				return ret;
+				if (ret != 0)
+					return ret;
+			}
+		} else {
+			ret = _regmap_raw_write(map, reg, wval,
+						val_bytes * val_count);
 		}
 	} else {
-		ret = _regmap_raw_write(map, reg, wval, val_bytes * val_count);
+		for (i = 0; i < val_count; i++) {
+			unsigned int ival;
+			ival = *(unsigned int *)(val + (i * val_bytes));
+			ret = _regmap_write(map, reg + (i * map->reg_stride),
+					ival);
+			if (ret != 0)
+				goto out;
+		}
 	}
 
-	if (val_bytes != 1)
+	if (val_bytes != 1 && map->bus)
 		kfree(wval);
 
 out:

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

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

* Re: [PATCH] regmap: Allow regmap_bulk_read() to work for "no-bus" regmaps
  2013-12-13 17:14                   ` Stephen Boyd
@ 2013-12-16 20:57                     ` Mark Brown
  -1 siblings, 0 replies; 77+ messages in thread
From: Mark Brown @ 2013-12-16 20:57 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Samuel Ortiz, Lee Jones, Srinivas Ramana, linux-kernel,
	linux-arm-msm, linux-arm-kernel

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

On Fri, Dec 13, 2013 at 09:14:07AM -0800, Stephen Boyd wrote:

> regmap_bulk_read() should decay to performing individual reads if
> we're using a "no-bus" regmap. Unfortunately, it returns an
> error because there is no map->bus pointer. Fix it.

Applied, thanks.

> > Yes, I'd expect the operation to work.  Your changes below are mostly
> > fine (we should add an additional check for values that aren't integer
> > numbers of bytes, I can add that) - can you send as a signed off patch
> > please and I'll apply?

> Here you go. Do you have any suggestions on how to make regmap_bulk_write()
> work?

Off the top of my head I'd expect it to just fall back onto
regmap_write() if it can't do raw I/O.  Or perhaps just change that loop
to do regmap_write() all the time since it's hardly the fast path.

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

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

* [PATCH] regmap: Allow regmap_bulk_read() to work for "no-bus" regmaps
@ 2013-12-16 20:57                     ` Mark Brown
  0 siblings, 0 replies; 77+ messages in thread
From: Mark Brown @ 2013-12-16 20:57 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Dec 13, 2013 at 09:14:07AM -0800, Stephen Boyd wrote:

> regmap_bulk_read() should decay to performing individual reads if
> we're using a "no-bus" regmap. Unfortunately, it returns an
> error because there is no map->bus pointer. Fix it.

Applied, thanks.

> > Yes, I'd expect the operation to work.  Your changes below are mostly
> > fine (we should add an additional check for values that aren't integer
> > numbers of bytes, I can add that) - can you send as a signed off patch
> > please and I'll apply?

> Here you go. Do you have any suggestions on how to make regmap_bulk_write()
> work?

Off the top of my head I'd expect it to just fall back onto
regmap_write() if it can't do raw I/O.  Or perhaps just change that loop
to do regmap_write() all the time since it's hardly the fast path.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131216/edc8d5c2/attachment.sig>

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

* Re: [PATCH 3/8] regmap: Add support for using regmap over ssbi
  2013-12-13 21:37                   ` Stephen Boyd
@ 2013-12-16 21:01                     ` Mark Brown
  -1 siblings, 0 replies; 77+ messages in thread
From: Mark Brown @ 2013-12-16 21:01 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Samuel Ortiz, Lee Jones, Srinivas Ramana, linux-kernel,
	linux-arm-msm, linux-arm-kernel

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

On Fri, Dec 13, 2013 at 01:37:07PM -0800, Stephen Boyd wrote:

> I came up with this (possibly ugly) patch. It works for my
> specific case but I'm not sure if unpacking the val bits into an
> unsigned int and passing that to _regmap_write() is sane. What do
> you think?

It's not lovely but it's about as good as it gets.  I'd probaly just
drop the raw single write case so it's simpler - just either raw write
the lot or write a register at a time with unpacking (and so refactor
the the loop that does the in place formatting into the raw case only
and not bother for the single write).

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

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

* [PATCH 3/8] regmap: Add support for using regmap over ssbi
@ 2013-12-16 21:01                     ` Mark Brown
  0 siblings, 0 replies; 77+ messages in thread
From: Mark Brown @ 2013-12-16 21:01 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Dec 13, 2013 at 01:37:07PM -0800, Stephen Boyd wrote:

> I came up with this (possibly ugly) patch. It works for my
> specific case but I'm not sure if unpacking the val bits into an
> unsigned int and passing that to _regmap_write() is sane. What do
> you think?

It's not lovely but it's about as good as it gets.  I'd probaly just
drop the raw single write case so it's simpler - just either raw write
the lot or write a register at a time with unpacking (and so refactor
the the loop that does the in place formatting into the raw case only
and not bother for the single write).
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131216/cf637c61/attachment.sig>

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

* [PATCH] regmap: Allow regmap_bulk_write() to work for "no-bus" regmaps
  2013-12-16 21:01                     ` Mark Brown
@ 2013-12-17  2:30                       ` Stephen Boyd
  -1 siblings, 0 replies; 77+ messages in thread
From: Stephen Boyd @ 2013-12-17  2:30 UTC (permalink / raw)
  To: Mark Brown
  Cc: Samuel Ortiz, Lee Jones, Srinivas Ramana, linux-kernel,
	linux-arm-msm, linux-arm-kernel

regmap_bulk_write() should decay to performing individual writes
if we're using a "no-bus" regmap. Unfortunately, it returns an
error because there is no map->bus pointer. Fix it.

Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---

On 12/16, Mark Brown wrote:
> On Fri, Dec 13, 2013 at 01:37:07PM -0800, Stephen Boyd wrote:
> 
> > I came up with this (possibly ugly) patch. It works for my
> > specific case but I'm not sure if unpacking the val bits into an
> > unsigned int and passing that to _regmap_write() is sane. What do
> > you think?
> 
> It's not lovely but it's about as good as it gets.  I'd probaly just
> drop the raw single write case so it's simpler - just either raw write
> the lot or write a register at a time with unpacking (and so refactor
> the the loop that does the in place formatting into the raw case only
> and not bother for the single write).

Ok how about this?

 drivers/base/regmap/regmap.c | 43 ++++++++++++++++++-------------------------
 1 file changed, 18 insertions(+), 25 deletions(-)

diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index 1ccd61b..12b80f6 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -1514,21 +1514,30 @@ int regmap_bulk_write(struct regmap *map, unsigned int reg, const void *val,
 {
 	int ret = 0, i;
 	size_t val_bytes = map->format.val_bytes;
-	void *wval;
 
-	if (!map->bus)
-		return -EINVAL;
-	if (!map->format.parse_inplace)
+	if (map->bus && !map->format.parse_inplace)
 		return -EINVAL;
 	if (reg % map->reg_stride)
 		return -EINVAL;
 
 	map->lock(map->lock_arg);
+	/*
+	 * Some devices don't support bulk write, for
+	 * them we have a series of single write operations.
+	 */
+	if (!map->bus || map->use_single_rw) {
+		for (i = 0; i < val_count; i++) {
+			unsigned int ival;
 
-	/* No formatting is require if val_byte is 1 */
-	if (val_bytes == 1) {
-		wval = (void *)val;
+			ival = *(unsigned int *)(val + (i * val_bytes));
+			ret = _regmap_write(map, reg + (i * map->reg_stride),
+					ival);
+			if (ret != 0)
+				goto out;
+		}
 	} else {
+		void *wval;
+
 		wval = kmemdup(val, val_count * val_bytes, GFP_KERNEL);
 		if (!wval) {
 			ret = -ENOMEM;
@@ -1537,27 +1546,11 @@ int regmap_bulk_write(struct regmap *map, unsigned int reg, const void *val,
 		}
 		for (i = 0; i < val_count * val_bytes; i += val_bytes)
 			map->format.parse_inplace(wval + i);
-	}
-	/*
-	 * Some devices does not support bulk write, for
-	 * them we have a series of single write operations.
-	 */
-	if (map->use_single_rw) {
-		for (i = 0; i < val_count; i++) {
-			ret = _regmap_raw_write(map,
-						reg + (i * map->reg_stride),
-						val + (i * val_bytes),
-						val_bytes);
-			if (ret != 0)
-				return ret;
-		}
-	} else {
+
 		ret = _regmap_raw_write(map, reg, wval, val_bytes * val_count);
-	}
 
-	if (val_bytes != 1)
 		kfree(wval);
-
+	}
 out:
 	map->unlock(map->lock_arg);
 	return ret;
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

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

* [PATCH] regmap: Allow regmap_bulk_write() to work for "no-bus" regmaps
@ 2013-12-17  2:30                       ` Stephen Boyd
  0 siblings, 0 replies; 77+ messages in thread
From: Stephen Boyd @ 2013-12-17  2:30 UTC (permalink / raw)
  To: linux-arm-kernel

regmap_bulk_write() should decay to performing individual writes
if we're using a "no-bus" regmap. Unfortunately, it returns an
error because there is no map->bus pointer. Fix it.

Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---

On 12/16, Mark Brown wrote:
> On Fri, Dec 13, 2013 at 01:37:07PM -0800, Stephen Boyd wrote:
> 
> > I came up with this (possibly ugly) patch. It works for my
> > specific case but I'm not sure if unpacking the val bits into an
> > unsigned int and passing that to _regmap_write() is sane. What do
> > you think?
> 
> It's not lovely but it's about as good as it gets.  I'd probaly just
> drop the raw single write case so it's simpler - just either raw write
> the lot or write a register at a time with unpacking (and so refactor
> the the loop that does the in place formatting into the raw case only
> and not bother for the single write).

Ok how about this?

 drivers/base/regmap/regmap.c | 43 ++++++++++++++++++-------------------------
 1 file changed, 18 insertions(+), 25 deletions(-)

diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index 1ccd61b..12b80f6 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -1514,21 +1514,30 @@ int regmap_bulk_write(struct regmap *map, unsigned int reg, const void *val,
 {
 	int ret = 0, i;
 	size_t val_bytes = map->format.val_bytes;
-	void *wval;
 
-	if (!map->bus)
-		return -EINVAL;
-	if (!map->format.parse_inplace)
+	if (map->bus && !map->format.parse_inplace)
 		return -EINVAL;
 	if (reg % map->reg_stride)
 		return -EINVAL;
 
 	map->lock(map->lock_arg);
+	/*
+	 * Some devices don't support bulk write, for
+	 * them we have a series of single write operations.
+	 */
+	if (!map->bus || map->use_single_rw) {
+		for (i = 0; i < val_count; i++) {
+			unsigned int ival;
 
-	/* No formatting is require if val_byte is 1 */
-	if (val_bytes == 1) {
-		wval = (void *)val;
+			ival = *(unsigned int *)(val + (i * val_bytes));
+			ret = _regmap_write(map, reg + (i * map->reg_stride),
+					ival);
+			if (ret != 0)
+				goto out;
+		}
 	} else {
+		void *wval;
+
 		wval = kmemdup(val, val_count * val_bytes, GFP_KERNEL);
 		if (!wval) {
 			ret = -ENOMEM;
@@ -1537,27 +1546,11 @@ int regmap_bulk_write(struct regmap *map, unsigned int reg, const void *val,
 		}
 		for (i = 0; i < val_count * val_bytes; i += val_bytes)
 			map->format.parse_inplace(wval + i);
-	}
-	/*
-	 * Some devices does not support bulk write, for
-	 * them we have a series of single write operations.
-	 */
-	if (map->use_single_rw) {
-		for (i = 0; i < val_count; i++) {
-			ret = _regmap_raw_write(map,
-						reg + (i * map->reg_stride),
-						val + (i * val_bytes),
-						val_bytes);
-			if (ret != 0)
-				return ret;
-		}
-	} else {
+
 		ret = _regmap_raw_write(map, reg, wval, val_bytes * val_count);
-	}
 
-	if (val_bytes != 1)
 		kfree(wval);
-
+	}
 out:
 	map->unlock(map->lock_arg);
 	return ret;
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

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

* Re: [PATCH] regmap: Allow regmap_bulk_write() to work for "no-bus" regmaps
  2013-12-17  2:30                       ` Stephen Boyd
@ 2013-12-18 18:45                         ` Mark Brown
  -1 siblings, 0 replies; 77+ messages in thread
From: Mark Brown @ 2013-12-18 18:45 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Samuel Ortiz, Lee Jones, Srinivas Ramana, linux-kernel,
	linux-arm-msm, linux-arm-kernel

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

On Mon, Dec 16, 2013 at 06:30:47PM -0800, Stephen Boyd wrote:

> -	/* No formatting is require if val_byte is 1 */
> -	if (val_bytes == 1) {
> -		wval = (void *)val;
> +			ival = *(unsigned int *)(val + (i * val_bytes));
> +			ret = _regmap_write(map, reg + (i * map->reg_stride),
> +					ival);
> +			if (ret != 0)
> +				goto out;

This doesn't quite work - val is an array of objects of the size of the
size of a register not of unsigned integers so you're parsing extra data
out there.  That possibly wasn't the best choice of API but we have
quite a few users now so ick.

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

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

* [PATCH] regmap: Allow regmap_bulk_write() to work for "no-bus" regmaps
@ 2013-12-18 18:45                         ` Mark Brown
  0 siblings, 0 replies; 77+ messages in thread
From: Mark Brown @ 2013-12-18 18:45 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Dec 16, 2013 at 06:30:47PM -0800, Stephen Boyd wrote:

> -	/* No formatting is require if val_byte is 1 */
> -	if (val_bytes == 1) {
> -		wval = (void *)val;
> +			ival = *(unsigned int *)(val + (i * val_bytes));
> +			ret = _regmap_write(map, reg + (i * map->reg_stride),
> +					ival);
> +			if (ret != 0)
> +				goto out;

This doesn't quite work - val is an array of objects of the size of the
size of a register not of unsigned integers so you're parsing extra data
out there.  That possibly wasn't the best choice of API but we have
quite a few users now so ick.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131218/035c6bc6/attachment.sig>

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

* Re: [PATCH] regmap: Allow regmap_bulk_write() to work for "no-bus" regmaps
  2013-12-18 18:45                         ` Mark Brown
@ 2013-12-23 20:05                           ` Stephen Boyd
  -1 siblings, 0 replies; 77+ messages in thread
From: Stephen Boyd @ 2013-12-23 20:05 UTC (permalink / raw)
  To: Mark Brown
  Cc: Samuel Ortiz, Lee Jones, Srinivas Ramana, linux-kernel,
	linux-arm-msm, linux-arm-kernel

On 12/18/13 10:45, Mark Brown wrote:
> On Mon, Dec 16, 2013 at 06:30:47PM -0800, Stephen Boyd wrote:
>
>> -	/* No formatting is require if val_byte is 1 */
>> -	if (val_bytes == 1) {
>> -		wval = (void *)val;
>> +			ival = *(unsigned int *)(val + (i * val_bytes));
>> +			ret = _regmap_write(map, reg + (i * map->reg_stride),
>> +					ival);
>> +			if (ret != 0)
>> +				goto out;
> This doesn't quite work - val is an array of objects of the size of the
> size of a register not of unsigned integers so you're parsing extra data
> out there.  That possibly wasn't the best choice of API but we have
> quite a few users now so ick.

Are you concerned that we'll read past the end of the val buffer? Do we
need to cast the pointer to be the appropriate size according to
val_bytes? Something like this?

                for (i = 0; i < val_count; i++) {
                        unsigned int ival;

                        switch (val_bytes) {
                        case 1: 
                                ival = *(u8 *)(val + (i * val_bytes));
                                break;
                        case 2: 
                                ival = *(u16 *)(val + (i * val_bytes));
                                break;
                        case 4: 
                                ival = *(u32 *)(val + (i * val_bytes));
                                break;
#ifdef CONFIG_64BIT
                        case 8: 
                                ival = *(u64 *)(val + (i * val_bytes));
                                break;
#endif
                        default:
                                ret = -EINVAL;
                                goto out;
                        }

                        ret = _regmap_write(map, reg + (i * map->reg_stride),
                                        ival);
                        if (ret != 0)
                                goto out;
                }

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

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

* [PATCH] regmap: Allow regmap_bulk_write() to work for "no-bus" regmaps
@ 2013-12-23 20:05                           ` Stephen Boyd
  0 siblings, 0 replies; 77+ messages in thread
From: Stephen Boyd @ 2013-12-23 20:05 UTC (permalink / raw)
  To: linux-arm-kernel

On 12/18/13 10:45, Mark Brown wrote:
> On Mon, Dec 16, 2013 at 06:30:47PM -0800, Stephen Boyd wrote:
>
>> -	/* No formatting is require if val_byte is 1 */
>> -	if (val_bytes == 1) {
>> -		wval = (void *)val;
>> +			ival = *(unsigned int *)(val + (i * val_bytes));
>> +			ret = _regmap_write(map, reg + (i * map->reg_stride),
>> +					ival);
>> +			if (ret != 0)
>> +				goto out;
> This doesn't quite work - val is an array of objects of the size of the
> size of a register not of unsigned integers so you're parsing extra data
> out there.  That possibly wasn't the best choice of API but we have
> quite a few users now so ick.

Are you concerned that we'll read past the end of the val buffer? Do we
need to cast the pointer to be the appropriate size according to
val_bytes? Something like this?

                for (i = 0; i < val_count; i++) {
                        unsigned int ival;

                        switch (val_bytes) {
                        case 1: 
                                ival = *(u8 *)(val + (i * val_bytes));
                                break;
                        case 2: 
                                ival = *(u16 *)(val + (i * val_bytes));
                                break;
                        case 4: 
                                ival = *(u32 *)(val + (i * val_bytes));
                                break;
#ifdef CONFIG_64BIT
                        case 8: 
                                ival = *(u64 *)(val + (i * val_bytes));
                                break;
#endif
                        default:
                                ret = -EINVAL;
                                goto out;
                        }

                        ret = _regmap_write(map, reg + (i * map->reg_stride),
                                        ival);
                        if (ret != 0)
                                goto out;
                }

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

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

* Re: [PATCH] regmap: Allow regmap_bulk_write() to work for "no-bus" regmaps
  2013-12-23 20:05                           ` Stephen Boyd
@ 2013-12-24 12:53                             ` Mark Brown
  -1 siblings, 0 replies; 77+ messages in thread
From: Mark Brown @ 2013-12-24 12:53 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Samuel Ortiz, Lee Jones, Srinivas Ramana, linux-kernel,
	linux-arm-msm, linux-arm-kernel

On Mon, Dec 23, 2013 at 12:05:53PM -0800, Stephen Boyd wrote:
> On 12/18/13 10:45, Mark Brown wrote:

> > This doesn't quite work - val is an array of objects of the size of the
> > size of a register not of unsigned integers so you're parsing extra data
> > out there.  That possibly wasn't the best choice of API but we have
> > quite a few users now so ick.

> Are you concerned that we'll read past the end of the val buffer? Do we
> need to cast the pointer to be the appropriate size according to
> val_bytes? Something like this?

That's one issue, the other is that if we try to read (say) and 8 bit
value as an unsigned int we'll not just read the value we're looking
for.

>                 for (i = 0; i < val_count; i++) {
>                         unsigned int ival;
> 
>                         switch (val_bytes) {
>                         case 1: 
>                                 ival = *(u8 *)(val + (i * val_bytes));
>                                 break;

I think we do sadly.  Or refactor the API to work in unsigned ints
which would've been more sensible in the first place but that'd make it
asymmetrical with the read API as it stands...

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

* [PATCH] regmap: Allow regmap_bulk_write() to work for "no-bus" regmaps
@ 2013-12-24 12:53                             ` Mark Brown
  0 siblings, 0 replies; 77+ messages in thread
From: Mark Brown @ 2013-12-24 12:53 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Dec 23, 2013 at 12:05:53PM -0800, Stephen Boyd wrote:
> On 12/18/13 10:45, Mark Brown wrote:

> > This doesn't quite work - val is an array of objects of the size of the
> > size of a register not of unsigned integers so you're parsing extra data
> > out there.  That possibly wasn't the best choice of API but we have
> > quite a few users now so ick.

> Are you concerned that we'll read past the end of the val buffer? Do we
> need to cast the pointer to be the appropriate size according to
> val_bytes? Something like this?

That's one issue, the other is that if we try to read (say) and 8 bit
value as an unsigned int we'll not just read the value we're looking
for.

>                 for (i = 0; i < val_count; i++) {
>                         unsigned int ival;
> 
>                         switch (val_bytes) {
>                         case 1: 
>                                 ival = *(u8 *)(val + (i * val_bytes));
>                                 break;

I think we do sadly.  Or refactor the API to work in unsigned ints
which would've been more sensible in the first place but that'd make it
asymmetrical with the read API as it stands...

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

* Re: [PATCH] regmap: Allow regmap_bulk_write() to work for "no-bus" regmaps
  2013-12-24 12:53                             ` Mark Brown
@ 2013-12-26 19:34                               ` Stephen Boyd
  -1 siblings, 0 replies; 77+ messages in thread
From: Stephen Boyd @ 2013-12-26 19:34 UTC (permalink / raw)
  To: Mark Brown
  Cc: Samuel Ortiz, Lee Jones, Srinivas Ramana, linux-kernel,
	linux-arm-msm, linux-arm-kernel

On 12/24, Mark Brown wrote:
> On Mon, Dec 23, 2013 at 12:05:53PM -0800, Stephen Boyd wrote:
> > On 12/18/13 10:45, Mark Brown wrote:
> 
> > > This doesn't quite work - val is an array of objects of the size of the
> > > size of a register not of unsigned integers so you're parsing extra data
> > > out there.  That possibly wasn't the best choice of API but we have
> > > quite a few users now so ick.
> 
> > Are you concerned that we'll read past the end of the val buffer? Do we
> > need to cast the pointer to be the appropriate size according to
> > val_bytes? Something like this?
> 
> That's one issue, the other is that if we try to read (say) and 8 bit
> value as an unsigned int we'll not just read the value we're looking
> for.

Ah right. My no-bus implementation was clearing out the upper 24
bits of the word so I could just send the u8 value. With this
approach that isn't necessary.

> 
> >                 for (i = 0; i < val_count; i++) {
> >                         unsigned int ival;
> > 
> >                         switch (val_bytes) {
> >                         case 1: 
> >                                 ival = *(u8 *)(val + (i * val_bytes));
> >                                 break;
> 
> I think we do sadly.  Or refactor the API to work in unsigned ints
> which would've been more sensible in the first place but that'd make it
> asymmetrical with the read API as it stands...

Ok it sounds like you're willing to go with this updated loop.
I'll resend a proper patch.

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

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

* [PATCH] regmap: Allow regmap_bulk_write() to work for "no-bus" regmaps
@ 2013-12-26 19:34                               ` Stephen Boyd
  0 siblings, 0 replies; 77+ messages in thread
From: Stephen Boyd @ 2013-12-26 19:34 UTC (permalink / raw)
  To: linux-arm-kernel

On 12/24, Mark Brown wrote:
> On Mon, Dec 23, 2013 at 12:05:53PM -0800, Stephen Boyd wrote:
> > On 12/18/13 10:45, Mark Brown wrote:
> 
> > > This doesn't quite work - val is an array of objects of the size of the
> > > size of a register not of unsigned integers so you're parsing extra data
> > > out there.  That possibly wasn't the best choice of API but we have
> > > quite a few users now so ick.
> 
> > Are you concerned that we'll read past the end of the val buffer? Do we
> > need to cast the pointer to be the appropriate size according to
> > val_bytes? Something like this?
> 
> That's one issue, the other is that if we try to read (say) and 8 bit
> value as an unsigned int we'll not just read the value we're looking
> for.

Ah right. My no-bus implementation was clearing out the upper 24
bits of the word so I could just send the u8 value. With this
approach that isn't necessary.

> 
> >                 for (i = 0; i < val_count; i++) {
> >                         unsigned int ival;
> > 
> >                         switch (val_bytes) {
> >                         case 1: 
> >                                 ival = *(u8 *)(val + (i * val_bytes));
> >                                 break;
> 
> I think we do sadly.  Or refactor the API to work in unsigned ints
> which would've been more sensible in the first place but that'd make it
> asymmetrical with the read API as it stands...

Ok it sounds like you're willing to go with this updated loop.
I'll resend a proper patch.

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

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

* [PATCH v2] regmap: Allow regmap_bulk_write() to work for "no-bus" regmaps
  2013-12-26 19:34                               ` Stephen Boyd
@ 2013-12-26 21:52                                 ` Stephen Boyd
  -1 siblings, 0 replies; 77+ messages in thread
From: Stephen Boyd @ 2013-12-26 21:52 UTC (permalink / raw)
  To: Mark Brown
  Cc: Samuel Ortiz, Lee Jones, Srinivas Ramana, linux-kernel,
	linux-arm-msm, linux-arm-kernel

regmap_bulk_write() should decay to performing individual writes
if we're using a "no-bus" regmap. Unfortunately, it returns an
error because there is no map->bus pointer. Fix it.

Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---

This is based on v3.13-rc4

 drivers/base/regmap/regmap.c | 62 ++++++++++++++++++++++++++------------------
 1 file changed, 37 insertions(+), 25 deletions(-)

diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index d043d80..13d32fd 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -1514,21 +1514,49 @@ int regmap_bulk_write(struct regmap *map, unsigned int reg, const void *val,
 {
 	int ret = 0, i;
 	size_t val_bytes = map->format.val_bytes;
-	void *wval;
 
-	if (!map->bus)
-		return -EINVAL;
-	if (!map->format.parse_inplace)
+	if (map->bus && !map->format.parse_inplace)
 		return -EINVAL;
 	if (reg % map->reg_stride)
 		return -EINVAL;
 
 	map->lock(map->lock_arg);
+	/*
+	 * Some devices don't support bulk write, for
+	 * them we have a series of single write operations.
+	 */
+	if (!map->bus || map->use_single_rw) {
+		for (i = 0; i < val_count; i++) {
+			unsigned int ival;
+
+			switch (val_bytes) {
+			case 1:
+				ival = *(u8 *)(val + (i * val_bytes));
+				break;
+			case 2:
+				ival = *(u16 *)(val + (i * val_bytes));
+				break;
+			case 4:
+				ival = *(u32 *)(val + (i * val_bytes));
+				break;
+#ifdef CONFIG_64BIT
+			case 8:
+				ival = *(u64 *)(val + (i * val_bytes));
+				break;
+#endif
+			default:
+				ret = -EINVAL;
+				goto out;
+			}
 
-	/* No formatting is require if val_byte is 1 */
-	if (val_bytes == 1) {
-		wval = (void *)val;
+			ret = _regmap_write(map, reg + (i * map->reg_stride),
+					ival);
+			if (ret != 0)
+				goto out;
+		}
 	} else {
+		void *wval;
+
 		wval = kmemdup(val, val_count * val_bytes, GFP_KERNEL);
 		if (!wval) {
 			ret = -ENOMEM;
@@ -1537,27 +1565,11 @@ int regmap_bulk_write(struct regmap *map, unsigned int reg, const void *val,
 		}
 		for (i = 0; i < val_count * val_bytes; i += val_bytes)
 			map->format.parse_inplace(wval + i);
-	}
-	/*
-	 * Some devices does not support bulk write, for
-	 * them we have a series of single write operations.
-	 */
-	if (map->use_single_rw) {
-		for (i = 0; i < val_count; i++) {
-			ret = _regmap_raw_write(map,
-						reg + (i * map->reg_stride),
-						val + (i * val_bytes),
-						val_bytes);
-			if (ret != 0)
-				goto out;
-		}
-	} else {
+
 		ret = _regmap_raw_write(map, reg, wval, val_bytes * val_count);
-	}
 
-	if (val_bytes != 1)
 		kfree(wval);
-
+	}
 out:
 	map->unlock(map->lock_arg);
 	return ret;
-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

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

* [PATCH v2] regmap: Allow regmap_bulk_write() to work for "no-bus" regmaps
@ 2013-12-26 21:52                                 ` Stephen Boyd
  0 siblings, 0 replies; 77+ messages in thread
From: Stephen Boyd @ 2013-12-26 21:52 UTC (permalink / raw)
  To: linux-arm-kernel

regmap_bulk_write() should decay to performing individual writes
if we're using a "no-bus" regmap. Unfortunately, it returns an
error because there is no map->bus pointer. Fix it.

Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---

This is based on v3.13-rc4

 drivers/base/regmap/regmap.c | 62 ++++++++++++++++++++++++++------------------
 1 file changed, 37 insertions(+), 25 deletions(-)

diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index d043d80..13d32fd 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -1514,21 +1514,49 @@ int regmap_bulk_write(struct regmap *map, unsigned int reg, const void *val,
 {
 	int ret = 0, i;
 	size_t val_bytes = map->format.val_bytes;
-	void *wval;
 
-	if (!map->bus)
-		return -EINVAL;
-	if (!map->format.parse_inplace)
+	if (map->bus && !map->format.parse_inplace)
 		return -EINVAL;
 	if (reg % map->reg_stride)
 		return -EINVAL;
 
 	map->lock(map->lock_arg);
+	/*
+	 * Some devices don't support bulk write, for
+	 * them we have a series of single write operations.
+	 */
+	if (!map->bus || map->use_single_rw) {
+		for (i = 0; i < val_count; i++) {
+			unsigned int ival;
+
+			switch (val_bytes) {
+			case 1:
+				ival = *(u8 *)(val + (i * val_bytes));
+				break;
+			case 2:
+				ival = *(u16 *)(val + (i * val_bytes));
+				break;
+			case 4:
+				ival = *(u32 *)(val + (i * val_bytes));
+				break;
+#ifdef CONFIG_64BIT
+			case 8:
+				ival = *(u64 *)(val + (i * val_bytes));
+				break;
+#endif
+			default:
+				ret = -EINVAL;
+				goto out;
+			}
 
-	/* No formatting is require if val_byte is 1 */
-	if (val_bytes == 1) {
-		wval = (void *)val;
+			ret = _regmap_write(map, reg + (i * map->reg_stride),
+					ival);
+			if (ret != 0)
+				goto out;
+		}
 	} else {
+		void *wval;
+
 		wval = kmemdup(val, val_count * val_bytes, GFP_KERNEL);
 		if (!wval) {
 			ret = -ENOMEM;
@@ -1537,27 +1565,11 @@ int regmap_bulk_write(struct regmap *map, unsigned int reg, const void *val,
 		}
 		for (i = 0; i < val_count * val_bytes; i += val_bytes)
 			map->format.parse_inplace(wval + i);
-	}
-	/*
-	 * Some devices does not support bulk write, for
-	 * them we have a series of single write operations.
-	 */
-	if (map->use_single_rw) {
-		for (i = 0; i < val_count; i++) {
-			ret = _regmap_raw_write(map,
-						reg + (i * map->reg_stride),
-						val + (i * val_bytes),
-						val_bytes);
-			if (ret != 0)
-				goto out;
-		}
-	} else {
+
 		ret = _regmap_raw_write(map, reg, wval, val_bytes * val_count);
-	}
 
-	if (val_bytes != 1)
 		kfree(wval);
-
+	}
 out:
 	map->unlock(map->lock_arg);
 	return ret;
-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

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

* Re: [PATCH v2] regmap: Allow regmap_bulk_write() to work for "no-bus" regmaps
  2013-12-26 21:52                                 ` Stephen Boyd
@ 2013-12-30 12:42                                   ` Mark Brown
  -1 siblings, 0 replies; 77+ messages in thread
From: Mark Brown @ 2013-12-30 12:42 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Samuel Ortiz, Lee Jones, Srinivas Ramana, linux-kernel,
	linux-arm-msm, linux-arm-kernel

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

On Thu, Dec 26, 2013 at 01:52:04PM -0800, Stephen Boyd wrote:
> regmap_bulk_write() should decay to performing individual writes
> if we're using a "no-bus" regmap. Unfortunately, it returns an
> error because there is no map->bus pointer. Fix it.

Applied, thanks.

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

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

* [PATCH v2] regmap: Allow regmap_bulk_write() to work for "no-bus" regmaps
@ 2013-12-30 12:42                                   ` Mark Brown
  0 siblings, 0 replies; 77+ messages in thread
From: Mark Brown @ 2013-12-30 12:42 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Dec 26, 2013 at 01:52:04PM -0800, Stephen Boyd wrote:
> regmap_bulk_write() should decay to performing individual writes
> if we're using a "no-bus" regmap. Unfortunately, it returns an
> error because there is no map->bus pointer. Fix it.

Applied, thanks.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131230/8e244316/attachment.sig>

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

end of thread, other threads:[~2013-12-30 12:43 UTC | newest]

Thread overview: 77+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-12-10 23:35 [PATCH 0/8] Modernize pm8921 with irqdomains + regmap Stephen Boyd
2013-12-10 23:35 ` Stephen Boyd
2013-12-10 23:35 ` [PATCH 1/8] mfd: ssbi: Remove platform data structs and hide ssbi type enum Stephen Boyd
2013-12-10 23:35   ` Stephen Boyd
2013-12-11  9:27   ` Lee Jones
2013-12-11  9:27     ` Lee Jones
2013-12-10 23:35 ` [PATCH 2/8] mfd: ssbi: Constify buffer in ssbi_write Stephen Boyd
2013-12-10 23:35   ` Stephen Boyd
2013-12-11  9:28   ` Lee Jones
2013-12-11  9:28     ` Lee Jones
2013-12-10 23:35 ` [PATCH 3/8] regmap: Add support for using regmap over ssbi Stephen Boyd
2013-12-10 23:35   ` Stephen Boyd
2013-12-10 23:50   ` Mark Brown
2013-12-10 23:50     ` Mark Brown
2013-12-11  0:13     ` Stephen Boyd
2013-12-11  0:13       ` Stephen Boyd
2013-12-11  0:51       ` Mark Brown
2013-12-11  0:51         ` Mark Brown
2013-12-11  1:32         ` Stephen Boyd
2013-12-11  1:32           ` Stephen Boyd
2013-12-11 13:27           ` Mark Brown
2013-12-11 13:27             ` Mark Brown
2013-12-12 23:13             ` Stephen Boyd
2013-12-12 23:13               ` Stephen Boyd
2013-12-13 10:41               ` Mark Brown
2013-12-13 10:41                 ` Mark Brown
2013-12-13 17:14                 ` [PATCH] regmap: Allow regmap_bulk_read() to work for "no-bus" regmaps Stephen Boyd
2013-12-13 17:14                   ` Stephen Boyd
2013-12-13 17:14                   ` Stephen Boyd
2013-12-16 20:57                   ` Mark Brown
2013-12-16 20:57                     ` Mark Brown
2013-12-13 21:37                 ` [PATCH 3/8] regmap: Add support for using regmap over ssbi Stephen Boyd
2013-12-13 21:37                   ` Stephen Boyd
2013-12-16 21:01                   ` Mark Brown
2013-12-16 21:01                     ` Mark Brown
2013-12-17  2:30                     ` [PATCH] regmap: Allow regmap_bulk_write() to work for "no-bus" regmaps Stephen Boyd
2013-12-17  2:30                       ` Stephen Boyd
2013-12-18 18:45                       ` Mark Brown
2013-12-18 18:45                         ` Mark Brown
2013-12-23 20:05                         ` Stephen Boyd
2013-12-23 20:05                           ` Stephen Boyd
2013-12-24 12:53                           ` Mark Brown
2013-12-24 12:53                             ` Mark Brown
2013-12-26 19:34                             ` Stephen Boyd
2013-12-26 19:34                               ` Stephen Boyd
2013-12-26 21:52                               ` [PATCH v2] " Stephen Boyd
2013-12-26 21:52                                 ` Stephen Boyd
2013-12-30 12:42                                 ` Mark Brown
2013-12-30 12:42                                   ` Mark Brown
2013-12-10 23:35 ` [PATCH 4/8] mfd: ssbi: Mark match table const Stephen Boyd
2013-12-10 23:35   ` Stephen Boyd
2013-12-11  9:29   ` Lee Jones
2013-12-11  9:29     ` Lee Jones
2013-12-10 23:35 ` [PATCH 5/8] mfd: Move pm8xxx-irq.c contents into only driver that uses it Stephen Boyd
2013-12-10 23:35   ` Stephen Boyd
2013-12-11  9:35   ` Lee Jones
2013-12-11  9:35     ` Lee Jones
2013-12-12 19:06     ` Stephen Boyd
2013-12-12 19:06       ` Stephen Boyd
2013-12-10 23:35 ` [PATCH 6/8] mfd: pm8921: Update for genirq changes Stephen Boyd
2013-12-10 23:35   ` Stephen Boyd
2013-12-11  9:48   ` Lee Jones
2013-12-11  9:48     ` Lee Jones
2013-12-10 23:35 ` [PATCH 7/8] mfd: pm8921: Migrate to irqdomains Stephen Boyd
2013-12-10 23:35   ` Stephen Boyd
2013-12-11  9:53   ` Lee Jones
2013-12-11  9:53     ` Lee Jones
2013-12-11 21:30   ` Courtney Cavin
2013-12-11 21:30     ` Courtney Cavin
2013-12-11 21:30     ` Courtney Cavin
2013-12-12 19:05     ` Stephen Boyd
2013-12-12 19:05       ` Stephen Boyd
2013-12-12 19:05       ` Stephen Boyd
2013-12-10 23:35 ` [PATCH 8/8] mfd: pm8921: Use ssbi regmap Stephen Boyd
2013-12-10 23:35   ` Stephen Boyd
2013-12-11  9:55   ` Lee Jones
2013-12-11  9:55     ` Lee Jones

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