[RFC] refactoring MTD cmdset ops, jedec_probe, and cfi_probe

[RFC] refactoring MTD cmdset ops, jedec_probe, and cfi_probe

[Eric W. Biederman]

Currently for NOR flash we have two general purpose probe
routines that cover almost every case: cfi_probe and jedec_probe.

cfi_probe was written first and it assumes that every command set
has a unique number.  For CFI chips that appears to hold true
as the command set identifier is part of the interface.  However
for devices detected by jedec_probe that is not quite true.

While all of the devices have command sets quite similar to those
supported by cfi_probe frequently there in not a one to one mapping of
functionality.  The biggest differences are that many AMD command set
chips detected by jedec_probe do not support all of cfi_cmdset_0002,
and that several of the firmware hub based devices have weird offset
unlock registers, that cfi_cmdset_0001 does not support.

As more features are implemented and the mtd drivers are becoming
more widely used these semantic mismatches are beginning to cause
maintenance problems.  Calling certain methods on chips
that do not support them could be quite error prone.

To remedy this I would like to have the probe functions fill in
not the CmdSet number but a structure full of function pointers.
And of course the upper layers need to be modified to cope.

Doing this refactoring will be a little bit painful but I don't see
any other way to keep the code base maintainable as our number
of supported chips grows.

I have both Intel and AMD chips to test against.  CFI and non CFI
all being used as motherboard BIOS chips.  So I should be able
to catch the vast majority of problems in testing.  All but the
delicate chip interleave case, but I should not actually be touching
that part of the code, and at least I am aware of the issue.

Does anyone have any problems with this refactoring?

Eric

Re: [RFC] refactoring MTD cmdset ops, jedec_probe, and cfi_probe

[David Woodhouse]

On Mon, 2004-07-12 at 21:13 -0600, Eric W. Biederman wrote:
> I would like to have the probe functions fill in
> not the CmdSet number but a structure full of function pointers.
> And of course the upper layers need to be modified to cope.

> I have both Intel and AMD chips to test against.  CFI and non CFI
> all being used as motherboard BIOS chips.  So I should be able
> to catch the vast majority of problems in testing.  All but the
> delicate chip interleave case, but I should not actually be touching
> that part of the code, and at least I am aware of the issue.

And I just rewrote that part of the code too :)

> Does anyone have any problems with this refactoring?

No, it seems like a good idea. It'll let us drop half of cfi_cmdset_0020
too.

Once I've done some testing on the new interleave / map access code I
was planning to send an update to Linus this week. That _really_ needs
to happen this week, so if you're going to do this then either do it
today, or wait a month. Personally I favour the former -- the whole
thing needs testing now anyway.

Can you get to #mtd on irc.freenode.net?

-- 
dwmw2

Re: [RFC] refactoring MTD cmdset ops, jedec_probe, and cfi_probe

[Eric W. Biederman]

David Woodhouse <dwmw2@infradead.org> writes:

> On Mon, 2004-07-12 at 21:13 -0600, Eric W. Biederman wrote:
> > I would like to have the probe functions fill in
> > not the CmdSet number but a structure full of function pointers.
> > And of course the upper layers need to be modified to cope.
> 
> > I have both Intel and AMD chips to test against.  CFI and non CFI
> > all being used as motherboard BIOS chips.  So I should be able
> > to catch the vast majority of problems in testing.  All but the
> > delicate chip interleave case, but I should not actually be touching
> > that part of the code, and at least I am aware of the issue.
> 
> And I just rewrote that part of the code too :)

That part does not look to bad...

Although there has been some error handling from cfi_cmdset_0002.c which
disturbs me.  Seeing as I'm good at finding flaky NOR flash parts...
 
> > Does anyone have any problems with this refactoring?
> 
> No, it seems like a good idea. It'll let us drop half of cfi_cmdset_0020
> too.
> 
> Once I've done some testing on the new interleave / map access code I
> was planning to send an update to Linus this week. That _really_ needs
> to happen this week, so if you're going to do this then either do it
> today, or wait a month. Personally I favour the former -- the whole
> thing needs testing now anyway.
> 
> Can you get to #mtd on irc.freenode.net?


I can do that, although tomorrow is a little busy.  IRC isn't my best medium.

I'm just winding down for bed so and won't be back for 8 hours or so.  It
looks like we both have some definite ideas so a quick chat might be helpful.

That time line is a bit challenging.  My safe guess was about a week to get
everything written and tested... 

Anyway I will do what I can.  I don't want to even think about how
complex a network I had to setup to get IPv6 going behind our firewall
at work this morning.    I stopped waiting for the sysadmins to set
things up properly....

Anyway see you in a couple of hours.

Eric

Re: [RFC] refactoring MTD cmdset ops, jedec_probe, and cfi_probe

[David Woodhouse]

On Tue, 2004-07-13 at 01:05 -0600, Eric W. Biederman wrote:
> That part does not look to bad...

Would be interesting to know if it works though ... :)

> Although there has been some error handling from cfi_cmdset_0002.c which
> disturbs me.  Seeing as I'm good at finding flaky NOR flash parts...

I assume you mean some error handling _removed_ from cfi_cmdset_0002.c 

I don't recall doing that myself... can you elaborate?
 
> That time line is a bit challenging.  My safe guess was about a week to get
> everything written and tested... 

Yeah, that seems reasonable. I'll sync to Linus this week anyway, then I
can do it again in August when I get back and the dust has settled on
your changes.

-- 
dwmw2

Re: [RFC] refactoring MTD cmdset ops, jedec_probe, and cfi_probe

[Eric W. Biederman]

David Woodhouse <dwmw2@infradead.org> writes:

> On Tue, 2004-07-13 at 01:05 -0600, Eric W. Biederman wrote:
> > That part does not look to bad...
> 
> Would be interesting to know if it works though ... :)
> 
> > Although there has been some error handling from cfi_cmdset_0002.c which
> > disturbs me.  Seeing as I'm good at finding flaky NOR flash parts...
> 
> I assume you mean some error handling _removed_ from cfi_cmdset_0002.c 
> 
> I don't recall doing that myself... can you elaborate?

Right.  Basically when Thayne was working on cfi_cmdset_0002 we got it stable
for the chips we care about.  But the code was so ugly someone rewrote the
code.  There were other priorities at the time so we have not been able to
get back and fix things up.

The primary thing was that the check that the written data is was what
we actually tried to write was removed.

> > That time line is a bit challenging.  My safe guess was about a week to get
> > everything written and tested... 
> 
> Yeah, that seems reasonable. I'll sync to Linus this week anyway, then I
> can do it again in August when I get back and the dust has settled on
> your changes.

Sounds sane.

Eric

Re: [RFC] refactoring MTD cmdset ops, jedec_probe, and cfi_probe

[Thayne Harbaugh]

On Tue, 2004-07-13 at 08:23 -0600, Eric W. Biederman wrote:
> David Woodhouse <dwmw2@infradead.org> writes:
> 
> > On Tue, 2004-07-13 at 01:05 -0600, Eric W. Biederman wrote:
> > > That part does not look to bad...
> > 
> > Would be interesting to know if it works though ... :)
> > 
> > > Although there has been some error handling from cfi_cmdset_0002.c which
> > > disturbs me.  Seeing as I'm good at finding flaky NOR flash parts...
> > 
> > I assume you mean some error handling _removed_ from cfi_cmdset_0002.c 
> > 
> > I don't recall doing that myself... can you elaborate?
> 
> Right.  Basically when Thayne was working on cfi_cmdset_0002 we got it stable
> for the chips we care about.  But the code was so ugly someone rewrote the
> code.  There were other priorities at the time so we have not been able to
> get back and fix things up.
> 
> The primary thing was that the check that the written data is was what
> we actually tried to write was removed.

There's a short list of things that have been removed or changed
significantly (although this is from memory and may not match the
current code):

* read-back check of written data

* retry of failed writes

* unlock address for some chips (although this is likely part of the big
rewrite)

In the end, it's much appreciated that everything was cleaned up - there
were some major things done that I wanted to do but was too timid to do
major overhauling of the code.  Unfortunately it was just continuing to
grow harrier and uglier.

Re: [RFC] refactoring MTD cmdset ops, jedec_probe, and cfi_probe

[Eric W. Biederman]

Thayne Harbaugh <tharbaugh@lnxi.com> writes:

> On Tue, 2004-07-13 at 08:23 -0600, Eric W. Biederman wrote:
> > David Woodhouse <dwmw2@infradead.org> writes:
> > 
> > > On Tue, 2004-07-13 at 01:05 -0600, Eric W. Biederman wrote:
> > > > That part does not look to bad...
> > > 
> > > Would be interesting to know if it works though ... :)
> > > 
> > > > Although there has been some error handling from cfi_cmdset_0002.c which
> > > > disturbs me.  Seeing as I'm good at finding flaky NOR flash parts...
> > > 
> > > I assume you mean some error handling _removed_ from cfi_cmdset_0002.c 
> > > 
> > > I don't recall doing that myself... can you elaborate?
> > 
> > Right.  Basically when Thayne was working on cfi_cmdset_0002 we got it stable
> > for the chips we care about.  But the code was so ugly someone rewrote the
> > code.  There were other priorities at the time so we have not been able to
> > get back and fix things up.
> > 
> > The primary thing was that the check that the written data is was what
> > we actually tried to write was removed.
> 
> There's a short list of things that have been removed or changed
> significantly (although this is from memory and may not match the
> current code):
> 
> * read-back check of written data

I have a patch  for that already.  Since we are not checking any other
status bits.  At least verifying the data is correct is useful.  
 
> * retry of failed writes

This one is more interesting.   I don't know if it should be generic or we should
just override do_write_one_word...

One way or another we will get this one back in there.
 
> * unlock address for some chips (although this is likely part of the big
> rewrite)
> 
> In the end, it's much appreciated that everything was cleaned up - there
> were some major things done that I wanted to do but was too timid to do
> major overhauling of the code.  Unfortunately it was just continuing to
> grow harrier and uglier.

Thayne this problem I am not familiar with.  Could I have some details?

Eric

Re: [RFC] refactoring MTD cmdset ops, jedec_probe, and cfi_probe

[Thayne Harbaugh]

On Tue, 2004-07-13 at 09:04 -0600, Eric W. Biederman wrote:
> Thayne Harbaugh <tharbaugh@lnxi.com> writes:

> > There's a short list of things that have been removed or changed
> > significantly (although this is from memory and may not match the
> > current code):
> > 
> > * read-back check of written data
> 
> I have a patch  for that already.  Since we are not checking any other
> status bits.  At least verifying the data is correct is useful.  

Wonderful.

> > * retry of failed writes
> 
> This one is more interesting.   I don't know if it should be generic or we should
> just override do_write_one_word...
> 
> One way or another we will get this one back in there.

Good.
 
> > * unlock address for some chips (although this is likely part of the big
> > rewrite)
> > 
> > In the end, it's much appreciated that everything was cleaned up - there
> > were some major things done that I wanted to do but was too timid to do
> > major overhauling of the code.  Unfortunately it was just continuing to
> > grow harrier and uglier.
> 
> Thayne this problem I am not familiar with.  Could I have some details?

Errrr . . . the problem of the unlock address or of the harry, ugly
code?

The unlock story is that there was *no* unlock code in cfi_cmdset_0002.
One of the newer SST chips requires unlocking of the blocks.  Unlike
other chips that need unlocking, the addresses for unlocking this
particular chip is not at the block address but below the address of the
entire chip.

It was strange, but I figured that since there wasn't already an unlock
function it wouldn't matter that the new one was chip specific and less
than ideal.  Of course no more than a few weeks after I put it in the
unlock function was needed for *sane* flash chips and so the unlock code
was *corrected*.

The problem with the harry and ugly code was that the status checking
code was reproduced in multiple spots (at least three).  It was bulky
and very sensitive.  All of it needed to be consolidated into a single
location.  That was part of the last, major rewrite that *fixed* some of
the things that should have been left in.

Re: [RFC] refactoring MTD cmdset ops, jedec_probe, and cfi_probe

[Eric W. Biederman]

David Woodhouse <dwmw2@infradead.org> writes:

> On Mon, 2004-07-12 at 21:13 -0600, Eric W. Biederman wrote:
> > I would like to have the probe functions fill in
> > not the CmdSet number but a structure full of function pointers.
> > And of course the upper layers need to be modified to cope.
> 
> > I have both Intel and AMD chips to test against.  CFI and non CFI
> > all being used as motherboard BIOS chips.  So I should be able
> > to catch the vast majority of problems in testing.  All but the
> > delicate chip interleave case, but I should not actually be touching
> > that part of the code, and at least I am aware of the issue.
> 
> And I just rewrote that part of the code too :)
> 
> > Does anyone have any problems with this refactoring?
> 
> No, it seems like a good idea. It'll let us drop half of cfi_cmdset_0020
> too.

