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@xxxxxxxxx> > --- > 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@xxxxxxxxxx>"); > 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);
