Re: [PATCH v3 0/7] iio: adc: palmas_gpadc: add iio events

[Jonathan Cameron]

On Fri, 7 Apr 2023 18:04:35 +0100
Jonathan Cameron <jic23@xxxxxxxxxx> wrote:

> On Wed,  5 Apr 2023 23:22:26 +0200
> Patrik Dahlström <risca@xxxxxxxxxxxxxx> wrote:
> 
> > This series is based on linux-next/master [1] and [2].
> > 
> > The palmas gpadc block has support for monitoring up to 2 ADC channels
> > and issue an interrupt if they reach past a set threshold. This can be
> > configured statically with device tree today, but it only gets enabled
> > when reaching sleep mode. Also, it doesn't look like anyone is using it.
> > 
> > Instead of this one special case, change the code so userspace can
> > configure the ADC channels to their own needs through the iio events
> > subsystem. The high and low threshold values can be set for every
> > channel, but only 2 thresholds can be enabled at a time. Trying to
> > enable more than 2 thresholds will result in an error.
> > 
> > The configured thresholds will wake up the system from sleep mode if
> > wakeup is enabled in /sys/devices/.../power/wakeup.
> > 
> > The old platform data was removed.
> > 
> > Thresholds, events, and wakeup were tested on omap5-uevm board. It wakes
> > up from sleep mode when wakeup is enabled and a threshold is passed. A
> > userspace tool for monitoring events and adjusting thresholds can be
> > found at [3].
> > 
> > V2 -> V3:
> > * Rebased to linux-next.  
> 
> As per reply to the earlier thread.  Don't base on linux-next.
> It can be very unstable though not so much later in a cycle like this.
> 
> If there isn't a lot of churn going on in the driver, fine to base on
> previous release kernel or rc1 (good to say if it is an rc1)
> 
> If there is churn underway (which is true here) then iio/togreg + extra
> patches lists that need to be applied listed in this cover letter.

Just goes to show I focused on the change log and skipped the rest :)
As you have it here is fine though change log could have mentioned the
extra patch as well even if just "Rebased to linux-next + devm patch."

In this case linux-next is close enough for this driver to the
iio/togreg tree that it doesn't matter that it shouldn't be used as a base
(no impact in this particular case I think).

Anyhow, all good. I noticed I'd misinterpreted what you'd done here
when I saw the context in one of the patches.  oops :)

Jonathan


> 
> I'm also fine with you just adding the devm patch to this series as
> the first patch.
> 
> Jonathan
> 
> 
> 
> > * Avoid reconfiguring events on error and when old == new value.
> > V1 -> V2:
> > * Begin by removing adc_wakeupX_data instead of doing it last.
> > * Split changes in smaller patches
> > 
> > [1] git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
> > [2] https://lore.kernel.org/linux-iio/20230318163039.56115-1-jic23@xxxxxxxxxx/
> > [3] https://github.com/Risca/pyra_vol_mon
> > 
> > Patrik Dahlström (7):
> >   iio: adc: palmas: remove adc_wakeupX_data
> >   iio: adc: palmas: replace "wakeup" with "event"
> >   iio: adc: palmas: use iio_event_direction for threshold polarity
> >   iio: adc: palmas: move eventX_enable into palmas_adc_event
> >   iio: adc: palmas: always reset events on unload
> >   iio: adc: palmas: add support for iio threshold events
> >   iio: adc: palmas: don't alter event config on suspend/resume
> > 
> >  drivers/iio/adc/palmas_gpadc.c | 559 +++++++++++++++++++++++++++------
> >  include/linux/mfd/palmas.h     |   8 -
> >  2 files changed, 464 insertions(+), 103 deletions(-)
> > 
> > 
> > base-commit: 8417c8f5007bf4567ccffda850a3157c7d905f67
> > prerequisite-patch-id: b0418c707db13f514400956596e9ebe91c25bba0  
>