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

[Jonathan Cameron <jic23@kernel.org>]

On Wed, 20 May 2026 20:03:26 -0500
David Lechner <dlechner@baylibre.com> wrote:

> On 5/20/26 12:12 PM, Aldo Conte wrote:
> > On 16/05/26 14:25, Jonathan Cameron wrote:  
> >> On Sat, 16 May 2026 13:04:13 +0200
> >> Aldo Conte <aldocontelk@gmail.com> wrote:
> >>  
> >>> On 15/05/26 20:05, Jonathan Cameron wrote:  
> >>>> On Wed, 13 May 2026 20:58:40 +0300
> >>>> Andy Shevchenko <andriy.shevchenko@intel.com> wrote:  
> >>>
> >>> ...
> >>>  
> >>>>
> >>>> 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.
> >>>>     
> >>>>>
> >>>>>     
> >>>>      
> >>>
> >>> Hi Jonathan, Andy and all!
> >>>
> >>> Thanks for your reviews! they’re teaching me so much!
> >>> If I understand correctly, I need to wait a week before submitting version v3
> >>> such that other reviewers can comment right?
> >>>
> >>> I’d like to take this opportunity to suggest two alternative lists of what
> >>> version v3 might look like in terms of patch order:
> >>>
> >>> First option does not consider the "== NULL with !" patch and so:
> >>> 1. iio: tcs3472: power down chip on probe failure (NEW precursor, bug fix)
> >>> 2. iio: tcs3472: sort headers alphabetically (same)
> >>> 3. iio: tcs3472: convert several functions to use guard(mutex)() (modified)
> >>> 4. iio: tcs3472: use devm for resource management (slimmed down)
> >>> 5. iio: tcs3472: use locale struct device * for remaining cases (NEW)
> >>> 6. iio: tcs3472: implement wait time and sampling frequency (revised)
> >>> 7. iio: tcs3472: move standalone return to default case (same as v2)
> >>>
> >>> Second version will consider it:
> >>> 1. iio: tcs3472: power down chip on probe failure (NEW precursor, bug fix)
> >>> 2. iio: tcs3472: sort headers alphabetically (same as v2)
> >>> 3. iio: tcs3472: convert several functions to use guard(mutex)() (modified)
> >>> 4. iio: tcs3472: replace == NULL with ! (NEW, optional)
> >>> 5. iio: tcs3472: use devm for resource management (slimmed down)
> >>> 6. iio: tcs3472: use locale struct device * for remaining cases (NEW)
> >>> 7. iio: tcs3472: implement wait time and sampling frequency (revised)
> >>> 8. iio: tcs3472: move standalone return to default case (same as v2)
> >>>
> >>>
> >>> What do you think?  
> >> Either is fine but swap the last two patches.  The wait time one is
> >> more complex than that trivial cleanup so might take more revisions.
> >>
> >> I frequently pick up the first part of series just to avoid people
> >> having to keep sending the same unchanged code to the list (that I've
> >> often forgotten about completely by the next version!)
> >>
> >> Jonathan
> >>  
> >>>
> >>>
> >>> Thanks,
> >>> Aldo  
> >>  
> > 
> > Hi Jonathan,
> > 
> > I have two quick questions.
> > 
> > First, Uwe's "[PATCH v2 0/7] iio: Rework i2c_device_id initialisation" also touches tcs3472.c. My series modifies the same file for unrelated reasons. Should I wait for Uwe's series to land and rebase on top, or is it fine to send now and let you handle the overlap? How does it work in  case?  
> 
> Just add a patch to your series that does the same change
> for that one file. Then it won't matter which one gets picked
> up first.
> 
Normally I'd say don't bother unless you are modifying that specific code.
If there is an actual conflict (rather than lines moving around a bit) then
indeed having such a patch is good.

If minor conflicts occur I'll just fix them up whilst applying.
Gets messy if conflict between a fix and a non fix if I don't notice
it, but otherwise it is easy enough.

J
> > 
> > Second, the first patch in my series fixes missing power-down on probe failure (goto err_powerdown). Do I need to add the “Fixes” tag?  
> 
> Sounds like it.
> 
> > 
> > Thanks for your time.
> > 
> > Aldo Conte
> >   
> 
>