[patch, 2.6.26-rc2] dvb: drivers/media/dvb/ttpci/fdump firmware build error fix

[patch, 2.6.26-rc2] dvb: drivers/media/dvb/ttpci/fdump firmware build error fix

[Ingo Molnar]

x86.git testing found the following build failure in 2.6.26-rc2:

  drivers/media/dvb/ttpci/fdump "/usr/lib/hotplug/firmware/dvb-ttpci-01.fw" dvb_ttpci_fw drivers/media/dvb/ttpci/av7110_firm.h
  firmware file '/usr/lib/hotplug/firmware/dvb-ttpci-01.fw' not found
  make[4]: *** [drivers/media/dvb/ttpci/av7110_firm.h] Error 255
  make[3]: *** [drivers/media/dvb/ttpci] Error 2

in this config:

  http://redhat.com/~mingo/misc/config-Mon_May_12_11_29_29_CEST_2008.bad

the reason for the build bug is that the driver's Kconfig ignores the 
PREVENT_FIRMWARE_BUILD=y config setting and tries to build the firmware 
which (like other firmware build methods) relies on a non-upstream 
firmware blob. The fix is to make the firmware build depend on 
!PREVENT_FIRMWARE_BUILD.

Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 drivers/media/dvb/ttpci/Kconfig |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux/drivers/media/dvb/ttpci/Kconfig
===================================================================
--- linux.orig/drivers/media/dvb/ttpci/Kconfig
+++ linux/drivers/media/dvb/ttpci/Kconfig
@@ -33,7 +33,7 @@ config DVB_AV7110
 
 config DVB_AV7110_FIRMWARE
 	bool "Compile AV7110 firmware into the driver"
-	depends on DVB_AV7110 && !STANDALONE
+	depends on DVB_AV7110 && !STANDALONE && !PREVENT_FIRMWARE_BUILD
 	default y if DVB_AV7110=y
 	help
 	  The AV7110 firmware is normally loaded by the firmware hotplug manager.

Re: [patch, 2.6.26-rc2] dvb: drivers/media/dvb/ttpci/fdump firmware build error fix

[Oliver Endriss]

Ingo Molnar wrote:
> 
> x86.git testing found the following build failure in 2.6.26-rc2:
> 
>   drivers/media/dvb/ttpci/fdump "/usr/lib/hotplug/firmware/dvb-ttpci-01.fw" dvb_ttpci_fw drivers/media/dvb/ttpci/av7110_firm.h
>   firmware file '/usr/lib/hotplug/firmware/dvb-ttpci-01.fw' not found
>   make[4]: *** [drivers/media/dvb/ttpci/av7110_firm.h] Error 255
>   make[3]: *** [drivers/media/dvb/ttpci] Error 2
> 
> in this config:
> 
>   http://redhat.com/~mingo/misc/config-Mon_May_12_11_29_29_CEST_2008.bad
> 
> the reason for the build bug is that the driver's Kconfig ignores the 
> PREVENT_FIRMWARE_BUILD=y config setting and tries to build the firmware 
> which (like other firmware build methods) relies on a non-upstream 
> firmware blob. The fix is to make the firmware build depend on 
> !PREVENT_FIRMWARE_BUILD.
> 
> Signed-off-by: Ingo Molnar <mingo@elte.hu>
> ---
>  drivers/media/dvb/ttpci/Kconfig |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Index: linux/drivers/media/dvb/ttpci/Kconfig
> ===================================================================
> --- linux.orig/drivers/media/dvb/ttpci/Kconfig
> +++ linux/drivers/media/dvb/ttpci/Kconfig
> @@ -33,7 +33,7 @@ config DVB_AV7110
>  
>  config DVB_AV7110_FIRMWARE
>  	bool "Compile AV7110 firmware into the driver"
> -	depends on DVB_AV7110 && !STANDALONE
> +	depends on DVB_AV7110 && !STANDALONE && !PREVENT_FIRMWARE_BUILD
>  	default y if DVB_AV7110=y
>  	help
>  	  The AV7110 firmware is normally loaded by the firmware hotplug manager.
> 

