On Wed, 14 Feb 2024 18:43:10 +0100
Ondřej Jirman <megi@xff.cz> wrote:
> Hi Jonathan,
Gah. Runtime pm always gives me a headache. I'd indeed misunderstood some
of what you are doing.
>
> On Wed, Feb 14, 2024 at 05:01:36PM +0000, Jonathan Cameron wrote:
> > On Mon, 12 Feb 2024 18:53:55 +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>
> >
> >
> > Hi a few comments (mostly on changes)
> >
> > The runtime_pm handling can be simplified somewhat if you
> > rearrange probe a little.
> >
> > > diff --git a/drivers/iio/magnetometer/af8133j.c b/drivers/iio/magnetometer/af8133j.c
> > > new file mode 100644
> > > index 000000000000..1f64a2337f6e
> > > --- /dev/null
> > > +++ b/drivers/iio/magnetometer/af8133j.c
> > > @@ -0,0 +1,528 @@
> >
> >
> > > +static int af8133j_take_measurement(struct af8133j_data *data)
> > > +{
> > > + unsigned int val;
> > > + int ret;
> > > +
> > > + ret = regmap_write(data->regmap,
> > > + AF8133J_REG_STATE, AF8133J_REG_STATE_WORK);
> > > + if (ret < 0)
> > > + return ret;
> > > +
> > > + /* The datasheet says "Mesaure Time <1.5ms" */
> > > + ret = regmap_read_poll_timeout(data->regmap, AF8133J_REG_STATUS, val,
> > > + val & AF8133J_REG_STATUS_ACQ,
> > > + 500, 1500);
> > > + if (ret < 0)
> > > + return ret;
> > > +
> > > + ret = regmap_write(data->regmap,
> > > + AF8133J_REG_STATE, AF8133J_REG_STATE_STBY);
> >
> > return regmap_write()
> >
> > regmap accesses return 0 or a negative error code enabling little code
> > reductions like this.
>
> Yeah, some reviewers dislike this, because modifying the code in the future
> creates a more unpleasant diff. But if you like this style, I don't mind
> changing it.
Always a gamble on chance of a modification coming.
In general I'd check regmap calls with if (ret) but don't feel that strongly
about that either.
So not really important either way.
>
> > > + if (ret < 0)
> > > + return ret;
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int af8133j_read_measurement(struct af8133j_data *data, __le16 buf[3])
> > > +{
> > > + struct device *dev = &data->client->dev;
> > > + int ret;
> > > +
> > > + ret = pm_runtime_resume_and_get(dev);
> > > + if (ret) {
> > > + /*
> > > + * Ignore EACCES because that happens when RPM is disabled
> > > + * during system sleep, while userspace leave eg. hrtimer
> > > + * trigger attached and IIO core keeps trying to do measurements.
> >
> > Yeah. We still need to fix that more elegantly :(
> >
> > > + */
> > > + if (ret != -EACCES)
> > > + dev_err(dev, "Failed to power on (%d)\n", ret);
> > > + return ret;
> > > + }
> > > +
> > > + scoped_guard(mutex, &data->mutex) {
> > > + ret = af8133j_take_measurement(data);
> > > + if (ret)
> > > + goto out_rpm_put;
> > > +
> > > + ret = regmap_bulk_read(data->regmap, AF8133J_REG_OUT,
> > > + buf, sizeof(__le16) * 3);
> > > + }
> > > +
> > > +out_rpm_put:
> > > + pm_runtime_mark_last_busy(dev);
> > > + pm_runtime_put_autosuspend(dev);
> > > +
> > > + return ret;
> > > +}
> > > +
> >
> >
> > > +
> > > +static int af8133j_set_scale(struct af8133j_data *data,
> > > + unsigned int val, unsigned int val2)
> > > +{
> > > + struct device *dev = &data->client->dev;
> > > + u8 range;
> > > + int ret = 0;
> > > +
> > > + if (af8133j_scales[0][0] == val && af8133j_scales[0][1] == val2)
> > > + range = AF8133J_REG_RANGE_12G;
> > > + else if (af8133j_scales[1][0] == val && af8133j_scales[1][1] == val2)
> > > + range = AF8133J_REG_RANGE_22G;
> > > + else
> > > + return -EINVAL;
> > > +
> > > + pm_runtime_disable(dev);
> > > +
> > > + /*
> > > + * When suspended, just store the new range to data->range to be
> > > + * applied later during power up.
> > Better to just do
> > pm_runtime_resume_and_get() here
> >
> > > + */
> > > + if (!pm_runtime_status_suspended(dev))
> > > + ret = regmap_write(data->regmap, AF8133J_REG_RANGE, range);
> > > +
> > > + pm_runtime_enable(dev);
> > and
> > pm_runtime_mark_last_busy(dev);
> > pm_runtime_put_autosuspend(dev);
> > here.
> >
> > The userspace interface is only way this function is called so rearrange
> > probe a little so that you don't need extra complexity in these functions.
>
> It doesn't make sense to wakeup the device for range change, because it will
> forget the range the moment it's powered off again, after changing the range.
Ah. I'd missed understood that. Thanks for extra explanation.
I'm not keen on the enable / disable dance but anything else is probably worse
(delaying update until we actually using it etc).
>
> Also this function has nothing to do with probe. data->range is authoritative
> value, not cache. It gets applied to HW on each power up.
>
> >
> > > +
> > > + data->range = range;
> >
> > If the write failed, generally don't update the cached value.
>
> Right.
>
> > > + return ret;
> > > +}
> > > +
> > > +static int af8133j_write_raw(struct iio_dev *indio_dev,
> > > + struct iio_chan_spec const *chan,
> > > + int val, int val2, long mask)
> > > +{
> > > + struct af8133j_data *data = iio_priv(indio_dev);
> > > + int ret;
> > > +
> > > + switch (mask) {
> > > + case IIO_CHAN_INFO_SCALE:
> > > + scoped_guard(mutex, &data->mutex)
> > > + ret = af8133j_set_scale(data, val, val2);
> >
> > Look more closely at what scoped_guard() does.
> > return af8133j_set_scale(data, val, val2);
> > is fine and simpler as no local variable needed.
>
> I did, it will not work as you suggest. It's implemented as for loop with
> condition, and the compiler will complain about fallthrough.
>
> I can do:
>
> scoped_guard(mutex, &data->mutex)
> return af8133j_set_scale(data, val, val2);
> return 0;
>
> but it looks weirder at first glance, at least to my eye.
I agree that bit is less than ideal, but with your code it should also
get confused about whether ret is ever set.
scoped_guard(mutex, &data->mutex)
return ...
unreachable();
perhaps? or just use a guard and add scope manually
case IIO_CHAN_INFO_SCALE: {
guard(mutex)(&data->mutex);
return af8133j_set_scale(...);
}
I'd go with this as the cleanest solution in this case.
>
> > > + return ret;
> > > + default:
> > > + return -EINVAL;
> > > + }
> > > +}
> >
> > > +static void af8133j_power_down_action(void *ptr)
> > > +{
> > > + struct af8133j_data *data = ptr;
> > > + struct device *dev = &data->client->dev;
> > > +
> > > + pm_runtime_disable(dev);
> > You group together unwinding of calls that occur in very
> > different places in probe. Don't do that as it leas
> > to disabling runtime pm having never enabled it
> > in some error paths. That may be safe but if fails the
> > obviously correct test.
>
> This whole disable/enable dance is here so that pm_runtime_status_suspended can
> be trusted. Not for disabling PM during device remove or in error paths.
>
> There's no imbalance here or problem with disabling PM when it's already
> disabled. Disable/enable is reference counted, and this function keeps the
> balance.
Whilst not buggy but I still want to be able to cleanly associate a given
bit of cleanup with what is being cleaned up. That is the path to
maintainable code longer term. Runtime PM does make a mess of doing this
but tends to have somewhat logical sets of calls that go together.
As long as we hold a reference, doesn't matter when we turn it on in probe()
Only the put_autosuspend has to come after we done talking to it.
>
> > So this is a good solution to the normal dance of turning power on
> > just to turn it off shortly afterwards.
> >
> > > + af8133j_power_down(data);
> > > + pm_runtime_enable(dev);
> > Why?
>
> See above. To keep the disable ref count balanced.
>
> Looks like actual RPM disable already happened at this point a bit earlier in
> another callback registered via devm_pm_runtime_enable(). I guess this
> pm_runtime_enable()/pm_runtime_disable() guard can just be skipped, because RPM
> is already disabled thanks to reverse ordering of devm callbacks during device
> remove. So while this is safe, it's redundant at this point and call to
> pm_runtime_status_suspended() is safe without this.
Yes, That's a side effect of only enabling it right at the end.
>
> > > +}
> > > +
> > > +static int af8133j_probe(struct i2c_client *client)
> > > +{
> > > + struct device *dev = &client->dev;
> > > + struct af8133j_data *data;
> > > + struct iio_dev *indio_dev;
> > > + struct regmap *regmap;
> > > + int ret, i;
> > > +
> > > + indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
> > > + if (!indio_dev)
> > > + return -ENOMEM;
> > > +
> > > + regmap = devm_regmap_init_i2c(client, &af8133j_regmap_config);
> > > + if (IS_ERR(regmap))
> > > + return dev_err_probe(dev, PTR_ERR(regmap),
> > > + "regmap initialization failed\n");
> > > +
> > > + data = iio_priv(indio_dev);
> > > + i2c_set_clientdata(client, indio_dev);
> > > + data->client = client;
> > > + data->regmap = regmap;
> > > + data->range = AF8133J_REG_RANGE_12G;
> > > + mutex_init(&data->mutex);
> > > +
> > > + data->reset_gpiod = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
> > > + if (IS_ERR(data->reset_gpiod))
> > > + return dev_err_probe(dev, PTR_ERR(data->reset_gpiod),
> > > + "Failed to get reset gpio\n");
> > > +
> > > + for (i = 0; i < ARRAY_SIZE(af8133j_supply_names); i++)
> > > + data->supplies[i].supply = af8133j_supply_names[i];
> > > + ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(data->supplies),
> > > + data->supplies);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + ret = iio_read_mount_matrix(dev, &data->orientation);
> > > + if (ret)
> > > + return dev_err_probe(dev, ret, "Failed to read mount matrix\n");
> > > +
> > > + ret = af8133j_power_up(data);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + pm_runtime_set_active(dev);
> > > +
> > > + ret = devm_add_action_or_reset(dev, af8133j_power_down_action, data);
> >
> > As mentioned above, this should only undo things done before this point.
> > So just the af8133j_power_down() I think.
>
> The callback doesn't do anything else but power down. It leaves everything
> else as is after it exits.
>
> > > + if (ret)
> > > + return ret;
> > > +
> > > + ret = af8133j_product_check(data);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + indio_dev->info = &af8133j_info;
> > > + indio_dev->name = "af8133j";
> > > + indio_dev->channels = af8133j_channels;
> > > + indio_dev->num_channels = ARRAY_SIZE(af8133j_channels);
> > > + indio_dev->modes = INDIO_DIRECT_MODE;
> > > +
> > > + ret = devm_iio_triggered_buffer_setup(dev, indio_dev, NULL,
> > > + &af8133j_trigger_handler, NULL);
> > > + if (ret < 0)
> > > + return dev_err_probe(&client->dev, ret,
> > > + "Failed to setup iio triggered buffer\n");
> > > +
> > > + ret = devm_iio_device_register(dev, indio_dev);
> > > + if (ret)
> > > + return dev_err_probe(dev, ret, "Failed to register iio device");
> > > +
> > > + pm_runtime_get_noresume(dev);
> >
> > > + pm_runtime_use_autosuspend(dev);
> > > + pm_runtime_set_autosuspend_delay(dev, 500);
> > > + ret = devm_pm_runtime_enable(dev);
> >
> > This already deals with pm_runtime_disable() so you shouldn't need do it manually.
>
> I'm not disabling RPM manually, it was just used as temporary guard to be able
> to read pm_runtime_status_suspended() safely.
I'd indeed misunderstood that. I forgot the oddity that runtime pm is effectively
reference counting in only one direction for enable / disable and the opposite
one for get and put.
pm_runtime_disable()
pm_runtime_disable()
pm_runtime_enable()
pm_runtime_enable()
pm_runtime_enable()
is fine, but
pm_runtime_enable()
pm_runtime_enable()
pm_runtime_disable()
pm_runtime_disable()
pm_runtime_enable()
is not.
Which makes sense when you realise it's all about ensuring it is off, but
never ensuring that it is turned on.
>
> > Also you want to enable that before the devm_iio_device_register() to avoid
> > problems with it not being available as the userspace interfaces are used.
> >
> > So just move this up a few lines.
>
> Good idea, thanks.
>
> kind regards,
> o.
>
> >
> >
> > > + if (ret)
> > > + return ret;
> > > +
> > > + pm_runtime_put_autosuspend(dev);
> > > +
> > > + return 0;
> > > +}
> >
@@ -6,6 +6,18 @@
menu "Magnetometer sensors"
+config AF8133J
+ tristate "Voltafield AF8133J 3-Axis Magnetometer"
+ depends on I2C
+ depends on OF
+ select REGMAP_I2C
+ help
+ Say yes here to build support for Voltafield AF8133J I2C-based
+ 3-axis magnetometer chip.
+
+ To compile this driver as a module, choose M here: the module
+ will be called af8133j.
+
config AK8974
tristate "Asahi Kasei AK8974 3-Axis Magnetometer"
depends on I2C
@@ -4,6 +4,7 @@
#
# When adding new entries keep the list in alphabetical order
+obj-$(CONFIG_AF8133J) += af8133j.o
obj-$(CONFIG_AK8974) += ak8974.o
obj-$(CONFIG_AK8975) += ak8975.o
obj-$(CONFIG_BMC150_MAGN) += bmc150_magn.o
new file mode 100644
@@ -0,0 +1,528 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * af8133j.c - Voltafield AF8133J magnetometer driver
+ *
+ * Copyright 2021 Icenowy Zheng <icenowy@aosc.io>
+ * Copyright 2024 Ondřej Jirman <megi@xff.cz>
+ */
+
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/pm_runtime.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+
+#include <linux/iio/iio.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/iio/triggered_buffer.h>
+
+#define AF8133J_REG_OUT 0x03
+#define AF8133J_REG_PCODE 0x00
+#define AF8133J_REG_PCODE_VAL 0x5e
+#define AF8133J_REG_STATUS 0x02
+#define AF8133J_REG_STATUS_ACQ BIT(0)
+#define AF8133J_REG_STATE 0x0a
+#define AF8133J_REG_STATE_STBY 0x00
+#define AF8133J_REG_STATE_WORK 0x01
+#define AF8133J_REG_RANGE 0x0b
+#define AF8133J_REG_RANGE_22G 0x12
+#define AF8133J_REG_RANGE_12G 0x34
+#define AF8133J_REG_SWR 0x11
+#define AF8133J_REG_SWR_PERFORM 0x81
+
+static const char * const af8133j_supply_names[] = {
+ "avdd",
+ "dvdd",
+};
+
+struct af8133j_data {
+ struct i2c_client *client;
+ struct regmap *regmap;
+ struct mutex mutex;
+ struct iio_mount_matrix orientation;
+
+ struct gpio_desc *reset_gpiod;
+ struct regulator_bulk_data supplies[ARRAY_SIZE(af8133j_supply_names)];
+
+ u8 range;
+};
+
+enum af8133j_axis {
+ AXIS_X = 0,
+ AXIS_Y,
+ AXIS_Z,
+};
+
+static struct iio_mount_matrix *
+af8133j_get_mount_matrix(struct iio_dev *indio_dev,
+ const struct iio_chan_spec *chan)
+{
+ struct af8133j_data *data = iio_priv(indio_dev);
+
+ return &data->orientation;
+}
+
+static const struct iio_chan_spec_ext_info af8133j_ext_info[] = {
+ IIO_MOUNT_MATRIX(IIO_SHARED_BY_DIR, af8133j_get_mount_matrix),
+ { }
+};
+
+#define AF8133J_CHANNEL(_si, _axis) { \
+ .type = IIO_MAGN, \
+ .modified = 1, \
+ .channel2 = IIO_MOD_ ## _axis, \
+ .address = AXIS_ ## _axis, \
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
+ .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
+ .info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_SCALE), \
+ .ext_info = af8133j_ext_info, \
+ .scan_index = _si, \
+ .scan_type = { \
+ .sign = 's', \
+ .realbits = 16, \
+ .storagebits = 16, \
+ .endianness = IIO_LE, \
+ }, \
+}
+
+static const struct iio_chan_spec af8133j_channels[] = {
+ AF8133J_CHANNEL(0, X),
+ AF8133J_CHANNEL(1, Y),
+ AF8133J_CHANNEL(2, Z),
+ IIO_CHAN_SOFT_TIMESTAMP(3),
+};
+
+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 < 0) {
+ 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;
+ }
+
+ return 0;
+}
+
+static int af8133j_reset(struct af8133j_data *data)
+{
+ struct device *dev = &data->client->dev;
+ int ret;
+
+ if (data->reset_gpiod) {
+ /* If we have GPIO reset line, use it */
+ gpiod_set_value_cansleep(data->reset_gpiod, 1);
+ udelay(10);
+ gpiod_set_value_cansleep(data->reset_gpiod, 0);
+ } else {
+ /* Otherwise use software reset */
+ ret = regmap_write(data->regmap, AF8133J_REG_SWR,
+ AF8133J_REG_SWR_PERFORM);
+ if (ret < 0) {
+ dev_err(dev, "Failed to reset the chip\n");
+ return ret;
+ }
+ }
+
+ /* Wait for reset to finish */
+ usleep_range(1000, 1100);
+
+ /* Restore range setting */
+ if (data->range == AF8133J_REG_RANGE_22G) {
+ ret = regmap_write(data->regmap, AF8133J_REG_RANGE, data->range);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
+static void af8133j_power_down(struct af8133j_data *data)
+{
+ gpiod_set_value_cansleep(data->reset_gpiod, 1);
+ regulator_bulk_disable(ARRAY_SIZE(data->supplies), data->supplies);
+}
+
+static int af8133j_power_up(struct af8133j_data *data)
+{
+ struct device *dev = &data->client->dev;
+ int ret;
+
+ ret = regulator_bulk_enable(ARRAY_SIZE(data->supplies), data->supplies);
+ if (ret) {
+ dev_err(dev, "Could not enable regulators\n");
+ return ret;
+ }
+
+ gpiod_set_value_cansleep(data->reset_gpiod, 0);
+
+ /* Wait for power on reset */
+ usleep_range(15000, 16000);
+
+ ret = af8133j_reset(data);
+ if (ret) {
+ af8133j_power_down(data);
+ return ret;
+ }
+
+ return 0;
+}
+
+static int af8133j_take_measurement(struct af8133j_data *data)
+{
+ unsigned int val;
+ int ret;
+
+ ret = regmap_write(data->regmap,
+ AF8133J_REG_STATE, AF8133J_REG_STATE_WORK);
+ if (ret < 0)
+ return ret;
+
+ /* The datasheet says "Mesaure Time <1.5ms" */
+ ret = regmap_read_poll_timeout(data->regmap, AF8133J_REG_STATUS, val,
+ val & AF8133J_REG_STATUS_ACQ,
+ 500, 1500);
+ if (ret < 0)
+ return ret;
+
+ ret = regmap_write(data->regmap,
+ AF8133J_REG_STATE, AF8133J_REG_STATE_STBY);
+ if (ret < 0)
+ return ret;
+
+ return 0;
+}
+
+static int af8133j_read_measurement(struct af8133j_data *data, __le16 buf[3])
+{
+ struct device *dev = &data->client->dev;
+ int ret;
+
+ ret = pm_runtime_resume_and_get(dev);
+ if (ret) {
+ /*
+ * Ignore EACCES because that happens when RPM is disabled
+ * during system sleep, while userspace leave eg. hrtimer
+ * trigger attached and IIO core keeps trying to do measurements.
+ */
+ if (ret != -EACCES)
+ dev_err(dev, "Failed to power on (%d)\n", ret);
+ return ret;
+ }
+
+ scoped_guard(mutex, &data->mutex) {
+ ret = af8133j_take_measurement(data);
+ if (ret)
+ goto out_rpm_put;
+
+ ret = regmap_bulk_read(data->regmap, AF8133J_REG_OUT,
+ buf, sizeof(__le16) * 3);
+ }
+
+out_rpm_put:
+ pm_runtime_mark_last_busy(dev);
+ pm_runtime_put_autosuspend(dev);
+
+ return ret;
+}
+
+static const int af8133j_scales[][2] = {
+ [0] = { 0, 366210 }, // 12 gauss
+ [1] = { 0, 671386 }, // 22 gauss
+};
+
+static int af8133j_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan, int *val,
+ int *val2, long mask)
+{
+ struct af8133j_data *data = iio_priv(indio_dev);
+ __le16 buf[3];
+ int ret;
+
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ ret = af8133j_read_measurement(data, buf);
+ if (ret < 0)
+ return ret;
+
+ *val = sign_extend32(le16_to_cpu(buf[chan->address]),
+ chan->scan_type.realbits - 1);
+ return IIO_VAL_INT;
+ case IIO_CHAN_INFO_SCALE:
+ *val = 0;
+
+ if (data->range == AF8133J_REG_RANGE_12G)
+ *val2 = af8133j_scales[0][1];
+ else
+ *val2 = af8133j_scales[1][1];
+
+ return IIO_VAL_INT_PLUS_NANO;
+ default:
+ return -EINVAL;
+ }
+}
+
+static int af8133j_read_avail(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ const int **vals, int *type, int *length,
+ long mask)
+{
+ switch (mask) {
+ case IIO_CHAN_INFO_SCALE:
+ *vals = (const int *)af8133j_scales;
+ *length = ARRAY_SIZE(af8133j_scales) * 2;
+ *type = IIO_VAL_INT_PLUS_NANO;
+ return IIO_AVAIL_LIST;
+ default:
+ return -EINVAL;
+ }
+}
+
+static int af8133j_set_scale(struct af8133j_data *data,
+ unsigned int val, unsigned int val2)
+{
+ struct device *dev = &data->client->dev;
+ u8 range;
+ int ret = 0;
+
+ if (af8133j_scales[0][0] == val && af8133j_scales[0][1] == val2)
+ range = AF8133J_REG_RANGE_12G;
+ else if (af8133j_scales[1][0] == val && af8133j_scales[1][1] == val2)
+ range = AF8133J_REG_RANGE_22G;
+ else
+ return -EINVAL;
+
+ pm_runtime_disable(dev);
+
+ /*
+ * When suspended, just store the new range to data->range to be
+ * applied later during power up.
+ */
+ if (!pm_runtime_status_suspended(dev))
+ ret = regmap_write(data->regmap, AF8133J_REG_RANGE, range);
+
+ pm_runtime_enable(dev);
+
+ data->range = range;
+ return ret;
+}
+
+static int af8133j_write_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int val, int val2, long mask)
+{
+ struct af8133j_data *data = iio_priv(indio_dev);
+ int ret;
+
+ switch (mask) {
+ case IIO_CHAN_INFO_SCALE:
+ scoped_guard(mutex, &data->mutex)
+ ret = af8133j_set_scale(data, val, val2);
+ return ret;
+ default:
+ return -EINVAL;
+ }
+}
+
+static int af8133j_write_raw_get_fmt(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ long mask)
+{
+ return IIO_VAL_INT_PLUS_NANO;
+}
+
+static const struct iio_info af8133j_info = {
+ .read_raw = af8133j_read_raw,
+ .read_avail = af8133j_read_avail,
+ .write_raw = af8133j_write_raw,
+ .write_raw_get_fmt = af8133j_write_raw_get_fmt,
+};
+
+static irqreturn_t af8133j_trigger_handler(int irq, void *p)
+{
+ struct iio_poll_func *pf = p;
+ struct iio_dev *indio_dev = pf->indio_dev;
+ struct af8133j_data *data = iio_priv(indio_dev);
+ s64 timestamp = iio_get_time_ns(indio_dev);
+ struct {
+ __le16 values[3];
+ s64 timestamp __aligned(8);
+ } sample;
+ int ret;
+
+ memset(&sample, 0, sizeof(sample));
+
+ ret = af8133j_read_measurement(data, sample.values);
+ if (ret)
+ goto out_done;
+
+ iio_push_to_buffers_with_timestamp(indio_dev, &sample, timestamp);
+
+out_done:
+ iio_trigger_notify_done(indio_dev->trig);
+
+ return IRQ_HANDLED;
+}
+
+static const struct regmap_config af8133j_regmap_config = {
+ .name = "af8133j_regmap",
+ .reg_bits = 8,
+ .val_bits = 8,
+ .max_register = AF8133J_REG_SWR,
+ .cache_type = REGCACHE_NONE,
+};
+
+static void af8133j_power_down_action(void *ptr)
+{
+ struct af8133j_data *data = ptr;
+ struct device *dev = &data->client->dev;
+
+ pm_runtime_disable(dev);
+ if (!pm_runtime_status_suspended(dev))
+ af8133j_power_down(data);
+ pm_runtime_enable(dev);
+}
+
+static int af8133j_probe(struct i2c_client *client)
+{
+ struct device *dev = &client->dev;
+ struct af8133j_data *data;
+ struct iio_dev *indio_dev;
+ struct regmap *regmap;
+ int ret, i;
+
+ indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
+ if (!indio_dev)
+ return -ENOMEM;
+
+ regmap = devm_regmap_init_i2c(client, &af8133j_regmap_config);
+ if (IS_ERR(regmap))
+ return dev_err_probe(dev, PTR_ERR(regmap),
+ "regmap initialization failed\n");
+
+ data = iio_priv(indio_dev);
+ i2c_set_clientdata(client, indio_dev);
+ data->client = client;
+ data->regmap = regmap;
+ data->range = AF8133J_REG_RANGE_12G;
+ mutex_init(&data->mutex);
+
+ data->reset_gpiod = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
+ if (IS_ERR(data->reset_gpiod))
+ return dev_err_probe(dev, PTR_ERR(data->reset_gpiod),
+ "Failed to get reset gpio\n");
+
+ for (i = 0; i < ARRAY_SIZE(af8133j_supply_names); i++)
+ data->supplies[i].supply = af8133j_supply_names[i];
+ ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(data->supplies),
+ data->supplies);
+ if (ret)
+ return ret;
+
+ ret = iio_read_mount_matrix(dev, &data->orientation);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to read mount matrix\n");
+
+ ret = af8133j_power_up(data);
+ if (ret)
+ return ret;
+
+ pm_runtime_set_active(dev);
+
+ ret = devm_add_action_or_reset(dev, af8133j_power_down_action, data);
+ if (ret)
+ return ret;
+
+ ret = af8133j_product_check(data);
+ if (ret)
+ return ret;
+
+ indio_dev->info = &af8133j_info;
+ indio_dev->name = "af8133j";
+ indio_dev->channels = af8133j_channels;
+ indio_dev->num_channels = ARRAY_SIZE(af8133j_channels);
+ indio_dev->modes = INDIO_DIRECT_MODE;
+
+ ret = devm_iio_triggered_buffer_setup(dev, indio_dev, NULL,
+ &af8133j_trigger_handler, NULL);
+ if (ret < 0)
+ return dev_err_probe(&client->dev, ret,
+ "Failed to setup iio triggered buffer\n");
+
+ ret = devm_iio_device_register(dev, indio_dev);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to register iio device");
+
+ pm_runtime_get_noresume(dev);
+ pm_runtime_use_autosuspend(dev);
+ pm_runtime_set_autosuspend_delay(dev, 500);
+ ret = devm_pm_runtime_enable(dev);
+ if (ret)
+ return ret;
+
+ pm_runtime_put_autosuspend(dev);
+
+ return 0;
+}
+
+static int af8133j_runtime_suspend(struct device *dev)
+{
+ struct iio_dev *indio_dev = dev_get_drvdata(dev);
+ struct af8133j_data *data = iio_priv(indio_dev);
+
+ af8133j_power_down(data);
+
+ return 0;
+}
+
+static int af8133j_runtime_resume(struct device *dev)
+{
+ struct iio_dev *indio_dev = dev_get_drvdata(dev);
+ struct af8133j_data *data = iio_priv(indio_dev);
+
+ return af8133j_power_up(data);
+}
+
+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)
+};
+
+static const struct of_device_id af8133j_of_match[] = {
+ { .compatible = "voltafield,af8133j", },
+ { }
+};
+MODULE_DEVICE_TABLE(of, af8133j_of_match);
+
+static const struct i2c_device_id af8133j_id[] = {
+ { "af8133j", 0 },
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, af8133j_id);
+
+static struct i2c_driver af8133j_driver = {
+ .driver = {
+ .name = "af8133j",
+ .of_match_table = af8133j_of_match,
+ .pm = pm_ptr(&af8133j_pm_ops),
+ },
+ .probe = af8133j_probe,
+ .id_table = af8133j_id,
+};
+
+module_i2c_driver(af8133j_driver);
+
+MODULE_AUTHOR("Icenowy Zheng <icenowy@aosc.io>");
+MODULE_AUTHOR("Ondřej Jirman <megi@xff.cz>");
+MODULE_DESCRIPTION("Voltafield AF8133J magnetic sensor driver");
+MODULE_LICENSE("GPL");