Hi Niklas,
On Wed, Nov 20, 2024 at 1:04 PM Niklas Söderlund
<niklas.soderlund+renesas@xxxxxxxxxxxx> wrote:
> The hardware calibration is fused on some, but not all, Gen3 and Gen4
> boards. The calibrations values are the same on both generations but
> located at different register offsets.
>
> Instead of having duplicated logic to read the and store the values
> create a helper function to do the reading and just feed it with the
> correct register addresses for each generation,
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@xxxxxxxxxxxx>
Thanks for your patch!
Reviewed-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
Still, some suggestions for improvement below...
> --- a/drivers/thermal/renesas/rcar_gen3_thermal.c
> +++ b/drivers/thermal/renesas/rcar_gen3_thermal.c
> @@ -253,60 +253,43 @@ static irqreturn_t rcar_gen3_thermal_irq(int irq, void *data)
> return IRQ_HANDLED;
> }
>
> +static void rcar_gen3_thermal_fetch_fuses(struct rcar_gen3_thermal_priv *priv,
> + u32 ptat0, u32 ptat1, u32 ptat2,
Perhaps ptat[1-3], to match REG_GEN3_PTAT[1-3]?
> + u32 thcode0, u32 thcode1, u32 thcode2,
> + u32 mask)
> +{
> static void rcar_gen3_thermal_read_fuses_gen3(struct rcar_gen3_thermal_priv *priv)
> {
[...]
> + rcar_gen3_thermal_fetch_fuses(priv,
> + REG_GEN3_PTAT1, REG_GEN3_PTAT2, REG_GEN3_PTAT3,
> + REG_GEN3_THCODE1, REG_GEN3_THCODE2, REG_GEN3_THCODE3,
> + GEN3_FUSE_MASK);
> }
>
> static void rcar_gen3_thermal_read_fuses_gen4(struct rcar_gen3_thermal_priv *priv)
> {
[...]
> + rcar_gen3_thermal_fetch_fuses(priv,
> + REG_GEN4_THSFMON16, REG_GEN4_THSFMON17, REG_GEN4_THSFMON15,
> + REG_GEN4_THSFMON01, REG_GEN4_THSFMON02, REG_GEN4_THSFMON00,
> + GEN4_FUSE_MASK);
> }
>
> static bool rcar_gen3_thermal_read_fuses(struct rcar_gen3_thermal_priv *priv)
As both rcar_gen3_thermal_read_fuses_gen[34] became wrappers around
rcar_gen3_thermal_fetch_fuses(), what about parameterizing by data
instead of by code? I.e. replace the rcar_thermal_info.read_fuse()
function pointer by a pointer to a structure?
struct rcar_gen3_thermal_fuse_info {
u32 ptat[3];
u32 thcode[3];
u32 mask;
};
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