Nak. because DVB_AV7110_FIRMWARE=y does not build any firmware.

If you set DVB_AV7110_FIRMWARE=y, the firmware file under
DVB_AV7110_FIRMWARE_FILE must exist...

CU
Oliver

-- 
----------------------------------------------------------------
VDR Remote Plugin 0.4.0: http://www.escape-edv.de/endriss/vdr/
----------------------------------------------------------------

Re: [patch, 2.6.26-rc2] dvb: drivers/media/dvb/ttpci/fdump firmware build error fix

[Johannes Stezenbach]

On Mon, May 12, 2008 at 07:30:10PM +0200, Oliver Endriss wrote:
> Ingo Molnar wrote:
>> -     depends on DVB_AV7110 && !STANDALONE
>> +     depends on DVB_AV7110 && !STANDALONE && !PREVENT_FIRMWARE_BUILD

> If you set DVB_AV7110_FIRMWARE=y, the firmware file under
> DVB_AV7110_FIRMWARE_FILE must exist...

The point of PREVENT_FIRMWARE_BUILD is to make build testing
with "make randconfig" possible. I think you should ACK
this patch, it doesn't have any downsides for users of
this driver.

Johannes

Re: [patch, 2.6.26-rc2] dvb: drivers/media/dvb/ttpci/fdump firmware build error fix

[Oliver Endriss]

Johannes Stezenbach wrote:
> On Mon, May 12, 2008 at 07:30:10PM +0200, Oliver Endriss wrote:
> > Ingo Molnar wrote:
> >> -     depends on DVB_AV7110 && !STANDALONE
> >> +     depends on DVB_AV7110 && !STANDALONE && !PREVENT_FIRMWARE_BUILD
> 
> > If you set DVB_AV7110_FIRMWARE=y, the firmware file under
> > DVB_AV7110_FIRMWARE_FILE must exist...
> 
> The point of PREVENT_FIRMWARE_BUILD is to make build testing
> with "make randconfig" possible. I think you should ACK
> this patch, it doesn't have any downsides for users of
> this driver.

No. If randconfig deselects both STANDALONE and PREVENT_FIRMWARE_BUILD,
the same problem will happen again...

The problem is that randconfig does not know what to do (i.e. provide a
firmware file) when it selects DVB_AV7110_FIRMWARE=y.

The driver depends on STANDALONE, and that should be sufficient:
| CONFIG_STANDALONE:
|   Select this option if you don't have magic firmware for drivers that
|   need it.

while CONFIG_PREVENT_FIRMWARE_BUILD means:
| CONFIG_PREVENT_FIRMWARE_BUILD:
|   Say yes to avoid building firmware. Firmware is usually shipped
|   with the driver, and only when updating the firmware a rebuild
|   should be made.

Imho randconfig should always set STANDALONE=y.

CU
Oliver

-- 
----------------------------------------------------------------
VDR Remote Plugin 0.4.0: http://www.escape-edv.de/endriss/vdr/
----------------------------------------------------------------

Re: [patch, 2.6.26-rc2] dvb: drivers/media/dvb/ttpci/fdump firmware build error fix

[Oliver Endriss]

Oliver Endriss wrote:
> Johannes Stezenbach wrote:
> > On Mon, May 12, 2008 at 07:30:10PM +0200, Oliver Endriss wrote:
> > > Ingo Molnar wrote:
> > >> -     depends on DVB_AV7110 && !STANDALONE
> > >> +     depends on DVB_AV7110 && !STANDALONE && !PREVENT_FIRMWARE_BUILD
> > 
> > > If you set DVB_AV7110_FIRMWARE=y, the firmware file under
> > > DVB_AV7110_FIRMWARE_FILE must exist...
> > 
> > The point of PREVENT_FIRMWARE_BUILD is to make build testing
> > with "make randconfig" possible. I think you should ACK
> > this patch, it doesn't have any downsides for users of
> > this driver.
> 
> No. If randconfig deselects both STANDALONE and PREVENT_FIRMWARE_BUILD,
> the same problem will happen again...
> 
> The problem is that randconfig does not know what to do (i.e. provide a
> firmware file) when it selects DVB_AV7110_FIRMWARE=y.
> 
> The driver depends on STANDALONE, and that should be sufficient:

Sorry, typo: The driver depends on !STANDALONE...

> | CONFIG_STANDALONE:
> |   Select this option if you don't have magic firmware for drivers that
> |   need it.
> 
> while CONFIG_PREVENT_FIRMWARE_BUILD means:
> | CONFIG_PREVENT_FIRMWARE_BUILD:
> |   Say yes to avoid building firmware. Firmware is usually shipped
> |   with the driver, and only when updating the firmware a rebuild
> |   should be made.
> 
> Imho randconfig should always set STANDALONE=y.

CU
Oliver

-- 
----------------------------------------------------------------
VDR Remote Plugin 0.4.0: http://www.escape-edv.de/endriss/vdr/
----------------------------------------------------------------

Re: [patch, 2.6.26-rc2] dvb: drivers/media/dvb/ttpci/fdump firmware build error fix

[Johannes Stezenbach]

On Mon, May 12, 2008 at 10:18:15PM +0200, Oliver Endriss wrote:
> Johannes Stezenbach wrote:
> > On Mon, May 12, 2008 at 07:30:10PM +0200, Oliver Endriss wrote:
> > > Ingo Molnar wrote:
> > >> -     depends on DVB_AV7110 && !STANDALONE
> > >> +     depends on DVB_AV7110 && !STANDALONE && !PREVENT_FIRMWARE_BUILD
> > 
> > > If you set DVB_AV7110_FIRMWARE=y, the firmware file under
> > > DVB_AV7110_FIRMWARE_FILE must exist...
> > 
> > The point of PREVENT_FIRMWARE_BUILD is to make build testing
> > with "make randconfig" possible. I think you should ACK
> > this patch, it doesn't have any downsides for users of
> > this driver.
> 
> No. If randconfig deselects both STANDALONE and PREVENT_FIRMWARE_BUILD,
> the same problem will happen again...

You can put PREVENT_FIRMWARE_BUILD=y in your allrandom.config.

> The driver depends on STANDALONE, and that should be sufficient:
> | CONFIG_STANDALONE:
> |   Select this option if you don't have magic firmware for drivers that
> |   need it.

this is for excluding drivers which don't _work_ without firmware

> while CONFIG_PREVENT_FIRMWARE_BUILD means:
> | CONFIG_PREVENT_FIRMWARE_BUILD:
> |   Say yes to avoid building firmware. Firmware is usually shipped
> |   with the driver, and only when updating the firmware a rebuild
> |   should be made.

and this is for excluding drivers which don't _build_ without firmware
(badly worded, it certainly doesn't mean to build the firmware,
but build a C source file from the firmware)


Johannes

Re: [patch, 2.6.26-rc2] dvb: drivers/media/dvb/ttpci/fdump firmware build error fix

[Ingo Molnar]

* Johannes Stezenbach <js@linuxtv.org> wrote:

> > > The point of PREVENT_FIRMWARE_BUILD is to make build testing with 
> > > "make randconfig" possible. I think you should ACK this patch, it 
> > > doesn't have any downsides for users of this driver.
> > 
> > No. If randconfig deselects both STANDALONE and 
> > PREVENT_FIRMWARE_BUILD, the same problem will happen again...
> 
> You can put PREVENT_FIRMWARE_BUILD=y in your allrandom.config.

yes. This is a clear bug and regression.

	Ingo

Re: [patch, 2.6.26-rc2] dvb: drivers/media/dvb/ttpci/fdump firmware build error fix

[Oliver Endriss]

Johannes Stezenbach wrote:
> On Mon, May 12, 2008 at 10:18:15PM +0200, Oliver Endriss wrote:
> > Johannes Stezenbach wrote:
> > > On Mon, May 12, 2008 at 07:30:10PM +0200, Oliver Endriss wrote:
> > > > Ingo Molnar wrote:
> > > >> -     depends on DVB_AV7110 && !STANDALONE
> > > >> +     depends on DVB_AV7110 && !STANDALONE && !PREVENT_FIRMWARE_BUILD
> > > 
> > > > If you set DVB_AV7110_FIRMWARE=y, the firmware file under
> > > > DVB_AV7110_FIRMWARE_FILE must exist...
> > > 
> > > The point of PREVENT_FIRMWARE_BUILD is to make build testing
> > > with "make randconfig" possible. I think you should ACK
> > > this patch, it doesn't have any downsides for users of
> > > this driver.
> > 
> > No. If randconfig deselects both STANDALONE and PREVENT_FIRMWARE_BUILD,
> > the same problem will happen again...
> 
> You can put PREVENT_FIRMWARE_BUILD=y in your allrandom.config.

Sure. And without the patch you can simply add
- STANDALONE=y or
- DVB_AV7110_FIRMWARE=n
to your allrandom.config. ;-)

> > The driver depends on STANDALONE, and that should be sufficient:
> > | CONFIG_STANDALONE:
> > |   Select this option if you don't have magic firmware for drivers that
> > |   need it.
> 
> this is for excluding drivers which don't _work_ without firmware
>
> > while CONFIG_PREVENT_FIRMWARE_BUILD means:
> > | CONFIG_PREVENT_FIRMWARE_BUILD:
> > |   Say yes to avoid building firmware. Firmware is usually shipped
> > |   with the driver, and only when updating the firmware a rebuild
> > |   should be made.
> 
> and this is for excluding drivers which don't _build_ without firmware
> (badly worded, it certainly doesn't mean to build the firmware,
> but build a C source file from the firmware)

Could someone please confirm this?
If Johannes is right, the help texts are misleeding and need
clarification.

What worries me, is that 'magic' options like CONFIG_STANDALONE and/or
CONFIG_PREVENT_FIRMWARE_BUILD hide options like DVB_AV7110_FIRMWARE from
the user. How should the average user know that he has to disable them?
This is not user-friendly!

CU
Oliver

-- 
----------------------------------------------------------------
VDR Remote Plugin 0.4.0: http://www.escape-edv.de/endriss/vdr/
----------------------------------------------------------------

Re: [patch, 2.6.26-rc2] dvb: drivers/media/dvb/ttpci/fdump firmware build error fix

[Ingo Molnar]

* Oliver Endriss <o.endriss@gmx.de> wrote:

> > > No. If randconfig deselects both STANDALONE and PREVENT_FIRMWARE_BUILD,
> > > the same problem will happen again...
> > 
> > You can put PREVENT_FIRMWARE_BUILD=y in your allrandom.config.
> 
> Sure. And without the patch you can simply add
> - STANDALONE=y or
> - DVB_AV7110_FIRMWARE=n
> to your allrandom.config. ;-)

Uhm, cart pulls horse alert. There's a reason PREVENT_FIRMWARE_BUILD 
exists and is used by all other drivers in the tree that have a firmware 
build method. Yours is the only one that regresses.

	Ingo

Re: [patch, 2.6.26-rc2] dvb: drivers/media/dvb/ttpci/fdump firmware build error fix

[Johannes Stezenbach]

On Tue, May 13, 2008 at 04:33:54PM +0200, Oliver Endriss wrote:
> Johannes Stezenbach wrote:
> > You can put PREVENT_FIRMWARE_BUILD=y in your allrandom.config.
> 
> Sure. And without the patch you can simply add
> - STANDALONE=y or
> - DVB_AV7110_FIRMWARE=n
> to your allrandom.config. ;-)

It is desirable to have one generic "don't break the build
because of missing firmware binaries" option, isn't it?

Johannes

