Re: [PATCH v2 1/2] it87: Add param to ignore ACPI resource conflicts

[Guenter Roeck <linux@roeck-us.net>]

On Sun, Oct 02, 2022 at 06:43:00PM +0100, Ahmad Khalifa wrote:
> Add in the parameter to ignore ACPI resource conflicts so that modprobe
> can use the force_id parameter when ACPI has the same address mapped.

force_id and ignore_resource_conflict are orthogonal / unrelated to each
other. Why create a dependency or correlation where none exists ?

The reason for introducing ignore_resource_conflict in the driver was that
some systems didn't like the system wide parameter (acpi_enforce_resources)
to ignore resource conflicts and failed to boot if it was set to lax
or disabled.

> 
> Signed-off-by: Ahmad Khalifa <ahmad@khalifa.ws>
> ---
>  drivers/hwmon/it87.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hwmon/it87.c b/drivers/hwmon/it87.c
> index 0e543dbe0a6b..ce579f5aebcf 100644
> --- a/drivers/hwmon/it87.c
> +++ b/drivers/hwmon/it87.c
> @@ -69,6 +69,10 @@ static unsigned short force_id;
>  module_param(force_id, ushort, 0);
>  MODULE_PARM_DESC(force_id, "Override the detected device ID");
>  
> +static bool ignore_resource_conflict;
> +module_param(ignore_resource_conflict, bool, false);

The third parameter is the access permission. It should be 0 or maybe 0000.
Why "false" ?

> +MODULE_PARM_DESC(ignore_resource_conflict, "Ignore ACPI resource conflict");
> +
>  static struct platform_device *it87_pdev[2];
>  
>  #define	REG_2E	0x2e	/* The register to read/write */
> @@ -3261,8 +3265,10 @@ static int __init it87_device_add(int index, unsigned short address,
>  	int err;
>  
>  	err = acpi_check_resource_conflict(&res);
> -	if (err)
> -		return err;
> +	if (err){

Formatting: Space between ) and { required.

Guenter

> +		if (!ignore_resource_conflict)
> +			return err;
> +	}
>  
>  	pdev = platform_device_alloc(DRVNAME, address);
>  	if (!pdev)
> -- 
> 2.30.2
>