Re: [PATCH 21/25] thermal/drivers/hisi: Prepare to add support for other hisi platforms

[Eduardo Valentin <edubezval@gmail.com>]

On Tue, Oct 10, 2017 at 08:02:46PM +0200, Daniel Lezcano wrote:
> From: Kevin Wangtao <kevin.wangtao@linaro.org>
> 
> For platform compatibility, add the tsensor ops to a thermal data structure.
> Each platform has its own probe function to register proper tsensor ops
> function to the pointer, platform related resource request are also implemented
> in the platform probe function.

Please fix minor check patch issues like:
CHECK: Please don't use multiple blank lines
#142: FILE: drivers/thermal/hisi_thermal.c:67:
 
 +

 CHECK: Please don't use multiple blank lines
 #218: FILE: drivers/thermal/hisi_thermal.c:297:
 +
 +

 CHECK: Alignment should match open parenthesis
 #335: FILE: drivers/thermal/hisi_thermal.c:424:
 +		ret = devm_request_threaded_irq(dev, data->irq, NULL,
 +				hisi_thermal_alarm_irq_thread,



> 
> Signed-off-by: Kevin Wangtao <kevin.wangtao@linaro.org>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---
>  drivers/thermal/hisi_thermal.c | 135 +++++++++++++++++++++++++++--------------
>  1 file changed, 89 insertions(+), 46 deletions(-)
> 
> diff --git a/drivers/thermal/hisi_thermal.c b/drivers/thermal/hisi_thermal.c
> index 8a70ab7..b5a7159 100644
> --- a/drivers/thermal/hisi_thermal.c
> +++ b/drivers/thermal/hisi_thermal.c
> @@ -23,6 +23,7 @@
>  #include <linux/module.h>
>  #include <linux/platform_device.h>
>  #include <linux/io.h>
> +#include <linux/of_device.h>
>  
>  #include "thermal_core.h"
>  
> @@ -30,7 +31,7 @@
>  #define HI6220_TEMP0_TH				(0x4)
>  #define HI6220_TEMP0_RST_TH			(0x8)
>  #define HI6220_TEMP0_CFG			(0xC)
> -#define HI6220_TEMP0_CFG_SS_MSK		(0xF000)
> +#define HI6220_TEMP0_CFG_SS_MSK			(0xF000)
>  #define HI6220_TEMP0_CFG_HDAK_MSK		(0x30)
>  #define HI6220_TEMP0_EN				(0x10)
>  #define HI6220_TEMP0_INT_EN			(0x14)
> @@ -41,7 +42,7 @@
>  #define HI6220_TEMP_BASE			(-60000)
>  #define HI6220_TEMP_RESET			(100000)
>  #define HI6220_TEMP_STEP			(785)
> -#define HI6220_TEMP_LAG			(3500)
> +#define HI6220_TEMP_LAG				(3500)
>  
>  #define HI6220_DEFAULT_SENSOR		2
>  
> @@ -52,6 +53,10 @@ struct hisi_thermal_sensor {
>  };
>  
>  struct hisi_thermal_data {
> +	int (*get_temp)(struct hisi_thermal_data *data);
> +	int (*enable_sensor)(struct hisi_thermal_data *data);
> +	int (*disable_sensor)(struct hisi_thermal_data *data);
> +	int (*irq_handler)(struct hisi_thermal_data *data);
>  	struct platform_device *pdev;
>  	struct clk *clk;
>  	struct hisi_thermal_sensor sensor;
> @@ -59,6 +64,7 @@ struct hisi_thermal_data {
>  	int irq;
>  };
>  
> +
>  /*
>   * The temperature computation on the tsensor is as follow:
>   *	Unit: millidegree Celsius
> @@ -192,7 +198,18 @@ static inline void hi6220_thermal_hdak_set(void __iomem *addr, int value)
>  	       (value << 4), addr + HI6220_TEMP0_CFG);
>  }
>  
> -static void hi6220_thermal_disable_sensor(struct hisi_thermal_data *data)
> +static int hi6220_thermal_irq_handler(struct hisi_thermal_data *data)
> +{
> +	hi6220_thermal_alarm_clear(data->regs, 1);
> +	return 0;
> +}
> +
> +static int hi6220_thermal_get_temp(struct hisi_thermal_data *data)
> +{
> +	return hi6220_thermal_get_temperature(data->regs);
> +}
> +
> +static int hi6220_thermal_disable_sensor(struct hisi_thermal_data *data)
>  {
>  	/* disable sensor module */
>  	hi6220_thermal_enable(data->regs, 0);
> @@ -200,9 +217,9 @@ static void hi6220_thermal_disable_sensor(struct hisi_thermal_data *data)
>  	hi6220_thermal_reset_enable(data->regs, 0);
>  
>  	clk_disable_unprepare(data->clk);
> +	return 0;
>  }
>  
> -
>  static int hi6220_thermal_enable_sensor(struct hisi_thermal_data *data)
>  {
>  	struct hisi_thermal_sensor *sensor = &data->sensor;
> @@ -240,12 +257,50 @@ static int hi6220_thermal_enable_sensor(struct hisi_thermal_data *data)
>  
>  	return 0;
>  }
> +
> +static int hi6220_thermal_probe(struct hisi_thermal_data *data)
> +{
> +	struct platform_device *pdev = data->pdev;
> +	struct device *dev = &pdev->dev;
> +	struct resource *res;
> +	int ret;
> +
> +	data->get_temp = hi6220_thermal_get_temp;
> +	data->enable_sensor = hi6220_thermal_enable_sensor;
> +	data->disable_sensor = hi6220_thermal_disable_sensor;
> +	data->irq_handler = hi6220_thermal_irq_handler;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	data->regs = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(data->regs)) {
> +		dev_err(dev, "failed to get io address\n");
> +		return PTR_ERR(data->regs);
> +	}
> +
> +	data->clk = devm_clk_get(dev, "thermal_clk");
> +	if (IS_ERR(data->clk)) {
> +		ret = PTR_ERR(data->clk);
> +		if (ret != -EPROBE_DEFER)
> +			dev_err(dev, "failed to get thermal clk: %d\n", ret);
> +		return ret;
> +	}
> +
> +	data->irq = platform_get_irq(pdev, 0);
> +	if (data->irq < 0)
> +		return data->irq;
> +
> +	data->sensor.id = HI6220_DEFAULT_SENSOR;
> +
> +	return 0;
> +}
> +
> +
>  static int hisi_thermal_get_temp(void *__data, int *temp)
>  {
>  	struct hisi_thermal_data *data = __data;
>  	struct hisi_thermal_sensor *sensor = &data->sensor;
>  
> -	*temp = hi6220_thermal_get_temperature(data->regs);
> +	*temp = data->get_temp(data);
>  
>  	dev_dbg(&data->pdev->dev, "id=%d, temp=%d, thres=%d\n",
>  		sensor->id, *temp, sensor->thres_temp);
> @@ -263,7 +318,7 @@ static irqreturn_t hisi_thermal_alarm_irq_thread(int irq, void *dev)
>  	struct hisi_thermal_sensor *sensor = &data->sensor;
>  	int temp = 0;
>  
> -	hi6220_thermal_alarm_clear(data->regs, 1);
> +	data->irq_handler(data);
>  
>  	hisi_thermal_get_temp(data, &temp);
>  
> @@ -284,14 +339,11 @@ static irqreturn_t hisi_thermal_alarm_irq_thread(int irq, void *dev)
>  
>  static int hisi_thermal_register_sensor(struct platform_device *pdev,
>  					struct hisi_thermal_data *data,
> -					struct hisi_thermal_sensor *sensor,
> -					int index)
> +					struct hisi_thermal_sensor *sensor)
>  {
>  	int ret, i;
>  	const struct thermal_trip *trip;
>  
> -	sensor->id = index;
> -
>  	sensor->tzd = devm_thermal_zone_of_sensor_register(&pdev->dev,
>  							   sensor->id, data,
>  							   &hisi_of_thermal_ops);
> @@ -316,7 +368,7 @@ static int hisi_thermal_register_sensor(struct platform_device *pdev,
>  }
>  
>  static const struct of_device_id of_hisi_thermal_match[] = {
> -	{ .compatible = "hisilicon,tsensor" },
> +	{ .compatible = "hisilicon,tsensor", .data = hi6220_thermal_probe },
>  	{ /* end */ }
>  };
>  MODULE_DEVICE_TABLE(of, of_hisi_thermal_match);
> @@ -333,58 +385,48 @@ static void hisi_thermal_toggle_sensor(struct hisi_thermal_sensor *sensor,
>  static int hisi_thermal_probe(struct platform_device *pdev)
>  {
>  	struct hisi_thermal_data *data;
> -	struct resource *res;
> +	int (*platform_probe)(struct hisi_thermal_data *);
> +	struct device *dev = &pdev->dev;
>  	int ret;
>  
> -	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
> +	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
>  	if (!data)
>  		return -ENOMEM;
>  
>  	data->pdev = pdev;
> +	platform_set_drvdata(pdev, data);
>  
> -	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -	data->regs = devm_ioremap_resource(&pdev->dev, res);
> -	if (IS_ERR(data->regs)) {
> -		dev_err(&pdev->dev, "failed to get io address\n");
> -		return PTR_ERR(data->regs);
> +	platform_probe = of_device_get_match_data(dev);

I get the compilation warn below:

  CHECK   drivers/thermal/hisi_thermal.c
drivers/thermal/hisi_thermal.c:399:24: warning: incorrect type in assignment (different modifiers)
drivers/thermal/hisi_thermal.c:399:24:    expected int ( *platform_probe )( ... )
drivers/thermal/hisi_thermal.c:399:24:    got void const *
  CC [M]  drivers/thermal/hisi_thermal.o

Please fix it.




> +	if (!platform_probe) {
> +		dev_err(dev, "failed to get probe func\n");
> +		return -EINVAL;
>  	}
>  
> -	data->irq = platform_get_irq(pdev, 0);
> -	if (data->irq < 0)
> -		return data->irq;
> -
> -	platform_set_drvdata(pdev, data);
> -
> -	data->clk = devm_clk_get(&pdev->dev, "thermal_clk");
> -	if (IS_ERR(data->clk)) {
> -		ret = PTR_ERR(data->clk);
> -		if (ret != -EPROBE_DEFER)
> -			dev_err(&pdev->dev,
> -				"failed to get thermal clk: %d\n", ret);
> +	ret = platform_probe(data);
> +	if (ret)
>  		return ret;
> -	}
>  
>  	ret = hisi_thermal_register_sensor(pdev, data,
> -					   &data->sensor,
> -					   HI6220_DEFAULT_SENSOR);
> +					   &data->sensor);
>  	if (ret) {
> -		dev_err(&pdev->dev, "failed to register thermal sensor: %d\n",
> -			ret);
> +		dev_err(dev, "failed to register thermal sensor: %d\n", ret);
>  		return ret;
>  	}
>  
> -	ret = hi6220_thermal_enable_sensor(data);
> +	ret = data->enable_sensor(data);
>  	if (ret) {
> -		dev_err(&pdev->dev, "Failed to setup the sensor: %d\n", ret);
> +		dev_err(dev, "Failed to setup the sensor: %d\n", ret);
>  		return ret;
>  	}
>  
> -	ret = devm_request_threaded_irq(&pdev->dev, data->irq, NULL,
> -					hisi_thermal_alarm_irq_thread,
> -					IRQF_ONESHOT, "hisi_thermal", data);
> -	if (ret < 0) {
> -		dev_err(&pdev->dev, "failed to request alarm irq: %d\n", ret);
> -		return ret;
> +	if (data->irq) {
> +		ret = devm_request_threaded_irq(dev, data->irq, NULL,
> +				hisi_thermal_alarm_irq_thread,
> +				IRQF_ONESHOT, "hisi_thermal", data);
> +		if (ret < 0) {
> +			dev_err(dev, "failed to request alarm irq: %d\n", ret);
> +			return ret;
> +		}
>  	}
>  
>  	hisi_thermal_toggle_sensor(&data->sensor, true);
> @@ -398,7 +440,8 @@ static int hisi_thermal_remove(struct platform_device *pdev)
>  	struct hisi_thermal_sensor *sensor = &data->sensor;
>  
>  	hisi_thermal_toggle_sensor(sensor, false);
> -	hi6220_thermal_disable_sensor(data);
> +
> +	data->disable_sensor(data);
>  
>  	return 0;
>  }
> @@ -408,7 +451,7 @@ static int hisi_thermal_suspend(struct device *dev)
>  {
>  	struct hisi_thermal_data *data = dev_get_drvdata(dev);
>  
> -	hi6220_thermal_disable_sensor(data);
> +	data->disable_sensor(data);
>  
>  	return 0;
>  }
> @@ -417,7 +460,7 @@ static int hisi_thermal_resume(struct device *dev)
>  {
>  	struct hisi_thermal_data *data = dev_get_drvdata(dev);
>  
> -	return hi6220_thermal_enable_sensor(data);
> +	return data->enable_sensor(data);
>  }
>  #endif
>  
> -- 
> 2.7.4
>