Re: [PATCH 1/2] PM / Runtime: Add sysfs switch for disablingdevice run-time PM (rev. 2)

[Pavel Machek]

Hi!

>  /*
> + *	control - Report/change current runtime PM setting of the device
> + *
> + *	Runtime power management of a device can be blocked with the help of
> + *	this attribute.  All devices have one of the following two values for
> + *	the power/control file:
> + *
> + *	 + "auto\n" to allow the device to be power managed at run time;
> + *	 + "on\n" to prevent the device from being power managed at run time;

I find interface with mandatory \n quite 'interesting'. 

Plus english is strange here. All devices have ... "auto" to allow...?
Strange formulation. List the values first, then add "write ... to allow"?

> + *	The default for all devices is "auto", which means that devices may be
> + *	subject to automatic power management, depending on their drivers.

Is it wise to specify 'auto' default value for devices without runtime
pm?


> +static ssize_t control_store(struct device * dev, struct device_attribute *attr,
> +			     const char * buf, size_t n)
> +{
> +	char *cp;
> +	int len = n;
> +
> +	cp = memchr(buf, '\n', n);
> +	if (cp)
> +		len = cp - buf;
> +	if (len == sizeof ctrl_auto - 1 && strncmp(buf, ctrl_auto, len) == 0)
> +		pm_runtime_allow(dev);

parenthesis after sizeof?

Do I read it correctly that all of

"auto"
"auto\n"
"auto\non\nIm confused"

will switch to auto?

(and sorry for late reply...)
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
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/