Ok recording what the reason for this that I got on irc.

cfi_cmdset_0020 is just a variant on cfi_cmdset_0001 that shares most of
the code.

A data sheet for the M58LW128A ( which erroneously reports cmdset 01 instead of 0x20)
is at:
http://www.st.com/stonline/books/pdf/docs/9229.pdf

cmdset 020 was written for the  M58LW064A and I doesn't currently have a datasheet reference
for on ST site.

Eric

Re: [RFC] refactoring MTD cmdset ops, jedec_probe, and cfi_probe

[Eric W. Biederman]

Thayne Harbaugh <tharbaugh@lnxi.com> writes:
> > Thayne this problem I am not familiar with.  Could I have some details?
> 
> Errrr . . . the problem of the unlock address or of the harry, ugly
> code?
> 
> The unlock story is that there was *no* unlock code in cfi_cmdset_0002.
> One of the newer SST chips requires unlocking of the blocks.  Unlike
> other chips that need unlocking, the addresses for unlocking this
> particular chip is not at the block address but below the address of the
> entire chip.

Ah. yes.  Sorry the way you phrased it I was thinking of the unlock_addr address
prefix that needs to be applied to commands.  The magic Firmware hub addresses
that are used for locking/unlocking block I am familiar with.  

Eric

 

Re: [RFC] refactoring MTD cmdset ops, jedec_probe, and cfi_probe

[Eric W. Biederman]

Thayne Harbaugh <tharbaugh@lnxi.com> writes:

> On Tue, 2004-07-13 at 09:04 -0600, Eric W. Biederman wrote:
> > Thayne Harbaugh <tharbaugh@lnxi.com> writes:
> 
> > > There's a short list of things that have been removed or changed
> > > significantly (although this is from memory and may not match the
> > > current code):
> > > 
> > > * read-back check of written data
> > 
> > I have a patch  for that already.  Since we are not checking any other
> > status bits.  At least verifying the data is correct is useful.  
> 
> Wonderful.
> 
> > > * retry of failed writes
> > 
> > This one is more interesting.  I don't know if it should be generic or we
> should
> 
> > just override do_write_one_word...
> > 
> > One way or another we will get this one back in there.
> 
> Good.

Looking at where we are Dave has just reworked the code for handling multiple
word sizes for reads and writes, and things are just starting to settle out.
And since he is going to be gone he will be sending these changes to Linus
later this week.

Since refactoring does not work well if I don't have a working code base
to start with. I will push for the appropriate fixes to the MTD command sets
so I know I am starting with a working base.  And then I will start refactoring
the command sets.

Eric

Re: [RFC] refactoring MTD cmdset ops, jedec_probe, and cfi_probe

[Josh Boyer]

On Tue, 2004-07-13 at 10:25, Eric W. Biederman wrote:
> Ok recording what the reason for this that I got on irc.
> 
> cfi_cmdset_0020 is just a variant on cfi_cmdset_0001 that shares most of
> the code.

Thinking about this more, I wouldn't worry too much about
cfi_cmdset_0020.c.  It's old, fairly unmaintained, and only has a single
user as far as I know.

If your changes break it, then it'll give someone and excuse to update
the code.  Eventually it should be merged with cfi_cmdset_0001.c anyway,
since M58LW128A reports command set 01, but really needs the
functionality of command set 20.  The exception list you talked about in
IRC might help with that.

But don't let me stop you from making it work if you want to :).

> A data sheet for the M58LW128A ( which erroneously reports cmdset 01 instead of 0x20)
> is at:
> http://www.st.com/stonline/books/pdf/docs/9229.pdf
> 
> cmdset 020 was written for the  M58LW064A and I doesn't currently have a datasheet reference
> for on ST site.

Probably because it isn't that common of a part, and newer versions of
that chip have been made available.

josh

Re: [RFC] refactoring MTD cmdset ops, jedec_probe, and cfi_probe

[Dan Post]

On 12 Jul 2004 21:13:11 -0600, Eric W. Biederman wrote
> I have both Intel and AMD chips to test against.  CFI and non CFI
> all being used as motherboard BIOS chips.  So I should be able
> to catch the vast majority of problems in testing.  All but the
> delicate chip interleave case, but I should not actually be touching
> that part of the code, and at least I am aware of the issue.

