[PATCH v5 2/6] staging: vc04_services: bcm2835-audio: Drop include Makefile directive

Umang Jain posted 6 patches 3 years, 2 months ago
There is a newer version of this series
[PATCH v5 2/6] staging: vc04_services: bcm2835-audio: Drop include Makefile directive
Posted by Umang Jain 3 years, 2 months ago
Drop the include directive they can break the build one only wants to
build a subdirectory. Replace with "../" for the includes, in the
bcm2835.h instead.

Suggested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
---
 drivers/staging/vc04_services/bcm2835-audio/Makefile  | 2 --
 drivers/staging/vc04_services/bcm2835-audio/bcm2835.h | 3 ++-
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/vc04_services/bcm2835-audio/Makefile b/drivers/staging/vc04_services/bcm2835-audio/Makefile
index fc7ac6112a3e..01ceebdf88e7 100644
--- a/drivers/staging/vc04_services/bcm2835-audio/Makefile
+++ b/drivers/staging/vc04_services/bcm2835-audio/Makefile
@@ -1,5 +1,3 @@
 # SPDX-License-Identifier: GPL-2.0
 obj-$(CONFIG_SND_BCM2835)	+= snd-bcm2835.o
 snd-bcm2835-objs		:= bcm2835.o bcm2835-ctl.o bcm2835-pcm.o bcm2835-vchiq.o
-
-ccflags-y += -I $(srctree)/$(src)/../include
diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835.h b/drivers/staging/vc04_services/bcm2835-audio/bcm2835.h
index 38b7451d77b2..0a81383c475a 100644
--- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835.h
+++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835.h
@@ -6,11 +6,12 @@
 
 #include <linux/device.h>
 #include <linux/wait.h>
-#include <linux/raspberrypi/vchiq.h>
 #include <sound/core.h>
 #include <sound/pcm.h>
 #include <sound/pcm-indirect.h>
 
+#include "../include/linux/raspberrypi/vchiq.h"
+
 #define MAX_SUBSTREAMS   (8)
 #define AVAIL_SUBSTREAMS_MASK  (0xff)
 
-- 
2.38.1
Re: [PATCH v5 2/6] staging: vc04_services: bcm2835-audio: Drop include Makefile directive
Posted by Laurent Pinchart 3 years, 2 months ago
Hi Umang,

Thank you for the patch.

On Thu, Jan 19, 2023 at 05:24:59PM +0530, Umang Jain wrote:
> Drop the include directive they can break the build one only wants to
> build a subdirectory. Replace with "../" for the includes, in the
> bcm2835.h instead.

I assume you meant

Drop the include directive. They can break the build, when one only
wants to build a subdirectory.

> Suggested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
> ---
>  drivers/staging/vc04_services/bcm2835-audio/Makefile  | 2 --
>  drivers/staging/vc04_services/bcm2835-audio/bcm2835.h | 3 ++-
>  2 files changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/vc04_services/bcm2835-audio/Makefile b/drivers/staging/vc04_services/bcm2835-audio/Makefile
> index fc7ac6112a3e..01ceebdf88e7 100644
> --- a/drivers/staging/vc04_services/bcm2835-audio/Makefile
> +++ b/drivers/staging/vc04_services/bcm2835-audio/Makefile
> @@ -1,5 +1,3 @@
>  # SPDX-License-Identifier: GPL-2.0
>  obj-$(CONFIG_SND_BCM2835)	+= snd-bcm2835.o
>  snd-bcm2835-objs		:= bcm2835.o bcm2835-ctl.o bcm2835-pcm.o bcm2835-vchiq.o
> -
> -ccflags-y += -I $(srctree)/$(src)/../include

The reason for this, I assume, is that the driver is in staging. The
vchiq.h file should live in include/linux/raspberrypi/, not
drivers/staging/vc04_services/include/linux/raspberrypi/, so an
additional include directory is added in order to use

#include <linux/raspberrypi/vchiq.h>

