* [PATCH] platform/x86: Switch to guard(mutex)
@ 2025-12-15 3:29 chen zhang
2025-12-23 9:44 ` Ilpo Järvinen
0 siblings, 1 reply; 3+ messages in thread
From: chen zhang @ 2025-12-15 3:29 UTC (permalink / raw)
To: hdegoede, hansg, ilpo.jarvinen, frank
Cc: platform-driver-x86, linux-kernel, chenzhang_0901, chen zhang
Instead of using the 'goto label; mutex_unlock()' pattern use
'guard(mutex)' which will release the mutex when it goes out of scope.
Signed-off-by: chen zhang <chenzhang@kylinos.cn>
---
drivers/platform/x86/hdaps.c | 37 +++++++++++++++---------------------
1 file changed, 15 insertions(+), 22 deletions(-)
diff --git a/drivers/platform/x86/hdaps.c b/drivers/platform/x86/hdaps.c
index f11f726d2062..2a11bcaa52f2 100644
--- a/drivers/platform/x86/hdaps.c
+++ b/drivers/platform/x86/hdaps.c
@@ -147,18 +147,16 @@ static int hdaps_readb_one(unsigned int port, u8 *val)
{
int ret;
- mutex_lock(&hdaps_mtx);
+ guard(mutex)(&hdaps_mtx);
/* do a sync refresh -- we need to be sure that we read fresh data */
ret = __device_refresh_sync();
if (ret)
- goto out;
+ return ret;
*val = inb(port);
__device_complete();
-out:
- mutex_unlock(&hdaps_mtx);
return ret;
}
@@ -208,12 +206,12 @@ static int hdaps_device_init(void)
{
int total, ret = -ENXIO;
- mutex_lock(&hdaps_mtx);
+ guard(mutex)(&hdaps_mtx);
outb(0x13, 0x1610);
outb(0x01, 0x161f);
if (__wait_latch(0x161f, 0x00))
- goto out;
+ return ret;
/*
* Most ThinkPads return 0x01.
@@ -226,7 +224,7 @@ static int hdaps_device_init(void)
if (__check_latch(0x1611, 0x03) &&
__check_latch(0x1611, 0x02) &&
__check_latch(0x1611, 0x01))
- goto out;
+ return ret;
printk(KERN_DEBUG "hdaps: initial latch check good (0x%02x)\n",
__get_latch(0x1611));
@@ -235,29 +233,29 @@ static int hdaps_device_init(void)
outb(0x81, 0x1611);
outb(0x01, 0x161f);
if (__wait_latch(0x161f, 0x00))
- goto out;
+ return ret;
if (__wait_latch(0x1611, 0x00))
- goto out;
+ return ret;
if (__wait_latch(0x1612, 0x60))
- goto out;
+ return ret;
if (__wait_latch(0x1613, 0x00))
- goto out;
+ return ret;
outb(0x14, 0x1610);
outb(0x01, 0x1611);
outb(0x01, 0x161f);
if (__wait_latch(0x161f, 0x00))
- goto out;
+ return ret;
outb(0x10, 0x1610);
outb(0xc8, 0x1611);
outb(0x00, 0x1612);
outb(0x02, 0x1613);
outb(0x01, 0x161f);
if (__wait_latch(0x161f, 0x00))
- goto out;
+ return ret;
if (__device_refresh_sync())
- goto out;
+ return ret;
if (__wait_latch(0x1611, 0x00))
- goto out;
+ return ret;
/* we have done our dance, now let's wait for the applause */
for (total = INIT_TIMEOUT_MSECS; total > 0; total -= INIT_WAIT_MSECS) {
@@ -273,8 +271,6 @@ static int hdaps_device_init(void)
msleep(INIT_WAIT_MSECS);
}
-out:
- mutex_unlock(&hdaps_mtx);
return ret;
}
@@ -322,17 +318,14 @@ static void hdaps_mousedev_poll(struct input_dev *input_dev)
{
int x, y;
- mutex_lock(&hdaps_mtx);
+ guard(mutex)(&hdaps_mtx);
if (__hdaps_read_pair(HDAPS_PORT_XPOS, HDAPS_PORT_YPOS, &x, &y))
- goto out;
+ return;
input_report_abs(input_dev, ABS_X, x - rest_x);
input_report_abs(input_dev, ABS_Y, y - rest_y);
input_sync(input_dev);
-
-out:
- mutex_unlock(&hdaps_mtx);
}
--
2.25.1
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] platform/x86: Switch to guard(mutex)
2025-12-15 3:29 [PATCH] platform/x86: Switch to guard(mutex) chen zhang
@ 2025-12-23 9:44 ` Ilpo Järvinen
0 siblings, 0 replies; 3+ messages in thread
From: Ilpo Järvinen @ 2025-12-23 9:44 UTC (permalink / raw)
To: chen zhang
Cc: hdegoede, Hans de Goede, frank, platform-driver-x86, LKML,
chenzhang_0901
On Mon, 15 Dec 2025, chen zhang wrote:
You're using too generic prefix from the shortlog (in the Subject).
> Instead of using the 'goto label; mutex_unlock()' pattern use
> 'guard(mutex)' which will release the mutex when it goes out of scope.
>
> Signed-off-by: chen zhang <chenzhang@kylinos.cn>
> ---
> drivers/platform/x86/hdaps.c | 37 +++++++++++++++---------------------
> 1 file changed, 15 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/platform/x86/hdaps.c b/drivers/platform/x86/hdaps.c
> index f11f726d2062..2a11bcaa52f2 100644
> --- a/drivers/platform/x86/hdaps.c
> +++ b/drivers/platform/x86/hdaps.c
> @@ -147,18 +147,16 @@ static int hdaps_readb_one(unsigned int port, u8 *val)
> {
> int ret;
>
> - mutex_lock(&hdaps_mtx);
> + guard(mutex)(&hdaps_mtx);
>
> /* do a sync refresh -- we need to be sure that we read fresh data */
> ret = __device_refresh_sync();
> if (ret)
> - goto out;
> + return ret;
>
> *val = inb(port);
> __device_complete();
>
> -out:
> - mutex_unlock(&hdaps_mtx);
> return ret;
return 0;
> }
>
> @@ -208,12 +206,12 @@ static int hdaps_device_init(void)
> {
> int total, ret = -ENXIO;
>
> - mutex_lock(&hdaps_mtx);
> + guard(mutex)(&hdaps_mtx);
>
> outb(0x13, 0x1610);
> outb(0x01, 0x161f);
> if (__wait_latch(0x161f, 0x00))
> - goto out;
> + return ret;
This is not an acceptable conversion. You clearly no longer need ret
variable to carry the information here but can just write:
return -ENXIO;
Please do realize if you make this kind of conversion changes to old code
which are on borderline whether they're more noise and burden (to
maintainers and reviewers) than useful, you should know all these
conventions so that no review cycles is wasted on trivial style issues
like this.
I know some subsystems do push back on patches like this. I find them
still somewhat useful but you should think the patch through before
sending, which you clearly didn't here. Cleaning up/refactoring is not
"easy" unless you do it the wrong way which is to offload thinking to the
reviewers/maintainers.
> /*
> * Most ThinkPads return 0x01.
> @@ -226,7 +224,7 @@ static int hdaps_device_init(void)
> if (__check_latch(0x1611, 0x03) &&
> __check_latch(0x1611, 0x02) &&
> __check_latch(0x1611, 0x01))
> - goto out;
> + return ret;
>
> printk(KERN_DEBUG "hdaps: initial latch check good (0x%02x)\n",
> __get_latch(0x1611));
> @@ -235,29 +233,29 @@ static int hdaps_device_init(void)
> outb(0x81, 0x1611);
> outb(0x01, 0x161f);
> if (__wait_latch(0x161f, 0x00))
> - goto out;
> + return ret;
> if (__wait_latch(0x1611, 0x00))
> - goto out;
> + return ret;
> if (__wait_latch(0x1612, 0x60))
> - goto out;
> + return ret;
> if (__wait_latch(0x1613, 0x00))
> - goto out;
> + return ret;
> outb(0x14, 0x1610);
> outb(0x01, 0x1611);
> outb(0x01, 0x161f);
> if (__wait_latch(0x161f, 0x00))
> - goto out;
> + return ret;
> outb(0x10, 0x1610);
> outb(0xc8, 0x1611);
> outb(0x00, 0x1612);
> outb(0x02, 0x1613);
> outb(0x01, 0x161f);
> if (__wait_latch(0x161f, 0x00))
> - goto out;
> + return ret;
> if (__device_refresh_sync())
> - goto out;
> + return ret;
> if (__wait_latch(0x1611, 0x00))
> - goto out;
> + return ret;
>
> /* we have done our dance, now let's wait for the applause */
> for (total = INIT_TIMEOUT_MSECS; total > 0; total -= INIT_WAIT_MSECS) {
> @@ -273,8 +271,6 @@ static int hdaps_device_init(void)
> msleep(INIT_WAIT_MSECS);
> }
>
> -out:
> - mutex_unlock(&hdaps_mtx);
> return ret;
> }
>
> @@ -322,17 +318,14 @@ static void hdaps_mousedev_poll(struct input_dev *input_dev)
> {
> int x, y;
>
> - mutex_lock(&hdaps_mtx);
> + guard(mutex)(&hdaps_mtx);
>
> if (__hdaps_read_pair(HDAPS_PORT_XPOS, HDAPS_PORT_YPOS, &x, &y))
> - goto out;
> + return;
>
> input_report_abs(input_dev, ABS_X, x - rest_x);
> input_report_abs(input_dev, ABS_Y, y - rest_y);
> input_sync(input_dev);
> -
> -out:
> - mutex_unlock(&hdaps_mtx);
> }
>
>
>
--
i.
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH] platform/x86: Switch to guard(mutex)
@ 2025-12-15 3:27 chen zhang
0 siblings, 0 replies; 3+ messages in thread
From: chen zhang @ 2025-12-15 3:27 UTC (permalink / raw)
To: hdegoede, ilpo.jarvinen, frank
Cc: platform-driver-x86, linux-kernel, chenzhang_0901, chen zhang
Instead of using the 'goto label; mutex_unlock()' pattern use
'guard(mutex)' which will release the mutex when it goes out of scope.
Signed-off-by: chen zhang <chenzhang@kylinos.cn>
---
drivers/platform/x86/hdaps.c | 37 +++++++++++++++---------------------
1 file changed, 15 insertions(+), 22 deletions(-)
diff --git a/drivers/platform/x86/hdaps.c b/drivers/platform/x86/hdaps.c
index f11f726d2062..2a11bcaa52f2 100644
--- a/drivers/platform/x86/hdaps.c
+++ b/drivers/platform/x86/hdaps.c
@@ -147,18 +147,16 @@ static int hdaps_readb_one(unsigned int port, u8 *val)
{
int ret;
- mutex_lock(&hdaps_mtx);
+ guard(mutex)(&hdaps_mtx);
/* do a sync refresh -- we need to be sure that we read fresh data */
ret = __device_refresh_sync();
if (ret)
- goto out;
+ return ret;
*val = inb(port);
__device_complete();
-out:
- mutex_unlock(&hdaps_mtx);
return ret;
}
@@ -208,12 +206,12 @@ static int hdaps_device_init(void)
{
int total, ret = -ENXIO;
- mutex_lock(&hdaps_mtx);
+ guard(mutex)(&hdaps_mtx);
outb(0x13, 0x1610);
outb(0x01, 0x161f);
if (__wait_latch(0x161f, 0x00))
- goto out;
+ return ret;
/*
* Most ThinkPads return 0x01.
@@ -226,7 +224,7 @@ static int hdaps_device_init(void)
if (__check_latch(0x1611, 0x03) &&
__check_latch(0x1611, 0x02) &&
__check_latch(0x1611, 0x01))
- goto out;
+ return ret;
printk(KERN_DEBUG "hdaps: initial latch check good (0x%02x)\n",
__get_latch(0x1611));
@@ -235,29 +233,29 @@ static int hdaps_device_init(void)
outb(0x81, 0x1611);
outb(0x01, 0x161f);
if (__wait_latch(0x161f, 0x00))
- goto out;
+ return ret;
if (__wait_latch(0x1611, 0x00))
- goto out;
+ return ret;
if (__wait_latch(0x1612, 0x60))
- goto out;
+ return ret;
if (__wait_latch(0x1613, 0x00))
- goto out;
+ return ret;
outb(0x14, 0x1610);
outb(0x01, 0x1611);
outb(0x01, 0x161f);
if (__wait_latch(0x161f, 0x00))
- goto out;
+ return ret;
outb(0x10, 0x1610);
outb(0xc8, 0x1611);
outb(0x00, 0x1612);
outb(0x02, 0x1613);
outb(0x01, 0x161f);
if (__wait_latch(0x161f, 0x00))
- goto out;
+ return ret;
if (__device_refresh_sync())
- goto out;
+ return ret;
if (__wait_latch(0x1611, 0x00))
- goto out;
+ return ret;
/* we have done our dance, now let's wait for the applause */
for (total = INIT_TIMEOUT_MSECS; total > 0; total -= INIT_WAIT_MSECS) {
@@ -273,8 +271,6 @@ static int hdaps_device_init(void)
msleep(INIT_WAIT_MSECS);
}
-out:
- mutex_unlock(&hdaps_mtx);
return ret;
}
@@ -322,17 +318,14 @@ static void hdaps_mousedev_poll(struct input_dev *input_dev)
{
int x, y;
- mutex_lock(&hdaps_mtx);
+ guard(mutex)(&hdaps_mtx);
if (__hdaps_read_pair(HDAPS_PORT_XPOS, HDAPS_PORT_YPOS, &x, &y))
- goto out;
+ return;
input_report_abs(input_dev, ABS_X, x - rest_x);
input_report_abs(input_dev, ABS_Y, y - rest_y);
input_sync(input_dev);
-
-out:
- mutex_unlock(&hdaps_mtx);
}
--
2.25.1
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-12-23 9:44 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-12-15 3:29 [PATCH] platform/x86: Switch to guard(mutex) chen zhang
2025-12-23 9:44 ` Ilpo Järvinen
-- strict thread matches above, loose matches on Subject: below --
2025-12-15 3:27 chen zhang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®