Re: [PATCH v5 0/5] staging: iio: adc: ad7816: Fix channel handling and refactor

Gabriel Shahrouzi
2025-04-21T13:49:43Z

On Mon, Apr 21, 2025 at 7:31 AM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Sat, 19 Apr 2025 21:49:05 -0400
> Gabriel Shahrouzi <gshahrouzi@gmail.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?
> >
> > Changes in v5:
> > - Use correct patch version.
> Generally I wouldn't resend for this. Instead a single email in
> reply to the messed up version saying it is infact v4 would have
> done the job.
Got it.
>
> Alternatively a quick reply to that thread to say it was messed
> up and please look for v5 would have worked to make a reader
> move on directly to the newer version
Got it.
>
>
> Jonathan
>
> > Changes in v4:
> > - Include missing bracket for condtional statement.
> > 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.
> >
> > 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(-)
> >
>