Re: [PATCH] mmc: USB SDIO/SD/MMC Host Controller (VUB300) driver Resubmission

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, 23 Jan 2011, Wolfram Sang wrote:

> On Sat, Jan 22, 2011 at 02:07:21PM -0500, Nicolas Pitre wrote:
> > On Sat, 22 Jan 2011, Wolfram Sang wrote:
> > 
> > > On Fri, Jan 21, 2011 at 04:14:46PM -0500, Nicolas Pitre wrote:
> > > > On Fri, 21 Jan 2011, Tony Olech wrote:
> > > > 
> > > > > Add a driver for Elan Digital System's VUB300 chip
> > > > > which is a USB connected SDIO/SDmem/MMC host controller.
> > > > > A VUB300 chip enables a USB 2.0 or USB 1.1 connected host
> > > > > computer to use SDIO/SD/MMC cards without the need for
> > > > > a directly connected, for example via PCI, SDIO host
> > > > > controller.
> > > > > 
> > > > > Signed-off-by: Anthony F Olech <tony.olech@xxxxxxxxxxxxxxxxxxxxxx>
> > > > > ---
> > > > > This is the second submission attempt.
> > > > > There are 5 "do not initialise statics" errors reported by scripts/checkpatch.pl
> > > > 
> > > > you probably should fix those.  They are due to lines such as:
> > > > 
> > > > static int pad_input_to_usb_pkt = 0;
> > > > static int disable_offload_processing = 0;
> > > > static int force_1_bit_data_xfers = 0;
> > > > static int force_polling_for_irqs = 0;
> > > 
> > > Should those be static anyhow? Being USB, you could probably hook up two of
> > > those and want to operate one of them in this and the other one in another
> > > mode?
> > 
> > You could.  Whether or not you would is another question.  Until then I 
> > don't think it is worth bothering with such corner cases for the initial 
> > merging of this driver.
> 
> A static variable which should be per-device is a corner-case? Frankly, I'd
> think it is a flaw. Unless, of course, these module_params are only rarely
> used. Which would lead to the question if they are really needed. There are
> quite a lot.

Given their name, I'd say they are there only for debugging purposes.  
So yes, they probably are rarely used, which doesn't mean they're 
useless.  Same thing goes for maxfreq and nodma in mvsdio.c for example.

> Also, I am not convinced of the need of a custom sysfs-file. Seeing its use
> cases might help to identify a need which is probably better solved
> generically.

I'm sure the author of this driver would be eager to hear from you with 
a concrete suggestion.


Nicolas
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux