diff mbox series

block: fix CDROM dependency on BLK_DEV

Message ID 20171102111952.4021454-1-arnd@arndb.de
State Accepted
Commit c091fbe9a26ee116d99e2c0ed010afb957a10365
Headers show
Series block: fix CDROM dependency on BLK_DEV | expand

Commit Message

Arnd Bergmann Nov. 2, 2017, 11:19 a.m. UTC
After the cdrom cleanup, I get randconfig warnings for some configurations:

warning: (BLK_DEV_IDECD && BLK_DEV_SR) selects CDROM which has unmet direct dependencies (BLK_DEV)

This adds an explicit BLK_DEV dependency for both drivers. The other
drivers that select 'CDROM' already have this and don't need a change.

Fixes: 2a750166a5be ("block: Rework drivers/cdrom/Makefile")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>

---
 drivers/ide/Kconfig  | 1 +
 drivers/scsi/Kconfig | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

-- 
2.9.0

Comments

Bart Van Assche Nov. 2, 2017, 2:59 p.m. UTC | #1
On Thu, 2017-11-02 at 12:19 +0100, Arnd Bergmann wrote:
> After the cdrom cleanup, I get randconfig warnings for some configurations:

> 

> warning: (BLK_DEV_IDECD && BLK_DEV_SR) selects CDROM which has unmet direct dependencies (BLK_DEV)


Hello Arnd,

Since Jens has already queued your patch it's too late to consider alternatives.
Anyway, since the cdrom driver calls block layer functions directly, have you
considered the following alternative?

diff --git a/drivers/block/Kconfig b/drivers/block/Kconfig
index 0dad1d2536f7..e7044c893817 100644
--- a/drivers/block/Kconfig
+++ b/drivers/block/Kconfig
@@ -69,6 +69,7 @@ config AMIGA_Z2RAM
 
 config CDROM
 	tristate "CD-ROM driver"
+	depends on BLK_DEV
 	help
 	  A CD-ROM is a pre-pressed optical compact disc which contains
 	  data. The name is an acronym which stands for "Compact Disc
Bart Van Assche Nov. 2, 2017, 3:01 p.m. UTC | #2
On Thu, 2017-11-02 at 08:27 -0600, Jens Axboe wrote:
> On 11/02/2017 05:19 AM, Arnd Bergmann wrote:

> > After the cdrom cleanup, I get randconfig warnings for some configurations:

> > 

> > warning: (BLK_DEV_IDECD && BLK_DEV_SR) selects CDROM which has unmet direct dependencies (BLK_DEV)

> > 

> > This adds an explicit BLK_DEV dependency for both drivers. The other

> > drivers that select 'CDROM' already have this and don't need a change.

> 

> Thanks Arnd, applied.


Hello Jens,

Can you wait at least 24 hours after a patch has been posted before applying it
such those who want to post review comments have a chance to do that?

Thanks,

Bart.
Arnd Bergmann Nov. 2, 2017, 3:07 p.m. UTC | #3
On Thu, Nov 2, 2017 at 3:59 PM, Bart Van Assche <Bart.VanAssche@wdc.com> wrote:
> On Thu, 2017-11-02 at 12:19 +0100, Arnd Bergmann wrote:

>> After the cdrom cleanup, I get randconfig warnings for some configurations:

>>

>> warning: (BLK_DEV_IDECD && BLK_DEV_SR) selects CDROM which has unmet direct dependencies (BLK_DEV)

>

> Hello Arnd,

>

> Since Jens has already queued your patch it's too late to consider alternatives.

> Anyway, since the cdrom driver calls block layer functions directly, have you

> considered the following alternative?

>

> diff --git a/drivers/block/Kconfig b/drivers/block/Kconfig

> index 0dad1d2536f7..e7044c893817 100644

> --- a/drivers/block/Kconfig

> +++ b/drivers/block/Kconfig

> @@ -69,6 +69,7 @@ config AMIGA_Z2RAM

>

>  config CDROM

>         tristate "CD-ROM driver"

> +       depends on BLK_DEV

>         help

>           A CD-ROM is a pre-pressed optical compact disc which contains

>           data. The name is an acronym which stands for "Compact Disc


This doesn't work, the problem is the existing dependency on 'BLK_DEV'
that is implied by
listing CONFIG_CDROM inside of the "if BLK_DEV" section. It would
probably be possible
to move CONFIG_CDROM' outside of this 'if' section and use 'select
BLK_DEV' inside it,
but that in turn would make the Kconfig logic even less intuitive than
it already is.

      Arnd
Jens Axboe Nov. 2, 2017, 3:11 p.m. UTC | #4
On 11/02/2017 09:01 AM, Bart Van Assche wrote:
> On Thu, 2017-11-02 at 08:27 -0600, Jens Axboe wrote:

>> On 11/02/2017 05:19 AM, Arnd Bergmann wrote:

>>> After the cdrom cleanup, I get randconfig warnings for some configurations:

>>>

>>> warning: (BLK_DEV_IDECD && BLK_DEV_SR) selects CDROM which has unmet direct dependencies (BLK_DEV)

>>>

>>> This adds an explicit BLK_DEV dependency for both drivers. The other

>>> drivers that select 'CDROM' already have this and don't need a change.

>>

>> Thanks Arnd, applied.

> 

> Hello Jens,

> 

> Can you wait at least 24 hours after a patch has been posted before applying it

> such those who want to post review comments have a chance to do that?


I generally at least do that, unless they are trivial...


-- 
Jens Axboe
diff mbox series

Patch

diff --git a/drivers/ide/Kconfig b/drivers/ide/Kconfig
index 7b92c591e4c1..cf1fb3fb5d26 100644
--- a/drivers/ide/Kconfig
+++ b/drivers/ide/Kconfig
@@ -117,6 +117,7 @@  config BLK_DEV_DELKIN
 
 config BLK_DEV_IDECD
 	tristate "Include IDE/ATAPI CDROM support"
+	depends on BLK_DEV
 	select IDE_ATAPI
 	select CDROM
 	---help---
diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
index 1f98f666d2ef..8a739b74cfb7 100644
--- a/drivers/scsi/Kconfig
+++ b/drivers/scsi/Kconfig
@@ -130,7 +130,7 @@  config CHR_DEV_OSST
 
 config BLK_DEV_SR
 	tristate "SCSI CDROM support"
-	depends on SCSI
+	depends on SCSI && BLK_DEV
 	select CDROM
 	---help---
 	  If you want to use a CD or DVD drive attached to your computer