lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Fri, 12 Aug 2022 17:10:38 +0000 From: Dmitry Rokosov <DDRokosov@...rdevices.ru> To: "akpm@...ux-foundation.org" <akpm@...ux-foundation.org>, "jic23@...nel.org" <jic23@...nel.org>, "robh+dt@...nel.org" <robh+dt@...nel.org>, "andriy.shevchenko@...ux.intel.com" <andriy.shevchenko@...ux.intel.com>, "andy.shevchenko@...il.com" <andy.shevchenko@...il.com>, "christophe.jaillet@...adoo.fr" <christophe.jaillet@...adoo.fr>, "stano.jakubek@...il.com" <stano.jakubek@...il.com>, "shawnguo@...nel.org" <shawnguo@...nel.org>, "stephan@...hold.net" <stephan@...hold.net>, "daniel.lezcano@...aro.org" <daniel.lezcano@...aro.org>, "wsa@...nel.org" <wsa@...nel.org>, "lars@...afoo.de" <lars@...afoo.de>, "Michael.Hennerich@...log.com" <Michael.Hennerich@...log.com>, "jbhayana@...gle.com" <jbhayana@...gle.com>, "lucas.demarchi@...el.com" <lucas.demarchi@...el.com>, "jani.nikula@...el.com" <jani.nikula@...el.com>, "linus.walleij@...aro.org" <linus.walleij@...aro.org> CC: "linux-iio@...r.kernel.org" <linux-iio@...r.kernel.org>, kernel <kernel@...rdevices.ru>, "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>, "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org> Subject: Re: [PATCH v5 6/7] iio: add MEMSensing MSA311 3-axis accelerometer driver 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
Powered by blists - more mailing lists