Flash driver probe/commandset separation.

Flash driver probe/commandset separation.

[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.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 majordomo@infradead.org

AW: Flash driver probe/commandset separation.

[Florian Schirmer / TayTron]

Hi!

>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 majordomo@infradead.org

Re: AW: Flash driver probe/commandset separation.

[David Woodhouse]

schirmer@taytron.net 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 majordomo@infradead.org

Re: Flash driver probe/commandset separation.

[Eric W. Biederman]

David Woodhouse <dwmw2@infradead.org> writes:

> 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. 

And I have a totally rewritten version of jedec.c hanging around
needing to be merged jedec.c was so nasty.

> 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.

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...
 
> 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?

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. 


> 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 majordomo@infradead.org

Re: Flash driver probe/commandset separation.

[David Woodhouse]

ebiederman@lnxi.com 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.


ebiederman@lnxi.com 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.


ebiederman@lnxi.com said:
>  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.  

Let them oops. They shouldn't ever get called. Does this mapping driver 
work on the SX164? I'm severely tempted... :)


ebiederman@lnxi.com 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

Re: Flash driver probe/commandset separation.

[Alice Hennessy]

David Woodhouse wrote:

> 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?

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

>
>
> 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 majordomo@infradead.org

Re: Flash driver probe/commandset separation.

[Nicolas Pitre]

Sorry for the delay -- I was on vacation.

On Sun, 29 Apr 2001, David Woodhouse wrote:

> 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

Re: Flash driver probe/commandset separation.

[David Woodhouse]

nico@cam.org 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

Re: Flash driver probe/commandset separation.

[Eric W. Biederman]

David Woodhouse <dwmw2@infradead.org> writes:

> nico@cam.org 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. 
        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

Re: Flash driver probe/commandset separation.

[David Woodhouse]

ebiederman@lnxi.com 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. 

> 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

Re: Flash driver probe/commandset separation.

[Eric W. Biederman]

David Woodhouse <dwmw2@infradead.org> writes:

> ebiederman@lnxi.com 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. 
> 
> > 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. 

I'll look I currently have the simply moved the destroy function into
cfi_probe.  

> 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).

Right, but something smiliar code work.  Or we could make them serve
dual duty.  

Eric