[alsa-devel] [PATCH 0/8] da9055: Driver initialisation fixes, add DT support
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
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
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@linaro.org
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
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
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
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)?
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.
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.
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.
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.
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
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
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
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
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
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.
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
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.
participants (4)
-
Adam Thomson -
Guenter Roeck -
Mark Brown -
Opensource [Adam Thomson]