| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <6362e889-7df2-4c61-8ad5-bfe199e451ec@redhat.com>
Date: Mon, 6 May 2024 14:03:13 +0200
From: Hans de Goede <hdegoede@...hat.com>
To: Charles Wang <charles.goodix@...il.com>, hadess@...ess.net,
dmitry.torokhov@...il.com, Richard Hughes <hughsient@...il.com>
Cc: linux-input@...r.kernel.org, linux-kernel@...r.kernel.org,
neil.armstrong@...aro.org
Subject: Re: [PATCH] Input: goodix-berlin - Add sysfs interface for reading
and writing touch IC registers
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@...il.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@...aro.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);
Powered by blists - more mailing lists