gpiolib: fix issue of gpio acpi handler
diff mbox

Message ID 1425654518-48935-1-git-send-email-qipeng.zha@intel.com
State New
Headers show

Commit Message

qipeng.zha March 6, 2015, 3:08 p.m. UTC
From: "qipeng.zha" <qipeng.zha@intel.com>

If GPIO driver use pin mapping, need to tranlate pin number
between ACPI table and GPIO driver.

This issue is found on one platform with Cherryview gpio
controller, kernel is hang when executed _PS0 method of
one ACPI device, since without this translation, it access
invalid gpiodesc array.

Verified it works again with this patch.

Signed-off-by: qipeng.zha <qipeng.zha@intel.com>
---
 drivers/gpio/gpiolib-acpi.c | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Mika Westerberg March 9, 2015, 11:12 a.m. UTC | #1
On Fri, Mar 06, 2015 at 11:08:38PM +0800, qipeng.zha wrote:
> From: "qipeng.zha" <qipeng.zha@intel.com>
> 
> If GPIO driver use pin mapping, need to tranlate pin number
                                          ^^^^^^^^
typo, should be "translate"

> between ACPI table and GPIO driver.
> 
> This issue is found on one platform with Cherryview gpio
> controller, kernel is hang when executed _PS0 method of
> one ACPI device, since without this translation, it access
> invalid gpiodesc array.
> 
> Verified it works again with this patch.
> 
> Signed-off-by: qipeng.zha <qipeng.zha@intel.com>
> ---
>  drivers/gpio/gpiolib-acpi.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
> index c0929d9..661e272 100644
> --- a/drivers/gpio/gpiolib-acpi.c
> +++ b/drivers/gpio/gpiolib-acpi.c
> @@ -551,6 +551,13 @@ acpi_gpio_adr_space_handler(u32 function, acpi_physical_address address,
>  		struct gpio_desc *desc;
>  		bool found;
>  
> +		/*Translate ACPI gpio pin to offset in gpio driver*/

/* Comments look like this */

notice that there is space between /* and beginning of the sentence.

But this comment does not add any new information so I would just drop
it.

Otherwise this patch seems right to me.

BTW, I wonder if we need to do the same in
acpi_gpiochip_request_interrupt()?

> +		pin = acpi_gpiochip_pin_to_gpio_offset(chip, pin);
> +		if (pin < 0) {
> +			status = AE_BAD_PARAMETER;
> +			goto out;
> +		}
> +
>  		mutex_lock(&achip->conn_lock);
>  
>  		found = false;
> -- 
> 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
Mika Westerberg March 9, 2015, 11:14 a.m. UTC | #2
On Mon, Mar 09, 2015 at 01:12:03PM +0200, Mika Westerberg wrote:
> On Fri, Mar 06, 2015 at 11:08:38PM +0800, qipeng.zha wrote:
> > From: "qipeng.zha" <qipeng.zha@intel.com>
> > 
> > If GPIO driver use pin mapping, need to tranlate pin number
>                                           ^^^^^^^^
> typo, should be "translate"
> 
> > between ACPI table and GPIO driver.
> > 
> > This issue is found on one platform with Cherryview gpio
> > controller, kernel is hang when executed _PS0 method of
> > one ACPI device, since without this translation, it access
> > invalid gpiodesc array.
> > 
> > Verified it works again with this patch.
> > 
> > Signed-off-by: qipeng.zha <qipeng.zha@intel.com>
> > ---
> >  drivers/gpio/gpiolib-acpi.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
> > index c0929d9..661e272 100644
> > --- a/drivers/gpio/gpiolib-acpi.c
> > +++ b/drivers/gpio/gpiolib-acpi.c
> > @@ -551,6 +551,13 @@ acpi_gpio_adr_space_handler(u32 function, acpi_physical_address address,
> >  		struct gpio_desc *desc;
> >  		bool found;
> >  
> > +		/*Translate ACPI gpio pin to offset in gpio driver*/
> 
> /* Comments look like this */
> 
> notice that there is space between /* and beginning of the sentence.
> 
> But this comment does not add any new information so I would just drop
> it.
> 
> Otherwise this patch seems right to me.

One more thing, though. The title of the patch should be more
descriptive. So that you get the idea what the patch is fixing when you
run "git log --oneline".
--
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

Patch
diff mbox

diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index c0929d9..661e272 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -551,6 +551,13 @@  acpi_gpio_adr_space_handler(u32 function, acpi_physical_address address,
 		struct gpio_desc *desc;
 		bool found;
 
+		/*Translate ACPI gpio pin to offset in gpio driver*/
+		pin = acpi_gpiochip_pin_to_gpio_offset(chip, pin);
+		if (pin < 0) {
+			status = AE_BAD_PARAMETER;
+			goto out;
+		}
+
 		mutex_lock(&achip->conn_lock);
 
 		found = false;