linux-hwmon.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH hwmon-next v1 1/1] hwmon: (mlxreg-fan) Fix macros for tacho fault reading
@ 2018-11-16 13:47 Vadim Pasternak
  2018-11-16 16:10 ` Guenter Roeck
  2018-11-16 16:15 ` Guenter Roeck
  0 siblings, 2 replies; 8+ messages in thread
From: Vadim Pasternak @ 2018-11-16 13:47 UTC (permalink / raw)
  To: linux; +Cc: linux-hwmon, michaelsh, Vadim Pasternak

Fix macros for tacometer fault reading.
This fix is relevant for three Mellanox systems MQMB7, MSN37, MSN34,
which are about to be released to the customers.
At the moment, none of them is at customers sites.

Fixes: 65afb4c8e7e4 ("hwmon: (mlxreg-fan) Add support for Mellanox FAN driver")
Signed-off-by: Vadim Pasternak <vadimp@mellanox.com>
---
 drivers/hwmon/mlxreg-fan.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hwmon/mlxreg-fan.c b/drivers/hwmon/mlxreg-fan.c
index de46577..d8fa4be 100644
--- a/drivers/hwmon/mlxreg-fan.c
+++ b/drivers/hwmon/mlxreg-fan.c
@@ -51,7 +51,7 @@
  */
 #define MLXREG_FAN_GET_RPM(rval, d, s)	(DIV_ROUND_CLOSEST(15000000 * 100, \
 					 ((rval) + (s)) * (d)))
-#define MLXREG_FAN_GET_FAULT(val, mask) (!!((val) ^ (mask)))
+#define MLXREG_FAN_GET_FAULT(val, mask) (!((val) ^ (mask)))
 #define MLXREG_FAN_PWM_DUTY2STATE(duty)	(DIV_ROUND_CLOSEST((duty) *	\
 					 MLXREG_FAN_MAX_STATE,		\
 					 MLXREG_FAN_MAX_DUTY))
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH hwmon-next v1 1/1] hwmon: (mlxreg-fan) Fix macros for tacho fault reading
  2018-11-16 13:47 [PATCH hwmon-next v1 1/1] hwmon: (mlxreg-fan) Fix macros for tacho fault reading Vadim Pasternak
@ 2018-11-16 16:10 ` Guenter Roeck
  2018-11-16 16:15 ` Guenter Roeck
  1 sibling, 0 replies; 8+ messages in thread
From: Guenter Roeck @ 2018-11-16 16:10 UTC (permalink / raw)
  To: Vadim Pasternak; +Cc: linux-hwmon, michaelsh

On Fri, Nov 16, 2018 at 01:47:11PM +0000, Vadim Pasternak wrote:
> Fix macros for tacometer fault reading.
> This fix is relevant for three Mellanox systems MQMB7, MSN37, MSN34,
> which are about to be released to the customers.
> At the moment, none of them is at customers sites.
> 
> Fixes: 65afb4c8e7e4 ("hwmon: (mlxreg-fan) Add support for Mellanox FAN driver")
> Signed-off-by: Vadim Pasternak <vadimp@mellanox.com>

Applied.

Thanks,
Guenter

