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

Message ID 20230405212233.4167986-1-risca@dalakolonin.se
Headers
Series iio: adc: palmas_gpadc: add iio events |

Message

Patrik Dahlström April 5, 2023, 9:22 p.m. UTC
  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.
* 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@kernel.org/
[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
  

Comments

Jonathan Cameron April 7, 2023, 5:04 p.m. UTC | #1
On Wed,  5 Apr 2023 23:22:26 +0200
Patrik Dahlström <risca@dalakolonin.se> 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.

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@kernel.org/
> [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
  
Jonathan Cameron April 7, 2023, 5:09 p.m. UTC | #2
On Fri, 7 Apr 2023 18:04:35 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> On Wed,  5 Apr 2023 23:22:26 +0200
> Patrik Dahlström <risca@dalakolonin.se> 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@kernel.org/
> > [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  
>
  
Patrik Dahlström April 8, 2023, 10:51 a.m. UTC | #3
On Fri, Apr 07, 2023 at 06:09:41PM +0100, Jonathan Cameron wrote:
> On Fri, 7 Apr 2023 18:04:35 +0100
> Jonathan Cameron <jic23@kernel.org> wrote:
> 
> > On Wed,  5 Apr 2023 23:22:26 +0200
> > Patrik Dahlström <risca@dalakolonin.se> 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).

I'll rebase to iio/togreg, fix up the cosmetics, and add the devm patch to
the series. Sounds good?

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

No worries, I'm just glad this is progressing :)

> 
> 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@kernel.org/
> > > [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  
> > 
>