On Friday 17 February 2012 11:20 PM, Greg KH wrote:
On Fri, Feb 17, 2012 at 07:26:29PM +0530, Aneesh V wrote:
[...]
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?
For those type of things, don't the iio framework provide what you need
and what? Or perhaps the cpufreq layer?
I don't think this driver fits into the iio framework. This is not a
sensor or IO kind of device. Neither does it generate events. The
primary responsibility of this driver is to re-configure the SDRAM
controller settings on all transient events affecting it after the
bootloader has set it up at boot-time. These are typically events
generated by other sub-systems in the kernel such as the clock
framework (DDR frequency change) regulator framework (voltage
transitions) etc. Temperature events are generated (by polling the
SDRAM device) and handled within the driver.
Neither do I think it will fit into cpufreq layer because DDR
frequency can be triggered by clock framework independent of cpufreq.
Moreover handling DDR frequency change is only one of the functions of
the driver.
>>> 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.
Then why are they in include/linux/ ?
My mistake. I will move them to more appropriate places.
"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/ ?
Who needs these files? If it's only the drivers themselves, then put it
in the same directory as the driver. If it's platform data, then put
it, and only it, in the include/linux/platform_data/ directory.
The above work has two components:
1. The driver
2. A small library with data from the SDRAM specs that the driver uses.
Alan Cox has suggested to move the library part to lib/ so the
corresponding header file jedec_ddr.h has to be in some common place.
Can this be in include/misc or do you have a better place to suggest?
The other one emif.h can be split into two parts, one for platform data
that can go under include/linux/platform_data/ and the rest in the same
directory as the driver, as you suggested.
I shall add documentation for the driver in the next revision.
That would be good. Please also cc: me on the next revision if you wish
me to take the patches (hint, get_maintainer.pl should have told you
this...)
Sure. Thanks.
- Aneesh
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
