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 | <20260521123125.4000a67a@jic23-huawei> |
On Wed, 20 May 2026 20:03:26 -0500 David Lechner <[email protected]> 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 <[email protected]> wrote: > >> > >>> On 15/05/26 20:05, Jonathan Cameron wrote: > >>>> On Wed, 13 May 2026 20:58:40 +0300 > >>>> Andy Shevchenko <[email protected]> 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 > > > >