Re: [PATCH v6 5/6] iio: imu: inv_icm42607: Implement MREGx register access

[Jonathan Cameron <jic23@kernel.org>]

Just a couple of trivial notes.
The regmap on regmap solution is quite nice.
Might be worth making sure you run tests with lockdep
though just in case there are any nasties hiding in doing that!

Jonathan

> diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
> index 75c9617e5d1e7..a4393b54cc108 100644
> --- a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
> +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
> @@ -18,63 +18,255 @@
>  #include <linux/time.h>
>  #include <linux/timekeeping.h>
>  #include <linux/types.h>
> +#include <linux/unaligned.h>
>  
>  #include <asm/byteorder.h>
>  
>  #include "inv_icm42607.h"
>  
> -static bool inv_icm42607_is_readable_reg(struct device *dev, unsigned int reg)
> +static int inv_icm42607_mclk_get(struct regmap *map, bool *idle_set)
>  {
> -	switch (reg) {
> -	case INV_ICM42607_REG_MCLK_RDY ... INV_ICM42607_REG_INT_CONFIG:
> -	case INV_ICM42607_REG_TEMP_DATA1 ... INV_ICM42607_REG_TMST_FSYNCL:
> -	case INV_ICM42607_REG_APEX_DATA4 ... INV_ICM42607_REG_INTF_CONFIG1:
> -	case INV_ICM42607_REG_INT_STATUS_DRDY ... INV_ICM42607_REG_FIFO_DATA:
> -	case INV_ICM42607_REG_WHOAMI:
> -		return true;
> +	unsigned int val;
> +	int ret, ret2;
> +
> +	*idle_set = false;
> +
> +	ret = regmap_test_bits(map, INV_ICM42607_REG_MCLK_RDY,
> +		       INV_ICM42607_MCLK_RDY_BIT);
Align as:
	ret = regmap_test_bits(map, INV_ICM42607_REG_MCLK_RDY,
			       INV_ICM42607_MCLK_RDY_BIT);
I think you lost a tab somewhere!

> +	if (ret < 0)
> +		return ret;
> +	if (ret)


> diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_i2c.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_i2c.c
> index 9f34199ce2949..61705b162f86f 100644
> --- a/drivers/iio/imu/inv_icm42607/inv_icm42607_i2c.c
> +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_i2c.c
> @@ -12,6 +12,12 @@
>  
>  #include "inv_icm42607.h"
>  
> +const struct regmap_config inv_icm42607_regmap_config = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +};
> +EXPORT_SYMBOL_NS_GPL(inv_icm42607_regmap_config, "IIO_ICM42607");
I think sashiko had a comment on this being used from the SPI code
which is a problem if so.

Jonathan