Re: [PATCH v2 5/5] iio: light: tcs3472: move standalone return to default case

Jonathan Cameron <[email protected]>
Newsgroups dev.linux.lists.linux-kernel-mentees,org.kernel.vger.linux-iio,org.kernel.vger.linux-kernel
Message-ID <20260515190517.70bb0ba5@jic23-huawei>
On Wed, 13 May 2026 20:58:40 +0300
Andy Shevchenko <[email protected]> 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.

> 
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.