1. Patchew
  2. linux
  3. View series

[PATCH] hwmon: (asus-ec-sensors) do not print from .probe()

Eugene Shalygin posted 1 patch 4 years, 4 months ago
There is a newer version of this series
drivers/hwmon/asus-ec-sensors.c | 17 +++++------------
1 file changed, 5 insertions(+), 12 deletions(-)
[PATCH] hwmon: (asus-ec-sensors) do not print from .probe()
Posted by Eugene Shalygin 4 years, 4 months ago 
Remove the call to dev_info() from the board detection function, which
is called from probe(), not only to be in line with hwmon driver rules, but
also because the message duplicates the error code returned from probe()
for that case (ENODEV).

Signed-off-by: Eugene Shalygin <eugene.shalygin@gmail.com>
---
 drivers/hwmon/asus-ec-sensors.c | 17 +++++------------
 1 file changed, 5 insertions(+), 12 deletions(-)

diff --git a/drivers/hwmon/asus-ec-sensors.c b/drivers/hwmon/asus-ec-sensors.c
index 0701ade16227..cbe1b987144a 100644
--- a/drivers/hwmon/asus-ec-sensors.c
+++ b/drivers/hwmon/asus-ec-sensors.c
@@ -597,18 +597,11 @@ static struct hwmon_chip_info asus_ec_chip_info = {
 	.ops = &asus_ec_hwmon_ops,
 };
 
-static unsigned long __init
-get_board_sensors(const struct device *dev)
+static unsigned long __init get_board_sensors(void)
 {
-	const struct dmi_system_id *dmi_entry;
-
-	dmi_entry = dmi_first_match(asus_ec_dmi_table);
-	if (!dmi_entry) {
-		dev_info(dev, "Unsupported board");
-		return 0;
-	}
-
-	return (unsigned long)dmi_entry->driver_data;
+	const struct dmi_system_id *dmi_entry =
+		dmi_first_match(asus_ec_dmi_table);
+	return dmi_entry ? (unsigned long)dmi_entry->driver_data : 0;
 }
 
 static int __init asus_ec_probe(struct platform_device *pdev)
@@ -625,7 +618,7 @@ static int __init asus_ec_probe(struct platform_device *pdev)
 	struct device *hwdev;
 	unsigned int i;
 
-	board_sensors = get_board_sensors(dev);
+	board_sensors = get_board_sensors();
 	if (!board_sensors)
 		return -ENODEV;
 
-- 
2.35.1
Re: [PATCH] hwmon: (asus-ec-sensors) do not print from .probe()
Posted by Guenter Roeck 4 years, 4 months ago 
On 2/16/22 23:23, Eugene Shalygin wrote:
> Remove the call to dev_info() from the board detection function, which
> is called from probe(), not only to be in line with hwmon driver rules, but
> also because the message duplicates the error code returned from probe()
> for that case (ENODEV).
> 
> Signed-off-by: Eugene Shalygin <eugene.shalygin@gmail.com>
> ---
>   drivers/hwmon/asus-ec-sensors.c | 17 +++++------------
>   1 file changed, 5 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/hwmon/asus-ec-sensors.c b/drivers/hwmon/asus-ec-sensors.c
> index 0701ade16227..cbe1b987144a 100644
> --- a/drivers/hwmon/asus-ec-sensors.c
> +++ b/drivers/hwmon/asus-ec-sensors.c
> @@ -597,18 +597,11 @@ static struct hwmon_chip_info asus_ec_chip_info = {
>   	.ops = &asus_ec_hwmon_ops,
>   };
>   
> -static unsigned long __init
> -get_board_sensors(const struct device *dev)
> +static unsigned long __init get_board_sensors(void)
>   {
> -	const struct dmi_system_id *dmi_entry;
> -
> -	dmi_entry = dmi_first_match(asus_ec_dmi_table);
> -	if (!dmi_entry) {
> -		dev_info(dev, "Unsupported board");
> -		return 0;
> -	}
> -
> -	return (unsigned long)dmi_entry->driver_data;
> +	const struct dmi_system_id *dmi_entry =
> +		dmi_first_match(asus_ec_dmi_table);
> +	return dmi_entry ? (unsigned long)dmi_entry->driver_data : 0;

Looks like you did not run checkpatch.

Either case, I think you should just drop this function In probe:

	const struct dmi_system_id *dmi_entry = dmi_first_match(asus_ec_dmi_table);

	...
	if (!dmi_entry)
		return -ENODEV;
	...
	ec_data->board_sensors = (unsigned long)dmi_entry->driver_data;

Guenter

>   }
>   
>   static int __init asus_ec_probe(struct platform_device *pdev)
> @@ -625,7 +618,7 @@ static int __init asus_ec_probe(struct platform_device *pdev)
>   	struct device *hwdev;
>   	unsigned int i;
>   
> -	board_sensors = get_board_sensors(dev);
> +	board_sensors = get_board_sensors();
>   	if (!board_sensors)
>   		return -ENODEV;
>
Re: [PATCH] hwmon: (asus-ec-sensors) do not print from .probe()
Posted by Eugene Shalygin 4 years, 4 months ago 
On Thu, 17 Feb 2022 at 19:26, Guenter Roeck <linux@roeck-us.net> wrote:
> Looks like you did not run checkpatch.

I did (0 errors/warnings/checks). What needs to be corrected?

> Either case, I think you should just drop this function In probe:

Yes, currently that function is tiny, but some tests with motherboards
from other families are done by users and if I add other families, the
information required for each board model will grow and in that case
I'd switch from dmi_system_id array to a custom struct to define all
the board-related data at at the same place, and to save some space in
the module binary, as unused parts of the dmi_system_id array already
take a quarter of the total binary size. So, the function will likely
get some more code soon.

Regards,
Eugene
Re: [PATCH] hwmon: (asus-ec-sensors) do not print from .probe()
Posted by Guenter Roeck 4 years, 4 months ago 
On 2/17/22 10:43, Eugene Shalygin wrote:
> On Thu, 17 Feb 2022 at 19:26, Guenter Roeck <linux@roeck-us.net> wrote:
>> Looks like you did not run checkpatch.
> 
> I did (0 errors/warnings/checks). What needs to be corrected?
> 

Interesting. It appears that the continuation line in the declaration
confuses it. Otherwise you would get:

WARNING: Missing a blank line after declarations

>> Either case, I think you should just drop this function In probe:
> 
> Yes, currently that function is tiny, but some tests with motherboards
> from other families are done by users and if I add other families, the
> information required for each board model will grow and in that case
> I'd switch from dmi_system_id array to a custom struct to define all
> the board-related data at at the same place, and to save some space in
> the module binary, as unused parts of the dmi_system_id array already
> take a quarter of the total binary size. So, the function will likely
> get some more code soon.
> 

Hmm, ok. Wouldn't you still need some kind of dmi match ?

Thanks,
Guenter

> Regards,
> Eugene
Re: [PATCH] hwmon: (asus-ec-sensors) do not print from .probe()
Posted by Eugene Shalygin 4 years, 4 months ago 
On Thu, 17 Feb 2022 at 20:33, Guenter Roeck <linux@roeck-us.net> wrote:
>
> On 2/17/22 10:43, Eugene Shalygin wrote:
> > On Thu, 17 Feb 2022 at 19:26, Guenter Roeck <linux@roeck-us.net> wrote:
> >> Looks like you did not run checkpatch.
> >
> > I did (0 errors/warnings/checks). What needs to be corrected?
> >
>
> Interesting. It appears that the continuation line in the declaration
> confuses it. Otherwise you would get:
>
> WARNING: Missing a blank line after declarations

Added in v2, thank you!

> >> Either case, I think you should just drop this function In probe:
> >
> > Yes, currently that function is tiny, but some tests with motherboards
> > from other families are done by users and if I add other families, the
> > information required for each board model will grow and in that case
> > I'd switch from dmi_system_id array to a custom struct to define all
> > the board-related data at at the same place, and to save some space in
> > the module binary, as unused parts of the dmi_system_id array already
> > take a quarter of the total binary size. So, the function will likely
> > get some more code soon.
> >
>
> Hmm, ok. Wouldn't you still need some kind of dmi match ?

Of course, just not via dmi_first_match():
https://github.com/zeule/asus-ec-sensors/blob/feature/prime-x470-pro-no-dmi/asus-ec-sensors.c#L787

Regards,
Eugene
Re: [PATCH] hwmon: (asus-ec-sensors) do not print from .probe()
Posted by Guenter Roeck 4 years, 4 months ago 
On 2/17/22 11:49, Eugene Shalygin wrote:
> On Thu, 17 Feb 2022 at 20:33, Guenter Roeck <linux@roeck-us.net> wrote:
>>
>> On 2/17/22 10:43, Eugene Shalygin wrote:
>>> On Thu, 17 Feb 2022 at 19:26, Guenter Roeck <linux@roeck-us.net> wrote:
>>>> Looks like you did not run checkpatch.
>>>
>>> I did (0 errors/warnings/checks). What needs to be corrected?
>>>
>>
>> Interesting. It appears that the continuation line in the declaration
>> confuses it. Otherwise you would get:
>>
>> WARNING: Missing a blank line after declarations
> 
> Added in v2, thank you!
> 
>>>> Either case, I think you should just drop this function In probe:
>>>
>>> Yes, currently that function is tiny, but some tests with motherboards
>>> from other families are done by users and if I add other families, the
>>> information required for each board model will grow and in that case
>>> I'd switch from dmi_system_id array to a custom struct to define all
>>> the board-related data at at the same place, and to save some space in
>>> the module binary, as unused parts of the dmi_system_id array already
>>> take a quarter of the total binary size. So, the function will likely
>>> get some more code soon.
>>>
>>
>> Hmm, ok. Wouldn't you still need some kind of dmi match ?
> 
> Of course, just not via dmi_first_match():
> https://github.com/zeule/asus-ec-sensors/blob/feature/prime-x470-pro-no-dmi/asus-ec-sensors.c#L787
> 

!strcmp(), and, yes, dmi functions can return NULL.

Guenter
Re: [PATCH] hwmon: (asus-ec-sensors) do not print from .probe()
Posted by Eugene Shalygin 4 years, 4 months ago 
On Thu, 17 Feb 2022 at 22:25, Guenter Roeck <linux@roeck-us.net> wrote:

> !strcmp(), and, yes, dmi functions can return NULL.

Thanks!

Regards,
Eugene

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.