linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] hwmon: (scpi) skip unsupported sensors properly
@ 2015-10-28 14:08 Sudeep Holla
  2015-10-28 15:39 ` Punit Agrawal
  2015-10-28 17:17 ` [PATCH v2] " Sudeep Holla
  0 siblings, 2 replies; 8+ messages in thread
From: Sudeep Holla @ 2015-10-28 14:08 UTC (permalink / raw)
  To: linux-kernel, Guenter Roeck, Punit Agrawal
  Cc: Sudeep Holla, lm-sensors, Jean Delvare

Currently it's assumed that firmware exports only the class of sensors
supported by the driver. However with newer firmware or SCPI protocol
revision, support for newer classes of sensors can be present.

The driver fails to probe with the following warning if an unsupported
class of sensor is encountered in the firmware.

sysfs: cannot create duplicate filename
	'/devices/platform/scpi/scpi:sensors/hwmon/hwmon0/'
------------[ cut here ]------------
WARNING: at fs/sysfs/dir.c:31
Modules linked in:

CPU: 0 PID: 6 Comm: kworker/u12:0 Not tainted 4.3.0-rc7 #137
Hardware name: ARM Juno development board (r0) (DT)
Workqueue: deferwq deferred_probe_work_func
PC is at sysfs_warn_dup+0x54/0x78
LR is at sysfs_warn_dup+0x54/0x78

This patch fixes the above issue by skipping through the unsupported
class of SCPI sensors.

Fixes: 68acc77a2d51 ("hwmon: Support thermal zones registration for SCP temperature sensors")
Fixes: ea98b29a05e9 ("hwmon: Support sensors exported via ARM SCP interface")
Cc: Punit Agrawal <punit.agrawal@arm.com>
Cc: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/hwmon/scpi-hwmon.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/hwmon/scpi-hwmon.c b/drivers/hwmon/scpi-hwmon.c
index 2c1241bbf9af..5b80cd7f5c86 100644
--- a/drivers/hwmon/scpi-hwmon.c
+++ b/drivers/hwmon/scpi-hwmon.c
@@ -117,7 +117,7 @@ static int scpi_hwmon_probe(struct platform_device *pdev)
 	struct scpi_ops *scpi_ops;
 	struct device *hwdev, *dev = &pdev->dev;
 	struct scpi_sensors *scpi_sensors;
-	int ret;
+	int ret, idx;

 	scpi_ops = get_scpi_ops();
 	if (!scpi_ops)
@@ -146,8 +146,8 @@ static int scpi_hwmon_probe(struct platform_device *pdev)

 	scpi_sensors->scpi_ops = scpi_ops;

-	for (i = 0; i < nr_sensors; i++) {
-		struct sensor_data *sensor = &scpi_sensors->data[i];
+	for (i = 0, idx = 0; i < nr_sensors; i++) {
+		struct sensor_data *sensor = &scpi_sensors->data[idx];

 		ret = scpi_ops->sensor_get_info(i, &sensor->info);
 		if (ret)
@@ -183,7 +183,7 @@ static int scpi_hwmon_probe(struct platform_device *pdev)
 			num_power++;
 			break;
 		default:
-			break;
+			continue;
 		}

 		sensor->dev_attr_input.attr.mode = S_IRUGO;
@@ -194,11 +194,12 @@ static int scpi_hwmon_probe(struct platform_device *pdev)
 		sensor->dev_attr_label.show = scpi_show_label;
 		sensor->dev_attr_label.attr.name = sensor->label;

-		scpi_sensors->attrs[i << 1] = &sensor->dev_attr_input.attr;
-		scpi_sensors->attrs[(i << 1) + 1] = &sensor->dev_attr_label.attr;
+		scpi_sensors->attrs[idx << 1] = &sensor->dev_attr_input.attr;
+		scpi_sensors->attrs[(idx << 1) + 1] = &sensor->dev_attr_label.attr;

-		sysfs_attr_init(scpi_sensors->attrs[i << 1]);
-		sysfs_attr_init(scpi_sensors->attrs[(i << 1) + 1]);
+		sysfs_attr_init(scpi_sensors->attrs[idx << 1]);
+		sysfs_attr_init(scpi_sensors->attrs[(idx << 1) + 1]);
+		idx++;
 	}

 	scpi_sensors->group.attrs = scpi_sensors->attrs;
