Re: [PATCH 1/3] iio: sps30: separate core and interface specific code

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 4/25/21 3:55 PM, Tomasz Duszynski wrote:
Move code responsible for handling i2c communication to a separate file.
Rationale for this change is preparation for adding support for serial
communication.

Signed-off-by: Tomasz Duszynski <tomasz.duszynski@xxxxxxxxxxx>

Hi,

The whole series looks really great. Couple of small comments inline.

[...]
diff --git a/drivers/iio/chemical/Kconfig b/drivers/iio/chemical/Kconfig
index 10bb431bc3ce..82af5f62fbc6 100644
--- a/drivers/iio/chemical/Kconfig
+++ b/drivers/iio/chemical/Kconfig
@@ -133,8 +133,6 @@ config SENSIRION_SGP30
config SPS30
  	tristate "SPS30 particulate matter sensor"
-	depends on I2C
-	select CRC8
  	select IIO_BUFFER
  	select IIO_TRIGGERED_BUFFER
  	help
@@ -144,6 +142,17 @@ config SPS30
  	  To compile this driver as a module, choose M here: the module will
  	  be called sps30.
+config SPS30_I2C
+	tristate "SPS30 particulate matter sensor I2C driver"
+	depends on SPS30 && I2C
+	select CRC8
Since the base module is not very useful without any of the drivers enabled, what you can do here is, make the base module non-user-selectable, e.g. remove the description text after the tristate and then do a `select SPS30` both here from he I2C module and the serial module.
+	help
+	  Say Y here to build support for the Sensirion SPS30 I2C interface
+	  driver.
+
+	  To compile this driver as a module, choose M here: the module will
+	  be called sps30_i2c.
+
  config VZ89X
  	tristate "SGX Sensortech MiCS VZ89X VOC sensor"
  	depends on I2C
diff --git a/drivers/iio/chemical/Makefile b/drivers/iio/chemical/Makefile
index fef63dd5bf92..41c264a229c0 100644
--- a/drivers/iio/chemical/Makefile
+++ b/drivers/iio/chemical/Makefile
@@ -17,4 +17,5 @@ obj-$(CONFIG_SCD30_I2C) += scd30_i2c.o
  obj-$(CONFIG_SCD30_SERIAL) += scd30_serial.o
  obj-$(CONFIG_SENSIRION_SGP30)	+= sgp30.o
  obj-$(CONFIG_SPS30) += sps30.o
+obj-$(CONFIG_SPS30_I2C) += sps30_i2c.o
  obj-$(CONFIG_VZ89X)		+= vz89x.o
diff --git a/drivers/iio/chemical/sps30.c b/drivers/iio/chemical/sps30.c
index 7486591588c3..ec9db99e324c 100644
--- a/drivers/iio/chemical/sps30.c
+++ b/drivers/iio/chemical/sps30.c
[...]
+EXPORT_SYMBOL(sps30_probe);
EXPORT_SYMBOL_GPL()
MODULE_AUTHOR("Tomasz Duszynski <tduszyns@xxxxxxxxx>");
  MODULE_DESCRIPTION("Sensirion SPS30 particulate matter sensor driver");
diff --git a/drivers/iio/chemical/sps30.h b/drivers/iio/chemical/sps30.h
new file mode 100644
index 000000000000..d2b7140fdeb4
--- /dev/null
+++ b/drivers/iio/chemical/sps30.h
@@ -0,0 +1,73 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _SPS30_H
+#define _SPS30_H
+
+struct sps30_state;
+struct sps30_ops {
+	int (*start_meas)(struct sps30_state *state);
+	int (*stop_meas)(struct sps30_state *state);
+	int (*read_meas)(struct sps30_state *state, int *meas, int num);
+	int (*reset)(struct sps30_state *state);
+	int (*clean_fan)(struct sps30_state *state);
+	int (*read_cleaning_period)(struct sps30_state *state, int *period);
+	int (*write_cleaning_period)(struct sps30_state *state, int period);

The interface for {read,write}_cleaning_period() should use __be32, since that is what is being passed around.

I was a bit confused when reviewing the uart driver why we can just memcpy an int into the output buffer without endianess problems.

+	int (*show_info)(struct sps30_state *state);
+};
+
[...]



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux