Re: [PATCH v11 1/5] thermal: rockchip: add driver for thermal

From: Dmitry Torokhov
Date: Thu Oct 16 2014 - 18:06:21 EST


Hi Caesar,

On Thu, Oct 16, 2014 at 09:31:37PM +0800, Caesar Wang wrote:
> +
> +struct rockchip_tsadc_platform_data {
> + unsigned long temp_passive;
> + unsigned long hw_shut_temp;
> + int reset_mode;
> +
> + void (*irq_handle)(void __iomem *reg);
> + void (*initialize)(int reset_mode, int chn, void __iomem *reg,
> + unsigned long hw_shut_temp);
> + int (*control)(void __iomem *reg, bool on, int chn);
> + int (*code_to_temp)(u32 code);
> + u32 (*temp_to_code)(int temp);
> + void (*set_alarm_temp)(int chn, void __iomem *reg,
> + unsigned long alarm_temp);
> +};

Any time I see platform data and pdata I think about board-specific
parameters that come either from board initialization code, device tree
or ACPI, rather than chipset specific data. Do you think we could rename
rockchip_tsadc_platform_data to rockchip_tsadc_chip? If you agree
please fold the patch below into your next revision.

More to come...

Thanks.

--
Dmitry

rockchip-thermal - rename rockchip_tsadc_platform_data

Signed-off-by: Dmitry Torokhov <dtor@xxxxxxxxxxxx>
---
drivers/thermal/rockchip_thermal.c | 66 ++++++++++++++++++++------------------
1 file changed, 35 insertions(+), 31 deletions(-)

diff --git a/drivers/thermal/rockchip_thermal.c b/drivers/thermal/rockchip_thermal.c
index 4ecc649..6d7472b 100644
--- a/drivers/thermal/rockchip_thermal.c
+++ b/drivers/thermal/rockchip_thermal.c
@@ -44,8 +44,24 @@ enum sensor_id {
SENSOR_ID_END,
};

