Hello Jonathan and Andy,

Please find a few comments below. They explain why I didn't do something
which was mentioned or suggested in the v4.

On Fri, Aug 12, 2022 at 04:52:29PM +0000, Dmitry Rokosov wrote:

[...]

+/*
+ * Possible Full Scale ranges
+ *
+ * Axis data is 12-bit signed value, so
+ *
+ * fs0 = (2 + 2) * 9.81 / (2^11) = 0.009580
+ * fs1 = (4 + 4) * 9.81 / (2^11) = 0.019160
+ * fs2 = (8 + 8) * 9.81 / (2^11) = 0.038320
+ * fs3 = (16 + 16) * 9.81 / (2^11) = 0.076641
+ */
+enum {
+	MSA311_FS_2G,
+	MSA311_FS_4G,
+	MSA311_FS_8G,
+	MSA311_FS_16G,
+};
+
+static const struct {
+	int val;
+	int val2;
+} msa311_fs_table[] = {
+	{0, 9580}, {0, 19160}, {0, 38320}, {0, 76641}
+};
+
+/* Possible Output Data Rate values */
+enum {
+	MSA311_ODR_1_HZ,
+	MSA311_ODR_1_95_HZ,
+	MSA311_ODR_3_9_HZ,
+	MSA311_ODR_7_81_HZ,
+	MSA311_ODR_15_63_HZ,
+	MSA311_ODR_31_25_HZ,
+	MSA311_ODR_62_5_HZ,
+	MSA311_ODR_125_HZ,
+	MSA311_ODR_250_HZ,
+	MSA311_ODR_500_HZ,
+	MSA311_ODR_1000_HZ,
+};
+
+static const struct {
+	int val;
+	int val2;
+} msa311_odr_table[] = {
+	{1, 0}, {1, 950000}, {3, 900000}, {7, 810000}, {15, 630000},
+	{31, 250000}, {62, 500000}, {125, 0}, {250, 0}, {500, 0}, {1000, 0}
+};

I didn't change odr and fs table structures to s32_fract, because they
don't have numerator/denominator format, but it's an integer.fractional
format. I didn't find such a common struct for this format. If we want to
generalize such a fractional number format, I suppose it's better to do it
in the separate patch series over the whole IIO subsystem, due to the many
val.val2-like structures inside it.

[...]

+/**
+ * struct msa311_priv - MSA311 internal private state
+ * @regs: Underlying I2C bus adapter used to abstract slave
+ *        register accesses
+ * @fields: Abstract objects for each registers fields access
+ * @dev: Device handler associated with appropriate bus client
+ * @lock: Protects msa311 device state between setup and data access routines
+ *        (power transitions, samp_freq/scale tune, retrieving axes data, etc)
+ * @chip_name: Chip name in the format "msa311-%hhx" % partid
+ * @new_data_trig: Optional NEW_DATA interrupt driven trigger used
+ *                 to notify external consumers a new sample is ready
+ * @vdd: Optional external voltage regulator for the device power supply
+ */
+struct msa311_priv {
+	struct regmap *regs;
+	struct regmap_field *fields[F_MAX_FIELDS];
+
+	struct device *dev;

I stay struct device *dev pointer in the msa311 private structure,
because without it code looks more grubby and we can't retrieve indio_dev
from msa311_priv object using container_of or something else
(it's just a dynamic pointer).

+	struct mutex lock;

I've removed the state guard comment, but checkpatch.pl is still raising
a warning.

[...]

+	err = msa311_chip_init(msa311);
+	if (err)
+		return err;
+
+	indio_dev->modes = INDIO_DIRECT_MODE;

I returned INDIO_DIRECT_MODE initialization, but actually I'm not sure
if it's needed when we setup buffer mode regardless of any optional
parameters.

[...]

-- 
Thank you,
Dmitry