Messages in this thread | | | Date | Sat, 22 Apr 2006 21:48:28 -0600 | From | "Jim Ramsay" <> | Subject | Re: Possible MTD bug in 2.6.15 |
| |
On 22/04/06, Thiago Galesi <thiagogalesi@gmail.com> wrote: > > > > Ok, a couple of comments/questions > > > > > > > > 1 - Wouldn't it be better to map all flash, and leave the unneeded > > > > part as read only? > > > > In general, yes. But this should either be enforced somewhere nicer > > (ie, die gracefully) so the kernel doesn't panic later, or be allowed > > as in my patch. > > The fundamental problem there seems to be a mismatch between what is > set by the user and what is read from the flash chip. As you mention > in your first message, (what it came across is that) you don't have > (physical / electrical) access to all the flash; not something I would > recommend (that is, having limited electrical connection to the flash)
Yes, that is exactly what is going on - we have only have electrical access to 32M of addresses regardless of the size of the actual chip installed, and unfortunately I can't change that. However, it's not really a problem from the electrical side of things - We can still access the lowest 32M on the chip, just the highest address pin isn't connected to anything.
> As for the options you propose - enforce and die gracefully (that is, > if there is a size mismatch, warning and purposely not working) seems > more correct than the second option.
I don't see why - The size mismatch doesn't prevent the flash chip from functioning. Isn't it better to help things work in more circumstances rather than in less?
And really, it's not a big stretch from what the code currently does to what my patch changes. At this point in the code, we know for a fact that we already have at least one flash chip. The math that's going on here with the 'max_chips' variable is to check if there is actually more than one physical chip implementing the entire reported size. The only mistake is that the math goes too far, shifting the count down to zero if the reported size is too small. This 'max_chips' should never be allowed to be lower than 1, because we really do know that there is at least one flash chip.
I agree with you that it's probably "more correct" to actually specify the real size of the chip in the 'map' struct before the CFI probe, but I can't think of any reason why specifying a smaller size should die (even gracefully) when it still works just fine.
Of course, maybe there are some models of flash chips where this wouldn't work and a graceful prevention of this would be important. But with our CFI-compliant chip, we don't see any problem with my patch applied.
-- Jim Ramsay "Me fail English? That's unpossible!" - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |