Flash driver probe/commandset separation.
We seem to have an unnecessary amount of duplication of flash chip driver code. We have three drivers for AMD/Fujitsu compatible chips - cfi_cmdset_0002.c, amd_flash.c and jedec.c. We have two drivers for Intel/ Sharp compatible chips - cfi_cmdset_0001.c and sharp.c.
Erwin noticed this and wrote code to fall back to an AMD-compatible probe for JEDEC ID if the CFI probe failed, and then use cfi_cmdset_0002.c if appropriate. I think that was the right approach, but I don't like merging the probes together like that. I think the mfr-specific probes for JEDEC ID should be kept separate. The map driver can always call them in order if the CFI probe fails, if it wants to.
What I'd like to do is provide core drivers for each of the different command sets, and separate probe functions which set up the necessary parameters and invoke the core drivers.
The current cfi_cmdset_000x drivers are the most generic, so I'd like to use those as the basis of the core drivers, and ensure that they can fully replace the functionality of the other individual drivers.
We end up with:
cmdset_intel (and sharp, etc.)
cmdset_amd (and fujitsu, macronix, etc.)
cfi_probe
intel_jedec_probe
amd_jedec_probe
Comments?
Even assuming people agree (or disagree unconvincingly :), I'm not sure whether I should hold off on this until after I've sync'd up with Linus, or whether I should get this all done first and worry about feeding the new code to Linus later on. Probably the latter.
--
dwmw2
To unsubscribe, send "unsubscribe mtd" to [email protected]
Hi!
Quoted 6 lines
Comments?
Even assuming people agree (or disagree unconvincingly :), I'm not sure whether I should hold off on this until after I've sync'd up with Linus, or whether I should get this all done first and worry about feeding the new code to Linus later on. Probably the latter.
Lets commit the tree as soon it is reported to be stable to Linus. I have no major problems at the moment. So from my point of view we're ready to do the sync. After that we can play with all the stuff again :) Probably we should set a deadline and then freeze the code. There is always someone who commits a ugly (non working) patch 10 seconds before all will be send to Linus :-)
Bye
Florian
To unsubscribe, send "unsubscribe mtd" to [email protected]
Quoted 6 lines · [email protected] said:
Lets commit the tree as soon it is reported to be stable to Linus. I
have no major problems at the moment. So from my point of view we're ready to do the sync. After that we can play with all the stuff again :) Probably we should set a deadline and then freeze the code. There is always someone who commits a ugly (non working) patch 10 seconds before all will be send to Linus :-)
Yeah, you're probably right. I've made most of it compile again. We should just fix up the CFI command sets and declare it frozen.
--
dwmw2
To unsubscribe, send "unsubscribe mtd" to [email protected]
Quoted 3 lines · David Woodhouse
We seem to have an unnecessary amount of duplication of flash chip driver code. We have three drivers for AMD/Fujitsu compatible chips -
cfi_cmdset_0002.c,amd_flash.candjedec.c.
And I have a totally rewritten version of jedec.c hanging around needing to be merged jedec.c was so nasty.
Quoted 9 lines
We have two drivers for Intel/
Sharp compatible chips -cfi_cmdset_0001.candsharp.c.Erwin noticed this and wrote code to fall back to an AMD-compatible probe for JEDEC ID if the CFI probe failed, and then use
cfi_cmdset_0002.cif appropriate. I think that was the right approach, but I don't like merging the probes together like that. I think the mfr-specific probes for JEDEC ID should be kept separate. The map driver can always call them in order if the CFI probe fails, if it wants to.
As long as someone can compile the kernel without support for a specifc probe from menuconfig and get an image that still compiles. I have had some problems with that...
Quoted 8 lines
What I'd like to do is provide core drivers for each of the different command sets, and separate probe functions which set up the necessary parameters and invoke the core drivers.
The current
cfi_cmdset_000xdrivers are the most generic, so I'd like to use those as the basis of the core drivers, and ensure that they can fully replace the functionality of the other individual drivers.
Quoted 8 lines
We end up with:
cmdset_intel(and sharp, etc.)
cmdset_amd(and fujitsu, macronix, etc.)
cfi_probe
intel_jedec_probe
amd_jedec_probeComments?
This sounds like right way to go.
Reservations:
I'm not certain seperating the jedec probe by manufacturer is necessary, and it wouldn't suprise me if ordering might be important in a jedec probe. (If so we might only want to seperate the tables of chips).
I wonder if we need something like a revision parameter to the cmdset code, so we don't try to use new additions to the cmdset on old chips.
And I would like to double check the bus size usages. As last time I
looked cfi_cmdset_002 would do 16 bit accesses on an 8 bit bus. My
mapping driver deliberately didn't implement 16 or 32 bit reads to so
attempting to call them would oops the kernel. Shoot me I need to
merge my alpha mapping driver with the main mtd tree.
Quoted 4 lines
Even assuming people agree (or disagree unconvincingly :), I'm not sure whether I should hold off on this until after I've sync'd up with Linus, or whether I should get this all done first and worry about feeding the new code to Linus later on. Probably the latter.
P.s.
What is the right way to merge my code with the main mtd tree?
Eric
To unsubscribe, send "unsubscribe mtd" to [email protected]
Quoted 4 lines · [email protected] said:
I'm not certain seperating the jedec probe by manufacturer is
necessary, and it wouldn't suprise me if ordering might be important in a jedec probe. (If so we might only want to seperate the tables of chips).
There are many different ways of probing a chip to find out what its JEDEC ID is. This is the problem that CFI was designed to solve.
The probes we're referring to as 'jedec probe' are one method. There's another for Intel-compatible chips. PCMCIA devices often have the JEDEC IDs listed in the CIS.
Once you have the JEDEC IDs, you can then pass control to the right command set driver. But you need to do the right magic to probe for it first.
Quoted 2 lines · [email protected] said:
I wonder if we need something like a revision parameter to the cmdset
code, so we don't try to use new additions to the cmdset on old chips.
We have that already, to a certain extent. We do buffer writes on Intel chips only if we've already determined that the chips support it.
Quoted 5 lines · [email protected] said:
And I would like to double check the bus size usages. As last time I
lookedcfi_cmdset_002would do 16 bit accesses on an 8 bit bus. My
mapping driver deliberately didn't implement 16 or 32 bit reads to so
attempting to call them would oops the kernel. Shoot me I need to
merge my alpha mapping driver with the main mtd tree.
Let them oops. They shouldn't ever get called. Does this mapping driver work on the SX164? I'm severely tempted... :)
Quoted 1 line · [email protected] said:
P.s. What is the right way to merge my code with the main mtd tree?
Send me patches and/or SSH keys.
--
dwmw2
Quoted 20 lines · David Woodhouse
We seem to have an unnecessary amount of duplication of flash chip driver code. We have three drivers for AMD/Fujitsu compatible chips -
cfi_cmdset_0002.c,amd_flash.candjedec.c. We have two drivers for Intel/ Sharp compatible chips -cfi_cmdset_0001.candsharp.c.Erwin noticed this and wrote code to fall back to an AMD-compatible probe for JEDEC ID if the CFI probe failed, and then use
cfi_cmdset_0002.cif appropriate. I think that was the right approach, but I don't like merging the probes together like that. I think the mfr-specific probes for JEDEC ID should be kept separate. The map driver can always call them in order if the CFI probe fails, if it wants to.What I'd like to do is provide core drivers for each of the different command sets, and separate probe functions which set up the necessary parameters and invoke the core drivers.
The current
cfi_cmdset_000xdrivers are the most generic, so I'd like to use those as the basis of the core drivers, and ensure that they can fully replace the functionality of the other individual drivers.
Quoted 8 lines
We end up with:
cmdset_intel(and sharp, etc.)
cmdset_amd(and fujitsu, macronix, etc.)
cfi_probe
intel_jedec_probe
amd_jedec_probeComments?
Definitely the way to go. The probe and the cmdset code should be totally independent;
the structures in cfi.h are already set up as the interface. If the cfi probe fails then fall back to the JEDEC ID and provide the
the cfi info for the chips that don't support it. While we're at it,
perhaps we can add dynamic discovery of endianness, buswidth and command set.
Alice
Quoted 9 lines
Even assuming people agree (or disagree unconvincingly :), I'm not sure whether I should hold off on this until after I've sync'd up with Linus, or whether I should get this all done first and worry about feeding the new code to Linus later on. Probably the latter.
--
dwmw2
To unsubscribe, send "unsubscribe mtd" to [email protected]
Sorry for the delay -- I was on vacation.
Quoted 4 lines · David Woodhouse
Even assuming people agree (or disagree unconvincingly :), I'm not sure whether I should hold off on this until after I've sync'd up with Linus, or whether I should get this all done first and worry about feeding the new code to Linus later on. Probably the latter.
Personally I think it would be best to send the curent code to Linus and resend another update eventually when the new stuff would have been proven stable.
"release early, release often"
Nicolas
Quoted 5 lines · [email protected] said:
Personally I think it would be best to send the curent code to Linus
and resend another update eventually when the new stuff would have been proven stable."release early, release often"
Yes, you're probably right.
TODO list then:
1. Make cfi_cmdset_000[12].c compile again :)
2. Fix the partition stuff as discussed, removing VIRTUAL_ER
3. Sanity check DiskOnChip and NFTL drivers.
Anything else?
--
dwmw2
Quoted 7 lines · David Woodhouse
Quoted 5 lines · [email protected] said:
Personally I think it would be best to send the curent code to Linus
and resend another update eventually when the new stuff would have been proven stable."release early, release often"
Quoted 7 lines
Yes, you're probably right.
TODO list then:
1. Make cfi_cmdset_000[12].c compile again :)
2. Fix the partition stuff as discussed, removingVIRTUAL_ER
3. Sanity check DiskOnChip and NFTL drivers.
4. Don't hardcode the cmdset in cfi_jedec.
I keep using very different flash chips so I want one probe path for all of the chips looks good.
Note there is a good argument for joining the probe paths Handling chip interleaving, unless we can make it simple we don't want that duplicated many times.
I'll try to have something working for 2 & 4 by the end of today. Any hints on why the cmdset drivers don't compile?
Eric
Quoted 3 lines · [email protected] said:
Note there is a good argument for joining the probe paths Handling
chip interleaving, unless we can make it simple we don't want that duplicated many times.
True. The build_cmd et al macros prevent duplication to a certain extent.
Quoted 2 lines
I'll try to have something working for 2 & 4 by the end of today. Any hints on why the cmdset drivers don't compile?
I changed the chip driver registration stuff. They need to have a struct mtd_chip_driver and set mtd->fldrv to point to it. And I'd like to make the inter_module_xxx stuff go away completely - maybe they can register with the code in chipreg instead, and get cfi_probe to look them up that way. That can wait a little while though - I'm not sure that's the correct approach (because they're not actually probes).
--
dwmw2
Quoted 5 lines · David Woodhouse
Quoted 3 lines · [email protected] said:
Note there is a good argument for joining the probe paths Handling
chip interleaving, unless we can make it simple we don't want that duplicated many times.
Quoted 7 lines
True. The
build_cmdet al macros prevent duplication to a certain extent.Quoted 2 lines
I'll try to have something working for 2 & 4 by the end of today. Any hints on why the cmdset drivers don't compile?
I changed the chip driver registration stuff. They need to have a struct
mtd_chip_driverand setmtd->fldrvto point to it.
I'll look I currently have the simply moved the destroy function into cfi_probe.
Quoted 5 lines
And I'd like to
make theinter_module_xxxstuff go away completely - maybe they can register with the code in chipreg instead, and getcfi_probeto look them up that way. That can wait a little while though - I'm not sure that's the correct approach (because they're not actually probes).
Right, but something smiliar code work. Or we could make them serve dual duty.
Eric