* Re: [PATCH v2 08/10] HID: i2c-hid: Support being a panel follower
From: kernel test robot @ 2023-06-08 7:14 UTC (permalink / raw)
To: Douglas Anderson, Jiri Kosina, Benjamin Tissoires,
Bjorn Andersson, Konrad Dybcio, Rob Herring, Frank Rowand,
Krzysztof Kozlowski, Conor Dooley, Neil Armstrong, Sam Ravnborg,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann
Cc: llvm, oe-kbuild-all, Douglas Anderson, devicetree,
cros-qcom-dts-watchers, linux-arm-msm, yangcong5, Dmitry Torokhov,
linux-kernel, dri-devel, Chris Morgan, linux-input, hsinyi
In-Reply-To: <20230607144931.v2.8.Ib1a98309c455cd7e26b931c69993d4fba33bbe15@changeid>
Hi Douglas,
kernel test robot noticed the following build errors:
[auto build test ERROR on robh/for-next]
[also build test ERROR on hid/for-next dtor-input/next dtor-input/for-linus drm-misc/drm-misc-next linus/master v6.4-rc5 next-20230607]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Douglas-Anderson/dt-bindings-HID-i2c-hid-Add-panel-property-to-i2c-hid-backed-touchscreens/20230608-055515
base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
patch link: https://lore.kernel.org/r/20230607144931.v2.8.Ib1a98309c455cd7e26b931c69993d4fba33bbe15%40changeid
patch subject: [PATCH v2 08/10] HID: i2c-hid: Support being a panel follower
config: i386-randconfig-i003-20230607 (https://download.01.org/0day-ci/archive/20230608/202306081419.Dzz0T4iW-lkp@intel.com/config)
compiler: clang version 15.0.7 (https://github.com/llvm/llvm-project.git 8dfdcc7b7bf66834a761bd8de445840ef68e4d1a)
reproduce (this is a W=1 build):
mkdir -p ~/bin
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
git remote add robh https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git
git fetch robh for-next
git checkout robh/for-next
b4 shazam https://lore.kernel.org/r/20230607144931.v2.8.Ib1a98309c455cd7e26b931c69993d4fba33bbe15@changeid
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang ~/bin/make.cross W=1 O=build_dir ARCH=i386 olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang ~/bin/make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202306081419.Dzz0T4iW-lkp@intel.com/
All errors (new ones prefixed by >>):
>> ld.lld: error: undefined symbol: drm_panel_add_follower
>>> referenced by i2c-hid-core.c:1159 (drivers/hid/i2c-hid/i2c-hid-core.c:1159)
>>> drivers/hid/i2c-hid/i2c-hid-core.o:(i2c_hid_core_probe) in archive vmlinux.a
--
>> ld.lld: error: undefined symbol: drm_panel_remove_follower
>>> referenced by i2c-hid-core.c:1218 (drivers/hid/i2c-hid/i2c-hid-core.c:1218)
>>> drivers/hid/i2c-hid/i2c-hid-core.o:(i2c_hid_core_remove) in archive vmlinux.a
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH v2 08/10] HID: i2c-hid: Support being a panel follower
From: kernel test robot @ 2023-06-08 5:18 UTC (permalink / raw)
To: Douglas Anderson, Jiri Kosina, Benjamin Tissoires,
Bjorn Andersson, Konrad Dybcio, Rob Herring, Frank Rowand,
Krzysztof Kozlowski, Conor Dooley, Neil Armstrong, Sam Ravnborg,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann
Cc: oe-kbuild-all, Douglas Anderson, devicetree,
cros-qcom-dts-watchers, linux-arm-msm, yangcong5, Dmitry Torokhov,
linux-kernel, dri-devel, Chris Morgan, linux-input, hsinyi
In-Reply-To: <20230607144931.v2.8.Ib1a98309c455cd7e26b931c69993d4fba33bbe15@changeid>
Hi Douglas,
kernel test robot noticed the following build errors:
[auto build test ERROR on robh/for-next]
[also build test ERROR on hid/for-next dtor-input/next dtor-input/for-linus]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Douglas-Anderson/dt-bindings-HID-i2c-hid-Add-panel-property-to-i2c-hid-backed-touchscreens/20230608-055515
base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
patch link: https://lore.kernel.org/r/20230607144931.v2.8.Ib1a98309c455cd7e26b931c69993d4fba33bbe15%40changeid
patch subject: [PATCH v2 08/10] HID: i2c-hid: Support being a panel follower
config: arc-randconfig-r021-20230607 (https://download.01.org/0day-ci/archive/20230608/202306081344.M0jNn0Ce-lkp@intel.com/config)
compiler: arceb-elf-gcc (GCC) 12.3.0
reproduce (this is a W=1 build):
mkdir -p ~/bin
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
git remote add robh https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git
git fetch robh for-next
git checkout robh/for-next
b4 shazam https://lore.kernel.org/r/20230607144931.v2.8.Ib1a98309c455cd7e26b931c69993d4fba33bbe15@changeid
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.3.0 ~/bin/make.cross W=1 O=build_dir ARCH=arc olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.3.0 ~/bin/make.cross W=1 O=build_dir ARCH=arc SHELL=/bin/bash
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202306081344.M0jNn0Ce-lkp@intel.com/
All errors (new ones prefixed by >>):
`.exit.text' referenced in section `__jump_table' of lib/test_dynamic_debug.o: defined in discarded section `.exit.text' of lib/test_dynamic_debug.o
`.exit.text' referenced in section `__jump_table' of lib/test_dynamic_debug.o: defined in discarded section `.exit.text' of lib/test_dynamic_debug.o
`.exit.text' referenced in section `__jump_table' of drivers/misc/phantom.o: defined in discarded section `.exit.text' of drivers/misc/phantom.o
`.exit.text' referenced in section `__jump_table' of drivers/misc/phantom.o: defined in discarded section `.exit.text' of drivers/misc/phantom.o
`.exit.text' referenced in section `__jump_table' of drivers/target/target_core_configfs.o: defined in discarded section `.exit.text' of drivers/target/target_core_configfs.o
`.exit.text' referenced in section `__jump_table' of drivers/target/target_core_configfs.o: defined in discarded section `.exit.text' of drivers/target/target_core_configfs.o
arceb-elf-ld: drivers/hid/i2c-hid/i2c-hid-core.o: in function `i2c_hid_core_remove':
drivers/hid/i2c-hid/i2c-hid-core.c:1218: undefined reference to `drm_panel_remove_follower'
>> arceb-elf-ld: drivers/hid/i2c-hid/i2c-hid-core.c:1218: undefined reference to `drm_panel_remove_follower'
arceb-elf-ld: drivers/hid/i2c-hid/i2c-hid-core.o: in function `i2c_hid_core_probe':
drivers/hid/i2c-hid/i2c-hid-core.c:1159: undefined reference to `drm_panel_add_follower'
>> arceb-elf-ld: drivers/hid/i2c-hid/i2c-hid-core.c:1159: undefined reference to `drm_panel_add_follower'
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH] HID: wacom: Use ktime_t rather than int when dealing with timestamps
From: Peter Hutterer @ 2023-06-08 2:51 UTC (permalink / raw)
To: Jason Gerecke
Cc: linux-input, linux-kernel, Benjamin Tissoires, Jiri Kosina,
Ping Cheng, Aaron Armstrong Skomra, Joshua Dickens, Jason Gerecke
In-Reply-To: <20230607214102.2113-1-jason.gerecke@wacom.com>
On Wed, Jun 07, 2023 at 02:41:02PM -0700, Jason Gerecke wrote:
> Code which interacts with timestamps needs to use the ktime_t type
> returned by functions like ktime_get. The int type does not offer
> enough space to store these values, and attempting to use it is a
> recipe for problems. In this particular case, overflows would occur
> when calculating/storing timestamps leading to incorrect values being
> reported to userspace. In some cases these bad timestamps cause input
> handling in userspace to appear hung.
>
> Link: https://gitlab.freedesktop.org/libinput/libinput/-/issues/901
> Fixes: 17d793f3ed53 ("HID: wacom: insert timestamp to packed Bluetooth (BT) events")
> Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Cheers,
Peter
> ---
> drivers/hid/wacom_wac.c | 4 ++--
> drivers/hid/wacom_wac.h | 2 +-
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
> index 2ccf83837134..2f16e47e4b69 100644
> --- a/drivers/hid/wacom_wac.c
> +++ b/drivers/hid/wacom_wac.c
> @@ -1314,7 +1314,7 @@ static void wacom_intuos_pro2_bt_pen(struct wacom_wac *wacom)
> struct input_dev *pen_input = wacom->pen_input;
> unsigned char *data = wacom->data;
> int number_of_valid_frames = 0;
> - int time_interval = 15000000;
> + ktime_t time_interval = 15000000;
> ktime_t time_packet_received = ktime_get();
> int i;
>
> @@ -1359,7 +1359,7 @@ static void wacom_intuos_pro2_bt_pen(struct wacom_wac *wacom)
> bool range = frame[0] & 0x20;
> bool invert = frame[0] & 0x10;
> int frames_number_reversed = number_of_valid_frames - i - 1;
> - int event_timestamp = time_packet_received - frames_number_reversed * time_interval;
> + ktime_t event_timestamp = time_packet_received - frames_number_reversed * time_interval;
>
> if (!valid)
> continue;
> diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h
> index 1a40bb8c5810..ee21bb260f22 100644
> --- a/drivers/hid/wacom_wac.h
> +++ b/drivers/hid/wacom_wac.h
> @@ -324,7 +324,7 @@ struct hid_data {
> int ps_connected;
> bool pad_input_event_flag;
> unsigned short sequence_number;
> - int time_delayed;
> + ktime_t time_delayed;
> };
>
> struct wacom_remote_data {
> --
> 2.41.0
>
^ permalink raw reply
* Re: [PATCH v3 2/2] HID: i2c-hid: elan: Add ili9882t timing
From: Doug Anderson @ 2023-06-07 23:49 UTC (permalink / raw)
To: Cong Yang
Cc: robh+dt, krzysztof.kozlowski+dt, conor+dt, dmitry.torokhov, jikos,
benjamin.tissoires, hsinyi, linux-input, devicetree, linux-kernel
In-Reply-To: <20230607133458.4075667-3-yangcong5@huaqin.corp-partner.google.com>
Hi,
On Wed, Jun 7, 2023 at 6:35 AM Cong Yang
<yangcong5@huaqin.corp-partner.google.com> wrote:
>
> The ili9882t is a TDDI IC (Touch with Display Driver). The
> datasheet specifies there should be 60ms between touch SDA
> sleep and panel RESX. Doug's series[1] allows panels and
> touchscreens to power on/off together, so we can add the 65 ms
> delay in i2c_hid_core_suspend before panel_unprepare.
>
> [1]: https: //lore.kernel.org/all/20230523193017.4109557-1-dianders@chromium.org/
FWIW: I posted v2 today:
https://lore.kernel.org/r/20230607215224.2067679-1-dianders@chromium.org
> Signed-off-by: Cong Yang <yangcong5@huaqin.corp-partner.google.com>
> ---
> drivers/hid/i2c-hid/i2c-hid-of-elan.c | 20 ++++++++++++++++----
> 1 file changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/hid/i2c-hid/i2c-hid-of-elan.c b/drivers/hid/i2c-hid/i2c-hid-of-elan.c
> index 76ddc8be1cbb..411d7ea2725d 100644
> --- a/drivers/hid/i2c-hid/i2c-hid-of-elan.c
> +++ b/drivers/hid/i2c-hid/i2c-hid-of-elan.c
> @@ -18,7 +18,8 @@
> #include "i2c-hid.h"
>
> struct elan_i2c_hid_chip_data {
> - unsigned int post_gpio_reset_delay_ms;
> + unsigned int post_gpio_reset_on_delay_ms;
> + unsigned int post_gpio_reset_off_delay_ms;
> unsigned int post_power_delay_ms;
> u16 hid_descriptor_address;
> };
I would prefer it if you would add something to the
"elan_i2c_hid_chip_data" indicating the name of the main supply. Set
it to "vcc33" for the elan touchscreen and the NULL for your new one.
It's probably worth adding a comment next to where you set it to NULL
that this touchscreen is tightly integrated with the panel and assumes
that the relevant power rails (other than the IO rail) have already
been turned on by the panel driver because we're a panel follower.
Otherwise someone is going to be super confused about how this could
work.
^ permalink raw reply
* [PATCH v2 10/10] arm64: dts: qcom: sc7180: Link trogdor touchscreens to the panels
From: Douglas Anderson @ 2023-06-07 21:49 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Bjorn Andersson, Konrad Dybcio,
Rob Herring, Frank Rowand, Krzysztof Kozlowski, Conor Dooley,
Neil Armstrong, Sam Ravnborg, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann
Cc: dri-devel, Dmitry Torokhov, linux-input, Daniel Vetter,
linux-kernel, hsinyi, cros-qcom-dts-watchers, devicetree,
yangcong5, linux-arm-msm, Chris Morgan, Douglas Anderson
In-Reply-To: <20230607215224.2067679-1-dianders@chromium.org>
Let's provide the proper link from the touchscreen to the panel on
trogdor devices where the touchscreen support it. This allows the OS
to power sequence the touchscreen more properly.
For the most part, this is just expected to marginally improve power
consumption while the screen is off. However, in at least one trogdor
model (wormdingler) it's suspected that this will fix some behavorial
corner cases when the panel power cycles (like for a modeset) without
the touchscreen power cycling.
NOTE: some trogdor variants use touchscreens that don't (yet) support
linking the touchscreen and the panel. Those variants are left alone.
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
(no changes since v1)
arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi | 1 +
arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi | 1 +
arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi | 1 +
arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom.dtsi | 1 +
arch/arm64/boot/dts/qcom/sc7180-trogdor-quackingstick.dtsi | 1 +
arch/arm64/boot/dts/qcom/sc7180-trogdor-wormdingler.dtsi | 1 +
6 files changed, 6 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi
index 8b8ea8af165d..b4f328d3e1f6 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi
@@ -104,6 +104,7 @@ ap_ts: touchscreen@5d {
interrupt-parent = <&tlmm>;
interrupts = <9 IRQ_TYPE_LEVEL_LOW>;
+ panel = <&panel>;
reset-gpios = <&tlmm 8 GPIO_ACTIVE_LOW>;
vdd-supply = <&pp3300_ts>;
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi
index b3ba23a88a0b..88aeb415bd5b 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi
@@ -116,6 +116,7 @@ ap_ts: touchscreen@14 {
interrupt-parent = <&tlmm>;
interrupts = <9 IRQ_TYPE_LEVEL_LOW>;
+ panel = <&panel>;
reset-gpios = <&tlmm 8 GPIO_ACTIVE_LOW>;
vdd-supply = <&pp3300_touch>;
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi
index 269007d73162..c65f18ea3e5c 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi
@@ -43,6 +43,7 @@ ap_ts: touchscreen@10 {
interrupt-parent = <&tlmm>;
interrupts = <9 IRQ_TYPE_LEVEL_LOW>;
+ panel = <&panel>;
post-power-on-delay-ms = <20>;
hid-descr-addr = <0x0001>;
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom.dtsi
index 6c5287bd27d6..d2aafd1ea672 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom.dtsi
@@ -102,6 +102,7 @@ ap_ts: touchscreen@10 {
interrupt-parent = <&tlmm>;
interrupts = <9 IRQ_TYPE_LEVEL_LOW>;
+ panel = <&panel>;
post-power-on-delay-ms = <20>;
hid-descr-addr = <0x0001>;
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-quackingstick.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-quackingstick.dtsi
index 8e7b42f843d4..0785873d1345 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-quackingstick.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-quackingstick.dtsi
@@ -99,6 +99,7 @@ ap_ts: touchscreen@10 {
interrupt-parent = <&tlmm>;
interrupts = <9 IRQ_TYPE_LEVEL_LOW>;
+ panel = <&panel>;
post-power-on-delay-ms = <20>;
hid-descr-addr = <0x0001>;
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-wormdingler.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-wormdingler.dtsi
index 262d6691abd9..f70f5b42c845 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-wormdingler.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-wormdingler.dtsi
@@ -154,6 +154,7 @@ ap_ts: touchscreen@1 {
interrupt-parent = <&tlmm>;
interrupts = <9 IRQ_TYPE_EDGE_FALLING>;
+ panel = <&panel>;
post-power-on-delay-ms = <70>;
hid-descr-addr = <0x0001>;
--
2.41.0.162.gfafddb0af9-goog
^ permalink raw reply related
* [PATCH v2 09/10] HID: i2c-hid: Do panel follower work on the system_wq
From: Douglas Anderson @ 2023-06-07 21:49 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Bjorn Andersson, Konrad Dybcio,
Rob Herring, Frank Rowand, Krzysztof Kozlowski, Conor Dooley,
Neil Armstrong, Sam Ravnborg, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann
Cc: dri-devel, Dmitry Torokhov, linux-input, Daniel Vetter,
linux-kernel, hsinyi, cros-qcom-dts-watchers, devicetree,
yangcong5, linux-arm-msm, Chris Morgan, Douglas Anderson
In-Reply-To: <20230607215224.2067679-1-dianders@chromium.org>
Turning on an i2c-hid device can be a slow process. This is why
i2c-hid devices use PROBE_PREFER_ASYNCHRONOUS. Unfortunately, when
we're a panel follower the i2c-hid power up sequence now blocks the
power on of the panel. Let's fix that by scheduling the work on the
system_wq.
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
Changes in v2:
- ihid_core_panel_prepare_work() is now static.
- Improve documentation for smp_wmb().
drivers/hid/i2c-hid/i2c-hid-core.c | 50 +++++++++++++++++++++++++++---
1 file changed, 46 insertions(+), 4 deletions(-)
diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
index 368db3ae612f..de1a0624be08 100644
--- a/drivers/hid/i2c-hid/i2c-hid-core.c
+++ b/drivers/hid/i2c-hid/i2c-hid-core.c
@@ -110,7 +110,9 @@ struct i2c_hid {
struct i2chid_ops *ops;
struct drm_panel_follower panel_follower;
+ struct work_struct panel_follower_prepare_work;
bool is_panel_follower;
+ bool prepare_work_finished;
};
static const struct i2c_hid_quirks {
@@ -1062,10 +1064,12 @@ static int i2c_hid_core_initial_power_up(struct i2c_hid *ihid)
return ret;
}
-static int i2c_hid_core_panel_prepared(struct drm_panel_follower *follower)
+static void ihid_core_panel_prepare_work(struct work_struct *work)
{
- struct i2c_hid *ihid = container_of(follower, struct i2c_hid, panel_follower);
+ struct i2c_hid *ihid = container_of(work, struct i2c_hid,
+ panel_follower_prepare_work);
struct hid_device *hid = ihid->hid;
+ int ret;
/*
* hid->version is set on the first power up. If it's still zero then
@@ -1073,15 +1077,52 @@ static int i2c_hid_core_panel_prepared(struct drm_panel_follower *follower)
* steps.
*/
if (!hid->version)
- return i2c_hid_core_initial_power_up(ihid);
+ ret = i2c_hid_core_initial_power_up(ihid);
+ else
+ ret = i2c_hid_core_resume(ihid);
- return i2c_hid_core_resume(ihid);
+ if (ret)
+ dev_warn(&ihid->client->dev, "Power on failed: %d\n", ret);
+ else
+ WRITE_ONCE(ihid->prepare_work_finished, true);
+
+ /*
+ * The work APIs provide a number of memory ordering guarantees
+ * including one that says that memory writes before schedule_work()
+ * are always visible to the work function, but they don't appear to
+ * guarantee that a write that happened in the work is visible after
+ * cancel_work_sync(). We'll add a write memory barrier here to match
+ * with i2c_hid_core_panel_unpreparing() to ensure that our write to
+ * prepare_work_finished is visible there.
+ */
+ smp_wmb();
+}
+
+static int i2c_hid_core_panel_prepared(struct drm_panel_follower *follower)
+{
+ struct i2c_hid *ihid = container_of(follower, struct i2c_hid, panel_follower);
+
+ /*
+ * Powering on a touchscreen can be a slow process. Queue the work to
+ * the system workqueue so we don't block the panel's power up.
+ */
+ WRITE_ONCE(ihid->prepare_work_finished, false);
+ schedule_work(&ihid->panel_follower_prepare_work);
+
+ return 0;
}
static int i2c_hid_core_panel_unpreparing(struct drm_panel_follower *follower)
{
struct i2c_hid *ihid = container_of(follower, struct i2c_hid, panel_follower);
+ cancel_work_sync(&ihid->panel_follower_prepare_work);
+
+ /* Match with ihid_core_panel_prepare_work() */
+ smp_rmb();
+ if (!READ_ONCE(ihid->prepare_work_finished))
+ return 0;
+
return i2c_hid_core_suspend(ihid);
}
@@ -1124,6 +1165,7 @@ int i2c_hid_core_probe(struct i2c_client *client, struct i2chid_ops *ops,
init_waitqueue_head(&ihid->wait);
mutex_init(&ihid->reset_lock);
+ INIT_WORK(&ihid->panel_follower_prepare_work, ihid_core_panel_prepare_work);
/* we need to allocate the command buffer without knowing the maximum
* size of the reports. Let's use HID_MIN_BUFFER_SIZE, then we do the
--
2.41.0.162.gfafddb0af9-goog
^ permalink raw reply related
* [PATCH v2 08/10] HID: i2c-hid: Support being a panel follower
From: Douglas Anderson @ 2023-06-07 21:49 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Bjorn Andersson, Konrad Dybcio,
Rob Herring, Frank Rowand, Krzysztof Kozlowski, Conor Dooley,
Neil Armstrong, Sam Ravnborg, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann
Cc: dri-devel, Dmitry Torokhov, linux-input, Daniel Vetter,
linux-kernel, hsinyi, cros-qcom-dts-watchers, devicetree,
yangcong5, linux-arm-msm, Chris Morgan, Douglas Anderson
In-Reply-To: <20230607215224.2067679-1-dianders@chromium.org>
As talked about in the patch ("drm/panel: Add a way for other devices
to follow panel state"), we really want to keep the power states of a
touchscreen and the panel it's attached to in sync with each other. In
that spirit, add support to i2c-hid to be a panel follower. This will
let the i2c-hid driver get informed when the panel is powered on and
off. From there we can match the i2c-hid device's power state to that
of the panel.
NOTE: this patch specifically _doesn't_ use pm_runtime to keep track
of / manage the power state of the i2c-hid device, even though my
first instinct said that would be the way to go. Specific problems
with using pm_runtime():
* The initial power up couldn't happen in a runtime resume function
since it create sub-devices and, apparently, that's not good to do
in your resume function.
* Managing our power state with pm_runtime meant fighting to make the
right thing happen at system suspend to prevent the system from
trying to resume us only to suspend us again. While this might be
able to be solved, it added complexity.
Overall the code without pm_runtime() ended up being smaller and
easier to understand.
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
Changes in v2:
- i2c_hid_core_panel_prepared() and ..._unpreparing() are now static.
drivers/hid/i2c-hid/i2c-hid-core.c | 82 +++++++++++++++++++++++++++++-
1 file changed, 81 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
index fa8a1ca43d7f..368db3ae612f 100644
--- a/drivers/hid/i2c-hid/i2c-hid-core.c
+++ b/drivers/hid/i2c-hid/i2c-hid-core.c
@@ -38,6 +38,8 @@
#include <linux/mutex.h>
#include <asm/unaligned.h>
+#include <drm/drm_panel.h>
+
#include "../hid-ids.h"
#include "i2c-hid.h"
@@ -107,6 +109,8 @@ struct i2c_hid {
struct mutex reset_lock;
struct i2chid_ops *ops;
+ struct drm_panel_follower panel_follower;
+ bool is_panel_follower;
};
static const struct i2c_hid_quirks {
@@ -1058,6 +1062,34 @@ static int i2c_hid_core_initial_power_up(struct i2c_hid *ihid)
return ret;
}
+static int i2c_hid_core_panel_prepared(struct drm_panel_follower *follower)
+{
+ struct i2c_hid *ihid = container_of(follower, struct i2c_hid, panel_follower);
+ struct hid_device *hid = ihid->hid;
+
+ /*
+ * hid->version is set on the first power up. If it's still zero then
+ * this is the first power on so we should perform initial power up
+ * steps.
+ */
+ if (!hid->version)
+ return i2c_hid_core_initial_power_up(ihid);
+
+ return i2c_hid_core_resume(ihid);
+}
+
+static int i2c_hid_core_panel_unpreparing(struct drm_panel_follower *follower)
+{
+ struct i2c_hid *ihid = container_of(follower, struct i2c_hid, panel_follower);
+
+ return i2c_hid_core_suspend(ihid);
+}
+
+static const struct drm_panel_follower_funcs i2c_hid_core_panel_follower_funcs = {
+ .panel_prepared = i2c_hid_core_panel_prepared,
+ .panel_unpreparing = i2c_hid_core_panel_unpreparing,
+};
+
int i2c_hid_core_probe(struct i2c_client *client, struct i2chid_ops *ops,
u16 hid_descriptor_address, u32 quirks)
{
@@ -1119,6 +1151,41 @@ int i2c_hid_core_probe(struct i2c_client *client, struct i2chid_ops *ops,
hid->bus = BUS_I2C;
hid->initial_quirks = quirks;
+ /*
+ * See if we're following a panel. If drm_panel_add_follower()
+ * returns no error then we are.
+ */
+ ihid->panel_follower.funcs = &i2c_hid_core_panel_follower_funcs;
+ ret = drm_panel_add_follower(&client->dev, &ihid->panel_follower);
+ if (!ret) {
+ /* We're a follower. That means we'll power things up later. */
+ ihid->is_panel_follower = true;
+
+ /*
+ * If we're not in control of our own power up/power down then
+ * we can't do the logic to manage wakeups. Give a warning if
+ * a user thought that was possible then force the capability
+ * off.
+ */
+ if (device_can_wakeup(&client->dev)) {
+ dev_warn(&client->dev, "Can't wakeup if following panel\n");
+ device_set_wakeup_capable(&client->dev, false);
+ }
+
+ return 0;
+ }
+
+ /*
+ * -ENODEV means that we're not following a panel, so any other error
+ * is a real problem (like -EPROBE_DEFER, -ENOMEM, ...).
+ */
+ if (ret != -ENODEV)
+ goto err_mem_free;
+
+ /*
+ * We're not following a panel. That's fine and means that we
+ * can power up right away.
+ */
ret = i2c_hid_core_initial_power_up(ihid);
if (ret)
goto err_mem_free;
@@ -1143,7 +1210,14 @@ void i2c_hid_core_remove(struct i2c_client *client)
struct i2c_hid *ihid = i2c_get_clientdata(client);
struct hid_device *hid;
- i2c_hid_core_power_down(ihid);
+ /*
+ * If we're a follower, the act of unfollowing will cause us to be
+ * powered down. Otherwise we need to manually do it.
+ */
+ if (ihid->is_panel_follower)
+ drm_panel_remove_follower(&ihid->panel_follower);
+ else
+ i2c_hid_core_power_down(ihid);
hid = ihid->hid;
hid_destroy_device(hid);
@@ -1171,6 +1245,9 @@ static int i2c_hid_core_pm_suspend(struct device *dev)
struct i2c_client *client = to_i2c_client(dev);
struct i2c_hid *ihid = i2c_get_clientdata(client);
+ if (ihid->is_panel_follower)
+ return 0;
+
return i2c_hid_core_suspend(ihid);
}
@@ -1179,6 +1256,9 @@ static int i2c_hid_core_pm_resume(struct device *dev)
struct i2c_client *client = to_i2c_client(dev);
struct i2c_hid *ihid = i2c_get_clientdata(client);
+ if (ihid->is_panel_follower)
+ return 0;
+
return i2c_hid_core_resume(ihid);
}
--
2.41.0.162.gfafddb0af9-goog
^ permalink raw reply related
* [PATCH v2 07/10] HID: i2c-hid: Make suspend and resume into helper functions
From: Douglas Anderson @ 2023-06-07 21:49 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Bjorn Andersson, Konrad Dybcio,
Rob Herring, Frank Rowand, Krzysztof Kozlowski, Conor Dooley,
Neil Armstrong, Sam Ravnborg, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann
Cc: dri-devel, Dmitry Torokhov, linux-input, Daniel Vetter,
linux-kernel, hsinyi, cros-qcom-dts-watchers, devicetree,
yangcong5, linux-arm-msm, Chris Morgan, Douglas Anderson
In-Reply-To: <20230607215224.2067679-1-dianders@chromium.org>
In a future patch we'd like to be able to call the current i2c-hid
suspend and resume functions from times other than system
suspend. Move the functions higher up in the file and have them take a
"struct i2c_hid" to make this simpler. We'll then add tiny wrappers of
the functions for use with system suspend.
This change is expected to have no functional effect.
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
(no changes since v1)
drivers/hid/i2c-hid/i2c-hid-core.c | 98 +++++++++++++++++-------------
1 file changed, 56 insertions(+), 42 deletions(-)
diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
index d29e6421ecba..fa8a1ca43d7f 100644
--- a/drivers/hid/i2c-hid/i2c-hid-core.c
+++ b/drivers/hid/i2c-hid/i2c-hid-core.c
@@ -941,6 +941,57 @@ static void i2c_hid_core_shutdown_tail(struct i2c_hid *ihid)
ihid->ops->shutdown_tail(ihid->ops);
}
+static int i2c_hid_core_suspend(struct i2c_hid *ihid)
+{
+ struct i2c_client *client = ihid->client;
+ struct hid_device *hid = ihid->hid;
+ int ret;
+
+ ret = hid_driver_suspend(hid, PMSG_SUSPEND);
+ if (ret < 0)
+ return ret;
+
+ /* Save some power */
+ i2c_hid_set_power(ihid, I2C_HID_PWR_SLEEP);
+
+ disable_irq(client->irq);
+
+ if (!device_may_wakeup(&client->dev))
+ i2c_hid_core_power_down(ihid);
+
+ return 0;
+}
+
+static int i2c_hid_core_resume(struct i2c_hid *ihid)
+{
+ struct i2c_client *client = ihid->client;
+ struct hid_device *hid = ihid->hid;
+ int ret;
+
+ if (!device_may_wakeup(&client->dev))
+ i2c_hid_core_power_up(ihid);
+
+ enable_irq(client->irq);
+
+ /* Instead of resetting device, simply powers the device on. This
+ * solves "incomplete reports" on Raydium devices 2386:3118 and
+ * 2386:4B33 and fixes various SIS touchscreens no longer sending
+ * data after a suspend/resume.
+ *
+ * However some ALPS touchpads generate IRQ storm without reset, so
+ * let's still reset them here.
+ */
+ if (ihid->quirks & I2C_HID_QUIRK_RESET_ON_RESUME)
+ ret = i2c_hid_hwreset(ihid);
+ else
+ ret = i2c_hid_set_power(ihid, I2C_HID_PWR_ON);
+
+ if (ret)
+ return ret;
+
+ return hid_driver_reset_resume(hid);
+}
+
/**
* i2c_hid_core_initial_power_up() - First time power up of the i2c-hid device.
* @ihid: The ihid object created during probe.
@@ -1115,61 +1166,24 @@ void i2c_hid_core_shutdown(struct i2c_client *client)
}
EXPORT_SYMBOL_GPL(i2c_hid_core_shutdown);
-static int i2c_hid_core_suspend(struct device *dev)
+static int i2c_hid_core_pm_suspend(struct device *dev)
{
struct i2c_client *client = to_i2c_client(dev);
struct i2c_hid *ihid = i2c_get_clientdata(client);
- struct hid_device *hid = ihid->hid;
- int ret;
-
- ret = hid_driver_suspend(hid, PMSG_SUSPEND);
- if (ret < 0)
- return ret;
- /* Save some power */
- i2c_hid_set_power(ihid, I2C_HID_PWR_SLEEP);
-
- disable_irq(client->irq);
-
- if (!device_may_wakeup(&client->dev))
- i2c_hid_core_power_down(ihid);
-
- return 0;
+ return i2c_hid_core_suspend(ihid);
}
-static int i2c_hid_core_resume(struct device *dev)
+static int i2c_hid_core_pm_resume(struct device *dev)
{
- int ret;
struct i2c_client *client = to_i2c_client(dev);
struct i2c_hid *ihid = i2c_get_clientdata(client);
- struct hid_device *hid = ihid->hid;
- if (!device_may_wakeup(&client->dev))
- i2c_hid_core_power_up(ihid);
-
- enable_irq(client->irq);
-
- /* Instead of resetting device, simply powers the device on. This
- * solves "incomplete reports" on Raydium devices 2386:3118 and
- * 2386:4B33 and fixes various SIS touchscreens no longer sending
- * data after a suspend/resume.
- *
- * However some ALPS touchpads generate IRQ storm without reset, so
- * let's still reset them here.
- */
- if (ihid->quirks & I2C_HID_QUIRK_RESET_ON_RESUME)
- ret = i2c_hid_hwreset(ihid);
- else
- ret = i2c_hid_set_power(ihid, I2C_HID_PWR_ON);
-
- if (ret)
- return ret;
-
- return hid_driver_reset_resume(hid);
+ return i2c_hid_core_resume(ihid);
}
const struct dev_pm_ops i2c_hid_core_pm = {
- SYSTEM_SLEEP_PM_OPS(i2c_hid_core_suspend, i2c_hid_core_resume)
+ SYSTEM_SLEEP_PM_OPS(i2c_hid_core_pm_suspend, i2c_hid_core_pm_resume)
};
EXPORT_SYMBOL_GPL(i2c_hid_core_pm);
--
2.41.0.162.gfafddb0af9-goog
^ permalink raw reply related
* [PATCH v2 06/10] HID: i2c-hid: Rearrange probe() to power things up later
From: Douglas Anderson @ 2023-06-07 21:49 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Bjorn Andersson, Konrad Dybcio,
Rob Herring, Frank Rowand, Krzysztof Kozlowski, Conor Dooley,
Neil Armstrong, Sam Ravnborg, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann
Cc: dri-devel, Dmitry Torokhov, linux-input, Daniel Vetter,
linux-kernel, hsinyi, cros-qcom-dts-watchers, devicetree,
yangcong5, linux-arm-msm, Chris Morgan, Douglas Anderson
In-Reply-To: <20230607215224.2067679-1-dianders@chromium.org>
In a future patch, we want to change i2c-hid not to necessarily power
up the touchscreen during probe. In preparation for that, rearrange
the probe function so that we put as much stuff _before_ powering up
the device as possible.
This change is expected to have no functional effect.
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
Changes in v2:
- i2c_hid_core_initial_power_up() is now static.
drivers/hid/i2c-hid/i2c-hid-core.c | 124 ++++++++++++++++++-----------
1 file changed, 77 insertions(+), 47 deletions(-)
diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
index 19d985c20a5c..d29e6421ecba 100644
--- a/drivers/hid/i2c-hid/i2c-hid-core.c
+++ b/drivers/hid/i2c-hid/i2c-hid-core.c
@@ -855,7 +855,8 @@ static int i2c_hid_init_irq(struct i2c_client *client)
irqflags = IRQF_TRIGGER_LOW;
ret = request_threaded_irq(client->irq, NULL, i2c_hid_irq,
- irqflags | IRQF_ONESHOT, client->name, ihid);
+ irqflags | IRQF_ONESHOT | IRQF_NO_AUTOEN,
+ client->name, ihid);
if (ret < 0) {
dev_warn(&client->dev,
"Could not register for %s interrupt, irq = %d,"
@@ -940,6 +941,72 @@ static void i2c_hid_core_shutdown_tail(struct i2c_hid *ihid)
ihid->ops->shutdown_tail(ihid->ops);
}
+/**
+ * i2c_hid_core_initial_power_up() - First time power up of the i2c-hid device.
+ * @ihid: The ihid object created during probe.
+ *
+ * This function is called at probe time.
+ *
+ * The initial power on is where we do some basic validation that the device
+ * exists, where we fetch the HID descriptor, and where we create the actual
+ * HID devices.
+ *
+ * Return: 0 or error code.
+ */
+static int i2c_hid_core_initial_power_up(struct i2c_hid *ihid)
+{
+ struct i2c_client *client = ihid->client;
+ struct hid_device *hid = ihid->hid;
+ int ret;
+
+ ret = i2c_hid_core_power_up(ihid);
+ if (ret)
+ return ret;
+
+ /* Make sure there is something at this address */
+ ret = i2c_smbus_read_byte(client);
+ if (ret < 0) {
+ i2c_hid_dbg(ihid, "nothing at this address: %d\n", ret);
+ ret = -ENXIO;
+ goto err;
+ }
+
+ ret = i2c_hid_fetch_hid_descriptor(ihid);
+ if (ret < 0) {
+ dev_err(&client->dev,
+ "Failed to fetch the HID Descriptor\n");
+ goto err;
+ }
+
+ enable_irq(client->irq);
+
+ hid->version = le16_to_cpu(ihid->hdesc.bcdVersion);
+ hid->vendor = le16_to_cpu(ihid->hdesc.wVendorID);
+ hid->product = le16_to_cpu(ihid->hdesc.wProductID);
+
+ hid->initial_quirks |= i2c_hid_get_dmi_quirks(hid->vendor,
+ hid->product);
+
+ snprintf(hid->name, sizeof(hid->name), "%s %04X:%04X",
+ client->name, (u16)hid->vendor, (u16)hid->product);
+ strscpy(hid->phys, dev_name(&client->dev), sizeof(hid->phys));
+
+ ihid->quirks = i2c_hid_lookup_quirk(hid->vendor, hid->product);
+
+ ret = hid_add_device(hid);
+ if (ret) {
+ if (ret != -ENODEV)
+ hid_err(client, "can't add hid device: %d\n", ret);
+ goto err;
+ }
+
+ return 0;
+
+err:
+ i2c_hid_core_power_down(ihid);
+ return ret;
+}
+
int i2c_hid_core_probe(struct i2c_client *client, struct i2chid_ops *ops,
u16 hid_descriptor_address, u32 quirks)
{
@@ -966,16 +1033,10 @@ int i2c_hid_core_probe(struct i2c_client *client, struct i2chid_ops *ops,
if (!ihid)
return -ENOMEM;
- ihid->ops = ops;
-
- ret = i2c_hid_core_power_up(ihid);
- if (ret)
- return ret;
-
i2c_set_clientdata(client, ihid);
+ ihid->ops = ops;
ihid->client = client;
-
ihid->wHIDDescRegister = cpu_to_le16(hid_descriptor_address);
init_waitqueue_head(&ihid->wait);
@@ -986,28 +1047,12 @@ int i2c_hid_core_probe(struct i2c_client *client, struct i2chid_ops *ops,
* real computation later. */
ret = i2c_hid_alloc_buffers(ihid, HID_MIN_BUFFER_SIZE);
if (ret < 0)
- goto err_powered;
-
+ return ret;
device_enable_async_suspend(&client->dev);
- /* Make sure there is something at this address */
- ret = i2c_smbus_read_byte(client);
- if (ret < 0) {
- i2c_hid_dbg(ihid, "nothing at this address: %d\n", ret);
- ret = -ENXIO;
- goto err_powered;
- }
-
- ret = i2c_hid_fetch_hid_descriptor(ihid);
- if (ret < 0) {
- dev_err(&client->dev,
- "Failed to fetch the HID Descriptor\n");
- goto err_powered;
- }
-
ret = i2c_hid_init_irq(client);
if (ret < 0)
- goto err_powered;
+ goto err_buffers_allocated;
hid = hid_allocate_device();
if (IS_ERR(hid)) {
@@ -1021,26 +1066,11 @@ int i2c_hid_core_probe(struct i2c_client *client, struct i2chid_ops *ops,
hid->ll_driver = &i2c_hid_ll_driver;
hid->dev.parent = &client->dev;
hid->bus = BUS_I2C;
- hid->version = le16_to_cpu(ihid->hdesc.bcdVersion);
- hid->vendor = le16_to_cpu(ihid->hdesc.wVendorID);
- hid->product = le16_to_cpu(ihid->hdesc.wProductID);
-
hid->initial_quirks = quirks;
- hid->initial_quirks |= i2c_hid_get_dmi_quirks(hid->vendor,
- hid->product);
-
- snprintf(hid->name, sizeof(hid->name), "%s %04X:%04X",
- client->name, (u16)hid->vendor, (u16)hid->product);
- strscpy(hid->phys, dev_name(&client->dev), sizeof(hid->phys));
-
- ihid->quirks = i2c_hid_lookup_quirk(hid->vendor, hid->product);
- ret = hid_add_device(hid);
- if (ret) {
- if (ret != -ENODEV)
- hid_err(client, "can't add hid device: %d\n", ret);
+ ret = i2c_hid_core_initial_power_up(ihid);
+ if (ret)
goto err_mem_free;
- }
return 0;
@@ -1050,9 +1080,9 @@ int i2c_hid_core_probe(struct i2c_client *client, struct i2chid_ops *ops,
err_irq:
free_irq(client->irq, ihid);
-err_powered:
- i2c_hid_core_power_down(ihid);
+err_buffers_allocated:
i2c_hid_free_buffers(ihid);
+
return ret;
}
EXPORT_SYMBOL_GPL(i2c_hid_core_probe);
@@ -1062,6 +1092,8 @@ void i2c_hid_core_remove(struct i2c_client *client)
struct i2c_hid *ihid = i2c_get_clientdata(client);
struct hid_device *hid;
+ i2c_hid_core_power_down(ihid);
+
hid = ihid->hid;
hid_destroy_device(hid);
@@ -1069,8 +1101,6 @@ void i2c_hid_core_remove(struct i2c_client *client)
if (ihid->bufsize)
i2c_hid_free_buffers(ihid);
-
- i2c_hid_core_power_down(ihid);
}
EXPORT_SYMBOL_GPL(i2c_hid_core_remove);
--
2.41.0.162.gfafddb0af9-goog
^ permalink raw reply related
* [PATCH v2 04/10] of: property: fw_devlink: Add a devlink for panel followers
From: Douglas Anderson @ 2023-06-07 21:49 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Bjorn Andersson, Konrad Dybcio,
Rob Herring, Frank Rowand, Krzysztof Kozlowski, Conor Dooley,
Neil Armstrong, Sam Ravnborg, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann
Cc: dri-devel, Dmitry Torokhov, linux-input, Daniel Vetter,
linux-kernel, hsinyi, cros-qcom-dts-watchers, devicetree,
yangcong5, linux-arm-msm, Chris Morgan, Douglas Anderson
In-Reply-To: <20230607215224.2067679-1-dianders@chromium.org>
Inform fw_devlink of the fact that a panel follower (like a
touchscreen) is effectively a consumer of the panel from the purposes
of fw_devlink.
NOTE: this patch isn't required for correctness but instead optimizes
probe order / helps avoid deferrals.
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
Since this is so small, I'd presume it's OK for it to go through a DRM
tree with the proper Ack. That being said, this patch is just an
optimization and thus it could land completely separately from the
rest and they could all meet up in mainline.
Changes in v2:
- ("Add a devlink for panel followers") new for v2.
drivers/of/property.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/of/property.c b/drivers/of/property.c
index ddc75cd50825..cf8dacf3e3b8 100644
--- a/drivers/of/property.c
+++ b/drivers/of/property.c
@@ -1266,6 +1266,7 @@ DEFINE_SIMPLE_PROP(pwms, "pwms", "#pwm-cells")
DEFINE_SIMPLE_PROP(resets, "resets", "#reset-cells")
DEFINE_SIMPLE_PROP(leds, "leds", NULL)
DEFINE_SIMPLE_PROP(backlight, "backlight", NULL)
+DEFINE_SIMPLE_PROP(panel, "panel", NULL)
DEFINE_SUFFIX_PROP(regulators, "-supply", NULL)
DEFINE_SUFFIX_PROP(gpio, "-gpio", "#gpio-cells")
@@ -1354,6 +1355,7 @@ static const struct supplier_bindings of_supplier_bindings[] = {
{ .parse_prop = parse_resets, },
{ .parse_prop = parse_leds, },
{ .parse_prop = parse_backlight, },
+ { .parse_prop = parse_panel, },
{ .parse_prop = parse_gpio_compat, },
{ .parse_prop = parse_interrupts, },
{ .parse_prop = parse_regulators, },
--
2.41.0.162.gfafddb0af9-goog
^ permalink raw reply related
* [PATCH v2 05/10] HID: i2c-hid: Switch to SYSTEM_SLEEP_PM_OPS()
From: Douglas Anderson @ 2023-06-07 21:49 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Bjorn Andersson, Konrad Dybcio,
Rob Herring, Frank Rowand, Krzysztof Kozlowski, Conor Dooley,
Neil Armstrong, Sam Ravnborg, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann
Cc: dri-devel, Dmitry Torokhov, linux-input, Daniel Vetter,
linux-kernel, hsinyi, cros-qcom-dts-watchers, devicetree,
yangcong5, linux-arm-msm, Chris Morgan, Douglas Anderson
In-Reply-To: <20230607215224.2067679-1-dianders@chromium.org>
The SYSTEM_SLEEP_PM_OPS() allows us to get rid of '#ifdef
CONFIG_PM_SLEEP', as talked about in commit 1a3c7bb08826 ("PM: core:
Add new *_PM_OPS macros, deprecate old ones").
This change is expected to have no functional effect.
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
(no changes since v1)
drivers/hid/i2c-hid/i2c-hid-core.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
index efbba0465eef..19d985c20a5c 100644
--- a/drivers/hid/i2c-hid/i2c-hid-core.c
+++ b/drivers/hid/i2c-hid/i2c-hid-core.c
@@ -1085,7 +1085,6 @@ void i2c_hid_core_shutdown(struct i2c_client *client)
}
EXPORT_SYMBOL_GPL(i2c_hid_core_shutdown);
-#ifdef CONFIG_PM_SLEEP
static int i2c_hid_core_suspend(struct device *dev)
{
struct i2c_client *client = to_i2c_client(dev);
@@ -1138,10 +1137,9 @@ static int i2c_hid_core_resume(struct device *dev)
return hid_driver_reset_resume(hid);
}
-#endif
const struct dev_pm_ops i2c_hid_core_pm = {
- SET_SYSTEM_SLEEP_PM_OPS(i2c_hid_core_suspend, i2c_hid_core_resume)
+ SYSTEM_SLEEP_PM_OPS(i2c_hid_core_suspend, i2c_hid_core_resume)
};
EXPORT_SYMBOL_GPL(i2c_hid_core_pm);
--
2.41.0.162.gfafddb0af9-goog
^ permalink raw reply related
* [PATCH v2 03/10] drm/panel: Add a way for other devices to follow panel state
From: Douglas Anderson @ 2023-06-07 21:49 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Bjorn Andersson, Konrad Dybcio,
Rob Herring, Frank Rowand, Krzysztof Kozlowski, Conor Dooley,
Neil Armstrong, Sam Ravnborg, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann
Cc: dri-devel, Dmitry Torokhov, linux-input, Daniel Vetter,
linux-kernel, hsinyi, cros-qcom-dts-watchers, devicetree,
yangcong5, linux-arm-msm, Chris Morgan, Douglas Anderson
In-Reply-To: <20230607215224.2067679-1-dianders@chromium.org>
These days, it's fairly common to see panels that have touchscreens
attached to them. The panel and the touchscreen can somewhat be
thought of as totally separate devices and, historically, this is how
Linux has treated them. However, treating them as separate isn't
necessarily the best way to model the two devices, it was just that
there was no better way. Specifically, there is little practical
reason to have the touchscreen powered on when the panel is turned
off, but if we model the devices separately we have no way to keep the
two devices' power states in sync with each other.
The issue described above makes it sound as if the problem here is
just about efficiency. We're wasting power keeping the touchscreen
powered up when the screen is off. While that's true, the problem can
go deeper. Specifically, hardware designers see that there's no reason
to have the touchscreen on while the screen is off and then build
hardware assuming that software would never turn the touchscreen on
while the screen is off.
In the very simplest case of hardware designs like this, the
touchscreen and the panel share some power rails. In most cases, this
turns out not to be terrible and is, again, just a little less
efficient. Specifically if we tell Linux that the touchscreen and the
panel are using the same rails then Linux will keep the rails on when
_either_ device is turned on. That ends to work OK-ish, but now if you
turn the panel off not only will the touchscreen remain powered, but
the power rails for the panel itself won't be switched off, burning
extra power.
The above two inefficiencies are _extra_ minor when you consider the
fact that laptops rarely spend much time with the screen off. The main
use case would be when an external screen (and presumably a power
supply) is attached.
Unfortunately, it gets worse from here. On sc7180-trogdor-homestar,
for instance, the display's TCON (timing controller) sometimes crashes
if you don't power cycle it whenever you stop and restart the video
stream (like during a modeset). The touchscreen keeping the power
rails on causes real problems. One proposal in the homestar timeframe
was to move the touchscreen to an always-on rail, dedicating the main
power rail to the panel. That caused _different_ problems as talked
about in commit 557e05fa9fdd ("HID: i2c-hid: goodix: Stop tying the
reset line to the regulator"). The end result of all of this was to
add an extra regulator to the board, increasing cost.
Recently, Cong Yang posted a patch [1] where things are even worse.
The panel and touch controller on that system seem even more
intimately tied together and really can't be thought of separately.
To address this issue, let's start allowing devices to register
themselves as "panel followers". These devices will get called after a
panel has been powered on and before a panel is powered off. This
makes the panel the primary device in charge of the power state, which
matches how userspace uses it.
The panel follower API should be fairly straightforward to use. The
current code assumes that panel followers are using device tree and
have a "panel" property pointing to the panel to follow. More
flexibility and non-DT implementations could be added as needed.
Right now, panel followers can follow the prepare/unprepare functions.
There could be arguments made that, instead, they should follow
enable/disable. I've chosen prepare/unprepare for now since those
functions are guaranteed to power up/power down the panel and it seems
better to start the process earlier.
A bit of explaining about why this is a roll-your-own API instead of
using something more standard:
1. In standard APIs in Linux, parent devices are automatically powered
on when a child needs power. Applying that here, it would mean that
we'd force the panel on any time someone was listening to the
touchscreen. That, unfortunately, would have broken homestar's need
(if we hadn't changed the hardware, as per above) where the panel
absolutely needs to be able to power cycle itself. While one could
argue that homestar is broken hardware and we shouldn't have the
API do backflips for it, _officially_ the eDP timing guidelines
agree with homestar's needs and the panel power sequencing diagrams
show power going off. It's nice to be able to support this.
2. We could, conceibably, try to add a new flag to device_link causing
the parent to be in charge of power. Then we could at least use
normal pm_runtime APIs. This sounds great, except that we run into
problems with initial probe. As talked about in the later patch
("HID: i2c-hid: Support being a panel follower") the initial power
on of a panel follower might need to do things (like add
sub-devices) that aren't allowed in a runtime_resume function.
The above complexities explain why this API isn't using common
functions. That being said, this patch is very small and
self-contained, so if someone was later able to adapt it to using more
common APIs while solving the above issues then that could happen in
the future.
[1] 20230519032316.3464732-1-yangcong5@huaqin.corp-partner.google.com
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
Changes in v2:
- Add even more text to the commit message.
- A few comment cleanups.
drivers/gpu/drm/drm_panel.c | 151 +++++++++++++++++++++++++++++++++++-
include/drm/drm_panel.h | 75 ++++++++++++++++++
2 files changed, 222 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
index 4e1c4e42575b..d103eda89f8e 100644
--- a/drivers/gpu/drm/drm_panel.c
+++ b/drivers/gpu/drm/drm_panel.c
@@ -58,6 +58,8 @@ void drm_panel_init(struct drm_panel *panel, struct device *dev,
const struct drm_panel_funcs *funcs, int connector_type)
{
INIT_LIST_HEAD(&panel->list);
+ INIT_LIST_HEAD(&panel->followers);
+ mutex_init(&panel->follower_lock);
panel->dev = dev;
panel->funcs = funcs;
panel->connector_type = connector_type;
@@ -105,6 +107,7 @@ EXPORT_SYMBOL(drm_panel_remove);
*/
int drm_panel_prepare(struct drm_panel *panel)
{
+ struct drm_panel_follower *follower;
int ret;
if (!panel)
@@ -115,14 +118,27 @@ int drm_panel_prepare(struct drm_panel *panel)
return 0;
}
+ mutex_lock(&panel->follower_lock);
+
if (panel->funcs && panel->funcs->prepare) {
ret = panel->funcs->prepare(panel);
if (ret < 0)
- return ret;
+ goto exit;
}
panel->prepared = true;
- return 0;
+ list_for_each_entry(follower, &panel->followers, list) {
+ ret = follower->funcs->panel_prepared(follower);
+ if (ret < 0)
+ dev_info(panel->dev, "%ps failed: %d\n",
+ follower->funcs->panel_prepared, ret);
+ }
+
+ ret = 0;
+exit:
+ mutex_unlock(&panel->follower_lock);
+
+ return ret;
}
EXPORT_SYMBOL(drm_panel_prepare);
@@ -139,6 +155,7 @@ EXPORT_SYMBOL(drm_panel_prepare);
*/
int drm_panel_unprepare(struct drm_panel *panel)
{
+ struct drm_panel_follower *follower;
int ret;
if (!panel)
@@ -149,14 +166,27 @@ int drm_panel_unprepare(struct drm_panel *panel)
return 0;
}
+ mutex_lock(&panel->follower_lock);
+
+ list_for_each_entry(follower, &panel->followers, list) {
+ ret = follower->funcs->panel_unpreparing(follower);
+ if (ret < 0)
+ dev_info(panel->dev, "%ps failed: %d\n",
+ follower->funcs->panel_unpreparing, ret);
+ }
+
if (panel->funcs && panel->funcs->unprepare) {
ret = panel->funcs->unprepare(panel);
if (ret < 0)
- return ret;
+ goto exit;
}
panel->prepared = false;
- return 0;
+ ret = 0;
+exit:
+ mutex_unlock(&panel->follower_lock);
+
+ return ret;
}
EXPORT_SYMBOL(drm_panel_unprepare);
@@ -342,6 +372,119 @@ int of_drm_get_panel_orientation(const struct device_node *np,
EXPORT_SYMBOL(of_drm_get_panel_orientation);
#endif
+/**
+ * drm_panel_add_follower() - Register something to follow panel state.
+ * @follower_dev: The 'struct device' for the follower.
+ * @follower: The panel follower descriptor for the follower.
+ *
+ * A panel follower is called right after preparing the panel and right before
+ * unpreparing the panel. It's primary intention is to power on an associated
+ * touchscreen, though it could be used for any similar devices. Multiple
+ * devices are allowed the follow the same panel.
+ *
+ * If a follower is added to a panel that's already been turned on, the
+ * follower's prepare callback is called right away.
+ *
+ * At the moment panels can only be followed on device tree enabled systems.
+ * The "panel" property of the follower points to the panel to be followed.
+ *
+ * Return: 0 or an error code. Note that -ENODEV means that we detected that
+ * follower_dev is not actually following a panel. The caller may
+ * choose to ignore this return value if following a panel is optional.
+ */
+int drm_panel_add_follower(struct device *follower_dev,
+ struct drm_panel_follower *follower)
+{
+ struct device_node *panel_np;
+ struct drm_panel *panel;
+ int ret;
+
+ panel_np = of_parse_phandle(follower_dev->of_node, "panel", 0);
+ if (!panel_np)
+ return -ENODEV;
+
+ panel = of_drm_find_panel(panel_np);
+ of_node_put(panel_np);
+ if (IS_ERR(panel))
+ return PTR_ERR(panel);
+
+ get_device(panel->dev);
+ follower->panel = panel;
+
+ mutex_lock(&panel->follower_lock);
+
+ list_add_tail(&follower->list, &panel->followers);
+ if (panel->prepared) {
+ ret = follower->funcs->panel_prepared(follower);
+ if (ret < 0)
+ dev_info(panel->dev, "%ps failed: %d\n",
+ follower->funcs->panel_prepared, ret);
+ }
+
+ mutex_unlock(&panel->follower_lock);
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_panel_add_follower);
+
+/**
+ * drm_panel_remove_follower() - Reverse drm_panel_add_follower().
+ * @follower: The panel follower descriptor for the follower.
+ *
+ * Undo drm_panel_add_follower(). This includes calling the follower's
+ * unprepare function if we're removed from a panel that's currently prepared.
+ *
+ * Return: 0 or an error code.
+ */
+void drm_panel_remove_follower(struct drm_panel_follower *follower)
+{
+ struct drm_panel *panel = follower->panel;
+ int ret;
+
+ mutex_lock(&panel->follower_lock);
+
+ if (panel->prepared) {
+ ret = follower->funcs->panel_unpreparing(follower);
+ if (ret < 0)
+ dev_info(panel->dev, "%ps failed: %d\n",
+ follower->funcs->panel_unpreparing, ret);
+ }
+ list_del_init(&follower->list);
+
+ mutex_unlock(&panel->follower_lock);
+
+ put_device(panel->dev);
+}
+EXPORT_SYMBOL(drm_panel_remove_follower);
+
+static void drm_panel_remove_follower_void(void *follower)
+{
+ drm_panel_remove_follower(follower);
+}
+
+/**
+ * devm_drm_panel_add_follower() - devm version of drm_panel_add_follower()
+ * @follower_dev: The 'struct device' for the follower.
+ * @follower: The panel follower descriptor for the follower.
+ *
+ * Handles calling drm_panel_remove_follower() using devm on the follower_dev.
+ *
+ * Return: 0 or an error code.
+ */
+int devm_drm_panel_add_follower(struct device *follower_dev,
+ struct drm_panel_follower *follower)
+{
+ int ret;
+
+ ret = drm_panel_add_follower(follower_dev, follower);
+ if (ret)
+ return ret;
+
+ return devm_add_action_or_reset(follower_dev,
+ drm_panel_remove_follower_void, follower);
+}
+EXPORT_SYMBOL(devm_drm_panel_add_follower);
+
#if IS_REACHABLE(CONFIG_BACKLIGHT_CLASS_DEVICE)
/**
* drm_panel_of_backlight - use backlight device node for backlight
diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
index c6cf75909389..e0a4d2f6f7fb 100644
--- a/include/drm/drm_panel.h
+++ b/include/drm/drm_panel.h
@@ -27,12 +27,14 @@
#include <linux/err.h>
#include <linux/errno.h>
#include <linux/list.h>
+#include <linux/mutex.h>
struct backlight_device;
struct dentry;
struct device_node;
struct drm_connector;
struct drm_device;
+struct drm_panel_follower;
struct drm_panel;
struct display_timing;
@@ -144,6 +146,45 @@ struct drm_panel_funcs {
void (*debugfs_init)(struct drm_panel *panel, struct dentry *root);
};
+struct drm_panel_follower_funcs {
+ /**
+ * @panel_prepared:
+ *
+ * Called after the panel has been powered on.
+ */
+ int (*panel_prepared)(struct drm_panel_follower *follower);
+
+ /**
+ * @panel_unpreparing:
+ *
+ * Called before the panel is powered off.
+ */
+ int (*panel_unpreparing)(struct drm_panel_follower *follower);
+};
+
+struct drm_panel_follower {
+ /**
+ * @funcs:
+ *
+ * Dependent device callbacks; should be initted by the caller.
+ */
+ const struct drm_panel_follower_funcs *funcs;
+
+ /**
+ * @list
+ *
+ * Used for linking into panel's list; set by drm_panel_add_follower().
+ */
+ struct list_head list;
+
+ /**
+ * @panel
+ *
+ * The panel we're dependent on; set by drm_panel_add_follower().
+ */
+ struct drm_panel *panel;
+};
+
/**
* struct drm_panel - DRM panel object
*/
@@ -189,6 +230,20 @@ struct drm_panel {
*/
struct list_head list;
+ /**
+ * @followers:
+ *
+ * A list of struct drm_panel_follower dependent on this panel.
+ */
+ struct list_head followers;
+
+ /**
+ * @followers_lock:
+ *
+ * Lock for followers list.
+ */
+ struct mutex follower_lock;
+
/**
* @prepare_prev_first:
*
@@ -246,6 +301,26 @@ static inline int of_drm_get_panel_orientation(const struct device_node *np,
}
#endif
+#if defined(CONFIG_DRM_PANEL)
+int drm_panel_add_follower(struct device *follower_dev,
+ struct drm_panel_follower *follower);
+void drm_panel_remove_follower(struct drm_panel_follower *follower);
+int devm_drm_panel_add_follower(struct device *follower_dev,
+ struct drm_panel_follower *follower);
+#else
+static inline int drm_panel_add_follower(struct device *follower_dev,
+ struct drm_panel_follower *follower)
+{
+ return -ENODEV;
+}
+static inline void drm_panel_remove_follower(struct drm_panel_follower *follower) { }
+static inline int devm_drm_panel_add_follower(struct device *follower_dev,
+ struct drm_panel_follower *follower)
+{
+ return -ENODEV;
+}
+#endif
+
#if IS_ENABLED(CONFIG_DRM_PANEL) && (IS_BUILTIN(CONFIG_BACKLIGHT_CLASS_DEVICE) || \
(IS_MODULE(CONFIG_DRM) && IS_MODULE(CONFIG_BACKLIGHT_CLASS_DEVICE)))
int drm_panel_of_backlight(struct drm_panel *panel);
--
2.41.0.162.gfafddb0af9-goog
^ permalink raw reply related
* [PATCH v2 02/10] drm/panel: Check for already prepared/enabled in drm_panel
From: Douglas Anderson @ 2023-06-07 21:49 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Bjorn Andersson, Konrad Dybcio,
Rob Herring, Frank Rowand, Krzysztof Kozlowski, Conor Dooley,
Neil Armstrong, Sam Ravnborg, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann
Cc: dri-devel, Dmitry Torokhov, linux-input, Daniel Vetter,
linux-kernel, hsinyi, cros-qcom-dts-watchers, devicetree,
yangcong5, linux-arm-msm, Chris Morgan, Douglas Anderson
In-Reply-To: <20230607215224.2067679-1-dianders@chromium.org>
In a whole pile of panel drivers, we have code to make the
prepare/unprepare/enable/disable callbacks behave as no-ops if they've
already been called. It's silly to have this code duplicated
everywhere. Add it to the core instead so that we can eventually
delete it from all the drivers. Note: to get some idea of the
duplicated code, try:
git grep 'if.*>prepared' -- drivers/gpu/drm/panel
git grep 'if.*>enabled' -- drivers/gpu/drm/panel
NOTE: arguably, the right thing to do here is actually to skip this
patch and simply remove all the extra checks from the individual
drivers. Perhaps the checks were needed at some point in time in the
past but maybe they no longer are? Certainly as we continue
transitioning over to "panel_bridge" then we expect there to be much
less variety in how these calls are made. When we're called as part of
the bridge chain, things should be pretty simple. In fact, there was
some discussion in the past about these checks [1], including a
discussion about whether the checks were needed and whether the calls
ought to be refcounted. At the time, I decided not to mess with it
because it felt too risky.
Looking closer at it now, I'm fairly certain that nothing in the
existing codebase is expecting these calls to be refcounted. The only
real question is whether someone is already doing something to ensure
prepare()/unprepare() match and enabled()/disable() match. I would say
that, even if there is something else ensuring that things match,
there's enough complexity that adding an extra bool and an extra
double-check here is a good idea. Let's add a drm_warn() to let people
know that it's considered a minor error to take advantage of
drm_panel's double-checking but we'll still make things work fine.
[1] https://lore.kernel.org/r/20210416153909.v4.27.I502f2a92ddd36c3d28d014dd75e170c2d405a0a5@changeid
Acked-by: Neil Armstrong <neil.armstrong@linaro.org>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
This has Neil's Ack and I could commit it to drm-misc-next, but for
now I'm holding off to see where this series ends up. If the series
ends up looking good we'll have to coordinate landing the various bits
between the drm and the hid trees and the second drm patch in my
series depends on this one.
If my series implodes I'll land this one on its own. In any case, once
this lands somewhere I'll take an AI to cleanup the panels.
(no changes since v1)
drivers/gpu/drm/drm_panel.c | 49 ++++++++++++++++++++++++++++++++-----
include/drm/drm_panel.h | 14 +++++++++++
2 files changed, 57 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
index f634371c717a..4e1c4e42575b 100644
--- a/drivers/gpu/drm/drm_panel.c
+++ b/drivers/gpu/drm/drm_panel.c
@@ -105,11 +105,22 @@ EXPORT_SYMBOL(drm_panel_remove);
*/
int drm_panel_prepare(struct drm_panel *panel)
{
+ int ret;
+
if (!panel)
return -EINVAL;
- if (panel->funcs && panel->funcs->prepare)
- return panel->funcs->prepare(panel);
+ if (panel->prepared) {
+ dev_warn(panel->dev, "Skipping prepare of already prepared panel\n");
+ return 0;
+ }
+
+ if (panel->funcs && panel->funcs->prepare) {
+ ret = panel->funcs->prepare(panel);
+ if (ret < 0)
+ return ret;
+ }
+ panel->prepared = true;
return 0;
}
@@ -128,11 +139,22 @@ EXPORT_SYMBOL(drm_panel_prepare);
*/
int drm_panel_unprepare(struct drm_panel *panel)
{
+ int ret;
+
if (!panel)
return -EINVAL;
- if (panel->funcs && panel->funcs->unprepare)
- return panel->funcs->unprepare(panel);
+ if (!panel->prepared) {
+ dev_warn(panel->dev, "Skipping unprepare of already unprepared panel\n");
+ return 0;
+ }
+
+ if (panel->funcs && panel->funcs->unprepare) {
+ ret = panel->funcs->unprepare(panel);
+ if (ret < 0)
+ return ret;
+ }
+ panel->prepared = false;
return 0;
}
@@ -155,11 +177,17 @@ int drm_panel_enable(struct drm_panel *panel)
if (!panel)
return -EINVAL;
+ if (panel->enabled) {
+ dev_warn(panel->dev, "Skipping enable of already enabled panel\n");
+ return 0;
+ }
+
if (panel->funcs && panel->funcs->enable) {
ret = panel->funcs->enable(panel);
if (ret < 0)
return ret;
}
+ panel->enabled = true;
ret = backlight_enable(panel->backlight);
if (ret < 0)
@@ -187,13 +215,22 @@ int drm_panel_disable(struct drm_panel *panel)
if (!panel)
return -EINVAL;
+ if (!panel->enabled) {
+ dev_warn(panel->dev, "Skipping disable of already disabled panel\n");
+ return 0;
+ }
+
ret = backlight_disable(panel->backlight);
if (ret < 0)
DRM_DEV_INFO(panel->dev, "failed to disable backlight: %d\n",
ret);
- if (panel->funcs && panel->funcs->disable)
- return panel->funcs->disable(panel);
+ if (panel->funcs && panel->funcs->disable) {
+ ret = panel->funcs->disable(panel);
+ if (ret < 0)
+ return ret;
+ }
+ panel->enabled = false;
return 0;
}
diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
index 432fab2347eb..c6cf75909389 100644
--- a/include/drm/drm_panel.h
+++ b/include/drm/drm_panel.h
@@ -198,6 +198,20 @@ struct drm_panel {
* the panel is powered up.
*/
bool prepare_prev_first;
+
+ /**
+ * @prepared:
+ *
+ * If true then the panel has been prepared.
+ */
+ bool prepared;
+
+ /**
+ * @enabled:
+ *
+ * If true then the panel has been enabled.
+ */
+ bool enabled;
};
void drm_panel_init(struct drm_panel *panel, struct device *dev,
--
2.41.0.162.gfafddb0af9-goog
^ permalink raw reply related
* [PATCH v2 01/10] dt-bindings: HID: i2c-hid: Add "panel" property to i2c-hid backed touchscreens
From: Douglas Anderson @ 2023-06-07 21:49 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Bjorn Andersson, Konrad Dybcio,
Rob Herring, Frank Rowand, Krzysztof Kozlowski, Conor Dooley,
Neil Armstrong, Sam Ravnborg, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann
Cc: dri-devel, Dmitry Torokhov, linux-input, Daniel Vetter,
linux-kernel, hsinyi, cros-qcom-dts-watchers, devicetree,
yangcong5, linux-arm-msm, Chris Morgan, Douglas Anderson
In-Reply-To: <20230607215224.2067679-1-dianders@chromium.org>
As talked about in the patch ("drm/panel: Add a way for other devices
to follow panel state"), touchscreens that are connected to panels are
generally expected to be power sequenced together with the panel
they're attached to. Today, nothing provides information allowing you
to find out that a touchscreen is connected to a panel. Let's add a
phandle for this.
The proerty is added to the generic touchscreen bindings and then
enabled in the bindings for the i2c-hid backed devices. This can and
should be added for other touchscreens in the future, but for now
let's start small.
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
Changes in v2:
- Move the description to the generic touchscreen.yaml.
- Update the desc to make it clearer it's only for integrated devices.
Documentation/devicetree/bindings/input/elan,ekth6915.yaml | 5 +++++
.../devicetree/bindings/input/goodix,gt7375p.yaml | 5 +++++
Documentation/devicetree/bindings/input/hid-over-i2c.yaml | 2 ++
.../devicetree/bindings/input/touchscreen/touchscreen.yaml | 7 +++++++
4 files changed, 19 insertions(+)
diff --git a/Documentation/devicetree/bindings/input/elan,ekth6915.yaml b/Documentation/devicetree/bindings/input/elan,ekth6915.yaml
index 05e6f2df604c..3e2d216c6432 100644
--- a/Documentation/devicetree/bindings/input/elan,ekth6915.yaml
+++ b/Documentation/devicetree/bindings/input/elan,ekth6915.yaml
@@ -13,6 +13,9 @@ description:
Supports the Elan eKTH6915 touchscreen controller.
This touchscreen controller uses the i2c-hid protocol with a reset GPIO.
+allOf:
+ - $ref: /schemas/input/touchscreen/touchscreen.yaml#
+
properties:
compatible:
items:
@@ -24,6 +27,8 @@ properties:
interrupts:
maxItems: 1
+ panel: true
+
reset-gpios:
description: Reset GPIO; not all touchscreens using eKTH6915 hook this up.
diff --git a/Documentation/devicetree/bindings/input/goodix,gt7375p.yaml b/Documentation/devicetree/bindings/input/goodix,gt7375p.yaml
index ce18d7dadae2..72212382d702 100644
--- a/Documentation/devicetree/bindings/input/goodix,gt7375p.yaml
+++ b/Documentation/devicetree/bindings/input/goodix,gt7375p.yaml
@@ -14,6 +14,9 @@ description:
This touchscreen uses the i2c-hid protocol but has some non-standard
power sequencing required.
+allOf:
+ - $ref: /schemas/input/touchscreen/touchscreen.yaml#
+
properties:
compatible:
oneOf:
@@ -30,6 +33,8 @@ properties:
interrupts:
maxItems: 1
+ panel: true
+
reset-gpios:
true
diff --git a/Documentation/devicetree/bindings/input/hid-over-i2c.yaml b/Documentation/devicetree/bindings/input/hid-over-i2c.yaml
index 7156b08f7645..138caad96a29 100644
--- a/Documentation/devicetree/bindings/input/hid-over-i2c.yaml
+++ b/Documentation/devicetree/bindings/input/hid-over-i2c.yaml
@@ -44,6 +44,8 @@ properties:
description: HID descriptor address
$ref: /schemas/types.yaml#/definitions/uint32
+ panel: true
+
post-power-on-delay-ms:
description: Time required by the device after enabling its regulators
or powering it on, before it is ready for communication.
diff --git a/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml b/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml
index 895592da9626..431c13335c40 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml
+++ b/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml
@@ -10,6 +10,13 @@ maintainers:
- Dmitry Torokhov <dmitry.torokhov@gmail.com>
properties:
+ panel:
+ description: If this touchscreen is integrally connected to a panel, this
+ is a reference to that panel. The presence of this reference indicates
+ that the touchscreen should be power sequenced together with the panel
+ and that they may share power and/or reset signals.
+ $ref: /schemas/types.yaml#/definitions/phandle
+
touchscreen-min-x:
description: minimum x coordinate reported
$ref: /schemas/types.yaml#/definitions/uint32
--
2.41.0.162.gfafddb0af9-goog
^ permalink raw reply related
* [PATCH v2 00/10] drm/panel and i2c-hid: Allow panels and touchscreens to power sequence together
From: Douglas Anderson @ 2023-06-07 21:49 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Bjorn Andersson, Konrad Dybcio,
Rob Herring, Frank Rowand, Krzysztof Kozlowski, Conor Dooley,
Neil Armstrong, Sam Ravnborg, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann
Cc: dri-devel, Dmitry Torokhov, linux-input, Daniel Vetter,
linux-kernel, hsinyi, cros-qcom-dts-watchers, devicetree,
yangcong5, linux-arm-msm, Chris Morgan, Douglas Anderson
The big motivation for this patch series is mostly described in the patch
("drm/panel: Add a way for other devices to follow panel state"), but to
quickly summarize here: for touchscreens that are connected to a panel we
need the ability to power sequence the two device together. This is not a
new need, but so far we've managed to get by through a combination of
inefficiency, added costs, or perhaps just a little bit of brokenness.
It's time to do better. This patch series allows us to do better.
Assuming that people think this patch series looks OK, we'll have to
figure out the right way to land it. The panel patches and i2c-hid
patches will go through very different trees and so either we'll need
an Ack from one side or the other or someone to create a tag for the
other tree to pull in. This will _probably_ require the true drm-misc
maintainers to get involved, not a lowly committer. ;-)
Version 2 of this patch series doesn't change too much. At a high level:
* I added all the forgotten "static" to functions.
* I've hopefully made the bindings better.
* I've integrated into fw_devlink.
* I cleaned up a few descriptions / comments.
This still needs someone to say that the idea looks OK or to suggest
an alternative that solves the problems. ;-)
Changes in v2:
- Move the description to the generic touchscreen.yaml.
- Update the desc to make it clearer it's only for integrated devices.
- Add even more text to the commit message.
- A few comment cleanups.
- ("Add a devlink for panel followers") new for v2.
- i2c_hid_core_initial_power_up() is now static.
- i2c_hid_core_panel_prepared() and ..._unpreparing() are now static.
- ihid_core_panel_prepare_work() is now static.
- Improve documentation for smp_wmb().
Douglas Anderson (10):
dt-bindings: HID: i2c-hid: Add "panel" property to i2c-hid backed
touchscreens
drm/panel: Check for already prepared/enabled in drm_panel
drm/panel: Add a way for other devices to follow panel state
of: property: fw_devlink: Add a devlink for panel followers
HID: i2c-hid: Switch to SYSTEM_SLEEP_PM_OPS()
HID: i2c-hid: Rearrange probe() to power things up later
HID: i2c-hid: Make suspend and resume into helper functions
HID: i2c-hid: Support being a panel follower
HID: i2c-hid: Do panel follower work on the system_wq
arm64: dts: qcom: sc7180: Link trogdor touchscreens to the panels
.../bindings/input/elan,ekth6915.yaml | 5 +
.../bindings/input/goodix,gt7375p.yaml | 5 +
.../bindings/input/hid-over-i2c.yaml | 2 +
.../input/touchscreen/touchscreen.yaml | 7 +
.../boot/dts/qcom/sc7180-trogdor-coachz.dtsi | 1 +
.../dts/qcom/sc7180-trogdor-homestar.dtsi | 1 +
.../boot/dts/qcom/sc7180-trogdor-lazor.dtsi | 1 +
.../boot/dts/qcom/sc7180-trogdor-pompom.dtsi | 1 +
.../qcom/sc7180-trogdor-quackingstick.dtsi | 1 +
.../dts/qcom/sc7180-trogdor-wormdingler.dtsi | 1 +
drivers/gpu/drm/drm_panel.c | 196 +++++++++-
drivers/hid/i2c-hid/i2c-hid-core.c | 338 +++++++++++++-----
drivers/of/property.c | 2 +
include/drm/drm_panel.h | 89 +++++
14 files changed, 555 insertions(+), 95 deletions(-)
--
2.41.0.162.gfafddb0af9-goog
^ permalink raw reply
* [PATCH v2] Input: synaptics-rmi4 - retry reading SMBus version on resume
From: Jeffery Miller @ 2023-06-07 21:41 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Jonathan Denose, loic.poulain, benjamin.tissoires, Andrew Duggan,
Andrew Duggan, Lyude Paul, jdenose, Jeffery Miller,
Benjamin Mugnier, Jonathan Cameron, Luca Ceresoli,
Uwe Kleine-König, linux-input, linux-kernel
On resume there can be a period of time after the
preceding serio_resume -> psmouse_deactivate call
where calls to rmi_smb_get_version fail with
-ENXIO.
The call path in rmi_smb_resume is rmi_smb_resume -> rmi_smb_reset ->
rmi_smb_enable_smbus_mode -> rmi_smb_get_version where
this failure would occur.
Add a 30ms delay and retry in the ENXIO case to ensure the following
rmi_driver_resume calls in rmi_smbus_resume can succeed.
This behavior was seen on a Lenovo T440p machine that required
a delay of approximately 7-12ms.
The 30ms delay was chosen based on [1].
With this patch the trimmed resume logs look similar to:
```
psmouse serio1: PM: calling serio_resume+0x0/0x8c @ 5399, parent: i8042
[5399] libps2:__ps2_command:316: psmouse serio1: f5 [] - 0/00000000 []
psmouse serio1: PM: serio_resume+0x0/0x8c returned 0 after 3259 usecs
...
rmi4_smbus 0-002c: PM: calling rmi_smb_resume ... @ 5454, parent: i2c-0
...
[5454] i2c_i801:i801_check_post:414: i801_smbus 0000:00:1f.3: No response
smbus_result: i2c-0 a=02c f=0000 c=fd BYTE_DATA rd res=-6
rmi4_smbus 0-002c: failed to get SMBus version number!
rmi4_smbus 0-002c: sleeping to retry getting the SMBus version number
...
rmi4_smbus 0-002c: PM: rmi_smb_resume ... returned 0 after 41351 usecs
```
[1]: https://lore.kernel.org/all/BYAPR03MB47572F2C65E52ED673238D41B2439@BYAPR03MB4757.namprd03.prod.outlook.com/
Signed-off-by: Jeffery Miller <jefferymiller@google.com>
---
Early boot dmesg include:
```
rmi4_smbus 0-002c: registering SMbus-connected sensor
rmi4_f01 rmi4-00.fn01: found RMI device, manufacturer: Synaptics, product: TM2722-001, fw id: 0
```
The resume order looks correct. The `psmouse serio1` resume returns
before the rmi_smb_resume is called showing the patch from
https://lore.kernel.org/all/89456fcd-a113-4c82-4b10-a9bcaefac68f@google.com/
is applied and working for that ordering.
I attempted to try to rule out some interaction between the concurrent
input resume calls for other i8042 devices.
Adding a 7ms delay after psmouse_deactivate which is called in the
preceding psmouse serio1 serio_resume function also allows
this version call to succeed.
If the rmi_smb_probe device_disable_async_suspend patch is applied
it will also avoid this issue. However the time between
the psmouse_deactivate call for serio_resume and rmi_smb_resume
was over 60ms on my test machine. This would naturally be long
enough to avoid this particular delay.
Changes in v2:
- Changed to a single retry of 30ms based on previous feedback.
drivers/input/rmi4/rmi_smbus.c | 22 +++++++++++++++++-----
1 file changed, 17 insertions(+), 5 deletions(-)
diff --git a/drivers/input/rmi4/rmi_smbus.c b/drivers/input/rmi4/rmi_smbus.c
index 4bf0e1df6a4a..b6d90c92e8a2 100644
--- a/drivers/input/rmi4/rmi_smbus.c
+++ b/drivers/input/rmi4/rmi_smbus.c
@@ -45,13 +45,25 @@ static int rmi_smb_get_version(struct rmi_smb_xport *rmi_smb)
int retval;
/* Check if for SMBus new version device by reading version byte. */
- retval = i2c_smbus_read_byte_data(client, SMB_PROTOCOL_VERSION_ADDRESS);
- if (retval < 0) {
+ for (int i = 0; i < 2; i++) {
+ if (i > 0) {
+ dev_warn(&client->dev, "sleeping to retry getting the SMBus version number\n");
+ fsleep(30000);
+ }
+ retval = i2c_smbus_read_byte_data(client,
+ SMB_PROTOCOL_VERSION_ADDRESS);
+ if (retval >= 0)
+ return retval + 1;
+
dev_err(&client->dev, "failed to get SMBus version number!\n");
- return retval;
+ /* There can be a delay on resume where the read returns
+ * -ENXIO. Retry to allow additional time for the read
+ * to become responsive.
+ */
+ if (retval != -ENXIO)
+ break;
}
-
- return retval + 1;
+ return retval;
}
/* SMB block write - wrapper over ic2_smb_write_block */
--
2.41.0.162.gfafddb0af9-goog
^ permalink raw reply related
* [PATCH] HID: wacom: Use ktime_t rather than int when dealing with timestamps
From: Jason Gerecke @ 2023-06-07 21:41 UTC (permalink / raw)
To: linux-input, linux-kernel, Benjamin Tissoires, Jiri Kosina
Cc: Ping Cheng, Aaron Armstrong Skomra, Joshua Dickens, Jason Gerecke
Code which interacts with timestamps needs to use the ktime_t type
returned by functions like ktime_get. The int type does not offer
enough space to store these values, and attempting to use it is a
recipe for problems. In this particular case, overflows would occur
when calculating/storing timestamps leading to incorrect values being
reported to userspace. In some cases these bad timestamps cause input
handling in userspace to appear hung.
Link: https://gitlab.freedesktop.org/libinput/libinput/-/issues/901
Fixes: 17d793f3ed53 ("HID: wacom: insert timestamp to packed Bluetooth (BT) events")
Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
---
drivers/hid/wacom_wac.c | 4 ++--
drivers/hid/wacom_wac.h | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index 2ccf83837134..2f16e47e4b69 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -1314,7 +1314,7 @@ static void wacom_intuos_pro2_bt_pen(struct wacom_wac *wacom)
struct input_dev *pen_input = wacom->pen_input;
unsigned char *data = wacom->data;
int number_of_valid_frames = 0;
- int time_interval = 15000000;
+ ktime_t time_interval = 15000000;
ktime_t time_packet_received = ktime_get();
int i;
@@ -1359,7 +1359,7 @@ static void wacom_intuos_pro2_bt_pen(struct wacom_wac *wacom)
bool range = frame[0] & 0x20;
bool invert = frame[0] & 0x10;
int frames_number_reversed = number_of_valid_frames - i - 1;
- int event_timestamp = time_packet_received - frames_number_reversed * time_interval;
+ ktime_t event_timestamp = time_packet_received - frames_number_reversed * time_interval;
if (!valid)
continue;
diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h
index 1a40bb8c5810..ee21bb260f22 100644
--- a/drivers/hid/wacom_wac.h
+++ b/drivers/hid/wacom_wac.h
@@ -324,7 +324,7 @@ struct hid_data {
int ps_connected;
bool pad_input_event_flag;
unsigned short sequence_number;
- int time_delayed;
+ ktime_t time_delayed;
};
struct wacom_remote_data {
--
2.41.0
^ permalink raw reply related
* Re: [git pull] Input updates for v6.4-rc5
From: pr-tracker-bot @ 2023-06-07 20:52 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Linus Torvalds, linux-kernel, linux-input
In-Reply-To: <ZH9zwxXvAd3kacKH@google.com>
The pull request you sent on Tue, 6 Jun 2023 10:58:27 -0700:
> git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git tags/input-for-v6.4-rc5
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/5f63595ebd82f56a2dd36ca013dd7f5ff2e2416a
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html
^ permalink raw reply
* Re: [PATCH] selftests: hid: Add touch tests for Wacom devices
From: Ping Cheng @ 2023-06-07 17:43 UTC (permalink / raw)
To: Joshua Dickens
Cc: linux-input, Jiri Kosina, Benjamin Tissoires,
Aaron Armstrong Skomra, Jason Gerecke
In-Reply-To: <20230511194754.129571-1-Joshua@Joshua-Dickens.com>
Hi maintainers,
Can one of you review the patch and let us know if there is anything
we need to change/update?
Thank you for your effort,
Ping
On Thu, May 11, 2023 at 12:48 PM Joshua Dickens
<joshua@joshua-dickens.com> wrote:
>
> Adding a wacom touch device to use the test_multitouch tests.
> Adding a 2 additional tests.
> - A test to check if a touch event is sent when the contact_id of the event is 0.
> - A test to check if a touch event is not sent when confidence is set to 0.
>
> Signed-off-by: Joshua Dickens <joshua.dickens@wacom.com>
> Reviewed-by: Jason Gerecke <jason.gerecke@wacom.com>
> ---
> .../selftests/hid/tests/test_wacom_generic.py | 84 ++++++++++++++++++-
> 1 file changed, 81 insertions(+), 3 deletions(-)
>
> diff --git a/tools/testing/selftests/hid/tests/test_wacom_generic.py b/tools/testing/selftests/hid/tests/test_wacom_generic.py
> index b1eb2bc787fc..f92fe8e02c1b 100644
> --- a/tools/testing/selftests/hid/tests/test_wacom_generic.py
> +++ b/tools/testing/selftests/hid/tests/test_wacom_generic.py
> @@ -31,6 +31,7 @@ from enum import Enum
> from hidtools.hut import HUT
> from hidtools.hid import HidUnit
> from . import base
> +from . import test_multitouch
> import libevdev
> import pytest
>
> @@ -517,7 +518,7 @@ class BaseTest:
> for usage in get_report_usages(report):
> yield usage
>
> - def assertName(self, uhdev):
> + def assertName(self, uhdev, type):
> """
> Assert that the name is as we expect.
>
> @@ -526,7 +527,7 @@ class BaseTest:
> this assertion from the base class to work properly.
> """
> evdev = uhdev.get_evdev()
> - expected_name = uhdev.name + " Pen"
> + expected_name = uhdev.name + type
> if "wacom" not in expected_name.lower():
> expected_name = "Wacom " + expected_name
> assert evdev.name == expected_name
> @@ -549,6 +550,12 @@ class BaseTest:
> usage_id("Generic Desktop", "Y"): PhysRange(
> PhysRange.CENTIMETER, 5, 150
> ),
> + usage_id("Digitizers", "Width"): PhysRange(
> + PhysRange.CENTIMETER, 5, 150
> + ),
> + usage_id("Digitizers", "Height"): PhysRange(
> + PhysRange.CENTIMETER, 5, 150
> + ),
> usage_id("Digitizers", "X Tilt"): PhysRange(PhysRange.DEGREE, 90, 180),
> usage_id("Digitizers", "Y Tilt"): PhysRange(PhysRange.DEGREE, 90, 180),
> usage_id("Digitizers", "Twist"): PhysRange(PhysRange.DEGREE, 358, 360),
> @@ -603,7 +610,17 @@ class BaseTest:
> pass
>
>
> -class TestOpaqueTablet(BaseTest.TestTablet):
> +class PenTabletTest(BaseTest.TestTablet):
> + def assertName(self, uhdev):
> + super().assertName(uhdev, " Pen")
> +
> +
> +class TouchTabletTest(BaseTest.TestTablet):
> + def assertName(self, uhdev):
> + super().assertName(uhdev, " Finger")
> +
> +
> +class TestOpaqueTablet(PenTabletTest):
> def create_device(self):
> return OpaqueTablet()
>
> @@ -842,3 +859,64 @@ class TestPTHX60_Pen(TestOpaqueCTLTablet):
> libevdev.InputEvent(libevdev.EV_KEY.BTN_0, 0),
> ],
> )
> +
> +
> +class TestDTH2452Tablet(test_multitouch.BaseTest.TestMultitouch, TouchTabletTest):
> + def create_device(self):
> + return test_multitouch.Digitizer(
> + "DTH 2452",
> + rdesc="05 0d 09 04 a1 01 85 0c 95 01 75 08 15 00 26 ff 00 81 03 09 54 81 02 09 22 a1 02 05 0d 95 01 75 01 25 01 09 42 81 02 81 03 09 47 81 02 95 05 81 03 09 51 26 ff 00 75 10 95 01 81 02 35 00 65 11 55 0e 05 01 09 30 26 a0 44 46 96 14 81 42 09 31 26 9a 26 46 95 0b 81 42 05 0d 75 08 95 01 15 00 09 48 26 5f 00 46 7c 14 81 02 09 49 25 35 46 7d 0b 81 02 45 00 65 00 55 00 c0 05 0d 09 22 a1 02 05 0d 95 01 75 01 25 01 09 42 81 02 81 03 09 47 81 02 95 05 81 03 09 51 26 ff 00 75 10 95 01 81 02 35 00 65 11 55 0e 05 01 09 30 26 a0 44 46 96 14 81 42 09 31 26 9a 26 46 95 0b 81 42 05 0d 75 08 95 01 15 00 09 48 26 5f 00 46 7c 14 81 02 09 49 25 35 46 7d 0b 81 02 45 00 65 00 55 00 c0 05 0d 09 22 a1 02 05 0d 95 01 75 01 25 01 09 42 81 02 81 03 09 47 81 02 95 05 81 03 09 51 26 ff 00 75 10 95 01 81 02 35 00 65 11 55 0e 05 01 09 30 26 a0 44 46 96 14 81 42 09 31 26 9a 26 46 95 0b 81 42 05 0d 75 08 95 01 15 00 09 48 26 5f 00 46 7c 14 81 02 09 49 25 35 46 7d 0b 81 02 45 00 65 00 55 00 c0 05 0d 09 22 a1 02 05 0d 95 01 75 01 25 01 09 42 81 02 81 03 09 47 81 02 95 05 81 03 09 51 26 ff 00 75 10 95 01 81 02 35 00 65 11 55 0e 05 01 09 30 26 a0 44 46 96 14 81 42 09 31 26 9a 26 46 95 0b 81 42 05 0d 75 08 95 01 15 00 09 48 26 5f 00 46 7c 14 81 02 09 49 25 35 46 7d 0b 81 02 45 00 65 00 55 00 c0 05 0d 09 22 a1 02 05 0d 95 01 75 01 25 01 09 42 81 02 81 03 09 47 81 02 95 05 81 03 09 51 26 ff 00 75 10 95 01 81 02 35 00 65 11 55 0e 05 01 09 30 26 a0 44 46 96 14 81 42 09 31 26 9a 26 46 95 0b 81 42 05 0d 75 08 95 01 15 00 09 48 26 5f 00 46 7c 14 81 02 09 49 25 35 46 7d 0b 81 02 45 00 65 00 55 00 c0 05 0d 27 ff ff 00 00 75 10 95 01 09 56 81 02 75 08 95 0e 81 03 09 55 26 ff 00 75 08 b1 02 85 0a 06 00 ff 09 c5 96 00 01 b1 02 c0 06 00 ff 09 01 a1 01 09 01 85 13 15 00 26 ff 00 75 08 95 3f 81 02 06 00 ff 09 01 15 00 26 ff 00 75 08 95 3f 91 02 c0",
> + input_info=(0x3, 0x056A, 0x0383),
> + )
> +
> + def test_contact_id_0(self):
> + """
> + Bring a finger in contact with the tablet, then hold it down and remove it.
> +
> + Ensure that even with contact ID = 0 which is usually given as an invalid
> + touch event by most tablets with the exception of a few, that given the
> + confidence bit is set to 1 it should process it as a valid touch to cover
> + the few tablets using contact ID = 0 as a valid touch value.
> + """
> + uhdev = self.uhdev
> + evdev = uhdev.get_evdev()
> +
> + t0 = test_multitouch.Touch(0, 50, 100)
> + r = uhdev.event([t0])
> + events = uhdev.next_sync_events()
> + self.debug_reports(r, uhdev, events)
> +
> + slot = self.get_slot(uhdev, t0, 0)
> +
> + assert libevdev.InputEvent(libevdev.EV_KEY.BTN_TOUCH, 1) in events
> + assert evdev.slots[slot][libevdev.EV_ABS.ABS_MT_TRACKING_ID] == 0
> + assert evdev.slots[slot][libevdev.EV_ABS.ABS_MT_POSITION_X] == 50
> + assert evdev.slots[slot][libevdev.EV_ABS.ABS_MT_POSITION_Y] == 100
> +
> + t0.tipswitch = False
> + if uhdev.quirks is None or "VALID_IS_INRANGE" not in uhdev.quirks:
> + t0.inrange = False
> + r = uhdev.event([t0])
> + events = uhdev.next_sync_events()
> + self.debug_reports(r, uhdev, events)
> + assert libevdev.InputEvent(libevdev.EV_KEY.BTN_TOUCH, 0) in events
> + assert evdev.slots[slot][libevdev.EV_ABS.ABS_MT_TRACKING_ID] == -1
> +
> + def test_confidence_false(self):
> + """
> + Bring a finger in contact with the tablet with confidence set to false.
> +
> + Ensure that the confidence bit being set to false should not result in a touch event.
> + """
> + uhdev = self.uhdev
> + evdev = uhdev.get_evdev()
> +
> + t0 = test_multitouch.Touch(1, 50, 100)
> + t0.confidence = False
> + r = uhdev.event([t0])
> + events = uhdev.next_sync_events()
> + self.debug_reports(r, uhdev, events)
> +
> + slot = self.get_slot(uhdev, t0, 0)
> +
> + assert not events
> \ No newline at end of file
> --
> 2.40.0
>
^ permalink raw reply
* [PATCH] Input: i8042 - add quirk for TUXEDO Gemini 17 Gen1/Clevo PD70PN
From: Werner Sembach @ 2023-06-07 17:33 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Werner Sembach, linux-input, linux-kernel
The touchpad of this device is both connected via PS/2 and i2c. This causes
strange behavior when both driver fight for control. The easy fix is to
prevent the PS/2 driver from accessing the mouse port as the full feature
set of the touchpad is only supported in the i2c interface anyway.
The strange behavior in this case is, that when an external screen is
connected and the notebook is closed, the pointer on the external screen is
moving to the lower right corner. When the notebook is opened again, this
movement stops, but the touchpad clicks are unresponsive afterwards until
reboot.
Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
Cc: stable@vger.kernel.org
---
drivers/input/serio/i8042-acpipnpio.h | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/input/serio/i8042-acpipnpio.h b/drivers/input/serio/i8042-acpipnpio.h
index 028e45bd050bf..1724d6cb8649d 100644
--- a/drivers/input/serio/i8042-acpipnpio.h
+++ b/drivers/input/serio/i8042-acpipnpio.h
@@ -1281,6 +1281,13 @@ static const struct dmi_system_id i8042_dmi_quirk_table[] __initconst = {
.driver_data = (void *)(SERIO_QUIRK_NOMUX | SERIO_QUIRK_RESET_ALWAYS |
SERIO_QUIRK_NOLOOP | SERIO_QUIRK_NOPNP)
},
+ /* See comment on TUXEDO InfinityBook S17 Gen6 / Clevo NS70MU above */
+ {
+ .matches = {
+ DMI_MATCH(DMI_BOARD_NAME, "PD5x_7xPNP_PNR_PNN_PNT"),
+ },
+ .driver_data = (void *)(SERIO_QUIRK_NOAUX)
+ },
{
.matches = {
DMI_MATCH(DMI_BOARD_NAME, "X170SM"),
--
2.34.1
^ permalink raw reply related
* Re: [PATCH RESEND v4 0/2] Fix Goodix touchscreen power leakage for MT8186 boards
From: Doug Anderson @ 2023-06-07 15:55 UTC (permalink / raw)
To: Fei Shao, Jiri Kosina
Cc: Rob Herring, Matthias Brugger, Jeff LaBundy, Benjamin Tissoires,
Conor Dooley, Dmitry Torokhov, Krzysztof Kozlowski,
Matthias Kaehlcke, Stephen Kitt, devicetree, linux-input,
linux-kernel
In-Reply-To: <20230524114233.RESEND.v4.2.I424e840ae6de3cdbd67394cf4efd24534f6434ba@changeid>
Hi Jiri
On Tue, May 23, 2023 at 8:45 PM Fei Shao <fshao@chromium.org> wrote:
>
> These changes are based on the series in [1], which modified the
> i2c-hid-of-goodix driver and removed the workaround for a power leakage
> issue, so the issue revisits on Mediatek MT8186 boards (Steelix).
>
> The root cause is that the touchscreen can be powered in different ways
> depending on the hardware designs, and it's not as easy to come up with
> a solution that is both simple and elegant for all the known designs.
>
> To address the issue, I ended up adding a new boolean property for the
> driver so that we can control the power up/down sequence depending on
> that.
>
> Adding a new property might not be the cleanest approach for this, but
> at least the intention would be easy enough to understand, and it
> introduces relatively small change to the code and fully preserves the
> original control flow.
>
> [1] https://lore.kernel.org/all/20230207024816.525938-1-dianders@chromium.org/
>
> Changes in v4:
> - Rebase on top of next-20230523
> - Collect the review tags
> - Minor coding style improvement
>
> Changes in v3:
> - In power-down, only skip the GPIO but not the regulator calls if the
> flag is set
>
> Changes in v2:
> - Use a more accurate property name and with "goodix," prefix
> - Drop the change to regulator_enable logic during power-up
>
> Fei Shao (2):
> dt-bindings: input: goodix: Add "goodix,no-reset-during-suspend"
> property
> HID: i2c-hid: goodix: Add support for "goodix,no-reset-during-suspend"
> property
>
> .../bindings/input/goodix,gt7375p.yaml | 9 +++++++++
> drivers/hid/i2c-hid/i2c-hid-of-goodix.c | 16 +++++++++++++++-
> 2 files changed, 24 insertions(+), 1 deletion(-)
Just confirming that you're good to land these two patches as
discussed previously [1]. Thanks! :-)
[1] https://lore.kernel.org/r/nycvar.YFH.7.76.2305231510270.29760@cbobk.fhfr.pm
^ permalink raw reply
* [PATCH v3] selftests/input: Introduce basic tests for evdev ioctls
From: Enric Balletbo i Serra @ 2023-06-07 15:32 UTC (permalink / raw)
To: linux-kselftest, linux-kernel
Cc: Shuah Khan, Dmitry Torokhov, Javier Martinez Canillas,
Dana Elfassy, linux-input, phuttere, Benjamin Tissoires,
Enric Balletbo i Serra, Muhammad Usama Anjum
This provides a basic infrastructure for the creation of tests for the evdev
interface. Most of this code is adapted from the libevdev wrapper library. While
most of evdev ioctls are covered and tested using libevdev tests there are some
evdev ioctls that aren't because are not supported (and will not be supported)
by libevdev [1]. So, adding, at least those tests, would make sense.
The test creates an uinput device (and an evdev device) so you can
call the wanted ioctl from userspace. So, to run those tests you need
to have support for uinput and evdev as well.
[1] For example, libevdev doesn't support setting EV_REP because it's inherently
racy - one libevdev context to set those values via the ioctl would cause all
other libevdev contexts on the same device to be out of sync. Since we do not
get notifications when the values changed, libevdev's buffered values for EV_REP
will remain whatever they were initially.
Signed-off-by: Enric Balletbo i Serra <eballetbo@kernel.org>
Acked-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
---
Test output:
TAP version 13
1..3
# Starting 3 tests from 1 test cases.
# RUN global.eviocgname_get_device_name ...
# OK global.eviocgname_get_device_name
ok 1 global.eviocgname_get_device_name
# RUN global.eviocgrep_get_repeat_settings ...
# OK global.eviocgrep_get_repeat_settings
ok 2 global.eviocgrep_get_repeat_settings
# RUN global.eviocsrep_set_repeat_settings ...
# OK global.eviocsrep_set_repeat_settings
ok 3 global.eviocsrep_set_repeat_settings
# PASSED: 3 / 3 tests passed.
# Totals: pass:3 fail:0 xfail:0 xpass:0 skip:0 error:0
Changes since v1:
- As UI_GET_SYSNAME has been around 3.15 remove the fallback code that
pre-dates that ioctl.
- Fix a bug in the va-args handling in selftest_uinput_create_device
calls.
- Fix typo s/an/and
- Implement a test case for EVIOCSREP.
Changes since v2:
- Fix a buf left in the va-args handling in
selftest_uinput_create_device calls.
- Replace fetch_syspath_and_devnode() function to open_devnode()
function that return the openend file descriptor, so we can reduce the
code.
tools/testing/selftests/Makefile | 1 +
tools/testing/selftests/input/.gitignore | 2 +
tools/testing/selftests/input/Makefile | 5 +
tools/testing/selftests/input/config | 3 +
tools/testing/selftests/input/evioc-test.c | 237 +++++++++++++++++++++
5 files changed, 248 insertions(+)
create mode 100644 tools/testing/selftests/input/.gitignore
create mode 100644 tools/testing/selftests/input/Makefile
create mode 100644 tools/testing/selftests/input/config
create mode 100644 tools/testing/selftests/input/evioc-test.c
diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index 90a62cf75008..29fc77168aa7 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -28,6 +28,7 @@ TARGETS += futex
TARGETS += gpio
TARGETS += hid
TARGETS += intel_pstate
+TARGETS += input
TARGETS += iommu
TARGETS += ipc
TARGETS += ir
diff --git a/tools/testing/selftests/input/.gitignore b/tools/testing/selftests/input/.gitignore
new file mode 100644
index 000000000000..37f5dff3255b
--- /dev/null
+++ b/tools/testing/selftests/input/.gitignore
@@ -0,0 +1,2 @@
+# SPDX-License-Identifier: GPL-2.0
+evioc-test
diff --git a/tools/testing/selftests/input/Makefile b/tools/testing/selftests/input/Makefile
new file mode 100644
index 000000000000..031729be0628
--- /dev/null
+++ b/tools/testing/selftests/input/Makefile
@@ -0,0 +1,5 @@
+# SPDX-License-Identifier: GPL-2.0
+CFLAGS += -D_GNU_SOURCE -std=gnu99 -Wl,-no-as-needed -Wall $(KHDR_INCLUDES)
+
+TEST_GEN_PROGS := evioc-test
+include ../lib.mk
diff --git a/tools/testing/selftests/input/config b/tools/testing/selftests/input/config
new file mode 100644
index 000000000000..b7512f3e6d8d
--- /dev/null
+++ b/tools/testing/selftests/input/config
@@ -0,0 +1,3 @@
+CONFIG_INPUT=y
+CONFIG_INPUT_EVDEV=y
+CONFIG_INPUT_UINPUT=m
diff --git a/tools/testing/selftests/input/evioc-test.c b/tools/testing/selftests/input/evioc-test.c
new file mode 100644
index 000000000000..ad7b93fe39cf
--- /dev/null
+++ b/tools/testing/selftests/input/evioc-test.c
@@ -0,0 +1,237 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2023 Red Hat, Inc.
+ *
+ * Part of the code in this file is inspired and copied from the libevdev wrapper library
+ * for evdev devices written by Peter Hutterer.
+ */
+
+#include <dirent.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <linux/uinput.h>
+#include <poll.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/stat.h>
+#include <time.h>
+#include <unistd.h>
+
+#include "../kselftest_harness.h"
+
+#define TEST_DEVICE_NAME "selftest input device"
+
+struct selftest_uinput {
+ int uinput_fd; /** file descriptor to uinput */
+ int evdev_fd; /** file descriptor to evdev */
+};
+
+static int is_event_device(const struct dirent *dent)
+{
+ return strncmp("event", dent->d_name, 5) == 0;
+}
+
+static int open_devnode(struct selftest_uinput *uidev)
+{
+#define SYS_INPUT_DIR "/sys/devices/virtual/input/"
+ char buf[sizeof(SYS_INPUT_DIR) + 64] = SYS_INPUT_DIR;
+ struct dirent **namelist;
+ char *devnode = NULL;
+ int ndev, i;
+ int rc;
+
+ rc = ioctl(uidev->uinput_fd,
+ UI_GET_SYSNAME(sizeof(buf) - strlen(SYS_INPUT_DIR)),
+ &buf[strlen(SYS_INPUT_DIR)]);
+ if (rc == -1) {
+ fprintf(stderr, "cannot get the sysfs name of the uinput device (%d)\n", rc);
+ return rc;
+ }
+
+ ndev = scandir(buf, &namelist, is_event_device, alphasort);
+ if (ndev <= 0)
+ return -1;
+
+ /* ndev should only ever be 1 */
+
+ for (i = 0; i < ndev; i++) {
+ if (!devnode && asprintf(&devnode, "/dev/input/%s",
+ namelist[i]->d_name) == -1)
+ devnode = NULL;
+ free(namelist[i]);
+ }
+
+ free(namelist);
+
+ return open(devnode, O_RDONLY);
+#undef SYS_INPUT_DIR
+}
+
+static void selftest_uinput_destroy(struct selftest_uinput *uidev)
+{
+ if (!uidev)
+ return;
+
+ if (uidev->uinput_fd >= 0)
+ ioctl(uidev->uinput_fd, UI_DEV_DESTROY, NULL);
+
+ close(uidev->evdev_fd);
+ close(uidev->uinput_fd);
+
+ free(uidev);
+}
+
+static int selftest_uinput_create_device(struct selftest_uinput **uidev, ...)
+{
+ struct selftest_uinput *new_device;
+ struct uinput_setup setup;
+ va_list args;
+ int rc, fd;
+ int type;
+
+ new_device = calloc(1, sizeof(struct selftest_uinput));
+ if (!new_device)
+ return -ENOMEM;
+
+ memset(&setup, 0, sizeof(setup));
+ strncpy(setup.name, TEST_DEVICE_NAME, UINPUT_MAX_NAME_SIZE - 1);
+ setup.id.vendor = 0x1234; /* sample vendor */
+ setup.id.product = 0x5678; /* sample product */
+ setup.id.bustype = BUS_USB;
+
+ fd = open("/dev/uinput", O_WRONLY | O_NONBLOCK);
+ if (fd < 0) {
+ fprintf(stderr, "cannot open uinput (%d): %m\n", errno);
+ goto error;
+ }
+
+ va_start(args, uidev);
+ rc = 0;
+ do {
+ type = va_arg(args, int);
+ if (type == -1)
+ break;
+ rc = ioctl(fd, UI_SET_EVBIT, type);
+ } while (rc == 0);
+ va_end(args);
+
+ rc = ioctl(fd, UI_DEV_SETUP, &setup);
+ if (rc == -1)
+ goto error;
+
+ rc = ioctl(fd, UI_DEV_CREATE, NULL);
+ if (rc == -1)
+ goto error;
+
+ new_device->uinput_fd = fd;
+
+ fd = open_devnode(new_device);
+ if (fd < 0) {
+ fprintf(stderr, "cannot open event device node (%d): %m\n", errno);
+ goto error;
+ }
+
+ new_device->evdev_fd = fd;
+ *uidev = new_device;
+
+ return 0;
+
+error:
+ rc = -errno;
+ selftest_uinput_destroy(new_device);
+ return rc;
+}
+
+TEST(eviocgname_get_device_name)
+{
+ struct selftest_uinput *uidev;
+ char buf[256];
+ int rc;
+
+ rc = selftest_uinput_create_device(&uidev);
+ ASSERT_EQ(0, rc);
+ ASSERT_NE(NULL, uidev);
+
+ memset(buf, 0, sizeof(buf));
+ /* ioctl to get the name */
+ rc = ioctl(uidev->evdev_fd, EVIOCGNAME(sizeof(buf) - 1), buf);
+ ASSERT_GE(rc, 0);
+ ASSERT_STREQ(TEST_DEVICE_NAME, buf);
+
+ selftest_uinput_destroy(uidev);
+}
+
+TEST(eviocgrep_get_repeat_settings)
+{
+ struct selftest_uinput *uidev;
+ int rep_values[2];
+ int rc;
+
+ memset(rep_values, 0, sizeof(rep_values));
+
+ rc = selftest_uinput_create_device(&uidev, -1);
+ ASSERT_EQ(0, rc);
+ ASSERT_NE(NULL, uidev);
+
+ /* ioctl to get the repeat rates values */
+ rc = ioctl(uidev->evdev_fd, EVIOCGREP, rep_values);
+ /* should fail because EV_REP is not set */
+ ASSERT_EQ(-1, rc);
+
+ selftest_uinput_destroy(uidev);
+
+ rc = selftest_uinput_create_device(&uidev, EV_REP, -1);
+ ASSERT_EQ(0, rc);
+ ASSERT_NE(NULL, uidev);
+
+ /* ioctl to get the repeat rates values */
+ rc = ioctl(uidev->evdev_fd, EVIOCGREP, rep_values);
+ ASSERT_EQ(0, rc);
+ /* should get the default delay and period values set by the kernel */
+ ASSERT_EQ(rep_values[0], 250);
+ ASSERT_EQ(rep_values[1], 33);
+
+ selftest_uinput_destroy(uidev);
+}
+
+TEST(eviocsrep_set_repeat_settings)
+{
+ struct selftest_uinput *uidev;
+ int rep_values[2];
+ int rc;
+
+ memset(rep_values, 0, sizeof(rep_values));
+
+ rc = selftest_uinput_create_device(&uidev, -1);
+ ASSERT_EQ(0, rc);
+ ASSERT_NE(NULL, uidev);
+
+ /* ioctl to set the repeat rates values */
+ rc = ioctl(uidev->evdev_fd, EVIOCSREP, rep_values);
+ /* should fail because EV_REP is not set */
+ ASSERT_EQ(-1, rc);
+
+ selftest_uinput_destroy(uidev);
+
+ rc = selftest_uinput_create_device(&uidev, EV_REP, -1);
+ ASSERT_EQ(0, rc);
+ ASSERT_NE(NULL, uidev);
+
+ /* ioctl to set the repeat rates values */
+ rep_values[0] = 500;
+ rep_values[1] = 200;
+ rc = ioctl(uidev->evdev_fd, EVIOCSREP, rep_values);
+ ASSERT_EQ(0, rc);
+
+ /* ioctl to get the repeat rates values */
+ rc = ioctl(uidev->evdev_fd, EVIOCGREP, rep_values);
+ ASSERT_EQ(0, rc);
+ /* should get the delay and period values set previously */
+ ASSERT_EQ(rep_values[0], 500);
+ ASSERT_EQ(rep_values[1], 200);
+
+ selftest_uinput_destroy(uidev);
+}
+
+TEST_HARNESS_MAIN
--
2.40.1
^ permalink raw reply related
* Re: [PATCH v3 1/2] dt-bindings: HID: i2c-hid: ilitek: Introduce bindings for Ilitek ili9882t
From: Krzysztof Kozlowski @ 2023-06-07 13:59 UTC (permalink / raw)
To: Cong Yang, robh+dt, krzysztof.kozlowski+dt, conor+dt,
dmitry.torokhov, jikos, benjamin.tissoires, dianders, hsinyi
Cc: linux-input, devicetree, linux-kernel
In-Reply-To: <20230607133458.4075667-2-yangcong5@huaqin.corp-partner.google.com>
On 07/06/2023 15:34, Cong Yang wrote:
> The ili9882t touch screen chip same as Elan eKTH6915 controller
> has a reset gpio. The difference is that ili9882t needs to use
> vccio-supply instead of vcc33-supply. Doug's series[1] allows panels
> and touchscreens to power on/off together, let's add a phandle for this.
>
> [1]: https: //lore.kernel.org/all/20230523193017.4109557-1-dianders@chromium.org/
>
> Signed-off-by: Cong Yang <yangcong5@huaqin.corp-partner.google.com>
> ---
> .../bindings/input/ilitek,ili9882t.yaml | 67 +++++++++++++++++++
> 1 file changed, 67 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/input/ilitek,ili9882t.yaml
>
> diff --git a/Documentation/devicetree/bindings/input/ilitek,ili9882t.yaml b/Documentation/devicetree/bindings/input/ilitek,ili9882t.yaml
> new file mode 100644
> index 000000000000..f69fa3aaf4c5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/ilitek,ili9882t.yaml
> @@ -0,0 +1,67 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/input/ilitek,ili9882t.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Ilitek ili9882t touchscreen controller
> +
> +maintainers:
> + - Dmitry Torokhov <dmitry.torokhov@gmail.com>
> +
> +description:
> + Supports the Ilitek ili9882t touchscreen controller.
> + This touchscreen controller uses the i2c-hid protocol with a reset GPIO.
> +
> +properties:
> + compatible:
> + items:
Drop items, just const.
> + - const: ilitek,ili9882t
> +
> + reg:
> + const: 0x41
> +
> + interrupts:
> + maxItems: 1
> +
> + panel:
> + description: If this is a touchscreen, the panel it's connected to. This
> + indicates that the panel and touchscreen are expected to be power
> + sequenced together.
> + $ref: /schemas/types.yaml#/definitions/phandle
"If this is a touchscreen"? Doesn't you description say this is
touchscreen controller? Then how this could be also not a touchscreen?
> +
> + reset-gpios:
> + description: Reset GPIO.
missing maxItems
Best regards,
Krzysztof
^ permalink raw reply
* [PATCH v3 2/2] HID: i2c-hid: elan: Add ili9882t timing
From: Cong Yang @ 2023-06-07 13:34 UTC (permalink / raw)
To: robh+dt, krzysztof.kozlowski+dt, conor+dt, dmitry.torokhov, jikos,
benjamin.tissoires, dianders, hsinyi
Cc: linux-input, devicetree, linux-kernel, Cong Yang
In-Reply-To: <20230607133458.4075667-1-yangcong5@huaqin.corp-partner.google.com>
The ili9882t is a TDDI IC (Touch with Display Driver). The
datasheet specifies there should be 60ms between touch SDA
sleep and panel RESX. Doug's series[1] allows panels and
touchscreens to power on/off together, so we can add the 65 ms
delay in i2c_hid_core_suspend before panel_unprepare.
[1]: https: //lore.kernel.org/all/20230523193017.4109557-1-dianders@chromium.org/
Signed-off-by: Cong Yang <yangcong5@huaqin.corp-partner.google.com>
---
drivers/hid/i2c-hid/i2c-hid-of-elan.c | 20 ++++++++++++++++----
1 file changed, 16 insertions(+), 4 deletions(-)
diff --git a/drivers/hid/i2c-hid/i2c-hid-of-elan.c b/drivers/hid/i2c-hid/i2c-hid-of-elan.c
index 76ddc8be1cbb..411d7ea2725d 100644
--- a/drivers/hid/i2c-hid/i2c-hid-of-elan.c
+++ b/drivers/hid/i2c-hid/i2c-hid-of-elan.c
@@ -18,7 +18,8 @@
#include "i2c-hid.h"
struct elan_i2c_hid_chip_data {
- unsigned int post_gpio_reset_delay_ms;
+ unsigned int post_gpio_reset_on_delay_ms;
+ unsigned int post_gpio_reset_off_delay_ms;
unsigned int post_power_delay_ms;
u16 hid_descriptor_address;
};
@@ -52,8 +53,8 @@ static int elan_i2c_hid_power_up(struct i2chid_ops *ops)
msleep(ihid_elan->chip_data->post_power_delay_ms);
gpiod_set_value_cansleep(ihid_elan->reset_gpio, 0);
- if (ihid_elan->chip_data->post_gpio_reset_delay_ms)
- msleep(ihid_elan->chip_data->post_gpio_reset_delay_ms);
+ if (ihid_elan->chip_data->post_gpio_reset_on_delay_ms)
+ msleep(ihid_elan->chip_data->post_gpio_reset_on_delay_ms);
return 0;
}
@@ -64,6 +65,9 @@ static void elan_i2c_hid_power_down(struct i2chid_ops *ops)
container_of(ops, struct i2c_hid_of_elan, ops);
gpiod_set_value_cansleep(ihid_elan->reset_gpio, 1);
+ if (ihid_elan->chip_data->post_gpio_reset_off_delay_ms)
+ msleep(ihid_elan->chip_data->post_gpio_reset_off_delay_ms);
+
regulator_disable(ihid_elan->vccio);
regulator_disable(ihid_elan->vcc33);
}
@@ -101,12 +105,20 @@ static int i2c_hid_of_elan_probe(struct i2c_client *client)
static const struct elan_i2c_hid_chip_data elan_ekth6915_chip_data = {
.post_power_delay_ms = 1,
- .post_gpio_reset_delay_ms = 300,
+ .post_gpio_reset_on_delay_ms = 300,
+ .hid_descriptor_address = 0x0001,
+};
+
+static const struct elan_i2c_hid_chip_data ilitek_ili9882t_chip_data = {
+ .post_power_delay_ms = 1,
+ .post_gpio_reset_on_delay_ms = 200,
+ .post_gpio_reset_off_delay_ms = 65,
.hid_descriptor_address = 0x0001,
};
static const struct of_device_id elan_i2c_hid_of_match[] = {
{ .compatible = "elan,ekth6915", .data = &elan_ekth6915_chip_data },
+ { .compatible = "ilitek,ili9882t", .data = &ilitek_ili9882t_chip_data },
{ }
};
MODULE_DEVICE_TABLE(of, elan_i2c_hid_of_match);
--
2.25.1
^ permalink raw reply related
* [PATCH v3 1/2] dt-bindings: HID: i2c-hid: ilitek: Introduce bindings for Ilitek ili9882t
From: Cong Yang @ 2023-06-07 13:34 UTC (permalink / raw)
To: robh+dt, krzysztof.kozlowski+dt, conor+dt, dmitry.torokhov, jikos,
benjamin.tissoires, dianders, hsinyi
Cc: linux-input, devicetree, linux-kernel, Cong Yang
In-Reply-To: <20230607133458.4075667-1-yangcong5@huaqin.corp-partner.google.com>
The ili9882t touch screen chip same as Elan eKTH6915 controller
has a reset gpio. The difference is that ili9882t needs to use
vccio-supply instead of vcc33-supply. Doug's series[1] allows panels
and touchscreens to power on/off together, let's add a phandle for this.
[1]: https: //lore.kernel.org/all/20230523193017.4109557-1-dianders@chromium.org/
Signed-off-by: Cong Yang <yangcong5@huaqin.corp-partner.google.com>
---
.../bindings/input/ilitek,ili9882t.yaml | 67 +++++++++++++++++++
1 file changed, 67 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/ilitek,ili9882t.yaml
diff --git a/Documentation/devicetree/bindings/input/ilitek,ili9882t.yaml b/Documentation/devicetree/bindings/input/ilitek,ili9882t.yaml
new file mode 100644
index 000000000000..f69fa3aaf4c5
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/ilitek,ili9882t.yaml
@@ -0,0 +1,67 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/ilitek,ili9882t.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Ilitek ili9882t touchscreen controller
+
+maintainers:
+ - Dmitry Torokhov <dmitry.torokhov@gmail.com>
+
+description:
+ Supports the Ilitek ili9882t touchscreen controller.
+ This touchscreen controller uses the i2c-hid protocol with a reset GPIO.
+
+properties:
+ compatible:
+ items:
+ - const: ilitek,ili9882t
+
+ reg:
+ const: 0x41
+
+ interrupts:
+ maxItems: 1
+
+ panel:
+ description: If this is a touchscreen, the panel it's connected to. This
+ indicates that the panel and touchscreen are expected to be power
+ sequenced together.
+ $ref: /schemas/types.yaml#/definitions/phandle
+
+ reset-gpios:
+ description: Reset GPIO.
+
+
+ vccio-supply:
+ description: The 1.8V supply to the touchscreen.
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - vccio-supply
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+ #include <dt-bindings/interrupt-controller/irq.h>
+
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ touchscreen: touchscreen@41 {
+ compatible = "ilitek,ili9882t";
+ reg = <0x41>;
+
+ interrupt-parent = <&pio>;
+ interrupts = <12 IRQ_TYPE_LEVEL_LOW>;
+
+ reset-gpios = <&pio 60 GPIO_ACTIVE_LOW>;
+ vccio-supply = <&mt6366_vio18_reg>;
+ };
+ };
--
2.25.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox