[LinuxBIOS] Please update http://www.linuxbios.org/Development_Guidelines

[Markus Boas]

I don't know how to make a valid patch.
I have added support for the Winbond W29EE011 Chip. This is an old device. But 
I found it on same mainboards.

Greeding
    Markus

[LinuxBIOS] Please update http://www.linuxbios.org/Development_Guidelines

[Stefan Reinauer]

* Markus Boas  [070829 15:11]:
> I don't know how to make a valid patch.

http://www.linuxbios.org/Development_Guidelines#How_to_contribute

[Markus Boas]

Am Mittwoch 29 August 2007 schrieb Stefan Reinauer:
> Markus Boas  [070829 15:11]:
> > I don't know how to make a valid patch.
>
> http://www.linuxbios.org/Development_Guidelines#How_to_contribute

I don't know what is going wrong, but now i have the patch.
I not realy worked befor with svn.

Greedings
    Markus

Signed-off-by: Markus Boas

[LinuxBIOS] Please update?http://www.linuxbios.org/Development_Guidelines

[Stefan Reinauer]

Great, thanks for the patch!

* Markus Boas  [070829 15:34]:
> Index: flashchips.c
>
> --- flashchips.c	(Revision 2750)
> +++ flashchips.c	(Arbeitskopie)
> @@ -83,6 +83,8 @@
>     probe_jedec,	erase_chip_jedec, write_49fl004},
>    {"W29C011",	WINBOND_ID,	W_29C011,	128, 128,
>     probe_jedec,	erase_chip_jedec, write_jedec},
> {"W29EE011",	WINBOND_ID,	W_29C011,	128, 128,
> probe_w29ee011,	erase_chip_jedec, write_jedec},

So is it on purpose that the 29C011 and the 29EE011 have the same ID but
need a different probing sequence?

[LinuxBIOS] Please update?http://www.linuxbios.org/Development_Guidelines

[Markus Boas]

Yes, but the W29EE011 is an old device so I think they have recycled the ID.

Am Mittwoch 29 August 2007 schrieb Stefan Reinauer:
> Great, thanks for the patch!
> Markus Boas  [070829 15:34]:
> > Index: flashchips.c
> >
> > --- flashchips.c	(Revision 2750)
> > +++ flashchips.c	(Arbeitskopie)
> > @@ -83,6 +83,8 @@
> >     probe_jedec,	erase_chip_jedec, write_49fl004},
> >    {"W29C011",	WINBOND_ID,	W_29C011,	128, 128,
> >     probe_jedec,	erase_chip_jedec, write_jedec},
> > {"W29EE011",	WINBOND_ID,	W_29C011,	128, 128,
> > probe_w29ee011,	erase_chip_jedec, write_jedec},
>
> So is it on purpose that the 29C011 and the 29EE011 have the same ID but
> need a different probing sequence?

[Markus Boas]

I forgot, I tested on a Geode GX1 system.
Read, write and verify theams to work.

Markus

Am Mittwoch 29 August 2007 schrieb Markus Boas:
> Am Mittwoch 29 August 2007 schrieb Stefan Reinauer:
> > Markus Boas  [070829 15:11]:
> > > I don't know how to make a valid patch.
> >
> > http://www.linuxbios.org/Development_Guidelines#How_to_contribute
>
> I don't know what is going wrong, but now i have the patch.
> I not realy worked befor with svn.
>
> Greedings
>    Markus
>
> Signed-off-by: Markus Boas

[LinuxBIOS] Please update?http://www.linuxbios.org/Development_Guidelines

[Uwe Hermann]

On Wed, Aug 29, 2007 at 03:34:19PM +0200, Markus Boas wrote:
> Index: w29ee011.c
>
> --- w29ee011.c	(Revision 0)
> +++ w29ee011.c	(Revision 0)
> @@ -0,0 +1,51 @@
> +/*
> +*  C Implementation: w29ee011
> +*
> +* Description: 
> +*
> +*
> +* Author: Markus Boas , (C) 2007
> +*
> +* Copyright: See COPYING file that comes with this distribution
> +*
> +*/

Please drop the above and use the generic LinuxBIOS header as per
http://linuxbios.org/Development_Guidelines#Common_License_Header

(yes, I know, the existing files are b0rked too, I'll fix that in a
minute)
> +#include "flash.h"
>
> +int probe_w29ee011(struct flashchip *flash)
> +{
> volatile uint8_t *bios = flash->virtual_memory;
> uint8_t id1, id2;
> /* Issue JEDEC Product ID Entry command */
> *(volatile uint8_t *)(bios + 0x5555) = 0xAA;
> myusec_delay(10);
> *(volatile uint8_t *)(bios + 0x2AAA) = 0x55;
> myusec_delay(10);
> *(volatile uint8_t *)(bios + 0x5555) = 0x80;
> myusec_delay(10);
> *(volatile uint8_t *)(bios + 0x5555) = 0xAA;
> myusec_delay(10);
> *(volatile uint8_t *)(bios + 0x2AAA) = 0x55;
> myusec_delay(10);
> *(volatile uint8_t *)(bios + 0x5555) = 0x60;
> myusec_delay(10);
> /* Read product ID */
> id1 = *(volatile uint8_t *)bios;
> id2 = *(volatile uint8_t *)(bios + 0x01);
> /* Issue JEDEC Product ID Exit command */
> *(volatile uint8_t *)(bios + 0x5555) = 0xAA;
> myusec_delay(10);
> *(volatile uint8_t *)(bios + 0x2AAA) = 0x55;
> myusec_delay(10);
> *(volatile uint8_t *)(bios + 0x5555) = 0xF0;
> myusec_delay(10);
> printf_debug("%s: id1 0x%x, id2 0x%x\n", __FUNCTION__, id1, id2);
> if (id1 == flash->manufacture_id && id2 == flash->model_id)
> return 1;
> return 0;
>
> +}
> Index: w29ee011.h
>
> --- w29ee011.h	(Revision 0)
> +++ w29ee011.h	(Revision 0)
> @@ -0,0 +1,18 @@
> +//
> +// C++ Interface: w29ee011
> +//
> +// Description: Support for the Winbond W29EE011
> +// Source: Datasheet
> +//
> +//
> +// Author: Markus Boas , (C) 2007
> +//
> +// Copyright: See COPYING file that comes with this distribution
> +//
> +//
> +#ifndef _W29EE011_H_
> +#define _W29EE011_H_
>
> +extern int probe_w29ee011(struct flashchip *flash);
>
> +#endif

Drop this file completely please, the function prototype is in flash.h
already and this file is useless.

Also, please update the README file with the newly supported chip.
Otherwise the patch looks good.

Thanks, Uwe.

[LinuxBIOS] Add Flash-chipsupport

[Markus Boas]

I hope this  one is better

Am Mittwoch 29 August 2007 schrieb Uwe Hermann:
> On Wed, Aug 29, 2007 at 03:34:19PM +0200, Markus Boas wrote:
> > Index: w29ee011.c
>
> Please drop the above and use the generic LinuxBIOS header as per
> http://linuxbios.org/Development_Guidelines#Common_License_Header
>
> (yes, I know, the existing files are b0rked too, I'll fix that in a
> minute)
>
> Drop this file completely please, the function prototype is in flash.h
> already and this file is useless.
>
> Also, please update the README file with the newly supported chip.
> Otherwise the patch looks good.
>
> Thanks, Uwe.

[LinuxBIOS] Add Flash-chipsupport

[Markus Boas]

I forgot the readme

Index: README
===================================================================
--- README      (Revision 2751)
+++ README      (Arbeitskopie)
@@ -147,6 +147,7 @@
 SyncMOS S29C51002T/B
 SyncMOS S29C51004T/B
 SyncMOS S29C31004T
+Winbond W29EE011
 Winbond W29C011
 Winbond W29C020C
 Winbond W49F002U

Am Mittwoch 29 August 2007 schrieb Markus Boas:
> I hope this  one is better
>
> Am Mittwoch 29 August 2007 schrieb Uwe Hermann:
> > On Wed, Aug 29, 2007 at 03:34:19PM +0200, Markus Boas wrote:
> > > Index: w29ee011.c
> >
> > Please drop the above and use the generic LinuxBIOS header as per
> > http://linuxbios.org/Development_Guidelines#Common_License_Header
> >
> > (yes, I know, the existing files are b0rked too, I'll fix that in a
> > minute)
> >
> > Drop this file completely please, the function prototype is in flash.h
> > already and this file is useless.
> >
> > Also, please update the README file with the newly supported chip.
> > Otherwise the patch looks good.
> >
> > Thanks, Uwe.

[LinuxBIOS] Add Flash-chipsupport

[Markus Boas]

So I hope I forget nothing

Greeding
    Markus Boas

Signed-off-by: Markus Boas

[LinuxBIOS] Add Flash-chipsupport

[Uwe Hermann]

On Thu, Aug 30, 2007 at 08:10:31AM +0200, Markus Boas wrote:
> Signed-off-by: Markus Boas

Thanks, committed in r2753 (with minor changes).

Uwe.

[LinuxBIOS] Add Flash-chipsupport

[Uwe Hermann]

On Wed, Aug 29, 2007 at 09:27:59PM +0200, Markus Boas wrote:
> I hope this  one is better

Almost, please add the Signed-off-by to _all_ patches you send
(you forgot it on this patch), and please also include the README
update in this patch.
> Index: flash.h
>
> --- flash.h	(Revision 2751)
> +++ flash.h	(Arbeitskopie)
> @@ -100,7 +100,9 @@
>
> #define WINBOND_ID		0xDA	/* Winbond */
>  #define W_29C011		0xC1
> +#define W_29EE011		0xC1
>  #define W_29C020C		0x45
> +#define W_29C040P		0x46

