* [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
* [PATCH 1/3] Revert "Input: olpc_apsp - enable 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, Lubomir Rintel
In-Reply-To: <20190121062255.551587-1-lkundrak@v3.sk>
Turns out this is not such a great idea. Once the SP clock is disabled,
it's not sufficient to just enable in order to bring the SP core back up.
It seems that the kernel has no business managing this clock. Just let
the firmware keep it enabled.
This reverts commit ed22cee91a88c47e564478b012fdbcb079653499.
Link: https://lore.kernel.org/lkml/154783267051.169631.3197836544646625747@swboyd.mtv.corp.google.com/
Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
---
.../devicetree/bindings/serio/olpc,ap-sp.txt | 4 ----
drivers/input/serio/olpc_apsp.c | 14 --------------
2 files changed, 18 deletions(-)
diff --git a/Documentation/devicetree/bindings/serio/olpc,ap-sp.txt b/Documentation/devicetree/bindings/serio/olpc,ap-sp.txt
index 36603419d6f8..0e72183f52bc 100644
--- a/Documentation/devicetree/bindings/serio/olpc,ap-sp.txt
+++ b/Documentation/devicetree/bindings/serio/olpc,ap-sp.txt
@@ -4,14 +4,10 @@ Required properties:
- compatible : "olpc,ap-sp"
- reg : base address and length of SoC's WTM registers
- interrupts : SP-AP interrupt
-- clocks : phandle + clock-specifier for the clock that drives the WTM
-- clock-names: should be "sp"
Example:
ap-sp@d4290000 {
compatible = "olpc,ap-sp";
reg = <0xd4290000 0x1000>;
interrupts = <40>;
- clocks = <&soc_clocks MMP2_CLK_SP>;
- clock-names = "sp";
}
diff --git a/drivers/input/serio/olpc_apsp.c b/drivers/input/serio/olpc_apsp.c
index bae08226e3d9..a7cfab3db9ee 100644
--- a/drivers/input/serio/olpc_apsp.c
+++ b/drivers/input/serio/olpc_apsp.c
@@ -23,7 +23,6 @@
#include <linux/of.h>
#include <linux/slab.h>
#include <linux/delay.h>
-#include <linux/clk.h>
/*
* The OLPC XO-1.75 and XO-4 laptops do not have a hardware PS/2 controller.
@@ -75,7 +74,6 @@ struct olpc_apsp {
struct serio *kbio;
struct serio *padio;
void __iomem *base;
- struct clk *clk;
int open_count;
int irq;
};
@@ -148,17 +146,11 @@ static int olpc_apsp_open(struct serio *port)
struct olpc_apsp *priv = port->port_data;
unsigned int tmp;
unsigned long l;
- int error;
if (priv->open_count++ == 0) {
- error = clk_prepare_enable(priv->clk);
- if (error)
- return error;
-
l = readl(priv->base + COMMAND_FIFO_STATUS);
if (!(l & CMD_STS_MASK)) {
dev_err(priv->dev, "SP cannot accept commands.\n");
- clk_disable_unprepare(priv->clk);
return -EIO;
}
@@ -179,8 +171,6 @@ static void olpc_apsp_close(struct serio *port)
/* Disable interrupt 0 */
tmp = readl(priv->base + PJ_INTERRUPT_MASK);
writel(tmp | INT_0, priv->base + PJ_INTERRUPT_MASK);
-
- clk_disable_unprepare(priv->clk);
}
}
@@ -208,10 +198,6 @@ static int olpc_apsp_probe(struct platform_device *pdev)
if (priv->irq < 0)
return priv->irq;
- priv->clk = devm_clk_get(&pdev->dev, "sp");
- if (IS_ERR(priv->clk))
- return PTR_ERR(priv->clk);
-
/* KEYBOARD */
kb_serio = kzalloc(sizeof(struct serio), GFP_KERNEL);
if (!kb_serio)
--
2.20.1
^ permalink raw reply related
* [PATCH 2/3] Revert "clk: mmp2: add 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, Lubomir Rintel
In-Reply-To: <20190121062255.551587-1-lkundrak@v3.sk>
It seems that the kernel has no business managing this clock: once the SP
clock is disabled, it's not sufficient to just enable in order to bring the
SP core back up. Just let the firmware keep it enabled and don't expose it
to drivers.
This reverts commit fc27c2394d96fd19854b7e2d3f0e60df0d86fc90.
Link: https://lore.kernel.org/lkml/154783267051.169631.3197836544646625747@swboyd.mtv.corp.google.com/
Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
---
drivers/clk/mmp/clk-of-mmp2.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/drivers/clk/mmp/clk-of-mmp2.c b/drivers/clk/mmp/clk-of-mmp2.c
index 61fefc046ec5..d083b860f083 100644
--- a/drivers/clk/mmp/clk-of-mmp2.c
+++ b/drivers/clk/mmp/clk-of-mmp2.c
@@ -53,7 +53,6 @@
#define APMU_DISP1 0x110
#define APMU_CCIC0 0x50
#define APMU_CCIC1 0xf4
-#define APMU_SP 0x68
#define MPMU_UART_PLL 0x14
struct mmp2_clk_unit {
@@ -210,8 +209,6 @@ static struct mmp_clk_mix_config ccic1_mix_config = {
.reg_info = DEFINE_MIX_REG_INFO(4, 16, 2, 6, 32),
};
-static DEFINE_SPINLOCK(sp_lock);
-
static struct mmp_param_mux_clk apmu_mux_clks[] = {
{MMP2_CLK_DISP0_MUX, "disp0_mux", disp_parent_names, ARRAY_SIZE(disp_parent_names), CLK_SET_RATE_PARENT, APMU_DISP0, 6, 2, 0, &disp0_lock},
{MMP2_CLK_DISP1_MUX, "disp1_mux", disp_parent_names, ARRAY_SIZE(disp_parent_names), CLK_SET_RATE_PARENT, APMU_DISP1, 6, 2, 0, &disp1_lock},
@@ -242,7 +239,6 @@ static struct mmp_param_gate_clk apmu_gate_clks[] = {
{MMP2_CLK_CCIC1, "ccic1_clk", "ccic1_mix_clk", CLK_SET_RATE_PARENT, APMU_CCIC1, 0x1b, 0x1b, 0x0, 0, &ccic1_lock},
{MMP2_CLK_CCIC1_PHY, "ccic1_phy_clk", "ccic1_mix_clk", CLK_SET_RATE_PARENT, APMU_CCIC1, 0x24, 0x24, 0x0, 0, &ccic1_lock},
{MMP2_CLK_CCIC1_SPHY, "ccic1_sphy_clk", "ccic1_sphy_div", CLK_SET_RATE_PARENT, APMU_CCIC1, 0x300, 0x300, 0x0, 0, &ccic1_lock},
- {MMP2_CLK_SP, "sp_clk", NULL, CLK_SET_RATE_PARENT, APMU_SP, 0x1b, 0x1b, 0x0, 0, &sp_lock},
};
static void mmp2_axi_periph_clk_init(struct mmp2_clk_unit *pxa_unit)
--
2.20.1
^ permalink raw reply related
* [PATCH 3/3] Revert "dt-bindings: marvell,mmp2: Add clock id for 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, Lubomir Rintel
In-Reply-To: <20190121062255.551587-1-lkundrak@v3.sk>
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.
Pretty sure nothing ever used this and it's safe to remove.
This reverts commit e8a2c779141415105825e65a4715f1130bba61b1.
Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
---
include/dt-bindings/clock/marvell,mmp2.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/include/dt-bindings/clock/marvell,mmp2.h b/include/dt-bindings/clock/marvell,mmp2.h
index 7b24fc791146..228a5e234af0 100644
--- a/include/dt-bindings/clock/marvell,mmp2.h
+++ b/include/dt-bindings/clock/marvell,mmp2.h
@@ -71,7 +71,6 @@
#define MMP2_CLK_CCIC1_MIX 117
#define MMP2_CLK_CCIC1_PHY 118
#define MMP2_CLK_CCIC1_SPHY 119
-#define MMP2_CLK_SP 120
#define MMP2_NR_CLKS 200
#endif
--
2.20.1
^ permalink raw reply related
* [PATCH v2] Input: elantech: Enable SMBus on new (2018+) systems
From: Kai-Heng Feng @ 2019-01-21 7:02 UTC (permalink / raw)
To: dmitry.torokhov
Cc: benjamin.tissoires, kt.liao, linux-input, linux-kernel,
Kai-Heng Feng
There are some new HP laptops with Elantech touchpad don't support
multitouch.
Currently we use ETP_NEW_IC_SMBUS_HOST_NOTIFY() to check if SMBus is
supported, but in addition to firmware version, the bus type also
informs us if the IC can support SMBus, so also check that.
In case of breaking old ICs, only enables SMBus on systems manufactured
after 2018, alongsides aforementioned checks.
Lastly, consolidats all check into elantech_use_host_notify() and use it
to determine whether to use PS/2 or SMBus.
Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
---
v2:
- Wording.
- Further restrain on older systems (< 2018).
drivers/input/mouse/elantech.c | 63 ++++++++++++++++++----------------
1 file changed, 34 insertions(+), 29 deletions(-)
diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
index 9fe075c137dc..2594130b0079 100644
--- a/drivers/input/mouse/elantech.c
+++ b/drivers/input/mouse/elantech.c
@@ -1799,6 +1799,39 @@ static int elantech_create_smbus(struct psmouse *psmouse,
leave_breadcrumbs);
}
+static bool elantech_use_host_notify(struct psmouse *psmouse,
+ struct elantech_device_info *info)
+{
+ bool host_notify = false;
+
+ if (ETP_NEW_IC_SMBUS_HOST_NOTIFY(info->fw_version))
+ host_notify = true;
+ else {
+ 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:
+ host_notify = true;
+ break;
+ default:
+ psmouse_dbg(psmouse,
+ "Ignoring SMBus bus provider %d.\n",
+ info->bus);
+ }
+ }
+
+ /* SMbus implementation is stable after 2018 */
+ return host_notify && (dmi_get_bios_year() >= 2018);
+}
+
/**
* elantech_setup_smbus - called once the PS/2 devices are enumerated
* and decides to instantiate a SMBus InterTouch device.
@@ -1818,7 +1851,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) ||
psmouse_matches_pnp_id(psmouse, i2c_blacklist_pnp_ids))
return -ENXIO;
}
@@ -1838,34 +1871,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 related
* Re: [PATCH v2] Input: elantech: Enable SMBus on new (2018+) systems
From: Benjamin Tissoires @ 2019-01-21 8:33 UTC (permalink / raw)
To: Kai-Heng Feng
Cc: Dmitry Torokhov, 廖崇榮,
open list:HID CORE LAYER, lkml
In-Reply-To: <20190121070258.1844-1-kai.heng.feng@canonical.com>
On Mon, Jan 21, 2019 at 8:03 AM Kai-Heng Feng
<kai.heng.feng@canonical.com> wrote:
>
> There are some new HP laptops with Elantech touchpad don't support
> multitouch.
>
> Currently we use ETP_NEW_IC_SMBUS_HOST_NOTIFY() to check if SMBus is
> supported, but in addition to firmware version, the bus type also
> informs us if the IC can support SMBus, so also check that.
>
> In case of breaking old ICs, only enables SMBus on systems manufactured
> after 2018, alongsides aforementioned checks.
>
> Lastly, consolidats all check into elantech_use_host_notify() and use it
> to determine whether to use PS/2 or SMBus.
>
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> ---
> v2:
> - Wording.
> - Further restrain on older systems (< 2018).
>
> drivers/input/mouse/elantech.c | 63 ++++++++++++++++++----------------
> 1 file changed, 34 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
> index 9fe075c137dc..2594130b0079 100644
> --- a/drivers/input/mouse/elantech.c
> +++ b/drivers/input/mouse/elantech.c
> @@ -1799,6 +1799,39 @@ static int elantech_create_smbus(struct psmouse *psmouse,
> leave_breadcrumbs);
> }
>
> +static bool elantech_use_host_notify(struct psmouse *psmouse,
> + struct elantech_device_info *info)
> +{
> + bool host_notify = false;
> +
> + if (ETP_NEW_IC_SMBUS_HOST_NOTIFY(info->fw_version))
> + host_notify = true;
> + else {
> + 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:
> + host_notify = true;
> + break;
> + default:
> + psmouse_dbg(psmouse,
> + "Ignoring SMBus bus provider %d.\n",
> + info->bus);
> + }
> + }
> +
> + /* SMbus implementation is stable after 2018 */
> + return host_notify && (dmi_get_bios_year() >= 2018);
Strictly speaking, the check for the year should be in the `switch
(info->bus)`, but OTOH, laptops with ETP_NEW_IC_SMBUS_HOST_NOTIFY
should be manufactured after 2018 too, so we should be good.
Acked-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Cheers,
Benjamin
> +}
> +
> /**
> * elantech_setup_smbus - called once the PS/2 devices are enumerated
> * and decides to instantiate a SMBus InterTouch device.
> @@ -1818,7 +1851,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) ||
> psmouse_matches_pnp_id(psmouse, i2c_blacklist_pnp_ids))
> return -ENXIO;
> }
> @@ -1838,34 +1871,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 v1] HID: intel-ish-hid: Switch to use new generic UUID API
From: Christoph Hellwig @ 2019-01-21 8:40 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Srinivas Pandruvada, linux-input, Even Xu, linux-kernel,
Christoph Hellwig
In-Reply-To: <20190110154804.89298-1-andriy.shevchenko@linux.intel.com>
On Thu, Jan 10, 2019 at 05:48:04PM +0200, Andy Shevchenko wrote:
> There are new types and helpers that are supposed to be used in new code.
>
> As a preparation to get rid of legacy types and API functions do
> the conversion here.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> drivers/hid/intel-ish-hid/ishtp-hid-client.c | 3 +--
> drivers/hid/intel-ish-hid/ishtp-hid.h | 6 +++---
> drivers/hid/intel-ish-hid/ishtp/bus.c | 19 ++++++++-----------
> drivers/hid/intel-ish-hid/ishtp/bus.h | 4 ++--
> drivers/hid/intel-ish-hid/ishtp/client.h | 2 +-
> drivers/hid/intel-ish-hid/ishtp/hbm.h | 2 +-
> 6 files changed, 16 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/hid/intel-ish-hid/ishtp-hid-client.c b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
> index e64243bc9c96..2246697ada1d 100644
> --- a/drivers/hid/intel-ish-hid/ishtp-hid-client.c
> +++ b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
> @@ -788,8 +788,7 @@ static int hid_ishtp_cl_probe(struct ishtp_cl_device *cl_device)
> if (!cl_device)
> return -ENODEV;
>
> - if (uuid_le_cmp(hid_ishtp_guid,
> - cl_device->fw_client->props.protocol_name) != 0)
> + if (!guid_equal(&hid_ishtp_guid, &cl_device->fw_client->props.protocol_name))
This adds an overly long line.
With that fixed:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply
* Re: [PATCH 1/7] Input: document meanings of KEY_SCREEN and KEY_ZOOM
From: Jiri Kosina @ 2019-01-21 10:11 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Benjamin Tissoires, Mauro Carvalho Chehab, linux-input,
linux-kernel, linux-media
In-Reply-To: <20190118233037.87318-1-dmitry.torokhov@gmail.com>
On Fri, 18 Jan 2019, Dmitry Torokhov wrote:
> 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
Acked-by: Jiri Kosina <jkosina@suse.cz>
for the HID changes, and feel free to take it through your tree as a
whole, I don't expect any major conflicts rising up from this.
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH 1/7] Input: document meanings of KEY_SCREEN and KEY_ZOOM
From: Benjamin Tissoires @ 2019-01-21 10:41 UTC (permalink / raw)
To: Jiri Kosina
Cc: Dmitry Torokhov, Mauro Carvalho Chehab, open list:HID CORE LAYER,
lkml, linux-media
In-Reply-To: <nycvar.YFH.7.76.1901211110190.6626@cbobk.fhfr.pm>
On Mon, Jan 21, 2019 at 11:11 AM Jiri Kosina <jikos@kernel.org> wrote:
>
> On Fri, 18 Jan 2019, Dmitry Torokhov wrote:
>
> > 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
>
> Acked-by: Jiri Kosina <jkosina@suse.cz>
Acked-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
>
> for the HID changes, and feel free to take it through your tree as a
> whole, I don't expect any major conflicts rising up from this.
Works for me too. My tests showed no issues, so that's OK from me.
Cheers,
Benjamin
>
> Thanks,
>
> --
> Jiri Kosina
> SUSE Labs
>
^ permalink raw reply
* Re: [PATCH 12/13] input: max77650: add onkey support
From: Bartosz Golaszewski @ 2019-01-21 10:52 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Rob Herring, Mark Rutland, Linus Walleij, Jacek Anaszewski,
Pavel Machek, Lee Jones, Sebastian Reichel, Liam Girdwood,
Mark Brown, Greg Kroah-Hartman, Linux Kernel Mailing List,
open list:GPIO SUBSYSTEM, devicetree, linux-input, linux-leds,
linux-pm, Bartosz Golaszewski
In-Reply-To: <20190119090318.GB187380@dtor-ws>
sob., 19 sty 2019 o 10:03 Dmitry Torokhov <dmitry.torokhov@gmail.com>
napisał(a):
>
> 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.
>
Hi Dmitry,
actually it's the other way around. Take a look at the function
prototype for devm_request_threaded_irq()[1].
The third parameter is the hard-irq handler (NULL in my patch), the
fourth is the thread function. Actually even if I did what you're
saying - it would never work as this is a nested irq for which the
hard-irq handler is never called.
For the rest: I'll fix it in v2.
Best regards,
Bartosz Golaszewski
[1] https://elixir.bootlin.com/linux/latest/source/kernel/irq/devres.c#L52
> > + 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
* Re: [PATCH 04/13] dt-bindings: gpio: add DT bindings for max77650
From: Linus Walleij @ 2019-01-21 14:04 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Rob Herring, Mark Rutland, Dmitry Torokhov, Jacek Anaszewski,
Pavel Machek, Lee Jones, Sebastian Reichel, Liam Girdwood,
Mark Brown, Greg Kroah-Hartman, linux-kernel@vger.kernel.org,
open list:GPIO SUBSYSTEM,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux Input, Linux LED Subsystem, Linux PM list,
Bartosz Golaszewski
In-Reply-To: <20190118134244.22253-5-brgl@bgdev.pl>
On Fri, Jan 18, 2019 at 2:43 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>
> Add the DT binding document for the GPIO module of max77650.
>
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
Very simple so not much to complain about :)
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH 10/13] gpio: max77650: add GPIO support
From: Linus Walleij @ 2019-01-21 14:20 UTC (permalink / raw)
To: Bartosz Golaszewski, Brian Masney
Cc: Rob Herring, Mark Rutland, Dmitry Torokhov, Jacek Anaszewski,
Pavel Machek, Lee Jones, Sebastian Reichel, Liam Girdwood,
Mark Brown, Greg Kroah-Hartman, linux-kernel@vger.kernel.org,
open list:GPIO SUBSYSTEM,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux Input, Linux LED Subsystem, Linux PM list,
Bartosz Golaszewski
In-Reply-To: <20190118134244.22253-11-brgl@bgdev.pl>
Hi Bartosz,
thanks for the patch!
On Fri, Jan 18, 2019 at 2:43 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>
> Add GPIO support for max77650 mfd device. This PMIC exposes a single
> GPIO line.
>
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
Overall you know for sure what you're doing so not much to
say about the GPIO chip etc. The .set_config() is nice and
helpful (maybe you will be able to also add pull up/down
using Thomas Petazzoni's new config interfaces!)
However enlighten me on this:
> +static int max77650_gpio_to_irq(struct gpio_chip *gc, unsigned int offset)
> +{
> + struct max77650_gpio_chip *chip = gpiochip_get_data(gc);
> +
> + return regmap_irq_get_virq(chip->irq_chip_data, MAX77650_INT_GPI);
> +}
I know this may be opening the gates to a lot of coding, but
isn't this IRQ hierarchical? I.e. that irqchip is not in the
node of the GPIO chip but in the node of the MFD top
device, with a 1:1 mapping between some of the IRQs
and a certain GPIO line.
Using regmap IRQ makes it confusing for me so I do not
know for sure if that is the case.
I am worried that you are recreating a problem (present in many
drivers, including some written by me, mea culpa) that Brian Masney
has been trying to solve for the gpiochip inside the SPMI
GPIO (drivers/pinctrl/qcom/pinctrl-spmi-gpio.c).
I have queued Brians refactoring and solution here:
https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git/log/?h=ib-qcom-spmi
And the overall description on what he's trying to achieve is
here:
https://marc.info/?l=linux-gpio&m=154793071511130&w=2
My problem description (which I fear will apply also to this
driver):
https://www.spinics.net/lists/linux-gpio/msg34655.html
I plan to merge Brians patches soon-ish to devel and then from
there try to construct more helpers in the gpiolib core,
and if possible fix some of the old drivers who rely on
.to_irq().
We will certainly fix ssbi-gpio as well, and that is a good start
since the Qualdcomm platforms are so pervasive in the
market.
But maybe this doesn't apply here? I am not the smartest...
Just want to make sure that it is possible to refer an
interrupt directly to this DT node, as it is indeed marked
as interrupt-controller.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH v3 1/4] dt-bindings: input: touchscreen: goodix: Document AVDD28-supply property
From: Rob Herring @ 2019-01-21 16:16 UTC (permalink / raw)
To: Jagan Teki
Cc: devicetree, Dmitry Torokhov, Chen-Yu Tsai, linux-input,
linux-kernel@vger.kernel.org, Michael Trimarchi, linux-amarula
In-Reply-To: <CAMty3ZCyq=tZMt6RC1aYJuEWgYp2zv1LBh+ZUwucE+g4RCoUaQ@mail.gmail.com>
On Fri, Jan 18, 2019 at 10:01 AM Jagan Teki <jagan@amarulasolutions.com> wrote:
>
> 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.
You are missing the 3 other supplies the chip has: AVDD22, DVDD12 and VDDIO.
Rob
^ permalink raw reply
* Re: [PATCH 10/13] gpio: max77650: add GPIO support
From: Bartosz Golaszewski @ 2019-01-21 17:07 UTC (permalink / raw)
To: Linus Walleij
Cc: Brian Masney, Rob Herring, Mark Rutland, Dmitry Torokhov,
Jacek Anaszewski, Pavel Machek, Lee Jones, Sebastian Reichel,
Liam Girdwood, Mark Brown, Greg Kroah-Hartman,
linux-kernel@vger.kernel.org, open list:GPIO SUBSYSTEM,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux Input, Linux LED Subsystem, Linux PM list
In-Reply-To: <CACRpkdZhy7yBZrGLxhRGaac7v5jZhgUSz9gZAhFXbevJbKjpZw@mail.gmail.com>
pon., 21 sty 2019 o 15:20 Linus Walleij <linus.walleij@linaro.org> napisał(a):
>
> Hi Bartosz,
>
> thanks for the patch!
>
> On Fri, Jan 18, 2019 at 2:43 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> > From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> >
> > Add GPIO support for max77650 mfd device. This PMIC exposes a single
> > GPIO line.
> >
> > Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>
> Overall you know for sure what you're doing so not much to
> say about the GPIO chip etc. The .set_config() is nice and
> helpful (maybe you will be able to also add pull up/down
> using Thomas Petazzoni's new config interfaces!)
>
> However enlighten me on this:
>
> > +static int max77650_gpio_to_irq(struct gpio_chip *gc, unsigned int offset)
> > +{
> > + struct max77650_gpio_chip *chip = gpiochip_get_data(gc);
> > +
> > + return regmap_irq_get_virq(chip->irq_chip_data, MAX77650_INT_GPI);
> > +}
>
> I know this may be opening the gates to a lot of coding, but
> isn't this IRQ hierarchical? I.e. that irqchip is not in the
> node of the GPIO chip but in the node of the MFD top
> device, with a 1:1 mapping between some of the IRQs
> and a certain GPIO line.
>
> Using regmap IRQ makes it confusing for me so I do not
> know for sure if that is the case.
>
> I am worried that you are recreating a problem (present in many
> drivers, including some written by me, mea culpa) that Brian Masney
> has been trying to solve for the gpiochip inside the SPMI
> GPIO (drivers/pinctrl/qcom/pinctrl-spmi-gpio.c).
>
> I have queued Brians refactoring and solution here:
> https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git/log/?h=ib-qcom-spmi
>
> And the overall description on what he's trying to achieve is
> here:
> https://marc.info/?l=linux-gpio&m=154793071511130&w=2
>
> My problem description (which I fear will apply also to this
> driver):
> https://www.spinics.net/lists/linux-gpio/msg34655.html
>
> I plan to merge Brians patches soon-ish to devel and then from
> there try to construct more helpers in the gpiolib core,
> and if possible fix some of the old drivers who rely on
> .to_irq().
>
> We will certainly fix ssbi-gpio as well, and that is a good start
> since the Qualdcomm platforms are so pervasive in the
> market.
>
> But maybe this doesn't apply here? I am not the smartest...
> Just want to make sure that it is possible to refer an
> interrupt directly to this DT node, as it is indeed marked
> as interrupt-controller.
>
Hi Linus,
Thank you for your review. While I think you're right about the issue
being present in this driver, I'm not sure it's really a problem. Do
we actually require every gpio-controller to also be a stand-alone
interrupt-controller? The binding document for the GPIO module doesn't
mention this - it only requires the gpio-controller property. Without
the "interrupt-controller" property dtc will bail-out if anyone uses
this node as the interrupt parent.
If I'm wrong and we do require it, then I think we need to update
Documentation/devicetree/bindings/gpio/gpio.txt.
Best regards,
Bartosz Golaszewski
PS: FYI since this submission I dropped the virtual irq number lookup
in sub-drivers in favor of resources setup by the parent driver[1] as
suggested by Dmitry in his review of the input module driver.
[1] https://github.com/brgl/linux/blob/topic/max77650_mfd_driver/drivers/gpio/gpio-max77650.c#L158
^ permalink raw reply
* Re: [PATCH] Input: synaptics - add PNP IDs for Dell XPS models to forcepad
From: Kim Phillips @ 2019-01-21 23:10 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Dmitry Torokhov, lkml, Paul Menzel, open list:HID CORE LAYER
In-Reply-To: <CAO-hwJLBd-VGAoMBpwEZhWY2khsDEz2qKh05tNjbNVhAzQCRJA@mail.gmail.com>
On 1/17/19 2:13 AM, Benjamin Tissoires wrote:
> On Wed, Jan 16, 2019 at 5:59 AM Kim Phillips <kim@kimphillips.com> wrote:
>>
>> On 1/15/19 2:57 AM, Benjamin Tissoires wrote:
>>> On Mon, Jan 14, 2019 at 7:40 PM Dmitry Torokhov
>>> <dmitry.torokhov@gmail.com> wrote:
>>>>
>>>> On Sat, Jan 12, 2019 at 04:04:36PM -0600, Kim Phillips wrote:
>>>>> On 1/11/19 7:40 PM, Dmitry Torokhov wrote:
>>>>>> Hi Kim,
>>>>>
>>>>> Hi Dmitry,
>>>>>
>>>>>> On Fri, Jan 11, 2019 at 02:54:30PM -0600, Kim Phillips wrote:
>>>>>>> This patch is the result of seeing this message:
>>>>>>>
>>>>>>> psmouse serio1: synaptics: Your touchpad (PNP: DLL087c PNP0f13) says it can support a different bus. If i2c-hid and hid-rmi are not used, you might want to try setting psmouse.synaptics_intertouch to 1 and report this to linux-input@vger.kernel.org.
>>>>>>>
>>>>>>> If I set psmouse.synaptics_intertouch=1, or add the PNP ID to
>>>>>>> smbus_pnp_ids, the touchpad continues to work, and the above message
>>>>>>> goes away, but we now get:
>>>>>>>
>>>>>>> psmouse serio1: synaptics: Trying to set up SMBus access
>>>>>>> psmouse serio1: synaptics: SMbus companion is not ready yet
>>>>>>>
>>>>>>> With this patch applied, i.e., the PNP IDs are added to the forcepad
>>>>>>> array, the touchpad continues to work and all of the above messages
>>>>>>> disappear.
>>>>>>
>>>>>> Are you sure the touchpad in XPSes is a forcepad (i.e. it does not have
>>>>>> physical button underneath it)? As far as I know there were only couple
>>>>>> of HP laptops with forcepads and when switching to RMI mode forcepads
>>>>>> need F21 handler that we do not currently have in the kernel.
>>>>>
>>>>> I see, no, I'm not sure, but assuming you're right, the IDs
>>>>> should be added to the smbus array instead, after fixing
>>>>> the SMbus "companion not ready" problem? Pointers for that and
>>>>> the below interrupts when touchpad idle after resume, welcome.
>>>>>
>>>>> Also, the link to get the RMI4 spec in
>>>>> Documentation/devicetree/bindings/input/rmi4/rmi_2d_sensor.txt
>>>>> is broken. Any pointers for that also appreciated.
>>>>
>>>> OK, sorting it all out some more:
>>>>
>>>> - because we do not have support for F21 necessary for forcepads adding
>>>> APIC ID to forcepad list actuallty disables SMbus companion mode, that
>>>> is why you no longer see "companion not ready" messages vs. setting
>>>> psmouse.synaptics_intertouch=1 module parameter.
>>>
>>> Yep
>>>
>>>>
>>>> - this does not really matter as your touchpad ends up being driven by
>>>> i2c-hid and hid-multitouch drivers, and that is how we wait it to
>>>> work, as we do not want to deviate from behavior on Windows since OEM
>>>> tested it (the device and firmware) in tha configuration.
>>>
>>> Yep too. The I2C-hid touchpads from Synaptics do not have the SMBus
>>> wired at all, so we can't enable SMBus for them. Also, the fact that
>>> the device gets loaded over i2c-hid means that we can't know that it
>>> is the case from the psmouse/synaptics point of view.
>>
>> Sigh, OK, I wasn't registering the word "not" when reading
>> "If i2c-hid and hid-rmi are not used" too quickly.
>>
>>>> - we need to figure out issue with interrupts on resume, maybe Benjamin
>>>> have seen it?
>>>
>>> First time I see it.
>>>
>>> I just tried on a XPS 9360 and kernel v4.18 (fedora) and nothing was a problem.
>>> I then tried on a XPS 9575 with v4.19, and here, the wacom I2C node is
>>> also keeping firing the interrupts, but not the touchpad. However,
>>> here, the interrupts are not stopping when I touch the touchscreen or
>>> if I approach a pen.
>>>
>>> Kim, rmmod-ing i2c-hid and modprobing back it with the parameter
>>> debug=1 doesn't show any events processed when the interrupts are
>>> firing. Do you see the same?
>>
>> After rmmodding i2c_hid, the WCOM488F and SYNA2393 entries in /proc/interrupts
>> somewhat expectedly disappear, however, the modprobe (with or without debug=1)
>> fails to bring them back, with these messages left in dmesg:
>>
>> [ 3882.073222] calling i2c_hid_driver_init+0x0/0x1000 [i2c_hid] @ 3082
>> [ 3882.180938] i2c_hid i2c-WCOM488F:00: HID over i2c has not been provided an Int IRQ
>> [ 3882.181060] i2c_hid: probe of i2c-WCOM488F:00 failed with error -22
>> [ 3882.181065] probe of i2c-WCOM488F:00 returned 0 after 496 usecs
>> [ 3882.289196] i2c_hid i2c-SYNA2393:00: HID over i2c has not been provided an Int IRQ
>> [ 3882.289318] i2c_hid: probe of i2c-SYNA2393:00 failed with error -22
>> [ 3882.289321] probe of i2c-SYNA2393:00 returned 0 after 508 usecs
>> [ 3882.289418] initcall i2c_hid_driver_init+0x0/0x1000 [i2c_hid] returned 0 after 211119 usecs
>
> Yes, that is a weird behavior I experience the other day also. It
> looks like rmmod-ing the driver simply kills the irq description
> attached to the i2c node. This waasn't the case previously, and I
> wonder if the issue is in i2c-hid (unlikely) or in i2c-core.
>
>>
>> In order to work around that problem, I set h2c_hid.debug=1 in the
>> boot command line, and after a resume, dmesg contains these messages,
>> relevant to i2c_hid:
>>
>> [ 267.235673] i2c_hid i2c-WCOM488F:00: calling i2c_hid_resume+0x0/0x140 [i2c_hid] @ 3078, parent: i2c-9
>> [ 267.235676] input input16: calling input_dev_resume+0x0/0x50 @ 3060, parent: card0
>> [ 267.235681] input input16: input_dev_resume+0x0/0x50 returned 0 after 0 usecs
>> [ 267.235682] i2c_hid i2c-WCOM488F:00: i2c_hid_set_power
>> [ 267.235687] input input17: calling input_dev_resume+0x0/0x50 @ 3060, parent: card0
>> [ 267.235689] i2c_hid i2c-WCOM488F:00: __i2c_hid_command: cmd=04 00 00 08
>> [ 267.235693] input input17: input_dev_resume+0x0/0x50 returned 0 after 0 usecs
>> [ 267.235698] idma64 idma64.1: calling platform_pm_resume+0x0/0x50 @ 3060, parent: 0000:00:15.1
>> [ 267.235701] idma64 idma64.1: platform_pm_resume+0x0/0x50 returned 0 after 0 usecs
>> [ 267.235706] i2c_designware i2c_designware.1: calling platform_pm_resume+0x0/0x50 @ 3060, parent: 0000:00:15.1
>> [ 267.235709] i2c_designware i2c_designware.1: platform_pm_resume+0x0/0x50 returned 0 after 0 usecs
>> [ 267.235718] rfkill rfkill0: calling rfkill_resume+0x0/0x60 @ 3060, parent: phy0
>> [ 267.235724] rfkill rfkill0: rfkill_resume+0x0/0x60 returned 0 after 3 usecs
>> [ 267.235768] i2c_hid i2c-SYNA2393:00: calling i2c_hid_resume+0x0/0x140 [i2c_hid] @ 3108, parent: i2c-10
>> [ 267.235774] i2c_hid i2c-SYNA2393:00: i2c_hid_set_power
>> [ 267.235776] i2c_hid i2c-SYNA2393:00: __i2c_hid_command: cmd=22 00 00 08
>> [ 267.236051] i2c_hid i2c-SYNA2393:00: i2c_hid_set_or_send_report
>> [ 267.236053] i2c_hid i2c-SYNA2393:00: __i2c_hid_command: cmd=22 00 3d 03 23 00 04 00 0d 00
>> [ 267.236076] i2c_hid i2c-WCOM488F:00: i2c_hid_set_or_send_report
>> [ 267.236080] i2c_hid i2c-WCOM488F:00: __i2c_hid_command: cmd=04 00 3b 03 05 00 42 00 0b 00 fd 89 a3 07 f3 02 20 8d 49 06 d7 9c ff ff 80 59 5c 13 d7 9c ff ff 08 00 00 08 d7 9c ff ff 01 00 00 00 00 00 00 00 16 00 00 00 66 69 6c 65 75 74 6c 2e 6d 65 73 73
>> [ 267.237691] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 3c 03 a8 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 60 01 00
>> [ 267.238137] i2c_hid i2c-SYNA2393:00: i2c_hid_set_or_send_report
>> [ 267.238140] i2c_hid i2c-SYNA2393:00: __i2c_hid_command: cmd=22 00 34 03 23 00 04 00 04 03
>> [ 267.238670] i2c_hid i2c-WCOM488F:00: i2c_hid_get_report
>> [ 267.238673] i2c_hid i2c-WCOM488F:00: __i2c_hid_command: cmd=04 00 3b 02 05 00
>> [ 267.239708] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 3c 03 a8 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 60 01 00
>> [ 267.240084] i2c_hid i2c-SYNA2393:00: i2c_hid_set_or_send_report
>> [ 267.240087] i2c_hid i2c-SYNA2393:00: __i2c_hid_command: cmd=22 00 36 03 23 00 04 00 06 03
>> [ 267.241377] i2c_hid i2c-WCOM488F:00: i2c_hid_set_or_send_report
>> [ 267.241380] i2c_hid i2c-WCOM488F:00: __i2c_hid_command: cmd=04 00 3b 03 05 00 42 00 0b 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> [ 267.241711] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 3c 03 a8 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 60 01 00
>> [ 267.243279] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 3c 03 a8 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 60 01 00
>> [ 267.243981] i2c_hid i2c-WCOM488F:00: i2c_hid_get_report
>> [ 267.243984] i2c_hid i2c-WCOM488F:00: __i2c_hid_command: cmd=04 00 3b 02 05 00
>> [ 267.244847] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 3c 03 a8 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 60 01 00
>> [ 267.245360] i2c_hid i2c-SYNA2393:00: i2c_hid_resume+0x0/0x140 [i2c_hid] returned 0 after 9360 usecs
>> [ 267.245455] input input27: calling input_dev_resume+0x0/0x50 @ 3060, parent: 0018:06CB:7A13.0002
>> [ 267.245460] input input27: input_dev_resume+0x0/0x50 returned 0 after 0 usecs
>> [ 267.246773] i2c_hid i2c-WCOM488F:00: i2c_hid_set_or_send_report
>> [ 267.246777] i2c_hid i2c-WCOM488F:00: __i2c_hid_command: cmd=04 00 3b 03 05 00 42 00 0b 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> [ 267.246919] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 3c 03 a8 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 60 01 00
>> [ 267.248517] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 3c 03 a8 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 60 01 00
>> [ 267.249383] i2c_hid i2c-WCOM488F:00: i2c_hid_get_report
>> [ 267.249386] i2c_hid i2c-WCOM488F:00: __i2c_hid_command: cmd=04 00 3b 02 05 00
>> [ 267.250116] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 3c 03 a8 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 60 01 00
>> [ 267.251767] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 3c 03 a8 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 60 01 00
>> [ 267.252137] i2c_hid i2c-WCOM488F:00: i2c_hid_set_or_send_report
>> [ 267.252140] i2c_hid i2c-WCOM488F:00: __i2c_hid_command: cmd=04 00 3b 03 05 00 42 00 0b 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> [ 267.253440] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 3c 03 a8 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 60 01 00
>> [ 267.254734] i2c_hid i2c-WCOM488F:00: i2c_hid_get_report
>> [ 267.254737] i2c_hid i2c-WCOM488F:00: __i2c_hid_command: cmd=04 00 3b 02 05 00
>> [ 267.255015] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 3c 03 a8 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 60 01 00
>> [ 267.256612] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 3c 03 a8 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 60 01 00
>> [ 267.257475] i2c_hid i2c-WCOM488F:00: i2c_hid_set_or_send_report
>> [ 267.257478] i2c_hid i2c-WCOM488F:00: __i2c_hid_command: cmd=04 00 3b 03 05 00 42 00 0b 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> [ 267.258275] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 3c 03 a8 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 60 01 00
>> [ 267.259852] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 3c 03 a8 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 60 01 00
>> [ 267.260046] i2c_hid i2c-WCOM488F:00: i2c_hid_get_report
>> [ 267.260049] i2c_hid i2c-WCOM488F:00: __i2c_hid_command: cmd=04 00 3b 02 05 00
>> [ 267.261441] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 3c 03 a8 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 60 01 00
>> [ 267.262789] i2c_hid i2c-WCOM488F:00: i2c_hid_set_or_send_report
>> [ 267.262792] i2c_hid i2c-WCOM488F:00: __i2c_hid_command: cmd=04 00 3b 03 05 00 42 00 0b 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> [ 267.263129] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 3c 03 a8 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 60 01 00
>> [ 267.264718] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 3c 03 a8 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 60 01 00
>> [ 267.265410] i2c_hid i2c-WCOM488F:00: i2c_hid_get_report
>> [ 267.265413] i2c_hid i2c-WCOM488F:00: __i2c_hid_command: cmd=04 00 3b 02 05 00
>> [ 267.266318] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 3c 03 a8 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 60 01 00
>> [ 267.267971] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 3c 03 a8 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 60 01 00
>> [ 267.268169] i2c_hid i2c-WCOM488F:00: i2c_hid_set_or_send_report
>> [ 267.268172] i2c_hid i2c-WCOM488F:00: __i2c_hid_command: cmd=04 00 3e 03 05 00 05 00 0e 02 00
>> [ 267.268728] i2c_hid i2c-WCOM488F:00: i2c_hid_resume+0x0/0x140 [i2c_hid] returned 0 after 32273 usecs
>> [ 267.269575] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 3c 03 a8 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 60 01 00
>> [ 267.271144] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 3c 03 a8 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 60 01 00
>> [ 267.272719] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 3c 03 a8 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 60 01 00
>> [ 267.274298] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 3c 03 a8 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 60 01 00
>>
>> ...and it goes on and on, where that same last line gets
>> repeated as the interrupts fire, until I start using the touchpad,
>> thereby stopping the interrupts.
>
> It looks like valid inputs. So I would expect the firmware to unset
> the IRQ line, but it doesn't do it.
>
> On an other thread, it has been brought to my attention that maybe we
> are not 100% compatible with the Windows driver regarding
> sleep/power_on commands. We expect the device to be ready while the
> Windows driver waits a little before sending a command. Maybe we can
> try to see if adding I2C_HID_QUIRK_DELAY_AFTER_SLEEP to this device
> changes something.
Thought I'd try DELAY_AFTER_SLEEP. Please double check whether
I did that correctly given these diagnostics:
[ 5.720289] input: SYNA2393:00 06CB:7A13 Touchpad as /devices/pci0000:00/0000:00:15.1/i2c_designware.1/i2c-10/i2c-SYNA2393:00/0018:06CB:7A13.0006/input/input20
[ 5.720362] hid-multitouch 0018:06CB:7A13.0006: input,hidraw5: I2C HID v1.00 Mouse [SYNA2393:00 06CB:7A13] on i2c-SYNA2393:00
I added both 2393 and 7A13 as model numbers just
because I wasn't sure which one was which:
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 27519eb8ee63..d297ad6744e2 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -1072,6 +1072,9 @@
#define USB_DEVICE_ID_SYNAPTICS_HD 0x0ac3
#define USB_DEVICE_ID_SYNAPTICS_QUAD_HD 0x1ac3
#define USB_DEVICE_ID_SYNAPTICS_TP_V103 0x5710
+#define I2C_VENDOR_ID_SYNAPTICS 0x06cb
+#define I2C_PRODUCT_ID_SYNAPTICS_2393 0x2393
+#define I2C_PRODUCT_ID_SYNAPTICS_7A13 0x7a13
#define USB_VENDOR_ID_TEXAS_INSTRUMENTS 0x2047
#define USB_DEVICE_ID_TEXAS_INSTRUMENTS_LENOVO_YOGA 0x0855
diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
index 8555ce7e737b..cfdd5f2e4781 100644
--- a/drivers/hid/i2c-hid/i2c-hid-core.c
+++ b/drivers/hid/i2c-hid/i2c-hid-core.c
@@ -179,6 +179,10 @@ 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 },
+ { I2C_VENDOR_ID_SYNAPTICS, I2C_PRODUCT_ID_SYNAPTICS_2393,
+ I2C_HID_QUIRK_DELAY_AFTER_SLEEP },
+ { I2C_VENDOR_ID_SYNAPTICS, I2C_PRODUCT_ID_SYNAPTICS_7A13,
+ I2C_HID_QUIRK_DELAY_AFTER_SLEEP },
{ 0, 0 }
};
@@ -419,14 +423,19 @@ static int i2c_hid_set_power(struct i2c_client *client, int power_state)
delay = jiffies_to_usecs(ihid->sleep_delay - now);
usleep_range(delay, delay + 1);
}
- }
+ dev_err(&client->dev, "%s %d: delayed after sleep with power on\n", __func__, __LINE__);
+ } else
+ dev_err(&client->dev, "%s %d: did NOT delay after sleep whilst powered on. No quirk?\n", __func__, __LINE__);
ret = __i2c_hid_command(client, &hid_set_power_cmd, power_state,
0, NULL, 0, NULL, 0);
if (ihid->quirks & I2C_HID_QUIRK_DELAY_AFTER_SLEEP &&
- power_state == I2C_HID_PWR_SLEEP)
+ power_state == I2C_HID_PWR_SLEEP) {
ihid->sleep_delay = jiffies + msecs_to_jiffies(20);
+ dev_err(&client->dev, "%s %d: delayed after sleep with power set to sleep\n", __func__, __LINE__);
+ } else
+ dev_err(&client->dev, "%s %d: did NOT delay after sleep at sleep. No quirk?\n", __func__, __LINE__);
if (ret)
dev_err(&client->dev, "failed to change power setting.\n");
This is as I give the suspend command, using the touchpad to
click on the pause button in Gnome (whilst holding down ALT):
[ 319.326944] i2c_hid i2c-SYNA2393:00: input: 20 00 03 03 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 e2 c4 01 00
[ 319.334357] i2c_hid i2c-SYNA2393:00: input: 20 00 03 03 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 2b c5 01 00
[ 319.341692] i2c_hid i2c-SYNA2393:00: input: 20 00 03 03 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 74 c5 01 00
[ 319.348994] i2c_hid i2c-SYNA2393:00: input: 20 00 03 03 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 bd c5 01 00
[ 319.355556] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 c6 01 00
...
[ 349.858808] PM: suspend entry (deep)
[ 349.858810] PM: Syncing filesystems ... done.
...
[ 349.985145] i2c_hid i2c-SYNA2393:00: calling acpi_subsys_suspend+0x0/0x60 @ 7, parent: i2c-10
[ 349.985147] i2c_hid i2c-SYNA2393:00: i2c_hid_set_power
[ 349.985149] i2c_hid i2c-SYNA2393:00: i2c_hid_set_power 428: did NOT delay after sleep whilst powered on. No quirk?
[ 349.985187] i2c_hid i2c-SYNA2393:00: __i2c_hid_command: cmd=22 00 01 08
[ 349.985555] i2c_hid i2c-SYNA2393:00: i2c_hid_set_power 436: delayed after sleep with power set to sleep
[ 349.985591] i2c_hid i2c-SYNA2393:00: acpi_subsys_suspend+0x0/0x60 returned 0 after 431 usecs
[ 349.985636] input input17: calling input_dev_suspend+0x0/0x60 @ 12762, parent: card0
[ 349.985639] input input17: input_dev_suspend+0x0/0x60 returned 0 after 0 usecs
[ 349.985642] input input16: calling input_dev_suspend+0x0/0x60 @ 12762, parent: card0
[ 349.985645] input input16: input_dev_suspend+0x0/0x60 returned 0 after 0 usecs
[ 349.985648] input input15: calling input_dev_suspend+0x0/0x60 @ 12762, parent: card0
[ 349.985650] input input15: input_dev_suspend+0x0/0x60 returned 0 after 0 usecs
[ 349.985653] input input14: calling input_dev_suspend+0x0/0x60 @ 12762, parent: card0
[ 349.985656] input input14: input_dev_suspend+0x0/0x60 returned 0 after 0 usecs
[ 349.985659] input input13: calling input_dev_suspend+0x0/0x60 @ 12762, parent: card0
[ 349.985661] input input13: input_dev_suspend+0x0/0x60 returned 0 after 0 usecs
[ 349.985664] input input12: calling input_dev_suspend+0x0/0x60 @ 12762, parent: card0
[ 349.985667] input input12: input_dev_suspend+0x0/0x60 returned 0 after 0 usecs
[ 349.985678] i2c_designware i2c_designware.1: calling platform_pm_suspend+0x0/0x50 @ 12762, parent: 0000:00:15.1
[ 349.985680] i2c_designware i2c_designware.1: platform_pm_suspend+0x0/0x50 returned 0 after 0 usecs
[ 349.985684] idma64 idma64.1: calling platform_pm_suspend+0x0/0x50 @ 12762, parent: 0000:00:15.1
[ 349.985695] idma64 idma64.1: platform_pm_suspend+0x0/0x50 returned 0 after 9 usecs
[ 349.985698] leds dell::kbd_backlight: calling led_suspend+0x0/0x30 @ 12762, parent: dell-laptop
[ 349.985700] leds dell::kbd_backlight: led_suspend+0x0/0x30 returned 0 after 0 usecs
[ 349.985711] coretemp coretemp.0: calling platform_pm_suspend+0x0/0x50 @ 12762, parent: platform
[ 349.985713] coretemp coretemp.0: platform_pm_suspend+0x0/0x50 returned 0 after 0 usecs
[ 349.985717] snd_hda_codec_hdmi hdaudioC0D2: calling pm_runtime_force_suspend+0x0/0xe0 @ 7, parent: 0000:00:1f.3
[ 349.985720] snd_hda_codec_hdmi hdaudioC0D2: pm_runtime_force_suspend+0x0/0xe0 returned 0 after 0 usecs
[ 349.985727] i2c_hid i2c-WCOM488F:00: calling acpi_subsys_suspend+0x0/0x60 @ 7, parent: i2c-9
[ 349.985729] i2c_hid i2c-WCOM488F:00: i2c_hid_set_power
[ 349.985730] i2c_hid i2c-WCOM488F:00: i2c_hid_set_power 428: did NOT delay after sleep whilst powered on. No quirk?
[ 349.985751] snd_hda_codec_realtek hdaudioC0D0: calling pm_runtime_force_suspend+0x0/0xe0 @ 311, parent: 0000:00:1f.3
[ 349.985765] i2c_hid i2c-WCOM488F:00: __i2c_hid_command: cmd=04 00 01 08
[ 349.986162] i2c_hid i2c-WCOM488F:00: i2c_hid_set_power 438: did NOT delay after sleep at sleep. No quirk?
[ 349.986199] i2c_hid i2c-WCOM488F:00: acpi_subsys_suspend+0x0/0x60 returned 0 after 457 usecs
[ 349.986250] input input11: calling input_dev_suspend+0x0/0x60 @ 12762, parent: 9DBB5994-A997-11DA-B012-B622A1EF5492
[ 349.986253] input input11: input_dev_suspend+0x0/0x60 returned 0 after 0 usecs
[ 349.986257] platform regulatory.0: calling platform_pm_suspend+0x0/0x50 @ 12762, parent: platform
[ 349.986259] platform regulatory.0: platform_pm_suspend+0x0/0x50 returned 0 after 0 usecs
[ 349.986262] dell-laptop dell-laptop: calling platform_pm_suspend+0x0/0x50 @ 12762, parent: platform
[ 349.986264] dell-laptop dell-laptop: platform_pm_suspend+0x0/0x50 returned 0 after 0 usecs
[ 349.986267] dell-smbios dell-smbios.1: calling platform_pm_suspend+0x0/0x50 @ 12762, parent: platform
[ 349.986269] dell-smbios dell-smbios.1: platform_pm_suspend+0x0/0x50 returned 0 after 0 usecs
[ 349.986272] dell-smbios dell-smbios.0: calling platform_pm_suspend+0x0/0x50 @ 12762, parent: platform
[ 349.986274] dell-smbios dell-smbios.0: platform_pm_suspend+0x0/0x50 returned 0 after 0 usecs
[ 349.986280] dcdbas dcdbas: calling platform_pm_suspend+0x0/0x50 @ 12762, parent: platform
[ 349.986282] dcdbas dcdbas: platform_pm_suspend+0x0/0x50 returned 0 after 0 usecs
[ 349.986293] leds input4::scrolllock: calling led_suspend+0x0/0x30 @ 12762, parent: input4
[ 349.986295] leds input4::scrolllock: led_suspend+0x0/0x30 returned 0 after 0 usecs
[ 349.986297] leds input4::capslock: calling led_suspend+0x0/0x30 @ 12762, parent: input4
[ 349.986299] leds input4::capslock: led_suspend+0x0/0x30 returned 0 after 0 usecs
[ 349.986301] leds input4::numlock: calling led_suspend+0x0/0x30 @ 12762, parent: input4
[ 349.986303] leds input4::numlock: led_suspend+0x0/0x30 returned 0 after 0 usecs
[ 349.986306] i2c_designware i2c_designware.0: calling platform_pm_suspend+0x0/0x50 @ 12762, parent: 0000:00:15.0
[ 349.986308] i2c_designware i2c_designware.0: platform_pm_suspend+0x0/0x50 returned 0 after 0 usecs
[ 349.986312] idma64 idma64.0: calling platform_pm_suspend+0x0/0x50 @ 12762, parent: 0000:00:15.0
[ 349.986322] idma64 idma64.0: platform_pm_suspend+0x0/0x50 returned 0 after 8 usecs
[ 349.986329] input input10: calling input_dev_suspend+0x0/0x60 @ 12762, parent: INT33D5:00
[ 349.986332] input input10: input_dev_suspend+0x0/0x60 returned 0 after 0 usecs
[ 349.986335] input input9: calling input_dev_suspend+0x0/0x60 @ 12762, parent: INT33D5:00
[ 349.986337] input input9: input_dev_suspend+0x0/0x60 returned 0 after 0 usecs
[ 349.986347] input input6: calling input_dev_suspend+0x0/0x60 @ 12762, parent: serio1
[ 349.986349] input input6: input_dev_suspend+0x0/0x60 returned 0 after 0 usecs
[ 349.986357] input input8: calling input_dev_suspend+0x0/0x60 @ 12762, parent: LNXVIDEO:01
[ 349.986360] input input8: input_dev_suspend+0x0/0x60 returned 0 after 0 usecs
[ 349.986363] input input7: calling input_dev_suspend+0x0/0x60 @ 12762, parent: LNXVIDEO:00
[ 349.986365] input input7: input_dev_suspend+0x0/0x60 returned 0 after 0 usecs
...
[ 349.987136] psmouse serio1: calling serio_suspend+0x0/0x20 @ 12762, parent: i8042
...
[ 351.598355] i2c_hid i2c-SYNA2393:00: calling acpi_subsys_suspend_late+0x0/0x60 @ 12784, parent: i2c-10
[ 351.598366] i2c_hid i2c-SYNA2393:00: acpi_subsys_suspend_late+0x0/0x60 returned 0 after 1 usecs
[ 351.598465] i2c_designware i2c_designware.1: calling dw_i2c_plat_suspend+0x0/0x40 @ 12762, parent: 0000:00:15.1
[ 351.598487] i2c_designware i2c_designware.1: dw_i2c_plat_suspend+0x0/0x40 returned 0 after 13 usecs
[ 351.598529] i2c_hid i2c-WCOM488F:00: calling acpi_subsys_suspend_late+0x0/0x60 @ 87, parent: i2c-9
[ 351.598540] i2c_hid i2c-WCOM488F:00: acpi_subsys_suspend_late+0x0/0x60 returned 0 after 1 usecs
[ 351.598609] i2c_designware i2c_designware.0: calling dw_i2c_plat_suspend+0x0/0x40 @ 12762, parent: 0000:00:15.0
[ 351.598628] i2c_designware i2c_designware.0: dw_i2c_plat_suspend+0x0/0x40 returned 0 after 12 usecs
...
[ 351.633232] i2c_hid i2c-SYNA2393:00: calling acpi_subsys_suspend_noirq+0x0/0x70 @ 12784, parent: i2c-10
[ 351.633235] i2c_hid i2c-SYNA2393:00: acpi_subsys_suspend_noirq+0x0/0x70 returned 0 after 0 usecs
[ 351.633258] i2c_hid i2c-WCOM488F:00: calling acpi_subsys_suspend_noirq+0x0/0x70 @ 12783, parent: i2c-9
[ 351.633261] i2c_hid i2c-WCOM488F:00: acpi_subsys_suspend_noirq+0x0/0x70 returned 0 after 0 usecs
Here is the relevant portion of the resume cycle,
which is similar to the one I posted earlier:
[ 353.256580] leds dell::kbd_backlight: led_resume+0x0/0x30 returned 0 after 0 usecs
[ 353.256585] idma64 idma64.1: calling platform_pm_resume+0x0/0x50 @ 12762, parent: 0000:00:15.1
[ 353.256589] idma64 idma64.1: platform_pm_resume+0x0/0x50 returned 0 after 0 usecs
[ 353.256594] i2c_designware i2c_designware.1: calling platform_pm_resume+0x0/0x50 @ 12762, parent: 0000:00:15.1
[ 353.256597] i2c_designware i2c_designware.1: platform_pm_resume+0x0/0x50 returned 0 after 0 usecs
[ 353.256606] input input12: calling input_dev_resume+0x0/0x50 @ 12762, parent: card0
[ 353.256611] input input12: input_dev_resume+0x0/0x50 returned 0 after 0 usecs
[ 353.256616] input input13: calling input_dev_resume+0x0/0x50 @ 12762, parent: card0
[ 353.256625] i2c_hid i2c-WCOM488F:00: calling i2c_hid_resume+0x0/0x140 [i2c_hid] @ 12809, parent: i2c-9
[ 353.256630] input input13: input_dev_resume+0x0/0x50 returned 0 after 0 usecs
[ 353.256632] i2c_hid i2c-WCOM488F:00: i2c_hid_set_power
[ 353.256635] i2c_hid i2c-WCOM488F:00: i2c_hid_set_power 428: did NOT delay after sleep whilst powered on. No quirk?
[ 353.256639] input input14: calling input_dev_resume+0x0/0x50 @ 12762, parent: card0
[ 353.256645] input input14: input_dev_resume+0x0/0x50 returned 0 after 0 usecs
[ 353.256708] i2c_hid i2c-WCOM488F:00: __i2c_hid_command: cmd=04 00 00 08
[ 353.256712] input input15: calling input_dev_resume+0x0/0x50 @ 12762, parent: card0
[ 353.256717] input input15: input_dev_resume+0x0/0x50 returned 0 after 0 usecs
[ 353.256722] input input16: calling input_dev_resume+0x0/0x50 @ 12762, parent: card0
[ 353.256727] input input16: input_dev_resume+0x0/0x50 returned 0 after 0 usecs
[ 353.256732] input input17: calling input_dev_resume+0x0/0x50 @ 12762, parent: card0
[ 353.256736] input input17: input_dev_resume+0x0/0x50 returned 0 after 0 usecs
[ 353.256759] i2c_hid i2c-SYNA2393:00: calling i2c_hid_resume+0x0/0x140 [i2c_hid] @ 12791, parent: i2c-10
[ 353.256767] i2c_hid i2c-SYNA2393:00: i2c_hid_set_power
[ 353.256771] i2c_hid i2c-SYNA2393:00: i2c_hid_set_power 426: delayed after sleep with power on
[ 353.256833] i2c_hid i2c-SYNA2393:00: __i2c_hid_command: cmd=22 00 00 08
[ 353.257109] i2c_hid i2c-WCOM488F:00: i2c_hid_set_power 438: did NOT delay after sleep at sleep. No quirk?
[ 353.257135] i2c_hid i2c-SYNA2393:00: i2c_hid_set_power 438: did NOT delay after sleep at sleep. No quirk?
[ 353.257176] i2c_hid i2c-WCOM488F:00: i2c_hid_set_or_send_report
[ 353.257233] i2c_hid i2c-WCOM488F:00: __i2c_hid_command: cmd=04 00 3b 03 05 00 42 00 0b 00 1c 1c 15 40 25 ac 00 10 ba 13 17 be ff ff 02 00 00 00 00 00 00 00 38 07 00 7e a3 94 ff ff 18 0f 00 7e a3 94 ff ff d8 29 ca 75 a3 94 ff ff b0 07 00 7e a3 94 ff ff
[ 353.257240] i2c_hid i2c-SYNA2393:00: i2c_hid_set_or_send_report
[ 353.257244] i2c_hid i2c-SYNA2393:00: __i2c_hid_command: cmd=22 00 3d 03 23 00 04 00 0d 00
[ 353.258737] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 c6 01 00
[ 353.259889] i2c_hid i2c-WCOM488F:00: i2c_hid_get_report
[ 353.259892] i2c_hid i2c-WCOM488F:00: __i2c_hid_command: cmd=04 00 3b 02 05 00
[ 353.260345] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 c6 01 00
[ 353.261936] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 c6 01 00
[ 353.262416] i2c_hid i2c-SYNA2393:00: i2c_hid_set_or_send_report
[ 353.262420] i2c_hid i2c-SYNA2393:00: __i2c_hid_command: cmd=22 00 34 03 23 00 04 00 04 03
[ 353.262646] i2c_hid i2c-WCOM488F:00: i2c_hid_set_or_send_report
[ 353.262650] i2c_hid i2c-WCOM488F:00: __i2c_hid_command: cmd=04 00 3b 03 05 00 42 00 0b 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 353.264051] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 c6 01 00
[ 353.265306] i2c_hid i2c-WCOM488F:00: i2c_hid_get_report
[ 353.265309] i2c_hid i2c-WCOM488F:00: __i2c_hid_command: cmd=04 00 3b 02 05 00
[ 353.265631] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 c6 01 00
[ 353.267232] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 c6 01 00
[ 353.267705] i2c_hid i2c-SYNA2393:00: i2c_hid_set_or_send_report
[ 353.267708] i2c_hid i2c-SYNA2393:00: __i2c_hid_command: cmd=22 00 36 03 23 00 04 00 06 03
[ 353.268076] i2c_hid i2c-WCOM488F:00: i2c_hid_set_or_send_report
[ 353.268080] i2c_hid i2c-WCOM488F:00: __i2c_hid_command: cmd=04 00 3b 03 05 00 42 00 0b 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 353.269304] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 c6 01 00
[ 353.270683] i2c_hid i2c-WCOM488F:00: i2c_hid_get_report
[ 353.270687] i2c_hid i2c-WCOM488F:00: __i2c_hid_command: cmd=04 00 3b 02 05 00
[ 353.270877] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 c6 01 00
[ 353.272471] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 c6 01 00
[ 353.272983] i2c_hid i2c-SYNA2393:00: i2c_hid_resume+0x0/0x140 [i2c_hid] returned 0 after 15834 usecs
[ 353.273091] input input27: calling input_dev_resume+0x0/0x50 @ 12762, parent: 0018:06CB:7A13.0002
[ 353.273096] input input27: input_dev_resume+0x0/0x50 returned 0 after 0 usecs
[ 353.273104] rfkill rfkill0: calling rfkill_resume+0x0/0x60 @ 12762, parent: phy0
[ 353.273112] rfkill rfkill0: rfkill_resume+0x0/0x60 returned 0 after 4 usecs
[ 353.273121] input input29: calling input_dev_resume+0x0/0x50 @ 12762, parent: 1-12:1.0
[ 353.273125] input input29: input_dev_resume+0x0/0x50 returned 0 after 0 usecs
[ 353.273130] rfkill rfkill1: calling rfkill_resume+0x0/0x60 @ 12762, parent: hci0
[ 353.273134] rfkill rfkill1: rfkill_resume+0x0/0x60 returned 0 after 0 usecs
[ 353.273436] i2c_hid i2c-WCOM488F:00: i2c_hid_set_or_send_report
[ 353.273439] i2c_hid i2c-WCOM488F:00: __i2c_hid_command: cmd=04 00 3b 03 05 00 42 00 0b 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 353.274593] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 c6 01 00
[ 353.276065] i2c_hid i2c-WCOM488F:00: i2c_hid_get_report
[ 353.276069] i2c_hid i2c-WCOM488F:00: __i2c_hid_command: cmd=04 00 3b 02 05 00
[ 353.276212] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 c6 01 00
[ 353.277809] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 c6 01 00
[ 353.278839] i2c_hid i2c-WCOM488F:00: i2c_hid_set_or_send_report
[ 353.278843] i2c_hid i2c-WCOM488F:00: __i2c_hid_command: cmd=04 00 3b 03 05 00 42 00 0b 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00: input: 20 00 03 01 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 c6 01 00
[ 353.281024] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 c6 01 00
[ 353.281449] i2c_hid i2c-WCOM488F:00: i2c_hid_get_report
[ 353.281453] i2c_hid i2c-WCOM488F:00: __i2c_hid_command: cmd=04 00 3b 02 05 00
[ 353.282614] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 c6 01 00
[ 353.284248] i2c_hid i2c-WCOM488F:00: i2c_hid_set_or_send_report
[ 353.284251] i2c_hid i2c-WCOM488F:00: __i2c_hid_command: cmd=04 00 3b 03 05 00 42 00 0b 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 353.284326] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 c6 01 00
[ 353.285890] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 c6 01 00
[ 353.286847] i2c_hid i2c-WCOM488F:00: i2c_hid_get_report
[ 353.286850] i2c_hid i2c-WCOM488F:00: __i2c_hid_command: cmd=04 00 3b 02 05 00
[ 353.287489] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 c6 01 00
[ 353.289099] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 c6 01 00
[ 353.289595] i2c_hid i2c-WCOM488F:00: i2c_hid_set_or_send_report
[ 353.289598] i2c_hid i2c-WCOM488F:00: __i2c_hid_command: cmd=04 00 3e 03 05 00 05 00 0e 02 00
[ 353.290178] i2c_hid i2c-WCOM488F:00: i2c_hid_resume+0x0/0x140 [i2c_hid] returned 0 after 32758 usecs
[ 353.290275] input input30: calling input_dev_resume+0x0/0x50 @ 12762, parent: 0018:056A:488F.0001
[ 353.290281] input input30: input_dev_resume+0x0/0x50 returned 0 after 0 usecs
[ 353.290287] input input31: calling input_dev_resume+0x0/0x50 @ 12762, parent: 0018:056A:488F.0001
[ 353.290292] input input31: input_dev_resume+0x0/0x50 returned 0 after 0 usecs
[ 353.290804] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 c6 01 00
[ 353.291066] acpi LNXPOWER:07: Turning OFF
[ 353.292115] acpi LNXPOWER:02: Turning OFF
[ 353.292403] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 c6 01 00
[ 353.293924] acpi LNXPOWER:01: Turning OFF
[ 353.294004] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 c6 01 00
[ 353.295015] OOM killer enabled.
[ 353.295017] Restarting tasks ...
[ 353.295617] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 c6 01 00
[ 353.297197] done.
[ 353.297238] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 c6 01 00
[ 353.298871] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 c6 01 00
[ 353.300494] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 c6 01 00
[ 353.302107] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 c6 01 00
[ 353.303732] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 c6 01 00
[ 353.305342] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 c6 01 00
[ 353.306955] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 c6 01 00
[ 353.308955] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 c6 01 00
...and so on until I touch the touchpad...
Note that, as without DELAY_AFTER_SLEEP, the last repeated
input values above after resume equal those in the last input
prior to suspend, so it's as if the touchpad's last interrupt
didn't get handled/acknowledged, and the contents of buffer
survived the suspend-resume cycle? I can't really tell what
those values mean - asked Dell for the manual for the touchpad
and got told they don't support Linux :(
Kim
^ permalink raw reply related
* [PATCH] input: keyboard: gpio-keys-polled: use input name from pdata if available
From: Enrico Weigelt, metux IT consult @ 2019-01-22 12:00 UTC (permalink / raw)
To: juhosg, nunojpg, linux-input, linux-gpio, linux-kernel
Instead of hardcoding the input name to the driver name ('gpio-keys-polled'),
allow the passing a name via platform data ('name' field was already present),
but default to old behaviour in case of NULL.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
drivers/input/keyboard/gpio_keys_polled.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/keyboard/gpio_keys_polled.c b/drivers/input/keyboard/gpio_keys_polled.c
index edc7262..3312186 100644
--- a/drivers/input/keyboard/gpio_keys_polled.c
+++ b/drivers/input/keyboard/gpio_keys_polled.c
@@ -272,7 +272,7 @@ static int gpio_keys_polled_probe(struct platform_device *pdev)
input = poll_dev->input;
- input->name = pdev->name;
+ input->name = (pdata->name ? pdata->name : pdev->name);
input->phys = DRV_NAME"/input0";
input->id.bustype = BUS_HOST;
--
1.9.1
^ permalink raw reply related
* Re: [PATCH v1] HID: intel-ish-hid: Switch to use new generic UUID API
From: Srinivas Pandruvada @ 2019-01-22 17:29 UTC (permalink / raw)
To: Andy Shevchenko, linux-input, Even Xu, linux-kernel,
Christoph Hellwig
In-Reply-To: <20190110154804.89298-1-andriy.shevchenko@linux.intel.com>
On Thu, 2019-01-10 at 17:48 +0200, Andy Shevchenko wrote:
> There are new types and helpers that are supposed to be used in new
> code.
>
> As a preparation to get rid of legacy types and API functions do
> the conversion here.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> ---
> drivers/hid/intel-ish-hid/ishtp-hid-client.c | 3 +--
> drivers/hid/intel-ish-hid/ishtp-hid.h | 6 +++---
> drivers/hid/intel-ish-hid/ishtp/bus.c | 19 ++++++++--------
> ---
> drivers/hid/intel-ish-hid/ishtp/bus.h | 4 ++--
> drivers/hid/intel-ish-hid/ishtp/client.h | 2 +-
> drivers/hid/intel-ish-hid/ishtp/hbm.h | 2 +-
> 6 files changed, 16 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/hid/intel-ish-hid/ishtp-hid-client.c
> b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
> index e64243bc9c96..2246697ada1d 100644
> --- a/drivers/hid/intel-ish-hid/ishtp-hid-client.c
> +++ b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
> @@ -788,8 +788,7 @@ static int hid_ishtp_cl_probe(struct
> ishtp_cl_device *cl_device)
> if (!cl_device)
> return -ENODEV;
>
> - if (uuid_le_cmp(hid_ishtp_guid,
> - cl_device->fw_client->props.protocol_name) !=
> 0)
> + if (!guid_equal(&hid_ishtp_guid, &cl_device->fw_client-
> >props.protocol_name))
> return -ENODEV;
>
> client_data = devm_kzalloc(&cl_device->dev,
> sizeof(*client_data),
> diff --git a/drivers/hid/intel-ish-hid/ishtp-hid.h
> b/drivers/hid/intel-ish-hid/ishtp-hid.h
> index f5c7eb79b7b5..1cd07a441cd4 100644
> --- a/drivers/hid/intel-ish-hid/ishtp-hid.h
> +++ b/drivers/hid/intel-ish-hid/ishtp-hid.h
> @@ -29,9 +29,9 @@
> client->cl_device->ishtp_dev, __VA_ARGS__)
>
> /* ISH Transport protocol (ISHTP in short) GUID */
> -static const uuid_le hid_ishtp_guid = UUID_LE(0x33AECD58, 0xB679,
> 0x4E54,
> - 0x9B, 0xD9, 0xA0, 0x4D,
> 0x34,
> - 0xF0, 0xC2, 0x26);
> +static const guid_t hid_ishtp_guid =
> + GUID_INIT(0x33AECD58, 0xB679, 0x4E54,
> + 0x9B, 0xD9, 0xA0, 0x4D, 0x34, 0xF0, 0xC2, 0x26);
>
> /* ISH HID message structure */
> struct hostif_msg_hdr {
> diff --git a/drivers/hid/intel-ish-hid/ishtp/bus.c
> b/drivers/hid/intel-ish-hid/ishtp/bus.c
> index 728dc6d4561a..1b6092b300fe 100644
> --- a/drivers/hid/intel-ish-hid/ishtp/bus.c
> +++ b/drivers/hid/intel-ish-hid/ishtp/bus.c
> @@ -133,18 +133,15 @@ int ishtp_write_message(struct ishtp_device
> *dev, struct ishtp_msg_hdr *hdr,
> *
> * Return: fw client index or -ENOENT if not found
> */
> -int ishtp_fw_cl_by_uuid(struct ishtp_device *dev, const uuid_le
> *uuid)
> +int ishtp_fw_cl_by_uuid(struct ishtp_device *dev, const guid_t
> *uuid)
> {
> - int i, res = -ENOENT;
> + unsigned int i;
>
> for (i = 0; i < dev->fw_clients_num; ++i) {
> - if (uuid_le_cmp(*uuid, dev-
> >fw_clients[i].props.protocol_name)
> - == 0) {
> - res = i;
> - break;
> - }
> + if (guid_equal(uuid, &dev-
> >fw_clients[i].props.protocol_name))
> + return i;
> }
> - return res;
> + return -ENOENT;
> }
> EXPORT_SYMBOL(ishtp_fw_cl_by_uuid);
>
> @@ -158,7 +155,7 @@ EXPORT_SYMBOL(ishtp_fw_cl_by_uuid);
> * Return: pointer of client information on success, NULL on
> failure.
> */
> struct ishtp_fw_client *ishtp_fw_cl_get_client(struct ishtp_device
> *dev,
> - const uuid_le *uuid)
> + const guid_t *uuid)
> {
> int i;
> unsigned long flags;
> @@ -401,7 +398,7 @@ static const struct device_type
> ishtp_cl_device_type = {
> * Return: ishtp_cl_device pointer or NULL on failure
> */
> static struct ishtp_cl_device *ishtp_bus_add_device(struct
> ishtp_device *dev,
> - uuid_le uuid, char
> *name)
> + guid_t uuid, char
> *name)
> {
> struct ishtp_cl_device *device;
> int status;
> @@ -629,7 +626,7 @@ int ishtp_bus_new_client(struct ishtp_device
> *dev)
> int i;
> char *dev_name;
> struct ishtp_cl_device *cl_device;
> - uuid_le device_uuid;
> + guid_t device_uuid;
>
> /*
> * For all reported clients, create an unconnected client and
> add its
> diff --git a/drivers/hid/intel-ish-hid/ishtp/bus.h
> b/drivers/hid/intel-ish-hid/ishtp/bus.h
> index b8a5bcc82536..babf19ba3ff6 100644
> --- a/drivers/hid/intel-ish-hid/ishtp/bus.h
> +++ b/drivers/hid/intel-ish-hid/ishtp/bus.h
> @@ -112,8 +112,8 @@ void ishtp_cl_driver_unregister(struct
> ishtp_cl_driver *driver);
>
> int ishtp_register_event_cb(struct ishtp_cl_device *device,
> void (*read_cb)(struct ishtp_cl_device
> *));
> -int ishtp_fw_cl_by_uuid(struct ishtp_device *dev, const uuid_le
> *cuuid);
> +int ishtp_fw_cl_by_uuid(struct ishtp_device *dev, const guid_t
> *cuuid);
> struct ishtp_fw_client *ishtp_fw_cl_get_client(struct
> ishtp_device *dev,
> - const uuid_le *uuid);
> + const guid_t *uuid);
>
> #endif /* _LINUX_ISHTP_CL_BUS_H */
> diff --git a/drivers/hid/intel-ish-hid/ishtp/client.h
> b/drivers/hid/intel-ish-hid/ishtp/client.h
> index 042f4c4853b1..e0df3eb611e6 100644
> --- a/drivers/hid/intel-ish-hid/ishtp/client.h
> +++ b/drivers/hid/intel-ish-hid/ishtp/client.h
> @@ -126,7 +126,7 @@ struct ishtp_cl {
> };
>
> /* Client connection managenment internal functions */
> -int ishtp_can_client_connect(struct ishtp_device *ishtp_dev, uuid_le
> *uuid);
> +int ishtp_can_client_connect(struct ishtp_device *ishtp_dev, guid_t
> *uuid);
> int ishtp_fw_cl_by_id(struct ishtp_device *dev, uint8_t client_id);
> void ishtp_cl_send_msg(struct ishtp_device *dev, struct ishtp_cl
> *cl);
> void recv_ishtp_cl_msg(struct ishtp_device *dev,
> diff --git a/drivers/hid/intel-ish-hid/ishtp/hbm.h
> b/drivers/hid/intel-ish-hid/ishtp/hbm.h
> index d96111cef7f8..7286e3600140 100644
> --- a/drivers/hid/intel-ish-hid/ishtp/hbm.h
> +++ b/drivers/hid/intel-ish-hid/ishtp/hbm.h
> @@ -149,7 +149,7 @@ struct hbm_host_enum_response {
> } __packed;
>
> struct ishtp_client_properties {
> - uuid_le protocol_name;
> + guid_t protocol_name;
> uint8_t protocol_version;
> uint8_t max_number_of_connections;
> uint8_t fixed_address;
^ permalink raw reply
* [PATCH 1/2] dt-bindings: input: sitronix-st1232: add compatible string for ST1633
From: Martin Kepplinger @ 2019-01-23 7:26 UTC (permalink / raw)
To: devicetree, linux-input
Cc: dmitry.torokhov, robh+dt, mark.rutland, chinyeow.sim.xt,
linux-kernel, Martin Kepplinger
From: Martin Kepplinger <martin.kepplinger@ginzinger.com>
The st1232 driver gains support for the ST1633 controller too; update
the bindings doc accordingly.
Signed-off-by: Martin Kepplinger <martin.kepplinger@ginzinger.com>
---
.../bindings/input/touchscreen/sitronix-st1232.txt | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/input/touchscreen/sitronix-st1232.txt b/Documentation/devicetree/bindings/input/touchscreen/sitronix-st1232.txt
index 64ad48b824a2..e73e826e0f2a 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/sitronix-st1232.txt
+++ b/Documentation/devicetree/bindings/input/touchscreen/sitronix-st1232.txt
@@ -1,7 +1,9 @@
-* Sitronix st1232 touchscreen controller
+* Sitronix st1232 or st1633 touchscreen controller
Required properties:
-- compatible: must be "sitronix,st1232"
+- compatible: must contain one of
+ * "sitronix,st1232"
+ * "sitronix,st1633"
- reg: I2C address of the chip
- interrupts: interrupt to which the chip is connected
--
2.20.1
^ permalink raw reply related
* [PATCH 2/2] Input: st1232 - add support for st1633
From: Martin Kepplinger @ 2019-01-23 7:26 UTC (permalink / raw)
To: devicetree, linux-input
Cc: dmitry.torokhov, robh+dt, mark.rutland, chinyeow.sim.xt,
linux-kernel, Martin Kepplinger
In-Reply-To: <20190123072651.31791-1-martink@posteo.de>
From: Martin Kepplinger <martin.kepplinger@ginzinger.com>
Add support for the Sitronix ST1633 touchscreen controller to the st1232
driver.
Signed-off-by: Martin Kepplinger <martin.kepplinger@ginzinger.com>
---
drivers/input/touchscreen/Kconfig | 6 +-
drivers/input/touchscreen/st1232.c | 130 +++++++++++++++++++++--------
2 files changed, 98 insertions(+), 38 deletions(-)
diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index 068dbbc610fc..7c597a49c265 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -1168,11 +1168,11 @@ config TOUCHSCREEN_SIS_I2C
module will be called sis_i2c.
config TOUCHSCREEN_ST1232
- tristate "Sitronix ST1232 touchscreen controllers"
+ tristate "Sitronix ST1232 or ST1633 touchscreen controllers"
depends on I2C
help
- Say Y here if you want to support Sitronix ST1232
- touchscreen controller.
+ Say Y here if you want to support the Sitronix ST1232
+ or ST1633 touchscreen controller.
If unsure, say N.
diff --git a/drivers/input/touchscreen/st1232.c b/drivers/input/touchscreen/st1232.c
index 11ff32c68025..dc0288f37fda 100644
--- a/drivers/input/touchscreen/st1232.c
+++ b/drivers/input/touchscreen/st1232.c
@@ -18,18 +18,21 @@
#include <linux/module.h>
#include <linux/of.h>
#include <linux/of_gpio.h>
+#include <linux/of_device.h>
#include <linux/pm_qos.h>
#include <linux/slab.h>
#include <linux/types.h>
#define ST1232_TS_NAME "st1232-ts"
+#define ST1633_TS_NAME "st1633-ts"
+
+enum {
+ st1232,
+ st1633,
+};
#define MIN_X 0x00
#define MIN_Y 0x00
-#define MAX_X 0x31f /* (800 - 1) */
-#define MAX_Y 0x1df /* (480 - 1) */
-#define MAX_AREA 0xff
-#define MAX_FINGERS 2
struct st1232_ts_finger {
u16 x;
@@ -38,12 +41,24 @@ struct st1232_ts_finger {
bool is_valid;
};
+struct st_chip_info {
+ u8 id;
+ u16 max_x;
+ u16 max_y;
+ u16 max_area;
+ u16 max_fingers;
+ u8 start_reg;
+};
+
struct st1232_ts_data {
struct i2c_client *client;
struct input_dev *input_dev;
- struct st1232_ts_finger finger[MAX_FINGERS];
struct dev_pm_qos_request low_latency_req;
int reset_gpio;
+ const struct st_chip_info *chip_info;
+ int read_buf_len;
+ u8 *read_buf;
+ struct st1232_ts_finger *finger;
};
static int st1232_ts_read_data(struct st1232_ts_data *ts)
@@ -52,40 +67,35 @@ static int st1232_ts_read_data(struct st1232_ts_data *ts)
struct i2c_client *client = ts->client;
struct i2c_msg msg[2];
int error;
- u8 start_reg;
- u8 buf[10];
+ int i, y;
+ u8 start_reg = ts->chip_info->start_reg;
+ u8 *buf = ts->read_buf;
- /* read touchscreen data from ST1232 */
+ /* read touchscreen data */
msg[0].addr = client->addr;
msg[0].flags = 0;
msg[0].len = 1;
msg[0].buf = &start_reg;
- start_reg = 0x10;
msg[1].addr = ts->client->addr;
msg[1].flags = I2C_M_RD;
- msg[1].len = sizeof(buf);
+ msg[1].len = ts->read_buf_len;
msg[1].buf = buf;
error = i2c_transfer(client->adapter, msg, 2);
if (error < 0)
return error;
- /* get "valid" bits */
- finger[0].is_valid = buf[2] >> 7;
- finger[1].is_valid = buf[5] >> 7;
+ for (i = 0, y = 0; i < ts->chip_info->max_fingers; i++, y += 3) {
+ finger[i].is_valid = buf[i + y] >> 7;
+ if (finger[i].is_valid) {
+ finger[i].x = ((buf[i + y] & 0x0070) << 4) | buf[i + 1];
+ finger[i].y = ((buf[i + y] & 0x0007) << 8) | buf[i + 2];
- /* get xy coordinate */
- if (finger[0].is_valid) {
- finger[0].x = ((buf[2] & 0x0070) << 4) | buf[3];
- finger[0].y = ((buf[2] & 0x0007) << 8) | buf[4];
- finger[0].t = buf[8];
- }
-
- if (finger[1].is_valid) {
- finger[1].x = ((buf[5] & 0x0070) << 4) | buf[6];
- finger[1].y = ((buf[5] & 0x0007) << 8) | buf[7];
- finger[1].t = buf[9];
+ /* st1232 includes a z-axis / touch strength */
+ if (ts->chip_info->id == st1232)
+ finger[i].t = buf[i + 6];
+ }
}
return 0;
@@ -104,11 +114,14 @@ static irqreturn_t st1232_ts_irq_handler(int irq, void *dev_id)
goto end;
/* multi touch protocol */
- for (i = 0; i < MAX_FINGERS; i++) {
+ for (i = 0; i < ts->chip_info->max_fingers; i++) {
if (!finger[i].is_valid)
continue;
- input_report_abs(input_dev, ABS_MT_TOUCH_MAJOR, finger[i].t);
+ if (ts->chip_info->id == st1232)
+ input_report_abs(input_dev, ABS_MT_TOUCH_MAJOR,
+ finger[i].t);
+
input_report_abs(input_dev, ABS_MT_POSITION_X, finger[i].x);
input_report_abs(input_dev, ABS_MT_POSITION_Y, finger[i].y);
input_mt_sync(input_dev);
@@ -142,12 +155,46 @@ static void st1232_ts_power(struct st1232_ts_data *ts, bool poweron)
gpio_direction_output(ts->reset_gpio, poweron);
}
+static const struct st_chip_info st_chip_info_table[] = {
+ [st1232] = {
+ .id = st1232,
+ .max_x = 0x31f, /* 800 - 1 */
+ .max_y = 0x1df, /* 480 -1 */
+ .max_area = 0xff,
+ .max_fingers = 2,
+ .start_reg = 0x12,
+ },
+ [st1633] = {
+ .id = st1633,
+ .max_x = 0x13f, /* 320 - 1 */
+ .max_y = 0x1df, /* 480 -1 */
+ .max_area = 0x00,
+ .max_fingers = 5,
+ .start_reg = 0x12,
+ },
+};
+
+static const struct of_device_id st1232_ts_dt_ids[] = {
+ { .compatible = "sitronix,st1232", .data = &st_chip_info_table[st1232] },
+ { .compatible = "sitronix,st1633", .data = &st_chip_info_table[st1633] },
+ { }
+};
+MODULE_DEVICE_TABLE(of, st1232_ts_dt_ids);
+
static int st1232_ts_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
struct st1232_ts_data *ts;
+ struct st1232_ts_finger *finger;
struct input_dev *input_dev;
int error;
+ const struct of_device_id *match;
+
+ match = of_match_device(st1232_ts_dt_ids, &client->dev);
+ if (!match) {
+ dev_err(&client->dev, "unknown device model\n");
+ return -ENODEV;
+ }
if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
dev_err(&client->dev, "need I2C_FUNC_I2C\n");
@@ -163,6 +210,19 @@ static int st1232_ts_probe(struct i2c_client *client,
if (!ts)
return -ENOMEM;
+ ts->chip_info = match->data;
+ ts->finger = devm_kzalloc(&client->dev,
+ sizeof(*finger) * ts->chip_info->max_fingers,
+ GFP_KERNEL);
+ if (!ts->finger)
+ return -ENOMEM;
+
+ /* allocate a buffer according to the number of registers to read */
+ ts->read_buf_len = ts->chip_info->max_fingers * 4;
+ ts->read_buf = devm_kzalloc(&client->dev, ts->read_buf_len, GFP_KERNEL);
+ if (!ts->read_buf)
+ return -ENOMEM;
+
input_dev = devm_input_allocate_device(&client->dev);
if (!input_dev)
return -ENOMEM;
@@ -192,9 +252,14 @@ static int st1232_ts_probe(struct i2c_client *client,
__set_bit(EV_KEY, input_dev->evbit);
__set_bit(EV_ABS, input_dev->evbit);
- input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR, 0, MAX_AREA, 0, 0);
- input_set_abs_params(input_dev, ABS_MT_POSITION_X, MIN_X, MAX_X, 0, 0);
- input_set_abs_params(input_dev, ABS_MT_POSITION_Y, MIN_Y, MAX_Y, 0, 0);
+ if (ts->chip_info->id == st1232)
+ input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR, 0,
+ ts->chip_info->max_area, 0, 0);
+
+ input_set_abs_params(input_dev, ABS_MT_POSITION_X,
+ MIN_X, ts->chip_info->max_x, 0, 0);
+ input_set_abs_params(input_dev, ABS_MT_POSITION_Y,
+ MIN_Y, ts->chip_info->max_y, 0, 0);
error = devm_request_threaded_irq(&client->dev, client->irq,
NULL, st1232_ts_irq_handler,
@@ -262,16 +327,11 @@ static SIMPLE_DEV_PM_OPS(st1232_ts_pm_ops,
static const struct i2c_device_id st1232_ts_id[] = {
{ ST1232_TS_NAME, 0 },
+ { ST1633_TS_NAME, 0 },
{ }
};
MODULE_DEVICE_TABLE(i2c, st1232_ts_id);
-static const struct of_device_id st1232_ts_dt_ids[] = {
- { .compatible = "sitronix,st1232", },
- { }
-};
-MODULE_DEVICE_TABLE(of, st1232_ts_dt_ids);
-
static struct i2c_driver st1232_ts_driver = {
.probe = st1232_ts_probe,
.remove = st1232_ts_remove,
--
2.20.1
^ permalink raw reply related
* Re: [PATCH] drm/bridge: sil_sii8620: depend on INPUT instead of selecting it.
From: Lukas Wunner @ 2019-01-23 8:45 UTC (permalink / raw)
To: Ronald Tschalär, Dmitry Torokhov
Cc: Andrzej Hajda, Inki Dae, Laurent Pinchart, dri-devel,
linux-kernel, linux-input
In-Reply-To: <20190122141311.10445-1-ronald@innovation.ch>
On Tue, Jan 22, 2019 at 06:13:11AM -0800, Ronald Tschalär wrote:
> commit d6abe6df706c66d803e6dd4fe98c1b6b7f125a56 (drm/bridge:
> sil_sii8620: do not have a dependency of RC_CORE) added a dependency on
> INPUT. However, this causes problems with other drivers, in particular
> an input driver that depends on MFD_INTEL_LPSS_PCI (to be added in a
> future commit):
>
> drivers/clk/Kconfig:9:error: recursive dependency detected!
> drivers/clk/Kconfig:9: symbol COMMON_CLK is selected by MFD_INTEL_LPSS
> drivers/mfd/Kconfig:566: symbol MFD_INTEL_LPSS is selected by MFD_INTEL_LPSS_PCI
> drivers/mfd/Kconfig:580: symbol MFD_INTEL_LPSS_PCI is implied by KEYBOARD_APPLESPI
> drivers/input/keyboard/Kconfig:73: symbol KEYBOARD_APPLESPI depends on INPUT
> drivers/input/Kconfig:8: symbol INPUT is selected by DRM_SIL_SII8620
> drivers/gpu/drm/bridge/Kconfig:83: symbol DRM_SIL_SII8620 depends on DRM_BRIDGE
> drivers/gpu/drm/bridge/Kconfig:1: symbol DRM_BRIDGE is selected by DRM_PL111
> drivers/gpu/drm/pl111/Kconfig:1: symbol DRM_PL111 depends on COMMON_CLK
>
> According to the docs, select should only be used for non-visible
> symbols. Furthermore almost all other references to INPUT throughout the
> kernel config are depends, not selects. Hence this change.
>
> CC: Inki Dae <inki.dae@samsung.com>
> CC: Andrzej Hajda <a.hajda@samsung.com>
> Signed-off-by: Ronald Tschalär <ronald@innovation.ch>
Reviewed-by: Lukas Wunner <lukas@wunner.de>
I think this needs to be merged through the input tree as a prerequisite
for the applespi.c driver (keyboard + touchpad driver for 2015+ MacBook,
MacBook Air and MacBook Pro which uses SPI instead of USB) to avoid
breaking the build. Adding Dmitry.
Thanks,
Lukas
> ---
> drivers/gpu/drm/bridge/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
> index 2fee47b0d50b..eabedc83f25c 100644
> --- a/drivers/gpu/drm/bridge/Kconfig
> +++ b/drivers/gpu/drm/bridge/Kconfig
> @@ -83,9 +83,9 @@ config DRM_PARADE_PS8622
> config DRM_SIL_SII8620
> tristate "Silicon Image SII8620 HDMI/MHL bridge"
> depends on OF
> + depends on INPUT
> select DRM_KMS_HELPER
> imply EXTCON
> - select INPUT
> select RC_CORE
> help
> Silicon Image SII8620 HDMI/MHL bridge chip driver.
> --
> 2.20.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH 2/2] Input: st1232 - add support for st1633
From: Martin Kepplinger @ 2019-01-23 10:07 UTC (permalink / raw)
To: Martin Kepplinger, devicetree, linux-input
Cc: dmitry.torokhov, robh+dt, mark.rutland, chinyeow.sim.xt,
linux-kernel
In-Reply-To: <20190123072651.31791-2-martink@posteo.de>
[-- Attachment #1: Type: text/plain, Size: 389 bytes --]
On 23.01.19 08:26, Martin Kepplinger wrote:
> From: Martin Kepplinger <martin.kepplinger@ginzinger.com>
>
> Add support for the Sitronix ST1633 touchscreen controller to the st1232
> driver.
>
FYI, here's a protocol spec:
www.ampdisplay.com/documents/pdf/AM-320480B6TZQW-TC0H.pdf
Let me know if you want that link in the commit message.
thanks
martin
[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3616 bytes --]
^ permalink raw reply
* Re: [PATCH 09/13] power: supply: max77650: add support for battery charger
From: Sebastian Reichel @ 2019-01-23 18:27 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Rob Herring, Mark Rutland, Linus Walleij, Dmitry Torokhov,
Jacek Anaszewski, Pavel Machek, Lee Jones, 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-10-brgl@bgdev.pl>
[-- Attachment #1: Type: text/plain, Size: 13205 bytes --]
Hi Bartosz,
Looks mostly ok, I have a few comments inline towards the end.
On Fri, Jan 18, 2019 at 02:42:40PM +0100, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>
> Add basic support for the battery charger for max77650 PMIC.
>
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> ---
> drivers/power/supply/Kconfig | 7 +
> drivers/power/supply/Makefile | 1 +
> drivers/power/supply/max77650-charger.c | 347 ++++++++++++++++++++++++
> 3 files changed, 355 insertions(+)
> create mode 100644 drivers/power/supply/max77650-charger.c
>
> diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
> index e901b9879e7e..0230c96fa94d 100644
> --- a/drivers/power/supply/Kconfig
> +++ b/drivers/power/supply/Kconfig
> @@ -499,6 +499,13 @@ config CHARGER_DETECTOR_MAX14656
> Revision 1.2 and can be found e.g. in Kindle 4/5th generation
> readers and certain LG devices.
>
> +config CHARGER_MAX77650
> + tristate "Maxim MAX77650 battery charger driver"
> + depends on MFD_MAX77650
> + help
> + Say Y to enable support for the battery charger control of MAX77650
> + PMICs.
> +
> config CHARGER_MAX77693
> tristate "Maxim MAX77693 battery charger driver"
> depends on MFD_MAX77693
> diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
> index b731c2a9b695..b73eb8c5c1a9 100644
> --- a/drivers/power/supply/Makefile
> +++ b/drivers/power/supply/Makefile
> @@ -70,6 +70,7 @@ obj-$(CONFIG_CHARGER_MANAGER) += charger-manager.o
> obj-$(CONFIG_CHARGER_LTC3651) += ltc3651-charger.o
> obj-$(CONFIG_CHARGER_MAX14577) += max14577_charger.o
> obj-$(CONFIG_CHARGER_DETECTOR_MAX14656) += max14656_charger_detector.o
> +obj-$(CONFIG_CHARGER_MAX77650) += max77650-charger.o
> obj-$(CONFIG_CHARGER_MAX77693) += max77693_charger.o
> obj-$(CONFIG_CHARGER_MAX8997) += max8997_charger.o
> obj-$(CONFIG_CHARGER_MAX8998) += max8998_charger.o
> diff --git a/drivers/power/supply/max77650-charger.c b/drivers/power/supply/max77650-charger.c
> new file mode 100644
> index 000000000000..fb9d8f933174
> --- /dev/null
> +++ b/drivers/power/supply/max77650-charger.c
> @@ -0,0 +1,347 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2018 BayLibre SAS
> + * Author: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> + *
> + * Battery charger driver for MAXIM 77650/77651 charger/power-supply.
> + */
> +
> +#include <linux/i2c.h>
> +#include <linux/interrupt.h>
> +#include <linux/mfd/max77650.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/power_supply.h>
> +#include <linux/regmap.h>
> +
> +#define MAX77650_CHARGER_ENABLED BIT(0)
> +#define MAX77650_CHARGER_DISABLED 0x00
> +#define MAX77650_CHARGER_CHG_EN_MASK BIT(0)
> +
> +#define MAX77650_CHARGER_CHG_DTLS_MASK GENMASK(7, 4)
> +#define MAX77650_CHARGER_CHG_DTLS_BITS(_reg) \
> + (((_reg) & MAX77650_CHARGER_CHG_DTLS_MASK) >> 4)
> +
> +#define MAX77650_CHARGER_CHG_OFF 0x00
> +#define MAX77650_CHARGER_CHG_PREQ 0x01
> +#define MAX77650_CHARGER_CHG_ON_CURR 0x02
> +#define MAX77650_CHARGER_CHG_ON_JCURR 0x03
> +#define MAX77650_CHARGER_CHG_ON_VOLT 0x04
> +#define MAX77650_CHARGER_CHG_ON_JVOLT 0x05
> +#define MAX77650_CHARGER_CHG_ON_TOPOFF 0x06
> +#define MAX77650_CHARGER_CHG_ON_JTOPOFF 0x07
> +#define MAX77650_CHARGER_CHG_DONE 0x08
> +#define MAX77650_CHARGER_CHG_JDONE 0x09
> +#define MAX77650_CHARGER_CHG_SUSP_PF 0x0a
> +#define MAX77650_CHARGER_CHG_SUSP_FCF 0x0b
> +#define MAX77650_CHARGER_CHG_SUSP_BTF 0x0c
> +
> +#define MAX77650_CHARGER_CHGIN_DTLS_MASK GENMASK(3, 2)
> +#define MAX77650_CHARGER_CHGIN_DTLS_BITS(_reg) \
> + (((_reg) & MAX77650_CHARGER_CHGIN_DTLS_MASK) >> 2)
> +
> +#define MAX77650_CHARGER_CHGIN_UVL 0x00
> +#define MAX77650_CHARGER_CHGIN_OVL 0x01
> +#define MAX77650_CHARGER_CHGIN_OKAY 0x11
> +
> +#define MAX77650_CHARGER_CHG_MASK BIT(1)
> +#define MAX77650_CHARGER_CHG_CHARGING(_reg) \
> + (((_reg) & MAX77650_CHARGER_CHG_MASK) > 1)
> +
> +#define MAX77650_CHARGER_VCHGIN_MIN_MASK 0xc0
> +#define MAX77650_CHARGER_VCHGIN_MIN_SHIFT(_val) ((_val) << 5)
> +
> +#define MAX77650_CHARGER_ICHGIN_LIM_MASK 0x1c
> +#define MAX77650_CHARGER_ICHGIN_LIM_SHIFT(_val) ((_val) << 2)
> +
> +struct max77650_charger_data {
> + struct regmap *map;
> + struct device *dev;
> +};
> +
> +static enum power_supply_property max77650_charger_properties[] = {
> + POWER_SUPPLY_PROP_STATUS,
> + POWER_SUPPLY_PROP_ONLINE,
> + POWER_SUPPLY_PROP_CHARGE_TYPE
> +};
> +
> +static const unsigned int max77650_charger_vchgin_min_table[] = {
> + 4000000, 4100000, 4200000, 4300000, 4400000, 4500000, 4600000, 4700000
> +};
> +
> +static const unsigned int max77650_charger_ichgin_lim_table[] = {
> + 95000, 190000, 285000, 380000, 475000
> +};
> +
> +static int max77650_charger_set_vchgin_min(struct max77650_charger_data *chg,
> + unsigned int val)
> +{
> + int i, rv;
> +
> + for (i = 0; i < ARRAY_SIZE(max77650_charger_vchgin_min_table); i++) {
> + if (val == max77650_charger_vchgin_min_table[i]) {
> + rv = regmap_update_bits(chg->map,
> + MAX77650_REG_CNFG_CHG_B,
> + MAX77650_CHARGER_VCHGIN_MIN_MASK,
> + MAX77650_CHARGER_VCHGIN_MIN_SHIFT(i));
> + if (rv)
> + return rv;
> +
> + return 0;
> + }
> + }
> +
> + return -EINVAL;
> +}
> +
> +static int max77650_charger_set_ichgin_lim(struct max77650_charger_data *chg,
> + unsigned int val)
> +{
> + int i, rv;
> +
> + for (i = 0; i < ARRAY_SIZE(max77650_charger_ichgin_lim_table); i++) {
> + if (val == max77650_charger_ichgin_lim_table[i]) {
> + rv = regmap_update_bits(chg->map,
> + MAX77650_REG_CNFG_CHG_B,
> + MAX77650_CHARGER_ICHGIN_LIM_MASK,
> + MAX77650_CHARGER_ICHGIN_LIM_SHIFT(i));
> + if (rv)
> + return rv;
> +
> + return 0;
> + }
> + }
> +
> + return -EINVAL;
> +}
> +
> +static void max77650_charger_enable(struct max77650_charger_data *chg)
> +{
> + int rv;
> +
> + rv = regmap_update_bits(chg->map,
> + MAX77650_REG_CNFG_CHG_B,
> + MAX77650_CHARGER_CHG_EN_MASK,
> + MAX77650_CHARGER_ENABLED);
> + if (rv)
> + dev_err(chg->dev, "unable to enable the charger: %d\n", rv);
> +}
> +
> +static void max77650_charger_disable(struct max77650_charger_data *chg)
> +{
> + int rv;
> +
> + rv = regmap_update_bits(chg->map,
> + MAX77650_REG_CNFG_CHG_B,
> + MAX77650_CHARGER_CHG_EN_MASK,
> + MAX77650_CHARGER_DISABLED);
> + if (rv)
> + dev_err(chg->dev, "unable to disable the charger: %d\n", rv);
> +}
> +
> +static irqreturn_t max77650_charger_check_status(int irq, void *data)
> +{
> + struct max77650_charger_data *chg = data;
> + int rv, reg;
> +
> + rv = regmap_read(chg->map, MAX77650_REG_STAT_CHG_B, ®);
> + if (rv) {
> + dev_err(chg->dev,
> + "unable to read the charger status: %d\n", rv);
> + return IRQ_HANDLED;
> + }
> +
> + switch (MAX77650_CHARGER_CHGIN_DTLS_BITS(reg)) {
> + case MAX77650_CHARGER_CHGIN_UVL:
> + dev_err(chg->dev, "undervoltage lockout detected, disabling charger\n");
> + max77650_charger_disable(chg);
> + break;
> + case MAX77650_CHARGER_CHGIN_OVL:
> + dev_err(chg->dev, "overvoltage lockout detected, disabling charger\n");
> + max77650_charger_disable(chg);
> + break;
> + case MAX77650_CHARGER_CHGIN_OKAY:
> + max77650_charger_enable(chg);
> + break;
> + default:
> + /* May be 0x10 - debouncing */
> + break;
> + }
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int max77650_charger_get_property(struct power_supply *psy,
> + enum power_supply_property psp,
> + union power_supply_propval *val)
> +{
> + struct max77650_charger_data *chg = power_supply_get_drvdata(psy);
> + int rv, reg;
> +
> + switch (psp) {
> + case POWER_SUPPLY_PROP_STATUS:
> + rv = regmap_read(chg->map, MAX77650_REG_STAT_CHG_B, ®);
> + if (rv)
> + return rv;
> +
> + if (MAX77650_CHARGER_CHG_CHARGING(reg)) {
> + val->intval = POWER_SUPPLY_STATUS_CHARGING;
> + break;
> + }
> +
> + switch (MAX77650_CHARGER_CHG_DTLS_BITS(reg)) {
> + case MAX77650_CHARGER_CHG_OFF:
> + case MAX77650_CHARGER_CHG_SUSP_PF:
> + case MAX77650_CHARGER_CHG_SUSP_FCF:
> + case MAX77650_CHARGER_CHG_SUSP_BTF:
> + val->intval = POWER_SUPPLY_STATUS_NOT_CHARGING;
> + break;
> + case MAX77650_CHARGER_CHG_PREQ:
> + case MAX77650_CHARGER_CHG_ON_CURR:
> + case MAX77650_CHARGER_CHG_ON_JCURR:
> + case MAX77650_CHARGER_CHG_ON_VOLT:
> + case MAX77650_CHARGER_CHG_ON_JVOLT:
> + case MAX77650_CHARGER_CHG_ON_TOPOFF:
> + case MAX77650_CHARGER_CHG_ON_JTOPOFF:
> + val->intval = POWER_SUPPLY_STATUS_CHARGING;
> + break;
> + case MAX77650_CHARGER_CHG_DONE:
> + val->intval = POWER_SUPPLY_STATUS_FULL;
> + break;
> + default:
> + val->intval = POWER_SUPPLY_STATUS_UNKNOWN;
> + }
> + break;
> + case POWER_SUPPLY_PROP_ONLINE:
> + rv = regmap_read(chg->map, MAX77650_REG_STAT_CHG_B, ®);
> + if (rv)
> + return rv;
> +
> + val->intval = MAX77650_CHARGER_CHG_CHARGING(reg);
> + break;
> + case POWER_SUPPLY_PROP_CHARGE_TYPE:
> + rv = regmap_read(chg->map, MAX77650_REG_STAT_CHG_B, ®);
> + if (rv)
> + return rv;
> +
> + if (!MAX77650_CHARGER_CHG_CHARGING(reg)) {
> + val->intval = POWER_SUPPLY_CHARGE_TYPE_NONE;
> + break;
> + }
> +
> + switch (MAX77650_CHARGER_CHG_DTLS_BITS(reg)) {
> + case MAX77650_CHARGER_CHG_PREQ:
> + case MAX77650_CHARGER_CHG_ON_CURR:
> + case MAX77650_CHARGER_CHG_ON_JCURR:
> + case MAX77650_CHARGER_CHG_ON_VOLT:
> + case MAX77650_CHARGER_CHG_ON_JVOLT:
> + val->intval = POWER_SUPPLY_CHARGE_TYPE_FAST;
> + break;
> + case MAX77650_CHARGER_CHG_ON_TOPOFF:
> + case MAX77650_CHARGER_CHG_ON_JTOPOFF:
> + val->intval = POWER_SUPPLY_CHARGE_TYPE_TRICKLE;
> + break;
> + default:
> + val->intval = POWER_SUPPLY_CHARGE_TYPE_UNKNOWN;
> + }
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static const struct power_supply_desc max77650_battery_desc = {
> + .name = "max77650",
> + .type = POWER_SUPPLY_TYPE_BATTERY,
This is a charger, not a battery. BATTERY type is for the fuel
gauges. You should be using POWER_SUPPLY_TYPE_USB for USB based
battery chargers and POWER_SUPPLY_TYPE_MAINS otherwise.
> + .get_property = max77650_charger_get_property,
> + .properties = max77650_charger_properties,
> + .num_properties = ARRAY_SIZE(max77650_charger_properties),
> +};
> +
> +static int max77650_charger_probe(struct platform_device *pdev)
> +{
> + struct regmap_irq_chip_data *irq_data;
> + struct power_supply_config pscfg = {};
> + struct max77650_charger_data *chg;
> + struct power_supply *battery;
> + struct device *dev, *parent;
> + int rv, chg_irq, chgin_irq;
> + struct i2c_client *i2c;
> + unsigned int prop;
> +
> + dev = &pdev->dev;
> + parent = dev->parent;
> + i2c = to_i2c_client(parent);
> + irq_data = i2c_get_clientdata(i2c);
> +
> + chg = devm_kzalloc(dev, sizeof(*chg), GFP_KERNEL);
> + if (!chg)
> + return -ENOMEM;
> +
> + chg->map = dev_get_regmap(parent, NULL);
> + if (!chg->map)
> + return -ENODEV;
> +
> + chg->dev = dev;
> +
> + pscfg.of_node = dev->of_node;
> + pscfg.drv_data = chg;
> +
> + chg_irq = regmap_irq_get_virq(irq_data, MAX77650_INT_CHG);
> + if (chg_irq <= 0)
> + return -EINVAL;
> +
> + chgin_irq = regmap_irq_get_virq(irq_data, MAX77650_INT_CHGIN);
> + if (chgin_irq <= 0)
> + return -EINVAL;
> +
> + rv = devm_request_threaded_irq(dev, chg_irq, NULL,
> + max77650_charger_check_status,
> + IRQF_ONESHOT, "chg", chg);
> + if (rv)
> + return rv;
> +
> + rv = devm_request_threaded_irq(dev, chgin_irq, NULL,
> + max77650_charger_check_status,
> + IRQF_ONESHOT, "chgin", chg);
> + if (rv)
> + return rv;
> +
> + battery = devm_power_supply_register(dev,
> + &max77650_battery_desc, &pscfg);
> + if (IS_ERR(battery))
> + return PTR_ERR(battery);
> +
> + rv = of_property_read_u32(dev->of_node, "maxim,vchgin-min", &prop);
> + if (rv == 0) {
> + rv = max77650_charger_set_vchgin_min(chg, prop);
> + if (rv)
> + return rv;
> + }
> +
> + rv = of_property_read_u32(dev->of_node, "maxim,ichgin-lim", &prop);
> + if (rv == 0) {
> + rv = max77650_charger_set_ichgin_lim(chg, prop);
> + if (rv)
> + return rv;
> + }
> +
> + return regmap_update_bits(chg->map,
> + MAX77650_REG_CNFG_CHG_B,
> + MAX77650_CHARGER_CHG_EN_MASK,
> + MAX77650_CHARGER_ENABLED);
should this be disabled on module remove?
> +}
> +
> +static struct platform_driver max77650_charger_driver = {
> + .driver = {
> + .name = "max77650-charger",
> + },
> + .probe = max77650_charger_probe,
> +};
> +module_platform_driver(max77650_charger_driver);
> +
> +MODULE_DESCRIPTION("MAXIM 77650/77651 charger driver");
> +MODULE_AUTHOR("Bartosz Golaszewski <bgolaszewski@baylibre.com>");
> +MODULE_LICENSE("GPL");
This is GPL2 or later, but "SPDX-License-Identifier: GPL-2.0" is not.
-- Sebastian
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH v4 0/9] platform/chrome: rtc: Add support for Wilco EC
From: Nick Crews @ 2019-01-23 18:33 UTC (permalink / raw)
To: linux-kernel
Cc: sjg, dmitry.torokhov, sre, linux-input, groeck, dlaurie,
Nick Crews, linux-rtc, Enric Balletbo i Serra, Alessandro Zummo,
Benson Leung, Nick Crews, Duncan Laurie, Alexandre Belloni
There is a new chromebook that contains a different Embedded Controller
(codename Wilco) than the rest of the chromebook series. Thus the kernel
requires a different driver than the already existing and generalized
cros_ec_* drivers. Specifically, this driver adds support for getting
and setting the RTC on the EC, adding a binary sysfs attribute
that receives ACPI events from the EC, adding a binary sysfs
attribute to request telemetry data from the EC (useful for enterprise
applications), adding a debugfs interface for sending/receiving raw byte
sequesnces to the EC, and adding normal sysfs attributes to get/set various
other properties on the EC. The core of the communication with the EC
is implemented in wilco_ec/mailbox.c, using a simple byte-level protocol
with a checksum, transmitted over an eSPI bus. For debugging purposes,
a raw attribute is also provided via debugfs which can write/read
arbitrary bytes to/from the eSPI bus.
The entry point of the driver is wilco_ec/core.c,
which is responsible for several tasks:
- Initialize the register interface that is used by wilco_ec_mailbox()
- Create a platform device which is picked up by the debugfs driver
- Create a platform device which is picked up by the RTC driver
- Initialize the sysfs entries
- Set up event handling
We attempted to adhere to the sysfs principles of "one piece of data per
attribute" as much as possible, and mostly succeded. However, with the
wilco_ec/adv_power.h attributes, which deal with scheduling power usage,
we found it most elegant to bundle setting event times for an entire day
into a single attribute, so at most you are using attributes formatted
as "%d %d %d %d %d %d". With the telemetry attribute, we had to use a
binary attribute, instead of the preferable human-readable ascii, in
order to keep secure the information which is proprietary to the
enterprise service provider. This opaque binary data will be read and
sent using a proprietary daemon running on the OS.
The RTC driver is exposed as a standard RTC driver with
read/write functionality.
For event notification, the Wilco EC can return extended events that
are not handled by standard ACPI objects. These events can
include hotkeys which map to standard functions like brightness
controls, or information about EC controlled features like the
charger or battery. These events are triggered with an
ACPI Notify(0x90) and the event data buffer is read through an ACPI
method provided by the BIOS which reads the event buffer from EC RAM.
These events are then processed, with hotkey events being sent
to the input subsystem and other events put into a queue which
can be read by a userspace daemon via a sysfs attribute.
The rest of the attributes are categorized as either "properties" or
"legacy". "legacy" implies that the attribute existed on the EC before it
was modified for ChromeOS, and "properties" implies that the attribute
exposes functionality that was added to the EC specifically for
ChromeOS. They are mostly boolean flags or percentages.
A full thread of the development of these patches can be found at
https://chromium-review.googlesource.com/c/1371034. This thread contains
comments and revisions that could be helpful in understanding how the
driver arrived at the state it is in now. The thread also contains some
ChromeOS specific patches that actually enable the driver. If you want
to test the patch yourself, you would have to install the ChromeOS SDK
and cherry pick in these patches.
I also wrote some integration tests using the Tast testing framework that
ChromeOS uses. It would require a full ChromeOS SDK to actually run the
tests, but the source of the tests, written in Go, are useful for
understanding what the desired behavior is. You can view the tests here:
https://chromium-review.googlesource.com/c/1372575
Thank you for your comments!
Changes in v4:
- add #define DRV_NAME
- remove redundant Kconfig nesting
- changed my email to @chromium.org
- Add better explanation of what core.c does
- Change debugfs driver to be a separate module
- Change me email to @chromium.org from @google.com
- Change CONFIG_WILCO_EC_SYSFS_RAW to
CONFIG_WILCO_EC_DEBUGFS
- Change me email to @chromium.org from @google.com
- Move "Add RTC driver" before "Add sysfs attributes" so that
it could get accepted earlier, since it is less contentious
- Move "Add RTC driver" before "Add sysfs attributes" so that
it could get accepted earlier, since it is less contentious
Changes in v3:
- Change <= to >= in mec_in_range()
- Add " - EC_HOST_CMD_REGION0" to offset arg for io_bytes_mec()
- remove unused ret in probe()
- Add newline spacer in probe()
- rm unnecessary res in get_resource()
- s/8bit/8-bit
- rm first sleep when sending command to EC
- Move the attribute to the debugfs system
- Move the implementation to debugfs.c
- Improve the raw hex parsing
- Encapsulate global variables in one object
- Add safety check when passing less than 3 bytes
- Move documentation to debugfs-wilco-ec
- rm #define for driver name
- Don't compute weekday when reading from RTC.
Still set weekday when writing, as RTC needs this
to control advanced power scheduling
- rm check for invalid month data
- Set range_min and range_max on rtc_device
- explicitly define toplevel_groups from the start,
so adding telem later makes sense
- Break version attribute into individual attributes
- rm unused WILCO_EC_ATTR_RW macro
- Moved some #defines from legacy.h to legacy.c
- change err check from "if (ret < 0)" to "if (ret)"
- Now bubble up error codes from within sysfs_init()
- Add comment that PID means Property ID
- rm some useless references to internal docs from documentation
- add err check on returned data size
- add check on read request offset and size
Changes in v2:
- Fixed kernel-doc comments
- Fixed include of linux/mfd/cros_ec_lpc_mec.h
- cros_ec_lpc_mec_in_range() returns -EINVAL on error
- Added parens around macro variables
- Remove COMPILE_TEST from Kconfig because inb()/outb()
won't work on anything but X86
- Add myself as module author
- Tweak mailbox()
- Add sysfs documentation
- rm duplicate EC_MAILBOX_DATA_SIZE defs
- Make docstrings follow kernel style
- Fix tags in commit msg
- Move Kconfig to subdirectory
- Reading raw now includes ASCII translation
- rm license boiler plate
- rm "wilco_ec_rtc -" prefix in docstring
- Make rtc driver its own module within the drivers/rtc/ directory
- Register a rtc device from core.c that is picked up by this driver
- Remove license boiler plate
- Remove "wilco_ec_sysfs -" docstring prefix
- Fix accidental Makefile deletion
- Add documentation for sysfs entries
- Change "enable ? 0 : 1" to "!enable"
- No longer override error code from sysfs_init()
- Put attributes in the legacy file to begin with, don't move later
- Remove duplicate error messages when init()ing sysfs
- rm "wilco_ec_event -" prefix from docstring
- rm license boiler plate
- Add sysfs directory documentation
- Fix cosmetics
- events are init()ed before subdrivers now
- rm license boiler plate
- rm "wilco_ec_properties -" prefix from docstring
- Add documentation
- rm license boiler plate
- rm "wilco_ec_adv_power - " prefix from docstring
- Add documentation
- make format strings in read() and store() functions static
Duncan Laurie (6):
cros_ec: Remove cros_ec dependency in lpc_mec
platform/chrome: Add new driver for Wilco EC
platform/chrome: Add support for raw commands in debugfs
platform/chrome: rtc: Add RTC driver
platform/chrome: Add sysfs attributes
platform/chrome: Add event handling
Nick Crews (3):
platform/chrome: Add EC properties
platform/chrome: Add peakshift and adv_batt_charging
platform/chrome: Add binary telemetry attributes
Documentation/ABI/testing/debugfs-wilco-ec | 23 +
.../ABI/testing/sysfs-platform-wilco-ec | 196 +++++++
drivers/platform/chrome/Kconfig | 4 +-
drivers/platform/chrome/Makefile | 2 +
drivers/platform/chrome/cros_ec_lpc_mec.c | 52 +-
drivers/platform/chrome/cros_ec_lpc_mec.h | 43 +-
drivers/platform/chrome/cros_ec_lpc_reg.c | 47 +-
drivers/platform/chrome/wilco_ec/Kconfig | 20 +
drivers/platform/chrome/wilco_ec/Makefile | 8 +
drivers/platform/chrome/wilco_ec/adv_power.c | 544 ++++++++++++++++++
drivers/platform/chrome/wilco_ec/adv_power.h | 183 ++++++
drivers/platform/chrome/wilco_ec/core.c | 164 ++++++
drivers/platform/chrome/wilco_ec/debugfs.c | 226 ++++++++
drivers/platform/chrome/wilco_ec/event.c | 347 +++++++++++
drivers/platform/chrome/wilco_ec/legacy.c | 103 ++++
drivers/platform/chrome/wilco_ec/legacy.h | 79 +++
drivers/platform/chrome/wilco_ec/mailbox.c | 236 ++++++++
drivers/platform/chrome/wilco_ec/properties.c | 344 +++++++++++
drivers/platform/chrome/wilco_ec/properties.h | 180 ++++++
drivers/platform/chrome/wilco_ec/sysfs.c | 245 ++++++++
drivers/platform/chrome/wilco_ec/telemetry.c | 73 +++
drivers/platform/chrome/wilco_ec/telemetry.h | 41 ++
drivers/platform/chrome/wilco_ec/util.h | 38 ++
drivers/rtc/Kconfig | 11 +
drivers/rtc/Makefile | 1 +
drivers/rtc/rtc-wilco-ec.c | 177 ++++++
include/linux/platform_data/wilco-ec.h | 186 ++++++
27 files changed, 3513 insertions(+), 60 deletions(-)
create mode 100644 Documentation/ABI/testing/debugfs-wilco-ec
create mode 100644 Documentation/ABI/testing/sysfs-platform-wilco-ec
create mode 100644 drivers/platform/chrome/wilco_ec/Kconfig
create mode 100644 drivers/platform/chrome/wilco_ec/Makefile
create mode 100644 drivers/platform/chrome/wilco_ec/adv_power.c
create mode 100644 drivers/platform/chrome/wilco_ec/adv_power.h
create mode 100644 drivers/platform/chrome/wilco_ec/core.c
create mode 100644 drivers/platform/chrome/wilco_ec/debugfs.c
create mode 100644 drivers/platform/chrome/wilco_ec/event.c
create mode 100644 drivers/platform/chrome/wilco_ec/legacy.c
create mode 100644 drivers/platform/chrome/wilco_ec/legacy.h
create mode 100644 drivers/platform/chrome/wilco_ec/mailbox.c
create mode 100644 drivers/platform/chrome/wilco_ec/properties.c
create mode 100644 drivers/platform/chrome/wilco_ec/properties.h
create mode 100644 drivers/platform/chrome/wilco_ec/sysfs.c
create mode 100644 drivers/platform/chrome/wilco_ec/telemetry.c
create mode 100644 drivers/platform/chrome/wilco_ec/telemetry.h
create mode 100644 drivers/platform/chrome/wilco_ec/util.h
create mode 100644 drivers/rtc/rtc-wilco-ec.c
create mode 100644 include/linux/platform_data/wilco-ec.h
--
2.20.1.321.g9e740568ce-goog
^ permalink raw reply
* [PATCH v4 1/9] cros_ec: Remove cros_ec dependency in lpc_mec
From: Nick Crews @ 2019-01-23 18:33 UTC (permalink / raw)
To: linux-kernel
Cc: sjg, dmitry.torokhov, sre, linux-input, groeck, dlaurie,
Duncan Laurie, Enric Balletbo i Serra, Nick Crews, Benson Leung
In-Reply-To: <20190123183325.92946-1-ncrews@chromium.org>
From: Duncan Laurie <dlaurie@google.com>
In order to allow this code to be re-used, remove the dependency
on the rest of the cros_ec code from the cros_ec_lpc_mec functions.
Instead of using a hardcoded register base address of 0x800 have
this be passed in to cros_ec_lpc_mec_init(). The existing cros_ec
use case now passes in the 0x800 base address this way.
There are some error checks that happen in cros_ec_lpc_mec_in_range()
that probably shouldn't be there, as they are checking kernel-space
callers and not user-space input. However, we'll just do the refactor in
this patch, and in a future patch might remove this error checking and
fix all the instances of code that calls this.
There's a similar problem in cros_ec_lpc_read_bytes(), where we return a
checksum, but on error just return 0. This should probably be changed so
that it returns int, but we don't want to have to mess with all the
calling code for this fix. Maybe we'll come back through later and fix
this.
Signed-off-by: Duncan Laurie <dlaurie@google.com>
Acked-for-chrome-platform-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Signed-off-by: Nick Crews <ncrews@chromium.org>
---
Changes in v4: None
Changes in v3:
- Change <= to >= in mec_in_range()
- Add " - EC_HOST_CMD_REGION0" to offset arg for io_bytes_mec()
Changes in v2:
- Fixed kernel-doc comments
- Fixed include of linux/mfd/cros_ec_lpc_mec.h
- cros_ec_lpc_mec_in_range() returns -EINVAL on error
- Added parens around macro variables
drivers/platform/chrome/cros_ec_lpc_mec.c | 52 +++++++++++++++++++----
drivers/platform/chrome/cros_ec_lpc_mec.h | 43 ++++++++++---------
drivers/platform/chrome/cros_ec_lpc_reg.c | 47 +++++++-------------
3 files changed, 83 insertions(+), 59 deletions(-)
diff --git a/drivers/platform/chrome/cros_ec_lpc_mec.c b/drivers/platform/chrome/cros_ec_lpc_mec.c
index c4edfa83e493..782e238a8d4e 100644
--- a/drivers/platform/chrome/cros_ec_lpc_mec.c
+++ b/drivers/platform/chrome/cros_ec_lpc_mec.c
@@ -23,7 +23,6 @@
#include <linux/delay.h>
#include <linux/io.h>
-#include <linux/mfd/cros_ec_commands.h>
#include <linux/mutex.h>
#include <linux/types.h>
@@ -34,6 +33,7 @@
* EC mutex because memmap data may be accessed without it being held.
*/
static struct mutex io_mutex;
+static u16 mec_emi_base, mec_emi_end;
/*
* cros_ec_lpc_mec_emi_write_address
@@ -46,10 +46,37 @@ static struct mutex io_mutex;
static void cros_ec_lpc_mec_emi_write_address(u16 addr,
enum cros_ec_lpc_mec_emi_access_mode access_type)
{
- /* Address relative to start of EMI range */
- addr -= MEC_EMI_RANGE_START;
- outb((addr & 0xfc) | access_type, MEC_EMI_EC_ADDRESS_B0);
- outb((addr >> 8) & 0x7f, MEC_EMI_EC_ADDRESS_B1);
+ outb((addr & 0xfc) | access_type, MEC_EMI_EC_ADDRESS_B0(mec_emi_base));
+ outb((addr >> 8) & 0x7f, MEC_EMI_EC_ADDRESS_B1(mec_emi_base));
+}
+
+/**
+ * cros_ec_lpc_mec_in_range() - Determine if addresses are in MEC EMI range.
+ *
+ * @offset: Address offset
+ * @length: Number of bytes to check
+ *
+ * Return: 1 if in range, 0 if not, and -EINVAL on failure
+ * such as the mec range not being initialized
+ */
+int cros_ec_lpc_mec_in_range(unsigned int offset, unsigned int length)
+{
+ if (length == 0)
+ return -EINVAL;
+
+ if (WARN_ON(mec_emi_base == 0 || mec_emi_end == 0))
+ return -EINVAL;
+
+ if (offset >= mec_emi_base && offset < mec_emi_end) {
+ if (WARN_ON(offset + length - 1 >= mec_emi_end))
+ return -EINVAL;
+ return 1;
+ }
+
+ if (WARN_ON(offset + length > mec_emi_base && offset < mec_emi_end))
+ return -EINVAL;
+
+ return 0;
}
/*
@@ -71,6 +98,11 @@ u8 cros_ec_lpc_io_bytes_mec(enum cros_ec_lpc_mec_io_type io_type,
u8 sum = 0;
enum cros_ec_lpc_mec_emi_access_mode access, new_access;
+ /* Return checksum of 0 if window is not initialized */
+ WARN_ON(mec_emi_base == 0 || mec_emi_end == 0);
+ if (mec_emi_base == 0 || mec_emi_end == 0)
+ return 0;
+
/*
* Long access cannot be used on misaligned data since reading B0 loads
* the data register and writing B3 flushes.
@@ -86,9 +118,9 @@ u8 cros_ec_lpc_io_bytes_mec(enum cros_ec_lpc_mec_io_type io_type,
cros_ec_lpc_mec_emi_write_address(offset, access);
/* Skip bytes in case of misaligned offset */
- io_addr = MEC_EMI_EC_DATA_B0 + (offset & 0x3);
+ io_addr = MEC_EMI_EC_DATA_B0(mec_emi_base) + (offset & 0x3);
while (i < length) {
- while (io_addr <= MEC_EMI_EC_DATA_B3) {
+ while (io_addr <= MEC_EMI_EC_DATA_B3(mec_emi_base)) {
if (io_type == MEC_IO_READ)
buf[i] = inb(io_addr++);
else
@@ -118,7 +150,7 @@ u8 cros_ec_lpc_io_bytes_mec(enum cros_ec_lpc_mec_io_type io_type,
}
/* Access [B0, B3] on each loop pass */
- io_addr = MEC_EMI_EC_DATA_B0;
+ io_addr = MEC_EMI_EC_DATA_B0(mec_emi_base);
}
done:
@@ -128,9 +160,11 @@ u8 cros_ec_lpc_io_bytes_mec(enum cros_ec_lpc_mec_io_type io_type,
}
EXPORT_SYMBOL(cros_ec_lpc_io_bytes_mec);
-void cros_ec_lpc_mec_init(void)
+void cros_ec_lpc_mec_init(unsigned int base, unsigned int end)
{
mutex_init(&io_mutex);
+ mec_emi_base = base;
+ mec_emi_end = end;
}
EXPORT_SYMBOL(cros_ec_lpc_mec_init);
diff --git a/drivers/platform/chrome/cros_ec_lpc_mec.h b/drivers/platform/chrome/cros_ec_lpc_mec.h
index 105068c0e919..896fffd174b3 100644
--- a/drivers/platform/chrome/cros_ec_lpc_mec.h
+++ b/drivers/platform/chrome/cros_ec_lpc_mec.h
@@ -24,8 +24,6 @@
#ifndef __CROS_EC_LPC_MEC_H
#define __CROS_EC_LPC_MEC_H
-#include <linux/mfd/cros_ec_commands.h>
-
enum cros_ec_lpc_mec_emi_access_mode {
/* 8-bit access */
ACCESS_TYPE_BYTE = 0x0,
@@ -45,27 +43,23 @@ enum cros_ec_lpc_mec_io_type {
MEC_IO_WRITE,
};
-/* Access IO ranges 0x800 thru 0x9ff using EMI interface instead of LPC */
-#define MEC_EMI_RANGE_START EC_HOST_CMD_REGION0
-#define MEC_EMI_RANGE_END (EC_LPC_ADDR_MEMMAP + EC_MEMMAP_SIZE)
-
/* EMI registers are relative to base */
-#define MEC_EMI_BASE 0x800
-#define MEC_EMI_HOST_TO_EC (MEC_EMI_BASE + 0)
-#define MEC_EMI_EC_TO_HOST (MEC_EMI_BASE + 1)
-#define MEC_EMI_EC_ADDRESS_B0 (MEC_EMI_BASE + 2)
-#define MEC_EMI_EC_ADDRESS_B1 (MEC_EMI_BASE + 3)
-#define MEC_EMI_EC_DATA_B0 (MEC_EMI_BASE + 4)
-#define MEC_EMI_EC_DATA_B1 (MEC_EMI_BASE + 5)
-#define MEC_EMI_EC_DATA_B2 (MEC_EMI_BASE + 6)
-#define MEC_EMI_EC_DATA_B3 (MEC_EMI_BASE + 7)
+#define MEC_EMI_HOST_TO_EC(MEC_EMI_BASE) ((MEC_EMI_BASE) + 0)
+#define MEC_EMI_EC_TO_HOST(MEC_EMI_BASE) ((MEC_EMI_BASE) + 1)
+#define MEC_EMI_EC_ADDRESS_B0(MEC_EMI_BASE) ((MEC_EMI_BASE) + 2)
+#define MEC_EMI_EC_ADDRESS_B1(MEC_EMI_BASE) ((MEC_EMI_BASE) + 3)
+#define MEC_EMI_EC_DATA_B0(MEC_EMI_BASE) ((MEC_EMI_BASE) + 4)
+#define MEC_EMI_EC_DATA_B1(MEC_EMI_BASE) ((MEC_EMI_BASE) + 5)
+#define MEC_EMI_EC_DATA_B2(MEC_EMI_BASE) ((MEC_EMI_BASE) + 6)
+#define MEC_EMI_EC_DATA_B3(MEC_EMI_BASE) ((MEC_EMI_BASE) + 7)
-/*
- * cros_ec_lpc_mec_init
+/**
+ * cros_ec_lpc_mec_init() - Initialize MEC I/O.
*
- * Initialize MEC I/O.
+ * @base: MEC EMI Base address
+ * @end: MEC EMI End address
*/
-void cros_ec_lpc_mec_init(void);
+void cros_ec_lpc_mec_init(unsigned int base, unsigned int end);
/*
* cros_ec_lpc_mec_destroy
@@ -74,6 +68,17 @@ void cros_ec_lpc_mec_init(void);
*/
void cros_ec_lpc_mec_destroy(void);
+/**
+ * cros_ec_lpc_mec_in_range() - Determine if addresses are in MEC EMI range.
+ *
+ * @offset: Address offset
+ * @length: Number of bytes to check
+ *
+ * Return: 1 if in range, 0 if not, and -EINVAL on failure
+ * such as the mec range not being initialized
+ */
+int cros_ec_lpc_mec_in_range(unsigned int offset, unsigned int length);
+
/**
* cros_ec_lpc_io_bytes_mec - Read / write bytes to MEC EMI port
*
diff --git a/drivers/platform/chrome/cros_ec_lpc_reg.c b/drivers/platform/chrome/cros_ec_lpc_reg.c
index fc23d535c404..1d9f3b7ffb3e 100644
--- a/drivers/platform/chrome/cros_ec_lpc_reg.c
+++ b/drivers/platform/chrome/cros_ec_lpc_reg.c
@@ -59,51 +59,36 @@ static u8 lpc_write_bytes(unsigned int offset, unsigned int length, u8 *msg)
u8 cros_ec_lpc_read_bytes(unsigned int offset, unsigned int length, u8 *dest)
{
- if (length == 0)
- return 0;
-
- /* Access desired range through EMI interface */
- if (offset >= MEC_EMI_RANGE_START && offset <= MEC_EMI_RANGE_END) {
- /* Ensure we don't straddle EMI region */
- if (WARN_ON(offset + length - 1 > MEC_EMI_RANGE_END))
- return 0;
+ int in_range = cros_ec_lpc_mec_in_range(offset, length);
- return cros_ec_lpc_io_bytes_mec(MEC_IO_READ, offset, length,
- dest);
- }
-
- if (WARN_ON(offset + length > MEC_EMI_RANGE_START &&
- offset < MEC_EMI_RANGE_START))
+ if (in_range < 0)
return 0;
- return lpc_read_bytes(offset, length, dest);
+ return in_range ?
+ cros_ec_lpc_io_bytes_mec(MEC_IO_READ,
+ offset - EC_HOST_CMD_REGION0,
+ length, dest) :
+ lpc_read_bytes(offset, length, dest);
}
u8 cros_ec_lpc_write_bytes(unsigned int offset, unsigned int length, u8 *msg)
{
- if (length == 0)
- return 0;
-
- /* Access desired range through EMI interface */
- if (offset >= MEC_EMI_RANGE_START && offset <= MEC_EMI_RANGE_END) {
- /* Ensure we don't straddle EMI region */
- if (WARN_ON(offset + length - 1 > MEC_EMI_RANGE_END))
- return 0;
+ int in_range = cros_ec_lpc_mec_in_range(offset, length);
- return cros_ec_lpc_io_bytes_mec(MEC_IO_WRITE, offset, length,
- msg);
- }
-
- if (WARN_ON(offset + length > MEC_EMI_RANGE_START &&
- offset < MEC_EMI_RANGE_START))
+ if (in_range < 0)
return 0;
- return lpc_write_bytes(offset, length, msg);
+ return in_range ?
+ cros_ec_lpc_io_bytes_mec(MEC_IO_WRITE,
+ offset - EC_HOST_CMD_REGION0,
+ length, msg) :
+ lpc_write_bytes(offset, length, msg);
}
void cros_ec_lpc_reg_init(void)
{
- cros_ec_lpc_mec_init();
+ cros_ec_lpc_mec_init(EC_HOST_CMD_REGION0,
+ EC_LPC_ADDR_MEMMAP + EC_MEMMAP_SIZE);
}
void cros_ec_lpc_reg_destroy(void)
--
2.20.1.321.g9e740568ce-goog
^ permalink raw reply related
* [PATCH v4 2/9] platform/chrome: Add new driver for Wilco EC
From: Nick Crews @ 2019-01-23 18:33 UTC (permalink / raw)
To: linux-kernel
Cc: sjg, dmitry.torokhov, sre, linux-input, groeck, dlaurie,
Duncan Laurie, Enric Balletbo i Serra, Nick Crews, Nick Crews,
Benson Leung
In-Reply-To: <20190123183325.92946-1-ncrews@chromium.org>
From: Duncan Laurie <dlaurie@google.com>
This EC is an incompatible variant of the typical Chrome OS embedded
controller. It uses the same low-level communication and a similar
protocol with some significant differences. The EC firmware does
not support the same mailbox commands so it is not registered as a
cros_ec device type.
Signed-off-by: Duncan Laurie <dlaurie@google.com>
Acked-for-chrome-platform-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Signed-off-by: Nick Crews <ncrews@chromium.org>
---
Changes in v4:
- add #define DRV_NAME
- remove redundant Kconfig nesting
- changed my email to @chromium.org
- Add better explanation of what core.c does
Changes in v3:
- remove unused ret in probe()
- Add newline spacer in probe()
- rm unnecessary res in get_resource()
- s/8bit/8-bit
- rm first sleep when sending command to EC
Changes in v2:
- Remove COMPILE_TEST from Kconfig because inb()/outb()
won't work on anything but X86
- Add myself as module author
- Tweak mailbox()
drivers/platform/chrome/Kconfig | 4 +-
drivers/platform/chrome/Makefile | 2 +
drivers/platform/chrome/wilco_ec/Kconfig | 11 +
drivers/platform/chrome/wilco_ec/Makefile | 4 +
drivers/platform/chrome/wilco_ec/core.c | 106 +++++++++
drivers/platform/chrome/wilco_ec/mailbox.c | 236 +++++++++++++++++++++
include/linux/platform_data/wilco-ec.h | 138 ++++++++++++
7 files changed, 500 insertions(+), 1 deletion(-)
create mode 100644 drivers/platform/chrome/wilco_ec/Kconfig
create mode 100644 drivers/platform/chrome/wilco_ec/Makefile
create mode 100644 drivers/platform/chrome/wilco_ec/core.c
create mode 100644 drivers/platform/chrome/wilco_ec/mailbox.c
create mode 100644 include/linux/platform_data/wilco-ec.h
diff --git a/drivers/platform/chrome/Kconfig b/drivers/platform/chrome/Kconfig
index 16b1615958aa..bf889adfd4ef 100644
--- a/drivers/platform/chrome/Kconfig
+++ b/drivers/platform/chrome/Kconfig
@@ -49,6 +49,8 @@ config CHROMEOS_TBMC
To compile this driver as a module, choose M here: the
module will be called chromeos_tbmc.
+source "drivers/platform/chrome/wilco_ec/Kconfig"
+
config CROS_EC_CTL
tristate
@@ -86,7 +88,7 @@ config CROS_EC_LPC
config CROS_EC_LPC_MEC
bool "ChromeOS Embedded Controller LPC Microchip EC (MEC) variant"
- depends on CROS_EC_LPC
+ depends on CROS_EC_LPC || WILCO_EC
default n
help
If you say Y here, a variant LPC protocol for the Microchip EC
diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile
index cd591bf872bb..7242ee2b13c5 100644
--- a/drivers/platform/chrome/Makefile
+++ b/drivers/platform/chrome/Makefile
@@ -13,3 +13,5 @@ cros_ec_lpcs-$(CONFIG_CROS_EC_LPC_MEC) += cros_ec_lpc_mec.o
obj-$(CONFIG_CROS_EC_LPC) += cros_ec_lpcs.o
obj-$(CONFIG_CROS_EC_PROTO) += cros_ec_proto.o
obj-$(CONFIG_CROS_KBD_LED_BACKLIGHT) += cros_kbd_led_backlight.o
+
+obj-$(CONFIG_WILCO_EC) += wilco_ec/
diff --git a/drivers/platform/chrome/wilco_ec/Kconfig b/drivers/platform/chrome/wilco_ec/Kconfig
new file mode 100644
index 000000000000..20945a301ec6
--- /dev/null
+++ b/drivers/platform/chrome/wilco_ec/Kconfig
@@ -0,0 +1,11 @@
+config WILCO_EC
+ tristate "ChromeOS Wilco Embedded Controller"
+ depends on ACPI && X86
+ select CROS_EC_LPC_MEC
+ help
+ If you say Y here, you get support for talking to the ChromeOS
+ Wilco EC over an eSPI bus. This uses a simple byte-level protocol
+ with a checksum.
+
+ To compile this driver as a module, choose M here: the
+ module will be called wilco_ec.
diff --git a/drivers/platform/chrome/wilco_ec/Makefile b/drivers/platform/chrome/wilco_ec/Makefile
new file mode 100644
index 000000000000..03b32301dc61
--- /dev/null
+++ b/drivers/platform/chrome/wilco_ec/Makefile
@@ -0,0 +1,4 @@
+# SPDX-License-Identifier: GPL-2.0
+
+wilco_ec-objs := core.o mailbox.o
+obj-$(CONFIG_WILCO_EC) += wilco_ec.o
diff --git a/drivers/platform/chrome/wilco_ec/core.c b/drivers/platform/chrome/wilco_ec/core.c
new file mode 100644
index 000000000000..7dcb3bde132a
--- /dev/null
+++ b/drivers/platform/chrome/wilco_ec/core.c
@@ -0,0 +1,106 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Core driver for Wilco Embedded Controller
+ *
+ * Copyright 2018 Google LLC
+ *
+ * This is the entry point for the drivers that control the Wilco EC.
+ * This driver is responsible for several tasks:
+ * - Initialize the register interface that is used by wilco_ec_mailbox()
+ * - Create a platform device which is picked up by the debugfs driver
+ * - Create a platform device which is picked up by the RTC driver
+ * - Initialize the sysfs entries
+ * - Set up event handling
+ */
+
+#include <linux/acpi.h>
+#include <linux/device.h>
+#include <linux/ioport.h>
+#include <linux/module.h>
+#include <linux/platform_data/wilco-ec.h>
+#include <linux/platform_device.h>
+
+#include "../cros_ec_lpc_mec.h"
+
+#define DRV_NAME "wilco-ec"
+
+static struct resource *wilco_get_resource(struct platform_device *pdev,
+ int index)
+{
+ struct device *dev = &pdev->dev;
+ struct resource *res;
+
+ res = platform_get_resource(pdev, IORESOURCE_IO, index);
+ if (!res) {
+ dev_dbg(dev, "couldn't find IO resource %d\n", index);
+ return res;
+ }
+
+ return devm_request_region(dev, res->start, resource_size(res),
+ dev_name(dev));
+}
+
+static int wilco_ec_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct wilco_ec_device *ec;
+
+ ec = devm_kzalloc(dev, sizeof(*ec), GFP_KERNEL);
+ if (!ec)
+ return -ENOMEM;
+
+ platform_set_drvdata(pdev, ec);
+ ec->dev = dev;
+ mutex_init(&ec->mailbox_lock);
+
+ /* Largest data buffer size requirement is extended data response */
+ ec->data_size = sizeof(struct wilco_ec_response) +
+ EC_MAILBOX_DATA_SIZE_EXTENDED;
+ ec->data_buffer = devm_kzalloc(dev, ec->data_size, GFP_KERNEL);
+ if (!ec->data_buffer)
+ return -ENOMEM;
+
+ /* Prepare access to IO regions provided by ACPI */
+ ec->io_data = wilco_get_resource(pdev, 0); /* Host Data */
+ ec->io_command = wilco_get_resource(pdev, 1); /* Host Command */
+ ec->io_packet = wilco_get_resource(pdev, 2); /* MEC EMI */
+ if (!ec->io_data || !ec->io_command || !ec->io_packet)
+ return -ENODEV;
+
+ /* Initialize cros_ec register interface for communication */
+ cros_ec_lpc_mec_init(ec->io_packet->start,
+ ec->io_packet->start + EC_MAILBOX_DATA_SIZE);
+
+ return 0;
+}
+
+static int wilco_ec_remove(struct platform_device *pdev)
+{
+ /* Teardown cros_ec interface */
+ cros_ec_lpc_mec_destroy();
+
+ return 0;
+}
+
+static const struct acpi_device_id wilco_ec_acpi_device_ids[] = {
+ { "GOOG000C", 0 },
+ { }
+};
+MODULE_DEVICE_TABLE(acpi, wilco_ec_acpi_device_ids);
+
+static struct platform_driver wilco_ec_driver = {
+ .driver = {
+ .name = DRV_NAME,
+ .acpi_match_table = wilco_ec_acpi_device_ids,
+ },
+ .probe = wilco_ec_probe,
+ .remove = wilco_ec_remove,
+};
+
+module_platform_driver(wilco_ec_driver);
+
+MODULE_AUTHOR("Nick Crews <ncrews@chromium.org>");
+MODULE_AUTHOR("Duncan Laurie <dlaurie@chromium.org>");
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("ChromeOS Wilco Embedded Controller driver");
+MODULE_ALIAS("platform:" DRV_NAME);
diff --git a/drivers/platform/chrome/wilco_ec/mailbox.c b/drivers/platform/chrome/wilco_ec/mailbox.c
new file mode 100644
index 000000000000..4920a86d83f9
--- /dev/null
+++ b/drivers/platform/chrome/wilco_ec/mailbox.c
@@ -0,0 +1,236 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Mailbox interface for Wilco Embedded Controller
+ *
+ * Copyright 2018 Google LLC
+ *
+ * The Wilco EC is similar to a typical ChromeOS embedded controller.
+ * It uses the same MEC based low-level communication and a similar
+ * protocol, but with some important differences. The EC firmware does
+ * not support the same mailbox commands so it is not registered as a
+ * cros_ec device type.
+ *
+ * Most messages follow a standard format, but there are some exceptions
+ * and an interface is provided to do direct/raw transactions that do not
+ * make assumptions about byte placement.
+ */
+
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/io.h>
+#include <linux/platform_data/wilco-ec.h>
+#include <linux/platform_device.h>
+
+#include "../cros_ec_lpc_mec.h"
+
+/* Version of mailbox interface */
+#define EC_MAILBOX_VERSION 0
+
+/* Command to start mailbox transaction */
+#define EC_MAILBOX_START_COMMAND 0xda
+
+/* Version of EC protocol */
+#define EC_MAILBOX_PROTO_VERSION 3
+
+/* Number of header bytes to be counted as data bytes */
+#define EC_MAILBOX_DATA_EXTRA 2
+
+/* Maximum timeout */
+#define EC_MAILBOX_TIMEOUT HZ
+
+/* EC response flags */
+#define EC_CMDR_DATA BIT(0) /* Data ready for host to read */
+#define EC_CMDR_PENDING BIT(1) /* Write pending to EC */
+#define EC_CMDR_BUSY BIT(2) /* EC is busy processing a command */
+#define EC_CMDR_CMD BIT(3) /* Last host write was a command */
+
+/**
+ * wilco_ec_response_timed_out() - Wait for EC response.
+ * @ec: EC device.
+ *
+ * Return: true if EC timed out, false if EC did not time out.
+ */
+static bool wilco_ec_response_timed_out(struct wilco_ec_device *ec)
+{
+ unsigned long timeout = jiffies + EC_MAILBOX_TIMEOUT;
+
+ do {
+ if (!(inb(ec->io_command->start) &
+ (EC_CMDR_PENDING | EC_CMDR_BUSY)))
+ return false;
+ usleep_range(100, 200);
+ } while (time_before(jiffies, timeout));
+
+ return true;
+}
+
+/**
+ * wilco_ec_checksum() - Compute 8-bit checksum over data range.
+ * @data: Data to checksum.
+ * @size: Number of bytes to checksum.
+ *
+ * Return: 8-bit checksum of provided data.
+ */
+static u8 wilco_ec_checksum(const void *data, size_t size)
+{
+ u8 *data_bytes = (u8 *)data;
+ u8 checksum = 0;
+ size_t i;
+
+ for (i = 0; i < size; i++)
+ checksum += data_bytes[i];
+
+ return checksum;
+}
+
+/**
+ * wilco_ec_prepare() - Prepare the request structure for the EC.
+ * @msg: EC message with request information.
+ * @rq: EC request structure to fill.
+ */
+static void wilco_ec_prepare(struct wilco_ec_message *msg,
+ struct wilco_ec_request *rq)
+{
+ memset(rq, 0, sizeof(*rq));
+
+ /* Handle messages without trimming bytes from the request */
+ if (msg->request_size && msg->flags & WILCO_EC_FLAG_RAW_REQUEST) {
+ rq->reserved_raw = *(u8 *)msg->request_data;
+ msg->request_size--;
+ memmove(msg->request_data, msg->request_data + 1,
+ msg->request_size);
+ }
+
+ /* Fill in request packet */
+ rq->struct_version = EC_MAILBOX_PROTO_VERSION;
+ rq->mailbox_id = msg->type;
+ rq->mailbox_version = EC_MAILBOX_VERSION;
+ rq->data_size = msg->request_size + EC_MAILBOX_DATA_EXTRA;
+ rq->command = msg->command;
+
+ /* Checksum header and data */
+ rq->checksum = wilco_ec_checksum(rq, sizeof(*rq));
+ rq->checksum += wilco_ec_checksum(msg->request_data, msg->request_size);
+ rq->checksum = -rq->checksum;
+}
+
+/**
+ * wilco_ec_transfer() - Perform actual data transfer.
+ * @ec: EC device.
+ * @msg: EC message data for request and response.
+ * @rq: Filled in request structure
+ *
+ * Context: ec->mailbox_lock should be held while using this function.
+ * Return: number of bytes received or negative error code on failure.
+ */
+int wilco_ec_transfer(struct wilco_ec_device *ec, struct wilco_ec_message *msg,
+ struct wilco_ec_request *rq)
+{
+ struct wilco_ec_response *rs;
+ u8 checksum;
+ u8 flag;
+ size_t size;
+
+ /* Write request header, then data */
+ cros_ec_lpc_io_bytes_mec(MEC_IO_WRITE, 0, sizeof(*rq), (u8 *)rq);
+ cros_ec_lpc_io_bytes_mec(MEC_IO_WRITE, sizeof(*rq), msg->request_size,
+ msg->request_data);
+
+ /* Start the command */
+ outb(EC_MAILBOX_START_COMMAND, ec->io_command->start);
+
+ /* Wait for it to complete */
+ if (wilco_ec_response_timed_out(ec)) {
+ dev_dbg(ec->dev, "response timed out\n");
+ return -ETIMEDOUT;
+ }
+
+ /* For some commands (eg shutdown) the EC will not respond, that's OK */
+ if (msg->flags & WILCO_EC_FLAG_NO_RESPONSE) {
+ dev_dbg(ec->dev, "EC does not respond to this command\n");
+ return 0;
+ }
+
+ /* Check result */
+ flag = inb(ec->io_data->start);
+ if (flag) {
+ dev_dbg(ec->dev, "bad response: 0x%02x\n", flag);
+ return -EIO;
+ }
+
+ if (msg->flags & WILCO_EC_FLAG_EXTENDED_DATA)
+ size = EC_MAILBOX_DATA_SIZE_EXTENDED;
+ else
+ size = EC_MAILBOX_DATA_SIZE;
+
+ /* Read back response */
+ rs = ec->data_buffer;
+ checksum = cros_ec_lpc_io_bytes_mec(MEC_IO_READ, 0,
+ sizeof(*rs) + size, (u8 *)rs);
+ if (checksum) {
+ dev_dbg(ec->dev, "bad packet checksum 0x%02x\n", rs->checksum);
+ return -EBADMSG;
+ }
+
+ /* Check that the EC reported success */
+ msg->result = rs->result;
+ if (msg->result) {
+ dev_dbg(ec->dev, "bad response: 0x%02x\n", msg->result);
+ return -EBADMSG;
+ }
+
+ /* Check the returned data size, skipping the header */
+ if (rs->data_size != size) {
+ dev_dbg(ec->dev, "unexpected packet size (%u != %zu)",
+ rs->data_size, size);
+ return -EMSGSIZE;
+ }
+
+ /* Skip 1 response data byte unless specified */
+ size = (msg->flags & WILCO_EC_FLAG_RAW_RESPONSE) ? 0 : 1;
+ if ((ssize_t) rs->data_size - size < msg->response_size) {
+ dev_dbg(ec->dev, "response data too short (%zd < %zu)",
+ (ssize_t) rs->data_size - size, msg->response_size);
+ return -EMSGSIZE;
+ }
+
+ /* Ignore response data bytes as requested */
+ memcpy(msg->response_data, rs->data + size, msg->response_size);
+
+ /* Return actual amount of data received */
+ return msg->response_size;
+}
+
+/**
+ * wilco_ec_mailbox() - Send EC request and receive EC response.
+ * @ec: EC device.
+ * @msg: EC message data for request and response.
+ *
+ * On entry msg->type, msg->flags, msg->command, msg->request_size,
+ * msg->response_size, and msg->request_data should all be filled in.
+ *
+ * On exit msg->result and msg->response_data will be filled.
+ *
+ * Return: number of bytes received or negative error code on failure.
+ */
+int wilco_ec_mailbox(struct wilco_ec_device *ec, struct wilco_ec_message *msg)
+{
+ struct wilco_ec_request *rq;
+ int ret;
+
+ dev_dbg(ec->dev, "cmd=%02x type=%04x flags=%02x rslen=%zu rqlen=%zu\n",
+ msg->command, msg->type, msg->flags, msg->response_size,
+ msg->request_size);
+
+ /* Prepare request packet */
+ rq = ec->data_buffer;
+ wilco_ec_prepare(msg, rq);
+
+ mutex_lock(&ec->mailbox_lock);
+ ret = wilco_ec_transfer(ec, msg, rq);
+ mutex_unlock(&ec->mailbox_lock);
+
+ return ret;
+
+}
+EXPORT_SYMBOL_GPL(wilco_ec_mailbox);
diff --git a/include/linux/platform_data/wilco-ec.h b/include/linux/platform_data/wilco-ec.h
new file mode 100644
index 000000000000..5477b8802f81
--- /dev/null
+++ b/include/linux/platform_data/wilco-ec.h
@@ -0,0 +1,138 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * ChromeOS Wilco Embedded Controller
+ *
+ * Copyright 2018 Google LLC
+ */
+
+#ifndef WILCO_EC_H
+#define WILCO_EC_H
+
+#include <linux/device.h>
+#include <linux/kernel.h>
+
+#define WILCO_EC_FLAG_NO_RESPONSE BIT(0) /* EC does not respond */
+#define WILCO_EC_FLAG_EXTENDED_DATA BIT(1) /* EC returns 256 data bytes */
+#define WILCO_EC_FLAG_RAW_REQUEST BIT(2) /* Do not trim request data */
+#define WILCO_EC_FLAG_RAW_RESPONSE BIT(3) /* Do not trim response data */
+#define WILCO_EC_FLAG_RAW (WILCO_EC_FLAG_RAW_REQUEST | \
+ WILCO_EC_FLAG_RAW_RESPONSE)
+
+/* Normal commands have a maximum 32 bytes of data */
+#define EC_MAILBOX_DATA_SIZE 32
+
+/* Extended commands have 256 bytes of response data */
+#define EC_MAILBOX_DATA_SIZE_EXTENDED 256
+
+/**
+ * struct wilco_ec_device - Wilco Embedded Controller handle.
+ * @dev: Device handle.
+ * @mailbox_lock: Mutex to ensure one mailbox command at a time.
+ * @io_command: I/O port for mailbox command. Provided by ACPI.
+ * @io_data: I/O port for mailbox data. Provided by ACPI.
+ * @io_packet: I/O port for mailbox packet data. Provided by ACPI.
+ * @data_buffer: Buffer used for EC communication. The same buffer
+ * is used to hold the request and the response.
+ * @data_size: Size of the data buffer used for EC communication.
+ */
+struct wilco_ec_device {
+ struct device *dev;
+ struct mutex mailbox_lock;
+ struct resource *io_command;
+ struct resource *io_data;
+ struct resource *io_packet;
+ void *data_buffer;
+ size_t data_size;
+};
+
+/**
+ * enum wilco_ec_msg_type - Message type to select a set of command codes.
+ * @WILCO_EC_MSG_LEGACY: Legacy EC messages for standard EC behavior.
+ * @WILCO_EC_MSG_PROPERTY: Get/Set/Sync EC controlled NVRAM property.
+ * @WILCO_EC_MSG_TELEMETRY: Telemetry data provided by the EC.
+ */
+enum wilco_ec_msg_type {
+ WILCO_EC_MSG_LEGACY = 0x00f0,
+ WILCO_EC_MSG_PROPERTY = 0x00f2,
+ WILCO_EC_MSG_TELEMETRY = 0x00f5,
+};
+
+/**
+ * struct wilco_ec_message - Request and response message.
+ * @type: Mailbox message type.
+ * @flags: Message flags.
+ * @command: Mailbox command code.
+ * @result: Result code from the EC. Non-zero indicates an error.
+ * @request_size: Number of bytes to send to the EC.
+ * @request_data: Buffer containing the request data.
+ * @response_size: Number of bytes expected from the EC.
+ * This is 32 by default and 256 if the flag
+ * is set for %WILCO_EC_FLAG_EXTENDED_DATA
+ * @response_data: Buffer containing the response data, should be
+ * response_size bytes and allocated by caller.
+ */
+struct wilco_ec_message {
+ enum wilco_ec_msg_type type;
+ u8 flags;
+ u8 command;
+ u8 result;
+ size_t request_size;
+ void *request_data;
+ size_t response_size;
+ void *response_data;
+};
+
+/**
+ * struct wilco_ec_request - Mailbox request message format.
+ * @struct_version: Should be %EC_MAILBOX_PROTO_VERSION
+ * @checksum: Sum of all bytes must be 0.
+ * @mailbox_id: Mailbox identifier, specifies the command set.
+ * @mailbox_version: Mailbox interface version %EC_MAILBOX_VERSION
+ * @reserved: Set to zero.
+ * @data_size: Length of request, data + last 2 bytes of the header.
+ * @command: Mailbox command code, unique for each mailbox_id set.
+ * @reserved_raw: Set to zero for most commands, but is used by
+ * some command types and for raw commands.
+ */
+struct wilco_ec_request {
+ u8 struct_version;
+ u8 checksum;
+ u16 mailbox_id;
+ u8 mailbox_version;
+ u8 reserved;
+ u16 data_size;
+ u8 command;
+ u8 reserved_raw;
+} __packed;
+
+/**
+ * struct wilco_ec_response - Mailbox response message format.
+ * @struct_version: Should be %EC_MAILBOX_PROTO_VERSION
+ * @checksum: Sum of all bytes must be 0.
+ * @result: Result code from the EC. Non-zero indicates an error.
+ * @data_size: Length of the response data buffer.
+ * @reserved: Set to zero.
+ * @mbox0: EC returned data at offset 0 is unused (always 0) so this byte
+ * is treated as part of the header instead of the data.
+ * @data: Response data buffer. Max size is %EC_MAILBOX_DATA_SIZE_EXTENDED.
+ */
+struct wilco_ec_response {
+ u8 struct_version;
+ u8 checksum;
+ u16 result;
+ u16 data_size;
+ u8 reserved[2];
+ u8 mbox0;
+ u8 data[0];
+} __packed;
+
+/**
+ * wilco_ec_mailbox() - Send request to the EC and receive the response.
+ * @ec: Wilco EC device.
+ * @msg: Wilco EC message.
+ *
+ * Return: Number of bytes received or negative error code on failure.
+ */
+int wilco_ec_mailbox(struct wilco_ec_device *ec, struct wilco_ec_message *msg);
+
+#endif /* WILCO_EC_H */
--
2.20.1.321.g9e740568ce-goog
^ 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