[coreboot] [PATCH 1/2] cs5536: Add a NAND device and do the IDE PCI header disabling on time.

[Mart Raudsepp]

Ühel kenal päeval, R, 2009-01-09 kell 07:33, kirjutas Myles Watson:
> 
> > -----Original Message-----
> > From: coreboot-bounces+mylesgw=gmail.com at coreboot.org [mailto:coreboot-
> > bounces+mylesgw=gmail.com at coreboot.org] On Behalf Of Mart Raudsepp
> > Sent: Friday, January 09, 2009 5:21 AM
> > To: coreboot at coreboot.org
> > Subject: [coreboot] [PATCH 1/2] cs5536: Add a NAND device and do the IDE
> > PCI header disabling on time.
> > 
> > cs5536: Add a NAND device and do the IDE PCI header disabling on time.
> > 
> > This implements a nand device, akin to the ide device to follow the
> > coreboot-v3 device tree design better.
> > It allows us to do the IDE PCI header early enough in a clean way - the
> > hide_vpci was called way too late
> > before - in phase6 of southbridge device, but we need the Flash header
> > active instead of IDE in the VSA2
> > before bus scans happen, or the PCI device gets disabled in coreboot
> > understanding by the time we get it
> > enabled in VSA2.
> > It makes NAND setup work better, but still not completely. There is a VSA2
> > bug for which I made a patch,
> > but waiting on a new binary to test if after that everything works or not.
> > A quick hack to workaround the
> > VSA2 bug suggests something further will still need fixing. There are also
> > more potential opportunities
> > to shuffle NAND code around to match v3 approach better, but that's a next
> > step for me after NAND setup
> > actually works right in the current form.
> > 
> > Also corrected the documentation of ide_init() to match current reality.
> > 
> > Signed-off-by: Mart Raudsepp <mart.raudsepp at artecdesign.ee>
> > ---
> >  mainboard/artecgroup/dbe61/dts  |    3 ++
> >  mainboard/artecgroup/dbe62/dts  |    3 ++
> >  southbridge/amd/cs5536/cs5536.c |   47 ++++++++++++++++++++++++----------
> > ----
> >  southbridge/amd/cs5536/nand     |   23 +++++++++++++++++++
> >  4 files changed, 59 insertions(+), 17 deletions(-)
> >  create mode 100644 southbridge/amd/cs5536/nand
> > 
> > diff --git a/mainboard/artecgroup/dbe61/dts
> > b/mainboard/artecgroup/dbe61/dts
> > index b443533..48df288 100644
> > --- a/mainboard/artecgroup/dbe61/dts
> > +++ b/mainboard/artecgroup/dbe61/dts
> > @@ -109,6 +109,9 @@ end
> >  			/* USB Port Power Handling setting. */
> >  			pph = "0xf5";
> >  		};
> > +		pci at f,1 {
> > +			/config/("southbridge/amd/cs5536/nand");
> > +		};
> >  		pci at f,2 {
> >  			/config/("southbridge/amd/cs5536/ide");
> >  		};
> 
> Shouldn't you remove the ide device if it's going to be hidden?

With this patch there is still an enable_ide that has a default of zero
via southbridge/amd/cs5536/ide. The 2/2 patch does the enable_ide
killing and removal of this entry as you suggest.

> > diff --git a/mainboard/artecgroup/dbe62/dts
> > b/mainboard/artecgroup/dbe62/dts
> > index 213c397..3cbe0ff 100644
> > --- a/mainboard/artecgroup/dbe62/dts
> > +++ b/mainboard/artecgroup/dbe62/dts
> > @@ -63,6 +63,9 @@
> >  			/* USB Port Power Handling setting. */
> >  			pph = "0xf5";
> >  		};
> > +		pci at f,1 {
> > +			/config/("southbridge/amd/cs5536/nand");
> > +		};
> >  		pci at f,2 {
> >  			/config/("southbridge/amd/cs5536/ide");
> >  		};
> 
> Same thing here.

And same reply :)

> Maybe it would be better to have IDE/NAND selection depend on a CMOS/NVRAM
> value, so that the same BIOS could be used with either choice.

The board doesn't have IDE stuff wired anywhere. No existing boards
supported by coreboot do to my knowledge. I'm thinking that if a board
appears that wants to do it, we'd have both devices in the dts and have
the mainboard (or NVRAM handler) specific code disable one of them
before phase2 of devices with dev_set_enabled(dev, 0)

> Could we make an ide_or_nand device that called your nand_phase2 if it was
> nand, and used the ide_init if it wasn't.  They seem so similar, and only
> one is allowed at a time.

How do we get pci at f,1 in case of Flash and pci at f,2 in case of IDE then
if there is an ide_or_nand device?


Mart Raudsepp