RE: [PATCH] gpiolib:change to use irq_alloc_descs_from to alloc virqs

["Wang, Yalin" <Yalin.Wang@xxxxxxxxxxxxxx>]

Hi Grygorii,

Yeah, you are right,
I really make a mistake .

I think both our patch are ok to fix this problem,
But we use different solutions.

My patch have a advantage than yours that
It alloc virqs in one time and when free it,
Doesn't need call irqa_find_mapping() one by one, performance
Is better .

For my patch, you said " As for me, you patch will completely disable Sparse IRQ feature :("

Why? Could you make me clear ? :)

Then we can decide which solution to use to fix this issue. 

Thanks

-----Original Message-----
> hi
> 
> but it is not safe with a little problem, when remove , should not 
> assume physical irq start from 0, i change like this, store 
> gpiochip->irq_base = first_irq;
> 
> ---
> 
> 
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 
> 15cc0bb..c5fb2c1 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -517,14 +517,14 @@ static int gpiochip_to_irq(struct gpio_chip *chip, unsigned offset)
>    */
>   static void gpiochip_irqchip_remove(struct gpio_chip *gpiochip)
>   {
> -	unsigned int offset;
> -
> +	int i;
>   	acpi_gpiochip_free_interrupts(gpiochip);
>   
>   	/* Remove all IRQ mappings and delete the domain */
>   	if (gpiochip->irqdomain) {
> -		for (offset = 0; offset < gpiochip->ngpio; offset++)
> -			irq_dispose_mapping(gpiochip->irq_base + offset);
> +		for (i = 0; i < gpiochip->ngpio; i++)
> +			irq_dispose_mapping(irq_find_mapping(gpiochip->irqdomain,
> +						gpiochip->irq_base + i));

No:) it should start from 0
irq_dispose_mapping(irq_find_mapping(gpiochip->irqdomain, i));

irq_domain_add_simple() uses hwirq_base == 0 and current implementation of gpiolib irqchip helpers doesn't support setting of custom hwirq_base.
  

>   		irq_domain_remove(gpiochip->irqdomain);
>   	}
>   
> @@ -596,6 +596,7 @@ int gpiochip_irqchip_add(struct gpio_chip *gpiochip,
>   		gpiochip->irqchip = NULL;
>   		return -EINVAL;
>   	}
> +	gpiochip->irq_base = first_irq;
>   	irqchip->irq_request_resources = gpiochip_irq_reqres;
>   	irqchip->irq_release_resources = gpiochip_irq_relres;
>   
> @@ -604,14 +605,10 @@ int gpiochip_irqchip_add(struct gpio_chip *gpiochip,
>   	 * any gpiochip calls. If the first_irq was zero, this is
>   	 * necessary to allocate descriptors for all IRQs.
>   	 */
> -	for (offset = 0; offset < gpiochip->ngpio; offset++) {
> -		irq_base = irq_create_mapping(gpiochip->irqdomain, offset);
> -		if (offset == 0)
> -			/*
> -			 * Store the base into the gpiochip to be used when
> -			 * unmapping the irqs.
> -			 */
> -			gpiochip->irq_base = irq_base;
> +	if (first_irq == 0) {
> +		for (offset = 0; offset < gpiochip->ngpio; offset++)
> +			irq_base = irq_create_mapping(gpiochip->irqdomain, offset);
> +
>   	}
>   
>   	acpi_gpiochip_request_interrupts(gpiochip);
> 
> 

--
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