Re: [PATCH 03/12] mtd: nand: omap: Move IRQ handling from GPMC to NAND driver

[Tony Lindgren]

* Roger Quadros <rogerq@xxxxxx> [150731 03:24]:
> 
> One more observation I've had is that using irqchip modelling for
> the 2 NAND events causes a performance impact.
> 
> Using mtd_oobtest I see the following on dra7-evm
> 
> 1) v4.2-rc4 with prefetch-polled (no IRQs used)
> mtd_speedtest: eraseblock write speed is 7142 KiB/s
> mtd_speedtest: eraseblock read speed is 13721 KiB/s
> 
> 2) v4.2-rc4 with prefetch-irq (IRQchip model)
> eraseblock write speed is 5475 KiB/s
> eraseblock read speed is 6420 KiB/s
> 
> 3) this series (*) with prefetch-irq (no IRQchip model, nand driver
> directly accesses irqstatus/irqenable)
> eraseblock write speed is 6564 KiB/s
> eraseblock read speed is 10850 KiB/s
> 
> (*) diff at the end is required on top to fix an issue with this series.
> 
> So should we continue IRQchip modelling for the NAND events
> or use the GPMC interrupt as shared and add APIs to access
> the NAND bits of the IRQSTATUS/ENABLE register.

In the long run chained IRQ would be the most flexible solution
for sure.

It might be worth checking why we have so much overhead with
the irqchip modelling compared to shared IRQ. If the overhead
is really all the extra IRQ handling then it seems the shared
interrupt plus an API to access IRQSTATUS/ENABLE is the way
to go. Up to you.

Regards,

Tony

 
> diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
> index fecc054..26ef2bd 100644
> --- a/drivers/mtd/nand/omap2.c
> +++ b/drivers/mtd/nand/omap2.c
> @@ -1832,13 +1832,14 @@ static int omap_get_dt_info(struct device *dev, struct omap_nand_info *info)
>  		for (i = 0; i < ARRAY_SIZE(nand_xfer_types); i++) {
>  			if (!strcasecmp(s, nand_xfer_types[i])) {
>  				info->xfer_type = i;
> -				break;
> +				goto next;
>  			}
>  		}
>  
>  		dev_err(dev, "unrecognized value for ti,nand-xfer-type\n");
>  		return -EINVAL;
>  	}
> +next:
>  
>  	of_get_nand_on_flash_bbt(child);
>  
> diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
> index 26ef2bd..e8bdff5 100644
> --- a/drivers/mtd/nand/omap2.c
> +++ b/drivers/mtd/nand/omap2.c
> @@ -670,17 +670,12 @@ static irqreturn_t omap_nand_irq(int this_irq, void *dev)
>  			goto done;
>  	}
>  
> -	/* Clear FIFOEVENT STATUS */
> -	irqstatus &= ~GPMC_IRQ_FIFOEVENT;
> -	writel(irqstatus, info->reg.gpmc_irqstatus);
> -
>  	return IRQ_HANDLED;
>  
>  done:
>  	complete(&info->comp);
>  
> -	/* Clear FIFOEVENT and TERMCOUNT STATUS */
> -	irqstatus &= ~(GPMC_IRQ_TERMCOUNT | GPMC_IRQ_FIFOEVENT);
> +	/* Clear IRQ STATUS */
>  	writel(irqstatus, info->reg.gpmc_irqstatus);
>  
>  	/* Disable Interrupt generation */
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/