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

[Jacky Bai]

> 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?

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
> >