hacking - need feedback

hacking - need feedback

[Tim Hockin]

All,

I'm trying to get support for our flashrom into the MTD codebase.  Upon
speaking with David Woodhouse, he wants some re-structuring.

Rather than cfi_probe looking for jedec devices, we probably want a
jedec_probe that hooks up to cmdset_0002.

I don't know everything there is to know about JEDEC, so I have a few
questions.

It seems that the majority of the code in cfi_probe.c applies equally to
both cfi and jedec chips (and probably others).

How should this be structured?  

Would life be easier if all maps just called do_map_probe_all() and that
would walk down the list of supported chiptypes?
	cfi_probe
	jedec_probe (new)
	jedec
	amd_flash
	ram
	rom

I understand jedec.c and amd_flash.c are present for compatibility, but we
want future stuff to use the common cmdset code.  

Is it true that the code in cfi_probe.c (for doing things like interleave,
multiple chips etc) should be common for CFI and JEDEC (and others?)  If
so, it should be shared, but it hardly makes sense that do_map_probe("cfi")
would find jedec chips...


Sorry I'm rambling.  I'm still trying to wrap my head around the lot of it,
and being new to the codebase, it is easy for me to say "rip out the old
stuff" :)

Tim
	
-- 
Tim Hockin
Systems Software Engineer
Sun Microsystems, Cobalt Server Appliances
thockin@sun.com

Re: hacking - need feedback

[David Woodhouse]

thockin@sun.com said:
>  Would life be easier if all maps just called do_map_probe_all() and
> that would walk down the list of supported chiptypes?

Maybe, but to be honest most map driver authors know what type of chip is 
going to be found. No harm in providing it, as long it remains possible to 
call the other probes individually.

> 	cfi_probe
> 	jedec_probe (new)
> 	jedec
> 	amd_flash
> 	ram
> 	rom

I think there'll be an intel_probe in there too, passing control to 
cfi_cmdset_000[13]. 


>  Is it true that the code in cfi_probe.c (for doing things like
> interleave, multiple chips etc) should be common for CFI and JEDEC
> (and others?)  If so, it should be shared, but it hardly makes sense
> that do_map_probe("cfi") would find jedec chips...

A lot of the required magic is actually in include/linux/mtd/cfi.h. to the 
extent that the contents of cfi_probe.c end up duplicated in jedec_probe.c, 
I think we should just live with it.


>  Sorry I'm rambling.  I'm still trying to wrap my head around the lot
> of it, and being new to the codebase, it is easy for me to say "rip
> out the old stuff" :)

Believe me, it's still easy to say "rip out the old stuff". :)


--
dwmw2