patch: beginnings of support for geode gx2, v2, and Kconfig

[ron minnich]

attached.

ron

patch: beginnings of support for geode gx2, v2, and Kconfig

[Myles Watson]

Acked-by: Myles Watson

patch: beginnings of support for geode gx2, v2, and Kconfig

[ron minnich]

On Fri, Aug 28, 2009 at 7:43 AM, Myles Watson wrote:
> Acked-by: Myles Watson

Committed revision 4611.

patch: beginnings of support for geode gx2, v2, and Kconfig

[Uwe Hermann]

On Thu, Aug 27, 2009 at 10:40:04PM -0700, ron minnich wrote:
> Index: src/northbridge/amd/gx2/Kconfig
>
> --- src/northbridge/amd/gx2/Kconfig	(revision 0)
> +++ src/northbridge/amd/gx2/Kconfig	(revision 0)
> @@ -0,0 +1,27 @@

[...]
> +config NORTHBRIDGE_AMD_GX2
> bool
> default n
>
> +config HAVE_HIGH_TABLES
> bool
> default y

What's the plan for HAVE_HIGH_TABLES? Some NB/SB files define it to y,
but the default is already y. Aren't all chipsets/boards supposed to
use HAVE_HIGH_TABLES=y already? Or are some not fully supported, yet?
> Index: src/northbridge/amd/gx2/northbridge.c
>
> --- src/northbridge/amd/gx2/northbridge.c	(revision 4596)
> +++ src/northbridge/amd/gx2/northbridge.c	(working copy)
> @@ -486,7 +486,6 @@
>    printk_debug("gx2 north: enable_dev\n");
>    void northbridgeinit(void);
>    void chipsetinit(struct northbridge_amd_gx2_config *nb);
> void setup_realmode_idt(void);
> void do_vsmbios(void);
>     /* Set the operations if it is a special bus type */
>     if (dev->path.type == DEVICE_PATH_PCI_DOMAIN) {
>
> @@ -499,8 +498,6 @@
>    	cpubug();	
>    	chipsetinit(nb);
>    	setup_gx2();
> /* do this here for now -- this chip really breaks our device model */
> setup_realmode_idt();
> do_vsmbios();
> graphics_init();
> dev->ops = &pci_domain_ops;

Why this? Intentional?
> Index: src/cpu/amd/model_gx2/Kconfig
>
> --- src/cpu/amd/model_gx2/Kconfig	(revision 0)
> +++ src/cpu/amd/model_gx2/Kconfig	(revision 0)
> @@ -0,0 +1,3 @@
> +config CPU_AMD_GX2
> bool
> default false

Please use n (not false) for consistency, I think most of the kconfigs
use that, and also the Linux kernel IIRC.

Uwe.

patch: beginnings of support for geode gx2, v2, and Kconfig

[ron minnich]

First off, thanks for the extra review.

On Fri, Aug 28, 2009 at 10:00 AM, Uwe Hermann wrote:
> On Thu, Aug 27, 2009 at 10:40:04PM -0700, ron minnich wrote:
> > Index: src/northbridge/amd/gx2/Kconfig
> >
> > --- src/northbridge/amd/gx2/Kconfig   (revision 0)
> > +++ src/northbridge/amd/gx2/Kconfig   (revision 0)
> > @@ -0,0 +1,27 @@
>
> [...]
> > +config NORTHBRIDGE_AMD_GX2
> > bool
> > default n
> >
> > +config HAVE_HIGH_TABLES
> > bool
> > default y
>
> What's the plan for HAVE_HIGH_TABLES? Some NB/SB files define it to y,
> but the default is already y. Aren't all chipsets/boards supposed to
> use HAVE_HIGH_TABLES=y already? Or are some not fully supported, yet?

This was crud that slipped in. I removed it in 4616.
> > Index: src/northbridge/amd/gx2/northbridge.c
> >
> > --- src/northbridge/amd/gx2/northbridge.c     (revision 4596)
> > +++ src/northbridge/amd/gx2/northbridge.c     (working copy)
> > @@ -486,7 +486,6 @@
> >       printk_debug("gx2 north: enable_dev\n");
> >       void northbridgeinit(void);
> >       void chipsetinit(struct northbridge_amd_gx2_config *nb);
> > void setup_realmode_idt(void);
> >
> > void do_vsmbios(void);
> >          /* Set the operations if it is a special bus type */
> >          if (dev->path.type == DEVICE_PATH_PCI_DOMAIN) {
> > @@ -499,8 +498,6 @@
> >               cpubug();
> >               chipsetinit(nb);
> >               setup_gx2();
> > /* do this here for now -- this chip really breaks our device model */
> > setup_realmode_idt();
> >
> > do_vsmbios();
> >               graphics_init();
> >               dev->ops = &pci_domain_ops;
>
> Why this? Intentional?

This has been in there forever, and I'm not yet willing to remove it.
> > Index: src/cpu/amd/model_gx2/Kconfig
> >
> > --- src/cpu/amd/model_gx2/Kconfig     (revision 0)
> > +++ src/cpu/amd/model_gx2/Kconfig     (revision 0)
> > @@ -0,0 +1,3 @@
> > +config CPU_AMD_GX2
> > bool
> > default false
>
> Please use n (not false) for consistency, I think most of the kconfigs
> use that, and also the Linux kernel IIRC.

Thanks for catching this, fixed in 4616.

ron