Hi Vadim,
I was no longer among the receipients despite being marked as M: for this
file. Also lkml is not there despite,
scripts/get_maintainer.pl -f drivers/platform/mellanox/mlxreg-hotplug.c
returning both so there's still something wrong in the way the receipients
are selected.
On Mon, 13 Jan 2025, Vadim Pasternak wrote:
> Hotplug platform data is common across the various systems, while
> hotplug driver should be able to configure only the instances relevant
> to specific system.
>
> For example, platform hoptplug data might contain descriptions for fan1,
> fan2, ..., fan{n}, while some systems equipped with all 'n' fans,
> others with less.
> Same for power units, power controllers, ASICs and so on.
>
> For detection of the real number of equipped devices capability
> registers are used.
> These registers used to indicate presence of hotplug devices through
> the bitmap.
>
> For some new big modular systems, these registers will provide presence
> by counters.
>
> Use slot parameter to determine whether capability register contains
> bitmask or counter.
>
> Some 'capability' registers can be shared between different resources.
> Use fields 'capability_bit' and 'capability_mask' for getting only
> relevant capability bits.
>
> Reviewed-by: Felix Radensky <fradensky@xxxxxxxxxx>
> Signed-off-by: Vadim Pasternak <vadimp@xxxxxxxxxx>
> ---
> drivers/platform/mellanox/mlxreg-hotplug.c | 23 ++++++++++++++++++++--
> 1 file changed, 21 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/platform/mellanox/mlxreg-hotplug.c b/drivers/platform/mellanox/mlxreg-hotplug.c
> index 0ce9fff1f7d4..3e480c322353 100644
> --- a/drivers/platform/mellanox/mlxreg-hotplug.c
> +++ b/drivers/platform/mellanox/mlxreg-hotplug.c
> @@ -274,6 +274,13 @@ static int mlxreg_hotplug_attr_init(struct mlxreg_hotplug_priv_data *priv)
> if (ret)
> return ret;
>
> + if (!regval)
> + continue;
> +
> + /* Remove non-relevant bits. */
> + if (item->capability_mask)
> + regval = rol32(regval & item->capability_mask,
> + item->capability_bit);
Is the intention here to really do _rotate_ bits or is it just normal
shifting going on? It might be the bits might never rotate past 32-bit
boundary so it is effectively just shifting but labeling it as rotate is
still wrong if bit rotate is not intended.
I see there are also two pre-existing rol32() calls inside
drivers/platform/mellanox/ with one of them talking about "shift" so I
suspect they might be also wrongly using rol32() that does rotate
> item->mask = GENMASK((regval & item->mask) - 1, 0);
> }
>
> @@ -294,7 +301,19 @@ static int mlxreg_hotplug_attr_init(struct mlxreg_hotplug_priv_data *priv)
> if (ret)
> return ret;
>
> - if (!(regval & data->bit)) {
> + /*
> + * In case slot field is provided, capability
> + * register contains counter, otherwise bitmask.
> + * Skip non-relevant entries if slot set and
> + * exceeds counter. Othewise validate entry by
> + * matching bitmask.
> + */
> + if (data->capability_mask)
> + regval = rol32(regval & data->capability_mask,
> + data->capability_bit);
Another rol32() here?
> + if (data->slot > regval) {
> + break;
> + } else if (!(regval & data->bit) && !data->slot) {
> data++;
> continue;
> }
> @@ -611,7 +630,7 @@ static int mlxreg_hotplug_set_irq(struct mlxreg_hotplug_priv_data *priv)
> if (ret)
> goto out;
>
> - if (!(regval & data->bit))
> + if (!(regval & data->bit) && !data->slot)
> item->mask &= ~BIT(j);
> }
> }
>
--
i.
