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.
> + */
> +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)
> +}
> +
...
> +
> +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);
