Re: [PATCH v2] mfd: tps6586x: add basic interrupt support

[Mike Rapoport <mike@compulab.co.il>]

Hi Gary,

Gary King wrote:
> Hi Samuel, Mike,
> 
> This patch incorporates Mike Rapoport's feedback on my initial patch
> to add interrupt support to the tps6586x driver.

Overall looks good to me, two nitpicking comments though.

> Changes since v1:
> 
> * Move the interrupt enum ordering to match the ACK register values
> 
> * Treat the INT_ACK registers as a single 32b unsigned int.
> 
> * Name and define all the interrupts on the device
> 
> * If reading the ACK registers in the interrupt thread fails, IRQ_NONE
>   is returned, rather than disabling the IRQ handler. This fixes
>   interrupts following suspend.
> 
> * The cascaded IRQ is passed through the i2c_client's .irq field, rather
>   than through platform data.
> 
> --
> From bf2f95da6f192e091d9ac0b30c186050aa5fd4e5 Mon Sep 17 00:00:00 2001
> From: Gary King <gking@nvidia.com>
> Date: Mon, 30 Aug 2010 16:16:55 -0700
> Subject: mfd: tps6586x: add basic interrupt support
> 
> add support for enabling and disabling tps6586x subdevice interrupts,
> and add support for the RTC interrupts

I think the "add support for the RTC interrupts" is not relevant anymore

> Signed-off-by: Gary King <gking@nvidia.com>
> ---
>  drivers/mfd/tps6586x.c       |  202 ++++++++++++++++++++++++++++++++++++++++++
>  include/linux/mfd/tps6586x.h |   31 +++++++
>  2 files changed, 233 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/mfd/tps6586x.c b/drivers/mfd/tps6586x.c
> index 4cde31e..626081c 100644
> --- a/drivers/mfd/tps6586x.c
> +++ b/drivers/mfd/tps6586x.c
> @@ -15,6 +15,8 @@
>   * published by the Free Software Foundation.
>   */
>  
> +#include <linux/interrupt.h>
> +#include <linux/irq.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/mutex.h>

[ snip ]

>  struct tps6586x_subdev_info {
>  	int		id;
>  	const char	*name;
> @@ -29,6 +59,7 @@ struct tps6586x_platform_data {
>  	struct tps6586x_subdev_info *subdevs;
>  
>  	int gpio_base;
> +	int irq_base;

not needed anymore

>  };
>  
>  /*


-- 
Sincerely yours,
Mike.