* [PATCH v3 0/4] Add Semtech SX9360 SAR Sensor support
@ 2021-12-13 2:40 Gwendal Grignou
2021-12-13 2:40 ` [PATCH v3 1/4] iio: add IIO_MOD_REFERENCE modifier Gwendal Grignou
` (3 more replies)
0 siblings, 4 replies; 14+ messages in thread
From: Gwendal Grignou @ 2021-12-13 2:40 UTC (permalink / raw)
To: robh+dt, jic23, lars, swboyd
Cc: andy.shevchenko, linux-iio, devicetree, Gwendal Grignou
Add a new Semtech SAR sensor SX9360.
It is a simpler version of SX9324, with only one phase instead of 4.
Leverage sx_common.h interface defined when SX9324 was added.
Major changes since v2:
- Register "modifier" as a new modifier.
- Fix device tree binding syntax
- Fix issues reported during sx9324 driver review:
- fix include with iwyu
-
Major changes since v1:
- Integrate errors found in sx9324 driver.
- Simplify whomai function.
- Fix cut and paste in bindings.
- Handle negative sysfs parameters.
Gwendal Grignou (3):
Gwendal Grignou (4):
iio: add IIO_MOD_REFERENCE modifier
iio: proximity: Add sx9360 support
dt-bindings: iio: Add sx9360 binding
iio: sx9360: Add dt-binding support
.../iio/proximity/semtech,sx9360.yaml | 89 ++
drivers/iio/industrialio-core.c | 1 +
drivers/iio/proximity/Kconfig | 14 +
drivers/iio/proximity/Makefile | 1 +
drivers/iio/proximity/sx9360.c | 891 ++++++++++++++++++
include/uapi/linux/iio/types.h | 1 +
6 files changed, 997 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/proximity/semtech,sx9360.yaml
create mode 100644 drivers/iio/proximity/sx9360.c
--
2.34.1.173.g76aa8bc2d0-goog
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v3 1/4] iio: add IIO_MOD_REFERENCE modifier
2021-12-13 2:40 [PATCH v3 0/4] Add Semtech SX9360 SAR Sensor support Gwendal Grignou
@ 2021-12-13 2:40 ` Gwendal Grignou
2021-12-16 15:59 ` Jonathan Cameron
2021-12-13 2:40 ` [PATCH v3 2/4] iio: proximity: Add sx9360 support Gwendal Grignou
` (2 subsequent siblings)
3 siblings, 1 reply; 14+ messages in thread
From: Gwendal Grignou @ 2021-12-13 2:40 UTC (permalink / raw)
To: robh+dt, jic23, lars, swboyd
Cc: andy.shevchenko, linux-iio, devicetree, Gwendal Grignou
Add modifier IIO_MOD_REFERENCE for reporting sx9360 reference
proximity measurement.
All modifier must be defined for libiio to recognize
|in_proximity_reference| as a channel.
Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
---
New in v3.
drivers/iio/industrialio-core.c | 1 +
include/uapi/linux/iio/types.h | 1 +
2 files changed, 2 insertions(+)
diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index 20d5178ca0739a..2b272f54de8ae9 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -134,6 +134,7 @@ static const char * const iio_modifier_names[] = {
[IIO_MOD_ETHANOL] = "ethanol",
[IIO_MOD_H2] = "h2",
[IIO_MOD_O2] = "o2",
+ [IIO_MOD_REFERENCE] = "reference",
};
/* relies on pairs of these shared then separate */
diff --git a/include/uapi/linux/iio/types.h b/include/uapi/linux/iio/types.h
index 48c13147c0a870..aa83a9b578502a 100644
--- a/include/uapi/linux/iio/types.h
+++ b/include/uapi/linux/iio/types.h
@@ -95,6 +95,7 @@ enum iio_modifier {
IIO_MOD_ETHANOL,
IIO_MOD_H2,
IIO_MOD_O2,
+ IIO_MOD_REFERENCE,
};
enum iio_event_type {
--
2.34.1.173.g76aa8bc2d0-goog
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v3 2/4] iio: proximity: Add sx9360 support
2021-12-13 2:40 [PATCH v3 0/4] Add Semtech SX9360 SAR Sensor support Gwendal Grignou
2021-12-13 2:40 ` [PATCH v3 1/4] iio: add IIO_MOD_REFERENCE modifier Gwendal Grignou
@ 2021-12-13 2:40 ` Gwendal Grignou
2021-12-15 1:14 ` Stephen Boyd
2021-12-16 16:21 ` Jonathan Cameron
2021-12-13 2:40 ` [PATCH v3 3/4] dt-bindings: iio: Add sx9360 binding Gwendal Grignou
2021-12-13 2:40 ` [PATCH v3 4/4] iio: sx9360: Add dt-binding support Gwendal Grignou
3 siblings, 2 replies; 14+ messages in thread
From: Gwendal Grignou @ 2021-12-13 2:40 UTC (permalink / raw)
To: robh+dt, jic23, lars, swboyd
Cc: andy.shevchenko, linux-iio, devicetree, Gwendal Grignou
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@chromium.org>
---
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 | 807 +++++++++++++++++++++++++++++++++
3 files changed, 822 insertions(+)
create mode 100644 drivers/iio/proximity/sx9360.c
diff --git a/drivers/iio/proximity/Kconfig b/drivers/iio/proximity/Kconfig
index aaddf97f9b2192..801926e55eb6d3 100644
--- a/drivers/iio/proximity/Kconfig
+++ b/drivers/iio/proximity/Kconfig
@@ -143,6 +143,20 @@ config SX9324
To compile this driver as a module, choose M here: the
module will be called sx9324.
+config SX9360
+ tristate "SX9360 Semtech proximity sensor"
+ select IIO_BUFFER
+ select IIO_TRIGGERED_BUFFER
+ select REGMAP_I2C
+ select SX_COMMON
+ depends on I2C
+ help
+ Say Y here to build a driver for Semtech's SX9360
+ proximity/button sensor, a simplified SX9324.
+
+ To compile this driver as a module, choose M here: the
+ module will be called sx9360.
+
config SX9500
tristate "SX9500 Semtech proximity sensor"
select IIO_BUFFER
diff --git a/drivers/iio/proximity/Makefile b/drivers/iio/proximity/Makefile
index cffe962b352718..cc838bb5408a89 100644
--- a/drivers/iio/proximity/Makefile
+++ b/drivers/iio/proximity/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_SRF04) += srf04.o
obj-$(CONFIG_SRF08) += srf08.o
obj-$(CONFIG_SX9310) += sx9310.o
obj-$(CONFIG_SX9324) += sx9324.o
+obj-$(CONFIG_SX9360) += sx9360.o
obj-$(CONFIG_SX_COMMON) += sx_common.o
obj-$(CONFIG_SX9500) += sx9500.o
obj-$(CONFIG_VCNL3020) += vcnl3020.o
diff --git a/drivers/iio/proximity/sx9360.c b/drivers/iio/proximity/sx9360.c
new file mode 100644
index 00000000000000..aebfbe541e0e04
--- /dev/null
+++ b/drivers/iio/proximity/sx9360.c
@@ -0,0 +1,807 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright 2021 Google LLC.
+ *
+ * Driver for Semtech's SX9360 capacitive proximity/button solution.
+ * Based on SX9360 driver and copy of datasheet at:
+ * https://edit.wpgdadawant.com/uploads/news_file/program/2019/30184/tech_files/program_30184_suggest_other_file.pdf
+ */
+
+#include <linux/acpi.h>
+#include <linux/bits.h>
+#include <linux/bitfield.h>
+#include <linux/delay.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/log2.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/pm.h>
+#include <linux/regmap.h>
+
+#include <linux/iio/iio.h>
+
+#include "sx_common.h"
+
+/* Nominal Oscillator Frequency. */
+#define SX9360_FOSC_MHZ 4
+#define SX9360_FOSC_HZ (SX9360_FOSC_MHZ * 1000000)
+
+/* Register definitions. */
+#define SX9360_REG_IRQ_SRC SX_COMMON_REG_IRQ_SRC
+#define SX9360_REG_STAT 0x01
+#define SX9360_REG_STAT_COMPSTAT_MASK GENMASK(2, 1)
+#define SX9360_REG_IRQ_MSK 0x02
+#define SX9360_CONVDONE_IRQ BIT(0)
+#define SX9360_FAR_IRQ BIT(2)
+#define SX9360_CLOSE_IRQ BIT(3)
+#define SX9360_REG_IRQ_CFG 0x03
+
+#define SX9360_REG_GNRL_CTRL0 0x10
+#define SX9360_REG_GNRL_CTRL0_PHEN_MASK GENMASK(1, 0)
+#define SX9360_REG_GNRL_CTRL1 0x11
+#define SX9360_REG_GNRL_CTRL1_SCANPERIOD_MASK GENMASK(2, 0)
+#define SX9360_REG_GNRL_CTRL2 0x12
+#define SX9360_REG_GNRL_CTRL2_PERIOD_102MS 0x32
+#define SX9360_REG_GNRL_REG_2_PERIOD_MS(_r) \
+ (((_r) * 8192) / (SX9360_FOSC_HZ / 1000))
+#define SX9360_REG_GNRL_FREQ_2_REG(_f) (((_f) * 8192) / SX9360_FOSC_HZ)
+#define SX9360_REG_GNRL_REG_2_FREQ(_r) (SX9360_FOSC_HZ / ((_r) * 8192))
+
+#define SX9360_REG_AFE_CTRL1 0x21
+#define SX9360_REG_AFE_PARAM0_PHR 0x22
+#define SX9360_REG_AFE_PARAM1_PHR 0x23
+#define SX9360_REG_AFE_PARAM0_PHM 0x24
+#define SX9360_REG_AFE_PARAM0_RSVD 0x08
+#define SX9360_REG_AFE_PARAM0_RESOLUTION_MASK GENMASK(2, 0)
+#define SX9360_REG_AFE_PARAM0_RESOLUTION_128 0x02
+#define SX9360_REG_AFE_PARAM1_PHM 0x25
+#define SX9360_REG_AFE_PARAM1_AGAIN_PHM_6PF 0x40
+#define SX9360_REG_AFE_PARAM1_FREQ_83_33HZ 0x06
+
+#define SX9360_REG_PROX_CTRL0_PHR 0x40
+#define SX9360_REG_PROX_CTRL0_PHM 0x41
+#define SX9360_REG_PROX_CTRL0_GAIN_MASK GENMASK(5, 3)
+#define SX9360_REG_PROX_CTRL0_GAIN_1 0x80
+#define SX9360_REG_PROX_CTRL0_RAWFILT_1P50 0x01
+#define SX9360_REG_PROX_CTRL1 0x42
+#define SX9360_REG_PROX_CTRL1_AVGNEG_THRESH_MASK GENMASK(5, 3)
+#define SX9360_REG_PROX_CTRL1_AVGNEG_THRESH_16K 0x20
+#define SX9360_REG_PROX_CTRL2 0x43
+#define SX9360_REG_PROX_CTRL2_AVGDEB_MASK GENMASK(7, 6)
+#define SX9360_REG_PROX_CTRL2_AVGDEB_2SAMPLES 0x40
+#define SX9360_REG_PROX_CTRL2_AVGPOS_THRESH_16K 0x20
+#define SX9360_REG_PROX_CTRL3 0x44
+#define SX9360_REG_PROX_CTRL3_AVGNEG_FILT_MASK GENMASK(5, 3)
+#define SX9360_REG_PROX_CTRL3_AVGNEG_FILT_2 0x08
+#define SX9360_REG_PROX_CTRL3_AVGPOS_FILT_MASK GENMASK(2, 0)
+#define SX9360_REG_PROX_CTRL3_AVGPOS_FILT_256 0x04
+#define SX9360_REG_PROX_CTRL4 0x45
+#define SX9360_REG_PROX_CTRL4_HYST_MASK GENMASK(5, 4)
+#define SX9360_REG_PROX_CTRL4_CLOSE_DEBOUNCE_MASK GENMASK(3, 2)
+#define SX9360_REG_PROX_CTRL4_FAR_DEBOUNCE_MASK GENMASK(1, 0)
+#define SX9360_REG_PROX_CTRL5 0x46
+#define SX9360_REG_PROX_CTRL5_PROXTHRESH_32 0x08
+
+#define SX9360_REG_REF_CORR0 0x60
+#define SX9360_REG_REF_CORR1 0x61
+
+#define SX9360_REG_USEFUL_PHR_MSB 0x90
+#define SX9360_REG_USEFUL_PHR_LSB 0x91
+
+#define SX9360_REG_OFFSET_PMR_MSB 0x92
+#define SX9360_REG_OFFSET_PMR_LSB 0x93
+
+#define SX9360_REG_USEFUL_PHM_MSB 0x94
+#define SX9360_REG_USEFUL_PHM_LSB 0x95
+
+#define SX9360_REG_AVG_PHM_MSB 0x96
+#define SX9360_REG_AVG_PHM_LSB 0x97
+
+#define SX9360_REG_DIFF_PHM_MSB 0x98
+#define SX9360_REG_DIFF_PHM_LSB 0x99
+
+#define SX9360_REG_OFFSET_PHM_MSB 0x9a
+#define SX9360_REG_OFFSET_PHM_LSB 0x9b
+
+#define SX9360_REG_USE_FILTER_MSB 0x9a
+#define SX9360_REG_USE_FILTER_LSB 0x9b
+
+#define SX9360_REG_RESET 0xcf
+/* Write this to REG_RESET to do a soft reset. */
+#define SX9360_SOFT_RESET 0xde
+
+#define SX9360_REG_WHOAMI 0xfa
+#define SX9360_WHOAMI_VALUE 0x60
+
+#define SX9360_REG_REVISION 0xfe
+
+/* 2 channels, Phase Reference and Measurement. */
+#define SX9360_NUM_CHANNELS 2
+
+static const struct iio_chan_spec sx9360_channels[] = {
+ {
+ .type = IIO_PROXIMITY,
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+ BIT(IIO_CHAN_INFO_HARDWAREGAIN),
+ .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
+ .info_mask_separate_available =
+ BIT(IIO_CHAN_INFO_HARDWAREGAIN),
+ .info_mask_shared_by_all_available =
+ BIT(IIO_CHAN_INFO_SAMP_FREQ),
+ .extend_name = "reference",
+ .address = SX9360_REG_USEFUL_PHR_MSB,
+ .channel = 0,
+ .scan_index = 0,
+ .scan_type = {
+ .sign = 's',
+ .realbits = 12,
+ .storagebits = 16,
+ .endianness = IIO_BE,
+ },
+ },
+ {
+ .type = IIO_PROXIMITY,
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+ BIT(IIO_CHAN_INFO_HARDWAREGAIN),
+ .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
+ .info_mask_separate_available =
+ BIT(IIO_CHAN_INFO_HARDWAREGAIN),
+ .info_mask_shared_by_all_available =
+ BIT(IIO_CHAN_INFO_SAMP_FREQ),
+ .address = SX9360_REG_USEFUL_PHM_MSB,
+ .event_spec = sx_common_events,
+ .num_event_specs = ARRAY_SIZE(sx_common_events),
+ .channel = 1,
+ .scan_index = 1,
+ .scan_type = {
+ .sign = 's',
+ .realbits = 12,
+ .storagebits = 16,
+ .endianness = IIO_BE,
+ },
+ },
+ IIO_CHAN_SOFT_TIMESTAMP(2),
+};
+
+/*
+ * 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
+ *
+ */
+static const struct {
+ int val;
+ int val2;
+} sx9360_samp_freq_interval[] = {
+ {0, 281250}, /* 4MHz / (8192 * 2047) */
+ {0, 281250},
+ {448, 281250}, /* 4MHz / 8192 */
+};
+
+static const struct regmap_range sx9360_writable_reg_ranges[] = {
+ /*
+ * To set COMPSTAT for compensation, even if datasheet says register is
+ * RO.
+ */
+ regmap_reg_range(SX9360_REG_STAT, SX9360_REG_IRQ_CFG),
+ regmap_reg_range(SX9360_REG_GNRL_CTRL0, SX9360_REG_GNRL_CTRL2),
+ regmap_reg_range(SX9360_REG_AFE_CTRL1, SX9360_REG_AFE_PARAM1_PHM),
+ regmap_reg_range(SX9360_REG_PROX_CTRL0_PHR, SX9360_REG_PROX_CTRL5),
+ regmap_reg_range(SX9360_REG_REF_CORR0, SX9360_REG_REF_CORR1),
+ regmap_reg_range(SX9360_REG_OFFSET_PMR_MSB, SX9360_REG_OFFSET_PMR_LSB),
+ regmap_reg_range(SX9360_REG_RESET, SX9360_REG_RESET),
+};
+
+static const struct regmap_access_table sx9360_writeable_regs = {
+ .yes_ranges = sx9360_writable_reg_ranges,
+ .n_yes_ranges = ARRAY_SIZE(sx9360_writable_reg_ranges),
+};
+
+/*
+ * All allocated registers are readable, so we just list unallocated
+ * ones.
+ */
+static const struct regmap_range sx9360_non_readable_reg_ranges[] = {
+ regmap_reg_range(SX9360_REG_IRQ_CFG + 1, SX9360_REG_GNRL_CTRL0 - 1),
+ regmap_reg_range(SX9360_REG_GNRL_CTRL2 + 1, SX9360_REG_AFE_CTRL1 - 1),
+ regmap_reg_range(SX9360_REG_AFE_PARAM1_PHM + 1,
+ SX9360_REG_PROX_CTRL0_PHR - 1),
+ regmap_reg_range(SX9360_REG_PROX_CTRL5 + 1, SX9360_REG_REF_CORR0 - 1),
+ regmap_reg_range(SX9360_REG_REF_CORR1 + 1,
+ SX9360_REG_USEFUL_PHR_MSB - 1),
+ regmap_reg_range(SX9360_REG_USE_FILTER_LSB + 1, SX9360_REG_RESET - 1),
+ regmap_reg_range(SX9360_REG_RESET + 1, SX9360_REG_WHOAMI - 1),
+ regmap_reg_range(SX9360_REG_WHOAMI + 1, SX9360_REG_REVISION - 1),
+};
+
+static const struct regmap_access_table sx9360_readable_regs = {
+ .no_ranges = sx9360_non_readable_reg_ranges,
+ .n_no_ranges = ARRAY_SIZE(sx9360_non_readable_reg_ranges),
+};
+
+static const struct regmap_range sx9360_volatile_reg_ranges[] = {
+ regmap_reg_range(SX9360_REG_IRQ_SRC, SX9360_REG_STAT),
+ regmap_reg_range(SX9360_REG_USEFUL_PHR_MSB, SX9360_REG_USE_FILTER_LSB),
+ regmap_reg_range(SX9360_REG_WHOAMI, SX9360_REG_WHOAMI),
+ regmap_reg_range(SX9360_REG_REVISION, SX9360_REG_REVISION),
+};
+
+static const struct regmap_access_table sx9360_volatile_regs = {
+ .yes_ranges = sx9360_volatile_reg_ranges,
+ .n_yes_ranges = ARRAY_SIZE(sx9360_volatile_reg_ranges),
+};
+
+static const struct regmap_config sx9360_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+
+ .max_register = SX9360_REG_REVISION,
+ .cache_type = REGCACHE_RBTREE,
+
+ .wr_table = &sx9360_writeable_regs,
+ .rd_table = &sx9360_readable_regs,
+ .volatile_table = &sx9360_volatile_regs,
+};
+
+static int sx9360_read_prox_data(struct sx_common_data *data,
+ const struct iio_chan_spec *chan,
+ __be16 *val)
+{
+ return regmap_bulk_read(data->regmap, chan->address, val, sizeof(*val));
+}
+
+/*
+ * If we have no interrupt support, we have to wait for a scan period
+ * after enabling a channel to get a result.
+ */
+static int sx9360_wait_for_sample(struct sx_common_data *data)
+{
+ int ret;
+ __be16 buf;
+
+ ret = regmap_bulk_read(data->regmap, SX9360_REG_GNRL_CTRL1,
+ &buf, sizeof(buf));
+ if (ret < 0)
+ return ret;
+ msleep(SX9360_REG_GNRL_REG_2_PERIOD_MS(be16_to_cpu(buf)));
+
+ return 0;
+}
+
+static int sx9360_read_gain(struct sx_common_data *data,
+ const struct iio_chan_spec *chan, int *val)
+{
+ unsigned int reg, regval;
+ int ret;
+
+ reg = SX9360_REG_PROX_CTRL0_PHR + chan->channel;
+ ret = regmap_read(data->regmap, reg, ®val);
+ if (ret)
+ return ret;
+
+ *val = 1 << FIELD_GET(SX9360_REG_PROX_CTRL0_GAIN_MASK, regval);
+
+ return IIO_VAL_INT;
+}
+
+static int sx9360_read_samp_freq(struct sx_common_data *data,
+ int *val, int *val2)
+{
+ int ret, divisor;
+ __be16 buf;
+
+ ret = regmap_bulk_read(data->regmap, SX9360_REG_GNRL_CTRL1,
+ &buf, sizeof(buf));
+ if (ret < 0)
+ return ret;
+ divisor = be16_to_cpu(buf);
+ if (divisor == 0) {
+ *val = 0;
+ return IIO_VAL_INT;
+ }
+
+ *val = SX9360_FOSC_HZ;
+ *val2 = be16_to_cpu(buf) * 8192;
+
+ return IIO_VAL_FRACTIONAL;
+}
+
+static int sx9360_read_raw(struct iio_dev *indio_dev,
+ const struct iio_chan_spec *chan,
+ int *val, int *val2, long mask)
+{
+ struct sx_common_data *data = iio_priv(indio_dev);
+ int ret;
+
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ ret = iio_device_claim_direct_mode(indio_dev);
+ if (ret)
+ return ret;
+
+ ret = sx_common_read_proximity(data, chan, val);
+ iio_device_release_direct_mode(indio_dev);
+ return ret;
+ case IIO_CHAN_INFO_HARDWAREGAIN:
+ ret = iio_device_claim_direct_mode(indio_dev);
+ if (ret)
+ return ret;
+
+ ret = sx9360_read_gain(data, chan, val);
+ iio_device_release_direct_mode(indio_dev);
+ return ret;
+ case IIO_CHAN_INFO_SAMP_FREQ:
+ return sx9360_read_samp_freq(data, val, val2);
+ default:
+ return -EINVAL;
+ }
+}
+
+static const int sx9360_gain_vals[] = { 1, 2, 4, 8 };
+
+static int sx9360_read_avail(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ const int **vals, int *type, int *length,
+ long mask)
+{
+ if (chan->type != IIO_PROXIMITY)
+ return -EINVAL;
+
+ switch (mask) {
+ case IIO_CHAN_INFO_HARDWAREGAIN:
+ *type = IIO_VAL_INT;
+ *length = ARRAY_SIZE(sx9360_gain_vals);
+ *vals = sx9360_gain_vals;
+ return IIO_AVAIL_LIST;
+ case IIO_CHAN_INFO_SAMP_FREQ:
+ *type = IIO_VAL_INT_PLUS_MICRO;
+ *length = ARRAY_SIZE(sx9360_samp_freq_interval) * 2;
+ *vals = (int *)sx9360_samp_freq_interval;
+ return IIO_AVAIL_RANGE;
+ }
+
+ return -EINVAL;
+}
+
+static int sx9360_set_samp_freq(struct sx_common_data *data,
+ int val, int val2)
+{
+ int ret, reg;
+ __be16 buf;
+
+ reg = val * 8192 / SX9360_FOSC_HZ + val2 * 8192 / (SX9360_FOSC_MHZ);
+ buf = cpu_to_be16(val);
+ mutex_lock(&data->mutex);
+
+ ret = regmap_bulk_write(data->regmap, SX9360_REG_GNRL_CTRL1, &buf,
+ sizeof(buf));
+
+ mutex_unlock(&data->mutex);
+
+ return ret;
+}
+
+static int sx9360_read_thresh(struct sx_common_data *data, int *val)
+{
+ unsigned int regval;
+ int ret;
+
+ ret = regmap_read(data->regmap, SX9360_REG_PROX_CTRL5, ®val);
+ if (ret)
+ return ret;
+
+ if (regval <= 1)
+ *val = regval;
+ else
+ *val = (regval * regval) / 2;
+
+ return IIO_VAL_INT;
+}
+
+static int sx9360_read_hysteresis(struct sx_common_data *data, int *val)
+{
+ unsigned int regval, pthresh;
+ int ret;
+
+ ret = sx9360_read_thresh(data, &pthresh);
+ if (ret < 0)
+ return ret;
+
+ ret = regmap_read(data->regmap, SX9360_REG_PROX_CTRL4, ®val);
+ if (ret)
+ return ret;
+
+ regval = FIELD_GET(SX9360_REG_PROX_CTRL4_HYST_MASK, regval);
+ if (!regval)
+ *val = 0;
+ else
+ *val = pthresh >> (5 - regval);
+
+ return IIO_VAL_INT;
+}
+
+static int sx9360_read_far_debounce(struct sx_common_data *data, int *val)
+{
+ unsigned int regval;
+ int ret;
+
+ ret = regmap_read(data->regmap, SX9360_REG_PROX_CTRL4, ®val);
+ if (ret)
+ return ret;
+
+ regval = FIELD_GET(SX9360_REG_PROX_CTRL4_FAR_DEBOUNCE_MASK, regval);
+ if (regval)
+ *val = 1 << regval;
+ else
+ *val = 0;
+
+ return IIO_VAL_INT;
+}
+
+static int sx9360_read_close_debounce(struct sx_common_data *data, int *val)
+{
+ unsigned int regval;
+ int ret;
+
+ ret = regmap_read(data->regmap, SX9360_REG_PROX_CTRL4, ®val);
+ if (ret)
+ return ret;
+
+ regval = FIELD_GET(SX9360_REG_PROX_CTRL4_CLOSE_DEBOUNCE_MASK, regval);
+ if (regval)
+ *val = 1 << regval;
+ else
+ *val = 0;
+
+ return IIO_VAL_INT;
+}
+
+static int sx9360_read_event_val(struct iio_dev *indio_dev,
+ const struct iio_chan_spec *chan,
+ enum iio_event_type type,
+ enum iio_event_direction dir,
+ enum iio_event_info info, int *val, int *val2)
+{
+ struct sx_common_data *data = iio_priv(indio_dev);
+
+ if (chan->type != IIO_PROXIMITY)
+ return -EINVAL;
+
+ switch (info) {
+ case IIO_EV_INFO_VALUE:
+ return sx9360_read_thresh(data, val);
+ case IIO_EV_INFO_PERIOD:
+ switch (dir) {
+ case IIO_EV_DIR_RISING:
+ return sx9360_read_far_debounce(data, val);
+ case IIO_EV_DIR_FALLING:
+ return sx9360_read_close_debounce(data, val);
+ default:
+ return -EINVAL;
+ }
+ case IIO_EV_INFO_HYSTERESIS:
+ return sx9360_read_hysteresis(data, val);
+ default:
+ return -EINVAL;
+ }
+}
+
+static int sx9360_write_thresh(struct sx_common_data *data, int _val)
+{
+ unsigned int val = _val;
+ int ret;
+
+ if (val >= 1)
+ val = int_sqrt(2 * val);
+
+ if (val > 0xff)
+ return -EINVAL;
+
+ mutex_lock(&data->mutex);
+ ret = regmap_write(data->regmap, SX9360_REG_PROX_CTRL5, val);
+ mutex_unlock(&data->mutex);
+
+ return ret;
+}
+
+static int sx9360_write_hysteresis(struct sx_common_data *data, int _val)
+{
+ unsigned int hyst, val = _val;
+ int ret, pthresh;
+
+ ret = sx9360_read_thresh(data, &pthresh);
+ if (ret < 0)
+ return ret;
+
+ if (val == 0)
+ hyst = 0;
+ else if (val >= pthresh >> 2)
+ hyst = 3;
+ else if (val >= pthresh >> 3)
+ hyst = 2;
+ else if (val >= pthresh >> 4)
+ hyst = 1;
+ else
+ return -EINVAL;
+
+ hyst = FIELD_PREP(SX9360_REG_PROX_CTRL4_HYST_MASK, hyst);
+ mutex_lock(&data->mutex);
+ ret = regmap_update_bits(data->regmap, SX9360_REG_PROX_CTRL4,
+ SX9360_REG_PROX_CTRL4_HYST_MASK, hyst);
+ mutex_unlock(&data->mutex);
+
+ return ret;
+}
+
+static int sx9360_write_far_debounce(struct sx_common_data *data, int _val)
+{
+ unsigned int regval, val = _val;
+ int ret;
+
+ if (val > 0)
+ val = ilog2(val);
+ if (!FIELD_FIT(SX9360_REG_PROX_CTRL4_FAR_DEBOUNCE_MASK, val))
+ return -EINVAL;
+
+ regval = FIELD_PREP(SX9360_REG_PROX_CTRL4_FAR_DEBOUNCE_MASK, val);
+
+ mutex_lock(&data->mutex);
+ ret = regmap_update_bits(data->regmap, SX9360_REG_PROX_CTRL4,
+ SX9360_REG_PROX_CTRL4_FAR_DEBOUNCE_MASK,
+ regval);
+ mutex_unlock(&data->mutex);
+
+ return ret;
+}
+
+static int sx9360_write_close_debounce(struct sx_common_data *data, int _val)
+{
+ unsigned int regval, val = _val;
+ int ret;
+
+ if (val > 0)
+ val = ilog2(val);
+ if (!FIELD_FIT(SX9360_REG_PROX_CTRL4_CLOSE_DEBOUNCE_MASK, val))
+ return -EINVAL;
+
+ regval = FIELD_PREP(SX9360_REG_PROX_CTRL4_CLOSE_DEBOUNCE_MASK, val);
+
+ mutex_lock(&data->mutex);
+ ret = regmap_update_bits(data->regmap, SX9360_REG_PROX_CTRL4,
+ SX9360_REG_PROX_CTRL4_CLOSE_DEBOUNCE_MASK,
+ regval);
+ mutex_unlock(&data->mutex);
+
+ return ret;
+}
+
+static int sx9360_write_event_val(struct iio_dev *indio_dev,
+ const struct iio_chan_spec *chan,
+ enum iio_event_type type,
+ enum iio_event_direction dir,
+ enum iio_event_info info, int val, int val2)
+{
+ struct sx_common_data *data = iio_priv(indio_dev);
+
+ if (chan->type != IIO_PROXIMITY)
+ return -EINVAL;
+
+ switch (info) {
+ case IIO_EV_INFO_VALUE:
+ return sx9360_write_thresh(data, val);
+ case IIO_EV_INFO_PERIOD:
+ switch (dir) {
+ case IIO_EV_DIR_RISING:
+ return sx9360_write_far_debounce(data, val);
+ case IIO_EV_DIR_FALLING:
+ return sx9360_write_close_debounce(data, val);
+ default:
+ return -EINVAL;
+ }
+ case IIO_EV_INFO_HYSTERESIS:
+ return sx9360_write_hysteresis(data, val);
+ default:
+ return -EINVAL;
+ }
+}
+
+static int sx9360_write_gain(struct sx_common_data *data,
+ const struct iio_chan_spec *chan, int val)
+{
+ unsigned int gain, reg;
+ int ret;
+
+ gain = ilog2(val);
+ reg = SX9360_REG_PROX_CTRL0_PHR + chan->channel;
+ gain = FIELD_PREP(SX9360_REG_PROX_CTRL0_GAIN_MASK, gain);
+
+ mutex_lock(&data->mutex);
+ ret = regmap_update_bits(data->regmap, reg,
+ SX9360_REG_PROX_CTRL0_GAIN_MASK,
+ gain);
+ mutex_unlock(&data->mutex);
+
+ return ret;
+}
+
+static int sx9360_write_raw(struct iio_dev *indio_dev,
+ const struct iio_chan_spec *chan, int val, int val2,
+ long mask)
+{
+ struct sx_common_data *data = iio_priv(indio_dev);
+
+ switch (mask) {
+ case IIO_CHAN_INFO_SAMP_FREQ:
+ return sx9360_set_samp_freq(data, val, val2);
+ case IIO_CHAN_INFO_HARDWAREGAIN:
+ return sx9360_write_gain(data, chan, val);
+ }
+
+ return -EINVAL;
+}
+
+/* Activate all channels and perform an initial compensation. */
+static int sx9360_init_compensation(struct iio_dev *indio_dev)
+{
+ struct sx_common_data *data = iio_priv(indio_dev);
+ unsigned int val;
+ int ret;
+
+ /* run the compensation phase on all channels */
+ ret = regmap_update_bits(data->regmap, SX9360_REG_STAT,
+ SX9360_REG_STAT_COMPSTAT_MASK,
+ SX9360_REG_STAT_COMPSTAT_MASK);
+ if (ret)
+ return ret;
+
+ ret = regmap_read_poll_timeout(data->regmap, SX9360_REG_STAT, val,
+ !(val & SX9360_REG_STAT_COMPSTAT_MASK),
+ 20000, 2000000);
+ if (ret == -ETIMEDOUT)
+ dev_err(&data->client->dev,
+ "initial compensation timed out: 0x%02x\n",
+ val);
+ return ret;
+}
+
+static int sx9360_check_whoami(struct device *dev,
+ struct iio_dev *indio_dev)
+{
+ /*
+ * Only one sensor for this driver. Assuming the device tree
+ * is correct, just set the sensor name.
+ */
+ indio_dev->name = "sx9360";
+ return 0;
+}
+
+static const struct sx_common_chip_info sx9360_chip_info = {
+ .reg_stat = SX9360_REG_STAT,
+ .reg_irq_msk = SX9360_REG_IRQ_MSK,
+ .reg_enable_chan = SX9360_REG_GNRL_CTRL0,
+ .reg_reset = SX9360_REG_RESET,
+
+ .mask_enable_chan = SX9360_REG_GNRL_CTRL0_PHEN_MASK,
+ .stat_offset = 3,
+ .num_channels = SX9360_NUM_CHANNELS,
+
+ .ops = {
+ .read_prox_data = sx9360_read_prox_data,
+ .check_whoami = sx9360_check_whoami,
+ .init_compensation = sx9360_init_compensation,
+ .wait_for_sample = sx9360_wait_for_sample,
+ },
+
+ .iio_channels = sx9360_channels,
+ .num_iio_channels = ARRAY_SIZE(sx9360_channels),
+ .iio_info = {
+ .read_raw = sx9360_read_raw,
+ .read_avail = sx9360_read_avail,
+ .read_event_value = sx9360_read_event_val,
+ .write_event_value = sx9360_write_event_val,
+ .write_raw = sx9360_write_raw,
+ .read_event_config = sx_common_read_event_config,
+ .write_event_config = sx_common_write_event_config,
+ },
+};
+
+static int sx9360_probe(struct i2c_client *client)
+{
+ return sx_common_probe(client, &sx9360_chip_info, &sx9360_regmap_config);
+}
+
+static int __maybe_unused sx9360_suspend(struct device *dev)
+{
+ struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
+ struct sx_common_data *data = iio_priv(indio_dev);
+ unsigned int regval;
+ int ret;
+
+ disable_irq_nosync(data->client->irq);
+
+ mutex_lock(&data->mutex);
+ ret = regmap_read(data->regmap, SX9360_REG_GNRL_CTRL0, ®val);
+
+ data->suspend_ctrl =
+ FIELD_GET(SX9360_REG_GNRL_CTRL0_PHEN_MASK, regval);
+
+ if (ret < 0)
+ goto out;
+
+ /* Disable all phases, send the device to sleep. */
+ ret = regmap_write(data->regmap, SX9360_REG_GNRL_CTRL0, 0);
+
+out:
+ mutex_unlock(&data->mutex);
+ return ret;
+}
+
+static int __maybe_unused sx9360_resume(struct device *dev)
+{
+ struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
+ struct sx_common_data *data = iio_priv(indio_dev);
+ int ret;
+
+ mutex_lock(&data->mutex);
+ ret = regmap_update_bits(data->regmap, SX9360_REG_GNRL_CTRL0,
+ SX9360_REG_GNRL_CTRL0_PHEN_MASK,
+ data->suspend_ctrl);
+ mutex_unlock(&data->mutex);
+ if (ret)
+ return ret;
+
+ enable_irq(data->client->irq);
+ return 0;
+}
+
+static const struct dev_pm_ops sx9360_pm_ops = {
+ SET_SYSTEM_SLEEP_PM_OPS(sx9360_suspend, sx9360_resume)
+};
+
+static const struct acpi_device_id sx9360_acpi_match[] = {
+ { "STH9360", SX9360_WHOAMI_VALUE},
+ { }
+};
+MODULE_DEVICE_TABLE(acpi, sx9360_acpi_match);
+
+static const struct of_device_id sx9360_of_match[] = {
+ { .compatible = "semtech,sx9360", (void *)SX9360_WHOAMI_VALUE},
+ { }
+};
+MODULE_DEVICE_TABLE(of, sx9360_of_match);
+
+static const struct i2c_device_id sx9360_id[] = {
+ {"sx9360", SX9360_WHOAMI_VALUE},
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, sx9360_id);
+
+static struct i2c_driver sx9360_driver = {
+ .driver = {
+ .name = "sx9360",
+ .acpi_match_table = sx9360_acpi_match,
+ .of_match_table = sx9360_of_match,
+ .pm = &sx9360_pm_ops,
+
+ /*
+ * Lots of i2c transfers in probe + over 200 ms waiting in
+ * sx9360_init_compensation() mean a slow probe; prefer async
+ * so we don't delay boot if we're builtin to the kernel.
+ */
+ .probe_type = PROBE_PREFER_ASYNCHRONOUS,
+ },
+ .probe_new = sx9360_probe,
+ .id_table = sx9360_id,
+};
+module_i2c_driver(sx9360_driver);
+
+MODULE_AUTHOR("Gwendal Grignou <gwendal@chromium.org>");
+MODULE_DESCRIPTION("Driver for Semtech SX9360 proximity sensor");
+MODULE_LICENSE("GPL v2");
--
2.34.1.173.g76aa8bc2d0-goog
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v3 3/4] dt-bindings: iio: Add sx9360 binding
2021-12-13 2:40 [PATCH v3 0/4] Add Semtech SX9360 SAR Sensor support Gwendal Grignou
2021-12-13 2:40 ` [PATCH v3 1/4] iio: add IIO_MOD_REFERENCE modifier Gwendal Grignou
2021-12-13 2:40 ` [PATCH v3 2/4] iio: proximity: Add sx9360 support Gwendal Grignou
@ 2021-12-13 2:40 ` Gwendal Grignou
2021-12-15 1:15 ` Stephen Boyd
2021-12-15 20:09 ` Rob Herring
2021-12-13 2:40 ` [PATCH v3 4/4] iio: sx9360: Add dt-binding support Gwendal Grignou
3 siblings, 2 replies; 14+ messages in thread
From: Gwendal Grignou @ 2021-12-13 2:40 UTC (permalink / raw)
To: robh+dt, jic23, lars, swboyd
Cc: andy.shevchenko, linux-iio, devicetree, Gwendal Grignou
Add binding to configure Semtech sx9360 sensor.
It is a simpler version of sx9324.
Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
---
Changes since v2:
- Use const instead of single enum.
- Use proper syntax for maximum/minimum
- Fix spelling errors.
Changes since v1:
- Fix cut and paste error.
- Add . at end of sentence.
.../iio/proximity/semtech,sx9360.yaml | 89 +++++++++++++++++++
1 file changed, 89 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/proximity/semtech,sx9360.yaml
diff --git a/Documentation/devicetree/bindings/iio/proximity/semtech,sx9360.yaml b/Documentation/devicetree/bindings/iio/proximity/semtech,sx9360.yaml
new file mode 100644
index 00000000000000..63e1a1fd00d4ca
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/proximity/semtech,sx9360.yaml
@@ -0,0 +1,89 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/proximity/semtech,sx9360.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Semtech's SX9360 capacitive proximity sensor
+
+maintainers:
+ - Gwendal Grignou <gwendal@chromium.org>
+ - Daniel Campello <campello@chromium.org>
+
+description: |
+ Semtech's SX9360 proximity sensor.
+
+properties:
+ compatible:
+ const: semtech,sx9360
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ description:
+ Generated by device to announce preceding read request has finished
+ and data is available or that a close/far proximity event has happened.
+ maxItems: 1
+
+ vdd-supply:
+ description: Main power supply
+
+ svdd-supply:
+ description: Host interface power supply
+
+ "#io-channel-cells":
+ const: 1
+
+ semtech,resolution:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ enum: [8, 16, 32, 64, 128, 256, 512, 1024]
+ description:
+ Capacitance measurement resolution. For both phases, "reference" and
+ "measurement". Higher the number, higher the resolution.
+ default: 128
+
+ semtech,proxraw-strength:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 0
+ maximum: 7
+ default: 1
+ description:
+ PROXRAW filter strength for both phases. A value of 0 represents off,
+ and other values represent 1-1/2^N.
+
+ semtech,avg-pos-strength:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ enum: [0, 16, 64, 128, 256, 512, 1024, 4294967295]
+ default: 16
+ description: |
+ Average positive filter strength. A value of 0 represents off and
+ UINT_MAX (4294967295) represents infinite. Other values
+ represent 1-1/N.
+
+required:
+ - compatible
+ - reg
+ - "#io-channel-cells"
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ proximity@28 {
+ compatible = "semtech,sx9360";
+ reg = <0x28>;
+ interrupt-parent = <&pio>;
+ interrupts = <5 IRQ_TYPE_LEVEL_LOW 5>;
+ vdd-supply = <&pp3300_a>;
+ svdd-supply = <&pp1800_prox>;
+ #io-channel-cells = <1>;
+ semtech,resolution = <256>;
+ semtech,proxraw-strength = <2>;
+ semtech,avg-pos-strength = <64>;
+ };
+ };
--
2.34.1.173.g76aa8bc2d0-goog
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v3 4/4] iio: sx9360: Add dt-binding support
2021-12-13 2:40 [PATCH v3 0/4] Add Semtech SX9360 SAR Sensor support Gwendal Grignou
` (2 preceding siblings ...)
2021-12-13 2:40 ` [PATCH v3 3/4] dt-bindings: iio: Add sx9360 binding Gwendal Grignou
@ 2021-12-13 2:40 ` Gwendal Grignou
2021-12-15 1:16 ` Stephen Boyd
2021-12-16 16:23 ` Jonathan Cameron
3 siblings, 2 replies; 14+ messages in thread
From: Gwendal Grignou @ 2021-12-13 2:40 UTC (permalink / raw)
To: robh+dt, jic23, lars, swboyd
Cc: andy.shevchenko, linux-iio, devicetree, Gwendal Grignou
Add support to configure sx9360 from dt-binding, to match device
hardware setup.
Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
---
Changes since v2:
- Add include when needed.
- Move default register constant to main patch.
No changes in v2.
drivers/iio/proximity/sx9360.c | 85 +++++++++++++++++++++++++++++++++-
1 file changed, 84 insertions(+), 1 deletion(-)
diff --git a/drivers/iio/proximity/sx9360.c b/drivers/iio/proximity/sx9360.c
index aebfbe541e0e04..30cc6549d301e6 100644
--- a/drivers/iio/proximity/sx9360.c
+++ b/drivers/iio/proximity/sx9360.c
@@ -18,6 +18,7 @@
#include <linux/mod_devicetable.h>
#include <linux/module.h>
#include <linux/pm.h>
+#include <linux/property.h>
#include <linux/regmap.h>
#include <linux/iio/iio.h>
@@ -64,6 +65,7 @@
#define SX9360_REG_PROX_CTRL0_PHM 0x41
#define SX9360_REG_PROX_CTRL0_GAIN_MASK GENMASK(5, 3)
#define SX9360_REG_PROX_CTRL0_GAIN_1 0x80
+#define SX9360_REG_PROX_CTRL0_RAWFILT_MASK GENMASK(2, 0)
#define SX9360_REG_PROX_CTRL0_RAWFILT_1P50 0x01
#define SX9360_REG_PROX_CTRL1 0x42
#define SX9360_REG_PROX_CTRL1_AVGNEG_THRESH_MASK GENMASK(5, 3)
@@ -647,6 +649,41 @@ static int sx9360_write_raw(struct iio_dev *indio_dev,
return -EINVAL;
}
+static const struct sx_common_reg_default sx9360_default_regs[] = {
+ { SX9360_REG_IRQ_MSK, 0x00 },
+ { SX9360_REG_IRQ_CFG, 0x00 },
+ /*
+ * The lower 2 bits should not be set as it enable sensors measurements.
+ * Turning the detection on before the configuration values are set to
+ * good values can cause the device to return erroneous readings.
+ */
+ { SX9360_REG_GNRL_CTRL0, 0x00 },
+ { SX9360_REG_GNRL_CTRL1, 0x00 },
+ { SX9360_REG_GNRL_CTRL2, SX9360_REG_GNRL_CTRL2_PERIOD_102MS },
+
+ { SX9360_REG_AFE_CTRL1, 0x00 },
+ { SX9360_REG_AFE_PARAM0_PHR, SX9360_REG_AFE_PARAM0_RSVD |
+ SX9360_REG_AFE_PARAM0_RESOLUTION_128 },
+ { SX9360_REG_AFE_PARAM1_PHR, SX9360_REG_AFE_PARAM1_AGAIN_PHM_6PF |
+ SX9360_REG_AFE_PARAM1_FREQ_83_33HZ },
+ { SX9360_REG_AFE_PARAM0_PHM, SX9360_REG_AFE_PARAM0_RSVD |
+ SX9360_REG_AFE_PARAM0_RESOLUTION_128 },
+ { SX9360_REG_AFE_PARAM1_PHM, SX9360_REG_AFE_PARAM1_AGAIN_PHM_6PF |
+ SX9360_REG_AFE_PARAM1_FREQ_83_33HZ },
+
+ { SX9360_REG_PROX_CTRL0_PHR, SX9360_REG_PROX_CTRL0_GAIN_1 |
+ SX9360_REG_PROX_CTRL0_RAWFILT_1P50 },
+ { SX9360_REG_PROX_CTRL0_PHM, SX9360_REG_PROX_CTRL0_GAIN_1 |
+ SX9360_REG_PROX_CTRL0_RAWFILT_1P50 },
+ { SX9360_REG_PROX_CTRL1, SX9360_REG_PROX_CTRL1_AVGNEG_THRESH_16K },
+ { SX9360_REG_PROX_CTRL2, SX9360_REG_PROX_CTRL2_AVGDEB_2SAMPLES |
+ SX9360_REG_PROX_CTRL2_AVGPOS_THRESH_16K },
+ { SX9360_REG_PROX_CTRL3, SX9360_REG_PROX_CTRL3_AVGNEG_FILT_2 |
+ SX9360_REG_PROX_CTRL3_AVGPOS_FILT_256 },
+ { SX9360_REG_PROX_CTRL4, 0x00 },
+ { SX9360_REG_PROX_CTRL5, SX9360_REG_PROX_CTRL5_PROXTHRESH_32 },
+};
+
/* Activate all channels and perform an initial compensation. */
static int sx9360_init_compensation(struct iio_dev *indio_dev)
{
@@ -671,6 +708,51 @@ static int sx9360_init_compensation(struct iio_dev *indio_dev)
return ret;
}
+static const struct sx_common_reg_default *
+sx9360_get_default_reg(struct device *dev, int idx,
+ struct sx_common_reg_default *reg_def)
+{
+ u32 raw = 0, pos = 0;
+ int ret;
+
+ memcpy(reg_def, &sx9360_default_regs[idx], sizeof(*reg_def));
+ switch (reg_def->reg) {
+ case SX9360_REG_AFE_PARAM0_PHR:
+ case SX9360_REG_AFE_PARAM0_PHM:
+ ret = device_property_read_u32(dev, "semtech,resolution", &raw);
+ if (ret)
+ break;
+
+ raw = ilog2(raw) - 3;
+
+ reg_def->def &= ~SX9360_REG_AFE_PARAM0_RESOLUTION_MASK;
+ reg_def->def |= FIELD_PREP(SX9360_REG_AFE_PARAM0_RESOLUTION_MASK, raw);
+ break;
+ case SX9360_REG_PROX_CTRL0_PHR:
+ case SX9360_REG_PROX_CTRL0_PHM:
+ ret = device_property_read_u32(dev, "semtech,proxraw-strength", &raw);
+ if (ret)
+ break;
+
+ reg_def->def &= ~SX9360_REG_PROX_CTRL0_RAWFILT_MASK;
+ reg_def->def |= FIELD_PREP(SX9360_REG_PROX_CTRL0_RAWFILT_MASK, raw);
+ break;
+ case SX9360_REG_PROX_CTRL3:
+ ret = device_property_read_u32(dev, "semtech,avg-pos-strength",
+ &pos);
+ if (ret)
+ break;
+
+ /* Powers of 2, except for a gap between 16 and 64 */
+ raw = clamp(ilog2(pos), 3, 11) - (pos >= 32 ? 4 : 3);
+ reg_def->def &= ~SX9360_REG_PROX_CTRL3_AVGPOS_FILT_MASK;
+ reg_def->def |= FIELD_PREP(SX9360_REG_PROX_CTRL3_AVGPOS_FILT_MASK, raw);
+ break;
+ }
+
+ return reg_def;
+}
+
static int sx9360_check_whoami(struct device *dev,
struct iio_dev *indio_dev)
{
@@ -691,12 +773,14 @@ static const struct sx_common_chip_info sx9360_chip_info = {
.mask_enable_chan = SX9360_REG_GNRL_CTRL0_PHEN_MASK,
.stat_offset = 3,
.num_channels = SX9360_NUM_CHANNELS,
+ .num_default_regs = ARRAY_SIZE(sx9360_default_regs),
.ops = {
.read_prox_data = sx9360_read_prox_data,
.check_whoami = sx9360_check_whoami,
.init_compensation = sx9360_init_compensation,
.wait_for_sample = sx9360_wait_for_sample,
+ .get_default_reg = sx9360_get_default_reg,
},
.iio_channels = sx9360_channels,
--
2.34.1.173.g76aa8bc2d0-goog
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v3 2/4] iio: proximity: Add sx9360 support
2021-12-13 2:40 ` [PATCH v3 2/4] iio: proximity: Add sx9360 support Gwendal Grignou
@ 2021-12-15 1:14 ` Stephen Boyd
2021-12-16 16:21 ` Jonathan Cameron
1 sibling, 0 replies; 14+ messages in thread
From: Stephen Boyd @ 2021-12-15 1:14 UTC (permalink / raw)
To: Gwendal Grignou, jic23, lars, robh+dt
Cc: andy.shevchenko, linux-iio, devicetree
Quoting Gwendal Grignou (2021-12-12 18:40:55)
> 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@chromium.org>
> ---
Reviewed-by: Stephen Boyd <swboyd@chromium.org>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 3/4] dt-bindings: iio: Add sx9360 binding
2021-12-13 2:40 ` [PATCH v3 3/4] dt-bindings: iio: Add sx9360 binding Gwendal Grignou
@ 2021-12-15 1:15 ` Stephen Boyd
2021-12-15 20:09 ` Rob Herring
1 sibling, 0 replies; 14+ messages in thread
From: Stephen Boyd @ 2021-12-15 1:15 UTC (permalink / raw)
To: Gwendal Grignou, jic23, lars, robh+dt
Cc: andy.shevchenko, linux-iio, devicetree
Quoting Gwendal Grignou (2021-12-12 18:40:56)
> Add binding to configure Semtech sx9360 sensor.
> It is a simpler version of sx9324.
>
> Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
> ---
Reviewed-by: Stephen Boyd <swboyd@chromium.org>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 4/4] iio: sx9360: Add dt-binding support
2021-12-13 2:40 ` [PATCH v3 4/4] iio: sx9360: Add dt-binding support Gwendal Grignou
@ 2021-12-15 1:16 ` Stephen Boyd
2021-12-16 16:23 ` Jonathan Cameron
1 sibling, 0 replies; 14+ messages in thread
From: Stephen Boyd @ 2021-12-15 1:16 UTC (permalink / raw)
To: Gwendal Grignou, jic23, lars, robh+dt
Cc: andy.shevchenko, linux-iio, devicetree
Quoting Gwendal Grignou (2021-12-12 18:40:57)
> Add support to configure sx9360 from dt-binding, to match device
> hardware setup.
>
> Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
> ---
Reviewed-by: Stephen Boyd <swboyd@chromium.org>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 3/4] dt-bindings: iio: Add sx9360 binding
2021-12-13 2:40 ` [PATCH v3 3/4] dt-bindings: iio: Add sx9360 binding Gwendal Grignou
2021-12-15 1:15 ` Stephen Boyd
@ 2021-12-15 20:09 ` Rob Herring
1 sibling, 0 replies; 14+ messages in thread
From: Rob Herring @ 2021-12-15 20:09 UTC (permalink / raw)
To: Gwendal Grignou
Cc: robh+dt, lars, devicetree, linux-iio, andy.shevchenko, swboyd, jic23
On Sun, 12 Dec 2021 18:40:56 -0800, Gwendal Grignou wrote:
> Add binding to configure Semtech sx9360 sensor.
> It is a simpler version of sx9324.
>
> Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
> ---
> Changes since v2:
> - Use const instead of single enum.
> - Use proper syntax for maximum/minimum
> - Fix spelling errors.
>
> Changes since v1:
> - Fix cut and paste error.
> - Add . at end of sentence.
>
> .../iio/proximity/semtech,sx9360.yaml | 89 +++++++++++++++++++
> 1 file changed, 89 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/proximity/semtech,sx9360.yaml
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 1/4] iio: add IIO_MOD_REFERENCE modifier
2021-12-13 2:40 ` [PATCH v3 1/4] iio: add IIO_MOD_REFERENCE modifier Gwendal Grignou
@ 2021-12-16 15:59 ` Jonathan Cameron
2021-12-17 20:24 ` Gwendal Grignou
0 siblings, 1 reply; 14+ messages in thread
From: Jonathan Cameron @ 2021-12-16 15:59 UTC (permalink / raw)
To: Gwendal Grignou
Cc: robh+dt, lars, swboyd, andy.shevchenko, linux-iio, devicetree
On Sun, 12 Dec 2021 18:40:54 -0800
Gwendal Grignou <gwendal@chromium.org> wrote:
> Add modifier IIO_MOD_REFERENCE for reporting sx9360 reference
> proximity measurement.
> All modifier must be defined for libiio to recognize
> |in_proximity_reference| as a channel.
>
> Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
Hmm. So the question is whether this is a valid modifier.
I'm not totally convinced, because I can see we might well
get stacking cases say
iio_concentration_o2_reference
However we do have precedence with 'ambient' which applies
to temperature sensors.
The alternative here would be to have it as a normal indexed
channel but with a label saying it is the reference.
Would that work for this case? If I were doing the ambient
case again I'd use label for that as well, but label is a more
recent addition to the ABI.
> ---
> New in v3.
>
> drivers/iio/industrialio-core.c | 1 +
> include/uapi/linux/iio/types.h | 1 +
> 2 files changed, 2 insertions(+)
>
> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index 20d5178ca0739a..2b272f54de8ae9 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -134,6 +134,7 @@ static const char * const iio_modifier_names[] = {
> [IIO_MOD_ETHANOL] = "ethanol",
> [IIO_MOD_H2] = "h2",
> [IIO_MOD_O2] = "o2",
> + [IIO_MOD_REFERENCE] = "reference",
> };
>
> /* relies on pairs of these shared then separate */
> diff --git a/include/uapi/linux/iio/types.h b/include/uapi/linux/iio/types.h
> index 48c13147c0a870..aa83a9b578502a 100644
> --- a/include/uapi/linux/iio/types.h
> +++ b/include/uapi/linux/iio/types.h
> @@ -95,6 +95,7 @@ enum iio_modifier {
> IIO_MOD_ETHANOL,
> IIO_MOD_H2,
> IIO_MOD_O2,
> + IIO_MOD_REFERENCE,
> };
>
> enum iio_event_type {
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 2/4] iio: proximity: Add sx9360 support
2021-12-13 2:40 ` [PATCH v3 2/4] iio: proximity: Add sx9360 support Gwendal Grignou
2021-12-15 1:14 ` Stephen Boyd
@ 2021-12-16 16:21 ` Jonathan Cameron
2021-12-18 0:27 ` Gwendal Grignou
1 sibling, 1 reply; 14+ messages in thread
From: Jonathan Cameron @ 2021-12-16 16:21 UTC (permalink / raw)
To: Gwendal Grignou
Cc: robh+dt, lars, swboyd, andy.shevchenko, linux-iio, devicetree
On Sun, 12 Dec 2021 18:40:55 -0800
Gwendal Grignou <gwendal@chromium.org> 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@chromium.org>
You don't use the modifier defined in the previous
patch...
> ---
> 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 | 807 +++++++++++++++++++++++++++++++++
> 3 files changed, 822 insertions(+)
> create mode 100644 drivers/iio/proximity/sx9360.c
>
....
> +static const struct iio_chan_spec sx9360_channels[] = {
> + {
> + .type = IIO_PROXIMITY,
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> + BIT(IIO_CHAN_INFO_HARDWAREGAIN),
> + .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
> + .info_mask_separate_available =
> + BIT(IIO_CHAN_INFO_HARDWAREGAIN),
> + .info_mask_shared_by_all_available =
> + BIT(IIO_CHAN_INFO_SAMP_FREQ),
> + .extend_name = "reference",
You defined the modifier for this and then didn't use it?
I've suggested in review of patch 1 you might want to use label though
via the read_label() callback.
> + .address = SX9360_REG_USEFUL_PHR_MSB,
> + .channel = 0,
> + .scan_index = 0,
> + .scan_type = {
> + .sign = 's',
> + .realbits = 12,
> + .storagebits = 16,
> + .endianness = IIO_BE,
> + },
> + },
> + {
> + .type = IIO_PROXIMITY,
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> + BIT(IIO_CHAN_INFO_HARDWAREGAIN),
> + .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
> + .info_mask_separate_available =
> + BIT(IIO_CHAN_INFO_HARDWAREGAIN),
> + .info_mask_shared_by_all_available =
> + BIT(IIO_CHAN_INFO_SAMP_FREQ),
> + .address = SX9360_REG_USEFUL_PHM_MSB,
> + .event_spec = sx_common_events,
> + .num_event_specs = ARRAY_SIZE(sx_common_events),
> + .channel = 1,
> + .scan_index = 1,
> + .scan_type = {
> + .sign = 's',
> + .realbits = 12,
> + .storagebits = 16,
> + .endianness = IIO_BE,
> + },
> + },
> + IIO_CHAN_SOFT_TIMESTAMP(2),
> +};
> +
...
> +
> +static int sx9360_read_samp_freq(struct sx_common_data *data,
> + int *val, int *val2)
> +{
> + int ret, divisor;
> + __be16 buf;
> +
> + ret = regmap_bulk_read(data->regmap, SX9360_REG_GNRL_CTRL1,
> + &buf, sizeof(buf));
> + if (ret < 0)
> + return ret;
> + divisor = be16_to_cpu(buf);
> + if (divisor == 0) {
> + *val = 0;
> + return IIO_VAL_INT;
> + }
> +
> + *val = SX9360_FOSC_HZ;
> + *val2 = be16_to_cpu(buf) * 8192;
*val2 = divisor * 8192;?
> +
> + return IIO_VAL_FRACTIONAL;
> +}
> +
...
> +static int sx9360_write_raw(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan, int val, int val2,
> + long mask)
> +{
> + struct sx_common_data *data = iio_priv(indio_dev);
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + return sx9360_set_samp_freq(data, val, val2);
> + case IIO_CHAN_INFO_HARDWAREGAIN:
> + return sx9360_write_gain(data, chan, val);
> + }
> +
Slight preference for this as a default in the switch.
> + return -EINVAL;
> +}
...
> +static int sx9360_check_whoami(struct device *dev,
> + struct iio_dev *indio_dev)
Will fit on one line under 80 chars I think..
> +{
> + /*
> + * Only one sensor for this driver. Assuming the device tree
> + * is correct, just set the sensor name.
> + */
> + indio_dev->name = "sx9360";
> + return 0;
> +}
> +
> +
> +static int __maybe_unused sx9360_suspend(struct device *dev)
> +{
> + struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
I don't feel particularly strongly about this, as there are arguments
either way but this is the same as
struct iio_dev *indio_dev = dev_get_drvdata(dev);
> + struct sx_common_data *data = iio_priv(indio_dev);
> + unsigned int regval;
> + int ret;
> +
> + disable_irq_nosync(data->client->irq);
> +
> + mutex_lock(&data->mutex);
> + ret = regmap_read(data->regmap, SX9360_REG_GNRL_CTRL0, ®val);
> +
> + data->suspend_ctrl =
> + FIELD_GET(SX9360_REG_GNRL_CTRL0_PHEN_MASK, regval);
> +
> + if (ret < 0)
> + goto out;
> +
> + /* Disable all phases, send the device to sleep. */
> + ret = regmap_write(data->regmap, SX9360_REG_GNRL_CTRL0, 0);
> +
> +out:
> + mutex_unlock(&data->mutex);
> + return ret;
> +}
> +
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 4/4] iio: sx9360: Add dt-binding support
2021-12-13 2:40 ` [PATCH v3 4/4] iio: sx9360: Add dt-binding support Gwendal Grignou
2021-12-15 1:16 ` Stephen Boyd
@ 2021-12-16 16:23 ` Jonathan Cameron
1 sibling, 0 replies; 14+ messages in thread
From: Jonathan Cameron @ 2021-12-16 16:23 UTC (permalink / raw)
To: Gwendal Grignou
Cc: robh+dt, lars, swboyd, andy.shevchenko, linux-iio, devicetree
On Sun, 12 Dec 2021 18:40:57 -0800
Gwendal Grignou <gwendal@chromium.org> wrote:
> Add support to configure sx9360 from dt-binding, to match device
> hardware setup.
>
> Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
This and patch 3 look fine to me, so mostly just a question of
modifier vs label for the reference channel.
Thanks,
Jonathan
> ---
> Changes since v2:
> - Add include when needed.
> - Move default register constant to main patch.
>
> No changes in v2.
>
> drivers/iio/proximity/sx9360.c | 85 +++++++++++++++++++++++++++++++++-
> 1 file changed, 84 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iio/proximity/sx9360.c b/drivers/iio/proximity/sx9360.c
> index aebfbe541e0e04..30cc6549d301e6 100644
> --- a/drivers/iio/proximity/sx9360.c
> +++ b/drivers/iio/proximity/sx9360.c
> @@ -18,6 +18,7 @@
> #include <linux/mod_devicetable.h>
> #include <linux/module.h>
> #include <linux/pm.h>
> +#include <linux/property.h>
> #include <linux/regmap.h>
>
> #include <linux/iio/iio.h>
> @@ -64,6 +65,7 @@
> #define SX9360_REG_PROX_CTRL0_PHM 0x41
> #define SX9360_REG_PROX_CTRL0_GAIN_MASK GENMASK(5, 3)
> #define SX9360_REG_PROX_CTRL0_GAIN_1 0x80
> +#define SX9360_REG_PROX_CTRL0_RAWFILT_MASK GENMASK(2, 0)
> #define SX9360_REG_PROX_CTRL0_RAWFILT_1P50 0x01
> #define SX9360_REG_PROX_CTRL1 0x42
> #define SX9360_REG_PROX_CTRL1_AVGNEG_THRESH_MASK GENMASK(5, 3)
> @@ -647,6 +649,41 @@ static int sx9360_write_raw(struct iio_dev *indio_dev,
> return -EINVAL;
> }
>
> +static const struct sx_common_reg_default sx9360_default_regs[] = {
> + { SX9360_REG_IRQ_MSK, 0x00 },
> + { SX9360_REG_IRQ_CFG, 0x00 },
> + /*
> + * The lower 2 bits should not be set as it enable sensors measurements.
> + * Turning the detection on before the configuration values are set to
> + * good values can cause the device to return erroneous readings.
> + */
> + { SX9360_REG_GNRL_CTRL0, 0x00 },
> + { SX9360_REG_GNRL_CTRL1, 0x00 },
> + { SX9360_REG_GNRL_CTRL2, SX9360_REG_GNRL_CTRL2_PERIOD_102MS },
> +
> + { SX9360_REG_AFE_CTRL1, 0x00 },
> + { SX9360_REG_AFE_PARAM0_PHR, SX9360_REG_AFE_PARAM0_RSVD |
> + SX9360_REG_AFE_PARAM0_RESOLUTION_128 },
> + { SX9360_REG_AFE_PARAM1_PHR, SX9360_REG_AFE_PARAM1_AGAIN_PHM_6PF |
> + SX9360_REG_AFE_PARAM1_FREQ_83_33HZ },
> + { SX9360_REG_AFE_PARAM0_PHM, SX9360_REG_AFE_PARAM0_RSVD |
> + SX9360_REG_AFE_PARAM0_RESOLUTION_128 },
> + { SX9360_REG_AFE_PARAM1_PHM, SX9360_REG_AFE_PARAM1_AGAIN_PHM_6PF |
> + SX9360_REG_AFE_PARAM1_FREQ_83_33HZ },
> +
> + { SX9360_REG_PROX_CTRL0_PHR, SX9360_REG_PROX_CTRL0_GAIN_1 |
> + SX9360_REG_PROX_CTRL0_RAWFILT_1P50 },
> + { SX9360_REG_PROX_CTRL0_PHM, SX9360_REG_PROX_CTRL0_GAIN_1 |
> + SX9360_REG_PROX_CTRL0_RAWFILT_1P50 },
> + { SX9360_REG_PROX_CTRL1, SX9360_REG_PROX_CTRL1_AVGNEG_THRESH_16K },
> + { SX9360_REG_PROX_CTRL2, SX9360_REG_PROX_CTRL2_AVGDEB_2SAMPLES |
> + SX9360_REG_PROX_CTRL2_AVGPOS_THRESH_16K },
> + { SX9360_REG_PROX_CTRL3, SX9360_REG_PROX_CTRL3_AVGNEG_FILT_2 |
> + SX9360_REG_PROX_CTRL3_AVGPOS_FILT_256 },
> + { SX9360_REG_PROX_CTRL4, 0x00 },
> + { SX9360_REG_PROX_CTRL5, SX9360_REG_PROX_CTRL5_PROXTHRESH_32 },
> +};
> +
> /* Activate all channels and perform an initial compensation. */
> static int sx9360_init_compensation(struct iio_dev *indio_dev)
> {
> @@ -671,6 +708,51 @@ static int sx9360_init_compensation(struct iio_dev *indio_dev)
> return ret;
> }
>
> +static const struct sx_common_reg_default *
> +sx9360_get_default_reg(struct device *dev, int idx,
> + struct sx_common_reg_default *reg_def)
> +{
> + u32 raw = 0, pos = 0;
> + int ret;
> +
> + memcpy(reg_def, &sx9360_default_regs[idx], sizeof(*reg_def));
> + switch (reg_def->reg) {
> + case SX9360_REG_AFE_PARAM0_PHR:
> + case SX9360_REG_AFE_PARAM0_PHM:
> + ret = device_property_read_u32(dev, "semtech,resolution", &raw);
> + if (ret)
> + break;
> +
> + raw = ilog2(raw) - 3;
> +
> + reg_def->def &= ~SX9360_REG_AFE_PARAM0_RESOLUTION_MASK;
> + reg_def->def |= FIELD_PREP(SX9360_REG_AFE_PARAM0_RESOLUTION_MASK, raw);
> + break;
> + case SX9360_REG_PROX_CTRL0_PHR:
> + case SX9360_REG_PROX_CTRL0_PHM:
> + ret = device_property_read_u32(dev, "semtech,proxraw-strength", &raw);
> + if (ret)
> + break;
> +
> + reg_def->def &= ~SX9360_REG_PROX_CTRL0_RAWFILT_MASK;
> + reg_def->def |= FIELD_PREP(SX9360_REG_PROX_CTRL0_RAWFILT_MASK, raw);
> + break;
> + case SX9360_REG_PROX_CTRL3:
> + ret = device_property_read_u32(dev, "semtech,avg-pos-strength",
> + &pos);
> + if (ret)
> + break;
> +
> + /* Powers of 2, except for a gap between 16 and 64 */
> + raw = clamp(ilog2(pos), 3, 11) - (pos >= 32 ? 4 : 3);
> + reg_def->def &= ~SX9360_REG_PROX_CTRL3_AVGPOS_FILT_MASK;
> + reg_def->def |= FIELD_PREP(SX9360_REG_PROX_CTRL3_AVGPOS_FILT_MASK, raw);
> + break;
> + }
> +
> + return reg_def;
> +}
> +
> static int sx9360_check_whoami(struct device *dev,
> struct iio_dev *indio_dev)
> {
> @@ -691,12 +773,14 @@ static const struct sx_common_chip_info sx9360_chip_info = {
> .mask_enable_chan = SX9360_REG_GNRL_CTRL0_PHEN_MASK,
> .stat_offset = 3,
> .num_channels = SX9360_NUM_CHANNELS,
> + .num_default_regs = ARRAY_SIZE(sx9360_default_regs),
>
> .ops = {
> .read_prox_data = sx9360_read_prox_data,
> .check_whoami = sx9360_check_whoami,
> .init_compensation = sx9360_init_compensation,
> .wait_for_sample = sx9360_wait_for_sample,
> + .get_default_reg = sx9360_get_default_reg,
> },
>
> .iio_channels = sx9360_channels,
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 1/4] iio: add IIO_MOD_REFERENCE modifier
2021-12-16 15:59 ` Jonathan Cameron
@ 2021-12-17 20:24 ` Gwendal Grignou
0 siblings, 0 replies; 14+ messages in thread
From: Gwendal Grignou @ 2021-12-17 20:24 UTC (permalink / raw)
To: Jonathan Cameron
Cc: robh+dt, lars, swboyd, andy.shevchenko, linux-iio, devicetree
On Thu, Dec 16, 2021 at 7:54 AM Jonathan Cameron
<jic23@jic23.retrosnub.co.uk> wrote:
>
> On Sun, 12 Dec 2021 18:40:54 -0800
> Gwendal Grignou <gwendal@chromium.org> wrote:
>
> > Add modifier IIO_MOD_REFERENCE for reporting sx9360 reference
> > proximity measurement.
> > All modifier must be defined for libiio to recognize
> > |in_proximity_reference| as a channel.
> >
> > Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
> Hmm. So the question is whether this is a valid modifier.
>
> I'm not totally convinced, because I can see we might well
> get stacking cases say
>
> iio_concentration_o2_reference
>
> However we do have precedence with 'ambient' which applies
> to temperature sensors.
>
> The alternative here would be to have it as a normal indexed
> channel but with a label saying it is the reference.
>
> Would that work for this case? If I were doing the ambient
> case again I'd use label for that as well, but label is a more
> recent addition to the ABI.
That would work, I am submitting a v4 without IIO_MOD_REFERENCE.
Gwendal.
>
> > ---
> > New in v3.
> >
> > drivers/iio/industrialio-core.c | 1 +
> > include/uapi/linux/iio/types.h | 1 +
> > 2 files changed, 2 insertions(+)
> >
> > diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> > index 20d5178ca0739a..2b272f54de8ae9 100644
> > --- a/drivers/iio/industrialio-core.c
> > +++ b/drivers/iio/industrialio-core.c
> > @@ -134,6 +134,7 @@ static const char * const iio_modifier_names[] = {
> > [IIO_MOD_ETHANOL] = "ethanol",
> > [IIO_MOD_H2] = "h2",
> > [IIO_MOD_O2] = "o2",
> > + [IIO_MOD_REFERENCE] = "reference",
> > };
> >
> > /* relies on pairs of these shared then separate */
> > diff --git a/include/uapi/linux/iio/types.h b/include/uapi/linux/iio/types.h
> > index 48c13147c0a870..aa83a9b578502a 100644
> > --- a/include/uapi/linux/iio/types.h
> > +++ b/include/uapi/linux/iio/types.h
> > @@ -95,6 +95,7 @@ enum iio_modifier {
> > IIO_MOD_ETHANOL,
> > IIO_MOD_H2,
> > IIO_MOD_O2,
> > + IIO_MOD_REFERENCE,
> > };
> >
> > enum iio_event_type {
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 2/4] iio: proximity: Add sx9360 support
2021-12-16 16:21 ` Jonathan Cameron
@ 2021-12-18 0:27 ` Gwendal Grignou
0 siblings, 0 replies; 14+ messages in thread
From: Gwendal Grignou @ 2021-12-18 0:27 UTC (permalink / raw)
To: Jonathan Cameron
Cc: robh+dt, lars, swboyd, andy.shevchenko, linux-iio, devicetree
On Thu, Dec 16, 2021 at 8:16 AM Jonathan Cameron
<jic23@jic23.retrosnub.co.uk> wrote:
>
> On Sun, 12 Dec 2021 18:40:55 -0800
> Gwendal Grignou <gwendal@chromium.org> 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@chromium.org>
>
> You don't use the modifier defined in the previous
> patch...
>
> > ---
> > 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 | 807 +++++++++++++++++++++++++++++++++
> > 3 files changed, 822 insertions(+)
> > create mode 100644 drivers/iio/proximity/sx9360.c
> >
> ....
>
>
> > +static const struct iio_chan_spec sx9360_channels[] = {
> > + {
> > + .type = IIO_PROXIMITY,
> > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> > + BIT(IIO_CHAN_INFO_HARDWAREGAIN),
> > + .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
> > + .info_mask_separate_available =
> > + BIT(IIO_CHAN_INFO_HARDWAREGAIN),
> > + .info_mask_shared_by_all_available =
> > + BIT(IIO_CHAN_INFO_SAMP_FREQ),
> > + .extend_name = "reference",
>
> You defined the modifier for this and then didn't use it?
> I've suggested in review of patch 1 you might want to use label though
> via the read_label() callback.
I see, I should have used .cannel2 instead of extend_name. Using label now.
>
>
> > + .address = SX9360_REG_USEFUL_PHR_MSB,
> > + .channel = 0,
> > + .scan_index = 0,
> > + .scan_type = {
> > + .sign = 's',
> > + .realbits = 12,
> > + .storagebits = 16,
> > + .endianness = IIO_BE,
> > + },
> > + },
> > + {
> > + .type = IIO_PROXIMITY,
> > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> > + BIT(IIO_CHAN_INFO_HARDWAREGAIN),
> > + .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
> > + .info_mask_separate_available =
> > + BIT(IIO_CHAN_INFO_HARDWAREGAIN),
> > + .info_mask_shared_by_all_available =
> > + BIT(IIO_CHAN_INFO_SAMP_FREQ),
> > + .address = SX9360_REG_USEFUL_PHM_MSB,
> > + .event_spec = sx_common_events,
> > + .num_event_specs = ARRAY_SIZE(sx_common_events),
> > + .channel = 1,
> > + .scan_index = 1,
> > + .scan_type = {
> > + .sign = 's',
> > + .realbits = 12,
> > + .storagebits = 16,
> > + .endianness = IIO_BE,
> > + },
> > + },
> > + IIO_CHAN_SOFT_TIMESTAMP(2),
> > +};
> > +
>
> ...
>
> > +
> > +static int sx9360_read_samp_freq(struct sx_common_data *data,
> > + int *val, int *val2)
> > +{
> > + int ret, divisor;
> > + __be16 buf;
> > +
> > + ret = regmap_bulk_read(data->regmap, SX9360_REG_GNRL_CTRL1,
> > + &buf, sizeof(buf));
> > + if (ret < 0)
> > + return ret;
> > + divisor = be16_to_cpu(buf);
> > + if (divisor == 0) {
> > + *val = 0;
> > + return IIO_VAL_INT;
> > + }
> > +
> > + *val = SX9360_FOSC_HZ;
> > + *val2 = be16_to_cpu(buf) * 8192;
>
> *val2 = divisor * 8192;?
Done
>
> > +
> > + return IIO_VAL_FRACTIONAL;
> > +}
> > +
>
> ...
>
> > +static int sx9360_write_raw(struct iio_dev *indio_dev,
> > + const struct iio_chan_spec *chan, int val, int val2,
> > + long mask)
> > +{
> > + struct sx_common_data *data = iio_priv(indio_dev);
> > +
> > + switch (mask) {
> > + case IIO_CHAN_INFO_SAMP_FREQ:
> > + return sx9360_set_samp_freq(data, val, val2);
> > + case IIO_CHAN_INFO_HARDWAREGAIN:
> > + return sx9360_write_gain(data, chan, val);
> > + }
> > +
>
> Slight preference for this as a default in the switch.
Done, changed sx9360_read_avail() as well.
>
> > + return -EINVAL;
> > +}
>
> ...
>
>
> > +static int sx9360_check_whoami(struct device *dev,
> > + struct iio_dev *indio_dev)
>
> Will fit on one line under 80 chars I think..
Done.
>
> > +{
> > + /*
> > + * Only one sensor for this driver. Assuming the device tree
> > + * is correct, just set the sensor name.
> > + */
> > + indio_dev->name = "sx9360";
> > + return 0;
> > +}
> > +
>
> > +
> > +static int __maybe_unused sx9360_suspend(struct device *dev)
> > +{
> > + struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
>
> I don't feel particularly strongly about this, as there are arguments
> either way but this is the same as
>
> struct iio_dev *indio_dev = dev_get_drvdata(dev);
>
Done in both suspend and resume.
> > + struct sx_common_data *data = iio_priv(indio_dev);
> > + unsigned int regval;
> > + int ret;
> > +
> > + disable_irq_nosync(data->client->irq);
> > +
> > + mutex_lock(&data->mutex);
> > + ret = regmap_read(data->regmap, SX9360_REG_GNRL_CTRL0, ®val);
> > +
> > + data->suspend_ctrl =
> > + FIELD_GET(SX9360_REG_GNRL_CTRL0_PHEN_MASK, regval);
> > +
> > + if (ret < 0)
> > + goto out;
> > +
> > + /* Disable all phases, send the device to sleep. */
> > + ret = regmap_write(data->regmap, SX9360_REG_GNRL_CTRL0, 0);
> > +
> > +out:
> > + mutex_unlock(&data->mutex);
> > + return ret;
> > +}
> > +
>
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2021-12-18 0:27 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-13 2:40 [PATCH v3 0/4] Add Semtech SX9360 SAR Sensor support Gwendal Grignou
2021-12-13 2:40 ` [PATCH v3 1/4] iio: add IIO_MOD_REFERENCE modifier Gwendal Grignou
2021-12-16 15:59 ` Jonathan Cameron
2021-12-17 20:24 ` Gwendal Grignou
2021-12-13 2:40 ` [PATCH v3 2/4] iio: proximity: Add sx9360 support Gwendal Grignou
2021-12-15 1:14 ` Stephen Boyd
2021-12-16 16:21 ` Jonathan Cameron
2021-12-18 0:27 ` Gwendal Grignou
2021-12-13 2:40 ` [PATCH v3 3/4] dt-bindings: iio: Add sx9360 binding Gwendal Grignou
2021-12-15 1:15 ` Stephen Boyd
2021-12-15 20:09 ` Rob Herring
2021-12-13 2:40 ` [PATCH v3 4/4] iio: sx9360: Add dt-binding support Gwendal Grignou
2021-12-15 1:16 ` Stephen Boyd
2021-12-16 16:23 ` Jonathan Cameron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).