[PATCH v4 0/7] Add TI EMIF SDRAM controller driver

[PATCH v4 0/7] Add TI EMIF SDRAM controller driver

[Shilimkar, Santosh]

2012/3/19 ??? <myungjoo.ham@samsung.com>:
>
> Kyungmin Park<kmpark@infradead.org> 2012-03-17 15:10 (GMT+09:00)
>> Hi,
>> On 3/17/12, Aneesh V wrote:
>> > Add a driver for the EMIF SDRAM controller used in TI SoCs
>> >
>> > EMIF is an SDRAM controller that supports, based on its revision,
>> > one or more of LPDDR2/DDR2/DDR3 protocols.This driver adds support
>> > for LPDDR2.
>> >
>> > The driver supports the following features:
>> > - Calculates the DDR AC timing parameters to be set in EMIF
>> >   registers using data from the device data-sheets and based
>> >   on the DDR frequency. If data from data-sheets is not available
>> >   default timing values from the JEDEC spec are used. These
>> >   will be safe, but not necessarily optimal
>> > - API for changing timings during DVFS or at boot-up
>
> This means that you alreeady have callbacks to create a devfreq device driver that supports DVFS on the device. This doesn't need to be a misc device driver then.
>
Nope. The callbacks mentioned above are mainly coming from the clock
framework. Ofcourse the clock node trigger
with voltage ramps would be triggered by some upper laver
governor/framework like CPUFreq/DEVFREQ but as
such this is not necessary. This is an indepdent driver and just like
CPUFREQ notifies it's users for the frequency
change, this driver will be notified to take any action on pre and
post notification. Note that, the driver will
be notified by regulator framework for the voltage ramp up/down cases
too with notifiers.

>> > - Temperature alert configuration and handling of temperature
>> >   alerts, if any for LPDDR2 devices
>> >   * temperature alert is based on periodic polling of MR4 mode
>> >     register in DDR devices automatically performed by hardware
>> >   * timings are de-rated and brought back to nominal when
>> >     temperature raises and falls respectively
>
> This can be a feature overriding "max_freq" inside the Omap EMIF devfreq device driver though it maybe (or not.. I just don't sure) be better to use thermal framework as well.
>
That's absolute abuse and there is no need to link the temperature
handling with DVFS. Reducing frequency can be one
of the cooling techniques but that's not related to the temperature
alert handling. Temperature handling in the EMIF
driver is strictly as per the JDEC specs and that is to de-rate the
timings when temperature threshold is crossed.

The idea is drivers should be independent of all the global policy
governors. Hope this is clear to
you.

Regards
Santosh

[PATCH v4 0/7] Add TI EMIF SDRAM controller driver

[Aneesh V]

Add a driver for the EMIF SDRAM controller used in TI SoCs

EMIF is an SDRAM controller that supports, based on its revision,
one or more of LPDDR2/DDR2/DDR3 protocols.This driver adds support
for LPDDR2.

The driver supports the following features:
- Calculates the DDR AC timing parameters to be set in EMIF
  registers using data from the device data-sheets and based
  on the DDR frequency. If data from data-sheets is not available
  default timing values from the JEDEC spec are used. These
  will be safe, but not necessarily optimal
- API for changing timings during DVFS or at boot-up
- Temperature alert configuration and handling of temperature
  alerts, if any for LPDDR2 devices
  * temperature alert is based on periodic polling of MR4 mode
    register in DDR devices automatically performed by hardware
  * timings are de-rated and brought back to nominal when
    temperature raises and falls respectively
- Cache of calculated register values to avoid re-calculating
  them

The driver will need some minor updates when it is eventually
integrated with Dynamic Voltage and Frequency Scaling (DVFS).
This can not be done now as DVFS support is not available in
the mainline yet.

Discussions with Santosh Shilimkar <santosh.shilimkar@ti.com>
were immensely helpful in shaping up the interfaces. Vibhore Vardhan
<vvardhan@gmail.com> did the initial code snippet for thermal
handling.

Testing: 
- The driver is tested on OMAP4430 SDP.
- The driver in a slightly adapted form is also tested on OMAP5.
- Since mainline kernel doesn't have DVFS support yet,
  testing was done using a test module.
- Temperature alert handling was tested with simulated interrupts
  and faked temperature values as testing all cases in real-life
  scenarios is difficult.
- Tested the driver as a module

Cc: Greg KH <greg@kroah.com>

v4:
- Converted instances of EXPORT_SYMBOL to EXPORT_SYMBOL_GPL
- Removed un-necessary "#ifndef __ASSEMBLY__'
- Minor formatting fix

v2:
- Fixed a bug found in the implementation of errata i728
  workaround
- Fixed the value of frequency printed in debugfs
- Dropped the hwmod patch as Paul has already posted a
  a hwmod series [1] that adds hwmod for EMIF
- Converted instances of __init to __init_or_module

[1] http://thread.gmane.org/gmane.linux.ports.arm.omap/72855

Aneesh V (7):
  misc: ddr: add LPDDR2 data from JESD209-2
  misc: emif: add register definitions for EMIF
  misc: emif: add basic infrastructure for EMIF driver
  misc: emif: handle frequency and voltage change events
  misc: emif: add interrupt and temperature handling
  misc: emif: add one-time settings
  misc: emif: add debugfs entries for emif

 Documentation/misc-devices/ti-emif.txt  |   58 ++
 drivers/misc/Kconfig                    |   12 +
 drivers/misc/Makefile                   |    1 +
 drivers/misc/emif.c                     | 1670 +++++++++++++++++++++++++++++++
 drivers/misc/emif.h                     |  589 +++++++++++
 include/linux/platform_data/emif_plat.h |  128 +++
 include/misc/jedec_ddr.h                |  175 ++++
 lib/Kconfig                             |    8 +
 lib/Makefile                            |    2 +
 lib/jedec_ddr_data.c                    |  135 +++
 10 files changed, 2778 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/misc-devices/ti-emif.txt
 create mode 100644 drivers/misc/emif.c
 create mode 100644 drivers/misc/emif.h
 create mode 100644 include/linux/platform_data/emif_plat.h
 create mode 100644 include/misc/jedec_ddr.h
 create mode 100644 lib/jedec_ddr_data.c

[PATCH v4 0/7] Add TI EMIF SDRAM controller driver

[Kyungmin Park]

Hi,

On 3/17/12, Aneesh V <aneesh@ti.com> wrote:
> Add a driver for the EMIF SDRAM controller used in TI SoCs
>
> EMIF is an SDRAM controller that supports, based on its revision,
> one or more of LPDDR2/DDR2/DDR3 protocols.This driver adds support
> for LPDDR2.
>
> The driver supports the following features:
> - Calculates the DDR AC timing parameters to be set in EMIF
>   registers using data from the device data-sheets and based
>   on the DDR frequency. If data from data-sheets is not available
>   default timing values from the JEDEC spec are used. These
>   will be safe, but not necessarily optimal
> - API for changing timings during DVFS or at boot-up
> - Temperature alert configuration and handling of temperature
>   alerts, if any for LPDDR2 devices
>   * temperature alert is based on periodic polling of MR4 mode
>     register in DDR devices automatically performed by hardware
>   * timings are de-rated and brought back to nominal when
>     temperature raises and falls respectively
> - Cache of calculated register values to avoid re-calculating
>   them
>
> The driver will need some minor updates when it is eventually
> integrated with Dynamic Voltage and Frequency Scaling (DVFS).
> This can not be done now as DVFS support is not available in
> the mainline yet.
Do you see the devfreq? it's designed for non-cpu device frequency.
It's role is similar with cpufreq.

Now samsung exynos uses devfreq for DRAM bus frequency.

Thank you,
Kyungmin Park
>
> Discussions with Santosh Shilimkar <santosh.shilimkar@ti.com>
> were immensely helpful in shaping up the interfaces. Vibhore Vardhan
> <vvardhan@gmail.com> did the initial code snippet for thermal
> handling.
>
> Testing:
> - The driver is tested on OMAP4430 SDP.
> - The driver in a slightly adapted form is also tested on OMAP5.
> - Since mainline kernel doesn't have DVFS support yet,
>   testing was done using a test module.
> - Temperature alert handling was tested with simulated interrupts
>   and faked temperature values as testing all cases in real-life
>   scenarios is difficult.
> - Tested the driver as a module
>
> Cc: Greg KH <greg@kroah.com>
>
> v4:
> - Converted instances of EXPORT_SYMBOL to EXPORT_SYMBOL_GPL
> - Removed un-necessary "#ifndef __ASSEMBLY__'
> - Minor formatting fix
>
> v2:
> - Fixed a bug found in the implementation of errata i728
>   workaround
> - Fixed the value of frequency printed in debugfs
> - Dropped the hwmod patch as Paul has already posted a
>   a hwmod series [1] that adds hwmod for EMIF
> - Converted instances of __init to __init_or_module
>
> [1] http://thread.gmane.org/gmane.linux.ports.arm.omap/72855
>
> Aneesh V (7):
>   misc: ddr: add LPDDR2 data from JESD209-2
>   misc: emif: add register definitions for EMIF
>   misc: emif: add basic infrastructure for EMIF driver
>   misc: emif: handle frequency and voltage change events
>   misc: emif: add interrupt and temperature handling
>   misc: emif: add one-time settings
>   misc: emif: add debugfs entries for emif
>
>  Documentation/misc-devices/ti-emif.txt  |   58 ++
>  drivers/misc/Kconfig                    |   12 +
>  drivers/misc/Makefile                   |    1 +
>  drivers/misc/emif.c                     | 1670
> +++++++++++++++++++++++++++++++
>  drivers/misc/emif.h                     |  589 +++++++++++
>  include/linux/platform_data/emif_plat.h |  128 +++
>  include/misc/jedec_ddr.h                |  175 ++++
>  lib/Kconfig                             |    8 +
>  lib/Makefile                            |    2 +
>  lib/jedec_ddr_data.c                    |  135 +++
>  10 files changed, 2778 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/misc-devices/ti-emif.txt
>  create mode 100644 drivers/misc/emif.c
>  create mode 100644 drivers/misc/emif.h
>  create mode 100644 include/linux/platform_data/emif_plat.h
>  create mode 100644 include/misc/jedec_ddr.h
>  create mode 100644 lib/jedec_ddr_data.c
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>

[PATCH v4 0/7] Add TI EMIF SDRAM controller driver

[Paul Walmsley]

Cc Mark Greer, Mark Salter

Hi Greg, Aneesh,

On Sat, 17 Mar 2012, Aneesh V wrote:

> Add a driver for the EMIF SDRAM controller used in TI SoCs
> 
> EMIF is an SDRAM controller that supports, based on its revision,
> one or more of LPDDR2/DDR2/DDR3 protocols.This driver adds support
> for LPDDR2.

Just checking to see what the current state of this series is.  Greg, are 
you considering this for merging, or are there remaining issues?  Aneesh, 
do you have any remaining issues to resolve with this set?

This is useful not only for OMAP4 and AM3517/3505, but also will probably 
be useful for the C6x chips that Mark Salter is working on.


- Paul

[PATCH v4 0/7] Add TI EMIF SDRAM controller driver

[Aneesh V]

Hi Paul,

On 04/11/2012 07:44 PM, Paul Walmsley wrote:
> Cc Mark Greer, Mark Salter
>
> Hi Greg, Aneesh,
>
> On Sat, 17 Mar 2012, Aneesh V wrote:
>
>> Add a driver for the EMIF SDRAM controller used in TI SoCs
>>
>> EMIF is an SDRAM controller that supports, based on its revision,
>> one or more of LPDDR2/DDR2/DDR3 protocols.This driver adds support
>> for LPDDR2.
>
> Just checking to see what the current state of this series is.  Greg, are
> you considering this for merging, or are there remaining issues?  Aneesh,
> do you have any remaining issues to resolve with this set?

No. I don't have anything remaining. It's just that I was on vacation
for a while and couldn't find time for this after that. If Greg is fine
with this, I can rebase it and send one final version with
Reviewed-By's from Santosh and Benoit.

thanks,
Aneesh

[PATCH v4 0/7] Add TI EMIF SDRAM controller driver

[Greg KH]

On Wed, Apr 11, 2012 at 08:44:39PM -0600, Paul Walmsley wrote:
> Cc Mark Greer, Mark Salter
> 
> Hi Greg, Aneesh,
> 
> On Sat, 17 Mar 2012, Aneesh V wrote:
> 
> > Add a driver for the EMIF SDRAM controller used in TI SoCs
> > 
> > EMIF is an SDRAM controller that supports, based on its revision,
> > one or more of LPDDR2/DDR2/DDR3 protocols.This driver adds support
> > for LPDDR2.
> 
> Just checking to see what the current state of this series is.  Greg, are 
> you considering this for merging, or are there remaining issues?  Aneesh, 
> do you have any remaining issues to resolve with this set?

What about the review comment about devfreq?

> This is useful not only for OMAP4 and AM3517/3505, but also will probably 
> be useful for the C6x chips that Mark Salter is working on.

It's still in my "to-review" queue, that I'm slowly making my way
through.  So it's not lost, but I would like to get the devfreq
interface question cleared up first.

thanks,

greg k-h

[PATCH v4 0/7] Add TI EMIF SDRAM controller driver

[Paul Walmsley]

thanks for the response Greg,

Hi Aneesh,

On Wed, 11 Apr 2012, Greg KH wrote:

> On Wed, Apr 11, 2012 at 08:44:39PM -0600, Paul Walmsley wrote:
> > On Sat, 17 Mar 2012, Aneesh V wrote:
> > 
> > > Add a driver for the EMIF SDRAM controller used in TI SoCs
> > > 
> > > EMIF is an SDRAM controller that supports, based on its revision,
> > > one or more of LPDDR2/DDR2/DDR3 protocols.This driver adds support
> > > for LPDDR2.
> > 
> > Just checking to see what the current state of this series is.  Greg, are 
> > you considering this for merging, or are there remaining issues?  Aneesh, 
> > do you have any remaining issues to resolve with this set?
> 
> What about the review comment about devfreq?
> 
> > This is useful not only for OMAP4 and AM3517/3505, but also will probably 
> > be useful for the C6x chips that Mark Salter is working on.
> 
> It's still in my "to-review" queue, that I'm slowly making my way
> through.  So it's not lost, but I would like to get the devfreq
> interface question cleared up first.

Aneesh, care to address the devfreq comments? 

I haven't looked at the series closely, so unfortunately I'm not presently 
in a position to comment.


- Paul

[PATCH v4 0/7] Add TI EMIF SDRAM controller driver

[Santosh Shilimkar]

On Thursday 12 April 2012 08:30 AM, Greg KH wrote:
> On Wed, Apr 11, 2012 at 08:44:39PM -0600, Paul Walmsley wrote:
>> Cc Mark Greer, Mark Salter
>>
>> Hi Greg, Aneesh,
>>
>> On Sat, 17 Mar 2012, Aneesh V wrote:
>>
>>> Add a driver for the EMIF SDRAM controller used in TI SoCs
>>>
>>> EMIF is an SDRAM controller that supports, based on its revision,
>>> one or more of LPDDR2/DDR2/DDR3 protocols.This driver adds support
>>> for LPDDR2.
>>
>> Just checking to see what the current state of this series is.  Greg, are 
>> you considering this for merging, or are there remaining issues?  Aneesh, 
>> do you have any remaining issues to resolve with this set?
> 
> What about the review comment about devfreq?
>
Devfreq is not suitable for this driver. I already replied on this
thread [1]

Acting on frequency change is just one function of the controller
driver and that too need not bed to attached with devfreq. The driver
has features like temperature handling as per JDEC specs, active power
managements modes, system wide suspend power management like self
refresh and also configuration which can help memory hotplug for
power savings and initialising the DDR timings to avoid boot-loader
defaults.The controller IP works in conjunction with PRCM (OMAP Power
IP) block to achieve some of this functionality.

I was hoping that we will have some thing like drivers/memory/*
but since it doesn't exist, we used drivers/misc.

Regards
Santosh
[1] https://lkml.org/lkml/2012/3/19/178

>> This is useful not only for OMAP4 and AM3517/3505, but also will probably 
>> be useful for the C6x chips that Mark Salter is working on.
> 
> It's still in my "to-review" queue, that I'm slowly making my way
> through.  So it's not lost, but I would like to get the devfreq
> interface question cleared up first.
> 
Let us know if you need more clarification on devfreq part.
Thanks for looking into it.

Regards
Santosh

[PATCH v4 0/7] Add TI EMIF SDRAM controller driver

[Greg KH]

On Thu, Apr 12, 2012 at 12:17:49PM +0530, Santosh Shilimkar wrote:
> I was hoping that we will have some thing like drivers/memory/*
> but since it doesn't exist, we used drivers/misc.

Why not create it?  I have no objection to that, it makes it more
obvious as to what this really is.

greg k-h

[PATCH v4 0/7] Add TI EMIF SDRAM controller driver

[Mohammed, Afzal]

Hi Greg,

On Thu, Apr 12, 2012 at 18:40:45, Greg KH wrote:
> On Thu, Apr 12, 2012 at 12:17:49PM +0530, Santosh Shilimkar wrote:
> > I was hoping that we will have some thing like drivers/memory/*
> > but since it doesn't exist, we used drivers/misc.
> 
> Why not create it?  I have no objection to that, it makes it more
> obvious as to what this really is.

There is another memory controller used in a few TI SoCs,
namely GPMC [1], do you prefer having it too there.

As of now it is not a driver, platform code handles GPMC, a patch
series for converting it into a driver (but still residing in
platform folder) was sent a few days back [2,3].


Regards
Afzal

[1]
GPMC (General Purpose Memory Controller) in brief:
GPMC is an unified memory controller dedicated to interfacing external
memory devices like
 Asynchronous SRAM like memories and application specific integrated circuit devices.
 Asynchronous, synchronous, and page mode burst NOR flash devices NAND flash
 Pseudo-SRAM devices

GPMC has to be configured as required by timings of the connected
peripheral. It needs to be configured only initially. Once it is
configured it can be used to handle different protocols like NAND,
NOR. Various kinds of devices like ethernet, uart, usb, fpga etc
can work using GPMC interface. GPMC has a seperate additional
functionality of NAND handling

[2] https://lkml.org/lkml/2012/4/5/210
[3] https://lkml.org/lkml/2012/4/5/212

[PATCH v4 0/7] Add TI EMIF SDRAM controller driver

[Greg KH]

On Thu, Apr 12, 2012 at 01:34:15PM +0000, Mohammed, Afzal wrote:
> Hi Greg,
> 
> On Thu, Apr 12, 2012 at 18:40:45, Greg KH wrote:
> > On Thu, Apr 12, 2012 at 12:17:49PM +0530, Santosh Shilimkar wrote:
> > > I was hoping that we will have some thing like drivers/memory/*
> > > but since it doesn't exist, we used drivers/misc.
> > 
> > Why not create it?  I have no objection to that, it makes it more
> > obvious as to what this really is.
> 
> There is another memory controller used in a few TI SoCs,
> namely GPMC [1], do you prefer having it too there.

Sure, why not?

> As of now it is not a driver, platform code handles GPMC, a patch
> series for converting it into a driver (but still residing in
> platform folder) was sent a few days back [2,3].

People are moving things out of the platform folder, so drivers/memory
makes sense.

thanks,

greg k-h

[PATCH v4 0/7] Add TI EMIF SDRAM controller driver

[Mohammed, Afzal]

Hi Greg,

On Thu, Apr 12, 2012 at 19:40:50, Greg KH wrote:
> > There is another memory controller used in a few TI SoCs,
> > namely GPMC [1], do you prefer having it too there.
> 
> Sure, why not?

Thanks a lot, we were struggling to find a suitable location for the driver.

Regards
Afzal 

[PATCH v4 0/7] Add TI EMIF SDRAM controller driver

[Kevin Hilman]

"Mohammed, Afzal" <afzal@ti.com> writes:

> Hi Greg,
>
> On Thu, Apr 12, 2012 at 18:40:45, Greg KH wrote:
>> On Thu, Apr 12, 2012 at 12:17:49PM +0530, Santosh Shilimkar wrote:
>> > I was hoping that we will have some thing like drivers/memory/*
>> > but since it doesn't exist, we used drivers/misc.
>> 
>> Why not create it?  I have no objection to that, it makes it more
>> obvious as to what this really is.
>
> There is another memory controller used in a few TI SoCs,
> namely GPMC [1], do you prefer having it too there.
>
> As of now it is not a driver, platform code handles GPMC, a patch
> series for converting it into a driver (but still residing in
> platform folder) was sent a few days back [2,3].

IMO, wherever EMIF ends up, GPMC should as well.

Kevin


> [1]
> GPMC (General Purpose Memory Controller) in brief:
> GPMC is an unified memory controller dedicated to interfacing external
> memory devices like
>  Asynchronous SRAM like memories and application specific integrated circuit devices.
>  Asynchronous, synchronous, and page mode burst NOR flash devices NAND flash
>  Pseudo-SRAM devices
>
> GPMC has to be configured as required by timings of the connected
> peripheral. It needs to be configured only initially. Once it is
> configured it can be used to handle different protocols like NAND,
> NOR. Various kinds of devices like ethernet, uart, usb, fpga etc
> can work using GPMC interface. GPMC has a seperate additional
> functionality of NAND handling
>
> [2] https://lkml.org/lkml/2012/4/5/210
> [3] https://lkml.org/lkml/2012/4/5/212
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4 0/7] Add TI EMIF SDRAM controller driver

[Paul Walmsley]

Hi

On Thu, 12 Apr 2012, Mohammed, Afzal wrote:

> On Thu, Apr 12, 2012 at 18:40:45, Greg KH wrote:
> > On Thu, Apr 12, 2012 at 12:17:49PM +0530, Santosh Shilimkar wrote:
> > > I was hoping that we will have some thing like drivers/memory/*
> > > but since it doesn't exist, we used drivers/misc.
> > 
> > Why not create it?  I have no objection to that, it makes it more
> > obvious as to what this really is.
> 
> There is another memory controller used in a few TI SoCs,
> namely GPMC [1], do you prefer having it too there.
> 
> As of now it is not a driver, platform code handles GPMC, a patch
> series for converting it into a driver (but still residing in
> platform folder) was sent a few days back [2,3].

Probably the GPMC driver should go into a slightly different place than 
SDRC/EMIF.  

GPMC is actually a general-purpose parallel bus driver.  It's used to 
interface Ethernet controllers, UARTs, FPGAs, NAND/NOR flash, SRAM, etc.  
It cannot be used to control DRAM, at least not without a separate DRAM 
controller chip.

SDRC/EMIF are both DRAM controllers.  That's all they do.  They can't be 
used to control anything else.  They implement DRAM refresh, etc.

So perhaps something like drivers/memory/dram/ for the SDRAM controllers, 
and maybe drivers/memory/ for the GPMC?


- Paul

[PATCH v4 0/7] Add TI EMIF SDRAM controller driver

[Cousson, Benoit]

+ Felipe,

Hi Paul,

On 4/12/2012 7:00 PM, Paul Walmsley wrote:
> Hi
>
> On Thu, 12 Apr 2012, Mohammed, Afzal wrote:
>
>> On Thu, Apr 12, 2012 at 18:40:45, Greg KH wrote:
>>> On Thu, Apr 12, 2012 at 12:17:49PM +0530, Santosh Shilimkar wrote:
>>>> I was hoping that we will have some thing like drivers/memory/*
>>>> but since it doesn't exist, we used drivers/misc.
>>>
>>> Why not create it?  I have no objection to that, it makes it more
>>> obvious as to what this really is.
>>
>> There is another memory controller used in a few TI SoCs,
>> namely GPMC [1], do you prefer having it too there.
>>
>> As of now it is not a driver, platform code handles GPMC, a patch
>> series for converting it into a driver (but still residing in
>> platform folder) was sent a few days back [2,3].
>
> Probably the GPMC driver should go into a slightly different place than
> SDRC/EMIF.
>
> GPMC is actually a general-purpose parallel bus driver.  It's used to
> interface Ethernet controllers, UARTs, FPGAs, NAND/NOR flash, SRAM, etc.
> It cannot be used to control DRAM, at least not without a separate DRAM
> controller chip.
>
> SDRC/EMIF are both DRAM controllers.  That's all they do.  They can't be
> used to control anything else.  They implement DRAM refresh, etc.

The LPDDR2 spec does consider as well NVM (Non Volatile Memory), so I 
think we should stick to driver/memory for EMIF.

> So perhaps something like drivers/memory/dram/ for the SDRAM controllers,
> and maybe drivers/memory/ for the GPMC?

In fact Felipe was considering something else for that kind of general 
purpose bus driver like GMPC, C2C and LLI...

... But I do not remember the name :-)

Regards,
Benoit

[PATCH v4 0/7] Add TI EMIF SDRAM controller driver

[Paul Walmsley]

Hi Beno?t,

On Thu, 12 Apr 2012, Cousson, Benoit wrote:

> The LPDDR2 spec does consider as well NVM (Non Volatile Memory), so I think we
> should stick to driver/memory for EMIF.

Hmm good point!

> > So perhaps something like drivers/memory/dram/ for the SDRAM controllers,
> > and maybe drivers/memory/ for the GPMC?
> 
> In fact Felipe was considering something else for that kind of general purpose
> bus driver like GMPC, C2C and LLI...
> 
> ... But I do not remember the name :-)

It would be nice if we could separate IP blocks that can drive an Ethernet 
controller or modem chip from the DRAM controllers; they are quite 
different beasts from a common API standpoint.


- Paul

[PATCH v4 0/7] Add TI EMIF SDRAM controller driver

[Cousson, Benoit]

On 4/12/2012 9:15 PM, Paul Walmsley wrote:
> Hi Beno?t,
>
> On Thu, 12 Apr 2012, Cousson, Benoit wrote:
>
>> The LPDDR2 spec does consider as well NVM (Non Volatile Memory), so I think we
>> should stick to driver/memory for EMIF.
>
> Hmm good point!
>
>>> So perhaps something like drivers/memory/dram/ for the SDRAM controllers,
>>> and maybe drivers/memory/ for the GPMC?
>>
>> In fact Felipe was considering something else for that kind of general purpose
>> bus driver like GMPC, C2C and LLI...
>>
>> ... But I do not remember the name :-)
>
> It would be nice if we could separate IP blocks that can drive an Ethernet
> controller or modem chip from the DRAM controllers; they are quite
> different beasts from a common API standpoint.

Yep, fully agree. In fact Felipe's suggestion was something like 
drivers/ocd for off-chip devices, but maybe something like drivers/gpbus 
will highlight a little bit more the bus controller aspect of such driver.

Regards,
Benoit

[PATCH v4 0/7] Add TI EMIF SDRAM controller driver

[Felipe Balbi]

On Thu, Apr 12, 2012 at 08:50:55PM +0200, Cousson, Benoit wrote:
> + Felipe,
> 
> Hi Paul,
> 
> On 4/12/2012 7:00 PM, Paul Walmsley wrote:
> >Hi
> >
> >On Thu, 12 Apr 2012, Mohammed, Afzal wrote:
> >
> >>On Thu, Apr 12, 2012 at 18:40:45, Greg KH wrote:
> >>>On Thu, Apr 12, 2012 at 12:17:49PM +0530, Santosh Shilimkar wrote:
> >>>>I was hoping that we will have some thing like drivers/memory/*
> >>>>but since it doesn't exist, we used drivers/misc.
> >>>
> >>>Why not create it?  I have no objection to that, it makes it more
> >>>obvious as to what this really is.
> >>
> >>There is another memory controller used in a few TI SoCs,
> >>namely GPMC [1], do you prefer having it too there.
> >>
> >>As of now it is not a driver, platform code handles GPMC, a patch
> >>series for converting it into a driver (but still residing in
> >>platform folder) was sent a few days back [2,3].
> >
> >Probably the GPMC driver should go into a slightly different place than
> >SDRC/EMIF.
> >
> >GPMC is actually a general-purpose parallel bus driver.  It's used to
> >interface Ethernet controllers, UARTs, FPGAs, NAND/NOR flash, SRAM, etc.
> >It cannot be used to control DRAM, at least not without a separate DRAM
> >controller chip.
> >
> >SDRC/EMIF are both DRAM controllers.  That's all they do.  They can't be
> >used to control anything else.  They implement DRAM refresh, etc.
> 
> The LPDDR2 spec does consider as well NVM (Non Volatile Memory), so I
> think we should stick to driver/memory for EMIF.
> 
> >So perhaps something like drivers/memory/dram/ for the SDRAM controllers,
> >and maybe drivers/memory/ for the GPMC?
> 
> In fact Felipe was considering something else for that kind of
> general purpose bus driver like GMPC, C2C and LLI...
> 
> ... But I do not remember the name :-)

the name matters very little :-) But the idea was to avoid writing yet
another bus driver and just use the platform_bus instead. We would do
all the channel/port setup before hand and far-end device driver
wouldn't have to know if it's integrated into the SoC or plugged though
LLI/C2C/GPMC.

>From the driver's perspective it would look the same.

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120413/f54b34a4/attachment.sig>

[PATCH v4 0/7] Add TI EMIF SDRAM controller driver

[Shilimkar, Santosh]

On Thu, Apr 12, 2012 at 6:40 PM, Greg KH <greg@kroah.com> wrote:
> On Thu, Apr 12, 2012 at 12:17:49PM +0530, Santosh Shilimkar wrote:
>> I was hoping that we will have some thing like drivers/memory/*
>> but since it doesn't exist, we used drivers/misc.
>
> Why not create it? ?I have no objection to that, it makes it more
> obvious as to what this really is.
>
Looks like I should have this question earlier.
EMIF driver is perfect candidate for drivers/memory/

Thanks Greg for suggestion. We will go ahead and
create drivers/memory and have EMIF drivers there.

Regards
Santosh

[PATCH v4 0/7] Add TI EMIF SDRAM controller driver

[V, Aneesh]

Hi Greg,

On Wed, Apr 11, 2012 at 8:00 PM, Greg KH <greg@kroah.com> wrote:
> On Wed, Apr 11, 2012 at 08:44:39PM -0600, Paul Walmsley wrote:
>> Cc Mark Greer, Mark Salter
>>
>> Hi Greg, Aneesh,
>>
>> On Sat, 17 Mar 2012, Aneesh V wrote:
>>
>> > Add a driver for the EMIF SDRAM controller used in TI SoCs
>> >
>> > EMIF is an SDRAM controller that supports, based on its revision,
>> > one or more of LPDDR2/DDR2/DDR3 protocols.This driver adds support
>> > for LPDDR2.
>>
>> Just checking to see what the current state of this series is. ?Greg, are
>> you considering this for merging, or are there remaining issues? ?Aneesh,
>> do you have any remaining issues to resolve with this set?
>
> What about the review comment about devfreq?

I see that Santosh has already commented on this.
My views are similar, that frequency update is only one of the
many functions of the driver.

>
>> This is useful not only for OMAP4 and AM3517/3505, but also will probably
>> be useful for the C6x chips that Mark Salter is working on.
>
> It's still in my "to-review" queue, that I'm slowly making my way
> through. ?So it's not lost, but I would like to get the devfreq
> interface question cleared up first.

Thanks. I will wait for your review then. Once I have your comments
I will re-work and submit in the new directory structure proposed.

thanks,
Aneesh

[PATCH v4 0/7] Add TI EMIF SDRAM controller driver

[Mohammed, Afzal]

Hi Aneesh,

On Fri, Apr 13, 2012 at 01:28:55, V, Aneesh wrote:

> Thanks. I will wait for your review then. Once I have your comments
> I will re-work and submit in the new directory structure proposed.

Do you have a plan on submitting EMIF driver in the new proposed
(drivers/memory) directory. Or shall I submit GPMC driver by creating
the new "memory" directory.

Regards
Afzal

[PATCH v4 0/7] Add TI EMIF SDRAM controller driver

[Shilimkar, Santosh]

Afzal,

On Mon, Apr 23, 2012 at 4:26 PM, Mohammed, Afzal <afzal@ti.com> wrote:
> Hi Aneesh,
>
> On Fri, Apr 13, 2012 at 01:28:55, V, Aneesh wrote:
>
>> Thanks. I will wait for your review then. Once I have your comments
>> I will re-work and submit in the new directory structure proposed.
>
> Do you have a plan on submitting EMIF driver in the new proposed
> (drivers/memory) directory. Or shall I submit GPMC driver by creating
> the new "memory" directory.
>
 Greg mentioned that he plans to do a review of EMIF driver. We were waiting
for his comments before sending the updated series.

Please go ahead in creating directory.

Regards
Santosh

[PATCH v4 0/7] Add TI EMIF SDRAM controller driver

[Mohammed, Afzal]

Hi Santosh,

On Mon, Apr 23, 2012 at 16:34:46, Shilimkar, Santosh wrote:
> Please go ahead in creating directory.

Thanks for the quick reply.

Regards
Afzal

[PATCH v4 0/7] Add TI EMIF SDRAM controller driver

[Greg KH]

On Mon, Apr 23, 2012 at 04:34:46PM +0530, Shilimkar, Santosh wrote:
> Afzal,
> 
> On Mon, Apr 23, 2012 at 4:26 PM, Mohammed, Afzal <afzal@ti.com> wrote:
> > Hi Aneesh,
> >
> > On Fri, Apr 13, 2012 at 01:28:55, V, Aneesh wrote:
> >
> >> Thanks. I will wait for your review then. Once I have your comments
> >> I will re-work and submit in the new directory structure proposed.
> >
> > Do you have a plan on submitting EMIF driver in the new proposed
> > (drivers/memory) directory. Or shall I submit GPMC driver by creating
> > the new "memory" directory.
> >
>  Greg mentioned that he plans to do a review of EMIF driver. We were waiting
> for his comments before sending the updated series.

No, don't wait for me, please resend, I was waiting for you :)

[PATCH v4 0/7] Add TI EMIF SDRAM controller driver

[Shilimkar, Santosh]

On Mon, Apr 23, 2012 at 7:57 PM, Greg KH <greg@kroah.com> wrote:
> On Mon, Apr 23, 2012 at 04:34:46PM +0530, Shilimkar, Santosh wrote:
>> Afzal,
>>
>> On Mon, Apr 23, 2012 at 4:26 PM, Mohammed, Afzal <afzal@ti.com> wrote:
>> > Hi Aneesh,
>> >
>> > On Fri, Apr 13, 2012 at 01:28:55, V, Aneesh wrote:
>> >
>> >> Thanks. I will wait for your review then. Once I have your comments
>> >> I will re-work and submit in the new directory structure proposed.
>> >
>> > Do you have a plan on submitting EMIF driver in the new proposed
>> > (drivers/memory) directory. Or shall I submit GPMC driver by creating
>> > the new "memory" directory.
>> >
>> ?Greg mentioned that he plans to do a review of EMIF driver. We were waiting
>> for his comments before sending the updated series.
>
> No, don't wait for me, please resend, I was waiting for you :)

Thanks Greg !!
Will start working on updating the patch set and post it on list.

Regards
Santosh

[PATCH v4 0/7] Add TI EMIF SDRAM controller driver

[Santosh Shilimkar]

Greg,

On Monday 23 April 2012 08:14 PM, Shilimkar, Santosh wrote:
> On Mon, Apr 23, 2012 at 7:57 PM, Greg KH <greg@kroah.com> wrote:
>> On Mon, Apr 23, 2012 at 04:34:46PM +0530, Shilimkar, Santosh wrote:
>>> Afzal,
>>>
>>> On Mon, Apr 23, 2012 at 4:26 PM, Mohammed, Afzal <afzal@ti.com> wrote:
>>>> Hi Aneesh,
>>>>
>>>> On Fri, Apr 13, 2012 at 01:28:55, V, Aneesh wrote:
>>>>
>>>>> Thanks. I will wait for your review then. Once I have your comments
>>>>> I will re-work and submit in the new directory structure proposed.
>>>>
>>>> Do you have a plan on submitting EMIF driver in the new proposed
>>>> (drivers/memory) directory. Or shall I submit GPMC driver by creating
>>>> the new "memory" directory.
>>>>
>>>  Greg mentioned that he plans to do a review of EMIF driver. We were waiting
>>> for his comments before sending the updated series.
>>
>> No, don't wait for me, please resend, I was waiting for you :)
> 
> Thanks Greg !!
> Will start working on updating the patch set and post it on list.
> 
Have posted the updated version [1] as per discussion on
this thread.

Regards
Santosh

[1] http://www.spinics.net/lists/arm-kernel/msg171869.html

[PATCH v4 0/7] Add TI EMIF SDRAM controller driver

[Mark Salter]

On Wed, 2012-04-11 at 20:44 -0600, Paul Walmsley wrote:
> This is useful not only for OMAP4 and AM3517/3505, but also will probably 
> be useful for the C6x chips that Mark Salter is working on.

I have been keeping an occasional eye on these patches but I don't see
any usefulness for the currently supported C6X chips. The current set
of supported chips use a completely different DDR controller. There is
an EMIF controller on some which provides a parallel bus for connecting
to external devices (NOR flash, etc) but it looks completely different
from the OMAP EMIF. I think the TI DaVinci chips have EMIF (and other IO
blocks) similar to C6X which could use common drivers.

--Mark