From: Petar Stoykov <pd.pstoykov@gmail.com>
Sensirion SDP500 is a digital differential pressure sensor. The sensor is
accessed over I2C.
Signed-off-by: Petar Stoykov <pd.pstoykov@gmail.com>
---
drivers/iio/pressure/Kconfig | 9 +++
drivers/iio/pressure/Makefile | 1 +
drivers/iio/pressure/sdp500.c | 153 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 163 insertions(+)
diff --git a/drivers/iio/pressure/Kconfig b/drivers/iio/pressure/Kconfig
index 95efa32e4289..5debdfbd5324 100644
--- a/drivers/iio/pressure/Kconfig
+++ b/drivers/iio/pressure/Kconfig
@@ -212,6 +212,15 @@ config MS5637
This driver can also be built as a module. If so, the module will
be called ms5637.
+config SDP500
+ tristate "Sensirion SDP500 differential pressure sensor I2C driver"
+ depends on I2C
+ help
+ Say Y here to build support for Sensirion SDP500 differential pressure
+ sensor I2C driver.
+ To compile this driver as a module, choose M here: the core module
+ will be called sdp500.
+
config IIO_ST_PRESS
tristate "STMicroelectronics pressure sensor Driver"
depends on (I2C || SPI_MASTER) && SYSFS
diff --git a/drivers/iio/pressure/Makefile b/drivers/iio/pressure/Makefile
index 436aec7e65f3..489ef7b7befa 100644
--- a/drivers/iio/pressure/Makefile
+++ b/drivers/iio/pressure/Makefile
@@ -25,6 +25,7 @@ obj-$(CONFIG_MS5611) += ms5611_core.o
obj-$(CONFIG_MS5611_I2C) += ms5611_i2c.o
obj-$(CONFIG_MS5611_SPI) += ms5611_spi.o
obj-$(CONFIG_MS5637) += ms5637.o
+obj-$(CONFIG_SDP500) += sdp500.o
obj-$(CONFIG_IIO_ST_PRESS) += st_pressure.o
st_pressure-y := st_pressure_core.o
st_pressure-$(CONFIG_IIO_BUFFER) += st_pressure_buffer.o
diff --git a/drivers/iio/pressure/sdp500.c b/drivers/iio/pressure/sdp500.c
new file mode 100644
index 000000000000..661c70bc1b5b
--- /dev/null
+++ b/drivers/iio/pressure/sdp500.c
@@ -0,0 +1,153 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Driver for Sensirion sdp500 and sdp510 pressure sensors
+ *
+ * Datasheet: https://sensirion.com/resource/datasheet/sdp600
+ */
+
+#include <linux/i2c.h>
+#include <linux/crc8.h>
+#include <linux/iio/iio.h>
+#include <linux/regulator/consumer.h>
+#include <asm/unaligned.h>
+
+#define SDP500_CRC8_POLYNOMIAL 0x31 // x8 + x5 + x4 + 1 (normalized to 0x31)
+#define SDP500_READ_SIZE 3
+
+#define SDP500_I2C_START_MEAS 0xF1
+
+struct sdp500_data {
+ struct device *dev;
+};
+
+DECLARE_CRC8_TABLE(sdp500_crc8_table);
+
+static int sdp500_start_measurement(struct sdp500_data *data)
+{
+ struct i2c_client *client = to_i2c_client(data->dev);
+
+ return i2c_smbus_write_byte(client, SDP500_I2C_START_MEAS);
+}
+
+static const struct iio_chan_spec sdp500_channels[] = {
+ {
+ .type = IIO_PRESSURE,
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+ BIT(IIO_CHAN_INFO_SCALE),
+ },
+};
+
+static int sdp500_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int *val, int *val2, long mask)
+{
+ int ret;
+ u8 rxbuf[SDP500_READ_SIZE];
+ u8 received_crc, calculated_crc;
+ struct sdp500_data *data = iio_priv(indio_dev);
+ struct i2c_client *client = to_i2c_client(data->dev);
+
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ ret = i2c_master_recv(client, rxbuf, SDP500_READ_SIZE);
+ if (ret < 0) {
+ dev_err(data->dev, "Failed to receive data");
+ return ret;
+ }
+ if (ret != SDP500_READ_SIZE) {
+ dev_err(data->dev, "Data is received wrongly");
+ return -EIO;
+ }
+
+ received_crc = rxbuf[2];
+ calculated_crc = crc8(sdp500_crc8_table, rxbuf, sizeof(rxbuf) - 1, 0x00);
+ if (received_crc != calculated_crc) {
+ dev_err(data->dev, "calculated crc = 0x%.2X, received 0x%.2X",
+ calculated_crc, received_crc);
+ return -EIO;
+ }
+
+ *val = get_unaligned_be16(rxbuf);
+ return IIO_VAL_INT;
+ case IIO_CHAN_INFO_SCALE:
+ *val = 1;
+ *val2 = 60;
+
+ return IIO_VAL_FRACTIONAL;
+ default:
+ return -EINVAL;
+ }
+}
+
+static const struct iio_info sdp500_info = {
+ .read_raw = &sdp500_read_raw,
+};
+
+static int sdp500_probe(struct i2c_client *client)
+{
+ struct iio_dev *indio_dev;
+ struct sdp500_data *data;
+ struct device *dev = &client->dev;
+ int ret;
+ u8 rxbuf[SDP500_READ_SIZE];
+
+ ret = devm_regulator_get_enable(dev, "vdd");
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to get and enable regulator\n");
+
+ indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
+ if (!indio_dev)
+ return -ENOMEM;
+
+ /* has to be done before the first i2c communication */
+ crc8_populate_msb(sdp500_crc8_table, SDP500_CRC8_POLYNOMIAL);
+
+ data = iio_priv(indio_dev);
+ data->dev = dev;
+
+ indio_dev->name = "sdp500";
+ indio_dev->channels = sdp500_channels;
+ indio_dev->info = &sdp500_info;
+ indio_dev->modes = INDIO_DIRECT_MODE;
+ indio_dev->num_channels = ARRAY_SIZE(sdp500_channels);
+
+ ret = sdp500_start_measurement(data);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to start measurement");
+
+ /* First measurement is not correct, read it out to get rid of it */
+ i2c_master_recv(client, rxbuf, SDP500_READ_SIZE);
+
+ ret = devm_iio_device_register(dev, indio_dev);
+ if (ret < 0)
+ return dev_err_probe(dev, ret, "Failed to register indio_dev");
+
+ return 0;
+}
+
+static const struct i2c_device_id sdp500_id[] = {
+ { "sdp500" },
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, sdp500_id);
+
+static const struct of_device_id sdp500_of_match[] = {
+ { .compatible = "sensirion,sdp500" },
+ { .compatible = "sensirion,sdp510" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, sdp500_of_match);
+
+static struct i2c_driver sdp500_driver = {
+ .driver = {
+ .name = "sensirion,sdp500",
+ .of_match_table = sdp500_of_match,
+ },
+ .probe = sdp500_probe,
+ .id_table = sdp500_id,
+};
+module_i2c_driver(sdp500_driver);
+
+MODULE_AUTHOR("Thomas Sioutas <thomas.sioutas@prodrive-technologies.com>");
+MODULE_DESCRIPTION("Driver for Sensirion SDP500 differential pressure sensor");
+MODULE_LICENSE("GPL");
--
2.39.2On Tue, Jul 02, 2024 at 04:59:09PM +0200, Petar Stoykov via B4 Relay wrote:
> From: Petar Stoykov <pd.pstoykov@gmail.com>
>
> Sensirion SDP500 is a digital differential pressure sensor. The sensor is
> accessed over I2C.
...
+ array_size.h
+ bits.h
+ dev_printk.h
+ errno.h
> +#include <linux/i2c.h>
> +#include <linux/crc8.h>
> +#include <linux/iio/iio.h>
+ mod_devicetable.h
+ module.h
> +#include <linux/regulator/consumer.h>
+ types.h
Keep them ordered, also you may split iio/ group out
> +#include <asm/unaligned.h>
linux/*
...blank line...
asm/*
...blank line...
iio/*
...
> +struct sdp500_data {
> + struct device *dev;
> +};
Why is this structure needed at all? You may put dev pointer directly, no?
--
With Best Regards,
Andy Shevchenko
On Tue, 02 Jul 2024 16:59:09 +0200
Petar Stoykov via B4 Relay <devnull+pd.pstoykov.gmail.com@kernel.org> wrote:
> From: Petar Stoykov <pd.pstoykov@gmail.com>
>
> Sensirion SDP500 is a digital differential pressure sensor. The sensor is
> accessed over I2C.
>
> Signed-off-by: Petar Stoykov <pd.pstoykov@gmail.com>
Hi Petar
Given you are going to be doing a v4, a few comments inline on things
to tidy up in here.
Note that I've already tagged what may be the last pull request from me
for the 6.11 merge window, so this is probably 6.12 material now anyway.
Hence plenty of time to tidy up.
Jonathan
> diff --git a/drivers/iio/pressure/sdp500.c b/drivers/iio/pressure/sdp500.c
> new file mode 100644
> index 000000000000..661c70bc1b5b
> --- /dev/null
> +++ b/drivers/iio/pressure/sdp500.c
> @@ -0,0 +1,153 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Driver for Sensirion sdp500 and sdp510 pressure sensors
> + *
> + * Datasheet: https://sensirion.com/resource/datasheet/sdp600
> + */
> +
> +#include <linux/i2c.h>
> +#include <linux/crc8.h>
> +#include <linux/iio/iio.h>
#include <linux/mod_devicetable.h> appropriate for the id tables.
> +#include <linux/regulator/consumer.h>
> +#include <asm/unaligned.h>
> +
> +#define SDP500_CRC8_POLYNOMIAL 0x31 // x8 + x5 + x4 + 1 (normalized to 0x31)
For IIO we tend to just use c style comments /* xxx */
and not c++ ones.
> +#define SDP500_READ_SIZE 3
> +static int sdp500_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int *val, int *val2, long mask)
> +{
> + int ret;
> + u8 rxbuf[SDP500_READ_SIZE];
> + u8 received_crc, calculated_crc;
> + struct sdp500_data *data = iio_priv(indio_dev);
> + struct i2c_client *client = to_i2c_client(data->dev);
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + ret = i2c_master_recv(client, rxbuf, SDP500_READ_SIZE);
> + if (ret < 0) {
> + dev_err(data->dev, "Failed to receive data");
> + return ret;
> + }
> + if (ret != SDP500_READ_SIZE) {
> + dev_err(data->dev, "Data is received wrongly");
> + return -EIO;
> + }
> +
> + received_crc = rxbuf[2];
> + calculated_crc = crc8(sdp500_crc8_table, rxbuf, sizeof(rxbuf) - 1, 0x00);
A line break in here to keep it under 80 chars would be good (see below).
> + if (received_crc != calculated_crc) {
> + dev_err(data->dev, "calculated crc = 0x%.2X, received 0x%.2X",
> + calculated_crc, received_crc);
> + return -EIO;
> + }
> +
> + *val = get_unaligned_be16(rxbuf);
> + return IIO_VAL_INT;
> + case IIO_CHAN_INFO_SCALE:
> + *val = 1;
> + *val2 = 60;
> +
> + return IIO_VAL_FRACTIONAL;
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static const struct iio_info sdp500_info = {
> + .read_raw = &sdp500_read_raw,
> +};
> +
> +static int sdp500_probe(struct i2c_client *client)
> +{
> + struct iio_dev *indio_dev;
> + struct sdp500_data *data;
> + struct device *dev = &client->dev;
> + int ret;
> + u8 rxbuf[SDP500_READ_SIZE];
> +
> + ret = devm_regulator_get_enable(dev, "vdd");
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to get and enable regulator\n");
Given you are going around again. Add a line break before "
General rule is stay under 80 chars unless it hurts readability.
> +
On Tue, Jul 2, 2024 at 10:16 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Tue, 02 Jul 2024 16:59:09 +0200
> Petar Stoykov via B4 Relay <devnull+pd.pstoykov.gmail.com@kernel.org> wrote:
>
> > From: Petar Stoykov <pd.pstoykov@gmail.com>
> >
> > Sensirion SDP500 is a digital differential pressure sensor. The sensor is
> > accessed over I2C.
> >
> > Signed-off-by: Petar Stoykov <pd.pstoykov@gmail.com>
> Hi Petar
>
> Given you are going to be doing a v4, a few comments inline on things
> to tidy up in here.
>
> Note that I've already tagged what may be the last pull request from me
> for the 6.11 merge window, so this is probably 6.12 material now anyway.
> Hence plenty of time to tidy up.
>
> Jonathan
>
Hi Jonathan,
I have no rush, thank you for the explanation!
> > diff --git a/drivers/iio/pressure/sdp500.c b/drivers/iio/pressure/sdp500.c
> > new file mode 100644
> > index 000000000000..661c70bc1b5b
> > --- /dev/null
> > +++ b/drivers/iio/pressure/sdp500.c
> > @@ -0,0 +1,153 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Driver for Sensirion sdp500 and sdp510 pressure sensors
> > + *
> > + * Datasheet: https://sensirion.com/resource/datasheet/sdp600
> > + */
> > +
> > +#include <linux/i2c.h>
> > +#include <linux/crc8.h>
> > +#include <linux/iio/iio.h>
>
> #include <linux/mod_devicetable.h> appropriate for the id tables.
I don't understand. Why add an extra header to a working driver?
I will add it and test the driver in the meantime.
>
> > +#include <linux/regulator/consumer.h>
> > +#include <asm/unaligned.h>
> > +
> > +#define SDP500_CRC8_POLYNOMIAL 0x31 // x8 + x5 + x4 + 1 (normalized to 0x31)
>
> For IIO we tend to just use c style comments /* xxx */
> and not c++ ones.
I try to follow this rule but once in a while I slip to my old habits.
My teachers all used // for C for whatever reason. Will fix.
>
> > +#define SDP500_READ_SIZE 3
>
> > +static int sdp500_read_raw(struct iio_dev *indio_dev,
> > + struct iio_chan_spec const *chan,
> > + int *val, int *val2, long mask)
> > +{
> > + int ret;
> > + u8 rxbuf[SDP500_READ_SIZE];
> > + u8 received_crc, calculated_crc;
> > + struct sdp500_data *data = iio_priv(indio_dev);
> > + struct i2c_client *client = to_i2c_client(data->dev);
> > +
> > + switch (mask) {
> > + case IIO_CHAN_INFO_RAW:
> > + ret = i2c_master_recv(client, rxbuf, SDP500_READ_SIZE);
> > + if (ret < 0) {
> > + dev_err(data->dev, "Failed to receive data");
> > + return ret;
> > + }
> > + if (ret != SDP500_READ_SIZE) {
> > + dev_err(data->dev, "Data is received wrongly");
> > + return -EIO;
> > + }
> > +
> > + received_crc = rxbuf[2];
> > + calculated_crc = crc8(sdp500_crc8_table, rxbuf, sizeof(rxbuf) - 1, 0x00);
> A line break in here to keep it under 80 chars would be good (see below).
I find the 80 chars too constraining and also saw that the limit was updated
to 100 somewhere in kernel.org which made me happy. But.. you seem to
feel strongly enough about this so I'll trim lines to 80.
>
> > + if (received_crc != calculated_crc) {
> > + dev_err(data->dev, "calculated crc = 0x%.2X, received 0x%.2X",
> > + calculated_crc, received_crc);
> > + return -EIO;
> > + }
> > +
> > + *val = get_unaligned_be16(rxbuf);
> > + return IIO_VAL_INT;
> > + case IIO_CHAN_INFO_SCALE:
> > + *val = 1;
> > + *val2 = 60;
> > +
> > + return IIO_VAL_FRACTIONAL;
> > + default:
> > + return -EINVAL;
> > + }
> > +}
> > +
> > +static const struct iio_info sdp500_info = {
> > + .read_raw = &sdp500_read_raw,
> > +};
> > +
> > +static int sdp500_probe(struct i2c_client *client)
> > +{
> > + struct iio_dev *indio_dev;
> > + struct sdp500_data *data;
> > + struct device *dev = &client->dev;
> > + int ret;
> > + u8 rxbuf[SDP500_READ_SIZE];
> > +
> > + ret = devm_regulator_get_enable(dev, "vdd");
> > + if (ret)
> > + return dev_err_probe(dev, ret, "Failed to get and enable regulator\n");
> Given you are going around again. Add a line break before "
> General rule is stay under 80 chars unless it hurts readability.
>
> > +
On Thu, 4 Jul 2024 10:09:26 +0200
Petar Stoykov <pd.pstoykov@gmail.com> wrote:
> On Tue, Jul 2, 2024 at 10:16 PM Jonathan Cameron <jic23@kernel.org> wrote:
> >
> > On Tue, 02 Jul 2024 16:59:09 +0200
> > Petar Stoykov via B4 Relay <devnull+pd.pstoykov.gmail.com@kernel.org> wrote:
> >
> > > From: Petar Stoykov <pd.pstoykov@gmail.com>
> > >
> > > Sensirion SDP500 is a digital differential pressure sensor. The sensor is
> > > accessed over I2C.
> > >
> > > Signed-off-by: Petar Stoykov <pd.pstoykov@gmail.com>
> > Hi Petar
> >
> > Given you are going to be doing a v4, a few comments inline on things
> > to tidy up in here.
> >
> > Note that I've already tagged what may be the last pull request from me
> > for the 6.11 merge window, so this is probably 6.12 material now anyway.
> > Hence plenty of time to tidy up.
> >
> > Jonathan
> >
>
> Hi Jonathan,
> I have no rush, thank you for the explanation!
>
> > > diff --git a/drivers/iio/pressure/sdp500.c b/drivers/iio/pressure/sdp500.c
> > > new file mode 100644
> > > index 000000000000..661c70bc1b5b
> > > --- /dev/null
> > > +++ b/drivers/iio/pressure/sdp500.c
> > > @@ -0,0 +1,153 @@
> > > +// SPDX-License-Identifier: GPL-2.0-only
> > > +/*
> > > + * Driver for Sensirion sdp500 and sdp510 pressure sensors
> > > + *
> > > + * Datasheet: https://sensirion.com/resource/datasheet/sdp600
> > > + */
> > > +
> > > +#include <linux/i2c.h>
> > > +#include <linux/crc8.h>
> > > +#include <linux/iio/iio.h>
> >
> > #include <linux/mod_devicetable.h> appropriate for the id tables.
>
> I don't understand. Why add an extra header to a working driver?
> I will add it and test the driver in the meantime.
It's included via another path today (at least on the architectures
you've tested with), but we more or less prefer to operate on an
include what you use principle.
There are exceptions where it is considered very obvious that
the headers will always be included but this isn't one of them.
(there is no hard list of which ones are included though :(
>
> >
> > > +#include <linux/regulator/consumer.h>
> > > +#include <asm/unaligned.h>
> > > +
> > > +#define SDP500_CRC8_POLYNOMIAL 0x31 // x8 + x5 + x4 + 1 (normalized to 0x31)
> >
> > For IIO we tend to just use c style comments /* xxx */
> > and not c++ ones.
>
> I try to follow this rule but once in a while I slip to my old habits.
> My teachers all used // for C for whatever reason. Will fix.
>
> >
> > > +#define SDP500_READ_SIZE 3
> >
> > > +static int sdp500_read_raw(struct iio_dev *indio_dev,
> > > + struct iio_chan_spec const *chan,
> > > + int *val, int *val2, long mask)
> > > +{
> > > + int ret;
> > > + u8 rxbuf[SDP500_READ_SIZE];
> > > + u8 received_crc, calculated_crc;
> > > + struct sdp500_data *data = iio_priv(indio_dev);
> > > + struct i2c_client *client = to_i2c_client(data->dev);
> > > +
> > > + switch (mask) {
> > > + case IIO_CHAN_INFO_RAW:
> > > + ret = i2c_master_recv(client, rxbuf, SDP500_READ_SIZE);
> > > + if (ret < 0) {
> > > + dev_err(data->dev, "Failed to receive data");
> > > + return ret;
> > > + }
> > > + if (ret != SDP500_READ_SIZE) {
> > > + dev_err(data->dev, "Data is received wrongly");
> > > + return -EIO;
> > > + }
> > > +
> > > + received_crc = rxbuf[2];
> > > + calculated_crc = crc8(sdp500_crc8_table, rxbuf, sizeof(rxbuf) - 1, 0x00);
> > A line break in here to keep it under 80 chars would be good (see below).
>
> I find the 80 chars too constraining and also saw that the limit was updated
> to 100 somewhere in kernel.org which made me happy. But.. you seem to
> feel strongly enough about this so I'll trim lines to 80.
100 is fine if it helps readability. I'd argue it doesn't significantly
help here.
Some parts of the kernel are fussier than others, but it's rare you'll get
told to increase your wrap length above 80 unless there is a readability
advantage.
Jonathan
On 02/07/2024 16:59, Petar Stoykov via B4 Relay wrote:
> From: Petar Stoykov <pd.pstoykov@gmail.com>
>
> Sensirion SDP500 is a digital differential pressure sensor. The sensor is
> accessed over I2C.
>
...
> +
> +static const struct of_device_id sdp500_of_match[] = {
> + { .compatible = "sensirion,sdp500" },
> + { .compatible = "sensirion,sdp510" },
Devices look compatible, so express it in the bindings with fallback to
sdp500 (oneOf, see example-schema).
Best regards,
Krzysztof
On Tue, Jul 2, 2024 at 5:17 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 02/07/2024 16:59, Petar Stoykov via B4 Relay wrote:
> > From: Petar Stoykov <pd.pstoykov@gmail.com>
> >
> > Sensirion SDP500 is a digital differential pressure sensor. The sensor is
> > accessed over I2C.
> >
>
> ...
>
> > +
> > +static const struct of_device_id sdp500_of_match[] = {
> > + { .compatible = "sensirion,sdp500" },
> > + { .compatible = "sensirion,sdp510" },
>
> Devices look compatible, so express it in the bindings with fallback to
> sdp500 (oneOf, see example-schema).
Hi Krzysztof,
I tried to understand the explanation in the example-schema. I must say it is
a bit confusing, but I can't offer improvement because I'm not sure I
understand it fully yet.
Can you verify if this is what you expect the bindings file to read?
properties:
compatible:
oneOf:
- items:
- const: sensirion,sdp510
- const: sensirion,sdp500
- items:
- const: sensirion,sdp500
So the device tree should have either this
compatible = "sensirion,sdp510", "sensirion,sdp500"
Or
compatible = "sensirion,sdp500"
The first would mean that 510 falls back to 500, right?
From what I've seen it is rare to have two strings in "compatible" so I didn't
know of this mechanism of "fallback" in the dts. I expected to just write the
name of my device (let's say "sensirion,sdp510") and the driver would handle it.
But I'm learning. Expectations change.
Regards,
Petar
>
> Best regards,
> Krzysztof
>
On 04/07/2024 09:49, Petar Stoykov wrote:
> On Tue, Jul 2, 2024 at 5:17 PM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 02/07/2024 16:59, Petar Stoykov via B4 Relay wrote:
>>> From: Petar Stoykov <pd.pstoykov@gmail.com>
>>>
>>> Sensirion SDP500 is a digital differential pressure sensor. The sensor is
>>> accessed over I2C.
>>>
>>
>> ...
>>
>>> +
>>> +static const struct of_device_id sdp500_of_match[] = {
>>> + { .compatible = "sensirion,sdp500" },
>>> + { .compatible = "sensirion,sdp510" },
>>
>> Devices look compatible, so express it in the bindings with fallback to
>> sdp500 (oneOf, see example-schema).
>
> Hi Krzysztof,
> I tried to understand the explanation in the example-schema. I must say it is
> a bit confusing, but I can't offer improvement because I'm not sure I
> understand it fully yet.
> Can you verify if this is what you expect the bindings file to read?
> properties:
> compatible:
> oneOf:
> - items:
> - const: sensirion,sdp510
> - const: sensirion,sdp500
> - items:
> - const: sensirion,sdp500
Almost.
oneOf:
- items:
- const: sensirion,sdp510
- const: sensirion,sdp500
- const: sensirion,sdp500
>
> So the device tree should have either this
> compatible = "sensirion,sdp510", "sensirion,sdp500"
> Or
> compatible = "sensirion,sdp500"
> The first would mean that 510 falls back to 500, right?
Yes
>
> From what I've seen it is rare to have two strings in "compatible" so I didn't
Rare? It's everywhere in SoC and in many places for standalone
devices/hardware.
> know of this mechanism of "fallback" in the dts. I expected to just write the
> name of my device (let's say "sensirion,sdp510") and the driver would handle it.
> But I'm learning. Expectations change.
Best regards,
Krzysztof
© 2016 - 2026 Red Hat, Inc.