* Re: [PATCH] ELAN touchpad i2c_hid bugs fix
From: Kai-Heng Feng @ 2019-03-21 4:08 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Hans de Goede, Benjamin Tissoires, hotwater438, Jiri Kosina,
Stephen Boyd, Sebastian Andrzej Siewior, Dmitry Torokhov,
open list:HID CORE LAYER, lkml
In-Reply-To: <CAHp75VcOG9H8u+tZyNN682r7+jsRZi8VoPPZ2uJPch1gNyFgmQ@mail.gmail.com>
at 01:18, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> On Wed, Mar 20, 2019 at 6:55 PM Kai-Heng Feng
> <kai.heng.feng@canonical.com> wrote:
>> at 23:39, Hans de Goede <hdegoede@redhat.com> wrote:
>>> On 3/20/19 3:37 PM, Benjamin Tissoires wrote:
>
>>> Benjamin, what I find interesting here is that the BOGUS_IRQ quirk
>>> is also used on Elan devices, I suspect that these Elan devices
>>> likely also need the I2C_HID_QUIRK_FORCE_TRIGGER_FALLING quirk
>>> and then they probably will no longer need the bogus IRQ flag,
>>> if you know about bugreports with an acpidump for any of the devices
>>> needing the bogus IRQ quirk, then I (or you) can check how the IRQ is
>>> declared there, I suspect it will be declared as level-low, just like
>>> with the laptop this patch was written for. And it probably need to
>>> be edge-falling instead of level-low just like this case.
>>
>> First, I’ve already tried using IRQF_TRIGGER_FALLING, unfortunately it
>> doesn’t solve the issue for me.
>>
>> I talked to Elan once, and they confirm the correct IRQ trigger is level
>> low. So forcing falling trigger may break other platforms.
>
> As far as I understood Vladislav the quirk he got from Elan as well.
Ok, then this is really weird.
>
>> Recently we found that Elan touchpad doesn’t like GpioInt() from its _CRS.
>> Once the Interrupt() is used instead, the issue goes away.
>
> IIRC i2c core tries to get interrupt from Interrupt() resource and
> then falls back to GpioInt().
> See i2c_acpi_get_info() and i2c_device_probe().
Here’s its ASL:
Scope (\_SB.PCI0.I2C4)
{
Device (TPD0)
{
Name (_ADR, One) // _ADR: Address
Name (_HID, "DELL08AE") // _HID: Hardware ID
Name (_CID, "PNP0C50" /* HID Protocol Device (I2C bus) */) // _CID: Compatible ID
Name (_UID, One) // _UID: Unique ID
Name (_S0W, 0x04) // _S0W: S0 Device Wake State
Name (SBFB, ResourceTemplate ()
{
I2cSerialBusV2 (0x002C, ControllerInitiated, 0x00061A80,
AddressingMode7Bit, "\\_SB.PCI0.I2C4",
0x00, ResourceConsumer, , Exclusive,
)
})
Name (SBFG, ResourceTemplate ()
{
GpioInt (Level, ActiveLow, ExclusiveAndWake, PullUp, 0x0000,
"\\_SB.GPO1", 0x00, ResourceConsumer, ,
)
{ // Pin list
0x0012
}
})
Name (SBFI, ResourceTemplate ()
{
Interrupt (ResourceConsumer, Level, ActiveLow, ExclusiveAndWake, ,, )
{
0x0000003C,
}
})
Method (_INI, 0, NotSerialized) // _INI: Initialize
{
}
Method (_STA, 0, NotSerialized) // _STA: Status
{
If ((TCPD == One))
{
Return (0x0F)
}
Return (Zero)
}
Method (_CRS, 0, NotSerialized) // _CRS: Current Resource Settings
{
If ((OSYS < 0x07DC))
{
Return (SBFI) /* \_SB_.PCI0.I2C4.TPD0.SBFI */
}
Return (ConcatenateResTemplate (SBFB, SBFG))
}
Method (_DSM, 4, NotSerialized) // _DSM: Device-Specific Method
{
If ((Arg0 == ToUUID ("3cdff6f7-4267-4555-ad05-b30a3d8938de") /* HID I2C Device */))
{
If ((Arg2 == Zero))
{
If ((Arg1 == One))
{
Return (Buffer (One)
{
0x03 // .
})
}
Else
{
Return (Buffer (One)
{
0x00 // .
})
}
}
ElseIf ((Arg2 == One))
{
Return (0x20)
}
Else
{
Return (Buffer (One)
{
0x00 // .
})
}
}
ElseIf ((Arg0 == ToUUID ("ef87eb82-f951-46da-84ec-14871ac6f84b")))
{
If ((Arg2 == Zero))
{
If ((Arg1 == One))
{
Return (Buffer (One)
{
0x03 // .
})
}
}
If ((Arg2 == One))
{
Return (ConcatenateResTemplate (SBFB, SBFG))
}
Return (Buffer (One)
{
0x00 // .
})
}
}
Else
{
Return (Buffer (One)
{
0x00 // .
})
}
}
}
}
Change SBFG to SBFI in its _CRS can workaround the issue.
Is ASL in this form possible to do the flow you described?
Kai-Heng
>
>> But I am not sure how to patch its DSDT/SSDT in i2c-hid.
>
> --
> With Best Regards,
> Andy Shevchenko
^ permalink raw reply
* [PATCH v7 0/4] input: touchscreen: Add goodix GT5553 CTP support
From: Jagan Teki @ 2019-03-21 8:21 UTC (permalink / raw)
To: Dmitry Torokhov, Bastien Nocera, Rob Herring
Cc: Henrik Rydberg, linux-input, linux-kernel, devicetree,
Mark Rutland, linux-amarula, Michael Trimarchi, Jagan Teki
This is v7 patchset for supporting goodix GT5553 CTP. Here is the
previous version[1]
Changes for v5:
- rebase on linux-next
Changes for v5:
- document bindings for required regulators, which are need during
power-on sequence
- enable, disable required regulators as described in power-on sequence
using normal regulator calls
- update the proper commi messages
Changes for v4:
- document AVDD22, DVDD12, VDDIO as optional properties
- use regulator bulk calls, for get, enable and disable functionalities
Changes for v4:
- devm_add_action_or_reset for disabling regulator
Changes for v3:
- add cover-letter
- s/ADVV28/AVDD28 on commit head
- fix few typo
Changes for v2:
- Rename vcc-supply with AVDD28-supply
- disable regulator in remove
- fix to setup regulator in probe code
- add chipdata
- drop example node in dt-bindings
[1] https://patchwork.kernel.org/cover/10819645/
Jagan Teki (4):
dt-bindings: input: touchscreen: goodix: Document regulator properties
Input: goodix - Add regulators suppot
dt-bindings: input: touchscreen: goodix: Add GT5663 compatible
Input: goodix - Add GT5663 CTP support
.../bindings/input/touchscreen/goodix.txt | 3 +
drivers/input/touchscreen/goodix.c | 60 +++++++++++++++++++
2 files changed, 63 insertions(+)
--
2.18.0.321.gffc6fa0e3
^ permalink raw reply
* [PATCH v7 1/4] dt-bindings: input: touchscreen: goodix: Document regulator properties
From: Jagan Teki @ 2019-03-21 8:21 UTC (permalink / raw)
To: Dmitry Torokhov, Bastien Nocera, Rob Herring
Cc: Henrik Rydberg, linux-input, linux-kernel, devicetree,
Mark Rutland, linux-amarula, Michael Trimarchi, Jagan Teki
In-Reply-To: <20190321082104.2874-1-jagan@amarulasolutions.com>
Goodix CTP controllers support analog, digital and gpio regulator
supplies on relevant controller pin configurations.
Out of which AVDD28 and VDDIO regulators are required in few goodix CTP
chips during power-on sequence.
AVDD22, DVDD12 regulators have no relevant functionality described from
datasheet [1].
So, document both AVDD28, VDDIO regulators into optional properties since
few of the goodix chip do work without these regulator power-on sequence.
[1] GT5663 Datasheet_English_20151106_Rev.01
Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
Documentation/devicetree/bindings/input/touchscreen/goodix.txt | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/input/touchscreen/goodix.txt b/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
index 8cf0b4d38a7e..255673250bbd 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
+++ b/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
@@ -19,6 +19,8 @@ Optional properties:
- irq-gpios : GPIO pin used for IRQ. The driver uses the
interrupt gpio pin as output to reset the device.
- reset-gpios : GPIO pin used for reset
+ - AVDD28-supply : Analog power supply regulator on AVDD28 pin
+ - VDDIO-supply : GPIO power supply regulator on VDDIO pin
- touchscreen-inverted-x
- touchscreen-inverted-y
- touchscreen-size-x
--
2.18.0.321.gffc6fa0e3
^ permalink raw reply related
* [PATCH v7 2/4] Input: goodix - Add regulators suppot
From: Jagan Teki @ 2019-03-21 8:21 UTC (permalink / raw)
To: Dmitry Torokhov, Bastien Nocera, Rob Herring
Cc: Henrik Rydberg, linux-input, linux-kernel, devicetree,
Mark Rutland, linux-amarula, Michael Trimarchi, Jagan Teki
In-Reply-To: <20190321082104.2874-1-jagan@amarulasolutions.com>
Goodix CTP controllers require AVDD28, VDDIO regulators for power-on
sequence.
The delay between these regualtor operations as per Power-on Timing
from datasheet[1] is 0 (T1 >= 0 usec).
So, enable and disable these regulators in proper order using normal
regulator functions without any delay in between.
[1] GT5663 Datasheet_English_20151106_Rev.01
Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
drivers/input/touchscreen/goodix.c | 58 ++++++++++++++++++++++++++++++
1 file changed, 58 insertions(+)
diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
index f57d82220a88..de5b80a08f41 100644
--- a/drivers/input/touchscreen/goodix.c
+++ b/drivers/input/touchscreen/goodix.c
@@ -27,6 +27,7 @@
#include <linux/delay.h>
#include <linux/irq.h>
#include <linux/interrupt.h>
+#include <linux/regulator/consumer.h>
#include <linux/slab.h>
#include <linux/acpi.h>
#include <linux/of.h>
@@ -47,6 +48,8 @@ struct goodix_ts_data {
struct touchscreen_properties prop;
unsigned int max_touch_num;
unsigned int int_trigger_type;
+ struct regulator *avdd28;
+ struct regulator *vddio;
struct gpio_desc *gpiod_int;
struct gpio_desc *gpiod_rst;
u16 id;
@@ -532,6 +535,24 @@ static int goodix_get_gpio_config(struct goodix_ts_data *ts)
return -EINVAL;
dev = &ts->client->dev;
+ ts->avdd28 = devm_regulator_get(dev, "AVDD28");
+ if (IS_ERR(ts->avdd28)) {
+ error = PTR_ERR(ts->avdd28);
+ if (error != -EPROBE_DEFER)
+ dev_err(dev,
+ "Failed to get AVDD28 regulator: %d\n", error);
+ return error;
+ }
+
+ ts->vddio = devm_regulator_get(dev, "VDDIO");
+ if (IS_ERR(ts->vddio)) {
+ error = PTR_ERR(ts->vddio);
+ if (error != -EPROBE_DEFER)
+ dev_err(dev,
+ "Failed to get VDDIO regulator: %d\n", error);
+ return error;
+ }
+
/* Get the interrupt GPIO pin number */
gpiod = devm_gpiod_get_optional(dev, GOODIX_GPIO_INT_NAME, GPIOD_IN);
if (IS_ERR(gpiod)) {
@@ -764,6 +785,17 @@ static void goodix_config_cb(const struct firmware *cfg, void *ctx)
complete_all(&ts->firmware_loading_complete);
}
+static void goodix_disable_regulator(void *arg)
+{
+ struct goodix_ts_data *ts = arg;
+
+ if (!IS_ERR(ts->vddio))
+ regulator_disable(ts->vddio);
+
+ if (!IS_ERR(ts->avdd28))
+ regulator_disable(ts->avdd28);
+}
+
static int goodix_ts_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
@@ -789,6 +821,32 @@ static int goodix_ts_probe(struct i2c_client *client,
if (error)
return error;
+ error = devm_add_action_or_reset(&client->dev,
+ goodix_disable_regulator, ts);
+ if (error)
+ return error;
+
+ /* power the controller */
+ if (!IS_ERR(ts->avdd28)) {
+ error = regulator_enable(ts->avdd28);
+ if (error) {
+ dev_err(&client->dev,
+ "Failed to enable AVDD28 regulator: %d\n",
+ error);
+ return error;
+ }
+ }
+
+ if (!IS_ERR(ts->vddio)) {
+ error = regulator_enable(ts->vddio);
+ if (error) {
+ dev_err(&client->dev,
+ "Failed to enable VDDIO regulator: %d\n",
+ error);
+ return error;
+ }
+ }
+
if (ts->gpiod_int && ts->gpiod_rst) {
/* reset the controller */
error = goodix_reset(ts);
--
2.18.0.321.gffc6fa0e3
^ permalink raw reply related
* [PATCH v7 3/4] dt-bindings: input: touchscreen: goodix: Add GT5663 compatible
From: Jagan Teki @ 2019-03-21 8:21 UTC (permalink / raw)
To: Dmitry Torokhov, Bastien Nocera, Rob Herring
Cc: Henrik Rydberg, linux-input, linux-kernel, devicetree,
Mark Rutland, linux-amarula, Michael Trimarchi, Jagan Teki
In-Reply-To: <20190321082104.2874-1-jagan@amarulasolutions.com>
GT5663 is capacitive touch controller with customized smart
wakeup gestures, it support chipdata which is similar to
existing GT1151 and require AVDD28 supply for some boards.
Document the compatible for the same.
Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
Documentation/devicetree/bindings/input/touchscreen/goodix.txt | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/input/touchscreen/goodix.txt b/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
index 255673250bbd..fc03ea4cf5ab 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
+++ b/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
@@ -3,6 +3,7 @@ Device tree bindings for Goodix GT9xx series touchscreen controller
Required properties:
- compatible : Should be "goodix,gt1151"
+ or "goodix,gt5663"
or "goodix,gt5688"
or "goodix,gt911"
or "goodix,gt9110"
--
2.18.0.321.gffc6fa0e3
^ permalink raw reply related
* [PATCH v7 4/4] Input: goodix - Add GT5663 CTP support
From: Jagan Teki @ 2019-03-21 8:21 UTC (permalink / raw)
To: Dmitry Torokhov, Bastien Nocera, Rob Herring
Cc: Henrik Rydberg, linux-input, linux-kernel, devicetree,
Mark Rutland, linux-amarula, Michael Trimarchi, Jagan Teki
In-Reply-To: <20190321082104.2874-1-jagan@amarulasolutions.com>
GT5663 is capacitive touch controller with customized smart
wakeup gestures.
Add support for it by adding compatible and supported chip data.
The chip data on GT5663 is similar to GT1151, like
- config data register has 0x8050 address
- config data register max len is 240
- config data checksum has 16-bit
Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
drivers/input/touchscreen/goodix.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
index de5b80a08f41..c558b091749c 100644
--- a/drivers/input/touchscreen/goodix.c
+++ b/drivers/input/touchscreen/goodix.c
@@ -219,6 +219,7 @@ static const struct goodix_chip_data *goodix_get_chip_data(u16 id)
{
switch (id) {
case 1151:
+ case 5663:
case 5688:
return >1x_chip_data;
@@ -1003,6 +1004,7 @@ MODULE_DEVICE_TABLE(acpi, goodix_acpi_match);
#ifdef CONFIG_OF
static const struct of_device_id goodix_of_match[] = {
{ .compatible = "goodix,gt1151" },
+ { .compatible = "goodix,gt5663" },
{ .compatible = "goodix,gt5688" },
{ .compatible = "goodix,gt911" },
{ .compatible = "goodix,gt9110" },
--
2.18.0.321.gffc6fa0e3
^ permalink raw reply related
* Re: [PATCH] ELAN touchpad i2c_hid bugs fix
From: Andy Shevchenko @ 2019-03-21 8:55 UTC (permalink / raw)
To: Kai-Heng Feng
Cc: Hans de Goede, Benjamin Tissoires, hotwater438, Jiri Kosina,
Stephen Boyd, Sebastian Andrzej Siewior, Dmitry Torokhov,
open list:HID CORE LAYER, lkml
In-Reply-To: <220FEDBD-9A71-4C14-A7D6-3850D515865F@canonical.com>
On Thu, Mar 21, 2019 at 6:08 AM Kai-Heng Feng
<kai.heng.feng@canonical.com> wrote:
> at 01:18, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > On Wed, Mar 20, 2019 at 6:55 PM Kai-Heng Feng
> > <kai.heng.feng@canonical.com> wrote:
> >> at 23:39, Hans de Goede <hdegoede@redhat.com> wrote:
> >>> On 3/20/19 3:37 PM, Benjamin Tissoires wrote:
> >> Recently we found that Elan touchpad doesn’t like GpioInt() from its _CRS.
> >> Once the Interrupt() is used instead, the issue goes away.
> >
> > IIRC i2c core tries to get interrupt from Interrupt() resource and
> > then falls back to GpioInt().
> > See i2c_acpi_get_info() and i2c_device_probe().
>
> Here’s its ASL:
> Name (SBFB, ResourceTemplate ()
> {
> I2cSerialBusV2 (0x002C, ControllerInitiated, 0x00061A80,
> AddressingMode7Bit, "\\_SB.PCI0.I2C4",
> 0x00, ResourceConsumer, , Exclusive,
> )
> })
> Name (SBFG, ResourceTemplate ()
> {
> GpioInt (Level, ActiveLow, ExclusiveAndWake, PullUp, 0x0000,
> "\\_SB.GPO1", 0x00, ResourceConsumer, ,
> )
> { // Pin list
> 0x0012
> }
> })
> Name (SBFI, ResourceTemplate ()
> {
> Interrupt (ResourceConsumer, Level, ActiveLow, ExclusiveAndWake, ,, )
> {
> 0x0000003C,
> }
> })
> Method (_CRS, 0, NotSerialized) // _CRS: Current Resource Settings
> {
> If ((OSYS < 0x07DC))
> {
> Return (SBFI) /* \_SB_.PCI0.I2C4.TPD0.SBFI */
> }
This will return only Interrupt() resource
>
> Return (ConcatenateResTemplate (SBFB, SBFG))
This one I2cSerialBus() and GpioInt().
> }
> }
> }
>
> Change SBFG to SBFI in its _CRS can workaround the issue.
> Is ASL in this form possible to do the flow you described?
Since it's enumerated in Linux as I2C device, it means it gets I2C and
GPIO resources.
So, no, it's not possible.
What are you describing might tell us about one of the following:
- touchpad should be switched to PS/2 mode in order to get working
- GPIO resource is not correct / bug in GPIO driver
I don't believe the first one is a case here.
If GPIO resource is not correct and main OS has some quirks, we need
to do similar in Linux.
Otherwise, debugging GPIO driver, starting from what exactly (pin
number, pin settings, etc) gets i2c-hid module.
Note, ACPICA and related stuff is done in order to be Windows compatible.
If you have settings in BIOS that defines OS to boot, it should be
chosen Windows.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH] ELAN touchpad i2c_hid bugs fix
From: Hans de Goede @ 2019-03-21 8:57 UTC (permalink / raw)
To: Kai-Heng Feng, Andy Shevchenko
Cc: Benjamin Tissoires, hotwater438, Jiri Kosina, Stephen Boyd,
Sebastian Andrzej Siewior, Dmitry Torokhov,
open list:HID CORE LAYER, lkml
In-Reply-To: <220FEDBD-9A71-4C14-A7D6-3850D515865F@canonical.com>
Hi,
On 21-03-19 05:08, Kai-Heng Feng wrote:
> at 01:18, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
>
>> On Wed, Mar 20, 2019 at 6:55 PM Kai-Heng Feng
>> <kai.heng.feng@canonical.com> wrote:
>>> at 23:39, Hans de Goede <hdegoede@redhat.com> wrote:
>>>> On 3/20/19 3:37 PM, Benjamin Tissoires wrote:
>>
>>>> Benjamin, what I find interesting here is that the BOGUS_IRQ quirk
>>>> is also used on Elan devices, I suspect that these Elan devices
>>>> likely also need the I2C_HID_QUIRK_FORCE_TRIGGER_FALLING quirk
>>>> and then they probably will no longer need the bogus IRQ flag,
>>>> if you know about bugreports with an acpidump for any of the devices
>>>> needing the bogus IRQ quirk, then I (or you) can check how the IRQ is
>>>> declared there, I suspect it will be declared as level-low, just like
>>>> with the laptop this patch was written for. And it probably need to
>>>> be edge-falling instead of level-low just like this case.
>>>
>>> First, I’ve already tried using IRQF_TRIGGER_FALLING, unfortunately it
>>> doesn’t solve the issue for me.
>>>
>>> I talked to Elan once, and they confirm the correct IRQ trigger is level
>>> low. So forcing falling trigger may break other platforms.
>>
>> As far as I understood Vladislav the quirk he got from Elan as well.
>
> Ok, then this is really weird.
>
>>
>>> Recently we found that Elan touchpad doesn’t like GpioInt() from its _CRS.
>>> Once the Interrupt() is used instead, the issue goes away.
>>
>> IIRC i2c core tries to get interrupt from Interrupt() resource and
>> then falls back to GpioInt().
>> See i2c_acpi_get_info() and i2c_device_probe().
>
> Here’s its ASL:
>
> Scope (\_SB.PCI0.I2C4)
> {
> Device (TPD0)
> {
> Name (_ADR, One) // _ADR: Address
> Name (_HID, "DELL08AE") // _HID: Hardware ID
> Name (_CID, "PNP0C50" /* HID Protocol Device (I2C bus) */) // _CID: Compatible ID
> Name (_UID, One) // _UID: Unique ID
> Name (_S0W, 0x04) // _S0W: S0 Device Wake State
> Name (SBFB, ResourceTemplate ()
> {
> I2cSerialBusV2 (0x002C, ControllerInitiated, 0x00061A80,
> AddressingMode7Bit, "\\_SB.PCI0.I2C4",
> 0x00, ResourceConsumer, , Exclusive,
> )
> })
> Name (SBFG, ResourceTemplate ()
> {
> GpioInt (Level, ActiveLow, ExclusiveAndWake, PullUp, 0x0000,
> "\\_SB.GPO1", 0x00, ResourceConsumer, ,
> )
> { // Pin list
> 0x0012
> }
> })
> Name (SBFI, ResourceTemplate ()
> {
> Interrupt (ResourceConsumer, Level, ActiveLow, ExclusiveAndWake, ,, )
> {
> 0x0000003C,
> }
> })
OK, so both interrupt definitions declare the interrupt as Level, ActiveLow, so forcing
falling-edge here *might* help too.
Regards,
Hans
^ permalink raw reply
* Re: [PATCH] ELAN touchpad i2c_hid bugs fix
From: Kai Heng Feng @ 2019-03-21 9:28 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Hans de Goede, Benjamin Tissoires, hotwater438, Jiri Kosina,
Stephen Boyd, Sebastian Andrzej Siewior, Dmitry Torokhov,
open list:HID CORE LAYER, lkml
In-Reply-To: <CAHp75VeavkzZDYK4CWAC62GciynQ3KYgmPpPRwsd3RcLFw4jxA@mail.gmail.com>
> On Mar 21, 2019, at 4:55 PM, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
>
> On Thu, Mar 21, 2019 at 6:08 AM Kai-Heng Feng
> <kai.heng.feng@canonical.com> wrote:
>> at 01:18, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
>>> On Wed, Mar 20, 2019 at 6:55 PM Kai-Heng Feng
>>> <kai.heng.feng@canonical.com> wrote:
>>>> at 23:39, Hans de Goede <hdegoede@redhat.com> wrote:
>>>>> On 3/20/19 3:37 PM, Benjamin Tissoires wrote:
>
>>>> Recently we found that Elan touchpad doesn’t like GpioInt() from its _CRS.
>>>> Once the Interrupt() is used instead, the issue goes away.
>>>
>>> IIRC i2c core tries to get interrupt from Interrupt() resource and
>>> then falls back to GpioInt().
>>> See i2c_acpi_get_info() and i2c_device_probe().
>>
>> Here’s its ASL:
>
>> Name (SBFB, ResourceTemplate ()
>> {
>> I2cSerialBusV2 (0x002C, ControllerInitiated, 0x00061A80,
>> AddressingMode7Bit, "\\_SB.PCI0.I2C4",
>> 0x00, ResourceConsumer, , Exclusive,
>> )
>> })
>> Name (SBFG, ResourceTemplate ()
>> {
>> GpioInt (Level, ActiveLow, ExclusiveAndWake, PullUp, 0x0000,
>> "\\_SB.GPO1", 0x00, ResourceConsumer, ,
>> )
>> { // Pin list
>> 0x0012
>> }
>> })
>> Name (SBFI, ResourceTemplate ()
>> {
>> Interrupt (ResourceConsumer, Level, ActiveLow, ExclusiveAndWake, ,, )
>> {
>> 0x0000003C,
>> }
>> })
>
>> Method (_CRS, 0, NotSerialized) // _CRS: Current Resource Settings
>> {
>
>> If ((OSYS < 0x07DC))
>> {
>> Return (SBFI) /* \_SB_.PCI0.I2C4.TPD0.SBFI */
>> }
>
> This will return only Interrupt() resource
>
>>
>> Return (ConcatenateResTemplate (SBFB, SBFG))
>
> This one I2cSerialBus() and GpioInt().
>
>> }
>
>
>> }
>> }
>>
>> Change SBFG to SBFI in its _CRS can workaround the issue.
>> Is ASL in this form possible to do the flow you described?
>
> Since it's enumerated in Linux as I2C device, it means it gets I2C and
> GPIO resources.
> So, no, it's not possible.
>
> What are you describing might tell us about one of the following:
> - touchpad should be switched to PS/2 mode in order to get working
> - GPIO resource is not correct / bug in GPIO driver
>
> I don't believe the first one is a case here.
> If GPIO resource is not correct and main OS has some quirks, we need
> to do similar in Linux.
How do I check quirks on Windows?
> Otherwise, debugging GPIO driver, starting from what exactly (pin
> number, pin settings, etc) gets i2c-hid module.
Ok, please let me know where should I start looking into.
>
> Note, ACPICA and related stuff is done in order to be Windows compatible.
> If you have settings in BIOS that defines OS to boot, it should be
> chosen Windows.
Yes, it’s Windows.
Kai-Heng
>
> --
> With Best Regards,
> Andy Shevchenko
^ permalink raw reply
* Re: [PATCH] ELAN touchpad i2c_hid bugs fix
From: Andy Shevchenko @ 2019-03-21 9:48 UTC (permalink / raw)
To: Kai-Heng Feng, Mario Limonciello
Cc: Hans de Goede, Benjamin Tissoires, hotwater438, Jiri Kosina,
Stephen Boyd, Sebastian Andrzej Siewior, Dmitry Torokhov,
open list:HID CORE LAYER, lkml
In-Reply-To: <220FEDBD-9A71-4C14-A7D6-3850D515865F@canonical.com>
+Cc: Mario
Mario, do you have any insights about the issue with touchpad on Dell
system described below?
On Thu, Mar 21, 2019 at 6:08 AM Kai-Heng Feng
<kai.heng.feng@canonical.com> wrote:
>
> at 01:18, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
>
> > On Wed, Mar 20, 2019 at 6:55 PM Kai-Heng Feng
> > <kai.heng.feng@canonical.com> wrote:
> >> at 23:39, Hans de Goede <hdegoede@redhat.com> wrote:
> >>> On 3/20/19 3:37 PM, Benjamin Tissoires wrote:
> >
> >>> Benjamin, what I find interesting here is that the BOGUS_IRQ quirk
> >>> is also used on Elan devices, I suspect that these Elan devices
> >>> likely also need the I2C_HID_QUIRK_FORCE_TRIGGER_FALLING quirk
> >>> and then they probably will no longer need the bogus IRQ flag,
> >>> if you know about bugreports with an acpidump for any of the devices
> >>> needing the bogus IRQ quirk, then I (or you) can check how the IRQ is
> >>> declared there, I suspect it will be declared as level-low, just like
> >>> with the laptop this patch was written for. And it probably need to
> >>> be edge-falling instead of level-low just like this case.
> >>
> >> First, I’ve already tried using IRQF_TRIGGER_FALLING, unfortunately it
> >> doesn’t solve the issue for me.
> >>
> >> I talked to Elan once, and they confirm the correct IRQ trigger is level
> >> low. So forcing falling trigger may break other platforms.
> >
> > As far as I understood Vladislav the quirk he got from Elan as well.
>
> Ok, then this is really weird.
>
> >
> >> Recently we found that Elan touchpad doesn’t like GpioInt() from its _CRS.
> >> Once the Interrupt() is used instead, the issue goes away.
> >
> > IIRC i2c core tries to get interrupt from Interrupt() resource and
> > then falls back to GpioInt().
> > See i2c_acpi_get_info() and i2c_device_probe().
>
> Here’s its ASL:
>
> Scope (\_SB.PCI0.I2C4)
> {
> Device (TPD0)
> {
> Name (_ADR, One) // _ADR: Address
> Name (_HID, "DELL08AE") // _HID: Hardware ID
> Name (_CID, "PNP0C50" /* HID Protocol Device (I2C bus) */) // _CID: Compatible ID
> Name (_UID, One) // _UID: Unique ID
> Name (_S0W, 0x04) // _S0W: S0 Device Wake State
> Name (SBFB, ResourceTemplate ()
> {
> I2cSerialBusV2 (0x002C, ControllerInitiated, 0x00061A80,
> AddressingMode7Bit, "\\_SB.PCI0.I2C4",
> 0x00, ResourceConsumer, , Exclusive,
> )
> })
> Name (SBFG, ResourceTemplate ()
> {
> GpioInt (Level, ActiveLow, ExclusiveAndWake, PullUp, 0x0000,
> "\\_SB.GPO1", 0x00, ResourceConsumer, ,
> )
> { // Pin list
> 0x0012
> }
> })
> Name (SBFI, ResourceTemplate ()
> {
> Interrupt (ResourceConsumer, Level, ActiveLow, ExclusiveAndWake, ,, )
> {
> 0x0000003C,
> }
> })
> Method (_INI, 0, NotSerialized) // _INI: Initialize
> {
> }
> Method (_STA, 0, NotSerialized) // _STA: Status
> {
> If ((TCPD == One))
> {
> Return (0x0F)
> }
>
> Return (Zero)
> }
> Method (_CRS, 0, NotSerialized) // _CRS: Current Resource Settings
> {
> If ((OSYS < 0x07DC))
> {
> Return (SBFI) /* \_SB_.PCI0.I2C4.TPD0.SBFI */
> }
>
> Return (ConcatenateResTemplate (SBFB, SBFG))
> }
> Method (_DSM, 4, NotSerialized) // _DSM: Device-Specific Method
> {
> If ((Arg0 == ToUUID ("3cdff6f7-4267-4555-ad05-b30a3d8938de") /* HID I2C Device */))
> {
> If ((Arg2 == Zero))
> {
> If ((Arg1 == One))
> {
> Return (Buffer (One)
> {
> 0x03 // .
> })
> }
> Else
> {
> Return (Buffer (One)
> {
> 0x00 // .
> })
> }
> }
> ElseIf ((Arg2 == One))
> {
> Return (0x20)
> }
> Else
> {
> Return (Buffer (One)
> {
> 0x00 // .
> })
> }
> }
> ElseIf ((Arg0 == ToUUID ("ef87eb82-f951-46da-84ec-14871ac6f84b")))
> {
> If ((Arg2 == Zero))
> {
> If ((Arg1 == One))
> {
> Return (Buffer (One)
> {
> 0x03 // .
> })
> }
> }
>
> If ((Arg2 == One))
> {
> Return (ConcatenateResTemplate (SBFB, SBFG))
> }
>
> Return (Buffer (One)
> {
> 0x00 // .
> })
> }
> }
> Else
> {
> Return (Buffer (One)
> {
> 0x00 // .
> })
> }
> }
> }
> }
>
> Change SBFG to SBFI in its _CRS can workaround the issue.
> Is ASL in this form possible to do the flow you described?
>
> Kai-Heng
>
> >
> >> But I am not sure how to patch its DSDT/SSDT in i2c-hid.
> >
> > --
> > With Best Regards,
> > Andy Shevchenko
>
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH 1/7] Input: document meanings of KEY_SCREEN and KEY_ZOOM
From: Mauro Carvalho Chehab @ 2019-03-21 12:17 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Jiri Kosina, Benjamin Tissoires, linux-input, linux-kernel,
linux-media
In-Reply-To: <20190118233037.87318-1-dmitry.torokhov@gmail.com>
Em Fri, 18 Jan 2019 15:30:31 -0800
Dmitry Torokhov <dmitry.torokhov@gmail.com> escreveu:
> It is hard to say what KEY_SCREEN and KEY_ZOOM mean, but historically DVB
> folks have used them to indicate switch to full screen mode. Later, they
> converged on using KEY_ZOOM to switch into full screen mode and KEY)SCREEN
> to control aspect ratio (see Documentation/media/uapi/rc/rc-tables.rst).
>
> Let's commit to these uses, and define:
>
> - KEY_FULL_SCREEN (and make KEY_ZOOM its alias)
> - KEY_ASPECT_RATIO (and make KEY_SCREEN its alias)
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Feel free to apply via your tree.
Acked-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
> ---
>
> Please let me know how we want merge this. Some of patches can be applied
> independently and I tried marking them as such, but some require new key
> names from input.h
>
> include/uapi/linux/input-event-codes.h | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h
> index ae366b87426a..bc5054e51bef 100644
> --- a/include/uapi/linux/input-event-codes.h
> +++ b/include/uapi/linux/input-event-codes.h
> @@ -439,10 +439,12 @@
> #define KEY_TITLE 0x171
> #define KEY_SUBTITLE 0x172
> #define KEY_ANGLE 0x173
> -#define KEY_ZOOM 0x174
> +#define KEY_FULL_SCREEN 0x174 /* AC View Toggle */
> +#define KEY_ZOOM KEY_FULL_SCREEN
> #define KEY_MODE 0x175
> #define KEY_KEYBOARD 0x176
> -#define KEY_SCREEN 0x177
> +#define KEY_ASPECT_RATIO 0x177 /* HUTRR37: Aspect */
> +#define KEY_SCREEN KEY_ASPECT_RATIO
> #define KEY_PC 0x178 /* Media Select Computer */
> #define KEY_TV 0x179 /* Media Select TV */
> #define KEY_TV2 0x17a /* Media Select Cable */
Thanks,
Mauro
^ permalink raw reply
* Re: [PATCH 2/7] [media] doc-rst: switch to new names for Full Screen/Aspect keys
From: Mauro Carvalho Chehab @ 2019-03-21 12:20 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Jiri Kosina, Benjamin Tissoires, linux-input, linux-kernel,
linux-media
In-Reply-To: <20190218072606.GG242714@dtor-ws>
Em Sun, 17 Feb 2019 23:26:06 -0800
Dmitry Torokhov <dmitry.torokhov@gmail.com> escreveu:
> On Fri, Jan 18, 2019 at 03:30:32PM -0800, Dmitry Torokhov wrote:
> > We defined better names for keys to activate full screen mode or
> > change aspect ratio (while keeping the existing keycodes to avoid
> > breaking userspace), so let's use them in the document.
> >
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > ---
> > Documentation/media/uapi/rc/rc-tables.rst | 4 ++--
>
> Mauro, do you want to take this through your tree or I should pick it up
> with the patch that does renames in uapi header?
Feel free to apply it via your tree. It probably makes sense to keep it
with the series that add the new codes.
Acked-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
>
> Thanks!
>
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/media/uapi/rc/rc-tables.rst b/Documentation/media/uapi/rc/rc-tables.rst
> > index c8ae9479f842..57797e56f45e 100644
> > --- a/Documentation/media/uapi/rc/rc-tables.rst
> > +++ b/Documentation/media/uapi/rc/rc-tables.rst
> > @@ -616,7 +616,7 @@ the remote via /dev/input/event devices.
> >
> > - .. row 78
> >
> > - - ``KEY_SCREEN``
> > + - ``KEY_ASPECT_RATIO``
> >
> > - Select screen aspect ratio
> >
> > @@ -624,7 +624,7 @@ the remote via /dev/input/event devices.
> >
> > - .. row 79
> >
> > - - ``KEY_ZOOM``
> > + - ``KEY_FULL_SCREEN``
> >
> > - Put device into zoom/full screen mode
> >
> > --
> > 2.20.1.321.g9e740568ce-goog
> >
>
Thanks,
Mauro
^ permalink raw reply
* Re: Re: [PATCH V2 2/2] Input: rotaty-encoder - Add DT binding document
From: Alexey Slepov @ 2019-03-22 1:04 UTC (permalink / raw)
To: Rob Herring, Dmitry Torokhov
Cc: Donghoon Han, linux-input, Daniel Mack, linux-kernel, devicetree
In-Reply-To: <20190115202948.GA26482@bogus>
Hello,
i used this rotary-encoder patch in my embedded project and found two
errors:
First, in drivers/input/misc/rotary_encoder.c,
at @@ -237,6 +244,16 @@:
instead of
+ if (err)
+ dev_err(dev, "unable to get keycodes: %d\n", err);
+ return err;
it must be
+ if (err) {
+ dev_err(dev, "unable to get keycodes: %d\n", err);
+ return err;
+ }
otherwise successful creation of device is not possible.
Second, a typo in
Documentation/devicetree/bindings/input/rotary-encoder.txt,
at @@ -48,3 +52,11 @@:
instead of
+ rotary-encoder,relative-keycode = <103>, <108>;
it should be
+ rotary-encoder,relative-keycodes = <103>, <108>;
otherwise keycodes are not found.
I am sorry, I know that E-Mail style is not good.
I have no time right now, but I'll be back in two weeks.
Someone, maybe Mr. Han, could submit a new version of the patch.
If not, I'll try to do it on my return. (it could take some time, since
I am new to patchwork)
Best Regards and thanks
Alexey Slepov
^ permalink raw reply
* Re: [RESEND PATCH v6 01/11] dt-bindings: mfd: add DT bindings for max77650
From: Pavel Machek @ 2019-03-22 8:57 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,
Rob Herring
In-Reply-To: <20190318174040.17899-2-brgl@bgdev.pl>
[-- Attachment #1: Type: text/plain, Size: 538 bytes --]
On Mon 2019-03-18 18:40:30, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>
> Add a DT binding document for max77650 ultra-low power PMIC. This
> describes the core mfd device and the GPIO module.
>
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> Reviewed-by: Rob Herring <robh@kernel.org>
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 02/11] dt-bindings: power: supply: add DT bindings for max77650
From: Pavel Machek @ 2019-03-22 9:00 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: <20190318174040.17899-3-brgl@bgdev.pl>
[-- Attachment #1: Type: text/plain, Size: 1893 bytes --]
On Mon 2019-03-18 18:40:31, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>
> Add the DT binding document for the battery charger module of max77650.
>
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> ---
> .../power/supply/max77650-charger.txt | 27 +++++++++++++++++++
> 1 file changed, 27 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/power/supply/max77650-charger.txt
>
> diff --git a/Documentation/devicetree/bindings/power/supply/max77650-charger.txt b/Documentation/devicetree/bindings/power/supply/max77650-charger.txt
> new file mode 100644
> index 000000000000..d25c95369616
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/supply/max77650-charger.txt
> @@ -0,0 +1,27 @@
> +Battery charger driver for MAX77650 PMIC from Maxim Integrated.
> +
> +This module is part of the MAX77650 MFD device. For more details
> +see Documentation/devicetree/bindings/mfd/max77650.txt.
> +
> +The charger is represented as a sub-node of the PMIC node on the device tree.
> +
> +Required properties:
> +--------------------
> +- compatible: Must be "maxim,max77650-charger"
> +
> +Optional properties:
> +--------------------
> +- min-microvolt: Minimum CHGIN regulation voltage (in microvolts). Must be
> + one of: 4000000, 4100000, 4200000, 4300000, 4400000,
> + 4500000, 4600000, 4700000.
Probably needs "max," prefix. And .. what does this mean? Will charger
shutdown if input is less than this?
> +- curr-lim-microamp: CHGIN input current limit (in microamps). Must be one of:
> + 95000, 190000, 285000, 380000, 475000.
"current-limit-microamp", I guess. And probably "max,current-limit-microamp".
--
(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 03/11] dt-bindings: leds: add DT bindings for max77650
From: Pavel Machek @ 2019-03-22 9:02 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,
Rob Herring
In-Reply-To: <20190318174040.17899-4-brgl@bgdev.pl>
[-- Attachment #1: Type: text/plain, Size: 651 bytes --]
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?
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: Pavel Machek @ 2019-03-22 9:09 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,
Rob Herring
In-Reply-To: <20190318174228.18194-5-brgl@bgdev.pl>
[-- Attachment #1: Type: text/plain, Size: 762 bytes --]
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?
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 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
* 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 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 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 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 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 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 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
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