Hi,
Arnd Bergmann wrote:
As very general comments, you should have the maintainer of the subsystem
(Pierre in this case) on Cc when posting a driver, and you should include
the patch inline in your mail, see Documentation/SubmittingPatches.
I have cc'ed both Pierre and Alex, but my first message was blocked
by the list as it contained html.
For inlinning, this is my first kernel patch. I have just followed
http://www.tux.org/lkml/#s2-2.
You should include the Makefile and Kconfig changes in the same patch/mail,
no point splitting these out.
Once again it was an advise from http://www.tux.org/lkml/#s1-10.
<http://www.tux.org/lkml/#s1-10>
Don't define your own DBG macro, instead use the predefined dev_dbg()
that has a similar definition.
Somewhere in 0.5-0.6 version this driver has issues with timeouts ,
which were
revealing in a non-debug kernel builds only . So this was a nessecity. I
will purge
them now.
Your mmc_tifm_irq_chip() function does a _very_ long delay of 100
miliseconds. This is normally not acceptable, since it is a noticeable
time in which the system is completely unresponsive. Maybe you can convert
the tasklet to a workqueue, which lets you call msleep instead of mdelay.
This is done intentionally to prevent a race condition when a card is
removed
and immediately reinserted. There may be a more complicated way to solve
this
issue, but didn't think about them. This only happens when an MMC/SD
card is
inserted/removed. And it takes at least as long to process the event in
other
parts of subsystem.
Your use of pci_map_sg() looks wrong, you simply can't assume that the
return value is '1' in general. I've stumbled over that same problem
in the sdhci driver, so it may be inherent to the mmc layer and not
be driver specific.
This is taken as is from [tifm_sd]. I suppose this relates to a hardware
limitation:
+ mmc->max_hw_segs = 1;
+ mmc->max_phys_segs = 1;
Best regards,
Sergey
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
