All of lore.kernel.org
 help / color / mirror / Atom feed
* [GIT PULL] intel-gpio for 5.4-2
@ 2019-10-09 16:50 Andy Shevchenko
  2019-10-09 16:50 ` [PATCH 1/5] gpio: merrifield: Restore use of irq_base Andy Shevchenko
                   ` (5 more replies)
  0 siblings, 6 replies; 15+ messages in thread
From: Andy Shevchenko @ 2019-10-09 16:50 UTC (permalink / raw)
  To: Linux GPIO; +Cc: Linus Walleij, Bartosz Golaszewski, Andy Shevchenko

Hi Linux GPIO  maintainers,

Small set of urgent fixes against Intel GPIO drivers that have been recently
converted to use GPIO core for instantiation of IRQ chip. No conflicts observed
with fixes branch of GPIO tree as of today.

Thanks,

With Best Regards,
Andy Shevchenko

The following changes since commit 54ecb8f7028c5eb3d740bb82b0f1d90f2df63c5c:

  Linux 5.4-rc1 (2019-09-30 10:35:40 -0700)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/andy/linux-gpio-intel.git tags/intel-gpio-v5.4-2

for you to fetch changes up to 6ed26a5326f6da6e1950b8476173df51a92a96be:

  gpio: lynxpoint: set default handler to be handle_bad_irq() (2019-10-09 19:00:13 +0300)

----------------------------------------------------------------
intel-gpio for v5.4-2

The conversion to use of GPIO core to instantiate IRQ chip produced
several issues among the drivers. Here is the set of fixes to:
 * initialize hardware before IRQ chip will be added
 * initialize hardware without NULL pointer dereference
 * assign IRQ base

The following is an automated git shortlog grouped by driver:

gpiolib:
 -  Initialize the hardware with a callback

intel-mid:
 -  Move hardware initialization to callback

lynxpoint:
 -  set default handler to be handle_bad_irq()
 -  Move hardware initialization to callback

merrifield:
 -  Move hardware initialization to callback
 -  Restore use of irq_base

----------------------------------------------------------------
Andy Shevchenko (6):
      gpio: merrifield: Restore use of irq_base
      gpiolib: Initialize the hardware with a callback
      gpio: intel-mid: Move hardware initialization to callback
      gpio: lynxpoint: Move hardware initialization to callback
      gpio: merrifield: Move hardware initialization to callback
      gpio: lynxpoint: set default handler to be handle_bad_irq()

 drivers/gpio/gpio-intel-mid.c  |  9 ++++++---
 drivers/gpio/gpio-lynxpoint.c  | 10 ++++++----
 drivers/gpio/gpio-merrifield.c |  9 ++++++---
 drivers/gpio/gpiolib.c         | 22 +++++++++++++++++++++-
 include/linux/gpio/driver.h    |  8 ++++++++
 5 files changed, 47 insertions(+), 11 deletions(-)

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 1/5] gpio: merrifield: Restore use of irq_base
  2019-10-09 16:50 [GIT PULL] intel-gpio for 5.4-2 Andy Shevchenko
@ 2019-10-09 16:50 ` Andy Shevchenko
  2019-10-11 16:21   ` Ferry Toth
  2019-10-09 16:50 ` [PATCH 2/5] gpiolib: Initialize the hardware with a callback Andy Shevchenko
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 15+ messages in thread
From: Andy Shevchenko @ 2019-10-09 16:50 UTC (permalink / raw)
  To: Linux GPIO; +Cc: Linus Walleij, Bartosz Golaszewski, Andy Shevchenko

During conversion to internal IRQ chip initialization the commit
  8f86a5b4ad67 ("gpio: merrifield: Pass irqchip when adding gpiochip")
lost the irq_base assignment.

drivers/gpio/gpio-merrifield.c: In function ‘mrfld_gpio_probe’:
drivers/gpio/gpio-merrifield.c:405:17: warning: variable ‘irq_base’ set but not used [-Wunused-but-set-variable]

Assign the girq->first to it.

Fixes: 8f86a5b4ad67 ("gpio: merrifield: Pass irqchip when adding gpiochip")
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/gpio/gpio-merrifield.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpio/gpio-merrifield.c b/drivers/gpio/gpio-merrifield.c
index 4f27ddfe1e2f..9596024c9161 100644
--- a/drivers/gpio/gpio-merrifield.c
+++ b/drivers/gpio/gpio-merrifield.c
@@ -455,6 +455,7 @@ static int mrfld_gpio_probe(struct pci_dev *pdev, const struct pci_device_id *id
 	if (!girq->parents)
 		return -ENOMEM;
 	girq->parents[0] = pdev->irq;
+	girq->first = irq_base;
 	girq->default_type = IRQ_TYPE_NONE;
 	girq->handler = handle_bad_irq;
 
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 2/5] gpiolib: Initialize the hardware with a callback
  2019-10-09 16:50 [GIT PULL] intel-gpio for 5.4-2 Andy Shevchenko
  2019-10-09 16:50 ` [PATCH 1/5] gpio: merrifield: Restore use of irq_base Andy Shevchenko
@ 2019-10-09 16:50 ` Andy Shevchenko
  2019-10-09 19:44   ` Hans de Goede
  2019-10-10 23:18   ` Linus Walleij
  2019-10-09 16:50 ` [PATCH 3/5] gpio: intel-mid: Move hardware initialization to callback Andy Shevchenko
                   ` (3 subsequent siblings)
  5 siblings, 2 replies; 15+ messages in thread
From: Andy Shevchenko @ 2019-10-09 16:50 UTC (permalink / raw)
  To: Linux GPIO
  Cc: Linus Walleij, Bartosz Golaszewski, Andy Shevchenko, Hans de Goede

After changing the drivers to use GPIO core to add an IRQ chip
it appears that some of them requires a hardware initialization
before adding the IRQ chip.

Add an optional callback ->init_hw() to allow that drivers
to initialize hardware if needed.

This change is a part of the fix NULL pointer dereference
brought to the several drivers recently.

Cc: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/gpio/gpiolib.c      | 22 +++++++++++++++++++++-
 include/linux/gpio/driver.h |  8 ++++++++
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index bdbc1649eafa..85601ad4fcd5 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -86,6 +86,7 @@ static int gpiochip_add_irqchip(struct gpio_chip *gpiochip,
 				struct lock_class_key *lock_key,
 				struct lock_class_key *request_key);
 static void gpiochip_irqchip_remove(struct gpio_chip *gpiochip);
+static int gpiochip_irqchip_init_hw(struct gpio_chip *gpiochip);
 static int gpiochip_irqchip_init_valid_mask(struct gpio_chip *gpiochip);
 static void gpiochip_irqchip_free_valid_mask(struct gpio_chip *gpiochip);
 
@@ -1406,6 +1407,10 @@ int gpiochip_add_data_with_key(struct gpio_chip *chip, void *data,
 
 	machine_gpiochip_add(chip);
 
+	ret = gpiochip_irqchip_init_hw(chip);
+	if (ret)
+		goto err_remove_acpi_chip;
+
 	ret = gpiochip_irqchip_init_valid_mask(chip);
 	if (ret)
 		goto err_remove_acpi_chip;
@@ -1622,6 +1627,16 @@ static struct gpio_chip *find_chip_by_name(const char *name)
  * The following is irqchip helper code for gpiochips.
  */
 
+static int gpiochip_irqchip_init_hw(struct gpio_chip *gc)
+{
+	struct gpio_irq_chip *girq = &gc->irq;
+
+	if (!girq->init_hw)
+		return 0;
+
+	return girq->init_hw(gc);
+}
+
 static int gpiochip_irqchip_init_valid_mask(struct gpio_chip *gc)
 {
 	struct gpio_irq_chip *girq = &gc->irq;
@@ -2446,8 +2461,13 @@ static inline int gpiochip_add_irqchip(struct gpio_chip *gpiochip,
 {
 	return 0;
 }
-
 static void gpiochip_irqchip_remove(struct gpio_chip *gpiochip) {}
+
+static inline int gpiochip_irqchip_init_hw(struct gpio_chip *gpiochip)
+{
+	return 0;
+}
+
 static inline int gpiochip_irqchip_init_valid_mask(struct gpio_chip *gpiochip)
 {
 	return 0;
diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
index f8245d67f070..5dd9c982e2cb 100644
--- a/include/linux/gpio/driver.h
+++ b/include/linux/gpio/driver.h
@@ -201,6 +201,14 @@ struct gpio_irq_chip {
 	 */
 	bool threaded;
 
+	/**
+	 * @init_hw: optional routine to initialize hardware before
+	 * an IRQ chip will be added. This is quite useful when
+	 * a particular driver wants to clear IRQ related registers
+	 * in order to avoid undesired events.
+	 */
+	int (*init_hw)(struct gpio_chip *chip);
+
 	/**
 	 * @init_valid_mask: optional routine to initialize @valid_mask, to be
 	 * used if not all GPIO lines are valid interrupts. Sometimes some
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 3/5] gpio: intel-mid: Move hardware initialization to callback
  2019-10-09 16:50 [GIT PULL] intel-gpio for 5.4-2 Andy Shevchenko
  2019-10-09 16:50 ` [PATCH 1/5] gpio: merrifield: Restore use of irq_base Andy Shevchenko
  2019-10-09 16:50 ` [PATCH 2/5] gpiolib: Initialize the hardware with a callback Andy Shevchenko
@ 2019-10-09 16:50 ` Andy Shevchenko
  2019-10-09 16:50 ` [PATCH 4/5] gpio: lynxpoint: " Andy Shevchenko
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 15+ messages in thread
From: Andy Shevchenko @ 2019-10-09 16:50 UTC (permalink / raw)
  To: Linux GPIO; +Cc: Linus Walleij, Bartosz Golaszewski, Andy Shevchenko

The driver wants to initialize related registers before IRQ chip will be added.
That's why move it to a corresponding callback. It also fixes the NULL pointer
dereference.

Fixes: 8069e69a9792 ("gpio: intel-mid: Pass irqchip when adding gpiochip")
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/gpio/gpio-intel-mid.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/gpio/gpio-intel-mid.c b/drivers/gpio/gpio-intel-mid.c
index 4d835f9089df..86a10c808ef6 100644
--- a/drivers/gpio/gpio-intel-mid.c
+++ b/drivers/gpio/gpio-intel-mid.c
@@ -293,8 +293,9 @@ static void intel_mid_irq_handler(struct irq_desc *desc)
 	chip->irq_eoi(data);
 }
 
-static void intel_mid_irq_init_hw(struct intel_mid_gpio *priv)
+static int intel_mid_irq_init_hw(struct gpio_chip *chip)
 {
+	struct intel_mid_gpio *priv = gpiochip_get_data(chip);
 	void __iomem *reg;
 	unsigned base;
 
@@ -309,6 +310,8 @@ static void intel_mid_irq_init_hw(struct intel_mid_gpio *priv)
 		reg = gpio_reg(&priv->chip, base, GEDR);
 		writel(~0, reg);
 	}
+
+	return 0;
 }
 
 static int __maybe_unused intel_gpio_runtime_idle(struct device *dev)
@@ -372,6 +375,7 @@ static int intel_gpio_probe(struct pci_dev *pdev,
 
 	girq = &priv->chip.irq;
 	girq->chip = &intel_mid_irqchip;
+	girq->init_hw = intel_mid_irq_init_hw;
 	girq->parent_handler = intel_mid_irq_handler;
 	girq->num_parents = 1;
 	girq->parents = devm_kcalloc(&pdev->dev, girq->num_parents,
@@ -384,9 +388,8 @@ static int intel_gpio_probe(struct pci_dev *pdev,
 	girq->default_type = IRQ_TYPE_NONE;
 	girq->handler = handle_simple_irq;
 
-	intel_mid_irq_init_hw(priv);
-
 	pci_set_drvdata(pdev, priv);
+
 	retval = devm_gpiochip_add_data(&pdev->dev, &priv->chip, priv);
 	if (retval) {
 		dev_err(&pdev->dev, "gpiochip_add error %d\n", retval);
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 4/5] gpio: lynxpoint: Move hardware initialization to callback
  2019-10-09 16:50 [GIT PULL] intel-gpio for 5.4-2 Andy Shevchenko
                   ` (2 preceding siblings ...)
  2019-10-09 16:50 ` [PATCH 3/5] gpio: intel-mid: Move hardware initialization to callback Andy Shevchenko
@ 2019-10-09 16:50 ` Andy Shevchenko
  2019-10-09 16:50 ` [PATCH 5/5] gpio: merrifield: " Andy Shevchenko
  2019-10-10 23:22 ` [GIT PULL] intel-gpio for 5.4-2 Linus Walleij
  5 siblings, 0 replies; 15+ messages in thread
From: Andy Shevchenko @ 2019-10-09 16:50 UTC (permalink / raw)
  To: Linux GPIO; +Cc: Linus Walleij, Bartosz Golaszewski, Andy Shevchenko

The driver wants to initialize related registers before IRQ chip will be added.
That's why move it to a corresponding callback. It also fixes the NULL pointer
dereference.

Fixes: 7b1e889436a1 ("gpio: lynxpoint: Pass irqchip when adding gpiochip")
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/gpio/gpio-lynxpoint.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/gpio/gpio-lynxpoint.c b/drivers/gpio/gpio-lynxpoint.c
index 6bb9741ad036..082cd143f430 100644
--- a/drivers/gpio/gpio-lynxpoint.c
+++ b/drivers/gpio/gpio-lynxpoint.c
@@ -294,8 +294,9 @@ static struct irq_chip lp_irqchip = {
 	.flags = IRQCHIP_SKIP_SET_WAKE,
 };
 
-static void lp_gpio_irq_init_hw(struct lp_gpio *lg)
+static int lp_gpio_irq_init_hw(struct gpio_chip *chip)
 {
+	struct lp_gpio *lg = gpiochip_get_data(chip);
 	unsigned long reg;
 	unsigned base;
 
@@ -307,6 +308,8 @@ static void lp_gpio_irq_init_hw(struct lp_gpio *lg)
 		reg = lp_gpio_reg(&lg->chip, base, LP_INT_STAT);
 		outl(0xffffffff, reg);
 	}
+
+	return 0;
 }
 
 static int lp_gpio_probe(struct platform_device *pdev)
@@ -364,6 +367,7 @@ static int lp_gpio_probe(struct platform_device *pdev)
 
 		girq = &gc->irq;
 		girq->chip = &lp_irqchip;
+		girq->init_hw = lp_gpio_irq_init_hw;
 		girq->parent_handler = lp_gpio_irq_handler;
 		girq->num_parents = 1;
 		girq->parents = devm_kcalloc(&pdev->dev, girq->num_parents,
@@ -374,8 +378,6 @@ static int lp_gpio_probe(struct platform_device *pdev)
 		girq->parents[0] = (unsigned)irq_rc->start;
 		girq->default_type = IRQ_TYPE_NONE;
 		girq->handler = handle_simple_irq;
-
-		lp_gpio_irq_init_hw(lg);
 	}
 
 	ret = devm_gpiochip_add_data(dev, gc, lg);
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 5/5] gpio: merrifield: Move hardware initialization to callback
  2019-10-09 16:50 [GIT PULL] intel-gpio for 5.4-2 Andy Shevchenko
                   ` (3 preceding siblings ...)
  2019-10-09 16:50 ` [PATCH 4/5] gpio: lynxpoint: " Andy Shevchenko
@ 2019-10-09 16:50 ` Andy Shevchenko
  2019-10-11 16:23   ` Ferry Toth
  2019-10-10 23:22 ` [GIT PULL] intel-gpio for 5.4-2 Linus Walleij
  5 siblings, 1 reply; 15+ messages in thread
From: Andy Shevchenko @ 2019-10-09 16:50 UTC (permalink / raw)
  To: Linux GPIO; +Cc: Linus Walleij, Bartosz Golaszewski, Andy Shevchenko

The driver wants to initialize related registers before IRQ chip will be added.
That's why move it to a corresponding callback. It also fixes the NULL pointer
dereference.

Fixes: 8f86a5b4ad67 ("gpio: merrifield: Pass irqchip when adding gpiochip")
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/gpio/gpio-merrifield.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/gpio/gpio-merrifield.c b/drivers/gpio/gpio-merrifield.c
index 9596024c9161..2f1e9da81c1e 100644
--- a/drivers/gpio/gpio-merrifield.c
+++ b/drivers/gpio/gpio-merrifield.c
@@ -362,8 +362,9 @@ static void mrfld_irq_handler(struct irq_desc *desc)
 	chained_irq_exit(irqchip, desc);
 }
 
-static void mrfld_irq_init_hw(struct mrfld_gpio *priv)
+static int mrfld_irq_init_hw(struct gpio_chip *chip)
 {
+	struct mrfld_gpio *priv = gpiochip_get_data(chip);
 	void __iomem *reg;
 	unsigned int base;
 
@@ -375,6 +376,8 @@ static void mrfld_irq_init_hw(struct mrfld_gpio *priv)
 		reg = gpio_reg(&priv->chip, base, GFER);
 		writel(0, reg);
 	}
+
+	return 0;
 }
 
 static const char *mrfld_gpio_get_pinctrl_dev_name(struct mrfld_gpio *priv)
@@ -447,6 +450,7 @@ static int mrfld_gpio_probe(struct pci_dev *pdev, const struct pci_device_id *id
 
 	girq = &priv->chip.irq;
 	girq->chip = &mrfld_irqchip;
+	girq->init_hw = mrfld_irq_init_hw;
 	girq->parent_handler = mrfld_irq_handler;
 	girq->num_parents = 1;
 	girq->parents = devm_kcalloc(&pdev->dev, girq->num_parents,
@@ -459,8 +463,6 @@ static int mrfld_gpio_probe(struct pci_dev *pdev, const struct pci_device_id *id
 	girq->default_type = IRQ_TYPE_NONE;
 	girq->handler = handle_bad_irq;
 
-	mrfld_irq_init_hw(priv);
-
 	pci_set_drvdata(pdev, priv);
 	retval = devm_gpiochip_add_data(&pdev->dev, &priv->chip, priv);
 	if (retval) {
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/5] gpiolib: Initialize the hardware with a callback
  2019-10-09 16:50 ` [PATCH 2/5] gpiolib: Initialize the hardware with a callback Andy Shevchenko
@ 2019-10-09 19:44   ` Hans de Goede
  2019-10-10  7:23     ` Andy Shevchenko
  2019-10-10 23:18   ` Linus Walleij
  1 sibling, 1 reply; 15+ messages in thread
From: Hans de Goede @ 2019-10-09 19:44 UTC (permalink / raw)
  To: Andy Shevchenko, Linux GPIO; +Cc: Linus Walleij, Bartosz Golaszewski

Hi,

On 09-10-2019 18:50, Andy Shevchenko wrote:
> After changing the drivers to use GPIO core to add an IRQ chip
> it appears that some of them requires a hardware initialization
> before adding the IRQ chip.
> 
> Add an optional callback ->init_hw() to allow that drivers
> to initialize hardware if needed.
> 
> This change is a part of the fix NULL pointer dereference
> brought to the several drivers recently.
> 
> Cc: Hans de Goede <hdegoede@redhat.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Hmm, IIRC Linus Walleij already added a callback for initializing the
mask before the irchip gets initialized which is basically intended for
what you want this callback for I think ?

Regards.

Hans



> ---
>   drivers/gpio/gpiolib.c      | 22 +++++++++++++++++++++-
>   include/linux/gpio/driver.h |  8 ++++++++
>   2 files changed, 29 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index bdbc1649eafa..85601ad4fcd5 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -86,6 +86,7 @@ static int gpiochip_add_irqchip(struct gpio_chip *gpiochip,
>   				struct lock_class_key *lock_key,
>   				struct lock_class_key *request_key);
>   static void gpiochip_irqchip_remove(struct gpio_chip *gpiochip);
> +static int gpiochip_irqchip_init_hw(struct gpio_chip *gpiochip);
>   static int gpiochip_irqchip_init_valid_mask(struct gpio_chip *gpiochip);
>   static void gpiochip_irqchip_free_valid_mask(struct gpio_chip *gpiochip);
>   
> @@ -1406,6 +1407,10 @@ int gpiochip_add_data_with_key(struct gpio_chip *chip, void *data,
>   
>   	machine_gpiochip_add(chip);
>   
> +	ret = gpiochip_irqchip_init_hw(chip);
> +	if (ret)
> +		goto err_remove_acpi_chip;
> +
>   	ret = gpiochip_irqchip_init_valid_mask(chip);
>   	if (ret)
>   		goto err_remove_acpi_chip;
> @@ -1622,6 +1627,16 @@ static struct gpio_chip *find_chip_by_name(const char *name)
>    * The following is irqchip helper code for gpiochips.
>    */
>   
> +static int gpiochip_irqchip_init_hw(struct gpio_chip *gc)
> +{
> +	struct gpio_irq_chip *girq = &gc->irq;
> +
> +	if (!girq->init_hw)
> +		return 0;
> +
> +	return girq->init_hw(gc);
> +}
> +
>   static int gpiochip_irqchip_init_valid_mask(struct gpio_chip *gc)
>   {
>   	struct gpio_irq_chip *girq = &gc->irq;
> @@ -2446,8 +2461,13 @@ static inline int gpiochip_add_irqchip(struct gpio_chip *gpiochip,
>   {
>   	return 0;
>   }
> -
>   static void gpiochip_irqchip_remove(struct gpio_chip *gpiochip) {}
> +
> +static inline int gpiochip_irqchip_init_hw(struct gpio_chip *gpiochip)
> +{
> +	return 0;
> +}
> +
>   static inline int gpiochip_irqchip_init_valid_mask(struct gpio_chip *gpiochip)
>   {
>   	return 0;
> diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
> index f8245d67f070..5dd9c982e2cb 100644
> --- a/include/linux/gpio/driver.h
> +++ b/include/linux/gpio/driver.h
> @@ -201,6 +201,14 @@ struct gpio_irq_chip {
>   	 */
>   	bool threaded;
>   
> +	/**
> +	 * @init_hw: optional routine to initialize hardware before
> +	 * an IRQ chip will be added. This is quite useful when
> +	 * a particular driver wants to clear IRQ related registers
> +	 * in order to avoid undesired events.
> +	 */
> +	int (*init_hw)(struct gpio_chip *chip);
> +
>   	/**
>   	 * @init_valid_mask: optional routine to initialize @valid_mask, to be
>   	 * used if not all GPIO lines are valid interrupts. Sometimes some
> 


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/5] gpiolib: Initialize the hardware with a callback
  2019-10-09 19:44   ` Hans de Goede
@ 2019-10-10  7:23     ` Andy Shevchenko
  2019-10-10  7:26       ` Andy Shevchenko
  0 siblings, 1 reply; 15+ messages in thread
From: Andy Shevchenko @ 2019-10-10  7:23 UTC (permalink / raw)
  To: Hans de Goede; +Cc: Linux GPIO, Linus Walleij, Bartosz Golaszewski

On Wed, Oct 09, 2019 at 09:44:31PM +0200, Hans de Goede wrote:
> On 09-10-2019 18:50, Andy Shevchenko wrote:
> > After changing the drivers to use GPIO core to add an IRQ chip
> > it appears that some of them requires a hardware initialization
> > before adding the IRQ chip.
> > 
> > Add an optional callback ->init_hw() to allow that drivers
> > to initialize hardware if needed.
> > 
> > This change is a part of the fix NULL pointer dereference
> > brought to the several drivers recently.
> > 
> > Cc: Hans de Goede <hdegoede@redhat.com>
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> Hmm, IIRC Linus Walleij already added a callback for initializing the
> mask before the irchip gets initialized which is basically intended for
> what you want this callback for I think ?

This is not about the mask, it's about hardware to be prepared before enabling.
Also init_valid_mask() will allocate memory which won't be needed.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/5] gpiolib: Initialize the hardware with a callback
  2019-10-10  7:23     ` Andy Shevchenko
@ 2019-10-10  7:26       ` Andy Shevchenko
  2019-10-10  7:41         ` Hans de Goede
  0 siblings, 1 reply; 15+ messages in thread
From: Andy Shevchenko @ 2019-10-10  7:26 UTC (permalink / raw)
  To: Hans de Goede; +Cc: Linux GPIO, Linus Walleij, Bartosz Golaszewski

On Thu, Oct 10, 2019 at 10:23:04AM +0300, Andy Shevchenko wrote:
> On Wed, Oct 09, 2019 at 09:44:31PM +0200, Hans de Goede wrote:
> > On 09-10-2019 18:50, Andy Shevchenko wrote:
> > > After changing the drivers to use GPIO core to add an IRQ chip
> > > it appears that some of them requires a hardware initialization
> > > before adding the IRQ chip.
> > > 
> > > Add an optional callback ->init_hw() to allow that drivers
> > > to initialize hardware if needed.
> > > 
> > > This change is a part of the fix NULL pointer dereference
> > > brought to the several drivers recently.
> > > 
> > > Cc: Hans de Goede <hdegoede@redhat.com>
> > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > 
> > Hmm, IIRC Linus Walleij already added a callback for initializing the
> > mask before the irchip gets initialized which is basically intended for
> > what you want this callback for I think ?
> 
> This is not about the mask, it's about hardware to be prepared before enabling.
> Also init_valid_mask() will allocate memory which won't be needed.

If you think this is not a proper approach, we have to revert all three patches
now (*) and think about better solution.

*) They broke the boot on all affected machines.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/5] gpiolib: Initialize the hardware with a callback
  2019-10-10  7:26       ` Andy Shevchenko
@ 2019-10-10  7:41         ` Hans de Goede
  0 siblings, 0 replies; 15+ messages in thread
From: Hans de Goede @ 2019-10-10  7:41 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: Linux GPIO, Linus Walleij, Bartosz Golaszewski

Hi,

On 10-10-2019 09:26, Andy Shevchenko wrote:
> On Thu, Oct 10, 2019 at 10:23:04AM +0300, Andy Shevchenko wrote:
>> On Wed, Oct 09, 2019 at 09:44:31PM +0200, Hans de Goede wrote:
>>> On 09-10-2019 18:50, Andy Shevchenko wrote:
>>>> After changing the drivers to use GPIO core to add an IRQ chip
>>>> it appears that some of them requires a hardware initialization
>>>> before adding the IRQ chip.
>>>>
>>>> Add an optional callback ->init_hw() to allow that drivers
>>>> to initialize hardware if needed.
>>>>
>>>> This change is a part of the fix NULL pointer dereference
>>>> brought to the several drivers recently.
>>>>
>>>> Cc: Hans de Goede <hdegoede@redhat.com>
>>>> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>>>
>>> Hmm, IIRC Linus Walleij already added a callback for initializing the
>>> mask before the irchip gets initialized which is basically intended for
>>> what you want this callback for I think ?
>>
>> This is not about the mask, it's about hardware to be prepared before enabling.
>> Also init_valid_mask() will allocate memory which won't be needed.
> 
> If you think this is not a proper approach, we have to revert all three patches
> now (*) and think about better solution.

If this is not about the valid-mask then this approach is probably fine,
sorry for the confusion. Lets wait and see what Linus Walleij has to say.

Regards,

Hans


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/5] gpiolib: Initialize the hardware with a callback
  2019-10-09 16:50 ` [PATCH 2/5] gpiolib: Initialize the hardware with a callback Andy Shevchenko
  2019-10-09 19:44   ` Hans de Goede
@ 2019-10-10 23:18   ` Linus Walleij
  1 sibling, 0 replies; 15+ messages in thread
