Re: [PATCH 3/3] gpio-langwell: implement irq shutdown interface

[Grant Likely <grant.likely@secretlab.ca>]

On Tue, 18 Dec 2012 17:52:13 -0800, David Cohen <david.a.cohen@intel.com> wrote:
> From: "Li, Ning" <ning.li@intel.com>
> 
> Signed-off-by: David Cohen <david.a.cohen@intel.com>
> Signed-off-by: Li, Ning <ning.li@intel.com>

I could use some help interpreting this patch since it doesn't have any
commit text describing why this patch is needed, nor does it have any
comments in the code describing what it is doing.

Having good commit text is important. Use it to tell me and future
readers why this patch is important. What it does and how it was tested.
Otherwise I end up trying to read your mind.

(The patch does actually look fine to me, but it isn't okay to neglect
a patch descirption on anything other than the most trivial of
patches... and even then it triggers severe eyebrow raising)

g.

> ---
>  drivers/gpio/gpio-langwell.c |   18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/gpio/gpio-langwell.c b/drivers/gpio/gpio-langwell.c
> index dc534a9..c702d3d 100644
> --- a/drivers/gpio/gpio-langwell.c
> +++ b/drivers/gpio/gpio-langwell.c
> @@ -219,12 +219,30 @@ static int lnw_irq_type(struct irq_data *d, unsigned type)
>  
>  static void lnw_irq_noop(struct irq_data *d) {}
>  
> +static void lnw_irq_shutdown(struct irq_data *d)
> +{
> +	struct lnw_gpio *lnw = irq_data_get_irq_chip_data(d);
> +	u32 gpio = irqd_to_hwirq(d);
> +	unsigned long flags;
> +	u32 value;
> +	void __iomem *grer = gpio_reg(&lnw->chip, gpio, GRER);
> +	void __iomem *gfer = gpio_reg(&lnw->chip, gpio, GFER);
> +
> +	spin_lock_irqsave(&lnw->lock, flags);
> +	value = readl(grer) & ~BIT(gpio % 32);
> +	writel(value, grer);
> +	value = readl(gfer) & ~BIT(gpio % 32);
> +	writel(value, gfer);
> +	spin_unlock_irqrestore(&lnw->lock, flags);
> +};
> +
>  static struct irq_chip lnw_irqchip = {
>  	.name		= "LNW-GPIO",
>  	.irq_mask	= lnw_irq_noop,
>  	.irq_unmask	= lnw_irq_noop,
>  	.irq_set_type	= lnw_irq_type,
>  	.irq_ack	= lnw_irq_noop,
> +	.irq_shutdown	= lnw_irq_shutdown,
>  };
>  
>  static DEFINE_PCI_DEVICE_TABLE(lnw_gpio_ids) = {   /* pin number */
> -- 
> 1.7.10.4
> 

-- 
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.