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

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

Hi Randy,

On 5/22/24 9:01 PM, Randy Dunlap wrote:
> Hi,
> 
> On 5/22/24 9:48 AM, Hans de Goede wrote:
>> +static int __init ts_parse_props(char *str)
>> +{
>> +	char *name, *value;
>> +	u32 u32val;
>> +	int i, ret;
>> +
>> +	/*
>> +	 * str is part of the static_command_line from init/main.c and poking
>> +	 * holes in that by writing 0 to it is allowed, as is taking long
>> +	 * lasting references to it.
>> +	 */
>> +	ts_cmdline_data.acpi_name = strsep(&str, ",");
>> +
>> +	for (i = 0; i < MAX_CMDLINE_PROPS; i++) {
>> +		name = strsep(&str, ",");
>> +		if (!name)
>> +			break;
>> +
>> +		/* Replace '=' with 0 and make value point past '=' or NULL */
>> +		value = name;
>> +		strsep(&value, "=");
>> +		if (!value) {
>> +			ts_cmdline_props[i] = PROPERTY_ENTRY_BOOL(name);
>> +		} else if (isdigit(value[0])) {
>> +			ret = kstrtou32(value, 10, &u32val);
>> +			if (ret)
>> +				return ret;
>> +
>> +			ts_cmdline_props[i] = PROPERTY_ENTRY_U32(name, u32val);
>> +		} else {
>> +			ts_cmdline_props[i] = PROPERTY_ENTRY_STRING(name, value);
>> +		}
>> +	}
>> +
>> +	if (!i)
>> +		return -EINVAL; /* No properties specified */
>> +
>> +	if (str)
>> +		return -ENOSPC; /* More then MAX_CMDLINE_PROPS properties specified */
>> +
>> +	ts_data = &ts_cmdline_data;
>> +	return 0;
>> +}
>> +__setup("i2c_touchscreen_props=", ts_parse_props);
> 
> __setup() is different from early_param() & its family.
> __setup() functions return 1 for "handled" and 0 for "not handled".
> 
> See include/linux/init.h.

Thank you for pointing this out, I looked at parse_args() which
expects the parse_unknown_fn handler to return 0 on success or
negative errno on error. But I see know there is a layer in between
which indeed works as you point out. I'll fix this for v2 and add
a pr_warn() about invalid syntax to the parser since the core does
not do this.

Regards,

Hans