drivers/thermal/rockchip_thermal.c | 53 +++++++++++++++++++++++++++++++++----- 1 file changed, 46 insertions(+), 7 deletions(-)
Most of the recent Rockchip devices do not have a GRF associated
with the tsadc IP. Let's avoid printing a warning on those devices.
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/thermal/rockchip_thermal.c | 53 +++++++++++++++++++++++++++++++++-----
1 file changed, 46 insertions(+), 7 deletions(-)
diff --git a/drivers/thermal/rockchip_thermal.c b/drivers/thermal/rockchip_thermal.c
index 3beff9b6fac3abe8948b56132b618ff1bed57217..1e8091cebd6673ab39fa0c4dee835c68aeb7e8b5 100644
--- a/drivers/thermal/rockchip_thermal.c
+++ b/drivers/thermal/rockchip_thermal.c
@@ -50,6 +50,18 @@ enum adc_sort_mode {
ADC_INCREMENT,
};
+/*
+ * The GRF availability depends on the specific SoC
+ * GRF_NONE: the SoC does not have a GRF associated with the tsadc
+ * GRF_OPTIONAL: the SoC has a GRF, but the driver can work without it
+ * GRF_MANDATORY: the SoC has a GRF and it is required for proper operation
+ */
+enum tsadc_grf_mode {
+ GRF_NONE,
+ GRF_OPTIONAL,
+ GRF_MANDATORY,
+};
+
#include "thermal_hwmon.h"
/**
@@ -97,6 +109,9 @@ struct rockchip_tsadc_chip {
enum tshut_mode tshut_mode;
enum tshut_polarity tshut_polarity;
+ /* GRF availability */
+ enum tsadc_grf_mode grf_mode;
+
/* Chip-wide methods */
void (*initialize)(struct regmap *grf,
void __iomem *reg, enum tshut_polarity p);
@@ -1099,6 +1114,8 @@ static const struct rockchip_tsadc_chip px30_tsadc_data = {
.chn_offset = 0,
.chn_num = 2, /* 2 channels for tsadc */
+ .grf_mode = GRF_MANDATORY,
+
.tshut_mode = TSHUT_MODE_CRU, /* default TSHUT via CRU */
.tshut_temp = 95000,
@@ -1123,6 +1140,8 @@ static const struct rockchip_tsadc_chip rv1108_tsadc_data = {
.chn_offset = 0,
.chn_num = 1, /* one channel for tsadc */
+ .grf_mode = GRF_NONE,
+
.tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
.tshut_polarity = TSHUT_LOW_ACTIVE, /* default TSHUT LOW ACTIVE */
.tshut_temp = 95000,
@@ -1148,6 +1167,8 @@ static const struct rockchip_tsadc_chip rk3228_tsadc_data = {
.chn_offset = 0,
.chn_num = 1, /* one channel for tsadc */
+ .grf_mode = GRF_NONE,
+
.tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
.tshut_polarity = TSHUT_LOW_ACTIVE, /* default TSHUT LOW ACTIVE */
.tshut_temp = 95000,
@@ -1173,6 +1194,8 @@ static const struct rockchip_tsadc_chip rk3288_tsadc_data = {
.chn_offset = 1,
.chn_num = 2, /* two channels for tsadc */
+ .grf_mode = GRF_NONE,
+
.tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
.tshut_polarity = TSHUT_LOW_ACTIVE, /* default TSHUT LOW ACTIVE */
.tshut_temp = 95000,
@@ -1198,6 +1221,8 @@ static const struct rockchip_tsadc_chip rk3328_tsadc_data = {
.chn_offset = 0,
.chn_num = 1, /* one channels for tsadc */
+ .grf_mode = GRF_NONE,
+
.tshut_mode = TSHUT_MODE_CRU, /* default TSHUT via CRU */
.tshut_temp = 95000,
@@ -1222,6 +1247,8 @@ static const struct rockchip_tsadc_chip rk3366_tsadc_data = {
.chn_offset = 0,
.chn_num = 2, /* two channels for tsadc */
+ .grf_mode = GRF_OPTIONAL,
+
.tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
.tshut_polarity = TSHUT_LOW_ACTIVE, /* default TSHUT LOW ACTIVE */
.tshut_temp = 95000,
@@ -1247,6 +1274,8 @@ static const struct rockchip_tsadc_chip rk3368_tsadc_data = {
.chn_offset = 0,
.chn_num = 2, /* two channels for tsadc */
+ .grf_mode = GRF_NONE,
+
.tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
.tshut_polarity = TSHUT_LOW_ACTIVE, /* default TSHUT LOW ACTIVE */
.tshut_temp = 95000,
@@ -1272,6 +1301,8 @@ static const struct rockchip_tsadc_chip rk3399_tsadc_data = {
.chn_offset = 0,
.chn_num = 2, /* two channels for tsadc */
+ .grf_mode = GRF_OPTIONAL,
+
.tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
.tshut_polarity = TSHUT_LOW_ACTIVE, /* default TSHUT LOW ACTIVE */
.tshut_temp = 95000,
@@ -1297,6 +1328,8 @@ static const struct rockchip_tsadc_chip rk3568_tsadc_data = {
.chn_offset = 0,
.chn_num = 2, /* two channels for tsadc */
+ .grf_mode = GRF_OPTIONAL,
+
.tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
.tshut_polarity = TSHUT_LOW_ACTIVE, /* default TSHUT LOW ACTIVE */
.tshut_temp = 95000,
@@ -1321,6 +1354,7 @@ static const struct rockchip_tsadc_chip rk3576_tsadc_data = {
/* top, big_core, little_core, ddr, npu, gpu */
.chn_offset = 0,
.chn_num = 6, /* six channels for tsadc */
+ .grf_mode = GRF_NONE,
.tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
.tshut_polarity = TSHUT_LOW_ACTIVE, /* default TSHUT LOW ACTIVE */
.tshut_temp = 95000,
@@ -1345,6 +1379,7 @@ static const struct rockchip_tsadc_chip rk3588_tsadc_data = {
/* top, big_core0, big_core1, little_core, center, gpu, npu */
.chn_offset = 0,
.chn_num = 7, /* seven channels for tsadc */
+ .grf_mode = GRF_NONE,
.tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
.tshut_polarity = TSHUT_LOW_ACTIVE, /* default TSHUT LOW ACTIVE */
.tshut_temp = 95000,
@@ -1572,7 +1607,7 @@ static int rockchip_configure_from_dt(struct device *dev,
struct device_node *np,
struct rockchip_thermal_data *thermal)
{
- u32 shut_temp, tshut_mode, tshut_polarity;
+ u32 shut_temp, tshut_mode, tshut_polarity, ret;
if (of_property_read_u32(np, "rockchip,hw-tshut-temp", &shut_temp)) {
dev_warn(dev,
@@ -1621,12 +1656,16 @@ static int rockchip_configure_from_dt(struct device *dev,
return -EINVAL;
}
- /* The tsadc wont to handle the error in here since some SoCs didn't
- * need this property.
- */
- thermal->grf = syscon_regmap_lookup_by_phandle(np, "rockchip,grf");
- if (IS_ERR(thermal->grf))
- dev_warn(dev, "Missing rockchip,grf property\n");
+ if (thermal->chip->grf_mode != GRF_NONE) {
+ thermal->grf = syscon_regmap_lookup_by_phandle(np, "rockchip,grf");
+ if (IS_ERR(thermal->grf)) {
+ ret = PTR_ERR(thermal->grf);
+ if (thermal->chip->grf_mode == GRF_OPTIONAL)
+ dev_warn(dev, "Missing rockchip,grf property\n");
+ else
+ return dev_err_probe(dev, ret, "Missing rockchip,grf property\n");
+ }
+ }
rockchip_get_trim_configuration(dev, np, thermal);
---
base-commit: c17b750b3ad9f45f2b6f7e6f7f4679844244f0b9
change-id: 20250818-thermal-rockchip-grf-warning-05f7f56286a2
Best regards,
--
Sebastian Reichel <sre@kernel.org>Hi Sebastian,
On Mon Aug 18, 2025 at 7:26 PM CEST, Sebastian Reichel wrote:
> Most of the recent Rockchip devices do not have a GRF associated
> with the tsadc IP. Let's avoid printing a warning on those devices.
Thanks for this patch :-)
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> ---
> drivers/thermal/rockchip_thermal.c | 53 +++++++++++++++++++++++++++++++++-----
> 1 file changed, 46 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/thermal/rockchip_thermal.c b/drivers/thermal/rockchip_thermal.c
> index 3beff9b6fac3abe8948b56132b618ff1bed57217..1e8091cebd6673ab39fa0c4dee835c68aeb7e8b5 100644
> --- a/drivers/thermal/rockchip_thermal.c
> +++ b/drivers/thermal/rockchip_thermal.c
> @@ -50,6 +50,18 @@ enum adc_sort_mode {
> ADC_INCREMENT,
> };
>
> +/*
> + * The GRF availability depends on the specific SoC
> + * GRF_NONE: the SoC does not have a GRF associated with the tsadc
> + * GRF_OPTIONAL: the SoC has a GRF, but the driver can work without it
> + * GRF_MANDATORY: the SoC has a GRF and it is required for proper operation
> + */
> +enum tsadc_grf_mode {
> + GRF_NONE,
> + GRF_OPTIONAL,
> + GRF_MANDATORY,
> +};
> +
> #include "thermal_hwmon.h"
>
> /**
> @@ -97,6 +109,9 @@ struct rockchip_tsadc_chip {
> enum tshut_mode tshut_mode;
> enum tshut_polarity tshut_polarity;
>
> + /* GRF availability */
> + enum tsadc_grf_mode grf_mode;
> +
> /* Chip-wide methods */
> void (*initialize)(struct regmap *grf,
> void __iomem *reg, enum tshut_polarity p);
> @@ -1099,6 +1114,8 @@ static const struct rockchip_tsadc_chip px30_tsadc_data = {
> .chn_offset = 0,
> .chn_num = 2, /* 2 channels for tsadc */
>
> + .grf_mode = GRF_MANDATORY,
> +
> .tshut_mode = TSHUT_MODE_CRU, /* default TSHUT via CRU */
> .tshut_temp = 95000,
>
> @@ -1123,6 +1140,8 @@ static const struct rockchip_tsadc_chip rv1108_tsadc_data = {
> .chn_offset = 0,
> .chn_num = 1, /* one channel for tsadc */
>
> + .grf_mode = GRF_NONE,
> +
> .tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
> .tshut_polarity = TSHUT_LOW_ACTIVE, /* default TSHUT LOW ACTIVE */
> .tshut_temp = 95000,
> @@ -1148,6 +1167,8 @@ static const struct rockchip_tsadc_chip rk3228_tsadc_data = {
> .chn_offset = 0,
> .chn_num = 1, /* one channel for tsadc */
>
> + .grf_mode = GRF_NONE,
> +
> .tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
> .tshut_polarity = TSHUT_LOW_ACTIVE, /* default TSHUT LOW ACTIVE */
> .tshut_temp = 95000,
> @@ -1173,6 +1194,8 @@ static const struct rockchip_tsadc_chip rk3288_tsadc_data = {
> .chn_offset = 1,
> .chn_num = 2, /* two channels for tsadc */
>
> + .grf_mode = GRF_NONE,
> +
> .tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
> .tshut_polarity = TSHUT_LOW_ACTIVE, /* default TSHUT LOW ACTIVE */
> .tshut_temp = 95000,
> @@ -1198,6 +1221,8 @@ static const struct rockchip_tsadc_chip rk3328_tsadc_data = {
> .chn_offset = 0,
> .chn_num = 1, /* one channels for tsadc */
>
> + .grf_mode = GRF_NONE,
> +
> .tshut_mode = TSHUT_MODE_CRU, /* default TSHUT via CRU */
> .tshut_temp = 95000,
>
> @@ -1222,6 +1247,8 @@ static const struct rockchip_tsadc_chip rk3366_tsadc_data = {
> .chn_offset = 0,
> .chn_num = 2, /* two channels for tsadc */
>
> + .grf_mode = GRF_OPTIONAL,
> +
> .tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
> .tshut_polarity = TSHUT_LOW_ACTIVE, /* default TSHUT LOW ACTIVE */
> .tshut_temp = 95000,
> @@ -1247,6 +1274,8 @@ static const struct rockchip_tsadc_chip rk3368_tsadc_data = {
> .chn_offset = 0,
> .chn_num = 2, /* two channels for tsadc */
>
> + .grf_mode = GRF_NONE,
> +
> .tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
> .tshut_polarity = TSHUT_LOW_ACTIVE, /* default TSHUT LOW ACTIVE */
> .tshut_temp = 95000,
> @@ -1272,6 +1301,8 @@ static const struct rockchip_tsadc_chip rk3399_tsadc_data = {
> .chn_offset = 0,
> .chn_num = 2, /* two channels for tsadc */
>
> + .grf_mode = GRF_OPTIONAL,
> +
> .tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
> .tshut_polarity = TSHUT_LOW_ACTIVE, /* default TSHUT LOW ACTIVE */
> .tshut_temp = 95000,
> @@ -1297,6 +1328,8 @@ static const struct rockchip_tsadc_chip rk3568_tsadc_data = {
> .chn_offset = 0,
> .chn_num = 2, /* two channels for tsadc */
>
> + .grf_mode = GRF_OPTIONAL,
> +
> .tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
> .tshut_polarity = TSHUT_LOW_ACTIVE, /* default TSHUT LOW ACTIVE */
> .tshut_temp = 95000,
> @@ -1321,6 +1354,7 @@ static const struct rockchip_tsadc_chip rk3576_tsadc_data = {
> /* top, big_core, little_core, ddr, npu, gpu */
> .chn_offset = 0,
> .chn_num = 6, /* six channels for tsadc */
> + .grf_mode = GRF_NONE,
> .tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
> .tshut_polarity = TSHUT_LOW_ACTIVE, /* default TSHUT LOW ACTIVE */
> .tshut_temp = 95000,
> @@ -1345,6 +1379,7 @@ static const struct rockchip_tsadc_chip rk3588_tsadc_data = {
> /* top, big_core0, big_core1, little_core, center, gpu, npu */
> .chn_offset = 0,
> .chn_num = 7, /* seven channels for tsadc */
> + .grf_mode = GRF_NONE,
> .tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
> .tshut_polarity = TSHUT_LOW_ACTIVE, /* default TSHUT LOW ACTIVE */
> .tshut_temp = 95000,
> @@ -1572,7 +1607,7 @@ static int rockchip_configure_from_dt(struct device *dev,
> struct device_node *np,
> struct rockchip_thermal_data *thermal)
> {
> - u32 shut_temp, tshut_mode, tshut_polarity;
> + u32 shut_temp, tshut_mode, tshut_polarity, ret;
>
> if (of_property_read_u32(np, "rockchip,hw-tshut-temp", &shut_temp)) {
> dev_warn(dev,
> @@ -1621,12 +1656,16 @@ static int rockchip_configure_from_dt(struct device *dev,
> return -EINVAL;
> }
>
> - /* The tsadc wont to handle the error in here since some SoCs didn't
> - * need this property.
> - */
> - thermal->grf = syscon_regmap_lookup_by_phandle(np, "rockchip,grf");
> - if (IS_ERR(thermal->grf))
> - dev_warn(dev, "Missing rockchip,grf property\n");
> + if (thermal->chip->grf_mode != GRF_NONE) {
> + thermal->grf = syscon_regmap_lookup_by_phandle(np, "rockchip,grf");
> + if (IS_ERR(thermal->grf)) {
> + ret = PTR_ERR(thermal->grf);
> + if (thermal->chip->grf_mode == GRF_OPTIONAL)
> + dev_warn(dev, "Missing rockchip,grf property\n");
> + else
> + return dev_err_probe(dev, ret, "Missing rockchip,grf property\n");
> + }
> + }
>
> rockchip_get_trim_configuration(dev, np, thermal);
I tested this patch on the following devices and found no regressions:
- Rock64 (rk3328)
- RockPro64 (rk3399)
- Quartz64 Model B (rk3566)
- NanoPi R5S (rk3568)
When tested on my Rock 5B (rk3588), I no longer saw this warning:
rockchip-thermal fec00000.tsadc: Missing rockchip,grf property
And found no regressions, so
Tested-by: Diederik de Haas <didi.debian@cknow.org>
Cheers,
Diederik
On 18/08/2025 6:26 pm, Sebastian Reichel wrote:
> Most of the recent Rockchip devices do not have a GRF associated
> with the tsadc IP. Let's avoid printing a warning on those devices.
>
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> ---
> drivers/thermal/rockchip_thermal.c | 53 +++++++++++++++++++++++++++++++++-----
> 1 file changed, 46 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/thermal/rockchip_thermal.c b/drivers/thermal/rockchip_thermal.c
> index 3beff9b6fac3abe8948b56132b618ff1bed57217..1e8091cebd6673ab39fa0c4dee835c68aeb7e8b5 100644
> --- a/drivers/thermal/rockchip_thermal.c
> +++ b/drivers/thermal/rockchip_thermal.c
> @@ -50,6 +50,18 @@ enum adc_sort_mode {
> ADC_INCREMENT,
> };
>
> +/*
> + * The GRF availability depends on the specific SoC
> + * GRF_NONE: the SoC does not have a GRF associated with the tsadc
> + * GRF_OPTIONAL: the SoC has a GRF, but the driver can work without it
> + * GRF_MANDATORY: the SoC has a GRF and it is required for proper operation
> + */
> +enum tsadc_grf_mode {
> + GRF_NONE,
> + GRF_OPTIONAL,
> + GRF_MANDATORY,
> +};
> +
> #include "thermal_hwmon.h"
>
> /**
> @@ -97,6 +109,9 @@ struct rockchip_tsadc_chip {
> enum tshut_mode tshut_mode;
> enum tshut_polarity tshut_polarity;
>
> + /* GRF availability */
> + enum tsadc_grf_mode grf_mode;
> +
> /* Chip-wide methods */
> void (*initialize)(struct regmap *grf,
> void __iomem *reg, enum tshut_polarity p);
> @@ -1099,6 +1114,8 @@ static const struct rockchip_tsadc_chip px30_tsadc_data = {
> .chn_offset = 0,
> .chn_num = 2, /* 2 channels for tsadc */
>
> + .grf_mode = GRF_MANDATORY,
> +
> .tshut_mode = TSHUT_MODE_CRU, /* default TSHUT via CRU */
> .tshut_temp = 95000,
>
> @@ -1123,6 +1140,8 @@ static const struct rockchip_tsadc_chip rv1108_tsadc_data = {
> .chn_offset = 0,
> .chn_num = 1, /* one channel for tsadc */
>
> + .grf_mode = GRF_NONE,
> +
> .tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
> .tshut_polarity = TSHUT_LOW_ACTIVE, /* default TSHUT LOW ACTIVE */
> .tshut_temp = 95000,
> @@ -1148,6 +1167,8 @@ static const struct rockchip_tsadc_chip rk3228_tsadc_data = {
> .chn_offset = 0,
> .chn_num = 1, /* one channel for tsadc */
>
> + .grf_mode = GRF_NONE,
> +
> .tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
> .tshut_polarity = TSHUT_LOW_ACTIVE, /* default TSHUT LOW ACTIVE */
> .tshut_temp = 95000,
> @@ -1173,6 +1194,8 @@ static const struct rockchip_tsadc_chip rk3288_tsadc_data = {
> .chn_offset = 1,
> .chn_num = 2, /* two channels for tsadc */
>
> + .grf_mode = GRF_NONE,
> +
> .tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
> .tshut_polarity = TSHUT_LOW_ACTIVE, /* default TSHUT LOW ACTIVE */
> .tshut_temp = 95000,
> @@ -1198,6 +1221,8 @@ static const struct rockchip_tsadc_chip rk3328_tsadc_data = {
> .chn_offset = 0,
> .chn_num = 1, /* one channels for tsadc */
>
> + .grf_mode = GRF_NONE,
> +
> .tshut_mode = TSHUT_MODE_CRU, /* default TSHUT via CRU */
> .tshut_temp = 95000,
>
> @@ -1222,6 +1247,8 @@ static const struct rockchip_tsadc_chip rk3366_tsadc_data = {
> .chn_offset = 0,
> .chn_num = 2, /* two channels for tsadc */
>
> + .grf_mode = GRF_OPTIONAL,
> +
> .tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
> .tshut_polarity = TSHUT_LOW_ACTIVE, /* default TSHUT LOW ACTIVE */
> .tshut_temp = 95000,
> @@ -1247,6 +1274,8 @@ static const struct rockchip_tsadc_chip rk3368_tsadc_data = {
> .chn_offset = 0,
> .chn_num = 2, /* two channels for tsadc */
>
> + .grf_mode = GRF_NONE,
> +
> .tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
> .tshut_polarity = TSHUT_LOW_ACTIVE, /* default TSHUT LOW ACTIVE */
> .tshut_temp = 95000,
> @@ -1272,6 +1301,8 @@ static const struct rockchip_tsadc_chip rk3399_tsadc_data = {
> .chn_offset = 0,
> .chn_num = 2, /* two channels for tsadc */
>
> + .grf_mode = GRF_OPTIONAL,
> +
> .tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
> .tshut_polarity = TSHUT_LOW_ACTIVE, /* default TSHUT LOW ACTIVE */
> .tshut_temp = 95000,
> @@ -1297,6 +1328,8 @@ static const struct rockchip_tsadc_chip rk3568_tsadc_data = {
> .chn_offset = 0,
> .chn_num = 2, /* two channels for tsadc */
>
> + .grf_mode = GRF_OPTIONAL,
> +
> .tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
> .tshut_polarity = TSHUT_LOW_ACTIVE, /* default TSHUT LOW ACTIVE */
> .tshut_temp = 95000,
> @@ -1321,6 +1354,7 @@ static const struct rockchip_tsadc_chip rk3576_tsadc_data = {
> /* top, big_core, little_core, ddr, npu, gpu */
> .chn_offset = 0,
> .chn_num = 6, /* six channels for tsadc */
> + .grf_mode = GRF_NONE,
> .tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
> .tshut_polarity = TSHUT_LOW_ACTIVE, /* default TSHUT LOW ACTIVE */
> .tshut_temp = 95000,
> @@ -1345,6 +1379,7 @@ static const struct rockchip_tsadc_chip rk3588_tsadc_data = {
> /* top, big_core0, big_core1, little_core, center, gpu, npu */
> .chn_offset = 0,
> .chn_num = 7, /* seven channels for tsadc */
> + .grf_mode = GRF_NONE,
> .tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
> .tshut_polarity = TSHUT_LOW_ACTIVE, /* default TSHUT LOW ACTIVE */
> .tshut_temp = 95000,
> @@ -1572,7 +1607,7 @@ static int rockchip_configure_from_dt(struct device *dev,
> struct device_node *np,
> struct rockchip_thermal_data *thermal)
> {
> - u32 shut_temp, tshut_mode, tshut_polarity;
> + u32 shut_temp, tshut_mode, tshut_polarity, ret;
>
> if (of_property_read_u32(np, "rockchip,hw-tshut-temp", &shut_temp)) {
> dev_warn(dev,
> @@ -1621,12 +1656,16 @@ static int rockchip_configure_from_dt(struct device *dev,
> return -EINVAL;
> }
>
> - /* The tsadc wont to handle the error in here since some SoCs didn't
> - * need this property.
> - */
> - thermal->grf = syscon_regmap_lookup_by_phandle(np, "rockchip,grf");
> - if (IS_ERR(thermal->grf))
> - dev_warn(dev, "Missing rockchip,grf property\n");
> + if (thermal->chip->grf_mode != GRF_NONE) {
> + thermal->grf = syscon_regmap_lookup_by_phandle(np, "rockchip,grf");
> + if (IS_ERR(thermal->grf)) {
> + ret = PTR_ERR(thermal->grf);
> + if (thermal->chip->grf_mode == GRF_OPTIONAL)
> + dev_warn(dev, "Missing rockchip,grf property\n");
> + else
> + return dev_err_probe(dev, ret, "Missing rockchip,grf property\n");
> + }
> + }
Nit: Does the lookup itself need to be made conditional? I think I'd
also agree that the "optional" mode seems suspect, so potentially it
could be a whole lot simpler, e.g.:
thermal->grf = syscon_regmap_lookup_by_phandle(np, "rockchip,grf");
if (IS_ERR(thermal->grf) && thermal->chip->grf_required)
return dev_err_probe(dev, PTR_ERR(thermal->grf),
"Missing rockchip,grf property\n");
Thanks,
Robin.
>
> rockchip_get_trim_configuration(dev, np, thermal);
>
>
> ---
> base-commit: c17b750b3ad9f45f2b6f7e6f7f4679844244f0b9
> change-id: 20250818-thermal-rockchip-grf-warning-05f7f56286a2
>
> Best regards,
Hello Robin,
On Tue, Aug 19, 2025 at 01:42:42PM +0100, Robin Murphy wrote:
> > + if (thermal->chip->grf_mode != GRF_NONE) {
> > + thermal->grf = syscon_regmap_lookup_by_phandle(np, "rockchip,grf");
> > + if (IS_ERR(thermal->grf)) {
> > + ret = PTR_ERR(thermal->grf);
> > + if (thermal->chip->grf_mode == GRF_OPTIONAL)
> > + dev_warn(dev, "Missing rockchip,grf property\n");
> > + else
> > + return dev_err_probe(dev, ret, "Missing rockchip,grf property\n");
> > + }
> > + }
>
> Nit: Does the lookup itself need to be made conditional? I think I'd
> also agree that the "optional" mode seems suspect, so potentially it
> could be a whole lot simpler, e.g.:
>
> thermal->grf = syscon_regmap_lookup_by_phandle(np, "rockchip,grf");
> if (IS_ERR(thermal->grf) && thermal->chip->grf_required)
> return dev_err_probe(dev, PTR_ERR(thermal->grf),
> "Missing rockchip,grf property\n");
I came up with the enum, because I think most platforms having
"optional" GRF support actually require it, so I want to keep the
warning. At the same time I don't want to mark them as GRF required
at this point, since that is potentially a DT ABI break. It really
needs to be checked per platform in the TRM and/or by testing on
real HW. With this patch we can easily handle this platform by
platform by moving them from GRF_OPTIONAL to GRF_MANDATORY without
affecting the unchecked platforms. Also switching a platform from
optional to required needs to be reflected in the DT binding. So
this involves a lot of work. I think it makes sense to carry the
slightly complex check in the driver's probe routine for now.
Greetings,
-- Sebastian
>
> Thanks,
> Robin.
>
> > rockchip_get_trim_configuration(dev, np, thermal);
> >
> > ---
> > base-commit: c17b750b3ad9f45f2b6f7e6f7f4679844244f0b9
> > change-id: 20250818-thermal-rockchip-grf-warning-05f7f56286a2
> >
> > Best regards,
>
>
Am Dienstag, 19. August 2025, 15:56:42 Mitteleuropäische Sommerzeit schrieb Sebastian Reichel:
> Hello Robin,
>
> On Tue, Aug 19, 2025 at 01:42:42PM +0100, Robin Murphy wrote:
> > > + if (thermal->chip->grf_mode != GRF_NONE) {
> > > + thermal->grf = syscon_regmap_lookup_by_phandle(np, "rockchip,grf");
> > > + if (IS_ERR(thermal->grf)) {
> > > + ret = PTR_ERR(thermal->grf);
> > > + if (thermal->chip->grf_mode == GRF_OPTIONAL)
> > > + dev_warn(dev, "Missing rockchip,grf property\n");
> > > + else
> > > + return dev_err_probe(dev, ret, "Missing rockchip,grf property\n");
> > > + }
> > > + }
> >
> > Nit: Does the lookup itself need to be made conditional? I think I'd
> > also agree that the "optional" mode seems suspect, so potentially it
> > could be a whole lot simpler, e.g.:
> >
> > thermal->grf = syscon_regmap_lookup_by_phandle(np, "rockchip,grf");
> > if (IS_ERR(thermal->grf) && thermal->chip->grf_required)
> > return dev_err_probe(dev, PTR_ERR(thermal->grf),
> > "Missing rockchip,grf property\n");
>
> I came up with the enum, because I think most platforms having
> "optional" GRF support actually require it, so I want to keep the
> warning. At the same time I don't want to mark them as GRF required
> at this point, since that is potentially a DT ABI break. It really
> needs to be checked per platform in the TRM and/or by testing on
> real HW. With this patch we can easily handle this platform by
> platform by moving them from GRF_OPTIONAL to GRF_MANDATORY without
> affecting the unchecked platforms. Also switching a platform from
> optional to required needs to be reflected in the DT binding. So
> this involves a lot of work. I think it makes sense to carry the
> slightly complex check in the driver's probe routine for now.
I did go digging now ... there are 3 variants marked as "optional":
rk3366_tsadc_data -> rockchip,rk3366-tsadc
never added any DTS
rk3399_tsadc_data -> rockchip,rk3399-tsadc
commit 95c27ba7bd92 ("arm64: dts: rockchip: add thermal nodes for rk3399 SoCs")
added the tsdadc node together with its rockchip,grf reference
rk3568_tsadc_data rockchip,rk3568-tsadc
commit 1330875dc2a3 ("arm64: dts: rockchip: add rk3568 tsadc nodes")
added the tsdadc node together with its rockchip,grf reference
So none of the platforms had ever a phase where we had the tsadc without
its grf-reference. So making the GRF mandatory for those, will not create
breakage. We could even tighten the binding to make that explicit.
Heiko
Hi Sebastian, kernel test robot noticed the following build warnings: [auto build test WARNING on c17b750b3ad9f45f2b6f7e6f7f4679844244f0b9] url: https://github.com/intel-lab-lkp/linux/commits/Sebastian-Reichel/thermal-rockchip-shut-up-GRF-warning/20250819-012807 base: c17b750b3ad9f45f2b6f7e6f7f4679844244f0b9 patch link: https://lore.kernel.org/r/20250818-thermal-rockchip-grf-warning-v1-1-134152c97097%40kernel.org patch subject: [PATCH] thermal: rockchip: shut up GRF warning config: riscv-randconfig-001-20250819 (https://download.01.org/0day-ci/archive/20250819/202508191909.EIZEWxiJ-lkp@intel.com/config) compiler: clang version 22.0.0git (https://github.com/llvm/llvm-project 93d24b6b7b148c47a2fa228a4ef31524fa1d9f3f) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250819/202508191909.EIZEWxiJ-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202508191909.EIZEWxiJ-lkp@intel.com/ All warnings (new ones prefixed by >>): >> Warning: drivers/thermal/rockchip_thermal.c:133 struct member 'grf_mode' not described in 'rockchip_tsadc_chip' -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki
© 2016 - 2025 Red Hat, Inc.