Re: [PATCH] cedrus: Drop unneeded CONFIG_OF dependency

[Paul Kocialkowski <paul.kocialkowski@bootlin.com>]

[-- 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 --]