On Tue, Dec 21, 2021 at 4:40 AM Jonathan Cameron <jic23@xxxxxxxxxx> wrote: > > On Sat, 18 Dec 2021 15:54:05 +0200 > Andy Shevchenko <andy.shevchenko@xxxxxxxxx> wrote: I am sorry, I forgot your comments. I submitted a v10. > > > On Sat, Dec 18, 2021 at 11:58 AM Gwendal Grignou <gwendal@xxxxxxxxxxxx> wrote: > > > > > > Before adding new Semtech sensors, move common logic to all Semtech SAR > > > sensor in its own file: > > > - interface with IIO subsystem, > > > - interrupt management, > > > - channel access conrol, > > > - event processing. > > > > > > The change adds a bidirectional interface between sx93xx and sx_common. > > > > ... > > > > > +/* All Semtech SAR sensor have IRQ bit in same order. */ > > > > sensors > > in the same done > > > > ... > > > > > +EXPORT_SYMBOL_GPL(sx_common_events); > > > > Here and in other places, can we switch to NS variants of the > > EXPORT_SYMBOL_GPL()? done > > We haven't yet done this for similar cases, but I agree it is > definitely a nice to have when we have a helper / core module > like this. Given the editorial stuff in here means Gwendal > is going to be rolling a v9 this seems a good time to start. > > As time allows we'll look to convert over similar modules in the > general interests of reducing namespace pollution. > > Ultimately I'd like to move the IIO core to a namespace as well > but that is a messy job and there are several things ahead of > if that will cause mass driver churn. > > Jonathan > > > > > > ... > > > > > +/** > > > + * sx_common_probe() - Common setup for Semtech SAR sensor > > > > > + * > > > > Here and in all similar cases, remove this redundant blank line. Done. > > > > > + * @client: I2C client object > > > + * @chip_info: Semtech sensor chip information. > > > + * @regmap_config: Sensor registers map configuration. > > > + */ > > > > ... > > > > > +#include <linux/types.h> > > > +#include <linux/iio/iio.h> > > > +#include <linux/iio/types.h> > > > +#include <linux/regulator/consumer.h> > > > > Perhaps grouped and ordered? > > > > + blank line. Done > > > > > +struct device; > > > +struct i2c_client; > > > +struct regmap_config; > > > +struct sx_common_data; > > > > ... > > > > > + * @num_channels: Number of channel/phase. > > > > channels or phases In IIO architecture, the sensor presents X channels. Buf in Semtech datasheet, they are named phases. Settle on channel, with an explanation at the first occurence. > > > > ... > > > > > + * @buffer: Bufffer to store raw samples. > > > > Buffer Done > > > > >