Hi Niklas, On Wed, Jul 7, 2021 at 3:14 PM Niklas Söderlund <niklas.soderlund+renesas@xxxxxxxxxxxx> 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@xxxxxxxxxxxx> 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@xxxxxxxxxxxxxx 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