[PATCH v4 11/13] hwmon: peci: Add dimmtemp driver

On Tue, 2021-11-23 at 07:56 -0800, Guenter Roeck wrote:

On Tue, Nov 23, 2021 at 03:07:04PM +0100, Iwona Winiarska wrote:

quoted

Add peci-dimmtemp driver for Temperature Sensor on DIMM readings that
are accessible via the processor PECI interface.

The main use case for the driver (and PECI interface) is out-of-band
management, where we're able to obtain thermal readings from an external
entity connected with PECI, e.g. BMC on server platforms.

Co-developed-by: Jae Hyun Yoo <redacted>
Signed-off-by: Jae Hyun Yoo <redacted>
Signed-off-by: Iwona Winiarska <iwona.winiarska@intel.com>
Reviewed-by: Pierre-Louis Bossart <redacted>
---

[ ... ]

quoted

+static int check_populated_dimms(struct peci_dimmtemp *priv)
+{
+???????int chan_rank_max = priv->gen_info->chan_rank_max;
+???????int dimm_idx_max = priv->gen_info->dimm_idx_max;
+???????u32 chan_rank_empty = 0;
+???????u64 dimm_mask = 0;
+???????int chan_rank, dimm_idx, ret;
+???????u32 pcs;
+
+???????BUILD_BUG_ON(BITS_PER_TYPE(chan_rank_empty) < CHAN_RANK_MAX);
+???????BUILD_BUG_ON(BITS_PER_TYPE(dimm_mask) < DIMM_NUMS_MAX);
+???????if (chan_rank_max * dimm_idx_max > DIMM_NUMS_MAX) {
+???????????????WARN_ONCE(1, "Unsupported number of DIMMs - chan_rank_max:
%d, dimm_idx_max: %d",
+???????????????????????? chan_rank_max, dimm_idx_max);
+???????????????return -EINVAL;
+???????}
+
+???????for (chan_rank = 0; chan_rank < chan_rank_max; chan_rank++) {
+???????????????ret = peci_pcs_read(priv->peci_dev, PECI_PCS_DDR_DIMM_TEMP,
chan_rank, &pcs);
+???????????????if (ret) {
+???????????????????????/*
+??????????????????????? * Overall, we expect either success or -EINVAL in
+??????????????????????? * order to determine whether DIMM is populated or
not.
+??????????????????????? * For anything else we fall back to deferring the
+??????????????????????? * detection to be performed at a later point in
time.
+??????????????????????? */
+???????????????????????if (ret == -EINVAL) {
+???????????????????????????????chan_rank_empty |= BIT(chan_rank);
+???????????????????????????????continue;
+???????????????????????}
+
+???????????????????????return -EAGAIN;
+???????????????}
+
+???????????????for (dimm_idx = 0; dimm_idx < dimm_idx_max; dimm_idx++)
+???????????????????????if (__dimm_temp(pcs, dimm_idx))
+???????????????????????????????dimm_mask |= BIT(chan_rank * dimm_idx_max +
dimm_idx);
+???????}
+
+???????/*
+??????? * If we got all -EINVALs, it means that the CPU doesn't have any
+??????? * DIMMs. Unfortunately, it may also happen at the very start of
+??????? * host platform boot. Retrying a couple of times lets us make sure
+??????? * that the state is persistent.
+??????? */
+???????if (chan_rank_empty == GENMASK(chan_rank_max - 1, 0)) {
+???????????????if (priv->no_dimm_retry_count < NO_DIMM_RETRY_COUNT_MAX) {
+???????????????????????priv->no_dimm_retry_count++;
+
+???????????????????????return -EAGAIN;
+???????????????} else {
+???????????????????????return -ENODEV;
+???????????????}

Static analyzers will complain "else after return is unnecessary".

I'll fix this in v5.

Thanks
-Iwona

Guenter