Re: [PATCH v2] platform/x86: touchscreen_dmi: Add support for setting touchscreen properties from cmdline

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 5/23/24 8:58 PM, Randy Dunlap wrote:
> Hi Hans,
> 
> On 5/23/24 7:36 AM, Hans de Goede wrote:
>>
>> Cc: Gregor Riepl <onitake@gmail.com>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>> Changes in v2:
>> - Refactor ts_data / ts_data_dmi handling a bit (addressing Andy's review)
>> - Accept hex/octal numbers (addressing Andy's review)
>> - Fix ts_parse_props return value (addressing Randy's review)
>> - Use ':' as separator instead of ',', ',' is used in "vendor,option" style
>>   property names, e.g. "silead,home-button"
>> - pr_warn() on invalid syntax since init/main.c does not do this
>> ---
>> Note assuming this gets favourable review(s) in a reasonable timeframe
>> I'm thinking about maybe even adding this to 6.10 as a fix since users
>> not being able to easily test Silead touchscreen settings has been an
>> issue for quite a while now. Without the cmdline option being used this
>> is a no-op so the chance of this causing regressions is close to 0.
>> ---
>>  .../admin-guide/kernel-parameters.txt         | 22 +++++
>>  drivers/platform/x86/touchscreen_dmi.c        | 81 ++++++++++++++++++-
>>  2 files changed, 99 insertions(+), 4 deletions(-)
> 
> The __setup() function return value and the documentation look good (well, a couple of nits
> in the documentation, but no big deal).
> 
>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
>> index 396137ee018d..7ac315a7c0c7 100644
>> --- a/Documentation/admin-guide/kernel-parameters.txt
>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>> @@ -1899,6 +1899,28 @@
>>  				Format:
>>  				<bus_id>,<clkrate>
>>  
> 
> I would use:
> 
>> +	i2c_touchscreen_props= [HW,ACPI,X86]
>> +			Set device-properties for ACPI enumerated I2C attached
> 
> 			                          ACPI-enumerated I2C-attached

Thank you, I have squashed in this suggestion while merging the patch into
my review-hans branch.

Regards,

Hans


> 
>> +			touchscreen, to e.g. fix coordinates of upside-down
>> +			mounted touchscreens. If you need this option please
>> +			submit a drivers/platform/x86/touchscreen_dmi.c patch
>> +			adding a DMI quirk for this.
>> +
>> +			Format:
>> +			<ACPI_HW_ID>:<prop_name>=<val>[:prop_name=val][:...]
>> +			Where <val> is one of:
>> +			Omit "=<val>" entirely	Set a boolean device-property
>> +			Unsigned number		Set a u32 device-property
>> +			Anything else		Set a string device-property
>> +
>> +			Examples (split over multiple lines):
>> +			i2c_touchscreen_props=GDIX1001:touchscreen-inverted-x:
>> +			touchscreen-inverted-y
>> +
>> +			i2c_touchscreen_props=MSSL1680:touchscreen-size-x=1920:
>> +			touchscreen-size-y=1080:touchscreen-inverted-y:
>> +			firmware-name=gsl1680-vendor-model.fw:silead,home-button
>> +
>>  	i8042.debug	[HW] Toggle i8042 debug mode
>>  	i8042.unmask_kbd_data
>>  			[HW] Enable printing of interrupt data from the KBD port
> 
> Thanks.