diff mbox series

media: i2c: Kconfig: Make MAX9271 a module

Message ID 20210208182006.178740-1-jacopo+renesas@jmondi.org (mailing list archive)
State Superseded
Delegated to: Kieran Bingham
Headers show
Series media: i2c: Kconfig: Make MAX9271 a module | expand

Commit Message

Jacopo Mondi Feb. 8, 2021, 6:20 p.m. UTC 
With the introduction of the RDACM21 camera module support in
commit a59f853b3b4b ("media: i2c: Add driver for RDACM21 camera module")
the symbols defined by the max9271 library were exported twice
if multiple users of the library were compiled in at the same time.

In example:
WARNING: modpost: drivers/media/i2c/rdacm21-camera_module:
'max9271_set_serial_link' exported twice. Previous export was in
drivers/media/i2c/rdacm20-camera_module.ko

Fix this by making the rdacm21 file a module and have the driver
using its functions select it.

Fixes: a59f853b3b4b ("media: i2c: Add driver for RDACM21 camera module")
Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Suggested-by: Mauro Carvalho Chehab <mchehab@kernel.org>
Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 drivers/media/i2c/Kconfig  | 5 +++++
 drivers/media/i2c/Makefile | 7 +++----
 2 files changed, 8 insertions(+), 4 deletions(-)

--
2.30.0

Comments

Sakari Ailus Feb. 8, 2021, 8:21 p.m. UTC | #1 
Hi Jacopo,

On Mon, Feb 08, 2021 at 07:20:06PM +0100, Jacopo Mondi wrote:
> With the introduction of the RDACM21 camera module support in
> commit a59f853b3b4b ("media: i2c: Add driver for RDACM21 camera module")
> the symbols defined by the max9271 library were exported twice
> if multiple users of the library were compiled in at the same time.
> 
> In example:
> WARNING: modpost: drivers/media/i2c/rdacm21-camera_module:
> 'max9271_set_serial_link' exported twice. Previous export was in
> drivers/media/i2c/rdacm20-camera_module.ko
> 
> Fix this by making the rdacm21 file a module and have the driver
> using its functions select it.
> 
> Fixes: a59f853b3b4b ("media: i2c: Add driver for RDACM21 camera module")
> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> Suggested-by: Mauro Carvalho Chehab <mchehab@kernel.org>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
>  drivers/media/i2c/Kconfig  | 5 +++++
>  drivers/media/i2c/Makefile | 7 +++----
>  2 files changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index 2d3dc0d82f9e..84645f751da3 100644
> --- a/drivers/media/i2c/Kconfig
> +++ b/drivers/media/i2c/Kconfig
> @@ -1240,12 +1240,16 @@ config VIDEO_NOON010PC30
> 
>  source "drivers/media/i2c/m5mols/Kconfig"
> 
> +config VIDEO_MAX9271

How about calling this VIDEO_MAX9271_HELPER instead? It's not a driver in
the proper sense of the word.

Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>

> +	tristate
> +
>  config VIDEO_RDACM20
>  	tristate "IMI RDACM20 camera support"
>  	depends on I2C
>  	select V4L2_FWNODE
>  	select VIDEO_V4L2_SUBDEV_API
>  	select MEDIA_CONTROLLER
> +	select VIDEO_MAX9271
>  	help
>  	  This driver supports the IMI RDACM20 GMSL camera, used in
>  	  ADAS systems.
> @@ -1259,6 +1263,7 @@ config VIDEO_RDACM21
>  	select V4L2_FWNODE
>  	select VIDEO_V4L2_SUBDEV_API
>  	select MEDIA_CONTROLLER
> +	select VIDEO_MAX9271
>  	help
>  	  This driver supports the IMI RDACM21 GMSL camera, used in
>  	  ADAS systems.
> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
> index 6bd22d63e1a7..c34a7de3158b 100644
> --- a/drivers/media/i2c/Makefile
> +++ b/drivers/media/i2c/Makefile
> @@ -125,10 +125,9 @@ obj-$(CONFIG_VIDEO_IMX319)	+= imx319.o
>  obj-$(CONFIG_VIDEO_IMX334)	+= imx334.o
>  obj-$(CONFIG_VIDEO_IMX355)	+= imx355.o
>  obj-$(CONFIG_VIDEO_MAX9286)	+= max9286.o
> -rdacm20-camera_module-objs	:= rdacm20.o max9271.o
> -obj-$(CONFIG_VIDEO_RDACM20)	+= rdacm20-camera_module.o
> -rdacm21-camera_module-objs	:= rdacm21.o max9271.o
> -obj-$(CONFIG_VIDEO_RDACM21)	+= rdacm21-camera_module.o
> +obj-$(CONFIG_VIDEO_MAX9271)	+= max9271.o
> +obj-$(CONFIG_VIDEO_RDACM20)	+= rdacm20.o
> +obj-$(CONFIG_VIDEO_RDACM21)	+= rdacm21.o
>  obj-$(CONFIG_VIDEO_ST_MIPID02) += st-mipid02.o
> 
>  obj-$(CONFIG_SDR_MAX2175) += max2175.o
Geert Uytterhoeven Feb. 8, 2021, 8:42 p.m. UTC | #2 
Hi Jacopo,