Eric, I have been working on updating cfi_cmdset_0001.c / cfi_probe to support
Intel(R) L18/30 parts.  My work hasn't been rolled up into the MTD yet,
primarily because I haven't been very agile--been really busy with other
things, but this is going to get better.

Changes to cfi_probe are minimal; L18 has the same cmdset.  The CFI version is
1.3, but this is handled by the cfi_cmdset_0001.c driver.  The only changes to
cfi_probe were read mode -> 0xFF since L18 doesn't seem to understand 0xF0 (it
broke XIP until I fixed it, but K3 worked fine with 0xF0).

I don't know how deep you plan on getting into the driver, but there are some
things to take into consideration when you refactor...  L18 has multiple
hardware "partitions"--you can read one section and be writing the other
section at the same time, essentially.  i.e. for a 32MiB chip you have 16
state machines, each for 2MiB of the chip.  There are a couple of rules.  You
can only be programming or erasing in one partition at a time, but you can
suspend an erase, program, suspend the program, resume program, resume erase
(or any subset thereof).  And there is an extended OTP space.  Those are the
biggest changes to keep in mind.

So I would appreciate it if you try not to really break the state machine.  I
will need to extend the state machine code if anything.  So please don't rip
it out / simplify it too much.  This will make my later work easier.  I saw
that Nico started to do some of this L18 work but rolled it back.

BTW, older flash parts, e.g. K3, J3, et al, are handled by the same driver, by
faking a single hwpart to cover the entire part.

What Intel(R) flash parts are you working with?  L18 support exists and I can
point you to some code, but it hasn't been rolled up into CVS yet.  Probably
because it targets a now-out-of-date version [thanks to all the activity
lately] and the changes are quite extensive...

Coincidentally, I work at Intel(R) Corporation.  If things get sticky with the
NOR flash support refactoring, give me a shout.

> Does anyone have any problems with this refactoring?

Sounds like a great idea.

Dan

Re: [RFC] refactoring MTD cmdset ops, jedec_probe, and cfi_probe

[Nicolas Pitre]

On Tue, 13 Jul 2004, Dan Post wrote:

> I don't know how deep you plan on getting into the driver, but there are some
> things to take into consideration when you refactor...  L18 has multiple
> hardware "partitions"--you can read one section and be writing the other
> section at the same time, essentially.  i.e. for a 32MiB chip you have 16
> state machines, each for 2MiB of the chip.

I committed support for that a while ago already.  SMP and preemption safe 
too.  Anything you think might be missing (besides the crude probing code
that certainly could benefit from more work)?


Nicolas

Re: [RFC] refactoring MTD cmdset ops, jedec_probe, and cfi_probe

[Dan Post]

On Tue, 13 Jul 2004 12:52:13 -0400 (EDT), Nicolas Pitre wrote
> On Tue, 13 Jul 2004, Dan Post wrote:
> > [L18 / hardware partitions]
> 
> I committed support for that a while ago already.  SMP and 
> preemption safe too.  Anything you think might be missing (besides 
> the crude probing code that certainly could benefit from more work)?

Ahhh, I thought you reversed that :)  Some must have slipped by.  David, if
you're listening, I miss your lovely cvsweb interface... any word on when that
will be back up?

I'm downloading the latest snapshot as we speak.  Maybe I'll have to wait
until tomorrow... ugly firewall doesn't let CVS through, so I'd miss a lot of
updates I think.  At any rate, I'll check it out and give it a try.

Here is a preliminary 'missing' list:

OTP [protection register] support.  I have some preliminary code that supports
reading / writing / locking / querying the primary + extended OTP space
(extended is only available on L18).  It's targeted against an ancient version
of the MTD, but I've been updating it to work against my djp1.  It's good
reference code, and the only thing that will change significantly is the code
that actually writes / reads the OTP bits.  There are several functions on top
of that to make access to the two regions transparent and contiguous.  I can't
hand out the OTP stuff today, but I'll push this up on my priority list.  Also
todo is make OTP work on chips with only the primary OTP space (e.g. K3).

XIP.  :)  This is my job.  It works in djp1, but needs porting to the modern
stuff.

Needs better probing code: I trust you on that one.

Dan

Re: [RFC] refactoring MTD cmdset ops, jedec_probe, and cfi_probe

[Nicolas Pitre]

On Tue, 13 Jul 2004, Dan Post wrote:

> On Tue, 13 Jul 2004 12:52:13 -0400 (EDT), Nicolas Pitre wrote
> > On Tue, 13 Jul 2004, Dan Post wrote:
> > > [L18 / hardware partitions]
> > 
> > I committed support for that a while ago already.  SMP and 
> > preemption safe too.  Anything you think might be missing (besides 
> > the crude probing code that certainly could benefit from more work)?
> 
> Ahhh, I thought you reversed that :)  Some must have slipped by.  David, if
> you're listening, I miss your lovely cvsweb interface... any word on when that
> will be back up?

I did reverse the first one:

|revision 1.142
|date: 2004/06/08 21:26:21;  author: nico;  state: Exp;  lines: +130 -25
|Support for the multi hw partitions of the Intel L18 flash chips.
|The non-partitioned cases should behave just like before.
|However I'd like for someone else to look at the added logic for the
|partitioned config in case I might have overlooked something.
|
|revision 1.143
|date: 2004/06/09 18:35:51;  author: nico;  state: Exp;  lines: +62 -195
|No, I'm nuts.  My curent approach for L18 support is getting more
|and more messy as I consider all the details that were still missing
|and needed extra mods here and there.  When I can't stand my own code
|anymore that's a sign that the whole thing should be scrapped.  And
|this commit is exactly that.  Stay tuned for an alternate approach.

And eventually:

|revision 1.145
|date: 2004/06/10 20:54:55;  author: nico;  state: Exp;  lines: +161 -2
|Second attempt at supporting the multipartition L18 Intel flash chips.
|Again, the non-partitioned case should not have its logic altered so this
|is not supposed to break existing users.  The partition support code could
|benefit from extra review though.

... plus a few subsequent revisions fixing  a couple glitches here
and there.


Nicolas

Re: [RFC] refactoring MTD cmdset ops, jedec_probe, and cfi_probe

[Eric W. Biederman]

ebiederman@lnxi.com (Eric W. Biederman) writes:

> Looking at where we are Dave has just reworked the code for handling multiple
> word sizes for reads and writes, and things are just starting to settle out.
> And since he is going to be gone he will be sending these changes to Linus
> later this week.


Ok. Where I am at.  

I have modified ichxrom so now is insanely flexibly.
It handles cfi and jedec parts.
It handles 1,2,4 byte bus widths.  (Since an LPC bus is packet
   based there is no natural bus width).

It still special cases Intel firmware hubs but that is disabled
when it is not dealing with them.  Both cases have been tested
both in 32bit and 64bit mode.

I have modified cfi_cmdset_0002 so that it retries in do_write_one_word.
For all of the other commands if an error occurs users can make
progress simply by retrying the command.   When do_write_one_word
fails all users can do is erase and rewrite the block.  Which
in the face of transient errors (especially from an SST chip)
can fail to make progress.

With those small fixes in place I have the mtd drivers working
for the primary cases I care about.  So I should be able
to use what Dave merges into 2.6. now :)

Actually making ichxrom handle nearly every NOR flash case under the
sun was less painful then I thought.  So unfortunately there is less
urgency to get the refactoring done now :(

However it is still on my TODO list so I will push it forward in a
little bit.

Eric

Re: [RFC] refactoring MTD cmdset ops, jedec_probe, and cfi_probe

[Eric W. Biederman]

Josh Boyer <jdub@us.ibm.com> writes:

> On Tue, 2004-07-13 at 10:25, Eric W. Biederman wrote:
> > Ok recording what the reason for this that I got on irc.
> > 
> > cfi_cmdset_0020 is just a variant on cfi_cmdset_0001 that shares most of
> > the code.
> 
> Thinking about this more, I wouldn't worry too much about
> cfi_cmdset_0020.c.  It's old, fairly unmaintained, and only has a single
> user as far as I know.
> 
> If your changes break it, then it'll give someone and excuse to update
> the code.  Eventually it should be merged with cfi_cmdset_0001.c anyway,
> since M58LW128A reports command set 01, but really needs the
> functionality of command set 20.  The exception list you talked about in
> IRC might help with that.
> 
> But don't let me stop you from making it work if you want to :).

So far I have been busy with other things so I have been taking the slow route
to refactoring.  I have just reported the cfi_fixup function from cfi_util
and have gotten nearly all of the variation in cfi_cmdset_0001 and cfi_cmdset_0002
under control.  

It is probably enough infrastructure so that cfi_cmdset_0001 and 0020 could be
merged.

For the deeper pieces of command set common code moving their helper functions
into cfi_util is probably a good place to start.

I'm hoping I have accomplished enough I don't need to touch the code again for quite a while but...

Eric

Re: [RFC] refactoring MTD cmdset ops, jedec_probe, and cfi_probe

[Eric W. Biederman]

ebiederman@lnxi.com (Eric W. Biederman) writes:

> ebiederman@lnxi.com (Eric W. Biederman) writes:
> 
> > Looking at where we are Dave has just reworked the code for handling multiple
> > word sizes for reads and writes, and things are just starting to settle out.
> > And since he is going to be gone he will be sending these changes to Linus
> > later this week.

Ok I am making a little headway every time I have to go into and touch
the mtd code.  Eventually things may look much cleaner.
 
> Ok. Where I am at.  
> 

> It still special cases Intel firmware hubs but that is disabled
> when it is not dealing with them.  Both cases have been tested
> both in 32bit and 64bit mode.

ichxrom now no longer special cases firmware hub parts,
and the firmware hub lock/unlock routines now test to make
certain you are not going to underflow your map driver, before
they run.

> However it is still on my TODO list so I will push it forward in a
> little bit.

Looking into the code I found a cfi_fixup routine in cfi_util.
I modified it to take a mtd_info pointer instead of map_info
and I have placed all of the command set generic mtd method
so they are initialized in cfi_fixup.

So for the short term it looks like simply improving the cfi_fixup
functions allows all of the special cases I know about to be
handled cleanup.

Beyond that I have synchronized the 2 generic map drivers I
maintain ichxrom and amd76xrom.   And their code is now almost
identical.  I now almost have a generic x86-rom-probe function.
I clearly have a case of the possible and awkward archived in
those to map drivers.  

My gut feel is that there is a lot of ways the code can be made
clear, simpler, possibly smaller and more reliable.  

But I don't quite have a handle on so I will let what I see sit and
take it's time.   Between my awkward cases and the suggesting of
moving more functions into cfi_util I can almost see starting places
but things get slippery when I look at the details so I have
found enough for now.

My checking comments are attached at the bottom since I felt
like I was about to repeat them.


- Move support firmware hub style lock and unlock into fhw_lock.h (from cfi_cmdset_0002)
- Move cfi_varsize_frob into cfi_util from cfi_cmdset_0001.c and cfi_cmdset_0002.c
- reduce gen_probe probe failuers to a debug level message
- Modify cfi_fixup to take a struct mtd_info instead of a struct map_info
  So that the fixup routines can modify the mtd functions.
- Modify cfi_cmdset_0001() to allocate and initialize the mtd structure
  before calling cfi_fixup.
- Modify cfi_cmdset_0002() to allocate and initialize the mtd structure
  before calling cfi_fixup.
- Refactor the hard coded fixups in cfi_cmdset_0001 and cfi_cmdset_0002
  so the improved cfi_fixup infrastructure.
- Rewrote amd76xrom and ichxrom.
  They now report their starting physical address in their name.
  They now both handle multiple bankwidth configurations
  They both can create multipe mtd devices.
  They both now assume the rom windows are properly opened by the BIOS
   or whatever runs previous to them.
  Their code is now synchromized so it is almost identical,
     and could be a starting point for a x86_rom_probe.


Good Night,
Eric

Re: [RFC] refactoring MTD cmdset ops, jedec_probe, and cfi_probe

[Josh Boyer]

On Thu, 2004-08-12 at 02:13, Eric W. Biederman wrote:
> So far I have been busy with other things so I have been taking the slow route
> to refactoring.  I have just reported the cfi_fixup function from cfi_util
> and have gotten nearly all of the variation in cfi_cmdset_0001 and cfi_cmdset_0002
> under control.  
> 
> It is probably enough infrastructure so that cfi_cmdset_0001 and 0020 could be
> merged.

Sorry for the delay.  Maybe I can convince Joern to get them merged. 
Most of the stuff can probably be done in a fixup function now that they
take a struct mtd_info *.

> 
> For the deeper pieces of command set common code moving their helper functions
> into cfi_util is probably a good place to start.
> 
> I'm hoping I have accomplished enough I don't need to touch the code again for quite a while but...

We can always hope :).

josh