Re: [PATCH v2 2/3] Input: ili210x - export ili251x version details via sysfs

[Marek Vasut <marex@denx.de>]

On 8/30/21 11:01 PM, Dmitry Torokhov wrote:

[...]

>> @@ -351,6 +360,108 @@ static int ili251x_firmware_update_resolution(struct device *dev)
>>   	return 0;
>>   }
>>   
>> +static ssize_t ili251x_firmware_version_show(struct device *dev,
>> +					     struct device_attribute *attr,
>> +					     char *buf)
>> +{
>> +	struct i2c_client *client = to_i2c_client(dev);
>> +	struct ili210x *priv = i2c_get_clientdata(client);
>> +	u8 fw[8];
>> +	int ret;
>> +
>> +	/* Get firmware version */
>> +	mutex_lock(&priv->lock);
>> +	ret = priv->chip->read_reg(client, REG_FIRMWARE_VERSION,
>> +				   &fw, sizeof(fw));
>> +	mutex_unlock(&priv->lock);
> 
> Could we query firmware version and mode at probe time (and also later
> after firmware update attempt) so that we do not need to introduce
> locking against interrupt handler?

This is a threaded interrupt handler and I don't expect much lock 
contention here.

The sysfs attribute readout would race with the interrupt handler and if 
it wasn't for the firmware update support, we could very well cache all 
those values. Except, the firmware update can change them all. Worse, if 
the interrupt were to fire during an update, it could break that update, 
and I want to prevent that from happening.

[...]