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

On 06.02.2018 22:31, Hans Verkuil wrote:

On 02/06/2018 10:01 PM, Florian Echtler wrote:

quoted

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);

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?

Best regards, Florian
-- 
SENT FROM MY DEC VT50 TERMINAL