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

On Tue, Aug 31, 2021 at 01:02:57AM +0200, Marek Vasut wrote:

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

[...]

quoted

quoted

@@ -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.

Usually we simply disable interrupts from the device when updating
firmware.

Thanks.

-- 
Dmitry