diff mbox

[5/5] add default control values as module parameters

Message ID 1517840981-12280-6-git-send-email-floe@butterbrot.org (mailing list archive)
State New, archived
Headers show

Commit Message

Florian Echtler Feb. 5, 2018, 2:29 p.m. UTC 
Signed-off-by: Florian Echtler <floe@butterbrot.org>
---
 drivers/input/touchscreen/sur40.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

Comments

Hans Verkuil Feb. 5, 2018, 2:56 p.m. UTC | #1 
On 02/05/2018 03:29 PM, Florian Echtler wrote:
> Signed-off-by: Florian Echtler <floe@butterbrot.org>

Please add a change log when you make a patch.

I for one would like to know why this has to be supplied as a module option.
Some documentation in the code would be helpful as well (e.g. I have no idea
what a 'vsvideo' is).

Regards,

	Hans

> ---
>  drivers/input/touchscreen/sur40.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/input/touchscreen/sur40.c b/drivers/input/touchscreen/sur40.c
> index c4b7cf1..d612f3f 100644
> --- a/drivers/input/touchscreen/sur40.c
> +++ b/drivers/input/touchscreen/sur40.c
> @@ -173,6 +173,14 @@ int sur40_v4l2_contrast   = SUR40_CONTRAST_DEF;   /* blacklevel   */
>  int sur40_v4l2_gain       = SUR40_GAIN_DEF;       /* gain         */
>  int sur40_v4l2_backlight  = 1;                    /* preprocessor */
>  
> +/* module parameters */
> +static uint irlevel = SUR40_BRIGHTNESS_DEF;
> +module_param(irlevel, uint, 0644);
> +MODULE_PARM_DESC(irlevel, "set default irlevel");
> +static uint vsvideo = SUR40_VSVIDEO_DEF;
> +module_param(vsvideo, uint, 0644);
> +MODULE_PARM_DESC(vsvideo, "set default vsvideo");
> +
>  static const struct v4l2_pix_format sur40_pix_format[] = {
>  	{
>  		.pixelformat = V4L2_TCH_FMT_TU08,
> @@ -372,6 +380,11 @@ static void sur40_open(struct input_polled_dev *polldev)
>  
>  	dev_dbg(sur40->dev, "open\n");
>  	sur40_init(sur40);
> +
> +	/* set default values */
> +	sur40_set_irlevel(sur40, irlevel);
> +	sur40_set_vsvideo(sur40, vsvideo);
> +	sur40_set_preprocessor(sur40, SUR40_BACKLIGHT_DEF);
>  }
>  
>  /* Disable device, polling has stopped. */
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Florian Echtler Feb. 6, 2018, 9:18 a.m. UTC | #2 
On 05.02.2018 15:56, Hans Verkuil wrote:
> Please add a change log when you make a patch.
> I for one would like to know why this has to be supplied as a module option.

The idea here was that each individual SUR40 device will likely have slightly
different "ideal" settings for the parameters, and by using the module options,
you can set them right away at startup.

I'm aware that the usual way to do this would be from userspace using v4l2-ctl,
but the SUR40 is sort of a special case: the video settings will also influence
the internal touch detection, and in the worst case, starting the driver with
the default parameters from flash will immediately cause so many false-positive
touch points to be detected that the graphical shell becomes unusable. This has
actually happened several times during testing, so we considered a module option
to be the easiest way for dealing with this.

> Some documentation in the code would be helpful as well (e.g. I have no idea
> what a 'vsvideo' is).
Right - will do that, too.

Best regards, Florian
diff mbox

Patch

diff --git a/drivers/input/touchscreen/sur40.c b/drivers/input/touchscreen/sur40.c
index c4b7cf1..d612f3f 100644
--- a/drivers/input/touchscreen/sur40.c
+++ b/drivers/input/touchscreen/sur40.c
@@ -173,6 +173,14 @@  int sur40_v4l2_contrast   = SUR40_CONTRAST_DEF;   /* blacklevel   */
 int sur40_v4l2_gain       = SUR40_GAIN_DEF;       /* gain         */
 int sur40_v4l2_backlight  = 1;                    /* preprocessor */
 
+/* module parameters */
+static uint irlevel = SUR40_BRIGHTNESS_DEF;
+module_param(irlevel, uint, 0644);
+MODULE_PARM_DESC(irlevel, "set default irlevel");
+static uint vsvideo = SUR40_VSVIDEO_DEF;
+module_param(vsvideo, uint, 0644);
+MODULE_PARM_DESC(vsvideo, "set default vsvideo");
+
 static const struct v4l2_pix_format sur40_pix_format[] = {
 	{
 		.pixelformat = V4L2_TCH_FMT_TU08,
@@ -372,6 +380,11 @@  static void sur40_open(struct input_polled_dev *polldev)
 
 	dev_dbg(sur40->dev, "open\n");
 	sur40_init(sur40);
+
+	/* set default values */
+	sur40_set_irlevel(sur40, irlevel);
+	sur40_set_vsvideo(sur40, vsvideo);
+	sur40_set_preprocessor(sur40, SUR40_BACKLIGHT_DEF);
 }
 
 /* Disable device, polling has stopped. */