Re: [PATCH 2/3] thermal: rcar_gen3_thermal: Add support for hardware trip points

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux