Re: [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+mylesgw=[e-mail] [mailto:coreboot-
> bounces+mylesgw=[e-mail]] On Behalf Of Mart Raudsepp
> Sent: Friday, January 09, 2009 5:21 AM
> To: [e-mail]
> 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
>
> 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@f,1 {
> 	/config/("southbridge/amd/cs5536/nand");
> };
> pci@f,2 {
> 	/config/("southbridge/amd/cs5536/ide");
> };

Shouldn't you remove the ide device if it's going to be hidden?
> 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@f,1 {
> 	/config/("southbridge/amd/cs5536/nand");
> };
> pci@f,2 {
> 	/config/("southbridge/amd/cs5536/ide");
> };

Same thing here.

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.

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.

Thanks,
Myles