* [PATCH v2 1/2] dt-bindings: input: sun4i-lradc-keys: Add A64 compatible
From: Luca Weiss @ 2019-06-04 17:21 UTC (permalink / raw)
Cc: Luca Weiss, Hans de Goede, Dmitry Torokhov, Rob Herring,
Mark Rutland, Maxime Ripard, Chen-Yu Tsai,
open list:SUN4I LOW RES ADC ATTACHED TABLET KEYS DRIVER,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
moderated list:ARM/Allwinner sunXi SoC support, open list,
~martijnbraam/pmos-upstream
Add the A64 compatible with a fallback to the A83T compatible.
Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
---
Changes from v1:
- New patch. Document new compatible string.
Documentation/devicetree/bindings/input/sun4i-lradc-keys.txt | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/input/sun4i-lradc-keys.txt b/Documentation/devicetree/bindings/input/sun4i-lradc-keys.txt
index 496125c6bfb7..507b737612ea 100644
--- a/Documentation/devicetree/bindings/input/sun4i-lradc-keys.txt
+++ b/Documentation/devicetree/bindings/input/sun4i-lradc-keys.txt
@@ -5,6 +5,7 @@ Required properties:
- compatible: should be one of the following string:
"allwinner,sun4i-a10-lradc-keys"
"allwinner,sun8i-a83t-r-lradc"
+ "allwinner,sun50i-a64-lradc", "allwinner,sun8i-a83t-r-lradc"
- reg: mmio address range of the chip
- interrupts: interrupt to which the chip is connected
- vref-supply: powersupply for the lradc reference voltage
--
2.21.0
^ permalink raw reply related
* Re: [PATCH 00/10] Move part of cros-ec out of MFD subsystem
From: Andy Shevchenko @ 2019-06-04 16:07 UTC (permalink / raw)
To: Enric Balletbo i Serra
Cc: Kate Stewart, Enno Luebbers, gwendal, Banajit Goswami,
Heiko Stuebner, Alexandre Belloni, linux-doc, Wolfram Sang,
Mark Brown, Manivannan Sadhasivam, Juergen Fitschen, alsa-devel,
Stefan Agner, Sebastian Reichel, Jilayne Lovejoy,
Benjamin Tissoires, Dmitry Torokhov, linux-i2c,
Peter Meerwald-Stadler, Dong Aisheng, Guenter Roeck,
Lorenzo Pieralisi, Ravi Chandra Sadineni <rav>
In-Reply-To: <20190604152019.16100-1-enric.balletbo@collabora.com>
On Tue, Jun 04, 2019 at 05:20:09PM +0200, Enric Balletbo i Serra wrote:
> Hi,
>
> This is the first attempt to clean up a bit more the cros-ec drivers
> to have a better separation on what is part of the MFD subsystem and what
> is part of platform/chrome.
>
> It'd be really nice have some reviews, acks and tested on different
> platforms from the chromiumos people before merge all this patchset, as
> this moves a lot of code.
>
> The major changes introduced by this patchset are:
> 1. Move the core driver to platform/chrome, as is not really related to
> an MFD device driver.
> 2. Create a new misc chardev driver to replace the chardev bits from
> cros-ec-dev (MFD)
> 3. Added some convenience structs in cros-ec-dev (MFD) to easy add more
> subdrivers and avoid to add more boiler plate.
>
> Once applied we have moved all the code to platform/chrome except the
> cros-ec-dev driver, which is the one that instantiates the different
> subdrivers as cells of the MFD device.
>
> I tested the following patches on Veyron, Kevin, Samus, Peach Pi and
> Peach Pit without noticing any problem, but they would need a lot of
> more tests. I'll continue testing while the reviewing process of this
> patchset.
>
If my tag helps, here it is:
Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Waiting for your feedback,
> Enric
>
>
> Enric Balletbo i Serra (10):
> mfd / platform: cros_ec: Handle chained ECs as platform devices
> mfd / platform: cros_ec: Move cros-ec core driver out from MFD
> mfd / platform: cros_ec: Miscellaneous character device to talk with
> the EC
> mfd: cros_ec: Switch to use the new cros-ec-chardev driver
> mfd / platform: cros_ec: Rename config to a better name
> mfd / platform: cros_ec: Reorganize platform and mfd includes
> mfd: cros_ec: Update with SPDX Licence identifier and fix description
> mfd: cros_ec: Use kzalloc and cros_ec_cmd_xfer_status helper
> mfd: cros_ec: Add convenience struct to define dedicated CrOS EC MCUs
> mfd: cros_ec: Add convenience struct to define autodetectable CrOS EC
> subdevices
>
> Documentation/ioctl/ioctl-number.txt | 2 +-
> drivers/extcon/Kconfig | 2 +-
> drivers/extcon/extcon-usbc-cros-ec.c | 3 +-
> drivers/hid/Kconfig | 2 +-
> drivers/hid/hid-google-hammer.c | 4 +-
> drivers/i2c/busses/Kconfig | 2 +-
> drivers/i2c/busses/i2c-cros-ec-tunnel.c | 4 +-
> drivers/iio/accel/cros_ec_accel_legacy.c | 3 +-
> drivers/iio/common/cros_ec_sensors/Kconfig | 2 +-
> .../common/cros_ec_sensors/cros_ec_sensors.c | 3 +-
> .../cros_ec_sensors/cros_ec_sensors_core.c | 3 +-
> drivers/iio/light/cros_ec_light_prox.c | 3 +-
> drivers/iio/pressure/cros_ec_baro.c | 3 +-
> drivers/input/keyboard/Kconfig | 2 +-
> drivers/input/keyboard/cros_ec_keyb.c | 4 +-
> drivers/media/platform/Kconfig | 3 +-
> .../media/platform/cros-ec-cec/cros-ec-cec.c | 4 +-
> drivers/mfd/Kconfig | 26 +-
> drivers/mfd/Makefile | 4 +-
> drivers/mfd/cros_ec_dev.c | 433 +++++-------------
> drivers/platform/chrome/Kconfig | 48 +-
> drivers/platform/chrome/Makefile | 2 +
> drivers/{mfd => platform/chrome}/cros_ec.c | 64 +--
> drivers/platform/chrome/cros_ec_chardev.c | 279 +++++++++++
> drivers/platform/chrome/cros_ec_debugfs.c | 3 +-
> drivers/platform/chrome/cros_ec_i2c.c | 12 +-
> drivers/platform/chrome/cros_ec_lightbar.c | 3 +-
> drivers/platform/chrome/cros_ec_lpc.c | 7 +-
> drivers/platform/chrome/cros_ec_lpc_reg.c | 4 +-
> drivers/platform/chrome/cros_ec_proto.c | 3 +-
> drivers/platform/chrome/cros_ec_rpmsg.c | 6 +-
> drivers/platform/chrome/cros_ec_spi.c | 12 +-
> drivers/platform/chrome/cros_ec_sysfs.c | 3 +-
> drivers/platform/chrome/cros_ec_trace.c | 2 +-
> drivers/platform/chrome/cros_ec_trace.h | 4 +-
> drivers/platform/chrome/cros_ec_vbc.c | 3 +-
> drivers/platform/chrome/cros_usbpd_logger.c | 5 +-
> drivers/power/supply/Kconfig | 2 +-
> drivers/power/supply/cros_usbpd-charger.c | 5 +-
> drivers/pwm/Kconfig | 2 +-
> drivers/pwm/pwm-cros-ec.c | 4 +-
> drivers/rtc/Kconfig | 2 +-
> drivers/rtc/rtc-cros-ec.c | 3 +-
> .../linux/iio/common/cros_ec_sensors_core.h | 3 +-
> include/linux/mfd/cros_ec.h | 302 +-----------
> .../{mfd => platform_data}/cros_ec_commands.h | 0
> include/linux/platform_data/cros_ec_proto.h | 315 +++++++++++++
> .../uapi/linux/cros_ec_chardev.h | 18 +-
> sound/soc/codecs/cros_ec_codec.c | 4 +-
> sound/soc/qcom/Kconfig | 2 +-
> 50 files changed, 902 insertions(+), 732 deletions(-)
> rename drivers/{mfd => platform/chrome}/cros_ec.c (85%)
> create mode 100644 drivers/platform/chrome/cros_ec_chardev.c
> rename include/linux/{mfd => platform_data}/cros_ec_commands.h (100%)
> create mode 100644 include/linux/platform_data/cros_ec_proto.h
> rename drivers/mfd/cros_ec_dev.h => include/uapi/linux/cros_ec_chardev.h (70%)
>
> --
> 2.20.1
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* [PATCH 06/10] mfd / platform: cros_ec: Reorganize platform and mfd includes
From: Enric Balletbo i Serra @ 2019-06-04 15:20 UTC (permalink / raw)
To: linux-kernel
Cc: gwendal, Guenter Roeck, Benson Leung, Lee Jones, kernel, dtor,
Mauro Carvalho Chehab, alsa-devel, Alessandro Zummo, linux-iio,
Fabien Lahoudere, Alexandre Belloni, linux-i2c, linux-rtc,
Heiko Stuebner, Brian Norris, Chanwoo Choi, Benjamin Tissoires,
Gustavo A. R. Silva, Sebastian Reichel, Rushikesh S Kadam
In-Reply-To: <20190604152019.16100-1-enric.balletbo@collabora.com>
There is a bit of mess between cros-ec mfd includes and platform
includes. For example, we have a linux/mfd/cros_ec.h include that
exports the interface implemented in platform/chrome/cros_ec_proto.c. Or
we have a linux/mfd/cros_ec_commands.h file that is non related to the
multifunction device (in the sense that is not exporting any function of
the mfd device). This causes crossed includes between mfd and
platform/chrome subsystems and makes the code difficult to read, apart
from creating 'curious' situations where a platform/chrome driver includes
a linux/mfd/cros_ec.h file just to get the exported functions that are
implemented in another platform/chrome driver.
In order to have a better separation on what the cros-ec multifunction
driver does and what the cros-ec core provides move and rework the
affected includes doing:
- Move cros_ec_commands.h to include/linux/platform_data/cros_ec_commands.h
- Get rid of the parts that are implemented in the platform/chrome/cros_ec_proto.c
driver from include/linux/mfd/cros_ec.h to a new file
include/linux/platform_data/cros_ec_proto.h
- Update all the drivers with the new includes, so
- Drivers that only need to know about the protocol include
- linux/platform_data/cros_ec_proto.h
- linux/platform_data/cros_ec_commands.h
- Drivers that need to know about the cros-ec mfd device also include
- linux/mfd/cros_ec.h
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---
drivers/extcon/extcon-usbc-cros-ec.c | 3 +-
drivers/hid/hid-google-hammer.c | 4 +-
drivers/i2c/busses/i2c-cros-ec-tunnel.c | 4 +-
drivers/iio/accel/cros_ec_accel_legacy.c | 3 +-
.../common/cros_ec_sensors/cros_ec_sensors.c | 3 +-
.../cros_ec_sensors/cros_ec_sensors_core.c | 3 +-
drivers/iio/light/cros_ec_light_prox.c | 3 +-
drivers/iio/pressure/cros_ec_baro.c | 3 +-
drivers/input/keyboard/cros_ec_keyb.c | 4 +-
.../media/platform/cros-ec-cec/cros-ec-cec.c | 4 +-
drivers/mfd/cros_ec_dev.c | 3 +-
drivers/platform/chrome/cros_ec.c | 3 +-
drivers/platform/chrome/cros_ec_chardev.c | 4 +-
drivers/platform/chrome/cros_ec_debugfs.c | 3 +-
drivers/platform/chrome/cros_ec_i2c.c | 4 +-
drivers/platform/chrome/cros_ec_lightbar.c | 3 +-
drivers/platform/chrome/cros_ec_lpc.c | 4 +-
drivers/platform/chrome/cros_ec_lpc_reg.c | 4 +-
drivers/platform/chrome/cros_ec_proto.c | 3 +-
drivers/platform/chrome/cros_ec_rpmsg.c | 4 +-
drivers/platform/chrome/cros_ec_spi.c | 4 +-
drivers/platform/chrome/cros_ec_sysfs.c | 3 +-
drivers/platform/chrome/cros_ec_trace.c | 2 +-
drivers/platform/chrome/cros_ec_trace.h | 4 +-
drivers/platform/chrome/cros_ec_vbc.c | 3 +-
drivers/platform/chrome/cros_usbpd_logger.c | 5 +-
drivers/power/supply/cros_usbpd-charger.c | 5 +-
drivers/pwm/pwm-cros-ec.c | 4 +-
drivers/rtc/rtc-cros-ec.c | 3 +-
.../linux/iio/common/cros_ec_sensors_core.h | 3 +-
include/linux/mfd/cros_ec.h | 306 -----------------
.../{mfd => platform_data}/cros_ec_commands.h | 0
include/linux/platform_data/cros_ec_proto.h | 315 ++++++++++++++++++
sound/soc/codecs/cros_ec_codec.c | 4 +-
34 files changed, 379 insertions(+), 351 deletions(-)
rename include/linux/{mfd => platform_data}/cros_ec_commands.h (100%)
create mode 100644 include/linux/platform_data/cros_ec_proto.h
diff --git a/drivers/extcon/extcon-usbc-cros-ec.c b/drivers/extcon/extcon-usbc-cros-ec.c
index 43c0a936ab82..5290cc2d19d9 100644
--- a/drivers/extcon/extcon-usbc-cros-ec.c
+++ b/drivers/extcon/extcon-usbc-cros-ec.c
@@ -6,10 +6,11 @@
#include <linux/extcon-provider.h>
#include <linux/kernel.h>
-#include <linux/mfd/cros_ec.h>
#include <linux/module.h>
#include <linux/notifier.h>
#include <linux/of.h>
+#include <linux/platform_data/cros_ec_commands.h>
+#include <linux/platform_data/cros_ec_proto.h>
#include <linux/platform_device.h>
#include <linux/slab.h>
#include <linux/sched.h>
diff --git a/drivers/hid/hid-google-hammer.c b/drivers/hid/hid-google-hammer.c
index ee5e0bdcf078..84f8c127ebdc 100644
--- a/drivers/hid/hid-google-hammer.c
+++ b/drivers/hid/hid-google-hammer.c
@@ -16,9 +16,9 @@
#include <linux/acpi.h>
#include <linux/hid.h>
#include <linux/leds.h>
-#include <linux/mfd/cros_ec.h>
-#include <linux/mfd/cros_ec_commands.h>
#include <linux/module.h>
+#include <linux/platform_data/cros_ec_commands.h>
+#include <linux/platform_data/cros_ec_proto.h>
#include <linux/platform_device.h>
#include <linux/pm_wakeup.h>
#include <asm/unaligned.h>
diff --git a/drivers/i2c/busses/i2c-cros-ec-tunnel.c b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
index 82bcd9a78759..c551aa96a2e3 100644
--- a/drivers/i2c/busses/i2c-cros-ec-tunnel.c
+++ b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
@@ -5,8 +5,8 @@
#include <linux/module.h>
#include <linux/i2c.h>
-#include <linux/mfd/cros_ec.h>
-#include <linux/mfd/cros_ec_commands.h>
+#include <linux/platform_data/cros_ec_commands.h>
+#include <linux/platform_data/cros_ec_proto.h>
#include <linux/platform_device.h>
#include <linux/slab.h>
diff --git a/drivers/iio/accel/cros_ec_accel_legacy.c b/drivers/iio/accel/cros_ec_accel_legacy.c
index 46bb2e421bb9..fd9a634f741e 100644
--- a/drivers/iio/accel/cros_ec_accel_legacy.c
+++ b/drivers/iio/accel/cros_ec_accel_legacy.c
@@ -18,9 +18,10 @@
#include <linux/iio/triggered_buffer.h>
#include <linux/kernel.h>
#include <linux/mfd/cros_ec.h>
-#include <linux/mfd/cros_ec_commands.h>
#include <linux/module.h>
#include <linux/slab.h>
+#include <linux/platform_data/cros_ec_commands.h>
+#include <linux/platform_data/cros_ec_proto.h>
#include <linux/platform_device.h>
#define DRV_NAME "cros-ec-accel-legacy"
diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
index 17af4e0fd5f8..40dc24ff0ee5 100644
--- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
+++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
@@ -17,8 +17,9 @@
#include <linux/iio/triggered_buffer.h>
#include <linux/kernel.h>
#include <linux/mfd/cros_ec.h>
-#include <linux/mfd/cros_ec_commands.h>
#include <linux/module.h>
+#include <linux/platform_data/cros_ec_commands.h>
+#include <linux/platform_data/cros_ec_proto.h>
#include <linux/platform_device.h>
#include <linux/slab.h>
diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
index 719a0df5aeeb..fd63315399ac 100644
--- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
+++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
@@ -14,9 +14,10 @@
#include <linux/iio/trigger_consumer.h>
#include <linux/kernel.h>
#include <linux/mfd/cros_ec.h>
-#include <linux/mfd/cros_ec_commands.h>
#include <linux/module.h>
#include <linux/slab.h>
+#include <linux/platform_data/cros_ec_commands.h>
+#include <linux/platform_data/cros_ec_proto.h>
#include <linux/platform_device.h>
static char *cros_ec_loc[] = {
diff --git a/drivers/iio/light/cros_ec_light_prox.c b/drivers/iio/light/cros_ec_light_prox.c
index 308ee6ff2e22..437e0eae9178 100644
--- a/drivers/iio/light/cros_ec_light_prox.c
+++ b/drivers/iio/light/cros_ec_light_prox.c
@@ -15,8 +15,9 @@
#include <linux/iio/trigger_consumer.h>
#include <linux/kernel.h>
#include <linux/mfd/cros_ec.h>
-#include <linux/mfd/cros_ec_commands.h>
#include <linux/module.h>
+#include <linux/platform_data/cros_ec_commands.h>
+#include <linux/platform_data/cros_ec_proto.h>
#include <linux/platform_device.h>
#include <linux/slab.h>
diff --git a/drivers/iio/pressure/cros_ec_baro.c b/drivers/iio/pressure/cros_ec_baro.c
index 034ce98d6e97..956dc01f1295 100644
--- a/drivers/iio/pressure/cros_ec_baro.c
+++ b/drivers/iio/pressure/cros_ec_baro.c
@@ -15,9 +15,10 @@
#include <linux/iio/trigger_consumer.h>
#include <linux/kernel.h>
#include <linux/mfd/cros_ec.h>
-#include <linux/mfd/cros_ec_commands.h>
#include <linux/module.h>
#include <linux/slab.h>
+#include <linux/platform_data/cros_ec_commands.h>
+#include <linux/platform_data/cros_ec_proto.h>
#include <linux/platform_device.h>
/*
diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
index d56001181598..2b71c5a51f90 100644
--- a/drivers/input/keyboard/cros_ec_keyb.c
+++ b/drivers/input/keyboard/cros_ec_keyb.c
@@ -22,8 +22,8 @@
#include <linux/slab.h>
#include <linux/sysrq.h>
#include <linux/input/matrix_keypad.h>
-#include <linux/mfd/cros_ec.h>
-#include <linux/mfd/cros_ec_commands.h>
+#include <linux/platform_data/cros_ec_commands.h>
+#include <linux/platform_data/cros_ec_proto.h>
#include <asm/unaligned.h>
diff --git a/drivers/media/platform/cros-ec-cec/cros-ec-cec.c b/drivers/media/platform/cros-ec-cec/cros-ec-cec.c
index 068df9888dbf..2e4e263a4a94 100644
--- a/drivers/media/platform/cros-ec-cec/cros-ec-cec.c
+++ b/drivers/media/platform/cros-ec-cec/cros-ec-cec.c
@@ -16,8 +16,8 @@
#include <linux/interrupt.h>
#include <media/cec.h>
#include <media/cec-notifier.h>
-#include <linux/mfd/cros_ec.h>
-#include <linux/mfd/cros_ec_commands.h>
+#include <linux/platform_data/cros_ec_commands.h>
+#include <linux/platform_data/cros_ec_proto.h>
#define DRV_NAME "cros-ec-cec"
diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
index c7a5dfa36874..5481df4e1216 100644
--- a/drivers/mfd/cros_ec_dev.c
+++ b/drivers/mfd/cros_ec_dev.c
@@ -7,11 +7,12 @@
#include <linux/mfd/core.h>
#include <linux/mfd/cros_ec.h>
-#include <linux/mfd/cros_ec_commands.h>
#include <linux/module.h>
#include <linux/mod_devicetable.h>
#include <linux/of_platform.h>
#include <linux/platform_device.h>
+#include <linux/platform_data/cros_ec_commands.h>
+#include <linux/platform_data/cros_ec_proto.h>
#include <linux/slab.h>
#define DRV_NAME "cros-ec-dev"
diff --git a/drivers/platform/chrome/cros_ec.c b/drivers/platform/chrome/cros_ec.c
index 11fced7917fc..9800597ccd96 100644
--- a/drivers/platform/chrome/cros_ec.c
+++ b/drivers/platform/chrome/cros_ec.c
@@ -21,7 +21,8 @@
#include <linux/interrupt.h>
#include <linux/slab.h>
#include <linux/module.h>
-#include <linux/mfd/cros_ec.h>
+#include <linux/platform_data/cros_ec_commands.h>
+#include <linux/platform_data/cros_ec_proto.h>
#include <linux/suspend.h>
#include <asm/unaligned.h>
diff --git a/drivers/platform/chrome/cros_ec_chardev.c b/drivers/platform/chrome/cros_ec_chardev.c
index 1a0a27080026..786b941a60df 100644
--- a/drivers/platform/chrome/cros_ec_chardev.c
+++ b/drivers/platform/chrome/cros_ec_chardev.c
@@ -9,10 +9,10 @@
#include <linux/device.h>
#include <linux/fs.h>
#include <linux/list.h>
-#include <linux/mfd/cros_ec.h>
-#include <linux/mfd/cros_ec_commands.h>
#include <linux/miscdevice.h>
#include <linux/module.h>
+#include <linux/platform_data/cros_ec_commands.h>
+#include <linux/platform_data/cros_ec_proto.h>
#include <linux/platform_device.h>
#include <linux/slab.h>
#include <linux/spinlock.h>
diff --git a/drivers/platform/chrome/cros_ec_debugfs.c b/drivers/platform/chrome/cros_ec_debugfs.c
index 4c2a27f6a6d0..b088d91be9c9 100644
--- a/drivers/platform/chrome/cros_ec_debugfs.c
+++ b/drivers/platform/chrome/cros_ec_debugfs.c
@@ -8,9 +8,10 @@
#include <linux/delay.h>
#include <linux/fs.h>
#include <linux/mfd/cros_ec.h>
-#include <linux/mfd/cros_ec_commands.h>
#include <linux/module.h>
#include <linux/mutex.h>
+#include <linux/platform_data/cros_ec_commands.h>
+#include <linux/platform_data/cros_ec_proto.h>
#include <linux/platform_device.h>
#include <linux/poll.h>
#include <linux/sched.h>
diff --git a/drivers/platform/chrome/cros_ec_i2c.c b/drivers/platform/chrome/cros_ec_i2c.c
index 6bb82dfa7dae..9bd97bc8454b 100644
--- a/drivers/platform/chrome/cros_ec_i2c.c
+++ b/drivers/platform/chrome/cros_ec_i2c.c
@@ -9,8 +9,8 @@
#include <linux/module.h>
#include <linux/i2c.h>
#include <linux/interrupt.h>
-#include <linux/mfd/cros_ec.h>
-#include <linux/mfd/cros_ec_commands.h>
+#include <linux/platform_data/cros_ec_commands.h>
+#include <linux/platform_data/cros_ec_proto.h>
#include <linux/platform_device.h>
#include <linux/slab.h>
diff --git a/drivers/platform/chrome/cros_ec_lightbar.c b/drivers/platform/chrome/cros_ec_lightbar.c
index d30a6650b0b5..caa26da2c788 100644
--- a/drivers/platform/chrome/cros_ec_lightbar.c
+++ b/drivers/platform/chrome/cros_ec_lightbar.c
@@ -9,8 +9,9 @@
#include <linux/fs.h>
#include <linux/kobject.h>
#include <linux/mfd/cros_ec.h>
-#include <linux/mfd/cros_ec_commands.h>
#include <linux/module.h>
+#include <linux/platform_data/cros_ec_commands.h>
+#include <linux/platform_data/cros_ec_proto.h>
#include <linux/platform_device.h>
#include <linux/sched.h>
#include <linux/types.h>
diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
index 2c7e654cf89c..0c976e95998a 100644
--- a/drivers/platform/chrome/cros_ec_lpc.c
+++ b/drivers/platform/chrome/cros_ec_lpc.c
@@ -16,9 +16,9 @@
#include <linux/delay.h>
#include <linux/io.h>
#include <linux/interrupt.h>
-#include <linux/mfd/cros_ec.h>
-#include <linux/mfd/cros_ec_commands.h>
#include <linux/module.h>
+#include <linux/platform_data/cros_ec_commands.h>
+#include <linux/platform_data/cros_ec_proto.h>
#include <linux/platform_device.h>
#include <linux/printk.h>
#include <linux/suspend.h>
diff --git a/drivers/platform/chrome/cros_ec_lpc_reg.c b/drivers/platform/chrome/cros_ec_lpc_reg.c
index 0f5cd0ac8b49..dec9a779e209 100644
--- a/drivers/platform/chrome/cros_ec_lpc_reg.c
+++ b/drivers/platform/chrome/cros_ec_lpc_reg.c
@@ -4,8 +4,8 @@
// Copyright (C) 2016 Google, Inc
#include <linux/io.h>
-#include <linux/mfd/cros_ec.h>
-#include <linux/mfd/cros_ec_commands.h>
+#include <linux/platform_data/cros_ec_commands.h>
+#include <linux/platform_data/cros_ec_proto.h>
#include "cros_ec_lpc_mec.h"
diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
index 3d2325197a68..f659f96bda12 100644
--- a/drivers/platform/chrome/cros_ec_proto.c
+++ b/drivers/platform/chrome/cros_ec_proto.c
@@ -3,10 +3,11 @@
//
// Copyright (C) 2015 Google, Inc
-#include <linux/mfd/cros_ec.h>
#include <linux/delay.h>
#include <linux/device.h>
#include <linux/module.h>
+#include <linux/platform_data/cros_ec_commands.h>
+#include <linux/platform_data/cros_ec_proto.h>
#include <linux/slab.h>
#include <asm/unaligned.h>
diff --git a/drivers/platform/chrome/cros_ec_rpmsg.c b/drivers/platform/chrome/cros_ec_rpmsg.c
index 520e507bfa54..9633e5417686 100644
--- a/drivers/platform/chrome/cros_ec_rpmsg.c
+++ b/drivers/platform/chrome/cros_ec_rpmsg.c
@@ -6,9 +6,9 @@
#include <linux/delay.h>
#include <linux/kernel.h>
#include <linux/module.h>
-#include <linux/mfd/cros_ec.h>
-#include <linux/mfd/cros_ec_commands.h>
#include <linux/of.h>
+#include <linux/platform_data/cros_ec_commands.h>
+#include <linux/platform_data/cros_ec_proto.h>
#include <linux/platform_device.h>
#include <linux/rpmsg.h>
#include <linux/slab.h>
diff --git a/drivers/platform/chrome/cros_ec_spi.c b/drivers/platform/chrome/cros_ec_spi.c
index 2e21f2776063..9006e1872942 100644
--- a/drivers/platform/chrome/cros_ec_spi.c
+++ b/drivers/platform/chrome/cros_ec_spi.c
@@ -6,9 +6,9 @@
#include <linux/delay.h>
#include <linux/kernel.h>
#include <linux/module.h>
-#include <linux/mfd/cros_ec.h>
-#include <linux/mfd/cros_ec_commands.h>
#include <linux/of.h>
+#include <linux/platform_data/cros_ec_commands.h>
+#include <linux/platform_data/cros_ec_proto.h>
#include <linux/platform_device.h>
#include <linux/slab.h>
#include <linux/spi/spi.h>
diff --git a/drivers/platform/chrome/cros_ec_sysfs.c b/drivers/platform/chrome/cros_ec_sysfs.c
index fe0b7614ae1b..0caeb8d0989d 100644
--- a/drivers/platform/chrome/cros_ec_sysfs.c
+++ b/drivers/platform/chrome/cros_ec_sysfs.c
@@ -9,8 +9,9 @@
#include <linux/fs.h>
#include <linux/kobject.h>
#include <linux/mfd/cros_ec.h>
-#include <linux/mfd/cros_ec_commands.h>
#include <linux/module.h>
+#include <linux/platform_data/cros_ec_commands.h>
+#include <linux/platform_data/cros_ec_proto.h>
#include <linux/platform_device.h>
#include <linux/printk.h>
#include <linux/slab.h>
diff --git a/drivers/platform/chrome/cros_ec_trace.c b/drivers/platform/chrome/cros_ec_trace.c
index 0a76412095a9..6f80ff4532ae 100644
--- a/drivers/platform/chrome/cros_ec_trace.c
+++ b/drivers/platform/chrome/cros_ec_trace.c
@@ -6,7 +6,7 @@
#define TRACE_SYMBOL(a) {a, #a}
// Generate the list using the following script:
-// sed -n 's/^#define \(EC_CMD_[[:alnum:]_]*\)\s.*/\tTRACE_SYMBOL(\1), \\/p' include/linux/mfd/cros_ec_commands.h
+// sed -n 's/^#define \(EC_CMD_[[:alnum:]_]*\)\s.*/\tTRACE_SYMBOL(\1), \\/p' include/linux/platform_data/cros_ec_commands.h
#define EC_CMDS \
TRACE_SYMBOL(EC_CMD_PROTO_VERSION), \
TRACE_SYMBOL(EC_CMD_HELLO), \
diff --git a/drivers/platform/chrome/cros_ec_trace.h b/drivers/platform/chrome/cros_ec_trace.h
index 7ae3b89c78b9..0dd4df30fa89 100644
--- a/drivers/platform/chrome/cros_ec_trace.h
+++ b/drivers/platform/chrome/cros_ec_trace.h
@@ -11,8 +11,10 @@
#if !defined(_CROS_EC_TRACE_H_) || defined(TRACE_HEADER_MULTI_READ)
#define _CROS_EC_TRACE_H_
+#include <linux/bits.h>
#include <linux/types.h>
-#include <linux/mfd/cros_ec.h>
+#include <linux/platform_data/cros_ec_commands.h>
+#include <linux/platform_data/cros_ec_proto.h>
#include <linux/tracepoint.h>
diff --git a/drivers/platform/chrome/cros_ec_vbc.c b/drivers/platform/chrome/cros_ec_vbc.c
index 8392a1ec33a7..cffe119e7a7a 100644
--- a/drivers/platform/chrome/cros_ec_vbc.c
+++ b/drivers/platform/chrome/cros_ec_vbc.c
@@ -7,8 +7,9 @@
#include <linux/of.h>
#include <linux/platform_device.h>
#include <linux/mfd/cros_ec.h>
-#include <linux/mfd/cros_ec_commands.h>
#include <linux/module.h>
+#include <linux/platform_data/cros_ec_commands.h>
+#include <linux/platform_data/cros_ec_proto.h>
#include <linux/slab.h>
#define DRV_NAME "cros-ec-vbc"
diff --git a/drivers/platform/chrome/cros_usbpd_logger.c b/drivers/platform/chrome/cros_usbpd_logger.c
index 7c7b267626a0..c549a9b49b56 100644
--- a/drivers/platform/chrome/cros_usbpd_logger.c
+++ b/drivers/platform/chrome/cros_usbpd_logger.c
@@ -6,10 +6,11 @@
*/
#include <linux/ktime.h>
-#include <linux/math64.h>
#include <linux/mfd/cros_ec.h>
-#include <linux/mfd/cros_ec_commands.h>
+#include <linux/math64.h>
#include <linux/module.h>
+#include <linux/platform_data/cros_ec_commands.h>
+#include <linux/platform_data/cros_ec_proto.h>
#include <linux/platform_device.h>
#include <linux/rtc.h>
diff --git a/drivers/power/supply/cros_usbpd-charger.c b/drivers/power/supply/cros_usbpd-charger.c
index 3a9ea94c3de3..6cc7c3910e09 100644
--- a/drivers/power/supply/cros_usbpd-charger.c
+++ b/drivers/power/supply/cros_usbpd-charger.c
@@ -5,9 +5,10 @@
* Copyright (c) 2014 - 2018 Google, Inc
*/
-#include <linux/module.h>
#include <linux/mfd/cros_ec.h>
-#include <linux/mfd/cros_ec_commands.h>
+#include <linux/module.h>
+#include <linux/platform_data/cros_ec_commands.h>
+#include <linux/platform_data/cros_ec_proto.h>
#include <linux/platform_device.h>
#include <linux/power_supply.h>
#include <linux/slab.h>
diff --git a/drivers/pwm/pwm-cros-ec.c b/drivers/pwm/pwm-cros-ec.c
index 98f6ac6cf6ab..85bea2d40b7d 100644
--- a/drivers/pwm/pwm-cros-ec.c
+++ b/drivers/pwm/pwm-cros-ec.c
@@ -6,8 +6,8 @@
*/
#include <linux/module.h>
-#include <linux/mfd/cros_ec.h>
-#include <linux/mfd/cros_ec_commands.h>
+#include <linux/platform_data/cros_ec_commands.h>
+#include <linux/platform_data/cros_ec_proto.h>
#include <linux/platform_device.h>
#include <linux/pwm.h>
#include <linux/slab.h>
diff --git a/drivers/rtc/rtc-cros-ec.c b/drivers/rtc/rtc-cros-ec.c
index 4d6bf9304ceb..6909e01936d9 100644
--- a/drivers/rtc/rtc-cros-ec.c
+++ b/drivers/rtc/rtc-cros-ec.c
@@ -6,8 +6,9 @@
#include <linux/kernel.h>
#include <linux/mfd/cros_ec.h>
-#include <linux/mfd/cros_ec_commands.h>
#include <linux/module.h>
+#include <linux/platform_data/cros_ec_commands.h>
+#include <linux/platform_data/cros_ec_proto.h>
#include <linux/platform_device.h>
#include <linux/rtc.h>
#include <linux/slab.h>
diff --git a/include/linux/iio/common/cros_ec_sensors_core.h b/include/linux/iio/common/cros_ec_sensors_core.h
index ce16445411ac..8a91669f5bed 100644
--- a/include/linux/iio/common/cros_ec_sensors_core.h
+++ b/include/linux/iio/common/cros_ec_sensors_core.h
@@ -18,7 +18,8 @@
#include <linux/iio/iio.h>
#include <linux/irqreturn.h>
-#include <linux/mfd/cros_ec.h>
+#include <linux/platform_data/cros_ec_commands.h>
+#include <linux/platform_data/cros_ec_proto.h>
enum {
CROS_EC_SENSOR_X,
diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
index 2a1372d167b9..e0bae49535e1 100644
--- a/include/linux/mfd/cros_ec.h
+++ b/include/linux/mfd/cros_ec.h
@@ -16,184 +16,7 @@
#ifndef __LINUX_MFD_CROS_EC_H
#define __LINUX_MFD_CROS_EC_H
-#include <linux/cdev.h>
#include <linux/device.h>
-#include <linux/notifier.h>
-#include <linux/mfd/cros_ec_commands.h>
-#include <linux/mutex.h>
-
-#define CROS_EC_DEV_NAME "cros_ec"
-#define CROS_EC_DEV_FP_NAME "cros_fp"
-#define CROS_EC_DEV_PD_NAME "cros_pd"
-#define CROS_EC_DEV_TP_NAME "cros_tp"
-#define CROS_EC_DEV_ISH_NAME "cros_ish"
-
-/*
- * The EC is unresponsive for a time after a reboot command. Add a
- * simple delay to make sure that the bus stays locked.
- */
-#define EC_REBOOT_DELAY_MS 50
-
-/*
- * Max bus-specific overhead incurred by request/responses.
- * I2C requires 1 additional byte for requests.
- * I2C requires 2 additional bytes for responses.
- * SPI requires up to 32 additional bytes for responses.
- */
-#define EC_PROTO_VERSION_UNKNOWN 0
-#define EC_MAX_REQUEST_OVERHEAD 1
-#define EC_MAX_RESPONSE_OVERHEAD 32
-
-/*
- * Command interface between EC and AP, for LPC, I2C and SPI interfaces.
- */
-enum {
- EC_MSG_TX_HEADER_BYTES = 3,
- EC_MSG_TX_TRAILER_BYTES = 1,
- EC_MSG_TX_PROTO_BYTES = EC_MSG_TX_HEADER_BYTES +
- EC_MSG_TX_TRAILER_BYTES,
- EC_MSG_RX_PROTO_BYTES = 3,
-
- /* Max length of messages for proto 2*/
- EC_PROTO2_MSG_BYTES = EC_PROTO2_MAX_PARAM_SIZE +
- EC_MSG_TX_PROTO_BYTES,
-
- EC_MAX_MSG_BYTES = 64 * 1024,
-};
-
-/**
- * struct cros_ec_command - Information about a ChromeOS EC command.
- * @version: Command version number (often 0).
- * @command: Command to send (EC_CMD_...).
- * @outsize: Outgoing length in bytes.
- * @insize: Max number of bytes to accept from the EC.
- * @result: EC's response to the command (separate from communication failure).
- * @data: Where to put the incoming data from EC and outgoing data to EC.
- */
-struct cros_ec_command {
- uint32_t version;
- uint32_t command;
- uint32_t outsize;
- uint32_t insize;
- uint32_t result;
- uint8_t data[0];
-};
-
-/**
- * struct cros_ec_device - Information about a ChromeOS EC device.
- * @phys_name: Name of physical comms layer (e.g. 'i2c-4').
- * @dev: Device pointer for physical comms device
- * @was_wake_device: True if this device was set to wake the system from
- * sleep at the last suspend.
- * @cros_class: The class structure for this device.
- * @cmd_readmem: Direct read of the EC memory-mapped region, if supported.
- * @offset: Is within EC_LPC_ADDR_MEMMAP region.
- * @bytes: Number of bytes to read. zero means "read a string" (including
- * the trailing '\0'). At most only EC_MEMMAP_SIZE bytes can be
- * read. Caller must ensure that the buffer is large enough for the
- * result when reading a string.
- * @max_request: Max size of message requested.
- * @max_response: Max size of message response.
- * @max_passthru: Max sice of passthru message.
- * @proto_version: The protocol version used for this device.
- * @priv: Private data.
- * @irq: Interrupt to use.
- * @id: Device id.
- * @din: Input buffer (for data from EC). This buffer will always be
- * dword-aligned and include enough space for up to 7 word-alignment
- * bytes also, so we can ensure that the body of the message is always
- * dword-aligned (64-bit). We use this alignment to keep ARM and x86
- * happy. Probably word alignment would be OK, there might be a small
- * performance advantage to using dword.
- * @dout: Output buffer (for data to EC). This buffer will always be
- * dword-aligned and include enough space for up to 7 word-alignment
- * bytes also, so we can ensure that the body of the message is always
- * dword-aligned (64-bit). We use this alignment to keep ARM and x86
- * happy. Probably word alignment would be OK, there might be a small
- * performance advantage to using dword.
- * @din_size: Size of din buffer to allocate (zero to use static din).
- * @dout_size: Size of dout buffer to allocate (zero to use static dout).
- * @wake_enabled: True if this device can wake the system from sleep.
- * @suspended: True if this device had been suspended.
- * @cmd_xfer: Send command to EC and get response.
- * Returns the number of bytes received if the communication
- * succeeded, but that doesn't mean the EC was happy with the
- * command. The caller should check msg.result for the EC's result
- * code.
- * @pkt_xfer: Send packet to EC and get response.
- * @lock: One transaction at a time.
- * @mkbp_event_supported: True if this EC supports the MKBP event protocol.
- * @host_sleep_v1: True if this EC supports the sleep v1 command.
- * @event_notifier: Interrupt event notifier for transport devices.
- * @event_data: Raw payload transferred with the MKBP event.
- * @event_size: Size in bytes of the event data.
- * @host_event_wake_mask: Mask of host events that cause wake from suspend.
- * @ec: The platform_device used by the mfd driver to interface with the
- * main EC.
- * @pd: The platform_device used by the mfd driver to interface with the
- * PD behind an EC.
- */
-struct cros_ec_device {
- /* These are used by other drivers that want to talk to the EC */
- const char *phys_name;
- struct device *dev;
- bool was_wake_device;
- struct class *cros_class;
- int (*cmd_readmem)(struct cros_ec_device *ec, unsigned int offset,
- unsigned int bytes, void *dest);
-
- /* These are used to implement the platform-specific interface */
- u16 max_request;
- u16 max_response;
- u16 max_passthru;
- u16 proto_version;
- void *priv;
- int irq;
- u8 *din;
- u8 *dout;
- int din_size;
- int dout_size;
- bool wake_enabled;
- bool suspended;
- int (*cmd_xfer)(struct cros_ec_device *ec,
- struct cros_ec_command *msg);
- int (*pkt_xfer)(struct cros_ec_device *ec,
- struct cros_ec_command *msg);
- struct mutex lock;
- bool mkbp_event_supported;
- bool host_sleep_v1;
- struct blocking_notifier_head event_notifier;
-
- struct ec_response_get_next_event_v1 event_data;
- int event_size;
- u32 host_event_wake_mask;
-
- /* The platform devices used by the mfd driver */
- struct platform_device *ec;
- struct platform_device *pd;
-};
-
-/**
- * struct cros_ec_sensor_platform - ChromeOS EC sensor platform information.
- * @sensor_num: Id of the sensor, as reported by the EC.
- */
-struct cros_ec_sensor_platform {
- u8 sensor_num;
-};
-
-/**
- * struct cros_ec_platform - ChromeOS EC platform information.
- * @ec_name: Name of EC device (e.g. 'cros-ec', 'cros-pd', ...)
- * used in /dev/ and sysfs.
- * @cmd_offset: Offset to apply for each command. Set when
- * registering a device behind another one.
- */
-struct cros_ec_platform {
- const char *ec_name;
- u16 cmd_offset;
-};
-
-struct cros_ec_debugfs;
/**
* struct cros_ec_dev - ChromeOS EC device entry point.
@@ -217,133 +40,4 @@ struct cros_ec_dev {
#define to_cros_ec_dev(dev) container_of(dev, struct cros_ec_dev, class_dev)
-/**
- * cros_ec_suspend() - Handle a suspend operation for the ChromeOS EC device.
- * @ec_dev: Device to suspend.
- *
- * This can be called by drivers to handle a suspend event.
- *
- * Return: 0 on success or negative error code.
- */
-int cros_ec_suspend(struct cros_ec_device *ec_dev);
-
-/**
- * cros_ec_resume() - Handle a resume operation for the ChromeOS EC device.
- * @ec_dev: Device to resume.
- *
- * This can be called by drivers to handle a resume event.
- *
- * Return: 0 on success or negative error code.
- */
-int cros_ec_resume(struct cros_ec_device *ec_dev);
-
-/**
- * cros_ec_prepare_tx() - Prepare an outgoing message in the output buffer.
- * @ec_dev: Device to register.
- * @msg: Message to write.
- *
- * This is intended to be used by all ChromeOS EC drivers, but at present
- * only SPI uses it. Once LPC uses the same protocol it can start using it.
- * I2C could use it now, with a refactor of the existing code.
- *
- * Return: 0 on success or negative error code.
- */
-int cros_ec_prepare_tx(struct cros_ec_device *ec_dev,
- struct cros_ec_command *msg);
-
-/**
- * cros_ec_check_result() - Check ec_msg->result.
- * @ec_dev: EC device.
- * @msg: Message to check.
- *
- * This is used by ChromeOS EC drivers to check the ec_msg->result for
- * errors and to warn about them.
- *
- * Return: 0 on success or negative error code.
- */
-int cros_ec_check_result(struct cros_ec_device *ec_dev,
- struct cros_ec_command *msg);
-
-/**
- * cros_ec_cmd_xfer() - Send a command to the ChromeOS EC.
- * @ec_dev: EC device.
- * @msg: Message to write.
- *
- * Call this to send a command to the ChromeOS EC. This should be used
- * instead of calling the EC's cmd_xfer() callback directly.
- *
- * Return: 0 on success or negative error code.
- */
-int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev,
- struct cros_ec_command *msg);
-
-/**
- * cros_ec_cmd_xfer_status() - Send a command to the ChromeOS EC.
- * @ec_dev: EC device.
- * @msg: Message to write.
- *
- * This function is identical to cros_ec_cmd_xfer, except it returns success
- * status only if both the command was transmitted successfully and the EC
- * replied with success status. It's not necessary to check msg->result when
- * using this function.
- *
- * Return: The number of bytes transferred on success or negative error code.
- */
-int cros_ec_cmd_xfer_status(struct cros_ec_device *ec_dev,
- struct cros_ec_command *msg);
-
-/**
- * cros_ec_register() - Register a new ChromeOS EC, using the provided info.
- * @ec_dev: Device to register.
- *
- * Before calling this, allocate a pointer to a new device and then fill
- * in all the fields up to the --private-- marker.
- *
- * Return: 0 on success or negative error code.
- */
-int cros_ec_register(struct cros_ec_device *ec_dev);
-
-/**
- * cros_ec_unregister() - Remove a ChromeOS EC.
- * @ec_dev: Device to unregister.
- *
- * Call this to deregister a ChromeOS EC, then clean up any private data.
- *
- * Return: 0 on success or negative error code.
- */
-int cros_ec_unregister(struct cros_ec_device *ec_dev);
-
-/**
- * cros_ec_query_all() - Query the protocol version supported by the
- * ChromeOS EC.
- * @ec_dev: Device to register.
- *
- * Return: 0 on success or negative error code.
- */
-int cros_ec_query_all(struct cros_ec_device *ec_dev);
-
-/**
- * cros_ec_get_next_event() - Fetch next event from the ChromeOS EC.
- * @ec_dev: Device to fetch event from.
- * @wake_event: Pointer to a bool set to true upon return if the event might be
- * treated as a wake event. Ignored if null.
- *
- * Return: negative error code on errors; 0 for no data; or else number of
- * bytes received (i.e., an event was retrieved successfully). Event types are
- * written out to @ec_dev->event_data.event_type on success.
- */
-int cros_ec_get_next_event(struct cros_ec_device *ec_dev, bool *wake_event);
-
-/**
- * cros_ec_get_host_event() - Return a mask of event set by the ChromeOS EC.
- * @ec_dev: Device to fetch event from.
- *
- * When MKBP is supported, when the EC raises an interrupt, we collect the
- * events raised and call the functions in the ec notifier. This function
- * is a helper to know which events are raised.
- *
- * Return: 0 on error or non-zero bitmask of one or more EC_HOST_EVENT_*.
- */
-u32 cros_ec_get_host_event(struct cros_ec_device *ec_dev);
-
#endif /* __LINUX_MFD_CROS_EC_H */
diff --git a/include/linux/mfd/cros_ec_commands.h b/include/linux/platform_data/cros_ec_commands.h
similarity index 100%
rename from include/linux/mfd/cros_ec_commands.h
rename to include/linux/platform_data/cros_ec_commands.h
diff --git a/include/linux/platform_data/cros_ec_proto.h b/include/linux/platform_data/cros_ec_proto.h
new file mode 100644
index 000000000000..34dd9e5c1779
--- /dev/null
+++ b/include/linux/platform_data/cros_ec_proto.h
@@ -0,0 +1,315 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * ChromeOS Embedded Controller protocol interface.
+ *
+ * Copyright (C) 2012 Google, Inc
+ */
+
+#ifndef __LINUX_CROS_EC_PROTO_H
+#define __LINUX_CROS_EC_PROTO_H
+
+#include <linux/device.h>
+#include <linux/mutex.h>
+#include <linux/notifier.h>
+
+#define CROS_EC_DEV_NAME "cros_ec"
+#define CROS_EC_DEV_FP_NAME "cros_fp"
+#define CROS_EC_DEV_ISH_NAME "cros_ish"
+#define CROS_EC_DEV_PD_NAME "cros_pd"
+#define CROS_EC_DEV_TP_NAME "cros_tp"
+
+/*
+ * The EC is unresponsive for a time after a reboot command. Add a
+ * simple delay to make sure that the bus stays locked.
+ */
+#define EC_REBOOT_DELAY_MS 50
+
+/*
+ * Max bus-specific overhead incurred by request/responses.
+ * I2C requires 1 additional byte for requests.
+ * I2C requires 2 additional bytes for responses.
+ * SPI requires up to 32 additional bytes for responses.
+ */
+#define EC_PROTO_VERSION_UNKNOWN 0
+#define EC_MAX_REQUEST_OVERHEAD 1
+#define EC_MAX_RESPONSE_OVERHEAD 32
+
+/*
+ * Command interface between EC and AP, for LPC, I2C and SPI interfaces.
+ */
+enum {
+ EC_MSG_TX_HEADER_BYTES = 3,
+ EC_MSG_TX_TRAILER_BYTES = 1,
+ EC_MSG_TX_PROTO_BYTES = EC_MSG_TX_HEADER_BYTES +
+ EC_MSG_TX_TRAILER_BYTES,
+ EC_MSG_RX_PROTO_BYTES = 3,
+
+ /* Max length of messages for proto 2*/
+ EC_PROTO2_MSG_BYTES = EC_PROTO2_MAX_PARAM_SIZE +
+ EC_MSG_TX_PROTO_BYTES,
+
+ EC_MAX_MSG_BYTES = 64 * 1024,
+};
+
+/**
+ * struct cros_ec_command - Information about a ChromeOS EC command.
+ * @version: Command version number (often 0).
+ * @command: Command to send (EC_CMD_...).
+ * @outsize: Outgoing length in bytes.
+ * @insize: Max number of bytes to accept from the EC.
+ * @result: EC's response to the command (separate from communication failure).
+ * @data: Where to put the incoming data from EC and outgoing data to EC.
+ */
+struct cros_ec_command {
+ uint32_t version;
+ uint32_t command;
+ uint32_t outsize;
+ uint32_t insize;
+ uint32_t result;
+ uint8_t data[0];
+};
+
+/**
+ * struct cros_ec_device - Information about a ChromeOS EC device.
+ * @phys_name: Name of physical comms layer (e.g. 'i2c-4').
+ * @dev: Device pointer for physical comms device
+ * @was_wake_device: True if this device was set to wake the system from
+ * sleep at the last suspend.
+ * @cros_class: The class structure for this device.
+ * @cmd_readmem: Direct read of the EC memory-mapped region, if supported.
+ * @offset: Is within EC_LPC_ADDR_MEMMAP region.
+ * @bytes: Number of bytes to read. zero means "read a string" (including
+ * the trailing '\0'). At most only EC_MEMMAP_SIZE bytes can be
+ * read. Caller must ensure that the buffer is large enough for the
+ * result when reading a string.
+ * @max_request: Max size of message requested.
+ * @max_response: Max size of message response.
+ * @max_passthru: Max sice of passthru message.
+ * @proto_version: The protocol version used for this device.
+ * @priv: Private data.
+ * @irq: Interrupt to use.
+ * @id: Device id.
+ * @din: Input buffer (for data from EC). This buffer will always be
+ * dword-aligned and include enough space for up to 7 word-alignment
+ * bytes also, so we can ensure that the body of the message is always
+ * dword-aligned (64-bit). We use this alignment to keep ARM and x86
+ * happy. Probably word alignment would be OK, there might be a small
+ * performance advantage to using dword.
+ * @dout: Output buffer (for data to EC). This buffer will always be
+ * dword-aligned and include enough space for up to 7 word-alignment
+ * bytes also, so we can ensure that the body of the message is always
+ * dword-aligned (64-bit). We use this alignment to keep ARM and x86
+ * happy. Probably word alignment would be OK, there might be a small
+ * performance advantage to using dword.
+ * @din_size: Size of din buffer to allocate (zero to use static din).
+ * @dout_size: Size of dout buffer to allocate (zero to use static dout).
+ * @wake_enabled: True if this device can wake the system from sleep.
+ * @suspended: True if this device had been suspended.
+ * @cmd_xfer: Send command to EC and get response.
+ * Returns the number of bytes received if the communication
+ * succeeded, but that doesn't mean the EC was happy with the
+ * command. The caller should check msg.result for the EC's result
+ * code.
+ * @pkt_xfer: Send packet to EC and get response.
+ * @lock: One transaction at a time.
+ * @mkbp_event_supported: True if this EC supports the MKBP event protocol.
+ * @host_sleep_v1: True if this EC supports the sleep v1 command.
+ * @event_notifier: Interrupt event notifier for transport devices.
+ * @event_data: Raw payload transferred with the MKBP event.
+ * @event_size: Size in bytes of the event data.
+ * @host_event_wake_mask: Mask of host events that cause wake from suspend.
+ * @ec: The platform_device used by the mfd driver to interface with the
+ * main EC.
+ * @pd: The platform_device used by the mfd driver to interface with the
+ * PD behind an EC.
+ */
+struct cros_ec_device {
+ /* These are used by other drivers that want to talk to the EC */
+ const char *phys_name;
+ struct device *dev;
+ bool was_wake_device;
+ struct class *cros_class;
+ int (*cmd_readmem)(struct cros_ec_device *ec, unsigned int offset,
+ unsigned int bytes, void *dest);
+
+ /* These are used to implement the platform-specific interface */
+ u16 max_request;
+ u16 max_response;
+ u16 max_passthru;
+ u16 proto_version;
+ void *priv;
+ int irq;
+ u8 *din;
+ u8 *dout;
+ int din_size;
+ int dout_size;
+ bool wake_enabled;
+ bool suspended;
+ int (*cmd_xfer)(struct cros_ec_device *ec,
+ struct cros_ec_command *msg);
+ int (*pkt_xfer)(struct cros_ec_device *ec,
+ struct cros_ec_command *msg);
+ struct mutex lock;
+ bool mkbp_event_supported;
+ bool host_sleep_v1;
+ struct blocking_notifier_head event_notifier;
+
+ struct ec_response_get_next_event_v1 event_data;
+ int event_size;
+ u32 host_event_wake_mask;
+
+ /* The platform devices used by the mfd driver */
+ struct platform_device *ec;
+ struct platform_device *pd;
+};
+
+/**
+ * struct cros_ec_sensor_platform - ChromeOS EC sensor platform information.
+ * @sensor_num: Id of the sensor, as reported by the EC.
+ */
+struct cros_ec_sensor_platform {
+ u8 sensor_num;
+};
+
+/**
+ * struct cros_ec_platform - ChromeOS EC platform information.
+ * @ec_name: Name of EC device (e.g. 'cros-ec', 'cros-pd', ...)
+ * used in /dev/ and sysfs.
+ * @cmd_offset: Offset to apply for each command. Set when
+ * registering a device behind another one.
+ */
+struct cros_ec_platform {
+ const char *ec_name;
+ u16 cmd_offset;
+};
+
+/**
+ * cros_ec_suspend() - Handle a suspend operation for the ChromeOS EC device.
+ * @ec_dev: Device to suspend.
+ *
+ * This can be called by drivers to handle a suspend event.
+ *
+ * Return: 0 on success or negative error code.
+ */
+int cros_ec_suspend(struct cros_ec_device *ec_dev);
+
+/**
+ * cros_ec_resume() - Handle a resume operation for the ChromeOS EC device.
+ * @ec_dev: Device to resume.
+ *
+ * This can be called by drivers to handle a resume event.
+ *
+ * Return: 0 on success or negative error code.
+ */
+int cros_ec_resume(struct cros_ec_device *ec_dev);
+
+/**
+ * cros_ec_prepare_tx() - Prepare an outgoing message in the output buffer.
+ * @ec_dev: Device to register.
+ * @msg: Message to write.
+ *
+ * This is intended to be used by all ChromeOS EC drivers, but at present
+ * only SPI uses it. Once LPC uses the same protocol it can start using it.
+ * I2C could use it now, with a refactor of the existing code.
+ *
+ * Return: 0 on success or negative error code.
+ */
+int cros_ec_prepare_tx(struct cros_ec_device *ec_dev,
+ struct cros_ec_command *msg);
+
+/**
+ * cros_ec_check_result() - Check ec_msg->result.
+ * @ec_dev: EC device.
+ * @msg: Message to check.
+ *
+ * This is used by ChromeOS EC drivers to check the ec_msg->result for
+ * errors and to warn about them.
+ *
+ * Return: 0 on success or negative error code.
+ */
+int cros_ec_check_result(struct cros_ec_device *ec_dev,
+ struct cros_ec_command *msg);
+
+/**
+ * cros_ec_cmd_xfer() - Send a command to the ChromeOS EC.
+ * @ec_dev: EC device.
+ * @msg: Message to write.
+ *
+ * Call this to send a command to the ChromeOS EC. This should be used
+ * instead of calling the EC's cmd_xfer() callback directly.
+ *
+ * Return: 0 on success or negative error code.
+ */
+int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev,
+ struct cros_ec_command *msg);
+
+/**
+ * cros_ec_cmd_xfer_status() - Send a command to the ChromeOS EC.
+ * @ec_dev: EC device.
+ * @msg: Message to write.
+ *
+ * This function is identical to cros_ec_cmd_xfer, except it returns success
+ * status only if both the command was transmitted successfully and the EC
+ * replied with success status. It's not necessary to check msg->result when
+ * using this function.
+ *
+ * Return: The number of bytes transferred on success or negative error code.
+ */
+int cros_ec_cmd_xfer_status(struct cros_ec_device *ec_dev,
+ struct cros_ec_command *msg);
+
+/**
+ * cros_ec_register() - Register a new ChromeOS EC, using the provided info.
+ * @ec_dev: Device to register.
+ *
+ * Before calling this, allocate a pointer to a new device and then fill
+ * in all the fields up to the --private-- marker.
+ *
+ * Return: 0 on success or negative error code.
+ */
+int cros_ec_register(struct cros_ec_device *ec_dev);
+
+/**
+ * cros_ec_unregister() - Remove a ChromeOS EC.
+ * @ec_dev: Device to unregister.
+ *
+ * Call this to deregister a ChromeOS EC, then clean up any private data.
+ *
+ * Return: 0 on success or negative error code.
+ */
+int cros_ec_unregister(struct cros_ec_device *ec_dev);
+
+/**
+ * cros_ec_query_all() - Query the protocol version supported by the
+ * ChromeOS EC.
+ * @ec_dev: Device to register.
+ *
+ * Return: 0 on success or negative error code.
+ */
+int cros_ec_query_all(struct cros_ec_device *ec_dev);
+
+/**
+ * cros_ec_get_next_event() - Fetch next event from the ChromeOS EC.
+ * @ec_dev: Device to fetch event from.
+ * @wake_event: Pointer to a bool set to true upon return if the event might be
+ * treated as a wake event. Ignored if null.
+ *
+ * Return: negative error code on errors; 0 for no data; or else number of
+ * bytes received (i.e., an event was retrieved successfully). Event types are
+ * written out to @ec_dev->event_data.event_type on success.
+ */
+int cros_ec_get_next_event(struct cros_ec_device *ec_dev, bool *wake_event);
+
+/**
+ * cros_ec_get_host_event() - Return a mask of event set by the ChromeOS EC.
+ * @ec_dev: Device to fetch event from.
+ *
+ * When MKBP is supported, when the EC raises an interrupt, we collect the
+ * events raised and call the functions in the ec notifier. This function
+ * is a helper to know which events are raised.
+ *
+ * Return: 0 on error or non-zero bitmask of one or more EC_HOST_EVENT_*.
+ */
+u32 cros_ec_get_host_event(struct cros_ec_device *ec_dev);
+
+#endif /* __LINUX_CROS_EC_PROTO_H */
diff --git a/sound/soc/codecs/cros_ec_codec.c b/sound/soc/codecs/cros_ec_codec.c
index 87830ed5ebf4..79bb4081d3c2 100644
--- a/sound/soc/codecs/cros_ec_codec.c
+++ b/sound/soc/codecs/cros_ec_codec.c
@@ -9,9 +9,9 @@
#include <linux/delay.h>
#include <linux/device.h>
#include <linux/kernel.h>
-#include <linux/mfd/cros_ec.h>
-#include <linux/mfd/cros_ec_commands.h>
#include <linux/module.h>
+#include <linux/platform_data/cros_ec_commands.h>
+#include <linux/platform_data/cros_ec_proto.h>
#include <linux/platform_device.h>
#include <sound/pcm.h>
#include <sound/pcm_params.h>
--
2.20.1
^ permalink raw reply related
* [PATCH 02/10] mfd / platform: cros_ec: Move cros-ec core driver out from MFD
From: Enric Balletbo i Serra @ 2019-06-04 15:20 UTC (permalink / raw)
To: linux-kernel
Cc: gwendal, Banajit Goswami, Vignesh R, Alexandre Belloni,
Wolfram Sang, linux-iio, Mark Brown, Juergen Fitschen, alsa-devel,
Stefan Agner, Douglas Anderson, Benjamin Tissoires,
Karthikeyan Ramasubramanian, linux-i2c, Peter Meerwald-Stadler,
Manivannan Sadhasivam, Guenter Roeck, kernel, dtor,
Lars-Peter Clausen, Jean Delvare, Jacky Bai, linux-rtc,
Andy Shevchenko, Sean
In-Reply-To: <20190604152019.16100-1-enric.balletbo@collabora.com>
Now, the ChromeOS EC core driver has nothing related to an MFD device, so
move that driver from the MFD subsystem to the platform/chrome subsystem.
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---
drivers/extcon/Kconfig | 2 +-
drivers/hid/Kconfig | 2 +-
drivers/i2c/busses/Kconfig | 2 +-
drivers/iio/common/cros_ec_sensors/Kconfig | 2 +-
drivers/input/keyboard/Kconfig | 2 +-
drivers/media/platform/Kconfig | 3 +--
drivers/mfd/Kconfig | 14 +-------------
drivers/mfd/Makefile | 2 --
drivers/platform/chrome/Kconfig | 21 +++++++++++++++++----
drivers/platform/chrome/Makefile | 1 +
drivers/{mfd => platform/chrome}/cros_ec.c | 0
drivers/power/supply/Kconfig | 2 +-
drivers/pwm/Kconfig | 2 +-
drivers/rtc/Kconfig | 2 +-
sound/soc/qcom/Kconfig | 2 +-
15 files changed, 29 insertions(+), 30 deletions(-)
rename drivers/{mfd => platform/chrome}/cros_ec.c (100%)
diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig
index 6f5af4196b8d..0ebc599c5e51 100644
--- a/drivers/extcon/Kconfig
+++ b/drivers/extcon/Kconfig
@@ -169,7 +169,7 @@ config EXTCON_USB_GPIO
config EXTCON_USBC_CROS_EC
tristate "ChromeOS Embedded Controller EXTCON support"
- depends on MFD_CROS_EC
+ depends on CROS_EC
help
Say Y here to enable USB Type C cable detection extcon support when
using Chrome OS EC based USB Type-C ports.
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 3872e03d9a59..a958b9625bba 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -376,7 +376,7 @@ config HOLTEK_FF
config HID_GOOGLE_HAMMER
tristate "Google Hammer Keyboard"
- depends on USB_HID && LEDS_CLASS && MFD_CROS_EC
+ depends on USB_HID && LEDS_CLASS && CROS_EC
---help---
Say Y here if you have a Google Hammer device.
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index ee5dfb5aee2a..42a224d08ec7 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -1336,7 +1336,7 @@ config I2C_SIBYTE
config I2C_CROS_EC_TUNNEL
tristate "ChromeOS EC tunnel I2C bus"
- depends on MFD_CROS_EC
+ depends on CROS_EC
help
If you say yes here you get an I2C bus that will tunnel i2c commands
through to the other side of the ChromeOS EC to the i2c bus
diff --git a/drivers/iio/common/cros_ec_sensors/Kconfig b/drivers/iio/common/cros_ec_sensors/Kconfig
index f9bf7ff7fcaf..55999104cd44 100644
--- a/drivers/iio/common/cros_ec_sensors/Kconfig
+++ b/drivers/iio/common/cros_ec_sensors/Kconfig
@@ -4,7 +4,7 @@
#
config IIO_CROS_EC_SENSORS_CORE
tristate "ChromeOS EC Sensors Core"
- depends on SYSFS && MFD_CROS_EC
+ depends on SYSFS && CROS_EC
select IIO_BUFFER
select IIO_TRIGGERED_BUFFER
help
diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
index 7c4f19dab34f..64555cc8d83e 100644
--- a/drivers/input/keyboard/Kconfig
+++ b/drivers/input/keyboard/Kconfig
@@ -729,7 +729,7 @@ config KEYBOARD_W90P910
config KEYBOARD_CROS_EC
tristate "ChromeOS EC keyboard"
select INPUT_MATRIXKMAP
- depends on MFD_CROS_EC
+ depends on CROS_EC
help
Say Y here to enable the matrix keyboard used by ChromeOS devices
and implemented on the ChromeOS EC. You must enable one bus option
diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index f2b5f27ebacb..adec7a0bfe1e 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -558,10 +558,9 @@ if CEC_PLATFORM_DRIVERS
config VIDEO_CROS_EC_CEC
tristate "ChromeOS EC CEC driver"
- depends on MFD_CROS_EC
+ depends on CROS_EC
select CEC_CORE
select CEC_NOTIFIER
- select CHROME_PLATFORMS
select CROS_EC_PROTO
help
If you say yes here you will get support for the
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index a17d275bf1d4..ad0a5de74ef2 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -211,21 +211,9 @@ config MFD_AXP20X_RSB
components like regulators or the PEK (Power Enable Key) under the
corresponding menus.
-config MFD_CROS_EC
- tristate "ChromeOS Embedded Controller"
- select MFD_CORE
- select CHROME_PLATFORMS
- select CROS_EC_PROTO
- depends on X86 || ARM || ARM64 || COMPILE_TEST
- help
- If you say Y here you get support for the ChromeOS Embedded
- Controller (EC) providing keyboard, battery and power services.
- You also need to enable the driver for the bus you are using. The
- protocol for talking to the EC is defined by the bus driver.
-
config MFD_CROS_EC_CHARDEV
tristate "Chrome OS Embedded Controller userspace device interface"
- depends on MFD_CROS_EC
+ depends on CROS_EC
---help---
This driver adds support to talk with the ChromeOS EC from userspace.
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 52b1a90ff515..32327dc6bb45 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -13,8 +13,6 @@ obj-$(CONFIG_MFD_ASIC3) += asic3.o tmio_core.o
obj-$(CONFIG_ARCH_BCM2835) += bcm2835-pm.o
obj-$(CONFIG_MFD_BCM590XX) += bcm590xx.o
obj-$(CONFIG_MFD_BD9571MWV) += bd9571mwv.o
-cros_ec_core-objs := cros_ec.o
-obj-$(CONFIG_MFD_CROS_EC) += cros_ec_core.o
obj-$(CONFIG_MFD_CROS_EC_CHARDEV) += cros_ec_dev.o
obj-$(CONFIG_MFD_EXYNOS_LPASS) += exynos-lpass.o
diff --git a/drivers/platform/chrome/Kconfig b/drivers/platform/chrome/Kconfig
index 35bb5a2663f0..9417b982ad92 100644
--- a/drivers/platform/chrome/Kconfig
+++ b/drivers/platform/chrome/Kconfig
@@ -50,9 +50,22 @@ config CHROMEOS_TBMC
To compile this driver as a module, choose M here: the
module will be called chromeos_tbmc.
+config CROS_EC
+ tristate "ChromeOS Embedded Controller"
+ select CROS_EC_PROTO
+ depends on X86 || ARM || ARM64 || COMPILE_TEST
+ help
+ If you say Y here you get support for the ChromeOS Embedded
+ Controller (EC) providing keyboard, battery and power services.
+ You also need to enable the driver for the bus you are using. The
+ protocol for talking to the EC is defined by the bus driver.
+
+ To compile this driver as a module, choose M here: the
+ module will be called cros_ec.
+
config CROS_EC_I2C
tristate "ChromeOS Embedded Controller (I2C)"
- depends on MFD_CROS_EC && I2C
+ depends on CROS_EC && I2C
help
If you say Y here, you get support for talking to the ChromeOS
@@ -62,7 +75,7 @@ config CROS_EC_I2C
config CROS_EC_RPMSG
tristate "ChromeOS Embedded Controller (rpmsg)"
- depends on MFD_CROS_EC && RPMSG && OF
+ depends on CROS_EC && RPMSG && OF
help
If you say Y here, you get support for talking to the ChromeOS EC
through rpmsg. This uses a simple byte-level protocol with a
@@ -87,7 +100,7 @@ config CROS_EC_ISHTP
config CROS_EC_SPI
tristate "ChromeOS Embedded Controller (SPI)"
- depends on MFD_CROS_EC && SPI
+ depends on CROS_EC && SPI
---help---
If you say Y here, you get support for talking to the ChromeOS EC
@@ -97,7 +110,7 @@ config CROS_EC_SPI
config CROS_EC_LPC
tristate "ChromeOS Embedded Controller (LPC)"
- depends on MFD_CROS_EC && ACPI && (X86 || COMPILE_TEST)
+ depends on CROS_EC && ACPI && (X86 || COMPILE_TEST)
help
If you say Y here, you get support for talking to the ChromeOS EC
over an LPC bus. This uses a simple byte-level protocol with a
diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile
index c5583c48d1e5..ebb57e21923b 100644
--- a/drivers/platform/chrome/Makefile
+++ b/drivers/platform/chrome/Makefile
@@ -6,6 +6,7 @@ CFLAGS_cros_ec_trace.o:= -I$(src)
obj-$(CONFIG_CHROMEOS_LAPTOP) += chromeos_laptop.o
obj-$(CONFIG_CHROMEOS_PSTORE) += chromeos_pstore.o
obj-$(CONFIG_CHROMEOS_TBMC) += chromeos_tbmc.o
+obj-$(CONFIG_CROS_EC) += cros_ec.o
obj-$(CONFIG_CROS_EC_I2C) += cros_ec_i2c.o
obj-$(CONFIG_CROS_EC_ISHTP) += cros_ec_ishtp.o
obj-$(CONFIG_CROS_EC_RPMSG) += cros_ec_rpmsg.o
diff --git a/drivers/mfd/cros_ec.c b/drivers/platform/chrome/cros_ec.c
similarity index 100%
rename from drivers/mfd/cros_ec.c
rename to drivers/platform/chrome/cros_ec.c
diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
index dd7da41f230c..e05140771845 100644
--- a/drivers/power/supply/Kconfig
+++ b/drivers/power/supply/Kconfig
@@ -656,7 +656,7 @@ config CHARGER_RT9455
config CHARGER_CROS_USBPD
tristate "ChromeOS EC based USBPD charger"
- depends on MFD_CROS_EC
+ depends on CROS_EC
default n
help
Say Y here to enable ChromeOS EC based USBPD charger
diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index dff5a93f7daa..99946e1bcc73 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -145,7 +145,7 @@ config PWM_CRC
config PWM_CROS_EC
tristate "ChromeOS EC PWM driver"
- depends on MFD_CROS_EC
+ depends on CROS_EC
help
PWM driver for exposing a PWM attached to the ChromeOS Embedded
Controller.
diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 5c0790eed656..4eb311569fc4 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -1265,7 +1265,7 @@ config RTC_DRV_ZYNQMP
config RTC_DRV_CROS_EC
tristate "Chrome OS EC RTC driver"
- depends on MFD_CROS_EC
+ depends on CROS_EC
help
If you say yes here you will get support for the
Chrome OS Embedded Controller's RTC.
diff --git a/sound/soc/qcom/Kconfig b/sound/soc/qcom/Kconfig
index 8e3e86619b35..60086858e920 100644
--- a/sound/soc/qcom/Kconfig
+++ b/sound/soc/qcom/Kconfig
@@ -99,7 +99,7 @@ config SND_SOC_MSM8996
config SND_SOC_SDM845
tristate "SoC Machine driver for SDM845 boards"
- depends on QCOM_APR && MFD_CROS_EC && I2C
+ depends on QCOM_APR && CROS_EC && I2C
select SND_SOC_QDSP6
select SND_SOC_QCOM_COMMON
select SND_SOC_RT5663
--
2.20.1
^ permalink raw reply related
* [PATCH 00/10] Move part of cros-ec out of MFD subsystem
From: Enric Balletbo i Serra @ 2019-06-04 15:20 UTC (permalink / raw)
To: linux-kernel
Cc: Kate Stewart, Enno Luebbers, gwendal, Banajit Goswami,
Heiko Stuebner, Alexandre Belloni, linux-doc, Wolfram Sang,
Mark Brown, Juergen Fitschen, alsa-devel, Stefan Agner,
Douglas Anderson, Jilayne Lovejoy, Benjamin Tissoires,
Dmitry Torokhov, linux-i2c, Peter Meerwald-Stadler,
Manivannan Sadhasivam, Guenter Roeck, Lorenzo Pieralisi,
Ravi Chandra Sadineni
Hi,
This is the first attempt to clean up a bit more the cros-ec drivers
to have a better separation on what is part of the MFD subsystem and what
is part of platform/chrome.
It'd be really nice have some reviews, acks and tested on different
platforms from the chromiumos people before merge all this patchset, as
this moves a lot of code.
The major changes introduced by this patchset are:
1. Move the core driver to platform/chrome, as is not really related to
an MFD device driver.
2. Create a new misc chardev driver to replace the chardev bits from
cros-ec-dev (MFD)
3. Added some convenience structs in cros-ec-dev (MFD) to easy add more
subdrivers and avoid to add more boiler plate.
Once applied we have moved all the code to platform/chrome except the
cros-ec-dev driver, which is the one that instantiates the different
subdrivers as cells of the MFD device.
I tested the following patches on Veyron, Kevin, Samus, Peach Pi and
Peach Pit without noticing any problem, but they would need a lot of
more tests. I'll continue testing while the reviewing process of this
patchset.
Waiting for your feedback,
Enric
Enric Balletbo i Serra (10):
mfd / platform: cros_ec: Handle chained ECs as platform devices
mfd / platform: cros_ec: Move cros-ec core driver out from MFD
mfd / platform: cros_ec: Miscellaneous character device to talk with
the EC
mfd: cros_ec: Switch to use the new cros-ec-chardev driver
mfd / platform: cros_ec: Rename config to a better name
mfd / platform: cros_ec: Reorganize platform and mfd includes
mfd: cros_ec: Update with SPDX Licence identifier and fix description
mfd: cros_ec: Use kzalloc and cros_ec_cmd_xfer_status helper
mfd: cros_ec: Add convenience struct to define dedicated CrOS EC MCUs
mfd: cros_ec: Add convenience struct to define autodetectable CrOS EC
subdevices
Documentation/ioctl/ioctl-number.txt | 2 +-
drivers/extcon/Kconfig | 2 +-
drivers/extcon/extcon-usbc-cros-ec.c | 3 +-
drivers/hid/Kconfig | 2 +-
drivers/hid/hid-google-hammer.c | 4 +-
drivers/i2c/busses/Kconfig | 2 +-
drivers/i2c/busses/i2c-cros-ec-tunnel.c | 4 +-
drivers/iio/accel/cros_ec_accel_legacy.c | 3 +-
drivers/iio/common/cros_ec_sensors/Kconfig | 2 +-
.../common/cros_ec_sensors/cros_ec_sensors.c | 3 +-
.../cros_ec_sensors/cros_ec_sensors_core.c | 3 +-
drivers/iio/light/cros_ec_light_prox.c | 3 +-
drivers/iio/pressure/cros_ec_baro.c | 3 +-
drivers/input/keyboard/Kconfig | 2 +-
drivers/input/keyboard/cros_ec_keyb.c | 4 +-
drivers/media/platform/Kconfig | 3 +-
.../media/platform/cros-ec-cec/cros-ec-cec.c | 4 +-
drivers/mfd/Kconfig | 26 +-
drivers/mfd/Makefile | 4 +-
drivers/mfd/cros_ec_dev.c | 433 +++++-------------
drivers/platform/chrome/Kconfig | 48 +-
drivers/platform/chrome/Makefile | 2 +
drivers/{mfd => platform/chrome}/cros_ec.c | 64 +--
drivers/platform/chrome/cros_ec_chardev.c | 279 +++++++++++
drivers/platform/chrome/cros_ec_debugfs.c | 3 +-
drivers/platform/chrome/cros_ec_i2c.c | 12 +-
drivers/platform/chrome/cros_ec_lightbar.c | 3 +-
drivers/platform/chrome/cros_ec_lpc.c | 7 +-
drivers/platform/chrome/cros_ec_lpc_reg.c | 4 +-
drivers/platform/chrome/cros_ec_proto.c | 3 +-
drivers/platform/chrome/cros_ec_rpmsg.c | 6 +-
drivers/platform/chrome/cros_ec_spi.c | 12 +-
drivers/platform/chrome/cros_ec_sysfs.c | 3 +-
drivers/platform/chrome/cros_ec_trace.c | 2 +-
drivers/platform/chrome/cros_ec_trace.h | 4 +-
drivers/platform/chrome/cros_ec_vbc.c | 3 +-
drivers/platform/chrome/cros_usbpd_logger.c | 5 +-
drivers/power/supply/Kconfig | 2 +-
drivers/power/supply/cros_usbpd-charger.c | 5 +-
drivers/pwm/Kconfig | 2 +-
drivers/pwm/pwm-cros-ec.c | 4 +-
drivers/rtc/Kconfig | 2 +-
drivers/rtc/rtc-cros-ec.c | 3 +-
.../linux/iio/common/cros_ec_sensors_core.h | 3 +-
include/linux/mfd/cros_ec.h | 302 +-----------
.../{mfd => platform_data}/cros_ec_commands.h | 0
include/linux/platform_data/cros_ec_proto.h | 315 +++++++++++++
.../uapi/linux/cros_ec_chardev.h | 18 +-
sound/soc/codecs/cros_ec_codec.c | 4 +-
sound/soc/qcom/Kconfig | 2 +-
50 files changed, 902 insertions(+), 732 deletions(-)
rename drivers/{mfd => platform/chrome}/cros_ec.c (85%)
create mode 100644 drivers/platform/chrome/cros_ec_chardev.c
rename include/linux/{mfd => platform_data}/cros_ec_commands.h (100%)
create mode 100644 include/linux/platform_data/cros_ec_proto.h
rename drivers/mfd/cros_ec_dev.h => include/uapi/linux/cros_ec_chardev.h (70%)
--
2.20.1
^ permalink raw reply
* Re: [PATCH] arm64: dts: allwinner: a64: Add lradc node
From: Luca Weiss @ 2019-06-04 15:07 UTC (permalink / raw)
To: Maxime Ripard
Cc: Hans de Goede, Dmitry Torokhov, Rob Herring, Mark Rutland,
Chen-Yu Tsai,
open list:SUN4I LOW RES ADC ATTACHED TABLET KEYS DRIVER,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
moderated list:ARM/Allwinner sunXi SoC support, open list
In-Reply-To: <20190604145907.j3fp52gxupd3bkih@flea>
[-- Attachment #1: Type: text/plain, Size: 2684 bytes --]
Hi Maxime,
sorry I forgot to mark it as a v2 and add the changelog to v1.
I've actually not split the patch into two on purpose as it's a pretty small
change and the patch adding support for the A83T didn't split out the device
tree changes - and recently in another patch of mine, the extra devicetree
patch was squashed into the driver patch by the maintainer while merging. I'll
fix this up asap.
Luca
On Dienstag, 4. Juni 2019 16:59:07 CEST Maxime Ripard wrote:
> Hi Luca,
>
> On Tue, Jun 04, 2019 at 04:42:53PM +0200, Luca Weiss wrote:
> > Add a node describing the KEYADC on the A64.
> >
> > Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
>
> You should have a changelog detailing which changes you made to your
> patch with the previous version, and why you made those changes (for
> example because we looked at the a83t datasheet and found the two
> controllers to be very similar).
>
> > ---
> >
> > .../devicetree/bindings/input/sun4i-lradc-keys.txt | 1 +
> > arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 8 ++++++++
> > 2 files changed, 9 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/input/sun4i-lradc-keys.txt
> > b/Documentation/devicetree/bindings/input/sun4i-lradc-keys.txt index
> > 496125c6bfb7..507b737612ea 100644
> > --- a/Documentation/devicetree/bindings/input/sun4i-lradc-keys.txt
> > +++ b/Documentation/devicetree/bindings/input/sun4i-lradc-keys.txt
> >
> > @@ -5,6 +5,7 @@ Required properties:
> > - compatible: should be one of the following string:
> > "allwinner,sun4i-a10-lradc-keys"
> > "allwinner,sun8i-a83t-r-lradc"
> >
> > + "allwinner,sun50i-a64-lradc", "allwinner,sun8i-a83t-r-
lradc"
> >
> > - reg: mmio address range of the chip
> > - interrupts: interrupt to which the chip is connected
> > - vref-supply: powersupply for the lradc reference voltage
> >
> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> > b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi index
> > 7734f70e1057..3a42352b5c9f 100644
> > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> > @@ -704,6 +704,14 @@
> >
> > status = "disabled";
> >
> > };
> >
> > + lradc: lradc@1c21800 {
> > + compatible = "allwinner,sun50i-a64-lradc",
> > + "allwinner,sun8i-a83t-r-
lradc";
> > + reg = <0x01c21800 0x400>;
> > + interrupts = <GIC_SPI 30
IRQ_TYPE_LEVEL_HIGH>;
> > + status = "disabled";
> > + };
> > +
>
> The bindings and the dt changes should be two different patches as
> well.
>
> Thanks!
> Maxime
>
> --
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH] arm64: dts: allwinner: a64: Add lradc node
From: Maxime Ripard @ 2019-06-04 14:59 UTC (permalink / raw)
To: Luca Weiss
Cc: Hans de Goede, Dmitry Torokhov, Rob Herring, Mark Rutland,
Chen-Yu Tsai,
open list:SUN4I LOW RES ADC ATTACHED TABLET KEYS DRIVER,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
moderated list:ARM/Allwinner sunXi SoC support, open list
In-Reply-To: <20190604144252.26965-1-luca@z3ntu.xyz>
[-- Attachment #1: Type: text/plain, Size: 2046 bytes --]
Hi Luca,
On Tue, Jun 04, 2019 at 04:42:53PM +0200, Luca Weiss wrote:
> Add a node describing the KEYADC on the A64.
>
> Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
You should have a changelog detailing which changes you made to your
patch with the previous version, and why you made those changes (for
example because we looked at the a83t datasheet and found the two
controllers to be very similar).
> ---
> .../devicetree/bindings/input/sun4i-lradc-keys.txt | 1 +
> arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 8 ++++++++
> 2 files changed, 9 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/input/sun4i-lradc-keys.txt b/Documentation/devicetree/bindings/input/sun4i-lradc-keys.txt
> index 496125c6bfb7..507b737612ea 100644
> --- a/Documentation/devicetree/bindings/input/sun4i-lradc-keys.txt
> +++ b/Documentation/devicetree/bindings/input/sun4i-lradc-keys.txt
> @@ -5,6 +5,7 @@ Required properties:
> - compatible: should be one of the following string:
> "allwinner,sun4i-a10-lradc-keys"
> "allwinner,sun8i-a83t-r-lradc"
> + "allwinner,sun50i-a64-lradc", "allwinner,sun8i-a83t-r-lradc"
> - reg: mmio address range of the chip
> - interrupts: interrupt to which the chip is connected
> - vref-supply: powersupply for the lradc reference voltage
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> index 7734f70e1057..3a42352b5c9f 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> @@ -704,6 +704,14 @@
> status = "disabled";
> };
>
> + lradc: lradc@1c21800 {
> + compatible = "allwinner,sun50i-a64-lradc",
> + "allwinner,sun8i-a83t-r-lradc";
> + reg = <0x01c21800 0x400>;
> + interrupts = <GIC_SPI 30 IRQ_TYPE_LEVEL_HIGH>;
> + status = "disabled";
> + };
> +
The bindings and the dt changes should be two different patches as
well.
Thanks!
Maxime
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* [PATCH] arm64: dts: allwinner: a64: Add lradc node
From: Luca Weiss @ 2019-06-04 14:42 UTC (permalink / raw)
Cc: Luca Weiss, Hans de Goede, Dmitry Torokhov, Rob Herring,
Mark Rutland, Maxime Ripard, Chen-Yu Tsai,
open list:SUN4I LOW RES ADC ATTACHED TABLET KEYS DRIVER,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
moderated list:ARM/Allwinner sunXi SoC support, open list
Add a node describing the KEYADC on the A64.
Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
---
.../devicetree/bindings/input/sun4i-lradc-keys.txt | 1 +
arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 8 ++++++++
2 files changed, 9 insertions(+)
diff --git a/Documentation/devicetree/bindings/input/sun4i-lradc-keys.txt b/Documentation/devicetree/bindings/input/sun4i-lradc-keys.txt
index 496125c6bfb7..507b737612ea 100644
--- a/Documentation/devicetree/bindings/input/sun4i-lradc-keys.txt
+++ b/Documentation/devicetree/bindings/input/sun4i-lradc-keys.txt
@@ -5,6 +5,7 @@ Required properties:
- compatible: should be one of the following string:
"allwinner,sun4i-a10-lradc-keys"
"allwinner,sun8i-a83t-r-lradc"
+ "allwinner,sun50i-a64-lradc", "allwinner,sun8i-a83t-r-lradc"
- reg: mmio address range of the chip
- interrupts: interrupt to which the chip is connected
- vref-supply: powersupply for the lradc reference voltage
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
index 7734f70e1057..3a42352b5c9f 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
@@ -704,6 +704,14 @@
status = "disabled";
};
+ lradc: lradc@1c21800 {
+ compatible = "allwinner,sun50i-a64-lradc",
+ "allwinner,sun8i-a83t-r-lradc";
+ reg = <0x01c21800 0x400>;
+ interrupts = <GIC_SPI 30 IRQ_TYPE_LEVEL_HIGH>;
+ status = "disabled";
+ };
+
i2s0: i2s@1c22000 {
#sound-dai-cells = <0>;
compatible = "allwinner,sun50i-a64-i2s",
--
2.21.0
^ permalink raw reply related
* Re: hid-related 5.2-rc1 boot hang
From: Hans de Goede @ 2019-06-04 13:12 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Jiri Kosina, Dave Hansen, open list:HID CORE LAYER, LKML
In-Reply-To: <CAO-hwJ+ZBuXtuk+i1Q9DwO=uLXDh4oToQyMWhK-8t+ZTS-jUjA@mail.gmail.com>
Hi,
On 04-06-19 14:25, Benjamin Tissoires wrote:
> On Tue, Jun 4, 2019 at 12:50 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> Hi,
>>
>> On 04-06-19 12:08, Benjamin Tissoires wrote:
>>> On Tue, Jun 4, 2019 at 9:51 AM Benjamin Tissoires
>>> <benjamin.tissoires@redhat.com> wrote:
>>>>
>>>> On Mon, Jun 3, 2019 at 4:17 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> On 03-06-19 15:55, Benjamin Tissoires wrote:
>>>>>> On Mon, Jun 3, 2019 at 11:51 AM Hans de Goede <hdegoede@redhat.com> wrote:
>>>>>>>
>>>>>>> Hi Again,
>>>>>>>
>>>>>>> On 03-06-19 11:11, Hans de Goede wrote:
>>>>>>> <snip>
>>>>>>>
>>>>>>>>> not sure about the rest of logitech issues yet) next week.
>>>>>>>>
>>>>>>>> The main problem seems to be the request_module patches. Although I also
>>>>>>
>>>>>> Can't we use request_module_nowait() instead, and set a reasonable
>>>>>> timeout that we detect only once to check if userspace is compatible:
>>>>>>
>>>>>> In pseudo-code:
>>>>>> if (!request_module_checked) {
>>>>>> request_module_nowait(name);
>>>>>> use_request_module = wait_event_timeout(wq,
>>>>>> first_module_loaded, 10 seconds in jiffies);
>>>>>> request_module_checked = true;
>>>>>> } else if (use_request_module) {
>>>>>> request_module(name);
>>>>>> }
>>>>>
>>>>> Well looking at the just attached dmesg , the modprobe
>>>>> when triggered by udev from userspace succeeds in about
>>>>> 0.5 seconds, so it seems that the modprobe hangs happens
>>>>> when called from within the kernel rather then from within
>>>>> userspace.
>>>>>
>>>>> What I do not know if is the hang is inside userspace, or
>>>>> maybe it happens when modprobe calls back into the kernel,
>>>>> if the hang happens when modprobe calls back into the kernel,
>>>>> then other modprobes (done from udev) likely will hang too
>>>>> since I think only 1 modprobe can happen at a time.
>>>>>
>>>>> I really wish we knew what distinguished working systems
>>>>> from non working systems :|
>>>>>
>>>>> I cannot find a common denominator; other then the systems
>>>>> are not running Fedora. So far we've reports from both Ubuntu 16.04
>>>>> and Tumbleweed, so software version wise these 2 are wide apart.
>>>>
>>>> I am trying to reproduce the lock locally, and installed an opensuse
>>>> Tumbleweed in a VM. When forwarding a Unifying receiver to the VM, I
>>>> do not see the lock with either my vanilla compiled kernel and the rpm
>>>> found in http://download.opensuse.org/repositories/Kernel:/HEAD/standard/x86_64/
>>>>
>>>> Next step is install Tumbleweed on bare metal, but I do not see how
>>>> this could introduce a difference (maybe USB2 vs 3).
>>>
>>> Making progress here.
>>>
>>> The difference between Ubuntu/Tumbleweed and Fedora: usbhid is shipped
>>> as a module while in Fedora usbhid is included in the kernel.
>>>
>>> If I rmmod hid_* and usbhid, then modprobe usbhid, the command hangs
>>> for 3 minutes.
>>> If usbhid is already loaded, inserting a receiver is immediate
>>> regarding the loading of the external modules.
>>>
>>> So my assumption is that when the device gets detected at boot, usbhid
>>> gets loaded by the kernel event, which in turns attempts to call
>>> __request_module, but the modprobe can't be fulfilled because it's
>>> already waiting for the initial usbhid modprobe to finish.
>>>
>>> Still don't know how to solve that, but I thought I should share.
>>
>> Hmm, we may be hitting the scenario described in the big comment
>> around line 3500 of kernel/module.c.
>
> Well, we are not locking during do_init_module(), but in waiting for
> the completion of request_module(). So as I read the trace, we wait
> for userspace to call/terminate modprobe.
>
>>
>> But I'm not sure that is what is happening here.
>>
>> Maybe you can put a WARN_ON(1) in request_module and look at the
>> backtrace ? That may help to figure out what is going on; or
>> alternatively it might help to find some way to detect this and
>> if it happens skip the request_module...
>
> Ftrace is much easier to deal with:
> ---
> /sys/kernel/debug/tracing # cat trace
> # tracer: function
> #
> # entries-in-buffer/entries-written: 4/4 #P:4
> #
> # _-----=> irqs-off
> # / _----=> need-resched
> # | / _---=> hardirq/softirq
> # || / _--=> preempt-depth
> # ||| / delay
> # TASK-PID CPU# |||| TIMESTAMP FUNCTION
> # | | | |||| | |
> modprobe-23236 [003] .... 9191.880917: __request_module
> <-hid_add_device
> modprobe-23236 [003] .... 9191.880937: <stack trace>
> => 0xffffffffc0851061
> => __request_module
> => hid_add_device
> => usbhid_probe
> => usb_probe_interface
> => really_probe
> => driver_probe_device
> => device_driver_attach
> => __driver_attach
> => bus_for_each_dev
> => bus_add_driver
> => driver_register
> => usb_register_driver
> => hid_init
> => do_one_initcall
> => do_init_module
> => load_module
> => __do_sys_finit_module
> => do_syscall_64
> => entry_SYSCALL_64_after_hwframe
> modprobe-23236 [003] .... 9191.892765: __request_module
> <-hid_add_device
> modprobe-23236 [003] .... 9191.892784: <stack trace>
> => 0xffffffffc0851061
> => __request_module
> => hid_add_device
> => usbhid_probe
> => usb_probe_interface
> => really_probe
> => driver_probe_device
> => device_driver_attach
> => __driver_attach
> => bus_for_each_dev
> => bus_add_driver
> => driver_register
> => usb_register_driver
> => hid_init
> => do_one_initcall
> => do_init_module
> => load_module
> => __do_sys_finit_module
> => do_syscall_64
> => entry_SYSCALL_64_after_hwframe
> ---
>
> This is what happen with the logitech receiver plugged in and while
> modprobing usbhid. The modprobe command hangs, and I took the trace
> here.
> If I hit Ctrl-C, the modprobe loading continue properly.
>
> There is nothing special here in the stack trace, except that there
> are 2 calls to request_module here (one for the touchscreen I guess
> and one for the logitech receiver).
>
> I have attached the call graph of the same sequence:
> wait_for_completion_killable() seems to be the culprit, we are waiting
> for userspace to notify it has done calling modprobe.
>
> My idea would be to defer any call to hid_add_device() into a
> workqueue and see if that unlocks the situation.
Yes that is probably a good solution. When the first bug reports came
in I was thinking that the nested hid_add_device calls from hid-logitech-dj
calling hid_add_device() were the problem, but those are already deferred
to a workqueue so those are not really nested.
Looking at various callers if hid_add_device, if we always defer, we also
need the caller to give a callback to call on add_device error, which would
then be used to free various resources related to the hid device.
If me make hid_add_device itself always defer, I guess we may want a non
deferred version of hid_add_device for the hid_add_device calls in
hid-logitech-dj.
In essence we are dealing with nested hid_add_device calls here right?
So another solution would be to have an atomic counter and call
atomic_long_inc_return on it before the hid_add_device and then if its
previous value was not 0, skip the request_module ?
This does mean though that we then may get inconsistent behavior if
2 unrelated hid_add_device-s are racing with each-other :|
Regards,
Hans
^ permalink raw reply
* Re: hid-related 5.2-rc1 boot hang
From: Benjamin Tissoires @ 2019-06-04 12:25 UTC (permalink / raw)
To: Hans de Goede; +Cc: Jiri Kosina, Dave Hansen, open list:HID CORE LAYER, LKML
In-Reply-To: <ea7b2dee-15a6-9b52-fbff-558c36cd72df@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 6271 bytes --]
On Tue, Jun 4, 2019 at 12:50 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi,
>
> On 04-06-19 12:08, Benjamin Tissoires wrote:
> > On Tue, Jun 4, 2019 at 9:51 AM Benjamin Tissoires
> > <benjamin.tissoires@redhat.com> wrote:
> >>
> >> On Mon, Jun 3, 2019 at 4:17 PM Hans de Goede <hdegoede@redhat.com> wrote:
> >>>
> >>> Hi,
> >>>
> >>> On 03-06-19 15:55, Benjamin Tissoires wrote:
> >>>> On Mon, Jun 3, 2019 at 11:51 AM Hans de Goede <hdegoede@redhat.com> wrote:
> >>>>>
> >>>>> Hi Again,
> >>>>>
> >>>>> On 03-06-19 11:11, Hans de Goede wrote:
> >>>>> <snip>
> >>>>>
> >>>>>>> not sure about the rest of logitech issues yet) next week.
> >>>>>>
> >>>>>> The main problem seems to be the request_module patches. Although I also
> >>>>
> >>>> Can't we use request_module_nowait() instead, and set a reasonable
> >>>> timeout that we detect only once to check if userspace is compatible:
> >>>>
> >>>> In pseudo-code:
> >>>> if (!request_module_checked) {
> >>>> request_module_nowait(name);
> >>>> use_request_module = wait_event_timeout(wq,
> >>>> first_module_loaded, 10 seconds in jiffies);
> >>>> request_module_checked = true;
> >>>> } else if (use_request_module) {
> >>>> request_module(name);
> >>>> }
> >>>
> >>> Well looking at the just attached dmesg , the modprobe
> >>> when triggered by udev from userspace succeeds in about
> >>> 0.5 seconds, so it seems that the modprobe hangs happens
> >>> when called from within the kernel rather then from within
> >>> userspace.
> >>>
> >>> What I do not know if is the hang is inside userspace, or
> >>> maybe it happens when modprobe calls back into the kernel,
> >>> if the hang happens when modprobe calls back into the kernel,
> >>> then other modprobes (done from udev) likely will hang too
> >>> since I think only 1 modprobe can happen at a time.
> >>>
> >>> I really wish we knew what distinguished working systems
> >>> from non working systems :|
> >>>
> >>> I cannot find a common denominator; other then the systems
> >>> are not running Fedora. So far we've reports from both Ubuntu 16.04
> >>> and Tumbleweed, so software version wise these 2 are wide apart.
> >>
> >> I am trying to reproduce the lock locally, and installed an opensuse
> >> Tumbleweed in a VM. When forwarding a Unifying receiver to the VM, I
> >> do not see the lock with either my vanilla compiled kernel and the rpm
> >> found in http://download.opensuse.org/repositories/Kernel:/HEAD/standard/x86_64/
> >>
> >> Next step is install Tumbleweed on bare metal, but I do not see how
> >> this could introduce a difference (maybe USB2 vs 3).
> >
> > Making progress here.
> >
> > The difference between Ubuntu/Tumbleweed and Fedora: usbhid is shipped
> > as a module while in Fedora usbhid is included in the kernel.
> >
> > If I rmmod hid_* and usbhid, then modprobe usbhid, the command hangs
> > for 3 minutes.
> > If usbhid is already loaded, inserting a receiver is immediate
> > regarding the loading of the external modules.
> >
> > So my assumption is that when the device gets detected at boot, usbhid
> > gets loaded by the kernel event, which in turns attempts to call
> > __request_module, but the modprobe can't be fulfilled because it's
> > already waiting for the initial usbhid modprobe to finish.
> >
> > Still don't know how to solve that, but I thought I should share.
>
> Hmm, we may be hitting the scenario described in the big comment
> around line 3500 of kernel/module.c.
Well, we are not locking during do_init_module(), but in waiting for
the completion of request_module(). So as I read the trace, we wait
for userspace to call/terminate modprobe.
>
> But I'm not sure that is what is happening here.
>
> Maybe you can put a WARN_ON(1) in request_module and look at the
> backtrace ? That may help to figure out what is going on; or
> alternatively it might help to find some way to detect this and
> if it happens skip the request_module...
Ftrace is much easier to deal with:
---
/sys/kernel/debug/tracing # cat trace
# tracer: function
#
# entries-in-buffer/entries-written: 4/4 #P:4
#
# _-----=> irqs-off
# / _----=> need-resched
# | / _---=> hardirq/softirq
# || / _--=> preempt-depth
# ||| / delay
# TASK-PID CPU# |||| TIMESTAMP FUNCTION
# | | | |||| | |
modprobe-23236 [003] .... 9191.880917: __request_module
<-hid_add_device
modprobe-23236 [003] .... 9191.880937: <stack trace>
=> 0xffffffffc0851061
=> __request_module
=> hid_add_device
=> usbhid_probe
=> usb_probe_interface
=> really_probe
=> driver_probe_device
=> device_driver_attach
=> __driver_attach
=> bus_for_each_dev
=> bus_add_driver
=> driver_register
=> usb_register_driver
=> hid_init
=> do_one_initcall
=> do_init_module
=> load_module
=> __do_sys_finit_module
=> do_syscall_64
=> entry_SYSCALL_64_after_hwframe
modprobe-23236 [003] .... 9191.892765: __request_module
<-hid_add_device
modprobe-23236 [003] .... 9191.892784: <stack trace>
=> 0xffffffffc0851061
=> __request_module
=> hid_add_device
=> usbhid_probe
=> usb_probe_interface
=> really_probe
=> driver_probe_device
=> device_driver_attach
=> __driver_attach
=> bus_for_each_dev
=> bus_add_driver
=> driver_register
=> usb_register_driver
=> hid_init
=> do_one_initcall
=> do_init_module
=> load_module
=> __do_sys_finit_module
=> do_syscall_64
=> entry_SYSCALL_64_after_hwframe
---
This is what happen with the logitech receiver plugged in and while
modprobing usbhid. The modprobe command hangs, and I took the trace
here.
If I hit Ctrl-C, the modprobe loading continue properly.
There is nothing special here in the stack trace, except that there
are 2 calls to request_module here (one for the touchscreen I guess
and one for the logitech receiver).
I have attached the call graph of the same sequence:
wait_for_completion_killable() seems to be the culprit, we are waiting
for userspace to notify it has done calling modprobe.
My idea would be to defer any call to hid_add_device() into a
workqueue and see if that unlocks the situation.
Cheers,
Benjamin
[-- Attachment #2: function_graph.txt --]
[-- Type: text/plain, Size: 21638 bytes --]
/sys/kernel/debug/tracing # cat trace
# tracer: function_graph
#
# CPU DURATION FUNCTION CALLS
# | | | | | | |
0) | __request_module() {
0) 0.256 us | current_is_async();
0) | security_kernel_module_request() {
0) 0.307 us | integrity_kernel_module_request();
0) 0.625 us | }
0) | kmem_cache_alloc_trace() {
0) | _cond_resched() {
0) 0.123 us | rcu_all_qs();
0) 0.355 us | }
0) 0.122 us | should_failslab();
0) | _cond_resched() {
0) 0.120 us | rcu_all_qs();
0) 0.355 us | }
0) 0.120 us | memcg_kmem_put_cache();
0) 1.563 us | }
0) | kstrdup() {
0) | __kmalloc_track_caller() {
0) 0.115 us | kmalloc_slab();
0) | _cond_resched() {
0) 0.118 us | rcu_all_qs();
0) 0.348 us | }
0) 0.119 us | should_failslab();
0) | _cond_resched() {
0) 0.115 us | rcu_all_qs();
0) 0.345 us | }
0) 0.119 us | memcg_kmem_put_cache();
0) 1.757 us | }
0) 2.040 us | }
0) | call_usermodehelper_setup() {
0) | kmem_cache_alloc_trace() {
0) | _cond_resched() {
0) 0.116 us | rcu_all_qs();
0) 0.345 us | }
0) 0.117 us | should_failslab();
0) | _cond_resched() {
0) 0.118 us | rcu_all_qs();
0) 0.347 us | }
0) 0.123 us | memcg_kmem_put_cache();
0) 1.792 us | }
0) 2.023 us | }
0) | call_usermodehelper_exec() {
0) | queue_work_on() {
0) | __queue_work() {
0) 0.126 us | get_work_pool();
0) 0.183 us | _raw_spin_lock();
0) | insert_work() {
0) 0.121 us | get_pwq.isra.0();
0) | wake_up_process() {
0) | try_to_wake_up() {
0) 0.227 us | _raw_spin_lock_irqsave();
0) | select_task_rq_fair() {
0) 0.133 us | available_idle_cpu();
0) 0.166 us | update_cfs_rq_h_load();
0) | select_idle_sibling() {
0) 0.137 us | available_idle_cpu();
0) 0.371 us | }
0) 1.299 us | }
0) 0.133 us | _raw_spin_lock();
0) 0.148 us | update_rq_clock();
0) | ttwu_do_activate() {
0) | activate_task() {
0) | enqueue_task_fair() {
0) | enqueue_entity() {
0) 0.123 us | update_curr();
0) | __update_load_avg_se() {
0) 0.118 us | __accumulate_pelt_segments();
0) 0.420 us | }
0) 0.125 us | __update_load_avg_cfs_rq();
0) 0.122 us | update_cfs_group();
0) 0.138 us | account_entity_enqueue();
0) 0.120 us | place_entity();
0) 0.119 us | __enqueue_entity();
0) 2.146 us | }
0) 0.115 us | hrtick_update();
0) 2.624 us | }
0) 2.890 us | }
0) | ttwu_do_wakeup() {
0) | check_preempt_curr() {
0) 0.130 us | resched_curr();
0) 0.417 us | }
0) 1.034 us | }
0) 4.269 us | }
0) 0.410 us | _raw_spin_unlock_irqrestore();
0) 7.466 us | }
0) 7.702 us | }
0) 8.164 us | }
0) 9.079 us | }
0) 9.370 us | }
0) | wait_for_completion_killable() {
0) | _cond_resched() {
0) 0.118 us | rcu_all_qs();
0) 0.365 us | }
0) 0.122 us | _raw_spin_lock_irq();
0) | schedule_timeout() {
0) | schedule() {
0) | rcu_note_context_switch() {
0) 0.147 us | rcu_qs();
0) 0.384 us | }
0) 0.124 us | _raw_spin_lock();
0) 0.144 us | update_rq_clock();
0) | deactivate_task() {
0) | dequeue_task_fair() {
0) | dequeue_entity() {
0) | update_curr() {
0) 0.121 us | update_min_vruntime();
0) 0.132 us | cpuacct_charge();
0) | __cgroup_account_cputime() {
0) 0.124 us | cgroup_rstat_updated();
0) 0.354 us | }
0) 1.088 us | }
0) 0.127 us | __update_load_avg_se();
0) 0.136 us | __update_load_avg_cfs_rq();
0) 0.118 us | clear_buddies();
0) 0.124 us | account_entity_dequeue();
0) 0.120 us | update_cfs_group();
0) 0.128 us | update_min_vruntime();
0) 2.780 us | }
0) 0.114 us | hrtick_update();
0) 3.242 us | }
0) 3.503 us | }
0) | pick_next_task_fair() {
0) 0.121 us | __msecs_to_jiffies();
0) 0.389 us | }
0) | pick_next_task_idle() {
0) | put_prev_task_fair() {
0) | put_prev_entity() {
0) 0.122 us | check_cfs_rq_runtime();
0) 0.355 us | }
0) 0.581 us | }
0) 0.123 us | __update_idle_core();
0) 1.055 us | }
0) 0.118 us | enter_lazy_tlb();
2) | finish_task_switch() {
2) | __mmdrop() {
2) | pgd_free() {
2) 0.248 us | _raw_spin_lock();
2) | free_pages() {
2) | free_pages.part.0() {
2) | __free_pages() {
2) | __free_pages_ok() {
2) | free_one_page() {
2) 0.213 us | _raw_spin_lock();
2) 0.219 us | __mod_zone_page_state();
2) 1.102 us | }
2) 1.839 us | }
2) 2.337 us | }
2) 2.672 us | }
2) 2.986 us | }
2) 4.044 us | }
2) 0.218 us | destroy_context_ldt();
2) | kmem_cache_free() {
2) 0.416 us | ___cache_free();
2) 1.051 us | }
2) 6.234 us | }
2) 8.542 us | }
2) * 13308.49 us | } /* schedule */
2) * 13308.85 us | } /* schedule_timeout */
2) 0.235 us | _raw_spin_lock_irq();
2) * 13310.47 us | } /* wait_for_completion_killable */
2) | free_modprobe_argv() {
2) | kfree() {
2) 0.213 us | ___cache_free();
2) 0.657 us | }
2) | kfree() {
2) 0.153 us | ___cache_free();
2) 0.557 us | }
2) 1.732 us | }
2) | kfree() {
2) 0.207 us | ___cache_free();
2) 0.734 us | }
2) | __wake_up() {
2) | __wake_up_common_lock() {
2) 0.265 us | _raw_spin_lock_irqsave();
2) 0.199 us | __wake_up_common();
2) 0.214 us | _raw_spin_unlock_irqrestore();
2) 1.333 us | }
2) 1.648 us | }
2) * 13325.51 us | } /* call_usermodehelper_exec */
2) | __wake_up() {
2) | __wake_up_common_lock() {
2) 0.195 us | _raw_spin_lock_irqsave();
2) 0.200 us | __wake_up_common();
2) 0.189 us | _raw_spin_unlock_irqrestore();
2) 1.274 us | }
2) 1.629 us | }
2) * 13337.37 us | } /* __request_module */
2) | __request_module() {
2) 0.279 us | current_is_async();
2) | security_kernel_module_request() {
2) 0.310 us | integrity_kernel_module_request();
2) 0.799 us | }
2) | kmem_cache_alloc_trace() {
2) | _cond_resched() {
2) 0.209 us | rcu_all_qs();
2) 0.598 us | }
2) 0.208 us | should_failslab();
2) | _cond_resched() {
2) 0.202 us | rcu_all_qs();
2) 0.597 us | }
2) 0.206 us | memcg_kmem_put_cache();
2) 2.596 us | }
2) | kstrdup() {
2) | __kmalloc_track_caller() {
2) 0.209 us | kmalloc_slab();
2) | _cond_resched() {
2) 0.200 us | rcu_all_qs();
2) 0.598 us | }
2) 0.399 us | should_failslab();
2) | _cond_resched() {
2) 0.179 us | rcu_all_qs();
2) 0.491 us | }
2) 0.174 us | memcg_kmem_put_cache();
2) 3.004 us | }
2) 3.424 us | }
2) | call_usermodehelper_setup() {
2) | kmem_cache_alloc_trace() {
2) | _cond_resched() {
2) 0.171 us | rcu_all_qs();
2) 0.816 us | }
2) 0.173 us | should_failslab();
2) | _cond_resched() {
2) 0.188 us | rcu_all_qs();
2) 0.544 us | }
2) 0.205 us | memcg_kmem_put_cache();
2) 2.728 us | }
2) 3.100 us | }
2) | call_usermodehelper_exec() {
2) | queue_work_on() {
2) | __queue_work() {
2) 0.213 us | get_work_pool();
2) 0.206 us | _raw_spin_lock();
2) | insert_work() {
2) 0.194 us | get_pwq.isra.0();
2) | wake_up_process() {
2) | try_to_wake_up() {
2) 0.248 us | _raw_spin_lock_irqsave();
2) | select_task_rq_fair() {
2) 0.205 us | available_idle_cpu();
2) 0.252 us | update_cfs_rq_h_load();
2) | select_idle_sibling() {
2) 0.277 us | available_idle_cpu();
2) 0.666 us | }
2) 2.074 us | }
2) 0.213 us | _raw_spin_lock();
2) 0.240 us | update_rq_clock();
2) | ttwu_do_activate() {
2) | activate_task() {
2) | enqueue_task_fair() {
2) | enqueue_entity() {
2) 0.209 us | update_curr();
2) | __update_load_avg_se() {
2) 0.198 us | __accumulate_pelt_segments();
2) 0.715 us | }
2) 0.221 us | __update_load_avg_cfs_rq();
2) 0.205 us | update_cfs_group();
2) 0.274 us | account_entity_enqueue();
2) 0.209 us | place_entity();
2) 0.211 us | __enqueue_entity();
2) 3.718 us | }
2) 0.208 us | hrtick_update();
2) 4.568 us | }
2) 5.005 us | }
2) | ttwu_do_wakeup() {
2) | check_preempt_curr() {
2) 0.222 us | resched_curr();
2) 0.700 us | }
2) 1.473 us | }
2) 7.098 us | }
2) 0.218 us | _raw_spin_unlock_irqrestore();
2) + 11.605 us | }
2) + 11.947 us | }
2) + 12.730 us | }
2) + 14.091 us | }
2) + 14.527 us | }
2) | wait_for_completion_killable() {
2) | _cond_resched() {
2) 0.210 us | rcu_all_qs();
2) 0.636 us | }
2) 0.241 us | _raw_spin_lock_irq();
2) | schedule_timeout() {
2) | schedule() {
2) | rcu_note_context_switch() {
2) 0.217 us | rcu_qs();
2) 0.618 us | }
2) 0.200 us | _raw_spin_lock();
2) 0.213 us | update_rq_clock();
2) | deactivate_task() {
2) | dequeue_task_fair() {
2) | dequeue_entity() {
2) | update_curr() {
2) 0.196 us | update_min_vruntime();
2) 0.239 us | cpuacct_charge();
2) | __cgroup_account_cputime() {
2) 0.202 us | cgroup_rstat_updated();
2) 0.638 us | }
2) 1.946 us | }
2) 0.210 us | __update_load_avg_se();
2) 0.213 us | __update_load_avg_cfs_rq();
2) 0.200 us | clear_buddies();
2) 0.212 us | account_entity_dequeue();
2) 0.203 us | update_cfs_group();
2) 0.207 us | update_min_vruntime();
2) 5.029 us | }
2) 0.194 us | hrtick_update();
2) 5.793 us | }
2) 6.205 us | }
2) | pick_next_task_fair() {
2) | update_blocked_averages() {
2) 0.149 us | _raw_spin_lock_irqsave();
2) 0.205 us | update_rq_clock();
2) 0.167 us | __update_load_avg_cfs_rq();
2) 0.206 us | update_rt_rq_load_avg();
2) 0.184 us | update_dl_rq_load_avg();
2) 0.183 us | _raw_spin_unlock_irqrestore();
2) 2.510 us | }
2) | load_balance() {
2) | find_busiest_group() {
2) 0.172 us | update_nohz_stats();
2) 0.176 us | idle_cpu();
2) 0.207 us | update_nohz_stats();
2) 1.818 us | }
2) 2.339 us | }
2) 0.171 us | __msecs_to_jiffies();
2) | load_balance() {
2) | find_busiest_group() {
2) 0.211 us | update_nohz_stats();
2) 0.203 us | update_nohz_stats();
2) 0.190 us | idle_cpu();
2) 0.216 us | update_nohz_stats();
2) 0.222 us | update_nohz_stats();
2) 2.711 us | }
2) 0.228 us | _raw_spin_lock_irqsave();
2) 0.262 us | update_rq_clock();
2) 0.233 us | can_migrate_task();
2) 0.322 us | can_migrate_task();
2) 0.222 us | update_cfs_rq_h_load();
2) | deactivate_task() {
2) | dequeue_task_fair() {
2) | dequeue_entity() {
2) | update_curr() {
2) 0.209 us | update_min_vruntime();
2) 0.259 us | cpuacct_charge();
2) | __cgroup_account_cputime() {
2) 0.193 us | cgroup_rstat_updated();
2) 0.738 us | }
2) 2.022 us | }
2) 0.223 us | __update_load_avg_se();
2) 0.213 us | __update_load_avg_cfs_rq();
2) 0.202 us | clear_buddies();
2) 0.215 us | account_entity_dequeue();
2) 0.204 us | update_cfs_group();
2) 0.202 us | update_min_vruntime();
2) 4.891 us | }
2) 0.205 us | hrtick_update();
2) 5.711 us | }
2) 6.143 us | }
2) | set_task_cpu() {
2) | migrate_task_rq_fair() {
2) | detach_entity_cfs_rq() {
2) 0.215 us | __update_load_avg_se();
2) 0.204 us | __update_load_avg_cfs_rq();
2) 0.219 us | propagate_entity_cfs_rq.isra.0();
2) 1.433 us | }
2) 1.814 us | }
2) 0.195 us | set_task_rq_fair();
2) 2.664 us | }
2) 0.202 us | _raw_spin_lock();
2) 0.219 us | update_rq_clock();
2) | attach_task() {
2) | activate_task() {
2) | enqueue_task_fair() {
2) | enqueue_entity() {
2) | update_curr() {
2) 0.225 us | update_min_vruntime();
2) 0.202 us | cpuacct_charge();
2) | __cgroup_account_cputime() {
2) 0.196 us | cgroup_rstat_updated();
2) 0.569 us | }
2) 2.120 us | }
2) 0.208 us | __update_load_avg_cfs_rq();
2) 0.219 us | attach_entity_load_avg();
2) 0.203 us | update_cfs_group();
2) 0.184 us | account_entity_enqueue();
2) 0.217 us | __enqueue_entity();
2) 4.678 us | }
2) 0.177 us | hrtick_update();
2) 5.408 us | }
2) 5.795 us | }
2) | check_preempt_curr() {
2) | check_preempt_wakeup() {
2) 0.205 us | update_curr();
2) 0.201 us | wakeup_preempt_entity.isra.0();
2) 0.970 us | }
2) 1.359 us | }
2) 7.688 us | }
2) + 23.511 us | }
2) 0.201 us | __msecs_to_jiffies();
2) 0.205 us | _raw_spin_lock();
2) 0.197 us | check_cfs_rq_runtime();
2) | pick_next_entity() {
2) 0.198 us | clear_buddies();
2) 0.600 us | }
2) | put_prev_entity() {
2) 0.188 us | check_cfs_rq_runtime();
2) 0.583 us | }
2) | set_next_entity() {
2) 0.209 us | __update_load_avg_se();
2) 0.215 us | __update_load_avg_cfs_rq();
2) 1.053 us | }
2) + 33.704 us | }
2) | switch_mm_irqs_off() {
2) 0.607 us | load_new_mm_cr3();
2) 2.798 us | }
3) 5.150 us | finish_task_switch();
--------------------------------------------------------------------------------
Ctrl-C hit, notice the delay
--------------------------------------------------------------------------------
3) $ 54568382 us | } /* schedule */
3) $ 54568383 us | } /* schedule_timeout */
3) 1.011 us | _raw_spin_lock_irq();
3) $ 54568388 us | } /* wait_for_completion_killable */
3) | __wake_up() {
3) | __wake_up_common_lock() {
3) 0.911 us | _raw_spin_lock_irqsave();
3) 1.015 us | __wake_up_common();
3) 0.958 us | _raw_spin_unlock_irqrestore();
3) 6.186 us | }
3) 7.650 us | }
3) $ 54568414 us | } /* call_usermodehelper_exec */
3) | __wake_up() {
3) | __wake_up_common_lock() {
3) 0.720 us | _raw_spin_lock_irqsave();
3) 0.826 us | __wake_up_common();
3) 0.850 us | _raw_spin_unlock_irqrestore();
3) 5.655 us | }
3) 7.309 us | }
3) $ 54568437 us | } /* __request_module */
^ permalink raw reply
* Re: hid-related 5.2-rc1 boot hang
From: Hans de Goede @ 2019-06-04 10:50 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Jiri Kosina, Dave Hansen, open list:HID CORE LAYER, LKML
In-Reply-To: <CAO-hwJK0j8SottiqCdDseBW_vR=GjKO4YrFCtjzYeUh-eKPOpA@mail.gmail.com>
Hi,
On 04-06-19 12:08, Benjamin Tissoires wrote:
> On Tue, Jun 4, 2019 at 9:51 AM Benjamin Tissoires
> <benjamin.tissoires@redhat.com> wrote:
>>
>> On Mon, Jun 3, 2019 at 4:17 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>>
>>> Hi,
>>>
>>> On 03-06-19 15:55, Benjamin Tissoires wrote:
>>>> On Mon, Jun 3, 2019 at 11:51 AM Hans de Goede <hdegoede@redhat.com> wrote:
>>>>>
>>>>> Hi Again,
>>>>>
>>>>> On 03-06-19 11:11, Hans de Goede wrote:
>>>>> <snip>
>>>>>
>>>>>>> not sure about the rest of logitech issues yet) next week.
>>>>>>
>>>>>> The main problem seems to be the request_module patches. Although I also
>>>>
>>>> Can't we use request_module_nowait() instead, and set a reasonable
>>>> timeout that we detect only once to check if userspace is compatible:
>>>>
>>>> In pseudo-code:
>>>> if (!request_module_checked) {
>>>> request_module_nowait(name);
>>>> use_request_module = wait_event_timeout(wq,
>>>> first_module_loaded, 10 seconds in jiffies);
>>>> request_module_checked = true;
>>>> } else if (use_request_module) {
>>>> request_module(name);
>>>> }
>>>
>>> Well looking at the just attached dmesg , the modprobe
>>> when triggered by udev from userspace succeeds in about
>>> 0.5 seconds, so it seems that the modprobe hangs happens
>>> when called from within the kernel rather then from within
>>> userspace.
>>>
>>> What I do not know if is the hang is inside userspace, or
>>> maybe it happens when modprobe calls back into the kernel,
>>> if the hang happens when modprobe calls back into the kernel,
>>> then other modprobes (done from udev) likely will hang too
>>> since I think only 1 modprobe can happen at a time.
>>>
>>> I really wish we knew what distinguished working systems
>>> from non working systems :|
>>>
>>> I cannot find a common denominator; other then the systems
>>> are not running Fedora. So far we've reports from both Ubuntu 16.04
>>> and Tumbleweed, so software version wise these 2 are wide apart.
>>
>> I am trying to reproduce the lock locally, and installed an opensuse
>> Tumbleweed in a VM. When forwarding a Unifying receiver to the VM, I
>> do not see the lock with either my vanilla compiled kernel and the rpm
>> found in http://download.opensuse.org/repositories/Kernel:/HEAD/standard/x86_64/
>>
>> Next step is install Tumbleweed on bare metal, but I do not see how
>> this could introduce a difference (maybe USB2 vs 3).
>
> Making progress here.
>
> The difference between Ubuntu/Tumbleweed and Fedora: usbhid is shipped
> as a module while in Fedora usbhid is included in the kernel.
>
> If I rmmod hid_* and usbhid, then modprobe usbhid, the command hangs
> for 3 minutes.
> If usbhid is already loaded, inserting a receiver is immediate
> regarding the loading of the external modules.
>
> So my assumption is that when the device gets detected at boot, usbhid
> gets loaded by the kernel event, which in turns attempts to call
> __request_module, but the modprobe can't be fulfilled because it's
> already waiting for the initial usbhid modprobe to finish.
>
> Still don't know how to solve that, but I thought I should share.
Hmm, we may be hitting the scenario described in the big comment
around line 3500 of kernel/module.c.
But I'm not sure that is what is happening here.
Maybe you can put a WARN_ON(1) in request_module and look at the
backtrace ? That may help to figure out what is going on; or
alternatively it might help to find some way to detect this and
if it happens skip the request_module...
Regards,
Hans
^ permalink raw reply
* Re: hid-related 5.2-rc1 boot hang
From: Benjamin Tissoires @ 2019-06-04 10:08 UTC (permalink / raw)
To: Hans de Goede; +Cc: Jiri Kosina, Dave Hansen, open list:HID CORE LAYER, LKML
In-Reply-To: <CAO-hwJ+5UYJMnuCS0UL4g45Xc181LraAzc-CMuYB2rcqKGe_Sw@mail.gmail.com>
On Tue, Jun 4, 2019 at 9:51 AM Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
>
> On Mon, Jun 3, 2019 at 4:17 PM Hans de Goede <hdegoede@redhat.com> wrote:
> >
> > Hi,
> >
> > On 03-06-19 15:55, Benjamin Tissoires wrote:
> > > On Mon, Jun 3, 2019 at 11:51 AM Hans de Goede <hdegoede@redhat.com> wrote:
> > >>
> > >> Hi Again,
> > >>
> > >> On 03-06-19 11:11, Hans de Goede wrote:
> > >> <snip>
> > >>
> > >>>> not sure about the rest of logitech issues yet) next week.
> > >>>
> > >>> The main problem seems to be the request_module patches. Although I also
> > >
> > > Can't we use request_module_nowait() instead, and set a reasonable
> > > timeout that we detect only once to check if userspace is compatible:
> > >
> > > In pseudo-code:
> > > if (!request_module_checked) {
> > > request_module_nowait(name);
> > > use_request_module = wait_event_timeout(wq,
> > > first_module_loaded, 10 seconds in jiffies);
> > > request_module_checked = true;
> > > } else if (use_request_module) {
> > > request_module(name);
> > > }
> >
> > Well looking at the just attached dmesg , the modprobe
> > when triggered by udev from userspace succeeds in about
> > 0.5 seconds, so it seems that the modprobe hangs happens
> > when called from within the kernel rather then from within
> > userspace.
> >
> > What I do not know if is the hang is inside userspace, or
> > maybe it happens when modprobe calls back into the kernel,
> > if the hang happens when modprobe calls back into the kernel,
> > then other modprobes (done from udev) likely will hang too
> > since I think only 1 modprobe can happen at a time.
> >
> > I really wish we knew what distinguished working systems
> > from non working systems :|
> >
> > I cannot find a common denominator; other then the systems
> > are not running Fedora. So far we've reports from both Ubuntu 16.04
> > and Tumbleweed, so software version wise these 2 are wide apart.
>
> I am trying to reproduce the lock locally, and installed an opensuse
> Tumbleweed in a VM. When forwarding a Unifying receiver to the VM, I
> do not see the lock with either my vanilla compiled kernel and the rpm
> found in http://download.opensuse.org/repositories/Kernel:/HEAD/standard/x86_64/
>
> Next step is install Tumbleweed on bare metal, but I do not see how
> this could introduce a difference (maybe USB2 vs 3).
Making progress here.
The difference between Ubuntu/Tumbleweed and Fedora: usbhid is shipped
as a module while in Fedora usbhid is included in the kernel.
If I rmmod hid_* and usbhid, then modprobe usbhid, the command hangs
for 3 minutes.
If usbhid is already loaded, inserting a receiver is immediate
regarding the loading of the external modules.
So my assumption is that when the device gets detected at boot, usbhid
gets loaded by the kernel event, which in turns attempts to call
__request_module, but the modprobe can't be fulfilled because it's
already waiting for the initial usbhid modprobe to finish.
Still don't know how to solve that, but I thought I should share.
Cheers,
Benjamin
^ permalink raw reply
* Re: hid-related 5.2-rc1 boot hang
From: Benjamin Tissoires @ 2019-06-04 8:53 UTC (permalink / raw)
To: Hans de Goede; +Cc: Jiri Kosina, Dave Hansen, open list:HID CORE LAYER, LKML
In-Reply-To: <c05929f4-00b6-e098-cd69-cd6539ccd3f1@redhat.com>
On Tue, Jun 4, 2019 at 10:36 AM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi,
>
> On 04-06-19 10:05, Hans de Goede wrote:
>
> <snip>
>
> >>>> We should likely just remove c52f from the list of supported devices.
> >>>> C52f receivers seem to have a different firmware as they are meant to
> >>>> work with different devices than C534. So I guess it is safer to not
> >>>> handle those right now and get the code in when it is ready.
> >>>
> >>> Ack. Can you prepare a patch to drop the c52f id?
> >>
> >> Yes. I have an other revert never submitted that I need to push, so I
> >> guess I can do a revert session today.
> >>
> >> I think I'll also buy one device with hopefully the C52F receiver as
> >> the report descriptors attached in
> >> https://bugzilla.kernel.org/show_bug.cgi?id=203619 seems different to
> >> what I would have expected.
> >
> > They are actually what I expected :)
> >
> > The first USB interface is a mouse boot class device, since this is a mouse
> > only receiver. This means that the mouse report is unnumbered and we need to
> > extend the unnumbered mouse-report handling to handle this case. Also the
> > device is using the same highres mouse-reports as the gaming receiver is.
> >
> > I'm actually preparing a patch right now which should fix this. Still might
> > be better to do the revert for 5.2 and get proper support for the c52f
> > receiver into 5.3.
>
> I've attached a patch to the bug:
> https://bugzilla.kernel.org/show_bug.cgi?id=203619
Cool, thanks.
>
> Which should fix this. It is quite simple and safe, so if we get testing
> feedback relatively soon, we could go with the fix instead of dropping the
> product-id, your call.
I should receive the M280 tomorrow, hopefully with the C52F. If the
receiver is correct and the tests are successful, I'd prefer to take
this one over the revert :)
Cheers,
Benjamin
^ permalink raw reply
* Re: hid-related 5.2-rc1 boot hang
From: Hans de Goede @ 2019-06-04 8:36 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Jiri Kosina, Dave Hansen, open list:HID CORE LAYER, LKML
In-Reply-To: <4548d196-b75f-c4d0-8f3c-3e734b9a758c@redhat.com>
Hi,
On 04-06-19 10:05, Hans de Goede wrote:
<snip>
>>>> We should likely just remove c52f from the list of supported devices.
>>>> C52f receivers seem to have a different firmware as they are meant to
>>>> work with different devices than C534. So I guess it is safer to not
>>>> handle those right now and get the code in when it is ready.
>>>
>>> Ack. Can you prepare a patch to drop the c52f id?
>>
>> Yes. I have an other revert never submitted that I need to push, so I
>> guess I can do a revert session today.
>>
>> I think I'll also buy one device with hopefully the C52F receiver as
>> the report descriptors attached in
>> https://bugzilla.kernel.org/show_bug.cgi?id=203619 seems different to
>> what I would have expected.
>
> They are actually what I expected :)
>
> The first USB interface is a mouse boot class device, since this is a mouse
> only receiver. This means that the mouse report is unnumbered and we need to
> extend the unnumbered mouse-report handling to handle this case. Also the
> device is using the same highres mouse-reports as the gaming receiver is.
>
> I'm actually preparing a patch right now which should fix this. Still might
> be better to do the revert for 5.2 and get proper support for the c52f
> receiver into 5.3.
I've attached a patch to the bug:
https://bugzilla.kernel.org/show_bug.cgi?id=203619
Which should fix this. It is quite simple and safe, so if we get testing
feedback relatively soon, we could go with the fix instead of dropping the
product-id, your call.
Regards,
Hans
^ permalink raw reply
* Re: hid-related 5.2-rc1 boot hang
From: Hans de Goede @ 2019-06-04 8:05 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Jiri Kosina, Dave Hansen, open list:HID CORE LAYER, LKML
In-Reply-To: <CAO-hwJ+5UYJMnuCS0UL4g45Xc181LraAzc-CMuYB2rcqKGe_Sw@mail.gmail.com>
Hi,
On 04-06-19 09:51, Benjamin Tissoires wrote:
> On Mon, Jun 3, 2019 at 4:17 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> Hi,
>>
>> On 03-06-19 15:55, Benjamin Tissoires wrote:
>>> On Mon, Jun 3, 2019 at 11:51 AM Hans de Goede <hdegoede@redhat.com> wrote:
>>>>
>>>> Hi Again,
>>>>
>>>> On 03-06-19 11:11, Hans de Goede wrote:
>>>> <snip>
>>>>
>>>>>> not sure about the rest of logitech issues yet) next week.
>>>>>
>>>>> The main problem seems to be the request_module patches. Although I also
>>>
>>> Can't we use request_module_nowait() instead, and set a reasonable
>>> timeout that we detect only once to check if userspace is compatible:
>>>
>>> In pseudo-code:
>>> if (!request_module_checked) {
>>> request_module_nowait(name);
>>> use_request_module = wait_event_timeout(wq,
>>> first_module_loaded, 10 seconds in jiffies);
>>> request_module_checked = true;
>>> } else if (use_request_module) {
>>> request_module(name);
>>> }
>>
>> Well looking at the just attached dmesg , the modprobe
>> when triggered by udev from userspace succeeds in about
>> 0.5 seconds, so it seems that the modprobe hangs happens
>> when called from within the kernel rather then from within
>> userspace.
>>
>> What I do not know if is the hang is inside userspace, or
>> maybe it happens when modprobe calls back into the kernel,
>> if the hang happens when modprobe calls back into the kernel,
>> then other modprobes (done from udev) likely will hang too
>> since I think only 1 modprobe can happen at a time.
>>
>> I really wish we knew what distinguished working systems
>> from non working systems :|
>>
>> I cannot find a common denominator; other then the systems
>> are not running Fedora. So far we've reports from both Ubuntu 16.04
>> and Tumbleweed, so software version wise these 2 are wide apart.
>
> I am trying to reproduce the lock locally, and installed an opensuse
> Tumbleweed in a VM. When forwarding a Unifying receiver to the VM, I
> do not see the lock with either my vanilla compiled kernel and the rpm
> found in http://download.opensuse.org/repositories/Kernel:/HEAD/standard/x86_64/
>
> Next step is install Tumbleweed on bare metal, but I do not see how
> this could introduce a difference (maybe USB2 vs 3).
Ok, thank you for looking into this.
>>>>> have 2 reports of problems with hid-logitech-dj driving the 0xc52f product-id,
>>>>> so we may need to drop that product-id from hid-logitech-dj, I'm working on
>>>>> that one...
>>>>
>>>> Besides the modprobe hanging issue, the only other issues all
>>>> (2 reporters) seem to be with 0xc52f receivers. We have a bug
>>>> open for this:
>>>>
>>>> https://bugzilla.kernel.org/show_bug.cgi?id=203619
>>>>
>>>> And I've asked the reporter of the second bug to add his logs
>>>> to that bug.
>>>
>>> We should likely just remove c52f from the list of supported devices.
>>> C52f receivers seem to have a different firmware as they are meant to
>>> work with different devices than C534. So I guess it is safer to not
>>> handle those right now and get the code in when it is ready.
>>
>> Ack. Can you prepare a patch to drop the c52f id?
>
> Yes. I have an other revert never submitted that I need to push, so I
> guess I can do a revert session today.
>
> I think I'll also buy one device with hopefully the C52F receiver as
> the report descriptors attached in
> https://bugzilla.kernel.org/show_bug.cgi?id=203619 seems different to
> what I would have expected.
They are actually what I expected :)
The first USB interface is a mouse boot class device, since this is a mouse
only receiver. This means that the mouse report is unnumbered and we need to
extend the unnumbered mouse-report handling to handle this case. Also the
device is using the same highres mouse-reports as the gaming receiver is.
I'm actually preparing a patch right now which should fix this. Still might
be better to do the revert for 5.2 and get proper support for the c52f
receiver into 5.3.
Regards,
Hans
^ permalink raw reply
* Re: hid-related 5.2-rc1 boot hang
From: Benjamin Tissoires @ 2019-06-04 7:51 UTC (permalink / raw)
To: Hans de Goede; +Cc: Jiri Kosina, Dave Hansen, open list:HID CORE LAYER, LKML
In-Reply-To: <e431dafc-0fb4-4be3-ac29-dcf125929090@redhat.com>
On Mon, Jun 3, 2019 at 4:17 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi,
>
> On 03-06-19 15:55, Benjamin Tissoires wrote:
> > On Mon, Jun 3, 2019 at 11:51 AM Hans de Goede <hdegoede@redhat.com> wrote:
> >>
> >> Hi Again,
> >>
> >> On 03-06-19 11:11, Hans de Goede wrote:
> >> <snip>
> >>
> >>>> not sure about the rest of logitech issues yet) next week.
> >>>
> >>> The main problem seems to be the request_module patches. Although I also
> >
> > Can't we use request_module_nowait() instead, and set a reasonable
> > timeout that we detect only once to check if userspace is compatible:
> >
> > In pseudo-code:
> > if (!request_module_checked) {
> > request_module_nowait(name);
> > use_request_module = wait_event_timeout(wq,
> > first_module_loaded, 10 seconds in jiffies);
> > request_module_checked = true;
> > } else if (use_request_module) {
> > request_module(name);
> > }
>
> Well looking at the just attached dmesg , the modprobe
> when triggered by udev from userspace succeeds in about
> 0.5 seconds, so it seems that the modprobe hangs happens
> when called from within the kernel rather then from within
> userspace.
>
> What I do not know if is the hang is inside userspace, or
> maybe it happens when modprobe calls back into the kernel,
> if the hang happens when modprobe calls back into the kernel,
> then other modprobes (done from udev) likely will hang too
> since I think only 1 modprobe can happen at a time.
>
> I really wish we knew what distinguished working systems
> from non working systems :|
>
> I cannot find a common denominator; other then the systems
> are not running Fedora. So far we've reports from both Ubuntu 16.04
> and Tumbleweed, so software version wise these 2 are wide apart.
I am trying to reproduce the lock locally, and installed an opensuse
Tumbleweed in a VM. When forwarding a Unifying receiver to the VM, I
do not see the lock with either my vanilla compiled kernel and the rpm
found in http://download.opensuse.org/repositories/Kernel:/HEAD/standard/x86_64/
Next step is install Tumbleweed on bare metal, but I do not see how
this could introduce a difference (maybe USB2 vs 3).
>
> >>> have 2 reports of problems with hid-logitech-dj driving the 0xc52f product-id,
> >>> so we may need to drop that product-id from hid-logitech-dj, I'm working on
> >>> that one...
> >>
> >> Besides the modprobe hanging issue, the only other issues all
> >> (2 reporters) seem to be with 0xc52f receivers. We have a bug
> >> open for this:
> >>
> >> https://bugzilla.kernel.org/show_bug.cgi?id=203619
> >>
> >> And I've asked the reporter of the second bug to add his logs
> >> to that bug.
> >
> > We should likely just remove c52f from the list of supported devices.
> > C52f receivers seem to have a different firmware as they are meant to
> > work with different devices than C534. So I guess it is safer to not
> > handle those right now and get the code in when it is ready.
>
> Ack. Can you prepare a patch to drop the c52f id?
Yes. I have an other revert never submitted that I need to push, so I
guess I can do a revert session today.
I think I'll also buy one device with hopefully the C52F receiver as
the report descriptors attached in
https://bugzilla.kernel.org/show_bug.cgi?id=203619 seems different to
what I would have expected.
Cheers,
Benjamin
^ permalink raw reply
* Re: [PATCH] HID: hid-logitech-hidpp: detect wireless lightspeed devices
From: Benjamin Tissoires @ 2019-06-04 7:02 UTC (permalink / raw)
To: Pedro Vanzella; +Cc: open list:HID CORE LAYER, Jiri Kosina, lkml
In-Reply-To: <20190603214438.2cnmrx7g2sakjdr4@Fenrir>
On Mon, Jun 3, 2019 at 11:44 PM Pedro Vanzella <pedro@pedrovanzella.com> wrote:
>
> On 05/28, Benjamin Tissoires wrote:
> > On Tue, May 28, 2019 at 6:30 PM Pedro Vanzella <pedro@pedrovanzella.com> wrote:
> > >
> > > Send a low device index when the device is connected via the lightspeed
> > > receiver so that the receiver will pass the message along to the device
> > > instead of responding. If we don't do that, we end up thinking it's a
> > > hidpp10 device and miss out on all new features available to newer devices.
> > >
> > > This will enable correct detection of the following models:
> > > G603, GPro, G305, G613, G900 and G903, and possibly others.
> >
> > Thanks for the patch.
> Thanks for reviewing it :)
>
> > However, there is already support for this receiver in Linus' tree:
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/hid/hid-logitech-dj.c?id=f5fb57a74e88bd1788f57bf77d587c91d4dc9d57
> >
> > With kernel 5.2-rc1, the connected device should already be handled by
> > hid-logitech-hidpp :)
> Why are the wireless receivers handled by hid-logitech-dj and the wired
> mice handled by hid-logitech-hidpp? They are, in the end, all hidpp
> devices, and having them all handled by the -hidpp driver with a quirk
> class would allow us to check for support for the battery voltage
> feature, as it seems to be an either-or scenario here.
Yep, and this is exactly what is happening:
- the receiver is handled through hid-logitech-dj -> it creates a
virtual HID device for the wireless physical device
- the actual wireless device is handled through hid-logitech-hidpp
(with the virtual HID device created above)
This has the advantage of presenting the wireless device in the same
way the wired device is. From hid-logitech-hidpp point of view, both
are regular HID++ devices.
Also, this makes sure each physical device gets its own product ID (we
are relying on the wireless product ID), meaning that userspace can
differentiate a G900 from a G613 when both are connected to a receiver
with the same product ID.
Hope that helps.
Cheers,
Benjamin
>
> - Pedro
> >
> > Cheers,
> > Benjamin
> >
> > >
> > > Signed-off-by: Pedro Vanzella <pedro@pedrovanzella.com>
> > > ---
> > > drivers/hid/hid-logitech-hidpp.c | 10 +++++++++-
> > > 1 file changed, 9 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
> > > index 72fc9c0566db..621fce141d9f 100644
> > > --- a/drivers/hid/hid-logitech-hidpp.c
> > > +++ b/drivers/hid/hid-logitech-hidpp.c
> > > @@ -62,6 +62,7 @@ MODULE_PARM_DESC(disable_tap_to_click,
> > > #define HIDPP_QUIRK_CLASS_K400 BIT(2)
> > > #define HIDPP_QUIRK_CLASS_G920 BIT(3)
> > > #define HIDPP_QUIRK_CLASS_K750 BIT(4)
> > > +#define HIDPP_QUIRK_CLASS_LIGHTSPEED BIT(5)
> > >
> > > /* bits 2..20 are reserved for classes */
> > > /* #define HIDPP_QUIRK_CONNECT_EVENTS BIT(21) disabled */
> > > @@ -236,7 +237,11 @@ static int __hidpp_send_report(struct hid_device *hdev,
> > > * set the device_index as the receiver, it will be overwritten by
> > > * hid_hw_request if needed
> > > */
> > > - hidpp_report->device_index = 0xff;
> > > + if (hidpp->quirks & HIDPP_QUIRK_CLASS_LIGHTSPEED) {
> > > + hidpp_report->device_index = 0x01;
> > > + } else {
> > > + hidpp_report->device_index = 0xff;
> > > + }
> > >
> > > if (hidpp->quirks & HIDPP_QUIRK_FORCE_OUTPUT_REPORTS) {
> > > ret = hid_hw_output_report(hdev, (u8 *)hidpp_report, fields_count);
> > > @@ -3753,6 +3758,9 @@ static const struct hid_device_id hidpp_devices[] = {
> > > HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0xC06B) },
> > > { /* Logitech G900 Gaming Mouse over USB */
> > > HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0xC081) },
> > > + { /* Logitech Gaming Mice over Lightspeed Receiver */
> > > + HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0xC539),
> > > + .driver_data = HIDPP_QUIRK_CLASS_LIGHTSPEED },
> > > { /* Logitech G920 Wheel over USB */
> > > HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_G920_WHEEL),
> > > .driver_data = HIDPP_QUIRK_CLASS_G920 | HIDPP_QUIRK_FORCE_OUTPUT_REPORTS},
> > > --
> > > 2.21.0
> > >
>
> --
> Pedro Vanzella
> pedrovanzella.com
> #include <paranoia.h>
> Don't Panic
^ permalink raw reply
* Re: [PATCH 1/2] Input: synaptics-rmi4 - clear irqs before set irqs
From: Christopher Heiny @ 2019-06-04 5:19 UTC (permalink / raw)
To: Aaron Ma, dmitry.torokhov@gmail.com, linux-input@vger.kernel.org,
linux-kernel@vger.kernel.org, Andrew Duggan,
benjamin.tissoires@redhat.com
In-Reply-To: <2995b27a-9ec8-eebe-78b6-2d3bf5098af8@canonical.com>
On Tue, 2019-06-04 at 10:45 +0800, Aaron Ma wrote:
> Hi Christopher:
>
> Have got time to review these 2 patches?
> Users reported it works fine since I sent out this patch.
Hi Aaron,
I've been poking around with this off and on. Unfortunately, more off
than on :-( but here's my current take:
rmi_driver_set_irq_bits() isn't going to be called all that often, and
it's not going to be called at all during normal operation, which is
where the most serious problem would occur.
I haven't entirely convinced myself that there couldn't be a problem
during repeated spontaneous device resets (for example, due to ESD, a
dodgy charger, or firmware bug, among other things). On the other
hand, all the scenarios I have come up with are both unlikely and so
contrived that the system is probably hosed regardless of what we do in
the driver.
Given that, I'm willing to accept the patch as is.
Cheers,
Chris
>
> Thanks,
> Aaron
>
> On 4/3/19 9:58 PM, Aaron Ma wrote:
> > Sure, take your time, if you have any questions let me know please.
> >
> > Thanks,
> > Aaron
^ permalink raw reply
* Re: [PATCH 1/2] Input: synaptics-rmi4 - clear irqs before set irqs
From: Aaron Ma @ 2019-06-04 2:45 UTC (permalink / raw)
To: Christopher Heiny, dmitry.torokhov@gmail.com,
linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
Andrew Duggan, benjamin.tissoires@redhat.com
In-Reply-To: <9321df87-5bc5-0c75-2815-f8602ecf9d86@canonical.com>
Hi Christopher:
Have got time to review these 2 patches?
Users reported it works fine since I sent out this patch.
Thanks,
Aaron
On 4/3/19 9:58 PM, Aaron Ma wrote:
> Sure, take your time, if you have any questions let me know please.
>
> Thanks,
> Aaron
^ permalink raw reply
* Re: [PATCH] HID: hid-logitech-hidpp: detect wireless lightspeed devices
From: Pedro Vanzella @ 2019-06-03 21:44 UTC (permalink / raw)
To: Benjamin Tissoires; +Cc: open list:HID CORE LAYER, Jiri Kosina, lkml
In-Reply-To: <CAO-hwJ+zAvDizJRpykky+D3pf1M1NhFGWztwyA4mJEv8C+nO-w@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3612 bytes --]
On 05/28, Benjamin Tissoires wrote:
> On Tue, May 28, 2019 at 6:30 PM Pedro Vanzella <pedro@pedrovanzella.com> wrote:
> >
> > Send a low device index when the device is connected via the lightspeed
> > receiver so that the receiver will pass the message along to the device
> > instead of responding. If we don't do that, we end up thinking it's a
> > hidpp10 device and miss out on all new features available to newer devices.
> >
> > This will enable correct detection of the following models:
> > G603, GPro, G305, G613, G900 and G903, and possibly others.
>
> Thanks for the patch.
Thanks for reviewing it :)
> However, there is already support for this receiver in Linus' tree:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/hid/hid-logitech-dj.c?id=f5fb57a74e88bd1788f57bf77d587c91d4dc9d57
>
> With kernel 5.2-rc1, the connected device should already be handled by
> hid-logitech-hidpp :)
Why are the wireless receivers handled by hid-logitech-dj and the wired
mice handled by hid-logitech-hidpp? They are, in the end, all hidpp
devices, and having them all handled by the -hidpp driver with a quirk
class would allow us to check for support for the battery voltage
feature, as it seems to be an either-or scenario here.
- Pedro
>
> Cheers,
> Benjamin
>
> >
> > Signed-off-by: Pedro Vanzella <pedro@pedrovanzella.com>
> > ---
> > drivers/hid/hid-logitech-hidpp.c | 10 +++++++++-
> > 1 file changed, 9 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
> > index 72fc9c0566db..621fce141d9f 100644
> > --- a/drivers/hid/hid-logitech-hidpp.c
> > +++ b/drivers/hid/hid-logitech-hidpp.c
> > @@ -62,6 +62,7 @@ MODULE_PARM_DESC(disable_tap_to_click,
> > #define HIDPP_QUIRK_CLASS_K400 BIT(2)
> > #define HIDPP_QUIRK_CLASS_G920 BIT(3)
> > #define HIDPP_QUIRK_CLASS_K750 BIT(4)
> > +#define HIDPP_QUIRK_CLASS_LIGHTSPEED BIT(5)
> >
> > /* bits 2..20 are reserved for classes */
> > /* #define HIDPP_QUIRK_CONNECT_EVENTS BIT(21) disabled */
> > @@ -236,7 +237,11 @@ static int __hidpp_send_report(struct hid_device *hdev,
> > * set the device_index as the receiver, it will be overwritten by
> > * hid_hw_request if needed
> > */
> > - hidpp_report->device_index = 0xff;
> > + if (hidpp->quirks & HIDPP_QUIRK_CLASS_LIGHTSPEED) {
> > + hidpp_report->device_index = 0x01;
> > + } else {
> > + hidpp_report->device_index = 0xff;
> > + }
> >
> > if (hidpp->quirks & HIDPP_QUIRK_FORCE_OUTPUT_REPORTS) {
> > ret = hid_hw_output_report(hdev, (u8 *)hidpp_report, fields_count);
> > @@ -3753,6 +3758,9 @@ static const struct hid_device_id hidpp_devices[] = {
> > HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0xC06B) },
> > { /* Logitech G900 Gaming Mouse over USB */
> > HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0xC081) },
> > + { /* Logitech Gaming Mice over Lightspeed Receiver */
> > + HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0xC539),
> > + .driver_data = HIDPP_QUIRK_CLASS_LIGHTSPEED },
> > { /* Logitech G920 Wheel over USB */
> > HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_G920_WHEEL),
> > .driver_data = HIDPP_QUIRK_CLASS_G920 | HIDPP_QUIRK_FORCE_OUTPUT_REPORTS},
> > --
> > 2.21.0
> >
--
Pedro Vanzella
pedrovanzella.com
#include <paranoia.h>
Don't Panic
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: hid-related 5.2-rc1 boot hang
From: Hans de Goede @ 2019-06-03 14:17 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Jiri Kosina, Dave Hansen, open list:HID CORE LAYER, LKML
In-Reply-To: <CAO-hwJKRRpsShw6B-YLmsEnjQ+iYtz+VmZK+VSRcDmiBwnS+oA@mail.gmail.com>
Hi,
On 03-06-19 15:55, Benjamin Tissoires wrote:
> On Mon, Jun 3, 2019 at 11:51 AM Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> Hi Again,
>>
>> On 03-06-19 11:11, Hans de Goede wrote:
>> <snip>
>>
>>>> not sure about the rest of logitech issues yet) next week.
>>>
>>> The main problem seems to be the request_module patches. Although I also
>
> Can't we use request_module_nowait() instead, and set a reasonable
> timeout that we detect only once to check if userspace is compatible:
>
> In pseudo-code:
> if (!request_module_checked) {
> request_module_nowait(name);
> use_request_module = wait_event_timeout(wq,
> first_module_loaded, 10 seconds in jiffies);
> request_module_checked = true;
> } else if (use_request_module) {
> request_module(name);
> }
Well looking at the just attached dmesg , the modprobe
when triggered by udev from userspace succeeds in about
0.5 seconds, so it seems that the modprobe hangs happens
when called from within the kernel rather then from within
userspace.
What I do not know if is the hang is inside userspace, or
maybe it happens when modprobe calls back into the kernel,
if the hang happens when modprobe calls back into the kernel,
then other modprobes (done from udev) likely will hang too
since I think only 1 modprobe can happen at a time.
I really wish we knew what distinguished working systems
from non working systems :|
I cannot find a common denominator; other then the systems
are not running Fedora. So far we've reports from both Ubuntu 16.04
and Tumbleweed, so software version wise these 2 are wide apart.
>>> have 2 reports of problems with hid-logitech-dj driving the 0xc52f product-id,
>>> so we may need to drop that product-id from hid-logitech-dj, I'm working on
>>> that one...
>>
>> Besides the modprobe hanging issue, the only other issues all
>> (2 reporters) seem to be with 0xc52f receivers. We have a bug
>> open for this:
>>
>> https://bugzilla.kernel.org/show_bug.cgi?id=203619
>>
>> And I've asked the reporter of the second bug to add his logs
>> to that bug.
>
> We should likely just remove c52f from the list of supported devices.
> C52f receivers seem to have a different firmware as they are meant to
> work with different devices than C534. So I guess it is safer to not
> handle those right now and get the code in when it is ready.
Ack. Can you prepare a patch to drop the c52f id?
Regards,
Hans
^ permalink raw reply
* Re: hid-related 5.2-rc1 boot hang
From: Benjamin Tissoires @ 2019-06-03 13:55 UTC (permalink / raw)
To: Hans de Goede; +Cc: Jiri Kosina, Dave Hansen, open list:HID CORE LAYER, LKML
In-Reply-To: <5471f010-cb42-c548-37e2-2b9c9eba1184@redhat.com>
On Mon, Jun 3, 2019 at 11:51 AM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi Again,
>
> On 03-06-19 11:11, Hans de Goede wrote:
> <snip>
>
> >> not sure about the rest of logitech issues yet) next week.
> >
> > The main problem seems to be the request_module patches. Although I also
Can't we use request_module_nowait() instead, and set a reasonable
timeout that we detect only once to check if userspace is compatible:
In pseudo-code:
if (!request_module_checked) {
request_module_nowait(name);
use_request_module = wait_event_timeout(wq,
first_module_loaded, 10 seconds in jiffies);
request_module_checked = true;
} else if (use_request_module) {
request_module(name);
}
> > have 2 reports of problems with hid-logitech-dj driving the 0xc52f product-id,
> > so we may need to drop that product-id from hid-logitech-dj, I'm working on
> > that one...
>
> Besides the modprobe hanging issue, the only other issues all
> (2 reporters) seem to be with 0xc52f receivers. We have a bug
> open for this:
>
> https://bugzilla.kernel.org/show_bug.cgi?id=203619
>
> And I've asked the reporter of the second bug to add his logs
> to that bug.
We should likely just remove c52f from the list of supported devices.
C52f receivers seem to have a different firmware as they are meant to
work with different devices than C534. So I guess it is safer to not
handle those right now and get the code in when it is ready.
Cheers,
Benjamin
^ permalink raw reply
* Re: hid-related 5.2-rc1 boot hang
From: Hans de Goede @ 2019-06-03 9:51 UTC (permalink / raw)
To: Jiri Kosina, Dave Hansen
Cc: Benjamin Tissoires, open list:HID CORE LAYER, LKML
In-Reply-To: <e158d983-1e7e-4c49-aaab-ff2092d36438@redhat.com>
Hi Again,
On 03-06-19 11:11, Hans de Goede wrote:
<snip>
>> not sure about the rest of logitech issues yet) next week.
>
> The main problem seems to be the request_module patches. Although I also
> have 2 reports of problems with hid-logitech-dj driving the 0xc52f product-id,
> so we may need to drop that product-id from hid-logitech-dj, I'm working on
> that one...
Besides the modprobe hanging issue, the only other issues all
(2 reporters) seem to be with 0xc52f receivers. We have a bug
open for this:
https://bugzilla.kernel.org/show_bug.cgi?id=203619
And I've asked the reporter of the second bug to add his logs
to that bug.
Regards,
Hans
^ permalink raw reply
* Re: hid-related 5.2-rc1 boot hang
From: Hans de Goede @ 2019-06-03 9:32 UTC (permalink / raw)
To: Jiri Kosina, Dave Hansen
Cc: Benjamin Tissoires, open list:HID CORE LAYER, LKML
In-Reply-To: <e158d983-1e7e-4c49-aaab-ff2092d36438@redhat.com>
Hi,
On 03-06-19 11:11, Hans de Goede wrote:
> Hi,
>
> On 01-06-19 00:15, Jiri Kosina wrote:
>> On Thu, 30 May 2019, Dave Hansen wrote:
>>
>>> On 5/29/19 2:17 AM, Hans de Goede wrote:
>>> ...
>>>> Dave, can you try building your initrd without the hid-logitech-dj module
>>>> included in the initrd?
>>>
>>> I did this on a vanilla 5.2-rc2 kernel (without the reverts) and still
>>> experienced the boot hang while the device was inserted.
>>>
>>>> Also can you check if your modprobe is provided by module-init-tools
>>>> or by kmod ?
>>>
>>> $ dpkg -S `which modprobe`
>>> kmod: /sbin/modprobe
>>
>> Benjamin, Hans, are you looking into this?
>
> Not really, I cannot reproduce the request_module problem. I was hoping some
> of the info from Dave would help to pinpoint it, but it does not :|
>
>> If not, I think we should start reverting (at least the request_module()
>> changes
>
> I agree we need to do something about the request_module changes.
>
> I myself was thinking about somehow making them conditional, e.g. we
> could add a (temporary) module option defaulting to false for this
> while we investigate further.
>
> I'm afraid that if we just revert we will never find the root cause and then
> we will be stuck with the suboptimal behavior of first the generic hid driver
> binding followed by a unbind + bind of the new driver shortly afterwards,
> which also leads to a ton of udev events being fired to userspace (well I
> guess this does make for a good stress test of the userspace hotplug code).
Quick update, we have another report of module-loading related problems
which are likely related:
https://bugzilla.kernel.org/show_bug.cgi?id=203741
In this case there is no hang, instead there is a 1 to 3 minute delay.
Regards,
Hans
^ permalink raw reply
* Re: hid-related 5.2-rc1 boot hang
From: Hans de Goede @ 2019-06-03 9:11 UTC (permalink / raw)
To: Jiri Kosina, Dave Hansen
Cc: Benjamin Tissoires, open list:HID CORE LAYER, LKML
In-Reply-To: <nycvar.YFH.7.76.1906010014150.1962@cbobk.fhfr.pm>
Hi,
On 01-06-19 00:15, Jiri Kosina wrote:
> On Thu, 30 May 2019, Dave Hansen wrote:
>
>> On 5/29/19 2:17 AM, Hans de Goede wrote:
>> ...
>>> Dave, can you try building your initrd without the hid-logitech-dj module
>>> included in the initrd?
>>
>> I did this on a vanilla 5.2-rc2 kernel (without the reverts) and still
>> experienced the boot hang while the device was inserted.
>>
>>> Also can you check if your modprobe is provided by module-init-tools
>>> or by kmod ?
>>
>> $ dpkg -S `which modprobe`
>> kmod: /sbin/modprobe
>
> Benjamin, Hans, are you looking into this?
Not really, I cannot reproduce the request_module problem. I was hoping some
of the info from Dave would help to pinpoint it, but it does not :|
> If not, I think we should start reverting (at least the request_module()
> changes
I agree we need to do something about the request_module changes.
I myself was thinking about somehow making them conditional, e.g. we
could add a (temporary) module option defaulting to false for this
while we investigate further.
I'm afraid that if we just revert we will never find the root cause and then
we will be stuck with the suboptimal behavior of first the generic hid driver
binding followed by a unbind + bind of the new driver shortly afterwards,
which also leads to a ton of udev events being fired to userspace (well I
guess this does make for a good stress test of the userspace hotplug code).
> not sure about the rest of logitech issues yet) next week.
The main problem seems to be the request_module patches. Although I also
have 2 reports of problems with hid-logitech-dj driving the 0xc52f product-id,
so we may need to drop that product-id from hid-logitech-dj, I'm working on
that one...
Regards,
Hans
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox