"Gopinath, Thara" <thara@xxxxxx> writes: >>>Thara Gopinath <thara@xxxxxx> writes: >>> >>>> This patch adds debug support to the voltage and smartreflex drivers. >>>> This means a whole bunch of voltage processor and smartreflex >>>> parameters are now visible through the pm debugfs. By default >>>> only a read of these parameters are permitted. If you need to >>>> write into them then >>>> echo 1 > /pm_debug/enable_sr_vp_debug >>> >>>Why a read-only interface by default? As a debug interface it seems >>>redundant to have to enable it. >>> > > Read-only interface by default so that we can read these values from > user space even if we do not want to manipulate it from user-side. > If we do not want to manipulate it from the user-side, then simply don't write to it. Remember, this is a debug interface, not a primary interface. I think the enable_sr_vp_debug flag should disappear, and it should be a read/write interface. If the values are changed via debugfs, then set some per-SR instance flag that can be checked. Basically, the current code is confusing because you're using the the flag called 'enable' to determine whether the user *might have* written the values. [...] >>>> + /* >>>> + * Getting vp errorgain based on the voltage. If the debug option is >>>> + * enabled allow the override of errorgain from user side. >>>> + */ >>> >>>As suggested in earlier comment, please use a specific flag that this >>>has been overridden instead of the 'debug enabled' flag (which should >>>disappear, IMO) > > What do you mean by a separate flag. You want a flag to be maintained > for just this purpose ? Yes. I want a flag to be maintained *specifically* for this purpose, instead of using a much more general flag that only means a user *might* have overridden the values, use one that specifically means a user *has* overridden the values. Kevin -- 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
