On 05/18/2012 06:57 PM, Johan Hovold wrote:

On Fri, May 18, 2012 at 06:34:01PM +0100, Jonathan Cameron wrote:

quoted

On 05/18/2012 01:27 PM, Johan Hovold wrote:

[...]

quoted

quoted

I really think that this should remain a device specific attribute as I
originally suggested. It's an integration parameter that needs to be set
precisely depending on the actual hardware setup (which analog light
sensor and other external components). 

Then it shouldn't be exposed to userspace.  If there is reason to vary
it from userspace then it is a calibration parameter and should be
treated like the other ones we have, if not it should be done from
dt or platform data.

quoted

The lm3533 also supports two types of light sensors: pwm- and analog-
output ones. The resistor select settings only applies when in analog
mode as the input is always high impedance otherwise. Thus a generic
attribute (such as calibscale or hardware gain) shouldn't be used as it
will have no effect whatsoever in PWM-mode.

I'm thus back at my original proposal, albeit with a different name (I
think a lot of this discussion could have been avoided had I not
misnamed the parameter "gain"): 

What:		/sys/bus/iio/devices/iio:deviceX/r_select
Description:
		Set the ALS internal pull-down resistor for analog input mode
		(1..127), such that,

		R_als = 200000 / r_select	(ohm)

		This setting is ignored in PWM-mode (input is always high
		impedance in PWM-mode).

I don't think much is gained from using ohm as the unit: it just adds
complexity and the selected resistor setting will likely not match the
input value anyway. It's better that the chip integrators have full
control over which resistor setting is actually used so that it matches
external components.

This smacks of something that should never be exposed to users.
I'd hide it away in platform data.

Fair enough. I'll drop the sysfs param and submit a patch for mfd-next
which adds r_select to the platform data.

cool. I'll review the rest of the patch with the assumption you'll do this.

Jonathan