+struct rockchip_tsadc_chip {
+ unsigned long temp_passive;
+ unsigned long hw_shut_temp;
+ enum reset_mode reset_mode;
+
+ void (*irq_handle)(void __iomem *reg);
+ void (*initialize)(enum reset_mode reset_mode, int chn,
+ void __iomem *reg,
+ unsigned long hw_shut_temp);
+ int (*control)(void __iomem *reg, bool on, int chn);
+ int (*code_to_temp)(u32 code);
+ u32 (*temp_to_code)(int temp);
+ void (*set_alarm_temp)(int chn, void __iomem *reg,
+ unsigned long alarm_temp);
+};
+
struct rockchip_thermal_data {
- const struct rockchip_tsadc_platform_data *pdata;
+ const struct rockchip_tsadc_chip *chip;
struct thermal_zone_device *tz[SENSOR_ID_END];
struct thermal_cooling_device *cdev;
void __iomem *regs;
@@ -62,21 +78,6 @@ struct rockchip_thermal_data {
struct clk *pclk;
};

-struct rockchip_tsadc_platform_data {
- unsigned long temp_passive;
- unsigned long hw_shut_temp;
- int reset_mode;
-
- void (*irq_handle)(void __iomem *reg);
- void (*initialize)(int reset_mode, int chn, void __iomem *reg,
- unsigned long hw_shut_temp);
- int (*control)(void __iomem *reg, bool on, int chn);
- int (*code_to_temp)(u32 code);
- u32 (*temp_to_code)(int temp);
- void (*set_alarm_temp)(int chn, void __iomem *reg,
- unsigned long alarm_temp);
-};
-
/* TSADC V2 Sensor info define: */
#define TSADCV2_AUTO_CON 0x04
#define TSADCV2_INT_EN 0x08
@@ -234,7 +235,8 @@ static bool rk_tsadcv2_get_tshut_polarity_high(void __iomem *regs)
* the channel, and you can set TSHUT output to gpio to reset the whole chip,
* and you can set TSHUT output to cru to reset the whole chip.
*/
-static void rk_tsadcv2_initialize(int reset_mode, int chn, void __iomem *regs,
+static void rk_tsadcv2_initialize(enum reset_mode reset_mode, int chn,
+ void __iomem *regs,
unsigned long hw_shut_temp)
{
u32 shutdown_value;
@@ -279,7 +281,7 @@ static void rk_tsadcv2_alarm_temp(int chn, void __iomem *regs,
TSADCV2_COMP_INT(chn));
}

-static const struct rockchip_tsadc_platform_data rk3288_tsadc_data = {
+static const struct rockchip_tsadc_chip rk3288_tsadc_data = {
.reset_mode = GPIO, /* default TSHUT via GPIO give PMIC */
.temp_passive = 80000,
.hw_shut_temp = 120000,
@@ -303,7 +305,7 @@ MODULE_DEVICE_TABLE(of, of_rockchip_thermal_match);
static void rockchip_set_alarm_temp(struct rockchip_thermal_data *data,
int alarm_temp, int chn)
{
- const struct rockchip_tsadc_platform_data *tsadc = data->pdata;
+ const struct rockchip_tsadc_chip *tsadc = data->chip;

data->alarm_temp = alarm_temp;

@@ -352,7 +354,7 @@ static void rockchip_thermal_initialize(struct rockchip_thermal_data *data)
static void rockchip_thermal_control(struct rockchip_thermal_data *data,
bool on, int chn)
{
- const struct rockchip_tsadc_platform_data *tsadc = data->pdata;
+ const struct rockchip_tsadc_chip *tsadc = data->chip;

if (tsadc->control)
tsadc->control(data->regs, on, chn);
@@ -372,7 +374,7 @@ static void rockchip_thermal_control(struct rockchip_thermal_data *data,
static irqreturn_t rockchip_thermal_alarm_irq_thread(int irq, void *dev)
{
struct rockchip_thermal_data *data = dev;
- const struct rockchip_tsadc_platform_data *tsadc = data->pdata;
+ const struct rockchip_tsadc_chip *tsadc = data->chip;
int chn;

tsadc->irq_handle(data->regs);
@@ -386,7 +388,7 @@ static irqreturn_t rockchip_thermal_alarm_irq_thread(int irq, void *dev)
static int rockchip_thermal_set_trips(void *zone, long low, long high)
{
struct rockchip_thermal_data *data = zone;
- const struct rockchip_tsadc_platform_data *tsadc = data->pdata;
+ const struct rockchip_tsadc_chip *tsadc = data->chip;

low = clamp_val(low, LONG_MIN, LONG_MAX);
high = clamp_val(high, LONG_MIN, LONG_MAX);
@@ -407,7 +409,7 @@ static int rockchip_thermal_set_trips(void *zone, long low, long high)
static int rockchip_thermal_get_cpu_temp(void *zone, long *out_temp)
{
struct rockchip_thermal_data *data = zone;
- const struct rockchip_tsadc_platform_data *tsadc = data->pdata;
+ const struct rockchip_tsadc_chip *tsadc = data->chip;
u32 val;

/* the A/D value of the channel last conversion need some time */
@@ -423,7 +425,7 @@ static int rockchip_thermal_get_cpu_temp(void *zone, long *out_temp)
static int rockchip_thermal_get_gpu_temp(void *zone, long *out_temp)
{
struct rockchip_thermal_data *data = zone;
- const struct rockchip_tsadc_platform_data *tsadc = data->pdata;
+ const struct rockchip_tsadc_chip *tsadc = data->chip;
u32 val;

/* the A/D value of the channel last conversion need some time */
@@ -444,19 +446,19 @@ static int rockchip_configure_from_dt(struct device *dev,

if (of_property_read_u32(np, "hw-shut-temp", &shut_temp)) {
dev_warn(dev, "Missing default shutdown temp property\n");
- data->hw_shut_temp = data->pdata->hw_shut_temp;
+ data->hw_shut_temp = data->chip->hw_shut_temp;
} else {
data->hw_shut_temp = shut_temp;
}

if (of_property_read_u32(np, "tsadc-ht-reset-mode", &reset_mode)) {
dev_warn(dev, "Missing default reset mode property\n");
- data->reset_mode = data->pdata->reset_mode;
+ data->reset_mode = data->chip->reset_mode;
} else {
data->reset_mode = reset_mode;
}

- data->temp_passive = data->pdata->temp_passive;
+ data->temp_passive = data->chip->temp_passive;

return 0;
}
@@ -464,7 +466,7 @@ static int rockchip_configure_from_dt(struct device *dev,
static int rockchip_thermal_probe(struct platform_device *pdev)
{
struct rockchip_thermal_data *data;
- const struct rockchip_tsadc_platform_data *tsadc;
+ const struct rockchip_tsadc_chip *tsadc;
const struct of_device_id *match;

struct cpumask clip_cpus;
@@ -489,10 +491,12 @@ static int rockchip_thermal_probe(struct platform_device *pdev)
match = of_match_node(of_rockchip_thermal_match, np);
if (!match)
return -ENXIO;
- data->pdata = (const struct rockchip_tsadc_platform_data *)match->data;
- if (!data->pdata)
+
+ tsadc = (const struct rockchip_tsadc_chip *)match->data;
+ if (!tsadc)
return -EINVAL;
- tsadc = data->pdata;
+
+ data->chip = tsadc;

data->clk = devm_clk_get(&pdev->dev, "tsadc");
if (IS_ERR(data->clk)) {
--
1.8.3.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/