[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <46288645.3090201@gmail.com>
Date: Fri, 20 Apr 2007 12:22:13 +0300
From: Sergey Yanovich <ynvich@...il.com>
To: unlisted-recipients:; (no To-header on input)
CC: linux-kernel@...r.kernel.org
Subject: Re: [mmc] alternative TI FM MMC/SD driver for 2.6.21-rc7
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 majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists