diff mbox

[3/3] pinctrl:Intel: make the high level interrupt working

Message ID 1457715962-108484-3-git-send-email-qipeng.zha@intel.com
State New
Headers show

Commit Message

qipeng.zha March 11, 2016, 5:06 p.m. UTC 
High level trigger mode of GPIO interrupt is not set correctly
in intel_gpio_irq_type(), and will make this kind of interrupt
not respond.

Signed-off-by: Qi Zheng <qi.zheng@intel.com>
Signed-off-by: Qipeng Zha <qipeng.zha@intel.com>
---
 drivers/pinctrl/intel/pinctrl-intel.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Mika Westerberg March 11, 2016, 9:49 a.m. UTC | #1 
On Sat, Mar 12, 2016 at 01:06:02AM +0800, Qipeng Zha wrote:
> High level trigger mode of GPIO interrupt is not set correctly
> in intel_gpio_irq_type(), and will make this kind of interrupt
> not respond.

Good finding.

> Signed-off-by: Qi Zheng <qi.zheng@intel.com>
> Signed-off-by: Qipeng Zha <qipeng.zha@intel.com>
> ---
>  drivers/pinctrl/intel/pinctrl-intel.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/pinctrl/intel/pinctrl-intel.c b/drivers/pinctrl/intel/pinctrl-intel.c
> index d6fe659..706a21f 100644
> --- a/drivers/pinctrl/intel/pinctrl-intel.c
> +++ b/drivers/pinctrl/intel/pinctrl-intel.c
> @@ -790,6 +790,8 @@ static int intel_gpio_irq_type(struct irq_data *d, unsigned type)
>  		value |= PADCFG0_RXEVCFG_EDGE << PADCFG0_RXEVCFG_SHIFT;
>  	} else if (type & IRQ_TYPE_LEVEL_LOW) {
>  		value |= PADCFG0_RXINV;
> +	} else if (type & IRQ_TYPE_LEVEL_HIGH) {
> +		;

What about following instead?

	} else if (type & IRQ_TYPE_LEVEL_MASK) {
		if (type & IRQ_TYPE_LEVEL_LOW)
			value |= PADCFG0_RXINV;
	}


>  	} else {
>  		value |= PADCFG0_RXEVCFG_DISABLED << PADCFG0_RXEVCFG_SHIFT;
>  	}
> -- 
> 1.8.3.2
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Zheng, Qi March 14, 2016, 1:26 a.m. UTC | #2 
On Sat, Mar 12, 2016 at 01:06:02AM +0800, Qipeng Zha wrote:
> High level trigger mode of GPIO interrupt is not set correctly in 
> intel_gpio_irq_type(), and will make this kind of interrupt not 
> respond.

> Good finding.

> Signed-off-by: Qi Zheng <qi.zheng@intel.com>
> Signed-off-by: Qipeng Zha <qipeng.zha@intel.com>
> ---
>  drivers/pinctrl/intel/pinctrl-intel.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/pinctrl/intel/pinctrl-intel.c 
> b/drivers/pinctrl/intel/pinctrl-intel.c
> index d6fe659..706a21f 100644
> --- a/drivers/pinctrl/intel/pinctrl-intel.c
> +++ b/drivers/pinctrl/intel/pinctrl-intel.c
> @@ -790,6 +790,8 @@ static int intel_gpio_irq_type(struct irq_data *d, unsigned type)
>  		value |= PADCFG0_RXEVCFG_EDGE << PADCFG0_RXEVCFG_SHIFT;
>  	} else if (type & IRQ_TYPE_LEVEL_LOW) {
>  		value |= PADCFG0_RXINV;
> +	} else if (type & IRQ_TYPE_LEVEL_HIGH) {
> +		;

> What about following instead?

Nothing need to do.
Because the default setting below  in the intel_gpio_irq_type is for high level mode interrupt. 
	value &= ~(PADCFG0_RXEVCFG_MASK | PADCFG0_RXINV);

>
>	} else if (type & IRQ_TYPE_LEVEL_MASK) {
>		if (type & IRQ_TYPE_LEVEL_LOW)
>			value |= PADCFG0_RXINV;
>	}



>  	} else {
>  		value |= PADCFG0_RXEVCFG_DISABLED << PADCFG0_RXEVCFG_SHIFT;
>  	}
> --
> 1.8.3.2
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Zheng, Qi March 14, 2016, 1:40 a.m. UTC | #3 
On Sat, Mar 12, 2016 at 01:06:02AM +0800, Qipeng Zha wrote:
> High level trigger mode of GPIO interrupt is not set correctly in 
> intel_gpio_irq_type(), and will make this kind of interrupt not 
> respond.

> Good finding.

> Signed-off-by: Qi Zheng <qi.zheng@intel.com>
> Signed-off-by: Qipeng Zha <qipeng.zha@intel.com>
> ---
>  drivers/pinctrl/intel/pinctrl-intel.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/pinctrl/intel/pinctrl-intel.c
> b/drivers/pinctrl/intel/pinctrl-intel.c
> index d6fe659..706a21f 100644
> --- a/drivers/pinctrl/intel/pinctrl-intel.c
> +++ b/drivers/pinctrl/intel/pinctrl-intel.c
> @@ -790,6 +790,8 @@ static int intel_gpio_irq_type(struct irq_data *d, unsigned type)
>  		value |= PADCFG0_RXEVCFG_EDGE << PADCFG0_RXEVCFG_SHIFT;
>  	} else if (type & IRQ_TYPE_LEVEL_LOW) {
>  		value |= PADCFG0_RXINV;
> +	} else if (type & IRQ_TYPE_LEVEL_HIGH) {
> +		;

> What about following instead?

> Nothing need to do.
> Because the default setting below  in the intel_gpio_irq_type is for high level mode interrupt. 
>	value &= ~(PADCFG0_RXEVCFG_MASK | PADCFG0_RXINV);

Sorry, misunderstood your points.
The following looks fine to me.
Thanks.

>
>	} else if (type & IRQ_TYPE_LEVEL_MASK) {
>		if (type & IRQ_TYPE_LEVEL_LOW)
>			value |= PADCFG0_RXINV;
>	}



>  	} else {
>  		value |= PADCFG0_RXEVCFG_DISABLED << PADCFG0_RXEVCFG_SHIFT;
>  	}
> --
> 1.8.3.2
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/pinctrl/intel/pinctrl-intel.c b/drivers/pinctrl/intel/pinctrl-intel.c
index d6fe659..706a21f 100644
--- a/drivers/pinctrl/intel/pinctrl-intel.c
+++ b/drivers/pinctrl/intel/pinctrl-intel.c
@@ -790,6 +790,8 @@  static int intel_gpio_irq_type(struct irq_data *d, unsigned type)
 		value |= PADCFG0_RXEVCFG_EDGE << PADCFG0_RXEVCFG_SHIFT;
 	} else if (type & IRQ_TYPE_LEVEL_LOW) {
 		value |= PADCFG0_RXINV;
+	} else if (type & IRQ_TYPE_LEVEL_HIGH) {
+		;
 	} else {
 		value |= PADCFG0_RXEVCFG_DISABLED << PADCFG0_RXEVCFG_SHIFT;
 	}