Patch for ST M50FLW080A support

[Claus Gindhart]

The generic jedec.c does not work for the ST M50FLW flash
devices, because they need an unlock command first.
For this reason, ST M50FLW support is moved to a 
new HW support module, because any change in jedec.c
would bear the risk to cause problems with the already
supported devices.
It's already tested with ST M50FLW080A; the other
chips of this family i dont have available, so i couldn
test it.

---

Mit freundlichen Grüßen / Best regards

Claus Gindhart
 SW R&D 
 Kontron Modular Computers
 phone :++49 (0)8341-803-374
 mailto:[e-mail] 
 http://www.kontron.com

Kontron Modular Computers GmbH
Geschaeftsfuehrer / Managing Directors: Ulrich Gehrmann, Thomas Sabisch 
Sitz der Gesellschaft / Registered Office: Kaufbeuren, Rechtsform / Legal: GmbH 
Amtsgericht / Local District Court Kempten, HRB Nr. / Trade Register No. 6195

The information contained in this document is CONFIDENTIAL and property of Kontron.
Any unauthorized review, use, disclosure or distribution is prohibited without express
written consent of Kontron. If you are not the intended recipient, please contact the
sender and destroy all copies of the original message and enclosed attachments.

Diese E-Mail enthält vertrauliche und/oder rechtlich geschützte Informationen und ist
Eigentum von Kontron. Die Verwendung und Weitergabe von jeglichen Inhalten ist
ohne ausdrückliche schriftliche Genehmigung von Kontron strikt untersagt. Wenn Sie
diese E-Mail irrtümlich erhalten haben, informieren Sie bitte sofort den Absender und
vernichten diese Mail und enthaltene Dokumente.

[Paul Menzel]

Dear Claus,

thank you for your great work.

Am Freitag, den 25.04.2008, 14:52 +0200 schrieb Claus Gindhart:
> The generic jedec.c does not work for the ST M50FLW flash
> devices, because they need an unlock command first.
> For this reason, ST M50FLW support is moved to a 
> new HW support module, because any change in jedec.c
> would bear the risk to cause problems with the already
> supported devices.
> It's already tested with ST M50FLW080A; the other
> chips of this family i dont have available, so i couldn
> test it.

Although I cannot test or verify your patch, I think you have to add a
Signed-Off-By line [1]. Otherwise your patch cannot be acked and then
commited.

Thanks,

Paul

[1] http://www.coreboot.org/Development_Guidelines#Sign-off_Procedure

[Peter Stuge]

Hello Claus,

On Fri, Apr 25, 2008 at 02:52:46PM +0200, Claus Gindhart wrote:
> The information contained in this document is CONFIDENTIAL and
> property of Kontron.
> Any unauthorized review, use, disclosure or distribution is
> prohibited without express written consent of Kontron.

Again, please discuss these sentences with your employer.
> Index: jedec.c

Your patch looks good after a quick review - except that it seems to
include quite a bit of what seems to be experimental code in jedec.c
that probably was not meant to be committed.
> +int write_page_stm50flw0x0x(volatile uint8_t *bios, uint8_t *src,
> 	   volatile uint8_t *dst, int page_size)
>
> +{

..
> +/* [e-mail]
> TODO
> I think, that verification is not required, but
> i leave it in anyway
> */
> dst = d;
> src = s;
> for (i = 0; i < page_size; i++) {
> if (*dst != *src) {
> 	rc = -1;
> 	break;
> }
> dst++;
> src++;
> }

You are correct - verification is done in other functions in flashrom
so please remove it from the write function.

On Fri, Apr 25, 2008 at 03:52:57PM +0200, Paul Menzel wrote:
> I think you have to add a Signed-Off-By line [1]. Otherwise your
> patch cannot be acked and then commited.
>
> [1] http://www.coreboot.org/Development_Guidelines#Sign-off_Procedure

Spot on, Paul.

Claus, please have a look at the development guidelines page which
describes the (simple) process for contributing code to the project.
Part of that is the sign-off procedure that Paul mentions.

It is important that all contributions come with an unambiguous
license, which is why I am so repetitive about the automatic
disclaimer in your outgoing email. I am sorry for the extra trouble.
Ideally you would be able to remove that text from your outgoing
email messages at least to this mailing list.

Best regards

//Peter

[Stefan Reinauer]

Peter Stuge wrote:
> H
> > +/* [e-mail]
> > TODO
> > I think, that verification is not required, but
> > i leave it in anyway
> > */
> > dst = d;
> > src = s;
> > for (i = 0; i < page_size; i++) {
> > if (*dst != *src) {
> > 	rc = -1;
> > 	break;
> > }
> > dst++;
> > src++;
> > }
>
> You are correct - verification is done in other functions in flashrom
> so please remove it from the write function.

I beg to rethink this. Some of the flash drivers use the verification to
do an immediate rewrite of a single sector.
This is required for some flaky parts. Having to write flashrom -wv
instead of -w to make sure the flash image was correctly written to the
chip is a bogus user interface.
While you might want to omit the erase, there's no reason not to know
about the failure of a given write.
> > I think you have to add a Signed-Off-By line [1]. Otherwise your
> > patch cannot be acked and then commited.
> >
> > [1] http://www.coreboot.org/Development_Guidelines#Sign-off_Procedure
>
> Spot on, Paul.
>
> Claus, please have a look at the development guidelines page which
> describes the (simple) process for contributing code to the project.
> Part of that is the sign-off procedure that Paul mentions.
>
> It is important that all contributions come with an unambiguous
> license, which is why I am so repetitive about the automatic
> disclaimer in your outgoing email. I am sorry for the extra trouble.
> Ideally you would be able to remove that text from your outgoing
> email messages at least to this mailing list.

Please be patient with our fresh corporate contributors. While I fully
agree with you, Peter, the attempts to change corporate rules and
culture are probably a lot more effort than contributing code, and
wildly out of the scope of our project.

Stefan

[Carl-Daniel Hailfinger]

Hi Claus,

Thanks for the patch!

On 25.04.2008 14:52, Claus Gindhart wrote:
> The generic jedec.c does not work for the ST M50FLW flash
> devices, because they need an unlock command first.
> For this reason, ST M50FLW support is moved to a 
> new HW support module, because any change in jedec.c
> would bear the risk to cause problems with the already
> supported devices.
> It's already tested with ST M50FLW080A; the other
> chips of this family i dont have available, so i couldn
> test it.

Some chips in the ST M50FW* family use the functions probe_82802ab,
erase_82802ab, write_82802ab. Can you take a look and check whether
these functions can also be used for the ST M50FLW* chips?

Regards,
Carl-Daniel