Re: [PATCH v5 3/6] staging: vc04_services: bcm2835-camera: Drop include Makefile directive

From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Umang Jain <umang.jain@ideasonboard.com>
Cc: linux-staging@lists.linux.dev,
	linux-rpi-kernel@lists.infradead.org,
	linux-arm-kernel@lists.infradead.org,
	linux-media@vger.kernel.org, linux-kernel@vger.kernel.org,
	Stefan Wahren <stefan.wahren@i2se.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Florian Fainelli <f.fainelli@gmail.com>,
	Adrien Thierry <athierry@redhat.com>,
	Dan Carpenter <error27@gmail.com>,
	Dave Stevenson <dave.stevenson@raspberrypi.com>,
	Kieran Bingham <kieran.bingham@ideasonboard.com>,
	Paul Elder <paul.elder@ideasonboard.com>
Subject: Re: [PATCH v5 3/6] staging: vc04_services: bcm2835-camera: Drop include Makefile directive
Date: Fri, 20 Jan 2023 03:32:00 +0200	[thread overview]
Message-ID: <Y8nvEC9EVdHfCuNq@pendragon.ideasonboard.com> (raw)
In-Reply-To: <20230119115503.268693-4-umang.jain@ideasonboard.com>

Hi Umang,

Thank you for the patch.

On Thu, Jan 19, 2023 at 05:25:00PM +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-camera files instead.

Same comment as in 2/6. I expect it applies to other patches in the
series too.

> Suggested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
> ---
>  drivers/staging/vc04_services/bcm2835-camera/Makefile  |  4 ----
>  .../vc04_services/bcm2835-camera/bcm2835-camera.c      | 10 +++++-----
>  .../staging/vc04_services/bcm2835-camera/controls.c    |  6 +++---
>  3 files changed, 8 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/staging/vc04_services/bcm2835-camera/Makefile b/drivers/staging/vc04_services/bcm2835-camera/Makefile
> index 3494c82b271a..203b93899b20 100644
> --- a/drivers/staging/vc04_services/bcm2835-camera/Makefile
> +++ b/drivers/staging/vc04_services/bcm2835-camera/Makefile
> @@ -4,7 +4,3 @@ bcm2835-v4l2-$(CONFIG_VIDEO_BCM2835) := \
>  	controls.o
>  
>  obj-$(CONFIG_VIDEO_BCM2835) += bcm2835-v4l2.o
> -
> -ccflags-y += \
> -	-I $(srctree)/$(src)/.. \
> -	-I $(srctree)/$(src)/../vchiq-mmal/
> diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
> index 797ebe2a973a..4f81765912ea 100644
> --- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
> +++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
> @@ -26,11 +26,11 @@
>  #include <linux/delay.h>
>  #include <linux/platform_device.h>
>  
> -#include "mmal-common.h"
> -#include "mmal-encodings.h"
> -#include "mmal-vchiq.h"
> -#include "mmal-msg.h"
> -#include "mmal-parameters.h"
> +#include "../vchiq-mmal/mmal-common.h"
> +#include "../vchiq-mmal/mmal-encodings.h"
> +#include "../vchiq-mmal/mmal-vchiq.h"
> +#include "../vchiq-mmal/mmal-msg.h"
> +#include "../vchiq-mmal/mmal-parameters.h"

Unlike the change in 2/6 that we may want to reconsider, this looks good
to me, even though it will be interesting to see what happens if we move
the vchiq core out of staging first. I expect in that case that the
headers will go to a directory under include/ (likely in
include/linux/soc/ ?), and this will be modified accordingly.

In the meantime, with the commit message updated,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

>  #include "bcm2835-camera.h"
>  
>  #define MIN_WIDTH 32
> diff --git a/drivers/staging/vc04_services/bcm2835-camera/controls.c b/drivers/staging/vc04_services/bcm2835-camera/controls.c
> index 5644d1d457b9..6bce45925bf1 100644
> --- a/drivers/staging/vc04_services/bcm2835-camera/controls.c
> +++ b/drivers/staging/vc04_services/bcm2835-camera/controls.c
> @@ -23,9 +23,9 @@
>  #include <media/v4l2-event.h>
>  #include <media/v4l2-common.h>
>  
> -#include "mmal-common.h"
> -#include "mmal-vchiq.h"
> -#include "mmal-parameters.h"
> +#include "../vchiq-mmal/mmal-common.h"
> +#include "../vchiq-mmal/mmal-vchiq.h"
> +#include "../vchiq-mmal/mmal-parameters.h"
>  #include "bcm2835-camera.h"
>  
>  /* The supported V4L2_CID_AUTO_EXPOSURE_BIAS values are from -4.0 to +4.0.

-- 
Regards,

Laurent Pinchart

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel