* [PATCH] ene_ub6250: Use macros for firmware names
@ 2012-07-24 20:31 Tim Gardner
2012-07-24 20:34 ` Greg Kroah-Hartman
2012-07-24 21:58 ` Betty Dall
0 siblings, 2 replies; 6+ messages in thread
From: Tim Gardner @ 2012-07-24 20:31 UTC (permalink / raw)
To: linux-kernel; +Cc: Tim Gardner, Greg Kroah-Hartman, linux-usb, usb-storage
Advertise firmware files using MODULE_FIRMWARE macros.
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-usb@vger.kernel.org
Cc: usb-storage@lists.one-eyed-alien.net
Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
---
drivers/usb/storage/ene_ub6250.c | 26 +++++++++++++++++++-------
1 file changed, 19 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/storage/ene_ub6250.c b/drivers/usb/storage/ene_ub6250.c
index b28f2ad..3fec82f 100644
--- a/drivers/usb/storage/ene_ub6250.c
+++ b/drivers/usb/storage/ene_ub6250.c
@@ -29,9 +29,21 @@
#include "protocol.h"
#include "debug.h"
+#define SD_INIT1_FIRMWARE "ene-ub6250/sd_init1.bin"
+#define SD_INIT2_FIRMWARE "ene-ub6250/sd_init2.bin"
+#define SD_RW_FIRMWARE "ene-ub6250/sd_rdwr.bin"
+#define MS_INIT_FIRMWARE "ene-ub6250/ms_init.bin"
+#define MSP_RW_FIRMWARE "ene-ub6250/msp_rdwr.bin"
+#define MS_RW_FIRMWARE "ene-ub6250/ms_rdwr.bin"
+
MODULE_DESCRIPTION("Driver for ENE UB6250 reader");
MODULE_LICENSE("GPL");
-
+MODULE_FIRMWARE(SD_INIT1_FIRMWARE);
+MODULE_FIRMWARE(SD_INIT2_FIRMWARE);
+MODULE_FIRMWARE(SD_RW_FIRMWARE);
+MODULE_FIRMWARE(MS_INIT_FIRMWARE);
+MODULE_FIRMWARE(MSP_RW_FIRMWARE);
+MODULE_FIRMWARE(MS_RW_FIRMWARE);
/*
* The table of devices
@@ -1883,28 +1895,28 @@ static int ene_load_bincode(struct us_data *us, unsigned char flag)
/* For SD */
case SD_INIT1_PATTERN:
US_DEBUGP("SD_INIT1_PATTERN\n");
- fw_name = "ene-ub6250/sd_init1.bin";
+ fw_name = SD_INIT1_FIRMWARE;
break;
case SD_INIT2_PATTERN:
US_DEBUGP("SD_INIT2_PATTERN\n");
- fw_name = "ene-ub6250/sd_init2.bin";
+ fw_name = SD_INIT2_FIRMWARE;
break;
case SD_RW_PATTERN:
US_DEBUGP("SD_RDWR_PATTERN\n");
- fw_name = "ene-ub6250/sd_rdwr.bin";
+ fw_name = SD_RW_FIRMWARE;
break;
/* For MS */
case MS_INIT_PATTERN:
US_DEBUGP("MS_INIT_PATTERN\n");
- fw_name = "ene-ub6250/ms_init.bin";
+ fw_name = MS_INIT_FIRMWARE;
break;
case MSP_RW_PATTERN:
US_DEBUGP("MSP_RW_PATTERN\n");
- fw_name = "ene-ub6250/msp_rdwr.bin";
+ fw_name = MSP_RW_FIRMWARE;
break;
case MS_RW_PATTERN:
US_DEBUGP("MS_RW_PATTERN\n");
- fw_name = "ene-ub6250/ms_rdwr.bin";
+ fw_name = MS_RW_FIRMWARE;
break;
default:
US_DEBUGP("----------- Unknown PATTERN ----------\n");
--
1.7.9.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] ene_ub6250: Use macros for firmware names
2012-07-24 20:31 [PATCH] ene_ub6250: Use macros for firmware names Tim Gardner
@ 2012-07-24 20:34 ` Greg Kroah-Hartman
2012-07-24 21:00 ` Tim Gardner
2012-07-24 21:58 ` Betty Dall
1 sibling, 1 reply; 6+ messages in thread
From: Greg Kroah-Hartman @ 2012-07-24 20:34 UTC (permalink / raw)
To: Tim Gardner; +Cc: linux-kernel, linux-usb, usb-storage
On Tue, Jul 24, 2012 at 02:31:09PM -0600, Tim Gardner wrote:
> Advertise firmware files using MODULE_FIRMWARE macros.
>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: linux-usb@vger.kernel.org
> Cc: usb-storage@lists.one-eyed-alien.net
> Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
> ---
> drivers/usb/storage/ene_ub6250.c | 26 +++++++++++++++++++-------
> 1 file changed, 19 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/usb/storage/ene_ub6250.c b/drivers/usb/storage/ene_ub6250.c
> index b28f2ad..3fec82f 100644
> --- a/drivers/usb/storage/ene_ub6250.c
> +++ b/drivers/usb/storage/ene_ub6250.c
> @@ -29,9 +29,21 @@
> #include "protocol.h"
> #include "debug.h"
>
> +#define SD_INIT1_FIRMWARE "ene-ub6250/sd_init1.bin"
> +#define SD_INIT2_FIRMWARE "ene-ub6250/sd_init2.bin"
> +#define SD_RW_FIRMWARE "ene-ub6250/sd_rdwr.bin"
> +#define MS_INIT_FIRMWARE "ene-ub6250/ms_init.bin"
> +#define MSP_RW_FIRMWARE "ene-ub6250/msp_rdwr.bin"
> +#define MS_RW_FIRMWARE "ene-ub6250/ms_rdwr.bin"
> +
> MODULE_DESCRIPTION("Driver for ENE UB6250 reader");
> MODULE_LICENSE("GPL");
> -
> +MODULE_FIRMWARE(SD_INIT1_FIRMWARE);
> +MODULE_FIRMWARE(SD_INIT2_FIRMWARE);
> +MODULE_FIRMWARE(SD_RW_FIRMWARE);
> +MODULE_FIRMWARE(MS_INIT_FIRMWARE);
> +MODULE_FIRMWARE(MSP_RW_FIRMWARE);
> +MODULE_FIRMWARE(MS_RW_FIRMWARE);
Why do you need the #defines here at all? What's wrong with just using
the file names in the MODULE_FIRMWARE() macro directly? That cuts the
size of the patch in half :)
thanks,
greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ene_ub6250: Use macros for firmware names
2012-07-24 20:34 ` Greg Kroah-Hartman
@ 2012-07-24 21:00 ` Tim Gardner
2012-07-24 21:29 ` Greg Kroah-Hartman
0 siblings, 1 reply; 6+ messages in thread
From: Tim Gardner @ 2012-07-24 21:00 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: linux-kernel, linux-usb, usb-storage
On 07/24/2012 02:34 PM, Greg Kroah-Hartman wrote:
> On Tue, Jul 24, 2012 at 02:31:09PM -0600, Tim Gardner wrote:
>> Advertise firmware files using MODULE_FIRMWARE macros.
>>
>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> Cc: linux-usb@vger.kernel.org
>> Cc: usb-storage@lists.one-eyed-alien.net
>> Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
>> ---
>> drivers/usb/storage/ene_ub6250.c | 26 +++++++++++++++++++-------
>> 1 file changed, 19 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/usb/storage/ene_ub6250.c b/drivers/usb/storage/ene_ub6250.c
>> index b28f2ad..3fec82f 100644
>> --- a/drivers/usb/storage/ene_ub6250.c
>> +++ b/drivers/usb/storage/ene_ub6250.c
>> @@ -29,9 +29,21 @@
>> #include "protocol.h"
>> #include "debug.h"
>>
>> +#define SD_INIT1_FIRMWARE "ene-ub6250/sd_init1.bin"
>> +#define SD_INIT2_FIRMWARE "ene-ub6250/sd_init2.bin"
>> +#define SD_RW_FIRMWARE "ene-ub6250/sd_rdwr.bin"
>> +#define MS_INIT_FIRMWARE "ene-ub6250/ms_init.bin"
>> +#define MSP_RW_FIRMWARE "ene-ub6250/msp_rdwr.bin"
>> +#define MS_RW_FIRMWARE "ene-ub6250/ms_rdwr.bin"
>> +
>> MODULE_DESCRIPTION("Driver for ENE UB6250 reader");
>> MODULE_LICENSE("GPL");
>> -
>> +MODULE_FIRMWARE(SD_INIT1_FIRMWARE);
>> +MODULE_FIRMWARE(SD_INIT2_FIRMWARE);
>> +MODULE_FIRMWARE(SD_RW_FIRMWARE);
>> +MODULE_FIRMWARE(MS_INIT_FIRMWARE);
>> +MODULE_FIRMWARE(MSP_RW_FIRMWARE);
>> +MODULE_FIRMWARE(MS_RW_FIRMWARE);
>
> Why do you need the #defines here at all? What's wrong with just using
> the file names in the MODULE_FIRMWARE() macro directly? That cuts the
> size of the patch in half :)
>
> thanks,
>
> greg k-h
>
If the firmware file name ever changes, then you'll have to find and
modify it in 2 places.
I don't really have a strong preference, but I would like to see
MODULE_FIRMWARE() used so I can cut down on the number of false
positives as I go through the kernel firmware directory and the
linux-firmware package to filter out unused files using modinfo.
rtg
--
Tim Gardner tim.gardner@canonical.com
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ene_ub6250: Use macros for firmware names
2012-07-24 21:00 ` Tim Gardner
@ 2012-07-24 21:29 ` Greg Kroah-Hartman
0 siblings, 0 replies; 6+ messages in thread
From: Greg Kroah-Hartman @ 2012-07-24 21:29 UTC (permalink / raw)
To: Tim Gardner; +Cc: linux-kernel, linux-usb, usb-storage
On Tue, Jul 24, 2012 at 03:00:06PM -0600, Tim Gardner wrote:
> On 07/24/2012 02:34 PM, Greg Kroah-Hartman wrote:
> >On Tue, Jul 24, 2012 at 02:31:09PM -0600, Tim Gardner wrote:
> >>Advertise firmware files using MODULE_FIRMWARE macros.
> >>
> >>Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> >>Cc: linux-usb@vger.kernel.org
> >>Cc: usb-storage@lists.one-eyed-alien.net
> >>Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
> >>---
> >> drivers/usb/storage/ene_ub6250.c | 26 +++++++++++++++++++-------
> >> 1 file changed, 19 insertions(+), 7 deletions(-)
> >>
> >>diff --git a/drivers/usb/storage/ene_ub6250.c b/drivers/usb/storage/ene_ub6250.c
> >>index b28f2ad..3fec82f 100644
> >>--- a/drivers/usb/storage/ene_ub6250.c
> >>+++ b/drivers/usb/storage/ene_ub6250.c
> >>@@ -29,9 +29,21 @@
> >> #include "protocol.h"
> >> #include "debug.h"
> >>
> >>+#define SD_INIT1_FIRMWARE "ene-ub6250/sd_init1.bin"
> >>+#define SD_INIT2_FIRMWARE "ene-ub6250/sd_init2.bin"
> >>+#define SD_RW_FIRMWARE "ene-ub6250/sd_rdwr.bin"
> >>+#define MS_INIT_FIRMWARE "ene-ub6250/ms_init.bin"
> >>+#define MSP_RW_FIRMWARE "ene-ub6250/msp_rdwr.bin"
> >>+#define MS_RW_FIRMWARE "ene-ub6250/ms_rdwr.bin"
> >>+
> >> MODULE_DESCRIPTION("Driver for ENE UB6250 reader");
> >> MODULE_LICENSE("GPL");
> >>-
> >>+MODULE_FIRMWARE(SD_INIT1_FIRMWARE);
> >>+MODULE_FIRMWARE(SD_INIT2_FIRMWARE);
> >>+MODULE_FIRMWARE(SD_RW_FIRMWARE);
> >>+MODULE_FIRMWARE(MS_INIT_FIRMWARE);
> >>+MODULE_FIRMWARE(MSP_RW_FIRMWARE);
> >>+MODULE_FIRMWARE(MS_RW_FIRMWARE);
> >
> >Why do you need the #defines here at all? What's wrong with just using
> >the file names in the MODULE_FIRMWARE() macro directly? That cuts the
> >size of the patch in half :)
> >
> >thanks,
> >
> >greg k-h
> >
>
> If the firmware file name ever changes, then you'll have to find and
> modify it in 2 places.
Oops, sorry, I missed the second place it was used in the file,
nevermind, time for more coffee...
greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ene_ub6250: Use macros for firmware names
2012-07-24 20:31 [PATCH] ene_ub6250: Use macros for firmware names Tim Gardner
2012-07-24 20:34 ` Greg Kroah-Hartman
@ 2012-07-24 21:58 ` Betty Dall
2012-07-27 16:53 ` [PATCH v2] " Tim Gardner
1 sibling, 1 reply; 6+ messages in thread
From: Betty Dall @ 2012-07-24 21:58 UTC (permalink / raw)
To: Tim Gardner; +Cc: linux-kernel, Greg Kroah-Hartman, linux-usb, usb-storage
Hi Tim,
I reviewed this patch and it looks good. Once small suggestion you can
take or leave...
On Tue, 2012-07-24 at 14:31 -0600, Tim Gardner wrote:
> Advertise firmware files using MODULE_FIRMWARE macros.
>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: linux-usb@vger.kernel.org
> Cc: usb-storage@lists.one-eyed-alien.net
> Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
> ---
> drivers/usb/storage/ene_ub6250.c | 26 +++++++++++++++++++-------
> 1 file changed, 19 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/usb/storage/ene_ub6250.c b/drivers/usb/storage/ene_ub6250.c
> index b28f2ad..3fec82f 100644
> --- a/drivers/usb/storage/ene_ub6250.c
> +++ b/drivers/usb/storage/ene_ub6250.c
> @@ -29,9 +29,21 @@
> #include "protocol.h"
> #include "debug.h"
>
> +#define SD_INIT1_FIRMWARE "ene-ub6250/sd_init1.bin"
> +#define SD_INIT2_FIRMWARE "ene-ub6250/sd_init2.bin"
> +#define SD_RW_FIRMWARE "ene-ub6250/sd_rdwr.bin"
> +#define MS_INIT_FIRMWARE "ene-ub6250/ms_init.bin"
> +#define MSP_RW_FIRMWARE "ene-ub6250/msp_rdwr.bin"
> +#define MS_RW_FIRMWARE "ene-ub6250/ms_rdwr.bin"
> +
> MODULE_DESCRIPTION("Driver for ENE UB6250 reader");
> MODULE_LICENSE("GPL");
> -
> +MODULE_FIRMWARE(SD_INIT1_FIRMWARE);
> +MODULE_FIRMWARE(SD_INIT2_FIRMWARE);
> +MODULE_FIRMWARE(SD_RW_FIRMWARE);
> +MODULE_FIRMWARE(MS_INIT_FIRMWARE);
> +MODULE_FIRMWARE(MSP_RW_FIRMWARE);
> +MODULE_FIRMWARE(MS_RW_FIRMWARE);
>
> /*
> * The table of devices
> @@ -1883,28 +1895,28 @@ static int ene_load_bincode(struct us_data *us, unsigned char flag)
> /* For SD */
> case SD_INIT1_PATTERN:
> US_DEBUGP("SD_INIT1_PATTERN\n");
> - fw_name = "ene-ub6250/sd_init1.bin";
> + fw_name = SD_INIT1_FIRMWARE;
> break;
> case SD_INIT2_PATTERN:
> US_DEBUGP("SD_INIT2_PATTERN\n");
> - fw_name = "ene-ub6250/sd_init2.bin";
> + fw_name = SD_INIT2_FIRMWARE;
> break;
> case SD_RW_PATTERN:
> US_DEBUGP("SD_RDWR_PATTERN\n");
All the other rdwr patterns are RW not RDWR. So you could change this
one to be SD_RW_PATTERN to be consistent with MSP_RW_PATTERN and
MS_RW_PATTERN. This is a nit.
> - fw_name = "ene-ub6250/sd_rdwr.bin";
> + fw_name = SD_RW_FIRMWARE;
> break;
> /* For MS */
> case MS_INIT_PATTERN:
> US_DEBUGP("MS_INIT_PATTERN\n");
> - fw_name = "ene-ub6250/ms_init.bin";
> + fw_name = MS_INIT_FIRMWARE;
> break;
> case MSP_RW_PATTERN:
> US_DEBUGP("MSP_RW_PATTERN\n");
> - fw_name = "ene-ub6250/msp_rdwr.bin";
> + fw_name = MSP_RW_FIRMWARE;
> break;
> case MS_RW_PATTERN:
> US_DEBUGP("MS_RW_PATTERN\n");
> - fw_name = "ene-ub6250/ms_rdwr.bin";
> + fw_name = MS_RW_FIRMWARE;
> break;
> default:
> US_DEBUGP("----------- Unknown PATTERN ----------\n");
-Betty
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] ene_ub6250: Use macros for firmware names
2012-07-24 21:58 ` Betty Dall
@ 2012-07-27 16:53 ` Tim Gardner
0 siblings, 0 replies; 6+ messages in thread
From: Tim Gardner @ 2012-07-27 16:53 UTC (permalink / raw)
To: linux-kernel; +Cc: Tim Gardner, Greg Kroah-Hartman, linux-usb, usb-storage
Advertise firmware files using MODULE_FIRMWARE macros.
Fix a debug string: SD_RDWR_PATTERN --> SD_RW_PATTERN
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-usb@vger.kernel.org
Cc: usb-storage@lists.one-eyed-alien.net
Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
---
drivers/usb/storage/ene_ub6250.c | 28 ++++++++++++++++++++--------
1 file changed, 20 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/storage/ene_ub6250.c b/drivers/usb/storage/ene_ub6250.c
index b28f2ad..95edee5 100644
--- a/drivers/usb/storage/ene_ub6250.c
+++ b/drivers/usb/storage/ene_ub6250.c
@@ -29,9 +29,21 @@
#include "protocol.h"
#include "debug.h"
+#define SD_INIT1_FIRMWARE "ene-ub6250/sd_init1.bin"
+#define SD_INIT2_FIRMWARE "ene-ub6250/sd_init2.bin"
+#define SD_RW_FIRMWARE "ene-ub6250/sd_rdwr.bin"
+#define MS_INIT_FIRMWARE "ene-ub6250/ms_init.bin"
+#define MSP_RW_FIRMWARE "ene-ub6250/msp_rdwr.bin"
+#define MS_RW_FIRMWARE "ene-ub6250/ms_rdwr.bin"
+
MODULE_DESCRIPTION("Driver for ENE UB6250 reader");
MODULE_LICENSE("GPL");
-
+MODULE_FIRMWARE(SD_INIT1_FIRMWARE);
+MODULE_FIRMWARE(SD_INIT2_FIRMWARE);
+MODULE_FIRMWARE(SD_RW_FIRMWARE);
+MODULE_FIRMWARE(MS_INIT_FIRMWARE);
+MODULE_FIRMWARE(MSP_RW_FIRMWARE);
+MODULE_FIRMWARE(MS_RW_FIRMWARE);
/*
* The table of devices
@@ -1883,28 +1895,28 @@ static int ene_load_bincode(struct us_data *us, unsigned char flag)
/* For SD */
case SD_INIT1_PATTERN:
US_DEBUGP("SD_INIT1_PATTERN\n");
- fw_name = "ene-ub6250/sd_init1.bin";
+ fw_name = SD_INIT1_FIRMWARE;
break;
case SD_INIT2_PATTERN:
US_DEBUGP("SD_INIT2_PATTERN\n");
- fw_name = "ene-ub6250/sd_init2.bin";
+ fw_name = SD_INIT2_FIRMWARE;
break;
case SD_RW_PATTERN:
- US_DEBUGP("SD_RDWR_PATTERN\n");
- fw_name = "ene-ub6250/sd_rdwr.bin";
+ US_DEBUGP("SD_RW_PATTERN\n");
+ fw_name = SD_RW_FIRMWARE;
break;
/* For MS */
case MS_INIT_PATTERN:
US_DEBUGP("MS_INIT_PATTERN\n");
- fw_name = "ene-ub6250/ms_init.bin";
+ fw_name = MS_INIT_FIRMWARE;
break;
case MSP_RW_PATTERN:
US_DEBUGP("MSP_RW_PATTERN\n");
- fw_name = "ene-ub6250/msp_rdwr.bin";
+ fw_name = MSP_RW_FIRMWARE;
break;
case MS_RW_PATTERN:
US_DEBUGP("MS_RW_PATTERN\n");
- fw_name = "ene-ub6250/ms_rdwr.bin";
+ fw_name = MS_RW_FIRMWARE;
break;
default:
US_DEBUGP("----------- Unknown PATTERN ----------\n");
--
1.7.9.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-07-27 16:52 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-07-24 20:31 [PATCH] ene_ub6250: Use macros for firmware names Tim Gardner
2012-07-24 20:34 ` Greg Kroah-Hartman
2012-07-24 21:00 ` Tim Gardner
2012-07-24 21:29 ` Greg Kroah-Hartman
2012-07-24 21:58 ` Betty Dall
2012-07-27 16:53 ` [PATCH v2] " Tim Gardner
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox