Hi Marek,
Thanks for your work!
On 2025-06-25 11:59:58 +0200, Marek Vasut wrote:
> The Working Sample R-Car SoCs may not yet have thermal sensor trimming
> values programmed into fuses, those fuses are blank instead. For such
> SoCs, the driver includes fallback trimming values. Those values are
> currently applied to all SoCs which use this driver.
>
> Introduce support for per-SoC fallback trimming values in preparation
> for SoCs which do not use these current trimming values. No functional
> change is intended here.
I like the change, only have one bikeshedding comment about naming.
>
> Signed-off-by: Marek Vasut <marek.vasut+renesas@xxxxxxxxxxx>
> ---
> Cc: "Niklas Söderlund" <niklas.soderlund@xxxxxxxxxxxx>
> Cc: "Rafael J. Wysocki" <rafael@xxxxxxxxxx>
> Cc: Daniel Lezcano <daniel.lezcano@xxxxxxxxxx>
> Cc: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
> Cc: Lukasz Luba <lukasz.luba@xxxxxxx>
> Cc: Magnus Damm <magnus.damm@xxxxxxxxx>
> Cc: Zhang Rui <rui.zhang@xxxxxxxxx>
> Cc: linux-pm@xxxxxxxxxxxxxxx
> Cc: linux-renesas-soc@xxxxxxxxxxxxxxx
> ---
> drivers/thermal/renesas/rcar_gen3_thermal.c | 42 ++++++++++++++-------
> 1 file changed, 28 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/thermal/renesas/rcar_gen3_thermal.c b/drivers/thermal/renesas/rcar_gen3_thermal.c
> index 24a702ee4c1f..a388bd3135e4 100644
> --- a/drivers/thermal/renesas/rcar_gen3_thermal.c
> +++ b/drivers/thermal/renesas/rcar_gen3_thermal.c
> @@ -73,11 +73,17 @@ struct rcar_gen3_thermal_fuse_info {
> u32 mask;
> };
>
> +struct rcar_gen3_thermal_fuse_default_info {
Maybe call this 'rcar_gen3_thermal_fuse_default' to reduce the symbol
length. In retrospect picking the preifx 'rcar_gen3_thermal_' was a
really bad idea on my part...
> + u32 ptat[3];
> + u32 thcodes[TSC_MAX_NUM][3];
> +};
> +
> struct rcar_thermal_info {
> int scale;
> int adj_below;
> int adj_above;
> const struct rcar_gen3_thermal_fuse_info *fuses;
> + const struct rcar_gen3_thermal_fuse_default_info *fuse_defaults;
> };
>
> struct equation_set_coef {
> @@ -289,6 +295,7 @@ static void rcar_gen3_thermal_fetch_fuses(struct rcar_gen3_thermal_priv *priv)
>
> static bool rcar_gen3_thermal_read_fuses(struct rcar_gen3_thermal_priv *priv)
> {
> + const struct rcar_gen3_thermal_fuse_default_info *fuse_defaults = priv->info->fuse_defaults;
> unsigned int i;
> u32 thscp;
>
> @@ -297,24 +304,16 @@ static bool rcar_gen3_thermal_read_fuses(struct rcar_gen3_thermal_priv *priv)
> if (!priv->info->fuses ||
> (thscp & THSCP_COR_PARA_VLD) != THSCP_COR_PARA_VLD) {
> /* Default THCODE values in case FUSEs are not set. */
> - static const int thcodes[TSC_MAX_NUM][3] = {
> - { 3397, 2800, 2221 },
> - { 3393, 2795, 2216 },
> - { 3389, 2805, 2237 },
> - { 3415, 2694, 2195 },
> - { 3356, 2724, 2244 },
> - };
> -
> - priv->ptat[0] = 2631;
> - priv->ptat[1] = 1509;
> - priv->ptat[2] = 435;
> + priv->ptat[0] = fuse_defaults->ptat[0];
> + priv->ptat[1] = fuse_defaults->ptat[1];
> + priv->ptat[2] = fuse_defaults->ptat[2];
>
> for (i = 0; i < priv->num_tscs; i++) {
> struct rcar_gen3_thermal_tsc *tsc = priv->tscs[i];
>
> - tsc->thcode[0] = thcodes[i][0];
> - tsc->thcode[1] = thcodes[i][1];
> - tsc->thcode[2] = thcodes[i][2];
> + tsc->thcode[0] = fuse_defaults->thcodes[i][0];
> + tsc->thcode[1] = fuse_defaults->thcodes[i][1];
> + tsc->thcode[2] = fuse_defaults->thcodes[i][2];
> }
>
> return false;
> @@ -361,11 +360,24 @@ static const struct rcar_gen3_thermal_fuse_info rcar_gen3_thermal_fuse_info_gen4
> .mask = GEN4_FUSE_MASK,
> };
>
> +static const struct rcar_gen3_thermal_fuse_default_info
> + rcar_gen3_thermal_fuse_default_info_gen3 = {
With the names adjusted above this could be
static const struct rcar_gen3_thermal_fuse_default rcar_gen3_thermal_fuses_default_gen3 = {
And that would fit the 100 char limit. We have lines that are 100 chars
long already in the file, so this is fine IMHO. Again having such a long
prefix string was a bad idea, sorry about that.
With this addressed,
Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@xxxxxxxxxxxx>
> + .ptat = { 2631, 1509, 435 },
> + .thcodes = {
> + { 3397, 2800, 2221 },
> + { 3393, 2795, 2216 },
> + { 3389, 2805, 2237 },
> + { 3415, 2694, 2195 },
> + { 3356, 2724, 2244 },
> + },
> +};
> +
> static const struct rcar_thermal_info rcar_m3w_thermal_info = {
> .scale = 157,
> .adj_below = -41,
> .adj_above = 116,
> .fuses = &rcar_gen3_thermal_fuse_info_gen3,
> + .fuse_defaults = &rcar_gen3_thermal_fuse_default_info_gen3,
> };
>
> static const struct rcar_thermal_info rcar_gen3_thermal_info = {
> @@ -373,6 +385,7 @@ static const struct rcar_thermal_info rcar_gen3_thermal_info = {
> .adj_below = -41,
> .adj_above = 126,
> .fuses = &rcar_gen3_thermal_fuse_info_gen3,
> + .fuse_defaults = &rcar_gen3_thermal_fuse_default_info_gen3,
> };
>
> static const struct rcar_thermal_info rcar_gen4_thermal_info = {
> @@ -380,6 +393,7 @@ static const struct rcar_thermal_info rcar_gen4_thermal_info = {
> .adj_below = -41,
> .adj_above = 126,
> .fuses = &rcar_gen3_thermal_fuse_info_gen4,
> + .fuse_defaults = &rcar_gen3_thermal_fuse_default_info_gen3,
> };
>
> static const struct of_device_id rcar_gen3_thermal_dt_ids[] = {
> --
> 2.47.2
>
--
Kind Regards,
Niklas Söderlund
