2024-03-27 21:53:29

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v1 1/1] platform/x86: quickstart: Miscellaneous improvements

There is a mix of a few improvements to the driver.
I have done this instead of review, so it can quickly be
folded into the original code (partially or fully).

Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/platform/x86/quickstart.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/platform/x86/quickstart.c b/drivers/platform/x86/quickstart.c
index ba3a7a25dda70..f686942662ccc 100644
--- a/drivers/platform/x86/quickstart.c
+++ b/drivers/platform/x86/quickstart.c
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: GPL-2.0-or-later
/*
- * quickstart.c - ACPI Direct App Launch driver
+ * ACPI Direct App Launch driver
*
* Copyright (C) 2024 Armin Wolf <[email protected]>
* Copyright (C) 2022 Arvid Norlander <[email protected]>
@@ -10,15 +10,18 @@
* <https://archive.org/details/microsoft-acpi-dirapplaunch>
*/

-#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
-
#include <linux/acpi.h>
+#include <linux/device.h>
+#include <linux/errno.h>
#include <linux/init.h>
#include <linux/input.h>
#include <linux/input/sparse-keymap.h>
-#include <linux/kernel.h>
+#include <linux/mod_devicetable.h>
#include <linux/module.h>
#include <linux/platform_device.h>
+#include <linux/pm_wakeup.h>
+#include <linux/printk.h>
+#include <linux/slab.h>
#include <linux/sysfs.h>
#include <linux/types.h>

@@ -165,7 +168,8 @@ static int quickstart_probe(struct platform_device *pdev)
data->dev = &pdev->dev;
dev_set_drvdata(&pdev->dev, data);

- /* We have to initialize the device wakeup before evaluating GHID because
+ /*
+ * We have to initialize the device wakeup before evaluating GHID because
* doing so will notify the device if the button was used to wake the machine
* from S5.
*/
@@ -202,7 +206,7 @@ static int quickstart_probe(struct platform_device *pdev)
}

static const struct acpi_device_id quickstart_device_ids[] = {
- { "PNP0C32", 0 },
+ { "PNP0C32" },
{ }
};
MODULE_DEVICE_TABLE(acpi, quickstart_device_ids);
--
2.44.0



2024-03-27 23:35:26

by Armin Wolf

[permalink] [raw]
Subject: Re: [PATCH v1 1/1] platform/x86: quickstart: Miscellaneous improvements

Am 27.03.24 um 22:52 schrieb Andy Shevchenko:

> There is a mix of a few improvements to the driver.
> I have done this instead of review, so it can quickly be
> folded into the original code (partially or fully).
>
> Signed-off-by: Andy Shevchenko <[email protected]>
> ---
> drivers/platform/x86/quickstart.c | 16 ++++++++++------
> 1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/platform/x86/quickstart.c b/drivers/platform/x86/quickstart.c
> index ba3a7a25dda70..f686942662ccc 100644
> --- a/drivers/platform/x86/quickstart.c
> +++ b/drivers/platform/x86/quickstart.c
> @@ -1,6 +1,6 @@
> // SPDX-License-Identifier: GPL-2.0-or-later
> /*
> - * quickstart.c - ACPI Direct App Launch driver
> + * ACPI Direct App Launch driver
> *
> * Copyright (C) 2024 Armin Wolf <[email protected]>
> * Copyright (C) 2022 Arvid Norlander <[email protected]>
> @@ -10,15 +10,18 @@
> * <https://archive.org/details/microsoft-acpi-dirapplaunch>
> */
>
> -#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> -

Hi,

that is the reason for removing this?

> #include <linux/acpi.h>
> +#include <linux/device.h>
> +#include <linux/errno.h>
> #include <linux/init.h>
> #include <linux/input.h>
> #include <linux/input/sparse-keymap.h>
> -#include <linux/kernel.h>
> +#include <linux/mod_devicetable.h>
> #include <linux/module.h>
> #include <linux/platform_device.h>
> +#include <linux/pm_wakeup.h>
> +#include <linux/printk.h>
> +#include <linux/slab.h>
> #include <linux/sysfs.h>
> #include <linux/types.h>
>
> @@ -165,7 +168,8 @@ static int quickstart_probe(struct platform_device *pdev)
> data->dev = &pdev->dev;
> dev_set_drvdata(&pdev->dev, data);
>
> - /* We have to initialize the device wakeup before evaluating GHID because
> + /*
> + * We have to initialize the device wakeup before evaluating GHID because
> * doing so will notify the device if the button was used to wake the machine
> * from S5.
> */
> @@ -202,7 +206,7 @@ static int quickstart_probe(struct platform_device *pdev)
> }
>
> static const struct acpi_device_id quickstart_device_ids[] = {
> - { "PNP0C32", 0 },
> + { "PNP0C32" },
> { }
> };
> MODULE_DEVICE_TABLE(acpi, quickstart_device_ids);

2024-03-28 08:51:10

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v1 1/1] platform/x86: quickstart: Miscellaneous improvements

On Thu, Mar 28, 2024 at 1:35 AM Armin Wolf <[email protected]> wrote:
> Am 27.03.24 um 22:52 schrieb Andy Shevchenko:

..

> > -#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt

> that is the reason for removing this?

Yes, it's a dead code. Likely the cargo cult line was copied from
another driver.

--
With Best Regards,
Andy Shevchenko

2024-03-29 00:24:36

by Armin Wolf

[permalink] [raw]
Subject: Re: [PATCH v1 1/1] platform/x86: quickstart: Miscellaneous improvements

Am 28.03.24 um 09:50 schrieb Andy Shevchenko:

> On Thu, Mar 28, 2024 at 1:35 AM Armin Wolf <[email protected]> wrote:
>> Am 27.03.24 um 22:52 schrieb Andy Shevchenko:
> ...
>
>>> -#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>> that is the reason for removing this?
> Yes, it's a dead code. Likely the cargo cult line was copied from
> another driver.
>
In that case: Reviewed-by: Armin Wolf <[email protected]>


2024-03-29 17:43:59

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v1 1/1] platform/x86: quickstart: Miscellaneous improvements

Fri, Mar 29, 2024 at 01:23:25AM +0100, Armin Wolf kirjoitti:
> Am 28.03.24 um 09:50 schrieb Andy Shevchenko:
> > On Thu, Mar 28, 2024 at 1:35 AM Armin Wolf <[email protected]> wrote:
> > > Am 27.03.24 um 22:52 schrieb Andy Shevchenko:

..

> > > > -#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> > > that is the reason for removing this?
> > Yes, it's a dead code. Likely the cargo cult line was copied from
> > another driver.
> >
> In that case: Reviewed-by: Armin Wolf <[email protected]>

Thank you!

You need to use a whole line for the tag.

P.S.
It seems Hans hadn't folded anything from here, so I will send a patch
(series?) next week.


--
With Best Regards,
Andy Shevchenko



2024-04-08 09:16:22

by Hans de Goede

[permalink] [raw]
Subject: Re: [PATCH v1 1/1] platform/x86: quickstart: Miscellaneous improvements

Hi,

On 3/29/24 6:43 PM, Andy Shevchenko wrote:
> Fri, Mar 29, 2024 at 01:23:25AM +0100, Armin Wolf kirjoitti:
>> Am 28.03.24 um 09:50 schrieb Andy Shevchenko:
>>> On Thu, Mar 28, 2024 at 1:35 AM Armin Wolf <[email protected]> wrote:
>>>> Am 27.03.24 um 22:52 schrieb Andy Shevchenko:
>
> ...
>
>>>>> -#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>>>> that is the reason for removing this?
>>> Yes, it's a dead code. Likely the cargo cult line was copied from
>>> another driver.
>>>
>> In that case: Reviewed-by: Armin Wolf <[email protected]>
>
> Thank you!
>
> You need to use a whole line for the tag.
>
> P.S.
> It seems Hans hadn't folded anything from here, so I will send a patch
> (series?) next week.

Sorry about being a bit slow with merging this. I usually process
the pdx86 patch-queue on Monday and last Monday was a bank holiday.

I'm merging this (and the other quickstart driver fixes) today:

Thank you for your patch, I've applied this patch to my review-hans
branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans

Note it will show up in my review-hans branch once I've pushed my
local branch there, which might take a while.

Once I've run some tests on this branch the patches there will be
added to the platform-drivers-x86/for-next branch and eventually
will be included in the pdx86 pull-request to Linus for the next
merge-window.

Regards,

Hans