All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tim Gardner <tim.gardner@canonical.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org,
	usb-storage@lists.one-eyed-alien.net
Subject: Re: [PATCH] ene_ub6250: Use macros for firmware names
Date: Tue, 24 Jul 2012 15:00:06 -0600	[thread overview]
Message-ID: <500F0CD6.7080907@canonical.com> (raw)
In-Reply-To: <20120724203448.GA1617@kroah.com>

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

  reply	other threads:[~2012-07-24 21:34 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
2012-07-24 21:29     ` Greg Kroah-Hartman
2012-07-24 21:58 ` Betty Dall
2012-07-27 16:53   ` [PATCH v2] " Tim Gardner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=500F0CD6.7080907@canonical.com \
    --to=tim.gardner@canonical.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=usb-storage@lists.one-eyed-alien.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.