| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID:
<OSCPR01MB14647520D9DACCC72844765A8FF75A@OSCPR01MB14647.jpnprd01.prod.outlook.com>
Date: Wed, 11 Jun 2025 10:54:19 +0000
From: John Madieu <john.madieu.xa@...renesas.com>
To: Biju Das <biju.das.jz@...renesas.com>, "conor+dt@...nel.org"
<conor+dt@...nel.org>, "daniel.lezcano@...aro.org"
<daniel.lezcano@...aro.org>, "geert+renesas@...der.be"
<geert+renesas@...der.be>, "krzk+dt@...nel.org" <krzk+dt@...nel.org>,
"rafael@...nel.org" <rafael@...nel.org>
CC: "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
"john.madieu@...il.com" <john.madieu@...il.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
"linux-renesas-soc@...r.kernel.org" <linux-renesas-soc@...r.kernel.org>,
"lukasz.luba@....com" <lukasz.luba@....com>, "magnus.damm@...il.com"
<magnus.damm@...il.com>, "robh@...nel.org" <robh@...nel.org>,
"rui.zhang@...el.com" <rui.zhang@...el.com>, "sboyd@...nel.org"
<sboyd@...nel.org>, "niklas.soderlund+renesas@...natech.se"
<niklas.soderlund+renesas@...natech.se>
Subject: RE: [PATCH v6 3/5] thermal: renesas: rzg3e: Add thermal driver for
the Renesas RZ/G3E SoC
Hi Biju,
> -----Original Message-----
> From: Biju Das <biju.das.jz@...renesas.com>
> Sent: Friday, June 6, 2025 8:17 AM
> To: John Madieu <john.madieu.xa@...renesas.com>; John Madieu
> <john.madieu.xa@...renesas.com>; conor+dt@...nel.org;
> daniel.lezcano@...aro.org; geert+renesas@...der.be; krzk+dt@...nel.org;
> rafael@...nel.org
>
> Hi John,
>
> > -----Original Message-----
> > From: John Madieu <john.madieu.xa@...renesas.com>
> > Sent: 22 May 2025 19:23
> > Subject: [PATCH v6 3/5] thermal: renesas: rzg3e: Add thermal driver
> > for the Renesas RZ/G3E SoC
> >
> > The RZ/G3E SoC integrates a Temperature Sensor Unit (TSU) block
> > designed to monitor the chip's junction temperature. This sensor is
> > connected to channel 1 of the APB port clock/reset and provides
> temperature measurements.
> >
> > It also requires calibration values stored in the system controller
> > registers for accurate temperature measurement. Add a driver for the
> Renesas RZ/G3E TSU.
> >
> > Signed-off-by: John Madieu <john.madieu.xa@...renesas.com>
> > ---
> >
> > Changes:
> >
> > v1 -> v2: fixes IRQ names
> > v2 -> v3: no changes
> > v3 -> v4: no changes
> > v5: removes curly braces arround single-line protected scoped guards
> > v6: Clarified comments in driver
> >
> > MAINTAINERS | 7 +
> > drivers/thermal/renesas/Kconfig | 7 +
> > drivers/thermal/renesas/Makefile | 1 +
> > drivers/thermal/renesas/rzg3e_thermal.c | 443
> > ++++++++++++++++++++++++
> > 4 files changed, 458 insertions(+)
> > create mode 100644 drivers/thermal/renesas/rzg3e_thermal.c
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS index
> > 79a8e2c73908..eb11494795e8 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -21161,6 +21161,13 @@ S: Maintained
> > F:
> Documentation/devicetree/bindings/iio/potentiometer/renesas,x9250.ya
> ml
> > F: drivers/iio/potentiometer/x9250.c
> >
> > +RENESAS RZ/G3E THERMAL SENSOR UNIT DRIVER
> > +M: John Madieu <john.madieu.xa@...renesas.com>
> > +L: linux-pm@...r.kernel.org
> > +S: Maintained
> > +F: Documentation/devicetree/bindings/thermal/renesas,r9a09g047-tsu.yaml
> > +F: drivers/thermal/renesas/rzg3e_thermal.c
> > +
> > RESET CONTROLLER FRAMEWORK
> > M: Philipp Zabel <p.zabel@...gutronix.de>
> > S: Maintained
> > diff --git a/drivers/thermal/renesas/Kconfig
> > b/drivers/thermal/renesas/Kconfig index dcf5fc5ae08e..10cf90fc4bfa
> > 100644
> > --- a/drivers/thermal/renesas/Kconfig
> > +++ b/drivers/thermal/renesas/Kconfig
> > @@ -26,3 +26,10 @@ config RZG2L_THERMAL
> > help
> > Enable this to plug the RZ/G2L thermal sensor driver into the
> Linux
> > thermal framework.
> > +
> > +config RZG3E_THERMAL
> > + tristate "Renesas RZ/G3E thermal driver"
> > + depends on ARCH_RENESAS || COMPILE_TEST
> > + help
> > + Enable this to plug the RZ/G3E thermal sensor driver into the
> Linux
> > + thermal framework.
> > diff --git a/drivers/thermal/renesas/Makefile
> > b/drivers/thermal/renesas/Makefile
> > index bf9cb3cb94d6..5a3eba0dedd0 100644
> > --- a/drivers/thermal/renesas/Makefile
> > +++ b/drivers/thermal/renesas/Makefile
> > @@ -3,3 +3,4 @@
> > obj-$(CONFIG_RCAR_GEN3_THERMAL) += rcar_gen3_thermal.o
> > obj-$(CONFIG_RCAR_THERMAL) += rcar_thermal.o
> > obj-$(CONFIG_RZG2L_THERMAL) += rzg2l_thermal.o
> > +obj-$(CONFIG_RZG3E_THERMAL) += rzg3e_thermal.o
> > diff --git a/drivers/thermal/renesas/rzg3e_thermal.c
> > b/drivers/thermal/renesas/rzg3e_thermal.c
> > new file mode 100644
> > index 000000000000..348229da9ef4
> > --- /dev/null
> > +++ b/drivers/thermal/renesas/rzg3e_thermal.c
> > @@ -0,0 +1,443 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Renesas RZ/G3E TSU Temperature Sensor Unit
> > + *
> > + * Copyright (C) 2025 Renesas Electronics Corporation */ #include
> > +<linux/clk.h> #include <linux/delay.h> #include <linux/err.h>
> > +#include <linux/interrupt.h> #include <linux/io.h> #include
> > +<linux/iopoll.h> #include <linux/kernel.h> #include
> > +<linux/mfd/syscon.h> #include <linux/module.h> #include
> > +<linux/of_device.h> #include <linux/platform_device.h> #include
> > +<linux/regmap.h> #include <linux/reset.h> #include <linux/thermal.h>
> > +#include <linux/units.h>
> > +
> > +#include "../thermal_hwmon.h"
> > +
> > +/* SYS Trimming register offsets macro */ #define SYS_TSU_TRMVAL(x)
> > +(0x330 + (x) * 4)
> > +
> > +/* TSU Register offsets and bits */
> > +#define TSU_SSUSR 0x00
> > +#define TSU_SSUSR_EN_TS BIT(0)
> > +#define TSU_SSUSR_ADC_PD_TS BIT(1)
> > +#define TSU_SSUSR_SOC_TS_EN BIT(2)
> > +
> > +#define TSU_STRGR 0x04
> > +#define TSU_STRGR_ADST BIT(0)
> > +
> > +#define TSU_SOSR1 0x08
> > +#define TSU_SOSR1_ADCT_8 0x03
> > +#define TSU_SOSR1_OUTSEL_AVERAGE BIT(9)
> > +
> > +/* Sensor Code Read Register */
> > +#define TSU_SCRR 0x10
> > +#define TSU_SCRR_OUT12BIT_TS GENMASK(11, 0)
> > +
> > +/* Sensor Status Register */
> > +#define TSU_SSR 0x14
> > +#define TSU_SSR_CONV_RUNNING BIT(0)
> > +
> > +/* Compare Mode Setting Register */
> > +#define TSU_CMSR 0x18
> > +#define TSU_CMSR_CMPEN BIT(0)
> > +#define TSU_CMSR_CMPCOND BIT(1)
> > +
> > +/* Lower Limit Setting Register */
> > +#define TSU_LLSR 0x1C
> > +#define TSU_LLSR_LIM GENMASK(11, 0)
> > +
> > +/* Upper Limit Setting Register */
> > +#define TSU_ULSR 0x20
> > +#define TSU_ULSR_ULIM GENMASK(11, 0)
> > +
> > +/* Interrupt Status Register */
> > +#define TSU_SISR 0x30
> > +#define TSU_SISR_ADF BIT(0)
> > +#define TSU_SISR_CMPF BIT(1)
> > +
> > +/* Interrupt Enable Register */
> > +#define TSU_SIER 0x34
> > +#define TSU_SIER_ADIE BIT(0)
> > +#define TSU_SIER_CMPIE BIT(1)
> > +
> > +/* Interrupt Clear Register */
> > +#define TSU_SICR 0x38
> > +#define TSU_SICR_ADCLR BIT(0)
> > +#define TSU_SICR_CMPCLR BIT(1)
> > +
> > +/* Temperature calculation constants */
> > +#define TSU_D 41
> > +#define TSU_E 126
> > +#define TSU_TRMVAL_MASK GENMASK(11, 0)
> > +
> > +#define TSU_POLL_DELAY_US 50
> > +#define TSU_TIMEOUT_US 10000
> > +#define TSU_MIN_CLOCK_RATE 24000000
> > +
> > +/**
> > + /* Start a conversion to trigger comparison */
> > + writel(TSU_STRGR_ADST, priv->base + TSU_STRGR);
> > +
> > + return 0;
> > +}
> > +
> > +static int rzg3e_thermal_get_trimming(struct rzg3e_thermal_priv
> > +*priv) {
> > + int ret;
> > +
> > + ret = regmap_read(priv->syscon, SYS_TSU_TRMVAL(0), &priv-
> >trmval[0]);
> > + if (ret)
> > + return ret;
> > +
> > + ret = regmap_read(priv->syscon, SYS_TSU_TRMVAL(1), &priv-
> >trmval[1]);
> > + if (ret)
> > + return ret;
>
> Just checking, which method is better for read/write as system controller
> registers needs to be configured by lot of Driver?
>
> 1) Current method using syscon regmap for read/write from client drivers.
>
> 2) Using a callback registered with sysc and sysc handles the read/write()
> during
> callback execution from client drivers.
>
> 3) Through exported API and sysc handles the read/write()
>
> Cheers,
> Biju
>
I think the current method is the best, as syscon/regmap would
serialize (and cache if need be) accesses. For the second case,
I think we would duplicate what syscon already does, as it provides
generic accesses for register/bit/masking read/write/update. For the
third case, we would need as many APIs in the sysc driver as logical
operations needed by client drivers.
I'm however open to any suggestion that might ease the review of the driver.
Regards,
John
> > +
> > + priv->trmval[0] &= TSU_TRMVAL_MASK;
> > + priv->trmval[1] &= TSU_TRMVAL_MASK;
> > +
> > + if (!priv->trmval[0] || !priv->trmval[1])
> > + return dev_err_probe(priv->dev, -EINVAL, "invalid trimming
> > +values");
> > +
> > + return 0;
> > +}
> > +
> > +static int rzg3e_thermal_change_mode(struct thermal_zone_device *tz,
> > + enum thermal_device_mode mode) {
> > + struct rzg3e_thermal_priv *priv = thermal_zone_device_priv(tz);
> > +
> > + if (mode == THERMAL_DEVICE_DISABLED)
> > + rzg3e_thermal_hw_disable(priv);
> > + else
> > + rzg3e_thermal_hw_enable(priv);
> > +
> > + priv->mode = mode;
> > + return 0;
> > +}
> > +
> > +static const struct thermal_zone_device_ops rzg3e_tz_of_ops = {
> > + .get_temp = rzg3e_thermal_get_temp,
> > + .set_trips = rzg3e_thermal_set_trips,
> > + .change_mode = rzg3e_thermal_change_mode, };
> > +
> > +static int rzg3e_thermal_probe(struct platform_device *pdev) {
> > + struct device *dev = &pdev->dev;
> > + struct rzg3e_thermal_priv *priv;
> > + struct reset_control *rstc;
> > + char *adc_name, *cmp_name;
> > + int adc_irq, cmp_irq;
> > + struct clk *clk;
> > + int ret;
> > +
> > + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> > + if (!priv)
> > + return -ENOMEM;
> > +
> > + priv->dev = dev;
> > +
> > + priv->base = devm_platform_ioremap_resource(pdev, 0);
> > + if (IS_ERR(priv->base))
> > + return dev_err_probe(dev, PTR_ERR(priv->base),
> > + "Failed to map I/O memory");
> > +
> > + priv->syscon = syscon_regmap_lookup_by_phandle(dev->of_node,
> > + "renesas,tsu-calibration-sys");
> > + if (IS_ERR(priv->syscon))
> > + return dev_err_probe(dev, PTR_ERR(priv->syscon),
> > + "Failed to get calibration syscon");
> > +
> > + adc_irq = platform_get_irq_byname(pdev, "adi");
> > + if (adc_irq < 0)
> > + return adc_irq;
> > +
> > + cmp_irq = platform_get_irq_byname(pdev, "adcmpi");
> > + if (cmp_irq < 0)
> > + return cmp_irq;
> > +
> > + rstc = devm_reset_control_get_exclusive_deasserted(dev, NULL);
> > + if (IS_ERR(rstc))
> > + return dev_err_probe(dev, PTR_ERR(rstc),
> > + "Failed to acquire deasserted reset");
> > +
> > + platform_set_drvdata(pdev, priv);
> > +
> > + spin_lock_init(&priv->reg_lock);
> > + init_completion(&priv->conv_complete);
> > +
> > + clk = devm_clk_get_enabled(dev, NULL);
> > + if (IS_ERR(clk))
> > + return dev_err_probe(dev, PTR_ERR(clk),
> > + "Failed to get and enable clock");
> > +
> > + if (clk_get_rate(clk) < TSU_MIN_CLOCK_RATE)
> > + return dev_err_probe(dev, -EINVAL,
> > + "Clock rate too low (minimum %d Hz
> required)",
> > + TSU_MIN_CLOCK_RATE);
> > +
> > + ret = rzg3e_thermal_get_trimming(priv);
> > + if (ret)
> > + return ret;
> > +
> > + adc_name = devm_kasprintf(dev, GFP_KERNEL, "%s-adc", dev_name(dev));
> > + if (!adc_name)
> > + return -ENOMEM;
> > +
> > + cmp_name = devm_kasprintf(dev, GFP_KERNEL, "%s-cmp", dev_name(dev));
> > + if (!cmp_name)
> > + return -ENOMEM;
> > +
> > + /* Unit in a known disabled mode */
> > + rzg3e_thermal_hw_disable(priv);
> > +
> > + ret = devm_request_irq(dev, adc_irq, rzg3e_thermal_adc_irq,
> > + IRQF_TRIGGER_RISING, adc_name, priv);
> > + if (ret)
> > + return dev_err_probe(dev, ret, "Failed to request ADC IRQ");
> > +
> > + ret = devm_request_threaded_irq(dev, cmp_irq, rzg3e_thermal_cmp_irq,
> > + rzg3e_thermal_cmp_threaded_irq,
> > + IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
> > + cmp_name, priv);
> > + if (ret)
> > + return dev_err_probe(dev, ret, "Failed to request comparison
> IRQ");
> > +
> > + /* Register Thermal Zone */
> > + priv->zone = devm_thermal_of_zone_register(dev, 0, priv,
> &rzg3e_tz_of_ops);
> > + if (IS_ERR(priv->zone))
> > + return dev_err_probe(dev, PTR_ERR(priv->zone),
> > + "Failed to register thermal zone");
> > +
> > + ret = devm_thermal_add_hwmon_sysfs(dev, priv->zone);
> > + if (ret)
> > + return dev_err_probe(dev, ret, "Failed to add hwmon sysfs");
> > +
> > + return 0;
> > +}
> > +
> > +static const struct of_device_id rzg3e_thermal_dt_ids[] = {
> > + { .compatible = "renesas,r9a09g047-tsu" },
> > + { /* sentinel */ }
> > +};
> > +MODULE_DEVICE_TABLE(of, rzg3e_thermal_dt_ids);
> > +
> > +static struct platform_driver rzg3e_thermal_driver = {
> > + .driver = {
> > + .name = "rzg3e_thermal",
> > + .of_match_table = rzg3e_thermal_dt_ids,
> > + },
> > + .probe = rzg3e_thermal_probe,
> > +};
> > +module_platform_driver(rzg3e_thermal_driver);
> > +
> > +MODULE_DESCRIPTION("Renesas RZ/G3E TSU Thermal Sensor Driver");
> > +MODULE_AUTHOR("John Madieu <john.madieu.xa@...renesas.com>");
> > +MODULE_LICENSE("GPL");
> > --
> > 2.25.1
Powered by blists - more mailing lists