Re: [PATCH 2/3] staging: bcm2835-audio: select BCM2835_VCHIQ rather then depending on it.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, 2017-03-01 at 16:51 +0300, Dan Carpenter wrote:
> On Tue, Feb 28, 2017 at 09:18:56PM +0100, Stefan Wahren wrote:
> > 
> > > Michael Zoran <mzoran@xxxxxxxxxxxx> hat am 28. Februar 2017 um
> > > 19:49 geschrieben:
> > > 
> > > 
> > > Change the audio's dependency on BCM2835_VCHIQ to a select.
> > > 
> > > Signed-off-by: Michael Zoran <mzoran@xxxxxxxxxxxx>
> > > ---
> > >  drivers/staging/vc04_services/bcm2835-audio/Kconfig | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/staging/vc04_services/bcm2835-audio/Kconfig
> > > b/drivers/staging/vc04_services/bcm2835-audio/Kconfig
> > > index b2e6d90ef1cb..479c9e3ace11 100644
> > > --- a/drivers/staging/vc04_services/bcm2835-audio/Kconfig
> > > +++ b/drivers/staging/vc04_services/bcm2835-audio/Kconfig
> > > @@ -1,7 +1,8 @@
> > >  config SND_BCM2835
> > >          tristate "BCM2835 Audio"
> > > -        depends on ARCH_BCM2835 && BCM2835_VCHIQ && SND
> > > +        depends on ARCH_BCM2835 && SND
> > >          select SND_PCM
> > > +	select BCM2835_VCHIQ
> > >          help
> > >            Say Y or M if you want to support BCM2835 built in
> > > audio
> > >  
> > 
> > AFAIK "depends on" is perferred instead of "select", because it's
> > causes less issues. Please explain in the commit messages instead
> > of the cover letter why this patch and patch 3 is necessary.
> 
> There is a place for both depends and select.
> 
> The thing is that users basically don't care about BCM2835_VCHIQ,
> they
> just want audio.  With a depend it shows what they don't care about
> and
> hides what they want which is the opposite of how it should be.
> 
> Michael's reference to Linus's email is too vague (not a URL) so I
> have
> no idea what he's talking about.  Selecting BCM2835_VCHIQ doesn't
> pull
> in very much code, and it's all essential anyway.
> 
> regards,
> dan carpenter
> 

Thanks Dan, I completely agree with what you are saying.  

Longer term, I was thinking that perhaps the core of VCHIQ needs to be
an independent config option itself which is possibly hidden.  Then
when audio or camera is selected(as Dan points out the stuff that
people actually care about), it would also select in the core.

The only complication is that the ioctl interface is useful as well
since it's required in Raspbian for some of the games such as
Minecraft_pi, and for hardware video decoding.

So I was thinking that would be 4 config options under the main menu:
1. VCHIQ Core(Hidden option)
2. VCHIQ API(Selects 1)
3. Audio(Selects 1)
4. Camera(Selects 1)

Linus's post is rather explicit so I'm not going to give a direct
reference or URL, but it's on the DRM archives if people want to find
it.  To sum it up if I understand correctly, someone added a prime
number library(which is something people don't care about) and made
some graphic components depend on it(stuff people see and care about). 
Linus felt that it should work as Dan describes and the prime number
library should get selected by the graphics components.



_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel




[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux