* [PATCH 0/3] Stop managing the SP clock
From: Lubomir Rintel @ 2019-01-21 6:22 UTC (permalink / raw)
To: Dmitry Torokhov, Michael Turquette
Cc: Stephen Boyd, Rob Herring, Mark Rutland, linux-input, devicetree,
linux-kernel, linux-clk
Hi,
Per discussion in [1] it seems that the kernel has no business managing
this clock: once the SP clock is disabled, it's not sufficient to just
enable it in order to bring the SP core back up.
Just let the firmware keep it enabled and don't expose it to drivers.
[1] https://lore.kernel.org/lkml/154783267051.169631.3197836544646625747@swboyd.mtv.corp.google.com/
The "secure processor" (SP) core runs the keyboard firmware on the
OLPC XO 1.75 laptop.
Lubo
^ permalink raw reply
* RE: [PATCH 1/1] Input: elantech: Use SMBus based on bus info
From: 廖崇榮 @ 2019-01-21 6:21 UTC (permalink / raw)
To: 'Kai-Heng Feng'
Cc: 'Benjamin Tissoires', 'Dmitry Torokhov',
'open list:HID CORE LAYER', 'lkml'
In-Reply-To: <11585D81-F3E5-4924-A010-4B6A11919216@canonical.com>
-----Original Message-----
From: Kai-Heng Feng [mailto:kai.heng.feng@canonical.com]
Sent: Monday, January 21, 2019 12:28 PM
To: 廖崇榮
Cc: Benjamin Tissoires; Dmitry Torokhov; open list:HID CORE LAYER; lkml
Subject: Re: [PATCH 1/1] Input: elantech: Use SMBus based on bus info
> On Jan 18, 2019, at 17:29, 廖崇榮 <kt.liao@emc.com.tw> wrote:
>
>
>
> -----Original Message-----
> From: Kai Heng Feng [mailto:kai.heng.feng@canonical.com]
> Sent: Friday, January 18, 2019 12:15 AM
> To: Benjamin Tissoires
> Cc: Dmitry Torokhov; 廖崇榮; open list:HID CORE LAYER; lkml
> Subject: Re: [PATCH 1/1] Input: elantech: Use SMBus based on bus info
>
>
>
>> On Jan 17, 2019, at 10:42 PM, Benjamin Tissoires
> <benjamin.tissoires@redhat.com> wrote:
>>
>> Hi Kai-Heng,
>>
>> On Thu, Jan 17, 2019 at 10:30 AM Kai-Heng Feng
>> <kai.heng.feng@canonical.com> wrote:
>>>
>>> There are some new HP laptops with Elantech touchpad don't support
>>> multitouch.
>>>
>>> Both ETP_BUS_SMB_HST_NTFY_ONLY and ETP_BUS_PS2_SMB_HST_NTFY devices
>>> can use SMBus to support 5 fingers touch, so we need to chech them too.
>>>
>>> So use elantech_use_host_notify() to do a more thouroughly check.
>>>
>>> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
>>> ---
>>> drivers/input/mouse/elantech.c | 58
>>> +++++++++++++++++-----------------
>>> 1 file changed, 29 insertions(+), 29 deletions(-)
>>>
>>> diff --git a/drivers/input/mouse/elantech.c
>>> b/drivers/input/mouse/elantech.c index 9fe075c137dc..5bcf1c147eb1
>>> 100644
>>> --- a/drivers/input/mouse/elantech.c
>>> +++ b/drivers/input/mouse/elantech.c
>>> @@ -1799,6 +1799,34 @@ static int elantech_create_smbus(struct
>>> psmouse
> *psmouse,
>>> leave_breadcrumbs); }
>>>
>>> +static bool elantech_use_host_notify(struct psmouse *psmouse,
>>> + struct elantech_device_info
>>> +*info) {
>>> + if (ETP_NEW_IC_SMBUS_HOST_NOTIFY(info->fw_version))
>>> + return true;
>>> +
>>> + switch (info->bus) {
>>> + case ETP_BUS_PS2_ONLY:
>>> + /* expected case */
>>> + break;
>>> + case ETP_BUS_SMB_ALERT_ONLY:
>>> + /* fall-through */
>>> + case ETP_BUS_PS2_SMB_ALERT:
>>> + psmouse_dbg(psmouse, "Ignoring SMBus provider
>>> + through
> alert protocol.\n");
>>> + break;
>>> + case ETP_BUS_SMB_HST_NTFY_ONLY:
>>> + /* fall-through */
>>> + case ETP_BUS_PS2_SMB_HST_NTFY:
>>> + return true;
>>> + default:
>>> + psmouse_dbg(psmouse,
>>> + "Ignoring SMBus bus provider %d.\n",
>>> + info->bus);
>>> + }
>>> +
>>> + return false;
>>> +}
>>> +
>>> /**
>>> * elantech_setup_smbus - called once the PS/2 devices are enumerated
>>> * and decides to instantiate a SMBus InterTouch device.
>>> @@ -1818,7 +1846,7 @@ static int elantech_setup_smbus(struct psmouse
> *psmouse,
>>> * i2c_blacklist_pnp_ids.
>>> * Old ICs are up to the user to decide.
>>> */
>>> - if (!ETP_NEW_IC_SMBUS_HOST_NOTIFY(info->fw_version) ||
>>> + if (!elantech_use_host_notify(psmouse, info) ||
>>
>> That was my initial approach of the series, but I ended up being more
>> conservative as this would flip all of the existing elantech SMBUS
>> capable touchpads to use elan_i2c.
>> And I didn't want to deal with 4/5 year old laptops that suddenly broke.
>>
>> So I wonder if you can restrict this default change to the recent
>> laptops (let's say 2018+). Maybe by looking at their FW version or
>> something else in the DMI?
>
> It was KT who told me that I should use ETP_BUS_PS2_SMB_HST_NTFY.
>
> As for date, KT still knows better than me.
>
> KT,
> Can you name a year which is safe enough to enable SMBus?
>
> I have discussed it internally.
> The internal rule for FW's SMbus implementation is stable after 2018
> If you meet some special case, please let me know.
Thanks for the info. I’ll use this for the V2 patch.
>
> BTW, The SMbus supporting is requested by HP this time, and there are
> plenty of HP laptop use old IC which doesn't meet "
> ETP_NEW_IC_SMBUS_HOST_NOTIFY”.
One more question, does ETP_BUS_SMB_HST_NTFY_ONLY means it should stick to SMBus, because it doesn’t support PS/2?
I’d like to merge all checks into elantech_use_host_notify() but ETP_BUS_SMB_HST_NTFY_ONLY caught my attention.
ETP_BUS_SMB_HST_NTFY_ONLY is for our previous planning but it never happen so far, and it won't happen in the future.
There are two cases for our released touchpad and they are " ETP_BUS_PS2_ONLY" and " ETP_BUS_PS2_SMB_HST_NTFY".
Thanks
KT
Kai-Heng
>
> Elan touchpad works well in PS/2 for HP, because it don't support
> TrackPoint.
> You may let old HP platform work as PS/2 for safety.
>
> Thanks
> KT
>
> Kai-Heng
>
>>
>> Cheers,
>> Benjamin
>>
>>> psmouse_matches_pnp_id(psmouse,
> i2c_blacklist_pnp_ids))
>>> return -ENXIO;
>>> }
>>> @@ -1838,34 +1866,6 @@ static int elantech_setup_smbus(struct
>>> psmouse
> *psmouse,
>>> return 0;
>>> }
>>>
>>> -static bool elantech_use_host_notify(struct psmouse *psmouse,
>>> - struct elantech_device_info *info)
>>> -{
>>> - if (ETP_NEW_IC_SMBUS_HOST_NOTIFY(info->fw_version))
>>> - return true;
>>> -
>>> - switch (info->bus) {
>>> - case ETP_BUS_PS2_ONLY:
>>> - /* expected case */
>>> - break;
>>> - case ETP_BUS_SMB_ALERT_ONLY:
>>> - /* fall-through */
>>> - case ETP_BUS_PS2_SMB_ALERT:
>>> - psmouse_dbg(psmouse, "Ignoring SMBus provider through
> alert protocol.\n");
>>> - break;
>>> - case ETP_BUS_SMB_HST_NTFY_ONLY:
>>> - /* fall-through */
>>> - case ETP_BUS_PS2_SMB_HST_NTFY:
>>> - return true;
>>> - default:
>>> - psmouse_dbg(psmouse,
>>> - "Ignoring SMBus bus provider %d.\n",
>>> - info->bus);
>>> - }
>>> -
>>> - return false;
>>> -}
>>> -
>>> int elantech_init_smbus(struct psmouse *psmouse) {
>>> struct elantech_device_info info;
>>> --
>>> 2.17.1
>
^ permalink raw reply
* Re: [PATCH 1/1] Input: elantech: Use SMBus based on bus info
From: Kai-Heng Feng @ 2019-01-21 4:28 UTC (permalink / raw)
To: 廖崇榮
Cc: Benjamin Tissoires, Dmitry Torokhov, open list:HID CORE LAYER,
lkml
In-Reply-To: <008c01d4af10$5b157f20$11407d60$@emc.com.tw>
> On Jan 18, 2019, at 17:29, 廖崇榮 <kt.liao@emc.com.tw> wrote:
>
>
>
> -----Original Message-----
> From: Kai Heng Feng [mailto:kai.heng.feng@canonical.com]
> Sent: Friday, January 18, 2019 12:15 AM
> To: Benjamin Tissoires
> Cc: Dmitry Torokhov; 廖崇榮; open list:HID CORE LAYER; lkml
> Subject: Re: [PATCH 1/1] Input: elantech: Use SMBus based on bus info
>
>
>
>> On Jan 17, 2019, at 10:42 PM, Benjamin Tissoires
> <benjamin.tissoires@redhat.com> wrote:
>>
>> Hi Kai-Heng,
>>
>> On Thu, Jan 17, 2019 at 10:30 AM Kai-Heng Feng
>> <kai.heng.feng@canonical.com> wrote:
>>>
>>> There are some new HP laptops with Elantech touchpad don't support
>>> multitouch.
>>>
>>> Both ETP_BUS_SMB_HST_NTFY_ONLY and ETP_BUS_PS2_SMB_HST_NTFY devices
>>> can use SMBus to support 5 fingers touch, so we need to chech them too.
>>>
>>> So use elantech_use_host_notify() to do a more thouroughly check.
>>>
>>> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
>>> ---
>>> drivers/input/mouse/elantech.c | 58
>>> +++++++++++++++++-----------------
>>> 1 file changed, 29 insertions(+), 29 deletions(-)
>>>
>>> diff --git a/drivers/input/mouse/elantech.c
>>> b/drivers/input/mouse/elantech.c index 9fe075c137dc..5bcf1c147eb1
>>> 100644
>>> --- a/drivers/input/mouse/elantech.c
>>> +++ b/drivers/input/mouse/elantech.c
>>> @@ -1799,6 +1799,34 @@ static int elantech_create_smbus(struct psmouse
> *psmouse,
>>> leave_breadcrumbs); }
>>>
>>> +static bool elantech_use_host_notify(struct psmouse *psmouse,
>>> + struct elantech_device_info
>>> +*info) {
>>> + if (ETP_NEW_IC_SMBUS_HOST_NOTIFY(info->fw_version))
>>> + return true;
>>> +
>>> + switch (info->bus) {
>>> + case ETP_BUS_PS2_ONLY:
>>> + /* expected case */
>>> + break;
>>> + case ETP_BUS_SMB_ALERT_ONLY:
>>> + /* fall-through */
>>> + case ETP_BUS_PS2_SMB_ALERT:
>>> + psmouse_dbg(psmouse, "Ignoring SMBus provider through
> alert protocol.\n");
>>> + break;
>>> + case ETP_BUS_SMB_HST_NTFY_ONLY:
>>> + /* fall-through */
>>> + case ETP_BUS_PS2_SMB_HST_NTFY:
>>> + return true;
>>> + default:
>>> + psmouse_dbg(psmouse,
>>> + "Ignoring SMBus bus provider %d.\n",
>>> + info->bus);
>>> + }
>>> +
>>> + return false;
>>> +}
>>> +
>>> /**
>>> * elantech_setup_smbus - called once the PS/2 devices are enumerated
>>> * and decides to instantiate a SMBus InterTouch device.
>>> @@ -1818,7 +1846,7 @@ static int elantech_setup_smbus(struct psmouse
> *psmouse,
>>> * i2c_blacklist_pnp_ids.
>>> * Old ICs are up to the user to decide.
>>> */
>>> - if (!ETP_NEW_IC_SMBUS_HOST_NOTIFY(info->fw_version) ||
>>> + if (!elantech_use_host_notify(psmouse, info) ||
>>
>> That was my initial approach of the series, but I ended up being more
>> conservative as this would flip all of the existing elantech SMBUS
>> capable touchpads to use elan_i2c.
>> And I didn't want to deal with 4/5 year old laptops that suddenly broke.
>>
>> So I wonder if you can restrict this default change to the recent
>> laptops (let's say 2018+). Maybe by looking at their FW version or
>> something else in the DMI?
>
> It was KT who told me that I should use ETP_BUS_PS2_SMB_HST_NTFY.
>
> As for date, KT still knows better than me.
>
> KT,
> Can you name a year which is safe enough to enable SMBus?
>
> I have discussed it internally.
> The internal rule for FW's SMbus implementation is stable after 2018
> If you meet some special case, please let me know.
Thanks for the info. I’ll use this for the V2 patch.
>
> BTW, The SMbus supporting is requested by HP this time, and there are plenty
> of HP laptop use old IC
> which doesn't meet " ETP_NEW_IC_SMBUS_HOST_NOTIFY”.
One more question, does ETP_BUS_SMB_HST_NTFY_ONLY means
it should stick to SMBus, because it doesn’t support PS/2?
I’d like to merge all checks into elantech_use_host_notify() but
ETP_BUS_SMB_HST_NTFY_ONLY caught my attention.
Kai-Heng
>
> Elan touchpad works well in PS/2 for HP, because it don't support
> TrackPoint.
> You may let old HP platform work as PS/2 for safety.
>
> Thanks
> KT
>
> Kai-Heng
>
>>
>> Cheers,
>> Benjamin
>>
>>> psmouse_matches_pnp_id(psmouse,
> i2c_blacklist_pnp_ids))
>>> return -ENXIO;
>>> }
>>> @@ -1838,34 +1866,6 @@ static int elantech_setup_smbus(struct psmouse
> *psmouse,
>>> return 0;
>>> }
>>>
>>> -static bool elantech_use_host_notify(struct psmouse *psmouse,
>>> - struct elantech_device_info *info)
>>> -{
>>> - if (ETP_NEW_IC_SMBUS_HOST_NOTIFY(info->fw_version))
>>> - return true;
>>> -
>>> - switch (info->bus) {
>>> - case ETP_BUS_PS2_ONLY:
>>> - /* expected case */
>>> - break;
>>> - case ETP_BUS_SMB_ALERT_ONLY:
>>> - /* fall-through */
>>> - case ETP_BUS_PS2_SMB_ALERT:
>>> - psmouse_dbg(psmouse, "Ignoring SMBus provider through
> alert protocol.\n");
>>> - break;
>>> - case ETP_BUS_SMB_HST_NTFY_ONLY:
>>> - /* fall-through */
>>> - case ETP_BUS_PS2_SMB_HST_NTFY:
>>> - return true;
>>> - default:
>>> - psmouse_dbg(psmouse,
>>> - "Ignoring SMBus bus provider %d.\n",
>>> - info->bus);
>>> - }
>>> -
>>> - return false;
>>> -}
>>> -
>>> int elantech_init_smbus(struct psmouse *psmouse) {
>>> struct elantech_device_info info;
>>> --
>>> 2.17.1
>
^ permalink raw reply
* Re: [PATCH v3] HID: i2c-hid: Ignore input report if there's no data present on Elan touchpanels
From: Kai-Heng Feng @ 2019-01-21 3:29 UTC (permalink / raw)
To: Benjamin Tissoires; +Cc: Jiri Kosina, open list:HID CORE LAYER, lkml
In-Reply-To: <CAO-hwJKo=EaPoXJLGUvjq3QRng1V1NkO7A5zYGojbPyxxyJYHA@mail.gmail.com>
> On Jan 18, 2019, at 23:50, Benjamin Tissoires <benjamin.tissoires@redhat.com> wrote:
>
> Hi Kai-Heng,
>
> On Mon, Jan 7, 2019 at 8:24 AM Kai-Heng Feng
> <kai.heng.feng@canonical.com> wrote:
>>
>> While using Elan touchpads, the message floods:
>> [ 136.138487] i2c_hid i2c-DELL08D6:00: i2c_hid_get_input: incomplete report (14/65535)
>>
>> Though the message flood is annoying, the device it self works without
>> any issue. I suspect that the device in question takes too much time to
>> pull the IRQ back to high after I2C host has done reading its data.
>>
>> Since the host receives all useful data, let's ignore the input report
>> when there's no data.
>>
>> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
>
> Thanks for the v3.
>
> This patch has just been brought to my attention, and I have one
> question before applying it:
> are those messages (without your patch) occurring all the time, or
> just after resume?
All the time.
The touchpad works fine though. The entire report is 0xff, so it can be
safely ignored.
>
> I wonder if the pm_suspend delay we talked about in the other thread
> would fix that issue in a cleaner way.
I’ve replied in another thread, unfortunately it can’t.
We can introduce msleep() between each commands though, but I don’t
think it’s good either.
Kai-Heng
>
> Cheers,
> Benjamin
>
>> ---
>> v3:
>> Fix compiler error/warnings.
>>
>> v2:
>> Use dev_warn_once() to warn the user about the situation.
>>
>> drivers/hid/i2c-hid/i2c-hid-core.c | 9 +++++++++
>> 1 file changed, 9 insertions(+)
>>
>> diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
>> index 8555ce7e737b..2f940c1de616 100644
>> --- a/drivers/hid/i2c-hid/i2c-hid-core.c
>> +++ b/drivers/hid/i2c-hid/i2c-hid-core.c
>> @@ -50,6 +50,7 @@
>> #define I2C_HID_QUIRK_NO_IRQ_AFTER_RESET BIT(1)
>> #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)
>>
>> /* flags */
>> #define I2C_HID_STARTED 0
>> @@ -179,6 +180,8 @@ static const struct i2c_hid_quirks {
>> I2C_HID_QUIRK_DELAY_AFTER_SLEEP },
>> { USB_VENDOR_ID_LG, I2C_DEVICE_ID_LG_8001,
>> I2C_HID_QUIRK_NO_RUNTIME_PM },
>> + { USB_VENDOR_ID_ELAN, HID_ANY_ID,
>> + I2C_HID_QUIRK_BOGUS_IRQ },
>> { 0, 0 }
>> };
>>
>> @@ -503,6 +506,12 @@ static void i2c_hid_get_input(struct i2c_hid *ihid)
>> return;
>> }
>>
>> + if (ihid->quirks & I2C_HID_QUIRK_BOGUS_IRQ && ret_size == 0xffff) {
>> + dev_warn_once(&ihid->client->dev, "%s: IRQ triggered but "
>> + "there's no data\n", __func__);
>> + return;
>> + }
>> +
>> if ((ret_size > size) || (ret_size < 2)) {
>> dev_err(&ihid->client->dev, "%s: incomplete report (%d/%d)\n",
>> __func__, size, ret_size);
>> --
>> 2.17.1
>>
^ permalink raw reply
* Re: [PATCH] HID: i2c-hid: Disable runtime PM on Goodix touchpad
From: Kai-Heng Feng @ 2019-01-21 3:23 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Hans de Goede, anisse, Jiri Kosina, open list:HID CORE LAYER,
lkml
In-Reply-To: <CAO-hwJ+OQAsJkJj2YRGOE=8NcXhS=XWwdfGSaLjY9Bdvcfn7BQ@mail.gmail.com>
> On Jan 17, 2019, at 20:35, Benjamin Tissoires <benjamin.tissoires@redhat.com> wrote:
>
> On Thu, Jan 17, 2019 at 12:41 PM Kai-Heng Feng
> <kai.heng.feng@canonical.com> wrote:
>>
>>
>>
>>> On Jan 17, 2019, at 16:06, Benjamin Tissoires <benjamin.tissoires@redhat.com> wrote:
>>>
>>> On Thu, Jan 17, 2019 at 6:02 AM Kai-Heng Feng
>>> <kai.heng.feng@canonical.com> wrote:
>> [snipped]
>>>> Goodix says the firmware needs at least 60ms to fully respond ON and
>>>> SLEEP command.
>>>
>>> I was about to say that this is not conformant to the specification.
>>> But looking at 7.2.8 SET_POWER of
>>> https://docs.microsoft.com/en-us/previous-versions/windows/hardware/design/dn642101(v=vs.85)
>>>
>>> They say:
>>> "The DEVICE must ensure that it transitions to the HOST specified
>>> Power State in under 1 second. "
>>> But in the RESPONSE paragraph: "The DEVICE shall not respond back
>>> after receiving the command. The DEVICE is mandated to enter that
>>> power state imminently."
>>>
>>> My interpretation was always that the device has to be in a ready
>>> state for new commands "immediately".
>>> However, the first sentence may suggest that the driver would block
>>> any command to the device before the 1 second timestamp.
>>>
>>>>
>>>> I’ll see if an 200ms autosuspend can solve all Goodix, LG and Raydium
>>>> touchpanels.
>>>
>>> We already have a I2C_HID_QUIRK_DELAY_AFTER_SLEEP quirk that delay
>>> operations after sleep by 20ms. Maybe we can keep the runtime PM but
>>> use the same timers to not confuse the hardware.
>>
>> Yes, but wouldn’t use pm_*_autosuspend() helpers can both solve the issue
>> and make the code more cleaner?
>
> You are probably correct :)
>
> I am not too familiar with the details of the runtime PM API, but if
> you can make this a barrier to prevent the host to call too many
> suspends/resumes in a row, that would be nice.
> And we might be able to ditch I2C_HID_QUIRK_DELAY_AFTER_SLEEP and
> I2C_HID_QUIRK_NO_RUNTIME_PM.
Ok, using autosuspend helpers doesn’t really solve the issue.
Although it can cover the case like fast ON/SLEEP triggered by
quick transition of display manager -> desktop session, but once
it gets runtime suspended, we can never sure when it’ll get
runtime resumed again. So a mleep() between each powering
commands is still needed.
So I think we should stick to I2C_HID_QUIRK_NO_RUNTIME_PM
for now.
Kai-Heng
>
> Adding the involved people in the thread.
>
> Cheers,
> Benjamin
^ permalink raw reply
* [PATCH] input: Fix the CONFIG_SPARC64 mixup
From: Deepa Dinamani @ 2019-01-21 1:49 UTC (permalink / raw)
To: dmitry.torokhov; +Cc: y2038, davem, linux-kernel, arnd, linux-input
Arnd Bergmann pointed out that CONFIG_* cannot be used
in a uapi header. Override with an equivalent conditional.
Fixes: 2e746942ebac ("Input: input_event - provide override for sparc64")
Fixes: 152194fe9c3f ("Input: extend usable life of event timestamps to 2106 on 32 bit systems")
Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
---
include/uapi/linux/input.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h
index ffab958bc512..f056b2a00d5c 100644
--- a/include/uapi/linux/input.h
+++ b/include/uapi/linux/input.h
@@ -32,7 +32,7 @@ struct input_event {
#define input_event_usec time.tv_usec
#else
__kernel_ulong_t __sec;
-#ifdef CONFIG_SPARC64
+#if defined(__sparc__) && defined(__arch64__)
unsigned int __usec;
#else
__kernel_ulong_t __usec;
--
2.17.1
_______________________________________________
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038
^ permalink raw reply related
* Re: [PATCH 11/13] leds: max77650: add LEDs support
From: Jacek Anaszewski @ 2019-01-20 16:39 UTC (permalink / raw)
To: Bartosz Golaszewski, Rob Herring, Mark Rutland, Linus Walleij,
Dmitry Torokhov, Pavel Machek, Lee Jones, Sebastian Reichel,
Liam Girdwood, Mark Brown, Greg Kroah-Hartman
Cc: linux-kernel, linux-gpio, devicetree, linux-input, linux-leds,
linux-pm, Bartosz Golaszewski
In-Reply-To: <20190118134244.22253-12-brgl@bgdev.pl>
Hi Bartosz,
Thank you for the patch.
I have few minor issues below.
On 1/18/19 2:42 PM, 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>
> ---
> drivers/leds/Kconfig | 6 ++
> drivers/leds/Makefile | 1 +
> drivers/leds/leds-max77650.c | 162 +++++++++++++++++++++++++++++++++++
> 3 files changed, 169 insertions(+)
> create mode 100644 drivers/leds/leds-max77650.c
>
> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> index a72f97fca57b..6e7a8f51eccc 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -608,6 +608,12 @@ config LEDS_TLC591XX
> This option enables support for Texas Instruments TLC59108
> and TLC59116 LED controllers.
>
> +config LEDS_MAX77650
> + tristate "LED support for Maxim MAX77650 PMIC"
> + depends on MFD_MAX77650
> + help
> + LEDs driver for MAX77650 family of PMICs from Maxim Integrated."
> +
> config LEDS_MAX77693
> tristate "LED support for MAX77693 Flash"
> depends on LEDS_CLASS_FLASH
> diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
> index 4c1b0054f379..f48b2404dbb7 100644
> --- a/drivers/leds/Makefile
> +++ b/drivers/leds/Makefile
> @@ -61,6 +61,7 @@ obj-$(CONFIG_LEDS_MC13783) += leds-mc13783.o
> obj-$(CONFIG_LEDS_NS2) += leds-ns2.o
> obj-$(CONFIG_LEDS_NETXBIG) += leds-netxbig.o
> obj-$(CONFIG_LEDS_ASIC3) += leds-asic3.o
> +obj-$(CONFIG_LEDS_MAX77650) += leds-max77650.o
> obj-$(CONFIG_LEDS_MAX77693) += leds-max77693.o
> obj-$(CONFIG_LEDS_MAX8997) += leds-max8997.o
> obj-$(CONFIG_LEDS_LM355x) += leds-lm355x.o
> diff --git a/drivers/leds/leds-max77650.c b/drivers/leds/leds-max77650.c
> new file mode 100644
> index 000000000000..be2bb1c60448
> --- /dev/null
> +++ b/drivers/leds/leds-max77650.c
> @@ -0,0 +1,162 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2018 BayLibre SAS
> + * Author: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> + *
> + * LEDS driver for MAXIM 77650/77651 charger/power-supply.
> + */
Please use uniform C++ comment style.
If in doubt, please refer to the following Linus statement
from [0]:
"And yes, feel free to replace block comments with // while at it."
> +#include <linux/i2c.h>
> +#include <linux/leds.h>
> +#include <linux/mfd/max77650.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +
> +#define MAX77650_NUM_LEDS 3
> +
> +#define MAX77650_LED_A_BASE 0x40
> +#define MAX77650_LED_B_BASE 0x43
> +
> +#define MAX77650_LED_BR_MASK GENMASK(4, 0)
> +#define MAX77650_LED_EN_MASK GENMASK(7, 6)
> +
> +/* Enable EN_LED_MSTR. */
> +#define MAX77650_LED_TOP_DEFAULT BIT(0)
> +
> +#define MAX77650_LED_ENABLE GENMASK(7, 6)
> +#define MAX77650_LED_DISABLE 0x00
> +
> +#define MAX77650_LED_A_DEFAULT MAX77650_LED_DISABLE
> +/* 100% on duty */
> +#define MAX77650_LED_B_DEFAULT GENMASK(3, 0)
> +
> +struct max77650_leds;
> +
> +struct max77650_led {
> + struct led_classdev cdev;
> + struct max77650_leds *parent;
> + unsigned int regA;
> + unsigned int regB;
> +};
> +
> +struct max77650_leds {
> + struct regmap *map;
> + struct max77650_led leds[MAX77650_NUM_LEDS];
> +};
> +
> +static struct max77650_led *max77650_to_led(struct led_classdev *cdev)
> +{
> + return container_of(cdev, struct max77650_led, cdev);
> +}
> +
> +static int max77650_leds_brightness_set(struct led_classdev *cdev,
> + enum led_brightness brightness)
I would change function names to max77650_led* (singular).
It would be consistent with most of the other LED class drivers.
LED driver for max77693 flash cell also uses this scheme
(drivers/leds/leds-max77693.c).
> +{
> + struct max77650_led *led = max77650_to_led(cdev);
> + struct regmap *regmap = led->parent->map;
> + int val, mask;
> +
> + mask = MAX77650_LED_BR_MASK | MAX77650_LED_EN_MASK;
> +
> + if (brightness == LED_OFF) {
> + val = MAX77650_LED_DISABLE;
> + } else {
> + val = MAX77650_LED_ENABLE;
> + /*
> + * We can set the brightness with 5-bit resolution.
> + *
> + * For brightness == 1, the bits we're writing will be 0, but
> + * since we keep LED_FS0 set to 12.8mA full-scale range, the
> + * LED will be lit slightly.
> + */
> + val |= brightness / 8;
> + }
> +
> + return regmap_update_bits(regmap, led->regA, mask, val);
> +}
> +
> +static int max77650_leds_probe(struct platform_device *pdev)
> +{
> + struct device_node *of_node, *child;
> + struct max77650_leds *leds;
> + struct max77650_led *led;
> + struct device *parent;
> + struct device *dev;
> + int rv, num_leds;
> + u32 reg;
> +
> + dev = &pdev->dev;
> + parent = dev->parent;
> + of_node = dev->of_node;
> +
> + if (!of_node)
> + return -ENODEV;
> +
> + leds = devm_kzalloc(dev, sizeof(*leds), GFP_KERNEL);
> + if (!leds)
> + return -ENOMEM;
> +
> + leds->map = dev_get_regmap(dev->parent, NULL);
> + if (!leds->map)
> + return -ENODEV;
> +
> + num_leds = of_get_child_count(of_node);
> + if (!num_leds || num_leds > MAX77650_NUM_LEDS)
> + return -ENODEV;
> +
> + for_each_child_of_node(of_node, child) {
> + rv = of_property_read_u32(child, "reg", ®);
> + if (rv || reg >= MAX77650_NUM_LEDS)
> + return -EINVAL;
> +
> + led = &leds->leds[reg];
> +
> + led->parent = leds;
> + led->regA = MAX77650_LED_A_BASE + reg;
> + led->regB = MAX77650_LED_B_BASE + reg;
> + led->cdev.brightness_set_blocking
> + = max77650_leds_brightness_set;
> +
> + led->cdev.name = of_get_property(child, "label", NULL);
> + if (!led->cdev.name)
> + led->cdev.name = child->name;
Please follow how other recent LED class drivers construct LED names,
e.g. drivers/leds/leds-cr0014114.c. We are in the course of creating
generic support for creating LED names, but until it is agreed upon
we've got to use other drivers as a reference.
> +
> + of_property_read_string(child, "linux,default-trigger",
> + &led->cdev.default_trigger);
> +
> + rv = devm_of_led_classdev_register(dev, child, &led->cdev);
> + if (rv)
> + return rv;
> +
> + rv = regmap_write(leds->map, led->regA,
> + MAX77650_LED_A_DEFAULT);
> + if (rv)
> + return rv;
> +
> + rv = regmap_write(leds->map, led->regB,
> + MAX77650_LED_B_DEFAULT);
> + if (rv)
> + return rv;
> + }
> +
> + rv = regmap_write(leds->map,
> + MAX77650_REG_CNFG_LED_TOP,
> + MAX77650_LED_TOP_DEFAULT);
> + if (rv)
> + return rv;
No need to check rv here:
return regmap_write(leds->map,
MAX77650_REG_CNFG_LED_TOP,
MAX77650_LED_TOP_DEFAULT)
> + return 0;
> +}
> +
> +static struct platform_driver max77650_leds_driver = {
> + .driver = {
> + .name = "max77650-leds",
s/leds/led/
> + },
> + .probe = max77650_leds_probe,
> +};
> +module_platform_driver(max77650_leds_driver);
> +
> +MODULE_DESCRIPTION("MAXIM 77650/77651 LED driver");
> +MODULE_AUTHOR("Bartosz Golaszewski <bgolaszewski@baylibre.com>");
> +MODULE_LICENSE("GPL");
s/GPL/GPL v2/
[0] https://lkml.org/lkml/2017/11/2/715
--
Best regards,
Jacek Anaszewski
^ permalink raw reply
* Re: [PATCH 05/13] dt-bindings: leds: add DT bindings for max77650
From: Jacek Anaszewski @ 2019-01-20 16:28 UTC (permalink / raw)
To: Bartosz Golaszewski, Rob Herring, Mark Rutland, Linus Walleij,
Dmitry Torokhov, Pavel Machek, Lee Jones, Sebastian Reichel,
Liam Girdwood, Mark Brown, Greg Kroah-Hartman
Cc: linux-kernel, linux-gpio, devicetree, linux-input, linux-leds,
linux-pm, Bartosz Golaszewski
In-Reply-To: <20190118134244.22253-6-brgl@bgdev.pl>
Hi Bartosz,
Thank you for the patch.
On 1/18/19 2:42 PM, 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>
> ---
> .../bindings/leds/leds-max77650.txt | 57 +++++++++++++++++++
> 1 file changed, 57 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/leds/leds-max77650.txt
>
> diff --git a/Documentation/devicetree/bindings/leds/leds-max77650.txt b/Documentation/devicetree/bindings/leds/leds-max77650.txt
> new file mode 100644
> index 000000000000..822b8893bc20
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/leds-max77650.txt
> @@ -0,0 +1,57 @@
> +LED 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 LED controller is represented as a sub-node of the PMIC node on
> +the device tree.
> +
> +This device has three current sinks.
> +
> +Required properties:
> +--------------------
> +- compatible: Must be "maxim,max77650-leds"
> +- #address-cells: Must be <1>.
> +- #size-cells: Must be <0>.
> +
> +Each LED is represented as a sub-node of the LED-controller node. Up to
> +three sub-nodes can be defined.
> +
> +Required properties of the sub-node:
> +------------------------------------
> +
> +- reg: Must be <0>, <1> or <2>.
> +
> +Optional properties of the sub-node:
> +------------------------------------
> +
> +- label: See Documentation/devicetree/bindings/leds/common.txt
> +- linux,default-trigger: See Documentation/devicetree/bindings/leds/common.txt
> +
> +For more details, please refer to the generic GPIO DT binding document
> +<devicetree/bindings/gpio/gpio.txt>.
> +
> +Example:
> +--------
> +
> + leds {
> + compatible = "maxim,max77650-leds";
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + led0 {
s/led0/led@0/
> + reg = <0>;
> + label = "max77650:blue:usr0";
> + };
> +
> + led1 {
s/led1/led@1/
> + reg = <1>;
> + label = "max77650:red:usr1";
> + linux,default-trigger = "heartbeat";
> + };
> +
> + led2 {
s/led2/led@2/
> + reg = <2>;
> + label = "max77650:green:usr2";
> + };
Please remove "max77650:" from labels and add it in the driver.
> + };
>
--
Best regards,
Jacek Anaszewski
^ permalink raw reply
* Re: [PATCH 12/13] input: max77650: add onkey support
From: Dmitry Torokhov @ 2019-01-19 9:03 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Rob Herring, Mark Rutland, Linus Walleij, Jacek Anaszewski,
Pavel Machek, Lee Jones, Sebastian Reichel, Liam Girdwood,
Mark Brown, Greg Kroah-Hartman, linux-kernel, linux-gpio,
devicetree, linux-input, linux-leds, linux-pm,
Bartosz Golaszewski
In-Reply-To: <20190118134244.22253-13-brgl@bgdev.pl>
Hi Bartosz,
On Fri, Jan 18, 2019 at 02:42:43PM +0100, 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>
> ---
> drivers/input/misc/Kconfig | 9 ++
> drivers/input/misc/Makefile | 1 +
> drivers/input/misc/max77650-onkey.c | 135 ++++++++++++++++++++++++++++
> 3 files changed, 145 insertions(+)
> create mode 100644 drivers/input/misc/max77650-onkey.c
>
> diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
> index ca59a2be9bc5..bb9c45c1269e 100644
> --- a/drivers/input/misc/Kconfig
> +++ b/drivers/input/misc/Kconfig
> @@ -180,6 +180,15 @@ config INPUT_M68K_BEEP
> tristate "M68k Beeper support"
> depends on M68K
>
> +config INPUT_MAX77650_ONKEY
> + tristate "Maxim MAX77650 ONKEY support"
> + depends on MFD_MAX77650
> + help
> + Support the ONKEY of the MAX77650 PMIC as an input device.
> +
> + To compile this driver as a module, choose M here: the module
> + will be called max77650-onkey.
> +
> config INPUT_MAX77693_HAPTIC
> tristate "MAXIM MAX77693/MAX77843 haptic controller support"
> depends on (MFD_MAX77693 || MFD_MAX77843) && PWM
> diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
> index 9d0f9d1ff68f..5bd53590ce60 100644
> --- a/drivers/input/misc/Makefile
> +++ b/drivers/input/misc/Makefile
> @@ -43,6 +43,7 @@ obj-$(CONFIG_INPUT_IXP4XX_BEEPER) += ixp4xx-beeper.o
> obj-$(CONFIG_INPUT_KEYSPAN_REMOTE) += keyspan_remote.o
> obj-$(CONFIG_INPUT_KXTJ9) += kxtj9.o
> obj-$(CONFIG_INPUT_M68K_BEEP) += m68kspkr.o
> +obj-$(CONFIG_INPUT_MAX77650_ONKEY) += max77650-onkey.o
> obj-$(CONFIG_INPUT_MAX77693_HAPTIC) += max77693-haptic.o
> obj-$(CONFIG_INPUT_MAX8925_ONKEY) += max8925_onkey.o
> obj-$(CONFIG_INPUT_MAX8997_HAPTIC) += max8997_haptic.o
> diff --git a/drivers/input/misc/max77650-onkey.c b/drivers/input/misc/max77650-onkey.c
> new file mode 100644
> index 000000000000..cc7e83f589cd
> --- /dev/null
> +++ b/drivers/input/misc/max77650-onkey.c
> @@ -0,0 +1,135 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2018 BayLibre SAS
> + * Author: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> + *
> + * ONKEY driver for MAXIM 77650/77651 charger/power-supply.
> + */
> +
> +#include <linux/i2c.h>
> +#include <linux/input.h>
> +#include <linux/interrupt.h>
> +#include <linux/mfd/max77650.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +
> +#define MAX77650_ONKEY_MODE_MASK BIT(3)
> +#define MAX77650_ONKEY_MODE_PUSH 0x00
> +#define MAX77650_ONKEY_MODE_SLIDE BIT(3)
> +
> +struct max77650_onkey {
> + struct input_dev *input;
> + unsigned int code;
> +};
> +
> +static irqreturn_t
> +max77650_onkey_report(struct max77650_onkey *onkey, int value)
> +{
> + input_report_key(onkey->input, onkey->code, value);
> + input_sync(onkey->input);
> +
> + return IRQ_HANDLED;
It is weird that report function returns irqreturn_t. I'd simply moved
input_report_key()/input_sync() into real IRQ handlers.
> +}
> +
> +static irqreturn_t max77650_onkey_falling(int irq, void *data)
> +{
> + struct max77650_onkey *onkey = data;
> +
> + return max77650_onkey_report(onkey, 0);
> +}
> +
> +static irqreturn_t max77650_onkey_rising(int irq, void *data)
> +{
> + struct max77650_onkey *onkey = data;
> +
> + return max77650_onkey_report(onkey, 1);
> +}
> +
> +static int max77650_onkey_probe(struct platform_device *pdev)
> +{
> + struct regmap_irq_chip_data *irq_data;
> + struct max77650_onkey *onkey;
> + struct device *dev, *parent;
> + int irq_r, irq_f, rv, mode;
Please call "rv" "error".
> + struct i2c_client *i2c;
> + const char *mode_prop;
> + struct regmap *map;
> +
> + dev = &pdev->dev;
> + parent = dev->parent;
> + i2c = to_i2c_client(parent);
> + irq_data = i2c_get_clientdata(i2c);
> +
> + map = dev_get_regmap(parent, NULL);
> + if (!map)
> + return -ENODEV;
> +
> + onkey = devm_kzalloc(dev, sizeof(*onkey), GFP_KERNEL);
> + if (!onkey)
> + return -ENOMEM;
> +
> + rv = device_property_read_u32(dev, "linux,code", &onkey->code);
> + if (rv)
> + onkey->code = KEY_POWER;
> +
> + rv = device_property_read_string(dev, "maxim,onkey-mode", &mode_prop);
> + if (rv)
> + mode_prop = "push";
> +
> + if (strcmp(mode_prop, "push") == 0)
> + mode = MAX77650_ONKEY_MODE_PUSH;
> + else if (strcmp(mode_prop, "slide") == 0)
> + mode = MAX77650_ONKEY_MODE_SLIDE;
> + else
> + return -EINVAL;
> +
> + rv = regmap_update_bits(map, MAX77650_REG_CNFG_GLBL,
> + MAX77650_ONKEY_MODE_MASK, mode);
> + if (rv)
> + return rv;
> +
> + irq_f = regmap_irq_get_virq(irq_data, MAX77650_INT_nEN_F);
> + if (irq_f <= 0)
> + return -EINVAL;
> +
> + irq_r = regmap_irq_get_virq(irq_data, MAX77650_INT_nEN_R);
> + if (irq_r <= 0)
> + return -EINVAL;
Ugh, it would be better if you handled IRQ mapping in the MFD piece and
passed it as resources of platform device. Then you'd simply call
platform_get_irq() here and did not have to reach into parent device for
"irq_dara".
> +
> + onkey->input = devm_input_allocate_device(dev);
> + if (!onkey->input)
> + return -ENOMEM;
> +
> + onkey->input->name = "max77650_onkey";
> + onkey->input->phys = "max77650_onkey/input0";
> + onkey->input->id.bustype = BUS_I2C;
> + onkey->input->dev.parent = dev;
Not needed since devm_input_allocate_device sets parent for you.
> + input_set_capability(onkey->input, EV_KEY, onkey->code);
> +
> + rv = devm_request_threaded_irq(dev, irq_f, NULL,
Why threaded interrupt with only hard interrupt handler? If parent
interrupt is threaded use "any_context_irq" here.
> + max77650_onkey_falling,
> + IRQF_ONESHOT, "onkey-down", onkey);
Why do you need oneshot with interrupt that is essentially not threaded?
> + if (rv)
> + return rv;
> +
> + rv = devm_request_threaded_irq(dev, irq_r, NULL,
> + max77650_onkey_rising,
> + IRQF_ONESHOT, "onkey-up", onkey);
> + if (rv)
> + return rv;
> +
> + return input_register_device(onkey->input);
> +}
> +
> +static struct platform_driver max77650_onkey_driver = {
> + .driver = {
> + .name = "max77650-onkey",
> + },
> + .probe = max77650_onkey_probe,
> +};
> +module_platform_driver(max77650_onkey_driver);
> +
> +MODULE_DESCRIPTION("MAXIM 77650/77651 ONKEY driver");
> +MODULE_AUTHOR("Bartosz Golaszewski <bgolaszewski@baylibre.com>");
> +MODULE_LICENSE("GPL");
SPDX header say GPL-2.0 so please "GPL v2" here as "GPL" means v2 and
later.
Thanks.
--
Dmitry
^ permalink raw reply
* [PATCH 7/7] HID: input: add mapping for "Toggle Display" key
From: Dmitry Torokhov @ 2019-01-18 23:30 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires; +Cc: linux-input, linux-kernel
In-Reply-To: <20190118233037.87318-1-dmitry.torokhov@gmail.com>
According to HUT 1.12 usage 0xb5 from the generic desktop page is reserved
for switching between external and internal display, so let's add the
mapping.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
This can be applied independently.
drivers/hid/hid-input.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index ecb1b6f26853..da76358cde06 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -677,6 +677,14 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
break;
}
+ if ((usage->hid & 0xf0) == 0xb0) { /* SC - Display */
+ switch (usage->hid & 0xf) {
+ case 0x05: map_key_clear(KEY_SWITCHVIDEOMODE); break;
+ default: goto ignore;
+ }
+ break;
+ }
+
/*
* Some lazy vendors declare 255 usages for System Control,
* leading to the creation of ABS_X|Y axis and too many others.
--
2.20.1.321.g9e740568ce-goog
^ permalink raw reply related
* [PATCH 6/7] HID: input: add mapping for "Full Screen" key
From: Dmitry Torokhov @ 2019-01-18 23:30 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires; +Cc: linux-input, linux-kernel
In-Reply-To: <20190118233037.87318-1-dmitry.torokhov@gmail.com>
According to HUT 1.12 usage 0x232 from the consumer page is reserved for
switching application between full screen and windowed mode, so let's add
the mapping.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
This needs the new key definition.
drivers/hid/hid-input.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index cebe8a8cce2e..ecb1b6f26853 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -1014,6 +1014,7 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
case 0x22d: map_key_clear(KEY_ZOOMIN); break;
case 0x22e: map_key_clear(KEY_ZOOMOUT); break;
case 0x22f: map_key_clear(KEY_ZOOMRESET); break;
+ case 0x232: map_key_clear(KEY_FULL_SCREEN); break;
case 0x233: map_key_clear(KEY_SCROLLUP); break;
case 0x234: map_key_clear(KEY_SCROLLDOWN); break;
case 0x238: map_rel(REL_HWHEEL); break;
--
2.20.1.321.g9e740568ce-goog
^ permalink raw reply related
* [PATCH 5/7] HID: input: add mapping for keyboard Brightness Up/Down/Toggle keys
From: Dmitry Torokhov @ 2019-01-18 23:30 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires; +Cc: linux-input, linux-kernel
In-Reply-To: <20190118233037.87318-1-dmitry.torokhov@gmail.com>
According to HUTRR73 usages 0x79, 0x7a and 0x7c from the consumer page
correspond to Brightness Up/Down/Toggle keys, so let's add the mappings.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
This can be applied independently.
drivers/hid/hid-input.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index 5f800e7b04f2..cebe8a8cce2e 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -900,6 +900,10 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
case 0x074: map_key_clear(KEY_BRIGHTNESS_MAX); break;
case 0x075: map_key_clear(KEY_BRIGHTNESS_AUTO); break;
+ case 0x079: map_key_clear(KEY_KBDILLUMUP); break;
+ case 0x07a: map_key_clear(KEY_KBDILLUMDOWN); break;
+ case 0x07c: map_key_clear(KEY_KBDILLUMTOGGLE); break;
+
case 0x082: map_key_clear(KEY_VIDEO_NEXT); break;
case 0x083: map_key_clear(KEY_LAST); break;
case 0x084: map_key_clear(KEY_ENTER); break;
--
2.20.1.321.g9e740568ce-goog
^ permalink raw reply related
* [PATCH 4/7] HID: input: add mapping for Expose/Overview key
From: Dmitry Torokhov @ 2019-01-18 23:30 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires; +Cc: linux-input, linux-kernel
In-Reply-To: <20190118233037.87318-1-dmitry.torokhov@gmail.com>
According to HUTRR77 usage 0x29f from the consumer page is reserved for
the Desktop application to present all running user’s application windows.
Linux defines KEY_SCALE to request Compiz Scale (Expose) mode, so let's
add the mapping.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
This can be applied independently.
drivers/hid/hid-input.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index def58c6aa835..5f800e7b04f2 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -1030,6 +1030,8 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
case 0x2cb: map_key_clear(KEY_KBDINPUTASSIST_ACCEPT); break;
case 0x2cc: map_key_clear(KEY_KBDINPUTASSIST_CANCEL); break;
+ case 0x29f: map_key_clear(KEY_SCALE); break;
+
default: map_key_clear(KEY_UNKNOWN);
}
break;
--
2.20.1.321.g9e740568ce-goog
^ permalink raw reply related
* [PATCH 3/7] HID: input: fix mapping of aspect ratio key
From: Dmitry Torokhov @ 2019-01-18 23:30 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: linux-input, linux-kernel, Mauro Carvalho Chehab
In-Reply-To: <20190118233037.87318-1-dmitry.torokhov@gmail.com>
According to HUTRR37 usage 0x6d from the consumer usage page corresponds
to action that selects the next available supported aspect ratio option
on a device which outputs or displays video. However KEY_ZOOM means
activate "Full Screen" mode, KEY_ASPECT_RATIO should be used instead.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/hid/hid-input.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index d6fab5798487..def58c6aa835 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -891,7 +891,7 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
case 0x06a: map_key_clear(KEY_GREEN); break;
case 0x06b: map_key_clear(KEY_BLUE); break;
case 0x06c: map_key_clear(KEY_YELLOW); break;
- case 0x06d: map_key_clear(KEY_ZOOM); break;
+ case 0x06d: map_key_clear(KEY_ASPECT_RATIO); break;
case 0x06f: map_key_clear(KEY_BRIGHTNESSUP); break;
case 0x070: map_key_clear(KEY_BRIGHTNESSDOWN); break;
--
2.20.1.321.g9e740568ce-goog
^ permalink raw reply related
* [PATCH 2/7] [media] doc-rst: switch to new names for Full Screen/Aspect keys
From: Dmitry Torokhov @ 2019-01-18 23:30 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Jiri Kosina, Benjamin Tissoires, linux-input, linux-kernel,
linux-media
In-Reply-To: <20190118233037.87318-1-dmitry.torokhov@gmail.com>
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 ++--
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
^ permalink raw reply related
* [PATCH 1/7] Input: document meanings of KEY_SCREEN and KEY_ZOOM
From: Dmitry Torokhov @ 2019-01-18 23:30 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Mauro Carvalho Chehab, linux-input, linux-kernel, linux-media
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>
---
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 */
--
2.20.1.321.g9e740568ce-goog
^ permalink raw reply related
* Re: [PATCH 08/13] regulator: max77650: add regulator support
From: Mark Brown @ 2019-01-18 18:36 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Bartosz Golaszewski, Rob Herring, Mark Rutland, Linus Walleij,
Dmitry Torokhov, Jacek Anaszewski, Pavel Machek, Lee Jones,
Sebastian Reichel, Liam Girdwood, Greg Kroah-Hartman, LKML,
linux-gpio, linux-devicetree, linux-input, linux-leds, linux-pm
In-Reply-To: <CAMpxmJX8OmnDtOhQjWi9edX-4pHu1daKiDkRdNAVw-PZsz9UXA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1202 bytes --]
On Fri, Jan 18, 2019 at 07:13:38PM +0100, Bartosz Golaszewski wrote:
> pt., 18 sty 2019 o 19:01 Mark Brown <broonie@kernel.org> napisał(a):
> > > +++ b/drivers/regulator/max77650-regulator.c
> > > @@ -0,0 +1,537 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Copyright (C) 2018 BayLibre SAS
> > > + * Author: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > Please make the entire header C++ style so it looks more intentional.
> Seems like there are more files in the kernel source using the mixed
> comment style for the SPDX identifier and I also prefer it over C++
> only. Would you mind if it stayed that way?
I'd prefer to change.
> > You don't need to do this, the core will do it for you (it will actually
> > still do it even with the above, it'll only fall back to using
> > config->init_data if it's own lookup fails).
> I added this loop specifically because the core would not pick up the
> init data from DT. What did I miss (some specific variable to assign)?
> I just noticed some other drivers do the same and thought it's the
> right thing to do.
You should set regulators_node and of_match, see regulator_of_get_init_node().
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH 08/13] regulator: max77650: add regulator support
From: Bartosz Golaszewski @ 2019-01-18 18:18 UTC (permalink / raw)
To: Mark Brown
Cc: Bartosz Golaszewski, Rob Herring, Mark Rutland, Linus Walleij,
Dmitry Torokhov, Jacek Anaszewski, Pavel Machek, Lee Jones,
Sebastian Reichel, Liam Girdwood, Greg Kroah-Hartman, LKML,
linux-gpio, linux-devicetree, linux-input, linux-leds, linux-pm
In-Reply-To: <20190118180148.GD6260@sirena.org.uk>
pt., 18 sty 2019 o 19:01 Mark Brown <broonie@kernel.org> napisał(a):
>
> On Fri, Jan 18, 2019 at 02:42:39PM +0100, Bartosz Golaszewski wrote:
>
> > Add regulator support for max77650. We support all four variants of this
> > PMIC including non-linear voltage table for max77651 SBB1 rail.
>
Sorry for spamming, I missed it the last time.
> Looks good, the ramping stuff might be a candidate for core (TBH I was
> sure we'd got that implemented already but we don't seem to) but that
> can be done later and the more complex one with non-linear steps does
> feel like it might have to stay in the driver anyway.
>
I'm thinking about implementing that in the core for the next release
cycle and then making this driver the first user.
Bart
> A couple of small nits:
>
> > +++ b/drivers/regulator/max77650-regulator.c
> > @@ -0,0 +1,537 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2018 BayLibre SAS
> > + * Author: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>
> Please make the entire header C++ style so it looks more intentional.
>
> > + for_each_child_of_node(dev->of_node, child) {
> > + if (!of_node_name_eq(child, rdesc->desc.name))
> > + continue;
> > +
> > + init_data = of_get_regulator_init_data(dev, child,
> > + &rdesc->desc);
> > + if (!init_data)
> > + return -ENOMEM;
> > +
> > + config.of_node = child;
> > + config.init_data = init_data;
> > + }
>
> You don't need to do this, the core will do it for you (it will actually
> still do it even with the above, it'll only fall back to using
> config->init_data if it's own lookup fails).
^ permalink raw reply
* Re: [PATCH 08/13] regulator: max77650: add regulator support
From: Bartosz Golaszewski @ 2019-01-18 18:13 UTC (permalink / raw)
To: Mark Brown
Cc: Bartosz Golaszewski, Rob Herring, Mark Rutland, Linus Walleij,
Dmitry Torokhov, Jacek Anaszewski, Pavel Machek, Lee Jones,
Sebastian Reichel, Liam Girdwood, Greg Kroah-Hartman, LKML,
linux-gpio, linux-devicetree, linux-input, linux-leds, linux-pm
In-Reply-To: <20190118180148.GD6260@sirena.org.uk>
pt., 18 sty 2019 o 19:01 Mark Brown <broonie@kernel.org> napisał(a):
>
> On Fri, Jan 18, 2019 at 02:42:39PM +0100, Bartosz Golaszewski wrote:
>
> > Add regulator support for max77650. We support all four variants of this
> > PMIC including non-linear voltage table for max77651 SBB1 rail.
>
> Looks good, the ramping stuff might be a candidate for core (TBH I was
> sure we'd got that implemented already but we don't seem to) but that
> can be done later and the more complex one with non-linear steps does
> feel like it might have to stay in the driver anyway.
>
> A couple of small nits:
>
> > +++ b/drivers/regulator/max77650-regulator.c
> > @@ -0,0 +1,537 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2018 BayLibre SAS
> > + * Author: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>
> Please make the entire header C++ style so it looks more intentional.
>
Seems like there are more files in the kernel source using the mixed
comment style for the SPDX identifier and I also prefer it over C++
only. Would you mind if it stayed that way?
> > + for_each_child_of_node(dev->of_node, child) {
> > + if (!of_node_name_eq(child, rdesc->desc.name))
> > + continue;
> > +
> > + init_data = of_get_regulator_init_data(dev, child,
> > + &rdesc->desc);
> > + if (!init_data)
> > + return -ENOMEM;
> > +
> > + config.of_node = child;
> > + config.init_data = init_data;
> > + }
>
> You don't need to do this, the core will do it for you (it will actually
> still do it even with the above, it'll only fall back to using
> config->init_data if it's own lookup fails).
I added this loop specifically because the core would not pick up the
init data from DT. What did I miss (some specific variable to assign)?
I just noticed some other drivers do the same and thought it's the
right thing to do.
Thanks,
Bart
^ permalink raw reply
* Re: [PATCH 08/13] regulator: max77650: add regulator support
From: Mark Brown @ 2019-01-18 18:01 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Rob Herring, Mark Rutland, Linus Walleij, Dmitry Torokhov,
Jacek Anaszewski, Pavel Machek, 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: <20190118134244.22253-9-brgl@bgdev.pl>
[-- Attachment #1: Type: text/plain, Size: 1305 bytes --]
On Fri, Jan 18, 2019 at 02:42:39PM +0100, Bartosz Golaszewski wrote:
> Add regulator support for max77650. We support all four variants of this
> PMIC including non-linear voltage table for max77651 SBB1 rail.
Looks good, the ramping stuff might be a candidate for core (TBH I was
sure we'd got that implemented already but we don't seem to) but that
can be done later and the more complex one with non-linear steps does
feel like it might have to stay in the driver anyway.
A couple of small nits:
> +++ b/drivers/regulator/max77650-regulator.c
> @@ -0,0 +1,537 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2018 BayLibre SAS
> + * Author: Bartosz Golaszewski <bgolaszewski@baylibre.com>
Please make the entire header C++ style so it looks more intentional.
> + for_each_child_of_node(dev->of_node, child) {
> + if (!of_node_name_eq(child, rdesc->desc.name))
> + continue;
> +
> + init_data = of_get_regulator_init_data(dev, child,
> + &rdesc->desc);
> + if (!init_data)
> + return -ENOMEM;
> +
> + config.of_node = child;
> + config.init_data = init_data;
> + }
You don't need to do this, the core will do it for you (it will actually
still do it even with the above, it'll only fall back to using
config->init_data if it's own lookup fails).
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH v3 1/4] dt-bindings: input: touchscreen: goodix: Document AVDD28-supply property
From: Jagan Teki @ 2019-01-18 16:01 UTC (permalink / raw)
To: Rob Herring, devicetree
Cc: Dmitry Torokhov, Chen-Yu Tsai, linux-input,
linux-kernel@vger.kernel.org, Michael Trimarchi, linux-amarula
In-Reply-To: <CAL_JsqKTnPVM7Qahy9ejQbPfENpLCdzf5K5uUTcPh8j3EiqWRA@mail.gmail.com>
On Wed, Jan 9, 2019 at 7:08 PM Rob Herring <robh@kernel.org> wrote:
>
> Please CC DT list if you want bindings reviewed.
Sorry I forgot.
>
> On Wed, Jan 9, 2019 at 1:40 AM Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> >
> > On Sat, Dec 15, 2018 at 08:47:59PM +0530, Jagan Teki wrote:
> > > Most of the Goodix CTP controllers are supply with AVDD28 pin.
> > > which need to supply for controllers like GT5663 on some boards
> > > to trigger the power.
> > >
> > > So, document the supply property so-that the require boards
> > > that used on GT5663 can enable it via device tree.
> > >
> > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > > ---
> > > 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 f7e95c52f3c7..c4622c983e08 100644
> > > --- a/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
> > > +++ b/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
> > > @@ -23,6 +23,7 @@ Optional properties:
> > > - touchscreen-inverted-y : Y axis is inverted (boolean)
> > > - touchscreen-swapped-x-y : X and Y axis are swapped (boolean)
> > > (swapping is done after inverting the axis)
> > > + - AVDD28-supply : Analog power supply regulator on AVDD28 pin
> >
> > I think we normally use lower case in DT bindings and rarely encode
> > voltage in the supply name unless we are dealing with several supplies
> > of the same kind, but I'll let Ron comment on this.
>
> Yes on lowercase though there are some exceptions.
>
> There's also a AVDD22 supply as well as DVDD12 and VDDIO. So we
> probably need to keep the voltage, but the binding is incomplete.
What is incomplete here? can you please elaborate.
^ permalink raw reply
* Re: [PATCH v3] HID: i2c-hid: Ignore input report if there's no data present on Elan touchpanels
From: Benjamin Tissoires @ 2019-01-18 15:50 UTC (permalink / raw)
To: Kai-Heng Feng; +Cc: Jiri Kosina, open list:HID CORE LAYER, lkml
In-Reply-To: <20190107072429.23828-1-kai.heng.feng@canonical.com>
Hi Kai-Heng,
On Mon, Jan 7, 2019 at 8:24 AM Kai-Heng Feng
<kai.heng.feng@canonical.com> wrote:
>
> While using Elan touchpads, the message floods:
> [ 136.138487] i2c_hid i2c-DELL08D6:00: i2c_hid_get_input: incomplete report (14/65535)
>
> Though the message flood is annoying, the device it self works without
> any issue. I suspect that the device in question takes too much time to
> pull the IRQ back to high after I2C host has done reading its data.
>
> Since the host receives all useful data, let's ignore the input report
> when there's no data.
>
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
Thanks for the v3.
This patch has just been brought to my attention, and I have one
question before applying it:
are those messages (without your patch) occurring all the time, or
just after resume?
I wonder if the pm_suspend delay we talked about in the other thread
would fix that issue in a cleaner way.
Cheers,
Benjamin
> ---
> v3:
> Fix compiler error/warnings.
>
> v2:
> Use dev_warn_once() to warn the user about the situation.
>
> drivers/hid/i2c-hid/i2c-hid-core.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
> index 8555ce7e737b..2f940c1de616 100644
> --- a/drivers/hid/i2c-hid/i2c-hid-core.c
> +++ b/drivers/hid/i2c-hid/i2c-hid-core.c
> @@ -50,6 +50,7 @@
> #define I2C_HID_QUIRK_NO_IRQ_AFTER_RESET BIT(1)
> #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)
>
> /* flags */
> #define I2C_HID_STARTED 0
> @@ -179,6 +180,8 @@ static const struct i2c_hid_quirks {
> I2C_HID_QUIRK_DELAY_AFTER_SLEEP },
> { USB_VENDOR_ID_LG, I2C_DEVICE_ID_LG_8001,
> I2C_HID_QUIRK_NO_RUNTIME_PM },
> + { USB_VENDOR_ID_ELAN, HID_ANY_ID,
> + I2C_HID_QUIRK_BOGUS_IRQ },
> { 0, 0 }
> };
>
> @@ -503,6 +506,12 @@ static void i2c_hid_get_input(struct i2c_hid *ihid)
> return;
> }
>
> + if (ihid->quirks & I2C_HID_QUIRK_BOGUS_IRQ && ret_size == 0xffff) {
> + dev_warn_once(&ihid->client->dev, "%s: IRQ triggered but "
> + "there's no data\n", __func__);
> + return;
> + }
> +
> if ((ret_size > size) || (ret_size < 2)) {
> dev_err(&ihid->client->dev, "%s: incomplete report (%d/%d)\n",
> __func__, size, ret_size);
> --
> 2.17.1
>
^ permalink raw reply
* [PATCH 13/13] MAINTAINERS: add an entry for max77650 mfd driver
From: Bartosz Golaszewski @ 2019-01-18 13:42 UTC (permalink / raw)
To: Rob Herring, Mark Rutland, Linus Walleij, Dmitry Torokhov,
Jacek Anaszewski, Pavel Machek, Lee Jones, Sebastian Reichel,
Liam Girdwood, Mark Brown, Greg Kroah-Hartman
Cc: linux-kernel, linux-gpio, devicetree, linux-input, linux-leds,
linux-pm, Bartosz Golaszewski
In-Reply-To: <20190118134244.22253-1-brgl@bgdev.pl>
From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
I plan on extending this set of drivers so add myself as maintainer.
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
MAINTAINERS | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 4d04cebb4a71..bb29fe250796 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9220,6 +9220,20 @@ S: Maintained
F: Documentation/devicetree/bindings/sound/max9860.txt
F: sound/soc/codecs/max9860.*
+MAXIM MAX77650 PMIC MFD DRIVER
+M: Bartosz Golaszewski <bgolaszewski@baylibre.com>
+L: linux-kernel@vger.kernel.org
+S: Maintained
+F: Documentation/devicetree/bindings/*/*max77650.txt
+F: Documentation/devicetree/bindings/*/max77650*.txt
+F: include/linux/mfd/max77650.h
+F: drivers/mfd/max77650.c
+F: drivers/regulator/max77650-regulator.c
+F: drivers/power/supply/max77650-charger.c
+F: drivers/input/misc/max77650-onkey.c
+F: drivers/leds/leds-max77650.c
+F: drivers/gpio/gpio-max77650.c
+
MAXIM MAX77802 PMIC REGULATOR DEVICE DRIVER
M: Javier Martinez Canillas <javier@dowhile0.org>
L: linux-kernel@vger.kernel.org
--
2.19.1
^ permalink raw reply related
* [PATCH 12/13] input: max77650: add onkey support
From: Bartosz Golaszewski @ 2019-01-18 13:42 UTC (permalink / raw)
To: Rob Herring, Mark Rutland, Linus Walleij, Dmitry Torokhov,
Jacek Anaszewski, Pavel Machek, Lee Jones, Sebastian Reichel,
Liam Girdwood, Mark Brown, Greg Kroah-Hartman
Cc: linux-kernel, linux-gpio, devicetree, linux-input, linux-leds,
linux-pm, Bartosz Golaszewski
In-Reply-To: <20190118134244.22253-1-brgl@bgdev.pl>
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>
---
drivers/input/misc/Kconfig | 9 ++
drivers/input/misc/Makefile | 1 +
drivers/input/misc/max77650-onkey.c | 135 ++++++++++++++++++++++++++++
3 files changed, 145 insertions(+)
create mode 100644 drivers/input/misc/max77650-onkey.c
diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index ca59a2be9bc5..bb9c45c1269e 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -180,6 +180,15 @@ config INPUT_M68K_BEEP
tristate "M68k Beeper support"
depends on M68K
+config INPUT_MAX77650_ONKEY
+ tristate "Maxim MAX77650 ONKEY support"
+ depends on MFD_MAX77650
+ help
+ Support the ONKEY of the MAX77650 PMIC as an input device.
+
+ To compile this driver as a module, choose M here: the module
+ will be called max77650-onkey.
+
config INPUT_MAX77693_HAPTIC
tristate "MAXIM MAX77693/MAX77843 haptic controller support"
depends on (MFD_MAX77693 || MFD_MAX77843) && PWM
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index 9d0f9d1ff68f..5bd53590ce60 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -43,6 +43,7 @@ obj-$(CONFIG_INPUT_IXP4XX_BEEPER) += ixp4xx-beeper.o
obj-$(CONFIG_INPUT_KEYSPAN_REMOTE) += keyspan_remote.o
obj-$(CONFIG_INPUT_KXTJ9) += kxtj9.o
obj-$(CONFIG_INPUT_M68K_BEEP) += m68kspkr.o
+obj-$(CONFIG_INPUT_MAX77650_ONKEY) += max77650-onkey.o
obj-$(CONFIG_INPUT_MAX77693_HAPTIC) += max77693-haptic.o
obj-$(CONFIG_INPUT_MAX8925_ONKEY) += max8925_onkey.o
obj-$(CONFIG_INPUT_MAX8997_HAPTIC) += max8997_haptic.o
diff --git a/drivers/input/misc/max77650-onkey.c b/drivers/input/misc/max77650-onkey.c
new file mode 100644
index 000000000000..cc7e83f589cd
--- /dev/null
+++ b/drivers/input/misc/max77650-onkey.c
@@ -0,0 +1,135 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2018 BayLibre SAS
+ * Author: Bartosz Golaszewski <bgolaszewski@baylibre.com>
+ *
+ * ONKEY driver for MAXIM 77650/77651 charger/power-supply.
+ */
+
+#include <linux/i2c.h>
+#include <linux/input.h>
+#include <linux/interrupt.h>
+#include <linux/mfd/max77650.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#define MAX77650_ONKEY_MODE_MASK BIT(3)
+#define MAX77650_ONKEY_MODE_PUSH 0x00
+#define MAX77650_ONKEY_MODE_SLIDE BIT(3)
+
+struct max77650_onkey {
+ struct input_dev *input;
+ unsigned int code;
+};
+
+static irqreturn_t
+max77650_onkey_report(struct max77650_onkey *onkey, int value)
+{
+ input_report_key(onkey->input, onkey->code, value);
+ input_sync(onkey->input);
+
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t max77650_onkey_falling(int irq, void *data)
+{
+ struct max77650_onkey *onkey = data;
+
+ return max77650_onkey_report(onkey, 0);
+}
+
+static irqreturn_t max77650_onkey_rising(int irq, void *data)
+{
+ struct max77650_onkey *onkey = data;
+
+ return max77650_onkey_report(onkey, 1);
+}
+
+static int max77650_onkey_probe(struct platform_device *pdev)
+{
+ struct regmap_irq_chip_data *irq_data;
+ struct max77650_onkey *onkey;
+ struct device *dev, *parent;
+ int irq_r, irq_f, rv, mode;
+ struct i2c_client *i2c;
+ const char *mode_prop;
+ struct regmap *map;
+
+ dev = &pdev->dev;
+ parent = dev->parent;
+ i2c = to_i2c_client(parent);
+ irq_data = i2c_get_clientdata(i2c);
+
+ map = dev_get_regmap(parent, NULL);
+ if (!map)
+ return -ENODEV;
+
+ onkey = devm_kzalloc(dev, sizeof(*onkey), GFP_KERNEL);
+ if (!onkey)
+ return -ENOMEM;
+
+ rv = device_property_read_u32(dev, "linux,code", &onkey->code);
+ if (rv)
+ onkey->code = KEY_POWER;
+
+ rv = device_property_read_string(dev, "maxim,onkey-mode", &mode_prop);
+ if (rv)
+ mode_prop = "push";
+
+ if (strcmp(mode_prop, "push") == 0)
+ mode = MAX77650_ONKEY_MODE_PUSH;
+ else if (strcmp(mode_prop, "slide") == 0)
+ mode = MAX77650_ONKEY_MODE_SLIDE;
+ else
+ return -EINVAL;
+
+ rv = regmap_update_bits(map, MAX77650_REG_CNFG_GLBL,
+ MAX77650_ONKEY_MODE_MASK, mode);
+ if (rv)
+ return rv;
+
+ irq_f = regmap_irq_get_virq(irq_data, MAX77650_INT_nEN_F);
+ if (irq_f <= 0)
+ return -EINVAL;
+
+ irq_r = regmap_irq_get_virq(irq_data, MAX77650_INT_nEN_R);
+ if (irq_r <= 0)
+ return -EINVAL;
+
+ onkey->input = devm_input_allocate_device(dev);
+ if (!onkey->input)
+ return -ENOMEM;
+
+ onkey->input->name = "max77650_onkey";
+ onkey->input->phys = "max77650_onkey/input0";
+ onkey->input->id.bustype = BUS_I2C;
+ onkey->input->dev.parent = dev;
+ input_set_capability(onkey->input, EV_KEY, onkey->code);
+
+ rv = devm_request_threaded_irq(dev, irq_f, NULL,
+ max77650_onkey_falling,
+ IRQF_ONESHOT, "onkey-down", onkey);
+ if (rv)
+ return rv;
+
+ rv = devm_request_threaded_irq(dev, irq_r, NULL,
+ max77650_onkey_rising,
+ IRQF_ONESHOT, "onkey-up", onkey);
+ if (rv)
+ return rv;
+
+ return input_register_device(onkey->input);
+}
+
+static struct platform_driver max77650_onkey_driver = {
+ .driver = {
+ .name = "max77650-onkey",
+ },
+ .probe = max77650_onkey_probe,
+};
+module_platform_driver(max77650_onkey_driver);
+
+MODULE_DESCRIPTION("MAXIM 77650/77651 ONKEY driver");
+MODULE_AUTHOR("Bartosz Golaszewski <bgolaszewski@baylibre.com>");
+MODULE_LICENSE("GPL");
--
2.19.1
^ permalink raw reply related
* [PATCH 11/13] leds: max77650: add LEDs support
From: Bartosz Golaszewski @ 2019-01-18 13:42 UTC (permalink / raw)
To: Rob Herring, Mark Rutland, Linus Walleij, Dmitry Torokhov,
Jacek Anaszewski, Pavel Machek, Lee Jones, Sebastian Reichel,
Liam Girdwood, Mark Brown, Greg Kroah-Hartman
Cc: linux-kernel, linux-gpio, devicetree, linux-input, linux-leds,
linux-pm, Bartosz Golaszewski
In-Reply-To: <20190118134244.22253-1-brgl@bgdev.pl>
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>
---
drivers/leds/Kconfig | 6 ++
drivers/leds/Makefile | 1 +
drivers/leds/leds-max77650.c | 162 +++++++++++++++++++++++++++++++++++
3 files changed, 169 insertions(+)
create mode 100644 drivers/leds/leds-max77650.c
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index a72f97fca57b..6e7a8f51eccc 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -608,6 +608,12 @@ config LEDS_TLC591XX
This option enables support for Texas Instruments TLC59108
and TLC59116 LED controllers.
+config LEDS_MAX77650
+ tristate "LED support for Maxim MAX77650 PMIC"
+ depends on MFD_MAX77650
+ help
+ LEDs driver for MAX77650 family of PMICs from Maxim Integrated."
+
config LEDS_MAX77693
tristate "LED support for MAX77693 Flash"
depends on LEDS_CLASS_FLASH
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index 4c1b0054f379..f48b2404dbb7 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -61,6 +61,7 @@ obj-$(CONFIG_LEDS_MC13783) += leds-mc13783.o
obj-$(CONFIG_LEDS_NS2) += leds-ns2.o
obj-$(CONFIG_LEDS_NETXBIG) += leds-netxbig.o
obj-$(CONFIG_LEDS_ASIC3) += leds-asic3.o
+obj-$(CONFIG_LEDS_MAX77650) += leds-max77650.o
obj-$(CONFIG_LEDS_MAX77693) += leds-max77693.o
obj-$(CONFIG_LEDS_MAX8997) += leds-max8997.o
obj-$(CONFIG_LEDS_LM355x) += leds-lm355x.o
diff --git a/drivers/leds/leds-max77650.c b/drivers/leds/leds-max77650.c
new file mode 100644
index 000000000000..be2bb1c60448
--- /dev/null
+++ b/drivers/leds/leds-max77650.c
@@ -0,0 +1,162 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2018 BayLibre SAS
+ * Author: Bartosz Golaszewski <bgolaszewski@baylibre.com>
+ *
+ * LEDS driver for MAXIM 77650/77651 charger/power-supply.
+ */
+
+#include <linux/i2c.h>
+#include <linux/leds.h>
+#include <linux/mfd/max77650.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#define MAX77650_NUM_LEDS 3
+
+#define MAX77650_LED_A_BASE 0x40
+#define MAX77650_LED_B_BASE 0x43
+
+#define MAX77650_LED_BR_MASK GENMASK(4, 0)
+#define MAX77650_LED_EN_MASK GENMASK(7, 6)
+
+/* Enable EN_LED_MSTR. */
+#define MAX77650_LED_TOP_DEFAULT BIT(0)
+
+#define MAX77650_LED_ENABLE GENMASK(7, 6)
+#define MAX77650_LED_DISABLE 0x00
+
+#define MAX77650_LED_A_DEFAULT MAX77650_LED_DISABLE
+/* 100% on duty */
+#define MAX77650_LED_B_DEFAULT GENMASK(3, 0)
+
+struct max77650_leds;
+
+struct max77650_led {
+ struct led_classdev cdev;
+ struct max77650_leds *parent;
+ unsigned int regA;
+ unsigned int regB;
+};
+
+struct max77650_leds {
+ struct regmap *map;
+ struct max77650_led leds[MAX77650_NUM_LEDS];
+};
+
+static struct max77650_led *max77650_to_led(struct led_classdev *cdev)
+{
+ return container_of(cdev, struct max77650_led, cdev);
+}
+
+static int max77650_leds_brightness_set(struct led_classdev *cdev,
+ enum led_brightness brightness)
+{
+ struct max77650_led *led = max77650_to_led(cdev);
+ struct regmap *regmap = led->parent->map;
+ int val, mask;
+
+ mask = MAX77650_LED_BR_MASK | MAX77650_LED_EN_MASK;
+
+ if (brightness == LED_OFF) {
+ val = MAX77650_LED_DISABLE;
+ } else {
+ val = MAX77650_LED_ENABLE;
+ /*
+ * We can set the brightness with 5-bit resolution.
+ *
+ * For brightness == 1, the bits we're writing will be 0, but
+ * since we keep LED_FS0 set to 12.8mA full-scale range, the
+ * LED will be lit slightly.
+ */
+ val |= brightness / 8;
+ }
+
+ return regmap_update_bits(regmap, led->regA, mask, val);
+}
+
+static int max77650_leds_probe(struct platform_device *pdev)
+{
+ struct device_node *of_node, *child;
+ struct max77650_leds *leds;
+ struct max77650_led *led;
+ struct device *parent;
+ struct device *dev;
+ int rv, num_leds;
+ u32 reg;
+
+ dev = &pdev->dev;
+ parent = dev->parent;
+ of_node = dev->of_node;
+
+ if (!of_node)
+ return -ENODEV;
+
+ leds = devm_kzalloc(dev, sizeof(*leds), GFP_KERNEL);
+ if (!leds)
+ return -ENOMEM;
+
+ leds->map = dev_get_regmap(dev->parent, NULL);
+ if (!leds->map)
+ return -ENODEV;
+
+ num_leds = of_get_child_count(of_node);
+ if (!num_leds || num_leds > MAX77650_NUM_LEDS)
+ return -ENODEV;
+
+ for_each_child_of_node(of_node, child) {
+ rv = of_property_read_u32(child, "reg", ®);
+ if (rv || reg >= MAX77650_NUM_LEDS)
+ return -EINVAL;
+
+ led = &leds->leds[reg];
+
+ led->parent = leds;
+ led->regA = MAX77650_LED_A_BASE + reg;
+ led->regB = MAX77650_LED_B_BASE + reg;
+ led->cdev.brightness_set_blocking
+ = max77650_leds_brightness_set;
+
+ led->cdev.name = of_get_property(child, "label", NULL);
+ if (!led->cdev.name)
+ led->cdev.name = child->name;
+
+ of_property_read_string(child, "linux,default-trigger",
+ &led->cdev.default_trigger);
+
+ rv = devm_of_led_classdev_register(dev, child, &led->cdev);
+ if (rv)
+ return rv;
+
+ rv = regmap_write(leds->map, led->regA,
+ MAX77650_LED_A_DEFAULT);
+ if (rv)
+ return rv;
+
+ rv = regmap_write(leds->map, led->regB,
+ MAX77650_LED_B_DEFAULT);
+ if (rv)
+ return rv;
+ }
+
+ rv = regmap_write(leds->map,
+ MAX77650_REG_CNFG_LED_TOP,
+ MAX77650_LED_TOP_DEFAULT);
+ if (rv)
+ return rv;
+
+ return 0;
+}
+
+static struct platform_driver max77650_leds_driver = {
+ .driver = {
+ .name = "max77650-leds",
+ },
+ .probe = max77650_leds_probe,
+};
+module_platform_driver(max77650_leds_driver);
+
+MODULE_DESCRIPTION("MAXIM 77650/77651 LED driver");
+MODULE_AUTHOR("Bartosz Golaszewski <bgolaszewski@baylibre.com>");
+MODULE_LICENSE("GPL");
--
2.19.1
^ permalink raw reply related
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