Re: [PATCH 1/2] drm/v3d: Avoid -Wconstant-logical-operand in nsecs_to_jiffies_timeout()

2023-07-27 Thread Nathan Chancellor
Hi Maira,

On Thu, Jul 27, 2023 at 11:01:27AM -0300, Maira Canal wrote:
> Hi Nathan,
> 
> On 7/18/23 18:44, Nathan Chancellor wrote:
> > A proposed update to clang's -Wconstant-logical-operand to warn when the
> > left hand side is a constant shows the following instance in
> > nsecs_to_jiffies_timeout() when NSEC_PER_SEC is not a multiple of HZ,
> > such as CONFIG_HZ=300:
> > 
> >In file included from drivers/gpu/drm/v3d/v3d_debugfs.c:12:
> >drivers/gpu/drm/v3d/v3d_drv.h:343:24: warning: use of logical '&&' with 
> > constant operand [-Wconstant-logical-operand]
> >  343 | if (NSEC_PER_SEC % HZ &&
> >  | ~ ^
> >drivers/gpu/drm/v3d/v3d_drv.h:343:24: note: use '&' for a bitwise 
> > operation
> >  343 | if (NSEC_PER_SEC % HZ &&
> >  |   ^~
> >  |   &
> >drivers/gpu/drm/v3d/v3d_drv.h:343:24: note: remove constant to silence 
> > this warning
> >1 warning generated.
> > 
> > Turn this into an explicit comparison against zero to make the
> > expression a boolean to make it clear this should be a logical check,
> > not a bitwise one.
> > 
> > Link: https://reviews.llvm.org/D142609
> > Signed-off-by: Nathan Chancellor 
> 
> Reviewed-by: Maíra Canal 
> 
> Thanks for all the hard work with clang! Let me know if you need someone
> to push it to drm-misc-next.

Yes I will, I do not have drm commit rights. I think the i915 patch can
go to drm-misc as well with Tvrtko's ack. Thanks a lot for taking a
look!

Cheers,
Nathan

> Best Regards,
> - Maíra
> 
> > ---
> >   drivers/gpu/drm/v3d/v3d_drv.h | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/v3d/v3d_drv.h b/drivers/gpu/drm/v3d/v3d_drv.h
> > index b74b1351bfc8..7f664a4b2a75 100644
> > --- a/drivers/gpu/drm/v3d/v3d_drv.h
> > +++ b/drivers/gpu/drm/v3d/v3d_drv.h
> > @@ -340,7 +340,7 @@ struct v3d_submit_ext {
> >   static inline unsigned long nsecs_to_jiffies_timeout(const u64 n)
> >   {
> > /* nsecs_to_jiffies64() does not guard against overflow */
> > -   if (NSEC_PER_SEC % HZ &&
> > +   if ((NSEC_PER_SEC % HZ) != 0 &&
> > div_u64(n, NSEC_PER_SEC) >= MAX_JIFFY_OFFSET / HZ)
> > return MAX_JIFFY_OFFSET;
> > 


Re: [PATCH 1/2] drm/v3d: Avoid -Wconstant-logical-operand in nsecs_to_jiffies_timeout()

2023-07-27 Thread Maira Canal

Hi Nathan,

On 7/18/23 18:44, Nathan Chancellor wrote:

A proposed update to clang's -Wconstant-logical-operand to warn when the
left hand side is a constant shows the following instance in
nsecs_to_jiffies_timeout() when NSEC_PER_SEC is not a multiple of HZ,
such as CONFIG_HZ=300:

   In file included from drivers/gpu/drm/v3d/v3d_debugfs.c:12:
   drivers/gpu/drm/v3d/v3d_drv.h:343:24: warning: use of logical '&&' with 
constant operand [-Wconstant-logical-operand]
 343 | if (NSEC_PER_SEC % HZ &&
 | ~ ^
   drivers/gpu/drm/v3d/v3d_drv.h:343:24: note: use '&' for a bitwise operation
 343 | if (NSEC_PER_SEC % HZ &&
 |   ^~
 |   &
   drivers/gpu/drm/v3d/v3d_drv.h:343:24: note: remove constant to silence this 
warning
   1 warning generated.

Turn this into an explicit comparison against zero to make the
expression a boolean to make it clear this should be a logical check,
not a bitwise one.

Link: https://reviews.llvm.org/D142609
Signed-off-by: Nathan Chancellor 


Reviewed-by: Maíra Canal 

Thanks for all the hard work with clang! Let me know if you need someone
to push it to drm-misc-next.

Best Regards,
- Maíra


---
  drivers/gpu/drm/v3d/v3d_drv.h | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/v3d/v3d_drv.h b/drivers/gpu/drm/v3d/v3d_drv.h
index b74b1351bfc8..7f664a4b2a75 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.h
+++ b/drivers/gpu/drm/v3d/v3d_drv.h
@@ -340,7 +340,7 @@ struct v3d_submit_ext {
  static inline unsigned long nsecs_to_jiffies_timeout(const u64 n)
  {
/* nsecs_to_jiffies64() does not guard against overflow */
-   if (NSEC_PER_SEC % HZ &&
+   if ((NSEC_PER_SEC % HZ) != 0 &&
div_u64(n, NSEC_PER_SEC) >= MAX_JIFFY_OFFSET / HZ)
return MAX_JIFFY_OFFSET;
  



Re: [PATCH 1/2] drm/v3d: Avoid -Wconstant-logical-operand in nsecs_to_jiffies_timeout()

2023-07-21 Thread Nick Desaulniers
On Tue, Jul 18, 2023 at 2:44 PM Nathan Chancellor  wrote:
>
> A proposed update to clang's -Wconstant-logical-operand to warn when the
> left hand side is a constant shows the following instance in
> nsecs_to_jiffies_timeout() when NSEC_PER_SEC is not a multiple of HZ,
> such as CONFIG_HZ=300:
>
>   In file included from drivers/gpu/drm/v3d/v3d_debugfs.c:12:
>   drivers/gpu/drm/v3d/v3d_drv.h:343:24: warning: use of logical '&&' with 
> constant operand [-Wconstant-logical-operand]
> 343 | if (NSEC_PER_SEC % HZ &&
> | ~ ^
>   drivers/gpu/drm/v3d/v3d_drv.h:343:24: note: use '&' for a bitwise operation
> 343 | if (NSEC_PER_SEC % HZ &&
> |   ^~
> |   &
>   drivers/gpu/drm/v3d/v3d_drv.h:343:24: note: remove constant to silence this 
> warning
>   1 warning generated.
>
> Turn this into an explicit comparison against zero to make the
> expression a boolean to make it clear this should be a logical check,
> not a bitwise one.
>
> Link: https://reviews.llvm.org/D142609
> Signed-off-by: Nathan Chancellor 

Thanks for the patch!
Reviewed-by: Nick Desaulniers 

> ---
>  drivers/gpu/drm/v3d/v3d_drv.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/v3d/v3d_drv.h b/drivers/gpu/drm/v3d/v3d_drv.h
> index b74b1351bfc8..7f664a4b2a75 100644
> --- a/drivers/gpu/drm/v3d/v3d_drv.h
> +++ b/drivers/gpu/drm/v3d/v3d_drv.h
> @@ -340,7 +340,7 @@ struct v3d_submit_ext {
>  static inline unsigned long nsecs_to_jiffies_timeout(const u64 n)
>  {
> /* nsecs_to_jiffies64() does not guard against overflow */
> -   if (NSEC_PER_SEC % HZ &&
> +   if ((NSEC_PER_SEC % HZ) != 0 &&
> div_u64(n, NSEC_PER_SEC) >= MAX_JIFFY_OFFSET / HZ)
> return MAX_JIFFY_OFFSET;
>
>
> --
> 2.41.0
>


-- 
Thanks,
~Nick Desaulniers


[PATCH 1/2] drm/v3d: Avoid -Wconstant-logical-operand in nsecs_to_jiffies_timeout()

2023-07-18 Thread Nathan Chancellor
A proposed update to clang's -Wconstant-logical-operand to warn when the
left hand side is a constant shows the following instance in
nsecs_to_jiffies_timeout() when NSEC_PER_SEC is not a multiple of HZ,
such as CONFIG_HZ=300:

  In file included from drivers/gpu/drm/v3d/v3d_debugfs.c:12:
  drivers/gpu/drm/v3d/v3d_drv.h:343:24: warning: use of logical '&&' with 
constant operand [-Wconstant-logical-operand]
343 | if (NSEC_PER_SEC % HZ &&
| ~ ^
  drivers/gpu/drm/v3d/v3d_drv.h:343:24: note: use '&' for a bitwise operation
343 | if (NSEC_PER_SEC % HZ &&
|   ^~
|   &
  drivers/gpu/drm/v3d/v3d_drv.h:343:24: note: remove constant to silence this 
warning
  1 warning generated.

Turn this into an explicit comparison against zero to make the
expression a boolean to make it clear this should be a logical check,
not a bitwise one.

Link: https://reviews.llvm.org/D142609
Signed-off-by: Nathan Chancellor 
---
 drivers/gpu/drm/v3d/v3d_drv.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/v3d/v3d_drv.h b/drivers/gpu/drm/v3d/v3d_drv.h
index b74b1351bfc8..7f664a4b2a75 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.h
+++ b/drivers/gpu/drm/v3d/v3d_drv.h
@@ -340,7 +340,7 @@ struct v3d_submit_ext {
 static inline unsigned long nsecs_to_jiffies_timeout(const u64 n)
 {
/* nsecs_to_jiffies64() does not guard against overflow */
-   if (NSEC_PER_SEC % HZ &&
+   if ((NSEC_PER_SEC % HZ) != 0 &&
div_u64(n, NSEC_PER_SEC) >= MAX_JIFFY_OFFSET / HZ)
return MAX_JIFFY_OFFSET;
 

-- 
2.41.0