diff mbox series

[v2,2/2] it87: check device is valid before using force_id

Message ID 20221002174259.14609-3-ahmad@khalifa.ws (mailing list archive)
State Changes Requested
Headers show
Series it87: Add param to ignore ACPI resource conflicts | expand

Commit Message

Ahmad Khalifa Oct. 2, 2022, 5:43 p.m. UTC
Check if there is a valid device before using force_id parameter value
in order to avoid registering two devices.

Signed-off-by: Ahmad Khalifa <ahmad@khalifa.ws>
---
 drivers/hwmon/it87.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Comments

Guenter Roeck Oct. 3, 2022, 6:01 p.m. UTC | #1
On Sun, Oct 02, 2022 at 06:43:02PM +0100, Ahmad Khalifa wrote:
> Check if there is a valid device before using force_id parameter value
> in order to avoid registering two devices.
> 

For the subject, please use "hwmon: (it87) ..."
in the future.

> Signed-off-by: Ahmad Khalifa <ahmad@khalifa.ws>
> ---
>  drivers/hwmon/it87.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hwmon/it87.c b/drivers/hwmon/it87.c
> index ce579f5aebcf..6d71f6c481c8 100644
> --- a/drivers/hwmon/it87.c
> +++ b/drivers/hwmon/it87.c
> @@ -2401,7 +2401,13 @@ static int __init it87_find(int sioaddr, unsigned short *address,
>  		return err;
>  
>  	err = -ENODEV;
> -	chip_type = force_id ? force_id : superio_inw(sioaddr, DEVID);
> +	chip_type = superio_inw(sioaddr, DEVID);
> +	/* check first so force_id doesn't register a second empty device */

The reasoning is wrong: the ID is checked to avoid registering a
non-existing or a completely incompatible device. It doesn't matter
if the skipped device is the first or the second device.

> +	if (chip_type == 0xffff)
> +		goto exit;
> +
> +	if (force_id)
> +		chip_type = force_id;
>  
>  	switch (chip_type) {
>  	case IT8705F_DEVID:
> -- 
> 2.30.2
>
Ahmad Khalifa Oct. 3, 2022, 8:28 p.m. UTC | #2
On 03/10/2022 19:01, Guenter Roeck wrote:
> On Sun, Oct 02, 2022 at 06:43:02PM +0100, Ahmad Khalifa wrote:
>> Check if there is a valid device before using force_id parameter value
>> in order to avoid registering two devices.
> For the subject, please use "hwmon: (it87) ..."
> in the future.

Will do with v3 of the patch.

>> +	/* check first so force_id doesn't register a second empty device */
> 
> The reasoning is wrong: the ID is checked to avoid registering a
> non-existing or a completely incompatible device. It doesn't matter
> if the skipped device is the first or the second device.

Non-existing is more accurate than empty, I can change to that in v3
The physical device doesn't exist, but the platform device is registered 
with no attributes.
diff mbox series

Patch

diff --git a/drivers/hwmon/it87.c b/drivers/hwmon/it87.c
index ce579f5aebcf..6d71f6c481c8 100644
--- a/drivers/hwmon/it87.c
+++ b/drivers/hwmon/it87.c
@@ -2401,7 +2401,13 @@  static int __init it87_find(int sioaddr, unsigned short *address,
 		return err;
 
 	err = -ENODEV;
-	chip_type = force_id ? force_id : superio_inw(sioaddr, DEVID);
+	chip_type = superio_inw(sioaddr, DEVID);
+	/* check first so force_id doesn't register a second empty device */
+	if (chip_type == 0xffff)
+		goto exit;
+
+	if (force_id)
+		chip_type = force_id;
 
 	switch (chip_type) {
 	case IT8705F_DEVID: