2023-07-11 15:27:13

by Naresh Solanki

Subject: [PATCH] hwmon: (dimmtemp) Support more than 32 DIMMs

From: Patrick Rudolph <[email protected]>

This patch introduces support for handling more than 32 DIMMs by
utilizing bitmap operations. The changes ensure that the driver can
handle a higher number of DIMMs efficiently.

Signed-off-by: Patrick Rudolph <[email protected]>
---
drivers/hwmon/peci/dimmtemp.c | 26 +++++++++++++++-----------
1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/drivers/hwmon/peci/dimmtemp.c b/drivers/hwmon/peci/dimmtemp.c
index ed968401f93c..ce89da3937a0 100644
--- a/drivers/hwmon/peci/dimmtemp.c
+++ b/drivers/hwmon/peci/dimmtemp.c
@@ -219,19 +219,21 @@ 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;
- u32 dimm_mask = 0;
- int chan_rank, dimm_idx, ret;
+ DECLARE_BITMAP(dimm_mask, DIMM_NUMS_MAX);
+ DECLARE_BITMAP(chan_rank_empty, CHAN_RANK_MAX);
+
+ int chan_rank, dimm_idx, ret, i;
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;
}

+ bitmap_zero(dimm_mask, DIMM_NUMS_MAX);
+ bitmap_zero(chan_rank_empty, CHAN_RANK_MAX);
+
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) {
@@ -242,7 +244,7 @@ static int check_populated_dimms(struct peci_dimmtemp *priv)
* detection to be performed at a later point in time.
*/
if (ret == -EINVAL) {
- chan_rank_empty |= BIT(chan_rank);
+ bitmap_set(chan_rank_empty, chan_rank, 1);
continue;
}

@@ -251,7 +253,7 @@ static int check_populated_dimms(struct peci_dimmtemp *priv)

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);
+ bitmap_set(dimm_mask, chan_rank * dimm_idx_max + dimm_idx, 1);
}

/*
@@ -260,7 +262,7 @@ static int check_populated_dimms(struct peci_dimmtemp *priv)
* 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 (bitmap_full(chan_rank_empty, chan_rank_max)) {
if (priv->no_dimm_retry_count < NO_DIMM_RETRY_COUNT_MAX) {
priv->no_dimm_retry_count++;

@@ -274,14 +276,16 @@ static int check_populated_dimms(struct peci_dimmtemp *priv)
* It's possible that memory training is not done yet. In this case we
* defer the detection to be performed at a later point in time.
*/
- if (!dimm_mask) {
+ if (bitmap_empty(dimm_mask, DIMM_NUMS_MAX)) {
priv->no_dimm_retry_count = 0;
return -EAGAIN;
}

- dev_dbg(priv->dev, "Scanned populated DIMMs: %#x\n", dimm_mask);
+ for_each_set_bit(i, dimm_mask, DIMM_NUMS_MAX) {
+ dev_dbg(priv->dev, "Found DIMM%#x\n", i);
+ }

- bitmap_from_arr32(priv->dimm_mask, &dimm_mask, DIMM_NUMS_MAX);
+ bitmap_copy(priv->dimm_mask, dimm_mask, DIMM_NUMS_MAX);

return 0;
}

base-commit: 4dbbaf8fbdbd13adc80731b2452257857e4c2d8b
--
2.41.0

2023-07-15 17:07:49

by Guenter Roeck

Subject: Re: [PATCH] hwmon: (dimmtemp) Support more than 32 DIMMs

On Tue, Jul 11, 2023 at 05:21:43PM +0200, Naresh Solanki wrote:
> From: Patrick Rudolph <[email protected]>
>
> This patch introduces support for handling more than 32 DIMMs by
> utilizing bitmap operations. The changes ensure that the driver can
> handle a higher number of DIMMs efficiently.
>
> Signed-off-by: Patrick Rudolph <[email protected]>

Applied to hwmon-next.

Note: I got multiple versions of this patch. In the future
please version your patches and provide change logs.

Thanks,
Guenter

> ---
> drivers/hwmon/peci/dimmtemp.c | 26 +++++++++++++++-----------
> 1 file changed, 15 insertions(+), 11 deletions(-)
>
>
> base-commit: 4dbbaf8fbdbd13adc80731b2452257857e4c2d8b
>
> diff --git a/drivers/hwmon/peci/dimmtemp.c b/drivers/hwmon/peci/dimmtemp.c
> index ed968401f93c..ce89da3937a0 100644
> --- a/drivers/hwmon/peci/dimmtemp.c
> +++ b/drivers/hwmon/peci/dimmtemp.c
> @@ -219,19 +219,21 @@ 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;
> - u32 dimm_mask = 0;
> - int chan_rank, dimm_idx, ret;
> + DECLARE_BITMAP(dimm_mask, DIMM_NUMS_MAX);
> + DECLARE_BITMAP(chan_rank_empty, CHAN_RANK_MAX);
> +
> + int chan_rank, dimm_idx, ret, i;
> 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;
> }
>
> + bitmap_zero(dimm_mask, DIMM_NUMS_MAX);
> + bitmap_zero(chan_rank_empty, CHAN_RANK_MAX);
> +
> 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) {
> @@ -242,7 +244,7 @@ static int check_populated_dimms(struct peci_dimmtemp *priv)
> * detection to be performed at a later point in time.
> */
> if (ret == -EINVAL) {
> - chan_rank_empty |= BIT(chan_rank);
> + bitmap_set(chan_rank_empty, chan_rank, 1);
> continue;
> }
>
> @@ -251,7 +253,7 @@ static int check_populated_dimms(struct peci_dimmtemp *priv)
>
> 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);
> + bitmap_set(dimm_mask, chan_rank * dimm_idx_max + dimm_idx, 1);
> }
>
> /*
> @@ -260,7 +262,7 @@ static int check_populated_dimms(struct peci_dimmtemp *priv)
> * 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 (bitmap_full(chan_rank_empty, chan_rank_max)) {
> if (priv->no_dimm_retry_count < NO_DIMM_RETRY_COUNT_MAX) {
> priv->no_dimm_retry_count++;
>
> @@ -274,14 +276,16 @@ static int check_populated_dimms(struct peci_dimmtemp *priv)
> * It's possible that memory training is not done yet. In this case we
> * defer the detection to be performed at a later point in time.
> */
> - if (!dimm_mask) {
> + if (bitmap_empty(dimm_mask, DIMM_NUMS_MAX)) {
> priv->no_dimm_retry_count = 0;
> return -EAGAIN;
> }
>
> - dev_dbg(priv->dev, "Scanned populated DIMMs: %#x\n", dimm_mask);
> + for_each_set_bit(i, dimm_mask, DIMM_NUMS_MAX) {
> + dev_dbg(priv->dev, "Found DIMM%#x\n", i);
> + }
>
> - bitmap_from_arr32(priv->dimm_mask, &dimm_mask, DIMM_NUMS_MAX);
> + bitmap_copy(priv->dimm_mask, dimm_mask, DIMM_NUMS_MAX);
>
> return 0;
> }

