[PATCH 1/2] iio: st_sensors: Remove some redundant includes

[PATCH 1/2] iio: st_sensors: Remove some redundant includes

[Christophe JAILLET]

st_sensors_(i2c|spi).h already include st_sensors.h, so there is no need
to include it explicitly.

Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
 drivers/iio/accel/st_accel_spi.c       | 1 -
 drivers/iio/gyro/st_gyro_i2c.c         | 1 -
 drivers/iio/gyro/st_gyro_spi.c         | 1 -
 drivers/iio/magnetometer/st_magn_i2c.c | 1 -
 drivers/iio/magnetometer/st_magn_spi.c | 1 -
 drivers/iio/pressure/st_pressure_i2c.c | 1 -
 drivers/iio/pressure/st_pressure_spi.c | 1 -
 7 files changed, 7 deletions(-)

diff --git a/drivers/iio/accel/st_accel_spi.c b/drivers/iio/accel/st_accel_spi.c
index f72a24f45322..ae5bf3b47209 100644
--- a/drivers/iio/accel/st_accel_spi.c
+++ b/drivers/iio/accel/st_accel_spi.c
@@ -13,7 +13,6 @@
 #include <linux/spi/spi.h>
 #include <linux/iio/iio.h>
 
-#include <linux/iio/common/st_sensors.h>
 #include <linux/iio/common/st_sensors_spi.h>
 #include "st_accel.h"
 
diff --git a/drivers/iio/gyro/st_gyro_i2c.c b/drivers/iio/gyro/st_gyro_i2c.c
index 5a10a3556ab0..ceebd246100c 100644
--- a/drivers/iio/gyro/st_gyro_i2c.c
+++ b/drivers/iio/gyro/st_gyro_i2c.c
@@ -13,7 +13,6 @@
 #include <linux/i2c.h>
 #include <linux/iio/iio.h>
 
-#include <linux/iio/common/st_sensors.h>
 #include <linux/iio/common/st_sensors_i2c.h>
 #include "st_gyro.h"
 
diff --git a/drivers/iio/gyro/st_gyro_spi.c b/drivers/iio/gyro/st_gyro_spi.c
index 22aaabe48e4a..afdb8dffd72d 100644
--- a/drivers/iio/gyro/st_gyro_spi.c
+++ b/drivers/iio/gyro/st_gyro_spi.c
@@ -13,7 +13,6 @@
 #include <linux/spi/spi.h>
 #include <linux/iio/iio.h>
 
-#include <linux/iio/common/st_sensors.h>
 #include <linux/iio/common/st_sensors_spi.h>
 #include "st_gyro.h"
 
diff --git a/drivers/iio/magnetometer/st_magn_i2c.c b/drivers/iio/magnetometer/st_magn_i2c.c
index 950826dd20bf..b9221794cf00 100644
--- a/drivers/iio/magnetometer/st_magn_i2c.c
+++ b/drivers/iio/magnetometer/st_magn_i2c.c
@@ -13,7 +13,6 @@
 #include <linux/i2c.h>
 #include <linux/iio/iio.h>
 
-#include <linux/iio/common/st_sensors.h>
 #include <linux/iio/common/st_sensors_i2c.h>
 #include "st_magn.h"
 
diff --git a/drivers/iio/magnetometer/st_magn_spi.c b/drivers/iio/magnetometer/st_magn_spi.c
index f203e1f87eec..5540500ba9fa 100644
--- a/drivers/iio/magnetometer/st_magn_spi.c
+++ b/drivers/iio/magnetometer/st_magn_spi.c
@@ -13,7 +13,6 @@
 #include <linux/spi/spi.h>
 #include <linux/iio/iio.h>
 
-#include <linux/iio/common/st_sensors.h>
 #include <linux/iio/common/st_sensors_spi.h>
 #include "st_magn.h"
 
diff --git a/drivers/iio/pressure/st_pressure_i2c.c b/drivers/iio/pressure/st_pressure_i2c.c
index 5101552e3f38..8ba0d1212401 100644
--- a/drivers/iio/pressure/st_pressure_i2c.c
+++ b/drivers/iio/pressure/st_pressure_i2c.c
@@ -14,7 +14,6 @@
 #include <linux/i2c.h>
 #include <linux/iio/iio.h>
 
-#include <linux/iio/common/st_sensors.h>
 #include <linux/iio/common/st_sensors_i2c.h>
 #include "st_pressure.h"
 
diff --git a/drivers/iio/pressure/st_pressure_spi.c b/drivers/iio/pressure/st_pressure_spi.c
index 25cca5ad7c55..ffa1970a5aeb 100644
--- a/drivers/iio/pressure/st_pressure_spi.c
+++ b/drivers/iio/pressure/st_pressure_spi.c
@@ -13,7 +13,6 @@
 #include <linux/spi/spi.h>
 #include <linux/iio/iio.h>
 
-#include <linux/iio/common/st_sensors.h>
 #include <linux/iio/common/st_sensors_spi.h>
 #include "st_pressure.h"
 
-- 
2.34.1

[PATCH 2/2] iio: st_sensors: Remove some redundant includes in st_sensors.h

[Christophe JAILLET]

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(-)

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>
-- 
2.34.1

Re: [PATCH 2/2] iio: st_sensors: Remove some redundant includes in st_sensors.h

[Jonathan Cameron]

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>

Re: [PATCH 2/2] iio: st_sensors: Remove some redundant includes in st_sensors.h

[Christophe JAILLET]

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>
> 
> 

Re: [PATCH 2/2] iio: st_sensors: Remove some redundant includes in st_sensors.h

[Jonathan Cameron]

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>  
> > 
> >   
> 

Re: [PATCH 1/2] iio: st_sensors: Remove some redundant includes