> ---
>  drivers/hwmon/mlxreg-fan.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/hwmon/mlxreg-fan.c b/drivers/hwmon/mlxreg-fan.c
> index de46577..d8fa4be 100644
> --- a/drivers/hwmon/mlxreg-fan.c
> +++ b/drivers/hwmon/mlxreg-fan.c
> @@ -51,7 +51,7 @@
>   */
>  #define MLXREG_FAN_GET_RPM(rval, d, s)	(DIV_ROUND_CLOSEST(15000000 * 100, \
>  					 ((rval) + (s)) * (d)))
> -#define MLXREG_FAN_GET_FAULT(val, mask) (!!((val) ^ (mask)))
> +#define MLXREG_FAN_GET_FAULT(val, mask) (!((val) ^ (mask)))
>  #define MLXREG_FAN_PWM_DUTY2STATE(duty)	(DIV_ROUND_CLOSEST((duty) *	\
>  					 MLXREG_FAN_MAX_STATE,		\
>  					 MLXREG_FAN_MAX_DUTY))

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH hwmon-next v1 1/1] hwmon: (mlxreg-fan) Fix macros for tacho fault reading
  2018-11-16 13:47 [PATCH hwmon-next v1 1/1] hwmon: (mlxreg-fan) Fix macros for tacho fault reading Vadim Pasternak
  2018-11-16 16:10 ` Guenter Roeck
@ 2018-11-16 16:15 ` Guenter Roeck
  2018-11-19  6:07   ` Vadim Pasternak
  1 sibling, 1 reply; 8+ messages in thread
From: Guenter Roeck @ 2018-11-16 16:15 UTC (permalink / raw)
  To: Vadim Pasternak; +Cc: linux-hwmon, michaelsh

On Fri, Nov 16, 2018 at 01:47:11PM +0000, Vadim Pasternak wrote:
> Fix macros for tacometer fault reading.
> This fix is relevant for three Mellanox systems MQMB7, MSN37, MSN34,
> which are about to be released to the customers.
> At the moment, none of them is at customers sites.
> 
> Fixes: 65afb4c8e7e4 ("hwmon: (mlxreg-fan) Add support for Mellanox FAN driver")
> Signed-off-by: Vadim Pasternak <vadimp@mellanox.com>
> ---
>  drivers/hwmon/mlxreg-fan.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/hwmon/mlxreg-fan.c b/drivers/hwmon/mlxreg-fan.c
> index de46577..d8fa4be 100644
> --- a/drivers/hwmon/mlxreg-fan.c
> +++ b/drivers/hwmon/mlxreg-fan.c
> @@ -51,7 +51,7 @@
>   */
>  #define MLXREG_FAN_GET_RPM(rval, d, s)	(DIV_ROUND_CLOSEST(15000000 * 100, \
>  					 ((rval) + (s)) * (d)))
> -#define MLXREG_FAN_GET_FAULT(val, mask) (!!((val) ^ (mask)))
> +#define MLXREG_FAN_GET_FAULT(val, mask) (!((val) ^ (mask)))

You might want to check if the "^" is correct here. It looks fishy,
since the expression only evaluates to true if a no bit of val outside
mask is set. I would have expected "&".

Guenter

>  #define MLXREG_FAN_PWM_DUTY2STATE(duty)	(DIV_ROUND_CLOSEST((duty) *	\
>  					 MLXREG_FAN_MAX_STATE,		\
>  					 MLXREG_FAN_MAX_DUTY))

^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [PATCH hwmon-next v1 1/1] hwmon: (mlxreg-fan) Fix macros for tacho fault reading
  2018-11-16 16:15 ` Guenter Roeck
@ 2018-11-19  6:07   ` Vadim Pasternak
  2018-11-19  6:07     ` Vadim Pasternak
  2018-11-19 16:15     ` Guenter Roeck
  0 siblings, 2 replies; 8+ messages in thread
From: Vadim Pasternak @ 2018-11-19  6:07 UTC (permalink / raw)
  To: Guenter Roeck; +Cc: linux-hwmon, Michael Shych



> -----Original Message-----
> From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck
> Sent: Friday, November 16, 2018 6:15 PM
> To: Vadim Pasternak <vadimp@mellanox.com>
> Cc: linux-hwmon@vger.kernel.org; Michael Shych <michaelsh@mellanox.com>
> Subject: Re: [PATCH hwmon-next v1 1/1] hwmon: (mlxreg-fan) Fix macros for
> tacho fault reading
> 
> On Fri, Nov 16, 2018 at 01:47:11PM +0000, Vadim Pasternak wrote:
> > Fix macros for tacometer fault reading.
> > This fix is relevant for three Mellanox systems MQMB7, MSN37, MSN34,
> > which are about to be released to the customers.
> > At the moment, none of them is at customers sites.
> >
> > Fixes: 65afb4c8e7e4 ("hwmon: (mlxreg-fan) Add support for Mellanox FAN
> > driver")
> > Signed-off-by: Vadim Pasternak <vadimp@mellanox.com>
> > ---
> >  drivers/hwmon/mlxreg-fan.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/hwmon/mlxreg-fan.c b/drivers/hwmon/mlxreg-fan.c
> > index de46577..d8fa4be 100644
> > --- a/drivers/hwmon/mlxreg-fan.c
> > +++ b/drivers/hwmon/mlxreg-fan.c
> > @@ -51,7 +51,7 @@
> >   */
> >  #define MLXREG_FAN_GET_RPM(rval, d, s)
> 	(DIV_ROUND_CLOSEST(15000000 * 100, \
> >  					 ((rval) + (s)) * (d)))
> > -#define MLXREG_FAN_GET_FAULT(val, mask) (!!((val) ^ (mask)))
> > +#define MLXREG_FAN_GET_FAULT(val, mask) (!((val) ^ (mask)))
> 
> You might want to check if the "^" is correct here. It looks fishy, since the
> expression only evaluates to true if a no bit of val outside mask is set. I would
> have expected "&".
> 
> Guenter

Hi Guenter,

Thanks for you your comment.

I'll follow your suggestion and change a macros to" 
#define MLXREG_FAN_GET_FAULT(val, mask) (!((val & mask) ^ (mask)))

> 
> >  #define MLXREG_FAN_PWM_DUTY2STATE(duty)
> 	(DIV_ROUND_CLOSEST((duty) *	\
> >  					 MLXREG_FAN_MAX_STATE,
> 	\
> >  					 MLXREG_FAN_MAX_DUTY))

^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [PATCH hwmon-next v1 1/1] hwmon: (mlxreg-fan) Fix macros for tacho fault reading
  2018-11-19  6:07   ` Vadim Pasternak
@ 2018-11-19  6:07     ` Vadim Pasternak
  2018-11-19 16:15     ` Guenter Roeck
  1 sibling, 0 replies; 8+ messages in thread
From: Vadim Pasternak @ 2018-11-19  6:07 UTC (permalink / raw)
  To: Guenter Roeck; +Cc: linux-hwmon, Michael Shych



> -----Original Message-----
> From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck
> Sent: Friday, November 16, 2018 6:15 PM
> To: Vadim Pasternak <vadimp@mellanox.com>
> Cc: linux-hwmon@vger.kernel.org; Michael Shych <michaelsh@mellanox.com>
> Subject: Re: [PATCH hwmon-next v1 1/1] hwmon: (mlxreg-fan) Fix macros for
> tacho fault reading
> 
> On Fri, Nov 16, 2018 at 01:47:11PM +0000, Vadim Pasternak wrote:
> > Fix macros for tacometer fault reading.
> > This fix is relevant for three Mellanox systems MQMB7, MSN37, MSN34,
> > which are about to be released to the customers.
> > At the moment, none of them is at customers sites.
> >
> > Fixes: 65afb4c8e7e4 ("hwmon: (mlxreg-fan) Add support for Mellanox FAN
> > driver")
> > Signed-off-by: Vadim Pasternak <vadimp@mellanox.com>
> > ---
> >  drivers/hwmon/mlxreg-fan.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/hwmon/mlxreg-fan.c b/drivers/hwmon/mlxreg-fan.c
> > index de46577..d8fa4be 100644
> > --- a/drivers/hwmon/mlxreg-fan.c
> > +++ b/drivers/hwmon/mlxreg-fan.c
> > @@ -51,7 +51,7 @@
> >   */
> >  #define MLXREG_FAN_GET_RPM(rval, d, s)
> 	(DIV_ROUND_CLOSEST(15000000 * 100, \
> >  					 ((rval) + (s)) * (d)))
> > -#define MLXREG_FAN_GET_FAULT(val, mask) (!!((val) ^ (mask)))
> > +#define MLXREG_FAN_GET_FAULT(val, mask) (!((val) ^ (mask)))
> 
> You might want to check if the "^" is correct here. It looks fishy, since the
> expression only evaluates to true if a no bit of val outside mask is set. I would
> have expected "&".
> 
> Guenter

Hi Guenter,

Thanks for you your comment.

I'll follow your suggestion and change a macros to" 
#define MLXREG_FAN_GET_FAULT(val, mask) (!((val & mask) ^ (mask)))

> 
> >  #define MLXREG_FAN_PWM_DUTY2STATE(duty)
> 	(DIV_ROUND_CLOSEST((duty) *	\
> >  					 MLXREG_FAN_MAX_STATE,
> 	\
> >  					 MLXREG_FAN_MAX_DUTY))

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH hwmon-next v1 1/1] hwmon: (mlxreg-fan) Fix macros for tacho fault reading
  2018-11-19  6:07   ` Vadim Pasternak
  2018-11-19  6:07     ` Vadim Pasternak
@ 2018-11-19 16:15     ` Guenter Roeck
  2018-11-20 14:46       ` Vadim Pasternak
  1 sibling, 1 reply; 8+ messages in thread
From: Guenter Roeck @ 2018-11-19 16:15 UTC (permalink / raw)
  To: Vadim Pasternak; +Cc: linux-hwmon, Michael Shych

On Mon, Nov 19, 2018 at 06:07:40AM +0000, Vadim Pasternak wrote:
> 
> 
> > -----Original Message-----
> > From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck
> > Sent: Friday, November 16, 2018 6:15 PM
> > To: Vadim Pasternak <vadimp@mellanox.com>
> > Cc: linux-hwmon@vger.kernel.org; Michael Shych <michaelsh@mellanox.com>
> > Subject: Re: [PATCH hwmon-next v1 1/1] hwmon: (mlxreg-fan) Fix macros for
> > tacho fault reading
> > 
> > On Fri, Nov 16, 2018 at 01:47:11PM +0000, Vadim Pasternak wrote:
> > > Fix macros for tacometer fault reading.
> > > This fix is relevant for three Mellanox systems MQMB7, MSN37, MSN34,
> > > which are about to be released to the customers.
> > > At the moment, none of them is at customers sites.
> > >
> > > Fixes: 65afb4c8e7e4 ("hwmon: (mlxreg-fan) Add support for Mellanox FAN
> > > driver")
> > > Signed-off-by: Vadim Pasternak <vadimp@mellanox.com>
> > > ---
> > >  drivers/hwmon/mlxreg-fan.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/hwmon/mlxreg-fan.c b/drivers/hwmon/mlxreg-fan.c
> > > index de46577..d8fa4be 100644
> > > --- a/drivers/hwmon/mlxreg-fan.c
> > > +++ b/drivers/hwmon/mlxreg-fan.c
> > > @@ -51,7 +51,7 @@
> > >   */
> > >  #define MLXREG_FAN_GET_RPM(rval, d, s)
> > 	(DIV_ROUND_CLOSEST(15000000 * 100, \
> > >  					 ((rval) + (s)) * (d)))
> > > -#define MLXREG_FAN_GET_FAULT(val, mask) (!!((val) ^ (mask)))
> > > +#define MLXREG_FAN_GET_FAULT(val, mask) (!((val) ^ (mask)))
> > 
> > You might want to check if the "^" is correct here. It looks fishy, since the
> > expression only evaluates to true if a no bit of val outside mask is set. I would
> > have expected "&".
> > 
> > Guenter
> 
> Hi Guenter,
> 
> Thanks for you your comment.
> 
> I'll follow your suggestion and change a macros to" 
> #define MLXREG_FAN_GET_FAULT(val, mask) (!((val & mask) ^ (mask)))
> 
That seems excessively complicated. Why the xor after the & ?

Guenter

> > 
> > >  #define MLXREG_FAN_PWM_DUTY2STATE(duty)
> > 	(DIV_ROUND_CLOSEST((duty) *	\
> > >  					 MLXREG_FAN_MAX_STATE,
> > 	\
> > >  					 MLXREG_FAN_MAX_DUTY))

^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [PATCH hwmon-next v1 1/1] hwmon: (mlxreg-fan) Fix macros for tacho fault reading
  2018-11-19 16:15     ` Guenter Roeck
@ 2018-11-20 14:46       ` Vadim Pasternak
  2018-11-20 16:23         ` Guenter Roeck
  0 siblings, 1 reply; 8+ messages in thread
From: Vadim Pasternak @ 2018-11-20 14:46 UTC (permalink / raw)
  To: Guenter Roeck; +Cc: linux-hwmon, Michael Shych



> -----Original Message-----
> From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck
> Sent: Monday, November 19, 2018 6:15 PM
> To: Vadim Pasternak <vadimp@mellanox.com>
> Cc: linux-hwmon@vger.kernel.org; Michael Shych <michaelsh@mellanox.com>
> Subject: Re: [PATCH hwmon-next v1 1/1] hwmon: (mlxreg-fan) Fix macros for
> tacho fault reading
> 
> On Mon, Nov 19, 2018 at 06:07:40AM +0000, Vadim Pasternak wrote:
> >
> >
> > > -----Original Message-----
> > > From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck
> > > Sent: Friday, November 16, 2018 6:15 PM
> > > To: Vadim Pasternak <vadimp@mellanox.com>
> > > Cc: linux-hwmon@vger.kernel.org; Michael Shych
> > > <michaelsh@mellanox.com>
> > > Subject: Re: [PATCH hwmon-next v1 1/1] hwmon: (mlxreg-fan) Fix
> > > macros for tacho fault reading
> > >
> > > On Fri, Nov 16, 2018 at 01:47:11PM +0000, Vadim Pasternak wrote:
> > > > Fix macros for tacometer fault reading.
> > > > This fix is relevant for three Mellanox systems MQMB7, MSN37,
> > > > MSN34, which are about to be released to the customers.
> > > > At the moment, none of them is at customers sites.
> > > >
> > > > Fixes: 65afb4c8e7e4 ("hwmon: (mlxreg-fan) Add support for Mellanox
> > > > FAN
> > > > driver")
> > > > Signed-off-by: Vadim Pasternak <vadimp@mellanox.com>
> > > > ---
> > > >  drivers/hwmon/mlxreg-fan.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/hwmon/mlxreg-fan.c
> > > > b/drivers/hwmon/mlxreg-fan.c index de46577..d8fa4be 100644
> > > > --- a/drivers/hwmon/mlxreg-fan.c
> > > > +++ b/drivers/hwmon/mlxreg-fan.c
> > > > @@ -51,7 +51,7 @@
> > > >   */
> > > >  #define MLXREG_FAN_GET_RPM(rval, d, s)
> > > 	(DIV_ROUND_CLOSEST(15000000 * 100, \
> > > >  					 ((rval) + (s)) * (d)))
> > > > -#define MLXREG_FAN_GET_FAULT(val, mask) (!!((val) ^ (mask)))
> > > > +#define MLXREG_FAN_GET_FAULT(val, mask) (!((val) ^ (mask)))
> > >
> > > You might want to check if the "^" is correct here. It looks fishy,
> > > since the expression only evaluates to true if a no bit of val
> > > outside mask is set. I would have expected "&".
> > >
> > > Guenter
> >
> > Hi Guenter,
> >
> > Thanks for you your comment.
> >
> > I'll follow your suggestion and change a macros to"
> > #define MLXREG_FAN_GET_FAULT(val, mask) (!((val & mask) ^ (mask)))
> >
> That seems excessively complicated. Why the xor after the & ?

I wanted to evaluate MLXREG_FAN_GET_FAULT to true in case HW
reports the fault.
For 8 bits addressing device in case of fault val is 0xff and mask is set to 0xff.
For 16 bits - val will be 0xffff and mask will be set to 0xffff.

So, just simply this macros will work for me:
#define MLXREG_FAN_GET_FAULT(val, mask) ((val) == (mask))
or in order to be on the safe side:
#define MLXREG_FAN_GET_FAULT(val, mask) ((val & mask) == (mask))

> 
> Guenter
> 
> > >
> > > >  #define MLXREG_FAN_PWM_DUTY2STATE(duty)
> > > 	(DIV_ROUND_CLOSEST((duty) *	\
> > > >  					 MLXREG_FAN_MAX_STATE,
> > > 	\
> > > >  					 MLXREG_FAN_MAX_DUTY))

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH hwmon-next v1 1/1] hwmon: (mlxreg-fan) Fix macros for tacho fault reading
  2018-11-20 14:46       ` Vadim Pasternak
@ 2018-11-20 16:23         ` Guenter Roeck
  0 siblings, 0 replies; 8+ messages in thread
From: Guenter Roeck @ 2018-11-20 16:23 UTC (permalink / raw)
  To: Vadim Pasternak; +Cc: linux-hwmon, Michael Shych

On Tue, Nov 20, 2018 at 02:46:05PM +0000, Vadim Pasternak wrote:
> 
> 
> > -----Original Message-----
> > From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck
> > Sent: Monday, November 19, 2018 6:15 PM
> > To: Vadim Pasternak <vadimp@mellanox.com>
> > Cc: linux-hwmon@vger.kernel.org; Michael Shych <michaelsh@mellanox.com>
> > Subject: Re: [PATCH hwmon-next v1 1/1] hwmon: (mlxreg-fan) Fix macros for
> > tacho fault reading
> > 
> > On Mon, Nov 19, 2018 at 06:07:40AM +0000, Vadim Pasternak wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck
> > > > Sent: Friday, November 16, 2018 6:15 PM
> > > > To: Vadim Pasternak <vadimp@mellanox.com>
> > > > Cc: linux-hwmon@vger.kernel.org; Michael Shych
> > > > <michaelsh@mellanox.com>
> > > > Subject: Re: [PATCH hwmon-next v1 1/1] hwmon: (mlxreg-fan) Fix
> > > > macros for tacho fault reading
> > > >
> > > > On Fri, Nov 16, 2018 at 01:47:11PM +0000, Vadim Pasternak wrote:
> > > > > Fix macros for tacometer fault reading.
> > > > > This fix is relevant for three Mellanox systems MQMB7, MSN37,
> > > > > MSN34, which are about to be released to the customers.
> > > > > At the moment, none of them is at customers sites.
> > > > >
> > > > > Fixes: 65afb4c8e7e4 ("hwmon: (mlxreg-fan) Add support for Mellanox
> > > > > FAN
> > > > > driver")
> > > > > Signed-off-by: Vadim Pasternak <vadimp@mellanox.com>
> > > > > ---
> > > > >  drivers/hwmon/mlxreg-fan.c | 2 +-
> > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/drivers/hwmon/mlxreg-fan.c
> > > > > b/drivers/hwmon/mlxreg-fan.c index de46577..d8fa4be 100644
> > > > > --- a/drivers/hwmon/mlxreg-fan.c
> > > > > +++ b/drivers/hwmon/mlxreg-fan.c
> > > > > @@ -51,7 +51,7 @@
> > > > >   */
> > > > >  #define MLXREG_FAN_GET_RPM(rval, d, s)
> > > > 	(DIV_ROUND_CLOSEST(15000000 * 100, \
> > > > >  					 ((rval) + (s)) * (d)))
> > > > > -#define MLXREG_FAN_GET_FAULT(val, mask) (!!((val) ^ (mask)))
> > > > > +#define MLXREG_FAN_GET_FAULT(val, mask) (!((val) ^ (mask)))
> > > >
> > > > You might want to check if the "^" is correct here. It looks fishy,
> > > > since the expression only evaluates to true if a no bit of val
> > > > outside mask is set. I would have expected "&".
> > > >
> > > > Guenter
> > >
> > > Hi Guenter,
> > >
> > > Thanks for you your comment.
> > >
> > > I'll follow your suggestion and change a macros to"
> > > #define MLXREG_FAN_GET_FAULT(val, mask) (!((val & mask) ^ (mask)))
> > >
> > That seems excessively complicated. Why the xor after the & ?
> 
> I wanted to evaluate MLXREG_FAN_GET_FAULT to true in case HW
> reports the fault.
> For 8 bits addressing device in case of fault val is 0xff and mask is set to 0xff.
> For 16 bits - val will be 0xffff and mask will be set to 0xffff.
> 
> So, just simply this macros will work for me:
> #define MLXREG_FAN_GET_FAULT(val, mask) ((val) == (mask))
> or in order to be on the safe side:
> #define MLXREG_FAN_GET_FAULT(val, mask) ((val & mask) == (mask))
> 
That would be much easier to understand.

Thanks,
Guenter

> > 
> > Guenter
> > 
> > > >
> > > > >  #define MLXREG_FAN_PWM_DUTY2STATE(duty)
> > > > 	(DIV_ROUND_CLOSEST((duty) *	\
> > > > >  					 MLXREG_FAN_MAX_STATE,
> > > > 	\
> > > > >  					 MLXREG_FAN_MAX_DUTY))

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2018-11-21  2:53 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-16 13:47 [PATCH hwmon-next v1 1/1] hwmon: (mlxreg-fan) Fix macros for tacho fault reading Vadim Pasternak
2018-11-16 16:10 ` Guenter Roeck
2018-11-16 16:15 ` Guenter Roeck
2018-11-19  6:07   ` Vadim Pasternak
2018-11-19  6:07     ` Vadim Pasternak
2018-11-19 16:15     ` Guenter Roeck
2018-11-20 14:46       ` Vadim Pasternak
2018-11-20 16:23         ` Guenter Roeck

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).