Hi,
On Tue, Aug 02, 2011 at 02:28:41PM +0300, Peter Ujfalusi wrote:
> From: Misael Lopez Cruz <misael.lopez@xxxxxx>
>
> TWL6040 IC provides analog high-end audio codec functions for
> handset applications. It contains several audio analog inputs
> and outputs as well as vibrator support. It's connected to the
> host processor via PDM interface for audio data communication.
> The audio modules are controlled by internal registers that
> can be accessed by I2C and PDM interface.
>
> TWL6040 MFD will be registered as a child of TWL-CORE, and will
> have two children of its own: twl6040-codec and twl6040-vibra.
>
> This driver is based on TWL4030 and WM8350 MFD drivers.
>
> Signed-off-by: Misael Lopez Cruz <misael.lopez@xxxxxx>
> Signed-off-by: Jorge Eduardo Candelaria <jorge.candelaria@xxxxxx>
> Signed-off-by: Margarita Olaya Cabrera <magi.olaya@xxxxxx>
> ---
> arch/arm/plat-omap/include/plat/irqs.h | 12 +-
> drivers/mfd/Kconfig | 6 +
> drivers/mfd/Makefile | 1 +
> drivers/mfd/twl-core.c | 4 +-
> drivers/mfd/twl6040-codec.c | 587 ++++++++++++++++++++++++++++++++
> drivers/mfd/twl6040-irq.c | 205 +++++++++++
> include/linux/i2c/twl.h | 1 +
> include/linux/mfd/twl6040-codec.h | 260 ++++++++++++++
> 8 files changed, 1072 insertions(+), 4 deletions(-)
> create mode 100644 drivers/mfd/twl6040-codec.c
> create mode 100644 drivers/mfd/twl6040-irq.c
> create mode 100644 include/linux/mfd/twl6040-codec.h
>
> diff --git a/arch/arm/plat-omap/include/plat/irqs.h b/arch/arm/plat-omap/include/plat/irqs.h
> index 5a25098..2cfba51 100644
> --- a/arch/arm/plat-omap/include/plat/irqs.h
> +++ b/arch/arm/plat-omap/include/plat/irqs.h
> @@ -407,11 +407,19 @@
> #endif
> #define TWL6030_IRQ_END (TWL6030_IRQ_BASE + TWL6030_BASE_NR_IRQS)
>
> +#define TWL6040_CODEC_IRQ_BASE TWL6030_IRQ_END
> +#ifdef CONFIG_TWL6040_CODEC
> +#define TWL6040_CODEC_NR_IRQS 6
> +#else
> +#define TWL6040_CODEC_NR_IRQS 0
> +#endif
> +#define TWL6040_CODEC_IRQ_END (TWL6040_CODEC_IRQ_BASE + TWL6040_CODEC_NR_IRQS)
since this is a new driver, please don't pullute this header and use
irq_alloc_descs() instead ?!?
> diff --git a/drivers/mfd/twl6040-codec.c b/drivers/mfd/twl6040-codec.c
> new file mode 100644
> index 0000000..a40cd07
> --- /dev/null
> +++ b/drivers/mfd/twl6040-codec.c
> @@ -0,0 +1,587 @@
> +/*
> + * MFD driver for TWL6040 codec submodule
> + *
> + * Authors: Misael Lopez Cruz <misael.lopez@xxxxxx>
> + * Jorge Eduardo Candelaria <jorge.candelaria@xxxxxx>
> + *
> + * Copyright: (C) 2011 Texas Instruments, Inc.
> + *
> + * 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.
> + *
> + * 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.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
> + * 02110-1301 USA
> + *
> + */
> +
> +#include <linux/module.h>
> +#include <linux/types.h>
> +#include <linux/slab.h>
> +#include <linux/kernel.h>
> +#include <linux/platform_device.h>
> +#include <linux/gpio.h>
> +#include <linux/delay.h>
> +#include <linux/i2c/twl.h>
> +#include <linux/mfd/core.h>
> +#include <linux/mfd/twl6040-codec.h>
> +
> +static struct platform_device *twl6040_dev;
this is useless, see below.
> +int twl6040_reg_read(struct twl6040 *twl6040, unsigned int reg)
> +{
> + int ret;
> + u8 val = 0;
> +
> + mutex_lock(&twl6040->io_mutex);
> + ret = twl_i2c_read_u8(TWL_MODULE_AUDIO_VOICE, &val, reg);
> + if (ret < 0) {
> + mutex_unlock(&twl6040->io_mutex);
> + return ret;
> + }
> + mutex_unlock(&twl6040->io_mutex);
> +
> + return val;
> +}
> +EXPORT_SYMBOL(twl6040_reg_read);
EXPORT_SYMBOL_GPL(), all users of this should be GPL so we have access
to the code. Ditto to all below.
> +int twl6040_reg_write(struct twl6040 *twl6040, unsigned int reg, u8 val)
> +{
> + int ret;
> +
> + mutex_lock(&twl6040->io_mutex);
> + ret = twl_i2c_write_u8(TWL_MODULE_AUDIO_VOICE, val, reg);
> + mutex_unlock(&twl6040->io_mutex);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL(twl6040_reg_write);
> +
> +int twl6040_set_bits(struct twl6040 *twl6040, unsigned int reg, u8 mask)
> +{
> + int ret;
> + u8 val;
> +
> + mutex_lock(&twl6040->io_mutex);
> + ret = twl_i2c_read_u8(TWL_MODULE_AUDIO_VOICE, &val, reg);
> + if (ret)
> + goto out;
> +
> + val |= mask;
> + ret = twl_i2c_write_u8(TWL_MODULE_AUDIO_VOICE, val, reg);
> +out:
> + mutex_unlock(&twl6040->io_mutex);
> + return ret;
> +}
> +EXPORT_SYMBOL(twl6040_set_bits);
> +
> +int twl6040_clear_bits(struct twl6040 *twl6040, unsigned int reg, u8 mask)
> +{
> + int ret;
> + u8 val;
> +
> + mutex_lock(&twl6040->io_mutex);
> + ret = twl_i2c_read_u8(TWL_MODULE_AUDIO_VOICE, &val, reg);
> + if (ret)
> + goto out;
> +
> + val &= ~mask;
> + ret = twl_i2c_write_u8(TWL_MODULE_AUDIO_VOICE, val, reg);
> +out:
> + mutex_unlock(&twl6040->io_mutex);
> + return ret;
> +}
> +EXPORT_SYMBOL(twl6040_clear_bits);
> +
> +/* twl6040 codec manual power-up sequence */
> +static int twl6040_power_up(struct twl6040 *twl6040)
> +{
> + u8 ldoctl, ncpctl, lppllctl;
> + int ret;
> +
> + /* enable high-side LDO, reference system and internal oscillator */
> + ldoctl = TWL6040_HSLDOENA | TWL6040_REFENA | TWL6040_OSCENA;
> + ret = twl6040_reg_write(twl6040, TWL6040_REG_LDOCTL, ldoctl);
> + if (ret)
> + return ret;
> + usleep_range(10000, 10500);
> +
> + /* enable negative charge pump */
> + ncpctl = TWL6040_NCPENA;
> + ret = twl6040_reg_write(twl6040, TWL6040_REG_NCPCTL, ncpctl);
> + if (ret)
> + goto ncp_err;
> + usleep_range(1000, 1500);
> +
> + /* enable low-side LDO */
> + ldoctl |= TWL6040_LSLDOENA;
> + ret = twl6040_reg_write(twl6040, TWL6040_REG_LDOCTL, ldoctl);
> + if (ret)
> + goto lsldo_err;
> + usleep_range(1000, 1500);
> +
> + /* enable low-power PLL */
> + lppllctl = TWL6040_LPLLENA;
> + ret = twl6040_reg_write(twl6040, TWL6040_REG_LPPLLCTL, lppllctl);
> + if (ret)
> + goto lppll_err;
> + usleep_range(5000, 5500);
> +
> + /* disable internal oscillator */
> + ldoctl &= ~TWL6040_OSCENA;
> + ret = twl6040_reg_write(twl6040, TWL6040_REG_LDOCTL, ldoctl);
> + if (ret)
> + goto osc_err;
> +
> + return 0;
> +
> +osc_err:
> + lppllctl &= ~TWL6040_LPLLENA;
> + twl6040_reg_write(twl6040, TWL6040_REG_LPPLLCTL, lppllctl);
> +lppll_err:
> + ldoctl &= ~TWL6040_LSLDOENA;
> + twl6040_reg_write(twl6040, TWL6040_REG_LDOCTL, ldoctl);
> +lsldo_err:
> + ncpctl &= ~TWL6040_NCPENA;
> + twl6040_reg_write(twl6040, TWL6040_REG_NCPCTL, ncpctl);
> +ncp_err:
> + ldoctl &= ~(TWL6040_HSLDOENA | TWL6040_REFENA | TWL6040_OSCENA);
> + twl6040_reg_write(twl6040, TWL6040_REG_LDOCTL, ldoctl);
> +
> + return ret;
> +}
> +
> +/* twl6040 codec manual power-down sequence */
> +static void twl6040_power_down(struct twl6040 *twl6040)
> +{
> + u8 ncpctl, ldoctl, lppllctl;
> +
> + ncpctl = twl6040_reg_read(twl6040, TWL6040_REG_NCPCTL);
> + ldoctl = twl6040_reg_read(twl6040, TWL6040_REG_LDOCTL);
> + lppllctl = twl6040_reg_read(twl6040, TWL6040_REG_LPPLLCTL);
> +
> + /* enable internal oscillator */
> + ldoctl |= TWL6040_OSCENA;
> + twl6040_reg_write(twl6040, TWL6040_REG_LDOCTL, ldoctl);
> + usleep_range(1000, 1500);
> +
> + /* disable low-power PLL */
> + lppllctl &= ~TWL6040_LPLLENA;
> + twl6040_reg_write(twl6040, TWL6040_REG_LPPLLCTL, lppllctl);
> +
> + /* disable low-side LDO */
> + ldoctl &= ~TWL6040_LSLDOENA;
> + twl6040_reg_write(twl6040, TWL6040_REG_LDOCTL, ldoctl);
> +
> + /* disable negative charge pump */
> + ncpctl &= ~TWL6040_NCPENA;
> + twl6040_reg_write(twl6040, TWL6040_REG_NCPCTL, ncpctl);
> +
> + /* disable high-side LDO, reference system and internal oscillator */
> + ldoctl &= ~(TWL6040_HSLDOENA | TWL6040_REFENA | TWL6040_OSCENA);
> + twl6040_reg_write(twl6040, TWL6040_REG_LDOCTL, ldoctl);
> +}
> +
> +static irqreturn_t twl6040_naudint_handler(int irq, void *data)
> +{
> + struct twl6040 *twl6040 = data;
> + u8 intid;
> +
> + intid = twl6040_reg_read(twl6040, TWL6040_REG_INTID);
> +
> + if (intid & TWL6040_READYINT)
> + complete(&twl6040->ready);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int twl6040_power_up_completion(struct twl6040 *twl6040,
> + int naudint)
> +{
> + int time_left;
> + u8 intid;
> +
> + time_left = wait_for_completion_timeout(&twl6040->ready,
> + msecs_to_jiffies(144));
should this be interruptible ?
> + if (!time_left) {
> + intid = twl6040_reg_read(twl6040, TWL6040_REG_INTID);
> + if (!(intid & TWL6040_READYINT)) {
> + dev_err(&twl6040_dev->dev,
you have twl6040 as argument to this function, meaning:
dev_err(twl6040->dev,
should work just fine.
> +int twl6040_power(struct twl6040 *twl6040, int on)
> +{
> + int audpwron = twl6040->audpwron;
> + int naudint = twl6040->irq;
> + int ret = 0;
> +
> + mutex_lock(&twl6040->mutex);
> +
> + if (on) {
> + /* already powered-up */
> + if (twl6040->power_count++)
> + goto out;
> +
> + if (gpio_is_valid(audpwron)) {
> + /* use AUDPWRON line */
> + gpio_set_value(audpwron, 1);
> + /* wait for power-up completion */
> + ret = twl6040_power_up_completion(twl6040, naudint);
> + if (ret) {
> + dev_err(&twl6040_dev->dev,
ditto here and all below.
> + "automatic power-down failed\n");
> + twl6040->power_count = 0;
> + goto out;
> + }
> + } else {
> + /* use manual power-up sequence */
> + ret = twl6040_power_up(twl6040);
> + if (ret) {
> + dev_err(&twl6040_dev->dev,
> + "manual power-up failed\n");
> + twl6040->power_count = 0;
> + goto out;
> + }
> + }
> + twl6040->pll = TWL6040_LPPLL_ID;
> + twl6040->sysclk = 19200000;
> + } else {
> + /* already powered-down */
> + if (!twl6040->power_count) {
> + dev_err(&twl6040_dev->dev,
> + "device is already powered-off\n");
> + ret = -EPERM;
> + goto out;
> + }
> +
> + if (--twl6040->power_count)
> + goto out;
> +
> + if (gpio_is_valid(audpwron)) {
> + /* use AUDPWRON line */
> + gpio_set_value(audpwron, 0);
> +
> + /* power-down sequence latency */
> + udelay(500);
> + } else {
> + /* use manual power-down sequence */
> + twl6040_power_down(twl6040);
> + }
> + twl6040->pll = TWL6040_NOPLL_ID;
> + twl6040->sysclk = 0;
> + }
> +
> +out:
> + mutex_unlock(&twl6040->mutex);
> + return ret;
> +}
> +EXPORT_SYMBOL(twl6040_power);
should this function be a ->runtime_resume/suspend method ? Then
children would simply pm_runtime_get_sync() and this would be called by
the pm runtime framework. Does it make sense ?
> +int twl6040_is_powered(struct twl6040 *twl6040)
> +{
> + return twl6040->power_count;
> +}
> +EXPORT_SYMBOL(twl6040_is_powered);
I'm not sure this should be needed. You allocate your children yourself,
so they will only probe after this has succesfully probed, rendering
this unneeded, right ?
> +int twl6040_set_pll(struct twl6040 *twl6040, enum twl6040_pll_id id,
> + unsigned int freq_in, unsigned int freq_out)
> +{
> + u8 hppllctl, lppllctl;
> + int ret = 0;
> +
> + mutex_lock(&twl6040->mutex);
> +
> + hppllctl = twl6040_reg_read(twl6040, TWL6040_REG_HPPLLCTL);
> + lppllctl = twl6040_reg_read(twl6040, TWL6040_REG_LPPLLCTL);
> +
> + switch (id) {
> + case TWL6040_LPPLL_ID:
> + /* low-power PLL divider */
> + switch (freq_out) {
> + case 17640000:
> + lppllctl |= TWL6040_LPLLFIN;
> + break;
> + case 19200000:
> + lppllctl &= ~TWL6040_LPLLFIN;
> + break;
> + default:
> + dev_err(&twl6040_dev->dev,
also don't need that global static pointer. twl6040->dev. Ditto all
below
> + "freq_out %d not supported\n", freq_out);
> + ret = -EINVAL;
> + goto pll_out;
> + }
> + twl6040_reg_write(twl6040, TWL6040_REG_LPPLLCTL, lppllctl);
> +
> + switch (freq_in) {
> + case 32768:
> + lppllctl |= TWL6040_LPLLENA;
> + twl6040_reg_write(twl6040, TWL6040_REG_LPPLLCTL,
> + lppllctl);
> + mdelay(5);
> + lppllctl &= ~TWL6040_HPLLSEL;
> + twl6040_reg_write(twl6040, TWL6040_REG_LPPLLCTL,
> + lppllctl);
> + hppllctl &= ~TWL6040_HPLLENA;
> + twl6040_reg_write(twl6040, TWL6040_REG_HPPLLCTL,
> + hppllctl);
> + break;
> + default:
> + dev_err(&twl6040_dev->dev,
> + "freq_in %d not supported\n", freq_in);
> + ret = -EINVAL;
> + goto pll_out;
> + }
> +
> + twl6040->pll = TWL6040_LPPLL_ID;
> + break;
> + case TWL6040_HPPLL_ID:
> + /* high-performance PLL can provide only 19.2 MHz */
> + if (freq_out != 19200000) {
> + dev_err(&twl6040_dev->dev,
> + "freq_out %d not supported\n", freq_out);
> + ret = -EINVAL;
> + goto pll_out;
> + }
> +
> + hppllctl &= ~TWL6040_MCLK_MSK;
> +
> + switch (freq_in) {
> + case 12000000:
> + /* PLL enabled, active mode */
> + hppllctl |= TWL6040_MCLK_12000KHZ |
> + TWL6040_HPLLENA;
> + break;
> + case 19200000:
> + /*
> + * PLL disabled
> + * (enable PLL if MCLK jitter quality
> + * doesn't meet specification)
> + */
> + hppllctl |= TWL6040_MCLK_19200KHZ;
> + break;
> + case 26000000:
> + /* PLL enabled, active mode */
> + hppllctl |= TWL6040_MCLK_26000KHZ |
> + TWL6040_HPLLENA;
> + break;
> + case 38400000:
> + /* PLL enabled, active mode */
> + hppllctl |= TWL6040_MCLK_38400KHZ |
> + TWL6040_HPLLENA;
> + break;
> + default:
> + dev_err(&twl6040_dev->dev,
> + "freq_in %d not supported\n", freq_in);
> + ret = -EINVAL;
> + goto pll_out;
> + }
> +
> + /* enable clock slicer to ensure input waveform is square */
> + hppllctl |= TWL6040_HPLLSQRENA;
> +
> + twl6040_reg_write(twl6040, TWL6040_REG_HPPLLCTL, hppllctl);
> + udelay(500);
> + lppllctl |= TWL6040_HPLLSEL;
> + twl6040_reg_write(twl6040, TWL6040_REG_LPPLLCTL, lppllctl);
> + lppllctl &= ~TWL6040_LPLLENA;
> + twl6040_reg_write(twl6040, TWL6040_REG_LPPLLCTL, lppllctl);
> +
> + twl6040->pll = TWL6040_HPPLL_ID;
> + break;
> + default:
> + dev_err(&twl6040_dev->dev, "unknown pll id %d\n", id);
> + ret = -EINVAL;
> + goto pll_out;
> + }
> +
> + twl6040->sysclk = freq_out;
> +
> +pll_out:
> + mutex_unlock(&twl6040->mutex);
> + return ret;
> +}
> +EXPORT_SYMBOL(twl6040_set_pll);
> +
> +enum twl6040_pll_id twl6040_get_pll(struct twl6040 *twl6040)
> +{
> + return twl6040->pll;
> +}
> +EXPORT_SYMBOL(twl6040_get_pll);
> +
> +unsigned int twl6040_get_sysclk(struct twl6040 *twl6040)
> +{
> + return twl6040->sysclk;
> +}
> +EXPORT_SYMBOL(twl6040_get_sysclk);
these three above look like they should be done via clk framework ??
> +static int __devinit twl6040_probe(struct platform_device *pdev)
> +{
> + struct twl4030_codec_data *pdata = pdev->dev.platform_data;
> + struct twl6040 *twl6040;
> + struct mfd_cell *cell = NULL;
> + int ret, children = 0;
> +
> + if (!pdata) {
> + dev_err(&pdev->dev, "Platform data is missing\n");
> + return -EINVAL;
> + }
> +
> + twl6040 = kzalloc(sizeof(struct twl6040), GFP_KERNEL);
sizeof(*twl6040) would allow to change the type without need to patch
this line. It's what's generally done on most drivers.
> + if (!twl6040)
> + return -ENOMEM;
> +
> + platform_set_drvdata(pdev, twl6040);
> +
> + twl6040_dev = pdev;
this is useless.
> + twl6040->dev = &pdev->dev;
> + twl6040->audpwron = pdata->audpwron_gpio;
> + twl6040->irq = pdata->naudint_irq;
> + twl6040->irq_base = pdata->irq_base;
> +
> + mutex_init(&twl6040->mutex);
> + mutex_init(&twl6040->io_mutex);
> + init_completion(&twl6040->ready);
> +
> + twl6040->rev = twl6040_reg_read(twl6040, TWL6040_REG_ASICREV);
> +
> + if (gpio_is_valid(twl6040->audpwron)) {
> + ret = gpio_request(twl6040->audpwron, "audpwron");
> + if (ret)
> + goto gpio1_err;
> +
> + ret = gpio_direction_output(twl6040->audpwron, 0);
> + if (ret)
> + goto gpio2_err;
> + }
what if gpio isn't valid ? Does it make sense to continue ? Is the GPIO
poweron pin a "must-have" or can you tie that pin to Vcc (or ground,
depending if it's active high or low) and have it working ?
> + /* ERRATA: Automatic power-up is not possible in ES1.0 */
> + if (twl6040_get_rev(twl6040) == 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,
this is really wrong. Didn't we agree on using tradicional
request_threaded_irq() here ? Also, the IRQ number should be passed via
struct resource.
> + 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 */
> + twl6040_set_bits(twl6040, TWL6040_REG_ACCCTL, TWL6040_I2CSEL);
> +
> + if (pdata->audio) {
> + cell = &twl6040->cells[children];
> + cell->name = "twl6040-codec";
> + cell->platform_data = pdata->audio;
> + cell->pdata_size = sizeof(*pdata->audio);
> + children++;
> + }
> +
> + if (pdata->vibra) {
> + cell = &twl6040->cells[children];
> + cell->name = "twl6040-vibra";
> + cell->platform_data = pdata->vibra;
> + cell->pdata_size = sizeof(*pdata->vibra);
> + children++;
> + }
> +
> + if (children) {
> + ret = mfd_add_devices(&pdev->dev, pdev->id, twl6040->cells,
> + children, NULL, 0);
> + if (ret)
> + goto mfd_err;
> + } else {
> + dev_err(&pdev->dev, "No platform data found for children\n");
> + ret = -ENODEV;
> + goto mfd_err;
> + }
> +
> + return 0;
> +
> +mfd_err:
> + if (twl6040->irq)
> + twl6040_free_irq(twl6040, TWL6040_IRQ_READY, twl6040);
> +irq_err:
> + if (twl6040->irq)
> + twl6040_irq_exit(twl6040);
> +gpio2_err:
> + if (gpio_is_valid(twl6040->audpwron))
> + gpio_free(twl6040->audpwron);
> +gpio1_err:
> + platform_set_drvdata(pdev, NULL);
> + kfree(twl6040);
> + twl6040_dev = NULL;
> + return ret;
> +}
> +
> +static int __devexit twl6040_remove(struct platform_device *pdev)
> +{
> + struct twl6040 *twl6040 = platform_get_drvdata(pdev);
> +
> + if (twl6040_is_powered(twl6040))
> + twl6040_power(twl6040, 0);
disabling it twice, shouldn't be a problem, so this is also useless
check.
> + if (gpio_is_valid(twl6040->audpwron))
> + gpio_free(twl6040->audpwron);
> +
> + twl6040_free_irq(twl6040, TWL6040_IRQ_READY, twl6040);
this has to be passed via struct resource.
> + if (twl6040->irq)
> + twl6040_irq_exit(twl6040);
> +
> + mfd_remove_devices(&pdev->dev);
> + platform_set_drvdata(pdev, NULL);
> + kfree(twl6040);
> + twl6040_dev = NULL;
> +
> + return 0;
> +}
> +
> +static struct platform_driver twl6040_driver = {
> + .probe = twl6040_probe,
> + .remove = __devexit_p(twl6040_remove),
> + .driver = {
> + .owner = THIS_MODULE,
> + .name = "twl6040-audio",
> + },
> +};
> +
> +static int __devinit twl6040_init(void)
> +{
> + return platform_driver_register(&twl6040_driver);
> +}
> +module_init(twl6040_init);
> +
> +static void __devexit twl6040_exit(void)
> +{
> + platform_driver_unregister(&twl6040_driver);
> +}
> +
> +module_exit(twl6040_exit);
> +
> +MODULE_DESCRIPTION("TWL6040 MFD");
> +MODULE_AUTHOR("Misael Lopez Cruz <misael.lopez@xxxxxx>");
> +MODULE_AUTHOR("Jorge Eduardo Candelaria <jorge.candelaria@xxxxxx>");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("platform:twl6040-audio");
> diff --git a/drivers/mfd/twl6040-irq.c b/drivers/mfd/twl6040-irq.c
> new file mode 100644
> index 0000000..ac776be
> --- /dev/null
> +++ b/drivers/mfd/twl6040-irq.c
> @@ -0,0 +1,205 @@
> +/*
> + * Interrupt controller support for TWL6040
> + *
> + * Author: Misael Lopez Cruz <misael.lopez@xxxxxx>
> + *
> + * Copyright: (C) 2011 Texas Instruments, Inc.
> + *
> + * 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.
> + *
> + * 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.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
> + * 02110-1301 USA
> + *
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/irq.h>
> +#include <linux/interrupt.h>
> +#include <linux/mfd/core.h>
> +#include <linux/mfd/twl6040-codec.h>
> +
> +struct twl6040_irq_data {
> + int mask;
> + int status;
> +};
> +
> +static struct twl6040_irq_data twl6040_irqs[] = {
> + {
> + .mask = TWL6040_THMSK,
> + .status = TWL6040_THINT,
> + },
> + {
> + .mask = TWL6040_PLUGMSK,
> + .status = TWL6040_PLUGINT | TWL6040_UNPLUGINT,
> + },
> + {
> + .mask = TWL6040_HOOKMSK,
> + .status = TWL6040_HOOKINT,
> + },
> + {
> + .mask = TWL6040_HFMSK,
> + .status = TWL6040_HFINT,
> + },
> + {
> + .mask = TWL6040_VIBMSK,
> + .status = TWL6040_VIBINT,
> + },
> + {
> + .mask = TWL6040_READYMSK,
> + .status = TWL6040_READYINT,
> + },
> +};
> +
> +static inline
> +struct twl6040_irq_data *irq_to_twl6040_irq(struct twl6040 *twl6040,
> + int irq)
> +{
> + return &twl6040_irqs[irq - twl6040->irq_base];
> +}
> +
> +static void twl6040_irq_lock(struct irq_data *data)
> +{
> + struct twl6040 *twl6040 = irq_data_get_irq_chip_data(data);
> +
> + mutex_lock(&twl6040->irq_mutex);
> +}
> +
> +static void twl6040_irq_sync_unlock(struct irq_data *data)
> +{
> + struct twl6040 *twl6040 = irq_data_get_irq_chip_data(data);
> +
> + /* write back to hardware any change in irq mask */
> + if (twl6040->irq_masks_cur != twl6040->irq_masks_cache) {
> + twl6040->irq_masks_cache = twl6040->irq_masks_cur;
> + twl6040_reg_write(twl6040, TWL6040_REG_INTMR,
> + twl6040->irq_masks_cur);
> + }
> +
> + mutex_unlock(&twl6040->irq_mutex);
> +}
> +
> +static void twl6040_irq_enable(struct irq_data *data)
> +{
> + struct twl6040 *twl6040 = irq_data_get_irq_chip_data(data);
> + struct twl6040_irq_data *irq_data = irq_to_twl6040_irq(twl6040,
> + data->irq);
> +
> + twl6040->irq_masks_cur &= ~irq_data->mask;
> +}
> +
> +static void twl6040_irq_disable(struct irq_data *data)
> +{
> + struct twl6040 *twl6040 = irq_data_get_irq_chip_data(data);
> + struct twl6040_irq_data *irq_data = irq_to_twl6040_irq(twl6040,
> + data->irq);
> +
> + twl6040->irq_masks_cur |= irq_data->mask;
> +}
> +
> +static struct irq_chip twl6040_irq_chip = {
> + .name = "twl6040",
> + .irq_bus_lock = twl6040_irq_lock,
> + .irq_bus_sync_unlock = twl6040_irq_sync_unlock,
> + .irq_enable = twl6040_irq_enable,
> + .irq_disable = twl6040_irq_disable,
> +};
> +
> +static irqreturn_t twl6040_irq_thread(int irq, void *data)
> +{
> + struct twl6040 *twl6040 = data;
> + u8 intid;
> + int i;
> +
> + intid = twl6040_reg_read(twl6040, TWL6040_REG_INTID);
what's available on this register ?? Are the status fields above bit
positions ? If so you can change this:
> + /* apply masking and report (backwards to handle READYINT first) */
> + for (i = ARRAY_SIZE(twl6040_irqs) - 1; i >= 0; i--) {
> + if (twl6040->irq_masks_cur & twl6040_irqs[i].mask)
> + intid &= ~twl6040_irqs[i].status;
> + if (intid & twl6040_irqs[i].status)
> + handle_nested_irq(twl6040->irq_base + i);
> + }
into something like this:
while (intid) {
unsigned long pending = __ffs(intid);
unsigned long irq;
intid &= ~BIT(pending);
irq = pending + twl6040->irq_base;
handle_nested_irq(irq);
}
and that twl6040_irq_data structure can go into the bin.
> + /* ack unmasked irqs */
> + twl6040_reg_write(twl6040, TWL6040_REG_INTID, intid);
I believe IRQ subsystem will handle this for you on your irq_ack() +
bus_sync_unlock() methods, right ? So you should provide them.
> + return IRQ_HANDLED;
> +}
> +
> +int twl6040_irq_init(struct twl6040 *twl6040)
> +{
> + int cur_irq, ret;
> + u8 val;
> +
> + mutex_init(&twl6040->irq_mutex);
> +
> + /* mask the individual interrupt sources */
> + twl6040->irq_masks_cur = TWL6040_ALLINT_MSK;
> + 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) {
base should be allocated here with irq_alloc_descs()
> + 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);
> + cur_irq++) {
> + irq_set_chip_data(cur_irq, twl6040);
> + irq_set_chip_and_handler(cur_irq, &twl6040_irq_chip,
> + handle_level_irq);
> + irq_set_nested_thread(cur_irq, 1);
> +
> + /* ARM needs us to explicitly flag the IRQ as valid
> + * and will set them noprobe when we do so. */
multiline comment style is wrong here.
> +#ifdef CONFIG_ARM
> + set_irq_flags(cur_irq, IRQF_VALID);
> +#else
> + irq_set_noprobe(cur_irq);
> +#endif
> + }
> +
> + ret = request_threaded_irq(twl6040->irq, NULL, twl6040_irq_thread,
> + IRQF_ONESHOT, "twl6040", twl6040);
I'm not sure you need IRQF_ONESHOT here, but that can be changed later.
> + if (ret) {
> + dev_err(twl6040->dev, "failed to request IRQ %d: %d\n",
> + twl6040->irq, ret);
> + return ret;
> + }
> +
> + /* reset interrupts */
> + val = twl6040_reg_read(twl6040, TWL6040_REG_INTID);
> +
> + /* interrupts cleared on write */
> + twl6040_clear_bits(twl6040, TWL6040_REG_ACCCTL, TWL6040_INTCLRMODE);
you should do these two before request_threaded_irq(), you might have a
real IRQ which will be missed.
> + return 0;
> +}
> +EXPORT_SYMBOL(twl6040_irq_init);
EXPORT_SYMBOL_GPL()
> +void twl6040_irq_exit(struct twl6040 *twl6040)
> +{
> + if (twl6040->irq)
this check should always be true. You only call this on error path of
probe() and exit path.
> + free_irq(twl6040->irq, twl6040);
> +}
> +EXPORT_SYMBOL(twl6040_irq_exit);
EXPORT_SYMBOL_GPL()
--
balbi
Attachment:
signature.asc
Description: Digital signature
