> -----Original Message-----
> From: Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx>
> Sent: Monday, 3 February 2025 15:50
> To: Vadim Pasternak <vadimp@xxxxxxxxxx>
> Cc: Hans de Goede <hdegoede@xxxxxxxxxx>; Michael Shych
> <michaelsh@xxxxxxxxxx>; Ciju Rajan K <crajank@xxxxxxxxxx>; Felix Radensky
> <fradensky@xxxxxxxxxx>; Oleksandr Shamray <oleksandrs@xxxxxxxxxx>;
> platform-driver-x86@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH v5 04/12] platform_data/mlxreg: Add capability bit and
> mask fields
>
> On Fri, 24 Jan 2025, Vadim Pasternak wrote:
>
> > Some 'capability' registers can be shared between different resources.
> > Add new fields 'capability_bit' and 'capability_mask' to structs
> > 'mlxreg_core_data' and 'mlxreg_core_item' for getting only relevant
> > capability bits.
> >
> > Reviewed-by: Felix Radensky <fradensky@xxxxxxxxxx>
> > Signed-off-by: Vadim Pasternak <vadimp@xxxxxxxxxx>
> > ---
> > include/linux/platform_data/mlxreg.h | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/include/linux/platform_data/mlxreg.h
> > b/include/linux/platform_data/mlxreg.h
> > index 0b9f81a6f753..d9f679752226 100644
> > --- a/include/linux/platform_data/mlxreg.h
> > +++ b/include/linux/platform_data/mlxreg.h
> > @@ -118,6 +118,8 @@ struct mlxreg_hotplug_device {
> > * @mask: attribute access mask;
> > * @bit: attribute effective bit;
> > * @capability: attribute capability register;
> > + * @capability_bit: started bit in attribute capability register;
>
> "started bit" sounds a bit clumsy.
Hi Ilpo!
After revising the code, I will drop ' capability_bit' field.
Not used in any configuration.
Thanks,
Vadim.
>
> > + * @capability_mask: mask in attribute capability register;
> > * @reg_prsnt: attribute presence register;
> > * @reg_sync: attribute synch register;
> > * @reg_pwr: attribute power register; @@ -138,6 +140,8 @@ struct
> > mlxreg_core_data {
> > u32 mask;
> > u32 bit;
> > u32 capability;
> > + u32 capability_bit;
> > + u32 capability_mask;
> > u32 reg_prsnt;
> > u32 reg_sync;
> > u32 reg_pwr;
> > @@ -162,6 +166,8 @@ struct mlxreg_core_data {
> > * @reg: group interrupt status register;
> > * @mask: group interrupt mask;
> > * @capability: group capability register;
> > + * @capability_bit: started bit in attribute capability register;
> > + * @capability_mask: mask in attribute capability register;
>
> This is probably a bit misleading as you masked with it before shifting with
> capability_bit (in patch 5) so the mask seems zeroth-bit based rather than a
> "mask in attribute capability register" ?
>
> > * @cache: last status value for elements fro the same group;
> > * @count: number of available elements in the group;
> > * @ind: element's index inside the group; @@ -175,6 +181,8 @@ struct
> > mlxreg_core_item {
> > u32 reg;
> > u32 mask;
> > u32 capability;
> > + u32 capability_bit;
> > + u32 capability_mask;
> > u32 cache;
> > u8 count;
> > u8 ind;
> >
>
> --
> i.
