Re: [PATCH 1/4] ACPI / PMIC: Do not register handlers for unhandled OpRegions

From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To: Hans de Goede <hdegoede@redhat.com>
Cc: "Rafael J . Wysocki" <rjw@rjwysocki.net>,
	Len Brown <lenb@kernel.org>, Lee Jones <lee.jones@linaro.org>,
	Mika Westerberg <mika.westerberg@linux.intel.com>,
	linux-acpi@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/4] ACPI / PMIC: Do not register handlers for unhandled OpRegions
Date: Fri, 25 Oct 2019 10:42:20 +0300	[thread overview]
Message-ID: <20191025074220.GY32742@smile.fi.intel.com> (raw)
In-Reply-To: <20191024213827.144974-2-hdegoede@redhat.com>

On Thu, Oct 24, 2019 at 11:38:24PM +0200, Hans de Goede wrote:
> For some model PMIC's used on Intel boards we do not know how to
> handle the power or thermal opregions because we have no documentation.
> 
> For example in the intel_pmic_chtwc.c driver thermal_table_count is 0,
> which means that our PMIC_THERMAL_OPREGION_ID handler will always fail
> with AE_BAD_PARAMETER, in this case it is better to simply not register
> the handler at all.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/acpi/pmic/intel_pmic.c | 20 +++++++++++++-------
>  1 file changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/acpi/pmic/intel_pmic.c b/drivers/acpi/pmic/intel_pmic.c
> index 452041398b34..a371f273f99d 100644
> --- a/drivers/acpi/pmic/intel_pmic.c
> +++ b/drivers/acpi/pmic/intel_pmic.c
> @@ -252,7 +252,7 @@ int intel_pmic_install_opregion_handler(struct device *dev, acpi_handle handle,
>  					struct regmap *regmap,
>  					struct intel_pmic_opregion_data *d)
>  {
> -	acpi_status status;
> +	acpi_status status = AE_OK;
>  	struct intel_pmic_opregion *opregion;
>  	int ret;
>  
> @@ -270,7 +270,8 @@ int intel_pmic_install_opregion_handler(struct device *dev, acpi_handle handle,
>  	opregion->regmap = regmap;
>  	opregion->lpat_table = acpi_lpat_get_conversion_table(handle);
>  
> -	status = acpi_install_address_space_handler(handle,
> +	if (d->power_table_count)
> +		status = acpi_install_address_space_handler(handle,
>  						    PMIC_POWER_OPREGION_ID,
>  						    intel_pmic_power_handler,
>  						    NULL, opregion);
> @@ -279,7 +280,8 @@ int intel_pmic_install_opregion_handler(struct device *dev, acpi_handle handle,
>  		goto out_error;
>  	}
>  
> -	status = acpi_install_address_space_handler(handle,
> +	if (d->thermal_table_count)
> +		status = acpi_install_address_space_handler(handle,
>  						    PMIC_THERMAL_OPREGION_ID,
>  						    intel_pmic_thermal_handler,
>  						    NULL, opregion);
> @@ -301,12 +303,16 @@ int intel_pmic_install_opregion_handler(struct device *dev, acpi_handle handle,
>  	return 0;
>  
>  out_remove_thermal_handler:
> -	acpi_remove_address_space_handler(handle, PMIC_THERMAL_OPREGION_ID,
> -					  intel_pmic_thermal_handler);
> +	if (d->thermal_table_count)
> +		acpi_remove_address_space_handler(handle,
> +						  PMIC_THERMAL_OPREGION_ID,
> +						  intel_pmic_thermal_handler);
>  
>  out_remove_power_handler:
> -	acpi_remove_address_space_handler(handle, PMIC_POWER_OPREGION_ID,
> -					  intel_pmic_power_handler);
> +	if (d->power_table_count)
> +		acpi_remove_address_space_handler(handle,
> +						  PMIC_POWER_OPREGION_ID,
> +						  intel_pmic_power_handler);
>  
>  out_error:
>  	acpi_lpat_free_conversion_table(opregion->lpat_table);
> -- 
> 2.23.0
> 

-- 
With Best Regards,
Andy Shevchenko