Re: [PATCH v5 3/4] thermal: qoriq: workaround unexpected temperature readings from tmu

[Frank Li <Frank.li@nxp.com>]

On Tue, Apr 21, 2026 at 09:56:18AM +0000, Jacky Bai wrote:
> > Subject: Re: [PATCH v5 3/4] thermal: qoriq: workaround unexpected
> > temperature readings from tmu
> >
>
> [...]
>
> > >
> > > +	/* ERR052243: If a raising or falling edge happens, try later */
> > > +	if (qoriq_tmu_has_errata(qdata->drvdata, TMU_ERR052243)) {
> > > +		regmap_read(qdata->regmap, REGS_TIDR, &tidr);
> > > +		if (tidr & TEMP_RATE_IRQ_MASK) {
> > > +			regmap_write(qdata->regmap, REGS_TIDR,
> > TEMP_RATE_IRQ_MASK);
> > > +			return -EAGAIN;
> > > +		}
> > > +	}
> > > +
> > >  	if (regmap_read_poll_timeout(qdata->regmap,
> > >  				     REGS_TRITSR(qsensor->id),
> > >  				     val,
> > > @@ -130,6 +159,15 @@ static int tmu_get_temp(struct
> > thermal_zone_device *tz, int *temp)
> > >  				     10 * USEC_PER_MSEC))
> > >  		return -ENODATA;
> > >
> > > +	/*ERR052243: If a raising or falling edge happens, try later */
> > > +	if (qoriq_tmu_has_errata(qdata->drvdata, TMU_ERR052243)) {
> > > +		regmap_read(qdata->regmap, REGS_TIDR, &tidr);
> > > +		if (tidr & TEMP_RATE_IRQ_MASK) {
> > > +			regmap_write(qdata->regmap, REGS_TIDR,
> > TEMP_RATE_IRQ_MASK);
> > > +			return -EAGAIN;
> > > +		}
> > > +	}
> > > +
> >
> > The above two code blocks are the same. Use helper function to avoid
> > duplicat code. And why need check twice?
> >
> > 	ret = qoriq_tmu_err052243()
> > 	if (ret)
> > 		return ret;
> >
>
> The first check is to make sure no error already happened. if any error, no need
> to polling read the sensor value from tmu, return early to save time. The secondary
> check is to make sure the sensor value just read out is still a valid one as the error may
> happen after the first check.
>
> It should be ok to remove the first check. As the error only happens in some rare
> corner case, the sensor read polling delay is acceptable.
>
> If only keep the secondary check, I think no need to wrap this code block into a helper function?

Yes

Frank

>
> BR
>
> > Frank
> >
> > >  	if (qdata->ver == TMU_VER1) {
> > >  		*temp = (val & GENMASK(7, 0)) * MILLIDEGREE_PER_DEGREE;
> > >  	} else {
> > > @@ -247,6 +285,14 @@ static void qoriq_tmu_init_device(struct
> > qoriq_tmu_data *data)
> > >  		regmap_write(data->regmap, REGS_V2_TEUMR(0), teumr0_val);
> > >  	}
> > >
> > > +	/* ERR052243: Set the raising & falling edge monitor */
> > > +	if (qoriq_tmu_has_errata(data->drvdata, TMU_ERR052243)) {
> > > +		regmap_write(data->regmap, TMRTRCTR, TMRTRCTR_EN |
> > > +			     FIELD_PREP(TMRTRCTR_TEMP_MASK,
> > TEMP_RATE_THR_LVL));
> > > +		regmap_write(data->regmap, TMFTRCTR, TMFTRCTR_EN |
> > > +			     FIELD_PREP(TMFTRCTR_TEMP_MASK,
> > TEMP_RATE_THR_LVL));
> > > +
> > > +	}
> > >  	/* Disable monitoring */
> > >  	regmap_write(data->regmap, REGS_TMR, TMR_DISABLE);  } @@
> > -400,6
> > > +446,7 @@ static const struct tmu_drvdata imx8mq_tmu_data = {
> > >
> > >  static const struct tmu_drvdata imx93_data = {
> > >  	.teumr0 = TEUMR0_V21,
> > > +	.tmu_errata = TMU_ERR052243,
> > >  };
> > >
> > >  static const struct of_device_id qoriq_tmu_match[] = {
> > >
> > > --
> > > 2.34.1
> > >