* Re: [PATCH 0/6] Convert joystick drivers to use new cleanup facilities
From: Dmitry Torokhov @ 2024-09-04 4:43 UTC (permalink / raw)
To: linux-input; +Cc: Erick Archer, Christophe JAILLET, linux-kernel
In-Reply-To: <20240904043104.1030257-1-dmitry.torokhov@gmail.com>
On Tue, Sep 03, 2024 at 09:30:57PM -0700, Dmitry Torokhov wrote:
> Hi,
>
> This series converts drivers found in drivers/input/keyboard to use new
This should have read drivers/input/joystick obviously...
> __free() and guard() cleanup facilities that simplify the code and
> ensure that all resources are released appropriately.
>
> Thanks!
>
> Dmitry Torokhov (6):
> Input: db9 - use guard notation when acquiring mutex
> Input: gamecon - use guard notation when acquiring mutex
> Input: iforce - use guard notation when acquiring mutex and spinlock
> Input: n64joy - use guard notation when acquiring mutex
> Input: turbografx - use guard notation when acquiring mutex
> Input: xpad - use guard notation when acquiring mutex and spinlock
>
> drivers/input/joystick/db9.c | 30 +++---
> drivers/input/joystick/gamecon.c | 22 ++---
> drivers/input/joystick/iforce/iforce-ff.c | 48 +++++----
> .../input/joystick/iforce/iforce-packets.c | 57 +++++------
> drivers/input/joystick/iforce/iforce-serio.c | 36 +++----
> drivers/input/joystick/iforce/iforce-usb.c | 13 ++-
> drivers/input/joystick/n64joy.c | 35 +++----
> drivers/input/joystick/turbografx.c | 22 ++---
> drivers/input/joystick/xpad.c | 99 +++++++------------
> 9 files changed, 152 insertions(+), 210 deletions(-)
>
> --
> Dmitry
--
Dmitry
^ permalink raw reply
* [PATCH 10/22] Input: ideapad_slidebar - use guard notation when acquiring spinlock
From: Dmitry Torokhov @ 2024-09-04 4:42 UTC (permalink / raw)
To: linux-input
Cc: Michael Hennerich, Ville Syrjala, Support Opensource, Eddie James,
Andrey Moiseev, Hans de Goede, Javier Carrasco, Jeff LaBundy,
linux-kernel
In-Reply-To: <20240904044244.1042174-1-dmitry.torokhov@gmail.com>
Using guard notation makes the code more compact and error handling
more robust by ensuring that locks are released in all code paths
when control leaves critical section.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/misc/ideapad_slidebar.c | 22 +++++-----------------
1 file changed, 5 insertions(+), 17 deletions(-)
diff --git a/drivers/input/misc/ideapad_slidebar.c b/drivers/input/misc/ideapad_slidebar.c
index fa4e7f67d713..592bd159a194 100644
--- a/drivers/input/misc/ideapad_slidebar.c
+++ b/drivers/input/misc/ideapad_slidebar.c
@@ -95,41 +95,29 @@ static struct platform_device *slidebar_platform_dev;
static u8 slidebar_pos_get(void)
{
- u8 res;
- unsigned long flags;
+ guard(spinlock_irqsave)(&io_lock);
- spin_lock_irqsave(&io_lock, flags);
outb(0xf4, 0xff29);
outb(0xbf, 0xff2a);
- res = inb(0xff2b);
- spin_unlock_irqrestore(&io_lock, flags);
-
- return res;
+ return inb(0xff2b);
}
static u8 slidebar_mode_get(void)
{
- u8 res;
- unsigned long flags;
+ guard(spinlock_irqsave)(&io_lock);
- spin_lock_irqsave(&io_lock, flags);
outb(0xf7, 0xff29);
outb(0x8b, 0xff2a);
- res = inb(0xff2b);
- spin_unlock_irqrestore(&io_lock, flags);
-
- return res;
+ return inb(0xff2b);
}
static void slidebar_mode_set(u8 mode)
{
- unsigned long flags;
+ guard(spinlock_irqsave)(&io_lock);
- spin_lock_irqsave(&io_lock, flags);
outb(0xf7, 0xff29);
outb(0x8b, 0xff2a);
outb(mode, 0xff2b);
- spin_unlock_irqrestore(&io_lock, flags);
}
static bool slidebar_i8042_filter(unsigned char data, unsigned char str,
--
2.46.0.469.g59c65b2a67-goog
^ permalink raw reply related
* [PATCH 09/22] Input: drv2667 - use guard notation when acquiring mutex
From: Dmitry Torokhov @ 2024-09-04 4:42 UTC (permalink / raw)
To: linux-input
Cc: Michael Hennerich, Ville Syrjala, Support Opensource, Eddie James,
Andrey Moiseev, Hans de Goede, Javier Carrasco, Jeff LaBundy,
linux-kernel
In-Reply-To: <20240904044244.1042174-1-dmitry.torokhov@gmail.com>
Using guard notation makes the code more compact and error handling
more robust by ensuring that mutexes are released in all code paths
when control leaves critical section.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/misc/drv2667.c | 44 +++++++++++++++++-------------------
1 file changed, 21 insertions(+), 23 deletions(-)
diff --git a/drivers/input/misc/drv2667.c b/drivers/input/misc/drv2667.c
index ad49845374b9..906292625f84 100644
--- a/drivers/input/misc/drv2667.c
+++ b/drivers/input/misc/drv2667.c
@@ -402,59 +402,57 @@ static int drv2667_probe(struct i2c_client *client)
static int drv2667_suspend(struct device *dev)
{
struct drv2667_data *haptics = dev_get_drvdata(dev);
- int ret = 0;
+ int error;
- mutex_lock(&haptics->input_dev->mutex);
+ guard(mutex)(&haptics->input_dev->mutex);
if (input_device_enabled(haptics->input_dev)) {
- ret = regmap_update_bits(haptics->regmap, DRV2667_CTRL_2,
- DRV2667_STANDBY, DRV2667_STANDBY);
- if (ret) {
+ error = regmap_update_bits(haptics->regmap, DRV2667_CTRL_2,
+ DRV2667_STANDBY, DRV2667_STANDBY);
+ if (error) {
dev_err(dev, "Failed to set standby mode\n");
regulator_disable(haptics->regulator);
- goto out;
+ return error;
}
- ret = regulator_disable(haptics->regulator);
- if (ret) {
+ error = regulator_disable(haptics->regulator);
+ if (error) {
dev_err(dev, "Failed to disable regulator\n");
regmap_update_bits(haptics->regmap,
DRV2667_CTRL_2,
DRV2667_STANDBY, 0);
+ return error;
}
}
-out:
- mutex_unlock(&haptics->input_dev->mutex);
- return ret;
+
+ return 0;
}
static int drv2667_resume(struct device *dev)
{
struct drv2667_data *haptics = dev_get_drvdata(dev);
- int ret = 0;
+ int error;
- mutex_lock(&haptics->input_dev->mutex);
+ guard(mutex)(&haptics->input_dev->mutex);
if (input_device_enabled(haptics->input_dev)) {
- ret = regulator_enable(haptics->regulator);
- if (ret) {
+ error = regulator_enable(haptics->regulator);
+ if (error) {
dev_err(dev, "Failed to enable regulator\n");
- goto out;
+ return error;
}
- ret = regmap_update_bits(haptics->regmap, DRV2667_CTRL_2,
- DRV2667_STANDBY, 0);
- if (ret) {
+ error = regmap_update_bits(haptics->regmap, DRV2667_CTRL_2,
+ DRV2667_STANDBY, 0);
+ if (error) {
dev_err(dev, "Failed to unset standby mode\n");
regulator_disable(haptics->regulator);
- goto out;
+ return error;
}
}
-out:
- mutex_unlock(&haptics->input_dev->mutex);
- return ret;
+ return 0;
}
static DEFINE_SIMPLE_DEV_PM_OPS(drv2667_pm_ops, drv2667_suspend, drv2667_resume);
--
2.46.0.469.g59c65b2a67-goog
^ permalink raw reply related
* [PATCH 08/22] Input: drv2665 - use guard notation when acquiring mutex
From: Dmitry Torokhov @ 2024-09-04 4:42 UTC (permalink / raw)
To: linux-input
Cc: Michael Hennerich, Ville Syrjala, Support Opensource, Eddie James,
Andrey Moiseev, Hans de Goede, Javier Carrasco, Jeff LaBundy,
linux-kernel
In-Reply-To: <20240904044244.1042174-1-dmitry.torokhov@gmail.com>
Using guard notation makes the code more compact and error handling
more robust by ensuring that mutexes are released in all code paths
when control leaves critical section.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/misc/drv2665.c | 44 +++++++++++++++++-------------------
1 file changed, 21 insertions(+), 23 deletions(-)
diff --git a/drivers/input/misc/drv2665.c b/drivers/input/misc/drv2665.c
index f98e4d765307..77ec96c7db76 100644
--- a/drivers/input/misc/drv2665.c
+++ b/drivers/input/misc/drv2665.c
@@ -225,59 +225,57 @@ static int drv2665_probe(struct i2c_client *client)
static int drv2665_suspend(struct device *dev)
{
struct drv2665_data *haptics = dev_get_drvdata(dev);
- int ret = 0;
+ int error;
- mutex_lock(&haptics->input_dev->mutex);
+ guard(mutex)(&haptics->input_dev->mutex);
if (input_device_enabled(haptics->input_dev)) {
- ret = regmap_update_bits(haptics->regmap, DRV2665_CTRL_2,
- DRV2665_STANDBY, DRV2665_STANDBY);
- if (ret) {
+ error = regmap_update_bits(haptics->regmap, DRV2665_CTRL_2,
+ DRV2665_STANDBY, DRV2665_STANDBY);
+ if (error) {
dev_err(dev, "Failed to set standby mode\n");
regulator_disable(haptics->regulator);
- goto out;
+ return error;
}
- ret = regulator_disable(haptics->regulator);
- if (ret) {
+ error = regulator_disable(haptics->regulator);
+ if (error) {
dev_err(dev, "Failed to disable regulator\n");
regmap_update_bits(haptics->regmap,
DRV2665_CTRL_2,
DRV2665_STANDBY, 0);
+ return error;
}
}
-out:
- mutex_unlock(&haptics->input_dev->mutex);
- return ret;
+
+ return 0;
}
static int drv2665_resume(struct device *dev)
{
struct drv2665_data *haptics = dev_get_drvdata(dev);
- int ret = 0;
+ int error;
- mutex_lock(&haptics->input_dev->mutex);
+ guard(mutex)(&haptics->input_dev->mutex);
if (input_device_enabled(haptics->input_dev)) {
- ret = regulator_enable(haptics->regulator);
- if (ret) {
+ error = regulator_enable(haptics->regulator);
+ if (error) {
dev_err(dev, "Failed to enable regulator\n");
- goto out;
+ return error;
}
- ret = regmap_update_bits(haptics->regmap, DRV2665_CTRL_2,
- DRV2665_STANDBY, 0);
- if (ret) {
+ error = regmap_update_bits(haptics->regmap, DRV2665_CTRL_2,
+ DRV2665_STANDBY, 0);
+ if (error) {
dev_err(dev, "Failed to unset standby mode\n");
regulator_disable(haptics->regulator);
- goto out;
+ return error;
}
}
-out:
- mutex_unlock(&haptics->input_dev->mutex);
- return ret;
+ return 0;
}
static DEFINE_SIMPLE_DEV_PM_OPS(drv2665_pm_ops, drv2665_suspend, drv2665_resume);
--
2.46.0.469.g59c65b2a67-goog
^ permalink raw reply related
* [PATCH 07/22] Input: drv260x - use guard notation when acquiring mutex
From: Dmitry Torokhov @ 2024-09-04 4:42 UTC (permalink / raw)
To: linux-input
Cc: Michael Hennerich, Ville Syrjala, Support Opensource, Eddie James,
Andrey Moiseev, Hans de Goede, Javier Carrasco, Jeff LaBundy,
linux-kernel
In-Reply-To: <20240904044244.1042174-1-dmitry.torokhov@gmail.com>
Using guard notation makes the code more compact and error handling
more robust by ensuring that mutexes are released in all code paths
when control leaves critical section.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/misc/drv260x.c | 50 +++++++++++++++++-------------------
1 file changed, 24 insertions(+), 26 deletions(-)
diff --git a/drivers/input/misc/drv260x.c b/drivers/input/misc/drv260x.c
index 61b503835aa6..96cd6a078c8a 100644
--- a/drivers/input/misc/drv260x.c
+++ b/drivers/input/misc/drv260x.c
@@ -537,64 +537,62 @@ static int drv260x_probe(struct i2c_client *client)
static int drv260x_suspend(struct device *dev)
{
struct drv260x_data *haptics = dev_get_drvdata(dev);
- int ret = 0;
+ int error;
- mutex_lock(&haptics->input_dev->mutex);
+ guard(mutex)(&haptics->input_dev->mutex);
if (input_device_enabled(haptics->input_dev)) {
- ret = regmap_update_bits(haptics->regmap,
- DRV260X_MODE,
- DRV260X_STANDBY_MASK,
- DRV260X_STANDBY);
- if (ret) {
+ error = regmap_update_bits(haptics->regmap,
+ DRV260X_MODE,
+ DRV260X_STANDBY_MASK,
+ DRV260X_STANDBY);
+ if (error) {
dev_err(dev, "Failed to set standby mode\n");
- goto out;
+ return error;
}
gpiod_set_value(haptics->enable_gpio, 0);
- ret = regulator_disable(haptics->regulator);
- if (ret) {
+ error = regulator_disable(haptics->regulator);
+ if (error) {
dev_err(dev, "Failed to disable regulator\n");
regmap_update_bits(haptics->regmap,
DRV260X_MODE,
DRV260X_STANDBY_MASK, 0);
+ return error;
}
}
-out:
- mutex_unlock(&haptics->input_dev->mutex);
- return ret;
+
+ return 0;
}
static int drv260x_resume(struct device *dev)
{
struct drv260x_data *haptics = dev_get_drvdata(dev);
- int ret = 0;
+ int error;
- mutex_lock(&haptics->input_dev->mutex);
+ guard(mutex)(&haptics->input_dev->mutex);
if (input_device_enabled(haptics->input_dev)) {
- ret = regulator_enable(haptics->regulator);
- if (ret) {
+ error = regulator_enable(haptics->regulator);
+ if (error) {
dev_err(dev, "Failed to enable regulator\n");
- goto out;
+ return error;
}
- ret = regmap_update_bits(haptics->regmap,
- DRV260X_MODE,
- DRV260X_STANDBY_MASK, 0);
- if (ret) {
+ error = regmap_update_bits(haptics->regmap,
+ DRV260X_MODE,
+ DRV260X_STANDBY_MASK, 0);
+ if (error) {
dev_err(dev, "Failed to unset standby mode\n");
regulator_disable(haptics->regulator);
- goto out;
+ return error;
}
gpiod_set_value(haptics->enable_gpio, 1);
}
-out:
- mutex_unlock(&haptics->input_dev->mutex);
- return ret;
+ return 0;
}
static DEFINE_SIMPLE_DEV_PM_OPS(drv260x_pm_ops, drv260x_suspend, drv260x_resume);
--
2.46.0.469.g59c65b2a67-goog
^ permalink raw reply related
* [PATCH 06/22] Input: kxtj9 - use guard notation when acquiring mutex/disabling irq
From: Dmitry Torokhov @ 2024-09-04 4:42 UTC (permalink / raw)
To: linux-input
Cc: Michael Hennerich, Ville Syrjala, Support Opensource, Eddie James,
Andrey Moiseev, Hans de Goede, Javier Carrasco, Jeff LaBundy,
linux-kernel
In-Reply-To: <20240904044244.1042174-1-dmitry.torokhov@gmail.com>
Using guard notation makes the code more compact and error handling
more robust by ensuring that mutexes are released and interrupts are
re-enabled in all code paths when control leaves critical section.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/misc/kxtj9.c | 14 ++++----------
1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/drivers/input/misc/kxtj9.c b/drivers/input/misc/kxtj9.c
index 837682cb2a7d..c6146bcee9f9 100644
--- a/drivers/input/misc/kxtj9.c
+++ b/drivers/input/misc/kxtj9.c
@@ -314,9 +314,8 @@ static ssize_t kxtj9_set_poll(struct device *dev, struct device_attribute *attr,
return error;
/* Lock the device to prevent races with open/close (and itself) */
- mutex_lock(&input_dev->mutex);
-
- disable_irq(client->irq);
+ guard(mutex)(&input_dev->mutex);
+ guard(disable_irq)(&client->irq);
/*
* Set current interval to the greater of the minimum interval or
@@ -326,9 +325,6 @@ static ssize_t kxtj9_set_poll(struct device *dev, struct device_attribute *attr,
kxtj9_update_odr(tj9, tj9->last_poll_interval);
- enable_irq(client->irq);
- mutex_unlock(&input_dev->mutex);
-
return count;
}
@@ -504,12 +500,11 @@ static int kxtj9_suspend(struct device *dev)
struct kxtj9_data *tj9 = i2c_get_clientdata(client);
struct input_dev *input_dev = tj9->input_dev;
- mutex_lock(&input_dev->mutex);
+ guard(mutex)(&input_dev->mutex);
if (input_device_enabled(input_dev))
kxtj9_disable(tj9);
- mutex_unlock(&input_dev->mutex);
return 0;
}
@@ -519,12 +514,11 @@ static int kxtj9_resume(struct device *dev)
struct kxtj9_data *tj9 = i2c_get_clientdata(client);
struct input_dev *input_dev = tj9->input_dev;
- mutex_lock(&input_dev->mutex);
+ guard(mutex)(&input_dev->mutex);
if (input_device_enabled(input_dev))
kxtj9_enable(tj9);
- mutex_unlock(&input_dev->mutex);
return 0;
}
--
2.46.0.469.g59c65b2a67-goog
^ permalink raw reply related
* [PATCH 05/22] Input: da7280 - use guard notation when acquiring mutex and spinlock
From: Dmitry Torokhov @ 2024-09-04 4:42 UTC (permalink / raw)
To: linux-input
Cc: Michael Hennerich, Ville Syrjala, Support Opensource, Eddie James,
Andrey Moiseev, Hans de Goede, Javier Carrasco, Jeff LaBundy,
linux-kernel
In-Reply-To: <20240904044244.1042174-1-dmitry.torokhov@gmail.com>
Using guard notation makes the code more compact and error handling
more robust by ensuring that locks are released in all code paths
when control leaves critical section.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/misc/da7280.c | 26 ++++++++++++--------------
1 file changed, 12 insertions(+), 14 deletions(-)
diff --git a/drivers/input/misc/da7280.c b/drivers/input/misc/da7280.c
index 1629b7ea4cbd..e4a605c6af15 100644
--- a/drivers/input/misc/da7280.c
+++ b/drivers/input/misc/da7280.c
@@ -1263,39 +1263,37 @@ static int da7280_suspend(struct device *dev)
{
struct da7280_haptic *haptics = dev_get_drvdata(dev);
- mutex_lock(&haptics->input_dev->mutex);
+ guard(mutex)(&haptics->input_dev->mutex);
/*
* Make sure no new requests will be submitted while device is
* suspended.
*/
- spin_lock_irq(&haptics->input_dev->event_lock);
- haptics->suspended = true;
- spin_unlock_irq(&haptics->input_dev->event_lock);
+ scoped_guard(spinlock_irq, &haptics->input_dev->event_lock) {
+ haptics->suspended = true;
+ }
da7280_haptic_stop(haptics);
- mutex_unlock(&haptics->input_dev->mutex);
-
return 0;
}
static int da7280_resume(struct device *dev)
{
struct da7280_haptic *haptics = dev_get_drvdata(dev);
- int retval;
+ int error;
- mutex_lock(&haptics->input_dev->mutex);
+ guard(mutex)(&haptics->input_dev->mutex);
- retval = da7280_haptic_start(haptics);
- if (!retval) {
- spin_lock_irq(&haptics->input_dev->event_lock);
+ error = da7280_haptic_start(haptics);
+ if (error)
+ return error;
+
+ scoped_guard(spinlock_irq, &haptics->input_dev->event_lock) {
haptics->suspended = false;
- spin_unlock_irq(&haptics->input_dev->event_lock);
}
- mutex_unlock(&haptics->input_dev->mutex);
- return retval;
+ return 0;
}
#ifdef CONFIG_OF
--
2.46.0.469.g59c65b2a67-goog
^ permalink raw reply related
* [PATCH 04/22] Input: cma3000_d0x - use guard notation when acquiring mutex
From: Dmitry Torokhov @ 2024-09-04 4:42 UTC (permalink / raw)
To: linux-input
Cc: Michael Hennerich, Ville Syrjala, Support Opensource, Eddie James,
Andrey Moiseev, Hans de Goede, Javier Carrasco, Jeff LaBundy,
linux-kernel
In-Reply-To: <20240904044244.1042174-1-dmitry.torokhov@gmail.com>
Using guard notation makes the code more compact and error handling
more robust by ensuring that mutexes are released in all code paths
when control leaves critical section.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/misc/cma3000_d0x.c | 16 ++++------------
1 file changed, 4 insertions(+), 12 deletions(-)
diff --git a/drivers/input/misc/cma3000_d0x.c b/drivers/input/misc/cma3000_d0x.c
index 0c68e924a1cc..cfc12332bee1 100644
--- a/drivers/input/misc/cma3000_d0x.c
+++ b/drivers/input/misc/cma3000_d0x.c
@@ -217,15 +217,13 @@ static int cma3000_open(struct input_dev *input_dev)
{
struct cma3000_accl_data *data = input_get_drvdata(input_dev);
- mutex_lock(&data->mutex);
+ guard(mutex)(&data->mutex);
if (!data->suspended)
cma3000_poweron(data);
data->opened = true;
- mutex_unlock(&data->mutex);
-
return 0;
}
@@ -233,40 +231,34 @@ static void cma3000_close(struct input_dev *input_dev)
{
struct cma3000_accl_data *data = input_get_drvdata(input_dev);
- mutex_lock(&data->mutex);
+ guard(mutex)(&data->mutex);
if (!data->suspended)
cma3000_poweroff(data);
data->opened = false;
-
- mutex_unlock(&data->mutex);
}
void cma3000_suspend(struct cma3000_accl_data *data)
{
- mutex_lock(&data->mutex);
+ guard(mutex)(&data->mutex);
if (!data->suspended && data->opened)
cma3000_poweroff(data);
data->suspended = true;
-
- mutex_unlock(&data->mutex);
}
EXPORT_SYMBOL(cma3000_suspend);
void cma3000_resume(struct cma3000_accl_data *data)
{
- mutex_lock(&data->mutex);
+ guard(mutex)(&data->mutex);
if (data->suspended && data->opened)
cma3000_poweron(data);
data->suspended = false;
-
- mutex_unlock(&data->mutex);
}
EXPORT_SYMBOL(cma3000_resume);
--
2.46.0.469.g59c65b2a67-goog
^ permalink raw reply related
* [PATCH 03/22] Input: cm109 - use guard notation when acquiring mutex and spinlock
From: Dmitry Torokhov @ 2024-09-04 4:42 UTC (permalink / raw)
To: linux-input
Cc: Michael Hennerich, Ville Syrjala, Support Opensource, Eddie James,
Andrey Moiseev, Hans de Goede, Javier Carrasco, Jeff LaBundy,
linux-kernel
In-Reply-To: <20240904044244.1042174-1-dmitry.torokhov@gmail.com>
Using guard notation makes the code more compact and error handling
more robust by ensuring that locks are released in all code paths
when control leaves critical section.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/misc/cm109.c | 167 ++++++++++++++++++-------------------
1 file changed, 79 insertions(+), 88 deletions(-)
diff --git a/drivers/input/misc/cm109.c b/drivers/input/misc/cm109.c
index 728325a2d574..0cfe5d4a573c 100644
--- a/drivers/input/misc/cm109.c
+++ b/drivers/input/misc/cm109.c
@@ -355,6 +355,35 @@ static void cm109_submit_buzz_toggle(struct cm109_dev *dev)
__func__, error);
}
+static void cm109_submit_ctl(struct cm109_dev *dev)
+{
+ int error;
+
+ guard(spinlock_irqsave)(&dev->ctl_submit_lock);
+
+ dev->irq_urb_pending = 0;
+
+ if (unlikely(dev->shutdown))
+ return;
+
+ if (dev->buzzer_state)
+ dev->ctl_data->byte[HID_OR0] |= BUZZER_ON;
+ else
+ dev->ctl_data->byte[HID_OR0] &= ~BUZZER_ON;
+
+ dev->ctl_data->byte[HID_OR1] = dev->keybit;
+ dev->ctl_data->byte[HID_OR2] = dev->keybit;
+
+ dev->buzzer_pending = 0;
+ dev->ctl_urb_pending = 1;
+
+ error = usb_submit_urb(dev->urb_ctl, GFP_ATOMIC);
+ if (error)
+ dev_err(&dev->intf->dev,
+ "%s: usb_submit_urb (urb_ctl) failed %d\n",
+ __func__, error);
+}
+
/*
* IRQ handler
*/
@@ -362,8 +391,6 @@ static void cm109_urb_irq_callback(struct urb *urb)
{
struct cm109_dev *dev = urb->context;
const int status = urb->status;
- int error;
- unsigned long flags;
dev_dbg(&dev->intf->dev, "### URB IRQ: [0x%02x 0x%02x 0x%02x 0x%02x] keybit=0x%02x\n",
dev->irq_data->byte[0],
@@ -401,32 +428,7 @@ static void cm109_urb_irq_callback(struct urb *urb)
}
out:
-
- spin_lock_irqsave(&dev->ctl_submit_lock, flags);
-
- dev->irq_urb_pending = 0;
-
- if (likely(!dev->shutdown)) {
-
- if (dev->buzzer_state)
- dev->ctl_data->byte[HID_OR0] |= BUZZER_ON;
- else
- dev->ctl_data->byte[HID_OR0] &= ~BUZZER_ON;
-
- dev->ctl_data->byte[HID_OR1] = dev->keybit;
- dev->ctl_data->byte[HID_OR2] = dev->keybit;
-
- dev->buzzer_pending = 0;
- dev->ctl_urb_pending = 1;
-
- error = usb_submit_urb(dev->urb_ctl, GFP_ATOMIC);
- if (error)
- dev_err(&dev->intf->dev,
- "%s: usb_submit_urb (urb_ctl) failed %d\n",
- __func__, error);
- }
-
- spin_unlock_irqrestore(&dev->ctl_submit_lock, flags);
+ cm109_submit_ctl(dev);
}
static void cm109_urb_ctl_callback(struct urb *urb)
@@ -434,7 +436,6 @@ static void cm109_urb_ctl_callback(struct urb *urb)
struct cm109_dev *dev = urb->context;
const int status = urb->status;
int error;
- unsigned long flags;
dev_dbg(&dev->intf->dev, "### URB CTL: [0x%02x 0x%02x 0x%02x 0x%02x]\n",
dev->ctl_data->byte[0],
@@ -449,35 +450,31 @@ static void cm109_urb_ctl_callback(struct urb *urb)
__func__, status);
}
- spin_lock_irqsave(&dev->ctl_submit_lock, flags);
+ guard(spinlock_irqsave)(&dev->ctl_submit_lock);
dev->ctl_urb_pending = 0;
- if (likely(!dev->shutdown)) {
-
- if (dev->buzzer_pending || status) {
- dev->buzzer_pending = 0;
- dev->ctl_urb_pending = 1;
- cm109_submit_buzz_toggle(dev);
- } else if (likely(!dev->irq_urb_pending)) {
- /* ask for key data */
- dev->irq_urb_pending = 1;
- error = usb_submit_urb(dev->urb_irq, GFP_ATOMIC);
- if (error)
- dev_err(&dev->intf->dev,
- "%s: usb_submit_urb (urb_irq) failed %d\n",
- __func__, error);
- }
- }
+ if (unlikely(dev->shutdown))
+ return;
- spin_unlock_irqrestore(&dev->ctl_submit_lock, flags);
+ if (dev->buzzer_pending || status) {
+ dev->buzzer_pending = 0;
+ dev->ctl_urb_pending = 1;
+ cm109_submit_buzz_toggle(dev);
+ } else if (likely(!dev->irq_urb_pending)) {
+ /* ask for key data */
+ dev->irq_urb_pending = 1;
+ error = usb_submit_urb(dev->urb_irq, GFP_ATOMIC);
+ if (error)
+ dev_err(&dev->intf->dev,
+ "%s: usb_submit_urb (urb_irq) failed %d\n",
+ __func__, error);
+ }
}
static void cm109_toggle_buzzer_async(struct cm109_dev *dev)
{
- unsigned long flags;
-
- spin_lock_irqsave(&dev->ctl_submit_lock, flags);
+ guard(spinlock_irqsave)(&dev->ctl_submit_lock);
if (dev->ctl_urb_pending) {
/* URB completion will resubmit */
@@ -486,8 +483,6 @@ static void cm109_toggle_buzzer_async(struct cm109_dev *dev)
dev->ctl_urb_pending = 1;
cm109_submit_buzz_toggle(dev);
}
-
- spin_unlock_irqrestore(&dev->ctl_submit_lock, flags);
}
static void cm109_toggle_buzzer_sync(struct cm109_dev *dev, int on)
@@ -556,32 +551,30 @@ static int cm109_input_open(struct input_dev *idev)
return error;
}
- mutex_lock(&dev->pm_mutex);
-
- dev->buzzer_state = 0;
- dev->key_code = -1; /* no keys pressed */
- dev->keybit = 0xf;
+ scoped_guard(mutex, &dev->pm_mutex) {
+ dev->buzzer_state = 0;
+ dev->key_code = -1; /* no keys pressed */
+ dev->keybit = 0xf;
- /* issue INIT */
- dev->ctl_data->byte[HID_OR0] = HID_OR_GPO_BUZ_SPDIF;
- dev->ctl_data->byte[HID_OR1] = dev->keybit;
- dev->ctl_data->byte[HID_OR2] = dev->keybit;
- dev->ctl_data->byte[HID_OR3] = 0x00;
+ /* issue INIT */
+ dev->ctl_data->byte[HID_OR0] = HID_OR_GPO_BUZ_SPDIF;
+ dev->ctl_data->byte[HID_OR1] = dev->keybit;
+ dev->ctl_data->byte[HID_OR2] = dev->keybit;
+ dev->ctl_data->byte[HID_OR3] = 0x00;
- dev->ctl_urb_pending = 1;
- error = usb_submit_urb(dev->urb_ctl, GFP_KERNEL);
- if (error) {
- dev->ctl_urb_pending = 0;
- dev_err(&dev->intf->dev, "%s: usb_submit_urb (urb_ctl) failed %d\n",
- __func__, error);
- } else {
- dev->open = 1;
+ dev->ctl_urb_pending = 1;
+ error = usb_submit_urb(dev->urb_ctl, GFP_KERNEL);
+ if (!error) {
+ dev->open = 1;
+ return 0;
+ }
}
- mutex_unlock(&dev->pm_mutex);
+ dev->ctl_urb_pending = 0;
+ usb_autopm_put_interface(dev->intf);
- if (error)
- usb_autopm_put_interface(dev->intf);
+ dev_err(&dev->intf->dev, "%s: usb_submit_urb (urb_ctl) failed %d\n",
+ __func__, error);
return error;
}
@@ -590,17 +583,15 @@ static void cm109_input_close(struct input_dev *idev)
{
struct cm109_dev *dev = input_get_drvdata(idev);
- mutex_lock(&dev->pm_mutex);
-
- /*
- * Once we are here event delivery is stopped so we
- * don't need to worry about someone starting buzzer
- * again
- */
- cm109_stop_traffic(dev);
- dev->open = 0;
-
- mutex_unlock(&dev->pm_mutex);
+ scoped_guard(mutex, &dev->pm_mutex) {
+ /*
+ * Once we are here event delivery is stopped so we
+ * don't need to worry about someone starting buzzer
+ * again
+ */
+ cm109_stop_traffic(dev);
+ dev->open = 0;
+ }
usb_autopm_put_interface(dev->intf);
}
@@ -823,9 +814,9 @@ static int cm109_usb_suspend(struct usb_interface *intf, pm_message_t message)
dev_info(&intf->dev, "cm109: usb_suspend (event=%d)\n", message.event);
- mutex_lock(&dev->pm_mutex);
+ guard(mutex)(&dev->pm_mutex);
+
cm109_stop_traffic(dev);
- mutex_unlock(&dev->pm_mutex);
return 0;
}
@@ -836,9 +827,9 @@ static int cm109_usb_resume(struct usb_interface *intf)
dev_info(&intf->dev, "cm109: usb_resume\n");
- mutex_lock(&dev->pm_mutex);
+ guard(mutex)(&dev->pm_mutex);
+
cm109_restore_state(dev);
- mutex_unlock(&dev->pm_mutex);
return 0;
}
--
2.46.0.469.g59c65b2a67-goog
^ permalink raw reply related
* [PATCH 02/22] Input: ati_remote2 - use guard notation when acquiring mutex
From: Dmitry Torokhov @ 2024-09-04 4:42 UTC (permalink / raw)
To: linux-input
Cc: Michael Hennerich, Ville Syrjala, Support Opensource, Eddie James,
Andrey Moiseev, Hans de Goede, Javier Carrasco, Jeff LaBundy,
linux-kernel
In-Reply-To: <20240904044244.1042174-1-dmitry.torokhov@gmail.com>
Using guard notation makes the code more compact and error handling
more robust by ensuring that mutexes are released in all code paths
when control leaves critical section.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/misc/ati_remote2.c | 57 +++++++++++---------------------
1 file changed, 19 insertions(+), 38 deletions(-)
diff --git a/drivers/input/misc/ati_remote2.c b/drivers/input/misc/ati_remote2.c
index 795f69edb4b2..e84649af801d 100644
--- a/drivers/input/misc/ati_remote2.c
+++ b/drivers/input/misc/ati_remote2.c
@@ -244,29 +244,21 @@ static int ati_remote2_open(struct input_dev *idev)
if (r) {
dev_err(&ar2->intf[0]->dev,
"%s(): usb_autopm_get_interface() = %d\n", __func__, r);
- goto fail1;
+ return r;
}
- mutex_lock(&ati_remote2_mutex);
+ scoped_guard(mutex, &ati_remote2_mutex) {
+ if (!(ar2->flags & ATI_REMOTE2_SUSPENDED)) {
+ r = ati_remote2_submit_urbs(ar2);
+ if (r)
+ break;
+ }
- if (!(ar2->flags & ATI_REMOTE2_SUSPENDED)) {
- r = ati_remote2_submit_urbs(ar2);
- if (r)
- goto fail2;
+ ar2->flags |= ATI_REMOTE2_OPENED;
}
- ar2->flags |= ATI_REMOTE2_OPENED;
-
- mutex_unlock(&ati_remote2_mutex);
-
usb_autopm_put_interface(ar2->intf[0]);
- return 0;
-
- fail2:
- mutex_unlock(&ati_remote2_mutex);
- usb_autopm_put_interface(ar2->intf[0]);
- fail1:
return r;
}
@@ -276,14 +268,12 @@ static void ati_remote2_close(struct input_dev *idev)
dev_dbg(&ar2->intf[0]->dev, "%s()\n", __func__);
- mutex_lock(&ati_remote2_mutex);
+ guard(mutex)(&ati_remote2_mutex);
if (!(ar2->flags & ATI_REMOTE2_SUSPENDED))
ati_remote2_kill_urbs(ar2);
ar2->flags &= ~ATI_REMOTE2_OPENED;
-
- mutex_unlock(&ati_remote2_mutex);
}
static void ati_remote2_input_mouse(struct ati_remote2 *ar2)
@@ -713,16 +703,14 @@ static ssize_t ati_remote2_store_channel_mask(struct device *dev,
return r;
}
- mutex_lock(&ati_remote2_mutex);
-
- if (mask != ar2->channel_mask) {
- r = ati_remote2_setup(ar2, mask);
- if (!r)
- ar2->channel_mask = mask;
+ scoped_guard(mutex, &ati_remote2_mutex) {
+ if (mask != ar2->channel_mask) {
+ r = ati_remote2_setup(ar2, mask);
+ if (!r)
+ ar2->channel_mask = mask;
+ }
}
- mutex_unlock(&ati_remote2_mutex);
-
usb_autopm_put_interface(ar2->intf[0]);
return r ? r : count;
@@ -892,15 +880,13 @@ static int ati_remote2_suspend(struct usb_interface *interface,
dev_dbg(&ar2->intf[0]->dev, "%s()\n", __func__);
- mutex_lock(&ati_remote2_mutex);
+ guard(mutex)(&ati_remote2_mutex);
if (ar2->flags & ATI_REMOTE2_OPENED)
ati_remote2_kill_urbs(ar2);
ar2->flags |= ATI_REMOTE2_SUSPENDED;
- mutex_unlock(&ati_remote2_mutex);
-
return 0;
}
@@ -917,7 +903,7 @@ static int ati_remote2_resume(struct usb_interface *interface)
dev_dbg(&ar2->intf[0]->dev, "%s()\n", __func__);
- mutex_lock(&ati_remote2_mutex);
+ guard(mutex)(&ati_remote2_mutex);
if (ar2->flags & ATI_REMOTE2_OPENED)
r = ati_remote2_submit_urbs(ar2);
@@ -925,8 +911,6 @@ static int ati_remote2_resume(struct usb_interface *interface)
if (!r)
ar2->flags &= ~ATI_REMOTE2_SUSPENDED;
- mutex_unlock(&ati_remote2_mutex);
-
return r;
}
@@ -943,11 +927,11 @@ static int ati_remote2_reset_resume(struct usb_interface *interface)
dev_dbg(&ar2->intf[0]->dev, "%s()\n", __func__);
- mutex_lock(&ati_remote2_mutex);
+ guard(mutex)(&ati_remote2_mutex);
r = ati_remote2_setup(ar2, ar2->channel_mask);
if (r)
- goto out;
+ return r;
if (ar2->flags & ATI_REMOTE2_OPENED)
r = ati_remote2_submit_urbs(ar2);
@@ -955,9 +939,6 @@ static int ati_remote2_reset_resume(struct usb_interface *interface)
if (!r)
ar2->flags &= ~ATI_REMOTE2_SUSPENDED;
- out:
- mutex_unlock(&ati_remote2_mutex);
-
return r;
}
--
2.46.0.469.g59c65b2a67-goog
^ permalink raw reply related
* [PATCH 01/22] Input: ad714x - use guard notation when acquiring mutex
From: Dmitry Torokhov @ 2024-09-04 4:42 UTC (permalink / raw)
To: linux-input
Cc: Michael Hennerich, Ville Syrjala, Support Opensource, Eddie James,
Andrey Moiseev, Hans de Goede, Javier Carrasco, Jeff LaBundy,
linux-kernel
In-Reply-To: <20240904044244.1042174-1-dmitry.torokhov@gmail.com>
Using guard notation makes the code more compact and error handling
more robust by ensuring that mutexes are released in all code paths
when control leaves critical section.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/misc/ad714x.c | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/drivers/input/misc/ad714x.c b/drivers/input/misc/ad714x.c
index 1acd8429c56c..d106f37df6bc 100644
--- a/drivers/input/misc/ad714x.c
+++ b/drivers/input/misc/ad714x.c
@@ -941,7 +941,7 @@ static irqreturn_t ad714x_interrupt_thread(int irq, void *data)
struct ad714x_chip *ad714x = data;
int i;
- mutex_lock(&ad714x->mutex);
+ guard(mutex)(&ad714x->mutex);
ad714x->read(ad714x, STG_LOW_INT_STA_REG, &ad714x->l_state, 3);
@@ -954,8 +954,6 @@ static irqreturn_t ad714x_interrupt_thread(int irq, void *data)
for (i = 0; i < ad714x->hw->touchpad_num; i++)
ad714x_touchpad_state_machine(ad714x, i);
- mutex_unlock(&ad714x->mutex);
-
return IRQ_HANDLED;
}
@@ -1169,13 +1167,11 @@ static int ad714x_suspend(struct device *dev)
dev_dbg(ad714x->dev, "%s enter\n", __func__);
- mutex_lock(&ad714x->mutex);
+ guard(mutex)(&ad714x->mutex);
data = ad714x->hw->sys_cfg_reg[AD714X_PWR_CTRL] | 0x3;
ad714x->write(ad714x, AD714X_PWR_CTRL, data);
- mutex_unlock(&ad714x->mutex);
-
return 0;
}
@@ -1184,7 +1180,7 @@ static int ad714x_resume(struct device *dev)
struct ad714x_chip *ad714x = dev_get_drvdata(dev);
dev_dbg(ad714x->dev, "%s enter\n", __func__);
- mutex_lock(&ad714x->mutex);
+ guard(mutex)(&ad714x->mutex);
/* resume to non-shutdown mode */
@@ -1197,8 +1193,6 @@ static int ad714x_resume(struct device *dev)
ad714x->read(ad714x, STG_LOW_INT_STA_REG, &ad714x->l_state, 3);
- mutex_unlock(&ad714x->mutex);
-
return 0;
}
--
2.46.0.469.g59c65b2a67-goog
^ permalink raw reply related
* [PATCH 00/22] Convert misc input drivers to use new cleanup facilities
From: Dmitry Torokhov @ 2024-09-04 4:42 UTC (permalink / raw)
To: linux-input
Cc: Michael Hennerich, Ville Syrjala, Support Opensource, Eddie James,
Andrey Moiseev, Hans de Goede, Javier Carrasco, Jeff LaBundy,
linux-kernel
Hi,
This series converts drivers found in drivers/input/misc to use new
__free() and guard() cleanup facilities that simplify the code and
ensure that all resources are released appropriately.
Thanks!
Dmitry Torokhov (22):
Input: ad714x - use guard notation when acquiring mutex
Input: ati_remote2 - use guard notation when acquiring mutex
Input: cm109 - use guard notation when acquiring mutex and spinlock
Input: cma3000_d0x - use guard notation when acquiring mutex
Input: da7280 - use guard notation when acquiring mutex and spinlock
Input: kxtj9 - use guard notation when acquiring mutex/disabling irq
Input: drv260x - use guard notation when acquiring mutex
Input: drv2665 - use guard notation when acquiring mutex
Input: drv2667 - use guard notation when acquiring mutex
Input: ideapad_slidebar - use guard notation when acquiring spinlock
Input: ibm-panel - use guard notation when acquiring spinlock
Input: iqs269a - use guard notation when acquiring mutex
Input: iqs269a - use cleanup facility for fwnodes
Input: iqs626a - use cleanup facility for fwnodes
Input: iqs7222 - use cleanup facility for fwnodes
Input: max8997_haptic - use guard notation when acquiring mutex
Input: pegasus_notetaker - use guard notation when acquiring mutex
Input: powermate - use guard notation when acquiring spinlock
Input: pwm-beeper - use guard notation when acquiring spinlock
Input: regulator-haptic - use guard notation when acquiring mutex
Input: rotary_encoder - use guard notation when acquiring mutex
Input: sparcspkr - use guard notation when acquiring spinlock
drivers/input/misc/ad714x.c | 12 +-
drivers/input/misc/ati_remote2.c | 57 +++-----
drivers/input/misc/cm109.c | 167 +++++++++++------------
drivers/input/misc/cma3000_d0x.c | 16 +--
drivers/input/misc/da7280.c | 26 ++--
drivers/input/misc/drv260x.c | 50 ++++---
drivers/input/misc/drv2665.c | 44 +++---
drivers/input/misc/drv2667.c | 44 +++---
drivers/input/misc/ibm-panel.c | 5 +-
drivers/input/misc/ideapad_slidebar.c | 22 +--
drivers/input/misc/iqs269a.c | 55 +++-----
drivers/input/misc/iqs626a.c | 22 +--
drivers/input/misc/iqs7222.c | 30 ++--
drivers/input/misc/kxtj9.c | 14 +-
drivers/input/misc/max8997_haptic.c | 15 +-
drivers/input/misc/powermate.c | 11 +-
drivers/input/misc/pwm-beeper.c | 12 +-
drivers/input/misc/regulator-haptic.c | 23 ++--
drivers/input/misc/rotary_encoder.c | 23 ++--
drivers/input/misc/sparcspkr.c | 10 +-
drivers/input/tablet/pegasus_notetaker.c | 86 ++++++------
21 files changed, 311 insertions(+), 433 deletions(-)
--
Dmitry
^ permalink raw reply
* [PATCH 6/6] Input: xpad - use guard notation when acquiring mutex and spinlock
From: Dmitry Torokhov @ 2024-09-04 4:31 UTC (permalink / raw)
To: linux-input; +Cc: Erick Archer, Christophe JAILLET, linux-kernel
In-Reply-To: <20240904043104.1030257-1-dmitry.torokhov@gmail.com>
Using guard notation makes the code more compact and error handling
more robust by ensuring that locks are released in all code paths
when control leaves critical section.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/joystick/xpad.c | 99 ++++++++++++-----------------------
1 file changed, 34 insertions(+), 65 deletions(-)
diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index 4eda18f4f46e..3e61df927277 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -1289,9 +1289,8 @@ static void xpad_irq_out(struct urb *urb)
struct device *dev = &xpad->intf->dev;
int status = urb->status;
int error;
- unsigned long flags;
- spin_lock_irqsave(&xpad->odata_lock, flags);
+ guard(spinlock_irqsave)(&xpad->odata_lock);
switch (status) {
case 0:
@@ -1325,8 +1324,6 @@ static void xpad_irq_out(struct urb *urb)
xpad->irq_out_active = false;
}
}
-
- spin_unlock_irqrestore(&xpad->odata_lock, flags);
}
static int xpad_init_output(struct usb_interface *intf, struct usb_xpad *xpad,
@@ -1391,10 +1388,8 @@ static int xpad_inquiry_pad_presence(struct usb_xpad *xpad)
{
struct xpad_output_packet *packet =
&xpad->out_packets[XPAD_OUT_CMD_IDX];
- unsigned long flags;
- int retval;
- spin_lock_irqsave(&xpad->odata_lock, flags);
+ guard(spinlock_irqsave)(&xpad->odata_lock);
packet->data[0] = 0x08;
packet->data[1] = 0x00;
@@ -1413,17 +1408,12 @@ static int xpad_inquiry_pad_presence(struct usb_xpad *xpad)
/* Reset the sequence so we send out presence first */
xpad->last_out_packet = -1;
- retval = xpad_try_sending_next_out_packet(xpad);
-
- spin_unlock_irqrestore(&xpad->odata_lock, flags);
-
- return retval;
+ return xpad_try_sending_next_out_packet(xpad);
}
static int xpad_start_xbox_one(struct usb_xpad *xpad)
{
- unsigned long flags;
- int retval;
+ int error;
if (usb_ifnum_to_if(xpad->udev, GIP_WIRED_INTF_AUDIO)) {
/*
@@ -1432,15 +1422,15 @@ static int xpad_start_xbox_one(struct usb_xpad *xpad)
* Controller for Series X|S (0x20d6:0x200e) to report the
* guide button.
*/
- retval = usb_set_interface(xpad->udev,
- GIP_WIRED_INTF_AUDIO, 0);
- if (retval)
+ error = usb_set_interface(xpad->udev,
+ GIP_WIRED_INTF_AUDIO, 0);
+ if (error)
dev_warn(&xpad->dev->dev,
"unable to disable audio interface: %d\n",
- retval);
+ error);
}
- spin_lock_irqsave(&xpad->odata_lock, flags);
+ guard(spinlock_irqsave)(&xpad->odata_lock);
/*
* Begin the init sequence by attempting to send a packet.
@@ -1448,16 +1438,11 @@ static int xpad_start_xbox_one(struct usb_xpad *xpad)
* sending any packets from the output ring.
*/
xpad->init_seq = 0;
- retval = xpad_try_sending_next_out_packet(xpad);
-
- spin_unlock_irqrestore(&xpad->odata_lock, flags);
-
- return retval;
+ return xpad_try_sending_next_out_packet(xpad);
}
static void xpadone_ack_mode_report(struct usb_xpad *xpad, u8 seq_num)
{
- unsigned long flags;
struct xpad_output_packet *packet =
&xpad->out_packets[XPAD_OUT_CMD_IDX];
static const u8 mode_report_ack[] = {
@@ -1465,7 +1450,7 @@ static void xpadone_ack_mode_report(struct usb_xpad *xpad, u8 seq_num)
0x00, GIP_CMD_VIRTUAL_KEY, GIP_OPT_INTERNAL, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00
};
- spin_lock_irqsave(&xpad->odata_lock, flags);
+ guard(spinlock_irqsave)(&xpad->odata_lock);
packet->len = sizeof(mode_report_ack);
memcpy(packet->data, mode_report_ack, packet->len);
@@ -1475,8 +1460,6 @@ static void xpadone_ack_mode_report(struct usb_xpad *xpad, u8 seq_num)
/* Reset the sequence so we send out the ack now */
xpad->last_out_packet = -1;
xpad_try_sending_next_out_packet(xpad);
-
- spin_unlock_irqrestore(&xpad->odata_lock, flags);
}
#ifdef CONFIG_JOYSTICK_XPAD_FF
@@ -1486,8 +1469,6 @@ static int xpad_play_effect(struct input_dev *dev, void *data, struct ff_effect
struct xpad_output_packet *packet = &xpad->out_packets[XPAD_OUT_FF_IDX];
__u16 strong;
__u16 weak;
- int retval;
- unsigned long flags;
if (effect->type != FF_RUMBLE)
return 0;
@@ -1495,7 +1476,7 @@ static int xpad_play_effect(struct input_dev *dev, void *data, struct ff_effect
strong = effect->u.rumble.strong_magnitude;
weak = effect->u.rumble.weak_magnitude;
- spin_lock_irqsave(&xpad->odata_lock, flags);
+ guard(spinlock_irqsave)(&xpad->odata_lock);
switch (xpad->xtype) {
case XTYPE_XBOX:
@@ -1561,15 +1542,10 @@ static int xpad_play_effect(struct input_dev *dev, void *data, struct ff_effect
dev_dbg(&xpad->dev->dev,
"%s - rumble command sent to unsupported xpad type: %d\n",
__func__, xpad->xtype);
- retval = -EINVAL;
- goto out;
+ return -EINVAL;
}
- retval = xpad_try_sending_next_out_packet(xpad);
-
-out:
- spin_unlock_irqrestore(&xpad->odata_lock, flags);
- return retval;
+ return xpad_try_sending_next_out_packet(xpad);
}
static int xpad_init_ff(struct usb_xpad *xpad)
@@ -1622,11 +1598,10 @@ static void xpad_send_led_command(struct usb_xpad *xpad, int command)
{
struct xpad_output_packet *packet =
&xpad->out_packets[XPAD_OUT_LED_IDX];
- unsigned long flags;
command %= 16;
- spin_lock_irqsave(&xpad->odata_lock, flags);
+ guard(spinlock_irqsave)(&xpad->odata_lock);
switch (xpad->xtype) {
case XTYPE_XBOX360:
@@ -1656,8 +1631,6 @@ static void xpad_send_led_command(struct usb_xpad *xpad, int command)
}
xpad_try_sending_next_out_packet(xpad);
-
- spin_unlock_irqrestore(&xpad->odata_lock, flags);
}
/*
@@ -1782,11 +1755,10 @@ static void xpad_stop_input(struct usb_xpad *xpad)
static void xpad360w_poweroff_controller(struct usb_xpad *xpad)
{
- unsigned long flags;
struct xpad_output_packet *packet =
&xpad->out_packets[XPAD_OUT_CMD_IDX];
- spin_lock_irqsave(&xpad->odata_lock, flags);
+ guard(spinlock_irqsave)(&xpad->odata_lock);
packet->data[0] = 0x00;
packet->data[1] = 0x00;
@@ -1806,8 +1778,6 @@ static void xpad360w_poweroff_controller(struct usb_xpad *xpad)
/* Reset the sequence so we send out poweroff now */
xpad->last_out_packet = -1;
xpad_try_sending_next_out_packet(xpad);
-
- spin_unlock_irqrestore(&xpad->odata_lock, flags);
}
static int xpad360w_start_input(struct usb_xpad *xpad)
@@ -2231,10 +2201,10 @@ static int xpad_suspend(struct usb_interface *intf, pm_message_t message)
if (auto_poweroff && xpad->pad_present)
xpad360w_poweroff_controller(xpad);
} else {
- mutex_lock(&input->mutex);
+ guard(mutex)(&input->mutex);
+
if (input_device_enabled(input))
xpad_stop_input(xpad);
- mutex_unlock(&input->mutex);
}
xpad_stop_output(xpad);
@@ -2246,26 +2216,25 @@ static int xpad_resume(struct usb_interface *intf)
{
struct usb_xpad *xpad = usb_get_intfdata(intf);
struct input_dev *input = xpad->dev;
- int retval = 0;
- if (xpad->xtype == XTYPE_XBOX360W) {
- retval = xpad360w_start_input(xpad);
- } else {
- mutex_lock(&input->mutex);
- if (input_device_enabled(input)) {
- retval = xpad_start_input(xpad);
- } else if (xpad->xtype == XTYPE_XBOXONE) {
- /*
- * Even if there are no users, we'll send Xbox One pads
- * the startup sequence so they don't sit there and
- * blink until somebody opens the input device again.
- */
- retval = xpad_start_xbox_one(xpad);
- }
- mutex_unlock(&input->mutex);
+ if (xpad->xtype == XTYPE_XBOX360W)
+ return xpad360w_start_input(xpad);
+
+ guard(mutex)(&input->mutex);
+
+ if (input_device_enabled(input))
+ return xpad_start_input(xpad);
+
+ if (xpad->xtype == XTYPE_XBOXONE) {
+ /*
+ * Even if there are no users, we'll send Xbox One pads
+ * the startup sequence so they don't sit there and
+ * blink until somebody opens the input device again.
+ */
+ return xpad_start_xbox_one(xpad);
}
- return retval;
+ return 0;
}
static struct usb_driver xpad_driver = {
--
2.46.0.469.g59c65b2a67-goog
^ permalink raw reply related
* [PATCH 5/6] Input: turbografx - use guard notation when acquiring mutex
From: Dmitry Torokhov @ 2024-09-04 4:31 UTC (permalink / raw)
To: linux-input; +Cc: Erick Archer, Christophe JAILLET, linux-kernel
In-Reply-To: <20240904043104.1030257-1-dmitry.torokhov@gmail.com>
Using guard notation makes the code more compact and error handling
more robust by ensuring that mutexes are released in all code paths
when control leaves critical section.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/joystick/turbografx.c | 22 ++++++++++------------
1 file changed, 10 insertions(+), 12 deletions(-)
diff --git a/drivers/input/joystick/turbografx.c b/drivers/input/joystick/turbografx.c
index eb8455c34e67..015010a928aa 100644
--- a/drivers/input/joystick/turbografx.c
+++ b/drivers/input/joystick/turbografx.c
@@ -103,33 +103,31 @@ static void tgfx_timer(struct timer_list *t)
static int tgfx_open(struct input_dev *dev)
{
struct tgfx *tgfx = input_get_drvdata(dev);
- int err;
- err = mutex_lock_interruptible(&tgfx->sem);
- if (err)
- return err;
+ scoped_guard(mutex_intr, &tgfx->sem) {
+ if (!tgfx->used++) {
+ parport_claim(tgfx->pd);
+ parport_write_control(tgfx->pd->port, 0x04);
+ mod_timer(&tgfx->timer, jiffies + TGFX_REFRESH_TIME);
+ }
- if (!tgfx->used++) {
- parport_claim(tgfx->pd);
- parport_write_control(tgfx->pd->port, 0x04);
- mod_timer(&tgfx->timer, jiffies + TGFX_REFRESH_TIME);
+ return 0;
}
- mutex_unlock(&tgfx->sem);
- return 0;
+ return -EINTR;
}
static void tgfx_close(struct input_dev *dev)
{
struct tgfx *tgfx = input_get_drvdata(dev);
- mutex_lock(&tgfx->sem);
+ guard(mutex)(&tgfx->sem);
+
if (!--tgfx->used) {
del_timer_sync(&tgfx->timer);
parport_write_control(tgfx->pd->port, 0x00);
parport_release(tgfx->pd);
}
- mutex_unlock(&tgfx->sem);
}
--
2.46.0.469.g59c65b2a67-goog
^ permalink raw reply related
* [PATCH 4/6] Input: n64joy - use guard notation when acquiring mutex
From: Dmitry Torokhov @ 2024-09-04 4:31 UTC (permalink / raw)
To: linux-input; +Cc: Erick Archer, Christophe JAILLET, linux-kernel
In-Reply-To: <20240904043104.1030257-1-dmitry.torokhov@gmail.com>
Using guard notation makes the code more compact and error handling
more robust by ensuring that mutexes are released in all code paths
when control leaves critical section.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/joystick/n64joy.c | 35 +++++++++++++++------------------
1 file changed, 16 insertions(+), 19 deletions(-)
diff --git a/drivers/input/joystick/n64joy.c b/drivers/input/joystick/n64joy.c
index b0986d2195d6..c344dbc0c493 100644
--- a/drivers/input/joystick/n64joy.c
+++ b/drivers/input/joystick/n64joy.c
@@ -191,35 +191,32 @@ static void n64joy_poll(struct timer_list *t)
static int n64joy_open(struct input_dev *dev)
{
struct n64joy_priv *priv = input_get_drvdata(dev);
- int err;
-
- err = mutex_lock_interruptible(&priv->n64joy_mutex);
- if (err)
- return err;
-
- if (!priv->n64joy_opened) {
- /*
- * We could use the vblank irq, but it's not important if
- * the poll point slightly changes.
- */
- timer_setup(&priv->timer, n64joy_poll, 0);
- mod_timer(&priv->timer, jiffies + msecs_to_jiffies(16));
- }
- priv->n64joy_opened++;
+ scoped_guard(mutex_intr, &priv->n64joy_mutex) {
+ if (!priv->n64joy_opened) {
+ /*
+ * We could use the vblank irq, but it's not important
+ * if the poll point slightly changes.
+ */
+ timer_setup(&priv->timer, n64joy_poll, 0);
+ mod_timer(&priv->timer, jiffies + msecs_to_jiffies(16));
+ }
- mutex_unlock(&priv->n64joy_mutex);
- return err;
+ priv->n64joy_opened++;
+ return 0;
+ }
+
+ return -EINTR;
}
static void n64joy_close(struct input_dev *dev)
{
struct n64joy_priv *priv = input_get_drvdata(dev);
- mutex_lock(&priv->n64joy_mutex);
+ guard(mutex)(&priv->n64joy_mutex);
+
if (!--priv->n64joy_opened)
del_timer_sync(&priv->timer);
- mutex_unlock(&priv->n64joy_mutex);
}
static const u64 __initconst scandata[] ____cacheline_aligned = {
--
2.46.0.469.g59c65b2a67-goog
^ permalink raw reply related
* [PATCH 3/6] Input: iforce - use guard notation when acquiring mutex and spinlock
From: Dmitry Torokhov @ 2024-09-04 4:31 UTC (permalink / raw)
To: linux-input; +Cc: Erick Archer, Christophe JAILLET, linux-kernel
In-Reply-To: <20240904043104.1030257-1-dmitry.torokhov@gmail.com>
Using guard notation makes the code more compact and error handling
more robust by ensuring that locks are released in all code paths
when control leaves critical section.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/joystick/iforce/iforce-ff.c | 48 +++++++---------
.../input/joystick/iforce/iforce-packets.c | 57 ++++++++-----------
drivers/input/joystick/iforce/iforce-serio.c | 36 +++++-------
drivers/input/joystick/iforce/iforce-usb.c | 13 ++---
4 files changed, 68 insertions(+), 86 deletions(-)
diff --git a/drivers/input/joystick/iforce/iforce-ff.c b/drivers/input/joystick/iforce/iforce-ff.c
index 95c0348843e6..8c78cbe553c8 100644
--- a/drivers/input/joystick/iforce/iforce-ff.c
+++ b/drivers/input/joystick/iforce/iforce-ff.c
@@ -21,14 +21,13 @@ static int make_magnitude_modifier(struct iforce* iforce,
unsigned char data[3];
if (!no_alloc) {
- mutex_lock(&iforce->mem_mutex);
- if (allocate_resource(&(iforce->device_memory), mod_chunk, 2,
- iforce->device_memory.start, iforce->device_memory.end, 2L,
- NULL, NULL)) {
- mutex_unlock(&iforce->mem_mutex);
+ guard(mutex)(&iforce->mem_mutex);
+
+ if (allocate_resource(&iforce->device_memory, mod_chunk, 2,
+ iforce->device_memory.start,
+ iforce->device_memory.end,
+ 2L, NULL, NULL))
return -ENOSPC;
- }
- mutex_unlock(&iforce->mem_mutex);
}
data[0] = LO(mod_chunk->start);
@@ -54,14 +53,13 @@ static int make_period_modifier(struct iforce* iforce,
period = TIME_SCALE(period);
if (!no_alloc) {
- mutex_lock(&iforce->mem_mutex);
- if (allocate_resource(&(iforce->device_memory), mod_chunk, 0x0c,
- iforce->device_memory.start, iforce->device_memory.end, 2L,
- NULL, NULL)) {
- mutex_unlock(&iforce->mem_mutex);
+ guard(mutex)(&iforce->mem_mutex);
+
+ if (allocate_resource(&iforce->device_memory, mod_chunk, 0x0c,
+ iforce->device_memory.start,
+ iforce->device_memory.end,
+ 2L, NULL, NULL))
return -ENOSPC;
- }
- mutex_unlock(&iforce->mem_mutex);
}
data[0] = LO(mod_chunk->start);
@@ -94,14 +92,13 @@ static int make_envelope_modifier(struct iforce* iforce,
fade_duration = TIME_SCALE(fade_duration);
if (!no_alloc) {
- mutex_lock(&iforce->mem_mutex);
+ guard(mutex)(&iforce->mem_mutex);
+
if (allocate_resource(&(iforce->device_memory), mod_chunk, 0x0e,
- iforce->device_memory.start, iforce->device_memory.end, 2L,
- NULL, NULL)) {
- mutex_unlock(&iforce->mem_mutex);
+ iforce->device_memory.start,
+ iforce->device_memory.end,
+ 2L, NULL, NULL))
return -ENOSPC;
- }
- mutex_unlock(&iforce->mem_mutex);
}
data[0] = LO(mod_chunk->start);
@@ -131,14 +128,13 @@ static int make_condition_modifier(struct iforce* iforce,
unsigned char data[10];
if (!no_alloc) {
- mutex_lock(&iforce->mem_mutex);
+ guard(mutex)(&iforce->mem_mutex);
+
if (allocate_resource(&(iforce->device_memory), mod_chunk, 8,
- iforce->device_memory.start, iforce->device_memory.end, 2L,
- NULL, NULL)) {
- mutex_unlock(&iforce->mem_mutex);
+ iforce->device_memory.start,
+ iforce->device_memory.end,
+ 2L, NULL, NULL))
return -ENOSPC;
- }
- mutex_unlock(&iforce->mem_mutex);
}
data[0] = LO(mod_chunk->start);
diff --git a/drivers/input/joystick/iforce/iforce-packets.c b/drivers/input/joystick/iforce/iforce-packets.c
index 763642c8cee9..8c2531e2977c 100644
--- a/drivers/input/joystick/iforce/iforce-packets.c
+++ b/drivers/input/joystick/iforce/iforce-packets.c
@@ -31,49 +31,42 @@ int iforce_send_packet(struct iforce *iforce, u16 cmd, unsigned char* data)
int c;
int empty;
int head, tail;
- unsigned long flags;
/*
* Update head and tail of xmit buffer
*/
- spin_lock_irqsave(&iforce->xmit_lock, flags);
-
- head = iforce->xmit.head;
- tail = iforce->xmit.tail;
-
-
- if (CIRC_SPACE(head, tail, XMIT_SIZE) < n+2) {
- dev_warn(&iforce->dev->dev,
- "not enough space in xmit buffer to send new packet\n");
- spin_unlock_irqrestore(&iforce->xmit_lock, flags);
- return -1;
- }
+ scoped_guard(spinlock_irqsave, &iforce->xmit_lock) {
+ head = iforce->xmit.head;
+ tail = iforce->xmit.tail;
+
+ if (CIRC_SPACE(head, tail, XMIT_SIZE) < n + 2) {
+ dev_warn(&iforce->dev->dev,
+ "not enough space in xmit buffer to send new packet\n");
+ return -1;
+ }
- empty = head == tail;
- XMIT_INC(iforce->xmit.head, n+2);
+ empty = head == tail;
+ XMIT_INC(iforce->xmit.head, n + 2);
/*
* Store packet in xmit buffer
*/
- iforce->xmit.buf[head] = HI(cmd);
- XMIT_INC(head, 1);
- iforce->xmit.buf[head] = LO(cmd);
- XMIT_INC(head, 1);
-
- c = CIRC_SPACE_TO_END(head, tail, XMIT_SIZE);
- if (n < c) c=n;
-
- memcpy(&iforce->xmit.buf[head],
- data,
- c);
- if (n != c) {
- memcpy(&iforce->xmit.buf[0],
- data + c,
- n - c);
+ iforce->xmit.buf[head] = HI(cmd);
+ XMIT_INC(head, 1);
+ iforce->xmit.buf[head] = LO(cmd);
+ XMIT_INC(head, 1);
+
+ c = CIRC_SPACE_TO_END(head, tail, XMIT_SIZE);
+ if (n < c)
+ c = n;
+
+ memcpy(&iforce->xmit.buf[head], data, c);
+ if (n != c)
+ memcpy(&iforce->xmit.buf[0], data + c, n - c);
+
+ XMIT_INC(head, n);
}
- XMIT_INC(head, n);
- spin_unlock_irqrestore(&iforce->xmit_lock, flags);
/*
* If necessary, start the transmission
*/
diff --git a/drivers/input/joystick/iforce/iforce-serio.c b/drivers/input/joystick/iforce/iforce-serio.c
index 2380546d7978..75b85c46dfa4 100644
--- a/drivers/input/joystick/iforce/iforce-serio.c
+++ b/drivers/input/joystick/iforce/iforce-serio.c
@@ -28,45 +28,39 @@ static void iforce_serio_xmit(struct iforce *iforce)
iforce);
unsigned char cs;
int i;
- unsigned long flags;
if (test_and_set_bit(IFORCE_XMIT_RUNNING, iforce->xmit_flags)) {
set_bit(IFORCE_XMIT_AGAIN, iforce->xmit_flags);
return;
}
- spin_lock_irqsave(&iforce->xmit_lock, flags);
+ guard(spinlock_irqsave)(&iforce->xmit_lock);
-again:
- if (iforce->xmit.head == iforce->xmit.tail) {
- iforce_clear_xmit_and_wake(iforce);
- spin_unlock_irqrestore(&iforce->xmit_lock, flags);
- return;
- }
+ do {
+ if (iforce->xmit.head == iforce->xmit.tail)
+ break;
- cs = 0x2b;
+ cs = 0x2b;
- serio_write(iforce_serio->serio, 0x2b);
+ serio_write(iforce_serio->serio, 0x2b);
- serio_write(iforce_serio->serio, iforce->xmit.buf[iforce->xmit.tail]);
- cs ^= iforce->xmit.buf[iforce->xmit.tail];
- XMIT_INC(iforce->xmit.tail, 1);
-
- for (i=iforce->xmit.buf[iforce->xmit.tail]; i >= 0; --i) {
serio_write(iforce_serio->serio,
iforce->xmit.buf[iforce->xmit.tail]);
cs ^= iforce->xmit.buf[iforce->xmit.tail];
XMIT_INC(iforce->xmit.tail, 1);
- }
- serio_write(iforce_serio->serio, cs);
+ for (i = iforce->xmit.buf[iforce->xmit.tail]; i >= 0; --i) {
+ serio_write(iforce_serio->serio,
+ iforce->xmit.buf[iforce->xmit.tail]);
+ cs ^= iforce->xmit.buf[iforce->xmit.tail];
+ XMIT_INC(iforce->xmit.tail, 1);
+ }
- if (test_and_clear_bit(IFORCE_XMIT_AGAIN, iforce->xmit_flags))
- goto again;
+ serio_write(iforce_serio->serio, cs);
- iforce_clear_xmit_and_wake(iforce);
+ } while (test_and_clear_bit(IFORCE_XMIT_AGAIN, iforce->xmit_flags));
- spin_unlock_irqrestore(&iforce->xmit_lock, flags);
+ iforce_clear_xmit_and_wake(iforce);
}
static int iforce_serio_get_id(struct iforce *iforce, u8 id,
diff --git a/drivers/input/joystick/iforce/iforce-usb.c b/drivers/input/joystick/iforce/iforce-usb.c
index cba92bd590a8..1f00f76b0174 100644
--- a/drivers/input/joystick/iforce/iforce-usb.c
+++ b/drivers/input/joystick/iforce/iforce-usb.c
@@ -25,13 +25,11 @@ static void __iforce_usb_xmit(struct iforce *iforce)
struct iforce_usb *iforce_usb = container_of(iforce, struct iforce_usb,
iforce);
int n, c;
- unsigned long flags;
- spin_lock_irqsave(&iforce->xmit_lock, flags);
+ guard(spinlock_irqsave)(&iforce->xmit_lock);
if (iforce->xmit.head == iforce->xmit.tail) {
iforce_clear_xmit_and_wake(iforce);
- spin_unlock_irqrestore(&iforce->xmit_lock, flags);
return;
}
@@ -45,7 +43,8 @@ static void __iforce_usb_xmit(struct iforce *iforce)
/* Copy rest of data then */
c = CIRC_CNT_TO_END(iforce->xmit.head, iforce->xmit.tail, XMIT_SIZE);
- if (n < c) c=n;
+ if (n < c)
+ c = n;
memcpy(iforce_usb->out->transfer_buffer + 1,
&iforce->xmit.buf[iforce->xmit.tail],
@@ -53,11 +52,12 @@ static void __iforce_usb_xmit(struct iforce *iforce)
if (n != c) {
memcpy(iforce_usb->out->transfer_buffer + 1 + c,
&iforce->xmit.buf[0],
- n-c);
+ n - c);
}
XMIT_INC(iforce->xmit.tail, n);
- if ( (n=usb_submit_urb(iforce_usb->out, GFP_ATOMIC)) ) {
+ n=usb_submit_urb(iforce_usb->out, GFP_ATOMIC);
+ if (n) {
dev_warn(&iforce_usb->intf->dev,
"usb_submit_urb failed %d\n", n);
iforce_clear_xmit_and_wake(iforce);
@@ -66,7 +66,6 @@ static void __iforce_usb_xmit(struct iforce *iforce)
/* The IFORCE_XMIT_RUNNING bit is not cleared here. That's intended.
* As long as the urb completion handler is not called, the transmiting
* is considered to be running */
- spin_unlock_irqrestore(&iforce->xmit_lock, flags);
}
static void iforce_usb_xmit(struct iforce *iforce)
--
2.46.0.469.g59c65b2a67-goog
^ permalink raw reply related
* [PATCH 2/6] Input: gamecon - use guard notation when acquiring mutex
From: Dmitry Torokhov @ 2024-09-04 4:30 UTC (permalink / raw)
To: linux-input; +Cc: Erick Archer, Christophe JAILLET, linux-kernel
In-Reply-To: <20240904043104.1030257-1-dmitry.torokhov@gmail.com>
Using guard notation makes the code more compact and error handling
more robust by ensuring that mutexes are released in all code paths
when control leaves critical section.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/joystick/gamecon.c | 22 ++++++++++------------
1 file changed, 10 insertions(+), 12 deletions(-)
diff --git a/drivers/input/joystick/gamecon.c b/drivers/input/joystick/gamecon.c
index c38de3094553..968c0e653f2e 100644
--- a/drivers/input/joystick/gamecon.c
+++ b/drivers/input/joystick/gamecon.c
@@ -765,33 +765,31 @@ static void gc_timer(struct timer_list *t)
static int gc_open(struct input_dev *dev)
{
struct gc *gc = input_get_drvdata(dev);
- int err;
- err = mutex_lock_interruptible(&gc->mutex);
- if (err)
- return err;
+ scoped_guard(mutex_intr, &gc->mutex) {
+ if (!gc->used++) {
+ parport_claim(gc->pd);
+ parport_write_control(gc->pd->port, 0x04);
+ mod_timer(&gc->timer, jiffies + GC_REFRESH_TIME);
+ }
- if (!gc->used++) {
- parport_claim(gc->pd);
- parport_write_control(gc->pd->port, 0x04);
- mod_timer(&gc->timer, jiffies + GC_REFRESH_TIME);
+ return 0;
}
- mutex_unlock(&gc->mutex);
- return 0;
+ return -EINTR;
}
static void gc_close(struct input_dev *dev)
{
struct gc *gc = input_get_drvdata(dev);
- mutex_lock(&gc->mutex);
+ guard(mutex)(&gc->mutex);
+
if (!--gc->used) {
del_timer_sync(&gc->timer);
parport_write_control(gc->pd->port, 0x00);
parport_release(gc->pd);
}
- mutex_unlock(&gc->mutex);
}
static int gc_setup_pad(struct gc *gc, int idx, int pad_type)
--
2.46.0.469.g59c65b2a67-goog
^ permalink raw reply related
* [PATCH 1/6] Input: db9 - use guard notation when acquiring mutex
From: Dmitry Torokhov @ 2024-09-04 4:30 UTC (permalink / raw)
To: linux-input; +Cc: Erick Archer, Christophe JAILLET, linux-kernel
In-Reply-To: <20240904043104.1030257-1-dmitry.torokhov@gmail.com>
Using guard notation makes the code more compact and error handling
more robust by ensuring that mutexes are released in all code paths
when control leaves critical section.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/joystick/db9.c | 30 ++++++++++++++----------------
1 file changed, 14 insertions(+), 16 deletions(-)
diff --git a/drivers/input/joystick/db9.c b/drivers/input/joystick/db9.c
index 682a29c27832..7ac0cfc3e786 100644
--- a/drivers/input/joystick/db9.c
+++ b/drivers/input/joystick/db9.c
@@ -505,24 +505,22 @@ static int db9_open(struct input_dev *dev)
{
struct db9 *db9 = input_get_drvdata(dev);
struct parport *port = db9->pd->port;
- int err;
- err = mutex_lock_interruptible(&db9->mutex);
- if (err)
- return err;
-
- if (!db9->used++) {
- parport_claim(db9->pd);
- parport_write_data(port, 0xff);
- if (db9_modes[db9->mode].reverse) {
- parport_data_reverse(port);
- parport_write_control(port, DB9_NORMAL);
+ scoped_guard(mutex_intr, &db9->mutex) {
+ if (!db9->used++) {
+ parport_claim(db9->pd);
+ parport_write_data(port, 0xff);
+ if (db9_modes[db9->mode].reverse) {
+ parport_data_reverse(port);
+ parport_write_control(port, DB9_NORMAL);
+ }
+ mod_timer(&db9->timer, jiffies + DB9_REFRESH_TIME);
}
- mod_timer(&db9->timer, jiffies + DB9_REFRESH_TIME);
+
+ return 0;
}
- mutex_unlock(&db9->mutex);
- return 0;
+ return -EINTR;
}
static void db9_close(struct input_dev *dev)
@@ -530,14 +528,14 @@ static void db9_close(struct input_dev *dev)
struct db9 *db9 = input_get_drvdata(dev);
struct parport *port = db9->pd->port;
- mutex_lock(&db9->mutex);
+ guard(mutex)(&db9->mutex);
+
if (!--db9->used) {
del_timer_sync(&db9->timer);
parport_write_control(port, 0x00);
parport_data_forward(port);
parport_release(db9->pd);
}
- mutex_unlock(&db9->mutex);
}
static void db9_attach(struct parport *pp)
--
2.46.0.469.g59c65b2a67-goog
^ permalink raw reply related
* [PATCH 0/6] Convert joystick drivers to use new cleanup facilities
From: Dmitry Torokhov @ 2024-09-04 4:30 UTC (permalink / raw)
To: linux-input; +Cc: Erick Archer, Christophe JAILLET, linux-kernel
Hi,
This series converts drivers found in drivers/input/keyboard to use new
__free() and guard() cleanup facilities that simplify the code and
ensure that all resources are released appropriately.
Thanks!
Dmitry Torokhov (6):
Input: db9 - use guard notation when acquiring mutex
Input: gamecon - use guard notation when acquiring mutex
Input: iforce - use guard notation when acquiring mutex and spinlock
Input: n64joy - use guard notation when acquiring mutex
Input: turbografx - use guard notation when acquiring mutex
Input: xpad - use guard notation when acquiring mutex and spinlock
drivers/input/joystick/db9.c | 30 +++---
drivers/input/joystick/gamecon.c | 22 ++---
drivers/input/joystick/iforce/iforce-ff.c | 48 +++++----
.../input/joystick/iforce/iforce-packets.c | 57 +++++------
drivers/input/joystick/iforce/iforce-serio.c | 36 +++----
drivers/input/joystick/iforce/iforce-usb.c | 13 ++-
drivers/input/joystick/n64joy.c | 35 +++----
drivers/input/joystick/turbografx.c | 22 ++---
drivers/input/joystick/xpad.c | 99 +++++++------------
9 files changed, 152 insertions(+), 210 deletions(-)
--
Dmitry
^ permalink raw reply
* Re: [RFC PATCH] HID: wacom: Stop mangling tool IDs
From: Peter Hutterer @ 2024-09-04 0:36 UTC (permalink / raw)
To: Gerecke, Jason
Cc: linux-input, Benjamin Tissoires, Jiri Kosina, Dmitry Torokhov,
Ping Cheng, Tobita, Tatsunosuke, Erin Skomra, Joshua Dickens,
Jason Gerecke
In-Reply-To: <20240903182633.870892-1-jason.gerecke@wacom.com>
On Tue, Sep 03, 2024 at 11:26:33AM -0700, Gerecke, Jason wrote:
> From: Jason Gerecke <jason.gerecke@wacom.com>
>
> In ancient times, an off-by-one-nybble error resulted in the Wacom
> driver sending "mangled" tool IDs to userspace. This mangling behavior
> was later enshrined into a function so that devices using the then-new
> generic codepath could share the same broken behavior. The mangled IDs
> have not historically been a major problem for userspace since few
> applications care about the exact numeric value of the IDs. As long as
> the IDs were unique it didn't much matter. Some programs (cross-
> platform and remote-desktop applications in particular) /do/ rely on
> the IDs being correct, however.
>
> This patch rids the driver of the mangling behavior.
>
> Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
> References: 493630b20389 ("Input: wacom - fix serial number handling on Cintiq 21UX2")
> References: 82527da319ee ("HID: wacom: Read and internally use corrected Intuos tool IDs")
> ---
> I'd like to get the opinion of the kernel maintainers on making a
> change like this at some point in the future. There are _very_ few
> userspace uses of these IDs (primarily: drivers, compositors, and
> tablet control panels) and my plan is to update those bits and then
> give some time for the changes to roll out to users before re-
> submitting this for real. I don't expect any kind of breakage since
> we'll be taking our time with the rollout and userspace needs to
> have handling for "unknown" IDs anyway (since Wacom periodically
> releases new pens).
Just to show my support :)
Acked-by: Peter Hutterer <peter.hutterer@who-t.net>
Cheers,
Peter
^ permalink raw reply
* Re: [PATCH] platform/x86: ideapad-laptop: Stop calling i8042_command()
From: Dmitry Torokhov @ 2024-09-03 23:55 UTC (permalink / raw)
To: Maxim Mikityanskiy
Cc: Hans de Goede, Ike Panhc, Ilpo Järvinen, Andy Shevchenko,
platform-driver-x86, linux-input, Jonathan Denose, stable
In-Reply-To: <ZsV6-NRkJLJhHxiq@google.com>
On Tue, Aug 20, 2024 at 10:28:24PM -0700, Dmitry Torokhov wrote:
> On Wed, Aug 21, 2024 at 12:40:34AM +0300, Maxim Mikityanskiy wrote:
> > On Tue, 20 Aug 2024 at 13:46:53 +0300, Maxim Mikityanskiy wrote:
> > > On Sun, 18 Aug 2024 at 13:30:37 -0700, Dmitry Torokhov wrote:
> > > >
> > > > Maybe something like below can work?
> > >
> > > Great patch, thank you, I'll test it and report the results. See some
> > > minor comments below.
> > >
> > > >
> > > >
> > > > platform/x86: ideapad-laptop: do not poke keyboard controller
> > > >
> > > > From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > > >
> > > > On Ideapad Z570 the driver tries to disable and reenable data coming
> > > > from the touchpad by poking directly into 8042 keyboard controller.
> > > > This may coincide with the controller resuming and leads to spews in
> > > > dmesg and potentially other instabilities.
> > > >
> > > > Instead of using i8042_command() to control the touchpad state create a
> > > > input handler that serves as a filter and drop events coming from the
> > > > touchpad when it is supposed to be off.
> > > >
> > > > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > > > ---
> > > > drivers/platform/x86/ideapad-laptop.c | 171 ++++++++++++++++++++++++++++++++-
> > > > 1 file changed, 168 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
> > > > index fcf13d88fd6e..2f40feefd5e3 100644
> > > > --- a/drivers/platform/x86/ideapad-laptop.c
> > > > +++ b/drivers/platform/x86/ideapad-laptop.c
> > > > @@ -17,7 +17,6 @@
> > > > #include <linux/device.h>
> > > > #include <linux/dmi.h>
> > > > #include <linux/fb.h>
> > > > -#include <linux/i8042.h>
> > > > #include <linux/init.h>
> > > > #include <linux/input.h>
> > > > #include <linux/input/sparse-keymap.h>
> > > > @@ -157,6 +156,13 @@ struct ideapad_private {
> > > > struct led_classdev led;
> > > > unsigned int last_brightness;
> > > > } fn_lock;
> > > > + struct {
> > > > + bool initialized;
> > > > + bool active;
> > > > + struct input_handler handler;
> > > > + struct input_dev *tp_dev;
> > > > + spinlock_t lock;
> > > > + } tp_switch;
> > > > };
> > > >
> > > > static bool no_bt_rfkill;
> > > > @@ -1236,6 +1242,158 @@ static void ideapad_check_special_buttons(struct ideapad_private *priv)
> > > > }
> > > > }
> > > >
> > > > +struct ideapad_tpswitch_handle {
> > > > + struct input_handle handle;
> > > > + struct ideapad_private *priv;
> > > > +};
> > > > +
> > > > +#define to_tpswitch_handle(h) \
> > > > + container_of(h, struct ideapad_tpswitch_handle, handle);
> > > > +
> > > > +static int ideapad_tpswitch_connect(struct input_handler *handler,
> > > > + struct input_dev *dev,
> > > > + const struct input_device_id *id)
> > > > +{
> > > > + struct ideapad_private *priv =
> > > > + container_of(handler, struct ideapad_private, tp_switch.handler);
> > > > + struct ideapad_tpswitch_handle *h;
> > > > + int error;
> > > > +
> > > > + h = kzalloc(sizeof(*h), GFP_KERNEL);
> > > > + if (!h)
> > > > + return -ENOMEM;
> > > > +
> > > > + h->priv = priv;
> > > > + h->handle.dev = dev;
> > > > + h->handle.handler = handler;
> > > > + h->handle.name = "ideapad-tpswitch";
> > > > +
> > > > + error = input_register_handle(&h->handle);
> > > > + if (error)
> > > > + goto err_free_handle;
> > > > +
> > > > + /*
> > > > + * FIXME: ideally we do not want to open the input device here
> > > > + * if there are no other users. We need a notion of "observer"
> > > > + * handlers in the input core.
> > > > + */
> > > > + error = input_open_device(&h->handle);
> > > > + if (error)
> > > > + goto err_unregister_handle;
> > > > +
> > > > + scoped_guard(spinlock_irq, &priv->tp_switch.lock)
> > > > + priv->tp_switch.tp_dev = dev;
> > > > +
> > > > + return 0;
> > > > +
> > > > + err_unregister_handle:
> > > > + input_unregister_handle(&h->handle);
> > > > +err_free_handle:
> > > > + kfree(h);
> > > > + return error;
> > > > +}
> > > > +
> > > > +static void ideapad_tpswitch_disconnect(struct input_handle *handle)
> > > > +{
> > > > + struct ideapad_tpswitch_handle *h = to_tpswitch_handle(handle);
> > > > + struct ideapad_private *priv = h->priv;
> > > > +
> > > > + scoped_guard(spinlock_irq, &priv->tp_switch.lock)
> > >
> > > Nice syntax, I didn't know about it before.
> > >
> > > > + priv->tp_switch.tp_dev = NULL;
> > > > +
> > > > + input_close_device(handle);
> > > > + input_unregister_handle(handle);
> > > > + kfree(h);
> > > > +}
> > > > +
> > > > +static bool ideapad_tpswitch_filter(struct input_handle *handle,
> > > > + unsigned int type, unsigned int code,
> > > > + int value)
> > > > +{
> > > > + struct ideapad_tpswitch_handle *h = to_tpswitch_handle(handle);
> > > > + struct ideapad_private *priv = h->priv;
> > > > +
> > > > + if (!priv->tp_switch.active)
> > >
> > > This check seems inverted. ideapad_tpswitch_toggle assigns true when the
> > > touchpad is enabled.
> >
> > I tested the patch on Z570 (with this check inverted), and it seems to
> > work great.
> >
> > Also tested what happens on resume from suspend: the laptop reenables
> > the touchpad (the LED turns off on suspend and blinks briefly on
> > resume), and the driver handles it properly.
>
> Great, thank you! Give me a couple of days and I think I will implement
> observer/passive handler support and we can figure out how to merge
> this...
OK, so if you could try the patch below that would be great.
Don't forget to set ".passive_observer = 1" in the ideapad handler.
Thanks!
--
Dmitry
Input: introduce notion of passive observers for input handlers
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/input.c | 7 +++++++
include/linux/input.h | 5 +++++
2 files changed, 12 insertions(+)
diff --git a/drivers/input/input.c b/drivers/input/input.c
index 54c57b267b25..60a9445d78d5 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -605,6 +605,9 @@ int input_open_device(struct input_handle *handle)
handle->open++;
+ if (handle->handler->passive_observer)
+ goto out;
+
if (dev->users++ || dev->inhibited) {
/*
* Device is already opened and/or inhibited,
@@ -668,6 +671,9 @@ void input_close_device(struct input_handle *handle)
__input_release_device(handle);
+ if (handle->handler->passive_observer)
+ goto out;
+
if (!--dev->users && !dev->inhibited) {
if (dev->poller)
input_dev_poller_stop(dev->poller);
@@ -684,6 +690,7 @@ void input_close_device(struct input_handle *handle)
synchronize_rcu();
}
+out:
mutex_unlock(&dev->mutex);
}
EXPORT_SYMBOL(input_close_device);
diff --git a/include/linux/input.h b/include/linux/input.h
index 89a0be6ee0e2..6437c35f0796 100644
--- a/include/linux/input.h
+++ b/include/linux/input.h
@@ -286,6 +286,10 @@ struct input_handle;
* @start: starts handler for given handle. This function is called by
* input core right after connect() method and also when a process
* that "grabbed" a device releases it
+ * @passive_observer: set to %true by drivers only interested in observing
+ * data stream from devices if there are other users present. Such
+ * drivers will not result in starting underlying hardware device
+ * when input_open_device() is called for their handles
* @legacy_minors: set to %true by drivers using legacy minor ranges
* @minor: beginning of range of 32 legacy minors for devices this driver
* can provide
@@ -321,6 +325,7 @@ struct input_handler {
void (*disconnect)(struct input_handle *handle);
void (*start)(struct input_handle *handle);
+ bool passive_observer;
bool legacy_minors;
int minor;
const char *name;
^ permalink raw reply related
* Re: [PATCH v26 31/33] ALSA: usb-audio: Add USB offload route kcontrol
From: Wesley Cheng @ 2024-09-03 23:52 UTC (permalink / raw)
To: Pierre-Louis Bossart, srinivas.kandagatla, mathias.nyman, perex,
conor+dt, dmitry.torokhov, corbet, broonie, lgirdwood, tiwai,
krzk+dt, Thinh.Nguyen, bgoswami, robh, gregkh
Cc: linux-kernel, devicetree, linux-sound, linux-input, linux-usb,
linux-arm-msm, linux-doc, alsa-devel
In-Reply-To: <b682bd8f-2743-42cf-b51f-1444faf4635f@linux.intel.com>
Hi Pierre,
On 8/30/2024 3:05 AM, Pierre-Louis Bossart wrote:
>
> On 8/29/24 21:41, Wesley Cheng wrote:
>> In order to allow userspace/applications know about USB offloading status,
>> expose a sound kcontrol that fetches information about which sound card
>> and PCM index the USB device is mapped to for supporting offloading. In
>> the USB audio offloading framework, the ASoC BE DAI link is the entity
>> responsible for registering to the SOC USB layer.
>>
>> It is expected for the USB SND offloading driver to add the kcontrol to the
>> sound card associated with the USB audio device. An example output would
>> look like:
> this is very odd, the offloading driver adds a control to another card?
>
> That seems like a rather important layering violation.
>
> I thought it was done the other way, the USB audio card created a
> control that would point to the other card/device.
The USB SND offloading vendor driver (qc_audio_offload), which technically co-exists with USB SND core, is the entity that will add the offload kcontrol to the USB audio device's sound card. Initially, I had the kcontrol creation as part of the USB SND mixer, but I believe Takashi preferred if the vendor driver did it instead.
>
>> tinymix -D 1 get 'USB Offload Playback Route PCM#0'
>> -1, -1 (range -1->255)
>>
>> This example signifies that there is no mapped ASoC path available for the
>> USB SND device.
> but that control would not even exist if the ASoC-based driver isn't probed.
>
> It's become really hard to follow, I've been on all this for 1.5hrs and
> losing track of the design.
Hence why it probably is a good idea to leave it within the USB offload vendor driver. There are checks to ensure that the ASoC based driver is probed/available before the kcontrols are created for a device. If there are devices that were identified before the ASoC components were probed, then the snd_usb_rediscover_devices() is triggered as part of soc-usb, and that would call the connect_cb() callback to the USB SND vendor offload driver to create the kcontrol.
Thanks
Wesley Cheng
>> tinymix -D 1 get 'USB Offload Playback Route PCM#0'
>> 0, 0 (range -1->255)
>>
>> This example signifies that the offload path is available over ASoC sound
>> card index#0 and PCM device#0.
>>
>> Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com>
>> ---
>> sound/usb/Kconfig | 10 +++
>> sound/usb/Makefile | 2 +
>> sound/usb/mixer_usb_offload.c | 102 ++++++++++++++++++++++++++++++
>> sound/usb/mixer_usb_offload.h | 17 +++++
>> sound/usb/qcom/Makefile | 2 +-
>> sound/usb/qcom/qc_audio_offload.c | 2 +
>> 6 files changed, 134 insertions(+), 1 deletion(-)
>> create mode 100644 sound/usb/mixer_usb_offload.c
>> create mode 100644 sound/usb/mixer_usb_offload.h
>>
>> diff --git a/sound/usb/Kconfig b/sound/usb/Kconfig
>> index 5cc3eaf53e6b..e3fbf9541d0b 100644
>> --- a/sound/usb/Kconfig
>> +++ b/sound/usb/Kconfig
>> @@ -176,10 +176,20 @@ config SND_BCD2000
>> To compile this driver as a module, choose M here: the module
>> will be called snd-bcd2000.
>>
>> +config SND_USB_OFFLOAD_MIXER
>> + tristate "USB Audio Offload mixer control"
>> + help
>> + Say Y to enable the USB audio offloading mixer controls. This
>> + exposes an USB offload capable kcontrol to signal to applications
>> + about which platform sound card can support USB audio offload.
>> + The returning values specify the mapped ASoC card and PCM device
>> + the USB audio device is associated to.
>> +
>> config SND_USB_AUDIO_QMI
>> tristate "Qualcomm Audio Offload driver"
>> depends on QCOM_QMI_HELPERS && SND_USB_AUDIO && USB_XHCI_SIDEBAND && SND_SOC_USB
>> select SND_PCM
>> + select SND_USB_OFFLOAD_MIXER
>> help
>> Say Y here to enable the Qualcomm USB audio offloading feature.
>>
>> diff --git a/sound/usb/Makefile b/sound/usb/Makefile
>> index 54a06a2f73ca..2f19f854944c 100644
>> --- a/sound/usb/Makefile
>> +++ b/sound/usb/Makefile
>> @@ -36,3 +36,5 @@ obj-$(CONFIG_SND_USB_US122L) += snd-usbmidi-lib.o
>>
>> obj-$(CONFIG_SND) += misc/ usx2y/ caiaq/ 6fire/ hiface/ bcd2000/ qcom/
>> obj-$(CONFIG_SND_USB_LINE6) += line6/
>> +
>> +obj-$(CONFIG_SND_USB_OFFLOAD_MIXER) += mixer_usb_offload.o
>> diff --git a/sound/usb/mixer_usb_offload.c b/sound/usb/mixer_usb_offload.c
>> new file mode 100644
>> index 000000000000..16e2fd634684
>> --- /dev/null
>> +++ b/sound/usb/mixer_usb_offload.c
>> @@ -0,0 +1,102 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (c) 2022-2024 Qualcomm Innovation Center, Inc. All rights reserved.
>> + */
>> +
>> +#include <linux/usb.h>
>> +
>> +#include <sound/core.h>
>> +#include <sound/control.h>
>> +#include <sound/soc-usb.h>
>> +
>> +#include "usbaudio.h"
>> +#include "card.h"
>> +#include "helper.h"
>> +#include "mixer.h"
>> +
>> +#include "mixer_usb_offload.h"
>> +
>> +#define PCM_IDX(n) ((n) & 0xffff)
>> +#define CARD_IDX(n) ((n) >> 16)
>> +
>> +static int
>> +snd_usb_offload_route_get(struct snd_kcontrol *kcontrol,
>> + struct snd_ctl_elem_value *ucontrol)
>> +{
>> + struct device *sysdev = snd_kcontrol_chip(kcontrol);
>> + int ret;
>> +
>> + ret = snd_soc_usb_update_offload_route(sysdev,
>> + CARD_IDX(kcontrol->private_value),
>> + PCM_IDX(kcontrol->private_value),
>> + SNDRV_PCM_STREAM_PLAYBACK,
>> + ucontrol->value.integer.value);
>> + if (ret < 0) {
>> + ucontrol->value.integer.value[0] = -1;
>> + ucontrol->value.integer.value[1] = -1;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int snd_usb_offload_route_info(struct snd_kcontrol *kcontrol,
>> + struct snd_ctl_elem_info *uinfo)
>> +{
>> + uinfo->type = SNDRV_CTL_ELEM_TYPE_INTEGER;
>> + uinfo->count = 2;
>> + uinfo->value.integer.min = -1;
>> + /* Arbitrary max value, as there is no 'limit' on number of PCM devices */
>> + uinfo->value.integer.max = 0xff;
>> +
>> + return 0;
>> +}
>> +
>> +static struct snd_kcontrol_new snd_usb_offload_mapped_ctl = {
>> + .iface = SNDRV_CTL_ELEM_IFACE_CARD,
>> + .access = SNDRV_CTL_ELEM_ACCESS_READ,
>> + .info = snd_usb_offload_route_info,
>> + .get = snd_usb_offload_route_get,
>> +};
>> +
>> +/**
>> + * snd_usb_offload_create_ctl() - Add USB offload bounded mixer
>> + * @chip - USB SND chip device
>> + *
>> + * Creates a sound control for a USB audio device, so that applications can
>> + * query for if there is an available USB audio offload path, and which
>> + * card is managing it.
>> + */
>> +int snd_usb_offload_create_ctl(struct snd_usb_audio *chip)
>> +{
>> + struct usb_device *udev = chip->dev;
>> + struct snd_kcontrol_new *chip_kctl;
>> + struct snd_usb_substream *subs;
>> + struct snd_usb_stream *as;
>> + char ctl_name[37];
>> + int ret;
>> +
>> + list_for_each_entry(as, &chip->pcm_list, list) {
>> + subs = &as->substream[SNDRV_PCM_STREAM_PLAYBACK];
>> + if (!subs->ep_num)
>> + continue;
>> +
>> + chip_kctl = &snd_usb_offload_mapped_ctl;
>> + chip_kctl->count = 1;
>> + /*
>> + * Store the associated USB SND card number and PCM index for
>> + * the kctl.
>> + */
>> + chip_kctl->private_value = as->pcm_index |
>> + chip->card->number << 16;
>> + sprintf(ctl_name, "USB Offload Playback Route PCM#%d",
>> + as->pcm_index);
>> + chip_kctl->name = ctl_name;
>> + ret = snd_ctl_add(chip->card, snd_ctl_new1(chip_kctl,
>> + udev->bus->sysdev));
>> + if (ret < 0)
>> + break;
>> + }
>> +
>> + return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(snd_usb_offload_create_ctl);
>> diff --git a/sound/usb/mixer_usb_offload.h b/sound/usb/mixer_usb_offload.h
>> new file mode 100644
>> index 000000000000..3f6e2a8b19c8
>> --- /dev/null
>> +++ b/sound/usb/mixer_usb_offload.h
>> @@ -0,0 +1,17 @@
>> +/* SPDX-License-Identifier: GPL-2.0
>> + *
>> + * Copyright (c) 2022-2024 Qualcomm Innovation Center, Inc. All rights reserved.
>> + */
>> +
>> +#ifndef __USB_OFFLOAD_MIXER_H
>> +#define __USB_OFFLOAD_MIXER_H
>> +
>> +#if IS_ENABLED(CONFIG_SND_USB_OFFLOAD_MIXER)
>> +int snd_usb_offload_create_ctl(struct snd_usb_audio *chip);
>> +#else
>> +static inline int snd_usb_offload_create_ctl(struct snd_usb_audio *chip)
>> +{
>> + return 0;
>> +}
>> +#endif
>> +#endif /* __USB_OFFLOAD_MIXER_H */
>> diff --git a/sound/usb/qcom/Makefile b/sound/usb/qcom/Makefile
>> index a81c9b28d484..4005e8391ab9 100644
>> --- a/sound/usb/qcom/Makefile
>> +++ b/sound/usb/qcom/Makefile
>> @@ -1,2 +1,2 @@
>> snd-usb-audio-qmi-objs := usb_audio_qmi_v01.o qc_audio_offload.o
>> -obj-$(CONFIG_SND_USB_AUDIO_QMI) += snd-usb-audio-qmi.o
>> \ No newline at end of file
>> +obj-$(CONFIG_SND_USB_AUDIO_QMI) += snd-usb-audio-qmi.o
>> diff --git a/sound/usb/qcom/qc_audio_offload.c b/sound/usb/qcom/qc_audio_offload.c
>> index a7ad15404fd1..5b9262a116be 100644
>> --- a/sound/usb/qcom/qc_audio_offload.c
>> +++ b/sound/usb/qcom/qc_audio_offload.c
>> @@ -36,6 +36,7 @@
>> #include "../helper.h"
>> #include "../pcm.h"
>> #include "../power.h"
>> +#include "../mixer_usb_offload.h"
>>
>> #include "usb_audio_qmi_v01.h"
>>
>> @@ -1703,6 +1704,7 @@ static void qc_usb_audio_offload_probe(struct snd_usb_audio *chip)
>> sdev->card_idx = chip->card->number;
>> sdev->chip_idx = chip->index;
>>
>> + snd_usb_offload_create_ctl(chip);
>> snd_soc_usb_connect(usb_get_usb_backend(udev), sdev);
>> }
>>
^ permalink raw reply
* Re: [PATCH v26 29/33] ALSA: usb-audio: qcom: Don't allow USB offload path if PCM device is in use
From: Wesley Cheng @ 2024-09-03 23:43 UTC (permalink / raw)
To: Pierre-Louis Bossart, srinivas.kandagatla, mathias.nyman, perex,
conor+dt, dmitry.torokhov, corbet, broonie, lgirdwood, tiwai,
krzk+dt, Thinh.Nguyen, bgoswami, robh, gregkh
Cc: linux-kernel, devicetree, linux-sound, linux-input, linux-usb,
linux-arm-msm, linux-doc, alsa-devel
In-Reply-To: <6e4e6e5f-dc55-4311-a658-5e2fcbeefab1@linux.intel.com>
Hi Pierre,
On 8/30/2024 2:55 AM, Pierre-Louis Bossart wrote:
>
> On 8/29/24 21:41, Wesley Cheng wrote:
>> Add proper checks and updates to the USB substream once receiving a USB QMI
>> stream enable request. If the substream is already in use from the non
>> offload path, reject the stream enable request. In addition, update the
>> USB substream opened parameter when enabling the offload path, so the
>> non offload path can be blocked.
>>
>> Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com>
>> ---
>> sound/usb/qcom/qc_audio_offload.c | 15 ++++++++++++++-
>> 1 file changed, 14 insertions(+), 1 deletion(-)
>>
>> diff --git a/sound/usb/qcom/qc_audio_offload.c b/sound/usb/qcom/qc_audio_offload.c
>> index e9f2fd6bbb41..0bd533f539e4 100644
>> --- a/sound/usb/qcom/qc_audio_offload.c
>> +++ b/sound/usb/qcom/qc_audio_offload.c
>> @@ -1482,12 +1482,17 @@ static void handle_uaudio_stream_req(struct qmi_handle *handle,
>> goto response;
>> }
>>
>> + mutex_lock(&chip->mutex);
>> if (req_msg->enable) {
>> - if (info_idx < 0 || chip->system_suspend) {
>> + if (info_idx < 0 || chip->system_suspend || subs->opened) {
>> ret = -EBUSY;
>> + mutex_unlock(&chip->mutex);
>> +
>> goto response;
>> }
>> + subs->opened = 1;
>> }
>> + mutex_unlock(&chip->mutex);
>>
>> if (req_msg->service_interval_valid) {
>> ret = get_data_interval_from_si(subs,
>> @@ -1509,6 +1514,11 @@ static void handle_uaudio_stream_req(struct qmi_handle *handle,
>> if (!ret)
>> ret = prepare_qmi_response(subs, req_msg, &resp,
>> info_idx);
>> + if (ret < 0) {
>> + mutex_lock(&chip->mutex);
>> + subs->opened = 0;
>> + mutex_unlock(&chip->mutex);
>> + }
>> } else {
>> info = &uadev[pcm_card_num].info[info_idx];
>> if (info->data_ep_pipe) {
>> @@ -1532,6 +1542,9 @@ static void handle_uaudio_stream_req(struct qmi_handle *handle,
>> }
>>
>> disable_audio_stream(subs);
>> + mutex_lock(&chip->mutex);
>> + subs->opened = 0;
>> + mutex_unlock(&chip->mutex);
>> }
>>
>
> This sounds ok but I wonder why all this needs to be done in
> Qualcomm-specific layers instead of soc-usb?
>
This is to prevent conflicts within the non-offload/legacy USB SND path and the USB SND offload vendor driver. Don't think we need to involve anything with ASoC in these checks.
Thanks
Wesley Cheng
^ permalink raw reply
* Re: [PATCH v26 28/33] ALSA: usb-audio: qcom: Introduce QC USB SND offloading support
From: Wesley Cheng @ 2024-09-03 23:41 UTC (permalink / raw)
To: Pierre-Louis Bossart, srinivas.kandagatla, mathias.nyman, perex,
conor+dt, dmitry.torokhov, corbet, broonie, lgirdwood, tiwai,
krzk+dt, Thinh.Nguyen, bgoswami, robh, gregkh
Cc: linux-kernel, devicetree, linux-sound, linux-input, linux-usb,
linux-arm-msm, linux-doc, alsa-devel
In-Reply-To: <e7955dd7-95b1-4999-a2a1-519e8d7297a6@linux.intel.com>
Hi Pierre,
On 8/30/2024 2:52 AM, Pierre-Louis Bossart wrote:
>> +/* Stream disable request timeout during USB device disconnect */
>> +#define DEV_RELEASE_WAIT_TIMEOUT 10000 /* in ms */
> 10s really? That seems rather large for a stream disable timeout...
Hmm, yes that is overkill, will adjust it accordingly.
>> +static struct snd_usb_platform_ops offload_ops = {
>> + .connect_cb = qc_usb_audio_offload_probe,
>> + .disconnect_cb = qc_usb_audio_offload_disconnect,
>> + .suspend_cb = qc_usb_audio_offload_suspend,
>> +};
> You probably want to explain why there's no .resume_cb?
>
> The comments mention also that the suspend_cb has to stop playback, but
> then who resumes playback :-)
>
I can add a comment. Ideally, the suspend_cb is only used for the case of PM suspend/system suspend. If usb autosuspend is enabled, then the QC offload driver will handle the voting based on the audio stream being active or not. Is there a use case where the ASoC layer re-opens any previously active audio streams so that userspace doesn't have to? Currently, I was under the assumption that the audio stream would have to be re-opened by the application.
Thanks
Wesley Cheng
^ permalink raw reply
* Re: [PATCH v26 26/33] ALSA: usb-audio: Prevent starting of audio stream if in use
From: Wesley Cheng @ 2024-09-03 23:21 UTC (permalink / raw)
To: Pierre-Louis Bossart, srinivas.kandagatla, mathias.nyman, perex,
conor+dt, dmitry.torokhov, corbet, broonie, lgirdwood, tiwai,
krzk+dt, Thinh.Nguyen, bgoswami, robh, gregkh
Cc: linux-kernel, devicetree, linux-sound, linux-input, linux-usb,
linux-arm-msm, linux-doc, alsa-devel
In-Reply-To: <e8b11cb4-cda1-4904-b83f-e124066758e3@linux.intel.com>
Hi Pierre,
On 8/30/2024 2:45 AM, Pierre-Louis Bossart wrote:
>
> On 8/29/24 21:40, Wesley Cheng wrote:
>> With USB audio offloading, an audio session is started from the ASoC
>> platform sound card and PCM devices. Likewise, the USB SND path is still
>> readily available for use, in case the non-offload path is desired. In
>> order to prevent the two entities from attempting to use the USB bus,
>> introduce a flag that determines when either paths are in use.
>>
>> If a PCM device is already in use, the check will return an error to
>> userspace notifying that the stream is currently busy. This ensures that
>> only one path is using the USB substream.
>>
>> Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com>
> Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
>
> I would also move this patch earlier in the series since it has no
> dependency on USB offload really, and if somehow it breaks USB audio
> regular paths we'd want to know early for bisection.
Sure I'll re-order this earlier since I'm going to send out another rev.
Thanks
Wesley Cheng
>
>> ---
>> sound/usb/card.h | 1 +
>> sound/usb/pcm.c | 29 ++++++++++++++++++++++++++---
>> 2 files changed, 27 insertions(+), 3 deletions(-)
>>
>> diff --git a/sound/usb/card.h b/sound/usb/card.h
>> index 15cda1730076..d8b8522e1613 100644
>> --- a/sound/usb/card.h
>> +++ b/sound/usb/card.h
>> @@ -165,6 +165,7 @@ struct snd_usb_substream {
>> unsigned int pkt_offset_adj; /* Bytes to drop from beginning of packets (for non-compliant devices) */
>> unsigned int stream_offset_adj; /* Bytes to drop from beginning of stream (for non-compliant devices) */
>>
>> + unsigned int opened:1; /* pcm device opened */
>> unsigned int running: 1; /* running status */
>> unsigned int period_elapsed_pending; /* delay period handling */
>>
>> diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c
>> index 18467da6fd9e..b24ee38fad72 100644
>> --- a/sound/usb/pcm.c
>> +++ b/sound/usb/pcm.c
>> @@ -1241,8 +1241,17 @@ static int snd_usb_pcm_open(struct snd_pcm_substream *substream)
>> struct snd_usb_stream *as = snd_pcm_substream_chip(substream);
>> struct snd_pcm_runtime *runtime = substream->runtime;
>> struct snd_usb_substream *subs = &as->substream[direction];
>> + struct snd_usb_audio *chip = subs->stream->chip;
>> int ret;
>>
>> + mutex_lock(&chip->mutex);
>> + if (subs->opened) {
>> + mutex_unlock(&chip->mutex);
>> + return -EBUSY;
>> + }
>> + subs->opened = 1;
>> + mutex_unlock(&chip->mutex);
>> +
>> runtime->hw = snd_usb_hardware;
>> /* need an explicit sync to catch applptr update in low-latency mode */
>> if (direction == SNDRV_PCM_STREAM_PLAYBACK &&
>> @@ -1259,13 +1268,23 @@ static int snd_usb_pcm_open(struct snd_pcm_substream *substream)
>>
>> ret = setup_hw_info(runtime, subs);
>> if (ret < 0)
>> - return ret;
>> + goto err_open;
>> ret = snd_usb_autoresume(subs->stream->chip);
>> if (ret < 0)
>> - return ret;
>> + goto err_open;
>> ret = snd_media_stream_init(subs, as->pcm, direction);
>> if (ret < 0)
>> - snd_usb_autosuspend(subs->stream->chip);
>> + goto err_resume;
>> +
>> + return 0;
>> +
>> +err_resume:
>> + snd_usb_autosuspend(subs->stream->chip);
>> +err_open:
>> + mutex_lock(&chip->mutex);
>> + subs->opened = 0;
>> + mutex_unlock(&chip->mutex);
>> +
>> return ret;
>> }
>>
>> @@ -1274,6 +1293,7 @@ static int snd_usb_pcm_close(struct snd_pcm_substream *substream)
>> int direction = substream->stream;
>> struct snd_usb_stream *as = snd_pcm_substream_chip(substream);
>> struct snd_usb_substream *subs = &as->substream[direction];
>> + struct snd_usb_audio *chip = subs->stream->chip;
>> int ret;
>>
>> snd_media_stop_pipeline(subs);
>> @@ -1287,6 +1307,9 @@ static int snd_usb_pcm_close(struct snd_pcm_substream *substream)
>>
>> subs->pcm_substream = NULL;
>> snd_usb_autosuspend(subs->stream->chip);
>> + mutex_lock(&chip->mutex);
>> + subs->opened = 0;
>> + mutex_unlock(&chip->mutex);
>>
>> return 0;
>> }
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox