* Re: [PATCH V3 5/9] iio: imu: inv_icm42607: Add Temperature Support in icm42607
From: David Lechner @ 2026-04-10 22:34 UTC (permalink / raw)
To: Chris Morgan, linux-iio
Cc: andy, nuno.sa, jic23, jean-baptiste.maneyrol, linux-rockchip,
devicetree, heiko, conor+dt, krzk+dt, robh, andriy.shevchenko,
Chris Morgan
In-Reply-To: <20260330195853.392877-6-macroalpha82@gmail.com>
On 3/30/26 2:58 PM, Chris Morgan wrote:
> From: Chris Morgan <macromorgan@hotmail.com>
>
> Add functions for reading temperature sensor data.
>
> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> ---
> drivers/iio/imu/inv_icm42607/inv_icm42607.h | 3 +
> .../iio/imu/inv_icm42607/inv_icm42607_core.c | 17 ++++
> .../iio/imu/inv_icm42607/inv_icm42607_temp.c | 81 +++++++++++++++++++
> .../iio/imu/inv_icm42607/inv_icm42607_temp.h | 30 +++++++
> 4 files changed, 131 insertions(+)
> create mode 100644 drivers/iio/imu/inv_icm42607/inv_icm42607_temp.c
> create mode 100644 drivers/iio/imu/inv_icm42607/inv_icm42607_temp.h
>
> diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607.h b/drivers/iio/imu/inv_icm42607/inv_icm42607.h
> index 5530fd3bc03f..086848c8fd3b 100644
> --- a/drivers/iio/imu/inv_icm42607/inv_icm42607.h
> +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607.h
> @@ -433,6 +433,9 @@ extern const struct dev_pm_ops inv_icm42607_pm_ops;
>
> u32 inv_icm42607_odr_to_period(enum inv_icm42607_odr odr);
>
> +int inv_icm42607_set_temp_conf(struct inv_icm42607_state *st, bool enable,
> + unsigned int *sleep_ms);
> +
> int inv_icm42607_debugfs_reg(struct iio_dev *indio_dev, unsigned int reg,
> unsigned int writeval, unsigned int *readval);
>
> diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
> index 344071089042..735a262dc103 100644
> --- a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
> +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
> @@ -164,6 +164,23 @@ static int inv_icm42607_set_pwr_mgmt0(struct inv_icm42607_state *st,
> return 0;
> }
>
> +int inv_icm42607_set_temp_conf(struct inv_icm42607_state *st, bool enable,
> + unsigned int *sleep_ms)
> +{
> + unsigned int val;
> + int ret;
> +
> + val = INV_ICM42607_TEMP_CONFIG0_FILTER(INV_ICM42607_FILTER_BW_34HZ);
> + ret = regmap_update_bits(st->map, INV_ICM42607_REG_TEMP_CONFIG0,
> + INV_ICM42607_TEMP_CONFIG0_FILTER_MASK, val);
> + if (ret)
> + return ret;
> +
> + return inv_icm42607_set_pwr_mgmt0(st, st->conf.gyro.mode,
> + st->conf.accel.mode, enable,
> + sleep_ms);
> +}
> +
> int inv_icm42607_debugfs_reg(struct iio_dev *indio_dev, unsigned int reg,
> unsigned int writeval, unsigned int *readval)
> {
> diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_temp.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_temp.c
> new file mode 100644
> index 000000000000..b42eb78cd960
> --- /dev/null
> +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_temp.c
> @@ -0,0 +1,81 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) 2026 InvenSense, Inc.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/device.h>
> +#include <linux/mutex.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/regmap.h>
> +#include <linux/iio/iio.h>
> +
> +#include "inv_icm42607.h"
> +#include "inv_icm42607_temp.h"
> +
> +static int inv_icm42607_temp_read(struct inv_icm42607_state *st, s16 *temp)
> +{
> + struct device *dev = regmap_get_device(st->map);
> + __be16 *raw;
> + int ret;
> +
> + PM_RUNTIME_ACQUIRE_AUTOSUSPEND(dev, pm);
> + if (PM_RUNTIME_ACQUIRE_ERR(&pm))
> + return -ENXIO;
> +
> + guard(mutex)(&st->lock);
> +
> + ret = inv_icm42607_set_temp_conf(st, true, NULL);
> + if (ret)
> + return ret;
> +
> + raw = (__be16 *)&st->buffer[0];
Can we make buffer __be16 buffer[] to avoid cast and ensure proper alignment?
> + ret = regmap_bulk_read(st->map, INV_ICM42607_REG_TEMP_DATA1, raw, sizeof(*raw));
> + if (ret)
> + return ret;
> +
> + *temp = (s16)be16_to_cpup(raw);
cast is not needed. temp is already s16.
> + if (*temp == INV_ICM42607_DATA_INVALID)
> + ret = -EINVAL;
> +
> + return ret;
> +}
> +
> +int inv_icm42607_temp_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int *val, int *val2, long mask)
> +{
> + struct inv_icm42607_state *st = iio_device_get_drvdata(indio_dev);
> + s16 temp;
> + int ret;
> +
> + if (chan->type != IIO_TEMP)
> + return -EINVAL;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + if (!iio_device_claim_direct(indio_dev))
> + return -EBUSY;
> + ret = inv_icm42607_temp_read(st, &temp);
> + iio_device_release_direct(indio_dev);
> + if (ret)
> + return ret;
> + *val = temp;
> + return IIO_VAL_INT;
> + /*
> + * T°C = (temp / 128) + 25
> + * Tm°C = 1000 * ((temp * 100 / 12800) + 25)
> + * scale: 100000 / 12800 ~= 7.8125
> + * offset: 25000
> + */
> + case IIO_CHAN_INFO_SCALE:
> + *val = 7;
> + *val2 = 812500;
> + return IIO_VAL_INT_PLUS_MICRO;
Could use IIO_VAL_INT_PLUS_NANO to get more exact value.
> + case IIO_CHAN_INFO_OFFSET:
> + *val = 25000;
> + return IIO_VAL_INT;
> + default:
> + return -EINVAL;
> + }
> +}
^ permalink raw reply
* Re: [PATCH 2/3] pmdomain: core: add support for power-domains-child-ids
From: Kevin Hilman @ 2026-04-10 22:25 UTC (permalink / raw)
To: Ulf Hansson
Cc: Rob Herring, Geert Uytterhoeven, linux-pm, devicetree,
linux-kernel, arm-scmi, linux-arm-kernel
In-Reply-To: <CAPDyKFrR2zyMFXTAkKs1XRgB-u5jSP256g730s=7SLuOZKsKVg@mail.gmail.com>
Ulf Hansson <ulf.hansson@linaro.org> writes:
> On Fri, 10 Apr 2026 at 02:45, Kevin Hilman <khilman@baylibre.com> wrote:
>>
>> Ulf Hansson <ulf.hansson@linaro.org> writes:
>>
>> > On Wed, 11 Mar 2026 at 01:19, Kevin Hilman (TI) <khilman@baylibre.com> wrote:
>> >>
>> >> Currently, PM domains can only support hierarchy for simple
>> >> providers (e.g. ones with #power-domain-cells = 0).
>> >>
>> >> Add support for oncell providers as well by adding a new property
>> >> `power-domains-child-ids` to describe the parent/child relationship.
>> >>
>> >> For example, an SCMI PM domain provider has multiple domains, each of
>> >> which might be a child of diffeent parent domains. In this example,
>> >> the parent domains are MAIN_PD and WKUP_PD:
>> >>
>> >> scmi_pds: protocol@11 {
>> >> reg = <0x11>;
>> >> #power-domain-cells = <1>;
>> >> power-domains = <&MAIN_PD>, <&WKUP_PD>;
>> >> power-domains-child-ids = <15>, <19>;
>> >> };
>> >>
>> >> With this example using the new property, SCMI PM domain 15 becomes a
>> >> child domain of MAIN_PD, and SCMI domain 19 becomes a child domain of
>> >> WKUP_PD.
>> >>
>> >> To support this feature, add two new core functions
>> >>
>> >> - of_genpd_add_child_ids()
>> >> - of_genpd_remove_child_ids()
>> >>
>> >> which can be called by pmdomain providers to add/remove child domains
>> >> if they support the new property power-domains-child-ids.
>> >>
>> >> Signed-off-by: Kevin Hilman (TI) <khilman@baylibre.com>
>> >
>> > Thanks for working on this! It certainly is a missing feature!
>>
>> You're welcome, thanks for the detailed review.
>>
>> >> ---
>> >> drivers/pmdomain/core.c | 169 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> >> include/linux/pm_domain.h | 16 ++++++++++++++++
>> >> 2 files changed, 185 insertions(+)
>> >>
>> >> diff --git a/drivers/pmdomain/core.c b/drivers/pmdomain/core.c
>> >> index 61c2277c9ce3..acb45dd540b7 100644
>> >> --- a/drivers/pmdomain/core.c
>> >> +++ b/drivers/pmdomain/core.c
>> >> @@ -2909,6 +2909,175 @@ static struct generic_pm_domain *genpd_get_from_provider(
>> >> return genpd;
>> >> }
>> >>
>> >> +/**
>> >> + * of_genpd_add_child_ids() - Parse power-domains-child-ids property
>> >> + * @np: Device node pointer associated with the PM domain provider.
>> >> + * @data: Pointer to the onecell data associated with the PM domain provider.
>> >> + *
>> >> + * Parse the power-domains and power-domains-child-ids properties to establish
>> >> + * parent-child relationships for PM domains. The power-domains property lists
>> >> + * parent domains, and power-domains-child-ids lists which child domain IDs
>> >> + * should be associated with each parent.
>> >> + *
>> >> + * Returns 0 on success, -ENOENT if properties don't exist, or negative error code.
>> >
>> > I think we should avoid returning specific error codes for specific
>> > errors, simply because it usually becomes messy.
>> >
>> > If I understand correctly the intent here is to allow the caller to
>> > check for -ENOENT and potentially avoid bailing out as it may not
>> > really be an error, right?
>>
>> Right, -ENOENT is not an error of parsing, it's to indicate that there
>> are no child-ids to be parsed.
>>
>> > Perhaps a better option is to return the number of children for whom
>> > we successfully assigned parents. Hence 0 or a positive value allows
>> > the caller to understand what happened. More importantly, a negative
>> > error code then really becomes an error for the caller to consider.
>>
>> I explored this a bit, but it gets messy quick. It means we have to
>> track cases where only some of the children were added as well as when
>> all children were added. Personally, I think this should be an "all or
>> nothing" thing. If all the children cannot be parsed/added, then none
>> of them should be added.
>>
>> This also allows the remove to not have to care about how many were
>> added, and just remove them all, with the additional benefit of not
>> having to track the state of how many children were successfully added.
>>
>
> I fully agree, it should be all or nothing. Failing with one
> child/parent should end up with an error code being returned.
>
> That said, it still seems to make perfect sense to return the number
> of children for whom we assigned parents for, no?
No, because what will the caller use that number for? If we are
assuming "all or nothing", what would we use it for (other than a debug print?)
It also makes it a bit confusing what a zero return value means. Does
that mean success? Or that zero children were added (which would be
fail.)
I prefer to keep it as is.
Kevin
^ permalink raw reply
* Re: [PATCH V3 3/9] iio: imu: inv_icm42607: Add I2C and SPI For icm42607
From: David Lechner @ 2026-04-10 22:21 UTC (permalink / raw)
To: Chris Morgan, linux-iio
Cc: andy, nuno.sa, jic23, jean-baptiste.maneyrol, linux-rockchip,
devicetree, heiko, conor+dt, krzk+dt, robh, andriy.shevchenko,
Chris Morgan
In-Reply-To: <20260330195853.392877-4-macroalpha82@gmail.com>
On 3/30/26 2:58 PM, Chris Morgan wrote:
> From: Chris Morgan <macromorgan@hotmail.com>
>
> Add I2C and SPI driver support for InvenSense ICM-42607 devices.
> Include runtime power management on each device.
Power management seems unrelated, so likey belongs in a separate patch.
>
> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> ---
> drivers/iio/imu/inv_icm42607/inv_icm42607.h | 14 ++
> .../iio/imu/inv_icm42607/inv_icm42607_core.c | 204 ++++++++++++++++++
> .../iio/imu/inv_icm42607/inv_icm42607_i2c.c | 93 ++++++++
> .../iio/imu/inv_icm42607/inv_icm42607_spi.c | 100 +++++++++
> 4 files changed, 411 insertions(+)
> create mode 100644 drivers/iio/imu/inv_icm42607/inv_icm42607_i2c.c
> create mode 100644 drivers/iio/imu/inv_icm42607/inv_icm42607_spi.c
>
> diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607.h b/drivers/iio/imu/inv_icm42607/inv_icm42607.h
> index 609188c40ffc..7d13091aa8df 100644
> --- a/drivers/iio/imu/inv_icm42607/inv_icm42607.h
> +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607.h
> @@ -11,6 +11,7 @@
> #include <linux/regmap.h>
> #include <linux/mutex.h>
> #include <linux/regulator/consumer.h>
> +#include <linux/pm.h>
> #include <linux/iio/iio.h>
> #include <linux/iio/common/inv_sensors_timestamp.h>
>
> @@ -21,6 +22,16 @@ enum inv_icm42607_chip {
> INV_CHIP_NB,
> };
>
> +/* serial bus slew rates */
> +enum inv_icm42607_slew_rate {
> + INV_ICM42607_SLEW_RATE_20_60NS,
> + INV_ICM42607_SLEW_RATE_12_36NS,
> + INV_ICM42607_SLEW_RATE_6_18NS,
> + INV_ICM42607_SLEW_RATE_4_12NS,
> + INV_ICM42607_SLEW_RATE_2_6NS,
> + INV_ICM42607_SLEW_RATE_INF_2NS,
> +};
> +
> enum inv_icm42607_sensor_mode {
> INV_ICM42607_SENSOR_MODE_OFF,
> INV_ICM42607_SENSOR_MODE_STANDBY,
> @@ -413,6 +424,9 @@ struct inv_icm42607_sensor_state {
>
> typedef int (*inv_icm42607_bus_setup)(struct inv_icm42607_state *);
>
> +extern const struct regmap_config inv_icm42607_regmap_config;
> +extern const struct dev_pm_ops inv_icm42607_pm_ops;
> +
> u32 inv_icm42607_odr_to_period(enum inv_icm42607_odr odr);
>
> int inv_icm42607_debugfs_reg(struct iio_dev *indio_dev, unsigned int reg,
> diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
> index 6b7078387568..da04c820dab2 100644
> --- a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
> +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
> @@ -12,12 +12,33 @@
> #include <linux/interrupt.h>
> #include <linux/irq.h>
> #include <linux/regulator/consumer.h>
> +#include <linux/pm_runtime.h>
> #include <linux/property.h>
> #include <linux/regmap.h>
> #include <linux/iio/iio.h>
>
> #include "inv_icm42607.h"
>
> +static const struct regmap_range_cfg inv_icm42607_regmap_ranges[] = {
> + {
> + .name = "user bank",
> + .range_min = 0x0000,
> + .range_max = 0x00FF,
> + .window_start = 0,
> + .window_len = 0x0100,
> + },
> +};
> +
> +const struct regmap_config inv_icm42607_regmap_config = {
> + .reg_bits = 8,
> + .val_bits = 8,
> + .max_register = 0x00FF,
> + .ranges = inv_icm42607_regmap_ranges,
> + .num_ranges = ARRAY_SIZE(inv_icm42607_regmap_ranges),
> + .cache_type = REGCACHE_NONE,
> +};
> +EXPORT_SYMBOL_NS_GPL(inv_icm42607_regmap_config, "IIO_ICM42607");
It would make more sense to include the regmap config in the first patch
since it is shared.
> +
> struct inv_icm42607_hw {
> uint8_t whoami;
> const char *name;
> @@ -86,6 +107,62 @@ u32 inv_icm42607_odr_to_period(enum inv_icm42607_odr odr)
> return odr_periods[odr];
> }
>
> +static int inv_icm42607_set_pwr_mgmt0(struct inv_icm42607_state *st,
> + enum inv_icm42607_sensor_mode gyro,
> + enum inv_icm42607_sensor_mode accel,
> + bool temp, unsigned int *sleep_ms)
> +{
> + enum inv_icm42607_sensor_mode oldgyro = st->conf.gyro.mode;
> + enum inv_icm42607_sensor_mode oldaccel = st->conf.accel.mode;
> + bool oldtemp = st->conf.temp_en;
> + unsigned int sleepval;
> + unsigned int val;
> + int ret;
> +
> + if (gyro == oldgyro && accel == oldaccel && temp == oldtemp)
> + return 0;
> +
> + val = INV_ICM42607_PWR_MGMT0_GYRO(gyro) |
> + INV_ICM42607_PWR_MGMT0_ACCEL(accel);
> + if (!temp)
> + val |= INV_ICM42607_PWR_MGMT0_ACCEL_LP_CLK_SEL;
> + ret = regmap_write(st->map, INV_ICM42607_REG_PWR_MGMT0, val);
> + if (ret)
> + return ret;
> +
> + st->conf.gyro.mode = gyro;
> + st->conf.accel.mode = accel;
> + st->conf.temp_en = temp;
> +
> + sleepval = 0;
> + if (temp && !oldtemp) {
> + if (sleepval < INV_ICM42607_TEMP_STARTUP_TIME_MS)
> + sleepval = INV_ICM42607_TEMP_STARTUP_TIME_MS;
> + }
> + if (accel != oldaccel && oldaccel == INV_ICM42607_SENSOR_MODE_OFF) {
> + usleep_range(200, 300);
> + if (sleepval < INV_ICM42607_ACCEL_STARTUP_TIME_MS)
> + sleepval = INV_ICM42607_ACCEL_STARTUP_TIME_MS;
> + }
> + if (gyro != oldgyro) {
> + if (oldgyro == INV_ICM42607_SENSOR_MODE_OFF) {
> + usleep_range(200, 300);
> + if (sleepval < INV_ICM42607_GYRO_STARTUP_TIME_MS)
> + sleepval = INV_ICM42607_GYRO_STARTUP_TIME_MS;
> + } else if (gyro == INV_ICM42607_SENSOR_MODE_OFF) {
> + if (sleepval < INV_ICM42607_GYRO_STOP_TIME_MS)
> + sleepval = INV_ICM42607_GYRO_STOP_TIME_MS;
> + }
> + }
> +
> + if (sleep_ms)
> + *sleep_ms = sleepval;
> + else if (sleepval)
> + msleep(sleepval);
> +
> + return 0;
> +}
> +
> int inv_icm42607_debugfs_reg(struct iio_dev *indio_dev, unsigned int reg,
> unsigned int writeval, unsigned int *readval)
> {
> @@ -219,6 +296,10 @@ static int inv_icm42607_enable_vddio_reg(struct inv_icm42607_state *st)
> static void inv_icm42607_disable_vddio_reg(void *_data)
> {
> struct inv_icm42607_state *st = _data;
> + struct device *dev = regmap_get_device(st->map);
> +
> + if (pm_runtime_status_suspended(dev))
> + return;
>
> regulator_disable(st->vddio_supply);
> }
> @@ -289,11 +370,134 @@ int inv_icm42607_core_probe(struct regmap *regmap, int chip,
>
> /* Setup chip registers (includes WHOAMI check, reset check, bus setup) */
> ret = inv_icm42607_setup(st, bus_setup);
> + if (ret)
> + return ret; /* Return error from setup (e.g., WHOAMI fail) */
> +
> + /* Setup runtime power management */
Would add a blank line here if this is meant to apply to more than the
following line. Otherwise it doesn't add much.
> + ret = devm_pm_runtime_set_active_enabled(dev);
> + if (ret)
> + return ret;
> +
> + pm_runtime_set_autosuspend_delay(dev, INV_ICM42607_SUSPEND_DELAY_MS);
> + pm_runtime_use_autosuspend(dev);
>
> return ret;
> }
> EXPORT_SYMBOL_NS_GPL(inv_icm42607_core_probe, "IIO_ICM42607");
>
...
> diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_i2c.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_i2c.c
> new file mode 100644
> index 000000000000..eb72973debc5
> --- /dev/null
> +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_i2c.c
> @@ -0,0 +1,93 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) 2026 InvenSense, Inc.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/device.h>
> +#include <linux/module.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/i2c.h>
> +#include <linux/regmap.h>
> +#include <linux/property.h>
> +
> +#include "inv_icm42607.h"
> +
> +static int inv_icm42607_i2c_bus_setup(struct inv_icm42607_state *st)
> +{
> + unsigned int mask, val;
> + int ret;
> +
> + ret = regmap_update_bits(st->map, INV_ICM42607_REG_INTF_CONFIG1,
> + INV_ICM42607_INTF_CONFIG1_I3C_DDR_EN |
> + INV_ICM42607_INTF_CONFIG1_I3C_SDR_EN, 0);
regmap_clear_bits()
> + if (ret)
> + return ret;
> +
> + mask = INV_ICM42607_DRIVE_CONFIG2_I2C_MASK;
Local mask variable isn't helping much.
> + val = INV_ICM42607_DRIVE_CONFIG2_I2C(INV_ICM42607_SLEW_RATE_12_36NS);
> + ret = regmap_update_bits(st->map, INV_ICM42607_REG_DRIVE_CONFIG2,
> + mask, val);
> + if (ret)
> + return ret;
> +
> + return regmap_update_bits(st->map, INV_ICM42607_REG_INTF_CONFIG0,
> + INV_ICM42607_INTF_CONFIG0_UI_SIFS_CFG_MASK,
> + INV_ICM42607_INTF_CONFIG0_UI_SIFS_CFG_SPI_DIS);
> +}
> +
> +static int inv_icm42607_probe(struct i2c_client *client)
> +{
> + const void *match;
> + enum inv_icm42607_chip chip;
> + struct regmap *regmap;
> +
> + if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_I2C_BLOCK))
> + return -EOPNOTSUPP;
> +
> + match = device_get_match_data(&client->dev);
Should be i2c_get_match_data(). And we recently decided to standardize
on not checking for NULL return.
> + if (!match)
> + return -EINVAL;
> + chip = (uintptr_t)match;
> +
> + regmap = devm_regmap_init_i2c(client, &inv_icm42607_regmap_config);
> + if (IS_ERR(regmap))
> + return PTR_ERR(regmap);
> +
> + return inv_icm42607_core_probe(regmap, chip, inv_icm42607_i2c_bus_setup);
> +}
> +
> +static const struct i2c_device_id inv_icm42607_id[] = {
> + { "icm42607", INV_CHIP_ICM42607 },
> + { "icm42607p", INV_CHIP_ICM42607P },
> + { }
> +};
> +MODULE_DEVICE_TABLE(i2c, inv_icm42607_id);
> +
> +static const struct of_device_id inv_icm42607_of_matches[] = {
> + {
> + .compatible = "invensense,icm42607",
> + .data = (void *)INV_CHIP_ICM42607,
> + }, {
> + .compatible = "invensense,icm42607p",
> + .data = (void *)INV_CHIP_ICM42607P,
> + },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, inv_icm42607_of_matches);
> +
> +static struct i2c_driver inv_icm42607_driver = {
> + .driver = {
> + .name = "inv-icm42607-i2c",
> + .of_match_table = inv_icm42607_of_matches,
> + .pm = pm_ptr(&inv_icm42607_pm_ops),
> + },
> + .id_table = inv_icm42607_id,
> + .probe = inv_icm42607_probe,
> +};
> +module_i2c_driver(inv_icm42607_driver);
> +
> +MODULE_AUTHOR("InvenSense, Inc.");
> +MODULE_DESCRIPTION("InvenSense ICM-42607x I2C driver");
> +MODULE_LICENSE("GPL");
> +MODULE_IMPORT_NS("IIO_ICM42607");
> diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_spi.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_spi.c
> new file mode 100644
> index 000000000000..51ce3deeb706
> --- /dev/null
> +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_spi.c
> @@ -0,0 +1,100 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) 2026 InvenSense, Inc.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/device.h>
> +#include <linux/module.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/spi/spi.h>
> +#include <linux/regmap.h>
> +#include <linux/property.h>
> +
> +#include "inv_icm42607.h"
> +
> +static int inv_icm42607_spi_bus_setup(struct inv_icm42607_state *st)
> +{
> + unsigned int mask, val;
> + int ret;
> +
> + ret = regmap_update_bits(st->map, INV_ICM42607_REG_DEVICE_CONFIG,
> + INV_ICM42607_DEVICE_CONFIG_SPI_AP_4WIRE,
> + INV_ICM42607_DEVICE_CONFIG_SPI_AP_4WIRE);
regmap_set_bits()
> + if (ret)
> + return ret;
> +
> + ret = regmap_update_bits(st->map, INV_ICM42607_REG_INTF_CONFIG1,
> + INV_ICM42607_INTF_CONFIG1_I3C_DDR_EN |
> + INV_ICM42607_INTF_CONFIG1_I3C_SDR_EN, 0);
regmap_clear_bits()
> + if (ret)
> + return ret;
> +
> + mask = INV_ICM42607_DRIVE_CONFIG3_SPI_MASK;
> + val = INV_ICM42607_DRIVE_CONFIG3_SPI(INV_ICM42607_SLEW_RATE_INF_2NS);
> + ret = regmap_update_bits(st->map, INV_ICM42607_REG_DRIVE_CONFIG3,
> + mask, val);
> + if (ret)
> + return ret;
> +
> + return regmap_update_bits(st->map, INV_ICM42607_REG_INTF_CONFIG0,
> + INV_ICM42607_INTF_CONFIG0_UI_SIFS_CFG_MASK,
> + INV_ICM42607_INTF_CONFIG0_UI_SIFS_CFG_I2C_DIS);
> +}
> +
> +static int inv_icm42607_probe(struct spi_device *spi)
> +{
> + const void *match;
> + enum inv_icm42607_chip chip;
> + struct regmap *regmap;
> +
> + match = device_get_match_data(&spi->dev);
> + if (!match)
> + return -EINVAL;
Should be spi_get_device_match_data(). And we recently decided to standardize
on not checking for NULL return.
> + chip = (uintptr_t)match;
uintptr_t is frowned upon in the kernel. Stick with kernel_ulong_t.
> +
> + regmap = devm_regmap_init_spi(spi, &inv_icm42607_regmap_config);
> + if (IS_ERR(regmap))
> + return dev_err_probe(&spi->dev, PTR_ERR(regmap),
> + "Failed to register spi regmap %ld\n",
> + PTR_ERR(regmap));
> +
> + return inv_icm42607_core_probe(regmap, chip,
> + inv_icm42607_spi_bus_setup);
> +}
> +
> +static const struct of_device_id inv_icm42607_of_matches[] = {
> + {
> + .compatible = "invensense,icm42607",
> + .data = (void *)INV_CHIP_ICM42607,
> + },
> + {
> + .compatible = "invensense,icm42607p",
> + .data = (void *)INV_CHIP_ICM42607P,
> + },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, inv_icm42607_of_matches);
> +
> +static const struct spi_device_id inv_icm42607_spi_id_table[] = {
> + { "icm42607", INV_CHIP_ICM42607 },
> + { "icm42607p", INV_CHIP_ICM42607P },
> + { },
No trailing comma.
> +};
> +MODULE_DEVICE_TABLE(spi, inv_icm42607_spi_id_table);
> +
> +static struct spi_driver inv_icm42607_driver = {
> + .driver = {
> + .name = "inv-icm42607-spi",
> + .of_match_table = inv_icm42607_of_matches,
> + .pm = &inv_icm42607_pm_ops,
> + },
> + .id_table = inv_icm42607_spi_id_table,
> + .probe = inv_icm42607_probe,
> +};
> +module_spi_driver(inv_icm42607_driver);
> +
> +MODULE_AUTHOR("InvenSense, Inc.");
> +MODULE_DESCRIPTION("InvenSense ICM-42607x SPI driver");
> +MODULE_LICENSE("GPL");
> +MODULE_IMPORT_NS("IIO_ICM42607");
^ permalink raw reply
* Re: [GIT PULL] RISC-V SpacemiT Devicetrees for v7.1
From: Linus Walleij @ 2026-04-10 22:19 UTC (permalink / raw)
To: Yixun Lan
Cc: soc, Yixun Lan, Arnd Bergmann, spacemit, linux-riscv, devicetree,
linux-kernel
In-Reply-To: <20260403123040-KYC0145825@kernel.org>
Hi Yixun,
I looked into this pull request.
I'm sorry if I do stupid mistakes in handling it, I'm new to maintaining
the SoC tree. Bear with me.
On Fri, Apr 3, 2026 at 2:32 PM Yixun Lan <dlan@kernel.org> wrote:
> Aurelien Jarno (7):
> riscv: dts: spacemit: drop incorrect pinctrl for combo PHY
(...)
> Yixun Lan (9):
> riscv: dts: spacemit: pcie: fix missing power regulator
[Fixes]
fatal: Not a valid object name linus/master
Commit: c68360c0d636 ("riscv: dts: spacemit: drop incorrect pinctrl
for combo PHY")
Fixes tag: Fixes: 0be016a4b5d1b9 ("riscv: dts: spacemit: PCIe and
PHY-related updates")
Has these problem(s):
- Inspect: Target SHA is not ancestor of Linus' master branch,
which means it is fixing commit in your branch
fatal: Not a valid object name linus/master
Commit: 8a9071299dec ("riscv: dts: spacemit: pcie: fix missing power regulator")
Fixes tag: Fixes: 0be016a4b5d1 ("riscv: dts: spacemit: PCIe and
PHY-related updates")
Has these problem(s):
- Inspect: Target SHA is not ancestor of Linus' master branch,
which means it is fixing commit in your branch
So this means you introduced bugs and fix them in the same pull request?
Why?
The practice is to squash such fixes into the offending patches when
presenting pull requests. But I went ahead anyway, trying to not be so
picky. (The commits are there, in your branch indeed.)
- Checked that it was in linux-next OK
- built DTBS OK
Pulled in, thanks.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH V3 2/9] iio: imu: inv_icm42607: Add Core for inv_icm42607 Driver
From: David Lechner @ 2026-04-10 22:06 UTC (permalink / raw)
To: Chris Morgan, linux-iio
Cc: andy, nuno.sa, jic23, jean-baptiste.maneyrol, linux-rockchip,
devicetree, heiko, conor+dt, krzk+dt, robh, andriy.shevchenko,
Chris Morgan
In-Reply-To: <20260330195853.392877-3-macroalpha82@gmail.com>
On 3/30/26 2:58 PM, Chris Morgan wrote:
> From: Chris Morgan <macromorgan@hotmail.com>
>
> Add the core component of a new inv_icm42607 driver. This includes
> a few setup functions and the full register definition in the
> header file.
>
> +#define INV_ICM42607_REG_PWR_MGMT0 0x1F
> +#define INV_ICM42607_PWR_MGMT0_ACCEL_LP_CLK_SEL BIT(7)
> +#define INV_ICM42607_PWR_MGMT0_IDLE BIT(4)
> +#define INV_ICM42607_PWR_MGMT0_GYRO_MODE_MASK GENMASK(3, 2)
> +#define INV_ICM42607_PWR_MGMT0_GYRO(_mode) \
> + FIELD_PREP(INV_ICM42607_PWR_MGMT0_GYRO_MODE_MASK, (_mode))
We usually try to avoid macros that hide FIELD_PREP(). IMHO, it makes
the code harder to read because you have to jump back and forth to
definitions to see if it really is that.
> +#define INV_ICM42607_PWR_MGMT0_ACCEL_MODE_MASK GENMASK(1, 0)
> +#define INV_ICM42607_PWR_MGMT0_ACCEL(_mode) \
> + FIELD_PREP(INV_ICM42607_PWR_MGMT0_ACCEL_MODE_MASK, (_mode))
> +
...
> +u32 inv_icm42607_odr_to_period(enum inv_icm42607_odr odr)
> +{
> + static u32 odr_periods[INV_ICM42607_ODR_NB] = {
> + /* Reserved values */
> + 0, 0, 0, 0, 0,
> + /* 1600Hz */
> + 625000,
> + /* 800Hz */
> + 1250000,
> + /* 400Hz */
> + 2500000,
> + /* 200Hz */
> + 5000000,
> + /* 100 Hz */
> + 10000000,
> + /* 50Hz */
> + 20000000,
> + /* 25Hz */
> + 40000000,
> + /* 12.5Hz */
> + 80000000,
> + /* 6.25Hz */
> + 160000000,
> + /* 3.125Hz */
> + 320000000,
> + /* 1.5625Hz */
> + 640000000,
> + };
No range checking to avoid out-of-bouds access?
> +
> + return odr_periods[odr];
> +}
> +
> +int inv_icm42607_debugfs_reg(struct iio_dev *indio_dev, unsigned int reg,
> + unsigned int writeval, unsigned int *readval)
> +{
> + struct inv_icm42607_state *st = iio_device_get_drvdata(indio_dev);
> +
> + guard(mutex)(&st->lock);
> +
> + if (readval)
> + return regmap_read(st->map, reg, readval);
> +
> + return regmap_write(st->map, reg, writeval);
> +}
> +
> +static int inv_icm42607_set_conf(struct inv_icm42607_state *st,
> + const struct inv_icm42607_conf *conf)
> +{
> + unsigned int val;
> + int ret;
> +
> + val = INV_ICM42607_PWR_MGMT0_GYRO(conf->gyro.mode) |
> + INV_ICM42607_PWR_MGMT0_ACCEL(conf->accel.mode);
Indent wrapped lines one tab or line up with INV_ after `= `.
> + /*
> + * No temperature enable reg in datasheet, but BSP driver
> + * selected RC oscillator clock in LP mode when temperature
> + * was disabled.
> + */
> + if (!conf->temp_en)
> + val |= INV_ICM42607_PWR_MGMT0_ACCEL_LP_CLK_SEL;
> + ret = regmap_write(st->map, INV_ICM42607_REG_PWR_MGMT0, val);
> + if (ret)
> + return ret;
> +
> + val = INV_ICM42607_GYRO_CONFIG0_FS_SEL(conf->gyro.fs) |
> + INV_ICM42607_GYRO_CONFIG0_ODR(conf->gyro.odr);
> + ret = regmap_write(st->map, INV_ICM42607_REG_GYRO_CONFIG0, val);
> + if (ret)
> + return ret;
> +
> + val = INV_ICM42607_ACCEL_CONFIG0_FS_SEL(conf->accel.fs) |
> + INV_ICM42607_ACCEL_CONFIG0_ODR(conf->accel.odr);
> + ret = regmap_write(st->map, INV_ICM42607_REG_ACCEL_CONFIG0, val);
> + if (ret)
> + return ret;
> +
> + val = INV_ICM42607_GYRO_CONFIG1_FILTER(conf->gyro.filter);
> + ret = regmap_write(st->map, INV_ICM42607_REG_GYRO_CONFIG1, val);
> + if (ret)
> + return ret;
> +
> + val = INV_ICM42607_ACCEL_CONFIG1_FILTER(conf->accel.filter);
> + ret = regmap_write(st->map, INV_ICM42607_REG_ACCEL_CONFIG1, val);
> + if (ret)
> + return ret;
> +
> + st->conf = *conf;
> +
> + return 0;
> +}
> +
> +/**
> + * inv_icm42607_setup() - check and setup chip
> + * @st: driver internal state
> + * @bus_setup: callback for setting up bus specific registers
> + *
> + * Returns 0 on success, a negative error code otherwise.
> + */
> +static int inv_icm42607_setup(struct inv_icm42607_state *st,
> + inv_icm42607_bus_setup bus_setup)
> +{
> + const struct inv_icm42607_hw *hw = &inv_icm42607_hw[st->chip];
> + const struct device *dev = regmap_get_device(st->map);
> + unsigned int val;
> + int ret;
> +
> + ret = regmap_read(st->map, INV_ICM42607_REG_WHOAMI, &val);
> + if (ret)
> + return ret;
> +
> + if (val != hw->whoami)
> + dev_warn_probe(dev, -ENODEV,
> + "invalid whoami %#02x expected %#02x (%s)\n",
> + val, hw->whoami, hw->name);
> +
> + st->name = hw->name;
> +
> + ret = regmap_write(st->map, INV_ICM42607_REG_SIGNAL_PATH_RESET,
> + INV_ICM42607_SIGNAL_PATH_RESET_SOFT_RESET);
> + if (ret)
> + return ret;
nit: Add blank line here.
> + msleep(INV_ICM42607_RESET_TIME_MS);
> +
> + ret = regmap_read(st->map, INV_ICM42607_REG_INT_STATUS, &val);
> + if (ret)
> + return ret;
> + if (!(val & INV_ICM42607_INT_STATUS_RESET_DONE))
> + return dev_err_probe(dev, -ENODEV,
> + "reset error, reset done bit not set\n");
Could also replace this and msleep with regmap_read_poll_timeout().
> +
> + ret = bus_setup(st);
> + if (ret)
> + return ret;
> +
> + ret = regmap_update_bits(st->map, INV_ICM42607_REG_INTF_CONFIG0,
> + INV_ICM42607_INTF_CONFIG0_SENSOR_DATA_ENDIAN,
> + INV_ICM42607_INTF_CONFIG0_SENSOR_DATA_ENDIAN);
Simplify with regmap_set_bits().
> + if (ret)
> + return ret;
> +
> + ret = regmap_update_bits(st->map, INV_ICM42607_REG_INTF_CONFIG1,
> + INV_ICM42607_INTF_CONFIG1_CLKSEL_MASK,
> + INV_ICM42607_INTF_CONFIG1_CLKSEL_PLL);
> + if (ret)
> + return ret;
> +
> + return inv_icm42607_set_conf(st, hw->conf);
> +}
> +
> +static int inv_icm42607_enable_vddio_reg(struct inv_icm42607_state *st)
> +{
> + int ret;
> +
> + ret = regulator_enable(st->vddio_supply);
> + if (ret)
> + return ret;
> +
> + usleep_range(3000, 4000);
Use fsleep() and add a comment to explain why the duration
was chosen.
> +
> + return 0;
> +}
> +
> +static void inv_icm42607_disable_vddio_reg(void *_data)
> +{
> + struct inv_icm42607_state *st = _data;
> +
> + regulator_disable(st->vddio_supply);
> +}
> +
> +int inv_icm42607_core_probe(struct regmap *regmap, int chip,
> + inv_icm42607_bus_setup bus_setup)
> +{
> + struct device *dev = regmap_get_device(regmap);
> + struct fwnode_handle *fwnode = dev_fwnode(dev);
> + struct inv_icm42607_state *st;
> + int irq, irq_type;
> + bool open_drain;
> + int ret;
> +
> + if (chip < INV_CHIP_INVALID || chip >= INV_CHIP_NB)
Only two chips are defined in regmap_read_poll_timeout, so this range
checking seems wrong.
> + dev_warn_probe(dev, -ENODEV,
> + "Invalid chip = %d\n", chip);
> +
> + /* get INT1 only supported interrupt or fallback to first interrupt */
> + irq = fwnode_irq_get_byname(fwnode, "INT1");
> + if (irq < 0 && irq != -EPROBE_DEFER) {
> + dev_info(dev, "no INT1 interrupt defined, fallback to first interrupt\n");
> + irq = fwnode_irq_get(fwnode, 0);
> + }
> + if (irq < 0)
> + return dev_err_probe(dev, irq, "error missing INT1 interrupt\n");
> +
> + irq_type = irq_get_trigger_type(irq);
> + if (!irq_type)
> + irq_type = IRQF_TRIGGER_FALLING;
> +
> + open_drain = device_property_read_bool(dev, "drive-open-drain");
> +
> + st = devm_kzalloc(dev, sizeof(*st), GFP_KERNEL);
> + if (!st)
> + return -ENOMEM;
> +
> + dev_set_drvdata(dev, st);
> + mutex_init(&st->lock);
> + st->chip = chip;
> + st->map = regmap;
> + st->irq = irq;
> +
> + ret = iio_read_mount_matrix(dev, &st->orientation);
> + if (ret) {
> + dev_err(dev, "failed to retrieve mounting matrix %d\n", ret);
> + return ret;
> + }
> +
> + ret = devm_regulator_get_enable(dev, "vdd");
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "Failed to get vdd regulator\n");
> +
> + msleep(INV_ICM42607_POWER_UP_TIME_MS);
> +
> + st->vddio_supply = devm_regulator_get(dev, "vddio");
If we aren't implementing power managament, we can just use
devm_regulator_get_enabled() and avoid the devm_add_action_or_reset().
> + if (IS_ERR(st->vddio_supply))
> + return PTR_ERR(st->vddio_supply);
> +
> + ret = inv_icm42607_enable_vddio_reg(st);
> + if (ret)
> + return ret;
> +
> + ret = devm_add_action_or_reset(dev, inv_icm42607_disable_vddio_reg, st);
> + if (ret)
> + return ret;
> +
> + /* Setup chip registers (includes WHOAMI check, reset check, bus setup) */
This comment would be better as part of the function doc comment.
> + ret = inv_icm42607_setup(st, bus_setup);
> +
> + return ret;
> +}
^ permalink raw reply
* Re: [PATCH v2] dt-binding: leds: publish common bindings under dual license
From: Gergo Koteles @ 2026-04-10 21:35 UTC (permalink / raw)
To: Corvin Köhne, linux-kernel
Cc: open list:LED SUBSYSTEM, Pavel Machek,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Corvin Köhne, Ashley Towns, Dan Murphy, INAGAKI Hiroshi,
Jacek Anaszewski, Olliver Schinagl, Pavel Machek,
Rafał Miłecki, Roderick Colenbrander,
Krzysztof Kozlowski
In-Reply-To: <20260408062942.7128-1-corvin.koehne@gmail.com>
On Wed, 2026-04-08 at 08:29 +0200, Corvin Köhne wrote:
> From: Corvin Köhne <c.koehne@beckhoff.com>
>
> Changes leds/common.h DT binding header file to be published under GPLv2
> or BSD-2-Clause license terms. This change allows this common LED
> bindings header file to be used in software components as bootloaders
> and OSes that are not published under GPLv2 terms.
>
> All contributors to leds/common.h file in copy.
>
> Cc: Ashley Towns <mail@ashleytowns.id.au>
> Cc: Dan Murphy <dmurphy@ti.com>
> Cc: Gergo Koteles <soyer@irl.hu>
> Cc: INAGAKI Hiroshi <musashino.open@gmail.com>
> Cc: Jacek Anaszewski <jacek.anaszewski@gmail.com>
> Cc: Olliver Schinagl <oliver@schinagl.nl>
> Cc: Pavel Machek <pavel@ucw.cz>
> Cc: Rafał Miłecki <rafal@milecki.pl>
> Cc: Roderick Colenbrander <roderick@gaikai.com>
> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
> Signed-off-by: Corvin Köhne <c.koehne@beckhoff.com>
> ---
> include/dt-bindings/leds/common.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/dt-bindings/leds/common.h b/include/dt-bindings/leds/common.h
> index 4f017bea0123..b7bafbaf7df3 100644
> --- a/include/dt-bindings/leds/common.h
> +++ b/include/dt-bindings/leds/common.h
> @@ -1,4 +1,4 @@
> -/* SPDX-License-Identifier: GPL-2.0 */
> +/* SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) */
> /*
> * This header provides macros for the common LEDs device tree bindings.
> *
I don't know if a single line change counts, but it's fine with me.
Acked-by: Gergo Koteles <soyer@irl.hu>
^ permalink raw reply
* Re: [PATCH v7 6/6] docs: iio: adc: ad4691: add driver documentation
From: David Lechner @ 2026-04-10 21:38 UTC (permalink / raw)
To: radu.sabau, Lars-Peter Clausen, Michael Hennerich,
Jonathan Cameron, Nuno Sá, Andy Shevchenko, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Uwe Kleine-König,
Liam Girdwood, Mark Brown, Linus Walleij, Bartosz Golaszewski,
Philipp Zabel, Jonathan Corbet, Shuah Khan
Cc: linux-iio, devicetree, linux-kernel, linux-pwm, linux-gpio,
linux-doc
In-Reply-To: <20260409-ad4692-multichannel-sar-adc-driver-v7-6-be375d4df2c5@analog.com>
On 4/9/26 10:28 AM, Radu Sabau via B4 Relay wrote:
> From: Radu Sabau <radu.sabau@analog.com>
>
> Add RST documentation for the AD4691 family ADC driver covering
> supported devices, IIO channels, operating modes, oversampling,
> reference voltage, LDO supply, reset, GP pins, SPI offload support,
> and buffer data format.
>
> Signed-off-by: Radu Sabau <radu.sabau@analog.com>
> ---
> Documentation/iio/ad4691.rst | 283 +++++++++++++++++++++++++++++++++++++++++++
> Documentation/iio/index.rst | 1 +
> MAINTAINERS | 1 +
> 3 files changed, 285 insertions(+)
>
> diff --git a/Documentation/iio/ad4691.rst b/Documentation/iio/ad4691.rst
> new file mode 100644
> index 000000000000..a1012c8b78a3
> --- /dev/null
> +++ b/Documentation/iio/ad4691.rst
> @@ -0,0 +1,283 @@
> +.. SPDX-License-Identifier: GPL-2.0-only
> +
> +=============
> +AD4691 driver
> +=============
One overall comment. This goes into driver implementation details quite a bit.
I think that is really better done as comments in the driver itself. And this
document should just focus on how to use the driver from the userspace point
of view.
> +Buffer data format
> +==================
> +
> +The IIO buffer data format (``in_voltageN_type``) is the same across all
> +paths: 16-bit unsigned big-endian samples with no shift.
> +
> ++-------------------------+-------------+----------+-------+
> +| Path | storagebits | realbits | shift |
> ++=========================+=============+==========+=======+
> +| Triggered buffer | 16 | 16 | 0 |
> ++-------------------------+-------------+----------+-------+
> +| CNV Burst offload (DMA) | 16 | 16 | 0 |
> ++-------------------------+-------------+----------+-------+
> +| Manual offload (DMA) | 16 | 16 | 0 |
> ++-------------------------+-------------+----------+-------+
Not sure this table is helpful since all values are the same everywhere.
Also, doesn't SPI offload have storagebits == 32?
> +
> +In the triggered-buffer path the SPI rx_buf for each transfer points directly
> +into the scan buffer, so the 16-bit big-endian result is written in place with
> +no additional copying.
> +
^ permalink raw reply
* Re: [PATCH v4 2/2] hwmon:(pmbus/xdp720) Add support for efuse xdp720
From: Guenter Roeck @ 2026-04-10 21:20 UTC (permalink / raw)
To: ASHISH YADAV
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-hwmon,
devicetree, linux-kernel, Ashish Yadav
In-Reply-To: <20260410070154.3313-3-Ashish.Yadav@infineon.com>
On Fri, Apr 10, 2026 at 12:31:54PM +0530, ASHISH YADAV wrote:
> From: Ashish Yadav <ashish.yadav@infineon.com>
>
> Add the pmbus driver for Infineon XDP720 Digital eFuse Controller.
>
> Signed-off-by: Ashish Yadav <ashish.yadav@infineon.com>
Applied.
Thanks,
Guenter
> ---
> XDP720 Digital eFuse Controller provides accurate system telemetry
> (V, I, P, T) and reports analog current at the IMON pin for post-processing.
>
> The Current and Power measurement depends on the RIMON and GIMON values.
> Please look into data sheet sections 5.4.2 and 5.4.4 for more details:
> https://www.infineon.com/assets/row/public/documents/24/49/infineon-xdp720-001-datasheet-en.pdf
>
> The GIMON (microA/A) depends on the 10th bit of TELEMETRY_AVG PMBUS Register.
> The value of RIMON (kohm) can be provided by the user through device tree using
> infineon,rimon-micro-ohms property.
> ---
> drivers/hwmon/pmbus/Kconfig | 9 +++
> drivers/hwmon/pmbus/Makefile | 1 +
> drivers/hwmon/pmbus/xdp720.c | 128 +++++++++++++++++++++++++++++++++++
> 3 files changed, 138 insertions(+)
> create mode 100644 drivers/hwmon/pmbus/xdp720.c
>
> diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
> index fc1273abe357..c419e3ecce90 100644
> --- a/drivers/hwmon/pmbus/Kconfig
> +++ b/drivers/hwmon/pmbus/Kconfig
> @@ -702,6 +702,15 @@ config SENSORS_XDP710
> This driver can also be built as a module. If so, the module will
> be called xdp710.
>
> +config SENSORS_XDP720
> + tristate "Infineon XDP720 family"
> + help
> + If you say yes here you get hardware monitoring support for Infineon
> + XDP720.
> +
> + This driver can also be built as a module. If so, the module will
> + be called xdp720.
> +
> config SENSORS_XDPE152
> tristate "Infineon XDPE152 family"
> help
> diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile
> index d6c86924f887..1cac7ccae79f 100644
> --- a/drivers/hwmon/pmbus/Makefile
> +++ b/drivers/hwmon/pmbus/Makefile
> @@ -68,6 +68,7 @@ obj-$(CONFIG_SENSORS_TPS546D24) += tps546d24.o
> obj-$(CONFIG_SENSORS_UCD9000) += ucd9000.o
> obj-$(CONFIG_SENSORS_UCD9200) += ucd9200.o
> obj-$(CONFIG_SENSORS_XDP710) += xdp710.o
> +obj-$(CONFIG_SENSORS_XDP720) += xdp720.o
> obj-$(CONFIG_SENSORS_XDPE122) += xdpe12284.o
> obj-$(CONFIG_SENSORS_XDPE152) += xdpe152c4.o
> obj-$(CONFIG_SENSORS_ZL6100) += zl6100.o
> diff --git a/drivers/hwmon/pmbus/xdp720.c b/drivers/hwmon/pmbus/xdp720.c
> new file mode 100644
> index 000000000000..8729a771f216
> --- /dev/null
> +++ b/drivers/hwmon/pmbus/xdp720.c
> @@ -0,0 +1,128 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Hardware monitoring driver for Infineon XDP720 Digital eFuse Controller
> + *
> + * Copyright (c) 2026 Infineon Technologies. All rights reserved.
> + */
> +
> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/of_device.h>
> +#include <linux/bitops.h>
> +#include <linux/math64.h>
> +#include "pmbus.h"
> +
> +/*
> + * The IMON resistor required to generate the system overcurrent protection.
> + * Arbitrary default Rimon value: 2k Ohm
> + */
> +#define XDP720_DEFAULT_RIMON 2000000000 /* 2k ohm */
> +#define XDP720_TELEMETRY_AVG 0xE9
> +
> +static struct pmbus_driver_info xdp720_info = {
> + .pages = 1,
> + .format[PSC_VOLTAGE_IN] = direct,
> + .format[PSC_VOLTAGE_OUT] = direct,
> + .format[PSC_CURRENT_OUT] = direct,
> + .format[PSC_POWER] = direct,
> + .format[PSC_TEMPERATURE] = direct,
> +
> + .m[PSC_VOLTAGE_IN] = 4653,
> + .b[PSC_VOLTAGE_IN] = 0,
> + .R[PSC_VOLTAGE_IN] = -2,
> + .m[PSC_VOLTAGE_OUT] = 4653,
> + .b[PSC_VOLTAGE_OUT] = 0,
> + .R[PSC_VOLTAGE_OUT] = -2,
> + /*
> + * Current and Power measurement depends on the RIMON (kOhm) and
> + * GIMON(microA/A) values.
> + */
> + .m[PSC_CURRENT_OUT] = 24668,
> + .b[PSC_CURRENT_OUT] = 0,
> + .R[PSC_CURRENT_OUT] = -4,
> + .m[PSC_POWER] = 4486,
> + .b[PSC_POWER] = 0,
> + .R[PSC_POWER] = -1,
> + .m[PSC_TEMPERATURE] = 54,
> + .b[PSC_TEMPERATURE] = 22521,
> + .R[PSC_TEMPERATURE] = -1,
> +
> + .func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_VOUT | PMBUS_HAVE_PIN |
> + PMBUS_HAVE_TEMP | PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_INPUT |
> + PMBUS_HAVE_STATUS_TEMP,
> +};
> +
> +static int xdp720_probe(struct i2c_client *client)
> +{
> + struct pmbus_driver_info *info;
> + int ret;
> + u32 rimon;
> + int gimon;
> +
> + info = devm_kmemdup(&client->dev, &xdp720_info, sizeof(*info),
> + GFP_KERNEL);
> + if (!info)
> + return -ENOMEM;
> +
> + ret = devm_regulator_get_enable(&client->dev, "vdd-vin");
> + if (ret)
> + return dev_err_probe(&client->dev, ret,
> + "failed to enable vdd-vin supply\n");
> +
> + ret = i2c_smbus_read_word_data(client, XDP720_TELEMETRY_AVG);
> + if (ret < 0) {
> + dev_err(&client->dev, "Can't get TELEMETRY_AVG\n");
> + return ret;
> + }
> +
> + ret >>= 10; /* 10th bit of TELEMETRY_AVG REG for GIMON Value */
> + ret &= GENMASK(0, 0);
> + if (ret == 1)
> + gimon = 18200; /* output gain 18.2 microA/A */
> + else
> + gimon = 9100; /* output gain 9.1 microA/A */
> +
> + if (of_property_read_u32(client->dev.of_node,
> + "infineon,rimon-micro-ohms", &rimon))
> + rimon = XDP720_DEFAULT_RIMON; /* Default if not set via DT */
> + if (rimon == 0)
> + return -EINVAL;
> +
> + /* Adapt the current and power scale for each instance */
> + info->m[PSC_CURRENT_OUT] = DIV64_U64_ROUND_CLOSEST((u64)
> + info->m[PSC_CURRENT_OUT] * rimon * gimon, 1000000000000ULL);
> + info->m[PSC_POWER] = DIV64_U64_ROUND_CLOSEST((u64)
> + info->m[PSC_POWER] * rimon * gimon, 1000000000000000ULL);
> +
> + return pmbus_do_probe(client, info);
> +}
> +
> +static const struct of_device_id xdp720_of_match[] = {
> + { .compatible = "infineon,xdp720" },
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, xdp720_of_match);
> +
> +static const struct i2c_device_id xdp720_id[] = {
> + { "xdp720" },
> + {}
> +};
> +MODULE_DEVICE_TABLE(i2c, xdp720_id);
> +
> +static struct i2c_driver xdp720_driver = {
> + .driver = {
> + .name = "xdp720",
> + .of_match_table = xdp720_of_match,
> + },
> + .probe = xdp720_probe,
> + .id_table = xdp720_id,
> +};
> +
> +module_i2c_driver(xdp720_driver);
> +
> +MODULE_AUTHOR("Ashish Yadav <ashish.yadav@infineon.com>");
> +MODULE_DESCRIPTION("PMBus driver for Infineon XDP720 Digital eFuse Controller");
> +MODULE_LICENSE("GPL");
> +MODULE_IMPORT_NS("PMBUS");
^ permalink raw reply
* Re: [PATCH v4 1/2] dt-bindings: hwmon/pmbus: Add Infineon XDP720
From: Guenter Roeck @ 2026-04-10 21:19 UTC (permalink / raw)
To: ASHISH YADAV
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-hwmon,
devicetree, linux-kernel, Ashish Yadav
In-Reply-To: <20260410070154.3313-2-Ashish.Yadav@infineon.com>
On Fri, Apr 10, 2026 at 12:31:53PM +0530, ASHISH YADAV wrote:
> From: Ashish Yadav <ashish.yadav@infineon.com>
>
> Add documentation for the device tree binding of the XDP720 eFuse.
>
> Signed-off-by: Ashish Yadav <ashish.yadav@infineon.com>
> Acked-by: Conor Dooley <conor.dooley@microchip.com>
Applied.
Thanks,
Guenter
^ permalink raw reply
* Re: [PATCH v7 5/6] iio: adc: ad4691: add oversampling support
From: David Lechner @ 2026-04-10 21:15 UTC (permalink / raw)
To: radu.sabau, Lars-Peter Clausen, Michael Hennerich,
Jonathan Cameron, Nuno Sá, Andy Shevchenko, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Uwe Kleine-König,
Liam Girdwood, Mark Brown, Linus Walleij, Bartosz Golaszewski,
Philipp Zabel, Jonathan Corbet, Shuah Khan
Cc: linux-iio, devicetree, linux-kernel, linux-pwm, linux-gpio,
linux-doc
In-Reply-To: <20260409-ad4692-multichannel-sar-adc-driver-v7-5-be375d4df2c5@analog.com>
On 4/9/26 10:28 AM, Radu Sabau via B4 Relay wrote:
> From: Radu Sabau <radu.sabau@analog.com>
>
> Add per-channel oversampling ratio (OSR) support for CNV burst mode.
> The accumulator depth register (ACC_DEPTH_IN) is programmed with the
> selected OSR at buffer enable time and before each single-shot read.
>
> Supported OSR values: 1, 2, 4, 8, 16, 32.
>
> Introduce AD4691_MANUAL_CHANNEL() for manual mode channels, which do
> not expose the oversampling ratio attribute since OSR is not applicable
> in that mode. A separate manual_channels array is added to
> struct ad4691_channel_info and selected at probe time; offload paths
> reuse the same arrays with num_channels capping access before the soft
> timestamp entry.
>
> The reported sampling frequency accounts for the active OSR:
> effective_freq = oscillator_freq / osr
Technically, the way this is implemented is fine according to IIO ABI
rules. Writing any attribute can cause others to change. It does
introduce a potential pitfall though. Currently, changing the OSR will
change the sampling frequency, so you have to always write oversampling_ratio
first, then write sampling_frequency to get what you asked for. If you want
to change the OSR and keep the same sample rate, you still have to write both
attributes again.
In other drivers, I've implemented it so that the requested sampling frequency
is stored any you always get the closest sampling frequency available based on
the oversampling ratio. This way, it doesn't matter which order you write
the attributes. In that case, the actual periodic trigger source isn't set up
until we actually start sampling.
>
> OSR defaults to 1 (no accumulation) for all channels.
>
...
> @@ -499,7 +570,7 @@ static int ad4691_get_sampling_freq(struct ad4691_state *st, int *val)
> if (ret)
> return ret;
>
> - *val = ad4691_osc_freqs_Hz[FIELD_GET(AD4691_OSC_FREQ_MASK, reg_val)];
> + *val = ad4691_osc_freqs_Hz[FIELD_GET(AD4691_OSC_FREQ_MASK, reg_val)] / osr;
I guess we don't have to worry about fractional values here?
> return IIO_VAL_INT;
> }
>
> @@ -536,6 +607,11 @@ static int ad4691_read_avail(struct iio_dev *indio_dev,
> *type = IIO_VAL_INT;
> *length = ARRAY_SIZE(ad4691_osc_freqs_Hz) - start;
> return IIO_AVAIL_LIST;
> + case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> + *vals = ad4691_oversampling_ratios;
> + *type = IIO_VAL_INT;
> + *length = ARRAY_SIZE(ad4691_oversampling_ratios);
> + return IIO_AVAIL_LIST;
> default:
> return -EINVAL;
> }
> @@ -566,6 +642,11 @@ static int ad4691_single_shot_read(struct iio_dev *indio_dev,
> if (ret)
> return ret;
>
> + ret = regmap_write(st->regmap, AD4691_ACC_DEPTH_IN(chan->channel),
> + st->osr[chan->channel]);
> + if (ret)
> + return ret;
> +
> ret = regmap_read(st->regmap, AD4691_OSC_FREQ_REG, ®_val);
> if (ret)
> return ret;
> @@ -575,8 +656,9 @@ static int ad4691_single_shot_read(struct iio_dev *indio_dev,
> return ret;
>
> osc_idx = FIELD_GET(AD4691_OSC_FREQ_MASK, reg_val);
> - /* Wait 2 oscillator periods for the conversion to complete. */
> - period_us = DIV_ROUND_UP(2UL * USEC_PER_SEC, ad4691_osc_freqs_Hz[osc_idx]);
> + /* Wait osr oscillator periods for all accumulator samples to complete. */
Why did we need to way 2 before and only 1 now when OSR == 1?
> + period_us = DIV_ROUND_UP((unsigned long)st->osr[chan->channel] * USEC_PER_SEC,
> + ad4691_osc_freqs_Hz[osc_idx]);
^ permalink raw reply
* Re: [PATCH v7 4/6] iio: adc: ad4691: add SPI offload support
From: David Lechner @ 2026-04-10 21:00 UTC (permalink / raw)
To: radu.sabau, Lars-Peter Clausen, Michael Hennerich,
Jonathan Cameron, Nuno Sá, Andy Shevchenko, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Uwe Kleine-König,
Liam Girdwood, Mark Brown, Linus Walleij, Bartosz Golaszewski,
Philipp Zabel, Jonathan Corbet, Shuah Khan
Cc: linux-iio, devicetree, linux-kernel, linux-pwm, linux-gpio,
linux-doc
In-Reply-To: <20260409-ad4692-multichannel-sar-adc-driver-v7-4-be375d4df2c5@analog.com>
On 4/9/26 10:28 AM, Radu Sabau via B4 Relay wrote:
> From: Radu Sabau <radu.sabau@analog.com>
>
> Add SPI offload support to enable DMA-based, CPU-independent data
> acquisition using the SPI Engine offload framework.
>
> When an SPI offload is available (devm_spi_offload_get() succeeds),
> the driver registers a DMA engine IIO buffer and uses dedicated buffer
> setup operations. If no offload is available the existing software
> triggered buffer path is used unchanged.
>
> Both CNV Burst Mode and Manual Mode support offload, but use different
> trigger mechanisms:
>
> CNV Burst Mode: the SPI Engine is triggered by the ADC's DATA_READY
> signal on the GP pin specified by the trigger-source consumer reference
> in the device tree (one cell = GP pin number 0-3). For this mode the
> driver acts as both an SPI offload consumer (DMA RX stream, message
> optimization) and a trigger source provider: it registers the
> GP/DATA_READY output via devm_spi_offload_trigger_register() so the
> offload framework can match the '#trigger-source-cells' phandle and
> automatically fire the SPI Engine DMA transfer at end-of-conversion.
>
> Manual Mode: the SPI Engine is triggered by a periodic trigger at
> the configured sampling frequency. The pre-built SPI message uses
> the pipelined CNV-on-CS protocol: N+1 16-bit transfers are issued
> for N active channels (the first result is discarded as garbage from
> the pipeline flush) and the remaining N results are captured by DMA.
>
> All offload transfers use 16-bit frames (bits_per_word=16, len=2).
> The channel scan_type (storagebits=16, shift=0, IIO_BE) is shared
> between the software triggered-buffer and offload paths; no separate
> scan_type or channel array is needed for the offload case. The
> ad4691_manual_channels[] array introduced in the triggered-buffer
> commit is reused here: it hides the IIO_CHAN_INFO_OVERSAMPLING_RATIO
> attribute, which is not applicable in Manual Mode.
>
> Kconfig gains a dependency on IIO_BUFFER_DMAENGINE.
>
> Signed-off-by: Radu Sabau <radu.sabau@analog.com>
> ---
> drivers/iio/adc/Kconfig | 2 +
> drivers/iio/adc/ad4691.c | 398 ++++++++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 395 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index d498f16c0816..fdc6565933c5 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -143,8 +143,10 @@ config AD4691
> tristate "Analog Devices AD4691 Family ADC Driver"
> depends on SPI
> select IIO_BUFFER
> + select IIO_BUFFER_DMAENGINE
> select IIO_TRIGGERED_BUFFER
> select REGMAP
> + select SPI_OFFLOAD
> help
> Say yes here to build support for Analog Devices AD4691 Family MuxSAR
> SPI analog to digital converters (ADC).
> diff --git a/drivers/iio/adc/ad4691.c b/drivers/iio/adc/ad4691.c
> index 3e5caa0972eb..839ea7f44c78 100644
> --- a/drivers/iio/adc/ad4691.c
> +++ b/drivers/iio/adc/ad4691.c
> @@ -22,6 +22,8 @@
> #include <linux/regulator/consumer.h>
> #include <linux/reset.h>
> #include <linux/spi/spi.h>
> +#include <linux/spi/offload/consumer.h>
> +#include <linux/spi/offload/provider.h>
> #include <linux/units.h>
> #include <linux/unaligned.h>
>
> @@ -43,6 +45,11 @@
>
> #define AD4691_CNV_DUTY_CYCLE_NS 380
> #define AD4691_CNV_HIGH_TIME_NS 430
> +/*
> + * Conservative default for the manual offload periodic trigger. Low enough
> + * to work safely out of the box across all OSR and channel count combinations.
> + */
> +#define AD4691_OFFLOAD_INITIAL_TRIGGER_HZ (100 * HZ_PER_KHZ)
>
> #define AD4691_SPI_CONFIG_A_REG 0x000
> #define AD4691_SW_RESET (BIT(7) | BIT(0))
> @@ -95,6 +102,8 @@
> #define AD4691_ACC_IN(n) (0x252 + (3 * (n)))
> #define AD4691_ACC_STS_DATA(n) (0x283 + (4 * (n)))
>
> +#define AD4691_OFFLOAD_BITS_PER_WORD 16
This is just the same as realbits in scan info. So could use that
directly instead.
> +
> static const char * const ad4691_supplies[] = { "avdd", "vio" };
>
> enum ad4691_ref_ctrl {
> @@ -114,6 +123,7 @@ struct ad4691_chip_info {
> const char *name;
> unsigned int max_rate;
> const struct ad4691_channel_info *sw_info;
> + const struct ad4691_channel_info *offload_info;
> };
>
> #define AD4691_CHANNEL(ch) \
> @@ -177,6 +187,18 @@ static const struct ad4691_channel_info ad4693_sw_info = {
> .num_channels = ARRAY_SIZE(ad4693_channels),
> };
>
> +static const struct ad4691_channel_info ad4691_offload_info = {
> + .channels = ad4691_channels,
> + /* No soft timestamp; num_channels caps access to 16. */
> + .num_channels = 16,
`ARRAY_SIZE(ad4691_channels) - 1` would make sense too.
> +};
> +
> +static const struct ad4691_channel_info ad4693_offload_info = {
> + .channels = ad4693_channels,
> + /* No soft timestamp; num_channels caps access to 8. */
> + .num_channels = 8,
> +};
> +
> /*
> * Internal oscillator frequency table. Index is the OSC_FREQ_REG[3:0] value.
> * Index 0 (1 MHz) is only valid for AD4692/AD4694; AD4691/AD4693 support
> @@ -207,24 +229,36 @@ static const struct ad4691_chip_info ad4691_chip_info = {
> .name = "ad4691",
> .max_rate = 500 * HZ_PER_KHZ,
> .sw_info = &ad4691_sw_info,
> + .offload_info = &ad4691_offload_info,
> };
>
> static const struct ad4691_chip_info ad4692_chip_info = {
> .name = "ad4692",
> .max_rate = 1 * HZ_PER_MHZ,
> .sw_info = &ad4691_sw_info,
> + .offload_info = &ad4691_offload_info,
> };
>
> static const struct ad4691_chip_info ad4693_chip_info = {
> .name = "ad4693",
> .max_rate = 500 * HZ_PER_KHZ,
> .sw_info = &ad4693_sw_info,
> + .offload_info = &ad4693_offload_info,
> };
>
> static const struct ad4691_chip_info ad4694_chip_info = {
> .name = "ad4694",
> .max_rate = 1 * HZ_PER_MHZ,
> .sw_info = &ad4693_sw_info,
> + .offload_info = &ad4693_offload_info,
> +};
> +
> +struct ad4691_offload_state {
> + struct spi_offload *spi;
I would call this "offload" or "instance". "spi" is usally the SPI
device handle.
> + struct spi_offload_trigger *trigger;
> + u64 trigger_hz;
> + u8 tx_cmd[17][2];
> + u8 tx_reset[4];
> };
>
...
> +
> +static int ad4691_cnv_burst_offload_buffer_predisable(struct iio_dev *indio_dev)
> +{
> + struct ad4691_state *st = iio_priv(indio_dev);
> + struct ad4691_offload_state *offload = st->offload;
> + int ret;
> +
> + spi_offload_trigger_disable(offload->spi, offload->trigger);
> +
> + ret = ad4691_sampling_enable(st, false);
> + if (ret)
> + return ret;
> +
> + ret = regmap_write(st->regmap, AD4691_STD_SEQ_CONFIG,
> + AD4691_SEQ_ALL_CHANNELS_OFF);
Why this extra step? We don't have it when unwinding in the
error path of the postenable function.
> + if (ret)
> + return ret;
> +
> + spi_unoptimize_message(&st->scan_msg);
> +
> + return ad4691_exit_conversion_mode(st);
> +}
> +
> +static const struct iio_buffer_setup_ops ad4691_cnv_burst_offload_buffer_setup_ops = {
> + .postenable = &ad4691_cnv_burst_offload_buffer_postenable,
> + .predisable = &ad4691_cnv_burst_offload_buffer_predisable,
> +};
> +
> static ssize_t sampling_frequency_show(struct device *dev,
> struct device_attribute *attr,
> char *buf)
^ permalink raw reply
* Re: [PATCH net-next v3 00/12] net: airoha: Support multiple net_devices connected to the same GDM port
From: Jakub Kicinski @ 2026-04-10 20:49 UTC (permalink / raw)
To: Lorenzo Bianconi
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Paolo Abeni,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Christian Marangi,
Benjamin Larsson, linux-arm-kernel, linux-mediatek, netdev,
devicetree, Xuegang Lu
In-Reply-To: <adiop-9Mo4ADfvfw@lore-desk>
On Fri, 10 Apr 2026 09:37:11 +0200 Lorenzo Bianconi wrote:
> > On Mon, 06 Apr 2026 12:34:05 +0200 Lorenzo Bianconi wrote:
> > > EN7581 or AN7583 SoCs support connecting multiple external SerDes (e.g.
> > > Ethernet or USB SerDes) to GDM3 or GDM4 ports via a hw arbiter that
> > > manages the traffic in a TDM manner. As a result multiple net_devices can
> > > connect to the same GDM{3,4} port and there is a theoretical "1:n"
> > > relation between GDM ports and net_devices.
> >
> > Looks like this driver uses page pool.
> > If you're sharing the same page pool across multiple netdevs
> > it must not be linked to a netdev.
>
> are you referring to slow.netdev pointer? If so, this is not set in airoha_eth
> driver.
Yes. Alright, thanks for checking. Pretty sure I saw it set somewhere
in a file called airoha* but must be another component :)
^ permalink raw reply
* Re: [PATCH v7 3/6] iio: adc: ad4691: add triggered buffer support
From: David Lechner @ 2026-04-10 20:46 UTC (permalink / raw)
To: radu.sabau, Lars-Peter Clausen, Michael Hennerich,
Jonathan Cameron, Nuno Sá, Andy Shevchenko, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Uwe Kleine-König,
Liam Girdwood, Mark Brown, Linus Walleij, Bartosz Golaszewski,
Philipp Zabel, Jonathan Corbet, Shuah Khan
Cc: linux-iio, devicetree, linux-kernel, linux-pwm, linux-gpio,
linux-doc
In-Reply-To: <20260409-ad4692-multichannel-sar-adc-driver-v7-3-be375d4df2c5@analog.com>
On 4/9/26 10:28 AM, Radu Sabau via B4 Relay wrote:
> From: Radu Sabau <radu.sabau@analog.com>
>
> Add buffered capture support using the IIO triggered buffer framework.
>
...
> @@ -201,8 +245,45 @@ struct ad4691_state {
> * atomicity of consecutive SPI operations.
> */
> struct mutex lock;
> + /*
> + * Per-buffer-enable lifetime resources:
> + * Manual Mode - a pre-built SPI message that clocks out N+1
> + * transfers in one go.
> + * CNV Burst Mode - a pre-built SPI message that clocks out 2*N
> + * transfers in one go.
> + */
> + struct spi_message scan_msg;
> + /* max 16 + 1 NOOP (manual) or 2*16 + 2 (CNV burst). */
> + struct spi_transfer scan_xfers[34];
> + /*
> + * CNV burst: 16 AVG_IN addresses + state-reset address + state-reset
> + * value = 18. Manual: 16 channel cmds + 1 NOOP = 17.
> + */
> + __be16 scan_tx[18];
Needs __aligned(IIO_DMA_MINALIGN) since it is used with SPI.
> + /* Scan buffer: one BE16 slot per channel (rx'd directly), plus timestamp */
> + struct {
> + __be16 vals[16];
> + aligned_s64 ts;
> + } scan;
Unless it is required that all channels are always enabled:
IIO_DECLARE_BUFFER_WITH_TS(__be16, scan_rx, 16);
In any case, needs to be DMA-safe for SPI.
> };
>
> +static int ad4691_cnv_burst_buffer_preenable(struct iio_dev *indio_dev)
> +{
> + struct ad4691_state *st = iio_priv(indio_dev);
> + unsigned int n_active;
> + unsigned int k, i;
> + int ret;
> +
> + n_active = bitmap_weight(indio_dev->active_scan_mask, iio_get_masklength(indio_dev));
> +
> + memset(st->scan_xfers, 0, (2 * n_active + 2) * sizeof(st->scan_xfers[0]));
> + memset(st->scan_tx, 0, (n_active + 2) * sizeof(st->scan_tx[0]));
Maybe simpler to just clear the whole thing? (same with other preenable)
> +
> + spi_message_init(&st->scan_msg);
> +
^ permalink raw reply
* Re: [PATCH v7 2/2] iio: dac: ad5706r: Add support for AD5706R DAC
From: David Lechner @ 2026-04-10 20:28 UTC (permalink / raw)
To: Alexis Czezar Torreno, Lars-Peter Clausen, Michael Hennerich,
Jonathan Cameron, Nuno Sá, Andy Shevchenko, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: linux-iio, devicetree, linux-kernel, Andy Shevchenko
In-Reply-To: <20260410-dev_ad5706r-v7-2-af93a4caa186@analog.com>
On 4/10/26 1:48 AM, Alexis Czezar Torreno wrote:
> Add support for the Analog Devices AD5706R, a 4-channel 16-bit
> current output digital-to-analog converter with SPI interface.
>
...
> +static int ad5706r_regmap_write(void *context, const void *data, size_t count)
> +{
> + struct ad5706r_state *st = context;
> + unsigned int num_bytes, val;
> + u16 reg;
> +
> + if (count != 4)
> + return -EINVAL;
> +
> + reg = get_unaligned_be16(data);
> + num_bytes = ad5706r_reg_len(reg);
> +
> + struct spi_transfer xfer = {
> + .tx_buf = st->tx_buf,
> + .len = num_bytes + 2,
> + };
> +
> + val = get_unaligned_be32(data);
> + put_unaligned_be32(val, &st->tx_buf[0]);
Can't we just do memcpy() instead of swapping the byte order twice?
> +
> + /* For single byte, copy the data to the correct position */
> + if (num_bytes == AD5706R_SINGLE_BYTE_LEN)
> + st->tx_buf[2] = st->tx_buf[3];
> +
> + return spi_sync_transfer(st->spi, &xfer, 1);
There isn't any special paramters in the xfer struct, so spi_write() should
work here and save a bit of code.
return spi_write(st->spi, data, num_bytes);
> +}
> +
> +static int ad5706r_regmap_read(void *context, const void *reg_buf,
> + size_t reg_size, void *val_buf, size_t val_size)
> +{
> + struct ad5706r_state *st = context;
> + unsigned int num_bytes;
> + u16 reg, cmd, val;
> + int ret;
> +
> + if (reg_size != 2 || val_size != 2)
> + return -EINVAL;
> +
> + reg = get_unaligned_be16(reg_buf);
> + num_bytes = ad5706r_reg_len(reg);
> +
> + /* Full duplex, device responds immediately after command */
> + struct spi_transfer xfer = {
> + .tx_buf = st->tx_buf,
> + .rx_buf = st->rx_buf,
> + .len = 2 + num_bytes,
> + };
> +
> + cmd = AD5706R_RD_MASK | (reg & AD5706R_ADDR_MASK);
> + put_unaligned_be16(cmd, &st->tx_buf[0]);
> + put_unaligned_be16(0, &st->tx_buf[2]);
Do we actually need to write 0s while reading?
Usually, we would just do a spi_write_then_read for something like
this.
> +
> + ret = spi_sync_transfer(st->spi, &xfer, 1);
> + if (ret)
> + return ret;
> +
> + /* Extract value from response (skip 2-byte command echo) */
> + if (num_bytes == AD5706R_SINGLE_BYTE_LEN)
> + val = st->rx_buf[2];
> + else if (num_bytes == AD5706R_DOUBLE_BYTE_LEN)
> + val = get_unaligned_be16(&st->rx_buf[2]);
> + else
> + return -EINVAL;
> +
> + put_unaligned_be16(val, val_buf);
Can't this all be simplified to memcpy(val_buf, &st->rx_buf[2], num_bytes); ?
Or the whole thing simplified to:
return spi_write_then_read(st->spi, reg_buf, 2, val_buf, num_bytes);
> +
> + return 0;
> +}
> +
^ permalink raw reply
* Re: [PATCH v2 3/4] staging: iio: magnetometer: Add QST QMC5883P driver
From: David Lechner @ 2026-04-10 20:02 UTC (permalink / raw)
To: Hardik Phalet, Greg Kroah-Hartman
Cc: Jonathan Cameron, Nuno Sá, Andy Shevchenko, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Brigham Campbell, Shuah Khan,
linux-iio, devicetree, linux-kernel, linux-staging
In-Reply-To: <20260409210639.3197576-4-hardik.phalet@pm.me>
On 4/9/26 4:07 PM, Hardik Phalet wrote:
> Add an IIO driver for the QST QMC5883P, a 3-axis anisotropic
> magneto-resistive (AMR) magnetometer with a 16-bit ADC, communicating
> over I2C. There is no existing upstream driver for this device.
>
> The driver supports:
> - Raw magnetic field readings on X, Y, and Z axes
> - Four full-scale ranges (+/-2 G, +/-8 G, +/-12 G, +/-30 G)
> - Configurable output data rate (10, 50, 100, 200 Hz)
> - Configurable oversampling ratio (1, 2, 4, 8)
> - Configurable downsampling ratio (1, 2, 4, 8) via a custom sysfs
> attribute
> - Optional vdd-supply regulator management
> - Runtime PM with a 2 s autosuspend delay
> - System suspend/resume with full chip reinitialisation and regmap
> cache resync to handle regulator power-loss
This is a little bit much to review all in one patch. Could be nice
to split out power management to a separate patch.
Oversampling/downsampling could be split to a separate patch or two
as well.
>
> Regmap with an rbtree cache is used throughout. CTRL_1 and CTRL_2
> bit fields are accessed via regmap_field to avoid read-modify-write
> races. The STATUS register is marked precious so regmap never reads
> it speculatively and clears the DRDY/OVFL bits unexpectedly.
>
>
> The probe-time init sequence is: soft reset, wait 1 ms, deassert
> reset, configure SET/RESET control, apply default ODR/OSR/DSR/RNG,
> enter normal mode. This ordering was determined empirically on
> hardware to produce reliable, non-zero axis readings.
>
> Cleanup is fully devm-managed: devm_pm_runtime_enable() handles
> runtime PM teardown, and a devm action registered before
> devm_iio_device_register() puts the chip to sleep on removal,
> ensuring the IIO interface is unregistered before the hardware is
> suspended.
>
> The driver is placed under drivers/staging/iio/magnetometer/ with a
> TODO file tracking the remaining work before it can graduate:
> - Triggered buffer support (iio_triggered_buffer_setup)
> - DRDY interrupt support
> - Self-test implementation
>
> Signed-off-by: Hardik Phalet <hardik.phalet@pm.me>
> ---
> drivers/staging/iio/Kconfig | 1 +
> drivers/staging/iio/Makefile | 1 +
> drivers/staging/iio/magnetometer/Kconfig | 20 +
> drivers/st
> aging/iio/magnetometer/Makefile | 7 +
> drivers/staging/iio/magnetometer/TODO | 5 +
> drivers/staging/iio/magnetometer/qmc5883p.c | 830 ++++++++++++++++++++
> 6 files changed, 864 insertions(+)
> create mode 100644 drivers/staging/iio/magnetometer/Kconfig
> create mode 100644 drivers/staging/iio/magnetometer/Makefile
> create mode 100644 drivers/staging/iio/magnetometer/TODO
> create mode 100644 drivers/staging/iio/magnetometer/qmc5883p.c
>
> diff --git a/drivers/staging/iio/Kconfig b/drivers/staging/iio/Kconfig
> index a60631c1f449..d363e163d248 100644
> --- a/drivers/staging/iio/Kconfig
> +++ b/drivers/staging/iio/Kconfig
> @@ -10,5 +10,6 @@ source "drivers/staging/iio/adc/Kconfig"
> source "drivers/staging/iio/addac/Kconfig"
> source "drivers/staging/iio/frequency/Kconfig"
> source "drivers/staging/iio/impedance-analyzer/Kconfig"
> +source "drivers/staging/iio/magnetometer/Kconfig"
>
> endmenu
> diff --git a/drivers/staging/iio/Makefile b/drivers/staging/iio/Makefile
> index 62
> 8583535393..7dcbb75d43f0 100644
> --- a/drivers/staging/iio/Makefile
> +++ b/drivers/staging/iio/Makefile
> @@ -8,3 +8,4 @@ obj-y += adc/
> obj-y += addac/
> obj-y += frequency/
> obj-y += impedance-analyzer/
> +obj-y += magnetometer/
> diff --git a/drivers/staging/iio/magnetometer/Kconfig b/drivers/staging/iio/magnetometer/Kconfig
> new file mode 100644
> index 000000000000..d631da9578a1
> --- /dev/null
> +++ b/drivers/staging/iio/magnetometer/Kconfig
> @@ -0,0 +1,20 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +#
> +# Magnetometer sensors
> +#
> +# When adding new entries keep the list in alphabetical order
> +
> +menu "Magnetometer sensors"
> +
> +config QMC5883P
> + tristate "QMC5883P 3-Axis Magnetometer"
> + depends on I2C
> + select REGMAP_I2C
> + help
> + Say yes here to build support for QMC5883P I2C-based
> + 3-axis magnetometer chip.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called qmc5883p.
> +
> +endmenu
> diff --git a/drivers/staging/iio/magnetometer/Ma
> kefile b/drivers/staging/iio/magnetometer/Makefile
> new file mode 100644
> index 000000000000..8e650f2e3b02
> --- /dev/null
> +++ b/drivers/staging/iio/magnetometer/Makefile
> @@ -0,0 +1,7 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +#
> +# Makefile for staging industrial I/O Magnetometer sensor devices
> +#
> +# When adding new entries keep the list in alphabetical order
> +
> +obj-$(CONFIG_QMC5883P) += qmc5883p.o
> diff --git a/drivers/staging/iio/magnetometer/TODO b/drivers/staging/iio/magnetometer/TODO
> new file mode 100644
> index 000000000000..6a8084c0dded
> --- /dev/null
> +++ b/drivers/staging/iio/magnetometer/TODO
> @@ -0,0 +1,5 @@
> +TODO
> +====
> +- Implement triggered buffer support (iio_triggered_buffer_setup)
> +- Add interrupt (DRDY) support
> +- Implement self-test (selftest regmap field is unused)
> diff --git a/drivers/staging/iio/magnetometer/qmc5883p.c b/drivers/staging/iio/magnetometer/qmc5883p.c
> new file mode 100644
> index 000000000000..d9758f1e0f4d
> --- /dev/null
> +++ b/drivers/
> staging/iio/magnetometer/qmc5883p.c
> @@ -0,0 +1,830 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * qmc5883p.c - QMC5883P magnetometer driver
> + *
> + * Copyright 2026 Hardik Phalet <hardik.phalet@pm.me>
> + */
> +
> +#include <linux/array_size.h>
> +#include <linux/bits.h>
> +#include <linux/cleanup.h>
> +#include <linux/delay.h>
> +#include <linux/dev_printk.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/i2c.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/iio/types.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/pm.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/unaligned.h>
> +
> +/* Register definition */
> +#define QMC5883P_REG_CHIP_ID 0x00
> +#define QMC5883P_REG_X_LSB 0x01
> +#define QMC5883P_REG_X_MSB 0x02
> +#define QMC5883P_REG_Y_LSB 0x03
> +#define QMC5883P_REG_Y_MSB 0x04
> +#define QMC58
> 83P_REG_Z_LSB 0x05
Were you manually editing the patch?
> +#define QMC5883P_REG_Z_MSB 0x06
> +#define QMC5883P_REG_STATUS 0x09
> +#define QMC5883P_REG_CTRL_1 0x0A
> +#define QMC5883P_REG_CTRL_2 0x0B
> +
> +/* Value definition */
> +#define QMC5883P_MODE_SUSPEND 0x00
> +#define QMC5883P_MODE_NORMAL 0x01
> +#define QMC5883P_MODE_SINGLE 0x02
> +#define QMC5883P_MODE_CONTINUOUS 0x03
> +
> +/* Output data rate */
> +#define QMC5883P_ODR_10 0x00
> +#define QMC5883P_ODR_50 0x01
> +#define QMC5883P_ODR_100 0x02
> +#define QMC5883P_ODR_200 0x03
> +
> +/* Oversampling rate */
> +#define QMC5883P_OSR_8 0x00
> +#define QMC5883P_OSR_4 0x01
> +#define QMC5883P_OSR_2 0x02
> +#define QMC5883P_OSR_1 0x03
> +
> +/* Downsampling rate */
> +#define QMC5883P_DSR_1 0x00
> +#define QMC5883P_DSR_2 0x01
> +#define QMC5883P_DSR_4 0x02
> +#define QMC5883P_DSR_8 0x03
> +
> +#define QMC5883P_RSTCTRL_SET_RESET \
> + 0x00 /* Set and reset on, i.e. the offset of device is renewed */
> +#define QMC5883P_RSTCTRL_SET_ONLY 0x01 /* Set only on */
> +#define QMC5883P_RSTCTRL_OFF 0x02 /* Set
> and reset off */
Or maybe you mail client mangled the patch? These wraps are
happening in many places.
> +
> +#define QMC5883P_RNG_30G 0x00
> +#define QMC5883P_RNG_12G 0x01
> +#define QMC5883P_RNG_08G 0x02
> +#define QMC5883P_RNG_02G 0x03
> +
> +#define QMC5883P_DEFAULT_ODR QMC5883P_ODR_100
> +#define QMC5883P_DEFAULT_OSR QMC5883P_OSR_4
> +#define QMC5883P_DEFAULT_DSR QMC5883P_DSR_4
> +#define QMC5883P_DEFAULT_RNG QMC5883P_RNG_08G
> +
> +#define QMC5883P_DRDY_POLL_US 1000
> +
> +#define QMC5883P_CHIP_ID 0x80
> +
> +#define QMC5883P_STATUS_DRDY BIT(0)
> +#define QMC5883P_STATUS_OVFL BIT(1)
> +
> +/*
> + * Scale factors in T/LSB for IIO_VAL_FRACTIONAL (val/val2), derived from
> + * datasheet Table 2 sensitivities (LSB/G) converted to LSB/T (1 G = 1e-4 T):
> + * sensitivity_T = sensitivity_G * 10000
> + * scale = 1 / sensitivity_T
> + *
> + * Index matches register value: RNG<1:0> = 0b00..0b11
> + */
> +static const int qmc5883p_scale[][2] = {
> + [QMC5883P_RNG_30G] = { 1, 10000000 },
> + [QMC5883P_RNG_12G] = { 1, 25000000 },
> + [QMC5883P_RNG_08G] = { 1, 37500000 },
> + [QMC5883P_RNG_02G] = { 1, 15
> 0000000 },
> +};
> +
> +static const int qmc5883p_odr[] = {
> + [QMC5883P_ODR_10] = 10,
> + [QMC5883P_ODR_50] = 50,
> + [QMC5883P_ODR_100] = 100,
> + [QMC5883P_ODR_200] = 200,
> +};
> +
> +static const int qmc5883p_osr[] = {
> + [QMC5883P_OSR_1] = 1,
> + [QMC5883P_OSR_2] = 2,
> + [QMC5883P_OSR_4] = 4,
> + [QMC5883P_OSR_8] = 8,
> +};
> +
> +static const unsigned int qmc5883p_dsr[] = {
> + [QMC5883P_DSR_1] = 1,
> + [QMC5883P_DSR_2] = 2,
> + [QMC5883P_DSR_4] = 4,
> + [QMC5883P_DSR_8] = 8,
> +};
> +
> +struct qmc5883p_rf {
> + struct regmap_field *osr;
> + struct regmap_field *dsr;
> + struct regmap_field *odr;
> + struct regmap_field *mode;
> + struct regmap_field *rng;
> + struct regmap_field *rstctrl;
> + struct regmap_field *sftrst;
> + struct regmap_field *selftest;
> + struct regmap_field *chip_id;
> +};
> +
> +static const struct regmap_range qmc5883p_readable_ranges[] = {
> + regmap_reg_range(QMC5883P_REG_CHIP_ID, QMC5883P_REG_STATUS),
> + regmap_reg_range(QMC5883P_REG_CTRL_1, QMC5883P_REG_CTRL_2),
> +};
> +
> +static con
> st struct regmap_range qmc5883p_writable_ranges[] = {
> + regmap_reg_range(QMC5883P_REG_CTRL_1, QMC5883P_REG_CTRL_2),
> +};
> +
> +/*
> + * Volatile registers: hardware updates these independently of the driver.
> + * regmap will never serve these from cache.
> + */
> +static const struct regmap_range qmc5883p_volatile_ranges[] = {
> + regmap_reg_range(QMC5883P_REG_X_LSB, QMC5883P_REG_Z_MSB),
> + regmap_reg_range(QMC5883P_REG_STATUS, QMC5883P_REG_STATUS),
> +};
> +
> +/*
> + * Precious registers: reading has a side effect (clears DRDY/OVFL bits).
> + * regmap will never read these speculatively.
> + */
> +static const struct regmap_range qmc5883p_precious_ranges[] = {
> + regmap_reg_range(QMC5883P_REG_STATUS, QMC5883P_REG_STATUS),
> +};
> +
> +static const struct regmap_access_table qmc5883p_readable_table = {
> + .yes_ranges = qmc5883p_readable_ranges,
> + .n_yes_ranges = ARRAY_SIZE(qmc5883p_readable_ranges),
> +};
> +
> +static const struct regmap_access_table qmc5883p_writable_table = {
> + .yes_ranges = qmc5
> 883p_writable_ranges,
> + .n_yes_ranges = ARRAY_SIZE(qmc5883p_writable_ranges),
> +};
> +
> +static const struct regmap_access_table qmc5883p_volatile_table = {
> + .yes_ranges = qmc5883p_volatile_ranges,
> + .n_yes_ranges = ARRAY_SIZE(qmc5883p_volatile_ranges),
> +};
> +
> +static const struct regmap_access_table qmc5883p_precious_table = {
> + .yes_ranges = qmc5883p_precious_ranges,
> + .n_yes_ranges = ARRAY_SIZE(qmc5883p_precious_ranges),
> +};
> +
> +static const struct regmap_config qmc5883p_regmap_config = {
> + .reg_bits = 8,
> + .val_bits = 8,
> + .max_register = 0x0B,
> + .cache_type = REGCACHE_RBTREE,
> + .rd_table = &qmc5883p_readable_table,
> + .wr_table = &qmc5883p_writable_table,
> + .volatile_table = &qmc5883p_volatile_table,
> + .precious_table = &qmc5883p_precious_table,
> +};
> +
> +struct qmc5883p_data {
> + struct device *dev;
> + struct regmap *regmap;
> + struct mutex mutex; /* protects regmap and rf field accesses */
> + struct qmc5883p_rf rf;
> +};
> +
> +enum qmc5883p_channels {
> + AXIS_X = 0
> ,
> + AXIS_Y,
> + AXIS_Z,
> +};
> +
> +static const struct reg_field qmc5883p_rf_osr =
> + REG_FIELD(QMC5883P_REG_CTRL_1, 4, 5);
> +static const struct reg_field qmc5883p_rf_dsr =
> + REG_FIELD(QMC5883P_REG_CTRL_1, 6, 7);
> +static const struct reg_field qmc5883p_rf_odr =
> + REG_FIELD(QMC5883P_REG_CTRL_1, 2, 3);
> +static const struct reg_field qmc5883p_rf_mode =
> + REG_FIELD(QMC5883P_REG_CTRL_1, 0, 1);
> +static const struct reg_field qmc5883p_rf_rng =
> + REG_FIELD(QMC5883P_REG_CTRL_2, 2, 3);
> +static const struct reg_field qmc5883p_rf_rstctrl =
> + REG_FIELD(QMC5883P_REG_CTRL_2, 0, 1);
> +static const struct reg_field qmc5883p_rf_sftrst =
> + REG_FIELD(QMC5883P_REG_CTRL_2, 7, 7);
> +static const struct reg_field qmc5883p_rf_selftest =
> + REG_FIELD(QMC5883P_REG_CTRL_2, 6, 6);
> +static const struct reg_field qmc5883p_rf_chip_id =
> + REG_FIELD(QMC5883P_REG_CHIP_ID, 0, 7);
> +
> +static int qmc5883p_rf_init(struct qmc5883p_data *data)
> +{
> + struct regmap *regmap = data->regmap;
> + struct device *dev = d
> ata->dev;
> + struct qmc5883p_rf *rf = &data->rf;
> +
> + rf->osr = devm_regmap_field_alloc(dev, regmap, qmc5883p_rf_osr);
> + if (IS_ERR(rf->osr))
> + return PTR_ERR(rf->osr);
> +
> + rf->dsr = devm_regmap_field_alloc(dev, regmap, qmc5883p_rf_dsr);
> + if (IS_ERR(rf->dsr))
> + return PTR_ERR(rf->dsr);
> +
> + rf->odr = devm_regmap_field_alloc(dev, regmap, qmc5883p_rf_odr);
> + if (IS_ERR(rf->odr))
> + return PTR_ERR(rf->odr);
> +
> + rf->mode = devm_regmap_field_alloc(dev, regmap, qmc5883p_rf_mode);
> + if (IS_ERR(rf->mode))
> + return PTR_ERR(rf->mode);
> +
> + rf->rng = devm_regmap_field_alloc(dev, regmap, qmc5883p_rf_rng);
> + if (IS_ERR(rf->rng))
> + return PTR_ERR(rf->rng);
> +
> + rf->rstctrl = devm_regmap_field_alloc(dev, regmap, qmc5883p_rf_rstctrl);
> + if (IS_ERR(rf->rstctrl))
> + return PTR_ERR(rf->rstctrl);
> +
> + rf->sftrst = devm_regmap_field_alloc(dev, regmap, qmc5883p_rf_sftrst);
> + if (IS_ERR(rf->sftrst))
> + return PTR_ERR(rf->sftrst);
> +
> + rf->selftest =
> + devm_regmap_field_alloc(de
> v, regmap, qmc5883p_rf_selftest);
> + if (IS_ERR(rf->selftest))
> + return PTR_ERR(rf->selftest);
> +
> + rf->chip_id = devm_regmap_field_alloc(dev, regmap, qmc5883p_rf_chip_id);
> + if (IS_ERR(rf->chip_id))
> + return PTR_ERR(rf->chip_id);
> +
> + return 0;
> +}
> +
> +static int qmc5883p_verify_chip_id(struct qmc5883p_data *data)
> +{
> + int ret, regval;
> +
> + ret = regmap_field_read(data->rf.chip_id, ®val);
> + if (ret)
> + return dev_err_probe(data->dev, ret,
> + "failed to read chip ID\n");
> +
> + if (regval != QMC5883P_CHIP_ID)
> + return dev_err_probe(data->dev, -ENODEV,
We don't consider ID match an error. It has happened too many times
that there is a compatible part with a different ID. This can just
be dev_info() and return success.
> + "unexpected chip ID 0x%02x, expected 0x%02x\n",
> + regval, QMC5883P_CHIP_ID);
> + return ret;
> +}
> +
> +static int qmc5883p_chip_init(struct qmc5883p_data *data)
> +{
> + int ret;
> +
> + ret = regmap_field_write(data->rf.sftrst, 1);
> + if (ret)
> + return ret;
> +
> + usleep_range(1000, 2000);
Use fsleep() instead and add a comment explaining why this specific duration
was selected.
> +
> + ret = regmap_field_write(data->rf.sftrst, 0);
> + if (ret)
> + return ret;
> +
> + ret = regmap_field_write
> (data->rf.rstctrl, QMC5883P_RSTCTRL_SET_RESET);
> + if (ret)
> + return ret;
> +
> + ret = regmap_field_write(data->rf.rng, QMC5883P_DEFAULT_RNG);
> + if (ret)
> + return ret;
> +
> + ret = regmap_field_write(data->rf.osr, QMC5883P_DEFAULT_OSR);
> + if (ret)
> + return ret;
> +
> + ret = regmap_field_write(data->rf.dsr, QMC5883P_DEFAULT_DSR);
> + if (ret)
> + return ret;
> +
> + ret = regmap_field_write(data->rf.odr, QMC5883P_DEFAULT_ODR);
> + if (ret)
> + return ret;
Since we just reset the chip, why do we need to set everything to a
new default instead of using the chip's default? If there is a good
reason, add a comment, otherwise we can leave this out.
> +
> + return regmap_field_write(data->rf.mode, QMC5883P_MODE_NORMAL);
Does this start sampling? Seems like it could be out of place here.
> +}
> +
> +/*
> + * qmc5883p_get_measure - read all three axes.
> + * Must be called with data->mutex held.
> + * Handles PM internally: resumes device, reads data, schedules autosuspend.
> + */
> +static int qmc5883p_get_measure(struct qmc5883p_data *data, s16 *x, s16 *y,
> + s16 *z)
> +{
> + int ret;
> + u8 reg_data[6];
> + unsigned int status;
> +
> + ret = pm_runtime_resume_and_get(data->dev);
> + if (ret < 0)
> + return ret;
> +
> + /*
> + * Poll the status register until DR
> DY is set or timeout.
> + * Read the whole register in one shot so that OVFL is captured from
> + * the same read: reading 0x09 clears both DRDY and OVFL, so a second
> + * read would always see OVFL=0.
> + * At ODR=10Hz one period is 100ms; use 150ms as a safe upper bound.
> + */
> + ret = regmap_read_poll_timeout(data->regmap, QMC5883P_REG_STATUS,
> + status, status & QMC5883P_STATUS_DRDY,
> + QMC5883P_DRDY_POLL_US, 150000);
Numbers with lots of 0s are easier to read as 150 * (MICRO / MILLI).
> + if (ret)
> + goto out;
> +
> + if (status & QMC5883P_STATUS_OVFL) {
> + dev_warn_ratelimited(data->dev,
> + "data overflow, consider reducing field range\n");
> + ret = -ERANGE;
> + goto out;
> + }
> +
> + ret = regmap_bulk_read(data->regmap, QMC5883P_REG_X_LSB, reg_data,
> + ARRAY_SIZE(reg_data));
> + if (ret)
> + goto out;
> +
> + *x = (s16)get_unaligned_le16(®_data[0]);
> + *y = (s16)get_unaligned_le16(®_data[2]);
> + *z = (s16)get_unaligned_le16(®_data[4]);
> +
> +out:
> + pm_runtime_mark_last_busy(data->dev);
> + pm_runtime_put_
> autosuspend(data->dev);
> + return ret;
> +}
> +
> +static int qmc5883p_write_scale(struct qmc5883p_data *data, int val, int val2)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(qmc5883p_scale); i++) {
> + if (qmc5883p_scale[i][0] == val && qmc5883p_scale[i][1] == val2)
> + return regmap_field_write(data->rf.rng, i);
> + }
> +
> + return -EINVAL;
> +}
> +
> +static int qmc5883p_write_odr(struct qmc5883p_data *data, int val)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(qmc5883p_odr); i++) {
> + if (qmc5883p_odr[i] == val)
> + return regmap_field_write(data->rf.odr, i);
> + }
> +
> + return -EINVAL;
> +}
> +
> +static int qmc5883p_write_osr(struct qmc5883p_data *data, int val)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(qmc5883p_osr); i++) {
> + if (qmc5883p_osr[i] == val)
> + return regmap_field_write(data->rf.osr, i);
> + }
> +
> + return -EINVAL;
> +}
> +
> +static ssize_t downsampling_ratio_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct iio_dev *indio
> _dev = dev_get_drvdata(dev);
> + struct qmc5883p_data *data = iio_priv(indio_dev);
> + unsigned int regval;
> + int ret;
> +
> + guard(mutex)(&data->mutex);
> +
> + ret = regmap_field_read(data->rf.dsr, ®val);
> + if (ret)
> + return ret;
> +
> + return sysfs_emit(buf, "%u\n", qmc5883p_dsr[regval]);
> +}
> +
> +static ssize_t downsampling_ratio_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t len)
> +{
> + struct iio_dev *indio_dev = dev_get_drvdata(dev);
> + struct qmc5883p_data *data = iio_priv(indio_dev);
> + unsigned int val;
> + int i, ret, restore;
> +
> + ret = kstrtouint(buf, 10, &val);
> + if (ret)
> + return ret;
> +
> + guard(mutex)(&data->mutex);
> +
> + ret = pm_runtime_resume_and_get(data->dev);
> + if (ret)
> + return ret;
> +
> + ret = regmap_field_write(data->rf.mode, QMC5883P_MODE_SUSPEND);
> + if (ret)
> + goto out;
> +
> + ret = -EINVAL;
> + for (i = 0; i < ARRAY_SIZE(qmc5883p_dsr); i++) {
> + if (qmc5883p_dsr[i] == val) {
> + ret = regmap_field_wr
> ite(data->rf.dsr, i);
> + break;
> + }
> + }
> +
> + restore = regmap_field_write(data->rf.mode, QMC5883P_MODE_NORMAL);
> + if (restore && !ret)
> + ret = restore;
> +
> +out:
> + pm_runtime_mark_last_busy(data->dev);
> + pm_runtime_put_autosuspend(data->dev);
> + return ret ? ret : (ssize_t)len;
> +}
> +
> +static int qmc5883p_read_raw(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan, int *val,
> + int *val2, long mask)
> +{
> + s16 x, y, z;
> + struct qmc5883p_data *data = iio_priv(indio_dev);
> + int ret;
> + unsigned int regval;
> +
> + guard(mutex)(&data->mutex);
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + ret = qmc5883p_get_measure(data, &x, &y, &z);
> + if (ret < 0)
> + return ret;
> + switch (chan->address) {
> + case AXIS_X:
> + *val = x;
> + break;
> + case AXIS_Y:
> + *val = y;
> + break;
> + case AXIS_Z:
> + *val = z;
> + break;
> + }
> + return IIO_VAL_INT;
> +
> + case IIO_CHAN_INFO_SCALE:
> + ret = regmap_field_read(data->rf.rng, ®val);
> + if (
> ret < 0)
> + return ret;
> + *val = qmc5883p_scale[regval][0];
> + *val2 = qmc5883p_scale[regval][1];
> + return IIO_VAL_FRACTIONAL;
> +
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + ret = regmap_field_read(data->rf.odr, ®val);
> + if (ret < 0)
> + return ret;
> + *val = qmc5883p_odr[regval];
> + return IIO_VAL_INT;
> +
> + case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> + ret = regmap_field_read(data->rf.osr, ®val);
> + if (ret < 0)
> + return ret;
> + *val = qmc5883p_osr[regval];
> + return IIO_VAL_INT;
> + }
> +
> + return -EINVAL;
> +}
> +
> +static int qmc5883p_write_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan, int val,
> + int val2, long mask)
> +{
> + struct qmc5883p_data *data = iio_priv(indio_dev);
> + int ret, restore;
> +
> + guard(mutex)(&data->mutex);
> +
> + ret = pm_runtime_resume_and_get(data->dev);
> + if (ret)
> + return ret;
> +
> + ret = regmap_field_write(data->rf.mode, QMC5883P_MODE_SUSPEND);
> + if (ret)
> + goto out;
> +
> + switch (mask) {
> + case IIO_
> CHAN_INFO_SAMP_FREQ:
> + ret = qmc5883p_write_odr(data, val);
> + break;
> + case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> + ret = qmc5883p_write_osr(data, val);
> + break;
> + case IIO_CHAN_INFO_SCALE:
> + ret = qmc5883p_write_scale(data, val, val2);
> + break;
> + default:
> + ret = -EINVAL;
> + break;
> + }
> +
> + restore = regmap_field_write(data->rf.mode, QMC5883P_MODE_NORMAL);
> + if (restore && !ret)
> + ret = restore;
> +
> +out:
> + pm_runtime_mark_last_busy(data->dev);
> + pm_runtime_put_autosuspend(data->dev);
> + return ret;
> +}
> +
> +/*
> + * qmc5883p_read_avail - expose available values to userspace.
> + *
> + * Creates the _available sysfs attributes automatically:
> + * in_magn_sampling_frequency_available
> + * in_magn_oversampling_ratio_available
> + * in_magn_scale_available
> + */
> +static int qmc5883p_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_SAMP_FREQ:
> + *vals = qmc5883p_odr;
> + *type = IIO_VAL_INT;
> + *length = ARRAY_SIZE(qmc5883p_odr);
> + return IIO_AVAIL_LIST;
> +
> + case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> + *vals = qmc5883p_osr;
> + *type = IIO_VAL_INT;
> + *length = ARRAY_SIZE(qmc5883p_osr);
> + return IIO_AVAIL_LIST;
> +
> + case IIO_CHAN_INFO_SCALE:
> + *vals = (const int *)qmc5883p_scale;
> + *type = IIO_VAL_FRACTIONAL;
> + *length = ARRAY_SIZE(qmc5883p_scale) * 2;
> + return IIO_AVAIL_LIST;
> +
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static IIO_DEVICE_ATTR(downsampling_ratio, 0644, downsampling_ratio_show,
> + downsampling_ratio_store, 0);
> +static IIO_CONST_ATTR(downsampling_ratio_available, "1 2 4 8");
As mentioned in the cover letter, we'd like to know more about what
this actually does. If there is a good reason it doesn't fit with
any existing filter attribute, then we'll need a patch to document
the sysfs ABI as well.
> +
> +static struct attribute *qmc5883p_attributes[] = {
> + &iio_dev_attr_downsampling_ratio.dev_attr.attr,
> + &iio_const_attr_downsampling_ratio_available.dev_attr.attr, NULL
> +};
> +
> +static const struct attribute_group qmc5883p_attribute_group = {
> + .attrs = qmc5883p_attribu
> tes,
> +};
> +
> +static const struct iio_info qmc5883p_info = {
> + .attrs = &qmc5883p_attribute_group,
> + .read_raw = qmc5883p_read_raw,
> + .write_raw = qmc5883p_write_raw,
> + .read_avail = qmc5883p_read_avail,
> +};
> +
> +static const struct iio_chan_spec qmc5883p_channels[] = {
> + {
> + .type = IIO_MAGN,
> + .channel2 = IIO_MOD_X,
> + .modified = 1,
> + .address = AXIS_X,
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> + BIT(IIO_CHAN_INFO_SCALE),
> + .info_mask_separate_available = BIT(IIO_CHAN_INFO_SCALE),
> + .info_mask_shared_by_type =
> + BIT(IIO_CHAN_INFO_SAMP_FREQ) |
> + BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
> + .info_mask_shared_by_type_available =
> + BIT(IIO_CHAN_INFO_SAMP_FREQ) |
> + BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
> + },
Could save some dupilcation by making a macro that takes X/Y/Z
as parameter. e.g.
#define QMC5883P_CHAN(ch) \
... \
.channel2 = IIO_MOD_##ch, \
... \
.address = AXIS_##ch, \
...
> + {
> + .type = IIO_MAGN,
> + .channel2 = IIO_MOD_Y,
> + .modified = 1,
> + .address = AXIS_Y,
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> + BIT(IIO_CHAN_INFO_SCALE),
> + .info_mask_separate_available = BIT(IIO_CHAN_IN
> FO_SCALE),
> + .info_mask_shared_by_type =
> + BIT(IIO_CHAN_INFO_SAMP_FREQ) |
> + BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
> + .info_mask_shared_by_type_available =
> + BIT(IIO_CHAN_INFO_SAMP_FREQ) |
> + BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
> + },
> + {
> + .type = IIO_MAGN,
> + .channel2 = IIO_MOD_Z,
> + .modified = 1,
> + .address = AXIS_Z,
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> + BIT(IIO_CHAN_INFO_SCALE),
> + .info_mask_separate_available = BIT(IIO_CHAN_INFO_SCALE),
> + .info_mask_shared_by_type =
> + BIT(IIO_CHAN_INFO_SAMP_FREQ) |
> + BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
> + .info_mask_shared_by_type_available =
> + BIT(IIO_CHAN_INFO_SAMP_FREQ) |
> + BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
> + },
> +};
> +
> +static void qmc5883p_suspend_action(void *arg)
> +{
> + struct qmc5883p_data *data = arg;
> +
> + regmap_field_write(data->rf.mode, QMC5883P_MODE_SUSPEND);
> +}
> +
> +static int qmc5883p_probe(struct i2c_client *client)
> +{
> + struct device *dev = &client->d
> ev;
> + struct qmc5883p_data *data;
> + struct iio_dev *indio_dev;
> + struct regmap *regmap;
> + int ret;
> +
> + indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
> + if (!indio_dev)
> + return -ENOMEM;
> +
> + regmap = devm_regmap_init_i2c(client, &qmc5883p_regmap_config);
> + if (IS_ERR(regmap))
> + return dev_err_probe(dev, PTR_ERR(regmap),
> + "regmap initialization failed\n");
> +
> + data = iio_priv(indio_dev);
> + data->dev = dev;
> + data->regmap = regmap;
> + mutex_init(&data->mutex);
> +
> + ret = devm_regulator_get_enable_optional(dev, "vdd");
> + if (ret && ret != -ENODEV)
> + return dev_err_probe(dev, ret,
> + "failed to get vdd regulator\n");
> +
> + /* Datasheet specifies up to 50 ms supply ramp + 250 us POR time. */
> + fsleep(50000);
> +
> + i2c_set_clientdata(client, indio_dev);
> +
> + ret = qmc5883p_rf_init(data);
Would be more logical to move this up right after regmap is declared.
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "failed to initialize regmap fields\n");
> +
> + ret = qmc5883p_verify_chip_id(data);
> + if (ret)
> + retur
> n ret;
> +
> + ret = qmc5883p_chip_init(data);
> + if (ret)
> + return dev_err_probe(dev, ret, "failed to initialize chip\n");
> +
> + indio_dev->name = "qmc5883p";
> + indio_dev->info = &qmc5883p_info;
> + indio_dev->modes = INDIO_DIRECT_MODE;
> + indio_dev->channels = qmc5883p_channels;
> + indio_dev->num_channels = ARRAY_SIZE(qmc5883p_channels);
> +
> + pm_runtime_set_autosuspend_delay(dev, 2000);
> + pm_runtime_use_autosuspend(dev);
> +
> + pm_runtime_set_active(dev);
> +
> + ret = devm_pm_runtime_enable(dev);
> + if (ret)
> + return ret;
> +
> + pm_runtime_mark_last_busy(dev);
> +
> + ret = devm_add_action_or_reset(dev, qmc5883p_suspend_action, data);
> + if (ret)
> + return ret;
> +
> + ret = devm_iio_device_register(dev, indio_dev);
Usually, we just return directly here. This pretty much doesn't ever fail.
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "failed to register IIO device\n");
> + return 0;
> +}
> +
> +static int qmc5883p_runtime_suspend(struct device *dev)
> +{
> + struct iio_dev *indio_dev = dev_get_drvdata(dev);
> + struct qmc5883p_data *data = iio_priv(indi
> o_dev);
> +
> + return regmap_field_write(data->rf.mode, QMC5883P_MODE_SUSPEND);
> +}
> +
> +static int qmc5883p_runtime_resume(struct device *dev)
> +{
> + struct iio_dev *indio_dev = dev_get_drvdata(dev);
> + struct qmc5883p_data *data = iio_priv(indio_dev);
> + int ret;
> +
> + ret = regmap_field_write(data->rf.mode, QMC5883P_MODE_NORMAL);
> + if (ret)
> + return ret;
> +
> + usleep_range(10000, 11000);
Again, fsleep() and comment.
> + return 0;
> +}
> +
> +static int qmc5883p_system_suspend(struct device *dev)
> +{
> + return pm_runtime_force_suspend(dev);
> +}
> +
> +static int qmc5883p_system_resume(struct device *dev)
> +{
> + struct iio_dev *indio_dev = dev_get_drvdata(dev);
> + struct qmc5883p_data *data = iio_priv(indio_dev);
> + int ret;
> +
> + ret = pm_runtime_force_resume(dev);
> + if (ret)
> + return ret;
> +
> + /*
> + * If the regulator was cut during system suspend, POR will have
> + * reset all registers. Reinitialise the chip and resync the
> + * regmap cache so that cached control register values are pushed
> + * back to har
> dware.
> + */
> + ret = qmc5883p_chip_init(data);
> + if (ret)
> + return ret;
> +
> + regcache_mark_dirty(data->regmap);
> + return regcache_sync(data->regmap);
> +}
> +
> +static const struct dev_pm_ops qmc5883p_dev_pm_ops = {
> + SYSTEM_SLEEP_PM_OPS(qmc5883p_system_suspend, qmc5883p_system_resume)
> + RUNTIME_PM_OPS(qmc5883p_runtime_suspend, qmc5883p_runtime_resume, NULL)
> +};
> +
> +static const struct of_device_id qmc5883p_of_match[] = {
> + { .compatible = "qst,qmc5883p" },
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, qmc5883p_of_match);
> +
> +static const struct i2c_device_id qmc5883p_id[] = {
> + { "qmc5883p", 0 },
> + {},
IIO style for this is:
{ }
space between braces and no trailing comma.
> +};
> +MODULE_DEVICE_TABLE(i2c, qmc5883p_id);
> +
^ permalink raw reply
* Re: [PATCH v2 4/4] MAINTAINERS: Add entry for QST QMC5883P magnetometer driver
From: David Lechner @ 2026-04-10 19:32 UTC (permalink / raw)
To: Hardik Phalet, Greg Kroah-Hartman
Cc: Jonathan Cameron, Nuno Sá, Andy Shevchenko, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Brigham Campbell, Shuah Khan,
linux-iio, devicetree, linux-kernel, linux-staging
In-Reply-To: <20260409210639.3197576-5-hardik.phalet@pm.me>
On 4/9/26 4:07 PM, Hardik Phalet wrote:
> Add a MAINTAINERS entry for the QST QMC5883P staging IIO driver,
> covering the driver source and its device tree binding.
>
> Signed-off-by: Hardik Phalet <hardik.phalet@pm.me>
> ---
> MAINTAINERS | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a92290fffa16..d0b9bfceb283 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -20274,6 +20274,13 @@ F: Documentation/networking/device_drivers/ethernet/freescale/dpaa2/overview.rst
> F: drivers/bus/fsl-mc/
> F: include/uapi/linux/fsl_mc.h
>
> +QST QMC5883P MAGNETOMETER DRIVER
> +M: Hardik Phalet <hardik.phalet@pm.me>
> +L: linux-iio@vger.kernel.org
> +S: Maintained
> +F: Documentation/devicetree/bindings/iio/magnetometer/qst,qmc5883p.yaml
> +F: drivers/staging/iio/magnetometer/
> +
> QT1010 MEDIA DRIVER
> L: linux-media@vger.kernel.org
> S: Orphan
This should be split up and added in the patches that actually
add the F: files. Most of it will go with the dt-bingings patch
and the one line added later with the driver.
^ permalink raw reply
* Re: [PATCH v2 0/4] Add QST QMC5883P magnetometer driver
From: David Lechner @ 2026-04-10 19:26 UTC (permalink / raw)
To: Hardik Phalet, Greg Kroah-Hartman
Cc: Jonathan Cameron, Nuno Sá, Andy Shevchenko, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Brigham Campbell, Shuah Khan,
linux-iio, devicetree, linux-kernel, linux-staging
In-Reply-To: <20260409210639.3197576-1-hardik.phalet@pm.me>
On 4/9/26 4:07 PM, Hardik Phalet wrote:
For a series this be, please wait at least a week for more feedback
before submitting the next revision.
> This series adds initial Linux support for the QST QMC5883P, a 3-axis
> anisotropic magneto-resistive (AMR) magnetometer with a 16-bit ADC that
> communicates over I2C. To my knowledge there is no existing upstream
> driver for this device.
>
> The driver supports:
> - Raw magnetic field readings on X, Y, and Z axes
> - Four selectable full-scale ranges (±2 G, ±8 G, ±12 G, ±30 G)
> - Configurable output data rate (10, 50, 100, 200 Hz)
> - Configurable oversampling ratio (1, 2, 4, 8)
> - Configurable downsampling ratio (1, 2, 4, 8) via a custom sysfs
What is the difference between oversampling and downsampling? I think
we have used some filter attribute for downsampling/decimation in some
other drivers so maybe that could be a good fit?
> attribute
> - Runtime PM with a 2 s autosuspend delay
> - System suspend/resume via pm_runtime_force_suspend/resume
>
> Regmap with an rbtree cache is used throughout. CTRL_1 and CTRL_2
> bit fields are accessed via regmap_field to avoid read-modify-write
> races. The STATUS register is marked precious so regmap never reads
> it speculatively and clears the DRDY/OVFL bits unexpectedly.
>
> The init sequence on probe is: soft reset → wait 1 ms → deassert
> reset → configure SET/RESET control → apply default ODR/OSR/DSR/RNG
> → enter normal mode. This ordering was determined empirically on
> hardware to produce reliable, non-zero axis readings.
>
> The driver is placed under drivers/staging/iio/magnetometer/ with a
> TODO file tracking the remaining work before it can graduate:
> - Triggered buffer support (iio_triggered_buffer_setup)
> - DRDY interrupt support
> - Self-test implementation
These are not reasons to have the driver in staging. It is fine
to have a driver that doesn't implement all functionality. We should
be able to add those features without breaking anything.
^ permalink raw reply
* [PATCH 35/35] arm64: dts: qcom: talos: Drop unused second PDC reg entry
From: Mukesh Ojha @ 2026-04-10 18:41 UTC (permalink / raw)
To: Thomas Gleixner, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bjorn Andersson, Konrad Dybcio
Cc: cros-qcom-dts-watchers, linux-arm-msm, linux-kernel, devicetree,
Mukesh Ojha
In-Reply-To: <20260410184124.1068210-1-mukesh.ojha@oss.qualcomm.com>
The PDC driver only maps the first register region (APSS DRV) via
of_address_to_resource(node, 0, ...). The second reg entry was never
accessed by the driver and can be removed.
Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
---
arch/arm64/boot/dts/qcom/talos.dtsi | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/talos.dtsi b/arch/arm64/boot/dts/qcom/talos.dtsi
index 4b038fb22071..e825ee1b957d 100644
--- a/arch/arm64/boot/dts/qcom/talos.dtsi
+++ b/arch/arm64/boot/dts/qcom/talos.dtsi
@@ -4080,8 +4080,7 @@ dispcc: clock-controller@af00000 {
pdc: interrupt-controller@b220000 {
compatible = "qcom,qcs615-pdc", "qcom,pdc";
- reg = <0x0 0x0b220000 0x0 0x10000>,
- <0x0 0x17c000f0 0x0 0x64>;
+ reg = <0x0 0x0b220000 0x0 0x10000>;
qcom,pdc-ranges = <0 480 94>, <94 609 31>, <125 63 1>;
interrupt-parent = <&intc>;
#interrupt-cells = <2>;
--
2.53.0
^ permalink raw reply related
* [PATCH 34/35] arm64: dts: qcom: sdx75: Drop unused second PDC reg entry
From: Mukesh Ojha @ 2026-04-10 18:41 UTC (permalink / raw)
To: Thomas Gleixner, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bjorn Andersson, Konrad Dybcio
Cc: cros-qcom-dts-watchers, linux-arm-msm, linux-kernel, devicetree,
Mukesh Ojha
In-Reply-To: <20260410184124.1068210-1-mukesh.ojha@oss.qualcomm.com>
The PDC driver only maps the first register region (APSS DRV) via
of_address_to_resource(node, 0, ...). The second reg entry was never
accessed by the driver and can be removed.
Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
---
arch/arm64/boot/dts/qcom/sdx75.dtsi | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/sdx75.dtsi b/arch/arm64/boot/dts/qcom/sdx75.dtsi
index 82a8107f2f1c..e740e123c1e4 100644
--- a/arch/arm64/boot/dts/qcom/sdx75.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdx75.dtsi
@@ -1100,8 +1100,7 @@ usb_1_dwc3_ss: endpoint {
pdc: interrupt-controller@b220000 {
compatible = "qcom,sdx75-pdc", "qcom,pdc";
- reg = <0x0 0xb220000 0x0 0x10000>,
- <0x0 0x174000f0 0x0 0x64>;
+ reg = <0x0 0xb220000 0x0 0x10000>;
qcom,pdc-ranges = <0 147 52>,
<52 266 32>,
<84 500 59>;
--
2.53.0
^ permalink raw reply related
* [PATCH 33/35] arm64: dts: qcom: sc8280xp: Drop unused second PDC reg entry
From: Mukesh Ojha @ 2026-04-10 18:41 UTC (permalink / raw)
To: Thomas Gleixner, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bjorn Andersson, Konrad Dybcio
Cc: cros-qcom-dts-watchers, linux-arm-msm, linux-kernel, devicetree,
Mukesh Ojha
In-Reply-To: <20260410184124.1068210-1-mukesh.ojha@oss.qualcomm.com>
The PDC driver only maps the first register region (APSS DRV) via
of_address_to_resource(node, 0, ...). The second reg entry was never
accessed by the driver and can be removed.
Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
---
arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
index 2c6e48495d20..59354f2474c9 100644
--- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
@@ -5095,7 +5095,7 @@ dispcc0: clock-controller@af00000 {
pdc: interrupt-controller@b220000 {
compatible = "qcom,sc8280xp-pdc", "qcom,pdc";
- reg = <0 0x0b220000 0 0x10000>, <0 0x17c000f0 0 0x60>;
+ reg = <0 0x0b220000 0 0x10000>;
qcom,pdc-ranges = <0 480 40>,
<40 140 14>,
<54 263 1>,
--
2.53.0
^ permalink raw reply related
* [PATCH 32/35] arm64: dts: qcom: monaco: Drop unused second PDC reg entry
From: Mukesh Ojha @ 2026-04-10 18:41 UTC (permalink / raw)
To: Thomas Gleixner, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bjorn Andersson, Konrad Dybcio
Cc: cros-qcom-dts-watchers, linux-arm-msm, linux-kernel, devicetree,
Mukesh Ojha
In-Reply-To: <20260410184124.1068210-1-mukesh.ojha@oss.qualcomm.com>
The PDC driver only maps the first register region (APSS DRV) via
of_address_to_resource(node, 0, ...). The second reg entry was never
accessed by the driver and can be removed.
Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
---
arch/arm64/boot/dts/qcom/monaco.dtsi | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/monaco.dtsi b/arch/arm64/boot/dts/qcom/monaco.dtsi
index 84b22866d2f3..cf71b559b094 100644
--- a/arch/arm64/boot/dts/qcom/monaco.dtsi
+++ b/arch/arm64/boot/dts/qcom/monaco.dtsi
@@ -5744,8 +5744,7 @@ dispcc: clock-controller@af00000 {
pdc: interrupt-controller@b220000 {
compatible = "qcom,qcs8300-pdc", "qcom,pdc";
- reg = <0x0 0xb220000 0x0 0x10000>,
- <0x0 0x17c000f0 0x0 0x64>;
+ reg = <0x0 0xb220000 0x0 0x10000>;
interrupt-parent = <&intc>;
#interrupt-cells = <2>;
interrupt-controller;
--
2.53.0
^ permalink raw reply related
* [PATCH 31/35] arm64: dts: qcom: milos: Drop unused second PDC reg entry
From: Mukesh Ojha @ 2026-04-10 18:41 UTC (permalink / raw)
To: Thomas Gleixner, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bjorn Andersson, Konrad Dybcio
Cc: cros-qcom-dts-watchers, linux-arm-msm, linux-kernel, devicetree,
Mukesh Ojha
In-Reply-To: <20260410184124.1068210-1-mukesh.ojha@oss.qualcomm.com>
The PDC driver only maps the first register region (APSS DRV) via
of_address_to_resource(node, 0, ...). The second reg entry was never
accessed by the driver and can be removed.
Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
---
arch/arm64/boot/dts/qcom/milos.dtsi | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/milos.dtsi b/arch/arm64/boot/dts/qcom/milos.dtsi
index 71941e6f49bf..a3c2ac8ca675 100644
--- a/arch/arm64/boot/dts/qcom/milos.dtsi
+++ b/arch/arm64/boot/dts/qcom/milos.dtsi
@@ -1564,8 +1564,7 @@ dispcc: clock-controller@af00000 {
pdc: interrupt-controller@b220000 {
compatible = "qcom,milos-pdc", "qcom,pdc";
- reg = <0x0 0x0b220000 0x0 0x10000>,
- <0x0 0x174000f0 0x0 0x64>;
+ reg = <0x0 0x0b220000 0x0 0x10000>;
interrupt-parent = <&intc>;
qcom,pdc-ranges = <0 480 40>, <40 140 11>, <51 527 47>,
--
2.53.0
^ permalink raw reply related
* [PATCH 30/35] arm64: dts: qcom: lemans: Drop unused second PDC reg entry
From: Mukesh Ojha @ 2026-04-10 18:41 UTC (permalink / raw)
To: Thomas Gleixner, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bjorn Andersson, Konrad Dybcio
Cc: cros-qcom-dts-watchers, linux-arm-msm, linux-kernel, devicetree,
Mukesh Ojha
In-Reply-To: <20260410184124.1068210-1-mukesh.ojha@oss.qualcomm.com>
The PDC driver only maps the first register region (APSS DRV) via
of_address_to_resource(node, 0, ...). The second reg entry was never
accessed by the driver and can be removed.
Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
---
arch/arm64/boot/dts/qcom/lemans.dtsi | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/lemans.dtsi b/arch/arm64/boot/dts/qcom/lemans.dtsi
index 1724df115873..e48c8ef7d8a1 100644
--- a/arch/arm64/boot/dts/qcom/lemans.dtsi
+++ b/arch/arm64/boot/dts/qcom/lemans.dtsi
@@ -5536,8 +5536,7 @@ dispcc0: clock-controller@af00000 {
pdc: interrupt-controller@b220000 {
compatible = "qcom,sa8775p-pdc", "qcom,pdc";
- reg = <0x0 0x0b220000 0x0 0x10000>,
- <0x0 0x17c000f0 0x0 0x64>;
+ reg = <0x0 0x0b220000 0x0 0x10000>;
qcom,pdc-ranges = <0 480 40>,
<40 140 14>,
<54 263 1>,
--
2.53.0
^ permalink raw reply related
* [PATCH 29/35] arm64: dts: qcom: kaanapali: Drop unused second PDC reg entry
From: Mukesh Ojha @ 2026-04-10 18:41 UTC (permalink / raw)
To: Thomas Gleixner, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bjorn Andersson, Konrad Dybcio
Cc: cros-qcom-dts-watchers, linux-arm-msm, linux-kernel, devicetree,
Mukesh Ojha
In-Reply-To: <20260410184124.1068210-1-mukesh.ojha@oss.qualcomm.com>
The PDC driver only maps the first register region (APSS DRV) via
of_address_to_resource(node, 0, ...). The second reg entry was never
accessed by the driver and can be removed.
Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
---
arch/arm64/boot/dts/qcom/kaanapali.dtsi | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/kaanapali.dtsi b/arch/arm64/boot/dts/qcom/kaanapali.dtsi
index 9ef57ad0ca71..7cc4ff48b3f5 100644
--- a/arch/arm64/boot/dts/qcom/kaanapali.dtsi
+++ b/arch/arm64/boot/dts/qcom/kaanapali.dtsi
@@ -960,8 +960,7 @@ opp-202000000 {
pdc: interrupt-controller@b220000 {
compatible = "qcom,kaanapali-pdc", "qcom,pdc";
- reg = <0x0 0x0b220000 0x0 0x10000>,
- <0x0 0x179600f0 0x0 0xf4>;
+ reg = <0x0 0x0b220000 0x0 0x10000>;
qcom,pdc-ranges = <0 745 38>,
<40 785 11>,
--
2.53.0
^ permalink raw reply related
* [PATCH 28/35] arm64: dts: qcom: qdu1000: Fix PDC reg size to single APSS DRV region
From: Mukesh Ojha @ 2026-04-10 18:41 UTC (permalink / raw)
To: Thomas Gleixner, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bjorn Andersson, Konrad Dybcio
Cc: cros-qcom-dts-watchers, linux-arm-msm, linux-kernel, devicetree,
Mukesh Ojha
In-Reply-To: <20260410184124.1068210-1-mukesh.ojha@oss.qualcomm.com>
The PDC reg size was 0x30000, covering three DRV regions. Linux only
needs the APSS DRV region which is a single 0x10000 window. Reduce
the size to 0x10000 to describe only the region actually used.
Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
---
arch/arm64/boot/dts/qcom/qdu1000.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/qcom/qdu1000.dtsi b/arch/arm64/boot/dts/qcom/qdu1000.dtsi
index cdfe40da5d33..4962c27d5d46 100644
--- a/arch/arm64/boot/dts/qcom/qdu1000.dtsi
+++ b/arch/arm64/boot/dts/qcom/qdu1000.dtsi
@@ -1054,7 +1054,7 @@ usb_1_dwc3_ss: endpoint {
pdc: interrupt-controller@b220000 {
compatible = "qcom,qdu1000-pdc", "qcom,pdc";
- reg = <0x0 0xb220000 0x0 0x30000>, <0x0 0x174000f0 0x0 0x64>;
+ reg = <0x0 0xb220000 0x0 0x10000>, <0x0 0x174000f0 0x0 0x64>;
qcom,pdc-ranges = <0 480 12>, <14 494 24>, <40 520 54>,
<94 609 31>, <125 63 1>;
#interrupt-cells = <2>;
--
2.53.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox