[PATCH 29/75] cx18: declare MODULE_FIRMWARE

[PATCH 29/75] cx18: declare MODULE_FIRMWARE

[Ben Hutchings]

Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
 drivers/media/video/cx18/cx18-av-firmware.c |    1 +
 drivers/media/video/cx18/cx18-dvb.c         |    2 ++
 drivers/media/video/cx18/cx18-firmware.c    |    3 +++
 3 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/drivers/media/video/cx18/cx18-av-firmware.c b/drivers/media/video/cx18/cx18-av-firmware.c
index b9e8cc5..137445c 100644
--- a/drivers/media/video/cx18/cx18-av-firmware.c
+++ b/drivers/media/video/cx18/cx18-av-firmware.c
@@ -32,6 +32,7 @@
 #define CX18_AI1_MUX_INVALID 0x30
 
 #define FWFILE "v4l-cx23418-dig.fw"
+MODULE_FIRMWARE(FWFILE);
 
 static int cx18_av_verifyfw(struct cx18 *cx, const struct firmware *fw)
 {
diff --git a/drivers/media/video/cx18/cx18-dvb.c b/drivers/media/video/cx18/cx18-dvb.c
index 51a0c33..9f70168 100644
--- a/drivers/media/video/cx18/cx18-dvb.c
+++ b/drivers/media/video/cx18/cx18-dvb.c
@@ -131,6 +131,8 @@ static int yuan_mpc718_mt352_reqfw(struct cx18_stream *stream,
 	return ret;
 }
 
+MODULE_FIRMWARE("dvb-cx18-mpc718-mt352.fw");
+
 static int yuan_mpc718_mt352_init(struct dvb_frontend *fe)
 {
 	struct cx18_dvb *dvb = container_of(fe->dvb,
diff --git a/drivers/media/video/cx18/cx18-firmware.c b/drivers/media/video/cx18/cx18-firmware.c
index 83cd559..4ac4b81 100644
--- a/drivers/media/video/cx18/cx18-firmware.c
+++ b/drivers/media/video/cx18/cx18-firmware.c
@@ -446,3 +446,6 @@ int cx18_firmware_init(struct cx18 *cx)
 	cx18_write_reg_expect(cx, 0x14001400, 0xc78110, 0x00001400, 0x14001400);
 	return 0;
 }
+
+MODULE_FIRMWARE("v4l-cx23418-cpu.fw");
+MODULE_FIRMWARE("v4l-cx23418-apu.fw");
-- 
1.6.5.2

Re: [PATCH 29/75] cx18: declare MODULE_FIRMWARE

[Andy Walls]

On Sat, 2009-11-07 at 21:51 +0000, Ben Hutchings wrote:
> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> ---
>  drivers/media/video/cx18/cx18-av-firmware.c |    1 +
>  drivers/media/video/cx18/cx18-dvb.c         |    2 ++
>  drivers/media/video/cx18/cx18-firmware.c    |    3 +++
>  3 files changed, 6 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/media/video/cx18/cx18-av-firmware.c b/drivers/media/video/cx18/cx18-av-firmware.c
> index b9e8cc5..137445c 100644
> --- a/drivers/media/video/cx18/cx18-av-firmware.c
> +++ b/drivers/media/video/cx18/cx18-av-firmware.c
> @@ -32,6 +32,7 @@
>  #define CX18_AI1_MUX_INVALID 0x30
>  
>  #define FWFILE "v4l-cx23418-dig.fw"
> +MODULE_FIRMWARE(FWFILE);
>  
>  static int cx18_av_verifyfw(struct cx18 *cx, const struct firmware *fw)
>  {
> diff --git a/drivers/media/video/cx18/cx18-dvb.c b/drivers/media/video/cx18/cx18-dvb.c
> index 51a0c33..9f70168 100644
> --- a/drivers/media/video/cx18/cx18-dvb.c
> +++ b/drivers/media/video/cx18/cx18-dvb.c
> @@ -131,6 +131,8 @@ static int yuan_mpc718_mt352_reqfw(struct cx18_stream *stream,
>  	return ret;
>  }
>  
> +MODULE_FIRMWARE("dvb-cx18-mpc718-mt352.fw");
> +

Ben,

This particular firmware is only needed by one relatively rare TV card.
Is there any way for MODULE_FIRMWARE advertisements to hint at
"mandatory" vs. "particular case(s)"?

Regards,
Andy

>  static int yuan_mpc718_mt352_init(struct dvb_frontend *fe)
>  {
>  	struct cx18_dvb *dvb = container_of(fe->dvb,
> diff --git a/drivers/media/video/cx18/cx18-firmware.c b/drivers/media/video/cx18/cx18-firmware.c
> index 83cd559..4ac4b81 100644
> --- a/drivers/media/video/cx18/cx18-firmware.c
> +++ b/drivers/media/video/cx18/cx18-firmware.c
> @@ -446,3 +446,6 @@ int cx18_firmware_init(struct cx18 *cx)
>  	cx18_write_reg_expect(cx, 0x14001400, 0xc78110, 0x00001400, 0x14001400);
>  	return 0;
>  }
> +
> +MODULE_FIRMWARE("v4l-cx23418-cpu.fw");
> +MODULE_FIRMWARE("v4l-cx23418-apu.fw");

Re: [PATCH 29/75] cx18: declare MODULE_FIRMWARE

[Ben Hutchings]

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

On Sat, 2009-11-07 at 20:40 -0500, Andy Walls wrote:
> On Sat, 2009-11-07 at 21:51 +0000, Ben Hutchings wrote:
> > Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> > ---
> >  drivers/media/video/cx18/cx18-av-firmware.c |    1 +
> >  drivers/media/video/cx18/cx18-dvb.c         |    2 ++
> >  drivers/media/video/cx18/cx18-firmware.c    |    3 +++
> >  3 files changed, 6 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/media/video/cx18/cx18-av-firmware.c b/drivers/media/video/cx18/cx18-av-firmware.c
> > index b9e8cc5..137445c 100644
> > --- a/drivers/media/video/cx18/cx18-av-firmware.c
> > +++ b/drivers/media/video/cx18/cx18-av-firmware.c
> > @@ -32,6 +32,7 @@
> >  #define CX18_AI1_MUX_INVALID 0x30
> >  
> >  #define FWFILE "v4l-cx23418-dig.fw"
> > +MODULE_FIRMWARE(FWFILE);
> >  
> >  static int cx18_av_verifyfw(struct cx18 *cx, const struct firmware *fw)
> >  {
> > diff --git a/drivers/media/video/cx18/cx18-dvb.c b/drivers/media/video/cx18/cx18-dvb.c
> > index 51a0c33..9f70168 100644
> > --- a/drivers/media/video/cx18/cx18-dvb.c
> > +++ b/drivers/media/video/cx18/cx18-dvb.c
> > @@ -131,6 +131,8 @@ static int yuan_mpc718_mt352_reqfw(struct cx18_stream *stream,
> >  	return ret;
> >  }
> >  
> > +MODULE_FIRMWARE("dvb-cx18-mpc718-mt352.fw");
> > +
> 
> Ben,
> 
> This particular firmware is only needed by one relatively rare TV card.
> Is there any way for MODULE_FIRMWARE advertisements to hint at
> "mandatory" vs. "particular case(s)"?

No, but perhaps there ought to be.  In this case the declaration could
be left out for now.  It is only critical to list all firmware in
drivers that may be needed for booting.

Ben.

-- 
Ben Hutchings
The generation of random numbers is too important to be left to chance.
                                                            - Robert Coveyou

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH 29/75] cx18: declare MODULE_FIRMWARE

[Andy Walls]

On Sun, 2009-11-08 at 01:53 +0000, Ben Hutchings wrote:
> On Sat, 2009-11-07 at 20:40 -0500, Andy Walls wrote:
> > On Sat, 2009-11-07 at 21:51 +0000, Ben Hutchings wrote:

> > >  
> > > +MODULE_FIRMWARE("dvb-cx18-mpc718-mt352.fw");
> > > +
> > 
> > Ben,
> > 
> > This particular firmware is only needed by one relatively rare TV card.
> > Is there any way for MODULE_FIRMWARE advertisements to hint at
> > "mandatory" vs. "particular case(s)"?
> 
> No, but perhaps there ought to be.  In this case the declaration could
> be left out for now.  It is only critical to list all firmware in
> drivers that may be needed for booting.

OK.  I don't know that a TV card driver is every *needed* for booting.
Maybe one day when I can net-boot with cable-modem like
functionality... ;)


I'm OK with the MODULE_FIRMWARE announcements in cx18 so long as
automatic behaviors like

1. persistent, repeatitive, or truly alarming user warnings, or
2. refusing to load the module due to missing firmware files

don't happen.

Regards,
Andy

> Ben.

Re: [PATCH 29/75] cx18: declare MODULE_FIRMWARE

[Hans Verkuil]

On Sunday 08 November 2009 03:08:56 Andy Walls wrote:
> On Sun, 2009-11-08 at 01:53 +0000, Ben Hutchings wrote:
> > On Sat, 2009-11-07 at 20:40 -0500, Andy Walls wrote:
> > > On Sat, 2009-11-07 at 21:51 +0000, Ben Hutchings wrote:
> 
> > > >  
> > > > +MODULE_FIRMWARE("dvb-cx18-mpc718-mt352.fw");
> > > > +
> > > 
> > > Ben,
> > > 
> > > This particular firmware is only needed by one relatively rare TV card.
> > > Is there any way for MODULE_FIRMWARE advertisements to hint at
> > > "mandatory" vs. "particular case(s)"?
> > 
> > No, but perhaps there ought to be.  In this case the declaration could
> > be left out for now.  It is only critical to list all firmware in
> > drivers that may be needed for booting.
> 
> OK.  I don't know that a TV card driver is every *needed* for booting.
> Maybe one day when I can net-boot with cable-modem like
> functionality... ;)
> 
> 
> I'm OK with the MODULE_FIRMWARE announcements in cx18 so long as
> automatic behaviors like
> 
> 1. persistent, repeatitive, or truly alarming user warnings, or
> 2. refusing to load the module due to missing firmware files
> 
> don't happen.

I agree with Andy here.

In the case of ivtv and cx18 (unless that changed since the last time I worked
on it) the cx firmware is actually not loaded when the module is inited but on
the first open() call. So it is not even that clear to me whether we want to
have these fairly large fw files in an initramfs image at all.

Regards,

	Hans

> 
> Regards,
> Andy
> 
> > Ben.
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 



-- 
Hans Verkuil - video4linux developer - sponsored by TANDBERG Telecom

Re: [PATCH 29/75] cx18: declare MODULE_FIRMWARE

[Andy Walls]

On Mon, 2009-11-09 at 11:06 +0100, Hans Verkuil wrote:
> On Sunday 08 November 2009 03:08:56 Andy Walls wrote:
> > On Sun, 2009-11-08 at 01:53 +0000, Ben Hutchings wrote:
> > > On Sat, 2009-11-07 at 20:40 -0500, Andy Walls wrote:
> > > > On Sat, 2009-11-07 at 21:51 +0000, Ben Hutchings wrote:
> > 
> > > > >  
> > > > > +MODULE_FIRMWARE("dvb-cx18-mpc718-mt352.fw");
> > > > > +
> > > > 
> > > > Ben,
> > > > 
> > > > This particular firmware is only needed by one relatively rare TV card.
> > > > Is there any way for MODULE_FIRMWARE advertisements to hint at
> > > > "mandatory" vs. "particular case(s)"?
> > > 
> > > No, but perhaps there ought to be.  In this case the declaration could
> > > be left out for now.  It is only critical to list all firmware in
> > > drivers that may be needed for booting.
> > 
> > OK.  I don't know that a TV card driver is every *needed* for booting.
> > Maybe one day when I can net-boot with cable-modem like
> > functionality... ;)
> > 
> > 
> > I'm OK with the MODULE_FIRMWARE announcements in cx18 so long as
> > automatic behaviors like
> > 
> > 1. persistent, repeatitive, or truly alarming user warnings, or
> > 2. refusing to load the module due to missing firmware files
> > 
> > don't happen.
> 
> I agree with Andy here.
> 
> In the case of ivtv and cx18 (unless that changed since the last time I worked
> on it) the cx firmware is actually not loaded when the module is inited but on
> the first open() call. So it is not even that clear to me whether we want to
> have these fairly large fw files in an initramfs image at all.
> 

I've been thinking about this all a bit more since I read Mauro's
comment.

MODULE_FIRMWARE() is essentially turning kernel driver modules into an
interactive, read-only, database for (a particular set of ?) end users.

The process of keeping MODULE_FIRMWARE declarations up to date will run
into all the incentive, governance, and maintenance problems that any
database has.  Due to lack incentive structure, one will end up with
missing data at any point in time, as the current patch series points
out.

It may be better to keep tabs on module firmware image names with a
database outside of the kernel *.[ch] files.  It could be a simple as a
text file somewhere.  I suspect it would have just as likely a chance or
better of being up to date at any point in time.  That would also be a
bit more flexible.  One could add additional fields to the records for
amplifying information (e.g required, optional, card xyz) without
perturbing a slew of kernel *.[ch] files.

My $0.02.

Regards,
Andy

Re: [PATCH 29/75] cx18: declare MODULE_FIRMWARE

[Mauro Carvalho Chehab]

Em Mon, 09 Nov 2009 07:03:02 -0500
Andy Walls <awalls@radix.net> escreveu:

> On Mon, 2009-11-09 at 11:06 +0100, Hans Verkuil wrote:
> > On Sunday 08 November 2009 03:08:56 Andy Walls wrote:
> > > On Sun, 2009-11-08 at 01:53 +0000, Ben Hutchings wrote:
> > > > On Sat, 2009-11-07 at 20:40 -0500, Andy Walls wrote:
> > > > > On Sat, 2009-11-07 at 21:51 +0000, Ben Hutchings wrote:
> > > 
> > > > > >  
> > > > > > +MODULE_FIRMWARE("dvb-cx18-mpc718-mt352.fw");
> > > > > > +
> > > > > 
> > > > > Ben,
> > > > > 
> > > > > This particular firmware is only needed by one relatively rare TV card.
> > > > > Is there any way for MODULE_FIRMWARE advertisements to hint at
> > > > > "mandatory" vs. "particular case(s)"?
> > > > 
> > > > No, but perhaps there ought to be.  In this case the declaration could
> > > > be left out for now.  It is only critical to list all firmware in
> > > > drivers that may be needed for booting.
> > > 
> > > OK.  I don't know that a TV card driver is every *needed* for booting.
> > > Maybe one day when I can net-boot with cable-modem like
> > > functionality... ;)
> > > 
> > > 
> > > I'm OK with the MODULE_FIRMWARE announcements in cx18 so long as
> > > automatic behaviors like
> > > 
> > > 1. persistent, repeatitive, or truly alarming user warnings, or
> > > 2. refusing to load the module due to missing firmware files
> > > 
> > > don't happen.
> > 
> > I agree with Andy here.
> > 
> > In the case of ivtv and cx18 (unless that changed since the last time I worked
> > on it) the cx firmware is actually not loaded when the module is inited but on
> > the first open() call. So it is not even that clear to me whether we want to
> > have these fairly large fw files in an initramfs image at all.
> > 
> 
> I've been thinking about this all a bit more since I read Mauro's
> comment.
> 
> MODULE_FIRMWARE() is essentially turning kernel driver modules into an
> interactive, read-only, database for (a particular set of ?) end users.
> 
> The process of keeping MODULE_FIRMWARE declarations up to date will run
> into all the incentive, governance, and maintenance problems that any
> database has.  Due to lack incentive structure, one will end up with
> missing data at any point in time, as the current patch series points
> out.
> 
> It may be better to keep tabs on module firmware image names with a
> database outside of the kernel *.[ch] files.  It could be a simple as a
> text file somewhere.  I suspect it would have just as likely a chance or
> better of being up to date at any point in time.  That would also be a
> bit more flexible.  One could add additional fields to the records for
> amplifying information (e.g required, optional, card xyz) without
> perturbing a slew of kernel *.[ch] files.

Yes, this will work better, IMHO, but won't cover the few cases where you
only know the frontend/tuner at the runtime, after probing the device
or reading the device eeprom.

If the need is to detect the firmware needs during installation, upgrades
or hardware changes, the simplest way is to use an userspace tool for the
request_firmware hotplug interface that catches the firmware requests after
loading and opening a V4L/DVB device. This will catch all cases and will
be a way easier than maintaining a database.


Cheers,
Mauro