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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 2015/5/18 12:43, Bintian wrote:
> 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.
> 
Pls forgive my newbie.

>> 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.
> 
Ok, thanks brother tian

> 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@xxxxxxxxxxxxx>
>>>>
>>>> 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@xxxxxxxxxx>
>>>> Signed-off-by: kongxinwei <kong.kongxinwei@xxxxxxxxxxxxx>
>>>> ---
>>>>   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@xxxxxxxxxxxxx>
>>>> + * Leo Yan <leo.yan@xxxxxxxxxx>
>>>> + *
>>>> + * 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@xxxxxxxxxxxxx>");
>>>> +MODULE_AUTHOR("Leo Yan <leo.yan@xxxxxxxxxx>");
>>>> +MODULE_DESCRIPTION("Hisilicon thermal driver");
>>>> +MODULE_LICENSE("GPL v2");
>>>> -- 
>>>> 1.9.1
>>>>
>>>>
>>
>>
>> .
>>
> 
> 
> .
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux