All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mohamad Raizudeen <raizudeen.kerneldev@gmail.com>
To: Joshua Crofts <joshua.crofts1@gmail.com>
Cc: jic23@kernel.org, dlechner@baylibre.com, andy@kernel.org,
	nuno.sa@analog.com, gregkh@linuxfoundation.org,
	skhan@linuxfoundation.org, me@brighamcampbell.com,
	jkoolstra@xs4all.nl, linux-iio@vger.kernel.org,
	linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org,
	Andy Shevchenko <andriy.shevchenko@intel.com>
Subject: Re: [PATCH v2] staging: iio: frequency: ad9832/ad9834: add comment explaining do_div usage
Date: Wed, 22 Jul 2026 21:33:33 +0530	[thread overview]
Message-ID: <amDp1VPSMF0P6r8T@kernel> (raw)
In-Reply-To: <20260722104617.000050fa@gmail.com>

On Wed, Jul 22, 2026 at 10:46:17AM +0200, Joshua Crofts wrote:
> On Wed, 22 Jul 2026 13:52:27 +0530
> Mohamad Raizudeen <raizudeen.kerneldev@gmail.com> wrote:
> > Previous attempts to change do_div() to div64_ul() were rejected because
> > mclk will always fit within 32 bits for this hardware, making do_div()
> > safe to use.
> > 
> > However, Coccinelle continues to flag this as a false positive. To
> > prevent future developers from submitting unnecessary fixes, add a
> > comment explaining why do_div() is intentionally kept.
> > 
> > Suggested-by: Andy Shevchenko <andriy.shevchenko@intel.com>
> > Signed-off-by: Mohamad Raizudeen <raizudeen.kerneldev@gmail.com>
> > ---
> 
> Please don't send a new version as a reply to the previous version,
> as it can break tooling such as b4 for patch review.
> 
> As for the comment, I think it's good, however I'm not sure whether
> this should be split into two patches, given that this is a small
> change. Andy, what do you think?
> 
> > diff --git a/drivers/staging/iio/frequency/ad9832.c b/drivers/staging/iio/frequency/ad9832.c
> > index 60c33e10c46f..6d046e9121e2 100644
> > --- a/drivers/staging/iio/frequency/ad9832.c
> > +++ b/drivers/staging/iio/frequency/ad9832.c
> > @@ -118,6 +118,13 @@ static unsigned long ad9832_calc_freqreg(unsigned long mclk, unsigned long fout)
> >  {
> >  	u64 freqreg = (u64)fout << AD9832_FREQ_BITS;
> >  
> > +       /*
> > +	* mclk is an unsigned long, which triggers a Coccinelle false positive
> > +	* warning about using do_div() for 64-by-32 division. However, mclk
> > +	* for this hardware will always fit within 32 bits, so do_div() is
> > +	* safe to use here.
> > +	*/
> > +
> 
> Unnecessary blank line here, you want to couple the comment with the
> line that it's describing.
> 
> >  	do_div(freqreg, mclk);
> >  	return freqreg;
> >  }
> > diff --git a/drivers/staging/iio/frequency/ad9834.c b/drivers/staging/iio/frequency/ad9834.c
> > index 33dfd723923c..8722f3a01cde 100644
> > --- a/drivers/staging/iio/frequency/ad9834.c
> > +++ b/drivers/staging/iio/frequency/ad9834.c
> > @@ -103,6 +103,13 @@ static unsigned int ad9834_calc_freqreg(unsigned long mclk, unsigned long fout)
> >  {
> >  	unsigned long long freqreg = (u64)fout * (u64)BIT(AD9834_FREQ_BITS);
> >  
> > +       /*
> > +	* mclk is an unsigned long, which triggers a Coccinelle false positive
> > +	* warning about using do_div() for 64-by-32 division. However, mclk
> > +	* for this hardware will always fit within 32 bits, so do_div() is
> > +	* safe to use here.
> > +	*/
> > +
> 
> Ditto.
> 
> >  	do_div(freqreg, mclk);
> >  	return freqreg;
> >  }
> 
> 
> 
> -- 
> Kind regards,
> Joshua Crofts

Hi Joshua,
Thanks you for reviewing the patch and for the clarifications.

I understood and will make sure v3 is sent as a new and separate email
thread without in-reply-to header so i doesn't break b4.

Sure, I will remove the unnecessary blank line.

I will hold off sending v3 until I hear from Andy regarding whether this should be split
into two patches or kept as one.

Thanks & regards,
Mohamad Raizudeen

  reply	other threads:[~2026-07-22 16:03 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-30  3:23 [PATCH] staging: iio: frequency: ad9832/ad9834: use div64_ul instead of do_div Mohamad Raizudeen
2026-06-30  7:39 ` Joshua Crofts
2026-06-30 12:02   ` Andy Shevchenko
2026-06-30 13:07     ` Joshua Crofts
2026-06-30 13:27       ` Dan Carpenter
2026-07-22  8:22 ` [PATCH v2] staging: iio: frequency: ad9832/ad9834: add comment explaining do_div usage Mohamad Raizudeen
2026-07-22  8:46   ` Joshua Crofts
2026-07-22 16:03     ` Mohamad Raizudeen [this message]
2026-08-06 20:56     ` Andy Shevchenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=amDp1VPSMF0P6r8T@kernel \
    --to=raizudeen.kerneldev@gmail.com \
    --cc=andriy.shevchenko@intel.com \
    --cc=andy@kernel.org \
    --cc=dlechner@baylibre.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jic23@kernel.org \
    --cc=jkoolstra@xs4all.nl \
    --cc=joshua.crofts1@gmail.com \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-staging@lists.linux.dev \
    --cc=me@brighamcampbell.com \
    --cc=nuno.sa@analog.com \
    --cc=skhan@linuxfoundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.