* Re: [PATCH] ACPI / PMIC: Remove unneeded check in tps68470_pmic_opregion_probe()
2024-07-05 11:34 [PATCH] ACPI / PMIC: Remove unneeded check in tps68470_pmic_opregion_probe() Aleksandr Mishin
@ 2024-07-06 11:56 ` Sakari Ailus
2024-07-06 13:47 ` Markus Elfring
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Sakari Ailus @ 2024-07-06 11:56 UTC (permalink / raw)
To: Aleksandr Mishin
Cc: Rajmohan Mani, Rafael J. Wysocki, Len Brown, Andy Shevchenko,
Mika Westerberg, linux-acpi, linux-kernel, lvc-project
On Fri, Jul 05, 2024 at 02:34:34PM +0300, Aleksandr Mishin wrote:
> In tps68470_pmic_opregion_probe() pointer 'dev' is compared to NULL which
> is useless.
>
> Fix this issue by removing unneeded check.
>
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
>
> Fixes: e13452ac3790 ("ACPI / PMIC: Add TI PMIC TPS68470 operation region driver")
> Signed-off-by: Aleksandr Mishin <amishin@t-argos.ru>
Thanks!
Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
> drivers/acpi/pmic/tps68470_pmic.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/acpi/pmic/tps68470_pmic.c b/drivers/acpi/pmic/tps68470_pmic.c
> index ebd03e472955..f8de8d0a528b 100644
> --- a/drivers/acpi/pmic/tps68470_pmic.c
> +++ b/drivers/acpi/pmic/tps68470_pmic.c
> @@ -376,7 +376,7 @@ static int tps68470_pmic_opregion_probe(struct platform_device *pdev)
> struct tps68470_pmic_opregion *opregion;
> acpi_status status;
>
> - if (!dev || !tps68470_regmap) {
> + if (!tps68470_regmap) {
> dev_warn(dev, "dev or regmap is NULL\n");
> return -EINVAL;
> }
--
Sakari Ailus
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] ACPI / PMIC: Remove unneeded check in tps68470_pmic_opregion_probe()
2024-07-05 11:34 [PATCH] ACPI / PMIC: Remove unneeded check in tps68470_pmic_opregion_probe() Aleksandr Mishin
2024-07-06 11:56 ` Sakari Ailus
@ 2024-07-06 13:47 ` Markus Elfring
2024-07-30 19:38 ` Andy Shevchenko
2024-07-30 22:53 ` [PATCH v2] " Aleksandr Mishin
3 siblings, 0 replies; 6+ messages in thread
From: Markus Elfring @ 2024-07-06 13:47 UTC (permalink / raw)
To: Aleksandr Mishin, linux-acpi, lvc-project, Rajmohan Mani, Sakari Ailus
Cc: LKML, Andy Shevchenko, Len Brown, Mika Westerberg, Rafael J. Wysocki
> In tps68470_pmic_opregion_probe() pointer 'dev' is compared to NULL which
> is useless.
because …?
> Fix this issue by removing unneeded check.
Another wording suggestion:
Thus remove a redundant check (and …?).
…
> +++ b/drivers/acpi/pmic/tps68470_pmic.c
> @@ -376,7 +376,7 @@ static int tps68470_pmic_opregion_probe(struct platform_device *pdev)
> struct tps68470_pmic_opregion *opregion;
> acpi_status status;
>
> - if (!dev || !tps68470_regmap) {
> + if (!tps68470_regmap) {
> dev_warn(dev, "dev or regmap is NULL\n");
…
Would you like to adjust the passed string literal accordingly?
Regards,
Markus
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] ACPI / PMIC: Remove unneeded check in tps68470_pmic_opregion_probe()
2024-07-05 11:34 [PATCH] ACPI / PMIC: Remove unneeded check in tps68470_pmic_opregion_probe() Aleksandr Mishin
2024-07-06 11:56 ` Sakari Ailus
2024-07-06 13:47 ` Markus Elfring
@ 2024-07-30 19:38 ` Andy Shevchenko
2024-07-30 22:53 ` [PATCH v2] " Aleksandr Mishin
3 siblings, 0 replies; 6+ messages in thread
From: Andy Shevchenko @ 2024-07-30 19:38 UTC (permalink / raw)
To: Aleksandr Mishin
Cc: Rajmohan Mani, Rafael J. Wysocki, Len Brown, Andy Shevchenko,
Mika Westerberg, Sakari Ailus, linux-acpi, linux-kernel,
lvc-project
On Fri, Jul 5, 2024 at 1:36 PM Aleksandr Mishin <amishin@t-argos.ru> wrote:
>
> In tps68470_pmic_opregion_probe() pointer 'dev' is compared to NULL which
> is useless.
>
> Fix this issue by removing unneeded check.
>
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
...
> - if (!dev || !tps68470_regmap) {
> + if (!tps68470_regmap) {
> dev_warn(dev, "dev or regmap is NULL\n");
Now this message is misleading.
> return -EINVAL;
> }
And I dunno why it's a warning and not an error level.
With all above being said, I would recommend to update to
if (!tps68470_regmap)
return dev_err_probe(dev, -EINVAL, "regmap is missing\n");
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v2] ACPI / PMIC: Remove unneeded check in tps68470_pmic_opregion_probe()
2024-07-05 11:34 [PATCH] ACPI / PMIC: Remove unneeded check in tps68470_pmic_opregion_probe() Aleksandr Mishin
` (2 preceding siblings ...)
2024-07-30 19:38 ` Andy Shevchenko
@ 2024-07-30 22:53 ` Aleksandr Mishin
2024-07-31 7:19 ` Andy Shevchenko
3 siblings, 1 reply; 6+ messages in thread
From: Aleksandr Mishin @ 2024-07-30 22:53 UTC (permalink / raw)
To: Rajmohan Mani
Cc: Aleksandr Mishin, Rafael J. Wysocki, Len Brown, Andy Shevchenko,
Mika Westerberg, Sakari Ailus, linux-acpi, linux-kernel,
lvc-project, Andy Shevchenko
In tps68470_pmic_opregion_probe() pointer 'dev' is compared to NULL which
is useless.
Fix this issue by removing unneeded check.
Found by Linux Verification Center (linuxtesting.org) with SVACE.
Fixes: e13452ac3790 ("ACPI / PMIC: Add TI PMIC TPS68470 operation region driver")
Suggested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Aleksandr Mishin <amishin@t-argos.ru>
Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
v1->v2:
- Add Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
(https://lore.kernel.org/all/Zokw6XRZxv0YqIiZ@kekkonen.localdomain/)
- Update log message and its level as suggested by Andy
(https://lore.kernel.org/all/CAHp75Vefw5FB1mK8v-FJ5MBeSo_N9fgiAFPdYw2w_OCX5UxrAA@mail.gmail.com/)
drivers/acpi/pmic/tps68470_pmic.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/acpi/pmic/tps68470_pmic.c b/drivers/acpi/pmic/tps68470_pmic.c
index ebd03e472955..0d1a82eeb4b0 100644
--- a/drivers/acpi/pmic/tps68470_pmic.c
+++ b/drivers/acpi/pmic/tps68470_pmic.c
@@ -376,10 +376,8 @@ static int tps68470_pmic_opregion_probe(struct platform_device *pdev)
struct tps68470_pmic_opregion *opregion;
acpi_status status;
- if (!dev || !tps68470_regmap) {
- dev_warn(dev, "dev or regmap is NULL\n");
- return -EINVAL;
- }
+ if (!tps68470_regmap)
+ return dev_err_probe(dev, -EINVAL, "regmap is missing\n");
if (!handle) {
dev_warn(dev, "acpi handle is NULL\n");
--
2.30.2
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] ACPI / PMIC: Remove unneeded check in tps68470_pmic_opregion_probe()
2024-07-30 22:53 ` [PATCH v2] " Aleksandr Mishin
@ 2024-07-31 7:19 ` Andy Shevchenko
0 siblings, 0 replies; 6+ messages in thread
From: Andy Shevchenko @ 2024-07-31 7:19 UTC (permalink / raw)
To: Aleksandr Mishin
Cc: Rajmohan Mani, Rafael J. Wysocki, Len Brown, Andy Shevchenko,
Mika Westerberg, Sakari Ailus, linux-acpi, linux-kernel,
lvc-project
On Wed, Jul 31, 2024 at 12:57 AM Aleksandr Mishin <amishin@t-argos.ru> wrote:
>
> In tps68470_pmic_opregion_probe() pointer 'dev' is compared to NULL which
> is useless.
>
> Fix this issue by removing unneeded check.
>
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
FWIW,
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
P.S. In case the first version is already applied feel free to use the
above tag for the modifications that lead to the code as after this
patch.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 6+ messages in thread