Re: [PATCH 1/2] ACPI / PMIC: Make some pointers, structure field and function argument as const

[Mika Westerberg]

On Fri, Nov 03, 2017 at 04:03:47PM +0100, Bhumika Goyal wrote:
> Make some pointers of type intel_pmic_opregion_data as const as they
> do not modify the fields of the structure they point too.
> After this change, make the data field of intel_pmic_opregion
> structure const as this data field is used for initializing the above pointers
> that are now const.
> Finally, make the struct intel_pmic_opregion_data * argument of the
> function intel_pmic_install_opregion_handler as const as it is only
> getting stored in the data field of the intel_pmic_opregion
> structure which is now made const.

One minor nit:

> Signed-off-by: Bhumika Goyal <bhumirks@xxxxxxxxx>
> ---
>  drivers/acpi/pmic/intel_pmic.c | 10 +++++-----
>  drivers/acpi/pmic/intel_pmic.h |  4 +++-
>  2 files changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/acpi/pmic/intel_pmic.c b/drivers/acpi/pmic/intel_pmic.c
> index ca18e0d..553fa4f 100644
> --- a/drivers/acpi/pmic/intel_pmic.c
> +++ b/drivers/acpi/pmic/intel_pmic.c
> @@ -32,7 +32,7 @@ struct intel_pmic_opregion {
>  	struct mutex lock;
>  	struct acpi_lpat_conversion_table *lpat_table;
>  	struct regmap *regmap;
> -	struct intel_pmic_opregion_data *data;
> +	const struct intel_pmic_opregion_data *data;
>  	struct intel_pmic_regs_handler_ctx ctx;
>  };
>  
> @@ -58,7 +58,7 @@ static acpi_status intel_pmic_power_handler(u32 function,
>  {
>  	struct intel_pmic_opregion *opregion = region_context;
>  	struct regmap *regmap = opregion->regmap;
> -	struct intel_pmic_opregion_data *d = opregion->data;
> +	const struct intel_pmic_opregion_data *d = opregion->data;
>  	int reg, bit, result;
>  
>  	if (bits != 32 || !value64)
> @@ -140,7 +140,7 @@ static int pmic_thermal_aux(struct intel_pmic_opregion *opregion, int reg,
>  static int pmic_thermal_pen(struct intel_pmic_opregion *opregion, int reg,
>  			    int bit, u32 function, u64 *value)
>  {
> -	struct intel_pmic_opregion_data *d = opregion->data;
> +	const struct intel_pmic_opregion_data *d = opregion->data;
>  	struct regmap *regmap = opregion->regmap;
>  
>  	if (!d->get_policy || !d->update_policy)
> @@ -176,7 +176,7 @@ static acpi_status intel_pmic_thermal_handler(u32 function,
>  		void *handler_context, void *region_context)
>  {
>  	struct intel_pmic_opregion *opregion = region_context;
> -	struct intel_pmic_opregion_data *d = opregion->data;
> +	const struct intel_pmic_opregion_data *d = opregion->data;
>  	int reg, bit, result;
>  
>  	if (bits != 32 || !value64)
> @@ -255,7 +255,7 @@ static acpi_status intel_pmic_regs_handler(u32 function,
>  
>  int intel_pmic_install_opregion_handler(struct device *dev, acpi_handle handle,
>  					struct regmap *regmap,
> -					struct intel_pmic_opregion_data *d)
> +					const struct intel_pmic_opregion_data *d)
>  {
>  	acpi_status status;
>  	struct intel_pmic_opregion *opregion;
> diff --git a/drivers/acpi/pmic/intel_pmic.h b/drivers/acpi/pmic/intel_pmic.h
> index e8bfa7b..754b7bd 100644
> --- a/drivers/acpi/pmic/intel_pmic.h
> +++ b/drivers/acpi/pmic/intel_pmic.h
> @@ -20,6 +20,8 @@ struct intel_pmic_opregion_data {
>  	int thermal_table_count;
>  };
>  
> -int intel_pmic_install_opregion_handler(struct device *dev, acpi_handle handle, struct regmap *regmap, struct intel_pmic_opregion_data *d);
> +int intel_pmic_install_opregion_handler(struct device *dev, acpi_handle handle,
> +					struct regmap *regmap,
> +					const struct intel_pmic_opregion_data *d);

I would rather write it like:

int intel_pmic_install_opregion_handler(struct device *dev, acpi_handle handle,
					struct regmap *regmap, const struct intel_pmic_opregion_data *d);

The rest looks good to me.