[Jonathan Cameron]

On Mon, 12 Jun 2023 22:13:36 +0200
Christophe JAILLET <christophe.jaillet@wanadoo.fr> wrote:

> st_sensors_(i2c|spi).h already include st_sensors.h, so there is no need
> to include it explicitly.
> 
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>

Mostly we try to avoid implicit includes that might not be around
for ever as it makes changing internals of headers have messy side effects.
In this particular case I'd argue that the _spi.h and _i2c.h should
not be including st_sensors.h rather than the other way around.

There is nothing in those headers that uses anything from
st_sensors.h

They should probably use forward definitions of
struct iio_dev and struct spi_device etc to avoid need to include
anything at all.

Jonathan
 


> ---
>  drivers/iio/accel/st_accel_spi.c       | 1 -
>  drivers/iio/gyro/st_gyro_i2c.c         | 1 -
>  drivers/iio/gyro/st_gyro_spi.c         | 1 -
>  drivers/iio/magnetometer/st_magn_i2c.c | 1 -
>  drivers/iio/magnetometer/st_magn_spi.c | 1 -
>  drivers/iio/pressure/st_pressure_i2c.c | 1 -
>  drivers/iio/pressure/st_pressure_spi.c | 1 -
>  7 files changed, 7 deletions(-)
> 
> diff --git a/drivers/iio/accel/st_accel_spi.c b/drivers/iio/accel/st_accel_spi.c
> index f72a24f45322..ae5bf3b47209 100644
> --- a/drivers/iio/accel/st_accel_spi.c
> +++ b/drivers/iio/accel/st_accel_spi.c
> @@ -13,7 +13,6 @@
>  #include <linux/spi/spi.h>
>  #include <linux/iio/iio.h>
>  
> -#include <linux/iio/common/st_sensors.h>
>  #include <linux/iio/common/st_sensors_spi.h>
>  #include "st_accel.h"
>  
> diff --git a/drivers/iio/gyro/st_gyro_i2c.c b/drivers/iio/gyro/st_gyro_i2c.c
> index 5a10a3556ab0..ceebd246100c 100644
> --- a/drivers/iio/gyro/st_gyro_i2c.c
> +++ b/drivers/iio/gyro/st_gyro_i2c.c
> @@ -13,7 +13,6 @@
>  #include <linux/i2c.h>
>  #include <linux/iio/iio.h>
>  
> -#include <linux/iio/common/st_sensors.h>
>  #include <linux/iio/common/st_sensors_i2c.h>
>  #include "st_gyro.h"
>  
> diff --git a/drivers/iio/gyro/st_gyro_spi.c b/drivers/iio/gyro/st_gyro_spi.c
> index 22aaabe48e4a..afdb8dffd72d 100644
> --- a/drivers/iio/gyro/st_gyro_spi.c
> +++ b/drivers/iio/gyro/st_gyro_spi.c
> @@ -13,7 +13,6 @@
>  #include <linux/spi/spi.h>
>  #include <linux/iio/iio.h>
>  
> -#include <linux/iio/common/st_sensors.h>
>  #include <linux/iio/common/st_sensors_spi.h>
>  #include "st_gyro.h"
>  
> diff --git a/drivers/iio/magnetometer/st_magn_i2c.c b/drivers/iio/magnetometer/st_magn_i2c.c
> index 950826dd20bf..b9221794cf00 100644
> --- a/drivers/iio/magnetometer/st_magn_i2c.c
> +++ b/drivers/iio/magnetometer/st_magn_i2c.c
> @@ -13,7 +13,6 @@
>  #include <linux/i2c.h>
>  #include <linux/iio/iio.h>
>  
> -#include <linux/iio/common/st_sensors.h>
>  #include <linux/iio/common/st_sensors_i2c.h>
>  #include "st_magn.h"
>  
> diff --git a/drivers/iio/magnetometer/st_magn_spi.c b/drivers/iio/magnetometer/st_magn_spi.c
> index f203e1f87eec..5540500ba9fa 100644
> --- a/drivers/iio/magnetometer/st_magn_spi.c
> +++ b/drivers/iio/magnetometer/st_magn_spi.c
> @@ -13,7 +13,6 @@
>  #include <linux/spi/spi.h>
>  #include <linux/iio/iio.h>
>  
> -#include <linux/iio/common/st_sensors.h>
>  #include <linux/iio/common/st_sensors_spi.h>
>  #include "st_magn.h"
>  
> diff --git a/drivers/iio/pressure/st_pressure_i2c.c b/drivers/iio/pressure/st_pressure_i2c.c
> index 5101552e3f38..8ba0d1212401 100644
> --- a/drivers/iio/pressure/st_pressure_i2c.c
> +++ b/drivers/iio/pressure/st_pressure_i2c.c
> @@ -14,7 +14,6 @@
>  #include <linux/i2c.h>
>  #include <linux/iio/iio.h>
>  
> -#include <linux/iio/common/st_sensors.h>
>  #include <linux/iio/common/st_sensors_i2c.h>
>  #include "st_pressure.h"
>  
> diff --git a/drivers/iio/pressure/st_pressure_spi.c b/drivers/iio/pressure/st_pressure_spi.c
> index 25cca5ad7c55..ffa1970a5aeb 100644
> --- a/drivers/iio/pressure/st_pressure_spi.c
> +++ b/drivers/iio/pressure/st_pressure_spi.c
> @@ -13,7 +13,6 @@
>  #include <linux/spi/spi.h>
>  #include <linux/iio/iio.h>
>  
> -#include <linux/iio/common/st_sensors.h>
>  #include <linux/iio/common/st_sensors_spi.h>
>  #include "st_pressure.h"
>