Re: [PATCH 1/4] hwmon (max6639): Use regmap

[Guenter Roeck]

On Tue, Apr 16, 2024 at 10:47:14PM +0530, Naresh Solanki wrote:
> Add regmap support.
> 

Missing (and not really utilizing) the benefits of using regmap.

> Signed-off-by: Naresh Solanki <naresh.solanki@xxxxxxxxxxxxx>
> ---
>  drivers/hwmon/Kconfig   |   1 +
>  drivers/hwmon/max6639.c | 157 ++++++++++++++++++++--------------------
>  2 files changed, 80 insertions(+), 78 deletions(-)
> 
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index c89776d91795..257ec5360e35 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -1223,6 +1223,7 @@ config SENSORS_MAX6621
>  config SENSORS_MAX6639
>  	tristate "Maxim MAX6639 sensor chip"
>  	depends on I2C
> +	select REGMAP_I2C
>  	help
>  	  If you say yes here you get support for the MAX6639
>  	  sensor chips.
> diff --git a/drivers/hwmon/max6639.c b/drivers/hwmon/max6639.c
> index aa7f21ab2395..1af93fc53cb5 100644
> --- a/drivers/hwmon/max6639.c
> +++ b/drivers/hwmon/max6639.c
> @@ -20,6 +20,7 @@
>  #include <linux/err.h>
>  #include <linux/mutex.h>
>  #include <linux/platform_data/max6639.h>
> +#include <linux/regmap.h>
>  
>  /* Addresses to scan */
>  static const unsigned short normal_i2c[] = { 0x2c, 0x2e, 0x2f, I2C_CLIENT_END };
> @@ -57,6 +58,8 @@ static const unsigned short normal_i2c[] = { 0x2c, 0x2e, 0x2f, I2C_CLIENT_END };
>  
>  #define MAX6639_FAN_CONFIG3_THERM_FULL_SPEED	0x40
>  
> +#define MAX6639_NDEV				2
> +
>  static const int rpm_ranges[] = { 2000, 4000, 8000, 16000 };
>  
>  #define FAN_FROM_REG(val, rpm_range)	((val) == 0 || (val) == 255 ? \
> @@ -67,7 +70,7 @@ static const int rpm_ranges[] = { 2000, 4000, 8000, 16000 };
>   * Client data (each client gets its own)
>   */
>  struct max6639_data {
> -	struct i2c_client *client;
> +	struct regmap *regmap;
>  	struct mutex update_lock;
>  	bool valid;		/* true if following fields are valid */
>  	unsigned long last_updated;	/* In jiffies */
> @@ -95,9 +98,8 @@ struct max6639_data {
>  static struct max6639_data *max6639_update_device(struct device *dev)
>  {
>  	struct max6639_data *data = dev_get_drvdata(dev);
> -	struct i2c_client *client = data->client;
>  	struct max6639_data *ret = data;
> -	int i;
> +	int i, err;
>  	int status_reg;
>  
>  	mutex_lock(&data->update_lock);
> @@ -105,39 +107,35 @@ static struct max6639_data *max6639_update_device(struct device *dev)

Conversions to regmap should drop all local caching and use regmap
for caching (where appropriate) instead.

Guenter