Re: [patch, 2.6.26-rc2] dvb: drivers/media/dvb/ttpci/fdump firmware build error fix

[Ingo Molnar]

* Johannes Stezenbach <js@linuxtv.org> wrote:

> On Tue, May 13, 2008 at 04:33:54PM +0200, Oliver Endriss wrote:
> > Johannes Stezenbach wrote:
> > > You can put PREVENT_FIRMWARE_BUILD=y in your allrandom.config.
> > 
> > Sure. And without the patch you can simply add
> > - STANDALONE=y or
> > - DVB_AV7110_FIRMWARE=n
> > to your allrandom.config. ;-)
> 
> It is desirable to have one generic "don't break the build because of 
> missing firmware binaries" option, isn't it?

it's a bit confusing. PREVENT_FIRMWARE_BUILD=y is the 'weaker' option - 
a driver might still be operational (with its default firmware), even if 
no firmware is built. STANDALONE=y is the stronger option - if a driver 
depends on that it means that the driver cannot be built (and used) at 
all without a firmware file. To be on the safe side users who have no 
firmware needs should set both options. (and that's the default as well)

	Ingo

Re: [patch, 2.6.26-rc2] dvb: drivers/media/dvb/ttpci/fdump firmware build error fix

[Johannes Stezenbach]

On Tue, May 13, 2008 at 06:42:38PM +0200, Ingo Molnar wrote:
> 
> * Johannes Stezenbach <js@linuxtv.org> wrote:
> 
> > On Tue, May 13, 2008 at 04:33:54PM +0200, Oliver Endriss wrote:
> > > Johannes Stezenbach wrote:
> > > > You can put PREVENT_FIRMWARE_BUILD=y in your allrandom.config.
> > > 
> > > Sure. And without the patch you can simply add
> > > - STANDALONE=y or
> > > - DVB_AV7110_FIRMWARE=n
> > > to your allrandom.config. ;-)
> > 
> > It is desirable to have one generic "don't break the build because of 
> > missing firmware binaries" option, isn't it?
> 
> it's a bit confusing. PREVENT_FIRMWARE_BUILD=y is the 'weaker' option - 
> a driver might still be operational (with its default firmware), even if 
> no firmware is built. STANDALONE=y is the stronger option - if a driver 
> depends on that it means that the driver cannot be built (and used) at 
> all without a firmware file. To be on the safe side users who have no 
> firmware needs should set both options. (and that's the default as well)

Seems like I'm confused now...

  CONFIG_STANDALONE
    Prompt: Select only drivers that don't need compile-time external firmware

  PREVENT_FIRMWARE_BUILD
    Prompt: Prevent firmware from being built

I guess selecting either one should prevent the build issue
you got, so your patch is right. But I also was wrong on
the meaning of STANDALONE...
Now I wonder why PREVENT_FIRMWARE_BUILD exists at all.


Johannes

Re: [patch, 2.6.26-rc2] dvb: drivers/media/dvb/ttpci/fdump firmware build error fix

[Oliver Endriss]

Johannes Stezenbach wrote:
> On Tue, May 13, 2008 at 06:42:38PM +0200, Ingo Molnar wrote:
> > 
> > * Johannes Stezenbach <js@linuxtv.org> wrote:
> > 
> > > On Tue, May 13, 2008 at 04:33:54PM +0200, Oliver Endriss wrote:
> > > > Johannes Stezenbach wrote:
> > > > > You can put PREVENT_FIRMWARE_BUILD=y in your allrandom.config.
> > > > 
> > > > Sure. And without the patch you can simply add
> > > > - STANDALONE=y or
> > > > - DVB_AV7110_FIRMWARE=n
> > > > to your allrandom.config. ;-)
> > > 
> > > It is desirable to have one generic "don't break the build because of 
> > > missing firmware binaries" option, isn't it?
> > 
> > it's a bit confusing. PREVENT_FIRMWARE_BUILD=y is the 'weaker' option - 
> > a driver might still be operational (with its default firmware), even if 
> > no firmware is built. STANDALONE=y is the stronger option - if a driver 
> > depends on that it means that the driver cannot be built (and used) at 
> > all without a firmware file. To be on the safe side users who have no 
> > firmware needs should set both options. (and that's the default as well)
> 
> Seems like I'm confused now...
> 
>   CONFIG_STANDALONE
>     Prompt: Select only drivers that don't need compile-time external firmware
> 
>   PREVENT_FIRMWARE_BUILD
>     Prompt: Prevent firmware from being built

After reading the text again I still think that my understanding is
correct [1].

CONFIG_STANDALONE disables drivers which require a firmware file at
compile-time. av7110 with DVB_AV7110_FIRMWARE=y is this kind of driver.

PREVENT_FIRMWARE_BUILD controls building (= compiling) the firmware from
source. As we do not ship the source of the av7110 firmware, this option
does not apply.

So everything should be fine 'as is'.

CU
Oliver

Notes:
[1] Of course, I am open to corrections from the kbuild gurus. ;-)

