Re: [PATCH 1/3] Input: touchscreen - Extend touchscreen_parse_properties() to allow overriding settings with a module option

[Bastien Nocera <hadess@hadess.net>]

On Sun, 2021-06-13 at 12:21 +0200, Hans de Goede wrote:
> On x86/ACPI platforms touchscreens mostly just work without needing
> any
> device/model specific configuration. But in some cases (mostly with
> Silead
> and Goodix touchscreens) it is still necessary to manually specify
> various
> touchscreen-properties on a per model basis.
> 
> This is handled by drivers/platform/x86/touchscreen_dmi.c which
> contains
> a large list of per-model touchscreen properties which it attaches to
> the
> (i2c)device before the touchscreen driver's probe() method gets
> called.
> This means that ATM changing these settings requires recompiling the
> kernel. This makes figuring out what settings/properties a specific
> touchscreen needs very hard for normal users to do.
> 
> Add a new, optional, settings_override string argument to
> touchscreen_parse_properties(), which takes a list of ; separated
> property-name=value pairs, e.g. :
> "touchscreen-size-x=1665;touchscreen-size-y=1140;touchscreen-swapped-
> x-y".
> 
> This new argument can be used by drivers to implement a module option
> which
> allows users to easily specify alternative settings for testing.
> 
> The 2 new touchscreen_property_read_u32() and
> touchscreen_property_read_bool() helpers are also exported so that
> drivers can use these to add settings-override support to the code
> for driver-specific properties.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Note instead of patching all touchscreen drivers we could also rename
> touchscreen_parse_properties() to
> touchscreen_parse_properties_with_override()
> and add a static inline wrapper which passes NULL. Just patching the
> drivers
> feels a bit cleaner to me though.

It would be easier to do that in a separate commit, or separate
commits, keeping just the new parsing code separate, even if you remove
the _with_override() variant the next commit.

I haven't reviewed the argument parsing code, but eep. If this were
user-space code, we'd have exported it and tried to feed it all kind of
garbage to see whether it parsed things properly, even if it's only run
on the author's machine. Can't say that I like it.

Cheers