2023-07-17 10:34:00

by Naresh Solanki

Subject: Re: [PATCH] hwmon: (dimmtemp) Support more than 32 DIMMs

Hi Guenter

On Sat, 15 Jul 2023 at 22:28, Guenter Roeck <[email protected]> wrote:
>
> On Tue, Jul 11, 2023 at 05:21:43PM +0200, Naresh Solanki wrote:
> > From: Patrick Rudolph <[email protected]>
> >
> > This patch introduces support for handling more than 32 DIMMs by
> > utilizing bitmap operations. The changes ensure that the driver can
> > handle a higher number of DIMMs efficiently.
> >
> > Signed-off-by: Patrick Rudolph <[email protected]>
>
> Applied to hwmon-next.
>
> Note: I got multiple versions of this patch. In the future
> please version your patches and provide change logs.
Sure thing. Thanks
~Naresh
>
> Thanks,
> Guenter
>
> > ---
> > drivers/hwmon/peci/dimmtemp.c | 26 +++++++++++++++-----------
> > 1 file changed, 15 insertions(+), 11 deletions(-)
> >
> >
> > base-commit: 4dbbaf8fbdbd13adc80731b2452257857e4c2d8b
> >
> > diff --git a/drivers/hwmon/peci/dimmtemp.c b/drivers/hwmon/peci/dimmtemp.c
> > index ed968401f93c..ce89da3937a0 100644
> > --- a/drivers/hwmon/peci/dimmtemp.c
> > +++ b/drivers/hwmon/peci/dimmtemp.c
> > @@ -219,19 +219,21 @@ 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;
> > - u32 dimm_mask = 0;
> > - int chan_rank, dimm_idx, ret;
> > + DECLARE_BITMAP(dimm_mask, DIMM_NUMS_MAX);
> > + DECLARE_BITMAP(chan_rank_empty, CHAN_RANK_MAX);
> > +
> > + int chan_rank, dimm_idx, ret, i;
> > 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;
> > }
> >
> > + bitmap_zero(dimm_mask, DIMM_NUMS_MAX);
> > + bitmap_zero(chan_rank_empty, CHAN_RANK_MAX);
> > +
> > 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) {
> > @@ -242,7 +244,7 @@ static int check_populated_dimms(struct peci_dimmtemp *priv)
> > * detection to be performed at a later point in time.
> > */
> > if (ret == -EINVAL) {
> > - chan_rank_empty |= BIT(chan_rank);
> > + bitmap_set(chan_rank_empty, chan_rank, 1);
> > continue;
> > }
> >
> > @@ -251,7 +253,7 @@ static int check_populated_dimms(struct peci_dimmtemp *priv)
> >
> > 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);
> > + bitmap_set(dimm_mask, chan_rank * dimm_idx_max + dimm_idx, 1);
> > }
> >
> > /*
> > @@ -260,7 +262,7 @@ static int check_populated_dimms(struct peci_dimmtemp *priv)
> > * 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 (bitmap_full(chan_rank_empty, chan_rank_max)) {
> > if (priv->no_dimm_retry_count < NO_DIMM_RETRY_COUNT_MAX) {
> > priv->no_dimm_retry_count++;
> >
> > @@ -274,14 +276,16 @@ static int check_populated_dimms(struct peci_dimmtemp *priv)
> > * It's possible that memory training is not done yet. In this case we
> > * defer the detection to be performed at a later point in time.
> > */
> > - if (!dimm_mask) {
> > + if (bitmap_empty(dimm_mask, DIMM_NUMS_MAX)) {
> > priv->no_dimm_retry_count = 0;
> > return -EAGAIN;
> > }
> >
> > - dev_dbg(priv->dev, "Scanned populated DIMMs: %#x\n", dimm_mask);
> > + for_each_set_bit(i, dimm_mask, DIMM_NUMS_MAX) {
> > + dev_dbg(priv->dev, "Found DIMM%#x\n", i);
> > + }
> >
> > - bitmap_from_arr32(priv->dimm_mask, &dimm_mask, DIMM_NUMS_MAX);
> > + bitmap_copy(priv->dimm_mask, dimm_mask, DIMM_NUMS_MAX);
> >
> > return 0;
> > }