[v4,0/4] Add support for AF8133J magnetometer

From: Ondrej Jirman <megi@xff.cz>

This series adds support for AF8133J magnetometer sensor. It's a simple
3-axis sensor with two sensitivity options and not much else to it.

This sensor is used on both Pinephone and Pinephone Pro. DT patches
adding it will come later, once this driver is merged.

Please take a look. :)

Thank you very much,
	Ondřej Jirman

v4:
- move RPM enable in probe function before iio device registration

v3:
- collect more tags
- if (ret < 0) -> (ret) where appropriate
- scoped guard move to af8133j_set_scale()
- remove pm_runtime_disable/enable guard from af8133j_power_down_action()
- pretty much just this:
  https://megous.com/dl/tmp/0001-if-ret-0-ret-where-appropriate.patch
  https://megous.com/dl/tmp/0002-scoped-guard-move-to-af8133j_set_scale.patch
  https://megous.com/dl/tmp/0003-remove-pm_runtime_disable-enable-guard-from-af8133j_.patch

v2:
- move maintainers patch to the end of series
- bindings:
  - fix compatible definition in bindings file
  - require power supplies
  - fix descriptions
- driver:
  - sort includes
  - rework RPM, the driver should now work with RPM disabled
    among other improvements
    - I've tested RPM left and right doing device bind/unbind under
      various conditions, system suspend under various conditions,
      etc.
  - use scoped_guard for mutexes
  - use devm for power down and handle power down correctly with both
    RPM enabled/disabled without tracking power state in data->powered
  - fix issue with changing scale while RPM suspended
  - various code formatting issues resolved
- as for sign-offs, I've added co-developed-by for people I know for
  sure worked on the driver, and left other tags as they were when
  I picked up the patch 2 years ago to my Linux branch

Icenowy Zheng (3):
  dt-bindings: vendor-prefix: Add prefix for Voltafield
  dt-bindings: iio: magnetometer: Add Voltafield AF8133J
  iio: magnetometer: add a driver for Voltafield AF8133J magnetometer

Ondrej Jirman (1):
  MAINTAINERS: Add an entry for AF8133J driver

 .../iio/magnetometer/voltafield,af8133j.yaml  |  60 ++
 .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
 MAINTAINERS                                   |   6 +
 drivers/iio/magnetometer/Kconfig              |  12 +
 drivers/iio/magnetometer/Makefile             |   1 +
 drivers/iio/magnetometer/af8133j.c            | 524 ++++++++++++++++++
 6 files changed, 605 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/magnetometer/voltafield,af8133j.yaml
 create mode 100644 drivers/iio/magnetometer/af8133j.c

Hi,

On 24-02-22 02:13, Ondřej Jirman wrote:
> From: Icenowy Zheng <icenowy@aosc.io>
> 
> AF8133J is a simple I2C-connected magnetometer, without interrupts.
> 
> Add a simple IIO driver for it.
> 
> Co-developed-by: Icenowy Zheng <icenowy@aosc.io>
> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> Signed-off-by: Dalton Durst <dalton@ubports.com>
> Signed-off-by: Shoji Keita <awaittrot@shjk.jp>
> Co-developed-by: Ondrej Jirman <megi@xff.cz>
> Signed-off-by: Ondrej Jirman <megi@xff.cz>
> Reviewed-by: Andrey Skvortsov <andrej.skvortzov@gmail.com>
> Tested-by: Andrey Skvortsov <andrej.skvortzov@gmail.com>
> ---
>  drivers/iio/magnetometer/Kconfig   |  12 +
>  drivers/iio/magnetometer/Makefile  |   1 +
>  drivers/iio/magnetometer/af8133j.c | 524 +++++++++++++++++++++++++++++
>  3 files changed, 537 insertions(+)
>  create mode 100644 drivers/iio/magnetometer/af8133j.c
> 
> diff --git a/drivers/iio/magnetometer/Kconfig b/drivers/iio/magnetometer/Kconfig
> index 38532d840f2a..cd2917d71904 100644
> --- a/drivers/iio/magnetometer/Kconfig
> +++ b/drivers/iio/magnetometer/Kconfig
> @@ -6,6 +6,18 @@

I've tested v4 driver again on 6.8-rc5. Works like before.

On Thu, 22 Feb 2024 02:13:37 +0100
Ondřej Jirman <megi@xff.cz> wrote:

> From: Icenowy Zheng <icenowy@aosc.io>
> 
> AF8133J is a simple I2C-connected magnetometer, without interrupts.
> 
> Add a simple IIO driver for it.
> 
> Co-developed-by: Icenowy Zheng <icenowy@aosc.io>
> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> Signed-off-by: Dalton Durst <dalton@ubports.com>
> Signed-off-by: Shoji Keita <awaittrot@shjk.jp>
> Co-developed-by: Ondrej Jirman <megi@xff.cz>
> Signed-off-by: Ondrej Jirman <megi@xff.cz>
> Reviewed-by: Andrey Skvortsov <andrej.skvortzov@gmail.com>
> Tested-by: Andrey Skvortsov <andrej.skvortzov@gmail.com>

One additional tweak from my local build tests.

static

> +const struct dev_pm_ops af8133j_pm_ops = {
> +	SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, pm_runtime_force_resume)
> +	RUNTIME_PM_OPS(af8133j_runtime_suspend, af8133j_runtime_resume, NULL)
> +};

Hello Jonathan,

On Sun, Feb 25, 2024 at 12:07:00PM +0000, Jonathan Cameron wrote:
> On Thu, 22 Feb 2024 02:13:37 +0100
> Ondřej Jirman <megi@xff.cz> wrote:
> 
> > From: Icenowy Zheng <icenowy@aosc.io>
> > 
> > AF8133J is a simple I2C-connected magnetometer, without interrupts.
> > 
> > Add a simple IIO driver for it.
> > 
> > Co-developed-by: Icenowy Zheng <icenowy@aosc.io>
> > Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> Check patch correct moaned that Icenowy is the author (from:)
> so doesn't need a co-developed.
> 
> > Signed-off-by: Dalton Durst <dalton@ubports.com>
> > Signed-off-by: Shoji Keita <awaittrot@shjk.jp>
> > Co-developed-by: Ondrej Jirman <megi@xff.cz>
> > Signed-off-by: Ondrej Jirman <megi@xff.cz>
> > Reviewed-by: Andrey Skvortsov <andrej.skvortzov@gmail.com>
> > Tested-by: Andrey Skvortsov <andrej.skvortzov@gmail.com>
> 
> Hi.
> 
> A few really minor things noticed during a final review.
> I'll tweak them whilst applying.  Diff is

Thank you very much for finishing touches.

> > +static int af8133j_product_check(struct af8133j_data *data)
> > +{
> > +	struct device *dev = &data->client->dev;
> > +	unsigned int val;
> > +	int ret;
> > +
> > +	ret = regmap_read(data->regmap, AF8133J_REG_PCODE, &val);
> > +	if (ret) {
> > +		dev_err(dev, "Error reading product code (%d)\n", ret);
> > +		return ret;
> > +	}
> > +
> > +	if (val != AF8133J_REG_PCODE_VAL) {
> > +		dev_err(dev, "Invalid product code (0x%02x)\n", val);
> > +		return -EINVAL;
> 
> This should be warn only and we should carry on regardless.  The reason
> behind this is to support fallback compatible values in DT to potentially enable
> a newer device to be supported on an older kernel.
> 
> Many IIO drivers do this wrong as my understanding on what counted on
> 'compatible' used to be different.  Long discussions on this with the DT
> maintainers led me to accept that letting ID checks fail was fine, but
> that a message was appropriate.   Often a fail here actually means no device.
> We have some exceptions to this rule for devices where we know the same
> FW ids are in use in the wild for devices supported by different Linux
> drivers - but those are thankfully rare!

Makes sense. If newer device variant has the same register meanings, but just
a different ID register, this way it can be supported without driver
modifications. I'll keep it in mind when doing ID checks in other drivers.

Thanks for all your detailed notes during the review. I learned a few
new subtleties.

Kind regards,
	o.

> > +	}
> > +
> > +	return 0;
> > +}
> > +
> }
> 
> > +static const int af8133j_scales[][2] = {
> > +	[0] = { 0, 366210 }, // 12 gauss
> > +	[1] = { 0, 671386 }, // 22 gauss
> Trivial so I'll fix it up: IIO comments are /* */
> not C++ style (with exception of the SPDX stuff that needs to be).
> > +};
> 
> 
>