Re: [PATCH] gpio: sch: Fix Oops on module load on Asus Eee PC 1201

[Colin Pitrat <colin.pitrat@xxxxxxxxx>]

On Fri, May 06, 2016 at 10:49:23PM +0100, Colin Pitrat wrote:
> This fixes the issue descirbe in bug 117531. 
> It's a regression introduced in linux 4.5 that causes a Oops at load of
> gpio_sch and prevents powering off the computer.
> 
> The patch consist in reverting commit 737c8fccf1c5b2aae3c6d9a66dce17e35fc39b71 
> (a.k.a 'gpio: sch: use gpiochip data pointer') that causes this regression.
> However, although it does work for me, I'm not sure of the impact of reverting
> only this part of the patch.
> 
> Signed-off-by: Colin Pitrat <colin.pitrat@xxxxxxxxx>
> 
> ---
>  drivers/gpio/gpio-sch.c | 23 +++++++++++++++++------
>  1 file changed, 17 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-sch.c b/drivers/gpio/gpio-sch.c
> index e85e753..a8a333ad 100644
> --- a/drivers/gpio/gpio-sch.c
> +++ b/drivers/gpio/gpio-sch.c
> @@ -41,6 +41,8 @@ struct sch_gpio {
>  	unsigned short resume_base;
>  };
>  
> +#define to_sch_gpio(gc)	container_of(gc, struct sch_gpio, chip)
> +
>  static unsigned sch_gpio_offset(struct sch_gpio *sch, unsigned gpio,
>  				unsigned reg)
>  {
> @@ -63,7 +65,7 @@ static unsigned sch_gpio_bit(struct sch_gpio *sch, unsigned gpio)
>  
>  static int sch_gpio_reg_get(struct gpio_chip *gc, unsigned gpio, unsigned reg)
>  {
> -	struct sch_gpio *sch = gpiochip_get_data(gc);
> +	struct sch_gpio *sch = to_sch_gpio(gc);
>  	unsigned short offset, bit;
>  	u8 reg_val;
>  
> @@ -78,7 +80,7 @@ static int sch_gpio_reg_get(struct gpio_chip *gc, unsigned gpio, unsigned reg)
>  static void sch_gpio_reg_set(struct gpio_chip *gc, unsigned gpio, unsigned reg,
>  			     int val)
>  {
> -	struct sch_gpio *sch = gpiochip_get_data(gc);
> +	struct sch_gpio *sch = to_sch_gpio(gc);
>  	unsigned short offset, bit;
>  	u8 reg_val;
>  
> @@ -95,7 +97,7 @@ static void sch_gpio_reg_set(struct gpio_chip *gc, unsigned gpio, unsigned reg,
>  
>  static int sch_gpio_direction_in(struct gpio_chip *gc, unsigned gpio_num)
>  {
> -	struct sch_gpio *sch = gpiochip_get_data(gc);
> +	struct sch_gpio *sch = to_sch_gpio(gc);
>  
>  	spin_lock(&sch->lock);
>  	sch_gpio_reg_set(gc, gpio_num, GIO, 1);
> @@ -110,7 +112,7 @@ static int sch_gpio_get(struct gpio_chip *gc, unsigned gpio_num)
>  
>  static void sch_gpio_set(struct gpio_chip *gc, unsigned gpio_num, int val)
>  {
> -	struct sch_gpio *sch = gpiochip_get_data(gc);
> +	struct sch_gpio *sch = to_sch_gpio(gc);
>  
>  	spin_lock(&sch->lock);
>  	sch_gpio_reg_set(gc, gpio_num, GLV, val);
> @@ -120,7 +122,7 @@ static void sch_gpio_set(struct gpio_chip *gc, unsigned gpio_num, int val)
>  static int sch_gpio_direction_out(struct gpio_chip *gc, unsigned gpio_num,
>  				  int val)
>  {
> -	struct sch_gpio *sch = gpiochip_get_data(gc);
> +	struct sch_gpio *sch = to_sch_gpio(gc);
>  
>  	spin_lock(&sch->lock);
>  	sch_gpio_reg_set(gc, gpio_num, GIO, 0);
> @@ -215,7 +217,15 @@ static int sch_gpio_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, sch);
>  
> -	return devm_gpiochip_add_data(&pdev->dev, &sch->chip, sch);
> +	return gpiochip_add(&sch->chip);
> +}
> +
> +static int sch_gpio_remove(struct platform_device *pdev)
> +{
> +	struct sch_gpio *sch = platform_get_drvdata(pdev);
> +
> +	gpiochip_remove(&sch->chip);
> +	return 0;
>  }
>  
>  static struct platform_driver sch_gpio_driver = {
> @@ -223,6 +233,7 @@ static struct platform_driver sch_gpio_driver = {
>  		.name = "sch_gpio",
>  	},
>  	.probe		= sch_gpio_probe,
> +	.remove		= sch_gpio_remove,
>  };
>  
>  module_platform_driver(sch_gpio_driver);
> -- 
> 2.8.2
> 

Oups sorry, this patch is wrong !
It reverts two changes instead of just the one causing the issue.
I'll send a v2 with only the required change reverted.

Sorry for the inconvenience.
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html