| Message ID | bd7fa0b07c85172ecba384e239cb0ecf0780766a.1686600780.git.christophe.jaillet@wanadoo.fr (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [1/2] iio: st_sensors: Remove some redundant includes | expand |
On Mon, 12 Jun 2023 22:13:37 +0200 Christophe JAILLET <christophe.jaillet@wanadoo.fr> wrote: > There is no need to include i2c.h and spi/spi.h in st_sensors.h. > Should it be needed, st_sensors_(i2c|spi).h should be used. Agreed these should be dropped from here. But I don't agree with statement that i2c.h and spi/spi.h should be gotten implicitly from st_sensors_i2c.h / st_sensors_spi.h If they are needed the should be included directly in the files where they are needed. Jonathan > > Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> > --- > Based on one of my script, this reduces the number of included files from > 573 files to 515 files when compiling drivers/iio/accel/st_accel_buffer.c > --- > include/linux/iio/common/st_sensors.h | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/include/linux/iio/common/st_sensors.h b/include/linux/iio/common/st_sensors.h > index 607c3a89a647..a1d3d57d6d6e 100644 > --- a/include/linux/iio/common/st_sensors.h > +++ b/include/linux/iio/common/st_sensors.h > @@ -10,8 +10,6 @@ > #ifndef ST_SENSORS_H > #define ST_SENSORS_H > > -#include <linux/i2c.h> > -#include <linux/spi/spi.h> > #include <linux/irqreturn.h> > #include <linux/iio/iio.h> > #include <linux/iio/trigger.h>
Le 17/06/2023 à 20:49, Jonathan Cameron a écrit : > On Mon, 12 Jun 2023 22:13:37 +0200 > Christophe JAILLET <christophe.jaillet@wanadoo.fr> wrote: > >> There is no need to include i2c.h and spi/spi.h in st_sensors.h. >> Should it be needed, st_sensors_(i2c|spi).h should be used. > Agreed these should be dropped from here. > > But I don't agree with statement that i2c.h and spi/spi.h should > be gotten implicitly from st_sensors_i2c.h / st_sensors_spi.h > If they are needed the should be included directly in the files > where they are needed. > > Jonathan Another option I thought about was to: - keep the includes in st_sensors.h - move function declaration in st_sensors.h int st_sensors_spi_configure(struct iio_dev *indio_dev, struct spi_device *spi); and int st_sensors_i2c_configure(struct iio_dev *indio_dev, struct i2c_client *client); - remove st_sensors_spi.h and st_sensors_i2c.h At least, it would simplify things a remove 2 mostly useless .h file. It would not give the benefit of removing some include for files that do not require spi.h or i2c.h, but as these includes are already there, it wouldn't make things worst :) What do you think of this approach? CJ > >> >> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> >> --- >> Based on one of my script, this reduces the number of included files from >> 573 files to 515 files when compiling drivers/iio/accel/st_accel_buffer.c >> --- >> include/linux/iio/common/st_sensors.h | 2 -- >> 1 file changed, 2 deletions(-) >> >> diff --git a/include/linux/iio/common/st_sensors.h b/include/linux/iio/common/st_sensors.h >> index 607c3a89a647..a1d3d57d6d6e 100644 >> --- a/include/linux/iio/common/st_sensors.h >> +++ b/include/linux/iio/common/st_sensors.h >> @@ -10,8 +10,6 @@ >> #ifndef ST_SENSORS_H >> #define ST_SENSORS_H >> >> -#include <linux/i2c.h> >> -#include <linux/spi/spi.h> >> #include <linux/irqreturn.h> >> #include <linux/iio/iio.h> >> #include <linux/iio/trigger.h> > >
On Mon, 19 Jun 2023 22:02:37 +0200 Christophe JAILLET <christophe.jaillet@wanadoo.fr> wrote: > Le 17/06/2023 à 20:49, Jonathan Cameron a écrit : > > On Mon, 12 Jun 2023 22:13:37 +0200 > > Christophe JAILLET <christophe.jaillet@wanadoo.fr> wrote: > > > >> There is no need to include i2c.h and spi/spi.h in st_sensors.h. > >> Should it be needed, st_sensors_(i2c|spi).h should be used. > > Agreed these should be dropped from here. > > > > But I don't agree with statement that i2c.h and spi/spi.h should > > be gotten implicitly from st_sensors_i2c.h / st_sensors_spi.h > > If they are needed the should be included directly in the files > > where they are needed. > > > > Jonathan > > Another option I thought about was to: > - keep the includes in st_sensors.h > > - move function declaration in st_sensors.h > int st_sensors_spi_configure(struct iio_dev *indio_dev, > struct spi_device *spi); > and > int st_sensors_i2c_configure(struct iio_dev *indio_dev, > struct i2c_client *client); > > - remove st_sensors_spi.h and st_sensors_i2c.h > > At least, it would simplify things a remove 2 mostly useless .h file. > > It would not give the benefit of removing some include for files that do > not require spi.h or i2c.h, but as these includes are already there, it > wouldn't make things worst :) > > What do you think of this approach? Gut feeling is not worth bothering... Having separate _i2c.h and _spi.h seems fine to me. I'm fine with this patch, just not the description. Jonathan > > CJ > > > > >> > >> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> > >> --- > >> Based on one of my script, this reduces the number of included files from > >> 573 files to 515 files when compiling drivers/iio/accel/st_accel_buffer.c > >> --- > >> include/linux/iio/common/st_sensors.h | 2 -- > >> 1 file changed, 2 deletions(-) > >> > >> diff --git a/include/linux/iio/common/st_sensors.h b/include/linux/iio/common/st_sensors.h > >> index 607c3a89a647..a1d3d57d6d6e 100644 > >> --- a/include/linux/iio/common/st_sensors.h > >> +++ b/include/linux/iio/common/st_sensors.h > >> @@ -10,8 +10,6 @@ > >> #ifndef ST_SENSORS_H > >> #define ST_SENSORS_H > >> > >> -#include <linux/i2c.h> > >> -#include <linux/spi/spi.h> > >> #include <linux/irqreturn.h> > >> #include <linux/iio/iio.h> > >> #include <linux/iio/trigger.h> > > > > >
diff --git a/include/linux/iio/common/st_sensors.h b/include/linux/iio/common/st_sensors.h index 607c3a89a647..a1d3d57d6d6e 100644 --- a/include/linux/iio/common/st_sensors.h +++ b/include/linux/iio/common/st_sensors.h @@ -10,8 +10,6 @@ #ifndef ST_SENSORS_H #define ST_SENSORS_H -#include <linux/i2c.h> -#include <linux/spi/spi.h> #include <linux/irqreturn.h> #include <linux/iio/iio.h> #include <linux/iio/trigger.h>
There is no need to include i2c.h and spi/spi.h in st_sensors.h. Should it be needed, st_sensors_(i2c|spi).h should be used. Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> --- Based on one of my script, this reduces the number of included files from 573 files to 515 files when compiling drivers/iio/accel/st_accel_buffer.c --- include/linux/iio/common/st_sensors.h | 2 -- 1 file changed, 2 deletions(-)