From: Linus Walleij @ 2019-10-10 23:18 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: Linux GPIO, Bartosz Golaszewski, Hans de Goede

On Wed, Oct 9, 2019 at 6:51 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> After changing the drivers to use GPIO core to add an IRQ chip
> it appears that some of them requires a hardware initialization
> before adding the IRQ chip.
>
> Add an optional callback ->init_hw() to allow that drivers
> to initialize hardware if needed.
>
> This change is a part of the fix NULL pointer dereference
> brought to the several drivers recently.
>
> Cc: Hans de Goede <hdegoede@redhat.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Ouch!

But a very nice fix.

I will pull in the series.

Thanks for fixing this up so nicely Andy!

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [GIT PULL] intel-gpio for 5.4-2
  2019-10-09 16:50 [GIT PULL] intel-gpio for 5.4-2 Andy Shevchenko
                   ` (4 preceding siblings ...)
  2019-10-09 16:50 ` [PATCH 5/5] gpio: merrifield: " Andy Shevchenko
@ 2019-10-10 23:22 ` Linus Walleij
  2019-10-11  8:35   ` Andy Shevchenko
  5 siblings, 1 reply; 15+ messages in thread
From: Linus Walleij @ 2019-10-10 23:22 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: Linux GPIO, Bartosz Golaszewski

On Wed, Oct 9, 2019 at 6:51 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> Small set of urgent fixes against Intel GPIO drivers that have been recently
> converted to use GPIO core for instantiation of IRQ chip. No conflicts observed
> with fixes branch of GPIO tree as of today.

Thanks Andy, I pulled this in so hopefully -next works, I will have
to rebase on v5.4-rc2 next week before sending this so will re-sign-off
at that point.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [GIT PULL] intel-gpio for 5.4-2
  2019-10-10 23:22 ` [GIT PULL] intel-gpio for 5.4-2 Linus Walleij
@ 2019-10-11  8:35   ` Andy Shevchenko
  0 siblings, 0 replies; 15+ messages in thread
From: Andy Shevchenko @ 2019-10-11  8:35 UTC (permalink / raw)
  To: Linus Walleij; +Cc: Linux GPIO, Bartosz Golaszewski

On Fri, Oct 11, 2019 at 01:22:06AM +0200, Linus Walleij wrote:
> On Wed, Oct 9, 2019 at 6:51 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> 
> > Small set of urgent fixes against Intel GPIO drivers that have been recently
> > converted to use GPIO core for instantiation of IRQ chip. No conflicts observed
> > with fixes branch of GPIO tree as of today.
> 
> Thanks Andy, I pulled this in so hopefully -next works, I will have
> to rebase on v5.4-rc2 next week before sending this so will re-sign-off
> at that point.

Thank you, Linus!

I guess you meant v5.4-rc3?

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/5] gpio: merrifield: Restore use of irq_base
  2019-10-09 16:50 ` [PATCH 1/5] gpio: merrifield: Restore use of irq_base Andy Shevchenko
@ 2019-10-11 16:21   ` Ferry Toth
  0 siblings, 0 replies; 15+ messages in thread
From: Ferry Toth @ 2019-10-11 16:21 UTC (permalink / raw)
  To: Andy Shevchenko, Linux GPIO; +Cc: Linus Walleij, Bartosz Golaszewski

Op 09-10-19 om 18:50 schreef Andy Shevchenko:
> During conversion to internal IRQ chip initialization the commit
>    8f86a5b4ad67 ("gpio: merrifield: Pass irqchip when adding gpiochip")
> lost the irq_base assignment.
> 
> drivers/gpio/gpio-merrifield.c: In function ‘mrfld_gpio_probe’:
> drivers/gpio/gpio-merrifield.c:405:17: warning: variable ‘irq_base’ set but not used [-Wunused-but-set-variable]
> 
> Assign the girq->first to it.
> 
> Fixes: 8f86a5b4ad67 ("gpio: merrifield: Pass irqchip when adding gpiochip")
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Fixes boot when applied 5.4-rc2

Tested-by: Ferry Toth <fntoth@gmail.com>
> ---
>   drivers/gpio/gpio-merrifield.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpio/gpio-merrifield.c b/drivers/gpio/gpio-merrifield.c
> index 4f27ddfe1e2f..9596024c9161 100644
> --- a/drivers/gpio/gpio-merrifield.c
> +++ b/drivers/gpio/gpio-merrifield.c
> @@ -455,6 +455,7 @@ static int mrfld_gpio_probe(struct pci_dev *pdev, const struct pci_device_id *id
>   	if (!girq->parents)
>   		return -ENOMEM;
>   	girq->parents[0] = pdev->irq;
> +	girq->first = irq_base;
>   	girq->default_type = IRQ_TYPE_NONE;
>   	girq->handler = handle_bad_irq;
>   
> 


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 5/5] gpio: merrifield: Move hardware initialization to callback
  2019-10-09 16:50 ` [PATCH 5/5] gpio: merrifield: " Andy Shevchenko
@ 2019-10-11 16:23   ` Ferry Toth
  0 siblings, 0 replies; 15+ messages in thread
From: Ferry Toth @ 2019-10-11 16:23 UTC (permalink / raw)
  To: Andy Shevchenko, Linux GPIO; +Cc: Linus Walleij, Bartosz Golaszewski

Op 09-10-19 om 18:50 schreef Andy Shevchenko:
> The driver wants to initialize related registers before IRQ chip will be added.
> That's why move it to a corresponding callback. It also fixes the NULL pointer
> dereference.
> 
> Fixes: 8f86a5b4ad67 ("gpio: merrifield: Pass irqchip when adding gpiochip")
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Fixes boot when applied to 5.4-rc2

Tested-by: Ferry Toth <fntoth@gmail.com>
> ---
>   drivers/gpio/gpio-merrifield.c | 8 +++++---
>   1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-merrifield.c b/drivers/gpio/gpio-merrifield.c
> index 9596024c9161..2f1e9da81c1e 100644
> --- a/drivers/gpio/gpio-merrifield.c
> +++ b/drivers/gpio/gpio-merrifield.c
> @@ -362,8 +362,9 @@ static void mrfld_irq_handler(struct irq_desc *desc)
>   	chained_irq_exit(irqchip, desc);
>   }
>   
> -static void mrfld_irq_init_hw(struct mrfld_gpio *priv)
> +static int mrfld_irq_init_hw(struct gpio_chip *chip)
>   {
> +	struct mrfld_gpio *priv = gpiochip_get_data(chip);
>   	void __iomem *reg;
>   	unsigned int base;
>   
> @@ -375,6 +376,8 @@ static void mrfld_irq_init_hw(struct mrfld_gpio *priv)
>   		reg = gpio_reg(&priv->chip, base, GFER);
>   		writel(0, reg);
>   	}
> +
> +	return 0;
>   }
>   
>   static const char *mrfld_gpio_get_pinctrl_dev_name(struct mrfld_gpio *priv)
> @@ -447,6 +450,7 @@ static int mrfld_gpio_probe(struct pci_dev *pdev, const struct pci_device_id *id
>   
>   	girq = &priv->chip.irq;
>   	girq->chip = &mrfld_irqchip;
> +	girq->init_hw = mrfld_irq_init_hw;
>   	girq->parent_handler = mrfld_irq_handler;
>   	girq->num_parents = 1;
>   	girq->parents = devm_kcalloc(&pdev->dev, girq->num_parents,
> @@ -459,8 +463,6 @@ static int mrfld_gpio_probe(struct pci_dev *pdev, const struct pci_device_id *id
>   	girq->default_type = IRQ_TYPE_NONE;
>   	girq->handler = handle_bad_irq;
>   
> -	mrfld_irq_init_hw(priv);
> -
>   	pci_set_drvdata(pdev, priv);
>   	retval = devm_gpiochip_add_data(&pdev->dev, &priv->chip, priv);
>   	if (retval) {
> 


^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2019-10-11 16:39 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-09 16:50 [GIT PULL] intel-gpio for 5.4-2 Andy Shevchenko
2019-10-09 16:50 ` [PATCH 1/5] gpio: merrifield: Restore use of irq_base Andy Shevchenko
2019-10-11 16:21   ` Ferry Toth
2019-10-09 16:50 ` [PATCH 2/5] gpiolib: Initialize the hardware with a callback Andy Shevchenko
2019-10-09 19:44   ` Hans de Goede
2019-10-10  7:23     ` Andy Shevchenko
2019-10-10  7:26       ` Andy Shevchenko
2019-10-10  7:41         ` Hans de Goede
2019-10-10 23:18   ` Linus Walleij
2019-10-09 16:50 ` [PATCH 3/5] gpio: intel-mid: Move hardware initialization to callback Andy Shevchenko
2019-10-09 16:50 ` [PATCH 4/5] gpio: lynxpoint: " Andy Shevchenko
2019-10-09 16:50 ` [PATCH 5/5] gpio: merrifield: " Andy Shevchenko
2019-10-11 16:23   ` Ferry Toth
2019-10-10 23:22 ` [GIT PULL] intel-gpio for 5.4-2 Linus Walleij
2019-10-11  8:35   ` Andy Shevchenko

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.