Thanks for your patch!

On Mon, Feb 8, 2021 at 7:22 PM Jacopo Mondi <jacopo+renesas@jmondi.org> wrote:
> With the introduction of the RDACM21 camera module support in
> commit a59f853b3b4b ("media: i2c: Add driver for RDACM21 camera module")
> the symbols defined by the max9271 library were exported twice
> if multiple users of the library were compiled in at the same time.
>
> In example:
> WARNING: modpost: drivers/media/i2c/rdacm21-camera_module:
> 'max9271_set_serial_link' exported twice. Previous export was in
> drivers/media/i2c/rdacm20-camera_module.ko
>
> Fix this by making the rdacm21 file a module and have the driver

max9271

> using its functions select it.
>
> Fixes: a59f853b3b4b ("media: i2c: Add driver for RDACM21 camera module")
> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> Suggested-by: Mauro Carvalho Chehab <mchehab@kernel.org>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

With the above fixed:
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert


--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Laurent Pinchart Feb. 8, 2021, 10:26 p.m. UTC | #3 
Hi Sakari and Jacopo,

On Mon, Feb 08, 2021 at 10:21:47PM +0200, Sakari Ailus wrote:
> On Mon, Feb 08, 2021 at 07:20:06PM +0100, Jacopo Mondi wrote:
> > With the introduction of the RDACM21 camera module support in
> > commit a59f853b3b4b ("media: i2c: Add driver for RDACM21 camera module")
> > the symbols defined by the max9271 library were exported twice
> > if multiple users of the library were compiled in at the same time.
> > 
> > In example:
> > WARNING: modpost: drivers/media/i2c/rdacm21-camera_module:
> > 'max9271_set_serial_link' exported twice. Previous export was in
> > drivers/media/i2c/rdacm20-camera_module.ko
> > 
> > Fix this by making the rdacm21 file a module and have the driver
> > using its functions select it.
> > 
> > Fixes: a59f853b3b4b ("media: i2c: Add driver for RDACM21 camera module")
> > Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> > Suggested-by: Mauro Carvalho Chehab <mchehab@kernel.org>
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > ---
> >  drivers/media/i2c/Kconfig  | 5 +++++
> >  drivers/media/i2c/Makefile | 7 +++----
> >  2 files changed, 8 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> > index 2d3dc0d82f9e..84645f751da3 100644
> > --- a/drivers/media/i2c/Kconfig
> > +++ b/drivers/media/i2c/Kconfig
> > @@ -1240,12 +1240,16 @@ config VIDEO_NOON010PC30
> > 
> >  source "drivers/media/i2c/m5mols/Kconfig"
> > 
> > +config VIDEO_MAX9271
> 
> How about calling this VIDEO_MAX9271_HELPER instead? It's not a driver in
> the proper sense of the word.

Not all Kconfig symbols refer to drivers. Should we rename V4L2_FWNODE
to V4L2_FWNODE_HELPER ? :-)

Of course the MAX9271 name may lead someone to believe that the symbol
refers to a driver. If you think we should really make this explicit,
I'd have a preference for LIB instead of HELPER.

Either way,

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

> Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> 
> > +	tristate
> > +
> >  config VIDEO_RDACM20
> >  	tristate "IMI RDACM20 camera support"
> >  	depends on I2C
> >  	select V4L2_FWNODE
> >  	select VIDEO_V4L2_SUBDEV_API
> >  	select MEDIA_CONTROLLER
> > +	select VIDEO_MAX9271
> >  	help
> >  	  This driver supports the IMI RDACM20 GMSL camera, used in
> >  	  ADAS systems.
> > @@ -1259,6 +1263,7 @@ config VIDEO_RDACM21
> >  	select V4L2_FWNODE
> >  	select VIDEO_V4L2_SUBDEV_API
> >  	select MEDIA_CONTROLLER
> > +	select VIDEO_MAX9271
> >  	help
> >  	  This driver supports the IMI RDACM21 GMSL camera, used in
> >  	  ADAS systems.
> > diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
> > index 6bd22d63e1a7..c34a7de3158b 100644
> > --- a/drivers/media/i2c/Makefile
> > +++ b/drivers/media/i2c/Makefile
> > @@ -125,10 +125,9 @@ obj-$(CONFIG_VIDEO_IMX319)	+= imx319.o
> >  obj-$(CONFIG_VIDEO_IMX334)	+= imx334.o
> >  obj-$(CONFIG_VIDEO_IMX355)	+= imx355.o
> >  obj-$(CONFIG_VIDEO_MAX9286)	+= max9286.o
> > -rdacm20-camera_module-objs	:= rdacm20.o max9271.o
> > -obj-$(CONFIG_VIDEO_RDACM20)	+= rdacm20-camera_module.o
> > -rdacm21-camera_module-objs	:= rdacm21.o max9271.o
> > -obj-$(CONFIG_VIDEO_RDACM21)	+= rdacm21-camera_module.o
> > +obj-$(CONFIG_VIDEO_MAX9271)	+= max9271.o
> > +obj-$(CONFIG_VIDEO_RDACM20)	+= rdacm20.o
> > +obj-$(CONFIG_VIDEO_RDACM21)	+= rdacm21.o
> >  obj-$(CONFIG_VIDEO_ST_MIPID02) += st-mipid02.o
> > 
> >  obj-$(CONFIG_SDR_MAX2175) += max2175.o
Sakari Ailus Feb. 9, 2021, 8:48 a.m. UTC | #4 
On Tue, Feb 09, 2021 at 12:26:33AM +0200, Laurent Pinchart wrote:
> Hi Sakari and Jacopo,
> 
> On Mon, Feb 08, 2021 at 10:21:47PM +0200, Sakari Ailus wrote:
> > On Mon, Feb 08, 2021 at 07:20:06PM +0100, Jacopo Mondi wrote:
> > > With the introduction of the RDACM21 camera module support in
> > > commit a59f853b3b4b ("media: i2c: Add driver for RDACM21 camera module")
> > > the symbols defined by the max9271 library were exported twice
> > > if multiple users of the library were compiled in at the same time.
> > > 
> > > In example:
> > > WARNING: modpost: drivers/media/i2c/rdacm21-camera_module:
> > > 'max9271_set_serial_link' exported twice. Previous export was in
> > > drivers/media/i2c/rdacm20-camera_module.ko
> > > 
> > > Fix this by making the rdacm21 file a module and have the driver
> > > using its functions select it.
> > > 
> > > Fixes: a59f853b3b4b ("media: i2c: Add driver for RDACM21 camera module")
> > > Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> > > Suggested-by: Mauro Carvalho Chehab <mchehab@kernel.org>
> > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > > ---
> > >  drivers/media/i2c/Kconfig  | 5 +++++
> > >  drivers/media/i2c/Makefile | 7 +++----
> > >  2 files changed, 8 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> > > index 2d3dc0d82f9e..84645f751da3 100644
> > > --- a/drivers/media/i2c/Kconfig
> > > +++ b/drivers/media/i2c/Kconfig
> > > @@ -1240,12 +1240,16 @@ config VIDEO_NOON010PC30
> > > 
> > >  source "drivers/media/i2c/m5mols/Kconfig"
> > > 
> > > +config VIDEO_MAX9271
> > 
> > How about calling this VIDEO_MAX9271_HELPER instead? It's not a driver in
> > the proper sense of the word.
> 
> Not all Kconfig symbols refer to drivers. Should we rename V4L2_FWNODE
> to V4L2_FWNODE_HELPER ? :-)
> 
> Of course the MAX9271 name may lead someone to believe that the symbol
> refers to a driver. If you think we should really make this explicit,
> I'd have a preference for LIB instead of HELPER.

LIB sounds good to me, too.

> 
> Either way,
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> > Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > 
> > > +	tristate
> > > +
> > >  config VIDEO_RDACM20
> > >  	tristate "IMI RDACM20 camera support"
> > >  	depends on I2C
> > >  	select V4L2_FWNODE
> > >  	select VIDEO_V4L2_SUBDEV_API
> > >  	select MEDIA_CONTROLLER
> > > +	select VIDEO_MAX9271
> > >  	help
> > >  	  This driver supports the IMI RDACM20 GMSL camera, used in
> > >  	  ADAS systems.
> > > @@ -1259,6 +1263,7 @@ config VIDEO_RDACM21
> > >  	select V4L2_FWNODE
> > >  	select VIDEO_V4L2_SUBDEV_API
> > >  	select MEDIA_CONTROLLER
> > > +	select VIDEO_MAX9271
> > >  	help
> > >  	  This driver supports the IMI RDACM21 GMSL camera, used in
> > >  	  ADAS systems.
> > > diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
> > > index 6bd22d63e1a7..c34a7de3158b 100644
> > > --- a/drivers/media/i2c/Makefile
> > > +++ b/drivers/media/i2c/Makefile
> > > @@ -125,10 +125,9 @@ obj-$(CONFIG_VIDEO_IMX319)	+= imx319.o
> > >  obj-$(CONFIG_VIDEO_IMX334)	+= imx334.o
> > >  obj-$(CONFIG_VIDEO_IMX355)	+= imx355.o
> > >  obj-$(CONFIG_VIDEO_MAX9286)	+= max9286.o
> > > -rdacm20-camera_module-objs	:= rdacm20.o max9271.o
> > > -obj-$(CONFIG_VIDEO_RDACM20)	+= rdacm20-camera_module.o
> > > -rdacm21-camera_module-objs	:= rdacm21.o max9271.o
> > > -obj-$(CONFIG_VIDEO_RDACM21)	+= rdacm21-camera_module.o
> > > +obj-$(CONFIG_VIDEO_MAX9271)	+= max9271.o
> > > +obj-$(CONFIG_VIDEO_RDACM20)	+= rdacm20.o
> > > +obj-$(CONFIG_VIDEO_RDACM21)	+= rdacm21.o
> > >  obj-$(CONFIG_VIDEO_ST_MIPID02) += st-mipid02.o
> > > 
> > >  obj-$(CONFIG_SDR_MAX2175) += max2175.o
> 
> -- 
> Regards,
> 
> Laurent Pinchart
Nathan Chancellor Feb. 9, 2021, 11:24 p.m. UTC | #5 
On Mon, Feb 08, 2021 at 07:20:06PM +0100, Jacopo Mondi wrote:
> With the introduction of the RDACM21 camera module support in
> commit a59f853b3b4b ("media: i2c: Add driver for RDACM21 camera module")
> the symbols defined by the max9271 library were exported twice
> if multiple users of the library were compiled in at the same time.
> 
> In example:
> WARNING: modpost: drivers/media/i2c/rdacm21-camera_module:
> 'max9271_set_serial_link' exported twice. Previous export was in
> drivers/media/i2c/rdacm20-camera_module.ko
> 
> Fix this by making the rdacm21 file a module and have the driver
> using its functions select it.
> 
> Fixes: a59f853b3b4b ("media: i2c: Add driver for RDACM21 camera module")
> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> Suggested-by: Mauro Carvalho Chehab <mchehab@kernel.org>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

That file needs a MODULE_LICENSE now otherwise there will be a fatal
build error:

ERROR: modpost: missing MODULE_LICENSE() in drivers/media/i2c/max9271.o

Cheers,
Nathan

