Re: [PATCH 3/3] iio: chemical: sps30: add device tree support
On Sun, 2 Dec 2018 19:29:44 +0100
Tomasz Duszynski wrote:
> On Sun, Nov 25, 2018 at 08:59:39AM +, Jonathan Cameron wrote:
> > On Sat, 24 Nov 2018 23:14:15 +0100
> > Tomasz Duszynski wrote:
> >
> > > Add device tree support for Sensirion SPS30 particulate
> > > matter sensor.
> > >
> > > Signed-off-by: Tomasz Duszynski
> > one comment inine, around the fact we are trying to move
> > to generic names in DT where ever possible. Now we don't
> > have a suitable one (IIRC) yet so time to make one up ;)
> >
> > +CC Rob for his input on that.
> > > ---
> > > .../bindings/iio/chemical/sensirion,sps30.txt| 12
> > > 1 file changed, 12 insertions(+)
> > > create mode 100644
> > > Documentation/devicetree/bindings/iio/chemical/sensirion,sps30.txt
> > >
> > > diff --git
> > > a/Documentation/devicetree/bindings/iio/chemical/sensirion,sps30.txt
> > > b/Documentation/devicetree/bindings/iio/chemical/sensirion,sps30.txt
> > > new file mode 100644
> > > index ..6eee2709b5b6
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/iio/chemical/sensirion,sps30.txt
> > > @@ -0,0 +1,12 @@
> > > +* Sensirion SPS30 particulate matter sensor
> > > +
> > > +Required properties:
> > > +- compatible: must be "sensirion,sps30"
> > > +- reg: the I2C address of the sensor
> > > +
> > > +Example:
> > > +
> > > +sps30@69 {
> > We should define a generic type. Rob, what would work for this
> > one?
> >
> > particlesensor@69?
> >
>
> Wouldn't air-pollution-sensor be somewhat more generic? At least
> wikipedia has some article about that. Various other names like
> particle-sensor, pm-sensor, particulate-matter-sensor,
> air-quality-sensor, tend to return more or less similar number
> of search hits. Which means there's no universal naming convention.
I have not strong feeling in favor of a particular choice.
Happy with wherever the discussion converges.
Jonathan
>
> > > + compatible = "sensirion,sps30";
> > > + reg = <0x69>;
> > > +};
> >
Re: [PATCH 3/3] iio: chemical: sps30: add device tree support
On Sun, Nov 25, 2018 at 08:59:39AM +, Jonathan Cameron wrote:
> On Sat, 24 Nov 2018 23:14:15 +0100
> Tomasz Duszynski wrote:
>
> > Add device tree support for Sensirion SPS30 particulate
> > matter sensor.
> >
> > Signed-off-by: Tomasz Duszynski
> one comment inine, around the fact we are trying to move
> to generic names in DT where ever possible. Now we don't
> have a suitable one (IIRC) yet so time to make one up ;)
>
> +CC Rob for his input on that.
> > ---
> > .../bindings/iio/chemical/sensirion,sps30.txt| 12
> > 1 file changed, 12 insertions(+)
> > create mode 100644
> > Documentation/devicetree/bindings/iio/chemical/sensirion,sps30.txt
> >
> > diff --git
> > a/Documentation/devicetree/bindings/iio/chemical/sensirion,sps30.txt
> > b/Documentation/devicetree/bindings/iio/chemical/sensirion,sps30.txt
> > new file mode 100644
> > index ..6eee2709b5b6
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/chemical/sensirion,sps30.txt
> > @@ -0,0 +1,12 @@
> > +* Sensirion SPS30 particulate matter sensor
> > +
> > +Required properties:
> > +- compatible: must be "sensirion,sps30"
> > +- reg: the I2C address of the sensor
> > +
> > +Example:
> > +
> > +sps30@69 {
> We should define a generic type. Rob, what would work for this
> one?
>
> particlesensor@69?
>
Wouldn't air-pollution-sensor be somewhat more generic? At least
wikipedia has some article about that. Various other names like
particle-sensor, pm-sensor, particulate-matter-sensor,
air-quality-sensor, tend to return more or less similar number
of search hits. Which means there's no universal naming convention.
> > + compatible = "sensirion,sps30";
> > + reg = <0x69>;
> > +};
>
Re: [PATCH 3/3] iio: chemical: sps30: add device tree support
Hi Tomasz,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on iio/togreg]
[also build test WARNING on v4.20-rc4 next-20181123]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Tomasz-Duszynski/add-support-for-Sensirion-SPS30-PM-sensor/20181125-172634
base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
config: x86_64-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All warnings (new ones prefixed by >>):
>> drivers/iio/chemical/sps30.c:69:26: warning: Variable length array is used.
drivers/iio/chemical/sps30.c: In function 'sps30_read_raw':
drivers/iio/chemical/sps30.c:237:7: warning: 'ret' may be used uninitialized
in this function [-Wmaybe-uninitialized]
if (ret)
^
vim +69 drivers/iio/chemical/sps30.c
0e4f3167f Tomasz Duszynski 2018-11-24 64
0e4f3167f Tomasz Duszynski 2018-11-24 65 static int
sps30_write_then_read(struct sps30_state *state, u8 *buf,
0e4f3167f Tomasz Duszynski 2018-11-24 66 int
buf_size, u8 *data, int data_size)
0e4f3167f Tomasz Duszynski 2018-11-24 67 {
0e4f3167f Tomasz Duszynski 2018-11-24 68 /* every two received data
bytes are checksummed */
0e4f3167f Tomasz Duszynski 2018-11-24 @69 u8 tmp[data_size + data_size /
2];
0e4f3167f Tomasz Duszynski 2018-11-24 70 int ret, i;
0e4f3167f Tomasz Duszynski 2018-11-24 71
0e4f3167f Tomasz Duszynski 2018-11-24 72 /*
0e4f3167f Tomasz Duszynski 2018-11-24 73 * Sensor does not support
repeated start so instead of
0e4f3167f Tomasz Duszynski 2018-11-24 74 * sending two i2c messages in
a row we just send one by one.
0e4f3167f Tomasz Duszynski 2018-11-24 75 */
0e4f3167f Tomasz Duszynski 2018-11-24 76 ret =
i2c_master_send(state->client, buf, buf_size);
0e4f3167f Tomasz Duszynski 2018-11-24 77 if (ret != buf_size)
0e4f3167f Tomasz Duszynski 2018-11-24 78 return ret < 0 ? ret :
-EIO;
0e4f3167f Tomasz Duszynski 2018-11-24 79
0e4f3167f Tomasz Duszynski 2018-11-24 80 if (!data)
0e4f3167f Tomasz Duszynski 2018-11-24 81 return 0;
0e4f3167f Tomasz Duszynski 2018-11-24 82
0e4f3167f Tomasz Duszynski 2018-11-24 83 ret =
i2c_master_recv(state->client, tmp, sizeof(tmp));
0e4f3167f Tomasz Duszynski 2018-11-24 84 if (ret != sizeof(tmp))
0e4f3167f Tomasz Duszynski 2018-11-24 85 return ret < 0 ? ret :
-EIO;
0e4f3167f Tomasz Duszynski 2018-11-24 86
0e4f3167f Tomasz Duszynski 2018-11-24 87 for (i = 0; i < sizeof(tmp); i
+= 3) {
0e4f3167f Tomasz Duszynski 2018-11-24 88 u8 crc =
crc8(sps30_crc8_table, &tmp[i], 2, CRC8_INIT_VALUE);
0e4f3167f Tomasz Duszynski 2018-11-24 89
0e4f3167f Tomasz Duszynski 2018-11-24 90 if (crc != tmp[i + 2]) {
0e4f3167f Tomasz Duszynski 2018-11-24 91
dev_err(&state->client->dev,
0e4f3167f Tomasz Duszynski 2018-11-24 92 "data
integrity check failed\n");
0e4f3167f Tomasz Duszynski 2018-11-24 93 return -EIO;
0e4f3167f Tomasz Duszynski 2018-11-24 94 }
0e4f3167f Tomasz Duszynski 2018-11-24 95
0e4f3167f Tomasz Duszynski 2018-11-24 96 *data++ = tmp[i];
0e4f3167f Tomasz Duszynski 2018-11-24 97 *data++ = tmp[i + 1];
0e4f3167f Tomasz Duszynski 2018-11-24 98 }
0e4f3167f Tomasz Duszynski 2018-11-24 99
0e4f3167f Tomasz Duszynski 2018-11-24 100 return 0;
0e4f3167f Tomasz Duszynski 2018-11-24 101 }
0e4f3167f Tomasz Duszynski 2018-11-24 102
:: The code at line 69 was first introduced by commit
:: 0e4f3167f739fa067d7e1ba672f0b46569d04d84 iio: chemical: add support for
Sensirion SPS30 sensor
:: TO: Tomasz Duszynski
:: CC: 0day robot
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
.config.gz
Description: application/gzip
Re: [PATCH 3/3] iio: chemical: sps30: add device tree support
On Sat, 24 Nov 2018 23:14:15 +0100
Tomasz Duszynski wrote:
> Add device tree support for Sensirion SPS30 particulate
> matter sensor.
>
> Signed-off-by: Tomasz Duszynski
one comment inine, around the fact we are trying to move
to generic names in DT where ever possible. Now we don't
have a suitable one (IIRC) yet so time to make one up ;)
+CC Rob for his input on that.
> ---
> .../bindings/iio/chemical/sensirion,sps30.txt| 12
> 1 file changed, 12 insertions(+)
> create mode 100644
> Documentation/devicetree/bindings/iio/chemical/sensirion,sps30.txt
>
> diff --git
> a/Documentation/devicetree/bindings/iio/chemical/sensirion,sps30.txt
> b/Documentation/devicetree/bindings/iio/chemical/sensirion,sps30.txt
> new file mode 100644
> index ..6eee2709b5b6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/chemical/sensirion,sps30.txt
> @@ -0,0 +1,12 @@
> +* Sensirion SPS30 particulate matter sensor
> +
> +Required properties:
> +- compatible: must be "sensirion,sps30"
> +- reg: the I2C address of the sensor
> +
> +Example:
> +
> +sps30@69 {
We should define a generic type. Rob, what would work for this
one?
particlesensor@69?
> + compatible = "sensirion,sps30";
> + reg = <0x69>;
> +};
[PATCH 3/3] iio: chemical: sps30: add device tree support
Add device tree support for Sensirion SPS30 particulate
matter sensor.
Signed-off-by: Tomasz Duszynski
---
.../bindings/iio/chemical/sensirion,sps30.txt| 12
1 file changed, 12 insertions(+)
create mode 100644
Documentation/devicetree/bindings/iio/chemical/sensirion,sps30.txt
diff --git a/Documentation/devicetree/bindings/iio/chemical/sensirion,sps30.txt
b/Documentation/devicetree/bindings/iio/chemical/sensirion,sps30.txt
new file mode 100644
index ..6eee2709b5b6
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/chemical/sensirion,sps30.txt
@@ -0,0 +1,12 @@
+* Sensirion SPS30 particulate matter sensor
+
+Required properties:
+- compatible: must be "sensirion,sps30"
+- reg: the I2C address of the sensor
+
+Example:
+
+sps30@69 {
+ compatible = "sensirion,sps30";
+ reg = <0x69>;
+};
--
2.19.2
