Thread (14 messages) flat view 14 messages, 5 authors, 2011-08-12
STALE5512d

[RFC PATCH 6/6] hwmon: OMAP4: On die temperature sensor driver

From: Felipe Balbi <hidden>
Date: 2011-08-12 08:44:55
Also in: linux-omap

Hi,

On Fri, Aug 12, 2011 at 08:56:29AM +0530, Rajendra Nayak wrote:
On 8/12/2011 3:07 AM, Roger Quadros wrote:
quoted
On 08/11/2011 01:55 PM, Felipe Balbi wrote:
quoted
Hi,

On Thu, Aug 11, 2011 at 09:54:09PM +0300, Felipe Balbi wrote:
quoted
quoted
quoted
you need some other way to handle this. Why do you need to manually set
the rate rather than having hwmod handle this for you ?

your argument that "it's a one time setting" is not enough to have this
in the driver. Drivers should not care about clocks anymore, this should
have been done on another layer.
Hwmod will have no idea on the rate required.
does the rate need to change ? Also, I have not mentioned hwmod anytime
i did mention hwmod, nevermind that part. Still I'm not sure where is
the right place to handle this.
Aren't the omap_device_pm_latency callbacks the right place to do it?

e.g. in the following snippet from mach-omap2/temp_sensor_device.c

+static struct omap_device_pm_latency omap_temp_sensor_latency[] = {
+	{
+	 .deactivate_func = omap_device_idle_hwmods,
+	 .activate_func = omap_device_enable_hwmods,
+	 .flags = OMAP_DEVICE_LATENCY_AUTO_ADJUST,
+	}
+};

instead of directly pointing activate_func to omap_device_enable_hwmods,
it could point to a function that sets the required clock rate and then
enables the hwmod.
FWIK, its a one time requirement to set the clock rate to the
right rate the device can operate in based on what a platform
supports. What you are suggesting would add the overhead of doing
this every time the device is runtime enabled/idled.
if it's a one time setting, why don't you just change the clock fwk to
handle this nicely ? Maybe provide a different ->enable() function which
would already set the correct rate ?

Russell, what would be the best way here ? driver needs clock to be at a
particular rate for the device to work, but it's a one time setting and
I don't think driver should be doing clk_get() - clk_enable() -
clk_set_rate(), where should that functionality be put ?

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110812/e7038205/attachment-0001.sig>
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help