* Re: [PATCH 1/1] hid: hid-sensor-custom: simplify getting .driver_data
From: Srinivas Pandruvada @ 2019-03-25 16:20 UTC (permalink / raw)
To: Wolfram Sang, linux-kernel
Cc: linux-renesas-soc, Jiri Kosina, Jonathan Cameron,
Benjamin Tissoires, linux-input, linux-iio
In-Reply-To: <20190319163638.31003-1-wsa+renesas@sang-engineering.com>
On Tue, 2019-03-19 at 17:36 +0100, Wolfram Sang wrote:
> We should get 'driver_data' from 'struct device' directly. Going via
> platform_device is an unneeded step back and forth.
>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> ---
>
> Build tested only. buildbot is happy.
>
> drivers/hid/hid-sensor-custom.c | 12 ++++--------
> 1 file changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/hid/hid-sensor-custom.c b/drivers/hid/hid-
> sensor-custom.c
> index bb012bc032e0..462e653a7bbb 100644
> --- a/drivers/hid/hid-sensor-custom.c
> +++ b/drivers/hid/hid-sensor-custom.c
> @@ -157,8 +157,7 @@ static int usage_id_cmp(const void *p1, const
> void *p2)
> static ssize_t enable_sensor_show(struct device *dev,
> struct device_attribute *attr, char
> *buf)
> {
> - struct platform_device *pdev = to_platform_device(dev);
> - struct hid_sensor_custom *sensor_inst =
> platform_get_drvdata(pdev);
> + struct hid_sensor_custom *sensor_inst = dev_get_drvdata(dev);
>
> return sprintf(buf, "%d\n", sensor_inst->enable);
> }
> @@ -237,8 +236,7 @@ static ssize_t enable_sensor_store(struct device
> *dev,
> struct device_attribute *attr,
> const char *buf, size_t count)
> {
> - struct platform_device *pdev = to_platform_device(dev);
> - struct hid_sensor_custom *sensor_inst =
> platform_get_drvdata(pdev);
> + struct hid_sensor_custom *sensor_inst = dev_get_drvdata(dev);
> int value;
> int ret = -EINVAL;
>
> @@ -283,8 +281,7 @@ static const struct attribute_group
> enable_sensor_attr_group = {
> static ssize_t show_value(struct device *dev, struct
> device_attribute *attr,
> char *buf)
> {
> - struct platform_device *pdev = to_platform_device(dev);
> - struct hid_sensor_custom *sensor_inst =
> platform_get_drvdata(pdev);
> + struct hid_sensor_custom *sensor_inst = dev_get_drvdata(dev);
> struct hid_sensor_hub_attribute_info *attribute;
> int index, usage, field_index;
> char name[HID_CUSTOM_NAME_LENGTH];
> @@ -392,8 +389,7 @@ static ssize_t show_value(struct device *dev,
> struct device_attribute *attr,
> static ssize_t store_value(struct device *dev, struct
> device_attribute *attr,
> const char *buf, size_t count)
> {
> - struct platform_device *pdev = to_platform_device(dev);
> - struct hid_sensor_custom *sensor_inst =
> platform_get_drvdata(pdev);
> + struct hid_sensor_custom *sensor_inst = dev_get_drvdata(dev);
> int index, field_index, usage;
> char name[HID_CUSTOM_NAME_LENGTH];
> int value;
^ permalink raw reply
* Re: [PATCH] ELAN touchpad i2c_hid bugs fix
From: Dmitry Torokhov @ 2019-03-25 16:02 UTC (permalink / raw)
To: Vladislav Dalechyn
Cc: Benjamin Tissoires, Jiri Kosina, kai.heng.feng, swboyd, bigeasy,
open list:HID CORE LAYER, lkml, hotwater438, Hans De Goede
In-Reply-To: <20190325125704.6585-1-hotwater438@tutanota.com>
Hi Vladislav,
On Mon, Mar 25, 2019 at 5:57 AM Vladislav Dalechyn
<vlad.dalechin@gmail.com> wrote:
>
> From: Vladislav Dalechyn <hotwater438@tutanota.com>
>
> Description: The ELAN1200:04F3:303E touchpad exposes several issues, all
> caused by an error setting the correct IRQ_TRIGGER flag:
> - i2c_hid incoplete error flood in journalctl;
> - Five finger tap kill's module so you have to restart it;
> - Two finger scoll is working incorrect and sometimes even when you
> raised one of two finger still thinks that you are scrolling.
>
> Fix all of these with a new quirk that corrects the trigger flag
> announced by the ACPI tables. (edge-falling).
I do not believe this is right solution. The driver makes liberal use
of disable_irq() and enable_irq() which may lead to lost edges and
touchpad stopping working altogether.
Usually the "extra" report is caused by GPIO controller clearing
interrupt condition at the wrong time (too early), or in unsafe or
racy fashion. You need to look there instead of adding quirk to
i2c-hid.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] HID: core: move Usage Page concatenation to hid_parser_main()
From: Nicolas Saenz Julienne @ 2019-03-25 15:12 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Jiri Kosina, oneukum, Junge, Terry, open list:HID CORE LAYER,
lkml
In-Reply-To: <CAO-hwJ+_CBJ1tkUpcedMM8dgbzyHk2GPdhnFxFN+bAckwQytmA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 6636 bytes --]
Hi Benjamin, Thanks for the review!
On Mon, 2019-03-25 at 16:08 +0100, Benjamin Tissoires wrote:
> On Mon, Mar 25, 2019 at 11:39 AM Benjamin Tissoires
> <benjamin.tissoires@redhat.com> wrote:
> > Hi Nicolas,
> >
> > On Tue, Mar 12, 2019 at 10:37 AM Nicolas Saenz Julienne
> > <nsaenzjulienne@suse.de> wrote:
> > > As seen on some USB wireless keyboards manufactured by Primax, the HID
> > > parser was using some assumptions that are not always true. In this case
> > > it's s the fact that, inside the scope of a main item, an Usage Page
> > > will always precede an Usage.
> > >
> > > The spec is not pretty clear as 6.2.2.7 states "Any usage that follows
> > > is interpreted as a Usage ID and concatenated with the Usage Page".
> > > While 6.2.2.8 states "When the parser encounters a main item it
> > > concatenates the last declared Usage Page with a Usage to form a
> > > complete usage value." Being somewhat contradictory it was decided to
> > > match Window's implementation, which follows 6.2.2.8.
> > >
> > > In summary, the patch moves the Usage Page concatenation from the local
> > > item parsing function to the main item parsing function.
> > >
> > > Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> > > ---
> >
> > Patch looks good to me.
> >
> > Terry, did you have time to review it?
> >
> > Cheers,
> > Benjamin
> >
> > > Note: A PR in hid-tools shoud show up anytime soon
> > >
> > > drivers/hid/hid-core.c | 30 ++++++++++++++++++------------
> > > include/linux/hid.h | 1 +
> > > 2 files changed, 19 insertions(+), 12 deletions(-)
> > >
> > > diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> > > index 9993b692598f..158468ef23a6 100644
> > > --- a/drivers/hid/hid-core.c
> > > +++ b/drivers/hid/hid-core.c
> > > @@ -218,13 +218,14 @@ static unsigned hid_lookup_collection(struct
> > > hid_parser *parser, unsigned type)
> > > * Add a usage to the temporary parser table.
> > > */
> > >
> > > -static int hid_add_usage(struct hid_parser *parser, unsigned usage)
> > > +static int hid_add_usage(struct hid_parser *parser, unsigned usage, __u8
> > > size)
> > > {
> > > if (parser->local.usage_index >= HID_MAX_USAGES) {
> > > hid_err(parser->device, "usage index exceeded\n");
> > > return -1;
> > > }
> > > parser->local.usage[parser->local.usage_index] = usage;
> > > + parser->local.usage_size[parser->local.usage_index] = size;
> > > parser->local.collection_index[parser->local.usage_index] =
> > > parser->collection_stack_ptr ?
> > > parser->collection_stack[parser->collection_stack_ptr - 1]
> > > : 0;
> > > @@ -486,10 +487,7 @@ static int hid_parser_local(struct hid_parser
> > > *parser, struct hid_item *item)
> > > return 0;
> > > }
> > >
> > > - if (item->size <= 2)
> > > - data = (parser->global.usage_page << 16) + data;
> > > -
> > > - return hid_add_usage(parser, data);
> > > + return hid_add_usage(parser, data, item->size);
> > >
> > > case HID_LOCAL_ITEM_TAG_USAGE_MINIMUM:
> > >
> > > @@ -498,9 +496,6 @@ static int hid_parser_local(struct hid_parser *parser,
> > > struct hid_item *item)
> > > return 0;
> > > }
> > >
> > > - if (item->size <= 2)
> > > - data = (parser->global.usage_page << 16) + data;
> > > -
> > > parser->local.usage_minimum = data;
> > > return 0;
> > >
> > > @@ -511,9 +506,6 @@ static int hid_parser_local(struct hid_parser *parser,
> > > struct hid_item *item)
> > > return 0;
> > > }
> > >
> > > - if (item->size <= 2)
> > > - data = (parser->global.usage_page << 16) + data;
> > > -
> > > count = data - parser->local.usage_minimum;
> > > if (count + parser->local.usage_index >= HID_MAX_USAGES) {
> > > /*
> > > @@ -533,7 +525,7 @@ static int hid_parser_local(struct hid_parser *parser,
> > > struct hid_item *item)
> > > }
> > >
> > > for (n = parser->local.usage_minimum; n <= data; n++)
> > > - if (hid_add_usage(parser, n)) {
> > > + if (hid_add_usage(parser, n, item->size)) {
> > > dbg_hid("hid_add_usage failed\n");
> > > return -1;
> > > }
> > > @@ -553,8 +545,22 @@ static int hid_parser_local(struct hid_parser
> > > *parser, struct hid_item *item)
> > >
> > > static int hid_parser_main(struct hid_parser *parser, struct hid_item
> > > *item)
> > > {
> > > + unsigned int usages;
> > > __u32 data;
> > > int ret;
> > > + int i;
> > > +
> > > + usages = max_t(unsigned, parser->local.usage_index,
> > > + parser->global.report_count);
> > > +
> > > + /*
> > > + * As per specification, 6.2.2.8:
> > > + * "When the parser encounters a main item it concatenates the
> > > last
> > > + * declared Usage Page with a Usage to form a complete usage
> > > value."
> > > + */
> > > + for (i = 0; i < usages; i++)
> > > + if (parser->local.usage_size[i] <= 2)
> > > + parser->local.usage[i] += parser-
> > > >global.usage_page << 16;
>
> Actually, the good thing of having a test suite, is that it raises bugs :)
>
> You are also missing the computation of the usage in hid_scan_main().
> This makes the autoloading of hid-multitouch fail, and thus the test
> suite failing.
Ok, I'll look into it and send a follow-up.
> Cheers,
> Benjamin
>
> > > data = item_udata(item);
> > >
> > > diff --git a/include/linux/hid.h b/include/linux/hid.h
> > > index f9707d1dcb58..d1fb4b678873 100644
> > > --- a/include/linux/hid.h
> > > +++ b/include/linux/hid.h
> > > @@ -417,6 +417,7 @@ struct hid_global {
> > >
> > > struct hid_local {
> > > unsigned usage[HID_MAX_USAGES]; /* usage array */
> > > + __u8 usage_size[HID_MAX_USAGES]; /* usage size array */
> > > unsigned collection_index[HID_MAX_USAGES]; /* collection index
> > > array */
> > > unsigned usage_index;
> > > unsigned usage_minimum;
> > > --
> > > 2.21.0
> > >
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH] HID: core: move Usage Page concatenation to hid_parser_main()
From: Benjamin Tissoires @ 2019-03-25 15:08 UTC (permalink / raw)
To: Nicolas Saenz Julienne
Cc: Jiri Kosina, oneukum, Junge, Terry, open list:HID CORE LAYER,
lkml
In-Reply-To: <CAO-hwJKo84vMDubVZM8Tye9wyGYkLC9sOVNUw=hUbzH1ktGv1A@mail.gmail.com>
On Mon, Mar 25, 2019 at 11:39 AM Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
>
> Hi Nicolas,
>
> On Tue, Mar 12, 2019 at 10:37 AM Nicolas Saenz Julienne
> <nsaenzjulienne@suse.de> wrote:
> >
> > As seen on some USB wireless keyboards manufactured by Primax, the HID
> > parser was using some assumptions that are not always true. In this case
> > it's s the fact that, inside the scope of a main item, an Usage Page
> > will always precede an Usage.
> >
> > The spec is not pretty clear as 6.2.2.7 states "Any usage that follows
> > is interpreted as a Usage ID and concatenated with the Usage Page".
> > While 6.2.2.8 states "When the parser encounters a main item it
> > concatenates the last declared Usage Page with a Usage to form a
> > complete usage value." Being somewhat contradictory it was decided to
> > match Window's implementation, which follows 6.2.2.8.
> >
> > In summary, the patch moves the Usage Page concatenation from the local
> > item parsing function to the main item parsing function.
> >
> > Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> > ---
>
> Patch looks good to me.
>
> Terry, did you have time to review it?
>
> Cheers,
> Benjamin
>
> >
> > Note: A PR in hid-tools shoud show up anytime soon
> >
> > drivers/hid/hid-core.c | 30 ++++++++++++++++++------------
> > include/linux/hid.h | 1 +
> > 2 files changed, 19 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> > index 9993b692598f..158468ef23a6 100644
> > --- a/drivers/hid/hid-core.c
> > +++ b/drivers/hid/hid-core.c
> > @@ -218,13 +218,14 @@ static unsigned hid_lookup_collection(struct hid_parser *parser, unsigned type)
> > * Add a usage to the temporary parser table.
> > */
> >
> > -static int hid_add_usage(struct hid_parser *parser, unsigned usage)
> > +static int hid_add_usage(struct hid_parser *parser, unsigned usage, __u8 size)
> > {
> > if (parser->local.usage_index >= HID_MAX_USAGES) {
> > hid_err(parser->device, "usage index exceeded\n");
> > return -1;
> > }
> > parser->local.usage[parser->local.usage_index] = usage;
> > + parser->local.usage_size[parser->local.usage_index] = size;
> > parser->local.collection_index[parser->local.usage_index] =
> > parser->collection_stack_ptr ?
> > parser->collection_stack[parser->collection_stack_ptr - 1] : 0;
> > @@ -486,10 +487,7 @@ static int hid_parser_local(struct hid_parser *parser, struct hid_item *item)
> > return 0;
> > }
> >
> > - if (item->size <= 2)
> > - data = (parser->global.usage_page << 16) + data;
> > -
> > - return hid_add_usage(parser, data);
> > + return hid_add_usage(parser, data, item->size);
> >
> > case HID_LOCAL_ITEM_TAG_USAGE_MINIMUM:
> >
> > @@ -498,9 +496,6 @@ static int hid_parser_local(struct hid_parser *parser, struct hid_item *item)
> > return 0;
> > }
> >
> > - if (item->size <= 2)
> > - data = (parser->global.usage_page << 16) + data;
> > -
> > parser->local.usage_minimum = data;
> > return 0;
> >
> > @@ -511,9 +506,6 @@ static int hid_parser_local(struct hid_parser *parser, struct hid_item *item)
> > return 0;
> > }
> >
> > - if (item->size <= 2)
> > - data = (parser->global.usage_page << 16) + data;
> > -
> > count = data - parser->local.usage_minimum;
> > if (count + parser->local.usage_index >= HID_MAX_USAGES) {
> > /*
> > @@ -533,7 +525,7 @@ static int hid_parser_local(struct hid_parser *parser, struct hid_item *item)
> > }
> >
> > for (n = parser->local.usage_minimum; n <= data; n++)
> > - if (hid_add_usage(parser, n)) {
> > + if (hid_add_usage(parser, n, item->size)) {
> > dbg_hid("hid_add_usage failed\n");
> > return -1;
> > }
> > @@ -553,8 +545,22 @@ static int hid_parser_local(struct hid_parser *parser, struct hid_item *item)
> >
> > static int hid_parser_main(struct hid_parser *parser, struct hid_item *item)
> > {
> > + unsigned int usages;
> > __u32 data;
> > int ret;
> > + int i;
> > +
> > + usages = max_t(unsigned, parser->local.usage_index,
> > + parser->global.report_count);
> > +
> > + /*
> > + * As per specification, 6.2.2.8:
> > + * "When the parser encounters a main item it concatenates the last
> > + * declared Usage Page with a Usage to form a complete usage value."
> > + */
> > + for (i = 0; i < usages; i++)
> > + if (parser->local.usage_size[i] <= 2)
> > + parser->local.usage[i] += parser->global.usage_page << 16;
Actually, the good thing of having a test suite, is that it raises bugs :)
You are also missing the computation of the usage in hid_scan_main().
This makes the autoloading of hid-multitouch fail, and thus the test
suite failing.
Cheers,
Benjamin
> >
> > data = item_udata(item);
> >
> > diff --git a/include/linux/hid.h b/include/linux/hid.h
> > index f9707d1dcb58..d1fb4b678873 100644
> > --- a/include/linux/hid.h
> > +++ b/include/linux/hid.h
> > @@ -417,6 +417,7 @@ struct hid_global {
> >
> > struct hid_local {
> > unsigned usage[HID_MAX_USAGES]; /* usage array */
> > + __u8 usage_size[HID_MAX_USAGES]; /* usage size array */
> > unsigned collection_index[HID_MAX_USAGES]; /* collection index array */
> > unsigned usage_index;
> > unsigned usage_minimum;
> > --
> > 2.21.0
> >
^ permalink raw reply
* [PATCH] ELAN touchpad i2c_hid bugs fix
From: Vladislav Dalechyn @ 2019-03-25 12:57 UTC (permalink / raw)
To: benjamin.tissoires, jikos, kai.heng.feng, swboyd, bigeasy, dtor,
linux-input, linux-kernel, hotwater438
Cc: Hans De Goede
From: Vladislav Dalechyn <hotwater438@tutanota.com>
Description: The ELAN1200:04F3:303E touchpad exposes several issues, all
caused by an error setting the correct IRQ_TRIGGER flag:
- i2c_hid incoplete error flood in journalctl;
- Five finger tap kill's module so you have to restart it;
- Two finger scoll is working incorrect and sometimes even when you
raised one of two finger still thinks that you are scrolling.
Fix all of these with a new quirk that corrects the trigger flag
announced by the ACPI tables. (edge-falling).
Reason behind moving i2c_hid_init_irq section described below:
i2c_hid_init_irq now checks for a quirk, so we must setup the quirks
before we init the irq, and we cannot setup the quirk earlier, so we must
init the irq later.
Co-developed-by: Hans De Goede <hdegoede@redhat.com>
Signed-off-by: Vladislav Dalechyn <hotwater438@tutanota.com>
---
drivers/hid/hid-ids.h | 1 +
drivers/hid/i2c-hid/i2c-hid-core.c | 25 +++++++++++++++----------
2 files changed, 16 insertions(+), 10 deletions(-)
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index b6d93f4ad037..660b4e0e912e 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -389,6 +389,7 @@
#define USB_DEVICE_ID_TOSHIBA_CLICK_L9W 0x0401
#define USB_DEVICE_ID_HP_X2 0x074d
#define USB_DEVICE_ID_HP_X2_10_COVER 0x0755
+#define I2C_DEVICE_ID_ELAN_TOUCHPAD 0x303e
#define USB_VENDOR_ID_ELECOM 0x056e
#define USB_DEVICE_ID_ELECOM_BM084 0x0061
diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
index 90164fed08d3..9b417914411f 100644
--- a/drivers/hid/i2c-hid/i2c-hid-core.c
+++ b/drivers/hid/i2c-hid/i2c-hid-core.c
@@ -51,6 +51,7 @@
#define I2C_HID_QUIRK_NO_RUNTIME_PM BIT(2)
#define I2C_HID_QUIRK_DELAY_AFTER_SLEEP BIT(3)
#define I2C_HID_QUIRK_BOGUS_IRQ BIT(4)
+#define I2C_HID_QUIRK_FORCE_TRIGGER_FALLING BIT(5)
/* flags */
#define I2C_HID_STARTED 0
@@ -182,8 +183,10 @@ static const struct i2c_hid_quirks {
I2C_HID_QUIRK_NO_RUNTIME_PM },
{ I2C_VENDOR_ID_GOODIX, I2C_DEVICE_ID_GOODIX_01F0,
I2C_HID_QUIRK_NO_RUNTIME_PM },
+ { USB_VENDOR_ID_ELAN, I2C_DEVICE_ID_ELAN_TOUCHPAD,
+ I2C_HID_QUIRK_BOGUS_IRQ | I2C_HID_QUIRK_FORCE_TRIGGER_FALLING },
{ USB_VENDOR_ID_ELAN, HID_ANY_ID,
- I2C_HID_QUIRK_BOGUS_IRQ },
+ I2C_HID_QUIRK_BOGUS_IRQ },
{ 0, 0 }
};
@@ -854,6 +857,8 @@ static int i2c_hid_init_irq(struct i2c_client *client)
if (!irq_get_trigger_type(client->irq))
irqflags = IRQF_TRIGGER_LOW;
+ if (ihid->quirks & I2C_HID_QUIRK_FORCE_TRIGGER_FALLING)
+ irqflags = IRQF_TRIGGER_FALLING;
ret = request_threaded_irq(client->irq, NULL, i2c_hid_irq,
irqflags | IRQF_ONESHOT, client->name, ihid);
@@ -1123,14 +1128,10 @@ static int i2c_hid_probe(struct i2c_client *client,
if (ret < 0)
goto err_pm;
- ret = i2c_hid_init_irq(client);
- if (ret < 0)
- goto err_pm;
-
hid = hid_allocate_device();
if (IS_ERR(hid)) {
ret = PTR_ERR(hid);
- goto err_irq;
+ goto err_pm;
}
ihid->hid = hid;
@@ -1149,11 +1150,15 @@ static int i2c_hid_probe(struct i2c_client *client,
ihid->quirks = i2c_hid_lookup_quirk(hid->vendor, hid->product);
+ ret = i2c_hid_init_irq(client);
+ if (ret < 0)
+ goto err_mem_free;
+
ret = hid_add_device(hid);
if (ret) {
if (ret != -ENODEV)
hid_err(client, "can't add hid device: %d\n", ret);
- goto err_mem_free;
+ goto err_irq;
}
if (!(ihid->quirks & I2C_HID_QUIRK_NO_RUNTIME_PM))
@@ -1161,12 +1166,12 @@ static int i2c_hid_probe(struct i2c_client *client,
return 0;
+err_irq:
+ free_irq(client->irq, ihid);
+
err_mem_free:
hid_destroy_device(hid);
-err_irq:
- free_irq(client->irq, ihid);
-
err_pm:
pm_runtime_put_noidle(&client->dev);
pm_runtime_disable(&client->dev);
--
2.20.1
^ permalink raw reply related
* Re: [PATCH] HID: core: move Usage Page concatenation to hid_parser_main()
From: Benjamin Tissoires @ 2019-03-25 10:39 UTC (permalink / raw)
To: Nicolas Saenz Julienne
Cc: Jiri Kosina, oneukum, Junge, Terry, open list:HID CORE LAYER,
lkml
In-Reply-To: <20190312093652.12829-1-nsaenzjulienne@suse.de>
Hi Nicolas,
On Tue, Mar 12, 2019 at 10:37 AM Nicolas Saenz Julienne
<nsaenzjulienne@suse.de> wrote:
>
> As seen on some USB wireless keyboards manufactured by Primax, the HID
> parser was using some assumptions that are not always true. In this case
> it's s the fact that, inside the scope of a main item, an Usage Page
> will always precede an Usage.
>
> The spec is not pretty clear as 6.2.2.7 states "Any usage that follows
> is interpreted as a Usage ID and concatenated with the Usage Page".
> While 6.2.2.8 states "When the parser encounters a main item it
> concatenates the last declared Usage Page with a Usage to form a
> complete usage value." Being somewhat contradictory it was decided to
> match Window's implementation, which follows 6.2.2.8.
>
> In summary, the patch moves the Usage Page concatenation from the local
> item parsing function to the main item parsing function.
>
> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> ---
Patch looks good to me.
Terry, did you have time to review it?
Cheers,
Benjamin
>
> Note: A PR in hid-tools shoud show up anytime soon
>
> drivers/hid/hid-core.c | 30 ++++++++++++++++++------------
> include/linux/hid.h | 1 +
> 2 files changed, 19 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index 9993b692598f..158468ef23a6 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -218,13 +218,14 @@ static unsigned hid_lookup_collection(struct hid_parser *parser, unsigned type)
> * Add a usage to the temporary parser table.
> */
>
> -static int hid_add_usage(struct hid_parser *parser, unsigned usage)
> +static int hid_add_usage(struct hid_parser *parser, unsigned usage, __u8 size)
> {
> if (parser->local.usage_index >= HID_MAX_USAGES) {
> hid_err(parser->device, "usage index exceeded\n");
> return -1;
> }
> parser->local.usage[parser->local.usage_index] = usage;
> + parser->local.usage_size[parser->local.usage_index] = size;
> parser->local.collection_index[parser->local.usage_index] =
> parser->collection_stack_ptr ?
> parser->collection_stack[parser->collection_stack_ptr - 1] : 0;
> @@ -486,10 +487,7 @@ static int hid_parser_local(struct hid_parser *parser, struct hid_item *item)
> return 0;
> }
>
> - if (item->size <= 2)
> - data = (parser->global.usage_page << 16) + data;
> -
> - return hid_add_usage(parser, data);
> + return hid_add_usage(parser, data, item->size);
>
> case HID_LOCAL_ITEM_TAG_USAGE_MINIMUM:
>
> @@ -498,9 +496,6 @@ static int hid_parser_local(struct hid_parser *parser, struct hid_item *item)
> return 0;
> }
>
> - if (item->size <= 2)
> - data = (parser->global.usage_page << 16) + data;
> -
> parser->local.usage_minimum = data;
> return 0;
>
> @@ -511,9 +506,6 @@ static int hid_parser_local(struct hid_parser *parser, struct hid_item *item)
> return 0;
> }
>
> - if (item->size <= 2)
> - data = (parser->global.usage_page << 16) + data;
> -
> count = data - parser->local.usage_minimum;
> if (count + parser->local.usage_index >= HID_MAX_USAGES) {
> /*
> @@ -533,7 +525,7 @@ static int hid_parser_local(struct hid_parser *parser, struct hid_item *item)
> }
>
> for (n = parser->local.usage_minimum; n <= data; n++)
> - if (hid_add_usage(parser, n)) {
> + if (hid_add_usage(parser, n, item->size)) {
> dbg_hid("hid_add_usage failed\n");
> return -1;
> }
> @@ -553,8 +545,22 @@ static int hid_parser_local(struct hid_parser *parser, struct hid_item *item)
>
> static int hid_parser_main(struct hid_parser *parser, struct hid_item *item)
> {
> + unsigned int usages;
> __u32 data;
> int ret;
> + int i;
> +
> + usages = max_t(unsigned, parser->local.usage_index,
> + parser->global.report_count);
> +
> + /*
> + * As per specification, 6.2.2.8:
> + * "When the parser encounters a main item it concatenates the last
> + * declared Usage Page with a Usage to form a complete usage value."
> + */
> + for (i = 0; i < usages; i++)
> + if (parser->local.usage_size[i] <= 2)
> + parser->local.usage[i] += parser->global.usage_page << 16;
>
> data = item_udata(item);
>
> diff --git a/include/linux/hid.h b/include/linux/hid.h
> index f9707d1dcb58..d1fb4b678873 100644
> --- a/include/linux/hid.h
> +++ b/include/linux/hid.h
> @@ -417,6 +417,7 @@ struct hid_global {
>
> struct hid_local {
> unsigned usage[HID_MAX_USAGES]; /* usage array */
> + __u8 usage_size[HID_MAX_USAGES]; /* usage size array */
> unsigned collection_index[HID_MAX_USAGES]; /* collection index array */
> unsigned usage_index;
> unsigned usage_minimum;
> --
> 2.21.0
>
^ permalink raw reply
* Re: [PATCH] ELAN touchpad i2c_hid bugs fix
From: Benjamin Tissoires @ 2019-03-25 9:23 UTC (permalink / raw)
To: Vladislav Dalechyn
Cc: Jiri Kosina, Kai Heng Feng, Stephen Boyd, bigeasy,
Dmitry Torokhov, open list:HID CORE LAYER, lkml, h0tw4t3r
In-Reply-To: <20190324191035.18705-1-hotwater438@tutanota.com>
Hi Vladislav,
we are almost there.
When submitting/applying a patch, we do run ./script/checkpatch.pl on
the final patch.
This gives us here 2 warnings (inlined below)
Also, can you versionize your submissions (use `-v` in git
format-patch: `git format-patch -v3 HEAD`).
On Sun, Mar 24, 2019 at 8:10 PM Vladislav Dalechyn
<vlad.dalechin@gmail.com> wrote:
>
> From: h0tw4t3r <hotwater438@tutanota.com>
checkpatch.pl complains here:
WARNING: Missing Signed-off-by: line by nominal patch author 'h0tw4t3r
<hotwater438@tutanota.com>'
Either:
- change your git settings and reset the author (git commit --amend
--reset-author)
- just drop this "From:" from the patch from git format-patch
>
> Description: The ELAN1200:04F3:303E touchpad exposes several issues, all
> caused by an error setting the correct IRQ_TRIGGER flag:
> - i2c_hid incoplete error flood in journalctl;
> - Five finger tap kill's module so you have to restart it;
> - Two finger scoll is working incorrect and sometimes even when you
> raised one of two finger still thinks that you are scrolling
>
> Fix all of these with a new quirk that corrects the trigger flag
> announced by the ACPI tables. (edge-falling).
>
> Reason behind moving i2c_hid_init_irq section described below:
> i2c_hid_init_irq now checks for a quirk, so we must setup the quirks
> before we init the irq, and we cannot setup the quirk earlier, so we must
> init the irq later.
>
> Signed-off-by: Vladislav Dalechyn <hotwater438@tutanota.com>
IIRC Andy said Co-developped-by: Hans de Goede <...> here. Does that stand Hans?
> ---
> drivers/hid/hid-ids.h | 1 +
> drivers/hid/i2c-hid/i2c-hid-core.c | 25 +++++++++++++++----------
> 2 files changed, 16 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index b6d93f4ad037..660b4e0e912e 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -389,6 +389,7 @@
> #define USB_DEVICE_ID_TOSHIBA_CLICK_L9W 0x0401
> #define USB_DEVICE_ID_HP_X2 0x074d
> #define USB_DEVICE_ID_HP_X2_10_COVER 0x0755
> +#define I2C_DEVICE_ID_ELAN_TOUCHPAD 0x303e
>
> #define USB_VENDOR_ID_ELECOM 0x056e
> #define USB_DEVICE_ID_ELECOM_BM084 0x0061
> diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
> index 90164fed08d3..9b417914411f 100644
> --- a/drivers/hid/i2c-hid/i2c-hid-core.c
> +++ b/drivers/hid/i2c-hid/i2c-hid-core.c
> @@ -51,6 +51,7 @@
> #define I2C_HID_QUIRK_NO_RUNTIME_PM BIT(2)
> #define I2C_HID_QUIRK_DELAY_AFTER_SLEEP BIT(3)
> #define I2C_HID_QUIRK_BOGUS_IRQ BIT(4)
> +#define I2C_HID_QUIRK_FORCE_TRIGGER_FALLING BIT(5)
>
> /* flags */
> #define I2C_HID_STARTED 0
> @@ -182,8 +183,10 @@ static const struct i2c_hid_quirks {
> I2C_HID_QUIRK_NO_RUNTIME_PM },
> { I2C_VENDOR_ID_GOODIX, I2C_DEVICE_ID_GOODIX_01F0,
> I2C_HID_QUIRK_NO_RUNTIME_PM },
> + { USB_VENDOR_ID_ELAN, I2C_DEVICE_ID_ELAN_TOUCHPAD,
> + I2C_HID_QUIRK_BOGUS_IRQ | I2C_HID_QUIRK_FORCE_TRIGGER_FALLING },
> { USB_VENDOR_ID_ELAN, HID_ANY_ID,
> - I2C_HID_QUIRK_BOGUS_IRQ },
> + I2C_HID_QUIRK_BOGUS_IRQ },
> { 0, 0 }
> };
>
> @@ -854,6 +857,8 @@ static int i2c_hid_init_irq(struct i2c_client *client)
>
> if (!irq_get_trigger_type(client->irq))
> irqflags = IRQF_TRIGGER_LOW;
> + if (ihid->quirks & I2C_HID_QUIRK_FORCE_TRIGGER_FALLING)
> + irqflags = IRQF_TRIGGER_FALLING;
>
> ret = request_threaded_irq(client->irq, NULL, i2c_hid_irq,
> irqflags | IRQF_ONESHOT, client->name, ihid);
> @@ -1123,14 +1128,10 @@ static int i2c_hid_probe(struct i2c_client *client,
> if (ret < 0)
> goto err_pm;
>
> - ret = i2c_hid_init_irq(client);
> - if (ret < 0)
> - goto err_pm;
> -
> hid = hid_allocate_device();
> if (IS_ERR(hid)) {
> ret = PTR_ERR(hid);
> - goto err_irq;
> + goto err_pm;
> }
>
> ihid->hid = hid;
> @@ -1149,11 +1150,15 @@ static int i2c_hid_probe(struct i2c_client *client,
>
> ihid->quirks = i2c_hid_lookup_quirk(hid->vendor, hid->product);
>
> + ret = i2c_hid_init_irq(client);
> + if (ret < 0)
> + goto err_mem_free;
> +
> ret = hid_add_device(hid);
> if (ret) {
> if (ret != -ENODEV)
> hid_err(client, "can't add hid device: %d\n", ret);
> - goto err_mem_free;
> + goto err_irq;
> }
>
> if (!(ihid->quirks & I2C_HID_QUIRK_NO_RUNTIME_PM))
> @@ -1161,12 +1166,12 @@ static int i2c_hid_probe(struct i2c_client *client,
>
> return 0;
>
> +err_irq:
> + free_irq(client->irq, ihid);
> +
WARNING: please, no spaces at the start of a line
#112: FILE: drivers/hid/i2c-hid/i2c-hid-core.c:1170:
+ free_irq(client->irq, ihid);$
Thanks for fixing these 3 minor issues, and we will be able to merge it :)
Cheers,
Benjamin
> err_mem_free:
> hid_destroy_device(hid);
>
> -err_irq:
> - free_irq(client->irq, ihid);
> -
> err_pm:
> pm_runtime_put_noidle(&client->dev);
> pm_runtime_disable(&client->dev);
> --
> 2.20.1
>
^ permalink raw reply
* [PATCH] ELAN touchpad i2c_hid bugs fix
From: Vladislav Dalechyn @ 2019-03-24 19:10 UTC (permalink / raw)
To: jikos, benjamin.tissoires, kai.heng.feng, swboyd, bigeasy, dtor,
linux-input, linux-kernel
Cc: h0tw4t3r
From: h0tw4t3r <hotwater438@tutanota.com>
Description: The ELAN1200:04F3:303E touchpad exposes several issues, all
caused by an error setting the correct IRQ_TRIGGER flag:
- i2c_hid incoplete error flood in journalctl;
- Five finger tap kill's module so you have to restart it;
- Two finger scoll is working incorrect and sometimes even when you
raised one of two finger still thinks that you are scrolling
Fix all of these with a new quirk that corrects the trigger flag
announced by the ACPI tables. (edge-falling).
Reason behind moving i2c_hid_init_irq section described below:
i2c_hid_init_irq now checks for a quirk, so we must setup the quirks
before we init the irq, and we cannot setup the quirk earlier, so we must
init the irq later.
Signed-off-by: Vladislav Dalechyn <hotwater438@tutanota.com>
---
drivers/hid/hid-ids.h | 1 +
drivers/hid/i2c-hid/i2c-hid-core.c | 25 +++++++++++++++----------
2 files changed, 16 insertions(+), 10 deletions(-)
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index b6d93f4ad037..660b4e0e912e 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -389,6 +389,7 @@
#define USB_DEVICE_ID_TOSHIBA_CLICK_L9W 0x0401
#define USB_DEVICE_ID_HP_X2 0x074d
#define USB_DEVICE_ID_HP_X2_10_COVER 0x0755
+#define I2C_DEVICE_ID_ELAN_TOUCHPAD 0x303e
#define USB_VENDOR_ID_ELECOM 0x056e
#define USB_DEVICE_ID_ELECOM_BM084 0x0061
diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
index 90164fed08d3..9b417914411f 100644
--- a/drivers/hid/i2c-hid/i2c-hid-core.c
+++ b/drivers/hid/i2c-hid/i2c-hid-core.c
@@ -51,6 +51,7 @@
#define I2C_HID_QUIRK_NO_RUNTIME_PM BIT(2)
#define I2C_HID_QUIRK_DELAY_AFTER_SLEEP BIT(3)
#define I2C_HID_QUIRK_BOGUS_IRQ BIT(4)
+#define I2C_HID_QUIRK_FORCE_TRIGGER_FALLING BIT(5)
/* flags */
#define I2C_HID_STARTED 0
@@ -182,8 +183,10 @@ static const struct i2c_hid_quirks {
I2C_HID_QUIRK_NO_RUNTIME_PM },
{ I2C_VENDOR_ID_GOODIX, I2C_DEVICE_ID_GOODIX_01F0,
I2C_HID_QUIRK_NO_RUNTIME_PM },
+ { USB_VENDOR_ID_ELAN, I2C_DEVICE_ID_ELAN_TOUCHPAD,
+ I2C_HID_QUIRK_BOGUS_IRQ | I2C_HID_QUIRK_FORCE_TRIGGER_FALLING },
{ USB_VENDOR_ID_ELAN, HID_ANY_ID,
- I2C_HID_QUIRK_BOGUS_IRQ },
+ I2C_HID_QUIRK_BOGUS_IRQ },
{ 0, 0 }
};
@@ -854,6 +857,8 @@ static int i2c_hid_init_irq(struct i2c_client *client)
if (!irq_get_trigger_type(client->irq))
irqflags = IRQF_TRIGGER_LOW;
+ if (ihid->quirks & I2C_HID_QUIRK_FORCE_TRIGGER_FALLING)
+ irqflags = IRQF_TRIGGER_FALLING;
ret = request_threaded_irq(client->irq, NULL, i2c_hid_irq,
irqflags | IRQF_ONESHOT, client->name, ihid);
@@ -1123,14 +1128,10 @@ static int i2c_hid_probe(struct i2c_client *client,
if (ret < 0)
goto err_pm;
- ret = i2c_hid_init_irq(client);
- if (ret < 0)
- goto err_pm;
-
hid = hid_allocate_device();
if (IS_ERR(hid)) {
ret = PTR_ERR(hid);
- goto err_irq;
+ goto err_pm;
}
ihid->hid = hid;
@@ -1149,11 +1150,15 @@ static int i2c_hid_probe(struct i2c_client *client,
ihid->quirks = i2c_hid_lookup_quirk(hid->vendor, hid->product);
+ ret = i2c_hid_init_irq(client);
+ if (ret < 0)
+ goto err_mem_free;
+
ret = hid_add_device(hid);
if (ret) {
if (ret != -ENODEV)
hid_err(client, "can't add hid device: %d\n", ret);
- goto err_mem_free;
+ goto err_irq;
}
if (!(ihid->quirks & I2C_HID_QUIRK_NO_RUNTIME_PM))
@@ -1161,12 +1166,12 @@ static int i2c_hid_probe(struct i2c_client *client,
return 0;
+err_irq:
+ free_irq(client->irq, ihid);
+
err_mem_free:
hid_destroy_device(hid);
-err_irq:
- free_irq(client->irq, ihid);
-
err_pm:
pm_runtime_put_noidle(&client->dev);
pm_runtime_disable(&client->dev);
--
2.20.1
^ permalink raw reply related
* Re: [PATCH] ELAN touchpad i2c_hid bugs fix
From: Hans de Goede @ 2019-03-24 18:37 UTC (permalink / raw)
To: hotwater438
Cc: Jikos, Benjamin Tissoires, Kai Heng Feng, Swboyd, Bigeasy, Dtor,
Linux Input, Linux Kernel
In-Reply-To: <LakgsCJ--3-1@tutanota.com>
Hi,
On 24-03-19 18:15, hotwater438@tutanota.com wrote:
> Hi,
>
> Sorry, I forgot to put the description.
>
> goto err_foo stuff changes were requested by Benjamin. Please tell if you think something is wrong.
>
> Regards, Vladislav.
>
> Here's the final patch:
>
> From 80576dd7ac193548ba747d287b5ab5606f642d00 Mon Sep 17 00:00:00 2001
> From: h0tw4t3r <hotwater438@tutanota.com <mailto:hotwater438@tutanota.com>>
> Date: Wed, 24 Mar 2019 19:14:22 +0200
> Subject: [PATCH] ELAN touchpad i2c_hid bugs fix
>
> Description: The ELAN1200:04F3:303E touchpad exposes several issues, all caused by
> an error setting the correct IRQ_TRIGGER flag:
> - i2c_hid incoplete error flood in journalctl;
> - Five finger tap kill's module so you have to restart it;
> - Two finger scoll is working incorrect and sometimes even when you raised one of two finger still thinks that you are scrolling
>
> Fix all of these with a new quirk that corrects the trigger flag
> announced by the ACPI tables. (edge-falling).
>
> Reason behind moving i2c_hid_init_irq section described below:
> i2c_hid_init_irq now checks for a quirk, so we must setup the quirks before we init the irq, and we cannot setup the quirk earlier, so we must init the irq later.
Some lines in your commit messages are longer then 75 chars, please
add an enter after aprox. 75 chars so that you have no lines longer
then 75 chars.
> Signed-off-by: Vladislav Dalechyn <hotwater438@tutanota.com <mailto:hotwater438@tutanota.com>>
> ---
> drivers/hid/hid-ids.h | 1 +
> drivers/hid/i2c-hid/i2c-hid-core.c | 25 +++++++++++++++----------
> 2 files changed, 16 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index b6d93f4ad037..660b4e0e912e 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -389,6 +389,7 @@
> #define USB_DEVICE_ID_TOSHIBA_CLICK_L9W 0x0401
> #define USB_DEVICE_ID_HP_X2 0x074d
> #define USB_DEVICE_ID_HP_X2_10_COVER 0x0755
> +#define I2C_DEVICE_ID_ELAN_TOUCHPAD 0x303e
>
> #define USB_VENDOR_ID_ELECOM 0x056e
> #define USB_DEVICE_ID_ELECOM_BM084 0x0061
> diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
> index 90164fed08d3..9b417914411f 100644
> --- a/drivers/hid/i2c-hid/i2c-hid-core.c
> +++ b/drivers/hid/i2c-hid/i2c-hid-core.c
> @@ -51,6 +51,7 @@
> #define I2C_HID_QUIRK_NO_RUNTIME_PM BIT(2)
> #define I2C_HID_QUIRK_DELAY_AFTER_SLEEP BIT(3)
> #define I2C_HID_QUIRK_BOGUS_IRQ BIT(4)
> +#define I2C_HID_QUIRK_FORCE_TRIGGER_FALLING BIT(5)
>
> /* flags */
> #define I2C_HID_STARTED 0
> @@ -182,8 +183,10 @@ static const struct i2c_hid_quirks {
> I2C_HID_QUIRK_NO_RUNTIME_PM },
> { I2C_VENDOR_ID_GOODIX, I2C_DEVICE_ID_GOODIX_01F0,
> I2C_HID_QUIRK_NO_RUNTIME_PM },
> + { USB_VENDOR_ID_ELAN, I2C_DEVICE_ID_ELAN_TOUCHPAD,
> + I2C_HID_QUIRK_BOGUS_IRQ | I2C_HID_QUIRK_FORCE_TRIGGER_FALLING },
> { USB_VENDOR_ID_ELAN, HID_ANY_ID,
> - I2C_HID_QUIRK_BOGUS_IRQ },
> + I2C_HID_QUIRK_BOGUS_IRQ },
> { 0, 0 }
> };
>
> @@ -854,6 +857,8 @@ static int i2c_hid_init_irq(struct i2c_client *client)
>
> if (!irq_get_trigger_type(client->irq))
> irqflags = IRQF_TRIGGER_LOW;
> + if (ihid->quirks & I2C_HID_QUIRK_FORCE_TRIGGER_FALLING)
> + irqflags = IRQF_TRIGGER_FALLING;
The patch has been completely mangled by your mail client, please
use "git send-email" to send a new version of the patch without
it getting mangled by your email client.
Regards,
Hans
^ permalink raw reply
* Re: [PATCH] HID: intel-ish-hid: ISH firmware loader client driver
From: Srinivas Pandruvada @ 2019-03-24 15:36 UTC (permalink / raw)
To: Rushikesh S Kadam, benjamin.tissoires, jikos
Cc: ncrews, jettrink, gwendal, linux-kernel, linux-input
In-Reply-To: <1553339772-25012-1-git-send-email-rushikesh.s.kadam@intel.com>
On Sat, 2019-03-23 at 16:46 +0530, Rushikesh S Kadam wrote:
> This driver adds support for loading Intel Integrated
> Sensor Hub (ISH) firmware from host file system to ISH
> SRAM and start execution.
>
> At power-on, the ISH subsystem shall boot to an interim
> Shim loader-firmware, which shall expose an ISHTP loader
> device.
>
> The driver implements an ISHTP client that communicates
> with the Shim ISHTP loader device over the intel-ish-hid
> stack, to download the main ISH firmware.
>
> Signed-off-by: Rushikesh S Kadam <rushikesh.s.kadam@intel.com>
Anybody in CC list here can add Reviewed-By/Tested-by tag?
One comment below for copyright year.
Thanks,
Srinivas
> ---
> The patches are baselined to hid git tree, branch for-5.2/ish
>
https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git/log/?h=for-5.2/ish
>
> drivers/hid/Makefile | 1 +
> drivers/hid/intel-ish-hid/Kconfig | 15 +
> drivers/hid/intel-ish-hid/Makefile | 3 +
> drivers/hid/intel-ish-hid/ishtp-fw-loader.c | 1103
> +++++++++++++++++++++++++++
> 4 files changed, 1122 insertions(+)
> create mode 100644 drivers/hid/intel-ish-hid/ishtp-fw-loader.c
>
> diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
> index 170163b..d8d393e 100644
> --- a/drivers/hid/Makefile
> +++ b/drivers/hid/Makefile
> @@ -134,3 +134,4 @@ obj-$(CONFIG_USB_KBD) += usbhid/
> obj-$(CONFIG_I2C_HID) += i2c-hid/
>
> obj-$(CONFIG_INTEL_ISH_HID) += intel-ish-hid/
> +obj-$(INTEL_ISH_FIRMWARE_DOWNLOADER) += intel-ish-hid/
> diff --git a/drivers/hid/intel-ish-hid/Kconfig b/drivers/hid/intel-
> ish-hid/Kconfig
> index 519e4c8..786adbc 100644
> --- a/drivers/hid/intel-ish-hid/Kconfig
> +++ b/drivers/hid/intel-ish-hid/Kconfig
> @@ -14,4 +14,19 @@ config INTEL_ISH_HID
> Broxton and Kaby Lake.
>
> Say Y here if you want to support Intel ISH. If unsure, say
> N.
> +
> +config INTEL_ISH_FIRMWARE_DOWNLOADER
> + tristate "Host Firmware Load feature for Intel ISH"
> + depends on INTEL_ISH_HID
> + depends on X86
> + help
> + The Integrated Sensor Hub (ISH) enables the kernel to offload
> + sensor polling and algorithm processing to a dedicated low
> power
> + processor in the chipset.
> +
> + The Host Firmware Load feature adds support to load the ISH
> + firmware from host file system at boot.
> +
> + Say M here if you want to support Host Firmware Loading
> feature
> + for Intel ISH. If unsure, say N.
> endmenu
> diff --git a/drivers/hid/intel-ish-hid/Makefile b/drivers/hid/intel-
> ish-hid/Makefile
> index 825b70a..2de97e4 100644
> --- a/drivers/hid/intel-ish-hid/Makefile
> +++ b/drivers/hid/intel-ish-hid/Makefile
> @@ -20,4 +20,7 @@ obj-$(CONFIG_INTEL_ISH_HID) += intel-ishtp-hid.o
> intel-ishtp-hid-objs := ishtp-hid.o
> intel-ishtp-hid-objs += ishtp-hid-client.o
>
> +obj-$(CONFIG_INTEL_ISH_FIRMWARE_DOWNLOADER) += intel-ishtp-loader.o
> +intel-ishtp-loader-objs += ishtp-fw-loader.o
> +
> ccflags-y += -Idrivers/hid/intel-ish-hid/ishtp
> diff --git a/drivers/hid/intel-ish-hid/ishtp-fw-loader.c
> b/drivers/hid/intel-ish-hid/ishtp-fw-loader.c
> new file mode 100644
> index 0000000..85d71d3
> --- /dev/null
> +++ b/drivers/hid/intel-ish-hid/ishtp-fw-loader.c
> @@ -0,0 +1,1103 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * ISH-TP client driver for ISH firmware loading
> + *
> + * Copyright (c) 2018, Intel Corporation.
Year 2019.
> + */
> +
> +#include <linux/firmware.h>
> +#include <linux/module.h>
> +#include <linux/pci.h>
> +#include <linux/intel-ish-client-if.h>
> +#include <linux/property.h>
> +#include <asm/cacheflush.h>
> +
> +/* ISH TX/RX ring buffer pool size */
> +#define LOADER_CL_RX_RING_SIZE 1
> +#define LOADER_CL_TX_RING_SIZE 1
> +
> +/*
> + * ISH Shim firmware loader reserves 4 Kb buffer in SRAM. The buffer
> is
> + * used to temporarily hold the data transferred from host to Shim
> firmware
> + * loader. Reason for the odd size of 3968 bytes? Each IPC transfer
> is 128
> + * bytes (= 4 bytes header + 124 bytes payload). So the 4 Kb buffer
> can
> + * hold maximum of 32 IPC transfers, which means we can have a max
> payload
> + * of 3968 bytes (= 32 x 124 payload).
> + */
> +#define LOADER_SHIM_IPC_BUF_SIZE 3968
> +
> +/**
> + * enum ish_loader_commands - ISH loader host commands.
> + * LOADER_CMD_XFER_QUERY Query the Shim firmware loader for
> capabilities
> + * LOADER_CMD_XFER_FRAGMENT Transfer one firmware image framgment
> at a
> + * time. The command may be executed
> multiple
> + * times until the entire firmware image
> is
> + * downloaded to SRAM.
> + * LOADER_CMD_START Start executing the main firmware.
> + */
> +enum ish_loader_commands {
> + LOADER_CMD_XFER_QUERY = 0,
> + LOADER_CMD_XFER_FRAGMENT,
> + LOADER_CMD_START,
> +};
> +
> +/* Command bit mask */
> +#define CMD_MASK GENMASK(6, 0)
> +#define IS_RESPONSE BIT(7)
> +
> +/*
> + * ISH firmware max delay for one transmit failure is 1 Hz,
> + * and firmware will retry 2 times, so 3 Hz is used for timeout.
> + */
> +#define ISHTP_SEND_TIMEOUT (3 * HZ)
> +
> +/*
> + * Loader transfer modes:
> + *
> + * LOADER_XFER_MODE_ISHTP mode uses the existing ISH-TP mechanims to
> + * transfer data. This may use IPC or DMA if supported in firmware.
> + * The buffer size is limited to 4 Kb by the IPC/ISH-TP protocol for
> + * both IPC & DMA (legacy).
> + *
> + * LOADER_XFER_MODE_DIRECT_DMA - firmware loading is a bit different
> + * from the sensor data streaming. Here we download a large (300+
> Kb)
> + * image directly to ISH SRAM memory. There is limited benefit of
> + * DMA'ing 300 Kb image in 4 Kb chucks limit. Hence, we introduce
> + * this "direct dma" mode, where we do not use ISH-TP for DMA, but
> + * instead manage the DMA directly in kernel driver and Shim
> firmware
> + * loader (allocate buf, break in chucks and transfer). This allows
> + * to overcome 4 Kb limit, and optimize the data flow path in
> firmware.
> + */
> +#define LOADER_XFER_MODE_DIRECT_DMA BIT(0)
> +#define LOADER_XFER_MODE_ISHTP BIT(1)
> +
> +/* ISH Transport Loader client unique GUID */
> +static const guid_t loader_ishtp_guid =
> + GUID_INIT(0xc804d06a, 0x55bd, 0x4ea7,
> + 0xad, 0xed, 0x1e, 0x31, 0x22, 0x8c, 0x76, 0xdc);
> +
> +#define FILENAME_SIZE 256
> +
> +/*
> + * The firmware loading latency will be minimum if we can DMA the
> + * entire ISH firmware image in one go. This requires that we
> allocate
> + * a large DMA buffer in kernel, which could be problematic on some
> + * platforms. So here we limit the DMA buf size via a module_param.
> + * We default to 4 pages, but a customer can set it to higher limit
> if
> + * deemed appropriate for his platform.
> + */
> +static int dma_buf_size_limit = 4 * PAGE_SIZE;
> +
> +/**
> + * struct loader_msg_hdr - Header for ISH Loader commands.
> + * @command: LOADER_CMD* commands. Bit 7 is the response.
> + * @status: Command response status. Non 0, is error
> condition.
> + *
> + * This structure is used as header for every command/data
> sent/received
> + * between Host driver and ISH Shim firmware loader.
> + */
> +struct loader_msg_hdr {
> + u8 command;
> + u8 reserved[2];
> + u8 status;
> +} __packed;
> +
> +struct loader_xfer_query {
> + struct loader_msg_hdr hdr;
> + u32 image_size;
> +} __packed;
> +
> +struct ish_fw_version {
> + u16 major;
> + u16 minor;
> + u16 hotfix;
> + u16 build;
> +} __packed;
> +
> +union loader_version {
> + u32 value;
> + struct {
> + u8 major;
> + u8 minor;
> + u8 hotfix;
> + u8 build;
> + };
> +} __packed;
> +
> +struct loader_capability {
> + u32 max_fw_image_size;
> + u32 xfer_mode;
> + u32 max_dma_buf_size; /* only for dma mode, multiples of
> cacheline */
> +} __packed;
> +
> +struct shim_fw_info {
> + struct ish_fw_version ish_fw_version;
> + u32 protocol_version;
> + union loader_version ldr_version;
> + struct loader_capability ldr_capability;
> +} __packed;
> +
> +struct loader_xfer_query_response {
> + struct loader_msg_hdr hdr;
> + struct shim_fw_info fw_info;
> +} __packed;
> +
> +struct loader_xfer_fragment {
> + struct loader_msg_hdr hdr;
> + u32 xfer_mode;
> + u32 offset;
> + u32 size;
> + u32 is_last;
> +} __packed;
> +
> +struct loader_xfer_ipc_fragment {
> + struct loader_xfer_fragment fragment;
> + u8 data[] ____cacheline_aligned; /* variable length payload
> here */
> +} __packed;
> +
> +struct loader_xfer_dma_fragment {
> + struct loader_xfer_fragment fragment;
> + u64 ddr_phys_addr;
> +} __packed;
> +
> +struct loader_start {
> + struct loader_msg_hdr hdr;
> +} __packed;
> +
> +/**
> + * struct ishtp_cl_data - Encapsulate per ISH-TP Client Data
> + * @flag_response Set true on receiving a firmware response to
> host
> + * loader command
> + * @cmd_resp_wait: Wait queue for Host firmware loading, where the
> + * client sends message to ISH firmware and wait
> for
> + * response
> + * @work_ishtp_reset: Work queue for reset handling
> + * @work_fw_load: Work queue for host firmware loading
> + * @flag_retry Flag for indicating host firmware
> loading should be
> + * retried
> + * @bad_recv_cnt: Running count of packets received with error
> + *
> + * This structure is used to store data per client
> + */
> +struct ishtp_cl_data {
> + struct ishtp_cl *loader_ishtp_cl;
> + struct ishtp_cl_device *cl_device;
> +
> + /* Completion flags */
> + bool flag_response;
> +
> + /* Copy buffer received in firmware "response" here */
> + void *response_data;
> + size_t response_size;
> +
> + /* Wait queue for ISH firmware message event */
> + wait_queue_head_t cmd_resp_wait;
> +
> + struct work_struct work_ishtp_reset;
> + struct work_struct work_fw_load;
> +
> + /*
> + * In certain failure scenrios, it makes sense to reset the
> + * the ISH subsystem and retry Host firmware loading
> + * (e.g. bad message packet, ENOMEM, etc.)
> + * On the other hand, failures due to protocol mismatch, etc
> + * are not recoverable. We do not retry.
> + *
> + * If set, the flag indictes that we should re-try the
> particular
> + * failure.
> + */
> + bool flag_retry;
> +
> + /* Statistics */
> + unsigned int bad_recv_cnt;
> +};
> +
> +#define IPC_FRAGMENT_DATA_PREAMBLE \
> + offsetof(struct loader_xfer_ipc_fragment, data)
> +
> +#define cl_data_to_dev(client_data) ishtp_device((client_data)-
> >cl_device)
> +
> +/**
> + * get_firmware_variant() - Gets the filename of firmware image to
> be
> + * loaded based on platform variant.
> + * @client_data Client data instance.
> + * @filename Returns firmware filename.
> + *
> + * Queries the firmware-name device property string.
> + *
> + * Return: 0 for success, negative error code for failure.
> + */
> +static int get_firmware_variant(struct ishtp_cl_data *client_data,
> + char *filename)
> +{
> + int rv;
> + const char *val;
> + struct device *devc = ishtp_get_pci_device(client_data-
> >cl_device);
> +
> + rv = device_property_read_string(devc, "firmware-name", &val);
> + if (rv < 0) {
> + dev_err(devc,
> + "Error: ISH firmware-name device property
> required\n");
> + return rv;
> + }
> + return snprintf(filename, FILENAME_SIZE, "intel/%s", val);
> +}
> +
> +/**
> + * report_bad_packets() Report bad packets
> + * @loader_ishtp_cl: Client instance to get stats
> + * @recv_buf: Raw received host interface message
> + *
> + * Dumps error in case bad packet is received
> + */
> +static void report_bad_packet(struct ishtp_cl *loader_ishtp_cl,
> + void *recv_buf)
> +{
> + struct loader_msg_hdr *hdr = recv_buf;
> + struct ishtp_cl_data *client_data =
> + ishtp_get_client_data(loader_ishtp_cl);
> +
> + client_data->bad_recv_cnt++;
> + dev_err(cl_data_to_dev(client_data),
> + "BAD packet: command=%02lx is_response=%u status=%02x
> total_bad=%u\n",
> + hdr->command & CMD_MASK,
> + hdr->command & IS_RESPONSE ? 1 : 0,
> + hdr->status,
> + client_data->bad_recv_cnt);
> +}
> +
> +/**
> + * loader_ish_hw_reset() - Reset ISH HW in bad state
> + * @loader_ishtp_cl Client instance to reset
> + *
> + * This function resets ISH hardware, which shall reload
> + * the Shim firmware loader, initiate ISH-TP interface reset,
> + * re-attach kernel loader driver, and repeat Host
> + * firmware load.
> + */
> +static inline void loader_ish_hw_reset(struct ishtp_cl
> *loader_ishtp_cl)
> +{
> + struct ishtp_cl_data *client_data =
> + ishtp_get_client_data(loader_ishtp_cl);
> +
> + dev_warn(cl_data_to_dev(client_data), "Reset the ISH
> subsystem\n");
> + ish_hw_reset(ishtp_get_ishtp_device(loader_ishtp_cl));
> +}
> +
> +/**
> + * loader_cl_send() Send message from host to firmware
> + * @client_data: Client data instance
> + * @msg Message buffer to send
> + * @msg_size Size of message
> + *
> + * Return: Received buffer size on success, negative error code on
> failure.
> + */
> +static int loader_cl_send(struct ishtp_cl_data *client_data,
> + u8 *msg, size_t msg_size)
> +{
> + int rv;
> + size_t data_len;
> + struct loader_msg_hdr *in_hdr;
> + struct loader_msg_hdr *out_hdr = (struct loader_msg_hdr *)msg;
> + struct ishtp_cl *loader_ishtp_cl = client_data-
> >loader_ishtp_cl;
> +
> + dev_dbg(cl_data_to_dev(client_data),
> + "%s: command=%02lx is_response=%u status=%02x\n",
> + __func__,
> + out_hdr->command & CMD_MASK,
> + out_hdr->command & IS_RESPONSE ? 1 : 0,
> + out_hdr->status);
> +
> + client_data->flag_response = false;
> + rv = ishtp_cl_send(loader_ishtp_cl, msg, msg_size);
> + if (rv < 0) {
> + dev_err(cl_data_to_dev(client_data),
> + "ishtp_cl_send error %d\n", rv);
> + return rv;
> + }
> +
> + wait_event_interruptible_timeout(client_data->cmd_resp_wait,
> + client_data->flag_response,
> + ISHTP_SEND_TIMEOUT);
> + if (!client_data->flag_response) {
> + dev_err(cl_data_to_dev(client_data),
> + "Timed out for response to command=%02lx",
> + out_hdr->command & CMD_MASK);
> + return -ETIMEDOUT;
> + }
> +
> + /* All response messages will contain a header */
> + data_len = client_data->response_size;
> + in_hdr = (struct loader_msg_hdr *)client_data->response_data;
> +
> + /* Sanity checks */
> + if (!(in_hdr->command & IS_RESPONSE)) {
> + dev_err(cl_data_to_dev(client_data),
> + "Invalid response to command\n");
> + return -EIO;
> + }
> +
> + if (in_hdr->status) {
> + dev_err(cl_data_to_dev(client_data),
> + "Loader returned status %d\n",
> + in_hdr->status);
> + return -EIO;
> + }
> +
> + return data_len;
> +}
> +
> +/**
> + * process_recv() - Receive and parse incoming packet
> + * @loader_ishtp_cl: Client instance to get stats
> + * @rb_in_proc: ISH received message buffer
> + *
> + * Parse the incoming packet. If it is a response packet then it
> will
> + * update flag_response and wake up the caller waiting to for the
> response.
> + */
> +static void process_recv(struct ishtp_cl *loader_ishtp_cl,
> + struct ishtp_cl_rb *rb_in_proc)
> +{
> + size_t data_len = rb_in_proc->buf_idx;
> + struct loader_msg_hdr *hdr =
> + (struct loader_msg_hdr *)rb_in_proc->buffer.data;
> + struct ishtp_cl_data *client_data =
> + ishtp_get_client_data(loader_ishtp_cl);
> +
> + /*
> + * All firmware messages have a header. Check buffer size
> + * before accessing elements inside.
> + */
> + if (data_len < sizeof(struct loader_msg_hdr)) {
> + dev_err(cl_data_to_dev(client_data),
> + "data size %zu is less than header %zu\n",
> + data_len, sizeof(struct loader_msg_hdr));
> + report_bad_packet(client_data->loader_ishtp_cl, hdr);
> + goto end_error;
> + }
> +
> + dev_dbg(cl_data_to_dev(client_data),
> + "%s: command=%02lx is_response=%u status=%02x\n",
> + __func__,
> + hdr->command & CMD_MASK,
> + hdr->command & IS_RESPONSE ? 1 : 0,
> + hdr->status);
> +
> + switch (hdr->command & CMD_MASK) {
> + case LOADER_CMD_XFER_QUERY:
> + case LOADER_CMD_XFER_FRAGMENT:
> + case LOADER_CMD_START:
> + /* Sanity check */
> + if (client_data->response_data || client_data-
> >flag_response) {
> + dev_err(cl_data_to_dev(client_data),
> + "Buffer overrun: previous firmware
> message not yet processed\n");
> + report_bad_packet(client_data->loader_ishtp_cl,
> hdr);
> + break;
> + }
> +
> + /*
> + * Copy the buffer received in firmware response for
> the
> + * calling thread.
> + */
> + client_data->response_data = kmalloc(data_len,
> GFP_KERNEL);
> + if (!client_data->response_data)
> + break;
> +
> + memcpy(client_data->response_data,
> + rb_in_proc->buffer.data, data_len);
> + client_data->response_size = data_len;
> +
> + /* Free the buffer */
> + ishtp_cl_io_rb_recycle(rb_in_proc);
> + rb_in_proc = NULL;
> +
> + /* Wake the calling thread */
> + client_data->flag_response = true;
> + wake_up_interruptible(&client_data->cmd_resp_wait);
> + break;
> +
> + default:
> + dev_err(cl_data_to_dev(client_data),
> + "Invalid command=%02lx\n",
> + hdr->command & CMD_MASK);
> + report_bad_packet(client_data->loader_ishtp_cl, hdr);
> + }
> +
> +end_error:
> + /* Free the buffer if we did not do above */
> + if (rb_in_proc)
> + ishtp_cl_io_rb_recycle(rb_in_proc);
> +}
> +
> +/**
> + * loader_cl_event_cb() - bus driver callback for incoming message
> + * @device: Pointer to the the ishtp client device for
> which
> + * this message is targeted
> + *
> + * Remove the packet from the list and process the message by
> calling
> + * process_recv
> + */
> +static void loader_cl_event_cb(struct ishtp_cl_device *cl_device)
> +{
> + struct ishtp_cl_rb *rb_in_proc;
> + struct ishtp_cl_data *client_data;
> + struct ishtp_cl *loader_ishtp_cl =
> ishtp_get_drvdata(cl_device);
> +
> + client_data = ishtp_get_client_data(loader_ishtp_cl);
> +
> + while ((rb_in_proc = ishtp_cl_rx_get_rb(loader_ishtp_cl)) !=
> NULL) {
> + if (!rb_in_proc->buffer.data) {
> + dev_warn(cl_data_to_dev(client_data),
> + "rb_in_proc->buffer.data returned
> null");
> + continue;
> + }
> +
> + /* Process the data packet from firmware */
> + process_recv(loader_ishtp_cl, rb_in_proc);
> + }
> +}
> +
> +/**
> + * ish_query_loader_prop() - Query ISH Shim firmware loader
> + * @client_data: Client data instance
> + * @fw: Poiner to fw data struct in host memory
> + *
> + * This function queries the ISH Shim firmware loader for
> capabilities.
> + *
> + * Return: 0 for success, negative error code for failure.
> + */
> +static int ish_query_loader_prop(struct ishtp_cl_data *client_data,
> + const struct firmware *fw,
> + struct shim_fw_info *fw_info)
> +{
> + int rv;
> + size_t data_len;
> + struct loader_msg_hdr *hdr;
> + struct loader_xfer_query ldr_xfer_query;
> + struct loader_xfer_query_response *ldr_xfer_query_resp;
> +
> + memset(&ldr_xfer_query, 0, sizeof(ldr_xfer_query));
> + ldr_xfer_query.hdr.command = LOADER_CMD_XFER_QUERY;
> + ldr_xfer_query.image_size = fw->size;
> + rv = loader_cl_send(client_data,
> + (u8 *)&ldr_xfer_query,
> + sizeof(ldr_xfer_query));
> + if (rv < 0) {
> + client_data->flag_retry = true;
> + goto end_error;
> + }
> +
> + /* Check buffer size before accessing the elements */
> + data_len = client_data->response_size;
> + if (data_len != sizeof(struct loader_xfer_query_response)) {
> + dev_err(cl_data_to_dev(client_data),
> + "data size %zu is not equal to size of
> loader_xfer_query_response %zu\n",
> + data_len, sizeof(struct
> loader_xfer_query_response));
> + hdr = (struct loader_msg_hdr *)client_data-
> >response_data;
> + report_bad_packet(client_data->loader_ishtp_cl, hdr);
> + client_data->flag_retry = true;
> + rv = -EMSGSIZE;
> + goto end_error;
> + }
> +
> + /* Save fw_info for use outside this function */
> + ldr_xfer_query_resp =
> + (struct loader_xfer_query_response *)client_data-
> >response_data;
> + *fw_info = ldr_xfer_query_resp->fw_info;
> +
> + /* Loader firmware properties */
> + dev_dbg(cl_data_to_dev(client_data),
> + "ish_fw_version: major=%d minor=%d hotfix=%d build=%d
> protocol_version=0x%x loader_version=%d\n",
> + fw_info->ish_fw_version.major,
> + fw_info->ish_fw_version.minor,
> + fw_info->ish_fw_version.hotfix,
> + fw_info->ish_fw_version.build,
> + fw_info->protocol_version,
> + fw_info->ldr_version.value);
> +
> + dev_dbg(cl_data_to_dev(client_data),
> + "loader_capability: max_fw_image_size=0x%x xfer_mode=%d
> max_dma_buf_size=0x%x dma_buf_size_limit=0x%x\n",
> + fw_info->ldr_capability.max_fw_image_size,
> + fw_info->ldr_capability.xfer_mode,
> + fw_info->ldr_capability.max_dma_buf_size,
> + dma_buf_size_limit);
> +
> + /* Sanity checks */
> + if (fw_info->ldr_capability.max_fw_image_size < fw->size) {
> + dev_err(cl_data_to_dev(client_data),
> + "ISH firmware size %zu is greater than Shim
> firmware loader max supported %d\n",
> + fw->size,
> + fw_info->ldr_capability.max_fw_image_size);
> + rv = -ENOSPC;
> + goto end_error;
> + }
> +
> + /* For DMA the buffer size should be multiple of cacheline size
> */
> + if ((fw_info->ldr_capability.xfer_mode &
> LOADER_XFER_MODE_DIRECT_DMA) &&
> + (fw_info->ldr_capability.max_dma_buf_size %
> L1_CACHE_BYTES)) {
> + dev_err(cl_data_to_dev(client_data),
> + "Shim firmware loader buffer size %d should be
> multipe of cacheline\n",
> + fw_info->ldr_capability.max_dma_buf_size);
> + rv = -EINVAL;
> + goto end_error;
> + }
> +
> +end_error:
> + /* Free ISH buffer if not done so in error case */
> + kfree(client_data->response_data);
> + client_data->response_data = NULL;
> + return rv;
> +}
> +
> +/**
> + * ish_fw_xfer_ishtp() Loads ISH firmware using ishtp
> interface
> + * @client_data: Client data instance
> + * @fw: Pointer to fw data struct in host
> memory
> + *
> + * This function uses ISH-TP to transfer ISH firmware from host to
> + * ISH SRAM. Lower layers may use IPC or DMA depending on firmware
> + * support.
> + *
> + * Return: 0 for success, negative error code for failure.
> + */
> +static int ish_fw_xfer_ishtp(struct ishtp_cl_data *client_data,
> + const struct firmware *fw)
> +{
> + int rv;
> + u32 fragment_offset, fragment_size, payload_max_size;
> + struct loader_xfer_ipc_fragment *ldr_xfer_ipc_frag;
> +
> + payload_max_size =
> + LOADER_SHIM_IPC_BUF_SIZE - IPC_FRAGMENT_DATA_PREAMBLE;
> +
> + ldr_xfer_ipc_frag = kzalloc(LOADER_SHIM_IPC_BUF_SIZE,
> GFP_KERNEL);
> + if (!ldr_xfer_ipc_frag) {
> + client_data->flag_retry = true;
> + return -ENOMEM;
> + }
> +
> + ldr_xfer_ipc_frag->fragment.hdr.command =
> LOADER_CMD_XFER_FRAGMENT;
> + ldr_xfer_ipc_frag->fragment.xfer_mode = LOADER_XFER_MODE_ISHTP;
> +
> + /* Break the firmware image into fragments and send as ISH-TP
> payload */
> + fragment_offset = 0;
> + while (fragment_offset < fw->size) {
> + if (fragment_offset + payload_max_size < fw->size) {
> + fragment_size = payload_max_size;
> + ldr_xfer_ipc_frag->fragment.is_last = 0;
> + } else {
> + fragment_size = fw->size - fragment_offset;
> + ldr_xfer_ipc_frag->fragment.is_last = 1;
> + }
> +
> + ldr_xfer_ipc_frag->fragment.offset = fragment_offset;
> + ldr_xfer_ipc_frag->fragment.size = fragment_size;
> + memcpy(ldr_xfer_ipc_frag->data,
> + &fw->data[fragment_offset],
> + fragment_size);
> +
> + dev_dbg(cl_data_to_dev(client_data),
> + "xfer_mode=ipc offset=0x%08x size=0x%08x
> is_last=%d\n",
> + ldr_xfer_ipc_frag->fragment.offset,
> + ldr_xfer_ipc_frag->fragment.size,
> + ldr_xfer_ipc_frag->fragment.is_last);
> +
> + rv = loader_cl_send(client_data,
> + (u8 *)ldr_xfer_ipc_frag,
> + IPC_FRAGMENT_DATA_PREAMBLE +
> fragment_size);
> + if (rv < 0) {
> + client_data->flag_retry = true;
> + goto end_err_resp_buf_release;
> + }
> +
> + /* Free ISH buffer once response is processed */
> + kfree(client_data->response_data);
> + client_data->response_data = NULL;
> +
> + fragment_offset += fragment_size;
> + }
> +
> + kfree(ldr_xfer_ipc_frag);
> + return 0;
> +
> +end_err_resp_buf_release:
> + /* Free ISH buffer if not done already, in error case */
> + kfree(client_data->response_data);
> + client_data->response_data = NULL;
> + kfree(ldr_xfer_ipc_frag);
> + return rv;
> +}
> +
> +/**
> + * ish_fw_xfer_direct_dma() - Loads ISH firmware using direct dma
> + * @client_data: Client data instance
> + * @fw: Poiner to fw data struct in host memory
> + *
> + * Host firmware load is a unique case where we need to download
> + * a large firmware image (200+ Kb). This function implements
> + * direct DMA transfer in kernel and ISH firmware. This allows
> + * us to overcome the ISH-TP 4 Kb limit, and allows us to DMA
> + * directly to ISH UMA at location of choice.
> + * Function depends on corresponding support in ISH firmware.
> + *
> + * Return: 0 for success, negative error code for failure.
> + */
> +static int ish_fw_xfer_direct_dma(struct ishtp_cl_data *client_data,
> + const struct firmware *fw,
> + struct shim_fw_info fw_info)
> +{
> + int rv;
> + void *dma_buf;
> + dma_addr_t dma_buf_phy;
> + u32 fragment_offset, fragment_size, payload_max_size;
> + struct loader_xfer_dma_fragment ldr_xfer_dma_frag;
> + struct device *devc = ishtp_get_pci_device(client_data-
> >cl_device);
> + u32 shim_fw_buf_size =
> + fw_info.ldr_capability.max_dma_buf_size;
> +
> + /*
> + * payload_max_size should be set to minimum of
> + * (1) Size of firmware to be loaded,
> + * (2) Max DMA buf size supported by Shim firmware,
> + * (3) DMA buffer size limit set by boot_param
> dma_buf_size_limit.
> + */
> + payload_max_size = min3(fw->size,
> + (size_t)shim_fw_buf_size,
> + (size_t)dma_buf_size_limit);
> +
> + /*
> + * Buffer size should be multiple of cacheline size
> + * if it's not, select the previous cacheline boundary.
> + */
> + payload_max_size &= ~(L1_CACHE_BYTES - 1);
> +
> + dma_buf = kmalloc(payload_max_size, GFP_KERNEL | GFP_DMA32);
> + if (!dma_buf) {
> + client_data->flag_retry = true;
> + return -ENOMEM;
> + }
> +
> + dma_buf_phy = dma_map_single(devc, dma_buf, payload_max_size,
> + DMA_TO_DEVICE);
> + if (dma_mapping_error(devc, dma_buf_phy)) {
> + dev_err(cl_data_to_dev(client_data), "DMA map
> failed\n");
> + client_data->flag_retry = true;
> + rv = -ENOMEM;
> + goto end_err_dma_buf_release;
> + }
> +
> + ldr_xfer_dma_frag.fragment.hdr.command =
> LOADER_CMD_XFER_FRAGMENT;
> + ldr_xfer_dma_frag.fragment.xfer_mode =
> LOADER_XFER_MODE_DIRECT_DMA;
> + ldr_xfer_dma_frag.ddr_phys_addr = (u64)dma_buf_phy;
> +
> + /* Send the firmware image in chucks of payload_max_size */
> + fragment_offset = 0;
> + while (fragment_offset < fw->size) {
> + if (fragment_offset + payload_max_size < fw->size) {
> + fragment_size = payload_max_size;
> + ldr_xfer_dma_frag.fragment.is_last = 0;
> + } else {
> + fragment_size = fw->size - fragment_offset;
> + ldr_xfer_dma_frag.fragment.is_last = 1;
> + }
> +
> + ldr_xfer_dma_frag.fragment.offset = fragment_offset;
> + ldr_xfer_dma_frag.fragment.size = fragment_size;
> + memcpy(dma_buf, &fw->data[fragment_offset],
> fragment_size);
> +
> + dma_sync_single_for_device(devc, dma_buf_phy,
> + payload_max_size,
> + DMA_TO_DEVICE);
> +
> + /*
> + * Flush cache here because the
> dma_sync_single_for_device()
> + * does not do for x86.
> + */
> + clflush_cache_range(dma_buf, payload_max_size);
> +
> + dev_dbg(cl_data_to_dev(client_data),
> + "xfer_mode=dma offset=0x%08x size=0x%x
> is_last=%d ddr_phys_addr=0x%016llx\n",
> + ldr_xfer_dma_frag.fragment.offset,
> + ldr_xfer_dma_frag.fragment.size,
> + ldr_xfer_dma_frag.fragment.is_last,
> + ldr_xfer_dma_frag.ddr_phys_addr);
> +
> + rv = loader_cl_send(client_data,
> + (u8 *)&ldr_xfer_dma_frag,
> + sizeof(ldr_xfer_dma_frag));
> + if (rv < 0) {
> + client_data->flag_retry = true;
> + goto end_err_resp_buf_release;
> + }
> +
> + /* Free ISH buffer once response is processed */
> + kfree(client_data->response_data);
> + client_data->response_data = NULL;
> +
> + fragment_offset += fragment_size;
> + }
> +
> + dma_unmap_single(devc, dma_buf_phy, payload_max_size,
> DMA_TO_DEVICE);
> + kfree(dma_buf);
> + return 0;
> +
> +end_err_resp_buf_release:
> + /* Free ISH buffer if not done already, in error case */
> + kfree(client_data->response_data);
> + client_data->response_data = NULL;
> + dma_unmap_single(devc, dma_buf_phy, payload_max_size,
> DMA_TO_DEVICE);
> +end_err_dma_buf_release:
> + kfree(dma_buf);
> + return rv;
> +}
> +
> +/**
> + * ish_fw_start() Start executing ISH main firmware
> + * @client_data: client data instance
> + *
> + * This function sends message to Shim firmware loader to start
> + * the execution of ISH main firmware.
> + *
> + * Return: 0 for success, negative error code for failure.
> + */
> +static int ish_fw_start(struct ishtp_cl_data *client_data)
> +{
> + int rv;
> + struct loader_start ldr_start;
> +
> + memset(&ldr_start, 0, sizeof(ldr_start));
> + ldr_start.hdr.command = LOADER_CMD_START;
> + rv = loader_cl_send(client_data,
> + (u8 *)&ldr_start,
> + sizeof(ldr_start));
> +
> + /* Free ISH buffer once response is processed */
> + kfree(client_data->response_data);
> + client_data->response_data = NULL;
> + return rv;
> +}
> +
> +/**
> + * load_fw_from_host() Loads ISH firmware from host
> + * @client_data: Client data instance
> + *
> + * This function loads the ISH firmware to ISH sram and starts
> execution
> + *
> + * Return: 0 for success, negative error code for failure.
> + */
> +static int load_fw_from_host(struct ishtp_cl_data *client_data)
> +{
> + int rv;
> + u32 xfer_mode;
> + char *filename;
> + const struct firmware *fw;
> + struct shim_fw_info fw_info;
> +
> + client_data->flag_retry = false;
> +
> + filename = kzalloc(FILENAME_SIZE, GFP_KERNEL);
> + if (!filename) {
> + rv = -ENOMEM;
> + goto end_error;
> + }
> +
> + /* Get filename of the ISH firmware to be loaded */
> + rv = get_firmware_variant(client_data, filename);
> + if (rv < 0)
> + goto end_err_filename_buf_release;
> +
> + rv = request_firmware(&fw, filename,
> cl_data_to_dev(client_data));
> + if (rv < 0)
> + goto end_err_filename_buf_release;
> +
> + /* Step 1: Query Shim firmware loader properties */
> +
> + rv = ish_query_loader_prop(client_data, fw, &fw_info);
> + if (rv < 0)
> + goto end_err_fw_release;
> +
> + /* Step 2: Send the main firmware image to be loaded, to ISH
> sram */
> +
> + xfer_mode = fw_info.ldr_capability.xfer_mode;
> + if (xfer_mode & LOADER_XFER_MODE_DIRECT_DMA) {
> + rv = ish_fw_xfer_direct_dma(client_data, fw, fw_info);
> + } else if (xfer_mode & LOADER_XFER_MODE_ISHTP) {
> + rv = ish_fw_xfer_ishtp(client_data, fw);
> + } else {
> + dev_err(cl_data_to_dev(client_data),
> + "No transfer mode selected in firmware\n");
> + rv = -EINVAL;
> + }
> + if (rv < 0)
> + goto end_err_fw_release;
> +
> + /* Step 3: Start ISH main firmware exeuction */
> +
> + rv = ish_fw_start(client_data);
> + if (rv < 0)
> + goto end_err_fw_release;
> +
> + release_firmware(fw);
> + kfree(filename);
> + dev_info(cl_data_to_dev(client_data), "ISH firmware %s
> loaded\n",
> + filename);
> + return 0;
> +
> +end_err_fw_release:
> + release_firmware(fw);
> +end_err_filename_buf_release:
> + kfree(filename);
> +end_error:
> + if (client_data->flag_retry) {
> + dev_warn(cl_data_to_dev(client_data),
> + "ISH host firmware load failed %d. Reset ISH &
> try again..\n",
> + rv);
> + loader_ish_hw_reset(client_data->loader_ishtp_cl);
> + } else {
> + dev_err(cl_data_to_dev(client_data),
> + "ISH host firmware load failed %d\n", rv);
> + }
> + return rv;
> +}
> +
> +static void load_fw_from_host_handler(struct work_struct *work)
> +{
> + struct ishtp_cl_data *client_data;
> +
> + client_data = container_of(work, struct ishtp_cl_data,
> + work_fw_load);
> + load_fw_from_host(client_data);
> +}
> +
> +/**
> + * loader_init() - Init function for ISH-TP client
> + * @loader_ishtp_cl: ISH-TP client instance
> + * @reset: true if called for init after reset
> + *
> + * Return: 0 for success, negative error code for failure
> + */
> +static int loader_init(struct ishtp_cl *loader_ishtp_cl, int reset)
> +{
> + int rv;
> + struct ishtp_fw_client *fw_client;
> + struct ishtp_cl_data *client_data =
> + ishtp_get_client_data(loader_ishtp_cl);
> +
> + dev_dbg(cl_data_to_dev(client_data), "reset flag: %d\n",
> reset);
> +
> + rv = ishtp_cl_link(loader_ishtp_cl);
> + if (rv < 0) {
> + dev_err(cl_data_to_dev(client_data), "ishtp_cl_link
> failed\n");
> + return rv;
> + }
> +
> + /* Connect to firmware client */
> + ishtp_set_tx_ring_size(loader_ishtp_cl,
> LOADER_CL_TX_RING_SIZE);
> + ishtp_set_rx_ring_size(loader_ishtp_cl,
> LOADER_CL_RX_RING_SIZE);
> +
> + fw_client =
> + ishtp_fw_cl_get_client(ishtp_get_ishtp_device(loader_is
> htp_cl),
> + &loader_ishtp_guid);
> + if (!fw_client) {
> + dev_err(cl_data_to_dev(client_data),
> + "ISH client uuid not found\n");
> + rv = -ENOENT;
> + goto err_cl_unlink;
> + }
> +
> + ishtp_cl_set_fw_client_id(loader_ishtp_cl,
> + ishtp_get_fw_client_id(fw_client));
> + ishtp_set_connection_state(loader_ishtp_cl,
> ISHTP_CL_CONNECTING);
> +
> + rv = ishtp_cl_connect(loader_ishtp_cl);
> + if (rv < 0) {
> + dev_err(cl_data_to_dev(client_data), "Client connect
> fail\n");
> + goto err_cl_unlink;
> + }
> +
> + dev_dbg(cl_data_to_dev(client_data), "Client connected\n");
> +
> + ishtp_register_event_cb(client_data->cl_device,
> loader_cl_event_cb);
> +
> + return 0;
> +
> +err_cl_unlink:
> + ishtp_cl_unlink(loader_ishtp_cl);
> + return rv;
> +}
> +
> +static void loader_deinit(struct ishtp_cl *loader_ishtp_cl)
> +{
> + ishtp_set_connection_state(loader_ishtp_cl,
> ISHTP_CL_DISCONNECTING);
> + ishtp_cl_disconnect(loader_ishtp_cl);
> + ishtp_cl_unlink(loader_ishtp_cl);
> + ishtp_cl_flush_queues(loader_ishtp_cl);
> +
> + /* Disband and free all Tx and Rx client-level rings */
> + ishtp_cl_free(loader_ishtp_cl);
> +}
> +
> +static void reset_handler(struct work_struct *work)
> +{
> + int rv;
> + struct ishtp_cl_data *client_data;
> + struct ishtp_cl *loader_ishtp_cl;
> + struct ishtp_cl_device *cl_device;
> +
> + client_data = container_of(work, struct ishtp_cl_data,
> + work_ishtp_reset);
> +
> + loader_ishtp_cl = client_data->loader_ishtp_cl;
> + cl_device = client_data->cl_device;
> +
> + /* Unlink, flush queues & start again */
> + ishtp_cl_unlink(loader_ishtp_cl);
> + ishtp_cl_flush_queues(loader_ishtp_cl);
> + ishtp_cl_free(loader_ishtp_cl);
> +
> + loader_ishtp_cl = ishtp_cl_allocate(cl_device);
> + if (!loader_ishtp_cl)
> + return;
> +
> + ishtp_set_drvdata(cl_device, loader_ishtp_cl);
> + ishtp_set_client_data(loader_ishtp_cl, client_data);
> + client_data->loader_ishtp_cl = loader_ishtp_cl;
> + client_data->cl_device = cl_device;
> +
> + rv = loader_init(loader_ishtp_cl, 1);
> + if (rv < 0) {
> + dev_err(ishtp_device(cl_device), "Reset Failed\n");
> + return;
> + }
> +
> + /* ISH firmware loading from host */
> + load_fw_from_host(client_data);
> +}
> +
> +/**
> + * loader_ishtp_cl_probe() - ISH-TP client driver probe
> + * @cl_device: ISH-TP client device instance
> + *
> + * This function gets called on device create on ISH-TP bus
> + *
> + * Return: 0 for success, negative error code for failure
> + */
> +static int loader_ishtp_cl_probe(struct ishtp_cl_device *cl_device)
> +{
> + struct ishtp_cl *loader_ishtp_cl;
> + struct ishtp_cl_data *client_data;
> + int rv;
> +
> + client_data = devm_kzalloc(ishtp_device(cl_device),
> + sizeof(*client_data),
> + GFP_KERNEL);
> + if (!client_data)
> + return -ENOMEM;
> +
> + loader_ishtp_cl = ishtp_cl_allocate(cl_device);
> + if (!loader_ishtp_cl)
> + return -ENOMEM;
> +
> + ishtp_set_drvdata(cl_device, loader_ishtp_cl);
> + ishtp_set_client_data(loader_ishtp_cl, client_data);
> + client_data->loader_ishtp_cl = loader_ishtp_cl;
> + client_data->cl_device = cl_device;
> +
> + init_waitqueue_head(&client_data->cmd_resp_wait);
> +
> + INIT_WORK(&client_data->work_ishtp_reset,
> + reset_handler);
> + INIT_WORK(&client_data->work_fw_load,
> + load_fw_from_host_handler);
> +
> + rv = loader_init(loader_ishtp_cl, 0);
> + if (rv < 0) {
> + ishtp_cl_free(loader_ishtp_cl);
> + return rv;
> + }
> + ishtp_get_device(cl_device);
> +
> + /* ISH firmware loading from host */
> + schedule_work(&client_data->work_fw_load);
> +
> + return 0;
> +}
> +
> +/**
> + * loader_ishtp_cl_remove() - ISH-TP client driver remove
> + * @cl_device: ISH-TP client device instance
> + *
> + * This function gets called on device remove on ISH-TP bus
> + *
> + * Return: 0
> + */
> +static int loader_ishtp_cl_remove(struct ishtp_cl_device *cl_device)
> +{
> + struct ishtp_cl_data *client_data;
> + struct ishtp_cl *loader_ishtp_cl =
> ishtp_get_drvdata(cl_device);
> +
> + client_data = ishtp_get_client_data(loader_ishtp_cl);
> +
> + /*
> + * The sequence of the following two cancel_work_sync() is
> + * important. The work_fw_load can in turn schedue
> + * work_ishtp_reset, so first cancel work_fw_load then
> + * cancel work_ishtp_reset.
> + */
> + cancel_work_sync(&client_data->work_fw_load);
> + cancel_work_sync(&client_data->work_ishtp_reset);
> + loader_deinit(loader_ishtp_cl);
> + ishtp_put_device(cl_device);
> +
> + return 0;
> +}
> +
> +/**
> + * loader_ishtp_cl_reset() - ISH-TP client driver reset
> + * @cl_device: ISH-TP client device instance
> + *
> + * This function gets called on device reset on ISH-TP bus
> + *
> + * Return: 0
> + */
> +static int loader_ishtp_cl_reset(struct ishtp_cl_device *cl_device)
> +{
> + struct ishtp_cl_data *client_data;
> + struct ishtp_cl *loader_ishtp_cl =
> ishtp_get_drvdata(cl_device);
> +
> + client_data = ishtp_get_client_data(loader_ishtp_cl);
> +
> + schedule_work(&client_data->work_ishtp_reset);
> +
> + return 0;
> +}
> +
> +static struct ishtp_cl_driver loader_ishtp_cl_driver = {
> + .name = "ish-loader",
> + .guid = &loader_ishtp_guid,
> + .probe = loader_ishtp_cl_probe,
> + .remove = loader_ishtp_cl_remove,
> + .reset = loader_ishtp_cl_reset,
> +};
> +
> +static int __init ish_loader_init(void)
> +{
> + return ishtp_cl_driver_register(&loader_ishtp_cl_driver,
> THIS_MODULE);
> +}
> +
> +static void __exit ish_loader_exit(void)
> +{
> + ishtp_cl_driver_unregister(&loader_ishtp_cl_driver);
> +}
> +
> +late_initcall(ish_loader_init);
> +module_exit(ish_loader_exit);
> +
> +module_param(dma_buf_size_limit, int, 0644);
> +MODULE_PARM_DESC(dma_buf_size_limit, "Limit the DMA buf size to this
> value in bytes");
> +
> +MODULE_DESCRIPTION("ISH ISH-TP Host firmware Loader Client Driver");
> +MODULE_AUTHOR("Rushikesh S Kadam <rushikesh.s.kadam@intel.com>");
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("ishtp:*");
^ permalink raw reply
* Re: [PATCH] ELAN touchpad i2c_hid bugs fix
From: Hans de Goede @ 2019-03-24 12:27 UTC (permalink / raw)
To: hotwater438, Jikos, Benjamin Tissoires, Kai Heng Feng, Swboyd,
Bigeasy, Dtor, Linux Input, Linux Kernel
In-Reply-To: <LaeGPSe--3-1@tutanota.com>
Hi,
On 23-03-19 12:18, hotwater438@tutanota.com wrote:
> Guys, I am a little bit confused. Do you mean that it is not kernel related issue, and it's the issue of ASUS BIOS?
Technically this is a BIOS bug, the BIOS should properly declare the IRQ as
edge-falling and then everything would just work.
But BIOS bugs is what quirks are for (unfortunately)
> And why do you think that triggering edge-falling quirk for this exactly this touchpad and exactly ELAN vendor can somehow break other systems?
I'm not seeing anything about breaking other systems in the discussion
(but I may have missed this) what we were discussing is also applying
the quirk to other systems with an Elan touchpad to see if it also
helps with issues on other systems.
Summarizing: Your patch is fine (minus the style issues and the goto error_foo stuff)
please fix those issues and submit a new version.
Regards,
Hans
>
> Regards,
> Vladislav
>
> Mar 21, 2019, 12:38 PM by hotwater438@tutanota.com:
>
> Hello! Thank you for an answer and for helping me with the patch. It is my very first contribution.
>
> About some indent problems: I am sorry, it was mainly caused by my .vimrc, and I will edit these errors and send a new patch. Though, there is one typo:
>
> +#define I2C_HID_QUIRK_FORCE_TRIGGER_FALLING BIT(5)
>
> /* flags */
> #define I2C_HID_STARTED 0
> @@ -182,8 +183,10 @@ static const struct i2c_hid_quirks {
> I2C_HID_QUIRK_NO_RUNTIME_PM },
> { I2C_VENDOR_ID_GOODIX, I2C_DEVICE_ID_GOODIX_01F0,
> I2C_HID_QUIRK_NO_RUNTIME_PM },
> + { USB_VENDOR_ID_ELAN, USB_DEVICE_ID_ELAN_TOUCHPAD,
> + I2C_HID_QUIRK_BOGUS_IRQ | I2C_HID_QUIRK_FORCE_TRIGGER_FALLING },
> { USB_VENDOR_ID_ELAN, HID_ANY_ID,
> - I2C_HID_QUIRK_BOGUS_IRQ },
> + I2C_HID_QUIRK_BOGUS_IRQ },
>
>
> same here, this line should not be changed.
>
> If you take a look at original file, you can notice extra space there.
>
> In your case, if you do not have any Elantech engineer who agreed to
> sign off the patch, you can definitively mention them this way.
>
> I don't have any.
>
> ret = request_threaded_irq(client->irq, NULL, i2c_hid_irq,
> irqflags | IRQF_ONESHOT, client->name, ihid);
> @@ -1123,10 +1128,6 @@ static int i2c_hid_probe(struct i2c_client *client,
> if (ret < 0)
> goto err_pm;
>
> - ret = i2c_hid_init_irq(client);
> - if (ret < 0)
> - goto err_pm;
> -
> hid = hid_allocate_device();
> if (IS_ERR(hid)) {
> ret = PTR_ERR(hid);
>
>
> the next call is "goto err_irq", which should be changed to "goto err_pm"
>
> @@ -1149,6 +1150,10 @@ static int i2c_hid_probe(struct i2c_client *client,
>
> ihid->quirks = i2c_hid_lookup_quirk(hid->vendor, hid->product);
>
> + ret = i2c_hid_init_irq(client);
> + if (ret < 0)
> + goto err_pm;
>
>
> this should be "err_mem_free"
>
> +
> ret = hid_add_device(hid);
> if (ret) {
> if (ret != -ENODEV)
>
>
> next one should be "err_irq"
>
> and the err_irq and err_mem_free should be inverted at the end of probe.
>
> I tried to understand this as hard as I could, but there may be some errors, so please review my latest patch:
>
> From 80576dd7ac193548ba747d287b5ab5606f642d00 Mon Sep 17 00:00:00 2001
> From: h0tw4t3r <hotwater438@tutanota.com <mailto:hotwater438@tutanota.com>>
> Date: Wed, 20 Mar 2019 00:41:22 +0200
> Subject: [PATCH] ELAN touchpad i2c_hid bugs fix
>
> Description: The ELAN1200:04F3:303E touchpad exposes several issues, all caused by
> an error setting the correct IRQ_TRIGGER flag:
> - i2c_hid incoplete error flood in journalctl;
> - Five finger tap kill's module so you have to restart it;
> - Two finger scoll is working incorrect and sometimes even when you raised one of two finger still thinks that you are scrolling
>
> Fix all of these with a new quirk that corrects the trigger flag
> announced by the ACPI tables. (edge-falling).
>
> Reason behind moving i2c_hid_init_irq section described below:
> i2c_hid_init_irq now checks for a quirk, so we must setup the quirks before we init the irq, and we cannot setup the quirk earlier, so we must init the irq later.
>
> Signed-off-by: Vladislav Dalechyn <hotwater438@tutanota.com <mailto:hotwater438@tutanota.com>>
> ---
> drivers/hid/hid-ids.h | 1 +
> drivers/hid/i2c-hid/i2c-hid-core.c | 25 +++++++++++++++----------
> 2 files changed, 16 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index b6d93f4ad037..660b4e0e912e 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -389,6 +389,7 @@
> #define USB_DEVICE_ID_TOSHIBA_CLICK_L9W 0x0401
> #define USB_DEVICE_ID_HP_X2 0x074d
> #define USB_DEVICE_ID_HP_X2_10_COVER 0x0755
> +#define I2C_DEVICE_ID_ELAN_TOUCHPAD 0x303e
>
> #define USB_VENDOR_ID_ELECOM 0x056e
> #define USB_DEVICE_ID_ELECOM_BM084 0x0061
> diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
> index 90164fed08d3..9b417914411f 100644
> --- a/drivers/hid/i2c-hid/i2c-hid-core.c
> +++ b/drivers/hid/i2c-hid/i2c-hid-core.c
> @@ -51,6 +51,7 @@
> #define I2C_HID_QUIRK_NO_RUNTIME_PM BIT(2)
> #define I2C_HID_QUIRK_DELAY_AFTER_SLEEP BIT(3)
> #define I2C_HID_QUIRK_BOGUS_IRQ BIT(4)
> +#define I2C_HID_QUIRK_FORCE_TRIGGER_FALLING BIT(5)
>
> /* flags */
> #define I2C_HID_STARTED 0
> @@ -182,8 +183,10 @@ static const struct i2c_hid_quirks {
> I2C_HID_QUIRK_NO_RUNTIME_PM },
> { I2C_VENDOR_ID_GOODIX, I2C_DEVICE_ID_GOODIX_01F0,
> I2C_HID_QUIRK_NO_RUNTIME_PM },
> + { USB_VENDOR_ID_ELAN, I2C_DEVICE_ID_ELAN_TOUCHPAD,
> + I2C_HID_QUIRK_BOGUS_IRQ | I2C_HID_QUIRK_FORCE_TRIGGER_FALLING },
> { USB_VENDOR_ID_ELAN, HID_ANY_ID,
> - I2C_HID_QUIRK_BOGUS_IRQ },
> + I2C_HID_QUIRK_BOGUS_IRQ },
> { 0, 0 }
> };
>
> @@ -854,6 +857,8 @@ static int i2c_hid_init_irq(struct i2c_client *client)
>
> if (!irq_get_trigger_type(client->irq))
> irqflags = IRQF_TRIGGER_LOW;
> + if (ihid->quirks & I2C_HID_QUIRK_FORCE_TRIGGER_FALLING)
> + irqflags = IRQF_TRIGGER_FALLING;
>
> ret = request_threaded_irq(client->irq, NULL, i2c_hid_irq,
> irqflags | IRQF_ONESHOT, client->name, ihid);
> @@ -1123,14 +1128,10 @@ static int i2c_hid_probe(struct i2c_client *client,
> if (ret < 0)
> goto err_pm;
>
> - ret = i2c_hid_init_irq(client);
> - if (ret < 0)
> - goto err_pm;
> -
> hid = hid_allocate_device();
> if (IS_ERR(hid)) {
> ret = PTR_ERR(hid);
> - goto err_irq;
> + goto err_pm;
> }
>
> ihid->hid = hid;
> @@ -1149,11 +1150,15 @@ static int i2c_hid_probe(struct i2c_client *client,
>
> ihid->quirks = i2c_hid_lookup_quirk(hid->vendor, hid->product);
>
> + ret = i2c_hid_init_irq(client);
> + if (ret < 0)
> + goto err_mem_free;
> +
> ret = hid_add_device(hid);
> if (ret) {
> if (ret != -ENODEV)
> hid_err(client, "can't add hid device: %d\n", ret);
> - goto err_mem_free;
> + goto err_irq;
> }
>
> if (!(ihid->quirks & I2C_HID_QUIRK_NO_RUNTIME_PM))
> @@ -1161,12 +1166,12 @@ static int i2c_hid_probe(struct i2c_client *client,
>
> return 0;
>
> +err_irq:
> + free_irq(client->irq, ihid);
> +
> err_mem_free:
> hid_destroy_device(hid);
>
> -err_irq:
> - free_irq(client->irq, ihid);
> -
> err_pm:
> pm_runtime_put_noidle(&client->dev);
> pm_runtime_disable(&client->dev);
> --
> 2.20.1
>
> Regards,
> Vladislav.
>
>
^ permalink raw reply
* [PATCH] xpad.c: Send necessary control transfer to certain Xbox controllers
From: Mike Salvatore @ 2019-03-23 16:46 UTC (permalink / raw)
To: dmitry.torokhov, linux-input
Cc: ramzeto, marcus.folkesson, flibitijibibo, aicommander,
leosperling97, gottox, frtherien, linux-kernel
[-- Attachment #1.1: Type: text/plain, Size: 4235 bytes --]
Hi,
I recently purchased an Xbox controller with USB ID 045e:028e. When I plugged
the device in, it was recognized as a joystick but button presses were not
recognized. Using evtest, I confirmed that the device was not sending any
information to the host when I pressed its buttons.
After doing some research, I found that this particular device requires that the
host send a control transfer before the device will begin sending any data to
the host. Below is a patch that introduces a mechanism into xpad.c for sending
control transfers to specific Xbox controllers at initialization.
After applying this patch to the xpad module, the Xbox controller is properly
initialized when plugged in and functions as expected. Other Xbox controllers
may also require control transfers at initialization, but I don't know which (if
any).
Regards,
Mike Salvatore
From 3051524e62d68b920019bcb50a713e736fcf4234 Mon Sep 17 00:00:00 2001
From: Mike Salvatore <mike.salvatore@canonical.com>
Date: Wed, 13 Mar 2019 22:11:37 -0400
Subject: [PATCH] Input: xpad - send control init message to certain Xbox
controllers
The Xbox controller with idVendor == 0x045e and idProduct == 0x028e
requires that a specific control transfer be sent from the host to the
device before the device will send data to the host.
This patch introduces an xboxone_control_packet struct and a mechanism
for sending control packets to devices that require them at
initialization.
Signed-off-by: Mike Salvatore <mike.salvatore@canonical.com>
---
drivers/input/joystick/xpad.c | 56 +++++++++++++++++++++++++++++++++++
1 file changed, 56 insertions(+)
diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index cfc8b94527b9..f45522b9ff1f 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -460,6 +460,25 @@ struct xboxone_init_packet {
.len = ARRAY_SIZE(_data), \
}
+struct xboxone_control_packet {
+ u16 idVendor;
+ u16 idProduct;
+ struct usb_ctrlrequest ctrlrequest;
+};
+
+#define XBOXONE_CONTROL_PKT(_vid, _pid, _reqtype, _req, _value, _index, _len) \
+ { \
+ .idVendor = (_vid), \
+ .idProduct = (_pid), \
+ .ctrlrequest = { \
+ .bRequestType = (_reqtype), \
+ .bRequest = (_req), \
+ .wValue = (_value), \
+ .wIndex = (_index), \
+ .wLength = (_len), \
+ }, \
+ }
+
/*
* This packet is required for all Xbox One pads with 2015
@@ -537,6 +556,13 @@ static const struct xboxone_init_packet xboxone_init_packets[] = {
XBOXONE_INIT_PKT(0x24c6, 0x543a, xboxone_rumbleend_init),
};
+static const struct xboxone_control_packet xboxone_control_packets[] = {
+ XBOXONE_CONTROL_PKT(0x045e, 0x028e,
+ USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_INTERFACE,
+ USB_REQ_CLEAR_FEATURE,
+ USB_DEVICE_REMOTE_WAKEUP, 0, 0),
+};
+
struct xpad_output_packet {
u8 data[XPAD_PKT_LEN];
u8 len;
@@ -1119,6 +1145,31 @@ static int xpad_init_output(struct usb_interface *intf, struct usb_xpad *xpad,
return error;
}
+static int xpad_init_control_msg(struct usb_xpad *xpad)
+{
+ struct usb_device *udev = xpad->udev;
+ size_t i;
+
+ for (i = 0; i < ARRAY_SIZE(xboxone_control_packets); i++) {
+ u16 idVendor = xboxone_control_packets[i].idVendor;
+ u16 idProduct = xboxone_control_packets[i].idProduct;
+
+ if (le16_to_cpu(udev->descriptor.idVendor) == idVendor
+ && le16_to_cpu(udev->descriptor.idProduct) == idProduct) {
+ const struct usb_ctrlrequest *ctrlrequest =
+ &(xboxone_control_packets[i].ctrlrequest);
+
+ return usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
+ ctrlrequest->bRequest,
+ ctrlrequest->bRequestType, ctrlrequest->wValue,
+ ctrlrequest->wIndex, NULL, ctrlrequest->wLength,
+ 2 * HZ);
+ }
+ }
+
+ return 0;
+}
+
static void xpad_stop_output(struct usb_xpad *xpad)
{
if (xpad->xtype != XTYPE_UNKNOWN) {
@@ -1839,6 +1890,11 @@ static int xpad_probe(struct usb_interface *intf, const struct usb_device_id *id
if (error)
goto err_deinit_output;
}
+
+ error = xpad_init_control_msg(xpad);
+ if (error)
+ goto err_deinit_output;
+
return 0;
err_deinit_output:
--
2.17.1
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH] HID: intel-ish-hid: ISH firmware loader client driver
From: Rushikesh S Kadam @ 2019-03-23 11:16 UTC (permalink / raw)
To: srinivas.pandruvada, benjamin.tissoires, jikos
Cc: ncrews, jettrink, gwendal, rushikesh.s.kadam, linux-kernel,
linux-input
This driver adds support for loading Intel Integrated
Sensor Hub (ISH) firmware from host file system to ISH
SRAM and start execution.
At power-on, the ISH subsystem shall boot to an interim
Shim loader-firmware, which shall expose an ISHTP loader
device.
The driver implements an ISHTP client that communicates
with the Shim ISHTP loader device over the intel-ish-hid
stack, to download the main ISH firmware.
Signed-off-by: Rushikesh S Kadam <rushikesh.s.kadam@intel.com>
---
The patches are baselined to hid git tree, branch for-5.2/ish
https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git/log/?h=for-5.2/ish
drivers/hid/Makefile | 1 +
drivers/hid/intel-ish-hid/Kconfig | 15 +
drivers/hid/intel-ish-hid/Makefile | 3 +
drivers/hid/intel-ish-hid/ishtp-fw-loader.c | 1103 +++++++++++++++++++++++++++
4 files changed, 1122 insertions(+)
create mode 100644 drivers/hid/intel-ish-hid/ishtp-fw-loader.c
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index 170163b..d8d393e 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -134,3 +134,4 @@ obj-$(CONFIG_USB_KBD) += usbhid/
obj-$(CONFIG_I2C_HID) += i2c-hid/
obj-$(CONFIG_INTEL_ISH_HID) += intel-ish-hid/
+obj-$(INTEL_ISH_FIRMWARE_DOWNLOADER) += intel-ish-hid/
diff --git a/drivers/hid/intel-ish-hid/Kconfig b/drivers/hid/intel-ish-hid/Kconfig
index 519e4c8..786adbc 100644
--- a/drivers/hid/intel-ish-hid/Kconfig
+++ b/drivers/hid/intel-ish-hid/Kconfig
@@ -14,4 +14,19 @@ config INTEL_ISH_HID
Broxton and Kaby Lake.
Say Y here if you want to support Intel ISH. If unsure, say N.
+
+config INTEL_ISH_FIRMWARE_DOWNLOADER
+ tristate "Host Firmware Load feature for Intel ISH"
+ depends on INTEL_ISH_HID
+ depends on X86
+ help
+ The Integrated Sensor Hub (ISH) enables the kernel to offload
+ sensor polling and algorithm processing to a dedicated low power
+ processor in the chipset.
+
+ The Host Firmware Load feature adds support to load the ISH
+ firmware from host file system at boot.
+
+ Say M here if you want to support Host Firmware Loading feature
+ for Intel ISH. If unsure, say N.
endmenu
diff --git a/drivers/hid/intel-ish-hid/Makefile b/drivers/hid/intel-ish-hid/Makefile
index 825b70a..2de97e4 100644
--- a/drivers/hid/intel-ish-hid/Makefile
+++ b/drivers/hid/intel-ish-hid/Makefile
@@ -20,4 +20,7 @@ obj-$(CONFIG_INTEL_ISH_HID) += intel-ishtp-hid.o
intel-ishtp-hid-objs := ishtp-hid.o
intel-ishtp-hid-objs += ishtp-hid-client.o
+obj-$(CONFIG_INTEL_ISH_FIRMWARE_DOWNLOADER) += intel-ishtp-loader.o
+intel-ishtp-loader-objs += ishtp-fw-loader.o
+
ccflags-y += -Idrivers/hid/intel-ish-hid/ishtp
diff --git a/drivers/hid/intel-ish-hid/ishtp-fw-loader.c b/drivers/hid/intel-ish-hid/ishtp-fw-loader.c
new file mode 100644
index 0000000..85d71d3
--- /dev/null
+++ b/drivers/hid/intel-ish-hid/ishtp-fw-loader.c
@@ -0,0 +1,1103 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * ISH-TP client driver for ISH firmware loading
+ *
+ * Copyright (c) 2018, Intel Corporation.
+ */
+
+#include <linux/firmware.h>
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/intel-ish-client-if.h>
+#include <linux/property.h>
+#include <asm/cacheflush.h>
+
+/* ISH TX/RX ring buffer pool size */
+#define LOADER_CL_RX_RING_SIZE 1
+#define LOADER_CL_TX_RING_SIZE 1
+
+/*
+ * ISH Shim firmware loader reserves 4 Kb buffer in SRAM. The buffer is
+ * used to temporarily hold the data transferred from host to Shim firmware
+ * loader. Reason for the odd size of 3968 bytes? Each IPC transfer is 128
+ * bytes (= 4 bytes header + 124 bytes payload). So the 4 Kb buffer can
+ * hold maximum of 32 IPC transfers, which means we can have a max payload
+ * of 3968 bytes (= 32 x 124 payload).
+ */
+#define LOADER_SHIM_IPC_BUF_SIZE 3968
+
+/**
+ * enum ish_loader_commands - ISH loader host commands.
+ * LOADER_CMD_XFER_QUERY Query the Shim firmware loader for capabilities
+ * LOADER_CMD_XFER_FRAGMENT Transfer one firmware image framgment at a
+ * time. The command may be executed multiple
+ * times until the entire firmware image is
+ * downloaded to SRAM.
+ * LOADER_CMD_START Start executing the main firmware.
+ */
+enum ish_loader_commands {
+ LOADER_CMD_XFER_QUERY = 0,
+ LOADER_CMD_XFER_FRAGMENT,
+ LOADER_CMD_START,
+};
+
+/* Command bit mask */
+#define CMD_MASK GENMASK(6, 0)
+#define IS_RESPONSE BIT(7)
+
+/*
+ * ISH firmware max delay for one transmit failure is 1 Hz,
+ * and firmware will retry 2 times, so 3 Hz is used for timeout.
+ */
+#define ISHTP_SEND_TIMEOUT (3 * HZ)
+
+/*
+ * Loader transfer modes:
+ *
+ * LOADER_XFER_MODE_ISHTP mode uses the existing ISH-TP mechanims to
+ * transfer data. This may use IPC or DMA if supported in firmware.
+ * The buffer size is limited to 4 Kb by the IPC/ISH-TP protocol for
+ * both IPC & DMA (legacy).
+ *
+ * LOADER_XFER_MODE_DIRECT_DMA - firmware loading is a bit different
+ * from the sensor data streaming. Here we download a large (300+ Kb)
+ * image directly to ISH SRAM memory. There is limited benefit of
+ * DMA'ing 300 Kb image in 4 Kb chucks limit. Hence, we introduce
+ * this "direct dma" mode, where we do not use ISH-TP for DMA, but
+ * instead manage the DMA directly in kernel driver and Shim firmware
+ * loader (allocate buf, break in chucks and transfer). This allows
+ * to overcome 4 Kb limit, and optimize the data flow path in firmware.
+ */
+#define LOADER_XFER_MODE_DIRECT_DMA BIT(0)
+#define LOADER_XFER_MODE_ISHTP BIT(1)
+
+/* ISH Transport Loader client unique GUID */
+static const guid_t loader_ishtp_guid =
+ GUID_INIT(0xc804d06a, 0x55bd, 0x4ea7,
+ 0xad, 0xed, 0x1e, 0x31, 0x22, 0x8c, 0x76, 0xdc);
+
+#define FILENAME_SIZE 256
+
+/*
+ * The firmware loading latency will be minimum if we can DMA the
+ * entire ISH firmware image in one go. This requires that we allocate
+ * a large DMA buffer in kernel, which could be problematic on some
+ * platforms. So here we limit the DMA buf size via a module_param.
+ * We default to 4 pages, but a customer can set it to higher limit if
+ * deemed appropriate for his platform.
+ */
+static int dma_buf_size_limit = 4 * PAGE_SIZE;
+
+/**
+ * struct loader_msg_hdr - Header for ISH Loader commands.
+ * @command: LOADER_CMD* commands. Bit 7 is the response.
+ * @status: Command response status. Non 0, is error condition.
+ *
+ * This structure is used as header for every command/data sent/received
+ * between Host driver and ISH Shim firmware loader.
+ */
+struct loader_msg_hdr {
+ u8 command;
+ u8 reserved[2];
+ u8 status;
+} __packed;
+
+struct loader_xfer_query {
+ struct loader_msg_hdr hdr;
+ u32 image_size;
+} __packed;
+
+struct ish_fw_version {
+ u16 major;
+ u16 minor;
+ u16 hotfix;
+ u16 build;
+} __packed;
+
+union loader_version {
+ u32 value;
+ struct {
+ u8 major;
+ u8 minor;
+ u8 hotfix;
+ u8 build;
+ };
+} __packed;
+
+struct loader_capability {
+ u32 max_fw_image_size;
+ u32 xfer_mode;
+ u32 max_dma_buf_size; /* only for dma mode, multiples of cacheline */
+} __packed;
+
+struct shim_fw_info {
+ struct ish_fw_version ish_fw_version;
+ u32 protocol_version;
+ union loader_version ldr_version;
+ struct loader_capability ldr_capability;
+} __packed;
+
+struct loader_xfer_query_response {
+ struct loader_msg_hdr hdr;
+ struct shim_fw_info fw_info;
+} __packed;
+
+struct loader_xfer_fragment {
+ struct loader_msg_hdr hdr;
+ u32 xfer_mode;
+ u32 offset;
+ u32 size;
+ u32 is_last;
+} __packed;
+
+struct loader_xfer_ipc_fragment {
+ struct loader_xfer_fragment fragment;
+ u8 data[] ____cacheline_aligned; /* variable length payload here */
+} __packed;
+
+struct loader_xfer_dma_fragment {
+ struct loader_xfer_fragment fragment;
+ u64 ddr_phys_addr;
+} __packed;
+
+struct loader_start {
+ struct loader_msg_hdr hdr;
+} __packed;
+
+/**
+ * struct ishtp_cl_data - Encapsulate per ISH-TP Client Data
+ * @flag_response Set true on receiving a firmware response to host
+ * loader command
+ * @cmd_resp_wait: Wait queue for Host firmware loading, where the
+ * client sends message to ISH firmware and wait for
+ * response
+ * @work_ishtp_reset: Work queue for reset handling
+ * @work_fw_load: Work queue for host firmware loading
+ * @flag_retry Flag for indicating host firmware loading should be
+ * retried
+ * @bad_recv_cnt: Running count of packets received with error
+ *
+ * This structure is used to store data per client
+ */
+struct ishtp_cl_data {
+ struct ishtp_cl *loader_ishtp_cl;
+ struct ishtp_cl_device *cl_device;
+
+ /* Completion flags */
+ bool flag_response;
+
+ /* Copy buffer received in firmware "response" here */
+ void *response_data;
+ size_t response_size;
+
+ /* Wait queue for ISH firmware message event */
+ wait_queue_head_t cmd_resp_wait;
+
+ struct work_struct work_ishtp_reset;
+ struct work_struct work_fw_load;
+
+ /*
+ * In certain failure scenrios, it makes sense to reset the
+ * the ISH subsystem and retry Host firmware loading
+ * (e.g. bad message packet, ENOMEM, etc.)
+ * On the other hand, failures due to protocol mismatch, etc
+ * are not recoverable. We do not retry.
+ *
+ * If set, the flag indictes that we should re-try the particular
+ * failure.
+ */
+ bool flag_retry;
+
+ /* Statistics */
+ unsigned int bad_recv_cnt;
+};
+
+#define IPC_FRAGMENT_DATA_PREAMBLE \
+ offsetof(struct loader_xfer_ipc_fragment, data)
+
+#define cl_data_to_dev(client_data) ishtp_device((client_data)->cl_device)
+
+/**
+ * get_firmware_variant() - Gets the filename of firmware image to be
+ * loaded based on platform variant.
+ * @client_data Client data instance.
+ * @filename Returns firmware filename.
+ *
+ * Queries the firmware-name device property string.
+ *
+ * Return: 0 for success, negative error code for failure.
+ */
+static int get_firmware_variant(struct ishtp_cl_data *client_data,
+ char *filename)
+{
+ int rv;
+ const char *val;
+ struct device *devc = ishtp_get_pci_device(client_data->cl_device);
+
+ rv = device_property_read_string(devc, "firmware-name", &val);
+ if (rv < 0) {
+ dev_err(devc,
+ "Error: ISH firmware-name device property required\n");
+ return rv;
+ }
+ return snprintf(filename, FILENAME_SIZE, "intel/%s", val);
+}
+
+/**
+ * report_bad_packets() Report bad packets
+ * @loader_ishtp_cl: Client instance to get stats
+ * @recv_buf: Raw received host interface message
+ *
+ * Dumps error in case bad packet is received
+ */
+static void report_bad_packet(struct ishtp_cl *loader_ishtp_cl,
+ void *recv_buf)
+{
+ struct loader_msg_hdr *hdr = recv_buf;
+ struct ishtp_cl_data *client_data =
+ ishtp_get_client_data(loader_ishtp_cl);
+
+ client_data->bad_recv_cnt++;
+ dev_err(cl_data_to_dev(client_data),
+ "BAD packet: command=%02lx is_response=%u status=%02x total_bad=%u\n",
+ hdr->command & CMD_MASK,
+ hdr->command & IS_RESPONSE ? 1 : 0,
+ hdr->status,
+ client_data->bad_recv_cnt);
+}
+
+/**
+ * loader_ish_hw_reset() - Reset ISH HW in bad state
+ * @loader_ishtp_cl Client instance to reset
+ *
+ * This function resets ISH hardware, which shall reload
+ * the Shim firmware loader, initiate ISH-TP interface reset,
+ * re-attach kernel loader driver, and repeat Host
+ * firmware load.
+ */
+static inline void loader_ish_hw_reset(struct ishtp_cl *loader_ishtp_cl)
+{
+ struct ishtp_cl_data *client_data =
+ ishtp_get_client_data(loader_ishtp_cl);
+
+ dev_warn(cl_data_to_dev(client_data), "Reset the ISH subsystem\n");
+ ish_hw_reset(ishtp_get_ishtp_device(loader_ishtp_cl));
+}
+
+/**
+ * loader_cl_send() Send message from host to firmware
+ * @client_data: Client data instance
+ * @msg Message buffer to send
+ * @msg_size Size of message
+ *
+ * Return: Received buffer size on success, negative error code on failure.
+ */
+static int loader_cl_send(struct ishtp_cl_data *client_data,
+ u8 *msg, size_t msg_size)
+{
+ int rv;
+ size_t data_len;
+ struct loader_msg_hdr *in_hdr;
+ struct loader_msg_hdr *out_hdr = (struct loader_msg_hdr *)msg;
+ struct ishtp_cl *loader_ishtp_cl = client_data->loader_ishtp_cl;
+
+ dev_dbg(cl_data_to_dev(client_data),
+ "%s: command=%02lx is_response=%u status=%02x\n",
+ __func__,
+ out_hdr->command & CMD_MASK,
+ out_hdr->command & IS_RESPONSE ? 1 : 0,
+ out_hdr->status);
+
+ client_data->flag_response = false;
+ rv = ishtp_cl_send(loader_ishtp_cl, msg, msg_size);
+ if (rv < 0) {
+ dev_err(cl_data_to_dev(client_data),
+ "ishtp_cl_send error %d\n", rv);
+ return rv;
+ }
+
+ wait_event_interruptible_timeout(client_data->cmd_resp_wait,
+ client_data->flag_response,
+ ISHTP_SEND_TIMEOUT);
+ if (!client_data->flag_response) {
+ dev_err(cl_data_to_dev(client_data),
+ "Timed out for response to command=%02lx",
+ out_hdr->command & CMD_MASK);
+ return -ETIMEDOUT;
+ }
+
+ /* All response messages will contain a header */
+ data_len = client_data->response_size;
+ in_hdr = (struct loader_msg_hdr *)client_data->response_data;
+
+ /* Sanity checks */
+ if (!(in_hdr->command & IS_RESPONSE)) {
+ dev_err(cl_data_to_dev(client_data),
+ "Invalid response to command\n");
+ return -EIO;
+ }
+
+ if (in_hdr->status) {
+ dev_err(cl_data_to_dev(client_data),
+ "Loader returned status %d\n",
+ in_hdr->status);
+ return -EIO;
+ }
+
+ return data_len;
+}
+
+/**
+ * process_recv() - Receive and parse incoming packet
+ * @loader_ishtp_cl: Client instance to get stats
+ * @rb_in_proc: ISH received message buffer
+ *
+ * Parse the incoming packet. If it is a response packet then it will
+ * update flag_response and wake up the caller waiting to for the response.
+ */
+static void process_recv(struct ishtp_cl *loader_ishtp_cl,
+ struct ishtp_cl_rb *rb_in_proc)
+{
+ size_t data_len = rb_in_proc->buf_idx;
+ struct loader_msg_hdr *hdr =
+ (struct loader_msg_hdr *)rb_in_proc->buffer.data;
+ struct ishtp_cl_data *client_data =
+ ishtp_get_client_data(loader_ishtp_cl);
+
+ /*
+ * All firmware messages have a header. Check buffer size
+ * before accessing elements inside.
+ */
+ if (data_len < sizeof(struct loader_msg_hdr)) {
+ dev_err(cl_data_to_dev(client_data),
+ "data size %zu is less than header %zu\n",
+ data_len, sizeof(struct loader_msg_hdr));
+ report_bad_packet(client_data->loader_ishtp_cl, hdr);
+ goto end_error;
+ }
+
+ dev_dbg(cl_data_to_dev(client_data),
+ "%s: command=%02lx is_response=%u status=%02x\n",
+ __func__,
+ hdr->command & CMD_MASK,
+ hdr->command & IS_RESPONSE ? 1 : 0,
+ hdr->status);
+
+ switch (hdr->command & CMD_MASK) {
+ case LOADER_CMD_XFER_QUERY:
+ case LOADER_CMD_XFER_FRAGMENT:
+ case LOADER_CMD_START:
+ /* Sanity check */
+ if (client_data->response_data || client_data->flag_response) {
+ dev_err(cl_data_to_dev(client_data),
+ "Buffer overrun: previous firmware message not yet processed\n");
+ report_bad_packet(client_data->loader_ishtp_cl, hdr);
+ break;
+ }
+
+ /*
+ * Copy the buffer received in firmware response for the
+ * calling thread.
+ */
+ client_data->response_data = kmalloc(data_len, GFP_KERNEL);
+ if (!client_data->response_data)
+ break;
+
+ memcpy(client_data->response_data,
+ rb_in_proc->buffer.data, data_len);
+ client_data->response_size = data_len;
+
+ /* Free the buffer */
+ ishtp_cl_io_rb_recycle(rb_in_proc);
+ rb_in_proc = NULL;
+
+ /* Wake the calling thread */
+ client_data->flag_response = true;
+ wake_up_interruptible(&client_data->cmd_resp_wait);
+ break;
+
+ default:
+ dev_err(cl_data_to_dev(client_data),
+ "Invalid command=%02lx\n",
+ hdr->command & CMD_MASK);
+ report_bad_packet(client_data->loader_ishtp_cl, hdr);
+ }
+
+end_error:
+ /* Free the buffer if we did not do above */
+ if (rb_in_proc)
+ ishtp_cl_io_rb_recycle(rb_in_proc);
+}
+
+/**
+ * loader_cl_event_cb() - bus driver callback for incoming message
+ * @device: Pointer to the the ishtp client device for which
+ * this message is targeted
+ *
+ * Remove the packet from the list and process the message by calling
+ * process_recv
+ */
+static void loader_cl_event_cb(struct ishtp_cl_device *cl_device)
+{
+ struct ishtp_cl_rb *rb_in_proc;
+ struct ishtp_cl_data *client_data;
+ struct ishtp_cl *loader_ishtp_cl = ishtp_get_drvdata(cl_device);
+
+ client_data = ishtp_get_client_data(loader_ishtp_cl);
+
+ while ((rb_in_proc = ishtp_cl_rx_get_rb(loader_ishtp_cl)) != NULL) {
+ if (!rb_in_proc->buffer.data) {
+ dev_warn(cl_data_to_dev(client_data),
+ "rb_in_proc->buffer.data returned null");
+ continue;
+ }
+
+ /* Process the data packet from firmware */
+ process_recv(loader_ishtp_cl, rb_in_proc);
+ }
+}
+
+/**
+ * ish_query_loader_prop() - Query ISH Shim firmware loader
+ * @client_data: Client data instance
+ * @fw: Poiner to fw data struct in host memory
+ *
+ * This function queries the ISH Shim firmware loader for capabilities.
+ *
+ * Return: 0 for success, negative error code for failure.
+ */
+static int ish_query_loader_prop(struct ishtp_cl_data *client_data,
+ const struct firmware *fw,
+ struct shim_fw_info *fw_info)
+{
+ int rv;
+ size_t data_len;
+ struct loader_msg_hdr *hdr;
+ struct loader_xfer_query ldr_xfer_query;
+ struct loader_xfer_query_response *ldr_xfer_query_resp;
+
+ memset(&ldr_xfer_query, 0, sizeof(ldr_xfer_query));
+ ldr_xfer_query.hdr.command = LOADER_CMD_XFER_QUERY;
+ ldr_xfer_query.image_size = fw->size;
+ rv = loader_cl_send(client_data,
+ (u8 *)&ldr_xfer_query,
+ sizeof(ldr_xfer_query));
+ if (rv < 0) {
+ client_data->flag_retry = true;
+ goto end_error;
+ }
+
+ /* Check buffer size before accessing the elements */
+ data_len = client_data->response_size;
+ if (data_len != sizeof(struct loader_xfer_query_response)) {
+ dev_err(cl_data_to_dev(client_data),
+ "data size %zu is not equal to size of loader_xfer_query_response %zu\n",
+ data_len, sizeof(struct loader_xfer_query_response));
+ hdr = (struct loader_msg_hdr *)client_data->response_data;
+ report_bad_packet(client_data->loader_ishtp_cl, hdr);
+ client_data->flag_retry = true;
+ rv = -EMSGSIZE;
+ goto end_error;
+ }
+
+ /* Save fw_info for use outside this function */
+ ldr_xfer_query_resp =
+ (struct loader_xfer_query_response *)client_data->response_data;
+ *fw_info = ldr_xfer_query_resp->fw_info;
+
+ /* Loader firmware properties */
+ dev_dbg(cl_data_to_dev(client_data),
+ "ish_fw_version: major=%d minor=%d hotfix=%d build=%d protocol_version=0x%x loader_version=%d\n",
+ fw_info->ish_fw_version.major,
+ fw_info->ish_fw_version.minor,
+ fw_info->ish_fw_version.hotfix,
+ fw_info->ish_fw_version.build,
+ fw_info->protocol_version,
+ fw_info->ldr_version.value);
+
+ dev_dbg(cl_data_to_dev(client_data),
+ "loader_capability: max_fw_image_size=0x%x xfer_mode=%d max_dma_buf_size=0x%x dma_buf_size_limit=0x%x\n",
+ fw_info->ldr_capability.max_fw_image_size,
+ fw_info->ldr_capability.xfer_mode,
+ fw_info->ldr_capability.max_dma_buf_size,
+ dma_buf_size_limit);
+
+ /* Sanity checks */
+ if (fw_info->ldr_capability.max_fw_image_size < fw->size) {
+ dev_err(cl_data_to_dev(client_data),
+ "ISH firmware size %zu is greater than Shim firmware loader max supported %d\n",
+ fw->size,
+ fw_info->ldr_capability.max_fw_image_size);
+ rv = -ENOSPC;
+ goto end_error;
+ }
+
+ /* For DMA the buffer size should be multiple of cacheline size */
+ if ((fw_info->ldr_capability.xfer_mode & LOADER_XFER_MODE_DIRECT_DMA) &&
+ (fw_info->ldr_capability.max_dma_buf_size % L1_CACHE_BYTES)) {
+ dev_err(cl_data_to_dev(client_data),
+ "Shim firmware loader buffer size %d should be multipe of cacheline\n",
+ fw_info->ldr_capability.max_dma_buf_size);
+ rv = -EINVAL;
+ goto end_error;
+ }
+
+end_error:
+ /* Free ISH buffer if not done so in error case */
+ kfree(client_data->response_data);
+ client_data->response_data = NULL;
+ return rv;
+}
+
+/**
+ * ish_fw_xfer_ishtp() Loads ISH firmware using ishtp interface
+ * @client_data: Client data instance
+ * @fw: Pointer to fw data struct in host memory
+ *
+ * This function uses ISH-TP to transfer ISH firmware from host to
+ * ISH SRAM. Lower layers may use IPC or DMA depending on firmware
+ * support.
+ *
+ * Return: 0 for success, negative error code for failure.
+ */
+static int ish_fw_xfer_ishtp(struct ishtp_cl_data *client_data,
+ const struct firmware *fw)
+{
+ int rv;
+ u32 fragment_offset, fragment_size, payload_max_size;
+ struct loader_xfer_ipc_fragment *ldr_xfer_ipc_frag;
+
+ payload_max_size =
+ LOADER_SHIM_IPC_BUF_SIZE - IPC_FRAGMENT_DATA_PREAMBLE;
+
+ ldr_xfer_ipc_frag = kzalloc(LOADER_SHIM_IPC_BUF_SIZE, GFP_KERNEL);
+ if (!ldr_xfer_ipc_frag) {
+ client_data->flag_retry = true;
+ return -ENOMEM;
+ }
+
+ ldr_xfer_ipc_frag->fragment.hdr.command = LOADER_CMD_XFER_FRAGMENT;
+ ldr_xfer_ipc_frag->fragment.xfer_mode = LOADER_XFER_MODE_ISHTP;
+
+ /* Break the firmware image into fragments and send as ISH-TP payload */
+ fragment_offset = 0;
+ while (fragment_offset < fw->size) {
+ if (fragment_offset + payload_max_size < fw->size) {
+ fragment_size = payload_max_size;
+ ldr_xfer_ipc_frag->fragment.is_last = 0;
+ } else {
+ fragment_size = fw->size - fragment_offset;
+ ldr_xfer_ipc_frag->fragment.is_last = 1;
+ }
+
+ ldr_xfer_ipc_frag->fragment.offset = fragment_offset;
+ ldr_xfer_ipc_frag->fragment.size = fragment_size;
+ memcpy(ldr_xfer_ipc_frag->data,
+ &fw->data[fragment_offset],
+ fragment_size);
+
+ dev_dbg(cl_data_to_dev(client_data),
+ "xfer_mode=ipc offset=0x%08x size=0x%08x is_last=%d\n",
+ ldr_xfer_ipc_frag->fragment.offset,
+ ldr_xfer_ipc_frag->fragment.size,
+ ldr_xfer_ipc_frag->fragment.is_last);
+
+ rv = loader_cl_send(client_data,
+ (u8 *)ldr_xfer_ipc_frag,
+ IPC_FRAGMENT_DATA_PREAMBLE + fragment_size);
+ if (rv < 0) {
+ client_data->flag_retry = true;
+ goto end_err_resp_buf_release;
+ }
+
+ /* Free ISH buffer once response is processed */
+ kfree(client_data->response_data);
+ client_data->response_data = NULL;
+
+ fragment_offset += fragment_size;
+ }
+
+ kfree(ldr_xfer_ipc_frag);
+ return 0;
+
+end_err_resp_buf_release:
+ /* Free ISH buffer if not done already, in error case */
+ kfree(client_data->response_data);
+ client_data->response_data = NULL;
+ kfree(ldr_xfer_ipc_frag);
+ return rv;
+}
+
+/**
+ * ish_fw_xfer_direct_dma() - Loads ISH firmware using direct dma
+ * @client_data: Client data instance
+ * @fw: Poiner to fw data struct in host memory
+ *
+ * Host firmware load is a unique case where we need to download
+ * a large firmware image (200+ Kb). This function implements
+ * direct DMA transfer in kernel and ISH firmware. This allows
+ * us to overcome the ISH-TP 4 Kb limit, and allows us to DMA
+ * directly to ISH UMA at location of choice.
+ * Function depends on corresponding support in ISH firmware.
+ *
+ * Return: 0 for success, negative error code for failure.
+ */
+static int ish_fw_xfer_direct_dma(struct ishtp_cl_data *client_data,
+ const struct firmware *fw,
+ struct shim_fw_info fw_info)
+{
+ int rv;
+ void *dma_buf;
+ dma_addr_t dma_buf_phy;
+ u32 fragment_offset, fragment_size, payload_max_size;
+ struct loader_xfer_dma_fragment ldr_xfer_dma_frag;
+ struct device *devc = ishtp_get_pci_device(client_data->cl_device);
+ u32 shim_fw_buf_size =
+ fw_info.ldr_capability.max_dma_buf_size;
+
+ /*
+ * payload_max_size should be set to minimum of
+ * (1) Size of firmware to be loaded,
+ * (2) Max DMA buf size supported by Shim firmware,
+ * (3) DMA buffer size limit set by boot_param dma_buf_size_limit.
+ */
+ payload_max_size = min3(fw->size,
+ (size_t)shim_fw_buf_size,
+ (size_t)dma_buf_size_limit);
+
+ /*
+ * Buffer size should be multiple of cacheline size
+ * if it's not, select the previous cacheline boundary.
+ */
+ payload_max_size &= ~(L1_CACHE_BYTES - 1);
+
+ dma_buf = kmalloc(payload_max_size, GFP_KERNEL | GFP_DMA32);
+ if (!dma_buf) {
+ client_data->flag_retry = true;
+ return -ENOMEM;
+ }
+
+ dma_buf_phy = dma_map_single(devc, dma_buf, payload_max_size,
+ DMA_TO_DEVICE);
+ if (dma_mapping_error(devc, dma_buf_phy)) {
+ dev_err(cl_data_to_dev(client_data), "DMA map failed\n");
+ client_data->flag_retry = true;
+ rv = -ENOMEM;
+ goto end_err_dma_buf_release;
+ }
+
+ ldr_xfer_dma_frag.fragment.hdr.command = LOADER_CMD_XFER_FRAGMENT;
+ ldr_xfer_dma_frag.fragment.xfer_mode = LOADER_XFER_MODE_DIRECT_DMA;
+ ldr_xfer_dma_frag.ddr_phys_addr = (u64)dma_buf_phy;
+
+ /* Send the firmware image in chucks of payload_max_size */
+ fragment_offset = 0;
+ while (fragment_offset < fw->size) {
+ if (fragment_offset + payload_max_size < fw->size) {
+ fragment_size = payload_max_size;
+ ldr_xfer_dma_frag.fragment.is_last = 0;
+ } else {
+ fragment_size = fw->size - fragment_offset;
+ ldr_xfer_dma_frag.fragment.is_last = 1;
+ }
+
+ ldr_xfer_dma_frag.fragment.offset = fragment_offset;
+ ldr_xfer_dma_frag.fragment.size = fragment_size;
+ memcpy(dma_buf, &fw->data[fragment_offset], fragment_size);
+
+ dma_sync_single_for_device(devc, dma_buf_phy,
+ payload_max_size,
+ DMA_TO_DEVICE);
+
+ /*
+ * Flush cache here because the dma_sync_single_for_device()
+ * does not do for x86.
+ */
+ clflush_cache_range(dma_buf, payload_max_size);
+
+ dev_dbg(cl_data_to_dev(client_data),
+ "xfer_mode=dma offset=0x%08x size=0x%x is_last=%d ddr_phys_addr=0x%016llx\n",
+ ldr_xfer_dma_frag.fragment.offset,
+ ldr_xfer_dma_frag.fragment.size,
+ ldr_xfer_dma_frag.fragment.is_last,
+ ldr_xfer_dma_frag.ddr_phys_addr);
+
+ rv = loader_cl_send(client_data,
+ (u8 *)&ldr_xfer_dma_frag,
+ sizeof(ldr_xfer_dma_frag));
+ if (rv < 0) {
+ client_data->flag_retry = true;
+ goto end_err_resp_buf_release;
+ }
+
+ /* Free ISH buffer once response is processed */
+ kfree(client_data->response_data);
+ client_data->response_data = NULL;
+
+ fragment_offset += fragment_size;
+ }
+
+ dma_unmap_single(devc, dma_buf_phy, payload_max_size, DMA_TO_DEVICE);
+ kfree(dma_buf);
+ return 0;
+
+end_err_resp_buf_release:
+ /* Free ISH buffer if not done already, in error case */
+ kfree(client_data->response_data);
+ client_data->response_data = NULL;
+ dma_unmap_single(devc, dma_buf_phy, payload_max_size, DMA_TO_DEVICE);
+end_err_dma_buf_release:
+ kfree(dma_buf);
+ return rv;
+}
+
+/**
+ * ish_fw_start() Start executing ISH main firmware
+ * @client_data: client data instance
+ *
+ * This function sends message to Shim firmware loader to start
+ * the execution of ISH main firmware.
+ *
+ * Return: 0 for success, negative error code for failure.
+ */
+static int ish_fw_start(struct ishtp_cl_data *client_data)
+{
+ int rv;
+ struct loader_start ldr_start;
+
+ memset(&ldr_start, 0, sizeof(ldr_start));
+ ldr_start.hdr.command = LOADER_CMD_START;
+ rv = loader_cl_send(client_data,
+ (u8 *)&ldr_start,
+ sizeof(ldr_start));
+
+ /* Free ISH buffer once response is processed */
+ kfree(client_data->response_data);
+ client_data->response_data = NULL;
+ return rv;
+}
+
+/**
+ * load_fw_from_host() Loads ISH firmware from host
+ * @client_data: Client data instance
+ *
+ * This function loads the ISH firmware to ISH sram and starts execution
+ *
+ * Return: 0 for success, negative error code for failure.
+ */
+static int load_fw_from_host(struct ishtp_cl_data *client_data)
+{
+ int rv;
+ u32 xfer_mode;
+ char *filename;
+ const struct firmware *fw;
+ struct shim_fw_info fw_info;
+
+ client_data->flag_retry = false;
+
+ filename = kzalloc(FILENAME_SIZE, GFP_KERNEL);
+ if (!filename) {
+ rv = -ENOMEM;
+ goto end_error;
+ }
+
+ /* Get filename of the ISH firmware to be loaded */
+ rv = get_firmware_variant(client_data, filename);
+ if (rv < 0)
+ goto end_err_filename_buf_release;
+
+ rv = request_firmware(&fw, filename, cl_data_to_dev(client_data));
+ if (rv < 0)
+ goto end_err_filename_buf_release;
+
+ /* Step 1: Query Shim firmware loader properties */
+
+ rv = ish_query_loader_prop(client_data, fw, &fw_info);
+ if (rv < 0)
+ goto end_err_fw_release;
+
+ /* Step 2: Send the main firmware image to be loaded, to ISH sram */
+
+ xfer_mode = fw_info.ldr_capability.xfer_mode;
+ if (xfer_mode & LOADER_XFER_MODE_DIRECT_DMA) {
+ rv = ish_fw_xfer_direct_dma(client_data, fw, fw_info);
+ } else if (xfer_mode & LOADER_XFER_MODE_ISHTP) {
+ rv = ish_fw_xfer_ishtp(client_data, fw);
+ } else {
+ dev_err(cl_data_to_dev(client_data),
+ "No transfer mode selected in firmware\n");
+ rv = -EINVAL;
+ }
+ if (rv < 0)
+ goto end_err_fw_release;
+
+ /* Step 3: Start ISH main firmware exeuction */
+
+ rv = ish_fw_start(client_data);
+ if (rv < 0)
+ goto end_err_fw_release;
+
+ release_firmware(fw);
+ kfree(filename);
+ dev_info(cl_data_to_dev(client_data), "ISH firmware %s loaded\n",
+ filename);
+ return 0;
+
+end_err_fw_release:
+ release_firmware(fw);
+end_err_filename_buf_release:
+ kfree(filename);
+end_error:
+ if (client_data->flag_retry) {
+ dev_warn(cl_data_to_dev(client_data),
+ "ISH host firmware load failed %d. Reset ISH & try again..\n",
+ rv);
+ loader_ish_hw_reset(client_data->loader_ishtp_cl);
+ } else {
+ dev_err(cl_data_to_dev(client_data),
+ "ISH host firmware load failed %d\n", rv);
+ }
+ return rv;
+}
+
+static void load_fw_from_host_handler(struct work_struct *work)
+{
+ struct ishtp_cl_data *client_data;
+
+ client_data = container_of(work, struct ishtp_cl_data,
+ work_fw_load);
+ load_fw_from_host(client_data);
+}
+
+/**
+ * loader_init() - Init function for ISH-TP client
+ * @loader_ishtp_cl: ISH-TP client instance
+ * @reset: true if called for init after reset
+ *
+ * Return: 0 for success, negative error code for failure
+ */
+static int loader_init(struct ishtp_cl *loader_ishtp_cl, int reset)
+{
+ int rv;
+ struct ishtp_fw_client *fw_client;
+ struct ishtp_cl_data *client_data =
+ ishtp_get_client_data(loader_ishtp_cl);
+
+ dev_dbg(cl_data_to_dev(client_data), "reset flag: %d\n", reset);
+
+ rv = ishtp_cl_link(loader_ishtp_cl);
+ if (rv < 0) {
+ dev_err(cl_data_to_dev(client_data), "ishtp_cl_link failed\n");
+ return rv;
+ }
+
+ /* Connect to firmware client */
+ ishtp_set_tx_ring_size(loader_ishtp_cl, LOADER_CL_TX_RING_SIZE);
+ ishtp_set_rx_ring_size(loader_ishtp_cl, LOADER_CL_RX_RING_SIZE);
+
+ fw_client =
+ ishtp_fw_cl_get_client(ishtp_get_ishtp_device(loader_ishtp_cl),
+ &loader_ishtp_guid);
+ if (!fw_client) {
+ dev_err(cl_data_to_dev(client_data),
+ "ISH client uuid not found\n");
+ rv = -ENOENT;
+ goto err_cl_unlink;
+ }
+
+ ishtp_cl_set_fw_client_id(loader_ishtp_cl,
+ ishtp_get_fw_client_id(fw_client));
+ ishtp_set_connection_state(loader_ishtp_cl, ISHTP_CL_CONNECTING);
+
+ rv = ishtp_cl_connect(loader_ishtp_cl);
+ if (rv < 0) {
+ dev_err(cl_data_to_dev(client_data), "Client connect fail\n");
+ goto err_cl_unlink;
+ }
+
+ dev_dbg(cl_data_to_dev(client_data), "Client connected\n");
+
+ ishtp_register_event_cb(client_data->cl_device, loader_cl_event_cb);
+
+ return 0;
+
+err_cl_unlink:
+ ishtp_cl_unlink(loader_ishtp_cl);
+ return rv;
+}
+
+static void loader_deinit(struct ishtp_cl *loader_ishtp_cl)
+{
+ ishtp_set_connection_state(loader_ishtp_cl, ISHTP_CL_DISCONNECTING);
+ ishtp_cl_disconnect(loader_ishtp_cl);
+ ishtp_cl_unlink(loader_ishtp_cl);
+ ishtp_cl_flush_queues(loader_ishtp_cl);
+
+ /* Disband and free all Tx and Rx client-level rings */
+ ishtp_cl_free(loader_ishtp_cl);
+}
+
+static void reset_handler(struct work_struct *work)
+{
+ int rv;
+ struct ishtp_cl_data *client_data;
+ struct ishtp_cl *loader_ishtp_cl;
+ struct ishtp_cl_device *cl_device;
+
+ client_data = container_of(work, struct ishtp_cl_data,
+ work_ishtp_reset);
+
+ loader_ishtp_cl = client_data->loader_ishtp_cl;
+ cl_device = client_data->cl_device;
+
+ /* Unlink, flush queues & start again */
+ ishtp_cl_unlink(loader_ishtp_cl);
+ ishtp_cl_flush_queues(loader_ishtp_cl);
+ ishtp_cl_free(loader_ishtp_cl);
+
+ loader_ishtp_cl = ishtp_cl_allocate(cl_device);
+ if (!loader_ishtp_cl)
+ return;
+
+ ishtp_set_drvdata(cl_device, loader_ishtp_cl);
+ ishtp_set_client_data(loader_ishtp_cl, client_data);
+ client_data->loader_ishtp_cl = loader_ishtp_cl;
+ client_data->cl_device = cl_device;
+
+ rv = loader_init(loader_ishtp_cl, 1);
+ if (rv < 0) {
+ dev_err(ishtp_device(cl_device), "Reset Failed\n");
+ return;
+ }
+
+ /* ISH firmware loading from host */
+ load_fw_from_host(client_data);
+}
+
+/**
+ * loader_ishtp_cl_probe() - ISH-TP client driver probe
+ * @cl_device: ISH-TP client device instance
+ *
+ * This function gets called on device create on ISH-TP bus
+ *
+ * Return: 0 for success, negative error code for failure
+ */
+static int loader_ishtp_cl_probe(struct ishtp_cl_device *cl_device)
+{
+ struct ishtp_cl *loader_ishtp_cl;
+ struct ishtp_cl_data *client_data;
+ int rv;
+
+ client_data = devm_kzalloc(ishtp_device(cl_device),
+ sizeof(*client_data),
+ GFP_KERNEL);
+ if (!client_data)
+ return -ENOMEM;
+
+ loader_ishtp_cl = ishtp_cl_allocate(cl_device);
+ if (!loader_ishtp_cl)
+ return -ENOMEM;
+
+ ishtp_set_drvdata(cl_device, loader_ishtp_cl);
+ ishtp_set_client_data(loader_ishtp_cl, client_data);
+ client_data->loader_ishtp_cl = loader_ishtp_cl;
+ client_data->cl_device = cl_device;
+
+ init_waitqueue_head(&client_data->cmd_resp_wait);
+
+ INIT_WORK(&client_data->work_ishtp_reset,
+ reset_handler);
+ INIT_WORK(&client_data->work_fw_load,
+ load_fw_from_host_handler);
+
+ rv = loader_init(loader_ishtp_cl, 0);
+ if (rv < 0) {
+ ishtp_cl_free(loader_ishtp_cl);
+ return rv;
+ }
+ ishtp_get_device(cl_device);
+
+ /* ISH firmware loading from host */
+ schedule_work(&client_data->work_fw_load);
+
+ return 0;
+}
+
+/**
+ * loader_ishtp_cl_remove() - ISH-TP client driver remove
+ * @cl_device: ISH-TP client device instance
+ *
+ * This function gets called on device remove on ISH-TP bus
+ *
+ * Return: 0
+ */
+static int loader_ishtp_cl_remove(struct ishtp_cl_device *cl_device)
+{
+ struct ishtp_cl_data *client_data;
+ struct ishtp_cl *loader_ishtp_cl = ishtp_get_drvdata(cl_device);
+
+ client_data = ishtp_get_client_data(loader_ishtp_cl);
+
+ /*
+ * The sequence of the following two cancel_work_sync() is
+ * important. The work_fw_load can in turn schedue
+ * work_ishtp_reset, so first cancel work_fw_load then
+ * cancel work_ishtp_reset.
+ */
+ cancel_work_sync(&client_data->work_fw_load);
+ cancel_work_sync(&client_data->work_ishtp_reset);
+ loader_deinit(loader_ishtp_cl);
+ ishtp_put_device(cl_device);
+
+ return 0;
+}
+
+/**
+ * loader_ishtp_cl_reset() - ISH-TP client driver reset
+ * @cl_device: ISH-TP client device instance
+ *
+ * This function gets called on device reset on ISH-TP bus
+ *
+ * Return: 0
+ */
+static int loader_ishtp_cl_reset(struct ishtp_cl_device *cl_device)
+{
+ struct ishtp_cl_data *client_data;
+ struct ishtp_cl *loader_ishtp_cl = ishtp_get_drvdata(cl_device);
+
+ client_data = ishtp_get_client_data(loader_ishtp_cl);
+
+ schedule_work(&client_data->work_ishtp_reset);
+
+ return 0;
+}
+
+static struct ishtp_cl_driver loader_ishtp_cl_driver = {
+ .name = "ish-loader",
+ .guid = &loader_ishtp_guid,
+ .probe = loader_ishtp_cl_probe,
+ .remove = loader_ishtp_cl_remove,
+ .reset = loader_ishtp_cl_reset,
+};
+
+static int __init ish_loader_init(void)
+{
+ return ishtp_cl_driver_register(&loader_ishtp_cl_driver, THIS_MODULE);
+}
+
+static void __exit ish_loader_exit(void)
+{
+ ishtp_cl_driver_unregister(&loader_ishtp_cl_driver);
+}
+
+late_initcall(ish_loader_init);
+module_exit(ish_loader_exit);
+
+module_param(dma_buf_size_limit, int, 0644);
+MODULE_PARM_DESC(dma_buf_size_limit, "Limit the DMA buf size to this value in bytes");
+
+MODULE_DESCRIPTION("ISH ISH-TP Host firmware Loader Client Driver");
+MODULE_AUTHOR("Rushikesh S Kadam <rushikesh.s.kadam@intel.com>");
+
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("ishtp:*");
--
1.9.1
^ permalink raw reply related
* [PATCH] input: pm8xxx-vibrator: fix a potential NULL pointer dereference
From: Kangjie Lu @ 2019-03-23 2:44 UTC (permalink / raw)
To: kjlu; +Cc: pakki001, Dmitry Torokhov, linux-input, linux-kernel
In case of_device_get_match_data fails to find the matched data,
returns -ENODEV
Signed-off-by: Kangjie Lu <kjlu@umn.edu>
---
drivers/input/misc/pm8xxx-vibrator.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/input/misc/pm8xxx-vibrator.c b/drivers/input/misc/pm8xxx-vibrator.c
index 7dd1c1fbe42a..740e59c11808 100644
--- a/drivers/input/misc/pm8xxx-vibrator.c
+++ b/drivers/input/misc/pm8xxx-vibrator.c
@@ -196,6 +196,8 @@ static int pm8xxx_vib_probe(struct platform_device *pdev)
vib->vib_input_dev = input_dev;
regs = of_device_get_match_data(&pdev->dev);
+ if (unlikely(!regs))
+ return -ENODEV;
/* operate in manual mode */
error = regmap_read(vib->regmap, regs->drv_addr, &val);
--
2.17.1
^ permalink raw reply related
* Re: [RESEND PATCH v6 09/11] leds: max77650: add LEDs support
From: Jacek Anaszewski @ 2019-03-22 19:40 UTC (permalink / raw)
To: Bartosz Golaszewski, Pavel Machek
Cc: Rob Herring, Mark Rutland, Linus Walleij, Dmitry Torokhov,
Lee Jones, Sebastian Reichel, Liam Girdwood, Greg Kroah-Hartman,
Linux Kernel Mailing List, open list:GPIO SUBSYSTEM, devicetree,
Linux Input, Linux LED Subsystem, Linux PM list,
Bartosz Golaszewski
In-Reply-To: <CAMRc=Mf1DxF1e63y+HsXtCFxZW8R0XWDLUXGy+4cp3zBS0_qhw@mail.gmail.com>
On 3/22/19 10:53 AM, Bartosz Golaszewski wrote:
> pt., 22 mar 2019 o 10:21 Pavel Machek <pavel@ucw.cz> napisał(a):
>>
>> On Mon 2019-03-18 18:42:26, Bartosz Golaszewski wrote:
>>> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>>>
>>> This adds basic support for LEDs for the max77650 PMIC. The device has
>>> three current sinks for driving LEDs.
>>>
>>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>>> Acked-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
>>
>> Acked-by: Pavel Machek <pavel@ucw.cz>
>>
>>> + label = of_get_property(child, "label", NULL);
>>> + if (!label) {
>>> + led->cdev.name = "max77650::";
>>> + } else {
>>> + led->cdev.name = devm_kasprintf(dev, GFP_KERNEL,
>>> + "max77650:%s", label);
>>> + if (!led->cdev.name)
>>> + return -ENOMEM;
>>> + }
>>
>> I'd rather not have the "max77650:" prefix in the LED name (as it is
>> useless).
>>
>
> I was instructed to do so by the LED subsystem maintainer.
Yes, let's keep things consistent, we will convert it to use generic
support for LED naming once that support is merged.
--
Best regards,
Jacek Anaszewski
^ permalink raw reply
* Re: [Xen-devel][PATCH] Input: xen-kbdfront - signal the backend that we disconnect
From: Oleksandr Andrushchenko @ 2019-03-22 11:09 UTC (permalink / raw)
To: Juergen Gross, xen-devel, linux-input, linux-kernel,
dmitry.torokhov, boris.ostrovsky
Cc: Volodymyr_Babchuk, Oleksandr Andrushchenko
In-Reply-To: <32add426-ae36-ecc9-aa3e-6169fe04f231@suse.com>
On 3/22/19 1:03 PM, Juergen Gross wrote:
> On 15/03/2019 10:23, Oleksandr Andrushchenko wrote:
>> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
>>
>> While disconnecting from the backend we clean up shared resources
>> (event channel, ring buffer), but never let the backend know about
>> that. This may lead to inconsistent backend state and/or shared
>> resources use.
>> Fix this by explicitly letting the backend know that frontend has
>> destroyed shared resources by changing its Xen bus state accordingly.
>>
>> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
>> ---
>> drivers/input/misc/xen-kbdfront.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/input/misc/xen-kbdfront.c b/drivers/input/misc/xen-kbdfront.c
>> index 24bc5c5d876f..ecb6e719e0e2 100644
>> --- a/drivers/input/misc/xen-kbdfront.c
>> +++ b/drivers/input/misc/xen-kbdfront.c
>> @@ -488,6 +488,8 @@ static int xenkbd_connect_backend(struct xenbus_device *dev,
>>
>> static void xenkbd_disconnect_backend(struct xenkbd_info *info)
>> {
>> + xenbus_switch_state(info->xbdev, XenbusStateClosing);
>> +
>> if (info->irq >= 0)
>> unbind_from_irqhandler(info->irq, info);
>> info->irq = -1;
>>
> As already stated for the related netfront patch: I'm not sure this
> is really what we want. Have you tested xl save/restore or migration
> of the guest with your patch applied?
Well, it comes out that this is not enough and needs much more work,
so please let's abandon this patch
>
> Juergen
Thank you,
Oleksandr
^ permalink raw reply
* Re: [Xen-devel][PATCH] Input: xen-kbdfront - signal the backend that we disconnect
From: Juergen Gross @ 2019-03-22 11:03 UTC (permalink / raw)
To: Oleksandr Andrushchenko, xen-devel, linux-input, linux-kernel,
dmitry.torokhov, boris.ostrovsky
Cc: Volodymyr_Babchuk, Oleksandr Andrushchenko
In-Reply-To: <20190315092348.32613-1-andr2000@gmail.com>
On 15/03/2019 10:23, Oleksandr Andrushchenko wrote:
> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
>
> While disconnecting from the backend we clean up shared resources
> (event channel, ring buffer), but never let the backend know about
> that. This may lead to inconsistent backend state and/or shared
> resources use.
> Fix this by explicitly letting the backend know that frontend has
> destroyed shared resources by changing its Xen bus state accordingly.
>
> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
> ---
> drivers/input/misc/xen-kbdfront.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/input/misc/xen-kbdfront.c b/drivers/input/misc/xen-kbdfront.c
> index 24bc5c5d876f..ecb6e719e0e2 100644
> --- a/drivers/input/misc/xen-kbdfront.c
> +++ b/drivers/input/misc/xen-kbdfront.c
> @@ -488,6 +488,8 @@ static int xenkbd_connect_backend(struct xenbus_device *dev,
>
> static void xenkbd_disconnect_backend(struct xenkbd_info *info)
> {
> + xenbus_switch_state(info->xbdev, XenbusStateClosing);
> +
> if (info->irq >= 0)
> unbind_from_irqhandler(info->irq, info);
> info->irq = -1;
>
As already stated for the related netfront patch: I'm not sure this
is really what we want. Have you tested xl save/restore or migration
of the guest with your patch applied?
Juergen
^ permalink raw reply
* Re: [RESEND PATCH v6 04/11] dt-bindings: input: add DT bindings for max77650
From: Pavel Machek @ 2019-03-22 9:59 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Rob Herring, Mark Rutland, Linus Walleij, Dmitry Torokhov,
Jacek Anaszewski, Lee Jones, Sebastian Reichel, Liam Girdwood,
Greg Kroah-Hartman, Linux Kernel Mailing List,
open list:GPIO SUBSYSTEM, devicetree, Linux Input,
Linux LED Subsystem, Linux PM list, Bartosz Golaszewski,
Rob Herring
In-Reply-To: <CAMRc=Md0-UkY07vkf+irqUKwHFSj=Nq3fBYKFRqNG9nzWv9YRw@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1138 bytes --]
On Fri 2019-03-22 10:55:26, Bartosz Golaszewski wrote:
> pt., 22 mar 2019 o 10:09 Pavel Machek <pavel@ucw.cz> napisał(a):
> >
> > On Mon 2019-03-18 18:42:21, Bartosz Golaszewski wrote:
> > > From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > >
> > > Add the DT binding document for the onkey module of max77650.
> > >
> > > Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > > Reviewed-by: Rob Herring <robh@kernel.org>
> >
> > > + onkey {
> > > + compatible = "maxim,max77650-onkey";
> > > + linux,code = <KEY_END>;
> > > + maxim,onkey-slide;
> > > + };
> >
> > This is certainly wrong.
> >
> > "End" key is normal key on a keyboard, certainly not some kind of
> > slider.
> >
> > And this controller is likely to be used for power key, not end key,
> > right?
>
> But this is just an example of available properties, not a real use-case.
We can have example that makes sense, right?
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* Re: [RESEND PATCH v6 04/11] dt-bindings: input: add DT bindings for max77650
From: Bartosz Golaszewski @ 2019-03-22 9:55 UTC (permalink / raw)
To: Pavel Machek
Cc: Rob Herring, Mark Rutland, Linus Walleij, Dmitry Torokhov,
Jacek Anaszewski, Lee Jones, Sebastian Reichel, Liam Girdwood,
Greg Kroah-Hartman, Linux Kernel Mailing List,
open list:GPIO SUBSYSTEM, devicetree, Linux Input,
Linux LED Subsystem, Linux PM list, Bartosz Golaszewski,
Rob Herring
In-Reply-To: <20190322090927.GG27015@amd>
pt., 22 mar 2019 o 10:09 Pavel Machek <pavel@ucw.cz> napisał(a):
>
> On Mon 2019-03-18 18:42:21, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> >
> > Add the DT binding document for the onkey module of max77650.
> >
> > Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > Reviewed-by: Rob Herring <robh@kernel.org>
>
> > + onkey {
> > + compatible = "maxim,max77650-onkey";
> > + linux,code = <KEY_END>;
> > + maxim,onkey-slide;
> > + };
>
> This is certainly wrong.
>
> "End" key is normal key on a keyboard, certainly not some kind of
> slider.
>
> And this controller is likely to be used for power key, not end key,
> right?
But this is just an example of available properties, not a real use-case.
Bart
>
> Pavel
> --
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
^ permalink raw reply
* Re: [RESEND PATCH v6 03/11] dt-bindings: leds: add DT bindings for max77650
From: Bartosz Golaszewski @ 2019-03-22 9:54 UTC (permalink / raw)
To: Pavel Machek
Cc: Rob Herring, Mark Rutland, Linus Walleij, Dmitry Torokhov,
Jacek Anaszewski, Lee Jones, Sebastian Reichel, Liam Girdwood,
Greg Kroah-Hartman, Linux Kernel Mailing List,
open list:GPIO SUBSYSTEM, devicetree, Linux Input,
Linux LED Subsystem, Linux PM list, Bartosz Golaszewski,
Rob Herring
In-Reply-To: <20190322090208.GF27015@amd>
pt., 22 mar 2019 o 10:02 Pavel Machek <pavel@ucw.cz> napisał(a):
>
> On Mon 2019-03-18 18:40:32, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> >
> > Add the DT binding document for the LEDs module of max77650.
> >
> > Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > Reviewed-by: Rob Herring <robh@kernel.org>
>
> Acked-by: Pavel Machek <pavel@ucw.cz>
>
> > + led@0 {
> > + reg = <0>;
> > + label = "blue:usr0";
> > + };
>
> I'd rather not see these "usrX" things... Does it have some kind of
> real function?
Not really. In the design we're using it for, it's unused and on the
development kit from MAXIM it's simple three colored leds.
Bart
> Pavel
> --
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
^ permalink raw reply
* Re: [RESEND PATCH v6 09/11] leds: max77650: add LEDs support
From: Bartosz Golaszewski @ 2019-03-22 9:53 UTC (permalink / raw)
To: Pavel Machek
Cc: Rob Herring, Mark Rutland, Linus Walleij, Dmitry Torokhov,
Jacek Anaszewski, Lee Jones, Sebastian Reichel, Liam Girdwood,
Greg Kroah-Hartman, Linux Kernel Mailing List,
open list:GPIO SUBSYSTEM, devicetree, Linux Input,
Linux LED Subsystem, Linux PM list, Bartosz Golaszewski
In-Reply-To: <20190322092124.GK27015@amd>
pt., 22 mar 2019 o 10:21 Pavel Machek <pavel@ucw.cz> napisał(a):
>
> On Mon 2019-03-18 18:42:26, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> >
> > This adds basic support for LEDs for the max77650 PMIC. The device has
> > three current sinks for driving LEDs.
> >
> > Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > Acked-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
>
> Acked-by: Pavel Machek <pavel@ucw.cz>
>
> > + label = of_get_property(child, "label", NULL);
> > + if (!label) {
> > + led->cdev.name = "max77650::";
> > + } else {
> > + led->cdev.name = devm_kasprintf(dev, GFP_KERNEL,
> > + "max77650:%s", label);
> > + if (!led->cdev.name)
> > + return -ENOMEM;
> > + }
>
> I'd rather not have the "max77650:" prefix in the LED name (as it is
> useless).
>
I was instructed to do so by the LED subsystem maintainer.
Bart
> Best regards,
> Pavel
> --
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
^ permalink raw reply
* Re: [RESEND PATCH v6 09/11] leds: max77650: add LEDs support
From: Pavel Machek @ 2019-03-22 9:21 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Rob Herring, Mark Rutland, Linus Walleij, Dmitry Torokhov,
Jacek Anaszewski, Lee Jones, Sebastian Reichel, Liam Girdwood,
Greg Kroah-Hartman, linux-kernel, linux-gpio, devicetree,
linux-input, linux-leds, linux-pm, Bartosz Golaszewski
In-Reply-To: <20190318174228.18194-10-brgl@bgdev.pl>
[-- Attachment #1: Type: text/plain, Size: 935 bytes --]
On Mon 2019-03-18 18:42:26, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>
> This adds basic support for LEDs for the max77650 PMIC. The device has
> three current sinks for driving LEDs.
>
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> Acked-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
Acked-by: Pavel Machek <pavel@ucw.cz>
> + label = of_get_property(child, "label", NULL);
> + if (!label) {
> + led->cdev.name = "max77650::";
> + } else {
> + led->cdev.name = devm_kasprintf(dev, GFP_KERNEL,
> + "max77650:%s", label);
> + if (!led->cdev.name)
> + return -ENOMEM;
> + }
I'd rather not have the "max77650:" prefix in the LED name (as it is
useless).
Best regards,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* Re: [RESEND PATCH v6 10/11] input: max77650: add onkey support
From: Pavel Machek @ 2019-03-22 9:18 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Rob Herring, Mark Rutland, Linus Walleij, Dmitry Torokhov,
Jacek Anaszewski, Lee Jones, Sebastian Reichel, Liam Girdwood,
Greg Kroah-Hartman, linux-kernel, linux-gpio, devicetree,
linux-input, linux-leds, linux-pm, Bartosz Golaszewski
In-Reply-To: <20190318174228.18194-11-brgl@bgdev.pl>
[-- Attachment #1: Type: text/plain, Size: 493 bytes --]
On Mon 2019-03-18 18:42:27, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>
> Add support for the push- and slide-button events for max77650.
>
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Acked-by: Pavel Machek <pavel@ucw.cz>
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* Re: [RESEND PATCH v6 06/11] mfd: max77650: new core mfd driver
From: Pavel Machek @ 2019-03-22 9:12 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Rob Herring, Mark Rutland, Linus Walleij, Dmitry Torokhov,
Jacek Anaszewski, Lee Jones, Sebastian Reichel, Liam Girdwood,
Greg Kroah-Hartman, linux-kernel, linux-gpio, devicetree,
linux-input, linux-leds, linux-pm, Bartosz Golaszewski
In-Reply-To: <20190318174228.18194-7-brgl@bgdev.pl>
[-- Attachment #1: Type: text/plain, Size: 1358 bytes --]
On Mon 2019-03-18 18:42:23, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>
> Add the core mfd driver for max77650 PMIC. We define five sub-devices
> for which the drivers will be added in subsequent patches.
>
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> ---
> drivers/mfd/Kconfig | 14 +++
> drivers/mfd/Makefile | 1 +
> drivers/mfd/max77650.c | 234 +++++++++++++++++++++++++++++++++++
> include/linux/mfd/max77650.h | 59 +++++++++
> 4 files changed, 308 insertions(+)
> create mode 100644 drivers/mfd/max77650.c
> create mode 100644 include/linux/mfd/max77650.h
>
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 0ce2d8dfc5f1..ade04e124aa0 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -733,6 +733,20 @@ config MFD_MAX77620
> provides common support for accessing the device; additional drivers
> must be enabled in order to use the functionality of the device.
>
> +config MFD_MAX77650
> + tristate "Maxim MAX77650/77651 PMIC Support"
> + depends on I2C
> + depends on OF || COMPILE_TEST
This says it will compile ok in !OF case. Will it?
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* Re: [RESEND PATCH v6 05/11] mfd: core: document mfd_add_devices()
From: Pavel Machek @ 2019-03-22 9:10 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Rob Herring, Mark Rutland, Linus Walleij, Dmitry Torokhov,
Jacek Anaszewski, Lee Jones, Sebastian Reichel, Liam Girdwood,
Greg Kroah-Hartman, linux-kernel, linux-gpio, devicetree,
linux-input, linux-leds, linux-pm, Bartosz Golaszewski
In-Reply-To: <20190318174228.18194-6-brgl@bgdev.pl>
[-- Attachment #1: Type: text/plain, Size: 412 bytes --]
On Mon 2019-03-18 18:42:22, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>
> Add a kernel doc for mfd_add_devices().
>
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
Acked-by: Pavel Machek <pavel@ucw.cz>
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ 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