| Message ID | 20240514115135.21410-1-charles.goodix@gmail.com (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | 240801c5f4f64616569990f744a81448c3d314b6 |
| Headers | show |
| Series | [v3] Input: goodix-berlin - Add sysfs interface for reading and writing touch IC registers | expand |
Hi Charles, On Tue, May 14, 2024 at 07:44:43PM +0800, Charles Wang wrote: > +static ssize_t registers_read(struct file *filp, struct kobject *kobj, > + struct bin_attribute *bin_attr, char *buf, loff_t off, size_t count) > +{ > + struct regmap *regmap; > + int error; > + > + regmap = dev_get_regmap(kobj_to_dev(kobj), NULL); We already have goodix_berlin_core->regmap, going through drvdata should be cheaper than scanning devres resources for the regmap data, so I'll change this. > + error = regmap_raw_read(regmap, (unsigned int)off, > + buf, count); > + > + return error ? error : count; > +} > + > +static ssize_t registers_write(struct file *filp, struct kobject *kobj, > + struct bin_attribute *bin_attr, char *buf, loff_t off, size_t count) > +{ > + struct regmap *regmap; > + int error; > + > + regmap = dev_get_regmap(kobj_to_dev(kobj), NULL); > + error = regmap_raw_write(regmap, (unsigned int)off, > + buf, count); > + > + return error ? error : count; > +} > + > +BIN_ATTR_RW(registers, 0); I do not think it is a good idea to allow the world read all registers. Any objection to make it BIN_ATTR_ADMIN_RW()? Thanks.
On Thu, May 30, 2024 at 04:53:11PM -0700, Dmitry Torokhov wrote: > Hi Charles, > > On Tue, May 14, 2024 at 07:44:43PM +0800, Charles Wang wrote: > > +static ssize_t registers_read(struct file *filp, struct kobject *kobj, > > + struct bin_attribute *bin_attr, char *buf, loff_t off, size_t count) > > +{ > > + struct regmap *regmap; > > + int error; > > + > > + regmap = dev_get_regmap(kobj_to_dev(kobj), NULL); > > We already have goodix_berlin_core->regmap, going through drvdata should > be cheaper than scanning devres resources for the regmap data, so I'll > change this. > > > + error = regmap_raw_read(regmap, (unsigned int)off, > > + buf, count); > > + > > + return error ? error : count; > > +} > > + > > +static ssize_t registers_write(struct file *filp, struct kobject *kobj, > > + struct bin_attribute *bin_attr, char *buf, loff_t off, size_t count) > > +{ > > + struct regmap *regmap; > > + int error; > > + > > + regmap = dev_get_regmap(kobj_to_dev(kobj), NULL); > > + error = regmap_raw_write(regmap, (unsigned int)off, > > + buf, count); > > + > > + return error ? error : count; > > +} > > + > > +BIN_ATTR_RW(registers, 0); > > I do not think it is a good idea to allow the world read all registers. > Any objection to make it BIN_ATTR_ADMIN_RW()? OK, since I have not heard anything I made the changes and applied the patch. Thanks.
diff --git a/drivers/input/touchscreen/goodix_berlin.h b/drivers/input/touchscreen/goodix_berlin.h index 1fd77eb69..38b6f9ddb 100644 --- a/drivers/input/touchscreen/goodix_berlin.h +++ b/drivers/input/touchscreen/goodix_berlin.h @@ -20,5 +20,6 @@ int goodix_berlin_probe(struct device *dev, int irq, const struct input_id *id, struct regmap *regmap); extern const struct dev_pm_ops goodix_berlin_pm_ops; +extern const struct attribute_group *goodix_berlin_groups[]; #endif diff --git a/drivers/input/touchscreen/goodix_berlin_core.c b/drivers/input/touchscreen/goodix_berlin_core.c index e7b41a926..020f8a31b 100644 --- a/drivers/input/touchscreen/goodix_berlin_core.c +++ b/drivers/input/touchscreen/goodix_berlin_core.c @@ -672,6 +672,49 @@ static void goodix_berlin_power_off_act(void *data) goodix_berlin_power_off(cd); } +static ssize_t registers_read(struct file *filp, struct kobject *kobj, + struct bin_attribute *bin_attr, char *buf, loff_t off, size_t count) +{ + struct regmap *regmap; + int error; + + regmap = dev_get_regmap(kobj_to_dev(kobj), NULL); + error = regmap_raw_read(regmap, (unsigned int)off, + buf, count); + + return error ? error : count; +} + +static ssize_t registers_write(struct file *filp, struct kobject *kobj, + struct bin_attribute *bin_attr, char *buf, loff_t off, size_t count) +{ + struct regmap *regmap; + int error; + + regmap = dev_get_regmap(kobj_to_dev(kobj), NULL); + error = regmap_raw_write(regmap, (unsigned int)off, + buf, count); + + return error ? error : count; +} + +BIN_ATTR_RW(registers, 0); + +static struct bin_attribute *goodix_berlin_bin_attrs[] = { + &bin_attr_registers, + NULL, +}; + +static const struct attribute_group goodix_berlin_attr_group = { + .bin_attrs = goodix_berlin_bin_attrs, +}; + +const struct attribute_group *goodix_berlin_groups[] = { + &goodix_berlin_attr_group, + NULL, +}; +EXPORT_SYMBOL_GPL(goodix_berlin_groups); + int goodix_berlin_probe(struct device *dev, int irq, const struct input_id *id, struct regmap *regmap) { diff --git a/drivers/input/touchscreen/goodix_berlin_i2c.c b/drivers/input/touchscreen/goodix_berlin_i2c.c index 6ed9aa808..b5f48315c 100644 --- a/drivers/input/touchscreen/goodix_berlin_i2c.c +++ b/drivers/input/touchscreen/goodix_berlin_i2c.c @@ -64,6 +64,7 @@ static struct i2c_driver goodix_berlin_i2c_driver = { .name = "goodix-berlin-i2c", .of_match_table = goodix_berlin_i2c_of_match, .pm = pm_sleep_ptr(&goodix_berlin_pm_ops), + .dev_groups = goodix_berlin_groups, }, .probe = goodix_berlin_i2c_probe, .id_table = goodix_berlin_i2c_id, diff --git a/drivers/input/touchscreen/goodix_berlin_spi.c b/drivers/input/touchscreen/goodix_berlin_spi.c index 4cc557da0..fe5739097 100644 --- a/drivers/input/touchscreen/goodix_berlin_spi.c +++ b/drivers/input/touchscreen/goodix_berlin_spi.c @@ -167,6 +167,7 @@ static struct spi_driver goodix_berlin_spi_driver = { .name = "goodix-berlin-spi", .of_match_table = goodix_berlin_spi_of_match, .pm = pm_sleep_ptr(&goodix_berlin_pm_ops), + .dev_groups = goodix_berlin_groups, }, .probe = goodix_berlin_spi_probe, .id_table = goodix_berlin_spi_ids,
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> --- Changes in v3: - export symbol goodix_berlin_groups - v2: https://lore.kernel.org/all/20240513123444.11617-1-charles.goodix@gmail.com/ Changes in v2: - use dev_groups to manager device attributes. - use dev_get_regmap to make show/store functions generic. - v1: https://lore.kernel.org/all/20240506114752.47204-1-charles.goodix@gmail.com/ --- drivers/input/touchscreen/goodix_berlin.h | 1 + .../input/touchscreen/goodix_berlin_core.c | 43 +++++++++++++++++++ drivers/input/touchscreen/goodix_berlin_i2c.c | 1 + drivers/input/touchscreen/goodix_berlin_spi.c | 1 + 4 files changed, 46 insertions(+)