diff mbox series

[3/3] iio: chemical: sps30: add device tree support

Message ID 20181124221415.10081-4-tduszyns@gmail.com (mailing list archive)
State New, archived
Headers show
Series add support for Sensirion SPS30 PM sensor | expand

Commit Message

Tomasz Duszynski Nov. 24, 2018, 10:14 p.m. UTC 
Add device tree support for Sensirion SPS30 particulate
matter sensor.

Signed-off-by: Tomasz Duszynski <tduszyns@gmail.com>
---
 .../bindings/iio/chemical/sensirion,sps30.txt        | 12 ++++++++++++
 1 file changed, 12 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/chemical/sensirion,sps30.txt

Comments

Jonathan Cameron Nov. 25, 2018, 8:59 a.m. UTC | #1 
On Sat, 24 Nov 2018 23:14:15 +0100
Tomasz Duszynski <tduszyns@gmail.com> wrote:

> Add device tree support for Sensirion SPS30 particulate
> matter sensor.
> 
> Signed-off-by: Tomasz Duszynski <tduszyns@gmail.com>
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 000000000000..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>;
> +};
kernel test robot Nov. 26, 2018, 4:11 a.m. UTC | #2 
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 <tduszyns@gmail.com>
:::::: CC: 0day robot <lkp@intel.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
Tomasz Duszynski Dec. 2, 2018, 6:29 p.m. UTC | #3 
On Sun, Nov 25, 2018 at 08:59:39AM +0000, Jonathan Cameron wrote:
> On Sat, 24 Nov 2018 23:14:15 +0100
> Tomasz Duszynski <tduszyns@gmail.com> wrote:
>
> > Add device tree support for Sensirion SPS30 particulate
> > matter sensor.
> >
> > Signed-off-by: Tomasz Duszynski <tduszyns@gmail.com>
> 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 000000000000..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>;
> > +};
>
Jonathan Cameron Dec. 3, 2018, 1:10 p.m. UTC | #4 
On Sun, 2 Dec 2018 19:29:44 +0100
Tomasz Duszynski <tduszyns@gmail.com> wrote:

> On Sun, Nov 25, 2018 at 08:59:39AM +0000, Jonathan Cameron wrote:
> > On Sat, 24 Nov 2018 23:14:15 +0100
> > Tomasz Duszynski <tduszyns@gmail.com> wrote:
> >  
> > > Add device tree support for Sensirion SPS30 particulate
> > > matter sensor.
> > >
> > > Signed-off-by: Tomasz Duszynski <tduszyns@gmail.com>  
> > 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 000000000000..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>;
> > > +};  
> >
diff mbox series

Patch

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 000000000000..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>;
+};