* Re: [PATCH] dt-bindings: input: mediatek,pmic-keys: Drop incomplete example
From: Krzysztof Kozlowski @ 2023-11-29 8:29 UTC (permalink / raw)
To: Rob Herring, Dmitry Torokhov, Krzysztof Kozlowski, Conor Dooley,
Matthias Brugger, AngeloGioacchino Del Regno, Chen Zhong
Cc: linux-input, devicetree, linux-kernel, linux-arm-kernel,
linux-mediatek
In-Reply-To: <20231128214816.3975893-1-robh@kernel.org>
On 28/11/2023 22:48, Rob Herring wrote:
> The example for the Mediatek PMIC keys is incomplete as the binding is
> the full PMIC, not just the sub-functions. It is preferred for MFD
> examples to be complete in the top-level MFD device binding rather than
> piecemeal in each sub-function binding.
>
> This also fixes an undocumented (by schema) compatible warning for
> "mediatek,mt6397".
>
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH] dt-bindings: input: sprd,sc27xx-vibrator: Drop incomplete example
From: Krzysztof Kozlowski @ 2023-11-29 8:30 UTC (permalink / raw)
To: Rob Herring, Dmitry Torokhov, Krzysztof Kozlowski, Conor Dooley,
Orson Zhai, Baolin Wang, Chunyan Zhang
Cc: linux-input, devicetree, linux-kernel
In-Reply-To: <20231128214809.3975719-1-robh@kernel.org>
On 28/11/2023 22:48, Rob Herring wrote:
> The example for the Spreadtrum SC27xx PMIC vibrator is incomplete as the
> binding is the full PMIC, not just the sub-functions. It is preferred
> for MFD examples to be complete in the top-level MFD device binding
> rather than piecemeal in each sub-function binding.
>
> This also fixes an undocumented (by schema) compatible warning for
> "sprd,sc2731".
>
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v4 2/2] Input: gpio-keys - Add system suspend support for dedicated wakeirqs
From: Tony Lindgren @ 2023-11-29 8:38 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-input,
devicetree, linux-kernel, linux-arm-kernel, Rob Herring,
Dhruva Gole
In-Reply-To: <ZWF9F9JHKJ-SjUjp@google.com>
* Dmitry Torokhov <dmitry.torokhov@gmail.com> [231125 04:50]:
> Hi Tony,
>
> On Fri, Nov 24, 2023 at 10:32:41AM +0200, Tony Lindgren wrote:
> > + /*
> > + * Wakeirq shares the handler with the main interrupt, it's only
> > + * active during system suspend. See gpio_keys_button_enable_wakeup()
> > + * and gpio_keys_button_disable_wakeup().
> > + */
> > + error = devm_request_any_context_irq(dev, bdata->wakeirq, isr,
> > + irqflags, wakedesc, bdata);
> > + if (error < 0) {
> > + dev_err(dev, "Unable to claim wakeirq %d; error %d\n",
> > + bdata->irq, error);
> > + return error;
> > + }
> > +
> > + /*
> > + * Disable wakeirq until suspend. IRQF_NO_AUTOEN won't work if
> > + * IRQF_SHARED was set based on !button->can_disable.
> > + */
> > + disable_irq_nosync(bdata->wakeirq);
>
> Why _nosync() here and below? Is there any harm in sing the normal
> variant?
Well they are enabled the same time anyways for a while, so I see no
harm using the normal variant here. Will post updated patches after
some testing.
Regards,
Tony
^ permalink raw reply
* Re: [PATCH] Input: i8042 - add noloop quirk for Acer P459-G2-M
From: Hans de Goede @ 2023-11-29 8:46 UTC (permalink / raw)
To: Esther Shimanovich, LKML
Cc: linux-input, Dmitry Torokhov, Jonathan Denose,
Mattijs Korpershoek, Werner Sembach
In-Reply-To: <20231128214851.1.Ibc66f1742765467808fb28a394f8f35af920aa49@changeid>
Hi,
On 11/28/23 22:48, Esther Shimanovich wrote:
> On the Acer P450-G2-M, after the user opens the laptop lid and the device
> resumes from S3 deep sleep, the screen remains dark for a few seconds.
> If the user presses a keyboard key while the screen is still dark, the
> mouse and keyboard stop functioning.
>
> To fix this bug, enable the noloop quirk.
>
> Signed-off-by: Esther Shimanovich <eshimanovich@chromium.org>
Thanks, patch looks good to me:
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Regards,
Hans
> ---
>
> drivers/input/serio/i8042-acpipnpio.h | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/input/serio/i8042-acpipnpio.h b/drivers/input/serio/i8042-acpipnpio.h
> index 028e45bd050bf..b81539bacb931 100644
> --- a/drivers/input/serio/i8042-acpipnpio.h
> +++ b/drivers/input/serio/i8042-acpipnpio.h
> @@ -941,6 +941,14 @@ static const struct dmi_system_id i8042_dmi_quirk_table[] __initconst = {
> },
> .driver_data = (void *)(SERIO_QUIRK_NOPNP)
> },
> + {
> + /* Acer TravelMate P459-G2-M */
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "Acer"),
> + DMI_MATCH(DMI_PRODUCT_NAME, "TravelMate P459-G2-M"),
> + },
> + .driver_data = (void *)(SERIO_QUIRK_NOLOOP)
> + },
> {
> /* ULI EV4873 - AUX LOOP does not work properly */
> .matches = {
^ permalink raw reply
* Re: [PATCH v5 1/4] pwm: rename pwm_apply_state() to pwm_apply_cansleep()
From: Sean Young @ 2023-11-29 9:08 UTC (permalink / raw)
To: Thierry Reding
Cc: linux-media, linux-pwm, Ivaylo Dimitrov, Uwe Kleine-König,
Jonathan Corbet, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
Tvrtko Ursulin, David Airlie, Daniel Vetter,
Javier Martinez Canillas, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Jean Delvare, Guenter Roeck,
Support Opensource, Dmitry Torokhov, Pavel Machek, Lee Jones,
Mauro Carvalho Chehab, Hans de Goede, Ilpo Järvinen,
Mark Gross, Liam Girdwood, Mark Brown, Daniel Thompson,
Jingoo Han, Helge Deller, Jani Nikula, linux-doc, linux-kernel,
intel-gfx, dri-devel, linux-hwmon, linux-input, linux-leds,
platform-driver-x86, linux-arm-kernel, linux-fbdev
In-Reply-To: <ZWClpnMRg_vjuI_R@orome.fritz.box>
On Fri, Nov 24, 2023 at 02:31:18PM +0100, Thierry Reding wrote:
> On Sat, Nov 18, 2023 at 04:16:17PM +0000, Sean Young wrote:
> > In order to introduce a pwm api which can be used from atomic context,
> > we will need two functions for applying pwm changes:
> >
> > int pwm_apply_cansleep(struct pwm *, struct pwm_state *);
> > int pwm_apply_atomic(struct pwm *, struct pwm_state *);
> >
> > This commit just deals with renaming pwm_apply_state(), a following
> > commit will introduce the pwm_apply_atomic() function.
>
> Sorry, I still don't agree with that _cansleep suffix. I think it's the
> wrong terminology. Just because something can sleep doesn't mean that it
> ever will. "Might sleep" is much more accurate because it says exactly
> what might happen and indicates what we're guarding against.
Sorry, I forgot about this in the last round. I've renamed it _might_sleep
in v6 which I'll post shortly.
Sean
^ permalink raw reply
* [PATCH v6 1/4] pwm: rename pwm_apply_state() to pwm_apply_might_sleep()
From: Sean Young @ 2023-11-29 9:13 UTC (permalink / raw)
To: linux-media, linux-pwm, Ivaylo Dimitrov, Thierry Reding,
Uwe Kleine-König, Jonathan Corbet, Jani Nikula,
Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin, David Airlie,
Daniel Vetter, Javier Martinez Canillas, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, Jean Delvare, Guenter Roeck,
Support Opensource, Dmitry Torokhov, Pavel Machek, Lee Jones,
Sean Young, Mauro Carvalho Chehab, Hans de Goede,
Ilpo Järvinen, Mark Gross, Liam Girdwood, Mark Brown,
Daniel Thompson, Jingoo Han, Helge Deller
Cc: Jani Nikula, linux-doc, linux-kernel, intel-gfx, dri-devel,
linux-hwmon, linux-input, linux-leds, platform-driver-x86,
linux-arm-kernel, linux-fbdev
In-Reply-To: <cover.1701248996.git.sean@mess.org>
In order to introduce a pwm api which can be used from atomic context,
we will need two functions for applying pwm changes:
int pwm_apply_might_sleep(struct pwm *, struct pwm_state *);
int pwm_apply_atomic(struct pwm *, struct pwm_state *);
This commit just deals with renaming pwm_apply_state(), a following
commit will introduce the pwm_apply_atomic() function.
Acked-by: Hans de Goede <hdegoede@redhat.com>
Acked-by: Jani Nikula <jani.nikula@intel.com>
Acked-by: Lee Jones <lee@kernel.org>
Signed-off-by: Sean Young <sean@mess.org>
---
Documentation/driver-api/pwm.rst | 8 +++---
.../gpu/drm/i915/display/intel_backlight.c | 6 ++--
drivers/gpu/drm/solomon/ssd130x.c | 2 +-
drivers/hwmon/pwm-fan.c | 8 +++---
drivers/input/misc/da7280.c | 4 +--
drivers/input/misc/pwm-beeper.c | 4 +--
drivers/input/misc/pwm-vibra.c | 8 +++---
drivers/leds/leds-pwm.c | 2 +-
drivers/leds/rgb/leds-pwm-multicolor.c | 4 +--
drivers/media/rc/pwm-ir-tx.c | 4 +--
drivers/platform/x86/lenovo-yogabook.c | 2 +-
drivers/pwm/core.c | 18 ++++++------
drivers/pwm/pwm-twl-led.c | 2 +-
drivers/pwm/pwm-vt8500.c | 2 +-
drivers/pwm/sysfs.c | 10 +++----
drivers/regulator/pwm-regulator.c | 4 +--
drivers/video/backlight/lm3630a_bl.c | 2 +-
drivers/video/backlight/lp855x_bl.c | 2 +-
drivers/video/backlight/pwm_bl.c | 12 ++++----
drivers/video/fbdev/ssd1307fb.c | 2 +-
include/linux/pwm.h | 28 +++++++++----------
21 files changed, 67 insertions(+), 67 deletions(-)
diff --git a/Documentation/driver-api/pwm.rst b/Documentation/driver-api/pwm.rst
index bb264490a87a..f1d8197c8c43 100644
--- a/Documentation/driver-api/pwm.rst
+++ b/Documentation/driver-api/pwm.rst
@@ -41,7 +41,7 @@ the getter, devm_pwm_get() and devm_fwnode_pwm_get(), also exist.
After being requested, a PWM has to be configured using::
- int pwm_apply_state(struct pwm_device *pwm, struct pwm_state *state);
+ int pwm_apply_might_sleep(struct pwm_device *pwm, struct pwm_state *state);
This API controls both the PWM period/duty_cycle config and the
enable/disable state.
@@ -57,13 +57,13 @@ If supported by the driver, the signal can be optimized, for example to improve
EMI by phase shifting the individual channels of a chip.
The pwm_config(), pwm_enable() and pwm_disable() functions are just wrappers
-around pwm_apply_state() and should not be used if the user wants to change
+around pwm_apply_might_sleep() and should not be used if the user wants to change
several parameter at once. For example, if you see pwm_config() and
pwm_{enable,disable}() calls in the same function, this probably means you
-should switch to pwm_apply_state().
+should switch to pwm_apply_might_sleep().
The PWM user API also allows one to query the PWM state that was passed to the
-last invocation of pwm_apply_state() using pwm_get_state(). Note this is
+last invocation of pwm_apply_might_sleep() using pwm_get_state(). Note this is
different to what the driver has actually implemented if the request cannot be
satisfied exactly with the hardware in use. There is currently no way for
consumers to get the actually implemented settings.
diff --git a/drivers/gpu/drm/i915/display/intel_backlight.c b/drivers/gpu/drm/i915/display/intel_backlight.c
index 2e8f17c04522..ff9b9918b0a1 100644
--- a/drivers/gpu/drm/i915/display/intel_backlight.c
+++ b/drivers/gpu/drm/i915/display/intel_backlight.c
@@ -274,7 +274,7 @@ static void ext_pwm_set_backlight(const struct drm_connector_state *conn_state,
struct intel_panel *panel = &to_intel_connector(conn_state->connector)->panel;
pwm_set_relative_duty_cycle(&panel->backlight.pwm_state, level, 100);
- pwm_apply_state(panel->backlight.pwm, &panel->backlight.pwm_state);
+ pwm_apply_might_sleep(panel->backlight.pwm, &panel->backlight.pwm_state);
}
static void
@@ -427,7 +427,7 @@ static void ext_pwm_disable_backlight(const struct drm_connector_state *old_conn
intel_backlight_set_pwm_level(old_conn_state, level);
panel->backlight.pwm_state.enabled = false;
- pwm_apply_state(panel->backlight.pwm, &panel->backlight.pwm_state);
+ pwm_apply_might_sleep(panel->backlight.pwm, &panel->backlight.pwm_state);
}
void intel_backlight_disable(const struct drm_connector_state *old_conn_state)
@@ -749,7 +749,7 @@ static void ext_pwm_enable_backlight(const struct intel_crtc_state *crtc_state,
pwm_set_relative_duty_cycle(&panel->backlight.pwm_state, level, 100);
panel->backlight.pwm_state.enabled = true;
- pwm_apply_state(panel->backlight.pwm, &panel->backlight.pwm_state);
+ pwm_apply_might_sleep(panel->backlight.pwm, &panel->backlight.pwm_state);
}
static void __intel_backlight_enable(const struct intel_crtc_state *crtc_state,
diff --git a/drivers/gpu/drm/solomon/ssd130x.c b/drivers/gpu/drm/solomon/ssd130x.c
index e0174f82e353..cce043a4a1dc 100644
--- a/drivers/gpu/drm/solomon/ssd130x.c
+++ b/drivers/gpu/drm/solomon/ssd130x.c
@@ -319,7 +319,7 @@ static int ssd130x_pwm_enable(struct ssd130x_device *ssd130x)
pwm_init_state(ssd130x->pwm, &pwmstate);
pwm_set_relative_duty_cycle(&pwmstate, 50, 100);
- pwm_apply_state(ssd130x->pwm, &pwmstate);
+ pwm_apply_might_sleep(ssd130x->pwm, &pwmstate);
/* Enable the PWM */
pwm_enable(ssd130x->pwm);
diff --git a/drivers/hwmon/pwm-fan.c b/drivers/hwmon/pwm-fan.c
index 6e4516c2ab89..b67bc9e833c0 100644
--- a/drivers/hwmon/pwm-fan.c
+++ b/drivers/hwmon/pwm-fan.c
@@ -151,7 +151,7 @@ static int pwm_fan_power_on(struct pwm_fan_ctx *ctx)
}
state->enabled = true;
- ret = pwm_apply_state(ctx->pwm, state);
+ ret = pwm_apply_might_sleep(ctx->pwm, state);
if (ret) {
dev_err(ctx->dev, "failed to enable PWM\n");
goto disable_regulator;
@@ -181,7 +181,7 @@ static int pwm_fan_power_off(struct pwm_fan_ctx *ctx)
state->enabled = false;
state->duty_cycle = 0;
- ret = pwm_apply_state(ctx->pwm, state);
+ ret = pwm_apply_might_sleep(ctx->pwm, state);
if (ret) {
dev_err(ctx->dev, "failed to disable PWM\n");
return ret;
@@ -207,7 +207,7 @@ static int __set_pwm(struct pwm_fan_ctx *ctx, unsigned long pwm)
period = state->period;
state->duty_cycle = DIV_ROUND_UP(pwm * (period - 1), MAX_PWM);
- ret = pwm_apply_state(ctx->pwm, state);
+ ret = pwm_apply_might_sleep(ctx->pwm, state);
if (ret)
return ret;
ret = pwm_fan_power_on(ctx);
@@ -278,7 +278,7 @@ static int pwm_fan_update_enable(struct pwm_fan_ctx *ctx, long val)
state,
&enable_regulator);
- pwm_apply_state(ctx->pwm, state);
+ pwm_apply_might_sleep(ctx->pwm, state);
pwm_fan_switch_power(ctx, enable_regulator);
pwm_fan_update_state(ctx, 0);
}
diff --git a/drivers/input/misc/da7280.c b/drivers/input/misc/da7280.c
index ce82548916bb..c1fa75c0f970 100644
--- a/drivers/input/misc/da7280.c
+++ b/drivers/input/misc/da7280.c
@@ -352,7 +352,7 @@ static int da7280_haptic_set_pwm(struct da7280_haptic *haptics, bool enabled)
state.duty_cycle = period_mag_multi;
}
- error = pwm_apply_state(haptics->pwm_dev, &state);
+ error = pwm_apply_might_sleep(haptics->pwm_dev, &state);
if (error)
dev_err(haptics->dev, "Failed to apply pwm state: %d\n", error);
@@ -1175,7 +1175,7 @@ static int da7280_probe(struct i2c_client *client)
/* Sync up PWM state and ensure it is off. */
pwm_init_state(haptics->pwm_dev, &state);
state.enabled = false;
- error = pwm_apply_state(haptics->pwm_dev, &state);
+ error = pwm_apply_might_sleep(haptics->pwm_dev, &state);
if (error) {
dev_err(dev, "Failed to apply PWM state: %d\n", error);
return error;
diff --git a/drivers/input/misc/pwm-beeper.c b/drivers/input/misc/pwm-beeper.c
index 1e731d8397c6..5b9aedf4362f 100644
--- a/drivers/input/misc/pwm-beeper.c
+++ b/drivers/input/misc/pwm-beeper.c
@@ -39,7 +39,7 @@ static int pwm_beeper_on(struct pwm_beeper *beeper, unsigned long period)
state.period = period;
pwm_set_relative_duty_cycle(&state, 50, 100);
- error = pwm_apply_state(beeper->pwm, &state);
+ error = pwm_apply_might_sleep(beeper->pwm, &state);
if (error)
return error;
@@ -138,7 +138,7 @@ static int pwm_beeper_probe(struct platform_device *pdev)
/* Sync up PWM state and ensure it is off. */
pwm_init_state(beeper->pwm, &state);
state.enabled = false;
- error = pwm_apply_state(beeper->pwm, &state);
+ error = pwm_apply_might_sleep(beeper->pwm, &state);
if (error) {
dev_err(dev, "failed to apply initial PWM state: %d\n",
error);
diff --git a/drivers/input/misc/pwm-vibra.c b/drivers/input/misc/pwm-vibra.c
index acac79c488aa..3e5ed685ed8f 100644
--- a/drivers/input/misc/pwm-vibra.c
+++ b/drivers/input/misc/pwm-vibra.c
@@ -56,7 +56,7 @@ static int pwm_vibrator_start(struct pwm_vibrator *vibrator)
pwm_set_relative_duty_cycle(&state, vibrator->level, 0xffff);
state.enabled = true;
- err = pwm_apply_state(vibrator->pwm, &state);
+ err = pwm_apply_might_sleep(vibrator->pwm, &state);
if (err) {
dev_err(pdev, "failed to apply pwm state: %d\n", err);
return err;
@@ -67,7 +67,7 @@ static int pwm_vibrator_start(struct pwm_vibrator *vibrator)
state.duty_cycle = vibrator->direction_duty_cycle;
state.enabled = true;
- err = pwm_apply_state(vibrator->pwm_dir, &state);
+ err = pwm_apply_might_sleep(vibrator->pwm_dir, &state);
if (err) {
dev_err(pdev, "failed to apply dir-pwm state: %d\n", err);
pwm_disable(vibrator->pwm);
@@ -160,7 +160,7 @@ static int pwm_vibrator_probe(struct platform_device *pdev)
/* Sync up PWM state and ensure it is off. */
pwm_init_state(vibrator->pwm, &state);
state.enabled = false;
- err = pwm_apply_state(vibrator->pwm, &state);
+ err = pwm_apply_might_sleep(vibrator->pwm, &state);
if (err) {
dev_err(&pdev->dev, "failed to apply initial PWM state: %d\n",
err);
@@ -174,7 +174,7 @@ static int pwm_vibrator_probe(struct platform_device *pdev)
/* Sync up PWM state and ensure it is off. */
pwm_init_state(vibrator->pwm_dir, &state);
state.enabled = false;
- err = pwm_apply_state(vibrator->pwm_dir, &state);
+ err = pwm_apply_might_sleep(vibrator->pwm_dir, &state);
if (err) {
dev_err(&pdev->dev, "failed to apply initial PWM state: %d\n",
err);
diff --git a/drivers/leds/leds-pwm.c b/drivers/leds/leds-pwm.c
index 2b3bf1353b70..4e3936a39d0e 100644
--- a/drivers/leds/leds-pwm.c
+++ b/drivers/leds/leds-pwm.c
@@ -54,7 +54,7 @@ static int led_pwm_set(struct led_classdev *led_cdev,
led_dat->pwmstate.duty_cycle = duty;
led_dat->pwmstate.enabled = true;
- return pwm_apply_state(led_dat->pwm, &led_dat->pwmstate);
+ return pwm_apply_might_sleep(led_dat->pwm, &led_dat->pwmstate);
}
__attribute__((nonnull))
diff --git a/drivers/leds/rgb/leds-pwm-multicolor.c b/drivers/leds/rgb/leds-pwm-multicolor.c
index 46cd062b8b24..e1a81e0109e8 100644
--- a/drivers/leds/rgb/leds-pwm-multicolor.c
+++ b/drivers/leds/rgb/leds-pwm-multicolor.c
@@ -51,8 +51,8 @@ static int led_pwm_mc_set(struct led_classdev *cdev,
priv->leds[i].state.duty_cycle = duty;
priv->leds[i].state.enabled = duty > 0;
- ret = pwm_apply_state(priv->leds[i].pwm,
- &priv->leds[i].state);
+ ret = pwm_apply_might_sleep(priv->leds[i].pwm,
+ &priv->leds[i].state);
if (ret)
break;
}
diff --git a/drivers/media/rc/pwm-ir-tx.c b/drivers/media/rc/pwm-ir-tx.c
index c5f37c03af9c..cf51e2760975 100644
--- a/drivers/media/rc/pwm-ir-tx.c
+++ b/drivers/media/rc/pwm-ir-tx.c
@@ -68,7 +68,7 @@ static int pwm_ir_tx(struct rc_dev *dev, unsigned int *txbuf,
for (i = 0; i < count; i++) {
state.enabled = !(i % 2);
- pwm_apply_state(pwm, &state);
+ pwm_apply_might_sleep(pwm, &state);
edge = ktime_add_us(edge, txbuf[i]);
delta = ktime_us_delta(edge, ktime_get());
@@ -77,7 +77,7 @@ static int pwm_ir_tx(struct rc_dev *dev, unsigned int *txbuf,
}
state.enabled = false;
- pwm_apply_state(pwm, &state);
+ pwm_apply_might_sleep(pwm, &state);
return count;
}
diff --git a/drivers/platform/x86/lenovo-yogabook.c b/drivers/platform/x86/lenovo-yogabook.c
index b8d0239192cb..fd62bf746ebd 100644
--- a/drivers/platform/x86/lenovo-yogabook.c
+++ b/drivers/platform/x86/lenovo-yogabook.c
@@ -435,7 +435,7 @@ static int yogabook_pdev_set_kbd_backlight(struct yogabook_data *data, u8 level)
.enabled = level,
};
- pwm_apply_state(data->kbd_bl_pwm, &state);
+ pwm_apply_might_sleep(data->kbd_bl_pwm, &state);
gpiod_set_value(data->kbd_bl_led_enable, level ? 1 : 0);
return 0;
}
diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index 29078486534d..fc92ba622e56 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -356,8 +356,8 @@ struct pwm_device *pwm_request_from_chip(struct pwm_chip *chip,
}
EXPORT_SYMBOL_GPL(pwm_request_from_chip);
-static void pwm_apply_state_debug(struct pwm_device *pwm,
- const struct pwm_state *state)
+static void pwm_apply_debug(struct pwm_device *pwm,
+ const struct pwm_state *state)
{
struct pwm_state *last = &pwm->last;
struct pwm_chip *chip = pwm->chip;
@@ -463,11 +463,11 @@ static void pwm_apply_state_debug(struct pwm_device *pwm,
}
/**
- * pwm_apply_state() - atomically apply a new state to a PWM device
+ * pwm_apply_might_sleep() - atomically apply a new state to a PWM device
* @pwm: PWM device
* @state: new state to apply
*/
-int pwm_apply_state(struct pwm_device *pwm, const struct pwm_state *state)
+int pwm_apply_might_sleep(struct pwm_device *pwm, const struct pwm_state *state)
{
struct pwm_chip *chip;
int err;
@@ -475,7 +475,7 @@ int pwm_apply_state(struct pwm_device *pwm, const struct pwm_state *state)
/*
* Some lowlevel driver's implementations of .apply() make use of
* mutexes, also with some drivers only returning when the new
- * configuration is active calling pwm_apply_state() from atomic context
+ * configuration is active calling pwm_apply_might_sleep() from atomic context
* is a bad idea. So make it explicit that calling this function might
* sleep.
*/
@@ -505,11 +505,11 @@ int pwm_apply_state(struct pwm_device *pwm, const struct pwm_state *state)
* only do this after pwm->state was applied as some
* implementations of .get_state depend on this
*/
- pwm_apply_state_debug(pwm, state);
+ pwm_apply_debug(pwm, state);
return 0;
}
-EXPORT_SYMBOL_GPL(pwm_apply_state);
+EXPORT_SYMBOL_GPL(pwm_apply_might_sleep);
/**
* pwm_capture() - capture and report a PWM signal
@@ -567,7 +567,7 @@ int pwm_adjust_config(struct pwm_device *pwm)
state.period = pargs.period;
state.polarity = pargs.polarity;
- return pwm_apply_state(pwm, &state);
+ return pwm_apply_might_sleep(pwm, &state);
}
/*
@@ -590,7 +590,7 @@ int pwm_adjust_config(struct pwm_device *pwm)
state.duty_cycle = state.period - state.duty_cycle;
}
- return pwm_apply_state(pwm, &state);
+ return pwm_apply_might_sleep(pwm, &state);
}
EXPORT_SYMBOL_GPL(pwm_adjust_config);
diff --git a/drivers/pwm/pwm-twl-led.c b/drivers/pwm/pwm-twl-led.c
index 625233f4703a..70eec1c66747 100644
--- a/drivers/pwm/pwm-twl-led.c
+++ b/drivers/pwm/pwm-twl-led.c
@@ -172,7 +172,7 @@ static int twl4030_pwmled_apply(struct pwm_chip *chip, struct pwm_device *pwm,
* We cannot skip calling ->config even if state->period ==
* pwm->state.period && state->duty_cycle == pwm->state.duty_cycle
* because we might have exited early in the last call to
- * pwm_apply_state because of !state->enabled and so the two values in
+ * pwm_apply_might_sleep because of !state->enabled and so the two values in
* pwm->state might not be configured in hardware.
*/
ret = twl4030_pwmled_config(pwm->chip, pwm,
diff --git a/drivers/pwm/pwm-vt8500.c b/drivers/pwm/pwm-vt8500.c
index 5568d5312d3c..3d1d11062558 100644
--- a/drivers/pwm/pwm-vt8500.c
+++ b/drivers/pwm/pwm-vt8500.c
@@ -206,7 +206,7 @@ static int vt8500_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
* We cannot skip calling ->config even if state->period ==
* pwm->state.period && state->duty_cycle == pwm->state.duty_cycle
* because we might have exited early in the last call to
- * pwm_apply_state because of !state->enabled and so the two values in
+ * pwm_apply_might_sleep because of !state->enabled and so the two values in
* pwm->state might not be configured in hardware.
*/
err = vt8500_pwm_config(pwm->chip, pwm, state->duty_cycle, state->period);
diff --git a/drivers/pwm/sysfs.c b/drivers/pwm/sysfs.c
index 8d1254761e4d..052ccadbdabf 100644
--- a/drivers/pwm/sysfs.c
+++ b/drivers/pwm/sysfs.c
@@ -62,7 +62,7 @@ static ssize_t period_store(struct device *child,
mutex_lock(&export->lock);
pwm_get_state(pwm, &state);
state.period = val;
- ret = pwm_apply_state(pwm, &state);
+ ret = pwm_apply_might_sleep(pwm, &state);
mutex_unlock(&export->lock);
return ret ? : size;
@@ -97,7 +97,7 @@ static ssize_t duty_cycle_store(struct device *child,
mutex_lock(&export->lock);
pwm_get_state(pwm, &state);
state.duty_cycle = val;
- ret = pwm_apply_state(pwm, &state);
+ ret = pwm_apply_might_sleep(pwm, &state);
mutex_unlock(&export->lock);
return ret ? : size;
@@ -144,7 +144,7 @@ static ssize_t enable_store(struct device *child,
goto unlock;
}
- ret = pwm_apply_state(pwm, &state);
+ ret = pwm_apply_might_sleep(pwm, &state);
unlock:
mutex_unlock(&export->lock);
@@ -194,7 +194,7 @@ static ssize_t polarity_store(struct device *child,
mutex_lock(&export->lock);
pwm_get_state(pwm, &state);
state.polarity = polarity;
- ret = pwm_apply_state(pwm, &state);
+ ret = pwm_apply_might_sleep(pwm, &state);
mutex_unlock(&export->lock);
return ret ? : size;
@@ -401,7 +401,7 @@ static int pwm_class_apply_state(struct pwm_export *export,
struct pwm_device *pwm,
struct pwm_state *state)
{
- int ret = pwm_apply_state(pwm, state);
+ int ret = pwm_apply_might_sleep(pwm, state);
/* release lock taken in pwm_class_get_state */
mutex_unlock(&export->lock);
diff --git a/drivers/regulator/pwm-regulator.c b/drivers/regulator/pwm-regulator.c
index 2aff6db748e2..698c420e0869 100644
--- a/drivers/regulator/pwm-regulator.c
+++ b/drivers/regulator/pwm-regulator.c
@@ -90,7 +90,7 @@ static int pwm_regulator_set_voltage_sel(struct regulator_dev *rdev,
pwm_set_relative_duty_cycle(&pstate,
drvdata->duty_cycle_table[selector].dutycycle, 100);
- ret = pwm_apply_state(drvdata->pwm, &pstate);
+ ret = pwm_apply_might_sleep(drvdata->pwm, &pstate);
if (ret) {
dev_err(&rdev->dev, "Failed to configure PWM: %d\n", ret);
return ret;
@@ -216,7 +216,7 @@ static int pwm_regulator_set_voltage(struct regulator_dev *rdev,
pwm_set_relative_duty_cycle(&pstate, dutycycle, duty_unit);
- ret = pwm_apply_state(drvdata->pwm, &pstate);
+ ret = pwm_apply_might_sleep(drvdata->pwm, &pstate);
if (ret) {
dev_err(&rdev->dev, "Failed to configure PWM: %d\n", ret);
return ret;
diff --git a/drivers/video/backlight/lm3630a_bl.c b/drivers/video/backlight/lm3630a_bl.c
index 8fcb62be597b..a3412c936ca2 100644
--- a/drivers/video/backlight/lm3630a_bl.c
+++ b/drivers/video/backlight/lm3630a_bl.c
@@ -180,7 +180,7 @@ static int lm3630a_pwm_ctrl(struct lm3630a_chip *pchip, int br, int br_max)
pchip->pwmd_state.enabled = pchip->pwmd_state.duty_cycle ? true : false;
- return pwm_apply_state(pchip->pwmd, &pchip->pwmd_state);
+ return pwm_apply_might_sleep(pchip->pwmd, &pchip->pwmd_state);
}
/* update and get brightness */
diff --git a/drivers/video/backlight/lp855x_bl.c b/drivers/video/backlight/lp855x_bl.c
index da1f124db69c..7075bfab59c4 100644
--- a/drivers/video/backlight/lp855x_bl.c
+++ b/drivers/video/backlight/lp855x_bl.c
@@ -234,7 +234,7 @@ static int lp855x_pwm_ctrl(struct lp855x *lp, int br, int max_br)
state.duty_cycle = div_u64(br * state.period, max_br);
state.enabled = state.duty_cycle;
- return pwm_apply_state(lp->pwm, &state);
+ return pwm_apply_might_sleep(lp->pwm, &state);
}
static int lp855x_bl_update_status(struct backlight_device *bl)
diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
index 289bd9ce4d36..35c716e9043c 100644
--- a/drivers/video/backlight/pwm_bl.c
+++ b/drivers/video/backlight/pwm_bl.c
@@ -103,7 +103,7 @@ static int pwm_backlight_update_status(struct backlight_device *bl)
pwm_get_state(pb->pwm, &state);
state.duty_cycle = compute_duty_cycle(pb, brightness, &state);
state.enabled = true;
- pwm_apply_state(pb->pwm, &state);
+ pwm_apply_might_sleep(pb->pwm, &state);
pwm_backlight_power_on(pb);
} else {
@@ -120,7 +120,7 @@ static int pwm_backlight_update_status(struct backlight_device *bl)
* inactive output.
*/
state.enabled = !pb->power_supply && !pb->enable_gpio;
- pwm_apply_state(pb->pwm, &state);
+ pwm_apply_might_sleep(pb->pwm, &state);
}
if (pb->notify_after)
@@ -528,7 +528,7 @@ static int pwm_backlight_probe(struct platform_device *pdev)
if (!state.period && (data->pwm_period_ns > 0))
state.period = data->pwm_period_ns;
- ret = pwm_apply_state(pb->pwm, &state);
+ ret = pwm_apply_might_sleep(pb->pwm, &state);
if (ret) {
dev_err(&pdev->dev, "failed to apply initial PWM state: %d\n",
ret);
@@ -633,7 +633,7 @@ static void pwm_backlight_remove(struct platform_device *pdev)
pwm_get_state(pb->pwm, &state);
state.duty_cycle = 0;
state.enabled = false;
- pwm_apply_state(pb->pwm, &state);
+ pwm_apply_might_sleep(pb->pwm, &state);
if (pb->exit)
pb->exit(&pdev->dev);
@@ -649,7 +649,7 @@ static void pwm_backlight_shutdown(struct platform_device *pdev)
pwm_get_state(pb->pwm, &state);
state.duty_cycle = 0;
state.enabled = false;
- pwm_apply_state(pb->pwm, &state);
+ pwm_apply_might_sleep(pb->pwm, &state);
}
#ifdef CONFIG_PM_SLEEP
@@ -673,7 +673,7 @@ static int pwm_backlight_suspend(struct device *dev)
pwm_get_state(pb->pwm, &state);
state.duty_cycle = 0;
state.enabled = false;
- pwm_apply_state(pb->pwm, &state);
+ pwm_apply_might_sleep(pb->pwm, &state);
if (pb->notify_after)
pb->notify_after(pb->dev, 0);
diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
index 5ae48e36fccb..1a4f90ea7d5a 100644
--- a/drivers/video/fbdev/ssd1307fb.c
+++ b/drivers/video/fbdev/ssd1307fb.c
@@ -347,7 +347,7 @@ static int ssd1307fb_init(struct ssd1307fb_par *par)
pwm_init_state(par->pwm, &pwmstate);
pwm_set_relative_duty_cycle(&pwmstate, 50, 100);
- pwm_apply_state(par->pwm, &pwmstate);
+ pwm_apply_might_sleep(par->pwm, &pwmstate);
/* Enable the PWM */
pwm_enable(par->pwm);
diff --git a/include/linux/pwm.h b/include/linux/pwm.h
index cda3597b84f2..5c5c456948a4 100644
--- a/include/linux/pwm.h
+++ b/include/linux/pwm.h
@@ -93,8 +93,8 @@ struct pwm_device {
* @state: state to fill with the current PWM state
*
* The returned PWM state represents the state that was applied by a previous call to
- * pwm_apply_state(). Drivers may have to slightly tweak that state before programming it to
- * hardware. If pwm_apply_state() was never called, this returns either the current hardware
+ * pwm_apply_might_sleep(). Drivers may have to slightly tweak that state before programming it to
+ * hardware. If pwm_apply_might_sleep() was never called, this returns either the current hardware
* state (if supported) or the default settings.
*/
static inline void pwm_get_state(const struct pwm_device *pwm,
@@ -158,20 +158,20 @@ static inline void pwm_get_args(const struct pwm_device *pwm,
}
/**
- * pwm_init_state() - prepare a new state to be applied with pwm_apply_state()
+ * pwm_init_state() - prepare a new state to be applied with pwm_apply_might_sleep()
* @pwm: PWM device
* @state: state to fill with the prepared PWM state
*
* This functions prepares a state that can later be tweaked and applied
- * to the PWM device with pwm_apply_state(). This is a convenient function
+ * to the PWM device with pwm_apply_might_sleep(). This is a convenient function
* that first retrieves the current PWM state and the replaces the period
* and polarity fields with the reference values defined in pwm->args.
* Once the function returns, you can adjust the ->enabled and ->duty_cycle
- * fields according to your needs before calling pwm_apply_state().
+ * fields according to your needs before calling pwm_apply_might_sleep().
*
* ->duty_cycle is initially set to zero to avoid cases where the current
* ->duty_cycle value exceed the pwm_args->period one, which would trigger
- * an error if the user calls pwm_apply_state() without adjusting ->duty_cycle
+ * an error if the user calls pwm_apply_might_sleep() without adjusting ->duty_cycle
* first.
*/
static inline void pwm_init_state(const struct pwm_device *pwm,
@@ -227,7 +227,7 @@ pwm_get_relative_duty_cycle(const struct pwm_state *state, unsigned int scale)
*
* pwm_init_state(pwm, &state);
* pwm_set_relative_duty_cycle(&state, 50, 100);
- * pwm_apply_state(pwm, &state);
+ * pwm_apply_might_sleep(pwm, &state);
*
* This functions returns -EINVAL if @duty_cycle and/or @scale are
* inconsistent (@scale == 0 or @duty_cycle > @scale).
@@ -307,7 +307,7 @@ struct pwm_chip {
#if IS_ENABLED(CONFIG_PWM)
/* PWM user APIs */
-int pwm_apply_state(struct pwm_device *pwm, const struct pwm_state *state);
+int pwm_apply_might_sleep(struct pwm_device *pwm, const struct pwm_state *state);
int pwm_adjust_config(struct pwm_device *pwm);
/**
@@ -335,7 +335,7 @@ static inline int pwm_config(struct pwm_device *pwm, int duty_ns,
state.duty_cycle = duty_ns;
state.period = period_ns;
- return pwm_apply_state(pwm, &state);
+ return pwm_apply_might_sleep(pwm, &state);
}
/**
@@ -356,7 +356,7 @@ static inline int pwm_enable(struct pwm_device *pwm)
return 0;
state.enabled = true;
- return pwm_apply_state(pwm, &state);
+ return pwm_apply_might_sleep(pwm, &state);
}
/**
@@ -375,7 +375,7 @@ static inline void pwm_disable(struct pwm_device *pwm)
return;
state.enabled = false;
- pwm_apply_state(pwm, &state);
+ pwm_apply_might_sleep(pwm, &state);
}
/* PWM provider APIs */
@@ -406,8 +406,8 @@ struct pwm_device *devm_fwnode_pwm_get(struct device *dev,
struct fwnode_handle *fwnode,
const char *con_id);
#else
-static inline int pwm_apply_state(struct pwm_device *pwm,
- const struct pwm_state *state)
+static inline int pwm_apply_might_sleep(struct pwm_device *pwm,
+ const struct pwm_state *state)
{
might_sleep();
return -ENOTSUPP;
@@ -524,7 +524,7 @@ static inline void pwm_apply_args(struct pwm_device *pwm)
state.period = pwm->args.period;
state.usage_power = false;
- pwm_apply_state(pwm, &state);
+ pwm_apply_might_sleep(pwm, &state);
}
struct pwm_lookup {
--
2.43.0
^ permalink raw reply related
* [PATCH v5 1/2] dt-bindings: input: gpio-keys: Allow optional dedicated wakeirq
From: Tony Lindgren @ 2023-11-29 11:06 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Rob Herring, linux-input, devicetree, linux-kernel
Allow configuring an optional dedicated wakeirq for gpio-keys that
some SoCs have.
Let's use the common interrupt naming "irq" and "wakeup" that we already
have in use for some drivers and subsystems like i2c framework.
Note that the gpio-keys interrupt property is optional. If only a gpio
property is specified, the driver tries to translate the gpio into an
interrupt.
Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
No changes since v3
Changes since v2:
- Fix indentation as noted by Rob's bot
- Add Reviewed-by from Rob
Changes since v1:
- Run make dt_binding_check on the binding
- Add better checks for interrupt-names as suggested by Rob, it is
now required if two interrupts are configured
- Add more decription entries
- Add a new example for key-wakeup
---
.../devicetree/bindings/input/gpio-keys.yaml | 41 ++++++++++++++++++-
1 file changed, 40 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/input/gpio-keys.yaml b/Documentation/devicetree/bindings/input/gpio-keys.yaml
--- a/Documentation/devicetree/bindings/input/gpio-keys.yaml
+++ b/Documentation/devicetree/bindings/input/gpio-keys.yaml
@@ -31,7 +31,23 @@ patternProperties:
maxItems: 1
interrupts:
- maxItems: 1
+ oneOf:
+ - items:
+ - description: Optional key interrupt or wakeup interrupt
+ - items:
+ - description: Key interrupt
+ - description: Wakeup interrupt
+
+ interrupt-names:
+ description:
+ Optional interrupt names, can be used to specify a separate dedicated
+ wake-up interrupt in addition to the gpio irq
+ oneOf:
+ - items:
+ - enum: [ irq, wakeup ]
+ - items:
+ - const: irq
+ - const: wakeup
label:
description: Descriptive name of the key.
@@ -97,6 +113,20 @@ patternProperties:
- required:
- gpios
+ allOf:
+ - if:
+ properties:
+ interrupts:
+ minItems: 2
+ required:
+ - interrupts
+ then:
+ properties:
+ interrupt-names:
+ minItems: 2
+ required:
+ - interrupt-names
+
dependencies:
wakeup-event-action: [ wakeup-source ]
linux,input-value: [ gpios ]
@@ -137,6 +167,15 @@ examples:
linux,code = <108>;
interrupts = <1 IRQ_TYPE_EDGE_FALLING>;
};
+
+ key-wakeup {
+ label = "GPIO Key WAKEUP";
+ linux,code = <143>;
+ interrupts-extended = <&intc 2 IRQ_TYPE_EDGE_FALLING>,
+ <&intc_wakeup 0 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "irq", "wakeup";
+ wakeup-source;
+ };
};
...
--
2.42.1
^ permalink raw reply
* [PATCH v5 2/2] Input: gpio-keys - Add system suspend support for dedicated wakeirqs
From: Tony Lindgren @ 2023-11-29 11:06 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Rob Herring, linux-input, devicetree, linux-kernel, Dhruva Gole
In-Reply-To: <20231129110618.27551-1-tony@atomide.com>
Some SoCs have a separate dedicated wake-up interrupt controller that can
be used to wake up the system from deeper idle states. We already support
configuring a separate interrupt for a gpio-keys button to be used with a
gpio line. However, we are lacking support system suspend for cases where
a separate interrupt needs to be used in deeper sleep modes.
Because of it's nature, gpio-keys does not know about the runtime PM state
of the button gpios, and may have several gpio buttons configured for each
gpio-keys device instance. Implementing runtime PM support for gpio-keys
does not help, and we cannot use drivers/base/power/wakeirq.c support. We
need to implement custom wakeirq support for gpio-keys.
For handling a dedicated wakeirq for system suspend, we enable and disable
it with gpio_keys_enable_wakeup() and gpio_keys_disable_wakeup() that we
already use based on device_may_wakeup().
Some systems may have a dedicated wakeirq that can also be used as the
main interrupt, this is already working for gpio-keys. Let's add some
wakeirq related comments while at it as the usage with a gpio line and
separate interrupt line may not be obvious.
Tested-by: Dhruva Gole <d-gole@ti.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
Changes since v5
- Use normal disable_irq() as suggested by Dmitry
No driver changes between v1 and v4
---
drivers/input/keyboard/gpio_keys.c | 69 ++++++++++++++++++++++++++++--
include/linux/gpio_keys.h | 2 +
2 files changed, 67 insertions(+), 4 deletions(-)
diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c
@@ -45,7 +45,9 @@ struct gpio_button_data {
unsigned int software_debounce; /* in msecs, for GPIO-driven buttons */
unsigned int irq;
+ unsigned int wakeirq;
unsigned int wakeup_trigger_type;
+
spinlock_t lock;
bool disabled;
bool key_pressed;
@@ -511,6 +513,7 @@ static int gpio_keys_setup_key(struct platform_device *pdev,
struct gpio_button_data *bdata = &ddata->data[idx];
irq_handler_t isr;
unsigned long irqflags;
+ const char *wakedesc;
int irq;
int error;
@@ -575,6 +578,14 @@ static int gpio_keys_setup_key(struct platform_device *pdev,
!gpiod_cansleep(bdata->gpiod);
}
+ /*
+ * If an interrupt was specified, use it instead of the gpio
+ * interrupt and use the gpio for reading the state. A separate
+ * interrupt may be used as the main button interrupt for
+ * runtime PM to detect events also in deeper idle states. If a
+ * dedicated wakeirq is used for system suspend only, see below
+ * for bdata->wakeirq setup.
+ */
if (button->irq) {
bdata->irq = button->irq;
} else {
@@ -672,6 +683,36 @@ static int gpio_keys_setup_key(struct platform_device *pdev,
return error;
}
+ if (!button->wakeirq)
+ return 0;
+
+ /* Use :wakeup suffix like drivers/base/power/wakeirq.c does */
+ wakedesc = devm_kasprintf(dev, GFP_KERNEL, "%s:wakeup", desc);
+ if (!wakedesc)
+ return -ENOMEM;
+
+ bdata->wakeirq = button->wakeirq;
+ irqflags |= IRQF_NO_SUSPEND;
+
+ /*
+ * Wakeirq shares the handler with the main interrupt, it's only
+ * active during system suspend. See gpio_keys_button_enable_wakeup()
+ * and gpio_keys_button_disable_wakeup().
+ */
+ error = devm_request_any_context_irq(dev, bdata->wakeirq, isr,
+ irqflags, wakedesc, bdata);
+ if (error < 0) {
+ dev_err(dev, "Unable to claim wakeirq %d; error %d\n",
+ bdata->irq, error);
+ return error;
+ }
+
+ /*
+ * Disable wakeirq until suspend. IRQF_NO_AUTOEN won't work if
+ * IRQF_SHARED was set based on !button->can_disable.
+ */
+ disable_irq(bdata->wakeirq);
+
return 0;
}
@@ -728,7 +769,7 @@ gpio_keys_get_devtree_pdata(struct device *dev)
struct gpio_keys_platform_data *pdata;
struct gpio_keys_button *button;
struct fwnode_handle *child;
- int nbuttons;
+ int nbuttons, irq;
nbuttons = device_get_child_node_count(dev);
if (nbuttons == 0)
@@ -750,9 +791,19 @@ gpio_keys_get_devtree_pdata(struct device *dev)
device_property_read_string(dev, "label", &pdata->name);
device_for_each_child_node(dev, child) {
- if (is_of_node(child))
- button->irq =
- irq_of_parse_and_map(to_of_node(child), 0);
+ if (is_of_node(child)) {
+ irq = of_irq_get_byname(to_of_node(child), "irq");
+ if (irq > 0)
+ button->irq = irq;
+
+ irq = of_irq_get_byname(to_of_node(child), "wakeup");
+ if (irq > 0)
+ button->wakeirq = irq;
+
+ if (!button->irq && !button->wakeirq)
+ button->irq =
+ irq_of_parse_and_map(to_of_node(child), 0);
+ }
if (fwnode_property_read_u32(child, "linux,code",
&button->code)) {
@@ -921,6 +972,11 @@ gpio_keys_button_enable_wakeup(struct gpio_button_data *bdata)
}
}
+ if (bdata->wakeirq) {
+ enable_irq(bdata->wakeirq);
+ disable_irq(bdata->irq);
+ }
+
return 0;
}
@@ -929,6 +985,11 @@ gpio_keys_button_disable_wakeup(struct gpio_button_data *bdata)
{
int error;
+ if (bdata->wakeirq) {
+ enable_irq(bdata->irq);
+ disable_irq(bdata->wakeirq);
+ }
+
/*
* The trigger type is always both edges for gpio-based keys and we do
* not support changing wakeup trigger for interrupt-based keys.
diff --git a/include/linux/gpio_keys.h b/include/linux/gpio_keys.h
--- a/include/linux/gpio_keys.h
+++ b/include/linux/gpio_keys.h
@@ -21,6 +21,7 @@ struct device;
* disable button via sysfs
* @value: axis value for %EV_ABS
* @irq: Irq number in case of interrupt keys
+ * @wakeirq: Optional dedicated wake-up interrupt
*/
struct gpio_keys_button {
unsigned int code;
@@ -34,6 +35,7 @@ struct gpio_keys_button {
bool can_disable;
int value;
unsigned int irq;
+ unsigned int wakeirq;
};
/**
--
2.42.1
^ permalink raw reply
* [PATCH 0/4] Convert some input drivers to use GPIO descriptors
From: Linus Walleij @ 2023-11-29 13:51 UTC (permalink / raw)
To: Dmitry Torokhov, Tony Lindgren; +Cc: linux-input, Linus Walleij
This clears out some oddities with unused GPIOs, or unused
platform data GPIOs passed around in the input subsystem.
This is done to get rid of bad examples and step-wise make
it possible to remove <linux/gpio.h>, see
drivers/gpio/TODO.
Cc <linux-omap@vger.kernel.org>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
Linus Walleij (4):
Input: navpoint - Convert to use GPIO descriptor
Input: tca6416-keypad - Drop unused include
Input: omap-keypad - Drop optional GPIO support
Input: as5011 - Convert to GPIO descriptor
drivers/input/joystick/as5011.c | 24 +++++++++---------
drivers/input/keyboard/omap-keypad.c | 16 +-----------
drivers/input/keyboard/tca6416-keypad.c | 1 -
drivers/input/mouse/navpoint.c | 41 +++++++++++--------------------
include/linux/input/as5011.h | 1 -
include/linux/input/navpoint.h | 1 -
include/linux/platform_data/keypad-omap.h | 3 ---
7 files changed, 27 insertions(+), 60 deletions(-)
---
base-commit: 226edd1152ffe82c080d8ddf5faef69278447c9b
change-id: 20231129-descriptors-input-0432d7f805a6
Best regards,
--
Linus Walleij <linus.walleij@linaro.org>
^ permalink raw reply
* [PATCH 1/4] Input: navpoint - Convert to use GPIO descriptor
From: Linus Walleij @ 2023-11-29 13:51 UTC (permalink / raw)
To: Dmitry Torokhov, Tony Lindgren; +Cc: linux-input, Linus Walleij
In-Reply-To: <20231129-descriptors-input-v1-0-9433162914a3@linaro.org>
The Navpoint driver uses a GPIO line, convert this to use
a GPIO descriptor. There are no in-kernel users but outoftree
users can easily be added or converted using a GPIO descriptor
table as with numerous other drivers.
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/input/mouse/navpoint.c | 41 +++++++++++++++--------------------------
include/linux/input/navpoint.h | 1 -
2 files changed, 15 insertions(+), 27 deletions(-)
diff --git a/drivers/input/mouse/navpoint.c b/drivers/input/mouse/navpoint.c
index c00dc1275da2..ba757783c258 100644
--- a/drivers/input/mouse/navpoint.c
+++ b/drivers/input/mouse/navpoint.c
@@ -10,7 +10,7 @@
#include <linux/platform_device.h>
#include <linux/clk.h>
#include <linux/delay.h>
-#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
#include <linux/input.h>
#include <linux/input/navpoint.h>
#include <linux/interrupt.h>
@@ -32,7 +32,7 @@ struct navpoint {
struct ssp_device *ssp;
struct input_dev *input;
struct device *dev;
- int gpio;
+ struct gpio_desc *gpiod;
int index;
u8 data[1 + HEADER_LENGTH(0xff)];
};
@@ -170,16 +170,14 @@ static void navpoint_up(struct navpoint *navpoint)
dev_err(navpoint->dev,
"timeout waiting for SSSR[CSS] to clear\n");
- if (gpio_is_valid(navpoint->gpio))
- gpio_set_value(navpoint->gpio, 1);
+ gpiod_set_value(navpoint->gpiod, 1);
}
static void navpoint_down(struct navpoint *navpoint)
{
struct ssp_device *ssp = navpoint->ssp;
- if (gpio_is_valid(navpoint->gpio))
- gpio_set_value(navpoint->gpio, 0);
+ gpiod_set_value(navpoint->gpiod, 0);
pxa_ssp_write_reg(ssp, SSCR0, 0);
@@ -216,18 +214,9 @@ static int navpoint_probe(struct platform_device *pdev)
return -EINVAL;
}
- if (gpio_is_valid(pdata->gpio)) {
- error = gpio_request_one(pdata->gpio, GPIOF_OUT_INIT_LOW,
- "SYNAPTICS_ON");
- if (error)
- return error;
- }
-
ssp = pxa_ssp_request(pdata->port, pdev->name);
- if (!ssp) {
- error = -ENODEV;
- goto err_free_gpio;
- }
+ if (!ssp)
+ return -ENODEV;
/* HaRET does not disable devices before jumping into Linux */
if (pxa_ssp_read_reg(ssp, SSCR0) & SSCR0_SSE) {
@@ -242,10 +231,18 @@ static int navpoint_probe(struct platform_device *pdev)
goto err_free_mem;
}
+ navpoint->gpiod = gpiod_get_optional(&pdev->dev,
+ NULL, GPIOD_OUT_LOW);
+ if (IS_ERR(navpoint->gpiod)) {
+ error = PTR_ERR(navpoint->gpiod);
+ dev_err(&pdev->dev, "error getting GPIO\n");
+ goto err_free_mem;
+ }
+ gpiod_set_consumer_name(navpoint->gpiod, "SYNAPTICS_ON");
+
navpoint->ssp = ssp;
navpoint->input = input;
navpoint->dev = &pdev->dev;
- navpoint->gpio = pdata->gpio;
input->name = pdev->name;
input->dev.parent = &pdev->dev;
@@ -288,17 +285,12 @@ static int navpoint_probe(struct platform_device *pdev)
input_free_device(input);
kfree(navpoint);
pxa_ssp_free(ssp);
-err_free_gpio:
- if (gpio_is_valid(pdata->gpio))
- gpio_free(pdata->gpio);
return error;
}
static void navpoint_remove(struct platform_device *pdev)
{
- const struct navpoint_platform_data *pdata =
- dev_get_platdata(&pdev->dev);
struct navpoint *navpoint = platform_get_drvdata(pdev);
struct ssp_device *ssp = navpoint->ssp;
@@ -308,9 +300,6 @@ static void navpoint_remove(struct platform_device *pdev)
kfree(navpoint);
pxa_ssp_free(ssp);
-
- if (gpio_is_valid(pdata->gpio))
- gpio_free(pdata->gpio);
}
static int navpoint_suspend(struct device *dev)
diff --git a/include/linux/input/navpoint.h b/include/linux/input/navpoint.h
index d464ffb4db52..5192ae3f5ec1 100644
--- a/include/linux/input/navpoint.h
+++ b/include/linux/input/navpoint.h
@@ -5,5 +5,4 @@
struct navpoint_platform_data {
int port; /* PXA SSP port for pxa_ssp_request() */
- int gpio; /* GPIO for power on/off */
};
--
2.34.1
^ permalink raw reply related
* [PATCH 2/4] Input: tca6416-keypad - Drop unused include
From: Linus Walleij @ 2023-11-29 13:51 UTC (permalink / raw)
To: Dmitry Torokhov, Tony Lindgren; +Cc: linux-input, Linus Walleij
In-Reply-To: <20231129-descriptors-input-v1-0-9433162914a3@linaro.org>
The TCA6416 keypad driver is including the legacy GPIO
header <linux/gpio.h> for no reason, it is not using any
of its symbols. Drop the header.
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/input/keyboard/tca6416-keypad.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/input/keyboard/tca6416-keypad.c b/drivers/input/keyboard/tca6416-keypad.c
index 8af59ced1ec2..677bc4baa5d1 100644
--- a/drivers/input/keyboard/tca6416-keypad.c
+++ b/drivers/input/keyboard/tca6416-keypad.c
@@ -14,7 +14,6 @@
#include <linux/slab.h>
#include <linux/interrupt.h>
#include <linux/workqueue.h>
-#include <linux/gpio.h>
#include <linux/i2c.h>
#include <linux/input.h>
#include <linux/tca6416_keypad.h>
--
2.34.1
^ permalink raw reply related
* [PATCH 3/4] Input: omap-keypad - Drop optional GPIO support
From: Linus Walleij @ 2023-11-29 13:51 UTC (permalink / raw)
To: Dmitry Torokhov, Tony Lindgren; +Cc: linux-input, Linus Walleij
In-Reply-To: <20231129-descriptors-input-v1-0-9433162914a3@linaro.org>
The driver supports passing some GPIO lines for rows and columns
through the driver data, but there is no in-kernel user of this.
Further the use seems convoluted because the GPIO lines are unused
in the driver, then explicitly free:ed when removing it without
being requested when probing it, which is assymetric and just
a recepie for disaster.
Remove the support for these unused GPIOs, if need be support can
be reestablished in an organized fashion using GPIO descriptors.
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/input/keyboard/omap-keypad.c | 16 +---------------
include/linux/platform_data/keypad-omap.h | 3 ---
2 files changed, 1 insertion(+), 18 deletions(-)
diff --git a/drivers/input/keyboard/omap-keypad.c b/drivers/input/keyboard/omap-keypad.c
index 454fb8675657..99023b9de35f 100644
--- a/drivers/input/keyboard/omap-keypad.c
+++ b/drivers/input/keyboard/omap-keypad.c
@@ -21,7 +21,6 @@
#include <linux/mutex.h>
#include <linux/errno.h>
#include <linux/slab.h>
-#include <linux/gpio.h>
#include <linux/platform_data/gpio-omap.h>
#include <linux/platform_data/keypad-omap.h>
#include <linux/soc/ti/omap1-io.h>
@@ -49,9 +48,6 @@ struct omap_kp {
static DECLARE_TASKLET_DISABLED_OLD(kp_tasklet, omap_kp_tasklet);
-static unsigned int *row_gpios;
-static unsigned int *col_gpios;
-
static irqreturn_t omap_kp_interrupt(int irq, void *dev_id)
{
/* disable keyboard interrupt and schedule for handling */
@@ -180,7 +176,7 @@ static int omap_kp_probe(struct platform_device *pdev)
struct omap_kp *omap_kp;
struct input_dev *input_dev;
struct omap_kp_platform_data *pdata = dev_get_platdata(&pdev->dev);
- int i, col_idx, row_idx, ret;
+ int col_idx, row_idx, ret;
unsigned int row_shift, keycodemax;
if (!pdata->rows || !pdata->cols || !pdata->keymap_data) {
@@ -209,11 +205,6 @@ static int omap_kp_probe(struct platform_device *pdev)
if (pdata->delay)
omap_kp->delay = pdata->delay;
- if (pdata->row_gpios && pdata->col_gpios) {
- row_gpios = pdata->row_gpios;
- col_gpios = pdata->col_gpios;
- }
-
omap_kp->rows = pdata->rows;
omap_kp->cols = pdata->cols;
@@ -276,11 +267,6 @@ static int omap_kp_probe(struct platform_device *pdev)
err3:
device_remove_file(&pdev->dev, &dev_attr_enable);
err2:
- for (i = row_idx - 1; i >= 0; i--)
- gpio_free(row_gpios[i]);
- for (i = col_idx - 1; i >= 0; i--)
- gpio_free(col_gpios[i]);
-
kfree(omap_kp);
input_free_device(input_dev);
diff --git a/include/linux/platform_data/keypad-omap.h b/include/linux/platform_data/keypad-omap.h
index 3e7c64c854f4..f3f1311cdf3a 100644
--- a/include/linux/platform_data/keypad-omap.h
+++ b/include/linux/platform_data/keypad-omap.h
@@ -19,9 +19,6 @@ struct omap_kp_platform_data {
bool rep;
unsigned long delay;
bool dbounce;
- /* specific to OMAP242x*/
- unsigned int *row_gpios;
- unsigned int *col_gpios;
};
/* Group (0..3) -- when multiple keys are pressed, only the
--
2.34.1
^ permalink raw reply related
* [PATCH 4/4] Input: as5011 - Convert to GPIO descriptor
From: Linus Walleij @ 2023-11-29 13:51 UTC (permalink / raw)
To: Dmitry Torokhov, Tony Lindgren; +Cc: linux-input, Linus Walleij
In-Reply-To: <20231129-descriptors-input-v1-0-9433162914a3@linaro.org>
This driver does not have any in-tree users but is passing a
legacy GPIO number through platform data.
Convert it to use a GPIO descriptor, new users or outoftree
users can easily be implemented using GPIO descriptor tables
or software nodes.
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/input/joystick/as5011.c | 24 +++++++++++-------------
include/linux/input/as5011.h | 1 -
2 files changed, 11 insertions(+), 14 deletions(-)
diff --git a/drivers/input/joystick/as5011.c b/drivers/input/joystick/as5011.c
index bf8b1cc0ea9c..f1822c19a289 100644
--- a/drivers/input/joystick/as5011.c
+++ b/drivers/input/joystick/as5011.c
@@ -13,7 +13,7 @@
#include <linux/i2c.h>
#include <linux/interrupt.h>
#include <linux/input.h>
-#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
#include <linux/delay.h>
#include <linux/input/as5011.h>
#include <linux/slab.h>
@@ -61,7 +61,7 @@ MODULE_LICENSE("GPL");
struct as5011_device {
struct input_dev *input_dev;
struct i2c_client *i2c_client;
- unsigned int button_gpio;
+ struct gpio_desc *button_gpiod;
unsigned int button_irq;
unsigned int axis_irq;
};
@@ -114,7 +114,7 @@ static int as5011_i2c_read(struct i2c_client *client,
static irqreturn_t as5011_button_interrupt(int irq, void *dev_id)
{
struct as5011_device *as5011 = dev_id;
- int val = gpio_get_value_cansleep(as5011->button_gpio);
+ int val = gpiod_get_value_cansleep(as5011->button_gpiod);
input_report_key(as5011->input_dev, BTN_JOYSTICK, !val);
input_sync(as5011->input_dev);
@@ -248,7 +248,6 @@ static int as5011_probe(struct i2c_client *client)
as5011->i2c_client = client;
as5011->input_dev = input_dev;
- as5011->button_gpio = plat_data->button_gpio;
as5011->axis_irq = plat_data->axis_irq;
input_dev->name = "Austria Microsystem as5011 joystick";
@@ -262,18 +261,20 @@ static int as5011_probe(struct i2c_client *client)
input_set_abs_params(as5011->input_dev, ABS_Y,
AS5011_MIN_AXIS, AS5011_MAX_AXIS, AS5011_FUZZ, AS5011_FLAT);
- error = gpio_request(as5011->button_gpio, "AS5011 button");
- if (error < 0) {
- dev_err(&client->dev, "Failed to request button gpio\n");
+ as5011->button_gpiod = devm_gpiod_get(&client->dev, NULL, GPIOD_IN);
+ if (IS_ERR(as5011->button_gpiod)) {
+ error = PTR_ERR(as5011->button_gpiod);
+ dev_err(&client->dev, "Failed to request button GPIO\n");
goto err_free_mem;
}
+ gpiod_set_consumer_name(as5011->button_gpiod, "AS5011 button");
- irq = gpio_to_irq(as5011->button_gpio);
+ irq = gpiod_to_irq(as5011->button_gpiod);
if (irq < 0) {
dev_err(&client->dev,
"Failed to get irq number for button gpio\n");
error = irq;
- goto err_free_button_gpio;
+ goto err_free_mem;
}
as5011->button_irq = irq;
@@ -286,7 +287,7 @@ static int as5011_probe(struct i2c_client *client)
if (error < 0) {
dev_err(&client->dev,
"Can't allocate button irq %d\n", as5011->button_irq);
- goto err_free_button_gpio;
+ goto err_free_mem;
}
error = as5011_configure_chip(as5011, plat_data);
@@ -317,8 +318,6 @@ static int as5011_probe(struct i2c_client *client)
free_irq(as5011->axis_irq, as5011);
err_free_button_irq:
free_irq(as5011->button_irq, as5011);
-err_free_button_gpio:
- gpio_free(as5011->button_gpio);
err_free_mem:
input_free_device(input_dev);
kfree(as5011);
@@ -332,7 +331,6 @@ static void as5011_remove(struct i2c_client *client)
free_irq(as5011->axis_irq, as5011);
free_irq(as5011->button_irq, as5011);
- gpio_free(as5011->button_gpio);
input_unregister_device(as5011->input_dev);
kfree(as5011);
diff --git a/include/linux/input/as5011.h b/include/linux/input/as5011.h
index 5fba52a56cd6..5705d5de3aea 100644
--- a/include/linux/input/as5011.h
+++ b/include/linux/input/as5011.h
@@ -7,7 +7,6 @@
*/
struct as5011_platform_data {
- unsigned int button_gpio;
unsigned int axis_irq; /* irq number */
unsigned long axis_irqflags;
char xp, xn; /* threshold for x axis */
--
2.34.1
^ permalink raw reply related
* [PATCH 00/12] selftests/hid: tablets fixes
From: Benjamin Tissoires @ 2023-11-29 15:24 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Shuah Khan, Peter Hutterer
Cc: linux-input, linux-kselftest, linux-kernel, Benjamin Tissoires
Hi,
the main trigger of this series was the XP-Pen issue[0].
Basically, the tablets tests were good-ish but couldn't
handle that tablet both in terms of emulation or in terms
of detection of issues.
So rework the tablets test a bit to be able to include the
XP-Pen patch later, once I have a kernel fix for it (right
now I only have a HID-BPF fix, meaning that the test will
fail if I include them).
Also, vmtest.sh needed a little bit of care, because
boot2container moved, and I made it easier to reuse in a CI
environment.
Cheers,
Benjamin
[0] https://lore.kernel.org/all/nycvar.YFH.7.76.2311012033290.29220@cbobk.fhfr.pm/
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
Benjamin Tissoires (12):
selftests/hid: vmtest.sh: update vm2c and container
selftests/hid: vmtest.sh: allow finer control on the build steps
selftests/hid: base: allow for multiple skip_if_uhdev
selftests/hid: tablets: remove unused class
selftests/hid: tablets: move the transitions to PenState
selftests/hid: tablets: move move_to function to PenDigitizer
selftests/hid: tablets: do not set invert when the eraser is used
selftests/hid: tablets: set initial data for tilt/twist
selftests/hid: tablets: add variants of states with buttons
selftests/hid: tablets: convert the primary button tests
selftests/hid: tablets: add a secondary barrel switch test
selftests/hid: tablets: be stricter for some transitions
tools/testing/selftests/hid/tests/base.py | 3 +-
tools/testing/selftests/hid/tests/test_tablet.py | 727 ++++++++++++++++-------
tools/testing/selftests/hid/vmtest.sh | 46 +-
3 files changed, 525 insertions(+), 251 deletions(-)
---
base-commit: 4ea4ed22b57846facd9cb4af5f67cb7bd2792cf3
change-id: 20231121-wip-selftests-001ac427e086
Best regards,
--
Benjamin Tissoires <bentiss@kernel.org>
^ permalink raw reply
* [PATCH 01/12] selftests/hid: vmtest.sh: update vm2c and container
From: Benjamin Tissoires @ 2023-11-29 15:24 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Shuah Khan, Peter Hutterer
Cc: linux-input, linux-kselftest, linux-kernel, Benjamin Tissoires
In-Reply-To: <20231129-wip-selftests-v1-0-ba15a1fe1b0d@kernel.org>
boot2container is now on an official project, so let's use that.
The container image is now the same I use for the CI, so let's keep
to it.
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
tools/testing/selftests/hid/vmtest.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/hid/vmtest.sh b/tools/testing/selftests/hid/vmtest.sh
index 4da48bf6b328..301b4e162336 100755
--- a/tools/testing/selftests/hid/vmtest.sh
+++ b/tools/testing/selftests/hid/vmtest.sh
@@ -19,12 +19,12 @@ esac
SCRIPT_DIR="$(dirname $(realpath $0))"
OUTPUT_DIR="$SCRIPT_DIR/results"
KCONFIG_REL_PATHS=("${SCRIPT_DIR}/config" "${SCRIPT_DIR}/config.common" "${SCRIPT_DIR}/config.${ARCH}")
-B2C_URL="https://gitlab.freedesktop.org/mupuf/boot2container/-/raw/master/vm2c.py"
+B2C_URL="https://gitlab.freedesktop.org/gfx-ci/boot2container/-/raw/main/vm2c.py"
NUM_COMPILE_JOBS="$(nproc)"
LOG_FILE_BASE="$(date +"hid_selftests.%Y-%m-%d_%H-%M-%S")"
LOG_FILE="${LOG_FILE_BASE}.log"
EXIT_STATUS_FILE="${LOG_FILE_BASE}.exit_status"
-CONTAINER_IMAGE="registry.freedesktop.org/libevdev/hid-tools/fedora/37:2023-02-17.1"
+CONTAINER_IMAGE="registry.freedesktop.org/bentiss/hid/fedora/39:2023-11-22.1"
TARGETS="${TARGETS:=$(basename ${SCRIPT_DIR})}"
DEFAULT_COMMAND="pip3 install hid-tools; make -C tools/testing/selftests TARGETS=${TARGETS} run_tests"
--
2.41.0
^ permalink raw reply related
* [PATCH 02/12] selftests/hid: vmtest.sh: allow finer control on the build steps
From: Benjamin Tissoires @ 2023-11-29 15:24 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Shuah Khan, Peter Hutterer
Cc: linux-input, linux-kselftest, linux-kernel, Benjamin Tissoires
In-Reply-To: <20231129-wip-selftests-v1-0-ba15a1fe1b0d@kernel.org>
vmtest.sh works great for a one shot test, but not so much for CI where
I want to build (with different configs) the bzImage in a separate
job than the one I am running it.
Add a "build_only" option to specify whether we need to boot the currently
built kernel in the vm.
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
tools/testing/selftests/hid/vmtest.sh | 42 ++++++++++++++++++++---------------
1 file changed, 24 insertions(+), 18 deletions(-)
diff --git a/tools/testing/selftests/hid/vmtest.sh b/tools/testing/selftests/hid/vmtest.sh
index 301b4e162336..52ada972833b 100755
--- a/tools/testing/selftests/hid/vmtest.sh
+++ b/tools/testing/selftests/hid/vmtest.sh
@@ -32,7 +32,7 @@ DEFAULT_COMMAND="pip3 install hid-tools; make -C tools/testing/selftests TARGETS
usage()
{
cat <<EOF
-Usage: $0 [-i] [-s] [-d <output_dir>] -- [<command>]
+Usage: $0 [-j N] [-s] [-b] [-d <output_dir>] -- [<command>]
<command> is the command you would normally run when you are in
the source kernel direcory. e.g:
@@ -55,6 +55,7 @@ Options:
-u) Update the boot2container script to a newer version.
-d) Update the output directory (default: ${OUTPUT_DIR})
+ -b) Run the only build steps for the kernel and the selftests
-j) Number of jobs for compilation, similar to -j in make
(default: ${NUM_COMPILE_JOBS})
-s) Instead of powering off the VM, start an interactive
@@ -191,8 +192,9 @@ main()
local command="${DEFAULT_COMMAND}"
local update_b2c="no"
local debug_shell="no"
+ local build_only="no"
- while getopts ':hsud:j:' opt; do
+ while getopts ':hsud:j:b' opt; do
case ${opt} in
u)
update_b2c="yes"
@@ -207,6 +209,9 @@ main()
command="/bin/sh"
debug_shell="yes"
;;
+ b)
+ build_only="yes"
+ ;;
h)
usage
exit 0
@@ -226,8 +231,7 @@ main()
shift $((OPTIND -1))
# trap 'catch "$?"' EXIT
-
- if [[ "${debug_shell}" == "no" ]]; then
+ if [[ "${build_only}" == "no" && "${debug_shell}" == "no" ]]; then
if [[ $# -eq 0 ]]; then
echo "No command specified, will run ${DEFAULT_COMMAND} in the vm"
else
@@ -267,24 +271,26 @@ main()
update_kconfig "${kernel_checkout}" "${kconfig_file}"
recompile_kernel "${kernel_checkout}" "${make_command}"
+ update_selftests "${kernel_checkout}" "${make_command}"
- if [[ "${update_b2c}" == "no" && ! -f "${b2c}" ]]; then
- echo "vm2c script not found in ${b2c}"
- update_b2c="yes"
- fi
+ if [[ "${build_only}" == "no" ]]; then
+ if [[ "${update_b2c}" == "no" && ! -f "${b2c}" ]]; then
+ echo "vm2c script not found in ${b2c}"
+ update_b2c="yes"
+ fi
- if [[ "${update_b2c}" == "yes" ]]; then
- download $B2C_URL $b2c
- chmod +x $b2c
- fi
+ if [[ "${update_b2c}" == "yes" ]]; then
+ download $B2C_URL $b2c
+ chmod +x $b2c
+ fi
- update_selftests "${kernel_checkout}" "${make_command}"
- run_vm "${kernel_checkout}" $b2c "${kernel_bzimage}" "${command}"
- if [[ "${debug_shell}" != "yes" ]]; then
- echo "Logs saved in ${OUTPUT_DIR}/${LOG_FILE}"
- fi
+ run_vm "${kernel_checkout}" $b2c "${kernel_bzimage}" "${command}"
+ if [[ "${debug_shell}" != "yes" ]]; then
+ echo "Logs saved in ${OUTPUT_DIR}/${LOG_FILE}"
+ fi
- exit $(cat ${OUTPUT_DIR}/${EXIT_STATUS_FILE})
+ exit $(cat ${OUTPUT_DIR}/${EXIT_STATUS_FILE})
+ fi
}
main "$@"
--
2.41.0
^ permalink raw reply related
* [PATCH 03/12] selftests/hid: base: allow for multiple skip_if_uhdev
From: Benjamin Tissoires @ 2023-11-29 15:24 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Shuah Khan, Peter Hutterer
Cc: linux-input, linux-kselftest, linux-kernel, Benjamin Tissoires
In-Reply-To: <20231129-wip-selftests-v1-0-ba15a1fe1b0d@kernel.org>
We can actually have multiple occurences of `skip_if_uhdev` if we follow
the information from the pytest doc[0].
This is not immediately used, but can be if we need multiple conditions
on a given test.
[0] https://docs.pytest.org/en/latest/historical-notes.html#update-marker-code
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
tools/testing/selftests/hid/tests/base.py | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/tools/testing/selftests/hid/tests/base.py b/tools/testing/selftests/hid/tests/base.py
index 1305cfc9646e..5d9c26dfc460 100644
--- a/tools/testing/selftests/hid/tests/base.py
+++ b/tools/testing/selftests/hid/tests/base.py
@@ -238,8 +238,7 @@ class BaseTestCase:
try:
with HIDTestUdevRule.instance():
with new_uhdev as self.uhdev:
- skip_cond = request.node.get_closest_marker("skip_if_uhdev")
- if skip_cond:
+ for skip_cond in request.node.iter_markers("skip_if_uhdev"):
test, message, *rest = skip_cond.args
if test(self.uhdev):
--
2.41.0
^ permalink raw reply related
* [PATCH 04/12] selftests/hid: tablets: remove unused class
From: Benjamin Tissoires @ 2023-11-29 15:24 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Shuah Khan, Peter Hutterer
Cc: linux-input, linux-kselftest, linux-kernel, Benjamin Tissoires
In-Reply-To: <20231129-wip-selftests-v1-0-ba15a1fe1b0d@kernel.org>
Looks like this is a leftover
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
tools/testing/selftests/hid/tests/test_tablet.py | 4 ----
1 file changed, 4 deletions(-)
diff --git a/tools/testing/selftests/hid/tests/test_tablet.py b/tools/testing/selftests/hid/tests/test_tablet.py
index 303ffff9ee8d..cd9c1269afa6 100644
--- a/tools/testing/selftests/hid/tests/test_tablet.py
+++ b/tools/testing/selftests/hid/tests/test_tablet.py
@@ -133,10 +133,6 @@ class PenState(Enum):
return tuple()
-class Data(object):
- pass
-
-
class Pen(object):
def __init__(self, x, y):
self.x = x
--
2.41.0
^ permalink raw reply related
* [PATCH 05/12] selftests/hid: tablets: move the transitions to PenState
From: Benjamin Tissoires @ 2023-11-29 15:24 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Shuah Khan, Peter Hutterer
Cc: linux-input, linux-kselftest, linux-kernel, Benjamin Tissoires
In-Reply-To: <20231129-wip-selftests-v1-0-ba15a1fe1b0d@kernel.org>
Those transitions have nothing to do with `Pen`, so migrate them to
`PenState`.
The hidden agenda is to remove `Pen` and integrate it into `PenDigitizer`
so that we can tweak the events in each state to emulate firmware bugs.
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
tools/testing/selftests/hid/tests/test_tablet.py | 212 +++++++++++------------
1 file changed, 106 insertions(+), 106 deletions(-)
diff --git a/tools/testing/selftests/hid/tests/test_tablet.py b/tools/testing/selftests/hid/tests/test_tablet.py
index cd9c1269afa6..18961758e4aa 100644
--- a/tools/testing/selftests/hid/tests/test_tablet.py
+++ b/tools/testing/selftests/hid/tests/test_tablet.py
@@ -132,104 +132,8 @@ class PenState(Enum):
return tuple()
-
-class Pen(object):
- def __init__(self, x, y):
- self.x = x
- self.y = y
- self.tipswitch = False
- self.tippressure = 15
- self.azimuth = 0
- self.inrange = False
- self.width = 10
- self.height = 10
- self.barrelswitch = False
- self.invert = False
- self.eraser = False
- self.x_tilt = 0
- self.y_tilt = 0
- self.twist = 0
- self._old_values = None
- self.current_state = None
-
- def _restore(self):
- if self._old_values is not None:
- for i in [
- "x",
- "y",
- "tippressure",
- "azimuth",
- "width",
- "height",
- "twist",
- "x_tilt",
- "y_tilt",
- ]:
- setattr(self, i, getattr(self._old_values, i))
-
- def move_to(self, state):
- # fill in the previous values
- if self.current_state == PenState.PEN_IS_OUT_OF_RANGE:
- self._restore()
-
- print(f"\n *** pen is moving to {state} ***")
-
- if state == PenState.PEN_IS_OUT_OF_RANGE:
- self._old_values = copy.copy(self)
- self.x = 0
- self.y = 0
- self.tipswitch = False
- self.tippressure = 0
- self.azimuth = 0
- self.inrange = False
- self.width = 0
- self.height = 0
- self.invert = False
- self.eraser = False
- self.x_tilt = 0
- self.y_tilt = 0
- self.twist = 0
- elif state == PenState.PEN_IS_IN_RANGE:
- self.tipswitch = False
- self.inrange = True
- self.invert = False
- self.eraser = False
- elif state == PenState.PEN_IS_IN_CONTACT:
- self.tipswitch = True
- self.inrange = True
- self.invert = False
- self.eraser = False
- elif state == PenState.PEN_IS_IN_RANGE_WITH_ERASING_INTENT:
- self.tipswitch = False
- self.inrange = True
- self.invert = True
- self.eraser = False
- elif state == PenState.PEN_IS_ERASING:
- self.tipswitch = False
- self.inrange = True
- self.invert = True
- self.eraser = True
-
- self.current_state = state
-
- def __assert_axis(self, evdev, axis, value):
- if (
- axis == libevdev.EV_KEY.BTN_TOOL_RUBBER
- and evdev.value[libevdev.EV_KEY.BTN_TOOL_RUBBER] is None
- ):
- return
-
- assert (
- evdev.value[axis] == value
- ), f"assert evdev.value[{axis}] ({evdev.value[axis]}) != {value}"
-
- def assert_expected_input_events(self, evdev):
- assert evdev.value[libevdev.EV_ABS.ABS_X] == self.x
- assert evdev.value[libevdev.EV_ABS.ABS_Y] == self.y
- assert self.current_state == PenState.from_evdev(evdev)
-
@staticmethod
- def legal_transitions() -> Dict[str, Tuple[PenState, ...]]:
+ def legal_transitions() -> Dict[str, Tuple["PenState", ...]]:
"""This is the first half of the Windows Pen Implementation state machine:
we don't have Invert nor Erase bits, so just move in/out-of-range or proximity.
https://docs.microsoft.com/en-us/windows-hardware/design/component-guidelines/windows-pen-states
@@ -255,7 +159,7 @@ class Pen(object):
}
@staticmethod
- def legal_transitions_with_invert() -> Dict[str, Tuple[PenState, ...]]:
+ def legal_transitions_with_invert() -> Dict[str, Tuple["PenState", ...]]:
"""This is the second half of the Windows Pen Implementation state machine:
we now have Invert and Erase bits, so move in/out or proximity with the intend
to erase.
@@ -293,7 +197,7 @@ class Pen(object):
}
@staticmethod
- def tolerated_transitions() -> Dict[str, Tuple[PenState, ...]]:
+ def tolerated_transitions() -> Dict[str, Tuple["PenState", ...]]:
"""This is not adhering to the Windows Pen Implementation state machine
but we should expect the kernel to behave properly, mostly for historical
reasons."""
@@ -306,7 +210,7 @@ class Pen(object):
}
@staticmethod
- def tolerated_transitions_with_invert() -> Dict[str, Tuple[PenState, ...]]:
+ def tolerated_transitions_with_invert() -> Dict[str, Tuple["PenState", ...]]:
"""This is the second half of the Windows Pen Implementation state machine:
we now have Invert and Erase bits, so move in/out or proximity with the intend
to erase.
@@ -321,7 +225,7 @@ class Pen(object):
}
@staticmethod
- def broken_transitions() -> Dict[str, Tuple[PenState, ...]]:
+ def broken_transitions() -> Dict[str, Tuple["PenState", ...]]:
"""Those tests are definitely not part of the Windows specification.
However, a half broken device might export those transitions.
For example, a pen that has the eraser button might wobble between
@@ -359,6 +263,102 @@ class Pen(object):
}
+class Pen(object):
+ def __init__(self, x, y):
+ self.x = x
+ self.y = y
+ self.tipswitch = False
+ self.tippressure = 15
+ self.azimuth = 0
+ self.inrange = False
+ self.width = 10
+ self.height = 10
+ self.barrelswitch = False
+ self.invert = False
+ self.eraser = False
+ self.x_tilt = 0
+ self.y_tilt = 0
+ self.twist = 0
+ self._old_values = None
+ self.current_state = None
+
+ def _restore(self):
+ if self._old_values is not None:
+ for i in [
+ "x",
+ "y",
+ "tippressure",
+ "azimuth",
+ "width",
+ "height",
+ "twist",
+ "x_tilt",
+ "y_tilt",
+ ]:
+ setattr(self, i, getattr(self._old_values, i))
+
+ def move_to(self, state):
+ # fill in the previous values
+ if self.current_state == PenState.PEN_IS_OUT_OF_RANGE:
+ self._restore()
+
+ print(f"\n *** pen is moving to {state} ***")
+
+ if state == PenState.PEN_IS_OUT_OF_RANGE:
+ self._old_values = copy.copy(self)
+ self.x = 0
+ self.y = 0
+ self.tipswitch = False
+ self.tippressure = 0
+ self.azimuth = 0
+ self.inrange = False
+ self.width = 0
+ self.height = 0
+ self.invert = False
+ self.eraser = False
+ self.x_tilt = 0
+ self.y_tilt = 0
+ self.twist = 0
+ elif state == PenState.PEN_IS_IN_RANGE:
+ self.tipswitch = False
+ self.inrange = True
+ self.invert = False
+ self.eraser = False
+ elif state == PenState.PEN_IS_IN_CONTACT:
+ self.tipswitch = True
+ self.inrange = True
+ self.invert = False
+ self.eraser = False
+ elif state == PenState.PEN_IS_IN_RANGE_WITH_ERASING_INTENT:
+ self.tipswitch = False
+ self.inrange = True
+ self.invert = True
+ self.eraser = False
+ elif state == PenState.PEN_IS_ERASING:
+ self.tipswitch = False
+ self.inrange = True
+ self.invert = True
+ self.eraser = True
+
+ self.current_state = state
+
+ def __assert_axis(self, evdev, axis, value):
+ if (
+ axis == libevdev.EV_KEY.BTN_TOOL_RUBBER
+ and evdev.value[libevdev.EV_KEY.BTN_TOOL_RUBBER] is None
+ ):
+ return
+
+ assert (
+ evdev.value[axis] == value
+ ), f"assert evdev.value[{axis}] ({evdev.value[axis]}) != {value}"
+
+ def assert_expected_input_events(self, evdev):
+ assert evdev.value[libevdev.EV_ABS.ABS_X] == self.x
+ assert evdev.value[libevdev.EV_ABS.ABS_Y] == self.y
+ assert self.current_state == PenState.from_evdev(evdev)
+
+
class PenDigitizer(base.UHIDTestDevice):
def __init__(
self,
@@ -486,7 +486,7 @@ class BaseTest:
@pytest.mark.parametrize("scribble", [True, False], ids=["scribble", "static"])
@pytest.mark.parametrize(
"state_list",
- [pytest.param(v, id=k) for k, v in Pen.legal_transitions().items()],
+ [pytest.param(v, id=k) for k, v in PenState.legal_transitions().items()],
)
def test_valid_pen_states(self, state_list, scribble):
"""This is the first half of the Windows Pen Implementation state machine:
@@ -498,7 +498,7 @@ class BaseTest:
@pytest.mark.parametrize("scribble", [True, False], ids=["scribble", "static"])
@pytest.mark.parametrize(
"state_list",
- [pytest.param(v, id=k) for k, v in Pen.tolerated_transitions().items()],
+ [pytest.param(v, id=k) for k, v in PenState.tolerated_transitions().items()],
)
def test_tolerated_pen_states(self, state_list, scribble):
"""This is not adhering to the Windows Pen Implementation state machine
@@ -515,7 +515,7 @@ class BaseTest:
"state_list",
[
pytest.param(v, id=k)
- for k, v in Pen.legal_transitions_with_invert().items()
+ for k, v in PenState.legal_transitions_with_invert().items()
],
)
def test_valid_invert_pen_states(self, state_list, scribble):
@@ -535,7 +535,7 @@ class BaseTest:
"state_list",
[
pytest.param(v, id=k)
- for k, v in Pen.tolerated_transitions_with_invert().items()
+ for k, v in PenState.tolerated_transitions_with_invert().items()
],
)
def test_tolerated_invert_pen_states(self, state_list, scribble):
@@ -553,7 +553,7 @@ class BaseTest:
@pytest.mark.parametrize("scribble", [True, False], ids=["scribble", "static"])
@pytest.mark.parametrize(
"state_list",
- [pytest.param(v, id=k) for k, v in Pen.broken_transitions().items()],
+ [pytest.param(v, id=k) for k, v in PenState.broken_transitions().items()],
)
def test_tolerated_broken_pen_states(self, state_list, scribble):
"""Those tests are definitely not part of the Windows specification.
--
2.41.0
^ permalink raw reply related
* [PATCH 06/12] selftests/hid: tablets: move move_to function to PenDigitizer
From: Benjamin Tissoires @ 2023-11-29 15:24 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Shuah Khan, Peter Hutterer
Cc: linux-input, linux-kselftest, linux-kernel, Benjamin Tissoires
In-Reply-To: <20231129-wip-selftests-v1-0-ba15a1fe1b0d@kernel.org>
We can easily subclass PenDigitizer for introducing firmware bugs when
subclassing Pen is harder.
Move move_to from Pen to PenDigitizer so we get that ability
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
tools/testing/selftests/hid/tests/test_tablet.py | 97 ++++++++++++------------
1 file changed, 50 insertions(+), 47 deletions(-)
diff --git a/tools/testing/selftests/hid/tests/test_tablet.py b/tools/testing/selftests/hid/tests/test_tablet.py
index 18961758e4aa..44a004ca69d1 100644
--- a/tools/testing/selftests/hid/tests/test_tablet.py
+++ b/tools/testing/selftests/hid/tests/test_tablet.py
@@ -282,7 +282,7 @@ class Pen(object):
self._old_values = None
self.current_state = None
- def _restore(self):
+ def restore(self):
if self._old_values is not None:
for i in [
"x",
@@ -297,50 +297,8 @@ class Pen(object):
]:
setattr(self, i, getattr(self._old_values, i))
- def move_to(self, state):
- # fill in the previous values
- if self.current_state == PenState.PEN_IS_OUT_OF_RANGE:
- self._restore()
-
- print(f"\n *** pen is moving to {state} ***")
-
- if state == PenState.PEN_IS_OUT_OF_RANGE:
- self._old_values = copy.copy(self)
- self.x = 0
- self.y = 0
- self.tipswitch = False
- self.tippressure = 0
- self.azimuth = 0
- self.inrange = False
- self.width = 0
- self.height = 0
- self.invert = False
- self.eraser = False
- self.x_tilt = 0
- self.y_tilt = 0
- self.twist = 0
- elif state == PenState.PEN_IS_IN_RANGE:
- self.tipswitch = False
- self.inrange = True
- self.invert = False
- self.eraser = False
- elif state == PenState.PEN_IS_IN_CONTACT:
- self.tipswitch = True
- self.inrange = True
- self.invert = False
- self.eraser = False
- elif state == PenState.PEN_IS_IN_RANGE_WITH_ERASING_INTENT:
- self.tipswitch = False
- self.inrange = True
- self.invert = True
- self.eraser = False
- elif state == PenState.PEN_IS_ERASING:
- self.tipswitch = False
- self.inrange = True
- self.invert = True
- self.eraser = True
-
- self.current_state = state
+ def backup(self):
+ self._old_values = copy.copy(self)
def __assert_axis(self, evdev, axis, value):
if (
@@ -384,6 +342,51 @@ class PenDigitizer(base.UHIDTestDevice):
continue
self.fields = [f.usage_name for f in r]
+ def move_to(self, pen, state):
+ # fill in the previous values
+ if pen.current_state == PenState.PEN_IS_OUT_OF_RANGE:
+ pen.restore()
+
+ print(f"\n *** pen is moving to {state} ***")
+
+ if state == PenState.PEN_IS_OUT_OF_RANGE:
+ pen.backup()
+ pen.x = 0
+ pen.y = 0
+ pen.tipswitch = False
+ pen.tippressure = 0
+ pen.azimuth = 0
+ pen.inrange = False
+ pen.width = 0
+ pen.height = 0
+ pen.invert = False
+ pen.eraser = False
+ pen.x_tilt = 0
+ pen.y_tilt = 0
+ pen.twist = 0
+ elif state == PenState.PEN_IS_IN_RANGE:
+ pen.tipswitch = False
+ pen.inrange = True
+ pen.invert = False
+ pen.eraser = False
+ elif state == PenState.PEN_IS_IN_CONTACT:
+ pen.tipswitch = True
+ pen.inrange = True
+ pen.invert = False
+ pen.eraser = False
+ elif state == PenState.PEN_IS_IN_RANGE_WITH_ERASING_INTENT:
+ pen.tipswitch = False
+ pen.inrange = True
+ pen.invert = True
+ pen.eraser = False
+ elif state == PenState.PEN_IS_ERASING:
+ pen.tipswitch = False
+ pen.inrange = True
+ pen.invert = True
+ pen.eraser = True
+
+ pen.current_state = state
+
def event(self, pen):
rs = []
r = self.create_report(application=self.cur_application, data=pen)
@@ -462,7 +465,7 @@ class BaseTest:
cur_state = PenState.PEN_IS_OUT_OF_RANGE
p = Pen(50, 60)
- p.move_to(PenState.PEN_IS_OUT_OF_RANGE)
+ uhdev.move_to(p, PenState.PEN_IS_OUT_OF_RANGE)
events = self.post(uhdev, p)
self.validate_transitions(cur_state, p, evdev, events)
@@ -475,7 +478,7 @@ class BaseTest:
events = self.post(uhdev, p)
self.validate_transitions(cur_state, p, evdev, events)
assert len(events) >= 3 # X, Y, SYN
- p.move_to(state)
+ uhdev.move_to(p, state)
if scribble and state != PenState.PEN_IS_OUT_OF_RANGE:
p.x += 1
p.y -= 1
--
2.41.0
^ permalink raw reply related
* [PATCH 07/12] selftests/hid: tablets: do not set invert when the eraser is used
From: Benjamin Tissoires @ 2023-11-29 15:24 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Shuah Khan, Peter Hutterer
Cc: linux-input, linux-kselftest, linux-kernel, Benjamin Tissoires
In-Reply-To: <20231129-wip-selftests-v1-0-ba15a1fe1b0d@kernel.org>
Turns out that the chart from Microsoft is not exactly what I got here:
when the rubber is used, and is touching the surface, invert can (should)
be set to 0...
[0] https://learn.microsoft.com/en-us/windows-hardware/design/component-guidelines/windows-pen-states
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
tools/testing/selftests/hid/tests/test_tablet.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/hid/tests/test_tablet.py b/tools/testing/selftests/hid/tests/test_tablet.py
index 44a004ca69d1..f93dfbb2a3e5 100644
--- a/tools/testing/selftests/hid/tests/test_tablet.py
+++ b/tools/testing/selftests/hid/tests/test_tablet.py
@@ -382,7 +382,7 @@ class PenDigitizer(base.UHIDTestDevice):
elif state == PenState.PEN_IS_ERASING:
pen.tipswitch = False
pen.inrange = True
- pen.invert = True
+ pen.invert = False
pen.eraser = True
pen.current_state = state
--
2.41.0
^ permalink raw reply related
* [PATCH 08/12] selftests/hid: tablets: set initial data for tilt/twist
From: Benjamin Tissoires @ 2023-11-29 15:24 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Shuah Khan, Peter Hutterer
Cc: linux-input, linux-kselftest, linux-kernel, Benjamin Tissoires
In-Reply-To: <20231129-wip-selftests-v1-0-ba15a1fe1b0d@kernel.org>
Avoids getting a null event when these usages are set
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
tools/testing/selftests/hid/tests/test_tablet.py | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/tools/testing/selftests/hid/tests/test_tablet.py b/tools/testing/selftests/hid/tests/test_tablet.py
index f93dfbb2a3e5..83f6501fe984 100644
--- a/tools/testing/selftests/hid/tests/test_tablet.py
+++ b/tools/testing/selftests/hid/tests/test_tablet.py
@@ -276,9 +276,9 @@ class Pen(object):
self.barrelswitch = False
self.invert = False
self.eraser = False
- self.x_tilt = 0
- self.y_tilt = 0
- self.twist = 0
+ self.xtilt = 1
+ self.ytilt = 1
+ self.twist = 1
self._old_values = None
self.current_state = None
@@ -292,8 +292,8 @@ class Pen(object):
"width",
"height",
"twist",
- "x_tilt",
- "y_tilt",
+ "xtilt",
+ "ytilt",
]:
setattr(self, i, getattr(self._old_values, i))
@@ -361,8 +361,8 @@ class PenDigitizer(base.UHIDTestDevice):
pen.height = 0
pen.invert = False
pen.eraser = False
- pen.x_tilt = 0
- pen.y_tilt = 0
+ pen.xtilt = 0
+ pen.ytilt = 0
pen.twist = 0
elif state == PenState.PEN_IS_IN_RANGE:
pen.tipswitch = False
--
2.41.0
^ permalink raw reply related
* [PATCH 09/12] selftests/hid: tablets: add variants of states with buttons
From: Benjamin Tissoires @ 2023-11-29 15:24 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Shuah Khan, Peter Hutterer
Cc: linux-input, linux-kselftest, linux-kernel, Benjamin Tissoires
In-Reply-To: <20231129-wip-selftests-v1-0-ba15a1fe1b0d@kernel.org>
Turns out that there are transitions that are unlikely to happen:
for example, having both the tip switch and a button being changed
at the same time (in the same report) would require either a very talented
and precise user or a very bad hardware with a very low sampling rate.
So instead of manually building the button test by hand and forgetting
about some cases, let's reuse the state machine and transitions we have.
This patch only adds the states and the valid transitions. The actual
tests will be replaced later.
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
tools/testing/selftests/hid/tests/test_tablet.py | 170 +++++++++++++++++++++--
1 file changed, 157 insertions(+), 13 deletions(-)
diff --git a/tools/testing/selftests/hid/tests/test_tablet.py b/tools/testing/selftests/hid/tests/test_tablet.py
index 83f6501fe984..80269d1a0f0a 100644
--- a/tools/testing/selftests/hid/tests/test_tablet.py
+++ b/tools/testing/selftests/hid/tests/test_tablet.py
@@ -21,22 +21,66 @@ logger = logging.getLogger("hidtools.test.tablet")
class PenState(Enum):
"""Pen states according to Microsoft reference:
https://docs.microsoft.com/en-us/windows-hardware/design/component-guidelines/windows-pen-states
- """
- PEN_IS_OUT_OF_RANGE = (False, None)
- PEN_IS_IN_RANGE = (False, libevdev.EV_KEY.BTN_TOOL_PEN)
- PEN_IS_IN_CONTACT = (True, libevdev.EV_KEY.BTN_TOOL_PEN)
- PEN_IS_IN_RANGE_WITH_ERASING_INTENT = (False, libevdev.EV_KEY.BTN_TOOL_RUBBER)
- PEN_IS_ERASING = (True, libevdev.EV_KEY.BTN_TOOL_RUBBER)
+ We extend it with the various buttons when we need to check them.
+ """
- def __init__(self, touch, tool):
+ PEN_IS_OUT_OF_RANGE = (False, None, None)
+ PEN_IS_IN_RANGE = (False, libevdev.EV_KEY.BTN_TOOL_PEN, None)
+ PEN_IS_IN_RANGE_WITH_BUTTON = (
+ False,
+ libevdev.EV_KEY.BTN_TOOL_PEN,
+ libevdev.EV_KEY.BTN_STYLUS,
+ )
+ PEN_IS_IN_RANGE_WITH_SECOND_BUTTON = (
+ False,
+ libevdev.EV_KEY.BTN_TOOL_PEN,
+ libevdev.EV_KEY.BTN_STYLUS2,
+ )
+ PEN_IS_IN_CONTACT = (True, libevdev.EV_KEY.BTN_TOOL_PEN, None)
+ PEN_IS_IN_CONTACT_WITH_BUTTON = (
+ True,
+ libevdev.EV_KEY.BTN_TOOL_PEN,
+ libevdev.EV_KEY.BTN_STYLUS,
+ )
+ PEN_IS_IN_CONTACT_WITH_SECOND_BUTTON = (
+ True,
+ libevdev.EV_KEY.BTN_TOOL_PEN,
+ libevdev.EV_KEY.BTN_STYLUS2,
+ )
+ PEN_IS_IN_RANGE_WITH_ERASING_INTENT = (False, libevdev.EV_KEY.BTN_TOOL_RUBBER, None)
+ PEN_IS_IN_RANGE_WITH_ERASING_INTENT_WITH_BUTTON = (
+ False,
+ libevdev.EV_KEY.BTN_TOOL_RUBBER,
+ libevdev.EV_KEY.BTN_STYLUS,
+ )
+ PEN_IS_IN_RANGE_WITH_ERASING_INTENT_WITH_SECOND_BUTTON = (
+ False,
+ libevdev.EV_KEY.BTN_TOOL_RUBBER,
+ libevdev.EV_KEY.BTN_STYLUS2,
+ )
+ PEN_IS_ERASING = (True, libevdev.EV_KEY.BTN_TOOL_RUBBER, None)
+ PEN_IS_ERASING_WITH_BUTTON = (
+ True,
+ libevdev.EV_KEY.BTN_TOOL_RUBBER,
+ libevdev.EV_KEY.BTN_STYLUS,
+ )
+ PEN_IS_ERASING_WITH_SECOND_BUTTON = (
+ True,
+ libevdev.EV_KEY.BTN_TOOL_RUBBER,
+ libevdev.EV_KEY.BTN_STYLUS2,
+ )
+
+ def __init__(self, touch, tool, button):
self.touch = touch
self.tool = tool
+ self.button = button
@classmethod
def from_evdev(cls, evdev) -> "PenState":
touch = bool(evdev.value[libevdev.EV_KEY.BTN_TOUCH])
tool = None
+ button = None
if (
evdev.value[libevdev.EV_KEY.BTN_TOOL_RUBBER]
and not evdev.value[libevdev.EV_KEY.BTN_TOOL_PEN]
@@ -53,7 +97,17 @@ class PenState(Enum):
):
raise ValueError("2 tools are not allowed")
- return cls((touch, tool))
+ # we take only the highest button in account
+ for b in [libevdev.EV_KEY.BTN_STYLUS, libevdev.EV_KEY.BTN_STYLUS2]:
+ if bool(evdev.value[b]):
+ button = b
+
+ # the kernel tends to insert an EV_SYN once removing the tool, so
+ # the button will be released after
+ if tool is None:
+ button = None
+
+ return cls((touch, tool, button))
def apply(self, events) -> "PenState":
if libevdev.EV_SYN.SYN_REPORT in events:
@@ -62,6 +116,8 @@ class PenState(Enum):
touch_found = False
tool = self.tool
tool_found = False
+ button = self.button
+ button_found = False
for ev in events:
if ev == libevdev.InputEvent(libevdev.EV_KEY.BTN_TOUCH):
@@ -76,12 +132,22 @@ class PenState(Enum):
if tool_found:
raise ValueError(f"duplicated BTN_TOOL_* in {events}")
tool_found = True
- if ev.value:
- tool = ev.code
- else:
- tool = None
+ tool = ev.code if ev.value else None
+ elif ev in (
+ libevdev.InputEvent(libevdev.EV_KEY.BTN_STYLUS),
+ libevdev.InputEvent(libevdev.EV_KEY.BTN_STYLUS2),
+ ):
+ if button_found:
+ raise ValueError(f"duplicated BTN_STYLUS* in {events}")
+ button_found = True
+ button = ev.code if ev.value else None
- new_state = PenState((touch, tool))
+ # the kernel tends to insert an EV_SYN once removing the tool, so
+ # the button will be released after
+ if tool is None:
+ button = None
+
+ new_state = PenState((touch, tool, button))
assert (
new_state in self.valid_transitions()
), f"moving from {self} to {new_state} is forbidden"
@@ -97,14 +163,20 @@ class PenState(Enum):
return (
PenState.PEN_IS_OUT_OF_RANGE,
PenState.PEN_IS_IN_RANGE,
+ PenState.PEN_IS_IN_RANGE_WITH_BUTTON,
+ PenState.PEN_IS_IN_RANGE_WITH_SECOND_BUTTON,
PenState.PEN_IS_IN_RANGE_WITH_ERASING_INTENT,
PenState.PEN_IS_IN_CONTACT,
+ PenState.PEN_IS_IN_CONTACT_WITH_BUTTON,
+ PenState.PEN_IS_IN_CONTACT_WITH_SECOND_BUTTON,
PenState.PEN_IS_ERASING,
)
if self == PenState.PEN_IS_IN_RANGE:
return (
PenState.PEN_IS_IN_RANGE,
+ PenState.PEN_IS_IN_RANGE_WITH_BUTTON,
+ PenState.PEN_IS_IN_RANGE_WITH_SECOND_BUTTON,
PenState.PEN_IS_OUT_OF_RANGE,
PenState.PEN_IS_IN_CONTACT,
)
@@ -112,6 +184,8 @@ class PenState(Enum):
if self == PenState.PEN_IS_IN_CONTACT:
return (
PenState.PEN_IS_IN_CONTACT,
+ PenState.PEN_IS_IN_CONTACT_WITH_BUTTON,
+ PenState.PEN_IS_IN_CONTACT_WITH_SECOND_BUTTON,
PenState.PEN_IS_IN_RANGE,
PenState.PEN_IS_OUT_OF_RANGE,
)
@@ -130,6 +204,38 @@ class PenState(Enum):
PenState.PEN_IS_OUT_OF_RANGE,
)
+ if self == PenState.PEN_IS_IN_RANGE_WITH_BUTTON:
+ return (
+ PenState.PEN_IS_IN_RANGE_WITH_BUTTON,
+ PenState.PEN_IS_IN_RANGE,
+ PenState.PEN_IS_OUT_OF_RANGE,
+ PenState.PEN_IS_IN_CONTACT_WITH_BUTTON,
+ )
+
+ if self == PenState.PEN_IS_IN_CONTACT_WITH_BUTTON:
+ return (
+ PenState.PEN_IS_IN_CONTACT_WITH_BUTTON,
+ PenState.PEN_IS_IN_CONTACT,
+ PenState.PEN_IS_IN_RANGE_WITH_BUTTON,
+ PenState.PEN_IS_OUT_OF_RANGE,
+ )
+
+ if self == PenState.PEN_IS_IN_RANGE_WITH_SECOND_BUTTON:
+ return (
+ PenState.PEN_IS_IN_RANGE_WITH_SECOND_BUTTON,
+ PenState.PEN_IS_IN_RANGE,
+ PenState.PEN_IS_OUT_OF_RANGE,
+ PenState.PEN_IS_IN_CONTACT_WITH_SECOND_BUTTON,
+ )
+
+ if self == PenState.PEN_IS_IN_CONTACT_WITH_SECOND_BUTTON:
+ return (
+ PenState.PEN_IS_IN_CONTACT_WITH_SECOND_BUTTON,
+ PenState.PEN_IS_IN_CONTACT,
+ PenState.PEN_IS_IN_RANGE_WITH_SECOND_BUTTON,
+ PenState.PEN_IS_OUT_OF_RANGE,
+ )
+
return tuple()
@staticmethod
@@ -364,26 +470,64 @@ class PenDigitizer(base.UHIDTestDevice):
pen.xtilt = 0
pen.ytilt = 0
pen.twist = 0
+ pen.barrelswitch = False
+ pen.secondarybarrelswitch = False
elif state == PenState.PEN_IS_IN_RANGE:
pen.tipswitch = False
pen.inrange = True
pen.invert = False
pen.eraser = False
+ pen.barrelswitch = False
+ pen.secondarybarrelswitch = False
elif state == PenState.PEN_IS_IN_CONTACT:
pen.tipswitch = True
pen.inrange = True
pen.invert = False
pen.eraser = False
+ pen.barrelswitch = False
+ pen.secondarybarrelswitch = False
+ elif state == PenState.PEN_IS_IN_RANGE_WITH_BUTTON:
+ pen.tipswitch = False
+ pen.inrange = True
+ pen.invert = False
+ pen.eraser = False
+ pen.barrelswitch = True
+ pen.secondarybarrelswitch = False
+ elif state == PenState.PEN_IS_IN_CONTACT_WITH_BUTTON:
+ pen.tipswitch = True
+ pen.inrange = True
+ pen.invert = False
+ pen.eraser = False
+ pen.barrelswitch = True
+ pen.secondarybarrelswitch = False
+ elif state == PenState.PEN_IS_IN_RANGE_WITH_SECOND_BUTTON:
+ pen.tipswitch = False
+ pen.inrange = True
+ pen.invert = False
+ pen.eraser = False
+ pen.barrelswitch = False
+ pen.secondarybarrelswitch = True
+ elif state == PenState.PEN_IS_IN_CONTACT_WITH_SECOND_BUTTON:
+ pen.tipswitch = True
+ pen.inrange = True
+ pen.invert = False
+ pen.eraser = False
+ pen.barrelswitch = False
+ pen.secondarybarrelswitch = True
elif state == PenState.PEN_IS_IN_RANGE_WITH_ERASING_INTENT:
pen.tipswitch = False
pen.inrange = True
pen.invert = True
pen.eraser = False
+ pen.barrelswitch = False
+ pen.secondarybarrelswitch = False
elif state == PenState.PEN_IS_ERASING:
pen.tipswitch = False
pen.inrange = True
pen.invert = False
pen.eraser = True
+ pen.barrelswitch = False
+ pen.secondarybarrelswitch = False
pen.current_state = state
--
2.41.0
^ permalink raw reply related
* [PATCH 10/12] selftests/hid: tablets: convert the primary button tests
From: Benjamin Tissoires @ 2023-11-29 15:24 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Shuah Khan, Peter Hutterer
Cc: linux-input, linux-kselftest, linux-kernel, Benjamin Tissoires
In-Reply-To: <20231129-wip-selftests-v1-0-ba15a1fe1b0d@kernel.org>
We get more descriptive in what we are doing, and also get more
information of what is actually being tested. Instead of having a non
exhaustive button changes that are semi-randomly done, we can describe
all the states we want to test.
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
tools/testing/selftests/hid/tests/test_tablet.py | 165 ++++++++++-------------
1 file changed, 69 insertions(+), 96 deletions(-)
diff --git a/tools/testing/selftests/hid/tests/test_tablet.py b/tools/testing/selftests/hid/tests/test_tablet.py
index 80269d1a0f0a..440a87170db1 100644
--- a/tools/testing/selftests/hid/tests/test_tablet.py
+++ b/tools/testing/selftests/hid/tests/test_tablet.py
@@ -302,6 +302,55 @@ class PenState(Enum):
),
}
+ @staticmethod
+ def legal_transitions_with_primary_button() -> Dict[str, Tuple["PenState", ...]]:
+ """We revisit the Windows Pen Implementation state machine:
+ we now have a primary button.
+ """
+ return {
+ "hover-button": (PenState.PEN_IS_IN_RANGE_WITH_BUTTON,),
+ "hover-button -> out-of-range": (
+ PenState.PEN_IS_IN_RANGE_WITH_BUTTON,
+ PenState.PEN_IS_OUT_OF_RANGE,
+ ),
+ "in-range -> button-press": (
+ PenState.PEN_IS_IN_RANGE,
+ PenState.PEN_IS_IN_RANGE_WITH_BUTTON,
+ ),
+ "in-range -> button-press -> button-release": (
+ PenState.PEN_IS_IN_RANGE,
+ PenState.PEN_IS_IN_RANGE_WITH_BUTTON,
+ PenState.PEN_IS_IN_RANGE,
+ ),
+ "in-range -> touch -> button-press -> button-release": (
+ PenState.PEN_IS_IN_RANGE,
+ PenState.PEN_IS_IN_CONTACT,
+ PenState.PEN_IS_IN_CONTACT_WITH_BUTTON,
+ PenState.PEN_IS_IN_CONTACT,
+ ),
+ "in-range -> touch -> button-press -> release -> button-release": (
+ PenState.PEN_IS_IN_RANGE,
+ PenState.PEN_IS_IN_CONTACT,
+ PenState.PEN_IS_IN_CONTACT_WITH_BUTTON,
+ PenState.PEN_IS_IN_RANGE_WITH_BUTTON,
+ PenState.PEN_IS_IN_RANGE,
+ ),
+ "in-range -> button-press -> touch -> release -> button-release": (
+ PenState.PEN_IS_IN_RANGE,
+ PenState.PEN_IS_IN_RANGE_WITH_BUTTON,
+ PenState.PEN_IS_IN_CONTACT_WITH_BUTTON,
+ PenState.PEN_IS_IN_RANGE_WITH_BUTTON,
+ PenState.PEN_IS_IN_RANGE,
+ ),
+ "in-range -> button-press -> touch -> button-release -> release": (
+ PenState.PEN_IS_IN_RANGE,
+ PenState.PEN_IS_IN_RANGE_WITH_BUTTON,
+ PenState.PEN_IS_IN_CONTACT_WITH_BUTTON,
+ PenState.PEN_IS_IN_CONTACT,
+ PenState.PEN_IS_IN_RANGE,
+ ),
+ }
+
@staticmethod
def tolerated_transitions() -> Dict[str, Tuple["PenState", ...]]:
"""This is not adhering to the Windows Pen Implementation state machine
@@ -645,7 +694,10 @@ class BaseTest:
@pytest.mark.parametrize("scribble", [True, False], ids=["scribble", "static"])
@pytest.mark.parametrize(
"state_list",
- [pytest.param(v, id=k) for k, v in PenState.tolerated_transitions().items()],
+ [
+ pytest.param(v, id=k)
+ for k, v in PenState.tolerated_transitions().items()
+ ],
)
def test_tolerated_pen_states(self, state_list, scribble):
"""This is not adhering to the Windows Pen Implementation state machine
@@ -653,6 +705,22 @@ class BaseTest:
reasons."""
self._test_states(state_list, scribble)
+ @pytest.mark.skip_if_uhdev(
+ lambda uhdev: "Barrel Switch" not in uhdev.fields,
+ "Device not compatible, missing Barrel Switch usage",
+ )
+ @pytest.mark.parametrize("scribble", [True, False], ids=["scribble", "static"])
+ @pytest.mark.parametrize(
+ "state_list",
+ [
+ pytest.param(v, id=k)
+ for k, v in PenState.legal_transitions_with_primary_button().items()
+ ],
+ )
+ def test_valid_primary_button_pen_states(self, state_list, scribble):
+ """Rework the transition state machine by adding the primary button."""
+ self._test_states(state_list, scribble)
+
@pytest.mark.skip_if_uhdev(
lambda uhdev: "Invert" not in uhdev.fields,
"Device not compatible, missing Invert usage",
@@ -710,101 +778,6 @@ class BaseTest:
state machine."""
self._test_states(state_list, scribble)
- @pytest.mark.skip_if_uhdev(
- lambda uhdev: "Barrel Switch" not in uhdev.fields,
- "Device not compatible, missing Barrel Switch usage",
- )
- def test_primary_button(self):
- """Primary button (stylus) pressed, reports as pressed even while hovering.
- Actual reporting from the device: hid=TIPSWITCH,BARRELSWITCH,INRANGE (code=TOUCH,STYLUS,PEN):
- { 0, 0, 1 } <- hover
- { 0, 1, 1 } <- primary button pressed
- { 0, 1, 1 } <- liftoff
- { 0, 0, 0 } <- leaves
- """
-
- uhdev = self.uhdev
- evdev = uhdev.get_evdev()
-
- p = Pen(50, 60)
- p.inrange = True
- events = self.post(uhdev, p)
- assert libevdev.InputEvent(libevdev.EV_KEY.BTN_TOOL_PEN, 1) in events
- assert evdev.value[libevdev.EV_ABS.ABS_X] == 50
- assert evdev.value[libevdev.EV_ABS.ABS_Y] == 60
- assert not evdev.value[libevdev.EV_KEY.BTN_STYLUS]
-
- p.barrelswitch = True
- events = self.post(uhdev, p)
- assert libevdev.InputEvent(libevdev.EV_KEY.BTN_STYLUS, 1) in events
-
- p.x += 1
- p.y -= 1
- events = self.post(uhdev, p)
- assert len(events) == 3 # X, Y, SYN
- assert libevdev.InputEvent(libevdev.EV_ABS.ABS_X, 51) in events
- assert libevdev.InputEvent(libevdev.EV_ABS.ABS_Y, 59) in events
-
- p.barrelswitch = False
- events = self.post(uhdev, p)
- assert libevdev.InputEvent(libevdev.EV_KEY.BTN_STYLUS, 0) in events
-
- p.inrange = False
- events = self.post(uhdev, p)
- assert libevdev.InputEvent(libevdev.EV_KEY.BTN_TOOL_PEN, 0) in events
-
- @pytest.mark.skip_if_uhdev(
- lambda uhdev: "Barrel Switch" not in uhdev.fields,
- "Device not compatible, missing Barrel Switch usage",
- )
- def test_contact_primary_button(self):
- """Primary button (stylus) pressed, reports as pressed even while hovering.
- Actual reporting from the device: hid=TIPSWITCH,BARRELSWITCH,INRANGE (code=TOUCH,STYLUS,PEN):
- { 0, 0, 1 } <- hover
- { 0, 1, 1 } <- primary button pressed
- { 1, 1, 1 } <- touch-down
- { 1, 1, 1 } <- still touch, scribble on the screen
- { 0, 1, 1 } <- liftoff
- { 0, 0, 0 } <- leaves
- """
-
- uhdev = self.uhdev
- evdev = uhdev.get_evdev()
-
- p = Pen(50, 60)
- p.inrange = True
- events = self.post(uhdev, p)
- assert libevdev.InputEvent(libevdev.EV_KEY.BTN_TOOL_PEN, 1) in events
- assert evdev.value[libevdev.EV_ABS.ABS_X] == 50
- assert evdev.value[libevdev.EV_ABS.ABS_Y] == 60
- assert not evdev.value[libevdev.EV_KEY.BTN_STYLUS]
-
- p.barrelswitch = True
- events = self.post(uhdev, p)
- assert libevdev.InputEvent(libevdev.EV_KEY.BTN_STYLUS, 1) in events
-
- p.tipswitch = True
- events = self.post(uhdev, p)
- assert libevdev.InputEvent(libevdev.EV_KEY.BTN_TOUCH, 1) in events
- assert evdev.value[libevdev.EV_KEY.BTN_STYLUS]
-
- p.x += 1
- p.y -= 1
- events = self.post(uhdev, p)
- assert len(events) == 3 # X, Y, SYN
- assert libevdev.InputEvent(libevdev.EV_ABS.ABS_X, 51) in events
- assert libevdev.InputEvent(libevdev.EV_ABS.ABS_Y, 59) in events
-
- p.tipswitch = False
- events = self.post(uhdev, p)
- assert libevdev.InputEvent(libevdev.EV_KEY.BTN_TOUCH, 0) in events
-
- p.barrelswitch = False
- p.inrange = False
- events = self.post(uhdev, p)
- assert libevdev.InputEvent(libevdev.EV_KEY.BTN_TOOL_PEN, 0) in events
- assert libevdev.InputEvent(libevdev.EV_KEY.BTN_STYLUS, 0) in events
-
class GXTP_pen(PenDigitizer):
def event(self, pen):
--
2.41.0
^ permalink raw reply related
* [PATCH 11/12] selftests/hid: tablets: add a secondary barrel switch test
From: Benjamin Tissoires @ 2023-11-29 15:24 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Shuah Khan, Peter Hutterer
Cc: linux-input, linux-kselftest, linux-kernel, Benjamin Tissoires
In-Reply-To: <20231129-wip-selftests-v1-0-ba15a1fe1b0d@kernel.org>
Some tablets report 2 barrel switches. We better test those too.
Use the same transistions description from the primary button tests.
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
tools/testing/selftests/hid/tests/test_tablet.py | 67 ++++++++++++++++++++++++
1 file changed, 67 insertions(+)
diff --git a/tools/testing/selftests/hid/tests/test_tablet.py b/tools/testing/selftests/hid/tests/test_tablet.py
index 440a87170db1..f24cf2e168a4 100644
--- a/tools/testing/selftests/hid/tests/test_tablet.py
+++ b/tools/testing/selftests/hid/tests/test_tablet.py
@@ -351,6 +351,56 @@ class PenState(Enum):
),
}
+ @staticmethod
+ def legal_transitions_with_secondary_button() -> Dict[str, Tuple["PenState", ...]]:
+ """We revisit the Windows Pen Implementation state machine:
+ we now have a secondary button.
+ Note: we don't looks for 2 buttons interactions.
+ """
+ return {
+ "hover-button": (PenState.PEN_IS_IN_RANGE_WITH_SECOND_BUTTON,),
+ "hover-button -> out-of-range": (
+ PenState.PEN_IS_IN_RANGE_WITH_SECOND_BUTTON,
+ PenState.PEN_IS_OUT_OF_RANGE,
+ ),
+ "in-range -> button-press": (
+ PenState.PEN_IS_IN_RANGE,
+ PenState.PEN_IS_IN_RANGE_WITH_SECOND_BUTTON,
+ ),
+ "in-range -> button-press -> button-release": (
+ PenState.PEN_IS_IN_RANGE,
+ PenState.PEN_IS_IN_RANGE_WITH_SECOND_BUTTON,
+ PenState.PEN_IS_IN_RANGE,
+ ),
+ "in-range -> touch -> button-press -> button-release": (
+ PenState.PEN_IS_IN_RANGE,
+ PenState.PEN_IS_IN_CONTACT,
+ PenState.PEN_IS_IN_CONTACT_WITH_SECOND_BUTTON,
+ PenState.PEN_IS_IN_CONTACT,
+ ),
+ "in-range -> touch -> button-press -> release -> button-release": (
+ PenState.PEN_IS_IN_RANGE,
+ PenState.PEN_IS_IN_CONTACT,
+ PenState.PEN_IS_IN_CONTACT_WITH_SECOND_BUTTON,
+ PenState.PEN_IS_IN_RANGE_WITH_SECOND_BUTTON,
+ PenState.PEN_IS_IN_RANGE,
+ ),
+ "in-range -> button-press -> touch -> release -> button-release": (
+ PenState.PEN_IS_IN_RANGE,
+ PenState.PEN_IS_IN_RANGE_WITH_SECOND_BUTTON,
+ PenState.PEN_IS_IN_CONTACT_WITH_SECOND_BUTTON,
+ PenState.PEN_IS_IN_RANGE_WITH_SECOND_BUTTON,
+ PenState.PEN_IS_IN_RANGE,
+ ),
+ "in-range -> button-press -> touch -> button-release -> release": (
+ PenState.PEN_IS_IN_RANGE,
+ PenState.PEN_IS_IN_RANGE_WITH_SECOND_BUTTON,
+ PenState.PEN_IS_IN_CONTACT_WITH_SECOND_BUTTON,
+ PenState.PEN_IS_IN_CONTACT,
+ PenState.PEN_IS_IN_RANGE,
+ ),
+ }
+
@staticmethod
def tolerated_transitions() -> Dict[str, Tuple["PenState", ...]]:
"""This is not adhering to the Windows Pen Implementation state machine
@@ -429,6 +479,7 @@ class Pen(object):
self.width = 10
self.height = 10
self.barrelswitch = False
+ self.secondarybarrelswitch = False
self.invert = False
self.eraser = False
self.xtilt = 1
@@ -721,6 +772,22 @@ class BaseTest:
"""Rework the transition state machine by adding the primary button."""
self._test_states(state_list, scribble)
+ @pytest.mark.skip_if_uhdev(
+ lambda uhdev: "Secondary Barrel Switch" not in uhdev.fields,
+ "Device not compatible, missing Secondary Barrel Switch usage",
+ )
+ @pytest.mark.parametrize("scribble", [True, False], ids=["scribble", "static"])
+ @pytest.mark.parametrize(
+ "state_list",
+ [
+ pytest.param(v, id=k)
+ for k, v in PenState.legal_transitions_with_secondary_button().items()
+ ],
+ )
+ def test_valid_secondary_button_pen_states(self, state_list, scribble):
+ """Rework the transition state machine by adding the secondary button."""
+ self._test_states(state_list, scribble)
+
@pytest.mark.skip_if_uhdev(
lambda uhdev: "Invert" not in uhdev.fields,
"Device not compatible, missing Invert usage",
--
2.41.0
^ 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