Why is this W_29C040P here? I don't see it used anywhere.
I guess we should remove it from the patch?
> Index: w29ee011.c
>
> --- w29ee011.c	(Revision 0)
> +++ w29ee011.c	(Revision 0)
> @@ -0,0 +1,58 @@
> +/*
> This file is part of the LinuxBIOS project.

^^^^^^^^^
                Make this "flashrom" please

Please repost with a Signed-off-by and the above fixes, I think this
is ready to be committed otherwise.

Uwe.

[LinuxBIOS] util/* as separate projects? (was: Re: Add Flash-chipsupport)

[Carl-Daniel Hailfinger]

On 30.08.2007 02:07, Uwe Hermann wrote:
> On Wed, Aug 29, 2007 at 09:27:59PM +0200, Markus Boas wrote:
> > Index: w29ee011.c
> >
> > --- w29ee011.c	(Revision 0)
> > +++ w29ee011.c	(Revision 0)
> > @@ -0,0 +1,58 @@
> > +/*
> > This file is part of the LinuxBIOS project.
>                            ^^^^^^^^^
>             Make this "flashrom" please

I disagree. In my opinion flashrom is a part of the LinuxBIOS project.
However, we should discuss that in general for all utilities.

Carl-Daniel

[LinuxBIOS] util/* as separate projects? (was: Re: Add Flash-chipsupport)

[Uwe Hermann]

On Thu, Aug 30, 2007 at 02:19:08AM +0200, Carl-Daniel Hailfinger wrote:
> > > This file is part of the LinuxBIOS project.
> >                            ^^^^^^^^^
> >             Make this "flashrom" please
>
> I disagree. In my opinion flashrom is a part of the LinuxBIOS project.

Hm, depends on the viewpoint and personal preference, I guess. Flashrom
shares some/most of the developers with LinuxBIOS, but that doesn't
make it part of the LinuxBIOS project (as in: the firmware software
called LinuxBIOS). It's an independant user-space command line tool,
which just happens to be developed by the same people as LinuxBIOS.

The same is probably true for lxbios, FILO, lbmenu, and other related tools.

Uwe.

[LinuxBIOS] util/* as separate projects? (was: Re: Add Flash-chipsupport)

[ron minnich]

On 8/30/07, Uwe Hermann  wrote:
> On Thu, Aug 30, 2007 at 02:19:08AM +0200, Carl-Daniel Hailfinger wrote:
> > > > This file is part of the LinuxBIOS project.
> > >                            ^^^^^^^^^
> > >             Make this "flashrom" please
> >
> > I disagree. In my opinion flashrom is a part of the LinuxBIOS project.
>
> Hm, depends on the viewpoint and personal preference, I guess. Flashrom
> shares some/most of the developers with LinuxBIOS, but that doesn't
> make it part of the LinuxBIOS project (as in: the firmware software
> called LinuxBIOS). It's an independant user-space command line tool,
> which just happens to be developed by the same people as LinuxBIOS.
>
> The same is probably true for lxbios, FILO, lbmenu, and other related tools.

It began as part of linuxbios. At the very least, the heritage of
these tools should be acknowledged in the comments.

ron

[LinuxBIOS] Add Flash-chipsupport

[Markus]

Am Thu, 30 Aug 2007 02:07:45 +0200
schrieb Uwe Hermann :
> On Wed, Aug 29, 2007 at 09:27:59PM +0200, Markus Boas wrote:
> > I hope this  one is better
>
> Almost, please add the Signed-off-by to _all_ patches you send
> (you forgot it on this patch), and please also include the README
> update in this patch.
> > Index: flash.h
> >
> > --- flash.h	(Revision 2751)
> > +++ flash.h	(Arbeitskopie)
> > @@ -100,7 +100,9 @@
> >
> > #define WINBOND_ID		0xDA	/* Winbond */
> >  #define W_29C011		0xC1
> > +#define W_29EE011		0xC1
> >  #define W_29C020C		0x45
> > +#define W_29C040P		0x46
>
> Why is this W_29C040P here? I don't see it used anywhere.
> I guess we should remove it from the patch?

You are right. The patch for the Winbond 29C040P isn't finisch yet.
> > Index: w29ee011.c
> >
> > --- w29ee011.c	(Revision 0)
> > +++ w29ee011.c	(Revision 0)
> > @@ -0,0 +1,58 @@
> > +/*
> > This file is part of the LinuxBIOS project.
>                            ^^^^^^^^^
>             Make this "flashrom" please
>
>
> Please repost with a Signed-off-by and the above fixes, I think this
> is ready to be committed otherwise.
>
> Uwe.