* [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
* 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
* [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
* 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
* [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 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 an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.