On Tue, Dec 21, 2021 at 4:54 AM Jonathan Cameron <jic23@xxxxxxxxxx> wrote:
>
> On Fri, 17 Dec 2021 16:27:03 -0800
> Gwendal Grignou <gwendal@xxxxxxxxxxxx> wrote:
>
> > A simplified version of SX9324, it only have one pin and
> > 2 phases (aka channels).
> > Only one event is presented.
> >
> > Signed-off-by: Gwendal Grignou <gwendal@xxxxxxxxxxxx>
> > Reviewed-by: Stephen Boyd <swboyd@xxxxxxxxxxxx>
> > ---
> > Changes since v3:
> > - Remove "reference" as a new modifier: it is not needed, indexed
> > channels and labels are used instead.
> > - Use dev_get_drvdata to access iio device structure.
> > - Use already calculated local variable
> > - Use default: clause to return when possible.
> >
> > Changes since v2:
> > - Fix issues reported during sx9324 driver review:
> > - fix include with iwyu
> > - Remove call to ACPI_PTR to prevent unused variable warning.
> > - Fix panic when setting frequency to 0.
> > - Add offset to decipher interrupt register
> > - Fix default register value.
> >
> > Changes since v1:
> > - Remove SX9360_DRIVER_NAME
> > - Simplify whoami function
> > - Define WHOAMI register value internally.
> > - Handle negative values when setting sysfs parameters.
> >
> > drivers/iio/proximity/Kconfig | 14 +
> > drivers/iio/proximity/Makefile | 1 +
> > drivers/iio/proximity/sx9360.c | 816 +++++++++++++++++++++++++++++++++
> > 3 files changed, 831 insertions(+)
> > create mode 100644 drivers/iio/proximity/sx9360.c
> >
> Hi Gwendal,
>
> A few trivial things that I'd just have tidied up whilst applying except
> that this is dependent on the earlier series anyway so I can't apply yet.
>
> > +
> > +/*
> > + * Each entry contains the integer part (val) and the fractional part, in micro
> > + * seconds. It conforms to the IIO output IIO_VAL_INT_PLUS_MICRO.
> > + *
> > + * The frequency control register holds the period, with a ~2ms increment.
> > + * Therefore the smallest frequency is 4MHz / (2047 * 8192),
> > + * The fastest is 4MHz / 8192.
> > + * The interval is not linear, but given there is 2047 possible value,
> > + * Returns the fake increment of (Max-Min)/2047
> > + *
>
> Trivial, but this blank line doesn't add anything so please remove.
Remove some in sx_common.h as well.
>
> > + */
> > +static const struct {
> > + int val;
> > + int val2;
> > +} sx9360_samp_freq_interval[] = {
> > + {0, 281250}, /* 4MHz / (8192 * 2047) */
> > + {0, 281250},
> > + {448, 281250}, /* 4MHz / 8192 */
> > +};
> > +
> ...
>
>
> > +static int sx9360_read_label(struct iio_dev *iio_dev, const struct iio_chan_spec *chan,
> > + char *label)
> > +{
> > + return snprintf(label, PAGE_SIZE, "%s\n", sx9360_channel_labels[chan->channel]);
>
> Trivial but preference for sysfs_emit()
> (Andy noted this for the other driver in a more complex case so I did a quick
> search for repeats)
Fixed in other patches as well.
>
> > +}
> > +
>
>
> ...
>
> > +
> > +static const struct dev_pm_ops sx9360_pm_ops = {
> > + SET_SYSTEM_SLEEP_PM_OPS(sx9360_suspend, sx9360_resume)
> > +};
>
> static SIMPLE_DEV_PM_OPS(sx9360_pm_ops, sx9360_suspend, sx9360_resume);
Fix in sx9310/24 and 60 driver as well.
>
>
