Re: [PATCH 5/5] add module parameters for default values

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 02/07/18 09:33, Florian Echtler wrote:
> On 06.02.2018 22:31, Hans Verkuil wrote:
>> On 02/06/2018 10:01 PM, Florian Echtler wrote:
>>> To allow setting custom parameters for the sensor directly at startup, the
>>> three primary controls are exposed as module parameters in this patch.
>>>
>>> +/* module parameters */
>>> +static uint brightness = SUR40_BRIGHTNESS_DEF;
>>> +module_param(brightness, uint, 0644);
>>> +MODULE_PARM_DESC(brightness, "set default brightness");
>>> +static uint contrast = SUR40_CONTRAST_DEF;
>>> +module_param(contrast, uint, 0644);
>>> +MODULE_PARM_DESC(contrast, "set default contrast");
>>> +static uint gain = SUR40_GAIN_DEF;
>>> +module_param(gain, uint, 0644);
>>> +MODULE_PARM_DESC(contrast, "set default gain");
>>
>> contrast -> gain
> 
> Ah, typo. Thanks, will fix that.
> 
>> Isn't 'initial gain' better than 'default gain'?
> 
> Probably correct, yes.
> 
>> If I load this module with gain=X, will the gain control also
>> start off at X? I didn't see any code for that.
> 
> This reminds me: how can I get/set the control from inside the driver?
> Should I use something like the following:
> 
> struct v4l2_ctrl *ctrl = v4l2_ctrl_find(&sur40->ctrls, V4L2_CID_BRIGHTNESS);
> int val = v4l2_ctrl_g_ctrl(ctrl);
> // modify val...
> v4l2_ctrl_s_ctrl(ctrl, val);

Yes, that's correct. Usually drivers store the ctrl in their state struct
when they create the control. That way you don't have to find it.

> 
>> It might be useful to add the allowed range in the description.
>> E.g.: "set initial gain, range=0-255". Perhaps mention even the
>> default value, but I'm not sure if that's really needed.
> 
> Good point, though - right now the code directly sets the registers without any
> clamping, I guess it would be better to call the control framework as mentioned
> above?

Easiest is just to use this value for the default when you create the
control. Just clamp it first.

E.g.:

static uint gain = SUR40_GAIN_DEF;
module_param(gain, uint, 0644);

...

gain = clamp(gain, SUR40_GAIN_MIN, SUR40_GAIN_MAX);
v4l2_ctrl_new_std(&sur40->ctrls, &sur40_ctrl_ops, V4L2_CID_GAIN,
	  SUR40_GAIN_MIN, SUR40_GAIN_MAX, 1, gain);

You need to clamp gain first, otherwise v4l2_ctrl_new_std would fail
if the given default value is out of range.

Regards,

	Hans



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux