[PATCH] cedrus: Drop unneeded CONFIG_OF dependency

[PATCH] cedrus: Drop unneeded CONFIG_OF dependency

[Ezequiel Garcia]

The driver is perfectly capable of being built without CONFIG_OF.
Remove this dependency, which is useful for compile-only tests.

Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
 drivers/staging/media/sunxi/cedrus/Kconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/media/sunxi/cedrus/Kconfig b/drivers/staging/media/sunxi/cedrus/Kconfig
index 17733e9a088f..59b8d1b29865 100644
--- a/drivers/staging/media/sunxi/cedrus/Kconfig
+++ b/drivers/staging/media/sunxi/cedrus/Kconfig
@@ -3,7 +3,6 @@ config VIDEO_SUNXI_CEDRUS
 	tristate "Allwinner Cedrus VPU driver"
 	depends on VIDEO_DEV && VIDEO_V4L2 && MEDIA_CONTROLLER
 	depends on HAS_DMA
-	depends on OF
 	depends on MEDIA_CONTROLLER_REQUEST_API
 	select SUNXI_SRAM
 	select VIDEOBUF2_DMA_CONTIG
-- 
2.26.0.rc2

Re: [PATCH] cedrus: Drop unneeded CONFIG_OF dependency

[Paul Kocialkowski]

[-- Attachment #1: Type: text/plain, Size: 1271 bytes --]

Hi Ezequiel,

On Thu 02 Apr 20, 16:46, Ezequiel Garcia wrote:
> The driver is perfectly capable of being built without CONFIG_OF.
> Remove this dependency, which is useful for compile-only tests.

Thanks for the patch!

Alas, the driver won't do anything useful without OF enabled, so it seems
useful to keep that dependency.

I would suggest making this a: depends on OF || COMPILE_TEST
instead. What do you think?

Cheers,

Paul

> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> ---
>  drivers/staging/media/sunxi/cedrus/Kconfig | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/staging/media/sunxi/cedrus/Kconfig b/drivers/staging/media/sunxi/cedrus/Kconfig
> index 17733e9a088f..59b8d1b29865 100644
> --- a/drivers/staging/media/sunxi/cedrus/Kconfig
> +++ b/drivers/staging/media/sunxi/cedrus/Kconfig
> @@ -3,7 +3,6 @@ config VIDEO_SUNXI_CEDRUS
>  	tristate "Allwinner Cedrus VPU driver"
>  	depends on VIDEO_DEV && VIDEO_V4L2 && MEDIA_CONTROLLER
>  	depends on HAS_DMA
> -	depends on OF
>  	depends on MEDIA_CONTROLLER_REQUEST_API
>  	select SUNXI_SRAM
>  	select VIDEOBUF2_DMA_CONTIG
> -- 
> 2.26.0.rc2
> 

-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] cedrus: Drop unneeded CONFIG_OF dependency

[Ezequiel Garcia]

On Fri, 2020-04-03 at 10:22 +0200, Paul Kocialkowski wrote:
> Hi Ezequiel,
> 
> On Thu 02 Apr 20, 16:46, Ezequiel Garcia wrote:
> > The driver is perfectly capable of being built without CONFIG_OF.
> > Remove this dependency, which is useful for compile-only tests.
> 
> Thanks for the patch!
> 
> Alas, the driver won't do anything useful without OF enabled, so it seems
> useful to keep that dependency.
> 

I'm not convinced about this argument, I like to have
dependencies on things the driver needs to compile
(except the machine option, see below).

> I would suggest making this a: depends on OF || COMPILE_TEST
> instead. What do you think?
> 

Although there are a handful, I don't see many drivers doing that.
To be honest, I don't like this much.

Also, if you want to make sure this driver can only be selected
where it makes sense, you consider having a ARCH_SUNXI || COMPILE_TEST.

IMO, the dependency on OF should be dropped as well.

Cheers,
Eze

> Cheers,
> 
> Paul
> 
> > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> > ---
> >  drivers/staging/media/sunxi/cedrus/Kconfig | 1 -
> >  1 file changed, 1 deletion(-)
> > 
> > diff --git a/drivers/staging/media/sunxi/cedrus/Kconfig b/drivers/staging/media/sunxi/cedrus/Kconfig
> > index 17733e9a088f..59b8d1b29865 100644
> > --- a/drivers/staging/media/sunxi/cedrus/Kconfig
> > +++ b/drivers/staging/media/sunxi/cedrus/Kconfig
> > @@ -3,7 +3,6 @@ config VIDEO_SUNXI_CEDRUS
> >  	tristate "Allwinner Cedrus VPU driver"
> >  	depends on VIDEO_DEV && VIDEO_V4L2 && MEDIA_CONTROLLER
> >  	depends on HAS_DMA
> > -	depends on OF
> >  	depends on MEDIA_CONTROLLER_REQUEST_API
> >  	select SUNXI_SRAM
> >  	select VIDEOBUF2_DMA_CONTIG
> > -- 
> > 2.26.0.rc2
> > 

