Re: [PATCH v2 4/4] Input: Add TouchNetix aXiom I2C Touchscreen support

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Thu, May 29, 2025 at 12:08:45AM +0200, Marco Felsch wrote:
> +	if (!entry->info)
> +		WARN(1, "Unsupported usage u%x used, driver bug!", i);

You just crashed the system and caused all data to be lost if this is
ever hit :(

As you did detect this, please handle the error and recover.  It's a bit
rude for a single i2c driver to take down a whole system, right?

> +#define AXIOM_SIMPLE_FW_DEVICE_ATTR(attr)					\
> +	static ssize_t								\
> +	fw_ ## attr ## _show(struct device *dev,				\
> +			     struct device_attribute *_attr, char *buf)		\
> +	{									\
> +		struct i2c_client *i2c = to_i2c_client(dev);			\
> +		struct axiom_data *ts = i2c_get_clientdata(i2c);		\
> +										\
> +		return sprintf(buf, "%u\n", ts->fw_##attr);			\

sysfs_emit() please for all sysfs show functions.

> +	axiom_u42_get_touchslots(ts);
> +	if (!ts->num_slots && update_in_process) {
> +		input_free_device(input);
> +		/*
> +		 * Skip input device registration but don't throw an error to
> +		 * not abort the update since some FW updates require a
> +		 * following CFG update to re-initialize the touchslot handling.
> +		 */
> +		if (update_in_process) {
> +			dev_info(dev, "No touchslots found after FW or CFG update, skip registering input device\n");

Why is this info?  What can a user do with this?  Shouldn't this be a
dev_warn() call at the least?

> +			return 0;

You return success, but the device is NOT set up properly, how is that
going to work?

thanks,

greg k-h