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

[Tony Lindgren]

* Roger Quadros <rogerq@xxxxxx> [140613 01:24]:
> On 06/13/2014 11:13 AM, Gupta, Pekon wrote:
> >> From: Tony Lindgren [mailto:tony@xxxxxxxxxxx]
> >>> * Roger Quadros <rogerq@xxxxxx> [140613 00:40]:
> >>>> On 06/13/2014 10:18 AM, Tony Lindgren wrote:
> >>>>> * Roger Quadros <rogerq@xxxxxx> [140611 01:58]:
> >>>
> >>> OK. But wait pin edge detection was not yet being used and I couldn't
> >>> think of how it would ever be used. Any ideas?
> >>
> >> Maybe to wake-up the system on bus activity or something?
> >>
> > Sorry, I wasn't able to review this series.
> > But just as pointer, GPMC driver was used for interfacing many
> > non-memory devices like Ethernet (smc91x) and in past GPMC has been
> > proved to work with camera devices too, but that's wasn't mainlined.
> > So keeping IRQ and few other things in GPMC driver is helpful.
> > 
> 
> On further study it seems that the wait pin edge detection is only used in the NAND controller use case.
> see section 10.1.5.14.2.2 Ready Pin Monitored by Hardware Interrupt

It seems they can be used for anything slow like NOR and NAND.
 
> For memory devices, no software wait pin intervention is necessary and doesn't even make sense.

Still seems that it's use can be generic though, not limited
to NAND.
 
> So I don't agree on managing the IRQSTATUS and IRQENABLE register in the GPMC driver. It is adding unnecessary complexity. I don't mind having a wrapper around it though like the other nand registers.

But all the consumer driver should need to do is request_irq()
on it? That's pretty much the most common interface we have
for drivers :)
 
> To be frank, I think it is cleaner if the NAND driver directly accesses the NAND registers.
> I don't see why we should make things complicated just because the hardware designers didn't create a clear register split between GPMC and NAND.

Because they are in separate hardware modules :)

Who knows why it was set up this way. Maybe the plan was to have
the common features in GPMC that then can be used by various MTD
devices.
 
> Only the GPMC_CONFIG register needs to remain with the GPMC driver.

And managing clocks and runtime PM in general. In any case, let's
not let drivers tinker with the GPMC registers directly though.
Some kind of abstraction via existing frameworks or with regmap
is needed.

Regards,

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