@@ -234,9 +235,9 @@ static int scpi_hwmon_probe(struct platform_device *pdev)
 			goto unregister_tzd;
 		}

-		zone->sensor_id = i;
+		zone->sensor_id = sensor->info.sensor_id;
 		zone->scpi_sensors = scpi_sensors;
-		zone->tzd = thermal_zone_of_sensor_register(dev, i, zone,
+		zone->tzd = thermal_zone_of_sensor_register(dev, zone->sensor_id, zone,
 							    &scpi_sensor_ops);
 		/*
 		 * The call to thermal_zone_of_sensor_register returns
--
1.9.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] hwmon: (scpi) skip unsupported sensors properly
  2015-10-28 14:08 [PATCH] hwmon: (scpi) skip unsupported sensors properly Sudeep Holla
@ 2015-10-28 15:39 ` Punit Agrawal
  2015-10-28 15:58   ` Sudeep Holla
  2015-10-28 17:17 ` [PATCH v2] " Sudeep Holla
  1 sibling, 1 reply; 8+ messages in thread
From: Punit Agrawal @ 2015-10-28 15:39 UTC (permalink / raw)
  To: Sudeep Holla; +Cc: linux-kernel, Guenter Roeck, lm-sensors, Jean Delvare

Hi Sudeep,

Sudeep Holla <sudeep.holla@arm.com> writes:

> Currently it's assumed that firmware exports only the class of sensors
> supported by the driver. However with newer firmware or SCPI protocol
> revision, support for newer classes of sensors can be present.
>
> The driver fails to probe with the following warning if an unsupported
> class of sensor is encountered in the firmware.
>
> sysfs: cannot create duplicate filename
> 	'/devices/platform/scpi/scpi:sensors/hwmon/hwmon0/'
> ------------[ cut here ]------------
> WARNING: at fs/sysfs/dir.c:31
> Modules linked in:
>
> CPU: 0 PID: 6 Comm: kworker/u12:0 Not tainted 4.3.0-rc7 #137
> Hardware name: ARM Juno development board (r0) (DT)
> Workqueue: deferwq deferred_probe_work_func
> PC is at sysfs_warn_dup+0x54/0x78
> LR is at sysfs_warn_dup+0x54/0x78
>

Thanks for spotting the issue and the fix below. Some comments below.


> This patch fixes the above issue by skipping through the unsupported
> class of SCPI sensors.
>
> Fixes: 68acc77a2d51 ("hwmon: Support thermal zones registration for SCP temperature sensors")
> Fixes: ea98b29a05e9 ("hwmon: Support sensors exported via ARM SCP interface")
> Cc: Punit Agrawal <punit.agrawal@arm.com>
> Cc: Guenter Roeck <linux@roeck-us.net>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>


> ---
>  drivers/hwmon/scpi-hwmon.c | 21 +++++++++++----------
>  1 file changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/hwmon/scpi-hwmon.c b/drivers/hwmon/scpi-hwmon.c
> index 2c1241bbf9af..5b80cd7f5c86 100644
> --- a/drivers/hwmon/scpi-hwmon.c
> +++ b/drivers/hwmon/scpi-hwmon.c

[...]

> @@ -234,9 +235,9 @@ static int scpi_hwmon_probe(struct platform_device *pdev)
>  			goto unregister_tzd;
>  		}
>
> -		zone->sensor_id = i;
> +		zone->sensor_id = sensor->info.sensor_id;

This shouldn't be changed . The zone->sensor_id is used to access the sensor
data in scpi_read_temp and will use the wrong index with the above
change. Which means...

>  		zone->scpi_sensors = scpi_sensors;
> -		zone->tzd = thermal_zone_of_sensor_register(dev, i, zone,
> +		zone->tzd = thermal_zone_of_sensor_register(dev, zone->sensor_id, zone,
>  							    &scpi_sensor_ops);

... the thermal zone registration should use sensor->info.sensor_id
instead of zone->sensor_id.

With these two changes, feel free to add

     Reviewed-by: Punit Agrawal <punit.agrawal@arm.com>

>  		/*
>  		 * The call to thermal_zone_of_sensor_register returns
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] hwmon: (scpi) skip unsupported sensors properly
  2015-10-28 15:39 ` Punit Agrawal
@ 2015-10-28 15:58   ` Sudeep Holla
  0 siblings, 0 replies; 8+ messages in thread
From: Sudeep Holla @ 2015-10-28 15:58 UTC (permalink / raw)
  To: Punit Agrawal
  Cc: Sudeep Holla, linux-kernel, Guenter Roeck, lm-sensors,
	Jean Delvare



On 28/10/15 15:39, Punit Agrawal wrote:
> Hi Sudeep,
>
> Sudeep Holla <sudeep.holla@arm.com> writes:
>
>> Currently it's assumed that firmware exports only the class of sensors
>> supported by the driver. However with newer firmware or SCPI protocol
>> revision, support for newer classes of sensors can be present.
>>
>> The driver fails to probe with the following warning if an unsupported
>> class of sensor is encountered in the firmware.
>>
>> sysfs: cannot create duplicate filename
>> 	'/devices/platform/scpi/scpi:sensors/hwmon/hwmon0/'
>> ------------[ cut here ]------------
>> WARNING: at fs/sysfs/dir.c:31
>> Modules linked in:
>>
>> CPU: 0 PID: 6 Comm: kworker/u12:0 Not tainted 4.3.0-rc7 #137
>> Hardware name: ARM Juno development board (r0) (DT)
>> Workqueue: deferwq deferred_probe_work_func
>> PC is at sysfs_warn_dup+0x54/0x78
>> LR is at sysfs_warn_dup+0x54/0x78
>>
>
> Thanks for spotting the issue and the fix below. Some comments below.
>
>
>> This patch fixes the above issue by skipping through the unsupported
>> class of SCPI sensors.
>>
>> Fixes: 68acc77a2d51 ("hwmon: Support thermal zones registration for SCP temperature sensors")
>> Fixes: ea98b29a05e9 ("hwmon: Support sensors exported via ARM SCP interface")
>> Cc: Punit Agrawal <punit.agrawal@arm.com>
>> Cc: Guenter Roeck <linux@roeck-us.net>
>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>
>
>> ---
>>   drivers/hwmon/scpi-hwmon.c | 21 +++++++++++----------
>>   1 file changed, 11 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/hwmon/scpi-hwmon.c b/drivers/hwmon/scpi-hwmon.c
>> index 2c1241bbf9af..5b80cd7f5c86 100644
>> --- a/drivers/hwmon/scpi-hwmon.c
>> +++ b/drivers/hwmon/scpi-hwmon.c
>
> [...]
>
>> @@ -234,9 +235,9 @@ static int scpi_hwmon_probe(struct platform_device *pdev)
>>   			goto unregister_tzd;
>>   		}
>>
>> -		zone->sensor_id = i;
>> +		zone->sensor_id = sensor->info.sensor_id;
>
> This shouldn't be changed . The zone->sensor_id is used to access the sensor
> data in scpi_read_temp and will use the wrong index with the above
> change. Which means...
>

Ah, right thanks for spotting this.

>>   		zone->scpi_sensors = scpi_sensors;
>> -		zone->tzd = thermal_zone_of_sensor_register(dev, i, zone,
>> +		zone->tzd = thermal_zone_of_sensor_register(dev, zone->sensor_id, zone,
>>   							    &scpi_sensor_ops);
>
> ... the thermal zone registration should use sensor->info.sensor_id
> instead of zone->sensor_id.
>
> With these two changes, feel free to add
>
>       Reviewed-by: Punit Agrawal <punit.agrawal@arm.com>

Will update the patch with these 2 changes and resend. Thanks for the
review.

-- 
Regards,
Sudeep

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2] hwmon: (scpi) skip unsupported sensors properly
  2015-10-28 14:08 [PATCH] hwmon: (scpi) skip unsupported sensors properly Sudeep Holla
  2015-10-28 15:39 ` Punit Agrawal
@ 2015-10-28 17:17 ` Sudeep Holla
  2015-10-29  4:59   ` Guenter Roeck
  2015-11-16 17:59   ` [v2] " Guenter Roeck
  1 sibling, 2 replies; 8+ messages in thread
From: Sudeep Holla @ 2015-10-28 17:17 UTC (permalink / raw)
  To: linux-kernel, Guenter Roeck, Punit Agrawal
  Cc: Sudeep Holla, lm-sensors, Jean Delvare

Currently it's assumed that firmware exports only the class of sensors
supported by the driver. However with newer firmware or SCPI protocol
revision, support for newer classes of sensors can be present.

The driver fails to probe with the following warning if an unsupported
class of sensor is encountered in the firmware.

sysfs: cannot create duplicate filename
	'/devices/platform/scpi/scpi:sensors/hwmon/hwmon0/'
------------[ cut here ]------------
WARNING: at fs/sysfs/dir.c:31
Modules linked in:

CPU: 0 PID: 6 Comm: kworker/u12:0 Not tainted 4.3.0-rc7 #137
Hardware name: ARM Juno development board (r0) (DT)
Workqueue: deferwq deferred_probe_work_func
PC is at sysfs_warn_dup+0x54/0x78
LR is at sysfs_warn_dup+0x54/0x78

This patch fixes the above issue by skipping through the unsupported
class of SCPI sensors.

Fixes: 68acc77a2d51 ("hwmon: Support thermal zones registration for SCP temperature sensors")
Fixes: ea98b29a05e9 ("hwmon: Support sensors exported via ARM SCP interface")
Cc: Guenter Roeck <linux@roeck-us.net>
Reviewed-by: Punit Agrawal <punit.agrawal@arm.com>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/hwmon/scpi-hwmon.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

Hi Guenter,

Either me/Punit will need ping you once the original driver is merged
via arm-soc so that you can pick this after that. Alternatively we
can push it via arm-soc but I wouldn't rush for that as it's not that
urgent. Is that fine with you ?

Regards,
Sudeep

diff --git a/drivers/hwmon/scpi-hwmon.c b/drivers/hwmon/scpi-hwmon.c
index 2c1241bbf9af..7e20567bc369 100644
--- a/drivers/hwmon/scpi-hwmon.c
+++ b/drivers/hwmon/scpi-hwmon.c
@@ -117,7 +117,7 @@ static int scpi_hwmon_probe(struct platform_device *pdev)
 	struct scpi_ops *scpi_ops;
 	struct device *hwdev, *dev = &pdev->dev;
 	struct scpi_sensors *scpi_sensors;
-	int ret;
+	int ret, idx;

 	scpi_ops = get_scpi_ops();
 	if (!scpi_ops)
@@ -146,8 +146,8 @@ static int scpi_hwmon_probe(struct platform_device *pdev)

 	scpi_sensors->scpi_ops = scpi_ops;

-	for (i = 0; i < nr_sensors; i++) {
-		struct sensor_data *sensor = &scpi_sensors->data[i];
+	for (i = 0, idx = 0; i < nr_sensors; i++) {
+		struct sensor_data *sensor = &scpi_sensors->data[idx];

 		ret = scpi_ops->sensor_get_info(i, &sensor->info);
 		if (ret)
@@ -183,7 +183,7 @@ static int scpi_hwmon_probe(struct platform_device *pdev)
 			num_power++;
 			break;
 		default:
-			break;
+			continue;
 		}

 		sensor->dev_attr_input.attr.mode = S_IRUGO;
@@ -194,11 +194,12 @@ static int scpi_hwmon_probe(struct platform_device *pdev)
 		sensor->dev_attr_label.show = scpi_show_label;
 		sensor->dev_attr_label.attr.name = sensor->label;

-		scpi_sensors->attrs[i << 1] = &sensor->dev_attr_input.attr;
-		scpi_sensors->attrs[(i << 1) + 1] = &sensor->dev_attr_label.attr;
+		scpi_sensors->attrs[idx << 1] = &sensor->dev_attr_input.attr;
+		scpi_sensors->attrs[(idx << 1) + 1] = &sensor->dev_attr_label.attr;

-		sysfs_attr_init(scpi_sensors->attrs[i << 1]);
-		sysfs_attr_init(scpi_sensors->attrs[(i << 1) + 1]);
+		sysfs_attr_init(scpi_sensors->attrs[idx << 1]);
+		sysfs_attr_init(scpi_sensors->attrs[(idx << 1) + 1]);
+		idx++;
 	}

 	scpi_sensors->group.attrs = scpi_sensors->attrs;
@@ -236,8 +237,8 @@ static int scpi_hwmon_probe(struct platform_device *pdev)

 		zone->sensor_id = i;
 		zone->scpi_sensors = scpi_sensors;
-		zone->tzd = thermal_zone_of_sensor_register(dev, i, zone,
-							    &scpi_sensor_ops);
+		zone->tzd = thermal_zone_of_sensor_register(dev,
+				sensor->info.sensor_id, zone, &scpi_sensor_ops);
 		/*
 		 * The call to thermal_zone_of_sensor_register returns
 		 * an error for sensors that are not associated with
--
1.9.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] hwmon: (scpi) skip unsupported sensors properly
  2015-10-28 17:17 ` [PATCH v2] " Sudeep Holla
@ 2015-10-29  4:59   ` Guenter Roeck
  2015-11-16 18:00     ` Punit Agrawal
  2015-11-16 17:59   ` [v2] " Guenter Roeck
  1 sibling, 1 reply; 8+ messages in thread
From: Guenter Roeck @ 2015-10-29  4:59 UTC (permalink / raw)
  To: Sudeep Holla; +Cc: linux-kernel, Punit Agrawal, lm-sensors, Jean Delvare

On Wed, Oct 28, 2015 at 05:17:31PM +0000, Sudeep Holla wrote:
> Currently it's assumed that firmware exports only the class of sensors
> supported by the driver. However with newer firmware or SCPI protocol
> revision, support for newer classes of sensors can be present.
> 
> The driver fails to probe with the following warning if an unsupported
> class of sensor is encountered in the firmware.
> 
> sysfs: cannot create duplicate filename
> 	'/devices/platform/scpi/scpi:sensors/hwmon/hwmon0/'
> ------------[ cut here ]------------
> WARNING: at fs/sysfs/dir.c:31
> Modules linked in:
> 
> CPU: 0 PID: 6 Comm: kworker/u12:0 Not tainted 4.3.0-rc7 #137
> Hardware name: ARM Juno development board (r0) (DT)
> Workqueue: deferwq deferred_probe_work_func
> PC is at sysfs_warn_dup+0x54/0x78
> LR is at sysfs_warn_dup+0x54/0x78
> 
> This patch fixes the above issue by skipping through the unsupported
> class of SCPI sensors.
> 
> Fixes: 68acc77a2d51 ("hwmon: Support thermal zones registration for SCP temperature sensors")
> Fixes: ea98b29a05e9 ("hwmon: Support sensors exported via ARM SCP interface")
> Cc: Guenter Roeck <linux@roeck-us.net>
> Reviewed-by: Punit Agrawal <punit.agrawal@arm.com>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>

Acked-by: Guenter Roeck <linux@roeck-us.net>

> ---
>  drivers/hwmon/scpi-hwmon.c | 21 +++++++++++----------
>  1 file changed, 11 insertions(+), 10 deletions(-)
> 
> Hi Guenter,
> 
> Either me/Punit will need ping you once the original driver is merged
> via arm-soc so that you can pick this after that. Alternatively we
> can push it via arm-soc but I wouldn't rush for that as it's not that
> urgent. Is that fine with you ?
> 
Either way is fine with me.

Guenter

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [v2] hwmon: (scpi) skip unsupported sensors properly
  2015-10-28 17:17 ` [PATCH v2] " Sudeep Holla
  2015-10-29  4:59   ` Guenter Roeck
@ 2015-11-16 17:59   ` Guenter Roeck
  2015-11-16 19:37     ` Punit Agrawal
  1 sibling, 1 reply; 8+ messages in thread
From: Guenter Roeck @ 2015-11-16 17:59 UTC (permalink / raw)
  To: Sudeep Holla; +Cc: linux-kernel, Punit Agrawal, Jean Delvare, lm-sensors

On Wed, Oct 28, 2015 at 05:17:31PM +0000, Sudeep Holla wrote:
> Currently it's assumed that firmware exports only the class of sensors
> supported by the driver. However with newer firmware or SCPI protocol
> revision, support for newer classes of sensors can be present.
> 
> The driver fails to probe with the following warning if an unsupported
> class of sensor is encountered in the firmware.
> 
> sysfs: cannot create duplicate filename
> 	'/devices/platform/scpi/scpi:sensors/hwmon/hwmon0/'
> ------------[ cut here ]------------
> WARNING: at fs/sysfs/dir.c:31
> Modules linked in:
> 
> CPU: 0 PID: 6 Comm: kworker/u12:0 Not tainted 4.3.0-rc7 #137
> Hardware name: ARM Juno development board (r0) (DT)
> Workqueue: deferwq deferred_probe_work_func
> PC is at sysfs_warn_dup+0x54/0x78
> LR is at sysfs_warn_dup+0x54/0x78
> 
> This patch fixes the above issue by skipping through the unsupported
> class of SCPI sensors.
> 
> Fixes: 68acc77a2d51 ("hwmon: Support thermal zones registration for SCP temperature sensors")
> Fixes: ea98b29a05e9 ("hwmon: Support sensors exported via ARM SCP interface")
> Cc: Guenter Roeck <linux@roeck-us.net>
> Reviewed-by: Punit Agrawal <punit.agrawal@arm.com>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> Acked-by: Guenter Roeck <linux@roeck-us.net>

In the assumption that this patch can now be applied, I queued it up
for the next -rc.

Guenter

> ---
>  drivers/hwmon/scpi-hwmon.c | 21 +++++++++++----------
>  1 file changed, 11 insertions(+), 10 deletions(-)
> 
> Hi Guenter,
> 
> Either me/Punit will need ping you once the original driver is merged
> via arm-soc so that you can pick this after that. Alternatively we
> can push it via arm-soc but I wouldn't rush for that as it's not that
> urgent. Is that fine with you ?
> 
> Regards,
> Sudeep
> 
> --
> 1.9.1
> 
> diff --git a/drivers/hwmon/scpi-hwmon.c b/drivers/hwmon/scpi-hwmon.c
> index 2c1241bbf9af..7e20567bc369 100644
> --- a/drivers/hwmon/scpi-hwmon.c
> +++ b/drivers/hwmon/scpi-hwmon.c
> @@ -117,7 +117,7 @@ static int scpi_hwmon_probe(struct platform_device *pdev)
>  	struct scpi_ops *scpi_ops;
>  	struct device *hwdev, *dev = &pdev->dev;
>  	struct scpi_sensors *scpi_sensors;
> -	int ret;
> +	int ret, idx;
> 
>  	scpi_ops = get_scpi_ops();
>  	if (!scpi_ops)
> @@ -146,8 +146,8 @@ static int scpi_hwmon_probe(struct platform_device *pdev)
> 
>  	scpi_sensors->scpi_ops = scpi_ops;
> 
> -	for (i = 0; i < nr_sensors; i++) {
> -		struct sensor_data *sensor = &scpi_sensors->data[i];
> +	for (i = 0, idx = 0; i < nr_sensors; i++) {
> +		struct sensor_data *sensor = &scpi_sensors->data[idx];
> 
>  		ret = scpi_ops->sensor_get_info(i, &sensor->info);
>  		if (ret)
> @@ -183,7 +183,7 @@ static int scpi_hwmon_probe(struct platform_device *pdev)
>  			num_power++;
>  			break;
>  		default:
> -			break;
> +			continue;
>  		}
> 
>  		sensor->dev_attr_input.attr.mode = S_IRUGO;
> @@ -194,11 +194,12 @@ static int scpi_hwmon_probe(struct platform_device *pdev)
>  		sensor->dev_attr_label.show = scpi_show_label;
>  		sensor->dev_attr_label.attr.name = sensor->label;
> 
> -		scpi_sensors->attrs[i << 1] = &sensor->dev_attr_input.attr;
> -		scpi_sensors->attrs[(i << 1) + 1] = &sensor->dev_attr_label.attr;
> +		scpi_sensors->attrs[idx << 1] = &sensor->dev_attr_input.attr;
> +		scpi_sensors->attrs[(idx << 1) + 1] = &sensor->dev_attr_label.attr;
> 
> -		sysfs_attr_init(scpi_sensors->attrs[i << 1]);
> -		sysfs_attr_init(scpi_sensors->attrs[(i << 1) + 1]);
> +		sysfs_attr_init(scpi_sensors->attrs[idx << 1]);
> +		sysfs_attr_init(scpi_sensors->attrs[(idx << 1) + 1]);
> +		idx++;
>  	}
> 
>  	scpi_sensors->group.attrs = scpi_sensors->attrs;
> @@ -236,8 +237,8 @@ static int scpi_hwmon_probe(struct platform_device *pdev)
> 
>  		zone->sensor_id = i;
>  		zone->scpi_sensors = scpi_sensors;
> -		zone->tzd = thermal_zone_of_sensor_register(dev, i, zone,
> -							    &scpi_sensor_ops);
> +		zone->tzd = thermal_zone_of_sensor_register(dev,
> +				sensor->info.sensor_id, zone, &scpi_sensor_ops);
>  		/*
>  		 * The call to thermal_zone_of_sensor_register returns
>  		 * an error for sensors that are not associated with

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] hwmon: (scpi) skip unsupported sensors properly
  2015-10-29  4:59   ` Guenter Roeck
@ 2015-11-16 18:00     ` Punit Agrawal
  0 siblings, 0 replies; 8+ messages in thread
From: Punit Agrawal @ 2015-11-16 18:00 UTC (permalink / raw)
  To: Guenter Roeck; +Cc: Sudeep Holla, linux-kernel, lm-sensors, Jean Delvare

Hi Guenter,

Guenter Roeck <linux@roeck-us.net> writes:

> On Wed, Oct 28, 2015 at 05:17:31PM +0000, Sudeep Holla wrote:
>> Currently it's assumed that firmware exports only the class of sensors
>> supported by the driver. However with newer firmware or SCPI protocol
>> revision, support for newer classes of sensors can be present.
>> 
>> The driver fails to probe with the following warning if an unsupported
>> class of sensor is encountered in the firmware.
>> 
>> sysfs: cannot create duplicate filename
>> 	'/devices/platform/scpi/scpi:sensors/hwmon/hwmon0/'
>> ------------[ cut here ]------------
>> WARNING: at fs/sysfs/dir.c:31
>> Modules linked in:
>> 
>> CPU: 0 PID: 6 Comm: kworker/u12:0 Not tainted 4.3.0-rc7 #137
>> Hardware name: ARM Juno development board (r0) (DT)
>> Workqueue: deferwq deferred_probe_work_func
>> PC is at sysfs_warn_dup+0x54/0x78
>> LR is at sysfs_warn_dup+0x54/0x78
>> 
>> This patch fixes the above issue by skipping through the unsupported
>> class of SCPI sensors.
>> 
>> Fixes: 68acc77a2d51 ("hwmon: Support thermal zones registration for SCP temperature sensors")
>> Fixes: ea98b29a05e9 ("hwmon: Support sensors exported via ARM SCP interface")
>> Cc: Guenter Roeck <linux@roeck-us.net>
>> Reviewed-by: Punit Agrawal <punit.agrawal@arm.com>
>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>
> Acked-by: Guenter Roeck <linux@roeck-us.net>
>
>> ---
>>  drivers/hwmon/scpi-hwmon.c | 21 +++++++++++----------
>>  1 file changed, 11 insertions(+), 10 deletions(-)
>> 
>> Hi Guenter,
>> 
>> Either me/Punit will need ping you once the original driver is merged
>> via arm-soc so that you can pick this after that. Alternatively we
>> can push it via arm-soc but I wouldn't rush for that as it's not that
>> urgent. Is that fine with you ?
>> 
> Either way is fine with me.

Now that rc1 is tagged, could you please pick this fix? It makes the
driver more robust when faced with changes to the firmware interface.

>
> Guenter
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [v2] hwmon: (scpi) skip unsupported sensors properly
  2015-11-16 17:59   ` [v2] " Guenter Roeck
@ 2015-11-16 19:37     ` Punit Agrawal
  0 siblings, 0 replies; 8+ messages in thread
From: Punit Agrawal @ 2015-11-16 19:37 UTC (permalink / raw)
  To: Guenter Roeck; +Cc: Sudeep Holla, linux-kernel, Jean Delvare, lm-sensors

Guenter Roeck <linux@roeck-us.net> writes:

> On Wed, Oct 28, 2015 at 05:17:31PM +0000, Sudeep Holla wrote:
>> Currently it's assumed that firmware exports only the class of sensors
>> supported by the driver. However with newer firmware or SCPI protocol
>> revision, support for newer classes of sensors can be present.
>> 
>> The driver fails to probe with the following warning if an unsupported
>> class of sensor is encountered in the firmware.
>> 
>> sysfs: cannot create duplicate filename
>> 	'/devices/platform/scpi/scpi:sensors/hwmon/hwmon0/'
>> ------------[ cut here ]------------
>> WARNING: at fs/sysfs/dir.c:31
>> Modules linked in:
>> 
>> CPU: 0 PID: 6 Comm: kworker/u12:0 Not tainted 4.3.0-rc7 #137
>> Hardware name: ARM Juno development board (r0) (DT)
>> Workqueue: deferwq deferred_probe_work_func
>> PC is at sysfs_warn_dup+0x54/0x78
>> LR is at sysfs_warn_dup+0x54/0x78
>> 
>> This patch fixes the above issue by skipping through the unsupported
>> class of SCPI sensors.
>> 
>> Fixes: 68acc77a2d51 ("hwmon: Support thermal zones registration for SCP temperature sensors")
>> Fixes: ea98b29a05e9 ("hwmon: Support sensors exported via ARM SCP interface")
>> Cc: Guenter Roeck <linux@roeck-us.net>
>> Reviewed-by: Punit Agrawal <punit.agrawal@arm.com>
>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>> Acked-by: Guenter Roeck <linux@roeck-us.net>
>
> In the assumption that this patch can now be applied, I queued it up
> for the next -rc.

Looks like our emails crossed on the wire. :)

Thanks for applying the patch.

Punit

>
> Guenter
>
>> ---
>>  drivers/hwmon/scpi-hwmon.c | 21 +++++++++++----------
>>  1 file changed, 11 insertions(+), 10 deletions(-)
>> 

[...]


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2015-11-16 19:38 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-28 14:08 [PATCH] hwmon: (scpi) skip unsupported sensors properly Sudeep Holla
2015-10-28 15:39 ` Punit Agrawal
2015-10-28 15:58   ` Sudeep Holla
2015-10-28 17:17 ` [PATCH v2] " Sudeep Holla
2015-10-29  4:59   ` Guenter Roeck
2015-11-16 18:00     ` Punit Agrawal
2015-11-16 17:59   ` [v2] " Guenter Roeck
2015-11-16 19:37     ` Punit Agrawal

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).