[alsa-devel] [PATCH v3 0/5] MFD/input/ASoC: twl6040: irq registration changes
Hello,
Changes since v2: - Fix the check in the slaves driver for the validity of the received irq - Mark Brown's ack added to patch 3
Intro mail froim v1: As Samuel, and Felipe requested for the twl6040 MFD series: http://marc.info/?l=linux-kernel&m=130978512225430&w=2 http://marc.info/?l=linux-omap&m=130978403224218&w=2
This is the update to remove the obfuscation around the threaded irq request in the twl6040 driver and it's slaves.
With this change the slaves uses directly the threaded irq request, and the core driver passes the irq number for the slaves to use (configured runtime, so machine drivers do not need to be updated).
Regards, Peter
PS: for the remaining comments from Samuel I have changed the series, since the requested change was small. I'm not intended to send the series again, unless it is requested (again the change from v6 is minimal - few lines has been removed/replaced).
--- Peter Ujfalusi (5): MFD: twl6040: Use resource to provide irq number for slaves input: twl6040-vibra: Do not use wrapper for irq request ASoC: twl6040: Do not use wrapper for irq request MFD: twl6040: Demand valid interrupt configuration MFD: twl6040: Remove wrapper for threaded irq request
drivers/input/misc/twl6040-vibra.c | 17 ++++++--- drivers/mfd/twl6040-core.c | 69 ++++++++++++++++++++++++----------- drivers/mfd/twl6040-irq.c | 16 +-------- include/linux/mfd/twl6040.h | 22 ----------- sound/soc/codecs/twl6040.c | 19 +++++++--- 5 files changed, 74 insertions(+), 69 deletions(-)
Provide the irq number for slaves via resource on the platform device. The irq number configuration is done in the twl6040-core at probe time, so machine drivers do not need to be modified.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- drivers/mfd/twl6040-core.c | 25 +++++++++++++++++++++++++ 1 files changed, 25 insertions(+), 0 deletions(-)
diff --git a/drivers/mfd/twl6040-core.c b/drivers/mfd/twl6040-core.c index cfaedb5..471f489 100644 --- a/drivers/mfd/twl6040-core.c +++ b/drivers/mfd/twl6040-core.c @@ -435,6 +435,18 @@ unsigned int twl6040_get_sysclk(struct twl6040 *twl6040) } EXPORT_SYMBOL(twl6040_get_sysclk);
+static struct resource twl6040_vibra_rsrc[] = { + { + .flags = IORESOURCE_IRQ, + }, +}; + +static struct resource twl6040_codec_rsrc[] = { + { + .flags = IORESOURCE_IRQ, + }, +}; + static int __devinit twl6040_probe(struct platform_device *pdev) { struct twl4030_audio_data *pdata = pdev->dev.platform_data; @@ -499,16 +511,29 @@ static int __devinit twl6040_probe(struct platform_device *pdev) twl6040_set_bits(twl6040, TWL6040_REG_ACCCTL, TWL6040_I2CSEL);
if (pdata->codec) { + int irq = twl6040->irq_base + TWL6040_IRQ_PLUG; + cell = &twl6040->cells[children]; cell->name = "twl6040-codec"; + twl6040_codec_rsrc[0].start = irq; + twl6040_codec_rsrc[0].end = irq; + cell->resources = twl6040_codec_rsrc; + cell->num_resources = ARRAY_SIZE(twl6040_codec_rsrc); cell->platform_data = pdata->codec; cell->pdata_size = sizeof(*pdata->codec); children++; }
if (pdata->vibra) { + int irq = twl6040->irq_base + TWL6040_IRQ_VIB; + cell = &twl6040->cells[children]; cell->name = "twl6040-vibra"; + twl6040_vibra_rsrc[0].start = irq; + twl6040_vibra_rsrc[0].end = irq; + cell->resources = twl6040_vibra_rsrc; + cell->num_resources = ARRAY_SIZE(twl6040_vibra_rsrc); + cell->platform_data = pdata->vibra; cell->pdata_size = sizeof(*pdata->vibra); children++;
The twl6040_request_irq/free_irq inline functions are going to be removed, so replace them with direct calls. The irq number is provided by the core driver via resource.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- drivers/input/misc/twl6040-vibra.c | 17 ++++++++++++----- 1 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/drivers/input/misc/twl6040-vibra.c b/drivers/input/misc/twl6040-vibra.c index dbf745d..c43002e 100644 --- a/drivers/input/misc/twl6040-vibra.c +++ b/drivers/input/misc/twl6040-vibra.c @@ -47,6 +47,7 @@ struct vibra_info { struct workqueue_struct *workqueue; struct work_struct play_work; struct mutex mutex; + int irq;
bool enabled; int weak_speed; @@ -277,6 +278,13 @@ static int __devinit twl6040_vibra_probe(struct platform_device *pdev) goto err_kzalloc; }
+ info->irq = platform_get_irq(pdev, 0); + if (info->irq < 0) { + dev_err(info->dev, "invalid irq\n"); + ret = -EINVAL; + goto err_kzalloc; + } + mutex_init(&info->mutex);
info->input_dev = input_allocate_device(); @@ -308,9 +316,8 @@ static int __devinit twl6040_vibra_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, info);
- ret = twl6040_request_irq(info->twl6040, TWL6040_IRQ_VIB, - twl6040_vib_irq_handler, 0, - "twl6040_irq_vib", info); + ret = request_threaded_irq(info->irq, NULL, twl6040_vib_irq_handler, 0, + "twl6040_irq_vib", info); if (ret) { dev_err(info->dev, "VIB IRQ request failed: %d\n", ret); goto err_irq; @@ -360,7 +367,7 @@ static int __devinit twl6040_vibra_probe(struct platform_device *pdev) err_voltage: regulator_bulk_free(ARRAY_SIZE(info->supplies), info->supplies); err_regulator: - twl6040_free_irq(info->twl6040, TWL6040_IRQ_VIB, info); + free_irq(info->irq, info); err_irq: input_unregister_device(info->input_dev); info->input_dev = NULL; @@ -379,7 +386,7 @@ static int __devexit twl6040_vibra_remove(struct platform_device *pdev) struct vibra_info *info = platform_get_drvdata(pdev);
input_unregister_device(info->input_dev); - twl6040_free_irq(info->twl6040, TWL6040_IRQ_VIB, info); + free_irq(info->irq, info); regulator_bulk_free(ARRAY_SIZE(info->supplies), info->supplies); destroy_workqueue(info->workqueue); kfree(info);
On Tue, Jul 05, 2011 at 10:49:33PM +0300, Peter Ujfalusi wrote:
The twl6040_request_irq/free_irq inline functions are going to be removed, so replace them with direct calls. The irq number is provided by the core driver via resource.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com
Acked-by: Dmitry Torokhov <dtormail.ru>
drivers/input/misc/twl6040-vibra.c | 17 ++++++++++++----- 1 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/drivers/input/misc/twl6040-vibra.c b/drivers/input/misc/twl6040-vibra.c index dbf745d..c43002e 100644 --- a/drivers/input/misc/twl6040-vibra.c +++ b/drivers/input/misc/twl6040-vibra.c @@ -47,6 +47,7 @@ struct vibra_info { struct workqueue_struct *workqueue; struct work_struct play_work; struct mutex mutex;
int irq;
bool enabled; int weak_speed;
@@ -277,6 +278,13 @@ static int __devinit twl6040_vibra_probe(struct platform_device *pdev) goto err_kzalloc; }
info->irq = platform_get_irq(pdev, 0);
if (info->irq < 0) {
dev_err(info->dev, "invalid irq\n");ret = -EINVAL;goto err_kzalloc;}
mutex_init(&info->mutex);
info->input_dev = input_allocate_device();
@@ -308,9 +316,8 @@ static int __devinit twl6040_vibra_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, info);
- ret = twl6040_request_irq(info->twl6040, TWL6040_IRQ_VIB,
twl6040_vib_irq_handler, 0,"twl6040_irq_vib", info);
- ret = request_threaded_irq(info->irq, NULL, twl6040_vib_irq_handler, 0,
"twl6040_irq_vib", info);@@ -360,7 +367,7 @@ static int __devinit twl6040_vibra_probe(struct platform_device *pdev) err_voltage: regulator_bulk_free(ARRAY_SIZE(info->supplies), info->supplies); err_regulator:
- twl6040_free_irq(info->twl6040, TWL6040_IRQ_VIB, info);
- free_irq(info->irq, info);
err_irq: input_unregister_device(info->input_dev); info->input_dev = NULL; @@ -379,7 +386,7 @@ static int __devexit twl6040_vibra_remove(struct platform_device *pdev) struct vibra_info *info = platform_get_drvdata(pdev);
input_unregister_device(info->input_dev);
- twl6040_free_irq(info->twl6040, TWL6040_IRQ_VIB, info);
- free_irq(info->irq, info); regulator_bulk_free(ARRAY_SIZE(info->supplies), info->supplies); destroy_workqueue(info->workqueue); kfree(info);
-- 1.7.6
The twl6040_request_irq/free_irq inline functions are going to be removed, so replace them with direct calls. The irq number is provided by the core driver via resource.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com Acked-by: Mark Brown broonie@opensource.wolfsonmicro.com --- sound/soc/codecs/twl6040.c | 19 ++++++++++++++----- 1 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/sound/soc/codecs/twl6040.c b/sound/soc/codecs/twl6040.c index 0145041..14e3a58 100644 --- a/sound/soc/codecs/twl6040.c +++ b/sound/soc/codecs/twl6040.c @@ -76,6 +76,7 @@ struct twl6040_jack_data {
/* codec private data */ struct twl6040_data { + int plug_irq; int codec_powered; int pll; int non_lp; @@ -1527,6 +1528,8 @@ static int twl6040_probe(struct snd_soc_codec *codec) { struct twl6040_data *priv; struct twl4030_codec_data *pdata = dev_get_platdata(codec->dev); + struct platform_device *pdev = container_of(codec->dev, + struct platform_device, dev); int ret = 0;
priv = kzalloc(sizeof(struct twl6040_data), GFP_KERNEL); @@ -1553,6 +1556,13 @@ static int twl6040_probe(struct snd_soc_codec *codec) priv->hf_right_step = 1; }
+ priv->plug_irq = platform_get_irq(pdev, 0); + if (priv->plug_irq < 0) { + dev_err(codec->dev, "invalid irq\n"); + ret = -EINVAL; + goto work_err; + } + priv->sysclk_constraints = &hp_constraints; priv->workqueue = create_singlethread_workqueue("twl6040-codec"); if (!priv->workqueue) { @@ -1581,9 +1591,8 @@ static int twl6040_probe(struct snd_soc_codec *codec) INIT_DELAYED_WORK(&priv->hs_delayed_work, twl6040_pga_hs_work); INIT_DELAYED_WORK(&priv->hf_delayed_work, twl6040_pga_hf_work);
- ret = twl6040_request_irq(codec->control_data, TWL6040_IRQ_PLUG, - twl6040_audio_handler, 0, - "twl6040_irq_plug", codec); + ret = request_threaded_irq(priv->plug_irq, NULL, twl6040_audio_handler, + 0, "twl6040_irq_plug", codec); if (ret) { dev_err(codec->dev, "PLUG IRQ request failed: %d\n", ret); goto plugirq_err; @@ -1604,7 +1613,7 @@ static int twl6040_probe(struct snd_soc_codec *codec) return 0;
bias_err: - twl6040_free_irq(codec->control_data, TWL6040_IRQ_PLUG, codec); + free_irq(priv->plug_irq, codec); plugirq_err: destroy_workqueue(priv->hs_workqueue); hswq_err: @@ -1621,7 +1630,7 @@ static int twl6040_remove(struct snd_soc_codec *codec) struct twl6040_data *priv = snd_soc_codec_get_drvdata(codec);
twl6040_set_bias_level(codec, SND_SOC_BIAS_OFF); - twl6040_free_irq(codec->control_data, TWL6040_IRQ_PLUG, codec); + free_irq(priv->plug_irq, codec); destroy_workqueue(priv->workqueue); destroy_workqueue(priv->hf_workqueue); destroy_workqueue(priv->hs_workqueue);
In order to operate correctly twl6040 needs correct interrupt configuration. The slave drivers (vibra, and ASoC codec) will refuse to probe, if the interrupt configuration is not correct. In this way some checks can be removed from the code.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- drivers/mfd/twl6040-core.c | 42 +++++++++++++++++++++--------------------- drivers/mfd/twl6040-irq.c | 16 +--------------- 2 files changed, 22 insertions(+), 36 deletions(-)
diff --git a/drivers/mfd/twl6040-core.c b/drivers/mfd/twl6040-core.c index 471f489..f71bb14 100644 --- a/drivers/mfd/twl6040-core.c +++ b/drivers/mfd/twl6040-core.c @@ -459,6 +459,12 @@ static int __devinit twl6040_probe(struct platform_device *pdev) return -EINVAL; }
+ /* In order to operate correctly we need valid interrupt config */ + if (!pdata->naudint_irq || !pdata->irq_base) { + dev_err(&pdev->dev, "Invalid IRQ configuration\n"); + return -EINVAL; + } + twl6040 = kzalloc(sizeof(struct twl6040), GFP_KERNEL); if (!twl6040) return -ENOMEM; @@ -491,20 +497,18 @@ static int __devinit twl6040_probe(struct platform_device *pdev) if (twl6040->rev == TWL6040_REV_ES1_0) twl6040->audpwron = -EINVAL;
- if (twl6040->irq) { - /* codec interrupt */ - ret = twl6040_irq_init(twl6040); - if (ret) - goto gpio2_err; - - ret = twl6040_request_irq(twl6040, TWL6040_IRQ_READY, - twl6040_naudint_handler, 0, - "twl6040_irq_ready", twl6040); - if (ret) { - dev_err(twl6040->dev, "READY IRQ request failed: %d\n", - ret); - goto irq_err; - } + /* codec interrupt */ + ret = twl6040_irq_init(twl6040); + if (ret) + goto gpio2_err; + + ret = twl6040_request_irq(twl6040, TWL6040_IRQ_READY, + twl6040_naudint_handler, 0, + "twl6040_irq_ready", twl6040); + if (ret) { + dev_err(twl6040->dev, "READY IRQ request failed: %d\n", + ret); + goto irq_err; }
/* dual-access registers controlled by I2C only */ @@ -553,11 +557,9 @@ static int __devinit twl6040_probe(struct platform_device *pdev) return 0;
mfd_err: - if (twl6040->irq) - twl6040_free_irq(twl6040, TWL6040_IRQ_READY, twl6040); + twl6040_free_irq(twl6040, TWL6040_IRQ_READY, twl6040); irq_err: - if (twl6040->irq) - twl6040_irq_exit(twl6040); + twl6040_irq_exit(twl6040); gpio2_err: if (gpio_is_valid(twl6040->audpwron)) gpio_free(twl6040->audpwron); @@ -579,9 +581,7 @@ static int __devexit twl6040_remove(struct platform_device *pdev) gpio_free(twl6040->audpwron);
twl6040_free_irq(twl6040, TWL6040_IRQ_READY, twl6040); - - if (twl6040->irq) - twl6040_irq_exit(twl6040); + twl6040_irq_exit(twl6040);
mfd_remove_devices(&pdev->dev); platform_set_drvdata(pdev, NULL); diff --git a/drivers/mfd/twl6040-irq.c b/drivers/mfd/twl6040-irq.c index 9380535..b3f8dda 100644 --- a/drivers/mfd/twl6040-irq.c +++ b/drivers/mfd/twl6040-irq.c @@ -148,19 +148,6 @@ int twl6040_irq_init(struct twl6040 *twl6040) twl6040->irq_masks_cache = TWL6040_ALLINT_MSK; twl6040_reg_write(twl6040, TWL6040_REG_INTMR, TWL6040_ALLINT_MSK);
- if (!twl6040->irq) { - dev_warn(twl6040->dev, - "no interrupt specified, no interrupts\n"); - twl6040->irq_base = 0; - return 0; - } - - if (!twl6040->irq_base) { - dev_err(twl6040->dev, - "no interrupt base specified, no interrupts\n"); - return 0; - } - /* Register them with genirq */ for (cur_irq = twl6040->irq_base; cur_irq < twl6040->irq_base + ARRAY_SIZE(twl6040_irqs); @@ -199,7 +186,6 @@ EXPORT_SYMBOL(twl6040_irq_init);
void twl6040_irq_exit(struct twl6040 *twl6040) { - if (twl6040->irq) - free_irq(twl6040->irq, twl6040); + free_irq(twl6040->irq, twl6040); } EXPORT_SYMBOL(twl6040_irq_exit);
Remove the twl6040_request_irq/free_irq inline functions, and use direct calls instead in the core driver to register the threaded irq.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- drivers/mfd/twl6040-core.c | 10 +++++----- include/linux/mfd/twl6040.h | 22 ---------------------- 2 files changed, 5 insertions(+), 27 deletions(-)
diff --git a/drivers/mfd/twl6040-core.c b/drivers/mfd/twl6040-core.c index f71bb14..6843977 100644 --- a/drivers/mfd/twl6040-core.c +++ b/drivers/mfd/twl6040-core.c @@ -502,9 +502,9 @@ static int __devinit twl6040_probe(struct platform_device *pdev) if (ret) goto gpio2_err;
- ret = twl6040_request_irq(twl6040, TWL6040_IRQ_READY, - twl6040_naudint_handler, 0, - "twl6040_irq_ready", twl6040); + ret = request_threaded_irq(twl6040->irq_base + TWL6040_IRQ_READY, + NULL, twl6040_naudint_handler, 0, + "twl6040_irq_ready", twl6040); if (ret) { dev_err(twl6040->dev, "READY IRQ request failed: %d\n", ret); @@ -557,7 +557,7 @@ static int __devinit twl6040_probe(struct platform_device *pdev) return 0;
mfd_err: - twl6040_free_irq(twl6040, TWL6040_IRQ_READY, twl6040); + free_irq(twl6040->irq_base + TWL6040_IRQ_READY, twl6040); irq_err: twl6040_irq_exit(twl6040); gpio2_err: @@ -580,7 +580,7 @@ static int __devexit twl6040_remove(struct platform_device *pdev) if (gpio_is_valid(twl6040->audpwron)) gpio_free(twl6040->audpwron);
- twl6040_free_irq(twl6040, TWL6040_IRQ_READY, twl6040); + free_irq(twl6040->irq_base + TWL6040_IRQ_READY, twl6040); twl6040_irq_exit(twl6040);
mfd_remove_devices(&pdev->dev); diff --git a/include/linux/mfd/twl6040.h b/include/linux/mfd/twl6040.h index c3c1de5..df890a2 100644 --- a/include/linux/mfd/twl6040.h +++ b/include/linux/mfd/twl6040.h @@ -215,28 +215,6 @@ struct twl6040 { u8 irq_masks_cache; };
-static inline int twl6040_request_irq(struct twl6040 *twl6040, int irq, - irq_handler_t handler, - unsigned long irqflags, - const char *name, - void *data) -{ - if (!twl6040->irq_base) - return -EINVAL; - - return request_threaded_irq(twl6040->irq_base + irq, NULL, handler, - irqflags, name, data); -} - -static inline void twl6040_free_irq(struct twl6040 *twl6040, int irq, - void *data) -{ - if (!twl6040->irq_base) - return; - - free_irq(twl6040->irq_base + irq, data); -} - int twl6040_reg_read(struct twl6040 *twl6040, unsigned int reg); int twl6040_reg_write(struct twl6040 *twl6040, unsigned int reg, u8 val);
Hi Samuel, Dmitry,
would you be able to take a look at this series?
Thank you, Péter
On Tuesday 05 July 2011 21:49:31 Ujfalusi, Peter wrote:
Hello,
Changes since v2:
- Fix the check in the slaves driver for the validity of the received irq
- Mark Brown's ack added to patch 3
Intro mail froim v1: As Samuel, and Felipe requested for the twl6040 MFD series: http://marc.info/?l=linux-kernel&m=130978512225430&w=2 http://marc.info/?l=linux-omap&m=130978403224218&w=2
This is the update to remove the obfuscation around the threaded irq request in the twl6040 driver and it's slaves.
With this change the slaves uses directly the threaded irq request, and the core driver passes the irq number for the slaves to use (configured runtime, so machine drivers do not need to be updated).
Regards, Peter
PS: for the remaining comments from Samuel I have changed the series, since the requested change was small. I'm not intended to send the series again, unless it is requested (again the change from v6 is minimal - few lines has been removed/replaced).
Peter Ujfalusi (5): MFD: twl6040: Use resource to provide irq number for slaves input: twl6040-vibra: Do not use wrapper for irq request ASoC: twl6040: Do not use wrapper for irq request MFD: twl6040: Demand valid interrupt configuration MFD: twl6040: Remove wrapper for threaded irq request
drivers/input/misc/twl6040-vibra.c | 17 ++++++--- drivers/mfd/twl6040-core.c | 69 ++++++++++++++++++++++++----------- drivers/mfd/twl6040-irq.c | 16 +-------- include/linux/mfd/twl6040.h | 22 ----------- sound/soc/codecs/twl6040.c | 19 +++++++--- 5 files changed, 74 insertions(+), 69 deletions(-)
participants (2)
-
Dmitry Torokhov -
Peter Ujfalusi