Message-ID: <CAKUZ0zLpRRPDBF5qaUXCukSn=uZFbDUG8UhXrQbpkrZyePe1Ww@mail.gmail.com>
Date: Mon, 21 Apr 2025 09:44:41 -0400
From: Gabriel Shahrouzi <gshahrouzi@...il.com>
To: Jonathan Cameron <jic23@...nel.org>
Cc: gregkh@...uxfoundation.org, lars@...afoo.de, linux-iio@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-staging@...ts.linux.dev, 
	Michael.Hennerich@...log.com, sonic.zhang@...log.com, vapier@...too.org, 
	skhan@...uxfoundation.org, linux-kernel-mentees@...ts.linux.dev
Subject: Re: [PATCH v3 0/5] staging: iio: adc: ad7816: Fix channel handling

On Mon, Apr 21, 2025 at 7:29 AM Jonathan Cameron <jic23@...nel.org> wrote:
>
> On Fri, 18 Apr 2025 16:47:34 -0400
> Gabriel Shahrouzi <gshahrouzi@...il.com> wrote:
>
> > The original patch combined a functional fix (allowing channel 7) with
> > several refactoring steps (introducing chip_info, renaming structs,
> > improving validation). As requested, these have now been separated.
> >
> > The series proceeds as follows:
> > 1. Fix: Allow diagnostic channel 7 for all device variants.
> > 2. Refactor: Rename the main state structure for clarity before introducing
> >    the new chip_info struct.
> > 3. Refactor: Introduce struct ad7816_chip_info to hold static per-variant
> >    data, update ID tables to store pointers, and switch to using
> >    device_get_match_data() for firmware-independent identification.
> >    This removes the old enum/id mechanism.
> > 4. Refactor: Add has_busy_pin to chip_info and use this flag to
> >    determine BUSY pin handling, replacing pointer comparisons.
> > 5. Refactor: Simplify channel validation logic using
> >    chip_info->max_channels, removing strcmp() checks.
> >
> > Regarding the 'fixes' tag: I've applied it only to the first commit
> > containing the core fix, primarily to make backporting easier. Is this
> > the standard practice, or should the tag typically be applied to
> > subsequent commits that build upon or are related to the fix as well?
> >
> > Chainges in v3:
> >       - Split the patch into smaller patches. Make the fix first
> >         followed by clean up.
> >       - Include missing channel for channel selection.
> >       - Address specific feedback regarding enums vs. chip_info data.
> >       - Use device_get_match_data() for device identification.
> >       - Move BUSY pin capability check into chip_info data.
> >       - Simplify channel validation using chip_info data.
> > Changes in v2:
> >         - Refactor by adding chip_info struct which simplifies
> >           conditional logic.
>
> Hi Gabriel,
>
> Whilst I appreciate the enthusiasm. Generally slow down a little!
> If there is a fundamental issue like an accidental sending of the wrong
> version then please reply to the thread cover letter to say that.
> Otherwise, even in the presence of build bot reports, it is good
> for any non trivial series to wait a little for reviews to come in.
> Ideally a week, but a few days can be fine if you already have a lot
> of feedback from reviewers.
Got it, makes more sense to respond to a fundamental error like that
in the cover letter than send an entire new patch series out which
bloats the mailing list.
>
> IIO is moderately high traffic and whilst we are good at hitting
> the button to mark a thread read, it still takes some time!
Got it.
>
> Jonathan
>
> >
> > Gabriel Shahrouzi (5):
> >   staging: iio: adc: ad7816: Allow channel 7 for all devices
> >   staging: iio: adc: ad7816: Rename state structure
> >   staging: iio: adc: ad7816: Introduce chip_info and use pointer
> >     matching
> >   staging: iio: adc: ad7816: Use chip_info for device capabilities
> >   staging: iio: adc: ad7816: Simplify channel validation using chip_info
> >
> >  drivers/staging/iio/adc/ad7816.c | 94 ++++++++++++++++++--------------
> >  1 file changed, 54 insertions(+), 40 deletions(-)
> >
>

Powered by blists - more mailing lists