Re: [RFC PATCH 0/8] Add TI EMIF SDRAM controller driver

[Aneesh V]

Greg,

On Thursday 16 February 2012 09:53 PM, Greg KH wrote:
> On Thu, Feb 16, 2012 at 04:21:11PM +0530, Santosh Shilimkar wrote:
> > Andrew, Greg,
> >
> > On Saturday 04 February 2012 05:46 PM, 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
> > > - 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 DVFS. This can not be done now as DVFS support
> > > is not available yet in mainline.
> > >
> > > Discussions with Santosh Shilimkar<santosh.shilimkar@xxxxxx>
> > > were immensely helpful in shaping up the interfaces. Vibhore Vardhan
> > > <vvardhan@xxxxxxxxx>  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.
> > [...]
> >
> > >   arch/arm/mach-omap2/omap_hwmod_44xx_data.c |  110 ++
> > >   drivers/misc/Kconfig                       |   20 +
> > >   drivers/misc/Makefile                      |    2 +
> > >   drivers/misc/emif.c                        | 1522 ++++++++++++++++++++++++++++
> > >   drivers/misc/emif_regs.h                   |  461 +++++++++
> > >   drivers/misc/jedec_ddr_data.c              |  141 +++
> > >   include/linux/emif.h                       |  257 +++++
> > >   include/linux/jedec_ddr.h                  |  174 ++++
> >
> > Any suggestion on where this driver can reside. It's a memory
> > controller driver which supports standard DDR functionality
> > as per JDEC specs including thermal alert. On top of
> > that it does support DVFS using the TI PRCM IP block.
>
> I don't know what any of those TLA words mean, so I really can't suggest

This is a driver for TI's memory controller(called EMIF). The
driver is needed for adjusting the controller settings on frequency,
voltage, and temperature changes. Any suggestion as to where this
should go?

> where this code should go.  But just from this diffstat, it looks like
> you are creating a new user/kernel interface, without documenting it
> anywhere, which isn't ok.

I think you are referring to the header files added in include/linux/
They are not creating new user/kernel interface per se.

"include/linux/jedec_ddr.h" is the interface to a library that contains
data from the DDR specs. "include/linux/emif.h" has definitions for
platform data needed by the driver. Maybe these should go to some other
sub-directory within include/ or include/linux/ ?

I shall add documentation for the driver in the next revision.

Thanks,
Aneesh
--
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/