platform/mellanox: Fix logic error in power state check in mlxreg-lc

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

Posted by Alok Tiwari 3 months, 1 week ago

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@nvidia.com>
Signed-off-by: Alok Tiwari <alok.a.tiwari@oracle.com>
---
 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;
-- 
2.46.0

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

Posted by Ilpo Järvinen 3 months, 1 week ago

On Mon, 30 Jun 2025, Alok Tiwari wrote:

> 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@nvidia.com>
> Signed-off-by: Alok Tiwari <alok.a.tiwari@oracle.com>

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.

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

Posted by Ilpo Järvinen 3 months, 1 week ago

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@nvidia.com>
> > Signed-off-by: Alok Tiwari <alok.a.tiwari@oracle.com>
> 
> 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.