| 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
| ||
|
Date: Thu, 9 Apr 2015 09:42:03 -0700
From: Dmitry Torokhov <dmitry.torokhov@...il.com>
To: Nick Dyer <nick.dyer@...ev.co.uk>
Cc: linux-input@...r.kernel.org, linux-kernel@...r.kernel.org,
Yufeng Shen <miletus@...omium.org>,
Benson Leung <bleung@...omium.org>,
Daniel Kurtz <djkurtz@...omium.org>,
Sjoerd Simons <sjoerd.simons@...labora.co.uk>,
Javier Martinez Canillas <javier.martinez@...labora.co.uk>,
Olof Johansson <olof@...om.net>
Subject: Re: [PATCH 2/2] Input: atmel_mxt_ts - allow specifying
device-specific configs
Hi Nick,
On Thu, Apr 09, 2015 at 12:03:14PM +0100, Nick Dyer wrote:
> Hi Dmitry-
>
> This is similar to something that I've already got in my tree:
>
> https://github.com/ndyer/linux/commit/449643df3c80
> https://github.com/ndyer/linux/commit/a520bbbfbdd1
>
> However, my version is based on the earlier Pixel code and allows updating
> the config at any point in time by writing to the sysfs node, not just at
> probe time, so it's a little more complex.
Yes, it is indeed a bit more complex and I am not sure we actually want
it. We are trying to move away from userspace-controllable
config/firmware names in ChromeOS - we produce images tailored for
particular device anyways, and we only need to differentiate between
touchpad and touchscreen on a device, so static (but different) config
names will work fine.
So for now I'd prefer merging this version and then possibly enhancing
it in the future.
The only question is: your binding scheme specifies "atmel,cfg_name"
whereas I think it reflects Linux driver behavior, not general hardware
property, so I think "linux," is better prefix for that, although I am
open to discuss this.
>
> Would you like me to test this along with the T100 changes you just merged?
That would be great.
>
> On 08/04/15 01:26, Dmitry Torokhov wrote:
> > Atmel controolers are very flexible and to function optimally require
>
> s/trool/troll/ :)
:) Thanks.
By the way, is is possible to check if device contains valid
configuration? Would config_crc be 0 if config stored in devices memory
was corrupted/damaged somehow? I wonder if we could avoid always
requesting config if device has some configuration (bit not necessarily
latest and greatest) already loaded.
>
> > device-specific configuration to be loaded. While the configuration is
> > stored in NVRAM and is normally persistent, sometimes it gets corrupted
> > and needs to be reloaded.
> >
> > Instead of using the same name, maxtouch.cfg, for all systems and all
> > devices, let's allow platform data to specify the name of configuration
> > file that should be loaded. This is especially useful for systems that
> > use Atmel controllers for both touchscren and touchpad, since their
> > configurations will surely differ.
> >
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@...il.com>
> > ---
> > .../devicetree/bindings/input/atmel,maxtouch.txt | 6 ++++++
> > drivers/input/touchscreen/atmel_mxt_ts.c | 18 +++++++++++++-----
> > include/linux/i2c/atmel_mxt_ts.h | 1 +
> > 3 files changed, 20 insertions(+), 5 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/input/atmel,maxtouch.txt b/Documentation/devicetree/bindings/input/atmel,maxtouch.txt
> > index 1852906..fd2344d 100644
> > --- a/Documentation/devicetree/bindings/input/atmel,maxtouch.txt
> > +++ b/Documentation/devicetree/bindings/input/atmel,maxtouch.txt
> > @@ -9,6 +9,12 @@ Required properties:
> > - interrupts: The sink for the touchpad's IRQ output
> > See ../interrupt-controller/interrupts.txt
> >
> > +Optional properties:
> > +
> > +- linux,config-name: name of configuration file that should be loaded
> > + into device for optimal functioning. If not specified "maxtouch.cfg"
> > + will be used.
> > +
> > Optional properties for main touchpad device:
> >
> > - linux,gpio-keymap: When enabled, the SPT_GPIOPWN_T19 object sends messages
> > diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
> > index dfc7309..0dcd455 100644
> > --- a/drivers/input/touchscreen/atmel_mxt_ts.c
> > +++ b/drivers/input/touchscreen/atmel_mxt_ts.c
> > @@ -2035,7 +2035,8 @@ static int mxt_initialize(struct mxt_data *data)
> > if (error)
> > goto err_free_object_table;
> >
> > - error = request_firmware_nowait(THIS_MODULE, true, MXT_CFG_NAME,
> > + error = request_firmware_nowait(THIS_MODULE, true,
> > + data->pdata->cfg_name ?: MXT_CFG_NAME,
> > &client->dev, GFP_KERNEL, data,
> > mxt_config_cb);
> > if (error) {
> > @@ -2375,20 +2376,21 @@ static void mxt_input_close(struct input_dev *dev)
> > #ifdef CONFIG_OF
> > static const struct mxt_platform_data *mxt_parse_dt(struct i2c_client *client)
> > {
> > + struct device_node *np;
> > struct mxt_platform_data *pdata;
> > u32 *keymap;
> > u32 keycode;
> > int proplen, i, ret;
> >
> > - if (!client->dev.of_node)
> > + np = client->dev.of_node;
> > + if (!np)
> > return ERR_PTR(-ENOENT);
> >
> > pdata = devm_kzalloc(&client->dev, sizeof(*pdata), GFP_KERNEL);
> > if (!pdata)
> > return ERR_PTR(-ENOMEM);
> >
> > - if (of_find_property(client->dev.of_node, "linux,gpio-keymap",
> > - &proplen)) {
> > + if (of_find_property(np, "linux,gpio-keymap", &proplen)) {
> > pdata->t19_num_keys = proplen / sizeof(u32);
> >
> > keymap = devm_kzalloc(&client->dev,
> > @@ -2398,7 +2400,7 @@ static const struct mxt_platform_data *mxt_parse_dt(struct i2c_client *client)
> > return ERR_PTR(-ENOMEM);
> >
> > for (i = 0; i < pdata->t19_num_keys; i++) {
> > - ret = of_property_read_u32_index(client->dev.of_node,
> > + ret = of_property_read_u32_index(np,
> > "linux,gpio-keymap", i, &keycode);
> > if (ret)
> > keycode = KEY_RESERVED;
> > @@ -2409,6 +2411,8 @@ static const struct mxt_platform_data *mxt_parse_dt(struct i2c_client *client)
> > pdata->t19_keymap = keymap;
> > }
> >
> > + of_property_read_string(np, "linux,config-name", &pdata->cfg_name);
> > +
> > return pdata;
> > }
> > #else
> > @@ -2439,11 +2443,15 @@ static struct mxt_acpi_platform_data samus_platform_data[] = {
> > .pdata = {
> > .t19_num_keys = ARRAY_SIZE(samus_touchpad_buttons),
> > .t19_keymap = samus_touchpad_buttons,
> > + .cfg_name = "samus-337t.raw",
> > },
> > },
> > {
> > /* Touchscreen */
> > .hid = "ATML0001",
> > + .pdata = {
> > + .cfg_name = "samus-2954t2.raw",
> > + },
> > },
> > { }
> > };
> > diff --git a/include/linux/i2c/atmel_mxt_ts.h b/include/linux/i2c/atmel_mxt_ts.h
> > index 02bf6ea..aeb8c9a 100644
> > --- a/include/linux/i2c/atmel_mxt_ts.h
> > +++ b/include/linux/i2c/atmel_mxt_ts.h
> > @@ -20,6 +20,7 @@ struct mxt_platform_data {
> > unsigned long irqflags;
> > u8 t19_num_keys;
> > const unsigned int *t19_keymap;
> > + const char *cfg_name;
> > };
> >
> > #endif /* __LINUX_ATMEL_MXT_TS_H */
> >
Thanks.
--
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists