* Re: [PATCH 1/2] drivers: base: swnode: link devices to software nodes
From: Dmitry Torokhov @ 2019-07-29 13:15 UTC (permalink / raw)
To: Heikki Krogerus
Cc: Linus Walleij, Rafael J . Wysocki,
Enrico Weigelt, metux IT consult, linux-input, linux-gpio,
linux-kernel, Andy Shevchenko
In-Reply-To: <20190729120715.GA28600@kuha.fi.intel.com>
On Mon, Jul 29, 2019 at 03:07:15PM +0300, Heikki Krogerus wrote:
> On Sat, Jul 13, 2019 at 12:52:58AM -0700, Dmitry Torokhov wrote:
> > It is helpful to know what device, if any, a software node is tied to, so
> > let's store a pointer to the device in software node structure. Note that
> > children software nodes will inherit their parent's device pointer, so we
> > do not have to traverse hierarchy to see what device the [sub]tree belongs
> > to.
> >
> > We will be using the device pointer to locate GPIO lookup tables for
> > devices with static properties.
> >
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > ---
> > drivers/base/property.c | 1 +
> > drivers/base/swnode.c | 35 ++++++++++++++++++++++++++++++++++-
> > include/linux/property.h | 5 +++++
> > 3 files changed, 40 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/base/property.c b/drivers/base/property.c
> > index 348b37e64944..3bc93d4b35c4 100644
> > --- a/drivers/base/property.c
> > +++ b/drivers/base/property.c
> > @@ -527,6 +527,7 @@ int device_add_properties(struct device *dev,
> > if (IS_ERR(fwnode))
> > return PTR_ERR(fwnode);
> >
> > + software_node_link_device(fwnode, dev);
> > set_secondary_fwnode(dev, fwnode);
> > return 0;
> > }
> > diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
> > index 7fc5a18e02ad..fd12eea539b6 100644
> > --- a/drivers/base/swnode.c
> > +++ b/drivers/base/swnode.c
> > @@ -24,6 +24,9 @@ struct software_node {
> >
> > /* properties */
> > const struct property_entry *properties;
> > +
> > + /* device this node is associated with */
> > + struct device *dev;
> > };
>
> Let's not do that! The nodes can be, and in many cases are, associated
> with multiple devices.
They do? Where? I see that set of properties can be shared between
several devices, but when we instantiate SW node we create a new
instance for device. This is also how ACPI and OF properties work; they
not shared between devices (or, rather, the kernel creates distinct _and
single_ devices for instance of ACPI or OF node). I do not think we want
swnodes work differently from the other firmware nodes.
>
> Every device is already linked with the software node kobject, so
> isn't it possible to simply walk trough those links in order to check
> the devices associated with the node?
No, we need to know the exact instance of a device, not a set of
devices, because even though some properties can be shared, others can
not. For example, even if 2 exactly same touch controllers in the system
have "reset-gpios" property, they won't be the same GPIO for the both of
them.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH 1/2] drivers: base: swnode: link devices to software nodes
From: Heikki Krogerus @ 2019-07-29 12:07 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Linus Walleij, Rafael J . Wysocki,
Enrico Weigelt, metux IT consult, linux-input, linux-gpio,
linux-kernel, Andy Shevchenko
In-Reply-To: <20190713075259.243565-2-dmitry.torokhov@gmail.com>
On Sat, Jul 13, 2019 at 12:52:58AM -0700, Dmitry Torokhov wrote:
> It is helpful to know what device, if any, a software node is tied to, so
> let's store a pointer to the device in software node structure. Note that
> children software nodes will inherit their parent's device pointer, so we
> do not have to traverse hierarchy to see what device the [sub]tree belongs
> to.
>
> We will be using the device pointer to locate GPIO lookup tables for
> devices with static properties.
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
> drivers/base/property.c | 1 +
> drivers/base/swnode.c | 35 ++++++++++++++++++++++++++++++++++-
> include/linux/property.h | 5 +++++
> 3 files changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/base/property.c b/drivers/base/property.c
> index 348b37e64944..3bc93d4b35c4 100644
> --- a/drivers/base/property.c
> +++ b/drivers/base/property.c
> @@ -527,6 +527,7 @@ int device_add_properties(struct device *dev,
> if (IS_ERR(fwnode))
> return PTR_ERR(fwnode);
>
> + software_node_link_device(fwnode, dev);
> set_secondary_fwnode(dev, fwnode);
> return 0;
> }
> diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
> index 7fc5a18e02ad..fd12eea539b6 100644
> --- a/drivers/base/swnode.c
> +++ b/drivers/base/swnode.c
> @@ -24,6 +24,9 @@ struct software_node {
>
> /* properties */
> const struct property_entry *properties;
> +
> + /* device this node is associated with */
> + struct device *dev;
> };
Let's not do that! The nodes can be, and in many cases are, associated
with multiple devices.
Every device is already linked with the software node kobject, so
isn't it possible to simply walk trough those links in order to check
the devices associated with the node?
thanks,
--
heikki
^ permalink raw reply
* Re: [PATCH 1/2] drivers: base: swnode: link devices to software nodes
From: Rafael J. Wysocki @ 2019-07-29 9:26 UTC (permalink / raw)
To: Linus Walleij
Cc: Dmitry Torokhov, Greg KH, Rafael J . Wysocki,
Enrico Weigelt, metux IT consult, Linux Input,
open list:GPIO SUBSYSTEM, linux-kernel@vger.kernel.org,
Andy Shevchenko, Heikki Krogerus
In-Reply-To: <CACRpkdZxJZyQD4WZ68hSNGXtGS23hHDv=rrnu9oFmMN0oRNb2w@mail.gmail.com>
On Monday, July 29, 2019 12:11:41 AM CEST Linus Walleij wrote:
> On Sat, Jul 13, 2019 at 9:53 AM Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
>
> > It is helpful to know what device, if any, a software node is tied to, so
> > let's store a pointer to the device in software node structure. Note that
> > children software nodes will inherit their parent's device pointer, so we
> > do not have to traverse hierarchy to see what device the [sub]tree belongs
> > to.
> >
> > We will be using the device pointer to locate GPIO lookup tables for
> > devices with static properties.
> >
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>
> If some device core person like Rafael and/or Greg can ACK it I can
> apply this patch to the GPIO tree.
I don't have any concerns regarding this, so please feel free to add
Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
to the commit when you apply it.
Thanks!
^ permalink raw reply
* [PATCH] Input: applespi - Fix build error
From: YueHaibing @ 2019-07-29 3:14 UTC (permalink / raw)
To: ronald, dmitry.torokhov, nikolas; +Cc: linux-kernel, linux-input, YueHaibing
If CONFIG_KEYBOARD_APPLESPI=y but CONFIG_LEDS_CLASS=m
building fails:
drivers/input/keyboard/applespi.o: In function `applespi_probe':
applespi.c:(.text+0x1fcd): undefined reference to `devm_led_classdev_register_ext'
Wrap it in LEDS_CLASS macro to fix this.
Reported-by: Hulk Robot <hulkci@huawei.com>
Fixes: 038b1a05eae6 ("Input: add Apple SPI keyboard and trackpad driver")
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
drivers/input/keyboard/applespi.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/input/keyboard/applespi.c b/drivers/input/keyboard/applespi.c
index acf34a5..9c50b09 100644
--- a/drivers/input/keyboard/applespi.c
+++ b/drivers/input/keyboard/applespi.c
@@ -1790,11 +1790,13 @@ static int applespi_probe(struct spi_device *spi)
applespi->backlight_info.default_trigger = "kbd-backlight";
applespi->backlight_info.brightness_set = applespi_set_bl_level;
+#ifdef CONFIG_LEDS_CLASS
sts = devm_led_classdev_register(&spi->dev, &applespi->backlight_info);
if (sts)
dev_warn(&applespi->spi->dev,
"Unable to register keyboard backlight class dev (%d)\n",
sts);
+#endif
/* set up debugfs entries for touchpad dimensions logging */
applespi->debugfs_root = debugfs_create_dir("applespi", NULL);
--
2.7.4
^ permalink raw reply related
* Re: [PATCH 2/2] gpiolib: add support for fetching descriptors from static properties
From: Linus Walleij @ 2019-07-28 22:15 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Rafael J . Wysocki, Enrico Weigelt, metux IT consult, Linux Input,
open list:GPIO SUBSYSTEM, linux-kernel@vger.kernel.org,
Andy Shevchenko, Heikki Krogerus
In-Reply-To: <20190713075259.243565-3-dmitry.torokhov@gmail.com>
On Sat, Jul 13, 2019 at 9:53 AM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> Now that static device properties understand notion of child nodes, let's
> teach gpiolib to tie such children and machine GPIO descriptor tables.
> We will continue using a single table for entire device, but instead of
> using connection ID as a lookup key in the GPIO descriptor table directly,
> we will perform additional translation: fwnode_get_named_gpiod() when
> dealing with property_set-backed fwnodes will try parsing string property
> with name matching connection ID and use result of the lookup as the key in
> the table:
>
> static const struct property_entry dev_child1_props[] __initconst = {
> ...
> PROPERTY_ENTRY_STRING("gpios", "child-1-gpios"),
> { }
> };
>
> static struct gpiod_lookup_table dev_gpiod_table = {
> .dev_id = "some-device",
> .table = {
> ...
> GPIO_LOOKUP_IDX("B", 1, "child-1-gpios", 1, GPIO_ACTIVE_LOW),
> ...
> },
> };
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
I'm pretty grateful for this since I think at one point I provoked this whole
series. :)
> +static struct gpio_desc *__fwnode_get_named_gpiod(struct fwnode_handle *fwnode,
I am allergic to __underscore_with_unclear_semantics() so I will
change this when applying to something with meaning (I even
like "inner_" better.)
Otherwise it's good to go when I get an ACK on the first patch.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH 1/2] drivers: base: swnode: link devices to software nodes
From: Linus Walleij @ 2019-07-28 22:11 UTC (permalink / raw)
To: Dmitry Torokhov, Greg KH
Cc: Rafael J . Wysocki, Enrico Weigelt, metux IT consult, Linux Input,
open list:GPIO SUBSYSTEM, linux-kernel@vger.kernel.org,
Andy Shevchenko, Heikki Krogerus
In-Reply-To: <20190713075259.243565-2-dmitry.torokhov@gmail.com>
On Sat, Jul 13, 2019 at 9:53 AM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> It is helpful to know what device, if any, a software node is tied to, so
> let's store a pointer to the device in software node structure. Note that
> children software nodes will inherit their parent's device pointer, so we
> do not have to traverse hierarchy to see what device the [sub]tree belongs
> to.
>
> We will be using the device pointer to locate GPIO lookup tables for
> devices with static properties.
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
If some device core person like Rafael and/or Greg can ACK it I can
apply this patch to the GPIO tree.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH v3 2/2] Input: soc_button_array - Add support for newer surface devices
From: Dmitry Torokhov @ 2019-07-28 10:01 UTC (permalink / raw)
To: Maximilian Luz
Cc: linux-kernel, linux-input, platform-driver-x86, Hans de Goede,
Chen Yu, Darren Hart, Andy Shevchenko, Benjamin Tissoires
In-Reply-To: <fb53b082-4d83-83a6-1ae6-b9fae9dc750f@gmail.com>
On Sat, Jul 27, 2019 at 02:01:26PM +0200, Maximilian Luz wrote:
> On 7/27/19 11:14 AM, Dmitry Torokhov wrote:
> > On Sat, Jul 20, 2019 at 05:05:11PM +0200, Maximilian Luz wrote:
> > > -
> > > - error = gpiod_count(dev, NULL);
> > > - if (error < 0) {
> > > - dev_dbg(dev, "no GPIO attached, ignoring...\n");
> > > - return -ENODEV;
> >
> > I do not think we need to move this into individual "check" functions.
> > It is needed in all cases so we should keep it here.
> >
> > How about version below?
>
> Makes sense, looks good to me!
OK, great, applied.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v3 1/2] platform/x86: surfacepro3_button: Fix device check
From: Dmitry Torokhov @ 2019-07-28 9:57 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Maximilian Luz, Linux Kernel Mailing List, linux-input,
Platform Driver, Hans de Goede, Chen Yu, Darren Hart,
Andy Shevchenko, Benjamin Tissoires
In-Reply-To: <CAHp75VdsL+-bhAUcxLKFKLZedN3gFD3jxnhELD1RtKGSHdagjw@mail.gmail.com>
On Sat, Jul 27, 2019 at 03:26:41PM +0300, Andy Shevchenko wrote:
> On Sat, Jul 27, 2019 at 12:15 PM Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> >
> > On Thu, Jul 25, 2019 at 08:57:53PM +0300, Andy Shevchenko wrote:
> > > On Sat, Jul 20, 2019 at 6:05 PM Maximilian Luz <luzmaximilian@gmail.com> wrote:
> > > >
> > > > Do not use the surfacepro3_button driver on newer Microsoft Surface
> > > > models, only use it on the Surface Pro 3 and 4. Newer models (5th, 6th
> > > > and possibly future generations) use the same device as the Surface Pro
> > > > 4 to represent their volume and power buttons (MSHW0040), but their
> > > > actual implementation is significantly different. This patch ensures
> > > > that the surfacepro3_button driver is only used on the Pro 3 and 4
> > > > models, allowing a different driver to bind on other models.
> > > >
> > >
> > > Acked-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> > >
> > > assuming it will go thru Input subsystem.
> >
> > I can take it, but I do not think it is dependent on the other change
> > and thus can go through platform tree as well.
>
> Pkease, take it. I do not expect any changes to the driver this cycle.
OK, applied, thank you.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v3 1/2] platform/x86: surfacepro3_button: Fix device check
From: Andy Shevchenko @ 2019-07-27 12:26 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Maximilian Luz, Linux Kernel Mailing List, linux-input,
Platform Driver, Hans de Goede, Chen Yu, Darren Hart,
Andy Shevchenko, Benjamin Tissoires
In-Reply-To: <20190727091541.GD795@penguin>
On Sat, Jul 27, 2019 at 12:15 PM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>
> On Thu, Jul 25, 2019 at 08:57:53PM +0300, Andy Shevchenko wrote:
> > On Sat, Jul 20, 2019 at 6:05 PM Maximilian Luz <luzmaximilian@gmail.com> wrote:
> > >
> > > Do not use the surfacepro3_button driver on newer Microsoft Surface
> > > models, only use it on the Surface Pro 3 and 4. Newer models (5th, 6th
> > > and possibly future generations) use the same device as the Surface Pro
> > > 4 to represent their volume and power buttons (MSHW0040), but their
> > > actual implementation is significantly different. This patch ensures
> > > that the surfacepro3_button driver is only used on the Pro 3 and 4
> > > models, allowing a different driver to bind on other models.
> > >
> >
> > Acked-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> >
> > assuming it will go thru Input subsystem.
>
> I can take it, but I do not think it is dependent on the other change
> and thus can go through platform tree as well.
Pkease, take it. I do not expect any changes to the driver this cycle.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v3 2/2] Input: soc_button_array - Add support for newer surface devices
From: Maximilian Luz @ 2019-07-27 12:01 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: linux-kernel, linux-input, platform-driver-x86, Hans de Goede,
Chen Yu, Darren Hart, Andy Shevchenko, Benjamin Tissoires
In-Reply-To: <20190727091443.GC795@penguin>
On 7/27/19 11:14 AM, Dmitry Torokhov wrote:
> On Sat, Jul 20, 2019 at 05:05:11PM +0200, Maximilian Luz wrote:
>> -
>> - error = gpiod_count(dev, NULL);
>> - if (error < 0) {
>> - dev_dbg(dev, "no GPIO attached, ignoring...\n");
>> - return -ENODEV;
>
> I do not think we need to move this into individual "check" functions.
> It is needed in all cases so we should keep it here.
>
> How about version below?
Makes sense, looks good to me!
Maximilian
^ permalink raw reply
* Re: [PATCH v3 1/2] platform/x86: surfacepro3_button: Fix device check
From: Dmitry Torokhov @ 2019-07-27 9:15 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Maximilian Luz, Linux Kernel Mailing List, linux-input,
Platform Driver, Hans de Goede, Chen Yu, Darren Hart,
Andy Shevchenko, Benjamin Tissoires
In-Reply-To: <CAHp75Ve+3c-TFeN3Dh-DB75Rjft8mY2DA8vNkrFyp7JK-ZOjDA@mail.gmail.com>
On Thu, Jul 25, 2019 at 08:57:53PM +0300, Andy Shevchenko wrote:
> On Sat, Jul 20, 2019 at 6:05 PM Maximilian Luz <luzmaximilian@gmail.com> wrote:
> >
> > Do not use the surfacepro3_button driver on newer Microsoft Surface
> > models, only use it on the Surface Pro 3 and 4. Newer models (5th, 6th
> > and possibly future generations) use the same device as the Surface Pro
> > 4 to represent their volume and power buttons (MSHW0040), but their
> > actual implementation is significantly different. This patch ensures
> > that the surfacepro3_button driver is only used on the Pro 3 and 4
> > models, allowing a different driver to bind on other models.
> >
>
> Acked-by: Andy Shevchenko <andy.shevchenko@gmail.com>
>
> assuming it will go thru Input subsystem.
I can take it, but I do not think it is dependent on the other change
and thus can go through platform tree as well.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v3 2/2] Input: soc_button_array - Add support for newer surface devices
From: Dmitry Torokhov @ 2019-07-27 9:14 UTC (permalink / raw)
To: Maximilian Luz
Cc: linux-kernel, linux-input, platform-driver-x86, Hans de Goede,
Chen Yu, Darren Hart, Andy Shevchenko, Benjamin Tissoires
In-Reply-To: <20190720150511.95076-3-luzmaximilian@gmail.com>
Hi Maximilian,
On Sat, Jul 20, 2019 at 05:05:11PM +0200, Maximilian Luz wrote:
> -
> - error = gpiod_count(dev, NULL);
> - if (error < 0) {
> - dev_dbg(dev, "no GPIO attached, ignoring...\n");
> - return -ENODEV;
I do not think we need to move this into individual "check" functions.
It is needed in all cases so we should keep it here.
How about version below?
Input: soc_button_array - add support for newer surface devices
From: Maximilian Luz <luzmaximilian@gmail.com>
Power and volume button support for 5th and 6th generation Microsoft
Surface devices via soc_button_array.
Note that these devices use the same MSHW0040 device as on the Surface
Pro 4, however the implementation is different (GPIOs vs. ACPI
notifications). Thus some checking is required to ensure we only load
this driver on the correct devices.
Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/misc/Kconfig | 6 +-
drivers/input/misc/soc_button_array.c | 105 +++++++++++++++++++++++++++++----
2 files changed, 96 insertions(+), 15 deletions(-)
diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index d07c1eb15aa6..7d9ae394e597 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -813,10 +813,10 @@ config INPUT_IDEAPAD_SLIDEBAR
config INPUT_SOC_BUTTON_ARRAY
tristate "Windows-compatible SoC Button Array"
- depends on KEYBOARD_GPIO
+ depends on KEYBOARD_GPIO && ACPI
help
- Say Y here if you have a SoC-based tablet that originally
- runs Windows 8.
+ Say Y here if you have a SoC-based tablet that originally runs
+ Windows 8 or a Microsoft Surface Book 2, Pro 5, Laptop 1 or later.
To compile this driver as a module, choose M here: the
module will be called soc_button_array.
diff --git a/drivers/input/misc/soc_button_array.c b/drivers/input/misc/soc_button_array.c
index 5e59f8e57f8e..6f0133fe1546 100644
--- a/drivers/input/misc/soc_button_array.c
+++ b/drivers/input/misc/soc_button_array.c
@@ -25,6 +25,11 @@ struct soc_button_info {
bool wakeup;
};
+struct soc_device_data {
+ const struct soc_button_info *button_info;
+ int (*check)(struct device *dev);
+};
+
/*
* Some of the buttons like volume up/down are auto repeat, while others
* are not. To support both, we register two platform devices, and put
@@ -87,8 +92,13 @@ soc_button_device_create(struct platform_device *pdev,
continue;
gpio = soc_button_lookup_gpio(&pdev->dev, info->acpi_index);
- if (!gpio_is_valid(gpio))
+ if (gpio < 0 && gpio != -ENOENT) {
+ error = gpio;
+ goto err_free_mem;
+ } else if (!gpio_is_valid(gpio)) {
+ /* Skip GPIO if not present */
continue;
+ }
gpio_keys[n_buttons].type = info->event_type;
gpio_keys[n_buttons].code = info->event_code;
@@ -309,23 +319,26 @@ static int soc_button_remove(struct platform_device *pdev)
static int soc_button_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
- const struct acpi_device_id *id;
- struct soc_button_info *button_info;
+ const struct soc_device_data *device_data;
+ const struct soc_button_info *button_info;
struct soc_button_data *priv;
struct platform_device *pd;
int i;
int error;
- id = acpi_match_device(dev->driver->acpi_match_table, dev);
- if (!id)
- return -ENODEV;
+ device_data = acpi_device_get_match_data(dev);
+ if (device_data && device_data->check) {
+ error = device_data->check(dev);
+ if (error)
+ return error;
+ }
- if (!id->driver_data) {
+ if (device_data && device_data->button_info) {
+ button_info = device_data->button_info;
+ } else {
button_info = soc_button_get_button_info(dev);
if (IS_ERR(button_info))
return PTR_ERR(button_info);
- } else {
- button_info = (struct soc_button_info *)id->driver_data;
}
error = gpiod_count(dev, NULL);
@@ -357,7 +370,7 @@ static int soc_button_probe(struct platform_device *pdev)
if (!priv->children[0] && !priv->children[1])
return -ENODEV;
- if (!id->driver_data)
+ if (!device_data || !device_data->button_info)
devm_kfree(dev, button_info);
return 0;
@@ -368,7 +381,7 @@ static int soc_button_probe(struct platform_device *pdev)
* is defined in section 2.8.7.2 of "Windows ACPI Design Guide for SoC
* Platforms"
*/
-static struct soc_button_info soc_button_PNP0C40[] = {
+static const struct soc_button_info soc_button_PNP0C40[] = {
{ "power", 0, EV_KEY, KEY_POWER, false, true },
{ "home", 1, EV_KEY, KEY_LEFTMETA, false, true },
{ "volume_up", 2, EV_KEY, KEY_VOLUMEUP, true, false },
@@ -377,9 +390,77 @@ static struct soc_button_info soc_button_PNP0C40[] = {
{ }
};
+static const struct soc_device_data soc_device_PNP0C40 = {
+ .button_info = soc_button_PNP0C40,
+};
+
+/*
+ * Special device check for Surface Book 2 and Surface Pro (2017).
+ * Both, the Surface Pro 4 (surfacepro3_button.c) and the above mentioned
+ * devices use MSHW0040 for power and volume buttons, however the way they
+ * have to be addressed differs. Make sure that we only load this drivers
+ * for the correct devices by checking the OEM Platform Revision provided by
+ * the _DSM method.
+ */
+#define MSHW0040_DSM_REVISION 0x01
+#define MSHW0040_DSM_GET_OMPR 0x02 // get OEM Platform Revision
+static const guid_t MSHW0040_DSM_UUID =
+ GUID_INIT(0x6fd05c69, 0xcde3, 0x49f4, 0x95, 0xed, 0xab, 0x16, 0x65,
+ 0x49, 0x80, 0x35);
+
+static int soc_device_check_MSHW0040(struct device *dev)
+{
+ acpi_handle handle = ACPI_HANDLE(dev);
+ union acpi_object *result;
+ u64 oem_platform_rev = 0; // valid revisions are nonzero
+
+ // get OEM platform revision
+ result = acpi_evaluate_dsm_typed(handle, &MSHW0040_DSM_UUID,
+ MSHW0040_DSM_REVISION,
+ MSHW0040_DSM_GET_OMPR, NULL,
+ ACPI_TYPE_INTEGER);
+
+ if (result) {
+ oem_platform_rev = result->integer.value;
+ ACPI_FREE(result);
+ }
+
+ /*
+ * If the revision is zero here, the _DSM evaluation has failed. This
+ * indicates that we have a Pro 4 or Book 1 and this driver should not
+ * be used.
+ */
+ if (oem_platform_rev == 0)
+ return -ENODEV;
+
+ dev_dbg(dev, "OEM Platform Revision %llu\n", oem_platform_rev);
+
+ return 0;
+}
+
+/*
+ * Button infos for Microsoft Surface Book 2 and Surface Pro (2017).
+ * Obtained from DSDT/testing.
+ */
+static const struct soc_button_info soc_button_MSHW0040[] = {
+ { "power", 0, EV_KEY, KEY_POWER, false, true },
+ { "volume_up", 2, EV_KEY, KEY_VOLUMEUP, true, false },
+ { "volume_down", 4, EV_KEY, KEY_VOLUMEDOWN, true, false },
+ { }
+};
+
+static const struct soc_device_data soc_device_MSHW0040 = {
+ .button_info = soc_button_MSHW0040,
+ .check = soc_device_check_MSHW0040,
+};
+
static const struct acpi_device_id soc_button_acpi_match[] = {
- { "PNP0C40", (unsigned long)soc_button_PNP0C40 },
+ { "PNP0C40", (unsigned long)&soc_device_PNP0C40 },
{ "ACPI0011", 0 },
+
+ /* Microsoft Surface Devices (5th and 6th generation) */
+ { "MSHW0040", (unsigned long)&soc_device_MSHW0040 },
+
{ }
};
--
Dmitry
^ permalink raw reply related
* Re: [RFC PATCH v2 1/4] dt-bindings: input: Add support for the MPR121 without interrupt line
From: Dmitry Torokhov @ 2019-07-27 8:01 UTC (permalink / raw)
To: Rob Herring
Cc: Michal Vokáč, Mark Rutland, Shawn Guo, Sascha Hauer,
Fabio Estevam, linux-input, devicetree, linux-kernel,
Pengutronix Kernel Team
In-Reply-To: <20190613223945.GA938@bogus>
On Thu, Jun 13, 2019 at 04:39:45PM -0600, Rob Herring wrote:
> On Fri, May 17, 2019 at 03:12:50PM +0200, Michal Vokáč wrote:
> > Normally, the MPR121 controller uses separate interrupt line to notify
> > the I2C host that a key was touched/released. To support platforms that
> > can not use the interrupt line, polling of the MPR121 registers can be
> > used.
>
> 'separate' from what?
>
> >
> > Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com>
> > ---
> > Changes since v1:
> > - Document the polled binding in the original file, do not create a new one.
> > (Rob)
> >
> > Documentation/devicetree/bindings/input/mpr121-touchkey.txt | 9 +++++++--
> > 1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/input/mpr121-touchkey.txt b/Documentation/devicetree/bindings/input/mpr121-touchkey.txt
> > index b7c61ee5841b..97f55273d473 100644
> > --- a/Documentation/devicetree/bindings/input/mpr121-touchkey.txt
> > +++ b/Documentation/devicetree/bindings/input/mpr121-touchkey.txt
> > @@ -1,9 +1,14 @@
> > -* Freescale MPR121 Controllor
> > +* Freescale MPR121 Controller
> >
> > Required Properties:
> > -- compatible: Should be "fsl,mpr121-touchkey"
> > +- compatible: Should be one of:
> > + - "fsl,mpr121-touchkey" - MPR121 with interrupt line
> > + - "fsl,mpr121-touchkey-polled" - MPR121 with polling
> > - reg: The I2C slave address of the device.
> > - interrupts: The interrupt number to the cpu.
> > + In case of "fsl,mpr121-touchkey-polled" the interrupt
> > + line is not used and hence the interrupts property is
> > + not required.
>
> Absence of the interrupts property is enough to determine polled mode
> and you don't need a separate compatible string.
I would prefer if we could distinguish between chip working in polled
mode intentionally vs DT writer simply forgetting to specify interrupt
property. Should we key the polling mode off "linux,poll-interval"
property? We probably going to need it anyway as not everyone needs the
same polling frequency.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [RFC PATCH v2 0/4] Input: mpr121-polled: Add polled driver for MPR121
From: Dmitry Torokhov @ 2019-07-27 7:31 UTC (permalink / raw)
To: Michal Vokáč
Cc: Rob Herring, Mark Rutland, Shawn Guo, Sascha Hauer, Fabio Estevam,
linux-input, devicetree, linux-kernel, Pengutronix Kernel Team
In-Reply-To: <dcee1139-c53f-5ea0-f387-a3aa5a9bf39f@ysoft.com>
On Fri, Jul 26, 2019 at 01:31:31PM +0200, Michal Vokáč wrote:
> On 25. 07. 19 16:40, Dmitry Torokhov wrote:
> > On Thu, Jul 25, 2019 at 02:58:02PM +0200, Michal Vokáč wrote:
> > > On 25. 07. 19 10:57, Dmitry Torokhov wrote:
> > > > Hi Michal,
> > > >
> > > > On Tue, May 21, 2019 at 08:51:17AM +0200, Michal Vokáč wrote:
> > > > > On 21. 05. 19 7:37, Dmitry Torokhov wrote:
> > > > > > Hi Michal,
> > > > > >
> > > > > > On Fri, May 17, 2019 at 03:12:49PM +0200, Michal Vokáč wrote:
> > > > > > > Hi,
> > > > > > >
> > > > > > > I have to deal with a situation where we have a custom i.MX6 based
> > > > > > > platform in production that uses the MPR121 touchkey controller.
> > > > > > > Unfortunately the chip is connected using only the I2C interface.
> > > > > > > The interrupt line is not used. Back in 2015 (Linux v3.14), my
> > > > > > > colleague modded the existing mpr121_touchkey.c driver to use polling
> > > > > > > instead of interrupt.
> > > > > > >
> > > > > > > For quite some time yet I am in a process of updating the product from
> > > > > > > the ancient Freescale v3.14 kernel to the latest mainline and pushing
> > > > > > > any needed changes upstream. The DT files for our imx6dl-yapp4 platform
> > > > > > > already made it into v5.1-rc.
> > > > > > >
> > > > > > > I rebased and updated our mpr121 patch to the latest mainline.
> > > > > > > It is created as a separate driver, similarly to gpio_keys_polled.
> > > > > > >
> > > > > > > The I2C device is quite susceptible to ESD. An ESD test quite often
> > > > > > > causes reset of the chip or some register randomly changes its value.
> > > > > > > The [PATCH 3/4] adds a write-through register cache. With the cache
> > > > > > > this state can be detected and the device can be re-initialied.
> > > > > > >
> > > > > > > The main question is: Is there any chance that such a polled driver
> > > > > > > could be accepted? Is it correct to implement it as a separate driver
> > > > > > > or should it be done as an option in the existing driver? I can not
> > > > > > > really imagine how I would do that though..
> > > > > > >
> > > > > > > There are also certain worries that the MPR121 chip may no longer be
> > > > > > > available in nonspecifically distant future. In case of EOL I will need
> > > > > > > to add a polled driver for an other touchkey chip. May it be already
> > > > > > > in mainline or a completely new one.
> > > > > >
> > > > > > I think that my addition of input_polled_dev was ultimately a wrong
> > > > > > thing to do. I am looking into enabling polling mode for regular input
> > > > > > devices as we then can enable polling mode in existing drivers.
> > > > >
> > > > > OK, that sounds good. Especially when one needs to switch from one chip
> > > > > to another that is already in tree, the need for a whole new polling
> > > > > driver is eliminated.
> > > >
> > > > Could you please try the patch below and see if it works for your use
> > > > case? Note that I have not tried running it, but it compiles so it must
> > > > be good ;)
> > >
> > > Hi Dmitry,
> > > Thank you very much for the patch!
> > > I gave it a shot and it seems you forgot to add the input-poller.h file
> > > to the patch.. it does not compile on my side :(
> >
> > Oops ;) Please see the updated patch below.
>
> Thank you, now it is (almost) good as you said :D
>
> > >
> > > > Input: add support for polling to input devices
> > > >
> > > > From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > > >
> > > > Separating "normal" and "polled" input devices was a mistake, as often we want
> > > > to allow the very same device work on both interrupt-driven and polled mode,
> > > > depending on the board on which the device is used.
> > > >
> > > > This introduces new APIs:
> > > >
> > > > - input_setup_polling
> > > > - input_set_poll_interval
> > > > - input_set_min_poll_interval
> > > > - input_set_max_poll_interval
> > > >
> > > > These new APIs allow switching an input device into polled mode with sysfs
> > > > attributes matching drivers using input_polled_dev APIs that will be eventually
> > > > removed.
> > >
> > > After reading this I am not really sure what else needs to be done
> > > to test/use the poller. I suspect I need to modify the input device
> > > driver (mpr121_touchkey.c in my case) like this:
> > >
> > > If the interrupt gpio is not provided in DT, the device driver probe
> > > function should:
> > > - not request the threaded interrupt
> > > - call input_setup_polling and provide it with poll_fn
> > > Can the mpr_touchkey_interrupt function be used as is for this
> > > purpose? The only problem I see is it returns IRQ_HANDLED.
> >
> > I'd factor out code suitable for polling from mpr_touchkey_interrupt()
> > and then do
> >
> > static irqreturn_t mpr_touchkey_interrupt(...)
> > {
> > mpr_touchkey_report(...);
> > return IRQ_HANDLED;
> > }
> >
>
> Probably a trivial problem for experienced kernel hacker but I can not
> wrap my head around this - the interrupt handler takes the mpr121
> device id as an argument while the poller poll_fn takes struct input_dev.
>
> I fail to figure out how to get the device id from the input device.
>
> Here is what I have:
>
> diff --git a/drivers/input/keyboard/mpr121_touchkey.c b/drivers/input/keyboard/mpr121_touchkey.c
> index e9ceaa16b46a..1124f77ee10a 100644
> --- a/drivers/input/keyboard/mpr121_touchkey.c
> +++ b/drivers/input/keyboard/mpr121_touchkey.c
> @@ -54,6 +54,10 @@
> /* MPR121 has 12 keys */
> #define MPR121_MAX_KEY_COUNT 12
> +#define MPR121_POLL_INTERVAL 50
> +#define MPR121_MIN_POLL_INTERVAL 10
> +#define MPR121_MAX_POLL_INTERVAL 200
> +
> struct mpr121_touchkey {
> struct i2c_client *client;
> struct input_dev *input_dev;
> @@ -115,9 +119,12 @@ static struct regulator *mpr121_vdd_supply_init(struct device *dev)
> return vdd_supply;
> }
> -static irqreturn_t mpr_touchkey_interrupt(int irq, void *dev_id)
> +static void mpr_touchkey_report(struct input_dev *dev)
> {
> - struct mpr121_touchkey *mpr121 = dev_id;
> + /*
> + * TODO the input_dev dev needs to be converted to mpr121 device id.
> + */
> + struct mpr121_touchkey *mpr121 = /* TODO */;
Use input_set_drvdata() in probe() and you can do
struct mpr121_touchkey *mpr121 = input_get_drvdata(dev);
here.
> struct i2c_client *client = mpr121->client;
> struct input_dev *input = mpr121->input_dev;
> unsigned long bit_changed;
> @@ -127,14 +134,14 @@ static irqreturn_t mpr_touchkey_interrupt(int irq, void *dev_id)
> reg = i2c_smbus_read_byte_data(client, ELE_TOUCH_STATUS_1_ADDR);
> if (reg < 0) {
> dev_err(&client->dev, "i2c read error [%d]\n", reg);
> - goto out;
> + return;
> }
> reg <<= 8;
> reg |= i2c_smbus_read_byte_data(client, ELE_TOUCH_STATUS_0_ADDR);
> if (reg < 0) {
> dev_err(&client->dev, "i2c read error [%d]\n", reg);
> - goto out;
> + return;
> }
> reg &= TOUCH_STATUS_MASK;
> @@ -155,8 +162,17 @@ static irqreturn_t mpr_touchkey_interrupt(int irq, void *dev_id)
> }
> input_sync(input);
> +}
> +
> +static irqreturn_t mpr_touchkey_interrupt(int irq, void *dev_id)
> +{
> + /*
> + * TODO
> + * mpr_touchkey_report takes struct input_dev as an argument,
> + * not the device id.
> + */
struct mpr121_touchkey *mpr121 = dev_id;
mpr_touchkey_report(mpr121->input);
> + mpr_touchkey_report(/* TODO */);
> -out:
> return IRQ_HANDLED;
> }
> @@ -232,11 +248,6 @@ static int mpr_touchkey_probe(struct i2c_client *client,
> int error;
> int i;
> - if (!client->irq) {
> - dev_err(dev, "irq number should not be zero\n");
> - return -EINVAL;
> - }
> -
> vdd_supply = mpr121_vdd_supply_init(dev);
> if (IS_ERR(vdd_supply))
> return PTR_ERR(vdd_supply);
> @@ -289,13 +300,27 @@ static int mpr_touchkey_probe(struct i2c_client *client,
> return error;
> }
> - error = devm_request_threaded_irq(dev, client->irq, NULL,
> - mpr_touchkey_interrupt,
> - IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> - dev->driver->name, mpr121);
> - if (error) {
> - dev_err(dev, "Failed to register interrupt\n");
> - return error;
> + if (client->irq) {
> + error = devm_request_threaded_irq(dev, client->irq, NULL,
> + mpr_touchkey_interrupt,
> + IRQF_TRIGGER_FALLING |
> + IRQF_ONESHOT,
> + dev->driver->name, mpr121);
> + if (error) {
> + dev_err(dev, "Failed to register interrupt\n");
> + return error;
> + }
> + } else {
> + dev_dbg(dev, "invalid IRQ number, using polling mode\n");
I think it would be better if we checked if poll interval device
property is present before setting polling mode, so that polling mode is
not activated simply because someone forgot to add interrupt
specification.
> + error = input_setup_polling(input_dev, mpr_touchkey_report);
> + if (error)
> + return error;
> +
> + input_set_poll_interval(input_dev, MPR121_POLL_INTERVAL);
> + input_set_min_poll_interval(input_dev,
> + MPR121_MIN_POLL_INTERVAL);
> + input_set_max_poll_interval(input_dev,
> + MPR121_MAX_POLL_INTERVAL);
> }
> error = input_register_device(input_dev);
> --
Thanks.
--
Dmitry
^ permalink raw reply
* [PATCH -next] Input: melfas_mip4 - remove set but not used variables
From: YueHaibing @ 2019-07-26 14:22 UTC (permalink / raw)
To: jeesw, dmitry.torokhov; +Cc: linux-kernel, linux-input, YueHaibing
Fixes gcc '-Wunused-but-set-variable' warning:
drivers/input/touchscreen/melfas_mip4.c: In function 'mip4_report_touch':
drivers/input/touchscreen/melfas_mip4.c:474:5: warning: variable 'size' set but not used [-Wunused-but-set-variable]
drivers/input/touchscreen/melfas_mip4.c:472:5: warning: variable 'pressure_stage' set but not used [-Wunused-but-set-variable]
drivers/input/touchscreen/melfas_mip4.c:469:7: warning: variable 'palm' set but not used [-Wunused-but-set-variable]
drivers/input/touchscreen/melfas_mip4.c:468:7: warning: variable 'hover' set but not used [-Wunused-but-set-variable]
They are never used so can be removed.
Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
drivers/input/touchscreen/melfas_mip4.c | 11 -----------
1 file changed, 11 deletions(-)
diff --git a/drivers/input/touchscreen/melfas_mip4.c b/drivers/input/touchscreen/melfas_mip4.c
index 247c3aa..d291a82 100644
--- a/drivers/input/touchscreen/melfas_mip4.c
+++ b/drivers/input/touchscreen/melfas_mip4.c
@@ -465,13 +465,9 @@ static void mip4_report_keys(struct mip4_ts *ts, u8 *packet)
static void mip4_report_touch(struct mip4_ts *ts, u8 *packet)
{
int id;
- bool hover;
- bool palm;
bool state;
u16 x, y;
- u8 pressure_stage = 0;
u8 pressure;
- u8 size;
u8 touch_major;
u8 touch_minor;
@@ -480,14 +476,11 @@ static void mip4_report_touch(struct mip4_ts *ts, u8 *packet)
case 1:
/* Touch only */
state = packet[0] & BIT(7);
- hover = packet[0] & BIT(5);
- palm = packet[0] & BIT(4);
id = (packet[0] & 0x0F) - 1;
x = ((packet[1] & 0x0F) << 8) | packet[2];
y = (((packet[1] >> 4) & 0x0F) << 8) |
packet[3];
pressure = packet[4];
- size = packet[5];
if (ts->event_format == 0) {
touch_major = packet[5];
touch_minor = packet[5];
@@ -501,14 +494,10 @@ static void mip4_report_touch(struct mip4_ts *ts, u8 *packet)
default:
/* Touch + Force(Pressure) */
id = (packet[0] & 0x0F) - 1;
- hover = packet[1] & BIT(2);
- palm = packet[1] & BIT(1);
state = packet[1] & BIT(0);
x = ((packet[2] & 0x0F) << 8) | packet[3];
y = (((packet[2] >> 4) & 0x0F) << 8) |
packet[4];
- size = packet[6];
- pressure_stage = (packet[7] & 0xF0) >> 4;
pressure = ((packet[7] & 0x0F) << 8) |
packet[8];
touch_major = packet[9];
--
2.7.4
^ permalink raw reply related
* Re: [RFC PATCH v2 0/4] Input: mpr121-polled: Add polled driver for MPR121
From: Michal Vokáč @ 2019-07-26 11:31 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Rob Herring, Mark Rutland, Shawn Guo, Sascha Hauer, Fabio Estevam,
linux-input, devicetree, linux-kernel, Pengutronix Kernel Team
In-Reply-To: <20190725144009.GA27432@penguin>
On 25. 07. 19 16:40, Dmitry Torokhov wrote:
> On Thu, Jul 25, 2019 at 02:58:02PM +0200, Michal Vokáč wrote:
>> On 25. 07. 19 10:57, Dmitry Torokhov wrote:
>>> Hi Michal,
>>>
>>> On Tue, May 21, 2019 at 08:51:17AM +0200, Michal Vokáč wrote:
>>>> On 21. 05. 19 7:37, Dmitry Torokhov wrote:
>>>>> Hi Michal,
>>>>>
>>>>> On Fri, May 17, 2019 at 03:12:49PM +0200, Michal Vokáč wrote:
>>>>>> Hi,
>>>>>>
>>>>>> I have to deal with a situation where we have a custom i.MX6 based
>>>>>> platform in production that uses the MPR121 touchkey controller.
>>>>>> Unfortunately the chip is connected using only the I2C interface.
>>>>>> The interrupt line is not used. Back in 2015 (Linux v3.14), my
>>>>>> colleague modded the existing mpr121_touchkey.c driver to use polling
>>>>>> instead of interrupt.
>>>>>>
>>>>>> For quite some time yet I am in a process of updating the product from
>>>>>> the ancient Freescale v3.14 kernel to the latest mainline and pushing
>>>>>> any needed changes upstream. The DT files for our imx6dl-yapp4 platform
>>>>>> already made it into v5.1-rc.
>>>>>>
>>>>>> I rebased and updated our mpr121 patch to the latest mainline.
>>>>>> It is created as a separate driver, similarly to gpio_keys_polled.
>>>>>>
>>>>>> The I2C device is quite susceptible to ESD. An ESD test quite often
>>>>>> causes reset of the chip or some register randomly changes its value.
>>>>>> The [PATCH 3/4] adds a write-through register cache. With the cache
>>>>>> this state can be detected and the device can be re-initialied.
>>>>>>
>>>>>> The main question is: Is there any chance that such a polled driver
>>>>>> could be accepted? Is it correct to implement it as a separate driver
>>>>>> or should it be done as an option in the existing driver? I can not
>>>>>> really imagine how I would do that though..
>>>>>>
>>>>>> There are also certain worries that the MPR121 chip may no longer be
>>>>>> available in nonspecifically distant future. In case of EOL I will need
>>>>>> to add a polled driver for an other touchkey chip. May it be already
>>>>>> in mainline or a completely new one.
>>>>>
>>>>> I think that my addition of input_polled_dev was ultimately a wrong
>>>>> thing to do. I am looking into enabling polling mode for regular input
>>>>> devices as we then can enable polling mode in existing drivers.
>>>>
>>>> OK, that sounds good. Especially when one needs to switch from one chip
>>>> to another that is already in tree, the need for a whole new polling
>>>> driver is eliminated.
>>>
>>> Could you please try the patch below and see if it works for your use
>>> case? Note that I have not tried running it, but it compiles so it must
>>> be good ;)
>>
>> Hi Dmitry,
>> Thank you very much for the patch!
>> I gave it a shot and it seems you forgot to add the input-poller.h file
>> to the patch.. it does not compile on my side :(
>
> Oops ;) Please see the updated patch below.
Thank you, now it is (almost) good as you said :D
>>
>>> Input: add support for polling to input devices
>>>
>>> From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>>>
>>> Separating "normal" and "polled" input devices was a mistake, as often we want
>>> to allow the very same device work on both interrupt-driven and polled mode,
>>> depending on the board on which the device is used.
>>>
>>> This introduces new APIs:
>>>
>>> - input_setup_polling
>>> - input_set_poll_interval
>>> - input_set_min_poll_interval
>>> - input_set_max_poll_interval
>>>
>>> These new APIs allow switching an input device into polled mode with sysfs
>>> attributes matching drivers using input_polled_dev APIs that will be eventually
>>> removed.
>>
>> After reading this I am not really sure what else needs to be done
>> to test/use the poller. I suspect I need to modify the input device
>> driver (mpr121_touchkey.c in my case) like this:
>>
>> If the interrupt gpio is not provided in DT, the device driver probe
>> function should:
>> - not request the threaded interrupt
>> - call input_setup_polling and provide it with poll_fn
>> Can the mpr_touchkey_interrupt function be used as is for this
>> purpose? The only problem I see is it returns IRQ_HANDLED.
>
> I'd factor out code suitable for polling from mpr_touchkey_interrupt()
> and then do
>
> static irqreturn_t mpr_touchkey_interrupt(...)
> {
> mpr_touchkey_report(...);
> return IRQ_HANDLED;
> }
>
Probably a trivial problem for experienced kernel hacker but I can not
wrap my head around this - the interrupt handler takes the mpr121
device id as an argument while the poller poll_fn takes struct input_dev.
I fail to figure out how to get the device id from the input device.
Here is what I have:
diff --git a/drivers/input/keyboard/mpr121_touchkey.c b/drivers/input/keyboard/mpr121_touchkey.c
index e9ceaa16b46a..1124f77ee10a 100644
--- a/drivers/input/keyboard/mpr121_touchkey.c
+++ b/drivers/input/keyboard/mpr121_touchkey.c
@@ -54,6 +54,10 @@
/* MPR121 has 12 keys */
#define MPR121_MAX_KEY_COUNT 12
+#define MPR121_POLL_INTERVAL 50
+#define MPR121_MIN_POLL_INTERVAL 10
+#define MPR121_MAX_POLL_INTERVAL 200
+
struct mpr121_touchkey {
struct i2c_client *client;
struct input_dev *input_dev;
@@ -115,9 +119,12 @@ static struct regulator *mpr121_vdd_supply_init(struct device *dev)
return vdd_supply;
}
-static irqreturn_t mpr_touchkey_interrupt(int irq, void *dev_id)
+static void mpr_touchkey_report(struct input_dev *dev)
{
- struct mpr121_touchkey *mpr121 = dev_id;
+ /*
+ * TODO the input_dev dev needs to be converted to mpr121 device id.
+ */
+ struct mpr121_touchkey *mpr121 = /* TODO */;
struct i2c_client *client = mpr121->client;
struct input_dev *input = mpr121->input_dev;
unsigned long bit_changed;
@@ -127,14 +134,14 @@ static irqreturn_t mpr_touchkey_interrupt(int irq, void *dev_id)
reg = i2c_smbus_read_byte_data(client, ELE_TOUCH_STATUS_1_ADDR);
if (reg < 0) {
dev_err(&client->dev, "i2c read error [%d]\n", reg);
- goto out;
+ return;
}
reg <<= 8;
reg |= i2c_smbus_read_byte_data(client, ELE_TOUCH_STATUS_0_ADDR);
if (reg < 0) {
dev_err(&client->dev, "i2c read error [%d]\n", reg);
- goto out;
+ return;
}
reg &= TOUCH_STATUS_MASK;
@@ -155,8 +162,17 @@ static irqreturn_t mpr_touchkey_interrupt(int irq, void *dev_id)
}
input_sync(input);
+}
+
+static irqreturn_t mpr_touchkey_interrupt(int irq, void *dev_id)
+{
+ /*
+ * TODO
+ * mpr_touchkey_report takes struct input_dev as an argument,
+ * not the device id.
+ */
+ mpr_touchkey_report(/* TODO */);
-out:
return IRQ_HANDLED;
}
@@ -232,11 +248,6 @@ static int mpr_touchkey_probe(struct i2c_client *client,
int error;
int i;
- if (!client->irq) {
- dev_err(dev, "irq number should not be zero\n");
- return -EINVAL;
- }
-
vdd_supply = mpr121_vdd_supply_init(dev);
if (IS_ERR(vdd_supply))
return PTR_ERR(vdd_supply);
@@ -289,13 +300,27 @@ static int mpr_touchkey_probe(struct i2c_client *client,
return error;
}
- error = devm_request_threaded_irq(dev, client->irq, NULL,
- mpr_touchkey_interrupt,
- IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
- dev->driver->name, mpr121);
- if (error) {
- dev_err(dev, "Failed to register interrupt\n");
- return error;
+ if (client->irq) {
+ error = devm_request_threaded_irq(dev, client->irq, NULL,
+ mpr_touchkey_interrupt,
+ IRQF_TRIGGER_FALLING |
+ IRQF_ONESHOT,
+ dev->driver->name, mpr121);
+ if (error) {
+ dev_err(dev, "Failed to register interrupt\n");
+ return error;
+ }
+ } else {
+ dev_dbg(dev, "invalid IRQ number, using polling mode\n");
+ error = input_setup_polling(input_dev, mpr_touchkey_report);
+ if (error)
+ return error;
+
+ input_set_poll_interval(input_dev, MPR121_POLL_INTERVAL);
+ input_set_min_poll_interval(input_dev,
+ MPR121_MIN_POLL_INTERVAL);
+ input_set_max_poll_interval(input_dev,
+ MPR121_MAX_POLL_INTERVAL);
}
error = input_register_device(input_dev);
--
^ permalink raw reply related
* [PATCH v2 10/10] docs: remove extra conf.py files
From: Mauro Carvalho Chehab @ 2019-07-26 11:31 UTC (permalink / raw)
Cc: Mauro Carvalho Chehab, Jonathan Corbet, Herbert Xu,
David S. Miller, Maarten Lankhorst, Maxime Ripard, Sean Paul,
David Airlie, Daniel Vetter, Dmitry Torokhov, Yoshinori Sato,
Rich Felker, Jaroslav Kysela, Takashi Iwai, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, H. Peter Anvin, x86, linux-doc,
linux-crypto, dri
In-Reply-To: <cover.1564139914.git.mchehab+samsung@kernel.org>
Now that the latex_documents are handled automatically, we can
remove those extra conf.py files.
Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
---
Documentation/admin-guide/conf.py | 10 ----------
Documentation/core-api/conf.py | 10 ----------
Documentation/crypto/conf.py | 10 ----------
Documentation/dev-tools/conf.py | 10 ----------
Documentation/doc-guide/conf.py | 10 ----------
Documentation/driver-api/80211/conf.py | 10 ----------
Documentation/driver-api/conf.py | 10 ----------
Documentation/driver-api/pm/conf.py | 10 ----------
Documentation/filesystems/conf.py | 10 ----------
Documentation/gpu/conf.py | 10 ----------
Documentation/input/conf.py | 10 ----------
Documentation/kernel-hacking/conf.py | 10 ----------
Documentation/maintainer/conf.py | 10 ----------
Documentation/media/conf.py | 12 ------------
Documentation/networking/conf.py | 10 ----------
Documentation/process/conf.py | 10 ----------
Documentation/sh/conf.py | 10 ----------
Documentation/sound/conf.py | 10 ----------
Documentation/userspace-api/conf.py | 10 ----------
Documentation/vm/conf.py | 10 ----------
Documentation/x86/conf.py | 10 ----------
21 files changed, 212 deletions(-)
delete mode 100644 Documentation/admin-guide/conf.py
delete mode 100644 Documentation/core-api/conf.py
delete mode 100644 Documentation/crypto/conf.py
delete mode 100644 Documentation/dev-tools/conf.py
delete mode 100644 Documentation/doc-guide/conf.py
delete mode 100644 Documentation/driver-api/80211/conf.py
delete mode 100644 Documentation/driver-api/conf.py
delete mode 100644 Documentation/driver-api/pm/conf.py
delete mode 100644 Documentation/filesystems/conf.py
delete mode 100644 Documentation/gpu/conf.py
delete mode 100644 Documentation/input/conf.py
delete mode 100644 Documentation/kernel-hacking/conf.py
delete mode 100644 Documentation/maintainer/conf.py
delete mode 100644 Documentation/media/conf.py
delete mode 100644 Documentation/networking/conf.py
delete mode 100644 Documentation/process/conf.py
delete mode 100644 Documentation/sh/conf.py
delete mode 100644 Documentation/sound/conf.py
delete mode 100644 Documentation/userspace-api/conf.py
delete mode 100644 Documentation/vm/conf.py
delete mode 100644 Documentation/x86/conf.py
diff --git a/Documentation/admin-guide/conf.py b/Documentation/admin-guide/conf.py
deleted file mode 100644
index 86f738953799..000000000000
--- a/Documentation/admin-guide/conf.py
+++ /dev/null
@@ -1,10 +0,0 @@
-# -*- coding: utf-8; mode: python -*-
-
-project = 'Linux Kernel User Documentation'
-
-tags.add("subproject")
-
-latex_documents = [
- ('index', 'linux-user.tex', 'Linux Kernel User Documentation',
- 'The kernel development community', 'manual'),
-]
diff --git a/Documentation/core-api/conf.py b/Documentation/core-api/conf.py
deleted file mode 100644
index db1f7659f3da..000000000000
--- a/Documentation/core-api/conf.py
+++ /dev/null
@@ -1,10 +0,0 @@
-# -*- coding: utf-8; mode: python -*-
-
-project = "Core-API Documentation"
-
-tags.add("subproject")
-
-latex_documents = [
- ('index', 'core-api.tex', project,
- 'The kernel development community', 'manual'),
-]
diff --git a/Documentation/crypto/conf.py b/Documentation/crypto/conf.py
deleted file mode 100644
index 4335d251ddf3..000000000000
--- a/Documentation/crypto/conf.py
+++ /dev/null
@@ -1,10 +0,0 @@
-# -*- coding: utf-8; mode: python -*-
-
-project = 'Linux Kernel Crypto API'
-
-tags.add("subproject")
-
-latex_documents = [
- ('index', 'crypto-api.tex', 'Linux Kernel Crypto API manual',
- 'The kernel development community', 'manual'),
-]
diff --git a/Documentation/dev-tools/conf.py b/Documentation/dev-tools/conf.py
deleted file mode 100644
index 7faafa3f7888..000000000000
--- a/Documentation/dev-tools/conf.py
+++ /dev/null
@@ -1,10 +0,0 @@
-# -*- coding: utf-8; mode: python -*-
-
-project = "Development tools for the kernel"
-
-tags.add("subproject")
-
-latex_documents = [
- ('index', 'dev-tools.tex', project,
- 'The kernel development community', 'manual'),
-]
diff --git a/Documentation/doc-guide/conf.py b/Documentation/doc-guide/conf.py
deleted file mode 100644
index fd3731182d5a..000000000000
--- a/Documentation/doc-guide/conf.py
+++ /dev/null
@@ -1,10 +0,0 @@
-# -*- coding: utf-8; mode: python -*-
-
-project = 'Linux Kernel Documentation Guide'
-
-tags.add("subproject")
-
-latex_documents = [
- ('index', 'kernel-doc-guide.tex', 'Linux Kernel Documentation Guide',
- 'The kernel development community', 'manual'),
-]
diff --git a/Documentation/driver-api/80211/conf.py b/Documentation/driver-api/80211/conf.py
deleted file mode 100644
index 4424b4b0b9c3..000000000000
--- a/Documentation/driver-api/80211/conf.py
+++ /dev/null
@@ -1,10 +0,0 @@
-# -*- coding: utf-8; mode: python -*-
-
-project = "Linux 802.11 Driver Developer's Guide"
-
-tags.add("subproject")
-
-latex_documents = [
- ('index', '80211.tex', project,
- 'The kernel development community', 'manual'),
-]
diff --git a/Documentation/driver-api/conf.py b/Documentation/driver-api/conf.py
deleted file mode 100644
index 202726d20088..000000000000
--- a/Documentation/driver-api/conf.py
+++ /dev/null
@@ -1,10 +0,0 @@
-# -*- coding: utf-8; mode: python -*-
-
-project = "The Linux driver implementer's API guide"
-
-tags.add("subproject")
-
-latex_documents = [
- ('index', 'driver-api.tex', project,
- 'The kernel development community', 'manual'),
-]
diff --git a/Documentation/driver-api/pm/conf.py b/Documentation/driver-api/pm/conf.py
deleted file mode 100644
index a89fac11272f..000000000000
--- a/Documentation/driver-api/pm/conf.py
+++ /dev/null
@@ -1,10 +0,0 @@
-# -*- coding: utf-8; mode: python -*-
-
-project = "Device Power Management"
-
-tags.add("subproject")
-
-latex_documents = [
- ('index', 'pm.tex', project,
- 'The kernel development community', 'manual'),
-]
diff --git a/Documentation/filesystems/conf.py b/Documentation/filesystems/conf.py
deleted file mode 100644
index ea44172af5c4..000000000000
--- a/Documentation/filesystems/conf.py
+++ /dev/null
@@ -1,10 +0,0 @@
-# -*- coding: utf-8; mode: python -*-
-
-project = "Linux Filesystems API"
-
-tags.add("subproject")
-
-latex_documents = [
- ('index', 'filesystems.tex', project,
- 'The kernel development community', 'manual'),
-]
diff --git a/Documentation/gpu/conf.py b/Documentation/gpu/conf.py
deleted file mode 100644
index 1757b040fb32..000000000000
--- a/Documentation/gpu/conf.py
+++ /dev/null
@@ -1,10 +0,0 @@
-# -*- coding: utf-8; mode: python -*-
-
-project = "Linux GPU Driver Developer's Guide"
-
-tags.add("subproject")
-
-latex_documents = [
- ('index', 'gpu.tex', project,
- 'The kernel development community', 'manual'),
-]
diff --git a/Documentation/input/conf.py b/Documentation/input/conf.py
deleted file mode 100644
index d2352fdc92ed..000000000000
--- a/Documentation/input/conf.py
+++ /dev/null
@@ -1,10 +0,0 @@
-# -*- coding: utf-8; mode: python -*-
-
-project = "The Linux input driver subsystem"
-
-tags.add("subproject")
-
-latex_documents = [
- ('index', 'linux-input.tex', project,
- 'The kernel development community', 'manual'),
-]
diff --git a/Documentation/kernel-hacking/conf.py b/Documentation/kernel-hacking/conf.py
deleted file mode 100644
index 3d8acf0f33ad..000000000000
--- a/Documentation/kernel-hacking/conf.py
+++ /dev/null
@@ -1,10 +0,0 @@
-# -*- coding: utf-8; mode: python -*-
-
-project = "Kernel Hacking Guides"
-
-tags.add("subproject")
-
-latex_documents = [
- ('index', 'kernel-hacking.tex', project,
- 'The kernel development community', 'manual'),
-]
diff --git a/Documentation/maintainer/conf.py b/Documentation/maintainer/conf.py
deleted file mode 100644
index 81e9eb7a7884..000000000000
--- a/Documentation/maintainer/conf.py
+++ /dev/null
@@ -1,10 +0,0 @@
-# -*- coding: utf-8; mode: python -*-
-
-project = 'Linux Kernel Development Documentation'
-
-tags.add("subproject")
-
-latex_documents = [
- ('index', 'maintainer.tex', 'Linux Kernel Development Documentation',
- 'The kernel development community', 'manual'),
-]
diff --git a/Documentation/media/conf.py b/Documentation/media/conf.py
deleted file mode 100644
index 1f194fcd2cae..000000000000
--- a/Documentation/media/conf.py
+++ /dev/null
@@ -1,12 +0,0 @@
-# -*- coding: utf-8; mode: python -*-
-
-# SPDX-License-Identifier: GPL-2.0
-
-project = 'Linux Media Subsystem Documentation'
-
-tags.add("subproject")
-
-latex_documents = [
- ('index', 'media.tex', 'Linux Media Subsystem Documentation',
- 'The kernel development community', 'manual'),
-]
diff --git a/Documentation/networking/conf.py b/Documentation/networking/conf.py
deleted file mode 100644
index 40f69e67a883..000000000000
--- a/Documentation/networking/conf.py
+++ /dev/null
@@ -1,10 +0,0 @@
-# -*- coding: utf-8; mode: python -*-
-
-project = "Linux Networking Documentation"
-
-tags.add("subproject")
-
-latex_documents = [
- ('index', 'networking.tex', project,
- 'The kernel development community', 'manual'),
-]
diff --git a/Documentation/process/conf.py b/Documentation/process/conf.py
deleted file mode 100644
index 1b01a80ad9ce..000000000000
--- a/Documentation/process/conf.py
+++ /dev/null
@@ -1,10 +0,0 @@
-# -*- coding: utf-8; mode: python -*-
-
-project = 'Linux Kernel Development Documentation'
-
-tags.add("subproject")
-
-latex_documents = [
- ('index', 'process.tex', 'Linux Kernel Development Documentation',
- 'The kernel development community', 'manual'),
-]
diff --git a/Documentation/sh/conf.py b/Documentation/sh/conf.py
deleted file mode 100644
index 1eb684a13ac8..000000000000
--- a/Documentation/sh/conf.py
+++ /dev/null
@@ -1,10 +0,0 @@
-# -*- coding: utf-8; mode: python -*-
-
-project = "SuperH architecture implementation manual"
-
-tags.add("subproject")
-
-latex_documents = [
- ('index', 'sh.tex', project,
- 'The kernel development community', 'manual'),
-]
diff --git a/Documentation/sound/conf.py b/Documentation/sound/conf.py
deleted file mode 100644
index 3f1fc5e74e7b..000000000000
--- a/Documentation/sound/conf.py
+++ /dev/null
@@ -1,10 +0,0 @@
-# -*- coding: utf-8; mode: python -*-
-
-project = "Linux Sound Subsystem Documentation"
-
-tags.add("subproject")
-
-latex_documents = [
- ('index', 'sound.tex', project,
- 'The kernel development community', 'manual'),
-]
diff --git a/Documentation/userspace-api/conf.py b/Documentation/userspace-api/conf.py
deleted file mode 100644
index 2eaf59f844e5..000000000000
--- a/Documentation/userspace-api/conf.py
+++ /dev/null
@@ -1,10 +0,0 @@
-# -*- coding: utf-8; mode: python -*-
-
-project = "The Linux kernel user-space API guide"
-
-tags.add("subproject")
-
-latex_documents = [
- ('index', 'userspace-api.tex', project,
- 'The kernel development community', 'manual'),
-]
diff --git a/Documentation/vm/conf.py b/Documentation/vm/conf.py
deleted file mode 100644
index 3b0b601af558..000000000000
--- a/Documentation/vm/conf.py
+++ /dev/null
@@ -1,10 +0,0 @@
-# -*- coding: utf-8; mode: python -*-
-
-project = "Linux Memory Management Documentation"
-
-tags.add("subproject")
-
-latex_documents = [
- ('index', 'memory-management.tex', project,
- 'The kernel development community', 'manual'),
-]
diff --git a/Documentation/x86/conf.py b/Documentation/x86/conf.py
deleted file mode 100644
index 33c5c3142e20..000000000000
--- a/Documentation/x86/conf.py
+++ /dev/null
@@ -1,10 +0,0 @@
-# -*- coding: utf-8; mode: python -*-
-
-project = "X86 architecture specific documentation"
-
-tags.add("subproject")
-
-latex_documents = [
- ('index', 'x86.tex', project,
- 'The kernel development community', 'manual'),
-]
--
2.21.0
^ permalink raw reply related
* Re: [PATCH v3] input: touchscreen: wm97xx-core: Fix possible null-pointer dereferences in wm97xx_ts_input_open()
From: Charles Keepax @ 2019-07-26 10:33 UTC (permalink / raw)
To: Jia-Ju Bai
Cc: dmitry.torokhov, allison, gregkh, tglx, rdunlap, patches,
linux-input, linux-kernel
In-Reply-To: <20190726102326.9266-1-baijiaju1990@gmail.com>
On Fri, Jul 26, 2019 at 06:23:26PM +0800, Jia-Ju Bai wrote:
> In wm97xx_ts_input_open(), there is an if statement on line 507 to check
> whether wm->mach_ops is NULL:
> if (wm->mach_ops && wm->mach_ops->acc_enabled)
>
> When wm->mach_ops is NULL, it is used on line 521:
> wm97xx_init_pen_irq(wm);
> BUG_ON(!wm->mach_ops->irq_enable);
> BUG_ON(!wm->mach_ops->irq_gpio);
> wm97xx_reg_write(..., reg & ~(wm->mach_ops->irq_gpio))
>
> Thus, possible null-pointer dereferences may occur.
>
> To fix these bugs, wm->mach_ops is checked at the beginning of
> wm97xx_init_pen_irq().
>
> These bugs found by a static analysis tool STCheck written by us.
>
> Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
> ---
> v2:
> * Add a new check of wm->mach_ops in wm97xx_init_pen_irq().
> Thank Charles for helpful advice.
>
> v3:
> * Print a message if wm->mach_ops is NULL in wm97xx_init_pen_irq().
> Thank Charles for helpful advice.
>
> ---
Acked-by: Charles Keepax <ckeepax@opensource.cirrus.com>
Thanks,
Charles
^ permalink raw reply
* [PATCH v3] input: touchscreen: wm97xx-core: Fix possible null-pointer dereferences in wm97xx_ts_input_open()
From: Jia-Ju Bai @ 2019-07-26 10:23 UTC (permalink / raw)
To: dmitry.torokhov, allison, gregkh, tglx, rdunlap
Cc: patches, linux-input, linux-kernel, Jia-Ju Bai
In wm97xx_ts_input_open(), there is an if statement on line 507 to check
whether wm->mach_ops is NULL:
if (wm->mach_ops && wm->mach_ops->acc_enabled)
When wm->mach_ops is NULL, it is used on line 521:
wm97xx_init_pen_irq(wm);
BUG_ON(!wm->mach_ops->irq_enable);
BUG_ON(!wm->mach_ops->irq_gpio);
wm97xx_reg_write(..., reg & ~(wm->mach_ops->irq_gpio))
Thus, possible null-pointer dereferences may occur.
To fix these bugs, wm->mach_ops is checked at the beginning of
wm97xx_init_pen_irq().
These bugs found by a static analysis tool STCheck written by us.
Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
---
v2:
* Add a new check of wm->mach_ops in wm97xx_init_pen_irq().
Thank Charles for helpful advice.
v3:
* Print a message if wm->mach_ops is NULL in wm97xx_init_pen_irq().
Thank Charles for helpful advice.
---
drivers/input/touchscreen/wm97xx-core.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/input/touchscreen/wm97xx-core.c b/drivers/input/touchscreen/wm97xx-core.c
index 0a174bd82915..bf754cb8965c 100644
--- a/drivers/input/touchscreen/wm97xx-core.c
+++ b/drivers/input/touchscreen/wm97xx-core.c
@@ -374,6 +374,11 @@ static int wm97xx_init_pen_irq(struct wm97xx *wm)
{
u16 reg;
+ if (!wm->mach_ops) {
+ dev_err(wm->dev, "mach_ops is NULL");
+ return -EINVAL;
+ }
+
/* If an interrupt is supplied an IRQ enable operation must also be
* provided. */
BUG_ON(!wm->mach_ops->irq_enable);
--
2.17.0
^ permalink raw reply related
* Re: [PATCH v2] input: touchscreen: wm97xx-core: Fix possible null-pointer dereferences in wm97xx_ts_input_open()
From: Charles Keepax @ 2019-07-26 9:28 UTC (permalink / raw)
To: Jia-Ju Bai
Cc: dmitry.torokhov, tglx, gregkh, allison, rdunlap, patches,
linux-input, linux-kernel
In-Reply-To: <20190726091436.8866-1-baijiaju1990@gmail.com>
On Fri, Jul 26, 2019 at 05:14:36PM +0800, Jia-Ju Bai wrote:
> In wm97xx_ts_input_open(), there is an if statement on line 507 to check
> whether wm->mach_ops is NULL:
> if (wm->mach_ops && wm->mach_ops->acc_enabled)
>
> When wm->mach_ops is NULL, it is used on line 521:
> wm97xx_init_pen_irq(wm);
> BUG_ON(!wm->mach_ops->irq_enable);
> BUG_ON(!wm->mach_ops->irq_gpio);
> wm97xx_reg_write(..., reg & ~(wm->mach_ops->irq_gpio))
>
> Thus, possible null-pointer dereferences may occur.
>
> To fix these bugs, wm->mach_ops is checked at the beginning of
> wm97xx_init_pen_irq().
>
> These bugs found by a static analysis tool STCheck written by us.
>
> Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
> ---
> v2:
> * Add a new check of wm->mach_ops in wm97xx_init_pen_irq().
> Thank Charles for helpful advice.
>
> ---
> drivers/input/touchscreen/wm97xx-core.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/input/touchscreen/wm97xx-core.c b/drivers/input/touchscreen/wm97xx-core.c
> index 0a174bd82915..50b016abf492 100644
> --- a/drivers/input/touchscreen/wm97xx-core.c
> +++ b/drivers/input/touchscreen/wm97xx-core.c
> @@ -374,6 +374,9 @@ static int wm97xx_init_pen_irq(struct wm97xx *wm)
> {
> u16 reg;
>
> + if (!wm->mach_ops)
> + return -EINVAL;
> +
Probably worth adding an printk in here too, the calling function
doesn't check the return value of this function so otherwise
there will be no indication this failed.
Thanks,
Charles
> /* If an interrupt is supplied an IRQ enable operation must also be
> * provided. */
> BUG_ON(!wm->mach_ops->irq_enable);
> --
> 2.17.0
>
^ permalink raw reply
* [PATCH v2] input: touchscreen: wm97xx-core: Fix possible null-pointer dereferences in wm97xx_ts_input_open()
From: Jia-Ju Bai @ 2019-07-26 9:14 UTC (permalink / raw)
To: dmitry.torokhov, tglx, gregkh, allison, rdunlap
Cc: patches, linux-input, linux-kernel, Jia-Ju Bai
In wm97xx_ts_input_open(), there is an if statement on line 507 to check
whether wm->mach_ops is NULL:
if (wm->mach_ops && wm->mach_ops->acc_enabled)
When wm->mach_ops is NULL, it is used on line 521:
wm97xx_init_pen_irq(wm);
BUG_ON(!wm->mach_ops->irq_enable);
BUG_ON(!wm->mach_ops->irq_gpio);
wm97xx_reg_write(..., reg & ~(wm->mach_ops->irq_gpio))
Thus, possible null-pointer dereferences may occur.
To fix these bugs, wm->mach_ops is checked at the beginning of
wm97xx_init_pen_irq().
These bugs found by a static analysis tool STCheck written by us.
Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
---
v2:
* Add a new check of wm->mach_ops in wm97xx_init_pen_irq().
Thank Charles for helpful advice.
---
drivers/input/touchscreen/wm97xx-core.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/input/touchscreen/wm97xx-core.c b/drivers/input/touchscreen/wm97xx-core.c
index 0a174bd82915..50b016abf492 100644
--- a/drivers/input/touchscreen/wm97xx-core.c
+++ b/drivers/input/touchscreen/wm97xx-core.c
@@ -374,6 +374,9 @@ static int wm97xx_init_pen_irq(struct wm97xx *wm)
{
u16 reg;
+ if (!wm->mach_ops)
+ return -EINVAL;
+
/* If an interrupt is supplied an IRQ enable operation must also be
* provided. */
BUG_ON(!wm->mach_ops->irq_enable);
--
2.17.0
^ permalink raw reply related
* Re: [PATCH] input: touchscreen: wm97xx-core: Fix possible null-pointer dereferences in wm97xx_ts_input_open()
From: Jia-Ju Bai @ 2019-07-26 9:08 UTC (permalink / raw)
To: Charles Keepax
Cc: dmitry.torokhov, gregkh, tglx, allison, rdunlap, patches,
linux-input, linux-kernel
In-Reply-To: <20190726090626.GA54126@ediswmail.ad.cirrus.com>
On 2019/7/26 17:06, Charles Keepax wrote:
> On Fri, Jul 26, 2019 at 04:48:16PM +0800, Jia-Ju Bai wrote:
>> In wm97xx_ts_input_open(), there is an if statement on line 507 to check
>> whether wm->mach_ops is NULL:
>> if (wm->mach_ops && wm->mach_ops->acc_enabled)
>>
>> When wm->mach_ops is NULL, it is used on line 521:
>> wm97xx_init_pen_irq(wm);
>> BUG_ON(!wm->mach_ops->irq_enable);
>> BUG_ON(!wm->mach_ops->irq_gpio);
>> wm97xx_reg_write(..., reg & ~(wm->mach_ops->irq_gpio))
>>
>> Thus, possible null-pointer dereferences may occur.
>>
>> To fix these bugs, wm->mach_ops is checked before calling
>> wm97xx_init_pen_irq().
>>
>> These bugs found by a static analysis tool STCheck written by us.
>>
>> Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
>> ---
>> drivers/input/touchscreen/wm97xx-core.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/input/touchscreen/wm97xx-core.c b/drivers/input/touchscreen/wm97xx-core.c
>> index 0a174bd82915..f7bd0726a577 100644
>> --- a/drivers/input/touchscreen/wm97xx-core.c
>> +++ b/drivers/input/touchscreen/wm97xx-core.c
>> @@ -517,7 +517,7 @@ static int wm97xx_ts_input_open(struct input_dev *idev)
>> wm->ts_reader_interval = wm->ts_reader_min_interval;
>>
>> wm->pen_is_down = 0;
>> - if (wm->pen_irq)
>> + if (wm->pen_irq && wm->mach_ops)
>> wm97xx_init_pen_irq(wm);
>> else
>> dev_err(wm->dev, "No IRQ specified\n");
> This doesn't quite feel like the right fix as it would print an
> error message saying "No IRQ specified", in the case the IRQ is
> specified but the mach_ops have not been set.
>
> I would suggest either extending the existing BUG_ON or adding a
> new check in wm97xx_init_pen_irq.
Thanks for the advice.
I will send a v2 patch of adding a new check in wm97xx_init_pen_irq().
Best wishes,
Jia-Ju Bai
^ permalink raw reply
* Re: [PATCH] input: touchscreen: wm97xx-core: Fix possible null-pointer dereferences in wm97xx_ts_input_open()
From: Charles Keepax @ 2019-07-26 9:06 UTC (permalink / raw)
To: Jia-Ju Bai
Cc: dmitry.torokhov, gregkh, tglx, allison, rdunlap, patches,
linux-input, linux-kernel
In-Reply-To: <20190726084816.8487-1-baijiaju1990@gmail.com>
On Fri, Jul 26, 2019 at 04:48:16PM +0800, Jia-Ju Bai wrote:
> In wm97xx_ts_input_open(), there is an if statement on line 507 to check
> whether wm->mach_ops is NULL:
> if (wm->mach_ops && wm->mach_ops->acc_enabled)
>
> When wm->mach_ops is NULL, it is used on line 521:
> wm97xx_init_pen_irq(wm);
> BUG_ON(!wm->mach_ops->irq_enable);
> BUG_ON(!wm->mach_ops->irq_gpio);
> wm97xx_reg_write(..., reg & ~(wm->mach_ops->irq_gpio))
>
> Thus, possible null-pointer dereferences may occur.
>
> To fix these bugs, wm->mach_ops is checked before calling
> wm97xx_init_pen_irq().
>
> These bugs found by a static analysis tool STCheck written by us.
>
> Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
> ---
> drivers/input/touchscreen/wm97xx-core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/input/touchscreen/wm97xx-core.c b/drivers/input/touchscreen/wm97xx-core.c
> index 0a174bd82915..f7bd0726a577 100644
> --- a/drivers/input/touchscreen/wm97xx-core.c
> +++ b/drivers/input/touchscreen/wm97xx-core.c
> @@ -517,7 +517,7 @@ static int wm97xx_ts_input_open(struct input_dev *idev)
> wm->ts_reader_interval = wm->ts_reader_min_interval;
>
> wm->pen_is_down = 0;
> - if (wm->pen_irq)
> + if (wm->pen_irq && wm->mach_ops)
> wm97xx_init_pen_irq(wm);
> else
> dev_err(wm->dev, "No IRQ specified\n");
This doesn't quite feel like the right fix as it would print an
error message saying "No IRQ specified", in the case the IRQ is
specified but the mach_ops have not been set.
I would suggest either extending the existing BUG_ON or adding a
new check in wm97xx_init_pen_irq.
Thanks,
Charles
> --
> 2.17.0
>
^ permalink raw reply
* [PATCH] input: touchscreen: wm97xx-core: Fix possible null-pointer dereferences in wm97xx_ts_input_open()
From: Jia-Ju Bai @ 2019-07-26 8:48 UTC (permalink / raw)
To: dmitry.torokhov, gregkh, tglx, allison, rdunlap
Cc: patches, linux-input, linux-kernel, Jia-Ju Bai
In wm97xx_ts_input_open(), there is an if statement on line 507 to check
whether wm->mach_ops is NULL:
if (wm->mach_ops && wm->mach_ops->acc_enabled)
When wm->mach_ops is NULL, it is used on line 521:
wm97xx_init_pen_irq(wm);
BUG_ON(!wm->mach_ops->irq_enable);
BUG_ON(!wm->mach_ops->irq_gpio);
wm97xx_reg_write(..., reg & ~(wm->mach_ops->irq_gpio))
Thus, possible null-pointer dereferences may occur.
To fix these bugs, wm->mach_ops is checked before calling
wm97xx_init_pen_irq().
These bugs found by a static analysis tool STCheck written by us.
Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
---
drivers/input/touchscreen/wm97xx-core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/touchscreen/wm97xx-core.c b/drivers/input/touchscreen/wm97xx-core.c
index 0a174bd82915..f7bd0726a577 100644
--- a/drivers/input/touchscreen/wm97xx-core.c
+++ b/drivers/input/touchscreen/wm97xx-core.c
@@ -517,7 +517,7 @@ static int wm97xx_ts_input_open(struct input_dev *idev)
wm->ts_reader_interval = wm->ts_reader_min_interval;
wm->pen_is_down = 0;
- if (wm->pen_irq)
+ if (wm->pen_irq && wm->mach_ops)
wm97xx_init_pen_irq(wm);
else
dev_err(wm->dev, "No IRQ specified\n");
--
2.17.0
^ permalink raw reply related
* Re: [PATCH v3 1/2] platform/x86: surfacepro3_button: Fix device check
From: Yu Chen @ 2019-07-26 1:48 UTC (permalink / raw)
To: Maximilian Luz
Cc: linux-kernel, linux-input, platform-driver-x86, Dmitry Torokhov,
Hans de Goede, Darren Hart, Andy Shevchenko, Benjamin Tissoires
In-Reply-To: <20190720150511.95076-2-luzmaximilian@gmail.com>
On Sat, Jul 20, 2019 at 05:05:10PM +0200, Maximilian Luz wrote:
> Do not use the surfacepro3_button driver on newer Microsoft Surface
> models, only use it on the Surface Pro 3 and 4. Newer models (5th, 6th
> and possibly future generations) use the same device as the Surface Pro
> 4 to represent their volume and power buttons (MSHW0040), but their
> actual implementation is significantly different. This patch ensures
> that the surfacepro3_button driver is only used on the Pro 3 and 4
> models, allowing a different driver to bind on other models.
>
> Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com>
> ---
Acked-by: Chen Yu <yu.c.chen@intel.com>
Thanks,
Chenyu
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox