[PATCH] Input: goodix-berlin - Add sysfs interface for reading and writing touch IC registers

[PATCH] Input: goodix-berlin - Add sysfs interface for reading and writing touch IC registers

[Charles Wang]

Export a sysfs interface that would allow reading and writing touchscreen
IC registers. With this interface many things can be done in usersapce
such as firmware updates. An example tool that utilizes this interface
for performing firmware updates can be found at [1].

[1] https://github.com/goodix/fwupdate_for_berlin_linux

Signed-off-by: Charles Wang <charles.goodix@gmail.com>
---
 drivers/input/touchscreen/goodix_berlin.h     |  2 +
 .../input/touchscreen/goodix_berlin_core.c    | 52 +++++++++++++++++++
 drivers/input/touchscreen/goodix_berlin_i2c.c |  6 +++
 drivers/input/touchscreen/goodix_berlin_spi.c |  6 +++
 4 files changed, 66 insertions(+)

diff --git a/drivers/input/touchscreen/goodix_berlin.h b/drivers/input/touchscreen/goodix_berlin.h
index 1fd77eb69..1741f2d15 100644
--- a/drivers/input/touchscreen/goodix_berlin.h
+++ b/drivers/input/touchscreen/goodix_berlin.h
@@ -19,6 +19,8 @@ struct regmap;
 int goodix_berlin_probe(struct device *dev, int irq, const struct input_id *id,
 			struct regmap *regmap);
 
+void goodix_berlin_remove(struct device *dev);
+
 extern const struct dev_pm_ops goodix_berlin_pm_ops;
 
 #endif
diff --git a/drivers/input/touchscreen/goodix_berlin_core.c b/drivers/input/touchscreen/goodix_berlin_core.c
index e7b41a926..e02160841 100644
--- a/drivers/input/touchscreen/goodix_berlin_core.c
+++ b/drivers/input/touchscreen/goodix_berlin_core.c
@@ -672,6 +672,44 @@ static void goodix_berlin_power_off_act(void *data)
 	goodix_berlin_power_off(cd);
 }
 
+static ssize_t goodix_berlin_registers_read(struct file *filp, struct kobject *kobj,
+	struct bin_attribute *bin_attr, char *buf, loff_t off, size_t count)
+{
+	struct goodix_berlin_core *cd;
+	struct device *dev;
+	int error;
+
+	dev = kobj_to_dev(kobj);
+	cd = dev_get_drvdata(dev);
+
+	error = regmap_raw_read(cd->regmap, (unsigned int)off,
+				buf, count);
+
+	return error ? error : count;
+}
+
+static ssize_t goodix_berlin_registers_write(struct file *filp, struct kobject *kobj,
+	struct bin_attribute *bin_attr, char *buf, loff_t off, size_t count)
+{
+	struct goodix_berlin_core *cd;
+	struct device *dev;
+	int error;
+
+	dev = kobj_to_dev(kobj);
+	cd = dev_get_drvdata(dev);
+
+	error = regmap_raw_write(cd->regmap, (unsigned int)off,
+				 buf, count);
+
+	return error ? error : count;
+}
+
+static struct bin_attribute goodix_berlin_registers_attr = {
+	.attr = {.name = "registers", .mode = 0600},
+	.read = goodix_berlin_registers_read,
+	.write = goodix_berlin_registers_write,
+};
+
 int goodix_berlin_probe(struct device *dev, int irq, const struct input_id *id,
 			struct regmap *regmap)
 {
@@ -743,6 +781,14 @@ int goodix_berlin_probe(struct device *dev, int irq, const struct input_id *id,
 
 	dev_set_drvdata(dev, cd);
 
+	error = sysfs_create_bin_file(&cd->dev->kobj,
+				      &goodix_berlin_registers_attr);
+
+	if (error) {
+		dev_err(dev, "unable to create sysfs file, err=%d\n", error);
+		return error;
+	}
+
 	dev_dbg(dev, "Goodix Berlin %s Touchscreen Controller",
 		cd->fw_version.patch_pid);
 
@@ -750,6 +796,12 @@ int goodix_berlin_probe(struct device *dev, int irq, const struct input_id *id,
 }
 EXPORT_SYMBOL_GPL(goodix_berlin_probe);
 
+void goodix_berlin_remove(struct device *dev)
+{
+	sysfs_remove_bin_file(&dev->kobj, &goodix_berlin_registers_attr);
+}
+EXPORT_SYMBOL_GPL(goodix_berlin_remove);
+
 MODULE_LICENSE("GPL");
 MODULE_DESCRIPTION("Goodix Berlin Core Touchscreen driver");
 MODULE_AUTHOR("Neil Armstrong <neil.armstrong@linaro.org>");
diff --git a/drivers/input/touchscreen/goodix_berlin_i2c.c b/drivers/input/touchscreen/goodix_berlin_i2c.c
index 6ed9aa808..35ef21cc8 100644
--- a/drivers/input/touchscreen/goodix_berlin_i2c.c
+++ b/drivers/input/touchscreen/goodix_berlin_i2c.c
@@ -46,6 +46,11 @@ static int goodix_berlin_i2c_probe(struct i2c_client *client)
 	return 0;
 }
 
+static void goodix_berlin_i2c_remove(struct i2c_client *client)
+{
+	goodix_berlin_remove(&client->dev);
+}
+
 static const struct i2c_device_id goodix_berlin_i2c_id[] = {
 	{ "gt9916", 0 },
 	{ }
@@ -66,6 +71,7 @@ static struct i2c_driver goodix_berlin_i2c_driver = {
 		.pm = pm_sleep_ptr(&goodix_berlin_pm_ops),
 	},
 	.probe = goodix_berlin_i2c_probe,
+	.remove = goodix_berlin_i2c_remove,
 	.id_table = goodix_berlin_i2c_id,
 };
 module_i2c_driver(goodix_berlin_i2c_driver);
diff --git a/drivers/input/touchscreen/goodix_berlin_spi.c b/drivers/input/touchscreen/goodix_berlin_spi.c
index 4cc557da0..8ffbe1289 100644
--- a/drivers/input/touchscreen/goodix_berlin_spi.c
+++ b/drivers/input/touchscreen/goodix_berlin_spi.c
@@ -150,6 +150,11 @@ static int goodix_berlin_spi_probe(struct spi_device *spi)
 	return 0;
 }
 
+static void goodix_berlin_spi_remove(struct spi_device *spi)
+{
+	goodix_berlin_remove(&spi->dev);
+}
+
 static const struct spi_device_id goodix_berlin_spi_ids[] = {
 	{ "gt9916" },
 	{ },
@@ -169,6 +174,7 @@ static struct spi_driver goodix_berlin_spi_driver = {
 		.pm = pm_sleep_ptr(&goodix_berlin_pm_ops),
 	},
 	.probe = goodix_berlin_spi_probe,
+	.remove = goodix_berlin_spi_remove,
 	.id_table = goodix_berlin_spi_ids,
 };
 module_spi_driver(goodix_berlin_spi_driver);
-- 
2.43.0

Re: [PATCH] Input: goodix-berlin - Add sysfs interface for reading and writing touch IC registers

[Hans de Goede]

Hi,

On 5/6/24 1:47 PM, Charles Wang wrote:
> Export a sysfs interface that would allow reading and writing touchscreen
> IC registers. With this interface many things can be done in usersapce
> such as firmware updates. An example tool that utilizes this interface
> for performing firmware updates can be found at [1].

I'm not sure if I'm a fan of adding an interface to export raw register
access for fwupdate.

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/input/touchscreen/goodix_fwupload.c

Contains update support for older goodix touchscreens and it is not
that big. I think it might be better to just have an in kernel fwupdate
driver for this and have a sysfs file to which to write the new firmware.

Adding Richard Hughes, fwupd maintainer to the Cc. Richard, do you have
any input on the suggested method for firmware updating ?

If raw register access is seen as a good solution, then I think this
should use regmap + some generic helpers (to be written) to export
regmap r/w access to userspace.

> [1] https://github.com/goodix/fwupdate_for_berlin_linux

Hmm, that tool seems to have some licensing issues there is an Apache License v2.0
LICENSE file, but the header of fwupdate.c claims it is confidential ...

Regards,

Hans


> Signed-off-by: Charles Wang <charles.goodix@gmail.com>
> ---
>  drivers/input/touchscreen/goodix_berlin.h     |  2 +
>  .../input/touchscreen/goodix_berlin_core.c    | 52 +++++++++++++++++++
>  drivers/input/touchscreen/goodix_berlin_i2c.c |  6 +++
>  drivers/input/touchscreen/goodix_berlin_spi.c |  6 +++
>  4 files changed, 66 insertions(+)
> 
> diff --git a/drivers/input/touchscreen/goodix_berlin.h b/drivers/input/touchscreen/goodix_berlin.h
> index 1fd77eb69..1741f2d15 100644
> --- a/drivers/input/touchscreen/goodix_berlin.h
> +++ b/drivers/input/touchscreen/goodix_berlin.h
> @@ -19,6 +19,8 @@ struct regmap;
>  int goodix_berlin_probe(struct device *dev, int irq, const struct input_id *id,
>  			struct regmap *regmap);
>  
> +void goodix_berlin_remove(struct device *dev);
> +
>  extern const struct dev_pm_ops goodix_berlin_pm_ops;
>  
>  #endif
> diff --git a/drivers/input/touchscreen/goodix_berlin_core.c b/drivers/input/touchscreen/goodix_berlin_core.c
> index e7b41a926..e02160841 100644
> --- a/drivers/input/touchscreen/goodix_berlin_core.c
> +++ b/drivers/input/touchscreen/goodix_berlin_core.c
> @@ -672,6 +672,44 @@ static void goodix_berlin_power_off_act(void *data)
>  	goodix_berlin_power_off(cd);
>  }
>  
> +static ssize_t goodix_berlin_registers_read(struct file *filp, struct kobject *kobj,
> +	struct bin_attribute *bin_attr, char *buf, loff_t off, size_t count)
> +{
> +	struct goodix_berlin_core *cd;
> +	struct device *dev;
> +	int error;
> +
> +	dev = kobj_to_dev(kobj);
> +	cd = dev_get_drvdata(dev);
> +
> +	error = regmap_raw_read(cd->regmap, (unsigned int)off,
> +				buf, count);
> +
> +	return error ? error : count;
> +}
> +
> +static ssize_t goodix_berlin_registers_write(struct file *filp, struct kobject *kobj,
> +	struct bin_attribute *bin_attr, char *buf, loff_t off, size_t count)
> +{
> +	struct goodix_berlin_core *cd;
> +	struct device *dev;
> +	int error;
> +
> +	dev = kobj_to_dev(kobj);
> +	cd = dev_get_drvdata(dev);
> +
> +	error = regmap_raw_write(cd->regmap, (unsigned int)off,
> +				 buf, count);
> +
> +	return error ? error : count;
> +}
> +
> +static struct bin_attribute goodix_berlin_registers_attr = {
> +	.attr = {.name = "registers", .mode = 0600},
> +	.read = goodix_berlin_registers_read,
> +	.write = goodix_berlin_registers_write,
> +};
> +
>  int goodix_berlin_probe(struct device *dev, int irq, const struct input_id *id,
>  			struct regmap *regmap)
>  {
> @@ -743,6 +781,14 @@ int goodix_berlin_probe(struct device *dev, int irq, const struct input_id *id,
>  
>  	dev_set_drvdata(dev, cd);
>  
> +	error = sysfs_create_bin_file(&cd->dev->kobj,
> +				      &goodix_berlin_registers_attr);
> +
> +	if (error) {
> +		dev_err(dev, "unable to create sysfs file, err=%d\n", error);
> +		return error;
> +	}
> +
>  	dev_dbg(dev, "Goodix Berlin %s Touchscreen Controller",
>  		cd->fw_version.patch_pid);
>  
> @@ -750,6 +796,12 @@ int goodix_berlin_probe(struct device *dev, int irq, const struct input_id *id,
>  }
>  EXPORT_SYMBOL_GPL(goodix_berlin_probe);
>  
> +void goodix_berlin_remove(struct device *dev)
> +{
> +	sysfs_remove_bin_file(&dev->kobj, &goodix_berlin_registers_attr);
> +}
> +EXPORT_SYMBOL_GPL(goodix_berlin_remove);
> +
>  MODULE_LICENSE("GPL");
>  MODULE_DESCRIPTION("Goodix Berlin Core Touchscreen driver");
>  MODULE_AUTHOR("Neil Armstrong <neil.armstrong@linaro.org>");
> diff --git a/drivers/input/touchscreen/goodix_berlin_i2c.c b/drivers/input/touchscreen/goodix_berlin_i2c.c
> index 6ed9aa808..35ef21cc8 100644
> --- a/drivers/input/touchscreen/goodix_berlin_i2c.c
> +++ b/drivers/input/touchscreen/goodix_berlin_i2c.c
> @@ -46,6 +46,11 @@ static int goodix_berlin_i2c_probe(struct i2c_client *client)
>  	return 0;
>  }
>  
> +static void goodix_berlin_i2c_remove(struct i2c_client *client)
> +{
> +	goodix_berlin_remove(&client->dev);
> +}
> +
>  static const struct i2c_device_id goodix_berlin_i2c_id[] = {
>  	{ "gt9916", 0 },
>  	{ }
> @@ -66,6 +71,7 @@ static struct i2c_driver goodix_berlin_i2c_driver = {
>  		.pm = pm_sleep_ptr(&goodix_berlin_pm_ops),
>  	},
>  	.probe = goodix_berlin_i2c_probe,
> +	.remove = goodix_berlin_i2c_remove,
>  	.id_table = goodix_berlin_i2c_id,
>  };
>  module_i2c_driver(goodix_berlin_i2c_driver);
> diff --git a/drivers/input/touchscreen/goodix_berlin_spi.c b/drivers/input/touchscreen/goodix_berlin_spi.c
> index 4cc557da0..8ffbe1289 100644
> --- a/drivers/input/touchscreen/goodix_berlin_spi.c
> +++ b/drivers/input/touchscreen/goodix_berlin_spi.c
> @@ -150,6 +150,11 @@ static int goodix_berlin_spi_probe(struct spi_device *spi)
>  	return 0;
>  }
>  
> +static void goodix_berlin_spi_remove(struct spi_device *spi)
> +{
> +	goodix_berlin_remove(&spi->dev);
> +}
> +
>  static const struct spi_device_id goodix_berlin_spi_ids[] = {
>  	{ "gt9916" },
>  	{ },
> @@ -169,6 +174,7 @@ static struct spi_driver goodix_berlin_spi_driver = {
>  		.pm = pm_sleep_ptr(&goodix_berlin_pm_ops),
>  	},
>  	.probe = goodix_berlin_spi_probe,
> +	.remove = goodix_berlin_spi_remove,
>  	.id_table = goodix_berlin_spi_ids,
>  };
>  module_spi_driver(goodix_berlin_spi_driver);

Re: [PATCH] Input: goodix-berlin - Add sysfs interface for reading and writing touch IC registers

[Richard Hughes]

On Mon, 6 May 2024 at 13:03, Hans de Goede <hdegoede@redhat.com> wrote:
> Adding Richard Hughes, fwupd maintainer to the Cc. Richard, do you have
> any input on the suggested method for firmware updating ?

I'm okay with either; it's obviously easier for fwupd to just squirt a
file into a sysfs file to update the firmware, but some devices also
need something a bit more nuanced -- e.g. putting the device into an
update mode, setting the power domains, fallback recovery and stuff
like that.

Richard

Re: [PATCH] Input: goodix-berlin - Add sysfs interface for reading and writing touch IC registers

[Dmitry Torokhov]

On Mon, May 06, 2024 at 02:03:13PM +0200, Hans de Goede wrote:
> Hi,
> 
> On 5/6/24 1:47 PM, Charles Wang wrote:
> > Export a sysfs interface that would allow reading and writing touchscreen
> > IC registers. With this interface many things can be done in usersapce
> > such as firmware updates. An example tool that utilizes this interface
> > for performing firmware updates can be found at [1].
> 
> I'm not sure if I'm a fan of adding an interface to export raw register
> access for fwupdate.
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/input/touchscreen/goodix_fwupload.c
> 
> Contains update support for older goodix touchscreens and it is not
> that big. I think it might be better to just have an in kernel fwupdate
> driver for this and have a sysfs file to which to write the new firmware.
> 
> Adding Richard Hughes, fwupd maintainer to the Cc. Richard, do you have
> any input on the suggested method for firmware updating ?
> 
> If raw register access is seen as a good solution, then I think this
> should use regmap + some generic helpers (to be written) to export
> regmap r/w access to userspace.

I think the less code we have in kernel the better, especially if in
cases where firmware flashing is not essential for device to work (i.e.
it the controller has a flash memory). That said IIRC Mark felt very
strongly about allowing regmap writes from userspace... but maybe he
softened the stance or we could have this functionality opt-in?

> 
> > [1] https://github.com/goodix/fwupdate_for_berlin_linux
> 
> Hmm, that tool seems to have some licensing issues there is an Apache License v2.0
> LICENSE file, but the header of fwupdate.c claims it is confidential ...
> 
> Regards,
> 
> Hans
> 
> 
> > Signed-off-by: Charles Wang <charles.goodix@gmail.com>
> > ---
> >  drivers/input/touchscreen/goodix_berlin.h     |  2 +
> >  .../input/touchscreen/goodix_berlin_core.c    | 52 +++++++++++++++++++
> >  drivers/input/touchscreen/goodix_berlin_i2c.c |  6 +++
> >  drivers/input/touchscreen/goodix_berlin_spi.c |  6 +++
> >  4 files changed, 66 insertions(+)
> > 
> > diff --git a/drivers/input/touchscreen/goodix_berlin.h b/drivers/input/touchscreen/goodix_berlin.h
> > index 1fd77eb69..1741f2d15 100644
> > --- a/drivers/input/touchscreen/goodix_berlin.h
> > +++ b/drivers/input/touchscreen/goodix_berlin.h
> > @@ -19,6 +19,8 @@ struct regmap;
> >  int goodix_berlin_probe(struct device *dev, int irq, const struct input_id *id,
> >  			struct regmap *regmap);
> >  
> > +void goodix_berlin_remove(struct device *dev);
> > +
> >  extern const struct dev_pm_ops goodix_berlin_pm_ops;
> >  
> >  #endif
> > diff --git a/drivers/input/touchscreen/goodix_berlin_core.c b/drivers/input/touchscreen/goodix_berlin_core.c
> > index e7b41a926..e02160841 100644
> > --- a/drivers/input/touchscreen/goodix_berlin_core.c
> > +++ b/drivers/input/touchscreen/goodix_berlin_core.c
> > @@ -672,6 +672,44 @@ static void goodix_berlin_power_off_act(void *data)
> >  	goodix_berlin_power_off(cd);
> >  }
> >  
> > +static ssize_t goodix_berlin_registers_read(struct file *filp, struct kobject *kobj,
> > +	struct bin_attribute *bin_attr, char *buf, loff_t off, size_t count)
> > +{
> > +	struct goodix_berlin_core *cd;
> > +	struct device *dev;
> > +	int error;
> > +
> > +	dev = kobj_to_dev(kobj);
> > +	cd = dev_get_drvdata(dev);
> > +
> > +	error = regmap_raw_read(cd->regmap, (unsigned int)off,
> > +				buf, count);
> > +
> > +	return error ? error : count;
> > +}
> > +
> > +static ssize_t goodix_berlin_registers_write(struct file *filp, struct kobject *kobj,
> > +	struct bin_attribute *bin_attr, char *buf, loff_t off, size_t count)
> > +{
> > +	struct goodix_berlin_core *cd;
> > +	struct device *dev;
> > +	int error;
> > +
> > +	dev = kobj_to_dev(kobj);
> > +	cd = dev_get_drvdata(dev);
> > +
> > +	error = regmap_raw_write(cd->regmap, (unsigned int)off,
> > +				 buf, count);
> > +
> > +	return error ? error : count;
> > +}
> > +
> > +static struct bin_attribute goodix_berlin_registers_attr = {
> > +	.attr = {.name = "registers", .mode = 0600},
> > +	.read = goodix_berlin_registers_read,
> > +	.write = goodix_berlin_registers_write,
> > +};
> > +
> >  int goodix_berlin_probe(struct device *dev, int irq, const struct input_id *id,
> >  			struct regmap *regmap)
> >  {
> > @@ -743,6 +781,14 @@ int goodix_berlin_probe(struct device *dev, int irq, const struct input_id *id,
> >  
> >  	dev_set_drvdata(dev, cd);
> >  
> > +	error = sysfs_create_bin_file(&cd->dev->kobj,
> > +				      &goodix_berlin_registers_attr);

If we want to instantiate attributes from the driver please utilize
dev_groups in respective driver structures to create and remove the
attributes automatically.

Thanks.

-- 
Dmitry

Re: [PATCH] Input: goodix-berlin - Add sysfs interface for reading and writing touch IC registers

[neil.armstrong]

On 07/05/2024 04:13, Dmitry Torokhov wrote:
> On Mon, May 06, 2024 at 02:03:13PM +0200, Hans de Goede wrote:
>> Hi,
>>
>> On 5/6/24 1:47 PM, Charles Wang wrote:
>>> Export a sysfs interface that would allow reading and writing touchscreen
>>> IC registers. With this interface many things can be done in usersapce
>>> such as firmware updates. An example tool that utilizes this interface
>>> for performing firmware updates can be found at [1].
>>
>> I'm not sure if I'm a fan of adding an interface to export raw register
>> access for fwupdate.
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/input/touchscreen/goodix_fwupload.c
>>
>> Contains update support for older goodix touchscreens and it is not
>> that big. I think it might be better to just have an in kernel fwupdate
>> driver for this and have a sysfs file to which to write the new firmware.
>>
>> Adding Richard Hughes, fwupd maintainer to the Cc. Richard, do you have
>> any input on the suggested method for firmware updating ?
>>
>> If raw register access is seen as a good solution, then I think this
>> should use regmap + some generic helpers (to be written) to export
>> regmap r/w access to userspace.
> 
> I think the less code we have in kernel the better, especially if in
> cases where firmware flashing is not essential for device to work (i.e.
> it the controller has a flash memory). That said IIRC Mark felt very
> strongly about allowing regmap writes from userspace... but maybe he
> softened the stance or we could have this functionality opt-in?

The update code is quite ugly, but we could probably limit the functionnality
and filtering the registers read/write to only the update registers.

Neil

> 
>>
>>> [1] https://github.com/goodix/fwupdate_for_berlin_linux
>>
>> Hmm, that tool seems to have some licensing issues there is an Apache License v2.0
>> LICENSE file, but the header of fwupdate.c claims it is confidential ...
>>
>> Regards,
>>
>> Hans
>>
>>
>>> Signed-off-by: Charles Wang <charles.goodix@gmail.com>
>>> ---
>>>   drivers/input/touchscreen/goodix_berlin.h     |  2 +
>>>   .../input/touchscreen/goodix_berlin_core.c    | 52 +++++++++++++++++++
>>>   drivers/input/touchscreen/goodix_berlin_i2c.c |  6 +++
>>>   drivers/input/touchscreen/goodix_berlin_spi.c |  6 +++
>>>   4 files changed, 66 insertions(+)
>>>
>>> diff --git a/drivers/input/touchscreen/goodix_berlin.h b/drivers/input/touchscreen/goodix_berlin.h
>>> index 1fd77eb69..1741f2d15 100644
>>> --- a/drivers/input/touchscreen/goodix_berlin.h
>>> +++ b/drivers/input/touchscreen/goodix_berlin.h
>>> @@ -19,6 +19,8 @@ struct regmap;
>>>   int goodix_berlin_probe(struct device *dev, int irq, const struct input_id *id,
>>>   			struct regmap *regmap);
>>>   
>>> +void goodix_berlin_remove(struct device *dev);
>>> +
>>>   extern const struct dev_pm_ops goodix_berlin_pm_ops;
>>>   
>>>   #endif
>>> diff --git a/drivers/input/touchscreen/goodix_berlin_core.c b/drivers/input/touchscreen/goodix_berlin_core.c
>>> index e7b41a926..e02160841 100644
>>> --- a/drivers/input/touchscreen/goodix_berlin_core.c
>>> +++ b/drivers/input/touchscreen/goodix_berlin_core.c
>>> @@ -672,6 +672,44 @@ static void goodix_berlin_power_off_act(void *data)
>>>   	goodix_berlin_power_off(cd);
>>>   }
>>>   
>>> +static ssize_t goodix_berlin_registers_read(struct file *filp, struct kobject *kobj,
>>> +	struct bin_attribute *bin_attr, char *buf, loff_t off, size_t count)
>>> +{
>>> +	struct goodix_berlin_core *cd;
>>> +	struct device *dev;
>>> +	int error;
>>> +
>>> +	dev = kobj_to_dev(kobj);
>>> +	cd = dev_get_drvdata(dev);
>>> +
>>> +	error = regmap_raw_read(cd->regmap, (unsigned int)off,
>>> +				buf, count);
>>> +
>>> +	return error ? error : count;
>>> +}
>>> +
>>> +static ssize_t goodix_berlin_registers_write(struct file *filp, struct kobject *kobj,
>>> +	struct bin_attribute *bin_attr, char *buf, loff_t off, size_t count)
>>> +{
>>> +	struct goodix_berlin_core *cd;
>>> +	struct device *dev;
>>> +	int error;
>>> +
>>> +	dev = kobj_to_dev(kobj);
>>> +	cd = dev_get_drvdata(dev);
>>> +
>>> +	error = regmap_raw_write(cd->regmap, (unsigned int)off,
>>> +				 buf, count);
>>> +
>>> +	return error ? error : count;
>>> +}
>>> +
>>> +static struct bin_attribute goodix_berlin_registers_attr = {
>>> +	.attr = {.name = "registers", .mode = 0600},
>>> +	.read = goodix_berlin_registers_read,
>>> +	.write = goodix_berlin_registers_write,
>>> +};
>>> +
>>>   int goodix_berlin_probe(struct device *dev, int irq, const struct input_id *id,
>>>   			struct regmap *regmap)
>>>   {
>>> @@ -743,6 +781,14 @@ int goodix_berlin_probe(struct device *dev, int irq, const struct input_id *id,
>>>   
>>>   	dev_set_drvdata(dev, cd);
>>>   
>>> +	error = sysfs_create_bin_file(&cd->dev->kobj,
>>> +				      &goodix_berlin_registers_attr);
> 
> If we want to instantiate attributes from the driver please utilize
> dev_groups in respective driver structures to create and remove the
> attributes automatically.
> 
> Thanks.
> 

Re: [PATCH] Input: goodix-berlin - Add sysfs interface for reading and writing touch IC registers

[Hans de Goede]

Hi,

On 5/7/24 4:13 AM, Dmitry Torokhov wrote:
> On Mon, May 06, 2024 at 02:03:13PM +0200, Hans de Goede wrote:
>> Hi,
>>
>> On 5/6/24 1:47 PM, Charles Wang wrote:
>>> Export a sysfs interface that would allow reading and writing touchscreen
>>> IC registers. With this interface many things can be done in usersapce
>>> such as firmware updates. An example tool that utilizes this interface
>>> for performing firmware updates can be found at [1].
>>
>> I'm not sure if I'm a fan of adding an interface to export raw register
>> access for fwupdate.
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/input/touchscreen/goodix_fwupload.c
>>
>> Contains update support for older goodix touchscreens and it is not
>> that big. I think it might be better to just have an in kernel fwupdate
>> driver for this and have a sysfs file to which to write the new firmware.
>>
>> Adding Richard Hughes, fwupd maintainer to the Cc. Richard, do you have
>> any input on the suggested method for firmware updating ?
>>
>> If raw register access is seen as a good solution, then I think this
>> should use regmap + some generic helpers (to be written) to export
>> regmap r/w access to userspace.
> 
> I think the less code we have in kernel the better,

Ok.

> especially if in
> cases where firmware flashing is not essential for device to work (i.e.
> it the controller has a flash memory).

Right the existing older goodix fw-upload is different because some
controllers are flash-less so they need a fw upload every boot.

> That said IIRC Mark felt very
> strongly about allowing regmap writes from userspace... but maybe he
> softened the stance or we could have this functionality opt-in?

Right when I was talking about generic helpers that was meant for
code re-use purposes. Actually exposing the regmap r/w functionality
to userspace is something which should be decided on a case by case
basis by the driver (IMHO).

Regards,

Hans



> 
>>
>>> [1] https://github.com/goodix/fwupdate_for_berlin_linux
>>
>> Hmm, that tool seems to have some licensing issues there is an Apache License v2.0
>> LICENSE file, but the header of fwupdate.c claims it is confidential ...
>>
>> Regards,
>>
>> Hans
>>
>>
>>> Signed-off-by: Charles Wang <charles.goodix@gmail.com>
>>> ---
>>>  drivers/input/touchscreen/goodix_berlin.h     |  2 +
>>>  .../input/touchscreen/goodix_berlin_core.c    | 52 +++++++++++++++++++
>>>  drivers/input/touchscreen/goodix_berlin_i2c.c |  6 +++
>>>  drivers/input/touchscreen/goodix_berlin_spi.c |  6 +++
>>>  4 files changed, 66 insertions(+)
>>>
>>> diff --git a/drivers/input/touchscreen/goodix_berlin.h b/drivers/input/touchscreen/goodix_berlin.h
>>> index 1fd77eb69..1741f2d15 100644
>>> --- a/drivers/input/touchscreen/goodix_berlin.h
>>> +++ b/drivers/input/touchscreen/goodix_berlin.h
>>> @@ -19,6 +19,8 @@ struct regmap;
>>>  int goodix_berlin_probe(struct device *dev, int irq, const struct input_id *id,
>>>  			struct regmap *regmap);
>>>  
>>> +void goodix_berlin_remove(struct device *dev);
>>> +
>>>  extern const struct dev_pm_ops goodix_berlin_pm_ops;
>>>  
>>>  #endif
>>> diff --git a/drivers/input/touchscreen/goodix_berlin_core.c b/drivers/input/touchscreen/goodix_berlin_core.c
>>> index e7b41a926..e02160841 100644
>>> --- a/drivers/input/touchscreen/goodix_berlin_core.c
>>> +++ b/drivers/input/touchscreen/goodix_berlin_core.c
>>> @@ -672,6 +672,44 @@ static void goodix_berlin_power_off_act(void *data)
>>>  	goodix_berlin_power_off(cd);
>>>  }
>>>  
>>> +static ssize_t goodix_berlin_registers_read(struct file *filp, struct kobject *kobj,
>>> +	struct bin_attribute *bin_attr, char *buf, loff_t off, size_t count)
>>> +{
>>> +	struct goodix_berlin_core *cd;
>>> +	struct device *dev;
>>> +	int error;
>>> +
>>> +	dev = kobj_to_dev(kobj);
>>> +	cd = dev_get_drvdata(dev);
>>> +
>>> +	error = regmap_raw_read(cd->regmap, (unsigned int)off,
>>> +				buf, count);
>>> +
>>> +	return error ? error : count;
>>> +}
>>> +
>>> +static ssize_t goodix_berlin_registers_write(struct file *filp, struct kobject *kobj,
>>> +	struct bin_attribute *bin_attr, char *buf, loff_t off, size_t count)
>>> +{
>>> +	struct goodix_berlin_core *cd;
>>> +	struct device *dev;
>>> +	int error;
>>> +
>>> +	dev = kobj_to_dev(kobj);
>>> +	cd = dev_get_drvdata(dev);
>>> +
>>> +	error = regmap_raw_write(cd->regmap, (unsigned int)off,
>>> +				 buf, count);
>>> +
>>> +	return error ? error : count;
>>> +}
>>> +
>>> +static struct bin_attribute goodix_berlin_registers_attr = {
>>> +	.attr = {.name = "registers", .mode = 0600},
>>> +	.read = goodix_berlin_registers_read,
>>> +	.write = goodix_berlin_registers_write,
>>> +};
>>> +
>>>  int goodix_berlin_probe(struct device *dev, int irq, const struct input_id *id,
>>>  			struct regmap *regmap)
>>>  {
>>> @@ -743,6 +781,14 @@ int goodix_berlin_probe(struct device *dev, int irq, const struct input_id *id,
>>>  
>>>  	dev_set_drvdata(dev, cd);
>>>  
>>> +	error = sysfs_create_bin_file(&cd->dev->kobj,
>>> +				      &goodix_berlin_registers_attr);
> 
> If we want to instantiate attributes from the driver please utilize
> dev_groups in respective driver structures to create and remove the
> attributes automatically.
> 
> Thanks.
> 

Re: [PATCH] Input: goodix-berlin - Add sysfs interface for reading and writing touch IC registers

[Charles Wang]

Hi,

On Mon, May 06, 2024 at 02:03:13PM +0200, Hans de Goede wrote:
> > [1] https://github.com/goodix/fwupdate_for_berlin_linux
> 
> Hmm, that tool seems to have some licensing issues there is an Apache License v2.0
> LICENSE file, but the header of fwupdate.c claims it is confidential ...

Thanks for pointing that out. The confidential claims has been removed from fwupdate.c
in the latest commits.

Thanks,

Charles

Re: [PATCH] Input: goodix-berlin - Add sysfs interface for reading and writing touch IC registers

[Charles Wang]

Hi,

On Mon, May 06, 2024 at 07:13:38PM -0700, Dmitry Torokhov wrote:
> If we want to instantiate attributes from the driver please utilize
> dev_groups in respective driver structures to create and remove the
> attributes automatically.

Ack

Thanks,

Charles

Re: [PATCH] Input: goodix-berlin - Add sysfs interface for reading and writing touch IC registers

[Hans de Goede]

Hi,

On 5/7/24 2:28 PM, Charles Wang wrote:
> Hi,
> 
> On Tue, May 07, 2024 at 10:25:29AM +0200, Hans de Goede wrote:
>> Hi,
>>
>> On 5/7/24 4:13 AM, Dmitry Torokhov wrote:
>>> On Mon, May 06, 2024 at 02:03:13PM +0200, Hans de Goede wrote:
>>>> Hi,
>>>>
>>>> On 5/6/24 1:47 PM, Charles Wang wrote:
>>>>> Export a sysfs interface that would allow reading and writing touchscreen
>>>>> IC registers. With this interface many things can be done in usersapce
>>>>> such as firmware updates. An example tool that utilizes this interface
>>>>> for performing firmware updates can be found at [1].
>>>>
>>>> I'm not sure if I'm a fan of adding an interface to export raw register
>>>> access for fwupdate.
>>>>
>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/input/touchscreen/goodix_fwupload.c
>>>>
>>>> Contains update support for older goodix touchscreens and it is not
>>>> that big. I think it might be better to just have an in kernel fwupdate
>>>> driver for this and have a sysfs file to which to write the new firmware.
>>>>
>>>> Adding Richard Hughes, fwupd maintainer to the Cc. Richard, do you have
>>>> any input on the suggested method for firmware updating ?
>>>>
>>>> If raw register access is seen as a good solution, then I think this
>>>> should use regmap + some generic helpers (to be written) to export
>>>> regmap r/w access to userspace.
>>>
>>> I think the less code we have in kernel the better,
>>
>> Ok.
>>
>>> especially if in
>>> cases where firmware flashing is not essential for device to work (i.e.
>>> it the controller has a flash memory).
>>
>> Right the existing older goodix fw-upload is different because some
>> controllers are flash-less so they need a fw upload every boot.
>>
>>> That said IIRC Mark felt very
>>> strongly about allowing regmap writes from userspace... but maybe he
>>> softened the stance or we could have this functionality opt-in?
>>
>> Right when I was talking about generic helpers that was meant for
>> code re-use purposes. Actually exposing the regmap r/w functionality
>> to userspace is something which should be decided on a case by case
>> basis by the driver (IMHO).
> 
> So what's the final conclusion, does the interface need to be modified?

I believe that the final conclusion is that the interface is fine.

Personally I think if the syfs store/show functions used for this
could be made into generic regmap helpers and then use those helpers
in the driver, so that if other drivers want similar functionality
they can re-use the show / store functions.

You should be able to use dev_get_regmap() to make the show/store
functions generic.

Regards,

Hans





>>>>> ---
>>>>>  drivers/input/touchscreen/goodix_berlin.h     |  2 +
>>>>>  .../input/touchscreen/goodix_berlin_core.c    | 52 +++++++++++++++++++
>>>>>  drivers/input/touchscreen/goodix_berlin_i2c.c |  6 +++
>>>>>  drivers/input/touchscreen/goodix_berlin_spi.c |  6 +++
>>>>>  4 files changed, 66 insertions(+)
>>>>>
>>>>> diff --git a/drivers/input/touchscreen/goodix_berlin.h b/drivers/input/touchscreen/goodix_berlin.h
>>>>> index 1fd77eb69..1741f2d15 100644
>>>>> --- a/drivers/input/touchscreen/goodix_berlin.h
>>>>> +++ b/drivers/input/touchscreen/goodix_berlin.h
>>>>> @@ -19,6 +19,8 @@ struct regmap;
>>>>>  int goodix_berlin_probe(struct device *dev, int irq, const struct input_id *id,
>>>>>  			struct regmap *regmap);
>>>>>  
>>>>> +void goodix_berlin_remove(struct device *dev);
>>>>> +
>>>>>  extern const struct dev_pm_ops goodix_berlin_pm_ops;
>>>>>  
>>>>>  #endif
>>>>> diff --git a/drivers/input/touchscreen/goodix_berlin_core.c b/drivers/input/touchscreen/goodix_berlin_core.c
>>>>> index e7b41a926..e02160841 100644
>>>>> --- a/drivers/input/touchscreen/goodix_berlin_core.c
>>>>> +++ b/drivers/input/touchscreen/goodix_berlin_core.c
>>>>> @@ -672,6 +672,44 @@ static void goodix_berlin_power_off_act(void *data)
>>>>>  	goodix_berlin_power_off(cd);
>>>>>  }
>>>>>  
>>>>> +static ssize_t goodix_berlin_registers_read(struct file *filp, struct kobject *kobj,
>>>>> +	struct bin_attribute *bin_attr, char *buf, loff_t off, size_t count)
>>>>> +{
>>>>> +	struct goodix_berlin_core *cd;
>>>>> +	struct device *dev;
>>>>> +	int error;
>>>>> +
>>>>> +	dev = kobj_to_dev(kobj);
>>>>> +	cd = dev_get_drvdata(dev);
>>>>> +
>>>>> +	error = regmap_raw_read(cd->regmap, (unsigned int)off,
>>>>> +				buf, count);
>>>>> +
>>>>> +	return error ? error : count;
>>>>> +}
>>>>> +
>>>>> +static ssize_t goodix_berlin_registers_write(struct file *filp, struct kobject *kobj,
>>>>> +	struct bin_attribute *bin_attr, char *buf, loff_t off, size_t count)
>>>>> +{
>>>>> +	struct goodix_berlin_core *cd;
>>>>> +	struct device *dev;
>>>>> +	int error;
>>>>> +
>>>>> +	dev = kobj_to_dev(kobj);
>>>>> +	cd = dev_get_drvdata(dev);
>>>>> +
>>>>> +	error = regmap_raw_write(cd->regmap, (unsigned int)off,
>>>>> +				 buf, count);
>>>>> +
>>>>> +	return error ? error : count;
>>>>> +}
>>>>> +
>>>>> +static struct bin_attribute goodix_berlin_registers_attr = {
>>>>> +	.attr = {.name = "registers", .mode = 0600},
>>>>> +	.read = goodix_berlin_registers_read,
>>>>> +	.write = goodix_berlin_registers_write,
>>>>> +};
>>>>> +
>>>>>  int goodix_berlin_probe(struct device *dev, int irq, const struct input_id *id,
>>>>>  			struct regmap *regmap)
>>>>>  {
>>>>> @@ -743,6 +781,14 @@ int goodix_berlin_probe(struct device *dev, int irq, const struct input_id *id,
>>>>>  
>>>>>  	dev_set_drvdata(dev, cd);
>>>>>  
>>>>> +	error = sysfs_create_bin_file(&cd->dev->kobj,
>>>>> +				      &goodix_berlin_registers_attr);
>>>
>>> If we want to instantiate attributes from the driver please utilize
>>> dev_groups in respective driver structures to create and remove the
>>> attributes automatically.
>>>
>>> Thanks.
>>>
>>
> 

Re: [PATCH] Input: goodix-berlin - Add sysfs interface for reading and writing touch IC registers

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1323 bytes --]

On Mon, May 06, 2024 at 07:13:38PM -0700, Dmitry Torokhov wrote:
> On Mon, May 06, 2024 at 02:03:13PM +0200, Hans de Goede wrote:

> > If raw register access is seen as a good solution, then I think this
> > should use regmap + some generic helpers (to be written) to export
> > regmap r/w access to userspace.

> I think the less code we have in kernel the better, especially if in
> cases where firmware flashing is not essential for device to work (i.e.
> it the controller has a flash memory). That said IIRC Mark felt very
> strongly about allowing regmap writes from userspace... but maybe he
> softened the stance or we could have this functionality opt-in?

I think unmediated raw register access is a terrible idea, you can't
safely write a driver if userspace can just go in and randomly write to
registers with no coordination with the running driver and for some
devices the kernel needs to ensure that any writes don't damage or
destabalise the system.  If a driver provides an interface that looks
like raw register accesses that's of course fine (I mean, a lot of
firmware formats basically boil down to register write sequences which
is clearly fine) but it should be the driver doing that and it should be
looking at what's going on and ensure that it's joined up with the needs
of the rest of the system.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] Input: goodix-berlin - Add sysfs interface for reading and writing touch IC registers

[Jeff LaBundy]

Hi all,

On Tue, May 07, 2024 at 11:36:41PM +0900, Mark Brown wrote:
> On Mon, May 06, 2024 at 07:13:38PM -0700, Dmitry Torokhov wrote:
> > On Mon, May 06, 2024 at 02:03:13PM +0200, Hans de Goede wrote:
> 
> > > If raw register access is seen as a good solution, then I think this
> > > should use regmap + some generic helpers (to be written) to export
> > > regmap r/w access to userspace.
> 
> > I think the less code we have in kernel the better, especially if in
> > cases where firmware flashing is not essential for device to work (i.e.
> > it the controller has a flash memory). That said IIRC Mark felt very
> > strongly about allowing regmap writes from userspace... but maybe he
> > softened the stance or we could have this functionality opt-in?
> 
> I think unmediated raw register access is a terrible idea, you can't
> safely write a driver if userspace can just go in and randomly write to
> registers with no coordination with the running driver and for some
> devices the kernel needs to ensure that any writes don't damage or
> destabalise the system.  If a driver provides an interface that looks
> like raw register accesses that's of course fine (I mean, a lot of
> firmware formats basically boil down to register write sequences which
> is clearly fine) but it should be the driver doing that and it should be
> looking at what's going on and ensure that it's joined up with the needs
> of the rest of the system.

I happen to agree here; especially in the case of writing new FW to a
flash; this is a very hardware-centric and device-specific function,
which by definition belongs in a kernel driver.

For example, many devices must be placed in a bootloader mode during
the FW update, and may clamp or toggle an interrupt pin during this
mode switch. If user space initiates this sequence while the driver is
unaware of this process, it may attempt to read status registers from
an I2C address that is temporarily offline.

A much more common design pattern is for the driver to expose one W/O
sysfs attribute for accepting the FW file name, and one R/O attribute
for displaying the current FW version in flash. A small script runs at
start-up to check the version against what is stored on "disk", and if
what is stored in flash is deemed out of date, the script writes to the
W/O attribute. This is the extent of user space's involvement.

Kind regards,
Jeff LaBundy

Re: [PATCH] Input: goodix-berlin - Add sysfs interface for reading and writing touch IC registers

[Dmitry Torokhov]

On Tue, May 07, 2024 at 10:14:28AM -0500, Jeff LaBundy wrote:
> Hi all,
> 
> On Tue, May 07, 2024 at 11:36:41PM +0900, Mark Brown wrote:
> > On Mon, May 06, 2024 at 07:13:38PM -0700, Dmitry Torokhov wrote:
> > > On Mon, May 06, 2024 at 02:03:13PM +0200, Hans de Goede wrote:
> > 
> > > > If raw register access is seen as a good solution, then I think this
> > > > should use regmap + some generic helpers (to be written) to export
> > > > regmap r/w access to userspace.
> > 
> > > I think the less code we have in kernel the better, especially if in
> > > cases where firmware flashing is not essential for device to work (i.e.
> > > it the controller has a flash memory). That said IIRC Mark felt very
> > > strongly about allowing regmap writes from userspace... but maybe he
> > > softened the stance or we could have this functionality opt-in?
> > 
> > I think unmediated raw register access is a terrible idea, you can't
> > safely write a driver if userspace can just go in and randomly write to
> > registers with no coordination with the running driver and for some
> > devices the kernel needs to ensure that any writes don't damage or
> > destabalise the system.  If a driver provides an interface that looks
> > like raw register accesses that's of course fine (I mean, a lot of
> > firmware formats basically boil down to register write sequences which
> > is clearly fine) but it should be the driver doing that and it should be
> > looking at what's going on and ensure that it's joined up with the needs
> > of the rest of the system.
> 
> I happen to agree here; especially in the case of writing new FW to a
> flash; this is a very hardware-centric and device-specific function,
> which by definition belongs in a kernel driver.
> 
> For example, many devices must be placed in a bootloader mode during
> the FW update, and may clamp or toggle an interrupt pin during this
> mode switch. If user space initiates this sequence while the driver is
> unaware of this process, it may attempt to read status registers from
> an I2C address that is temporarily offline.

