* [PATCH 0/3] thermal: rcar_gen3_thermal: Add support for trip points
@ 2021-07-07 13:13 Niklas Söderlund
2021-07-07 13:13 ` [PATCH 1/3] thermal: rcar_gen3_thermal: Create struct for OF match data Niklas Söderlund
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Niklas Söderlund @ 2021-07-07 13:13 UTC (permalink / raw)
To: Daniel Lezcano, linux-pm; +Cc: linux-renesas-soc, Niklas Söderlund
Hi Daniel,
This small series adds support for the .set_trips() to the
rcar_gen3_thermal driver on the platforms that supports it.
Patch 1/3 prepares for the new feature by expanding the OF match data
while patch 2/3 adds the actual change. Patch 3/3 is a drive-by fix of a
datatype found while working on this feature.
The work is based on-top of thermal/next [1] and tested on M3-N and V3U
without any regressions or other issues.
1. fe6a6de6692e7f71 ("thermal/drivers/int340x/processor_thermal: Fix tcc setting")
Niklas Söderlund (3):
thermal: rcar_gen3_thermal: Create struct for OF match data
thermal: rcar_gen3_thermal: Add support for hardware trip points
thermal: rcar_gen3_thermal: Fix datatype for loop counter
drivers/thermal/rcar_gen3_thermal.c | 170 ++++++++++++++++++++++++----
1 file changed, 150 insertions(+), 20 deletions(-)
--
2.32.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/3] thermal: rcar_gen3_thermal: Create struct for OF match data
2021-07-07 13:13 [PATCH 0/3] thermal: rcar_gen3_thermal: Add support for trip points Niklas Söderlund
@ 2021-07-07 13:13 ` Niklas Söderlund
2021-07-07 14:42 ` Geert Uytterhoeven
2021-07-07 13:13 ` [PATCH 2/3] thermal: rcar_gen3_thermal: Add support for hardware trip points Niklas Söderlund
2021-07-07 13:13 ` [PATCH 3/3] thermal: rcar_gen3_thermal: Fix datatype for loop counter Niklas Söderlund
2 siblings, 1 reply; 10+ messages in thread
From: Niklas Söderlund @ 2021-07-07 13:13 UTC (permalink / raw)
To: Daniel Lezcano, linux-pm; +Cc: linux-renesas-soc, Niklas Söderlund
Prepare for storing more information in the OF match data by using a
struct instead of a single integer value. There is no functional change.
Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
drivers/thermal/rcar_gen3_thermal.c | 39 +++++++++++++++++++----------
1 file changed, 26 insertions(+), 13 deletions(-)
diff --git a/drivers/thermal/rcar_gen3_thermal.c b/drivers/thermal/rcar_gen3_thermal.c
index fdf16aa34eb470c7..770f1daae5379053 100644
--- a/drivers/thermal/rcar_gen3_thermal.c
+++ b/drivers/thermal/rcar_gen3_thermal.c
@@ -79,6 +79,10 @@ struct equation_coefs {
int b2;
};
+struct rcar_gen3_thermal_info {
+ int ths_tj_1;
+};
+
struct rcar_gen3_thermal_tsc {
void __iomem *base;
struct thermal_zone_device *zone;
@@ -88,6 +92,7 @@ struct rcar_gen3_thermal_tsc {
};
struct rcar_gen3_thermal_priv {
+ const struct rcar_gen3_thermal_info *info;
struct rcar_gen3_thermal_tsc *tscs[TSC_MAX_NUM];
unsigned int num_tscs;
void (*thermal_init)(struct rcar_gen3_thermal_tsc *tsc);
@@ -243,44 +248,50 @@ static void rcar_gen3_thermal_init(struct rcar_gen3_thermal_tsc *tsc)
usleep_range(1000, 2000);
}
-static const int rcar_gen3_ths_tj_1 = 126;
-static const int rcar_gen3_ths_tj_1_m3_w = 116;
+static const struct rcar_gen3_thermal_info rcar_gen3_thermal_gen3 = {
+ .ths_tj_1 = 126,
+};
+
+static const struct rcar_gen3_thermal_info rcar_gen3_thermal_m3w = {
+ .ths_tj_1 = 116,
+};
+
static const struct of_device_id rcar_gen3_thermal_dt_ids[] = {
{
.compatible = "renesas,r8a774a1-thermal",
- .data = &rcar_gen3_ths_tj_1_m3_w,
+ .data = &rcar_gen3_thermal_m3w,
},
{
.compatible = "renesas,r8a774b1-thermal",
- .data = &rcar_gen3_ths_tj_1,
+ .data = &rcar_gen3_thermal_gen3,
},
{
.compatible = "renesas,r8a774e1-thermal",
- .data = &rcar_gen3_ths_tj_1,
+ .data = &rcar_gen3_thermal_gen3,
},
{
.compatible = "renesas,r8a7795-thermal",
- .data = &rcar_gen3_ths_tj_1,
+ .data = &rcar_gen3_thermal_gen3,
},
{
.compatible = "renesas,r8a7796-thermal",
- .data = &rcar_gen3_ths_tj_1_m3_w,
+ .data = &rcar_gen3_thermal_m3w,
},
{
.compatible = "renesas,r8a77961-thermal",
- .data = &rcar_gen3_ths_tj_1_m3_w,
+ .data = &rcar_gen3_thermal_m3w,
},
{
.compatible = "renesas,r8a77965-thermal",
- .data = &rcar_gen3_ths_tj_1,
+ .data = &rcar_gen3_thermal_gen3,
},
{
.compatible = "renesas,r8a77980-thermal",
- .data = &rcar_gen3_ths_tj_1,
+ .data = &rcar_gen3_thermal_gen3,
},
{
.compatible = "renesas,r8a779a0-thermal",
- .data = &rcar_gen3_ths_tj_1,
+ .data = &rcar_gen3_thermal_gen3,
},
{},
};
@@ -307,7 +318,6 @@ static int rcar_gen3_thermal_probe(struct platform_device *pdev)
{
struct rcar_gen3_thermal_priv *priv;
struct device *dev = &pdev->dev;
- const int *ths_tj_1 = of_device_get_match_data(dev);
struct resource *res;
struct thermal_zone_device *zone;
int ret, i;
@@ -320,6 +330,8 @@ static int rcar_gen3_thermal_probe(struct platform_device *pdev)
if (!priv)
return -ENOMEM;
+ priv->info = of_device_get_match_data(dev);
+
priv->thermal_init = rcar_gen3_thermal_init;
if (soc_device_match(r8a7795es1))
priv->thermal_init = rcar_gen3_thermal_init_r8a7795es1;
@@ -352,7 +364,8 @@ static int rcar_gen3_thermal_probe(struct platform_device *pdev)
priv->tscs[i] = tsc;
priv->thermal_init(tsc);
- rcar_gen3_thermal_calc_coefs(tsc, ptat, thcodes[i], *ths_tj_1);
+ rcar_gen3_thermal_calc_coefs(tsc, ptat, thcodes[i],
+ priv->info->ths_tj_1);
zone = devm_thermal_zone_of_sensor_register(dev, i, tsc,
&rcar_gen3_tz_of_ops);
--
2.32.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/3] thermal: rcar_gen3_thermal: Add support for hardware trip points
2021-07-07 13:13 [PATCH 0/3] thermal: rcar_gen3_thermal: Add support for trip points Niklas Söderlund
2021-07-07 13:13 ` [PATCH 1/3] thermal: rcar_gen3_thermal: Create struct for OF match data Niklas Söderlund
@ 2021-07-07 13:13 ` Niklas Söderlund
2021-07-07 14:58 ` Geert Uytterhoeven
2021-07-07 13:13 ` [PATCH 3/3] thermal: rcar_gen3_thermal: Fix datatype for loop counter Niklas Söderlund
2 siblings, 1 reply; 10+ messages in thread
From: Niklas Söderlund @ 2021-07-07 13:13 UTC (permalink / raw)
To: Daniel Lezcano, linux-pm; +Cc: linux-renesas-soc, Niklas Söderlund
All supported hardware except V3U is capable of generating interrupts
to the CPU when the temperature go below or above a set value. Use this
to implement support for the set_trip() feature of the thermal core on
supported hardware.
The V3U have its interrupts routed to the ECM module and therefore can
not be used to implement set_trip() as the driver can't be made aware of
when the interrupt triggers.
Each TSC is capable of tracking up-to three different temperatures while
only two are needed to implement the tracking of the thermal window.
Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
drivers/thermal/rcar_gen3_thermal.c | 130 ++++++++++++++++++++++++++--
1 file changed, 123 insertions(+), 7 deletions(-)
diff --git a/drivers/thermal/rcar_gen3_thermal.c b/drivers/thermal/rcar_gen3_thermal.c
index 770f1daae5379053..a438e05e7ca7f20e 100644
--- a/drivers/thermal/rcar_gen3_thermal.c
+++ b/drivers/thermal/rcar_gen3_thermal.c
@@ -81,6 +81,7 @@ struct equation_coefs {
struct rcar_gen3_thermal_info {
int ths_tj_1;
+ bool have_irq;
};
struct rcar_gen3_thermal_tsc {
@@ -95,7 +96,8 @@ struct rcar_gen3_thermal_priv {
const struct rcar_gen3_thermal_info *info;
struct rcar_gen3_thermal_tsc *tscs[TSC_MAX_NUM];
unsigned int num_tscs;
- void (*thermal_init)(struct rcar_gen3_thermal_tsc *tsc);
+ void (*thermal_init)(struct rcar_gen3_thermal_priv *priv,
+ struct rcar_gen3_thermal_tsc *tsc);
};
static inline u32 rcar_gen3_thermal_read(struct rcar_gen3_thermal_tsc *tsc,
@@ -195,16 +197,75 @@ static int rcar_gen3_thermal_get_temp(void *devdata, int *temp)
return 0;
}
+static int rcar_gen3_thermal_mcelsius_to_temp(struct rcar_gen3_thermal_tsc *tsc,
+ int mcelsius)
+{
+ int celsius, val;
+
+ celsius = DIV_ROUND_CLOSEST(mcelsius, 1000);
+ if (celsius <= INT_FIXPT(tsc->tj_t))
+ val = celsius * tsc->coef.a1 + tsc->coef.b1;
+ else
+ val = celsius * tsc->coef.a2 + tsc->coef.b2;
+
+ return INT_FIXPT(val);
+}
+
+static int rcar_gen3_thermal_set_trips(void *devdata, int low, int high)
+{
+ struct rcar_gen3_thermal_tsc *tsc = devdata;
+ u32 irqmsk = 0;
+
+ if (low != -INT_MAX) {
+ irqmsk |= IRQ_TEMPD1;
+ rcar_gen3_thermal_write(tsc, REG_GEN3_IRQTEMP1,
+ rcar_gen3_thermal_mcelsius_to_temp(tsc, low));
+ }
+
+ if (high != INT_MAX) {
+ irqmsk |= IRQ_TEMP2;
+ rcar_gen3_thermal_write(tsc, REG_GEN3_IRQTEMP2,
+ rcar_gen3_thermal_mcelsius_to_temp(tsc, high));
+ }
+
+ rcar_gen3_thermal_write(tsc, REG_GEN3_IRQMSK, irqmsk);
+
+ return 0;
+}
+
static const struct thermal_zone_of_device_ops rcar_gen3_tz_of_ops = {
.get_temp = rcar_gen3_thermal_get_temp,
+ .set_trips = rcar_gen3_thermal_set_trips,
};
+static const struct thermal_zone_of_device_ops rcar_gen3_tz_of_ops_no_irq = {
+ .get_temp = rcar_gen3_thermal_get_temp,
+};
+
+static irqreturn_t rcar_gen3_thermal_irq(int irq, void *data)
+{
+ struct rcar_gen3_thermal_priv *priv = data;
+ unsigned int i;
+ u32 status;
+
+ for (i = 0; i < priv->num_tscs; i++) {
+ status = rcar_gen3_thermal_read(priv->tscs[i], REG_GEN3_IRQSTR);
+ rcar_gen3_thermal_write(priv->tscs[i], REG_GEN3_IRQSTR, 0);
+ if (status)
+ thermal_zone_device_update(priv->tscs[i]->zone,
+ THERMAL_EVENT_UNSPECIFIED);
+ }
+
+ return IRQ_HANDLED;
+}
+
static const struct soc_device_attribute r8a7795es1[] = {
{ .soc_id = "r8a7795", .revision = "ES1.*" },
{ /* sentinel */ }
};
-static void rcar_gen3_thermal_init_r8a7795es1(struct rcar_gen3_thermal_tsc *tsc)
+static void rcar_gen3_thermal_init_r8a7795es1(struct rcar_gen3_thermal_priv *priv,
+ struct rcar_gen3_thermal_tsc *tsc)
{
rcar_gen3_thermal_write(tsc, REG_GEN3_CTSR, CTSR_THBGR);
rcar_gen3_thermal_write(tsc, REG_GEN3_CTSR, 0x0);
@@ -215,6 +276,9 @@ static void rcar_gen3_thermal_init_r8a7795es1(struct rcar_gen3_thermal_tsc *tsc)
rcar_gen3_thermal_write(tsc, REG_GEN3_IRQCTL, 0x3F);
rcar_gen3_thermal_write(tsc, REG_GEN3_IRQMSK, 0);
+ if (priv->info->have_irq)
+ rcar_gen3_thermal_write(tsc, REG_GEN3_IRQEN,
+ IRQ_TEMPD1 | IRQ_TEMP2);
rcar_gen3_thermal_write(tsc, REG_GEN3_CTSR,
CTSR_PONM | CTSR_AOUT | CTSR_THBGR | CTSR_VMEN);
@@ -228,7 +292,8 @@ static void rcar_gen3_thermal_init_r8a7795es1(struct rcar_gen3_thermal_tsc *tsc)
usleep_range(1000, 2000);
}
-static void rcar_gen3_thermal_init(struct rcar_gen3_thermal_tsc *tsc)
+static void rcar_gen3_thermal_init(struct rcar_gen3_thermal_priv *priv,
+ struct rcar_gen3_thermal_tsc *tsc)
{
u32 reg_val;
@@ -240,6 +305,9 @@ static void rcar_gen3_thermal_init(struct rcar_gen3_thermal_tsc *tsc)
rcar_gen3_thermal_write(tsc, REG_GEN3_IRQCTL, 0);
rcar_gen3_thermal_write(tsc, REG_GEN3_IRQMSK, 0);
+ if (priv->info->have_irq)
+ rcar_gen3_thermal_write(tsc, REG_GEN3_IRQEN,
+ IRQ_TEMPD1 | IRQ_TEMP2);
reg_val = rcar_gen3_thermal_read(tsc, REG_GEN3_THCTR);
reg_val |= THCTR_THSST;
@@ -250,10 +318,16 @@ static void rcar_gen3_thermal_init(struct rcar_gen3_thermal_tsc *tsc)
static const struct rcar_gen3_thermal_info rcar_gen3_thermal_gen3 = {
.ths_tj_1 = 126,
+ .have_irq = true,
};
static const struct rcar_gen3_thermal_info rcar_gen3_thermal_m3w = {
.ths_tj_1 = 116,
+ .have_irq = true,
+};
+
+static const struct rcar_gen3_thermal_info rcar_gen3_thermal_v3u = {
+ .ths_tj_1 = 126,
};
static const struct of_device_id rcar_gen3_thermal_dt_ids[] = {
@@ -291,7 +365,7 @@ static const struct of_device_id rcar_gen3_thermal_dt_ids[] = {
},
{
.compatible = "renesas,r8a779a0-thermal",
- .data = &rcar_gen3_thermal_gen3,
+ .data = &rcar_gen3_thermal_v3u,
},
{},
};
@@ -314,8 +388,37 @@ static void rcar_gen3_hwmon_action(void *data)
thermal_remove_hwmon_sysfs(zone);
}
+static int rcar_gen3_thermal_request_irqs(struct rcar_gen3_thermal_priv *priv,
+ struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ unsigned int i;
+ char *irqname;
+ int ret, irq;
+
+ for (i = 0; i < 2; i++) {
+ irq = platform_get_irq(pdev, i);
+ if (irq < 0)
+ return irq;
+
+ irqname = devm_kasprintf(dev, GFP_KERNEL, "%s:ch%d",
+ dev_name(dev), i);
+ if (!irqname)
+ return -ENOMEM;
+
+ ret = devm_request_threaded_irq(dev, irq, NULL,
+ rcar_gen3_thermal_irq,
+ IRQF_ONESHOT, irqname, priv);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
static int rcar_gen3_thermal_probe(struct platform_device *pdev)
{
+ const struct thermal_zone_of_device_ops *tz_of_ops;
struct rcar_gen3_thermal_priv *priv;
struct device *dev = &pdev->dev;
struct resource *res;
@@ -338,6 +441,15 @@ static int rcar_gen3_thermal_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, priv);
+ tz_of_ops = &rcar_gen3_tz_of_ops_no_irq;
+ if (priv->info->have_irq) {
+ ret = rcar_gen3_thermal_request_irqs(priv, pdev);
+ if (ret)
+ return ret;
+
+ tz_of_ops = &rcar_gen3_tz_of_ops;
+ }
+
pm_runtime_enable(dev);
pm_runtime_get_sync(dev);
@@ -363,12 +475,12 @@ static int rcar_gen3_thermal_probe(struct platform_device *pdev)
priv->tscs[i] = tsc;
- priv->thermal_init(tsc);
+ priv->thermal_init(priv, tsc);
rcar_gen3_thermal_calc_coefs(tsc, ptat, thcodes[i],
priv->info->ths_tj_1);
zone = devm_thermal_zone_of_sensor_register(dev, i, tsc,
- &rcar_gen3_tz_of_ops);
+ tz_of_ops);
if (IS_ERR(zone)) {
dev_err(dev, "Can't register thermal zone\n");
ret = PTR_ERR(zone);
@@ -414,8 +526,12 @@ static int __maybe_unused rcar_gen3_thermal_resume(struct device *dev)
for (i = 0; i < priv->num_tscs; i++) {
struct rcar_gen3_thermal_tsc *tsc = priv->tscs[i];
+ struct thermal_zone_device *zone = tsc->zone;
- priv->thermal_init(tsc);
+ priv->thermal_init(priv, tsc);
+ if (priv->info->have_irq)
+ rcar_gen3_thermal_set_trips(tsc, zone->prev_low_trip,
+ zone->prev_high_trip);
}
return 0;
--
2.32.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 3/3] thermal: rcar_gen3_thermal: Fix datatype for loop counter
2021-07-07 13:13 [PATCH 0/3] thermal: rcar_gen3_thermal: Add support for trip points Niklas Söderlund
2021-07-07 13:13 ` [PATCH 1/3] thermal: rcar_gen3_thermal: Create struct for OF match data Niklas Söderlund
2021-07-07 13:13 ` [PATCH 2/3] thermal: rcar_gen3_thermal: Add support for hardware trip points Niklas Söderlund
@ 2021-07-07 13:13 ` Niklas Söderlund
2021-07-07 14:42 ` Geert Uytterhoeven
2 siblings, 1 reply; 10+ messages in thread
From: Niklas Söderlund @ 2021-07-07 13:13 UTC (permalink / raw)
To: Daniel Lezcano, linux-pm; +Cc: linux-renesas-soc, Niklas Söderlund
The loop counter 'i' should be unsigned int to match struct
rcar_gen3_thermal_priv num_tscs where it's stored.
Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
drivers/thermal/rcar_gen3_thermal.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/thermal/rcar_gen3_thermal.c b/drivers/thermal/rcar_gen3_thermal.c
index a438e05e7ca7f20e..8ca4bfee7bca4122 100644
--- a/drivers/thermal/rcar_gen3_thermal.c
+++ b/drivers/thermal/rcar_gen3_thermal.c
@@ -423,7 +423,8 @@ static int rcar_gen3_thermal_probe(struct platform_device *pdev)
struct device *dev = &pdev->dev;
struct resource *res;
struct thermal_zone_device *zone;
- int ret, i;
+ unsigned int i;
+ int ret;
/* default values if FUSEs are missing */
/* TODO: Read values from hardware on supported platforms */
--
2.32.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] thermal: rcar_gen3_thermal: Fix datatype for loop counter
2021-07-07 13:13 ` [PATCH 3/3] thermal: rcar_gen3_thermal: Fix datatype for loop counter Niklas Söderlund
@ 2021-07-07 14:42 ` Geert Uytterhoeven
2021-07-07 18:55 ` Niklas Söderlund
0 siblings, 1 reply; 10+ messages in thread
From: Geert Uytterhoeven @ 2021-07-07 14:42 UTC (permalink / raw)
To: Niklas Söderlund; +Cc: Daniel Lezcano, Linux PM list, Linux-Renesas
Hi Niklas,
Thanks for your patch!
On Wed, Jul 7, 2021 at 3:14 PM Niklas Söderlund
<niklas.soderlund+renesas@ragnatech.se> wrote:
> The loop counter 'i' should be unsigned int to match struct
> rcar_gen3_thermal_priv num_tscs where it's stored.
It is also stored in rcar_gen3_thermal_tsc.id, which is still (signed) int.
> --- a/drivers/thermal/rcar_gen3_thermal.c
> +++ b/drivers/thermal/rcar_gen3_thermal.c
> @@ -423,7 +423,8 @@ static int rcar_gen3_thermal_probe(struct platform_device *pdev)
> struct device *dev = &pdev->dev;
> struct resource *res;
> struct thermal_zone_device *zone;
> - int ret, i;
> + unsigned int i;
> + int ret;
>
> /* default values if FUSEs are missing */
> /* TODO: Read values from hardware on supported platforms */
And perhaps:
- dev_info(dev, "TSC%d: Loaded %d trip points\n", i, ret);
+ dev_info(dev, "TSC%u: Loaded %d trip points\n", i, ret);
?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] thermal: rcar_gen3_thermal: Create struct for OF match data
2021-07-07 13:13 ` [PATCH 1/3] thermal: rcar_gen3_thermal: Create struct for OF match data Niklas Söderlund
@ 2021-07-07 14:42 ` Geert Uytterhoeven
0 siblings, 0 replies; 10+ messages in thread
From: Geert Uytterhoeven @ 2021-07-07 14:42 UTC (permalink / raw)
To: Niklas Söderlund; +Cc: Daniel Lezcano, Linux PM list, Linux-Renesas
On Wed, Jul 7, 2021 at 3:14 PM Niklas Söderlund
<niklas.soderlund+renesas@ragnatech.se> wrote:
> Prepare for storing more information in the OF match data by using a
> struct instead of a single integer value. There is no functional change.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] thermal: rcar_gen3_thermal: Add support for hardware trip points
2021-07-07 13:13 ` [PATCH 2/3] thermal: rcar_gen3_thermal: Add support for hardware trip points Niklas Söderlund
@ 2021-07-07 14:58 ` Geert Uytterhoeven
2021-07-07 18:53 ` Niklas Söderlund
0 siblings, 1 reply; 10+ messages in thread
From: Geert Uytterhoeven @ 2021-07-07 14:58 UTC (permalink / raw)
To: Niklas Söderlund; +Cc: Daniel Lezcano, Linux PM list, Linux-Renesas
Hi Niklas,
On Wed, Jul 7, 2021 at 3:14 PM Niklas Söderlund
<niklas.soderlund+renesas@ragnatech.se> wrote:
> All supported hardware except V3U is capable of generating interrupts
> to the CPU when the temperature go below or above a set value. Use this
> to implement support for the set_trip() feature of the thermal core on
> supported hardware.
>
> The V3U have its interrupts routed to the ECM module and therefore can
> not be used to implement set_trip() as the driver can't be made aware of
> when the interrupt triggers.
>
> Each TSC is capable of tracking up-to three different temperatures while
> only two are needed to implement the tracking of the thermal window.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Thanks for your patch!
> --- a/drivers/thermal/rcar_gen3_thermal.c
> +++ b/drivers/thermal/rcar_gen3_thermal.c
> @@ -81,6 +81,7 @@ struct equation_coefs {
>
> struct rcar_gen3_thermal_info {
> int ths_tj_1;
> + bool have_irq;
Do you need this flag? See below.
> };
>
> struct rcar_gen3_thermal_tsc {
> @@ -95,7 +96,8 @@ struct rcar_gen3_thermal_priv {
> const struct rcar_gen3_thermal_info *info;
> struct rcar_gen3_thermal_tsc *tscs[TSC_MAX_NUM];
> unsigned int num_tscs;
> - void (*thermal_init)(struct rcar_gen3_thermal_tsc *tsc);
> + void (*thermal_init)(struct rcar_gen3_thermal_priv *priv,
Do you need priv? See below.
> + struct rcar_gen3_thermal_tsc *tsc);
> };
>
> static inline u32 rcar_gen3_thermal_read(struct rcar_gen3_thermal_tsc *tsc,
> @@ -195,16 +197,75 @@ static int rcar_gen3_thermal_get_temp(void *devdata, int *temp)
> static const struct thermal_zone_of_device_ops rcar_gen3_tz_of_ops = {
> .get_temp = rcar_gen3_thermal_get_temp,
> + .set_trips = rcar_gen3_thermal_set_trips,
> };
>
> +static const struct thermal_zone_of_device_ops rcar_gen3_tz_of_ops_no_irq = {
> + .get_temp = rcar_gen3_thermal_get_temp,
> +};
What about having a single non-const thermal_zone_of_device_ops,
and filling in .set_trip when interrupts are present?
> @@ -240,6 +305,9 @@ static void rcar_gen3_thermal_init(struct rcar_gen3_thermal_tsc *tsc)
>
> rcar_gen3_thermal_write(tsc, REG_GEN3_IRQCTL, 0);
> rcar_gen3_thermal_write(tsc, REG_GEN3_IRQMSK, 0);
> + if (priv->info->have_irq)
I think you can check for the presence of tsc->zone->ops->set_trips instead.
> + rcar_gen3_thermal_write(tsc, REG_GEN3_IRQEN,
> + IRQ_TEMPD1 | IRQ_TEMP2);
>
> reg_val = rcar_gen3_thermal_read(tsc, REG_GEN3_THCTR);
> reg_val |= THCTR_THSST;
> @@ -314,8 +388,37 @@ static void rcar_gen3_hwmon_action(void *data)
> thermal_remove_hwmon_sysfs(zone);
> }
>
> +static int rcar_gen3_thermal_request_irqs(struct rcar_gen3_thermal_priv *priv,
> + struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + unsigned int i;
> + char *irqname;
> + int ret, irq;
> +
> + for (i = 0; i < 2; i++) {
> + irq = platform_get_irq(pdev, i);
Would it make sense to use platform_get_irq_optional() instead,
to auto-detect variants with and without interrupt support?
> + if (irq < 0)
> + return irq;
> +
> + irqname = devm_kasprintf(dev, GFP_KERNEL, "%s:ch%d",
> + dev_name(dev), i);
> + if (!irqname)
> + return -ENOMEM;
> +
> + ret = devm_request_threaded_irq(dev, irq, NULL,
> + rcar_gen3_thermal_irq,
> + IRQF_ONESHOT, irqname, priv);
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> +}
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] thermal: rcar_gen3_thermal: Add support for hardware trip points
2021-07-07 14:58 ` Geert Uytterhoeven
@ 2021-07-07 18:53 ` Niklas Söderlund
0 siblings, 0 replies; 10+ messages in thread
From: Niklas Söderlund @ 2021-07-07 18:53 UTC (permalink / raw)
To: Geert Uytterhoeven; +Cc: Daniel Lezcano, Linux PM list, Linux-Renesas
Hi Geert,
Thanks for your feedback.
On 2021-07-07 16:58:33 +0200, Geert Uytterhoeven wrote:
> Hi Niklas,
>
> On Wed, Jul 7, 2021 at 3:14 PM Niklas Söderlund
> <niklas.soderlund+renesas@ragnatech.se> wrote:
> > All supported hardware except V3U is capable of generating interrupts
> > to the CPU when the temperature go below or above a set value. Use this
> > to implement support for the set_trip() feature of the thermal core on
> > supported hardware.
> >
> > The V3U have its interrupts routed to the ECM module and therefore can
> > not be used to implement set_trip() as the driver can't be made aware of
> > when the interrupt triggers.
> >
> > Each TSC is capable of tracking up-to three different temperatures while
> > only two are needed to implement the tracking of the thermal window.
> >
> > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
>
> Thanks for your patch!
>
> > --- a/drivers/thermal/rcar_gen3_thermal.c
> > +++ b/drivers/thermal/rcar_gen3_thermal.c
> > @@ -81,6 +81,7 @@ struct equation_coefs {
> >
> > struct rcar_gen3_thermal_info {
> > int ths_tj_1;
> > + bool have_irq;
>
> Do you need this flag? See below.
>
> > };
> >
> > struct rcar_gen3_thermal_tsc {
> > @@ -95,7 +96,8 @@ struct rcar_gen3_thermal_priv {
> > const struct rcar_gen3_thermal_info *info;
> > struct rcar_gen3_thermal_tsc *tscs[TSC_MAX_NUM];
> > unsigned int num_tscs;
> > - void (*thermal_init)(struct rcar_gen3_thermal_tsc *tsc);
> > + void (*thermal_init)(struct rcar_gen3_thermal_priv *priv,
>
> Do you need priv? See below.
>
> > + struct rcar_gen3_thermal_tsc *tsc);
> > };
> >
> > static inline u32 rcar_gen3_thermal_read(struct rcar_gen3_thermal_tsc *tsc,
> > @@ -195,16 +197,75 @@ static int rcar_gen3_thermal_get_temp(void *devdata, int *temp)
>
> > static const struct thermal_zone_of_device_ops rcar_gen3_tz_of_ops = {
> > .get_temp = rcar_gen3_thermal_get_temp,
> > + .set_trips = rcar_gen3_thermal_set_trips,
> > };
> >
> > +static const struct thermal_zone_of_device_ops rcar_gen3_tz_of_ops_no_irq = {
> > + .get_temp = rcar_gen3_thermal_get_temp,
> > +};
>
> What about having a single non-const thermal_zone_of_device_ops,
> and filling in .set_trip when interrupts are present?
I could, for some reason I like ops structs to be cost. It prevents me
from modifying it's members after they have been used in a framework
init function somewhere which judged the driver capabilities on its
populated members ;-)
But this is a simple thing in this driver, I will give it a try for v2.
>
> > @@ -240,6 +305,9 @@ static void rcar_gen3_thermal_init(struct rcar_gen3_thermal_tsc *tsc)
> >
> > rcar_gen3_thermal_write(tsc, REG_GEN3_IRQCTL, 0);
> > rcar_gen3_thermal_write(tsc, REG_GEN3_IRQMSK, 0);
> > + if (priv->info->have_irq)
>
> I think you can check for the presence of tsc->zone->ops->set_trips instead.
Good idea!
>
> > + rcar_gen3_thermal_write(tsc, REG_GEN3_IRQEN,
> > + IRQ_TEMPD1 | IRQ_TEMP2);
> >
> > reg_val = rcar_gen3_thermal_read(tsc, REG_GEN3_THCTR);
> > reg_val |= THCTR_THSST;
>
> > @@ -314,8 +388,37 @@ static void rcar_gen3_hwmon_action(void *data)
> > thermal_remove_hwmon_sysfs(zone);
> > }
> >
> > +static int rcar_gen3_thermal_request_irqs(struct rcar_gen3_thermal_priv *priv,
> > + struct platform_device *pdev)
> > +{
> > + struct device *dev = &pdev->dev;
> > + unsigned int i;
> > + char *irqname;
> > + int ret, irq;
> > +
> > + for (i = 0; i < 2; i++) {
> > + irq = platform_get_irq(pdev, i);
>
> Would it make sense to use platform_get_irq_optional() instead,
> to auto-detect variants with and without interrupt support?
The bindings require interrupts be present for all variants but V3U. But
since auto-detect is a good thing and with the method you describe it's
easy to add, will give it a try for a v2.
>
> > + if (irq < 0)
> > + return irq;
> > +
> > + irqname = devm_kasprintf(dev, GFP_KERNEL, "%s:ch%d",
> > + dev_name(dev), i);
> > + if (!irqname)
> > + return -ENOMEM;
> > +
> > + ret = devm_request_threaded_irq(dev, irq, NULL,
> > + rcar_gen3_thermal_irq,
> > + IRQF_ONESHOT, irqname, priv);
> > + if (ret)
> > + return ret;
> > + }
> > +
> > + return 0;
> > +}
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
--
Regards,
Niklas Söderlund
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] thermal: rcar_gen3_thermal: Fix datatype for loop counter
2021-07-07 14:42 ` Geert Uytterhoeven
@ 2021-07-07 18:55 ` Niklas Söderlund
2021-07-07 21:12 ` Geert Uytterhoeven
0 siblings, 1 reply; 10+ messages in thread
From: Niklas Söderlund @ 2021-07-07 18:55 UTC (permalink / raw)
To: Geert Uytterhoeven; +Cc: Daniel Lezcano, Linux PM list, Linux-Renesas
Hi Geert,
Thanks for your feedback.
On 2021-07-07 16:42:10 +0200, Geert Uytterhoeven wrote:
> Hi Niklas,
>
> Thanks for your patch!
>
> On Wed, Jul 7, 2021 at 3:14 PM Niklas Söderlund
> <niklas.soderlund+renesas@ragnatech.se> wrote:
> > The loop counter 'i' should be unsigned int to match struct
> > rcar_gen3_thermal_priv num_tscs where it's stored.
>
> It is also stored in rcar_gen3_thermal_tsc.id, which is still (signed) int.
Thanks.
>
> > --- a/drivers/thermal/rcar_gen3_thermal.c
> > +++ b/drivers/thermal/rcar_gen3_thermal.c
> > @@ -423,7 +423,8 @@ static int rcar_gen3_thermal_probe(struct platform_device *pdev)
> > struct device *dev = &pdev->dev;
> > struct resource *res;
> > struct thermal_zone_device *zone;
> > - int ret, i;
> > + unsigned int i;
> > + int ret;
> >
> > /* default values if FUSEs are missing */
> > /* TODO: Read values from hardware on supported platforms */
>
> And perhaps:
>
> - dev_info(dev, "TSC%d: Loaded %d trip points\n", i, ret);
> + dev_info(dev, "TSC%u: Loaded %d trip points\n", i, ret);
>
> ?
Good catch, why do my compiler not warn for this? Did you catch this by
review or have you some compiler magic I'm missing?
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
--
Regards,
Niklas Söderlund
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] thermal: rcar_gen3_thermal: Fix datatype for loop counter
2021-07-07 18:55 ` Niklas Söderlund
@ 2021-07-07 21:12 ` Geert Uytterhoeven
0 siblings, 0 replies; 10+ messages in thread
From: Geert Uytterhoeven @ 2021-07-07 21:12 UTC (permalink / raw)
To: Niklas Söderlund; +Cc: Daniel Lezcano, Linux PM list, Linux-Renesas
Hi Niklas,
On Wed, Jul 7, 2021 at 8:56 PM Niklas Söderlund
<niklas.soderlund+renesas@ragnatech.se> wrote:
> On 2021-07-07 16:42:10 +0200, Geert Uytterhoeven wrote:
> > On Wed, Jul 7, 2021 at 3:14 PM Niklas Söderlund
> > <niklas.soderlund+renesas@ragnatech.se> wrote:
> > > The loop counter 'i' should be unsigned int to match struct
> > > rcar_gen3_thermal_priv num_tscs where it's stored.
> >
> > It is also stored in rcar_gen3_thermal_tsc.id, which is still (signed) int.
>
> Thanks.
>
> >
> > > --- a/drivers/thermal/rcar_gen3_thermal.c
> > > +++ b/drivers/thermal/rcar_gen3_thermal.c
> > > @@ -423,7 +423,8 @@ static int rcar_gen3_thermal_probe(struct platform_device *pdev)
> > > struct device *dev = &pdev->dev;
> > > struct resource *res;
> > > struct thermal_zone_device *zone;
> > > - int ret, i;
> > > + unsigned int i;
> > > + int ret;
> > >
> > > /* default values if FUSEs are missing */
> > > /* TODO: Read values from hardware on supported platforms */
> >
> > And perhaps:
> >
> > - dev_info(dev, "TSC%d: Loaded %d trip points\n", i, ret);
> > + dev_info(dev, "TSC%u: Loaded %d trip points\n", i, ret);
> >
> > ?
>
> Good catch, why do my compiler not warn for this? Did you catch this by
Because the types are (somewhat) compatible.
> review or have you some compiler magic I'm missing?
Review. You changed the type of a variable, I checked all places where
the variable is used.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-07-07 21:12 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-07 13:13 [PATCH 0/3] thermal: rcar_gen3_thermal: Add support for trip points Niklas Söderlund
2021-07-07 13:13 ` [PATCH 1/3] thermal: rcar_gen3_thermal: Create struct for OF match data Niklas Söderlund
2021-07-07 14:42 ` Geert Uytterhoeven
2021-07-07 13:13 ` [PATCH 2/3] thermal: rcar_gen3_thermal: Add support for hardware trip points Niklas Söderlund
2021-07-07 14:58 ` Geert Uytterhoeven
2021-07-07 18:53 ` Niklas Söderlund
2021-07-07 13:13 ` [PATCH 3/3] thermal: rcar_gen3_thermal: Fix datatype for loop counter Niklas Söderlund
2021-07-07 14:42 ` Geert Uytterhoeven
2021-07-07 18:55 ` Niklas Söderlund
2021-07-07 21:12 ` Geert Uytterhoeven
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).