> ---
>  drivers/media/i2c/Kconfig  | 5 +++++
>  drivers/media/i2c/Makefile | 7 +++----
>  2 files changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index 2d3dc0d82f9e..84645f751da3 100644
> --- a/drivers/media/i2c/Kconfig
> +++ b/drivers/media/i2c/Kconfig
> @@ -1240,12 +1240,16 @@ config VIDEO_NOON010PC30
> 
>  source "drivers/media/i2c/m5mols/Kconfig"
> 
> +config VIDEO_MAX9271
> +	tristate
> +
>  config VIDEO_RDACM20
>  	tristate "IMI RDACM20 camera support"
>  	depends on I2C
>  	select V4L2_FWNODE
>  	select VIDEO_V4L2_SUBDEV_API
>  	select MEDIA_CONTROLLER
> +	select VIDEO_MAX9271
>  	help
>  	  This driver supports the IMI RDACM20 GMSL camera, used in
>  	  ADAS systems.
> @@ -1259,6 +1263,7 @@ config VIDEO_RDACM21
>  	select V4L2_FWNODE
>  	select VIDEO_V4L2_SUBDEV_API
>  	select MEDIA_CONTROLLER
> +	select VIDEO_MAX9271
>  	help
>  	  This driver supports the IMI RDACM21 GMSL camera, used in
>  	  ADAS systems.
> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
> index 6bd22d63e1a7..c34a7de3158b 100644
> --- a/drivers/media/i2c/Makefile
> +++ b/drivers/media/i2c/Makefile
> @@ -125,10 +125,9 @@ obj-$(CONFIG_VIDEO_IMX319)	+= imx319.o
>  obj-$(CONFIG_VIDEO_IMX334)	+= imx334.o
>  obj-$(CONFIG_VIDEO_IMX355)	+= imx355.o
>  obj-$(CONFIG_VIDEO_MAX9286)	+= max9286.o
> -rdacm20-camera_module-objs	:= rdacm20.o max9271.o
> -obj-$(CONFIG_VIDEO_RDACM20)	+= rdacm20-camera_module.o
> -rdacm21-camera_module-objs	:= rdacm21.o max9271.o
> -obj-$(CONFIG_VIDEO_RDACM21)	+= rdacm21-camera_module.o
> +obj-$(CONFIG_VIDEO_MAX9271)	+= max9271.o
> +obj-$(CONFIG_VIDEO_RDACM20)	+= rdacm20.o
> +obj-$(CONFIG_VIDEO_RDACM21)	+= rdacm21.o
>  obj-$(CONFIG_VIDEO_ST_MIPID02) += st-mipid02.o
> 
>  obj-$(CONFIG_SDR_MAX2175) += max2175.o
> --
> 2.30.0
>
diff mbox series

Patch

diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 2d3dc0d82f9e..84645f751da3 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -1240,12 +1240,16 @@  config VIDEO_NOON010PC30

 source "drivers/media/i2c/m5mols/Kconfig"

+config VIDEO_MAX9271
+	tristate
+
 config VIDEO_RDACM20
 	tristate "IMI RDACM20 camera support"
 	depends on I2C
 	select V4L2_FWNODE
 	select VIDEO_V4L2_SUBDEV_API
 	select MEDIA_CONTROLLER
+	select VIDEO_MAX9271
 	help
 	  This driver supports the IMI RDACM20 GMSL camera, used in
 	  ADAS systems.
@@ -1259,6 +1263,7 @@  config VIDEO_RDACM21
 	select V4L2_FWNODE
 	select VIDEO_V4L2_SUBDEV_API
 	select MEDIA_CONTROLLER
+	select VIDEO_MAX9271
 	help
 	  This driver supports the IMI RDACM21 GMSL camera, used in
 	  ADAS systems.
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index 6bd22d63e1a7..c34a7de3158b 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -125,10 +125,9 @@  obj-$(CONFIG_VIDEO_IMX319)	+= imx319.o
 obj-$(CONFIG_VIDEO_IMX334)	+= imx334.o
 obj-$(CONFIG_VIDEO_IMX355)	+= imx355.o
 obj-$(CONFIG_VIDEO_MAX9286)	+= max9286.o
-rdacm20-camera_module-objs	:= rdacm20.o max9271.o
-obj-$(CONFIG_VIDEO_RDACM20)	+= rdacm20-camera_module.o
-rdacm21-camera_module-objs	:= rdacm21.o max9271.o
-obj-$(CONFIG_VIDEO_RDACM21)	+= rdacm21-camera_module.o
+obj-$(CONFIG_VIDEO_MAX9271)	+= max9271.o
+obj-$(CONFIG_VIDEO_RDACM20)	+= rdacm20.o
+obj-$(CONFIG_VIDEO_RDACM21)	+= rdacm21.o
 obj-$(CONFIG_VIDEO_ST_MIPID02) += st-mipid02.o

 obj-$(CONFIG_SDR_MAX2175) += max2175.o