Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1725340

Re: [PATCH 02/13] thermal/drivers/hisi: Remove the multiple sensors support

Show all headers | View raw

On 01/09/2017 16:05, Leo Yan wrote:
> Hi Daniel,
> 
> On Wed, Aug 30, 2017 at 10:47:26AM +0200, Daniel Lezcano wrote:
>> By essence, the tsensor does not really support multiple sensor at the same
>> time. It allows to set a sensor and use it to get the temperature, another
>> sensor could be switched but with a delay of 3-5ms. It is difficult to read
>> simultaneously several sensors without a big delay.
>>
>> Today, just one sensor is used, it is not necessary to deal with multiple
>> sensors in the code. Remove them and if it is needed in the future add them
>> on top of a code which will be clean up in the meantime.
>>
>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>> ---
>>  drivers/thermal/hisi_thermal.c | 77 +++++++++++-------------------------------
>>  1 file changed, 20 insertions(+), 57 deletions(-)
>>
>> diff --git a/drivers/thermal/hisi_thermal.c b/drivers/thermal/hisi_thermal.c
>> index 8381696..92b6889 100644
>> --- a/drivers/thermal/hisi_thermal.c
>> +++ b/drivers/thermal/hisi_thermal.c
>> @@ -39,6 +39,7 @@
>>  #define HISI_TEMP_RESET			(100000)
>>  
>>  #define HISI_MAX_SENSORS		4
>> +#define HISI_DEFAULT_SENSOR		2
> 
> So if we always set the sensor id 2 as default sensor, that means it's
> pointless to bind sensor id in dts. E.g. I change to bind thermal
> sensor 0, then it's impossible to bind successfully:
> 
> -                               thermal-sensors = <&tsensor 2>;
> +                               thermal-sensors = <&tsensor 0>;
> 
> It's okay for this change, do you think we need reflect this in DT
> binding doc [1] ?
> 
> [1] Documentation/devicetree/bindings/thermal/hisilicon-thermal.txt

Actually, the current behavior is to go through all the sensors and
register them. By side effect, we end up by registering a sensor with an
id matching the one specified in the DT, others are failing.

That is because there is something missing in the of-thermal API where
we can't call thermal_zone_of_sensor_register() without a matching index.

That is probably why we have in the drivers/thermal/qoriq_thermal.c file
the qoriq_tmu_get_sensor_id() which inspect the thermal-zone DT
structure for the thermal-sensors and read the sensor's id from there in
order to reuse it for thermal_zone_of_sensor_register(). Needless to say
that is not the right place to do that.

IMHO, as nothing gets changed for the moment on hi6220, we can live with
HISI_DEFAULT_SENSOR until we sort out what to do with
thermal_zone_of_sensor_register().


>>  struct hisi_thermal_sensor {
>>  	struct hisi_thermal_data *thermal;
>> @@ -53,11 +54,10 @@ struct hisi_thermal_data {
>>  	struct mutex thermal_lock;    /* protects register data */
>>  	struct platform_device *pdev;
>>  	struct clk *clk;
>> -	struct hisi_thermal_sensor sensors[HISI_MAX_SENSORS];
>> -
>> -	int irq, irq_bind_sensor;
>> +	struct hisi_thermal_sensor sensors;
>> +	int irq;
>>  	bool irq_enabled;
>> -
>> +	
> 
> Repots whitespace error when I apply patch.

Ok, I will fix that.

[ ... ]

Thanks for the review.

  -- Daniel

-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH 01/13] thermal/drivers/hisi: Fix missing interrupt enablement Daniel Lezcano <daniel.lezcano@linaro.org> - 2017-08-30 10:50 +0200
  [PATCH 02/13] thermal/drivers/hisi: Remove the multiple sensors support Daniel Lezcano <daniel.lezcano@linaro.org> - 2017-08-30 10:50 +0200
    Re: [PATCH 02/13] thermal/drivers/hisi: Remove the multiple sensors  support Leo Yan <leo.yan@linaro.org> - 2017-09-01 16:10 +0200
      Re: [PATCH 02/13] thermal/drivers/hisi: Remove the multiple sensors  support Daniel Lezcano <daniel.lezcano@linaro.org> - 2017-09-01 22:50 +0200
  [PATCH 06/13] thermal/drivers/hisi: Remove pointless lock Daniel Lezcano <daniel.lezcano@linaro.org> - 2017-08-30 11:00 +0200
    Re: [PATCH 06/13] thermal/drivers/hisi: Remove pointless lock Leo Yan <leo.yan@linaro.org> - 2017-09-01 16:50 +0200
  [PATCH 03/13] thermal/drivers/hisi: Fix kernel panic on alarm interrupt Daniel Lezcano <daniel.lezcano@linaro.org> - 2017-08-30 11:00 +0200
    Re: [PATCH 03/13] thermal/drivers/hisi: Fix kernel panic on alarm  interrupt Leo Yan <leo.yan@linaro.org> - 2017-09-01 16:20 +0200
  [PATCH 08/13] thermal/drivers/hisi: Fix configuration register setting Daniel Lezcano <daniel.lezcano@linaro.org> - 2017-08-30 11:00 +0200
    Re: [PATCH 08/13] thermal/drivers/hisi: Fix configuration register  setting Leo Yan <leo.yan@linaro.org> - 2017-09-02 05:00 +0200
      Re: [PATCH 08/13] thermal/drivers/hisi: Fix configuration register  setting Daniel Lezcano <daniel.lezcano@linaro.org> - 2017-09-02 10:40 +0200
        Re: [PATCH 08/13] thermal/drivers/hisi: Fix configuration register  setting Leo Yan <leo.yan@linaro.org> - 2017-09-04 03:00 +0200
          Re: [PATCH 08/13] thermal/drivers/hisi: Fix configuration register  setting Daniel Lezcano <daniel.lezcano@linaro.org> - 2017-09-04 11:20 +0200
  [PATCH 09/13] thermal/drivers/hisi: Remove costly sensor inspection Daniel Lezcano <daniel.lezcano@linaro.org> - 2017-08-30 11:00 +0200
    Re: [PATCH 09/13] thermal/drivers/hisi: Remove costly sensor  inspection Leo Yan <leo.yan@linaro.org> - 2017-09-02 05:30 +0200
      Re: [PATCH 09/13] thermal/drivers/hisi: Remove costly sensor  inspection Daniel Lezcano <daniel.lezcano@linaro.org> - 2017-09-02 15:20 +0200
        Re: [PATCH 09/13] thermal/drivers/hisi: Remove costly sensor  inspection Leo Yan <leo.yan@linaro.org> - 2017-09-04 03:00 +0200
          Re: [PATCH 09/13] thermal/drivers/hisi: Remove costly sensor  inspection Daniel Lezcano <daniel.lezcano@linaro.org> - 2017-09-04 13:30 +0200
            Re: [PATCH 09/13] thermal/drivers/hisi: Remove costly sensor  inspection Leo Yan <leo.yan@linaro.org> - 2017-09-04 16:40 +0200
  [PATCH 05/13] thermal/drivers/hisi: Fix multiple alarm interrupts firing Daniel Lezcano <daniel.lezcano@linaro.org> - 2017-08-30 11:00 +0200
    Re: [PATCH 05/13] thermal/drivers/hisi: Fix multiple alarm  interrupts firing Leo Yan <leo.yan@linaro.org> - 2017-09-01 16:50 +0200
  Re: [PATCH 01/13] thermal/drivers/hisi: Fix missing interrupt  enablement Leo Yan <leo.yan@linaro.org> - 2017-09-01 10:40 +0200

csiph-web