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


Groups > linux.kernel > #1725117

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

From Leo Yan <leo.yan@linaro.org>
Newsgroups linux.kernel
Subject Re: [PATCH 02/13] thermal/drivers/hisi: Remove the multiple sensors support
Date 2017-09-01 16:10 +0200
Message-ID <ukUNX-68B-3@gated-at.bofh.it> (permalink)
References <uk6Rb-63h-5@gated-at.bofh.it> <uk6Rd-63h-31@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


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

>  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.

>  	void __iomem *regs;
>  };
>  
> @@ -113,7 +113,7 @@ static void hisi_thermal_enable_bind_irq_sensor
>  
>  	mutex_lock(&data->thermal_lock);
>  
> -	sensor = &data->sensors[data->irq_bind_sensor];
> +	sensor = &data->sensors;
>  
>  	/* setting the hdak time */
>  	writel(0x0, data->regs + TEMP0_CFG);
> @@ -160,31 +160,8 @@ static int hisi_thermal_get_temp(void *_sensor, int *temp)
>  	struct hisi_thermal_sensor *sensor = _sensor;
>  	struct hisi_thermal_data *data = sensor->thermal;
>  
> -	int sensor_id = -1, i;
> -	long max_temp = 0;
> -
>  	*temp = hisi_thermal_get_sensor_temp(data, sensor);
>  
> -	sensor->sensor_temp = *temp;
> -
> -	for (i = 0; i < HISI_MAX_SENSORS; i++) {
> -		if (!data->sensors[i].tzd)
> -			continue;
> -
> -		if (data->sensors[i].sensor_temp >= max_temp) {
> -			max_temp = data->sensors[i].sensor_temp;
> -			sensor_id = i;
> -		}
> -	}
> -
> -	/* If no sensor has been enabled, then skip to enable irq */
> -	if (sensor_id == -1)
> -		return 0;
> -
> -	mutex_lock(&data->thermal_lock);
> -	data->irq_bind_sensor = sensor_id;
> -	mutex_unlock(&data->thermal_lock);
> -
>  	dev_dbg(&data->pdev->dev, "id=%d, irq=%d, temp=%d, thres=%d\n",
>  		sensor->id, data->irq_enabled, *temp, sensor->thres_temp);
>  	/*
> @@ -197,7 +174,7 @@ static int hisi_thermal_get_temp(void *_sensor, int *temp)
>  		return 0;
>  	}
>  
> -	if (max_temp < sensor->thres_temp) {
> +	if (*temp < sensor->thres_temp) {
>  		data->irq_enabled = true;
>  		hisi_thermal_enable_bind_irq_sensor(data);
>  		enable_irq(data->irq);
> @@ -224,22 +201,16 @@ static irqreturn_t hisi_thermal_alarm_irq_thread(int irq, void *dev)
>  {
>  	struct hisi_thermal_data *data = dev;
>  	struct hisi_thermal_sensor *sensor;
> -	int i;
>  
>  	mutex_lock(&data->thermal_lock);
> -	sensor = &data->sensors[data->irq_bind_sensor];
> +	sensor = &data->sensors;
>  
>  	dev_crit(&data->pdev->dev, "THERMAL ALARM: T > %d\n",
>  		 sensor->thres_temp / 1000);
>  	mutex_unlock(&data->thermal_lock);
>  
> -	for (i = 0; i < HISI_MAX_SENSORS; i++) {
> -		if (!data->sensors[i].tzd)
> -			continue;
> -
> -		thermal_zone_device_update(data->sensors[i].tzd,
> -					   THERMAL_EVENT_UNSPECIFIED);
> -	}
> +	thermal_zone_device_update(data->sensors.tzd,
> +				   THERMAL_EVENT_UNSPECIFIED);
>  
>  	return IRQ_HANDLED;
>  }
> @@ -296,7 +267,6 @@ static int hisi_thermal_probe(struct platform_device *pdev)
>  {
>  	struct hisi_thermal_data *data;
>  	struct resource *res;
> -	int i;
>  	int ret;
>  
>  	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
> @@ -347,16 +317,17 @@ static int hisi_thermal_probe(struct platform_device *pdev)
>  	hisi_thermal_enable_bind_irq_sensor(data);
>  	data->irq_enabled = true;
>  
> -	for (i = 0; i < HISI_MAX_SENSORS; ++i) {
> -		ret = hisi_thermal_register_sensor(pdev, data,
> -						   &data->sensors[i], i);
> -		if (ret)
> -			dev_err(&pdev->dev,
> -				"failed to register thermal sensor: %d\n", ret);
> -		else
> -			hisi_thermal_toggle_sensor(&data->sensors[i], true);
> +	ret = hisi_thermal_register_sensor(pdev, data,
> +					   &data->sensors,
> +					   HISI_DEFAULT_SENSOR);
> +	if (ret) {
> +		dev_err(&pdev->dev, "failed to register thermal sensor: %d\n",
> +			ret);
> +		return ret;
>  	}
>  
> +	hisi_thermal_toggle_sensor(&data->sensors, true);
> +
>  	enable_irq(data->irq);
>  
>  	return 0;
> @@ -365,17 +336,9 @@ static int hisi_thermal_probe(struct platform_device *pdev)
>  static int hisi_thermal_remove(struct platform_device *pdev)
>  {
>  	struct hisi_thermal_data *data = platform_get_drvdata(pdev);
> -	int i;
> -
> -	for (i = 0; i < HISI_MAX_SENSORS; i++) {
> -		struct hisi_thermal_sensor *sensor = &data->sensors[i];
> -
> -		if (!sensor->tzd)
> -			continue;
> -
> -		hisi_thermal_toggle_sensor(sensor, false);
> -	}
> +	struct hisi_thermal_sensor *sensor = &data->sensors;
>  
> +	hisi_thermal_toggle_sensor(sensor, false);
>  	hisi_thermal_disable_sensor(data);
>  	clk_disable_unprepare(data->clk);
>  
> -- 
> 2.7.4
> 

Back to linux.kernel | Previous | NextPrevious 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