Re: [PATCH] hwmon: peci: dimmtemp: Retry unstable DIMM mask detection

[Guenter Roeck]

On Fri, Sep 18, 2026 at 11:18:31AM +0800, Jian Zhang wrote:
> Add a stable DIMM mask check around PECI DIMM discovery.
> 
> During host DIMM initialization, PECI can answer the DIMM temperature
> configuration reads before every DIMM has reached a stable ready state.
> There is no separate all-DIMM-ready signal for this driver to wait on,
> so a single early read can capture only part of the populated DIMM mask.
> Retrying the mask check avoids locking that transient state into the
> hwmon device layout.
> 
> The first successful scan is saved, and the driver immediately scans
> again before registering hwmon channels. If the two masks differ, the
> driver returns -EAGAIN so the existing delayed detection work can retry
> instead of exposing a partial channel set.
> 
> Signed-off-by: Jian Zhang <zhangjian.3032@xxxxxxxxxxxxx>
> ---
> 
> This was tested on an affected platform where probing during
> host DIMM initialization could expose an incomplete DIMM mask. With the
> extra read, the driver observed the completed DIMM mask and exported the
> expected hwmon channels.
> 
> This is intentionally a conservative retry of the existing detection
> path. It does not add a sleep or force another delayed-work interval
> between the two reads, so it avoids adding an unconditional probe delay.
> However, this also means it is not a full proof that the host DIMM
> initialization has completed.
> 
> I am not aware of an Intel-provided all-DIMM-ready indication for this
> path. Feedback would be appreciated if there is a better way to decide
> that the PECI DIMM mask is stable before registering the hwmon device.
> 
>  drivers/hwmon/peci/dimmtemp.c | 28 +++++++++++++++++++++++++++-
>  1 file changed, 27 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hwmon/peci/dimmtemp.c b/drivers/hwmon/peci/dimmtemp.c
> index bd3e8715dfec..983e1e415ec2 100644
> --- a/drivers/hwmon/peci/dimmtemp.c
> +++ b/drivers/hwmon/peci/dimmtemp.c
> @@ -285,6 +285,32 @@ static int check_populated_dimms(struct peci_dimmtemp *priv)
>  	return 0;
>  }
>  
> +static int check_populated_dimms_stable(struct peci_dimmtemp *priv)
> +{
> +	DECLARE_BITMAP(dimm_mask, DIMM_NUMS_MAX);
> +	int ret;
> +
> +	ret = check_populated_dimms(priv);
> +	if (ret)
> +		return ret;
> +
> +	bitmap_copy(dimm_mask, priv->dimm_mask, DIMM_NUMS_MAX);
> +
> +	ret = check_populated_dimms(priv);
> +	if (ret)
> +		return ret;
> +
> +	if (!bitmap_equal(dimm_mask, priv->dimm_mask, DIMM_NUMS_MAX)) {
> +		dev_dbg(priv->dev,
> +			"Deferred unstable DIMM mask: %*pbl -> %*pbl\n",
> +			DIMM_NUMS_MAX, dimm_mask, DIMM_NUMS_MAX,
> +			priv->dimm_mask);
> +		return -EAGAIN;
> +	}
> +
> +	return 0;
> +}

I agree with the idea, but check_populated_dimms() already has retry
support. I don't see why it would make sense to add retry detection
(because additional DIMMs were detected) on top of retry detection
(because no DIMMs were detected).

Thanks,
Guenter

> +
>  static int create_dimm_temp_label(struct peci_dimmtemp *priv, int chan)
>  {
>  	int rank = chan / priv->gen_info->dimm_idx_max;
> @@ -322,7 +348,7 @@ static int create_dimm_temp_info(struct peci_dimmtemp *priv)
>  	 * All other states mean that the platform never reached the state that
>  	 * allows to check DIMM state - causing us to retry later on.
>  	 */
> -	ret = check_populated_dimms(priv);
> +	ret = check_populated_dimms_stable(priv);
>  	if (ret == -ENODEV) {
>  		dev_dbg(priv->dev, "No DIMMs found\n");
>  		return 0;