LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Serge Semin <fancer.lancer@gmail.com>
To: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Serge Semin <Sergey.Semin@baikalelectronics.ru>,
	Lee Jones <lee.jones@linaro.org>,
	linux-kernel@vger.kernel.org, linux-gpio@vger.kernel.org,
	Hoan Tran <hoan@os.amperecomputing.com>,
	Linus Walleij <linus.walleij@linaro.org>,
	Bartosz Golaszewski <bgolaszewski@baylibre.com>
Subject: Re: [PATCH v1 1/4] gpio: dwapb: Unify ACPI enumeration checks in get_irq() and configure_irqs()
Date: Mon, 2 Aug 2021 16:40:21 +0300	[thread overview]
Message-ID: <20210802134021.flrkpmlrcjfxdrdr@mobilestation> (raw)
In-Reply-To: <20210726125436.58685-1-andriy.shevchenko@linux.intel.com>

Hello Andy
Thanks for the cleanup series. A tiny note is below.

On Mon, Jul 26, 2021 at 03:54:33PM +0300, Andy Shevchenko wrote:
> Shared IRQ is only enabled for ACPI enumeration, there is no need
> to have a special flag for that, since we simple can test if device
> has been enumerated by ACPI. This unifies the checks in dwapb_get_irq()
> and dwapb_configure_irqs().
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/gpio/gpio-dwapb.c                | 13 ++++++-------
>  drivers/mfd/intel_quark_i2c_gpio.c       |  1 -
>  include/linux/platform_data/gpio-dwapb.h |  1 -
>  3 files changed, 6 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-dwapb.c b/drivers/gpio/gpio-dwapb.c
> index 3eb13d6d31ef..f6ae69d5d644 100644
> --- a/drivers/gpio/gpio-dwapb.c
> +++ b/drivers/gpio/gpio-dwapb.c
> @@ -436,12 +436,7 @@ static void dwapb_configure_irqs(struct dwapb_gpio *gpio,
>  	pirq->irqchip.irq_set_wake = dwapb_irq_set_wake;
>  #endif
>  

> -	if (!pp->irq_shared) {
> -		girq->num_parents = pirq->nr_irqs;
> -		girq->parents = pirq->irq;
> -		girq->parent_handler_data = gpio;
> -		girq->parent_handler = dwapb_irq_handler;
> -	} else {
> +	if (has_acpi_companion(gpio->dev)) {

Before this patch the platform flag irq_shared has been as kind of a
hint regarding the shared IRQ case being covered here. But now it
doesn't seem obvious why we've got the ACPI and ACPI-less cases
differently handled. What about adding a small comment about that?
E.g. like this: "Intel ACPI-based platforms mostly have the DW APB
GPIO IRQ lane shared between several devices. In that case the
parental IRQ has to be handled in the shared way so to be properly
delivered to all the connected devices." or something more detailed
for your preference. After that the rest of the comments in the
if-clause could be discarded.

Other than that, feel free to add:
>  drivers/gpio/gpio-dwapb.c                | 13 ++++++-------
Acked-by: Serge Semin <fancer.lancer@gmail.com>
Tested-by: Serge Semin <fancer.lancer@gmail.com>

-Sergey

>  		/* This will let us handle the parent IRQ in the driver */
>  		girq->num_parents = 0;
>  		girq->parents = NULL;
> @@ -458,6 +453,11 @@ static void dwapb_configure_irqs(struct dwapb_gpio *gpio,
>  			dev_err(gpio->dev, "error requesting IRQ\n");
>  			goto err_kfree_pirq;
>  		}
> +	} else {
> +		girq->num_parents = pirq->nr_irqs;
> +		girq->parents = pirq->irq;
> +		girq->parent_handler_data = gpio;
> +		girq->parent_handler = dwapb_irq_handler;
>  	}
>  
>  	girq->chip = &pirq->irqchip;
> @@ -581,7 +581,6 @@ static struct dwapb_platform_data *dwapb_gpio_get_pdata(struct device *dev)
>  			pp->ngpio = DWAPB_MAX_GPIOS;
>  		}
>  
> -		pp->irq_shared	= false;
>  		pp->gpio_base	= -1;
>  
>  		/*
> diff --git a/drivers/mfd/intel_quark_i2c_gpio.c b/drivers/mfd/intel_quark_i2c_gpio.c
> index 01935ae4e9e1..a43993e38b6e 100644
> --- a/drivers/mfd/intel_quark_i2c_gpio.c
> +++ b/drivers/mfd/intel_quark_i2c_gpio.c
> @@ -227,7 +227,6 @@ static int intel_quark_gpio_setup(struct pci_dev *pdev)
>  	pdata->properties->ngpio	= INTEL_QUARK_MFD_NGPIO;
>  	pdata->properties->gpio_base	= INTEL_QUARK_MFD_GPIO_BASE;
>  	pdata->properties->irq[0]	= pci_irq_vector(pdev, 0);
> -	pdata->properties->irq_shared	= true;
>  
>  	cell->platform_data = pdata;
>  	cell->pdata_size = sizeof(*pdata);
> diff --git a/include/linux/platform_data/gpio-dwapb.h b/include/linux/platform_data/gpio-dwapb.h
> index 0aa5c6720259..535e5ed549d9 100644
> --- a/include/linux/platform_data/gpio-dwapb.h
> +++ b/include/linux/platform_data/gpio-dwapb.h
> @@ -14,7 +14,6 @@ struct dwapb_port_property {
>  	unsigned int	ngpio;
>  	unsigned int	gpio_base;
>  	int		irq[DWAPB_MAX_GPIOS];
> -	bool		irq_shared;
>  };
>  
>  struct dwapb_platform_data {
> -- 
> 2.30.2
> 

  parent reply	other threads:[~2021-08-02 13:40 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-26 12:54 Andy Shevchenko
2021-07-26 12:54 ` [PATCH v1 2/4] gpio: dwapb: Read GPIO base from gpio-base property Andy Shevchenko
2021-08-02 13:58   ` Serge Semin
2021-08-02 15:52     ` Andy Shevchenko
2021-08-04 12:44       ` Serge Semin
2021-08-04 14:43         ` Andy Shevchenko
2021-08-11 12:40           ` Linus Walleij
2021-08-11 12:47             ` Serge Semin
2021-08-11 12:37   ` Linus Walleij
2021-08-11 13:11     ` Andy Shevchenko
2021-07-26 12:54 ` [PATCH v1 3/4] mfd: intel_quark_i2c_gpio: Convert GPIO to use software nodes Andy Shevchenko
2021-08-11  8:38   ` Linus Walleij
2021-08-11 10:55     ` Andy Shevchenko
2021-08-16 13:05   ` Lee Jones
2021-08-16 13:18     ` Andy Shevchenko
2021-08-16 13:33       ` Lee Jones
2021-08-16 14:00         ` Andy Shevchenko
2021-08-16 14:19           ` Lee Jones
2021-08-16 14:53             ` Andy Shevchenko
2021-08-17  7:26               ` Lee Jones
2021-08-17 11:23                 ` Andy Shevchenko
2021-08-18  6:34                   ` Lee Jones
2021-08-18 11:04                     ` Andy Shevchenko
2021-09-21 15:25                   ` Lee Jones
2021-07-26 12:54 ` [PATCH v1 4/4] gpio: dwapb: Get rid of legacy platform data Andy Shevchenko
2021-08-02 14:07   ` Serge Semin
2021-08-02 15:54     ` Andy Shevchenko
2021-08-02  8:48 ` [PATCH v1 1/4] gpio: dwapb: Unify ACPI enumeration checks in get_irq() and configure_irqs() Lee Jones
2021-08-02 13:40 ` Serge Semin [this message]
2021-08-02 18:37   ` Andy Shevchenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210802134021.flrkpmlrcjfxdrdr@mobilestation \
    --to=fancer.lancer@gmail.com \
    --cc=Sergey.Semin@baikalelectronics.ru \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=bgolaszewski@baylibre.com \
    --cc=hoan@os.amperecomputing.com \
    --cc=lee.jones@linaro.org \
    --cc=linus.walleij@linaro.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --subject='Re: [PATCH v1 1/4] gpio: dwapb: Unify ACPI enumeration checks in get_irq() and configure_irqs()' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).