diff mbox series

[v3,1/2] hwmon: (emc2305) fix unable to probe emc2301/2/3

Message ID 20221206055331.170459-1-nanpuyue@gmail.com (mailing list archive)
State Accepted
Headers show
Series [v3,1/2] hwmon: (emc2305) fix unable to probe emc2301/2/3 | expand

Commit Message

Xingjiang Qiao Dec. 6, 2022, 5:53 a.m. UTC 
The definitions of 'EMC2305_REG_PRODUCT_ID' and 'EMC2305_REG_DEVICE' are
both '0xfd', they actually return the same value, but the values returned
by emc2301/2/3/5 are different, so probe emc2301/2/3 will fail, This patch
fixes that.

Signed-off-by: Xingjiang Qiao <nanpuyue@gmail.com>
---
 drivers/hwmon/emc2305.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

Comments

Guenter Roeck Dec. 6, 2022, 7:03 a.m. UTC | #1 
On 12/5/22 21:53, Xingjiang Qiao wrote:
> The definitions of 'EMC2305_REG_PRODUCT_ID' and 'EMC2305_REG_DEVICE' are
> both '0xfd', they actually return the same value, but the values returned
> by emc2301/2/3/5 are different, so probe emc2301/2/3 will fail, This patch
> fixes that.
> 
> Signed-off-by: Xingjiang Qiao <nanpuyue@gmail.com>

Please stop sending new versions of your patches as reply to previous versions,
and please provide change logs. I have no idea what is different between
versions 2 and 3 of this patch, and v2 as well as v3 almost got lost.

Guenter

> ---
>   drivers/hwmon/emc2305.c | 7 +------
>   1 file changed, 1 insertion(+), 6 deletions(-)
> 
> diff --git a/drivers/hwmon/emc2305.c b/drivers/hwmon/emc2305.c
> index aa1f25add0b6..9a78ca22541e 100644
> --- a/drivers/hwmon/emc2305.c
> +++ b/drivers/hwmon/emc2305.c
> @@ -16,7 +16,6 @@ static const unsigned short
>   emc2305_normal_i2c[] = { 0x27, 0x2c, 0x2d, 0x2e, 0x2f, 0x4c, 0x4d, I2C_CLIENT_END };
>   
>   #define EMC2305_REG_DRIVE_FAIL_STATUS	0x27
> -#define EMC2305_REG_DEVICE		0xfd
>   #define EMC2305_REG_VENDOR		0xfe
>   #define EMC2305_FAN_MAX			0xff
>   #define EMC2305_FAN_MIN			0x00
> @@ -524,7 +523,7 @@ static int emc2305_probe(struct i2c_client *client, const struct i2c_device_id *
>   	struct device *dev = &client->dev;
>   	struct emc2305_data *data;
>   	struct emc2305_platform_data *pdata;
> -	int vendor, device;
> +	int vendor;
>   	int ret;
>   	int i;
>   
> @@ -535,10 +534,6 @@ static int emc2305_probe(struct i2c_client *client, const struct i2c_device_id *
>   	if (vendor != EMC2305_VENDOR)
>   		return -ENODEV;
>   
> -	device = i2c_smbus_read_byte_data(client, EMC2305_REG_DEVICE);
> -	if (device != EMC2305_DEVICE)
> -		return -ENODEV;
> -
>   	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
>   	if (!data)
>   		return -ENOMEM;
Xingjiang Qiao Dec. 6, 2022, 12:01 p.m. UTC | #2 
> Please stop sending new versions of your patches as reply to previous versions,
> and please provide change logs. I have no idea what is different between
> versions 2 and 3 of this patch, and v2 as well as v3 almost got lost.
> 
> Guenter

Sorry about that.

And the changlogs of the patch 1/2 is:

- V1 -> V2: Just remove the check for 'EMC2305_REG_DEVICE' instead of
  moving the functionality of 'emc2305_identify'
- V2 -> V3: Reword the commit as there is no emc2304

changlogs of the patch 2/2 is:

- V1 -> V2: No substantive changes, was going to send later, but v3 has
  being sent before that (sorry about that again)
- V1 -> V3: Add 'emc2305_set_cur_state_shim' to avoid updating
  'last_thermal_state' when cooling state is set by 'hwmon' subsystem

The v3 patch 2/2 can be found at:
https://lore.kernel.org/all/20221206055331.170459-2-nanpuyue@gmail.com/

Should I resend them?

One more question about 'EMC2305_FAN_MAX_STATE':
https://lore.kernel.org/all/20221206053029.169506-1-nanpuyue@gmail.com/

> The value range of the chip's 'FAN DRIVE SETTING' register is 0-255.
> But currently, the driver can only set up to 10 values at most (even via
> the 'hwmon' subsystem) when the 'thermal' subsystem is reachable, and
> there is no way to increase this limit via "emc2305_platform_data".
> 
> Should the "pdata->max_state > EMC2305_FAN_MAX_STATE" check be removed?
> Or 'EMC2305_FAN_MAX_STATE' should be defined as '0xff'?
> 
> This should be the third patch?
> 
> 
> > 	pdata = dev_get_platdata(&client->dev);
> > 	if (pdata) {
> > 		if (!pdata->max_state || pdata->max_state > EMC2305_FAN_MAX_STATE)
> > 			return -EINVAL;
> > 		data->max_state = pdata->max_state;
Guenter Roeck Dec. 6, 2022, 10:39 p.m. UTC | #3 
On Tue, Dec 06, 2022 at 01:53:30PM +0800, Xingjiang Qiao wrote:
> The definitions of 'EMC2305_REG_PRODUCT_ID' and 'EMC2305_REG_DEVICE' are
> both '0xfd', they actually return the same value, but the values returned
> by emc2301/2/3/5 are different, so probe emc2301/2/3 will fail, This patch
> fixes that.
> 
> Signed-off-by: Xingjiang Qiao <nanpuyue@gmail.com>

Applied.

Thanks,
Guenter

> ---
>  drivers/hwmon/emc2305.c | 7 +------
>  1 file changed, 1 insertion(+), 6 deletions(-)
> 
> diff --git a/drivers/hwmon/emc2305.c b/drivers/hwmon/emc2305.c
> index aa1f25add0b6..9a78ca22541e 100644
> --- a/drivers/hwmon/emc2305.c
> +++ b/drivers/hwmon/emc2305.c
> @@ -16,7 +16,6 @@ static const unsigned short
>  emc2305_normal_i2c[] = { 0x27, 0x2c, 0x2d, 0x2e, 0x2f, 0x4c, 0x4d, I2C_CLIENT_END };
>  
>  #define EMC2305_REG_DRIVE_FAIL_STATUS	0x27
> -#define EMC2305_REG_DEVICE		0xfd
>  #define EMC2305_REG_VENDOR		0xfe
>  #define EMC2305_FAN_MAX			0xff
>  #define EMC2305_FAN_MIN			0x00
> @@ -524,7 +523,7 @@ static int emc2305_probe(struct i2c_client *client, const struct i2c_device_id *
>  	struct device *dev = &client->dev;
>  	struct emc2305_data *data;
>  	struct emc2305_platform_data *pdata;
> -	int vendor, device;
> +	int vendor;
>  	int ret;
>  	int i;
>  
> @@ -535,10 +534,6 @@ static int emc2305_probe(struct i2c_client *client, const struct i2c_device_id *
>  	if (vendor != EMC2305_VENDOR)
>  		return -ENODEV;
>  
> -	device = i2c_smbus_read_byte_data(client, EMC2305_REG_DEVICE);
> -	if (device != EMC2305_DEVICE)
> -		return -ENODEV;
> -
>  	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
>  	if (!data)
>  		return -ENOMEM;
diff mbox series

Patch

diff --git a/drivers/hwmon/emc2305.c b/drivers/hwmon/emc2305.c
index aa1f25add0b6..9a78ca22541e 100644
--- a/drivers/hwmon/emc2305.c
+++ b/drivers/hwmon/emc2305.c
@@ -16,7 +16,6 @@  static const unsigned short
 emc2305_normal_i2c[] = { 0x27, 0x2c, 0x2d, 0x2e, 0x2f, 0x4c, 0x4d, I2C_CLIENT_END };
 
 #define EMC2305_REG_DRIVE_FAIL_STATUS	0x27
-#define EMC2305_REG_DEVICE		0xfd
 #define EMC2305_REG_VENDOR		0xfe
 #define EMC2305_FAN_MAX			0xff
 #define EMC2305_FAN_MIN			0x00
@@ -524,7 +523,7 @@  static int emc2305_probe(struct i2c_client *client, const struct i2c_device_id *
 	struct device *dev = &client->dev;
 	struct emc2305_data *data;
 	struct emc2305_platform_data *pdata;
-	int vendor, device;
+	int vendor;
 	int ret;
 	int i;
 
@@ -535,10 +534,6 @@  static int emc2305_probe(struct i2c_client *client, const struct i2c_device_id *
 	if (vendor != EMC2305_VENDOR)
 		return -ENODEV;
 
-	device = i2c_smbus_read_byte_data(client, EMC2305_REG_DEVICE);
-	if (device != EMC2305_DEVICE)
-		return -ENODEV;
-
 	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
 	if (!data)
 		return -ENOMEM;