diff mbox series

[v1,1/5] hwmon: (lm75) Add kind field to struct lm75_data

Message ID 20190709095052.7964-2-iker.perez@codethink.co.uk (mailing list archive)
State Changes Requested
Headers show
Series Help with lm75.c changes | expand

Commit Message

Iker Perez July 9, 2019, 9:50 a.m. UTC 
From: Iker Perez del Palomar Sustatxa <iker.perez@codethink.co.uk>

The purpose of this change is to store the kind of device the
kernel is working with.

Signed-off-by: Iker Perez del Palomar Sustatxa <iker.perez@codethink.co.uk>
---
 drivers/hwmon/lm75.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

Comments

Guenter Roeck July 9, 2019, 1:20 p.m. UTC | #1 
On 7/9/19 2:50 AM, Iker Perez wrote:
> From: Iker Perez del Palomar Sustatxa <iker.perez@codethink.co.uk>
> 
> The purpose of this change is to store the kind of device the
> kernel is working with.
> 
> Signed-off-by: Iker Perez del Palomar Sustatxa <iker.perez@codethink.co.uk>
> ---
>   drivers/hwmon/lm75.c | 10 ++++++----
>   1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
> index 3fb9c0a2d6d0..0209e0719784 100644
> --- a/drivers/hwmon/lm75.c
> +++ b/drivers/hwmon/lm75.c
> @@ -68,6 +68,7 @@ struct lm75_data {
>   	u8			resolution;	/* In bits, between 9 and 16 */
>   	u8			resolution_limits;
>   	unsigned int		sample_time;	/* In ms */
> +	enum lm75_type		kind;
>   };
>   
>   /*-----------------------------------------------------------------------*/
> @@ -237,15 +238,16 @@ lm75_probe(struct i2c_client *client, const struct i2c_device_id *id)
>   	struct device *dev = &client->dev;
>   	struct device *hwmon_dev;
>   	struct lm75_data *data;
> +	struct lm75_data device_data;
>   	int status, err;
>   	u8 set_mask, clr_mask;
>   	int new;
> -	enum lm75_type kind;
>   
> +	data = &device_data;

This makes no sense.

>   	if (client->dev.of_node)
> -		kind = (enum lm75_type)of_device_get_match_data(&client->dev);
> +		data->kind = (enum lm75_type)of_device_get_match_data(&client->dev);
>   	else
> -		kind = id->driver_data;
> +		data->kind = id->driver_data;
>   
... because data is overwritten a couple of lines below when it is allocated.

You would do something like

	data->kind = kind;

after data was allocated. Looking at the above code, though, I really wonder if
and how you tested it.

Guenter

>   	if (!i2c_check_functionality(client->adapter,
>   			I2C_FUNC_SMBUS_BYTE_DATA | I2C_FUNC_SMBUS_WORD_DATA))
> @@ -267,7 +269,7 @@ lm75_probe(struct i2c_client *client, const struct i2c_device_id *id)
>   	set_mask = 0;
>   	clr_mask = LM75_SHUTDOWN;		/* continuous conversions */
>   
> -	switch (kind) {
> +	switch (data->kind) {
>   	case adt75:
>   		clr_mask |= 1 << 5;		/* not one-shot mode */
>   		data->resolution = 12;
>
diff mbox series

Patch

diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
index 3fb9c0a2d6d0..0209e0719784 100644
--- a/drivers/hwmon/lm75.c
+++ b/drivers/hwmon/lm75.c
@@ -68,6 +68,7 @@  struct lm75_data {
 	u8			resolution;	/* In bits, between 9 and 16 */
 	u8			resolution_limits;
 	unsigned int		sample_time;	/* In ms */
+	enum lm75_type		kind;
 };
 
 /*-----------------------------------------------------------------------*/
@@ -237,15 +238,16 @@  lm75_probe(struct i2c_client *client, const struct i2c_device_id *id)
 	struct device *dev = &client->dev;
 	struct device *hwmon_dev;
 	struct lm75_data *data;
+	struct lm75_data device_data;
 	int status, err;
 	u8 set_mask, clr_mask;
 	int new;
-	enum lm75_type kind;
 
+	data = &device_data;
 	if (client->dev.of_node)
-		kind = (enum lm75_type)of_device_get_match_data(&client->dev);
+		data->kind = (enum lm75_type)of_device_get_match_data(&client->dev);
 	else
-		kind = id->driver_data;
+		data->kind = id->driver_data;
 
 	if (!i2c_check_functionality(client->adapter,
 			I2C_FUNC_SMBUS_BYTE_DATA | I2C_FUNC_SMBUS_WORD_DATA))
@@ -267,7 +269,7 @@  lm75_probe(struct i2c_client *client, const struct i2c_device_id *id)
 	set_mask = 0;
 	clr_mask = LM75_SHUTDOWN;		/* continuous conversions */
 
-	switch (kind) {
+	switch (data->kind) {
 	case adt75:
 		clr_mask |= 1 << 5;		/* not one-shot mode */
 		data->resolution = 12;