[PATCH 0/8] da9055: Driver initialisation fixes, add DT support

[PATCH 0/8] da9055: Driver initialisation fixes, add DT support

[Adam Thomson]

This patch series provides the following updates for DA9055 drivers:

 - Fixes an issue with da9055 driver initialisation (conflicting device ids) of
   PMIC (MFD) and CODEC drivers.
 - Add initial DT support for DA9055 related drivers, including binding
   documentation.
 - Remove conflicting use of platform_get_irq_byname() in driver probes.
 - Remove unnecessary resource structures for MFD cells as a result of removing
   use of platform_get_irq_byname().

Adam Thomson (8):
  ASoC: da9055: Fix device registration of PMIC and CODEC devices
  ASoC: da9055: Add DT support for CODEC
  mfd: da9055: Add DT support for PMIC
  regulator: da9055: Add DT support
  onkey: da9055: Remove use of platform_get_irq_byname()
  hwmon: da9055: Remove use of platform_get_irq_byname()
  rtc: da9055: Remove use of platform_get_irq_byname()
  mfd: da9055: Remove unused resource structures for mfd cells.

 Documentation/devicetree/bindings/mfd/da9055.txt   |   73 ++++++++++++++++++++
 Documentation/devicetree/bindings/sound/da9055.txt |   22 ++++++
 drivers/hwmon/da9055-hwmon.c                       |    7 +--
 drivers/input/misc/da9055_onkey.c                  |    9 +--
 drivers/mfd/da9055-core.c                          |   46 ------------
 drivers/mfd/da9055-i2c.c                           |   20 +++++-
 drivers/regulator/da9055-regulator.c               |   59 ++++++++++++++--
 drivers/rtc/rtc-da9055.c                           |    3 +-
 sound/soc/codecs/da9055.c                          |   19 +++++-
 9 files changed, 188 insertions(+), 70 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mfd/da9055.txt
 create mode 100644 Documentation/devicetree/bindings/sound/da9055.txt

[PATCH 1/8] ASoC: da9055: Fix device registration of PMIC and CODEC devices

[Adam Thomson]

Currently the I2C device Ids conflict for the MFD and CODEC so
cannot be both instantiated on one platform. This patch updates
the Ids and names to make them unique from each other.

It should be noted that the I2C addresses for both PMIC and CODEC
are modifiable so instantiation of the two are kept as separate
devices, rather than instantiating the CODEC from the MFD code.

Signed-off-by: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
---
 drivers/mfd/da9055-i2c.c  |   12 ++++++++++--
 sound/soc/codecs/da9055.c |   11 +++++++++--
 2 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/drivers/mfd/da9055-i2c.c b/drivers/mfd/da9055-i2c.c
index 13af7e5..8103e43 100644
--- a/drivers/mfd/da9055-i2c.c
+++ b/drivers/mfd/da9055-i2c.c
@@ -53,17 +53,25 @@ static int da9055_i2c_remove(struct i2c_client *i2c)
 	return 0;
 }

+/*
+ * DO NOT change the device Ids. The naming is intentionally specific as both
+ * the PMIC and CODEC parts of this chip are instantiated separately as I2C
+ * devices (both have configurable I2C addresses, and are to all intents and
+ * purposes separate). As a result there are specific DA9055 ids for PMIC
+ * and CODEC, which must be different to operate together.
+ */
 static struct i2c_device_id da9055_i2c_id[] = {
-	{"da9055", 0},
+	{"da9055-pmic", 0},
 	{ }
 };
+MODULE_DEVICE_TABLE(i2c, da9055_i2c_id);

 static struct i2c_driver da9055_i2c_driver = {
 	.probe = da9055_i2c_probe,
 	.remove = da9055_i2c_remove,
 	.id_table = da9055_i2c_id,
 	.driver = {
-		.name = "da9055",
+		.name = "da9055-pmic",
 		.owner = THIS_MODULE,
 	},
 };
diff --git a/sound/soc/codecs/da9055.c b/sound/soc/codecs/da9055.c
index 52b79a4..4228126 100644
--- a/sound/soc/codecs/da9055.c
+++ b/sound/soc/codecs/da9055.c
@@ -1523,8 +1523,15 @@ static int da9055_remove(struct i2c_client *client)
 	return 0;
 }

+/*
+ * DO NOT change the device Ids. The naming is intentionally specific as both
+ * the CODEC and PMIC parts of this chip are instantiated separately as I2C
+ * devices (both have configurable I2C addresses, and are to all intents and
+ * purposes separate). As a result there are specific DA9055 Ids for CODEC
+ * and PMIC, which must be different to operate together.
+ */
 static const struct i2c_device_id da9055_i2c_id[] = {
-	{ "da9055", 0 },
+	{ "da9055-codec", 0 },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, da9055_i2c_id);
@@ -1532,7 +1539,7 @@ MODULE_DEVICE_TABLE(i2c, da9055_i2c_id);
 /* I2C codec control layer */
 static struct i2c_driver da9055_i2c_driver = {
 	.driver = {
-		.name = "da9055",
+		.name = "da9055-codec",
 		.owner = THIS_MODULE,
 	},
 	.probe		= da9055_i2c_probe,
--
1.7.0.4

Re: [PATCH 1/8] ASoC: da9055: Fix device registration of PMIC and CODEC devices

[Mark Brown]

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

On Wed, Feb 05, 2014 at 05:48:32PM +0000, Adam Thomson wrote:
> Currently the I2C device Ids conflict for the MFD and CODEC so
> cannot be both instantiated on one platform. This patch updates
> the Ids and names to make them unique from each other.

Acked-by: Mark Brown <broonie-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

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

[PATCH 2/8] ASoC: da9055: Add DT support for CODEC

[Adam Thomson]

Signed-off-by: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
---
 Documentation/devicetree/bindings/sound/da9055.txt |   22 ++++++++++++++++++++
 sound/soc/codecs/da9055.c                          |    8 +++++++
 2 files changed, 30 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/sound/da9055.txt

diff --git a/Documentation/devicetree/bindings/sound/da9055.txt b/Documentation/devicetree/bindings/sound/da9055.txt
new file mode 100644
index 0000000..ed1b7cc
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/da9055.txt
@@ -0,0 +1,22 @@
+* Dialog DA9055 Audio CODEC
+
+DA9055 provides Audio CODEC support (I2C only).
+
+The Audio CODEC device in DA9055 has it's own I2C address which is configurable,
+so the device is instantiated separately from the PMIC (MFD) device.
+
+For details on accompanying PMIC I2C device, see the following:
+Documentation/devicetree/bindings/mfd/da9055.txt
+
+Required properties:
+
+  - compatible: "dlg,da9055-codec"
+  - reg: Specifies the I2C slave address
+
+
+Example:
+
+	codec: da9055-codec@1a {
+		compatible = "dlg,da9055-codec";
+		reg = <0x1a>;
+	};
diff --git a/sound/soc/codecs/da9055.c b/sound/soc/codecs/da9055.c
index 4228126..be31f3c 100644
--- a/sound/soc/codecs/da9055.c
+++ b/sound/soc/codecs/da9055.c
@@ -18,6 +18,8 @@
 #include <linux/regmap.h>
 #include <linux/slab.h>
 #include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
 #include <sound/pcm.h>
 #include <sound/pcm_params.h>
 #include <sound/soc.h>
@@ -1536,11 +1538,17 @@ static const struct i2c_device_id da9055_i2c_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, da9055_i2c_id);

+static const struct of_device_id da9055_of_match[] = {
+	{ .compatible = "dlg,da9055-codec", },
+	{ }
+};
+
 /* I2C codec control layer */
 static struct i2c_driver da9055_i2c_driver = {
 	.driver = {
 		.name = "da9055-codec",
 		.owner = THIS_MODULE,
+		.of_match_table = of_match_ptr(da9055_of_match),
 	},
 	.probe		= da9055_i2c_probe,
 	.remove		= da9055_remove,
--
1.7.0.4

[PATCH 3/8] mfd: da9055: Add DT support for PMIC

[Adam Thomson]

Signed-off-by: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
---
 Documentation/devicetree/bindings/mfd/da9055.txt |   73 ++++++++++++++++++++++
 drivers/mfd/da9055-i2c.c                         |    8 +++
 2 files changed, 81 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mfd/da9055.txt

diff --git a/Documentation/devicetree/bindings/mfd/da9055.txt b/Documentation/devicetree/bindings/mfd/da9055.txt
new file mode 100644
index 0000000..f903c3f
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/da9055.txt
@@ -0,0 +1,73 @@
+* Dialog DA9055 Power Management Integrated Circuit (PMIC)
+
+DA9055 consists of a large and varied group of sub-devices (I2C Only):
+
+Device			 Supply Names	 Description
+------			 ------------	 -----------
+da9055-gpio		:		: GPIOs
+da9055-regulator	:		: Regulators
+da9055-onkey		:		: On key
+da9055-rtc		:		: RTC
+da9055-hwmon		:		: ADC
+da9055-watchdog		:		: Watchdog
+
+The CODEC device in DA9055 has a separate, configurable I2C address and so
+is instantiated separately from the PMIC.
+
+For details on accompanying CODEC I2C device, see the following:
+Documentation/devicetree/bindings/sound/da9055.txt
+
+======
+
+Required properties:
+- compatible : Should be "dlg,da9055-pmic"
+- reg: Specifies the I2C slave address (defaults to 0x5a but can be modified)
+- interrupt-parent: Specifies the phandle of the interrupt controller to which
+  the IRQs from da9055 are delivered to.
+- interrupts: IRQ line info for da9055 chip.
+- interrupt-controller: da9055 has internal IRQs (has own IRQ domain).
+- #interrupt-cells: Should be 1, is the local IRQ number for da9055.
+
+Sub-nodes:
+- regulators : Contain the regulator nodes. The DA9055 regulators are
+  bound using their names as listed below:
+
+    buck1     : regulator BUCK1
+    buck2     : regulator BUCK2
+    ldo1      : regulator LDO1
+    ldo2      : regulator LDO2
+    ldo3      : regulator LDO3
+    ldo4      : regulator LDO4
+    ldo5      : regulator LDO5
+    ldo6      : regulator LDO6
+
+  The bindings details of individual regulator device can be found in:
+  Documentation/devicetree/bindings/regulator/regulator.txt
+
+
+Example:
+
+	pmic: da9055-pmic@5a {
+		compatible = "dlg,da9055-pmic";
+		reg = <0x5a>;
+		interrupt-parent = <&intc>;
+		interrupts = <5 0x8>;
+		interrupt-controller;
+		#interrupt-cells = <1>;
+
+		regulators {
+			buck1: BUCK1 {
+				regulator-min-microvolt = <725000>;
+				regulator-max-microvolt = <2075000>;
+			};
+
+			buck2: BUCK2 {
+				regulator-min-microvolt = <925000>;
+				regulator-max-microvolt = <2500000>;
+			};
+			ldo1: LDO1 {
+				regulator-min-microvolt = <900000>;
+				regulator-max-microvolt = <3300000>;
+			};
+		};
+	};
diff --git a/drivers/mfd/da9055-i2c.c b/drivers/mfd/da9055-i2c.c
index 8103e43..366a3e2 100644
--- a/drivers/mfd/da9055-i2c.c
+++ b/drivers/mfd/da9055-i2c.c
@@ -15,6 +15,8 @@
 #include <linux/device.h>
 #include <linux/i2c.h>
 #include <linux/err.h>
+#include <linux/of.h>
+#include <linux/of_device.h>

 #include <linux/mfd/da9055/core.h>

@@ -66,6 +68,11 @@ static struct i2c_device_id da9055_i2c_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, da9055_i2c_id);

+static const struct of_device_id da9055_of_match[] = {
+	{ .compatible = "dlg,da9055-pmic", .data = (void *)&da9055_i2c_id[0] },
+	{ }
+};
+
 static struct i2c_driver da9055_i2c_driver = {
 	.probe = da9055_i2c_probe,
 	.remove = da9055_i2c_remove,
@@ -73,6 +80,7 @@ static struct i2c_driver da9055_i2c_driver = {
 	.driver = {
 		.name = "da9055-pmic",
 		.owner = THIS_MODULE,
+		.of_match_table = of_match_ptr(da9055_of_match),
 	},
 };

--
1.7.0.4

[PATCH 4/8] regulator: da9055: Add DT support

[Adam Thomson]

Signed-off-by: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
---
 drivers/regulator/da9055-regulator.c |   59 +++++++++++++++++++++++++++++++---
 1 files changed, 54 insertions(+), 5 deletions(-)

diff --git a/drivers/regulator/da9055-regulator.c b/drivers/regulator/da9055-regulator.c
index 7f34020..bc41137 100644
--- a/drivers/regulator/da9055-regulator.c
+++ b/drivers/regulator/da9055-regulator.c
@@ -19,6 +19,10 @@
 #include <linux/platform_device.h>
 #include <linux/regulator/driver.h>
 #include <linux/regulator/machine.h>
+#ifdef CONFIG_OF
+#include <linux/of.h>
+#include <linux/regulator/of_regulator.h>
+#endif /* CONFIG_OF */

 #include <linux/mfd/da9055/core.h>
 #include <linux/mfd/da9055/reg.h>
@@ -446,6 +450,9 @@ static int da9055_gpio_init(struct da9055_regulator *regulator,
 	struct da9055_regulator_info *info = regulator->info;
 	int ret = 0;

+	if (!pdata)
+		return 0;
+
 	if (pdata->gpio_ren && pdata->gpio_ren[id]) {
 		char name[18];
 		int gpio_mux = pdata->gpio_ren[id];
@@ -530,6 +537,46 @@ static inline struct da9055_regulator_info *find_regulator_info(int id)
 	return NULL;
 }

+#ifdef CONFIG_OF
+static int da9055_regulator_dt_init(struct platform_device *pdev,
+				    struct da9055_regulator *regulator,
+				    struct regulator_config *config)
+{
+	struct device_node *nproot, *np;
+
+	nproot = of_node_get(pdev->dev.parent->of_node);
+	if (!nproot)
+		return -ENODEV;
+
+	nproot = of_find_node_by_name(nproot, "regulators");
+	if (!nproot)
+		return -ENODEV;
+
+	for_each_child_of_node(nproot, np) {
+		if (!of_node_cmp(np->name,
+				 regulator->info->reg_desc.name)) {
+			config->init_data = of_get_regulator_init_data(
+				&pdev->dev, np);
+			config->of_node = np;
+			break;
+		}
+	}
+	of_node_put(nproot);
+
+	if (!config->of_node)
+		return -ENODEV;
+
+	return 0;
+}
+#else
+static inline da9055_regulator_dt_init(struct platform_device *pdev,
+				       struct da9055_regulator *regulator,
+				       struct regulator_config *config)
+{
+	return -ENODEV;
+}
+#endif /* CONFIG_OF */
+
 static int da9055_regulator_probe(struct platform_device *pdev)
 {
 	struct regulator_config config = { };
@@ -538,9 +585,6 @@ static int da9055_regulator_probe(struct platform_device *pdev)
 	struct da9055_pdata *pdata = dev_get_platdata(da9055->dev);
 	int ret, irq;

-	if (pdata == NULL || pdata->regulators[pdev->id] == NULL)
-		return -ENODEV;
-
 	regulator = devm_kzalloc(&pdev->dev, sizeof(struct da9055_regulator),
 				 GFP_KERNEL);
 	if (!regulator)
@@ -559,6 +603,11 @@ static int da9055_regulator_probe(struct platform_device *pdev)

 	if (pdata && pdata->regulators)
 		config.init_data = pdata->regulators[pdev->id];
+	else {
+		ret = da9055_regulator_dt_init(pdev, regulator, &config);
+		if (ret < 0)
+			return ret;
+	}

 	ret = da9055_gpio_init(regulator, &config, pdata, pdev->id);
 	if (ret < 0)
@@ -575,8 +624,8 @@ static int da9055_regulator_probe(struct platform_device *pdev)

 	/* Only LDO 5 and 6 has got the over current interrupt */
 	if (pdev->id == DA9055_ID_LDO5 || pdev->id ==  DA9055_ID_LDO6) {
-		irq = platform_get_irq_byname(pdev, "REGULATOR");
-		irq = regmap_irq_get_virq(da9055->irq_data, irq);
+		irq = regmap_irq_get_virq(da9055->irq_data,
+					  DA9055_IRQ_REGULATOR);
 		ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
 						da9055_ldo5_6_oc_irq,
 						IRQF_TRIGGER_HIGH |
--
1.7.0.4

Re: [PATCH 4/8] regulator: da9055: Add DT support

[Mark Brown]

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

On Wed, Feb 05, 2014 at 05:48:35PM +0000, Adam Thomson wrote:

> +#ifdef CONFIG_OF
> +#include <linux/of.h>
> +#include <linux/regulator/of_regulator.h>
> +#endif /* CONFIG_OF */

Don't do ifdefs for includes like this, it's not worth it.

> +	for_each_child_of_node(nproot, np) {
> +		if (!of_node_cmp(np->name,
> +				 regulator->info->reg_desc.name)) {
> +			config->init_data = of_get_regulator_init_data(
> +				&pdev->dev, np);
> +			config->of_node = np;
> +			break;
> +		}
> +	}

I think you're looking for of_regulator_match() here.

>  	if (pdata && pdata->regulators)
>  		config.init_data = pdata->regulators[pdev->id];
> +	else {
> +		ret = da9055_regulator_dt_init(pdev, regulator, &config);
> +		if (ret < 0)
> +			return ret;
> +	}

Coding style, both sides of the if should have braces if one does.

>  	/* Only LDO 5 and 6 has got the over current interrupt */
>  	if (pdev->id == DA9055_ID_LDO5 || pdev->id ==  DA9055_ID_LDO6) {
> -		irq = platform_get_irq_byname(pdev, "REGULATOR");
> -		irq = regmap_irq_get_virq(da9055->irq_data, irq);
> +		irq = regmap_irq_get_virq(da9055->irq_data,
> +					  DA9055_IRQ_REGULATOR);

This seems like a bit of a step backwards - what happened in the MFD
(and why didn't it update the users to avoid breaking bisection)?

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

Re: [PATCH 4/8] regulator: da9055: Add DT support

[Opensource [Adam Thomson]]

On Wed, Feb 05, 2014 at 18:37:21PM +0000, Mark Brown wrote:

> On Wed, Feb 05, 2014 at 05:48:35PM +0000, Adam Thomson wrote:
> 
> > +#ifdef CONFIG_OF
> > +#include <linux/of.h>
> > +#include <linux/regulator/of_regulator.h>
> > +#endif /* CONFIG_OF */
> 
> Don't do ifdefs for includes like this, it's not worth it.

Fine. Seen examples of both in the kernel, but happy to remove it.

> 
> > +	for_each_child_of_node(nproot, np) {
> > +		if (!of_node_cmp(np->name,
> > +				 regulator->info->reg_desc.name)) {
> > +			config->init_data = of_get_regulator_init_data(
> > +				&pdev->dev, np);
> > +			config->of_node = np;
> > +			break;
> > +		}
> > +	}
> 
> I think you're looking for of_regulator_match() here.

Used another driver as an example for this, but if there's a better method
then I'm happy to use it. Will have a look.

> 
> >  	if (pdata && pdata->regulators)
> >  		config.init_data = pdata->regulators[pdev->id];
> > +	else {
> > +		ret = da9055_regulator_dt_init(pdev, regulator, &config);
> > +		if (ret < 0)
> > +			return ret;
> > +	}
> 
> Coding style, both sides of the if should have braces if one does.

Fine. Will update.

> 
> >  	/* Only LDO 5 and 6 has got the over current interrupt */
> >  	if (pdev->id == DA9055_ID_LDO5 || pdev->id ==  DA9055_ID_LDO6) {
> > -		irq = platform_get_irq_byname(pdev, "REGULATOR");
> > -		irq = regmap_irq_get_virq(da9055->irq_data, irq);
> > +		irq = regmap_irq_get_virq(da9055->irq_data,
> > +					  DA9055_IRQ_REGULATOR);
> 
> This seems like a bit of a step backwards - what happened in the MFD
> (and why didn't it update the users to avoid breaking bisection)?

I tested this on target, when doing tests for devicetree. What was happening was
that platform_get_irq_byname() was returning the VIRQ number already (368 in one
test case where onkey was being probed) rather than the local IRQ number for the
device (the resource information seemed to have been updated with the VIRQ
number instead of the local IRQ number). So when that was passed to
regmap_irq_get_virq() it would then return an incorrect IRQ number (0 in the
same scenario, when I enabled DEBUG in irqdomain.c, I would see the message
"error: hwirq 0x170 is too large for da9055_irq"). That incorrect irq was then
being passed to devm_request_threaded_irq() which subsequently failed. This is
why I made the change. Is it preferrable to use platform_get_irq_byname()
instead of regmap_irq_get_virq() as using them both doesn't seem to work, unless
I'm missing something fundamental here.

Re: [PATCH 4/8] regulator: da9055: Add DT support

[Mark Brown]

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

On Thu, Feb 06, 2014 at 11:31:13AM +0000, Opensource [Adam Thomson] wrote:
> On Wed, Feb 05, 2014 at 18:37:21PM +0000, Mark Brown wrote:

> > > +	for_each_child_of_node(nproot, np) {
> > > +		if (!of_node_cmp(np->name,
> > > +				 regulator->info->reg_desc.name)) {
> > > +			config->init_data = of_get_regulator_init_data(
> > > +				&pdev->dev, np);
> > > +			config->of_node = np;
> > > +			break;
> > > +		}
> > > +	}

> > I think you're looking for of_regulator_match() here.

> Used another driver as an example for this, but if there's a better method
> then I'm happy to use it. Will have a look.

That's probably an older driver - the code was factored out at some
point but lots of drivers don't get much love.

> > >  	/* Only LDO 5 and 6 has got the over current interrupt */
> > >  	if (pdev->id == DA9055_ID_LDO5 || pdev->id ==  DA9055_ID_LDO6) {
> > > -		irq = platform_get_irq_byname(pdev, "REGULATOR");
> > > -		irq = regmap_irq_get_virq(da9055->irq_data, irq);
> > > +		irq = regmap_irq_get_virq(da9055->irq_data,
> > > +					  DA9055_IRQ_REGULATOR);

> > This seems like a bit of a step backwards - what happened in the MFD
> > (and why didn't it update the users to avoid breaking bisection)?

> I tested this on target, when doing tests for devicetree. What was happening was
> that platform_get_irq_byname() was returning the VIRQ number already (368 in one
> test case where onkey was being probed) rather than the local IRQ number for the
> device (the resource information seemed to have been updated with the VIRQ
> number instead of the local IRQ number). So when that was passed to
> regmap_irq_get_virq() it would then return an incorrect IRQ number (0 in the
> same scenario, when I enabled DEBUG in irqdomain.c, I would see the message
> "error: hwirq 0x170 is too large for da9055_irq"). That incorrect irq was then
> being passed to devm_request_threaded_irq() which subsequently failed. This is
> why I made the change. Is it preferrable to use platform_get_irq_byname()
> instead of regmap_irq_get_virq() as using them both doesn't seem to work, unless
> I'm missing something fundamental here.

What's happening here is that the MFD framework has done the lookup for
you when passing the interrupt resource through - you should just use
platform_get_irq_byname() and save a little code in the driver.  If it's
behaving differently on DT and non-DT systems it seems better to figure
out why and then make it consistent.

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

RE: [PATCH 4/8] regulator: da9055: Add DT support

[Opensource [Adam Thomson]]

On Thu, Feb 06, 2014 at 12:04:52PM +0000, Mark Brown wrote:

> > Used another driver as an example for this, but if there's a better method
> > then I'm happy to use it. Will have a look.
> 
> That's probably an older driver - the code was factored out at some
> point but lots of drivers don't get much love.

Yep, fair enough. Will sort it.

> What's happening here is that the MFD framework has done the lookup for
> you when passing the interrupt resource through - you should just use
> platform_get_irq_byname() and save a little code in the driver.  If it's
> behaving differently on DT and non-DT systems it seems better to figure
> out why and then make it consistent.

Is what I summised but I obviously chose the wrong direction for my fix.
Will use platform_get_irq_byname() solely, and will make sure all works on
both DT and non-DT setups. If not I'll dig further.

Re: [PATCH 4/8] regulator: da9055: Add DT support

[Mark Brown]

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

On Thu, Feb 06, 2014 at 12:22:13PM +0000, Opensource [Adam Thomson] wrote:

> Is what I summised but I obviously chose the wrong direction for my fix.
> Will use platform_get_irq_byname() solely, and will make sure all works on
> both DT and non-DT setups. If not I'll dig further.

Yeah.  In general I'd say that if you've got two options for something
like this the option that makes the driver smaller or which replaces
more code with data is normally going to be the right way to go.

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

[PATCH 6/8] hwmon: da9055: Remove use of platform_get_irq_byname()

[Adam Thomson]

Signed-off-by: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
---
 drivers/hwmon/da9055-hwmon.c |    7 ++-----
 1 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/hwmon/da9055-hwmon.c b/drivers/hwmon/da9055-hwmon.c
index 029ecab..1867682 100644
--- a/drivers/hwmon/da9055-hwmon.c
+++ b/drivers/hwmon/da9055-hwmon.c
@@ -274,11 +274,8 @@ static int da9055_hwmon_probe(struct platform_device *pdev)

 	platform_set_drvdata(pdev, hwmon);

-	hwmon_irq = platform_get_irq_byname(pdev, "HWMON");
-	if (hwmon_irq < 0)
-		return hwmon_irq;
-
-	hwmon_irq = regmap_irq_get_virq(hwmon->da9055->irq_data, hwmon_irq);
+	hwmon_irq = regmap_irq_get_virq(hwmon->da9055->irq_data,
+					DA9055_IRQ_HWMON);
 	if (hwmon_irq < 0)
 		return hwmon_irq;

--
1.7.0.4

[PATCH 5/8] onkey: da9055: Remove use of platform_get_irq_byname()

[Adam Thomson]

Using platform_get_irq_byname() to retrieve the IRQ number
returns the VIRQ number rather than the local IRQ number for
the device. Passing that value then into regmap_irq_get_virq()
causes a failure because the function is expecting the local
IRQ number (e.g. 0, 1, 2, 3, etc). This patch removes use of
platform_get_irq_byname() and uses the local IRQ number instead.

Signed-off-by: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
---
 drivers/input/misc/da9055_onkey.c |    9 +--------
 1 files changed, 1 insertions(+), 8 deletions(-)

diff --git a/drivers/input/misc/da9055_onkey.c b/drivers/input/misc/da9055_onkey.c
index 4b11ede..4842c6c 100644
--- a/drivers/input/misc/da9055_onkey.c
+++ b/drivers/input/misc/da9055_onkey.c
@@ -79,13 +79,6 @@ static int da9055_onkey_probe(struct platform_device *pdev)
 	struct input_dev *input_dev;
 	int irq, err;

-	irq = platform_get_irq_byname(pdev, "ONKEY");
-	if (irq < 0) {
-		dev_err(&pdev->dev,
-			"Failed to get an IRQ for input device, %d\n", irq);
-		return -EINVAL;
-	}
-
 	onkey = devm_kzalloc(&pdev->dev, sizeof(*onkey), GFP_KERNEL);
 	if (!onkey) {
 		dev_err(&pdev->dev, "Failed to allocate memory\n");
@@ -109,7 +102,7 @@ static int da9055_onkey_probe(struct platform_device *pdev)

 	INIT_DELAYED_WORK(&onkey->work, da9055_onkey_work);

-	irq = regmap_irq_get_virq(da9055->irq_data, irq);
+	irq = regmap_irq_get_virq(da9055->irq_data, DA9055_IRQ_NONKEY);
 	err = request_threaded_irq(irq, NULL, da9055_onkey_irq,
 				   IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
 				   "ONKEY", onkey);
--
1.7.0.4

[PATCH 7/8] rtc: da9055: Remove use of platform_get_irq_byname()

[Adam Thomson]

Signed-off-by: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
---
 drivers/rtc/rtc-da9055.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/rtc/rtc-da9055.c b/drivers/rtc/rtc-da9055.c
index 48cb2ac..6e06840 100644
--- a/drivers/rtc/rtc-da9055.c
+++ b/drivers/rtc/rtc-da9055.c
@@ -301,8 +301,7 @@ static int da9055_rtc_probe(struct platform_device *pdev)
 		goto err_rtc;
 	}

-	alm_irq = platform_get_irq_byname(pdev, "ALM");
-	alm_irq = regmap_irq_get_virq(rtc->da9055->irq_data, alm_irq);
+	alm_irq = regmap_irq_get_virq(rtc->da9055->irq_data, DA9055_IRQ_ALARM);
 	ret = devm_request_threaded_irq(&pdev->dev, alm_irq, NULL,
 					da9055_rtc_alm_irq,
 					IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
--
1.7.0.4

[PATCH 8/8] mfd: da9055: Remove unused resource structures for mfd cells.

[Adam Thomson]

Signed-off-by: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
---
 drivers/mfd/da9055-core.c |   46 ---------------------------------------------
 1 files changed, 0 insertions(+), 46 deletions(-)

diff --git a/drivers/mfd/da9055-core.c b/drivers/mfd/da9055-core.c
index caf8dcf..5cf8ca6 100644
--- a/drivers/mfd/da9055-core.c
+++ b/drivers/mfd/da9055-core.c
@@ -258,42 +258,6 @@ struct regmap_config da9055_regmap_config = {
 };
 EXPORT_SYMBOL_GPL(da9055_regmap_config);

-static struct resource da9055_onkey_resource = {
-	.name = "ONKEY",
-	.start = DA9055_IRQ_NONKEY,
-	.end   = DA9055_IRQ_NONKEY,
-	.flags = IORESOURCE_IRQ,
-};
-
-static struct resource da9055_rtc_resource[] = {
-	{
-		.name = "ALM",
-		.start = DA9055_IRQ_ALARM,
-		.end   = DA9055_IRQ_ALARM,
-		.flags = IORESOURCE_IRQ,
-	},
-	{
-		.name = "TICK",
-		.start = DA9055_IRQ_TICK,
-		.end   = DA9055_IRQ_TICK,
-		.flags = IORESOURCE_IRQ,
-	},
-};
-
-static struct resource da9055_hwmon_resource = {
-	.name = "HWMON",
-	.start = DA9055_IRQ_HWMON,
-	.end   = DA9055_IRQ_HWMON,
-	.flags = IORESOURCE_IRQ,
-};
-
-static struct resource da9055_ld05_6_resource = {
-	.name = "REGULATOR",
-	.start = DA9055_IRQ_REGULATOR,
-	.end   = DA9055_IRQ_REGULATOR,
-	.flags = IORESOURCE_IRQ,
-};
-
 static const struct mfd_cell da9055_devs[] = {
 	{
 		.of_compatible = "dialog,da9055-gpio",
@@ -333,33 +297,23 @@ static const struct mfd_cell da9055_devs[] = {
 		.of_compatible = "dialog,da9055-regulator",
 		.name = "da9055-regulator",
 		.id = 7,
-		.resources = &da9055_ld05_6_resource,
-		.num_resources = 1,
 	},
 	{
 		.of_compatible = "dialog,da9055-regulator",
 		.name = "da9055-regulator",
-		.resources = &da9055_ld05_6_resource,
-		.num_resources = 1,
 		.id = 8,
 	},
 	{
 		.of_compatible = "dialog,da9055-onkey",
 		.name = "da9055-onkey",
-		.resources = &da9055_onkey_resource,
-		.num_resources = 1,
 	},
 	{
 		.of_compatible = "dialog,da9055-rtc",
 		.name = "da9055-rtc",
-		.resources = da9055_rtc_resource,
-		.num_resources = ARRAY_SIZE(da9055_rtc_resource),
 	},
 	{
 		.of_compatible = "dialog,da9055-hwmon",
 		.name = "da9055-hwmon",
-		.resources = &da9055_hwmon_resource,
-		.num_resources = 1,
 	},
 	{
 		.of_compatible = "dialog,da9055-watchdog",
--
1.7.0.4

Re: [PATCH 0/8] da9055: Driver initialisation fixes, add DT support

[Guenter Roeck]

On Wed, Feb 05, 2014 at 05:48:28PM +0000, Adam Thomson wrote:
> This patch series provides the following updates for DA9055 drivers:
> 
>  - Fixes an issue with da9055 driver initialisation (conflicting device ids) of
>    PMIC (MFD) and CODEC drivers.
>  - Add initial DT support for DA9055 related drivers, including binding
>    documentation.
>  - Remove conflicting use of platform_get_irq_byname() in driver probes.

Adam,

You don't really explain what the problem actually is. Can you elaborate ?

Also, I have been using platform_get_irq() to get the interrupt resource
in mfd client drivers and similar situations. Wouldn't this work here as well
if you don't want to use platform_get_irq_byname() ?

Thanks,
Guenter

>  - Remove unnecessary resource structures for MFD cells as a result of removing
>    use of platform_get_irq_byname().
> 
> Adam Thomson (8):
>   ASoC: da9055: Fix device registration of PMIC and CODEC devices
>   ASoC: da9055: Add DT support for CODEC
>   mfd: da9055: Add DT support for PMIC
>   regulator: da9055: Add DT support
>   onkey: da9055: Remove use of platform_get_irq_byname()
>   hwmon: da9055: Remove use of platform_get_irq_byname()
>   rtc: da9055: Remove use of platform_get_irq_byname()
>   mfd: da9055: Remove unused resource structures for mfd cells.
> 
>  Documentation/devicetree/bindings/mfd/da9055.txt   |   73 ++++++++++++++++++++
>  Documentation/devicetree/bindings/sound/da9055.txt |   22 ++++++
>  drivers/hwmon/da9055-hwmon.c                       |    7 +--
>  drivers/input/misc/da9055_onkey.c                  |    9 +--
>  drivers/mfd/da9055-core.c                          |   46 ------------
>  drivers/mfd/da9055-i2c.c                           |   20 +++++-
>  drivers/regulator/da9055-regulator.c               |   59 ++++++++++++++--
>  drivers/rtc/rtc-da9055.c                           |    3 +-
>  sound/soc/codecs/da9055.c                          |   19 +++++-
>  9 files changed, 188 insertions(+), 70 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/mfd/da9055.txt
>  create mode 100644 Documentation/devicetree/bindings/sound/da9055.txt
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 0/8] da9055: Driver initialisation fixes, add DT support

[Opensource [Adam Thomson]]

On Thu, Feb 06, 2014 at 00:54:11AM +0000, Guenter Roeck wrote:

> Adam,
> 
> You don't really explain what the problem actually is. Can you elaborate ?

Sorry, yes. For the conflicting device Ids, both the PMIC and the CODEC used
the same I2C Id string, which meant if you tried to intiate both together on the
same bus, then the second would fail.

For the removal of platform_get_irq_byname(), the reason for this was that it
was conflicting with regmap_irq_get_virq() when the IRQ value returned from
platform_get_irq_byname() was being passed to regmap_irq_get_virq(). The result
for the code was that it would try to request a threaded IRQ using an invalid
IRQ number (have also described this further in patch 0004 mail thread,
https://lkml.org/lkml/2014/2/6/126).

> 
> Also, I have been using platform_get_irq() to get the interrupt resource
> in mfd client drivers and similar situations. Wouldn't this work here as well
> if you don't want to use platform_get_irq_byname() ?

What I could've done is use platform_get_irq_byname() and avoided using
regmap_irq_get_virq() as I would already have the correct VIRQ to pass to
request_threaded_irq(), but I figured that using regmap_irq_get_virq() made
more sense at the time, and was unable to use both.

Re: [PATCH 0/8] da9055: Driver initialisation fixes, add DT support

[Guenter Roeck]

On 02/06/2014 03:46 AM, Opensource [Adam Thomson] wrote:
> On Thu, Feb 06, 2014 at 00:54:11AM +0000, Guenter Roeck wrote:
>
>> Adam,
>>
>> You don't really explain what the problem actually is. Can you elaborate ?
>
> Sorry, yes. For the conflicting device Ids, both the PMIC and the CODEC used
> the same I2C Id string, which meant if you tried to intiate both together on the
> same bus, then the second would fail.
>
> For the removal of platform_get_irq_byname(), the reason for this was that it
> was conflicting with regmap_irq_get_virq() when the IRQ value returned from
> platform_get_irq_byname() was being passed to regmap_irq_get_virq(). The result
> for the code was that it would try to request a threaded IRQ using an invalid
> IRQ number (have also described this further in patch 0004 mail thread,
> https://lkml.org/lkml/2014/2/6/126).
>
>>
>> Also, I have been using platform_get_irq() to get the interrupt resource
>> in mfd client drivers and similar situations. Wouldn't this work here as well
>> if you don't want to use platform_get_irq_byname() ?
>
> What I could've done is use platform_get_irq_byname() and avoided using
> regmap_irq_get_virq() as I would already have the correct VIRQ to pass to
> request_threaded_irq(), but I figured that using regmap_irq_get_virq() made
> more sense at the time, and was unable to use both.
>

I may be missing something, but I think the problem may be that you are
doing two mappings instead of just one. I don't think you need to call
regmap_irq_get_virq() at all.

Guenter

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 0/8] da9055: Driver initialisation fixes, add DT support

[Opensource [Adam Thomson]]

On Thu, Feb 06, 2014 at 13:23:47PM +0000, Guenter Roeck wrote:

> > What I could've done is use platform_get_irq_byname() and avoided using
> > regmap_irq_get_virq() as I would already have the correct VIRQ to pass to
> > request_threaded_irq(), but I figured that using regmap_irq_get_virq() made
> > more sense at the time, and was unable to use both.
> >
> 
> I may be missing something, but I think the problem may be that you are
> doing two mappings instead of just one. I don't think you need to call
> regmap_irq_get_virq() at all.

Yes, you're correct. The issue was already there in the code and I was
attempting to fix it. When I made the change I figured using only
regmap_irq_get_virq() was the way to go, but seems like I chose the wrong
option. Will make the changes (remove regmap_irq_get_virq()), test on both DT
and non-DT platforms, and then will re-submit the patches.