| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20171017043604.GA7954@localhost.localdomain>
Date: Mon, 16 Oct 2017 21:36:05 -0700
From: Eduardo Valentin <edubezval@...il.com>
To: Daniel Lezcano <daniel.lezcano@...aro.org>
Cc: rui.zhang@...el.com, linux-pm@...r.kernel.org,
linux-kernel@...r.kernel.org, kevin.wangtao@...aro.org
Subject: Re: [PATCH 21/25] thermal/drivers/hisi: Prepare to add support for
other hisi platforms
On Tue, Oct 10, 2017 at 08:02:46PM +0200, Daniel Lezcano wrote:
> From: Kevin Wangtao <kevin.wangtao@...aro.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@...aro.org>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@...aro.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
>
Powered by blists - more mailing lists