mbox series

[v3,00/20] iio:adc:ad7280a Cleanup and proposed staging graduation.

Message ID 20220206190328.333093-1-jic23@kernel.org (mailing list archive)
Headers show
Series iio:adc:ad7280a Cleanup and proposed staging graduation. | expand

Message

Jonathan Cameron Feb. 6, 2022, 7:03 p.m. UTC
From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Changes since v2:
All in response to Marcelo Schmitt doing a really detailed review and
testing against the QEMU model.

- Moved elements of patch 2 to patch 5 to avoid missing defines.
- Patch 6: Amend ordering to avoid updated software cached values until the
  hardware write has succeeded.
- Patch 12 Fixed wrong callbacks in no_irq case
- Patch 14: Changed all numeric properties to u32 to avoid needing to
  specify bit depth in dts.
- Patch 14: Typo fix.
- Patch 15: New Patch to drop the handling of partial aux alert channels as
  it is buggy (and always was). Anyone wanting to bring this back will
  hopefully have hardware to test as it will be fiddly.
- Patch 15 (old number, now 16). Update dt bindings to drop the
  adi,temp-alert-last-chan given new patch 15 drops the support.
- New Patch 18: Change cb_mask software cache to not include the offset of 2
  and instead apply that only when writing the registers.
- New Patch 19: More conservative timings to allow 105 degree Centigrade
  operation.
- Patch 20 is moving the new code so obviously all the above apply to that
  as well!

Hi All,

This one proved an interesting diversion.

Work done against a somewhat hacked up QEMU emulation of 3 daisy chained
ad7280a devices (18 channels).  Note that the emulation isn't complete
but does do chaining, CRC, and readout of channels etc in a fashion that
worked with the original driver (up to the bug in patch 1) and continues
to work with the updated version. I've not intention to upstream the
emulation (as would need to make it more completed and flexible), but
happy to share it with anyone who is interested.

I briefly flirted with posting a patch to just drop the driver entirely,
but the part is still available and it looked like fun + isn't going
to greatly impact maintainability of the subsystem long term so is low
cost even if it becomes unavailable sometime soon.

There are lots of things we could do after this set to improved the driver
and make things more flexible, but it should basically 'just work'

Anyhow, as normal for staging graduations, last patch has rename detection
turned off so that people can easily see what I am proposing we move
out of staging.

All comments welcome and thanks to Marcelo for reviewing this beast!

Thanks,

Jonathan


Jonathan Cameron (20):
  staging:iio:adc:ad7280a: Fix handing of device address bit reversing.
  staging:iio:adc:ad7280a: Register define cleanup.
  staging:iio:adc:ad7280a: rename _read() to _read_reg()
  staging:iio:adc:ad7280a: Split buff[2] into tx and rx parts
  staging:iio:adc:ad7280a: Use bitfield ops to managed fields in
    transfers.
  staging:iio:adc:ad7280a: Switch to standard event control
  staging:iio:adc:ad7280a: Standardize extended ABI naming
  staging:iio:adc:ad7280a: Drop unused timestamp channel.
  staging:iio:adc:ad7280a: Trivial comment formatting cleanup
  staging:iio:adc:ad7280a: Make oversampling_ratio a runtime control
  staging:iio:adc:ad7280a: Cleanup includes
  staging:iio:ad7280a: Reflect optionality of irq in ABI
  staging:iio:adc:ad7280a: Use a local dev pointer to avoid &spi->dev
  staging:iio:adc:ad7280a: Use device properties to replace platform
    data.
  staging:iio:adc:ad7280a: Drop buggy support for early termination of
    AUX alert.
  dt-bindings:iio:adc:ad7280a: Add binding
  iio:adc:ad7280a: Document ABI for cell balance switches
  staging:iio:adc:ad7280a: Remove shift from cb_mask state cache.
  staging:iio:adc:ad7280a: Use more conservative delays to allow 105C
    operation.
  iio:adc:ad7280a: Move out of staging

 .../ABI/testing/sysfs-bus-iio-adc-ad7280a     |   13 +
 .../bindings/iio/adc/adi,ad7280a.yaml         |   77 ++
 drivers/iio/adc/Kconfig                       |   11 +
 drivers/iio/adc/Makefile                      |    1 +
 drivers/iio/adc/ad7280a.c                     | 1111 +++++++++++++++++
 drivers/staging/iio/adc/Kconfig               |   11 -
 drivers/staging/iio/adc/Makefile              |    1 -
 drivers/staging/iio/adc/ad7280a.c             | 1044 ----------------
 drivers/staging/iio/adc/ad7280a.h             |   37 -
 9 files changed, 1213 insertions(+), 1093 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-adc-ad7280a
 create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad7280a.yaml
 create mode 100644 drivers/iio/adc/ad7280a.c
 delete mode 100644 drivers/staging/iio/adc/ad7280a.c
 delete mode 100644 drivers/staging/iio/adc/ad7280a.h

Comments

Marcelo Schmitt Feb. 13, 2022, 8:11 p.m. UTC | #1
On 02/06, Jonathan Cameron wrote:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> Changes since v2:
> All in response to Marcelo Schmitt doing a really detailed review and
> testing against the QEMU model.
> 
> - Moved elements of patch 2 to patch 5 to avoid missing defines.
> - Patch 6: Amend ordering to avoid updated software cached values until the
>   hardware write has succeeded.
> - Patch 12 Fixed wrong callbacks in no_irq case
> - Patch 14: Changed all numeric properties to u32 to avoid needing to
>   specify bit depth in dts.
> - Patch 14: Typo fix.
> - Patch 15: New Patch to drop the handling of partial aux alert channels as
>   it is buggy (and always was). Anyone wanting to bring this back will
>   hopefully have hardware to test as it will be fiddly.
> - Patch 15 (old number, now 16). Update dt bindings to drop the
>   adi,temp-alert-last-chan given new patch 15 drops the support.
> - New Patch 18: Change cb_mask software cache to not include the offset of 2
>   and instead apply that only when writing the registers.
> - New Patch 19: More conservative timings to allow 105 degree Centigrade
>   operation.
> - Patch 20 is moving the new code so obviously all the above apply to that
>   as well!

I had another look into this set (including the new patches), and it seems all good. :)
Also, thanks for clarifying my doubts about the driver on the previous thread.
So,
Reviewed-by: Marcelo Schmitt <marcelo.schmitt1@gmail.com>
for the whole series.

It will be nice to see another driver getting out of staging. :D

Thanks,

Marcelo

> 
> Hi All,
> 
> This one proved an interesting diversion.
> 
> Work done against a somewhat hacked up QEMU emulation of 3 daisy chained
> ad7280a devices (18 channels).  Note that the emulation isn't complete
> but does do chaining, CRC, and readout of channels etc in a fashion that
> worked with the original driver (up to the bug in patch 1) and continues
> to work with the updated version. I've not intention to upstream the
> emulation (as would need to make it more completed and flexible), but
> happy to share it with anyone who is interested.
> 
> I briefly flirted with posting a patch to just drop the driver entirely,
> but the part is still available and it looked like fun + isn't going
> to greatly impact maintainability of the subsystem long term so is low
> cost even if it becomes unavailable sometime soon.
> 
> There are lots of things we could do after this set to improved the driver
> and make things more flexible, but it should basically 'just work'
> 
> Anyhow, as normal for staging graduations, last patch has rename detection
> turned off so that people can easily see what I am proposing we move
> out of staging.
> 
> All comments welcome and thanks to Marcelo for reviewing this beast!
> 
> Thanks,
> 
> Jonathan
> 
> 
> Jonathan Cameron (20):
>   staging:iio:adc:ad7280a: Fix handing of device address bit reversing.
>   staging:iio:adc:ad7280a: Register define cleanup.
>   staging:iio:adc:ad7280a: rename _read() to _read_reg()
>   staging:iio:adc:ad7280a: Split buff[2] into tx and rx parts
>   staging:iio:adc:ad7280a: Use bitfield ops to managed fields in
>     transfers.
>   staging:iio:adc:ad7280a: Switch to standard event control
>   staging:iio:adc:ad7280a: Standardize extended ABI naming
>   staging:iio:adc:ad7280a: Drop unused timestamp channel.
>   staging:iio:adc:ad7280a: Trivial comment formatting cleanup
>   staging:iio:adc:ad7280a: Make oversampling_ratio a runtime control
>   staging:iio:adc:ad7280a: Cleanup includes
>   staging:iio:ad7280a: Reflect optionality of irq in ABI
>   staging:iio:adc:ad7280a: Use a local dev pointer to avoid &spi->dev
>   staging:iio:adc:ad7280a: Use device properties to replace platform
>     data.
>   staging:iio:adc:ad7280a: Drop buggy support for early termination of
>     AUX alert.
>   dt-bindings:iio:adc:ad7280a: Add binding
>   iio:adc:ad7280a: Document ABI for cell balance switches
>   staging:iio:adc:ad7280a: Remove shift from cb_mask state cache.
>   staging:iio:adc:ad7280a: Use more conservative delays to allow 105C
>     operation.
>   iio:adc:ad7280a: Move out of staging
> 
>  .../ABI/testing/sysfs-bus-iio-adc-ad7280a     |   13 +
>  .../bindings/iio/adc/adi,ad7280a.yaml         |   77 ++
>  drivers/iio/adc/Kconfig                       |   11 +
>  drivers/iio/adc/Makefile                      |    1 +
>  drivers/iio/adc/ad7280a.c                     | 1111 +++++++++++++++++
>  drivers/staging/iio/adc/Kconfig               |   11 -
>  drivers/staging/iio/adc/Makefile              |    1 -
>  drivers/staging/iio/adc/ad7280a.c             | 1044 ----------------
>  drivers/staging/iio/adc/ad7280a.h             |   37 -
>  9 files changed, 1213 insertions(+), 1093 deletions(-)
>  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-adc-ad7280a
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad7280a.yaml
>  create mode 100644 drivers/iio/adc/ad7280a.c
>  delete mode 100644 drivers/staging/iio/adc/ad7280a.c
>  delete mode 100644 drivers/staging/iio/adc/ad7280a.h
> 
> -- 
> 2.35.1
>
Jonathan Cameron Feb. 18, 2022, 11:59 a.m. UTC | #2
On Sun, 13 Feb 2022 17:11:11 -0300
Marcelo Schmitt <marcelo.schmitt1@gmail.com> wrote:

> On 02/06, Jonathan Cameron wrote:
> > From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > 
> > Changes since v2:
> > All in response to Marcelo Schmitt doing a really detailed review and
> > testing against the QEMU model.
> > 
> > - Moved elements of patch 2 to patch 5 to avoid missing defines.
> > - Patch 6: Amend ordering to avoid updated software cached values until the
> >   hardware write has succeeded.
> > - Patch 12 Fixed wrong callbacks in no_irq case
> > - Patch 14: Changed all numeric properties to u32 to avoid needing to
> >   specify bit depth in dts.
> > - Patch 14: Typo fix.
> > - Patch 15: New Patch to drop the handling of partial aux alert channels as
> >   it is buggy (and always was). Anyone wanting to bring this back will
> >   hopefully have hardware to test as it will be fiddly.
> > - Patch 15 (old number, now 16). Update dt bindings to drop the
> >   adi,temp-alert-last-chan given new patch 15 drops the support.
> > - New Patch 18: Change cb_mask software cache to not include the offset of 2
> >   and instead apply that only when writing the registers.
> > - New Patch 19: More conservative timings to allow 105 degree Centigrade
> >   operation.
> > - Patch 20 is moving the new code so obviously all the above apply to that
> >   as well!  
> 
> I had another look into this set (including the new patches), and it seems all good. :)
> Also, thanks for clarifying my doubts about the driver on the previous thread.
> So,
> Reviewed-by: Marcelo Schmitt <marcelo.schmitt1@gmail.com>
> for the whole series.
> 
> It will be nice to see another driver getting out of staging. :D

Hi Marcelo,

Thanks for your help with this.

Applied to the togreg branch of iio.git and pushed out as testing to
let 0-day see if it can find anything we missed.

Guess I'll have to figure out which one of the remaining staging drivers
to tackle next in my (very) slow moving project to get them all out...

I actually have hardware for the adt7316 so maybe time to wire that up
though IIRC I could only get SPI to work intermittently due to
my dubious soldering.  Keeping it all in software by using emulation
for this one was nice :)

Jonathan

> 
> Thanks,
> 
> Marcelo
> 
> > 
> > Hi All,
> > 
> > This one proved an interesting diversion.
> > 
> > Work done against a somewhat hacked up QEMU emulation of 3 daisy chained
> > ad7280a devices (18 channels).  Note that the emulation isn't complete
> > but does do chaining, CRC, and readout of channels etc in a fashion that
> > worked with the original driver (up to the bug in patch 1) and continues
> > to work with the updated version. I've not intention to upstream the
> > emulation (as would need to make it more completed and flexible), but
> > happy to share it with anyone who is interested.
> > 
> > I briefly flirted with posting a patch to just drop the driver entirely,
> > but the part is still available and it looked like fun + isn't going
> > to greatly impact maintainability of the subsystem long term so is low
> > cost even if it becomes unavailable sometime soon.
> > 
> > There are lots of things we could do after this set to improved the driver
> > and make things more flexible, but it should basically 'just work'
> > 
> > Anyhow, as normal for staging graduations, last patch has rename detection
> > turned off so that people can easily see what I am proposing we move
> > out of staging.
> > 
> > All comments welcome and thanks to Marcelo for reviewing this beast!
> > 
> > Thanks,
> > 
> > Jonathan
> > 
> > 
> > Jonathan Cameron (20):
> >   staging:iio:adc:ad7280a: Fix handing of device address bit reversing.
> >   staging:iio:adc:ad7280a: Register define cleanup.
> >   staging:iio:adc:ad7280a: rename _read() to _read_reg()
> >   staging:iio:adc:ad7280a: Split buff[2] into tx and rx parts
> >   staging:iio:adc:ad7280a: Use bitfield ops to managed fields in
> >     transfers.
> >   staging:iio:adc:ad7280a: Switch to standard event control
> >   staging:iio:adc:ad7280a: Standardize extended ABI naming
> >   staging:iio:adc:ad7280a: Drop unused timestamp channel.
> >   staging:iio:adc:ad7280a: Trivial comment formatting cleanup
> >   staging:iio:adc:ad7280a: Make oversampling_ratio a runtime control
> >   staging:iio:adc:ad7280a: Cleanup includes
> >   staging:iio:ad7280a: Reflect optionality of irq in ABI
> >   staging:iio:adc:ad7280a: Use a local dev pointer to avoid &spi->dev
> >   staging:iio:adc:ad7280a: Use device properties to replace platform
> >     data.
> >   staging:iio:adc:ad7280a: Drop buggy support for early termination of
> >     AUX alert.
> >   dt-bindings:iio:adc:ad7280a: Add binding
> >   iio:adc:ad7280a: Document ABI for cell balance switches
> >   staging:iio:adc:ad7280a: Remove shift from cb_mask state cache.
> >   staging:iio:adc:ad7280a: Use more conservative delays to allow 105C
> >     operation.
> >   iio:adc:ad7280a: Move out of staging
> > 
> >  .../ABI/testing/sysfs-bus-iio-adc-ad7280a     |   13 +
> >  .../bindings/iio/adc/adi,ad7280a.yaml         |   77 ++
> >  drivers/iio/adc/Kconfig                       |   11 +
> >  drivers/iio/adc/Makefile                      |    1 +
> >  drivers/iio/adc/ad7280a.c                     | 1111 +++++++++++++++++
> >  drivers/staging/iio/adc/Kconfig               |   11 -
> >  drivers/staging/iio/adc/Makefile              |    1 -
> >  drivers/staging/iio/adc/ad7280a.c             | 1044 ----------------
> >  drivers/staging/iio/adc/ad7280a.h             |   37 -
> >  9 files changed, 1213 insertions(+), 1093 deletions(-)
> >  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-adc-ad7280a
> >  create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad7280a.yaml
> >  create mode 100644 drivers/iio/adc/ad7280a.c
> >  delete mode 100644 drivers/staging/iio/adc/ad7280a.c
> >  delete mode 100644 drivers/staging/iio/adc/ad7280a.h
> > 
> > -- 
> > 2.35.1
> >