Re: [PATCH] platform/mellanox: Fix logic error in power state check in mlxreg-lc

[Ilpo Järvinen]

On Mon, 30 Jun 2025, Ilpo Järvinen wrote:
> On Mon, 30 Jun 2025, Alok Tiwari wrote:

Also please move mlxreg-lc into prefixes (see some earlier commits made 
into that file).

> > Fixes a logic issue in mlxreg_lc_completion_notify() where the
> > intention was to check if MLXREG_LC_POWERED flag is not set before
> > powering on the device.
> > 
> > The original code used "state & ~MLXREG_LC_POWERED" to check for the
> > absence of the POWERED bit. However this condition evaluates to true
> > even when other bits are set, leading to potentially incorrect
> > behavior.
> > 
> > Corrected the logic to explicitly check for the absence of
> > MLXREG_LC_POWERED using !(state & MLXREG_LC_POWERED).
> > 
> > Suggested-by: Vadim Pasternak <vadimp@xxxxxxxxxx>
> > Signed-off-by: Alok Tiwari <alok.a.tiwari@xxxxxxxxxx>
> 
> Makes sense but please add Fixes tag and resubmit as v2.
> 
> > ---
> >  drivers/platform/mellanox/mlxreg-lc.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/platform/mellanox/mlxreg-lc.c b/drivers/platform/mellanox/mlxreg-lc.c
> > index aee395bb48ae4..8681ceb7144ba 100644
> > --- a/drivers/platform/mellanox/mlxreg-lc.c
> > +++ b/drivers/platform/mellanox/mlxreg-lc.c
> > @@ -688,7 +688,7 @@ static int mlxreg_lc_completion_notify(void *handle, struct i2c_adapter *parent,
> >  	if (regval & mlxreg_lc->data->mask) {
> >  		mlxreg_lc->state |= MLXREG_LC_SYNCED;
> >  		mlxreg_lc_state_update_locked(mlxreg_lc, MLXREG_LC_SYNCED, 1);
> > -		if (mlxreg_lc->state & ~MLXREG_LC_POWERED) {
> > +		if (!(mlxreg_lc->state & MLXREG_LC_POWERED)) {
> >  			err = mlxreg_lc_power_on_off(mlxreg_lc, 1);
> >  			if (err)
> >  				goto mlxreg_lc_regmap_power_on_off_fail;
> > 
> 
> 

-- 
 i.