lkml.org 
[lkml]   [2019]   [May]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] media: atmel: atmel-isc: split driver into driver base and isc
From
Date
On 5/9/19 11:09 AM, Eugen.Hristev@microchip.com wrote:
> From: Eugen Hristev <eugen.hristev@microchip.com>
>
> This splits the Atmel ISC driver into a common base: atmel-isc-base.c
> and the driver probe/dt part , atmel-isc.c
> This is needed to keep a common ground for the sensor controller which will
> be reused.
> The atmel-isc will use the common symbols inside the atmel-isc-base
> Future driver will also use the same symbols and redefine different aspects,
> for a different version of the ISC.
> This is done to avoid complete code duplication by creating a totally
> different driver for the new variant of the ISC.
>
> Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
> ---
>
> Hello,
>
> This patch applies on top of my other patches:
>
> media: atmel: atmel-isc: make try_fmt error less verbose
> media: atmel: atmel-isc: add support for DO_WHITE_BALANCE
> media: atmel: atmel-isc: reworked white balance feature
>
> Thanks !
>
> MAINTAINERS | 2 +
> drivers/media/platform/atmel/Makefile | 1 +
> drivers/media/platform/atmel/atmel-isc-base.c | 2144 +++++++++++++++++++++++
> drivers/media/platform/atmel/atmel-isc.c | 2311 +------------------------
> drivers/media/platform/atmel/atmel-isc.h | 210 +++
> 5 files changed, 2372 insertions(+), 2296 deletions(-)
> create mode 100644 drivers/media/platform/atmel/atmel-isc-base.c
> create mode 100644 drivers/media/platform/atmel/atmel-isc.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 878588c..8799779 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -10110,6 +10110,8 @@ M: Eugen Hristev <eugen.hristev@microchip.com>
> L: linux-media@vger.kernel.org
> S: Supported
> F: drivers/media/platform/atmel/atmel-isc.c
> +F: drivers/media/platform/atmel/atmel-isc.h
> +F: drivers/media/platform/atmel/atmel-isc-base.c
> F: drivers/media/platform/atmel/atmel-isc-regs.h
> F: Documentation/devicetree/bindings/media/atmel-isc.txt
>
> diff --git a/drivers/media/platform/atmel/Makefile b/drivers/media/platform/atmel/Makefile
> index 27000d0..9c8566e 100644
> --- a/drivers/media/platform/atmel/Makefile
> +++ b/drivers/media/platform/atmel/Makefile
> @@ -1,2 +1,3 @@
> obj-$(CONFIG_VIDEO_ATMEL_ISC) += atmel-isc.o
> obj-$(CONFIG_VIDEO_ATMEL_ISI) += atmel-isi.o
> +obj-$(CONFIG_VIDEO_ATMEL_ISC) += atmel-isc-base.o

This doesn't work. If a module consists of two sources, then you must make sure
those sources do not have the same name as the module.

So you have to rename atmel-isc.c.

See e.g. drivers/media/platform/rcar-vin/Makefile on how that's done for
CONFIG_VIDEO_RCAR_VIN.

<snip>

> diff --git a/drivers/media/platform/atmel/atmel-isc.h b/drivers/media/platform/atmel/atmel-isc.h
> new file mode 100644
> index 0000000..1e285f1
> --- /dev/null
> +++ b/drivers/media/platform/atmel/atmel-isc.h
> @@ -0,0 +1,210 @@

<snip>

> +
> +#if defined(CONFIG_VIDEO_ATMEL_ISC)

Why this #if???

> +#define ATMEL_ISC_NAME "atmel-isc"
> +#endif
> +
> +#endif
>

If I configure atmel-isc as a module, then I get these errors:

CC [M] drivers/media/platform/atmel/atmel-isc-base.o
drivers/media/platform/atmel/atmel-isc-base.c: In function ‘isc_querycap’:
drivers/media/platform/atmel/atmel-isc-base.c:906:23: error: ‘ATMEL_ISC_NAME’ undeclared (first use in this function); did you mean ‘CTL_MAXNAME’?

strscpy(cap->driver, ATMEL_ISC_NAME, sizeof(cap->driver));
^~~~~~~~~~~~~~
CTL_MAXNAME
drivers/media/platform/atmel/atmel-isc-base.c:906:23: note: each undeclared identifier is reported only once for each function it appears in

drivers/media/platform/atmel/atmel-isc-base.c: In function ‘isc_async_complete’:
drivers/media/platform/atmel/atmel-isc-base.c:2063:22: error: ‘ATMEL_ISC_NAME’ undeclared (first use in this function); did you mean ‘CTL_MAXNAME’?

strscpy(vdev->name, ATMEL_ISC_NAME, sizeof(vdev->name));
^~~~~~~~~~~~~~
CTL_MAXNAME
drivers/media/platform/atmel/atmel-isc.c: In function ‘atmel_isc_probe’:
drivers/media/platform/atmel/atmel-isc.c:180:11: error: ‘ATMEL_ISC_NAME’ undeclared (first use in this function); did you mean ‘CTL_MAXNAME’?

ATMEL_ISC_NAME, isc);
^~~~~~~~~~~~~~
CTL_MAXNAME

Probably related to one or both of my comments above.

Regards,

Hans

\
 
 \ /
  Last update: 2019-05-29 11:41    [W:0.070 / U:1.052 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site