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

2014-07-01 Thread Roger Quadros
+Mark, Alexander, Ivan

Hi Tony,

On 06/13/2014 03:08 PM, Tony Lindgren wrote:
 * Roger Quadros rog...@ti.com [140613 04:43]:
 On 06/13/2014 01:46 PM, Tony Lindgren wrote:
 * Roger Quadros rog...@ti.com [140613 01:24]:
 On 06/13/2014 11:13 AM, Gupta, Pekon wrote:
 From: Tony Lindgren [mailto:t...@atomide.com]
 * Roger Quadros rog...@ti.com [140613 00:40]:
 On 06/13/2014 10:18 AM, Tony Lindgren wrote:
 * Roger Quadros rog...@ti.com [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.

 But NOR driver never requests for any IRQ.

 We should not confuse this wait pin edge interrupt with NOR bus cycle WAIT 
 pin mechanism.
 That is configured using GPMC_CONFIG1 register via WAITPINSELECT and 
 WAITREAD/WRITEMONITORING bits.
 That wait pin handling is done completely in hardware and doesn't need any 
 software intervention.
 Imagine using it for interrupt for every bus cycle wait. It will be dead 
 slow and unusable.

 The WAIT edge interrupt mechanism is exclusively for NAND use case to notify 
 the status of READY pin after a block/page operation.
 
 OK thanks for clarifying it. 
  
 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 :)

 the client driver side is easy, but it adds unnecessary complication to 
 model it as IRQ chip and assign a line for each event. Since it is going to 
 be used exclusively by NAND we should avoid IRQ chip modeling.
 
 OK up to you if there are no other users for it. 
  
 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.

 OK. I agree about using some kind of abstraction instead of direct access.
 
 Yes and like we chatted on irc, adding a syscon mapping for for
 the NAND specific registers might do the trick here.

After looking at the syscon driver, which relies on regmap, it seems that 
regmap was designed for slow control busses like I2C/SPI and using it for NAND 
controller register access will have a significant negative impact on 
performance. In the NAND case the register writes are used for each NAND 
command cycle and the reads for ECC checks (every page).

See how much code regmap_read and regmap_mmio_read() translates to for a simple 
register read i.e. readl().
http://lxr.free-electrons.com/source/drivers/base/regmap/regmap.c#L1944
http://lxr.free-electrons.com/source/drivers/base/regmap/regmap-mmio.c#L129

So I'm not so sure of using regmap/syscon for NAND controller register access.

Could there be any other abstraction method of sharing the register space 
between GPMC and NAND driver?
I've also added Ivan to the thread, the author of memory/ti-aemif.c driver, to 
check if he faced any issues with shared register access of the AEMIF/NAND 
registers.

cheers,
-roger

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2014-07-01 Thread Tony Lindgren
* Roger Quadros rog...@ti.com [140701 03:13]:
 On 06/13/2014 03:08 PM, Tony Lindgren wrote:
  * Roger Quadros rog...@ti.com [140613 04:43]:
 
  OK. I agree about using some kind of abstraction instead of direct access.
  
  Yes and like we chatted on irc, adding a syscon mapping for for
  the NAND specific registers might do the trick here.
 
 After looking at the syscon driver, which relies on regmap, it seems that 
 regmap was designed for slow control busses like I2C/SPI and using it for 
 NAND controller register access will have a significant negative impact on 
 performance. In the NAND case the register writes are used for each NAND 
 command cycle and the reads for ECC checks (every page).
 
 See how much code regmap_read and regmap_mmio_read() translates to for a 
 simple register read i.e. readl().
 http://lxr.free-electrons.com/source/drivers/base/regmap/regmap.c#L1944
 http://lxr.free-electrons.com/source/drivers/base/regmap/regmap-mmio.c#L129
 
 So I'm not so sure of using regmap/syscon for NAND controller register access.

OK yes I agree, it's not a good solution for a constant
register access that's needed for the ECC registers.
 
 Could there be any other abstraction method of sharing the register space 
 between GPMC and NAND driver?
 I've also added Ivan to the thread, the author of memory/ti-aemif.c driver, 
 to check if he faced any issues with shared register access of the AEMIF/NAND 
 registers.

If there's no common framework available for GPMC to implement,
it's best to just export few functions from gpmc.c for the ECC
calculations.

Regards,

Tony
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2014-06-13 Thread Tony Lindgren
* Roger Quadros rog...@ti.com [140611 01:58]:
 Since the Interrupt Events are used only by the NAND driver,
 there is no point in managing the Interrupt registers
 in the GPMC driver and complicating it with irqchip modeling.
 
 Let's manage the interrupt registers directly in the NAND driver
 and get rid of irqchip model from GPMC driver.
 
 Get rid of IRQ commands and unused commands from gpmc_configure() in
 the GPMC driver.

This seems like a step backward to me. The GPMC interrupt enable
register can do edge detection on the wait pins, how is that
limited to NAND?

Further, let's not start mixing GPMC hardware module register
access with the NAND driver register access. They can be clocked
separately. And bugs in the NAND driver can cause issues in other
GPMC using drivers.

Regards,

Tony
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2014-06-13 Thread Roger Quadros
On 06/13/2014 10:18 AM, Tony Lindgren wrote:
 * Roger Quadros rog...@ti.com [140611 01:58]:
 Since the Interrupt Events are used only by the NAND driver,
 there is no point in managing the Interrupt registers
 in the GPMC driver and complicating it with irqchip modeling.

 Let's manage the interrupt registers directly in the NAND driver
 and get rid of irqchip model from GPMC driver.

 Get rid of IRQ commands and unused commands from gpmc_configure() in
 the GPMC driver.
 
 This seems like a step backward to me. The GPMC interrupt enable
 register can do edge detection on the wait pins, how is that
 limited to NAND?

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?

 
 Further, let's not start mixing GPMC hardware module register
 access with the NAND driver register access. They can be clocked
 separately. And bugs in the NAND driver can cause issues in other
 GPMC using drivers.

I understood that NAND controller is integrated into the GPMC module and they 
are clocked
the same. Not sure why the hardware designers would keep the registers so 
closely knit.

FYI. memory/ti-amif.c and mtd/nand/davinci_nand.c share the AMIF register space 
in the
same way. I thought it'd be nice to be consistent across TI drivers.

cheers,
-roger
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2014-06-13 Thread Tony Lindgren
* Roger Quadros rog...@ti.com [140613 00:40]:
 On 06/13/2014 10:18 AM, Tony Lindgren wrote:
  * Roger Quadros rog...@ti.com [140611 01:58]:
  Since the Interrupt Events are used only by the NAND driver,
  there is no point in managing the Interrupt registers
  in the GPMC driver and complicating it with irqchip modeling.
 
  Let's manage the interrupt registers directly in the NAND driver
  and get rid of irqchip model from GPMC driver.
 
  Get rid of IRQ commands and unused commands from gpmc_configure() in
  the GPMC driver.
  
  This seems like a step backward to me. The GPMC interrupt enable
  register can do edge detection on the wait pins, how is that
  limited to NAND?
 
 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? 

  Further, let's not start mixing GPMC hardware module register
  access with the NAND driver register access. They can be clocked
  separately. And bugs in the NAND driver can cause issues in other
  GPMC using drivers.
 
 I understood that NAND controller is integrated into the GPMC module and they 
 are clocked
 the same. Not sure why the hardware designers would keep the registers so 
 closely knit.

Yeah. Maybe regmap could provide some abstraction to the the
NAND registers.
 
 FYI. memory/ti-amif.c and mtd/nand/davinci_nand.c share the AMIF register 
 space in the
 same way. I thought it'd be nice to be consistent across TI drivers.

Probably they did not yet learn the problems caused by it :)

Regards,

Tony
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2014-06-13 Thread Gupta, Pekon

From: Tony Lindgren [mailto:t...@atomide.com]
* Roger Quadros rog...@ti.com [140613 00:40]:
 On 06/13/2014 10:18 AM, Tony Lindgren wrote:
  * Roger Quadros rog...@ti.com [140611 01:58]:
  Since the Interrupt Events are used only by the NAND driver,
  there is no point in managing the Interrupt registers
  in the GPMC driver and complicating it with irqchip modeling.
 
  Let's manage the interrupt registers directly in the NAND driver
  and get rid of irqchip model from GPMC driver.
 
  Get rid of IRQ commands and unused commands from gpmc_configure() in
  the GPMC driver.
 
  This seems like a step backward to me. The GPMC interrupt enable
  register can do edge detection on the wait pins, how is that
  limited to NAND?

 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.




  Further, let's not start mixing GPMC hardware module register
  access with the NAND driver register access. They can be clocked
  separately. And bugs in the NAND driver can cause issues in other
  GPMC using drivers.

 I understood that NAND controller is integrated into the GPMC module and 
 they are clocked
 the same. Not sure why the hardware designers would keep the registers so 
 closely knit.

Yeah. Maybe regmap could provide some abstraction to the the
NAND registers.

As you mentioned, GPMC has two set of registers:
(a) Chip-select registers (CONFIGx_cs) for device specific parameters
 (like device-width, signal-timings, etc) which are statically programmed
during probe or via DT.
(b) ECC registers which are continuously reconfigured based on
 ECC engine.

*Ideal Scenario*
NAND driver should be considered equivalent to protocol driver,
Therefore ideally it should use only those registers which are
specific to NAND (b).

*Actual Scenario*
But most NAND device today are ONFI compliant and they have
almost all device parameters like device-width, signal-timings
burned on-die in an ONFI page. These values are read back from
NAND device during device_probe() and then re-configured back
Chip-select registers (a).
Hence NAND driver needs access of both (a) and (b), which is why
You need to export complete GPMC register set to NAND driver.
However this is not the case and has been discussed earlier too..

http://lists.infradead.org/pipermail/linux-mtd/2013-October/049284.html
http://lists.infradead.org/pipermail/linux-mtd/2013-October/049347.html
(Just pointing out my version of history, would be good to read the
entire discussion. But the summary was that we need to re-configure
some GPMC chip-select registers (a) based on probe done in
NAND driver. So we need all GPMC registers exposed to NAND driver).




 FYI. memory/ti-amif.c and mtd/nand/davinci_nand.c share the AMIF register 
 space in the
 same way. I thought it'd be nice to be consistent across TI drivers.

Probably they did not yet learn the problems caused by it :)

I havn't reviewed the ti-amif.c driver completely but I think they too
configure device signal timing statically based on DT. But as per
today this is frowned upon because:

(1) Its difficult for layman user to decipher NAND signal timings
from datasheet and then convert it into controller understandable DT

(2) ONFI parameter page on NAND has these timings specified
on-die itself, and these timings are characterized for best performance
so NAND driver should re-configure these timings after probe.
Refer below mail from 'Rob Herring robherri...@gmail.com'
http://lists.infradead.org/pipermail/linux-mtd/2014-April/053488.html


Considering all these details, please re-review the changes you plan
for GPMC driver.

with regards, pekon
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2014-06-13 Thread Roger Quadros
On 06/13/2014 11:13 AM, Gupta, Pekon wrote:
 
 From: Tony Lindgren [mailto:t...@atomide.com]
 * Roger Quadros rog...@ti.com [140613 00:40]:
 On 06/13/2014 10:18 AM, Tony Lindgren wrote:
 * Roger Quadros rog...@ti.com [140611 01:58]:
 Since the Interrupt Events are used only by the NAND driver,
 there is no point in managing the Interrupt registers
 in the GPMC driver and complicating it with irqchip modeling.

 Let's manage the interrupt registers directly in the NAND driver
 and get rid of irqchip model from GPMC driver.

 Get rid of IRQ commands and unused commands from gpmc_configure() in
 the GPMC driver.

 This seems like a step backward to me. The GPMC interrupt enable
 register can do edge detection on the wait pins, how is that
 limited to NAND?

 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

For memory devices, no software wait pin intervention is necessary and doesn't 
even make sense.

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.

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.

Only the GPMC_CONFIG register needs to remain with the GPMC driver.

cheers,
-roger

 
 
 
 Further, let's not start mixing GPMC hardware module register
 access with the NAND driver register access. They can be clocked
 separately. And bugs in the NAND driver can cause issues in other
 GPMC using drivers.

 I understood that NAND controller is integrated into the GPMC module and 
 they are clocked
 the same. Not sure why the hardware designers would keep the registers so 
 closely knit.

 Yeah. Maybe regmap could provide some abstraction to the the
 NAND registers.

 As you mentioned, GPMC has two set of registers:
 (a) Chip-select registers (CONFIGx_cs) for device specific parameters
  (like device-width, signal-timings, etc) which are statically programmed
 during probe or via DT.
 (b) ECC registers which are continuously reconfigured based on
  ECC engine.
 
 *Ideal Scenario*
 NAND driver should be considered equivalent to protocol driver,
 Therefore ideally it should use only those registers which are
 specific to NAND (b).
 
 *Actual Scenario*
 But most NAND device today are ONFI compliant and they have
 almost all device parameters like device-width, signal-timings
 burned on-die in an ONFI page. These values are read back from
 NAND device during device_probe() and then re-configured back
 Chip-select registers (a).
 Hence NAND driver needs access of both (a) and (b), which is why
 You need to export complete GPMC register set to NAND driver.
 However this is not the case and has been discussed earlier too..
 
 http://lists.infradead.org/pipermail/linux-mtd/2013-October/049284.html
 http://lists.infradead.org/pipermail/linux-mtd/2013-October/049347.html
 (Just pointing out my version of history, would be good to read the
 entire discussion. But the summary was that we need to re-configure
 some GPMC chip-select registers (a) based on probe done in
 NAND driver. So we need all GPMC registers exposed to NAND driver).
 
 
 
 
 FYI. memory/ti-amif.c and mtd/nand/davinci_nand.c share the AMIF register 
 space in the
 same way. I thought it'd be nice to be consistent across TI drivers.

 Probably they did not yet learn the problems caused by it :)

 I havn't reviewed the ti-amif.c driver completely but I think they too
 configure device signal timing statically based on DT. But as per
 today this is frowned upon because:
 
 (1) Its difficult for layman user to decipher NAND signal timings
 from datasheet and then convert it into controller understandable DT
 
 (2) ONFI parameter page on NAND has these timings specified
 on-die itself, and these timings are characterized for best performance
 so NAND driver should re-configure these timings after probe.
 Refer below mail from 'Rob Herring robherri...@gmail.com'
 http://lists.infradead.org/pipermail/linux-mtd/2014-April/053488.html
 
 
 Considering all these details, please re-review the changes you plan
 for GPMC driver.
 
 with regards, pekon
 

--
To unsubscribe from this list: send the line unsubscribe 

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

2014-06-13 Thread Tony Lindgren
* Roger Quadros rog...@ti.com [140613 01:24]:
 On 06/13/2014 11:13 AM, Gupta, Pekon wrote:
  From: Tony Lindgren [mailto:t...@atomide.com]
  * Roger Quadros rog...@ti.com [140613 00:40]:
  On 06/13/2014 10:18 AM, Tony Lindgren wrote:
  * Roger Quadros rog...@ti.com [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-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2014-06-13 Thread Roger Quadros
On 06/13/2014 01:46 PM, Tony Lindgren wrote:
 * Roger Quadros rog...@ti.com [140613 01:24]:
 On 06/13/2014 11:13 AM, Gupta, Pekon wrote:
 From: Tony Lindgren [mailto:t...@atomide.com]
 * Roger Quadros rog...@ti.com [140613 00:40]:
 On 06/13/2014 10:18 AM, Tony Lindgren wrote:
 * Roger Quadros rog...@ti.com [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.

But NOR driver never requests for any IRQ.

We should not confuse this wait pin edge interrupt with NOR bus cycle WAIT pin 
mechanism.
That is configured using GPMC_CONFIG1 register via WAITPINSELECT and 
WAITREAD/WRITEMONITORING bits.
That wait pin handling is done completely in hardware and doesn't need any 
software intervention.
Imagine using it for interrupt for every bus cycle wait. It will be dead slow 
and unusable.

The WAIT edge interrupt mechanism is exclusively for NAND use case to notify 
the status of READY pin after a block/page operation.

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

the client driver side is easy, but it adds unnecessary complication to model 
it as IRQ chip and assign a line for each event. Since it is going to be used 
exclusively by NAND we should avoid IRQ chip modeling.

  
 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.

OK. I agree about using some kind of abstraction instead of direct access.

cheers,
-roger
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2014-06-13 Thread Tony Lindgren
* Roger Quadros rog...@ti.com [140613 04:43]:
 On 06/13/2014 01:46 PM, Tony Lindgren wrote:
  * Roger Quadros rog...@ti.com [140613 01:24]:
  On 06/13/2014 11:13 AM, Gupta, Pekon wrote:
  From: Tony Lindgren [mailto:t...@atomide.com]
  * Roger Quadros rog...@ti.com [140613 00:40]:
  On 06/13/2014 10:18 AM, Tony Lindgren wrote:
  * Roger Quadros rog...@ti.com [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.
 
 But NOR driver never requests for any IRQ.
 
 We should not confuse this wait pin edge interrupt with NOR bus cycle WAIT 
 pin mechanism.
 That is configured using GPMC_CONFIG1 register via WAITPINSELECT and 
 WAITREAD/WRITEMONITORING bits.
 That wait pin handling is done completely in hardware and doesn't need any 
 software intervention.
 Imagine using it for interrupt for every bus cycle wait. It will be dead slow 
 and unusable.
 
 The WAIT edge interrupt mechanism is exclusively for NAND use case to notify 
 the status of READY pin after a block/page operation.

OK thanks for clarifying it. 
 
  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 :)
 
 the client driver side is easy, but it adds unnecessary complication to model 
 it as IRQ chip and assign a line for each event. Since it is going to be used 
 exclusively by NAND we should avoid IRQ chip modeling.

OK up to you if there are no other users for it. 
 
  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.
 
 OK. I agree about using some kind of abstraction instead of direct access.

Yes and like we chatted on irc, adding a syscon mapping for for
the NAND specific registers might do the trick here.

Regards,

Tony
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html