When the code will get out of staging, vchiq.h will go to
include/linux/raspberrypi/, the extra include directory will be dropped,
and all will be well without having to change any source file.

With this patch, we'll have to undo the change below to
drivers/staging/vc04_services/bcm2835-audio/bcm2835.h when vc04_services
will get out of staging.

Greg, is that what you prefer ?

> diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835.h b/drivers/staging/vc04_services/bcm2835-audio/bcm2835.h
> index 38b7451d77b2..0a81383c475a 100644
> --- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835.h
> +++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835.h
> @@ -6,11 +6,12 @@
>  
>  #include <linux/device.h>
>  #include <linux/wait.h>
> -#include <linux/raspberrypi/vchiq.h>
>  #include <sound/core.h>
>  #include <sound/pcm.h>
>  #include <sound/pcm-indirect.h>
>  
> +#include "../include/linux/raspberrypi/vchiq.h"
> +
>  #define MAX_SUBSTREAMS   (8)
>  #define AVAIL_SUBSTREAMS_MASK  (0xff)
>  

-- 
Regards,

Laurent Pinchart
Re: [PATCH v5 2/6] staging: vc04_services: bcm2835-audio: Drop include Makefile directive
Posted by Greg Kroah-Hartman 3 years, 2 months ago
On Fri, Jan 20, 2023 at 03:28:05AM +0200, Laurent Pinchart wrote:
> Hi Umang,
> 
> Thank you for the patch.
> 
> On Thu, Jan 19, 2023 at 05:24:59PM +0530, Umang Jain wrote:
> > Drop the include directive they can break the build one only wants to
> > build a subdirectory. Replace with "../" for the includes, in the
> > bcm2835.h instead.
> 
> I assume you meant
> 
> Drop the include directive. They can break the build, when one only
> wants to build a subdirectory.
> 
> > Suggested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
> > ---
> >  drivers/staging/vc04_services/bcm2835-audio/Makefile  | 2 --
> >  drivers/staging/vc04_services/bcm2835-audio/bcm2835.h | 3 ++-
> >  2 files changed, 2 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/staging/vc04_services/bcm2835-audio/Makefile b/drivers/staging/vc04_services/bcm2835-audio/Makefile
> > index fc7ac6112a3e..01ceebdf88e7 100644
> > --- a/drivers/staging/vc04_services/bcm2835-audio/Makefile
> > +++ b/drivers/staging/vc04_services/bcm2835-audio/Makefile
> > @@ -1,5 +1,3 @@
> >  # SPDX-License-Identifier: GPL-2.0
> >  obj-$(CONFIG_SND_BCM2835)	+= snd-bcm2835.o
> >  snd-bcm2835-objs		:= bcm2835.o bcm2835-ctl.o bcm2835-pcm.o bcm2835-vchiq.o
> > -
> > -ccflags-y += -I $(srctree)/$(src)/../include
> 
> The reason for this, I assume, is that the driver is in staging. The
> vchiq.h file should live in include/linux/raspberrypi/, not
> drivers/staging/vc04_services/include/linux/raspberrypi/, so an
> additional include directory is added in order to use
> 
> #include <linux/raspberrypi/vchiq.h>
> 
> When the code will get out of staging, vchiq.h will go to
> include/linux/raspberrypi/, the extra include directory will be dropped,
> and all will be well without having to change any source file.
> 
> With this patch, we'll have to undo the change below to
> drivers/staging/vc04_services/bcm2835-audio/bcm2835.h when vc04_services
> will get out of staging.
> 
> Greg, is that what you prefer ?

I prefer the drivers to NOT use include ccflags in the kernel as it
breaks the build when trying to build just a subdirectory.  If/when this
code ever gets out of staging, then the include lines can be fixed up to
point to the correct location of wherever the files move to.

So I like this patch, but as I couldn't take the first one, this and the
rest did not apply so I'll wait for them to be resubmitted.

thanks,

greg k-h