Linux IIO development
 help / color / mirror / Atom feed
From: Jonathan Cameron <jic23@kernel.org>
To: Andy Shevchenko <andriy.shevchenko@intel.com>
Cc: Aldo Conte <aldocontelk@gmail.com>,
	dlechner@baylibre.com, nuno.sa@analog.com, andy@kernel.org,
	shuah@kernel.org, linux-iio@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	linux-kernel-mentees@lists.linux.dev
Subject: Re: [PATCH v2 5/5] iio: light: tcs3472: move standalone return to default case
Date: Fri, 15 May 2026 19:05:17 +0100	[thread overview]
Message-ID: <20260515190517.70bb0ba5@jic23-huawei> (raw)
In-Reply-To: <agS70Dph8-zQT-e6@ashevche-desk.local>

On Wed, 13 May 2026 20:58:40 +0300
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

> On Wed, May 13, 2026 at 06:12:25PM +0200, Aldo Conte wrote:
> > On 5/13/26 13:23, Andy Shevchenko wrote:  
> > > On Wed, May 13, 2026 at 12:32:15AM +0200, Aldo Conte wrote:  
> 
> ...
> 
> > Thanks for the review. Before I send v3, I want to make sure I split
> > the series the way you want.
> > 
> > My plan is to split v2 into 7 patches:
> > 
> >   1. sort headers (same as v2)
> >   2. convert locking to guard(mutex) (drop Suggested-by, clean up
> >      trigger_handler)
> >   3. replace == NULL with ! (add Suggested-by: Joshua Crofts)
> >   4. use devm for resource management (drop Suggested-by, remove the
> >      dev_info change, add devm_mutex_init)
I made a few suggestions in reply to that patch on stuff to move out
of it etc.

> >   5. use 'dev' for dev_info() calls (new, taken out of patch 4)
> >   6. implement wait time and sampling_frequency (drop Suggested-by,
> >      fix macro style, simplify cycle_us and all the other revisions)
> >   7. move standalone return to default case (same as v2)  
> 
> (Also collect tags for the patches that are not [drastically] changed, like #7
>  here.)
> 
> > Two things I am not sure about:
> > 
> >   - Should patch 3 ("replace == NULL with !") really be its own patch,
> >     or do you prefer it inside patch 4, since that line is right next
> >     to devm_iio_device_alloc?  
> 
> Jonathan is okay to combine in some cases, but I'm not sure if this is small
> enough. Ask him?

Slight preference for a separate patch - or just skip it.  There are lots
more things that could be tidied up in this driver I think. You don't have
to do them all!

> 
> >   - Is patch 5 (dev_info refactor) in the right place after the devm
> >     patch?  
> 
> Basically sort patches by severity, with this in mind seems you suggested
> the right order.

I would add for this that, to reduce churn it's fine to add the
struct device *dev = ... bit and use it in new devm calls. Just do the
remainder in the follow on commit.

> 
> 


  reply	other threads:[~2026-05-15 18:05 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-12 22:32 [PATCH v2 0/5] devm conversion, wait time, locking cleanup Aldo Conte
2026-05-12 22:32 ` [PATCH v2 1/5] iio: light: tcs3472: sort headers alphabetically Aldo Conte
2026-05-13  8:15   ` Joshua Crofts
2026-05-12 22:32 ` [PATCH v2 2/5] iio: light: tcs3472: convert remaining locking to guard(mutex) Aldo Conte
2026-05-13  7:47   ` Joshua Crofts
2026-05-13 11:00     ` Andy Shevchenko
2026-05-13 10:58   ` Andy Shevchenko
2026-05-15 15:18   ` Jonathan Cameron
2026-05-12 22:32 ` [PATCH v2 3/5] iio: light: tcs3472: use devm for resource management Aldo Conte
2026-05-13  8:07   ` Joshua Crofts
2026-05-13 11:02   ` Andy Shevchenko
2026-05-13 20:29   ` Aldo Conte
2026-05-15 15:21     ` Jonathan Cameron
2026-05-15 17:19   ` Jonathan Cameron
2026-05-12 22:32 ` [PATCH v2 4/5] iio: light: tcs3472: implement wait time and sampling frequency Aldo Conte
2026-05-13 11:17   ` Andy Shevchenko
2026-05-15 15:57     ` Aldo Conte
2026-05-15 18:01   ` Jonathan Cameron
2026-05-12 22:32 ` [PATCH v2 5/5] iio: light: tcs3472: move standalone return to default case Aldo Conte
2026-05-13  8:16   ` Joshua Crofts
2026-05-13 11:23   ` Andy Shevchenko
2026-05-13 16:12     ` Aldo Conte
2026-05-13 17:58       ` Andy Shevchenko
2026-05-15 18:05         ` Jonathan Cameron [this message]
2026-05-15 15:11 ` [PATCH v2 0/5] devm conversion, wait time, locking cleanup Jonathan Cameron

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=20260515190517.70bb0ba5@jic23-huawei \
    --to=jic23@kernel.org \
    --cc=aldocontelk@gmail.com \
    --cc=andriy.shevchenko@intel.com \
    --cc=andy@kernel.org \
    --cc=dlechner@baylibre.com \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel-mentees@lists.linux.dev \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nuno.sa@analog.com \
    --cc=shuah@kernel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox