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

[Myles Watson]

> -----Original Message-----
> From: coreboot-bounces at coreboot.org [mailto:coreboot-bounces at coreboot.org]
> On Behalf Of Mart Raudsepp
> Sent: Friday, January 09, 2009 7:41 AM
> To: coreboot at coreboot.org
> Subject: Re: [coreboot] [PATCH 1/2] cs5536: Add a NAND device and do the
> IDE PCI header disabling on time.
> 
> Ü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)

That's the answer to the question I should have asked. :)  Thanks for the
clarification.

> > 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?

Yes.  They were separate suggestions.

Thanks,
Myles