And yet we have i2c-dev and hidraw that are often successfully used to
flash the firmware, do diagnostics, etc. without encumbering the kernel.
They are more likely to work on ACPI systems because such systems have
separation between power management and function pieces (whereas on
non-ACPI systems power management is crammed into the same driver and it
is not possible to properly power up device without wiring up the rest
of it). This is something that I feel we will have to fix in the long
term.

> 
> A much more common design pattern is for the driver to expose one W/O
> sysfs attribute for accepting the FW file name, and one R/O attribute
> for displaying the current FW version in flash. A small script runs at
> start-up to check the version against what is stored on "disk", and if
> what is stored in flash is deemed out of date, the script writes to the
> W/O attribute. This is the extent of user space's involvement.

This however means that code that is not used 99.9999 percent of the
time has to stay in the kernel, occupying precious memory. I agree
that if firmware update is very involved and needs precise control of
interrupts coming from the device and has certain timing restriction,
then in-kernel implementation is preferable, but in many instance
userspace updaters work just fine.

Thanks.

-- 
Dmitry

Re: [PATCH] Input: goodix-berlin - Add sysfs interface for reading and writing touch IC registers

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1047 bytes --]

On Tue, May 07, 2024 at 02:09:59PM -0700, Dmitry Torokhov wrote:
> On Tue, May 07, 2024 at 10:14:28AM -0500, Jeff LaBundy wrote:

