Pierre Ossman wrote:
On Fri, 31 Jul 2009 11:54:07 +0100
Matt Fleming <matt@xxxxxxxxxxxxxxxxx> wrote:
On Fri, Jul 31, 2009 at 12:26:23PM +0200, Pierre Ossman wrote:
[PATCH 0/32] mmc and omap_hsmmc patches
http://marc.info/?t=124722953900010&r=1&w=2
I haven't looked through these at all. The ones affecting the core
probably need some thorough reviews.
I did notice the patch to say which cards a controller supports though,
and I'm very sceptical about that one. The scanning process should work
anyway, and the performance impact should be negligible as it is only
on init. So that patch only adds complexity and confusion IMO.
How much complexity does it really add? Surely it's better to give the
host controller driver writers the ability to not entertain supporting
some cards if they cannot be used? If they want to avoid the scanning
process for certain cards, why not let them?
Let's look at the pros and cons of this:
But the cons are all subjective.
Con:
- The scanning code gets less clear as you increase the number of
possible paths through it.
- Different systems will have different init sequences, possibly
provoking bugs in the cards.
- Host driver writers now have more capability bits they have to
consider. And these might be less than obvious since SD/MMC/SDIO are
normally compatible so these bits seem useless.
- With the current logic (which was better in the first version),
"normal" drivers will have to explicitly state that they work as
intended by setting all bits.
And the pro is objective.
Pro:
- A slightly reduced scanning time.
That's great! Why do you disregard this so easily?
I simply don't see it as being worth it. Linux patches generally need
to provide the answer to "Why?", not just be able to avoid "Why not?".
You have just supplied answers to both "Why?" and "Why not?".
In my opinion "Why?" outweighs "Why not?" because the pro is objective and
the cons and not.
--
To unsubscribe from this list: send the line "unsubscribe linux-embedded" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