-- 
----------------------------------------------------------------
VDR Remote Plugin 0.4.0: http://www.escape-edv.de/endriss/vdr/
----------------------------------------------------------------

Re: [patch, 2.6.26-rc2] dvb: drivers/media/dvb/ttpci/fdump firmware build error fix

[Ingo Molnar]

* Oliver Endriss <o.endriss@gmx.de> wrote:

> Sure. And without the patch you can simply add
> - STANDALONE=y or
> - DVB_AV7110_FIRMWARE=n
> to your allrandom.config. ;-)
 
hm, you are right - i've added STANDALONE=y previously but it went 
missing in action. So forget my patch, it was user error - the driver is 
right in that it already depends on STANDALONE :-) I guess STANDALONE 
should enable PREVENT_FIRMWARE_BUILD as well to avoid such errors ...

	Ingo

Re: [patch, 2.6.26-rc2] dvb: drivers/media/dvb/ttpci/fdump firmware build error fix

[Johannes Stezenbach]

On Tue, May 13, 2008 at 06:41:40PM +0200, Ingo Molnar wrote:
> 
> * Oliver Endriss <o.endriss@gmx.de> wrote:
> 
> > Sure. And without the patch you can simply add
> > - STANDALONE=y or
> > - DVB_AV7110_FIRMWARE=n
> > to your allrandom.config. ;-)
>  
> hm, you are right - i've added STANDALONE=y previously but it went 
> missing in action. So forget my patch, it was user error - the driver is 
> right in that it already depends on STANDALONE :-) I guess STANDALONE 
> should enable PREVENT_FIRMWARE_BUILD as well to avoid such errors ...

If you do _build_ testing you don't care if the driver
won't work because of missing firmware. With STANDALONE=y
you exclude drivers from build testing.

Johannes

Re: [patch, 2.6.26-rc2] dvb: drivers/media/dvb/ttpci/fdump firmware build error fix

[Oliver Endriss]

Ingo Molnar wrote:
> 
> * Oliver Endriss <o.endriss@gmx.de> wrote:
> 
> > Sure. And without the patch you can simply add
> > - STANDALONE=y or
> > - DVB_AV7110_FIRMWARE=n
> > to your allrandom.config. ;-)
>  
> hm, you are right - i've added STANDALONE=y previously but it went 
> missing in action. So forget my patch, it was user error - the driver is 
> right in that it already depends on STANDALONE :-)

Ok. I already wondered why this issue came up now.
The dependencies of DVB_AV7110_FIRMWARE did not change during the last
4 years...

> I guess STANDALONE  
> should enable PREVENT_FIRMWARE_BUILD as well to avoid such errors ...

Yes, it does not make much sense to build the firmware if the driver
will not be compiled anyway.

CU
Oliver

-- 
----------------------------------------------------------------
VDR Remote Plugin 0.4.0: http://www.escape-edv.de/endriss/vdr/
----------------------------------------------------------------