[PATCH] xc5000: Add MODULE_FIRMWARE statements

[PATCH] xc5000: Add MODULE_FIRMWARE statements

[Tim Gardner]

This will make modinfo more useful with regard
to discovering necessary firmware files.

Cc: Mauro Carvalho Chehab <mchehab@infradead.org>
Cc: Michael Krufky <mkrufky@kernellabs.com>
Cc: Eddi De Pieri <eddi@depieri.net>
Cc: linux-media@vger.kernel.org
Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
---
 drivers/media/common/tuners/xc5000.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/media/common/tuners/xc5000.c b/drivers/media/common/tuners/xc5000.c
index dcca42c..4d33f86 100644
--- a/drivers/media/common/tuners/xc5000.c
+++ b/drivers/media/common/tuners/xc5000.c
@@ -210,13 +210,15 @@ struct xc5000_fw_cfg {
 	u16 size;
 };
 
+#define XC5000A_FIRMWARE "dvb-fe-xc5000-1.6.114.fw"
 static const struct xc5000_fw_cfg xc5000a_1_6_114 = {
-	.name = "dvb-fe-xc5000-1.6.114.fw",
+	.name = XC5000A_FIRMWARE,
 	.size = 12401,
 };
 
+#define XC5000C_FIRMWARE "dvb-fe-xc5000c-41.024.5.fw"
 static const struct xc5000_fw_cfg xc5000c_41_024_5 = {
-	.name = "dvb-fe-xc5000c-41.024.5.fw",
+	.name = XC5000C_FIRMWARE,
 	.size = 16497,
 };
 
@@ -1253,3 +1255,5 @@ EXPORT_SYMBOL(xc5000_attach);
 MODULE_AUTHOR("Steven Toth");
 MODULE_DESCRIPTION("Xceive xc5000 silicon tuner driver");
 MODULE_LICENSE("GPL");
+MODULE_FIRMWARE(XC5000A_FIRMWARE);
+MODULE_FIRMWARE(XC5000C_FIRMWARE);
-- 
1.7.9.5

Re: [PATCH] xc5000: Add MODULE_FIRMWARE statements

[Devin Heitmueller]

On Wed, Jul 25, 2012 at 9:15 AM, Tim Gardner <tim.gardner@canonical.com> wrote:
> This will make modinfo more useful with regard
> to discovering necessary firmware files.
>
> Cc: Mauro Carvalho Chehab <mchehab@infradead.org>
> Cc: Michael Krufky <mkrufky@kernellabs.com>
> Cc: Eddi De Pieri <eddi@depieri.net>
> Cc: linux-media@vger.kernel.org
> Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
> ---
>  drivers/media/common/tuners/xc5000.c |    8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/common/tuners/xc5000.c b/drivers/media/common/tuners/xc5000.c
> index dcca42c..4d33f86 100644
> --- a/drivers/media/common/tuners/xc5000.c
> +++ b/drivers/media/common/tuners/xc5000.c
> @@ -210,13 +210,15 @@ struct xc5000_fw_cfg {
>         u16 size;
>  };
>
> +#define XC5000A_FIRMWARE "dvb-fe-xc5000-1.6.114.fw"
>  static const struct xc5000_fw_cfg xc5000a_1_6_114 = {
> -       .name = "dvb-fe-xc5000-1.6.114.fw",
> +       .name = XC5000A_FIRMWARE,
>         .size = 12401,
>  };
>
> +#define XC5000C_FIRMWARE "dvb-fe-xc5000c-41.024.5.fw"
>  static const struct xc5000_fw_cfg xc5000c_41_024_5 = {
> -       .name = "dvb-fe-xc5000c-41.024.5.fw",
> +       .name = XC5000C_FIRMWARE,
>         .size = 16497,
>  };
>
> @@ -1253,3 +1255,5 @@ EXPORT_SYMBOL(xc5000_attach);
>  MODULE_AUTHOR("Steven Toth");
>  MODULE_DESCRIPTION("Xceive xc5000 silicon tuner driver");
>  MODULE_LICENSE("GPL");
> +MODULE_FIRMWARE(XC5000A_FIRMWARE);
> +MODULE_FIRMWARE(XC5000C_FIRMWARE);
> --

Hi Tim,

I'm just eyeballing the patch and I'm not familiar with this new
functionality, but where are the new macros you're specifying actually
defined?  You're swapping out the filename for XC5000A_FIRMWARE, but
where is the actual reference to "dvb-fe-xc5000-1.6.114.fw"?

Also, Mauro, can I merge this into my tree first rather than you
pulling it direct?  I've got a whole patch series for xc5000 that I'm
slated to issue a PULL for this weekend, and I *really* don't want to
rebase the series for a four line change (which will definitely cause
a conflict).

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com

Re: [PATCH] xc5000: Add MODULE_FIRMWARE statements

[Tim Gardner]

On 07/25/2012 07:24 AM, Devin Heitmueller wrote:
> On Wed, Jul 25, 2012 at 9:15 AM, Tim Gardner <tim.gardner@canonical.com> wrote:
>> This will make modinfo more useful with regard
>> to discovering necessary firmware files.
>>
>> Cc: Mauro Carvalho Chehab <mchehab@infradead.org>
>> Cc: Michael Krufky <mkrufky@kernellabs.com>
>> Cc: Eddi De Pieri <eddi@depieri.net>
>> Cc: linux-media@vger.kernel.org
>> Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
>> ---
>>  drivers/media/common/tuners/xc5000.c |    8 ++++++--
>>  1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/media/common/tuners/xc5000.c b/drivers/media/common/tuners/xc5000.c
>> index dcca42c..4d33f86 100644
>> --- a/drivers/media/common/tuners/xc5000.c
>> +++ b/drivers/media/common/tuners/xc5000.c
>> @@ -210,13 +210,15 @@ struct xc5000_fw_cfg {
>>         u16 size;
>>  };
>>
>> +#define XC5000A_FIRMWARE "dvb-fe-xc5000-1.6.114.fw"
>>  static const struct xc5000_fw_cfg xc5000a_1_6_114 = {
>> -       .name = "dvb-fe-xc5000-1.6.114.fw",
>> +       .name = XC5000A_FIRMWARE,
>>         .size = 12401,
>>  };
>>
>> +#define XC5000C_FIRMWARE "dvb-fe-xc5000c-41.024.5.fw"
>>  static const struct xc5000_fw_cfg xc5000c_41_024_5 = {
>> -       .name = "dvb-fe-xc5000c-41.024.5.fw",
>> +       .name = XC5000C_FIRMWARE,
>>         .size = 16497,
>>  };
>>
>> @@ -1253,3 +1255,5 @@ EXPORT_SYMBOL(xc5000_attach);
>>  MODULE_AUTHOR("Steven Toth");
>>  MODULE_DESCRIPTION("Xceive xc5000 silicon tuner driver");
>>  MODULE_LICENSE("GPL");
>> +MODULE_FIRMWARE(XC5000A_FIRMWARE);
>> +MODULE_FIRMWARE(XC5000C_FIRMWARE);
>> --
> 
> Hi Tim,
> 
> I'm just eyeballing the patch and I'm not familiar with this new
> functionality, but where are the new macros you're specifying actually
> defined?  You're swapping out the filename for XC5000A_FIRMWARE, but
> where is the actual reference to "dvb-fe-xc5000-1.6.114.fw"?
> 

Devin - Please have a closer look. XC5000A_FIRMWARE and XC5000C_FIRMWARE
are defined in the patch.

MODULE_FIRMWARE() is defined in linux/module.h. It creates a firmware
section such that modinfo can print the names of the firmware files that
may possibly be in use by this module.

rtg
-- 
Tim Gardner tim.gardner@canonical.com

Re: [PATCH] xc5000: Add MODULE_FIRMWARE statements

[Antti Palosaari]

On 07/25/2012 04:24 PM, Devin Heitmueller wrote:
> On Wed, Jul 25, 2012 at 9:15 AM, Tim Gardner <tim.gardner@canonical.com> wrote:
>> This will make modinfo more useful with regard
>> to discovering necessary firmware files.
>>
>> Cc: Mauro Carvalho Chehab <mchehab@infradead.org>
>> Cc: Michael Krufky <mkrufky@kernellabs.com>
>> Cc: Eddi De Pieri <eddi@depieri.net>
>> Cc: linux-media@vger.kernel.org
>> Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
>> ---
>>   drivers/media/common/tuners/xc5000.c |    8 ++++++--
>>   1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/media/common/tuners/xc5000.c b/drivers/media/common/tuners/xc5000.c
>> index dcca42c..4d33f86 100644
>> --- a/drivers/media/common/tuners/xc5000.c
>> +++ b/drivers/media/common/tuners/xc5000.c
>> @@ -210,13 +210,15 @@ struct xc5000_fw_cfg {
>>          u16 size;
>>   };
>>
>> +#define XC5000A_FIRMWARE "dvb-fe-xc5000-1.6.114.fw"
>>   static const struct xc5000_fw_cfg xc5000a_1_6_114 = {
>> -       .name = "dvb-fe-xc5000-1.6.114.fw",
>> +       .name = XC5000A_FIRMWARE,
>>          .size = 12401,
>>   };
>>
>> +#define XC5000C_FIRMWARE "dvb-fe-xc5000c-41.024.5.fw"
>>   static const struct xc5000_fw_cfg xc5000c_41_024_5 = {
>> -       .name = "dvb-fe-xc5000c-41.024.5.fw",
>> +       .name = XC5000C_FIRMWARE,
>>          .size = 16497,
>>   };
>>
>> @@ -1253,3 +1255,5 @@ EXPORT_SYMBOL(xc5000_attach);
>>   MODULE_AUTHOR("Steven Toth");
>>   MODULE_DESCRIPTION("Xceive xc5000 silicon tuner driver");
>>   MODULE_LICENSE("GPL");
>> +MODULE_FIRMWARE(XC5000A_FIRMWARE);
>> +MODULE_FIRMWARE(XC5000C_FIRMWARE);
>> --
>
> Hi Tim,
>
> I'm just eyeballing the patch and I'm not familiar with this new
> functionality, but where are the new macros you're specifying actually
> defined?  You're swapping out the filename for XC5000A_FIRMWARE, but
> where is the actual reference to "dvb-fe-xc5000-1.6.114.fw"?
>
> Also, Mauro, can I merge this into my tree first rather than you
> pulling it direct?  I've got a whole patch series for xc5000 that I'm
> slated to issue a PULL for this weekend, and I *really* don't want to
> rebase the series for a four line change (which will definitely cause
> a conflict).
>
> Devin

Also this issue has been spoken earlier and nacked. It was 2009 when Ben 
Hutchings sends large patch serie adding MODULE_FIRMWARE for every 
Linux-Media driver. I am not sure if arguments are changed after that to 
allow it now.
http://www.mail-archive.com/linux-media@vger.kernel.org/msg11373.html

regards
Antti

-- 
http://palosaari.fi/

Re: [PATCH] xc5000: Add MODULE_FIRMWARE statements

[Tim Gardner]

On 07/25/2012 07:48 AM, Antti Palosaari wrote:
> On 07/25/2012 04:24 PM, Devin Heitmueller wrote:
>> On Wed, Jul 25, 2012 at 9:15 AM, Tim Gardner
>> <tim.gardner@canonical.com> wrote:
>>> This will make modinfo more useful with regard
>>> to discovering necessary firmware files.
>>>
>>> Cc: Mauro Carvalho Chehab <mchehab@infradead.org>
>>> Cc: Michael Krufky <mkrufky@kernellabs.com>
>>> Cc: Eddi De Pieri <eddi@depieri.net>
>>> Cc: linux-media@vger.kernel.org
>>> Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
>>> ---
>>>   drivers/media/common/tuners/xc5000.c |    8 ++++++--
>>>   1 file changed, 6 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/media/common/tuners/xc5000.c
>>> b/drivers/media/common/tuners/xc5000.c
>>> index dcca42c..4d33f86 100644
>>> --- a/drivers/media/common/tuners/xc5000.c
>>> +++ b/drivers/media/common/tuners/xc5000.c
>>> @@ -210,13 +210,15 @@ struct xc5000_fw_cfg {
>>>          u16 size;
>>>   };
>>>
>>> +#define XC5000A_FIRMWARE "dvb-fe-xc5000-1.6.114.fw"
>>>   static const struct xc5000_fw_cfg xc5000a_1_6_114 = {
>>> -       .name = "dvb-fe-xc5000-1.6.114.fw",
>>> +       .name = XC5000A_FIRMWARE,
>>>          .size = 12401,
>>>   };
>>>
>>> +#define XC5000C_FIRMWARE "dvb-fe-xc5000c-41.024.5.fw"
>>>   static const struct xc5000_fw_cfg xc5000c_41_024_5 = {
>>> -       .name = "dvb-fe-xc5000c-41.024.5.fw",
>>> +       .name = XC5000C_FIRMWARE,
>>>          .size = 16497,
>>>   };
>>>
>>> @@ -1253,3 +1255,5 @@ EXPORT_SYMBOL(xc5000_attach);
>>>   MODULE_AUTHOR("Steven Toth");
>>>   MODULE_DESCRIPTION("Xceive xc5000 silicon tuner driver");
>>>   MODULE_LICENSE("GPL");
>>> +MODULE_FIRMWARE(XC5000A_FIRMWARE);
>>> +MODULE_FIRMWARE(XC5000C_FIRMWARE);
>>> -- 
>>
>> Hi Tim,
>>
>> I'm just eyeballing the patch and I'm not familiar with this new
>> functionality, but where are the new macros you're specifying actually
>> defined?  You're swapping out the filename for XC5000A_FIRMWARE, but
>> where is the actual reference to "dvb-fe-xc5000-1.6.114.fw"?
>>
>> Also, Mauro, can I merge this into my tree first rather than you
>> pulling it direct?  I've got a whole patch series for xc5000 that I'm
>> slated to issue a PULL for this weekend, and I *really* don't want to
>> rebase the series for a four line change (which will definitely cause
>> a conflict).
>>
>> Devin
> 
> Also this issue has been spoken earlier and nacked. It was 2009 when Ben
> Hutchings sends large patch serie adding MODULE_FIRMWARE for every
> Linux-Media driver. I am not sure if arguments are changed after that to
> allow it now.
> http://www.mail-archive.com/linux-media@vger.kernel.org/msg11373.html
> 
> regards
> Antti
> 

That discussion went completely off the rails. MODULE_FIRMWARE() is
purely informational and describes the firmware files that your driver
_might_ need. I'm using this information to pare down firmware files to
those that are actually used by the kernel. It makes quite a difference
in space required for Live CDs and on users disks.

rtg
-- 
Tim Gardner tim.gardner@canonical.com

Re: [PATCH] xc5000: Add MODULE_FIRMWARE statements

[Devin Heitmueller]

On Wed, Jul 25, 2012 at 9:43 AM, Tim Gardner <tim.gardner@canonical.com> wrote:
> Devin - Please have a closer look. XC5000A_FIRMWARE and XC5000C_FIRMWARE
> are defined in the patch.

Yup, my bad.  I looked at the patch twice but for some reason didn't
see the #define.

I'm not really taking a position on whether this approach is good or not.

Mauro, let me know if this should be accepted and if so I will stick
it onto the end of my tree before sending it upstream this weekend.

Thanks,

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com

Re: [PATCH] xc5000: Add MODULE_FIRMWARE statements

[Mauro Carvalho Chehab]

Em 25-07-2012 10:48, Antti Palosaari escreveu:
> On 07/25/2012 04:24 PM, Devin Heitmueller wrote:
>> On Wed, Jul 25, 2012 at 9:15 AM, Tim Gardner <tim.gardner@canonical.com> wrote:
>>> This will make modinfo more useful with regard
>>> to discovering necessary firmware files.
>>>
>>> Cc: Mauro Carvalho Chehab <mchehab@infradead.org>
>>> Cc: Michael Krufky <mkrufky@kernellabs.com>
>>> Cc: Eddi De Pieri <eddi@depieri.net>
>>> Cc: linux-media@vger.kernel.org
>>> Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
>>> ---
>>>   drivers/media/common/tuners/xc5000.c |    8 ++++++--
>>>   1 file changed, 6 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/media/common/tuners/xc5000.c b/drivers/media/common/tuners/xc5000.c
>>> index dcca42c..4d33f86 100644
>>> --- a/drivers/media/common/tuners/xc5000.c
>>> +++ b/drivers/media/common/tuners/xc5000.c
>>> @@ -210,13 +210,15 @@ struct xc5000_fw_cfg {
>>>          u16 size;
>>>   };
>>>
>>> +#define XC5000A_FIRMWARE "dvb-fe-xc5000-1.6.114.fw"
>>>   static const struct xc5000_fw_cfg xc5000a_1_6_114 = {
>>> -       .name = "dvb-fe-xc5000-1.6.114.fw",
>>> +       .name = XC5000A_FIRMWARE,
>>>          .size = 12401,
>>>   };
>>>
>>> +#define XC5000C_FIRMWARE "dvb-fe-xc5000c-41.024.5.fw"
>>>   static const struct xc5000_fw_cfg xc5000c_41_024_5 = {
>>> -       .name = "dvb-fe-xc5000c-41.024.5.fw",
>>> +       .name = XC5000C_FIRMWARE,
>>>          .size = 16497,
>>>   };
>>>
>>> @@ -1253,3 +1255,5 @@ EXPORT_SYMBOL(xc5000_attach);
>>>   MODULE_AUTHOR("Steven Toth");
>>>   MODULE_DESCRIPTION("Xceive xc5000 silicon tuner driver");
>>>   MODULE_LICENSE("GPL");
>>> +MODULE_FIRMWARE(XC5000A_FIRMWARE);
>>> +MODULE_FIRMWARE(XC5000C_FIRMWARE);
>>> -- 
>>
>> Hi Tim,
>>
>> I'm just eyeballing the patch and I'm not familiar with this new
>> functionality, but where are the new macros you're specifying actually
>> defined?  You're swapping out the filename for XC5000A_FIRMWARE, but
>> where is the actual reference to "dvb-fe-xc5000-1.6.114.fw"?
>>
>> Also, Mauro, can I merge this into my tree first rather than you
>> pulling it direct?  I've got a whole patch series for xc5000 that I'm
>> slated to issue a PULL for this weekend, and I *really* don't want to
>> rebase the series for a four line change (which will definitely cause
>> a conflict).

Devin,

As you didn't send your pull request in time for 3.6, I'll add this one,
as otherwise it will miss the 3.6 bus. 

You don't need to rebase your pull request due to that anyway, as, if a
merge conflict happens, I'll fix it, as it should be trivial.

I'm fixing trivial merge conflicts like that all the time. Just today, I
fixed maybe dozens of similar ones.

The conflicts that I ask developers to rework are the ones where the complex
logic is modified by two different patches.

>>
>> Devin
> 
> Also this issue has been spoken earlier and nacked. It was 2009 when Ben Hutchings sends large patch serie adding MODULE_FIRMWARE for every Linux-Media driver. I am not sure if arguments are changed after that to allow it now.
> http://www.mail-archive.com/linux-media@vger.kernel.org/msg11373.html

On Ben's patchset, firmwares were added to the bridge driver. That's wrong.

Adding it to the driver that actually uses the firmware looks OK on my eyes.

> regards
> Antti

Regards,
Mauro