Re: [PATCH] gpio: pl061: Support implementations without GPIOINTR line

[Alexander Sverdlin <alexander.sverdlin@xxxxxxxxx>]

Hello all!

Please ignore the below patch, the implementation is incomplete!

On 17/03/2021 16:59, Alexander A Sverdlin wrote:
> From: Alexander Sverdlin <alexander.sverdlin@xxxxxxxxx>
> 
> There are several implementations of PL061 which lack GPIOINTR signal in
> hardware and only have individual GPIOMIS[7:0] interrupts. Use the
> hierarchical interrupt support of the gpiolib in these cases (if at least 8
> IRQs are configured for the PL061).
> 
> One in-tree example is arch/arm/boot/dts/axm55xx.dtsi, PL061 instances have
> 8 IRQs defined, but current driver supports only the first one, so only one
> pin would work as IRQ trigger.
> 
> Link: https://lore.kernel.org/linux-gpio/CACRpkdZpYzpMDWqJobSYH=JHgB74HbCQihOtexs+sVyo6SRJdA@xxxxxxxxxxxxxx/
> Signed-off-by: Alexander Sverdlin <alexander.sverdlin@xxxxxxxxx>
> ---
>  drivers/gpio/Kconfig      |  1 +
>  drivers/gpio/gpio-pl061.c | 46 +++++++++++++++++++++++++++++++++++++++-------
>  2 files changed, 40 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index b8013cf..6601758 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -426,6 +426,7 @@ config GPIO_PL061
>  	depends on ARM_AMBA
>  	select IRQ_DOMAIN
>  	select GPIOLIB_IRQCHIP
> +	select IRQ_DOMAIN_HIERARCHY
>  	help
>  	  Say yes here to support the PrimeCell PL061 GPIO device
>  
> diff --git a/drivers/gpio/gpio-pl061.c b/drivers/gpio/gpio-pl061.c
> index 3439120..3c70386 100644
> --- a/drivers/gpio/gpio-pl061.c
> +++ b/drivers/gpio/gpio-pl061.c
> @@ -282,6 +282,23 @@ static int pl061_irq_set_wake(struct irq_data *d, unsigned int state)
>  	return irq_set_irq_wake(pl061->parent_irq, state);
>  }
>  
> +static int pl061_child_to_parent_hwirq(struct gpio_chip *gc, unsigned int child,
> +				       unsigned int child_type,
> +				       unsigned int *parent,
> +				       unsigned int *parent_type)
> +{
> +	struct amba_device *adev = to_amba_device(gc->parent);
> +	unsigned int irq = adev->irq[child];
> +	struct irq_data *d = irq_get_irq_data(irq);
> +
> +	if (!d)
> +		return -EINVAL;
> +
> +	*parent_type = irqd_get_trigger_type(d);
> +	*parent = irqd_to_hwirq(d);
> +	return 0;
> +}
> +
>  static int pl061_probe(struct amba_device *adev, const struct amba_id *id)
>  {
>  	struct device *dev = &adev->dev;
> @@ -332,16 +349,31 @@ static int pl061_probe(struct amba_device *adev, const struct amba_id *id)
>  
>  	girq = &pl061->gc.irq;
>  	girq->chip = &pl061->irq_chip;
> -	girq->parent_handler = pl061_irq_handler;
> -	girq->num_parents = 1;
> -	girq->parents = devm_kcalloc(dev, 1, sizeof(*girq->parents),
> -				     GFP_KERNEL);
> -	if (!girq->parents)
> -		return -ENOMEM;
> -	girq->parents[0] = irq;
>  	girq->default_type = IRQ_TYPE_NONE;
>  	girq->handler = handle_bad_irq;
>  
> +	/*
> +	 * There are some PL061 implementations which lack GPIOINTR in hardware
> +	 * and only have individual GPIOMIS[7:0] signals. We distinguish them by
> +	 * the number of IRQs assigned to the AMBA device.
> +	 */
> +	if (!adev->irq[PL061_GPIO_NR - 1]) {
> +		WARN_ON(adev->irq[1]);
> +
> +		girq->parent_handler = pl061_irq_handler;
> +		girq->num_parents = 1;
> +		girq->parents = devm_kcalloc(dev, 1, sizeof(*girq->parents),
> +					     GFP_KERNEL);
> +		if (!girq->parents)
> +			return -ENOMEM;
> +		girq->parents[0] = irq;
> +	} else {
> +		girq->fwnode = dev->fwnode;
> +		girq->parent_domain =
> +			irq_get_irq_data(adev->irq[PL061_GPIO_NR - 1])->domain;
> +		girq->child_to_parent_hwirq = pl061_child_to_parent_hwirq;
> +	}
> +
>  	ret = devm_gpiochip_add_data(dev, &pl061->gc, pl061);
>  	if (ret)
>  		return ret;
> 

-- 
Best regards,
Alexander Sverdlin.