Re: [PATCH] cedrus: Drop unneeded CONFIG_OF dependency

[Paul Kocialkowski]

[-- Attachment #1: Type: text/plain, Size: 2489 bytes --]

Hi Ezequiel,

On Fri 03 Apr 20, 09:21, Ezequiel Garcia wrote:
> On Fri, 2020-04-03 at 10:22 +0200, Paul Kocialkowski wrote:
> > Hi Ezequiel,
> > 
> > On Thu 02 Apr 20, 16:46, Ezequiel Garcia wrote:
> > > The driver is perfectly capable of being built without CONFIG_OF.
> > > Remove this dependency, which is useful for compile-only tests.
> > 
> > Thanks for the patch!
> > 
> > Alas, the driver won't do anything useful without OF enabled, so it seems
> > useful to keep that dependency.
> 
> I'm not convinced about this argument, I like to have
> dependencies on things the driver needs to compile
> (except the machine option, see below).

Is there a general kernel-wide policy about that?

I must admit that I never really understood if Kconfig depends are supposed to
strictly reflect build dependencies or dependencies that must reasonably be
expected for the driver to be useful. The arch dependency (which is farily
common) seems to suggest the latter.

Maxime, any thought about that?

> > I would suggest making this a: depends on OF || COMPILE_TEST
> > instead. What do you think?
> 
> Although there are a handful, I don't see many drivers doing that.
> To be honest, I don't like this much.
> 
> Also, if you want to make sure this driver can only be selected
> where it makes sense, you consider having a ARCH_SUNXI || COMPILE_TEST.

Fair enough, that would be the consistent choice with OF || COMPILE_TEST.

> IMO, the dependency on OF should be dropped as well.
>
> Cheers,
> Eze
> 
> > Cheers,
> > 
> > Paul
> > 
> > > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> > > ---
> > >  drivers/staging/media/sunxi/cedrus/Kconfig | 1 -
> > >  1 file changed, 1 deletion(-)
> > > 
> > > diff --git a/drivers/staging/media/sunxi/cedrus/Kconfig b/drivers/staging/media/sunxi/cedrus/Kconfig
> > > index 17733e9a088f..59b8d1b29865 100644
> > > --- a/drivers/staging/media/sunxi/cedrus/Kconfig
> > > +++ b/drivers/staging/media/sunxi/cedrus/Kconfig
> > > @@ -3,7 +3,6 @@ config VIDEO_SUNXI_CEDRUS
> > >  	tristate "Allwinner Cedrus VPU driver"
> > >  	depends on VIDEO_DEV && VIDEO_V4L2 && MEDIA_CONTROLLER
> > >  	depends on HAS_DMA
> > > -	depends on OF
> > >  	depends on MEDIA_CONTROLLER_REQUEST_API
> > >  	select SUNXI_SRAM
> > >  	select VIDEOBUF2_DMA_CONTIG
> > > -- 
> > > 2.26.0.rc2
> > > 
> 
> 

-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] cedrus: Drop unneeded CONFIG_OF dependency

[Ezequiel Garcia]

Hello Paul,

On Mon, 2020-04-20 at 17:29 +0200, Paul Kocialkowski wrote:
> Hi Ezequiel,
> 
> On Fri 03 Apr 20, 09:21, Ezequiel Garcia wrote:
> > On Fri, 2020-04-03 at 10:22 +0200, Paul Kocialkowski wrote:
> > > Hi Ezequiel,
> > > 
> > > On Thu 02 Apr 20, 16:46, Ezequiel Garcia wrote:
> > > > The driver is perfectly capable of being built without CONFIG_OF.
> > > > Remove this dependency, which is useful for compile-only tests.
> > > 
> > > Thanks for the patch!
> > > 
> > > Alas, the driver won't do anything useful without OF enabled, so it seems
> > > useful to keep that dependency.
> > 
> > I'm not convinced about this argument, I like to have
> > dependencies on things the driver needs to compile
> > (except the machine option, see below).
> 
> Is there a general kernel-wide policy about that?
> 

I don't think there is.

> I must admit that I never really understood if Kconfig depends are supposed to
> strictly reflect build dependencies or dependencies that must reasonably be
> expected for the driver to be useful. The arch dependency (which is farily
> common) seems to suggest the latter.
> 

Indeed, it's a good point. Just forget this patch :-)

As a note, some drivers use something like
(ARCH_FOO && OF) || COMPILE_TEST to allow compile
testing in the absence of OF support, if that's
something that you care for.

I do think the ARCH_SUNXI dependency makes sense.

Thanks,
Ezequiel