Re: [PATCH RESEND v4 2/2] thermal: hisilicon: add new hisilicon thermal sensor driver

[Bintian <bintian.wang@huawei.com>]

Hello Xinwei, Eduardo,

On 2015/5/18 12:03, Xinwei Kong wrote:
> hi Eduardo,
>
> this following review log is from our min systerm patchset. this thermal
> sensor driver is important part for 96boards. If our thermal sensor can't
> be merged into kernel and the min systerm be merged. the board is not safe
> for using man (causing fire).
Not so serious,  I explained this problem in another email.

> So i wish you can accept this patchset before
> the min systerm patchset is accepted.
Xinwei, please let maintainer judge the quality of your patch and decide
to merge or not.

Thanks,

Bintian
>
> thanks
> Xinwei
>
>
> On 2015/5/7 17:02, Will Deacon wrote:> Hi Bintian,
>>
>> On Tue, May 05, 2015 at 01:06:34PM +0100, Bintian Wang wrote:
>>> PSCI is enabled in device tree and there is no problem to boot all the
>>> octal cores, and the CPU hotplug is also working now, you can download
>>> and compile the latest firmware based on the following link to run this
>>> patch set:
>>> https://github.com/96boards/documentation/wiki/UEFI
>>>
>>> Changes v4:
>>> * Rebase to kernel 4.1-rc1
>>> * Delete "arm,cortex-a15-gic" from the gic node in dts
>>
>> I gave these patches a go on top of -rc2 using the ATF and UEFI you link to
>> above.
>>
>> The good news is that the thing booted and all the cores entered at EL2.
>> Thanks!
>>
>> The bad news is that running hackbench quickly got the *heatsink*
>> temperature to 73 degress C and rising (measured with an infrared
>> thermometer).
>>
>> So my question is, does this SoC have an automatic thermal cut out? Whilst
>> I'm all for merging enabling code into the kernel, if it really relies on
>> the kernel to stop it from catching fire, maybe it's not a great idea
>> putting these patches into people's hands just yet.
>>
>> Will
>>
>> .
>>
>
> On 2015/5/12 9:29, Eduardo Valentin wrote:
>> Dear Kongxinwei,
>>
>>
>> Apologize for late answer. I really missed this one. Let's make it for
>> the next merge window!
>>
>> Please find a couple of comments as follows.
>>
>>
>> On Mon, May 04, 2015 at 10:52:20AM +0800, Xinwei Kong wrote:
>>> From: kongxinwei <kong.kongxinwei@hisilicon.com>
>>>
>>> This patch adds the support for hisilicon thermal sensor, within
>>> hisilicon SoC. there will register sensors for thermal framework
>>> and use device tree to bind cooling device.
>>>
>>> Signed-off-by: Leo Yan <leo.yan@linaro.org>
>>> Signed-off-by: kongxinwei <kong.kongxinwei@hisilicon.com>
>>> ---
>>>   drivers/thermal/Kconfig        |   8 +
>>>   drivers/thermal/Makefile       |   1 +
>>>   drivers/thermal/hisi_thermal.c | 437 +++++++++++++++++++++++++++++++++++++++++
>>>   3 files changed, 446 insertions(+)
>>>   create mode 100644 drivers/thermal/hisi_thermal.c
>>
>>
>> Please, make sure you have a clean ./checkpatch.pl --strict output.
>> Today, in this version, I see the following summary:
>>
>> # ./scripts/checkpatch.pl --strict /tmp/a
>> total: 0 errors, 2 warnings, 7 checks, 455 lines checked
>>
>> /tmp/a has style problems, please review.
>>
>> If any of these errors are false positives, please report
>> them to the maintainer, see CHECKPATCH in MAINTAINERS.
>>
>> Can you please clean them up?
>>
>>>
>>> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
>>> index af40db0..81aee01 100644
>>> --- a/drivers/thermal/Kconfig
>>> +++ b/drivers/thermal/Kconfig
>>> @@ -136,6 +136,14 @@ config THERMAL_EMULATION
>>>   	  because userland can easily disable the thermal policy by simply
>>>   	  flooding this sysfs node with low temperature values.
>>>
>>> +config HISI_THERMAL
>>> +	tristate "Hisilicon thermal driver"
>>> +	depends on ARCH_HISI && CPU_THERMAL && OF
>>> +	help
>>> +	  Enable this to plug hisilicon's thermal sensor driver into the Linux
>>> +	  thermal framework. cpufreq is used as the cooling device to throttle
>>> +	  CPUs when the passive trip is crossed.
>>> +
>>>   config IMX_THERMAL
>>>   	tristate "Temperature sensor driver for Freescale i.MX SoCs"
>>>   	depends on CPU_THERMAL
>>> diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
>>> index fa0dc48..08ae7ac 100644
>>> --- a/drivers/thermal/Makefile
>>> +++ b/drivers/thermal/Makefile
>>> @@ -39,3 +39,4 @@ obj-$(CONFIG_TI_SOC_THERMAL)	+= ti-soc-thermal/
>>>   obj-$(CONFIG_INT340X_THERMAL)  += int340x_thermal/
>>>   obj-$(CONFIG_ST_THERMAL)	+= st/
>>>   obj-$(CONFIG_TEGRA_SOCTHERM)	+= tegra_soctherm.o
>>> +obj-$(CONFIG_HISI_THERMAL)     += hisi_thermal.o
>>> diff --git a/drivers/thermal/hisi_thermal.c b/drivers/thermal/hisi_thermal.c
>>> new file mode 100644
>>> index 0000000..255c71b
>>> --- /dev/null
>>> +++ b/drivers/thermal/hisi_thermal.c
>>> @@ -0,0 +1,437 @@
>>> +/*
>>> + * Hisilicon thermal sensor driver
>>> + *
>>> + * Copyright (c) 2014-2015 Hisilicon Limited.
>>> + * Copyright (c) 2014-2015 Linaro Limited.
>>> + *
>>> + * Xinwei Kong <kong.kongxinwei@hisilicon.com>
>>> + * Leo Yan <leo.yan@linaro.org>
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License version 2 as
>>> + * published by the Free Software Foundation.
>>> + *
>>> + * This program is distributed "as is" WITHOUT ANY WARRANTY of any
>>> + * kind, whether express or implied; without even the implied warranty
>>> + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>>> + * GNU General Public License for more details.
>>> + */
>>> +
>>> +#include <linux/cpufreq.h>
>>
>> I don't see why you need this header.
>>
>>> +#include <linux/delay.h>
>>> +#include <linux/device.h>
>>> +#include <linux/init.h>
>>> +#include <linux/interrupt.h>
>>> +#include <linux/io.h>
>>> +#include <linux/kernel.h>
>>> +#include <linux/mfd/syscon.h>
>>
>> neither this
>>
>>> +#include <linux/module.h>
>>> +#include <linux/of.h>
>>> +#include <linux/of_device.h>
>>> +#include <linux/platform_device.h>
>>> +#include <linux/regmap.h>
>>
>> or this
>>
>>> +#include <linux/slab.h>
>>> +#include <linux/thermal.h>
>>> +#include <linux/types.h>
>>> +
>>> +#include "thermal_core.h"
>>
>>
>> Please review the above includes. Leave only those you are using.
>>
>>> +
>>> +#define TEMP0_TH			(0x4)
>>> +#define TEMP0_RST_TH			(0x8)
>>> +#define TEMP0_CFG			(0xC)
>>> +#define TEMP0_EN			(0x10)
>>> +#define TEMP0_INT_EN			(0x14)
>>> +#define TEMP0_INT_CLR			(0x18)
>>> +#define TEMP0_RST_MSK			(0x1C)
>>> +#define TEMP0_VALUE			(0x28)
>>> +
>>> +#define HISI_TEMP_BASE			(-60)
>>> +#define HISI_TEMP_RESET			(100000)
>>> +
>>> +#define HISI_MAX_SENSORS		4
>>> +
>>> +struct hisi_thermal_sensor {
>>> +	struct hisi_thermal_data *thermal;
>>> +	struct thermal_zone_device *tzd;
>>> +	const struct thermal_trip *trip;
>>> +
>>> +	uint32_t id;
>>> +	uint32_t thres_temp;
>>> +	uint32_t reset_temp;
>>
>> reset_temp is never used in this driver.
>>
>>> +};
>>> +
>>> +struct hisi_thermal_data {
>>> +	struct mutex thermal_lock;
>>> +	struct platform_device *pdev;
>>> +	struct clk *clk;
>>> +
>>> +	int irq, irq_bind_sensor;
>>> +	bool irq_enabled;
>>> +
>>> +	unsigned int sensors_num;
>>> +	long sensor_temp[HISI_MAX_SENSORS];
>>> +	struct hisi_thermal_sensor sensors[HISI_MAX_SENSORS];
>>
>> Does it make sense to make sensor_temp part of hisi_thermal_sensors?
>>
>> Then you would access it through sensors.sensor_temp
>>
>>> +
>>> +	void __iomem *regs;
>>> +};
>>> +
>>> +/* in millicelsius */
>>> +static inline int _step_to_temp(int step)
>>> +{
>>> +	/*
>>> +	 * Every step equals (1 * 200) / 255 celsius, and finally
>>> +	 * need convert to millicelsius.
>>> +	 */
>>> +	return (HISI_TEMP_BASE + (step * 200 / 255)) * 1000;
>>> +}
>>> +
>>> +static inline int _temp_to_step(int temp)
>>> +{
>>> +	return ((temp / 1000 - HISI_TEMP_BASE) * 255 / 200);
>>> +}
>>> +
>>> +static long hisi_thermal_get_sensor_temp(struct hisi_thermal_data *data,
>>> +					 struct hisi_thermal_sensor *sensor)
>>> +{
>>> +	int val;
>>> +
>>> +	mutex_lock(&data->thermal_lock);
>>> +
>>> +	/* disable interrupt */
>>> +	writel(0x0, data->regs + TEMP0_INT_EN);
>>> +	writel(0x1, data->regs + TEMP0_INT_CLR);
>>> +
>>> +	/* disable module firstly */
>>> +	writel(0x0, data->regs + TEMP0_EN);
>>> +
>>> +	/* select sensor id */
>>> +	writel((sensor->id << 12), data->regs + TEMP0_CFG);
>>> +
>>> +	/* enable module */
>>> +	writel(0x1, data->regs + TEMP0_EN);
>>> +
>>> +	usleep_range(3000, 5000);
>>> +
>>> +	val = readl(data->regs + TEMP0_VALUE);
>>> +	val = _step_to_temp(val);
>>> +
>>> +	/* adjust for negative value */
>>> +	val = (val < 0) ? 0 : val;
>>
>> Why?
>>
>> What does val < 0 mean here? Does it mean negative temperature or an
>> error?
>>
>> BTW, There is ongoing work to allow, at least representing, negative
>> temperature.
>>
>>
>>> +
>>> +	mutex_unlock(&data->thermal_lock);
>>> +
>>> +	return val;
>>> +}
>>> +
>>> +static void hisi_thermal_bind_irq(struct hisi_thermal_data *data)
>>> +{
>>> +	struct hisi_thermal_sensor *sensor;
>>> +
>>> +	sensor = &data->sensors[data->irq_bind_sensor];
>>> +
>>> +	mutex_lock(&data->thermal_lock);
>>> +
>>> +	/* setting the hdak time */
>>> +	writel(0x0, data->regs + TEMP0_CFG);
>>> +
>>> +	/* disable module firstly */
>>> +	writel(0x0, data->regs + TEMP0_RST_MSK);
>>> +	writel(0x0, data->regs + TEMP0_EN);
>>> +
>>
>> Sorry, I get confused when you tell me you need to disable the module
>> every time you need to read temp / change irq configuration.
>>
>> Is this really required?
>>
>>> +	/* select sensor id */
>>> +	writel((sensor->id << 12), data->regs + TEMP0_CFG);
>>> +
>>> +	/* enable for interrupt */
>>> +	writel(_temp_to_step(sensor->thres_temp)
>>> +			| 0x0FFFFFF00, data->regs + TEMP0_TH);
>>
>>
>> nit: break after the | operator.
>>
>>> +
>>> +	writel(_temp_to_step(HISI_TEMP_RESET), data->regs + TEMP0_RST_TH);
>>> +
>>> +	/* enable module */
>>> +	writel(0x1, data->regs + TEMP0_RST_MSK);
>>> +	writel(0x1, data->regs + TEMP0_EN);
>>> +
>>> +	writel(0x0, data->regs + TEMP0_INT_CLR);
>>> +	writel(0x1, data->regs + TEMP0_INT_EN);
>>> +
>>> +	usleep_range(3000, 5000);
>>> +
>>> +	mutex_unlock(&data->thermal_lock);
>>> +
>> nit: remove empty line.
>>
>>> +}
>>> +
>>> +static void hisi_thermal_enable_sensor(struct hisi_thermal_data *data)
>>> +{
>>> +	hisi_thermal_bind_irq(data);
>>> +}
>>> +
>>> +static void hisi_thermal_disable_sensor(struct hisi_thermal_data *data)
>>> +{
>>> +
>>> +	mutex_lock(&data->thermal_lock);
>>> +
>>> +	/* disable sensor module */
>>> +	writel(0x0, data->regs + TEMP0_INT_EN);
>>> +	writel(0x0, data->regs + TEMP0_RST_MSK);
>>> +	writel(0x0, data->regs + TEMP0_EN);
>>> +
>>> +	mutex_unlock(&data->thermal_lock);
>>> +
>>
>> nit: remove empty line.
>>
>>> +}
>>> +
>>> +static int hisi_thermal_get_temp(void *_sensor, long *temp)
>>> +{
>>> +	struct hisi_thermal_sensor *sensor = _sensor;
>>> +	struct hisi_thermal_data *data = sensor->thermal;
>>> +
>>> +	int sensor_id = 0, i;
>>> +	long max_temp = 0;
>>> +
>>> +	*temp = hisi_thermal_get_sensor_temp(data, sensor);
>>> +
>>> +	data->sensor_temp[sensor->id] = *temp;
>>> +
>>> +	for (i = 0; i < HISI_MAX_SENSORS; i++) {
>>> +		if (data->sensor_temp[i] >= max_temp) {
>>> +			max_temp = data->sensor_temp[i];
>>> +			sensor_id = i;
>>> +		}
>>> +	}
>>> +
>>> +	data->irq_bind_sensor = sensor_id;
>>
>> I believe this irq_bind_sensor needs to be protected by a lock.
>>
>> You read it in t_irq context, write it here while getting temp, and read
>> it in bind irq.
>>
>> I still don't follow why you need it though, can you please explain?
>>
>> In any case, seams to be racy, given that it is shared by the four
>> sensors, but you don't lock it to use it.
>>
>>> +
>>> +	dev_dbg(&data->pdev->dev, "id=%d, irq=%d, temp=%ld, thres=%d\n",
>>> +		sensor->id, data->irq_enabled, *temp, sensor->thres_temp);
>>> +	/*
>>> +	 * Bind irq to sensor for two cases:
>>> +	 *   Reenable alarm IRQ if temperature below threshold;
>>> +	 *   if irq has been enabled, always set it;
>>> +	 */
>>> +	if (data->irq_enabled) {
>>> +		hisi_thermal_bind_irq(data);
>>> +		return 0;
>>> +	}
>>> +
>>> +	if (max_temp < sensor->thres_temp) {
>>> +		data->irq_enabled = true;
>>> +		hisi_thermal_bind_irq(data);
>>> +		enable_irq(data->irq);
>>> +	}
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static struct thermal_zone_of_device_ops hisi_of_thermal_ops = {
>>> +	.get_temp = hisi_thermal_get_temp,
>>> +};
>>> +
>>> +static irqreturn_t hisi_thermal_alarm_irq(int irq, void *dev)
>>> +{
>>> +	struct hisi_thermal_data *data = dev;
>>> +
>>> +	disable_irq_nosync(irq);
>>> +	data->irq_enabled = false;
>>> +
>>> +	return IRQ_WAKE_THREAD;
>>> +}
>>> +
>>> +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;
>>> +
>>> +	sensor = &data->sensors[data->irq_bind_sensor];
>>> +
>>> +	dev_crit(&data->pdev->dev, "THERMAL ALARM: T > %d\n",
>>> +		 sensor->thres_temp / 1000);
>>
>> Do you really want to be printing this every time you receive an IRQ?
>>
>>> +
>>> +	for (i = 0; i < data->sensors_num; i++)
>>> +		thermal_zone_device_update(data->sensors[i].tzd);
>>
>>
>> Can you please educate me on how this chip works?
>>
>> You receive a thermal alarm from the chip, does it really  mean you have
>> to update all thermal zones? Or do you have a way to check which sensor
>> generated the alarm irq?
>>
>>> +
>>> +	return IRQ_HANDLED;
>>> +}
>>> +
>>> +static int hisi_thermal_register_sensor(struct platform_device *pdev,
>>> +					struct hisi_thermal_data *data,
>>> +					struct hisi_thermal_sensor *sensor,
>>> +					int index)
>>> +{
>>> +	int ret, i;
>>> +
>>> +	sensor->id = index;
>>> +	sensor->thermal = data;
>>> +
>>> +	sensor->tzd = thermal_zone_of_sensor_register(&pdev->dev, sensor->id,
>>> +				sensor, &hisi_of_thermal_ops);
>>> +	if (IS_ERR(sensor->tzd)) {
>>> +		ret = PTR_ERR(sensor->tzd);
>>> +		dev_err(&pdev->dev, "failed to register sensor id %d: %d\n",
>>> +			sensor->id, ret);
>>> +		return ret;
>>> +	}
>>> +
>>> +	sensor->trip = of_thermal_get_trip_points(sensor->tzd);
>>> +
>>> +	for (i = 0; i < of_thermal_get_ntrips(sensor->tzd); i++) {
>>> +		if (sensor->trip[i].type == THERMAL_TRIP_PASSIVE) {
>>
>> what if you have more than one passive trip? You just care about the
>> first?
>>
>>> +			sensor->thres_temp = sensor->trip[i].temperature;
>>
>> Looks like you use sensor->trip only for filling thres_temp here. Does
>> it make sense to use a local variable in this function, given you won't
>> use sensor->trip anywhere else?
>>
>>> +			break;
>>> +		}
>>> +	}
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static const struct of_device_id of_hisi_thermal_match[] = {
>>> +	{ .compatible = "hisilicon,tsensor" },
>>> +	{ /* end */ }
>>> +};
>>> +MODULE_DEVICE_TABLE(of, of_hisi_thermal_match);
>>> +
>>> +static void hisi_thermal_toggle_sensor(struct hisi_thermal_sensor *sensor,
>>> +				       bool on)
>>> +{
>>> +	struct thermal_zone_device *tzd = sensor->tzd;
>>> +
>>> +	tzd->ops->set_mode(tzd,
>>> +		on ? THERMAL_DEVICE_ENABLED : THERMAL_DEVICE_DISABLED);
>>> +}
>>> +
>>> +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);
>>> +	if (!data)
>>> +		return -ENOMEM;
>>> +
>>> +	mutex_init(&data->thermal_lock);
>>> +	data->pdev = pdev;
>>> +
>>> +	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);
>>> +	}
>>> +
>>> +	data->irq = platform_get_irq(pdev, 0);
>>> +	if (data->irq < 0)
>>> +		return data->irq;
>>> +
>>> +	ret = devm_request_threaded_irq(&pdev->dev, data->irq,
>>> +			hisi_thermal_alarm_irq, hisi_thermal_alarm_irq_thread,
>>> +			0, "hisi_thermal", data);
>>> +	if (ret < 0) {
>>> +		dev_err(&pdev->dev, "failed to request alarm irq: %d\n", ret);
>>> +		return ret;
>>> +	}
>>> +
>>> +	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);
>>> +		return ret;
>>> +	}
>>> +
>>> +	/* enable clock for thermal */
>>> +	ret = clk_prepare_enable(data->clk);
>>> +	if (ret) {
>>> +		dev_err(&pdev->dev, "failed to enable thermal clk: %d\n", ret);
>>> +		return ret;
>>> +	}
>>> +
>>> +	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);
>>> +			goto err_get_sensor_data;
>>> +		}
>>> +	}
>>> +
>>> +	hisi_thermal_enable_sensor(data);
>>> +	data->irq_enabled = true;
>>> +
>>> +	for (i = 0; i < data->sensors_num; i++)
>>> +		hisi_thermal_toggle_sensor(&data->sensors[i], true);
>>> +
>>> +	return 0;
>>> +
>>> +err_get_sensor_data:
>>> +	clk_disable_unprepare(data->clk);
>>
>> nit: empty line here to follow your pattern.
>>
>>> +	return ret;
>>> +}
>>> +
>>> +static int hisi_thermal_remove(struct platform_device *pdev)
>>> +{
>>> +	struct hisi_thermal_data *data = platform_get_drvdata(pdev);
>>> +	int i;
>>> +
>>> +	for (i = 0; i < data->sensors_num; i++) {
>>> +		struct hisi_thermal_sensor *sensor = &data->sensors[i];
>>> +
>>> +		hisi_thermal_toggle_sensor(sensor, false);
>>> +		thermal_zone_of_sensor_unregister(&pdev->dev, sensor->tzd);
>>> +	}
>>> +
>>> +	hisi_thermal_disable_sensor(data);
>>> +	clk_disable_unprepare(data->clk);
>>
>> nit: empty line here to follow your pattern.
>>
>>> +	return 0;
>>> +}
>>> +
>>> +#ifdef CONFIG_PM_SLEEP
>>> +static int hisi_thermal_suspend(struct device *dev)
>>> +{
>>> +	struct hisi_thermal_data *data = dev_get_drvdata(dev);
>>> +
>>> +	hisi_thermal_disable_sensor(data);
>>> +	data->irq_enabled = false;
>>> +
>>> +	clk_disable_unprepare(data->clk);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static int hisi_thermal_resume(struct device *dev)
>>> +{
>>> +	struct hisi_thermal_data *data = dev_get_drvdata(dev);
>>> +
>>> +	clk_prepare_enable(data->clk);
>>> +
>>> +	data->irq_enabled = true;
>>> +	hisi_thermal_enable_sensor(data);
>>> +
>>> +	return 0;
>>> +}
>>> +#endif
>>> +
>>> +static SIMPLE_DEV_PM_OPS(hisi_thermal_pm_ops,
>>> +			 hisi_thermal_suspend, hisi_thermal_resume);
>>> +
>>> +static struct platform_driver hisi_thermal_driver = {
>>> +	.driver = {
>>> +		.name	= "hisi_thermal",
>>> +		.owner  = THIS_MODULE,
>>> +		.pm	= &hisi_thermal_pm_ops,
>>> +		.of_match_table = of_hisi_thermal_match,
>>
>>
>> nit: align on the '=' operator.
>>
>>> +	},
>>> +	.probe		= hisi_thermal_probe,
>>> +	.remove		= hisi_thermal_remove,
>>> +};
>>> +
>>> +module_platform_driver(hisi_thermal_driver);
>>> +
>>> +MODULE_AUTHOR("Xinwei Kong <kong.kongxinwei@hisilicon.com>");
>>> +MODULE_AUTHOR("Leo Yan <leo.yan@linaro.org>");
>>> +MODULE_DESCRIPTION("Hisilicon thermal driver");
>>> +MODULE_LICENSE("GPL v2");
>>> --
>>> 1.9.1
>>>
>>>
>
>
> .
>