> > For example, many devices must be placed in a bootloader mode during
> > the FW update, and may clamp or toggle an interrupt pin during this
> > mode switch. If user space initiates this sequence while the driver is
> > unaware of this process, it may attempt to read status registers from
> > an I2C address that is temporarily offline.

> And yet we have i2c-dev and hidraw that are often successfully used to
> flash the firmware, do diagnostics, etc. without encumbering the kernel.

Yeah, those seem like a reasonable enough model for safer devices - they
do the exclusion thing so you don't have a real driver running at the
same time.  For things like PMICs there's some concerns of course.

The other model I've seen used BTW is to expose a MTD device, if the
device actually looks like a MTD device (perhaps even is just a flash
that's fairly directly exposed) that minimises the kernel code quite
well.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] Input: goodix-berlin - Add sysfs interface for reading and writing touch IC registers

[Richard Hughes]

On Wed, 8 May 2024 at 03:37, Mark Brown <broonie@kernel.org> wrote:
> The other model I've seen used BTW is to expose a MTD device, if the
> device actually looks like a MTD device (perhaps even is just a flash
> that's fairly directly exposed) that minimises the kernel code quite
> well.

If it helps, fwupd already uses mtd for other devices too, although at
the moment we're using it only for system firmware -- e.g. intel-spi
style. The MTD subsystem doesn't give fwupd much info about the
{removable} device itself, and that can pose a problem unless you
start using heuristics about the parent device to match firmware to
the mtd device.

Richard.

Re: [PATCH] Input: goodix-berlin - Add sysfs interface for reading and writing touch IC registers

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 535 bytes --]

On Wed, May 08, 2024 at 09:39:41AM +0100, Richard Hughes wrote:

> If it helps, fwupd already uses mtd for other devices too, although at
> the moment we're using it only for system firmware -- e.g. intel-spi
> style. The MTD subsystem doesn't give fwupd much info about the
> {removable} device itself, and that can pose a problem unless you
> start using heuristics about the parent device to match firmware to
> the mtd device.

FWIW I know SolareFlare network cards used to do this too, though I
don't know about current products.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]