Re: [LinuxBIOS] [PATCH] Via CN700

[Corey Osgood]

New patch attached, I think I've cleared everything up that can be.
Board patch will have to wait until tomorrow, I need to be up in <5 hours.

Uwe Hermann wrote:
> On Thu, Nov 15, 2007 at 12:38:01PM -0500, Corey Osgood wrote:
> > Add support for the Via CN700 northbridge, with C7 CPU and DDR2 RAM.
>
> Nice! Quick review below:

Thanks! Couple responses, tried to shorten it up a bit too.
> > */
> >
> > +static void c7_cpu_setup(device_t dev)
> > +{
> > /* Host bus interface registers (D0F2 0x50-0x67) */
> > pci_write_config8(dev, 0x50, 0x88);
> > pci_write_config8(dev, 0x51, 0x7a);
> > pci_write_config8(dev, 0x52, 0x6f);
> > pci_write_config8(dev, 0x53, 0x88);
> > pci_write_config8(dev, 0x54, 0x10);//was 0x1e, modifried
> > pci_write_config8(dev, 0x55, 0x16);
> > pci_write_config8(dev, 0x56, 0x01);
> > //pci_write_config8(dev, 0x57, 0x61);//set@200MHz dram clock
> > pci_write_config8(dev, 0x59, 0x60);
> > pci_write_config8(dev, 0x5d, 0xb2);
> > pci_write_config8(dev, 0x5e, 0x88);
> > pci_write_config8(dev, 0x5f, 0xc7);
> > pci_write_config8(dev, 0x60, 0xff);
> > pci_write_config8(dev, 0x61, 0xff);
> > pci_write_config8(dev, 0x62, 0x0f);
> > pci_write_config8(dev, 0x63, 0xff);
> > pci_write_config8(dev, 0x64, 0xff);
> > pci_write_config8(dev, 0x65, 0x0f);
> > pci_write_config8(dev, 0x66, 0xff);
> > pci_write_config8(dev, 0x67, 0x70);
> > /* Host Bus IO Circuit (D0F2 0x70-0x7d) */
> > pci_write_config8(dev, 0x70, 0x33);
> > pci_write_config8(dev, 0x71, 0x00);
> > pci_write_config8(dev, 0x72, 0x33);
> > pci_write_config8(dev, 0x73, 0x00);
> > pci_write_config8(dev, 0x74, 0x00);
> > pci_write_config8(dev, 0x75, 0x28);
> > pci_write_config8(dev, 0x76, 0x74);
> > pci_write_config8(dev, 0x77, 0x00);
> > pci_write_config8(dev, 0x78, 0x0a);
> > pci_write_config8(dev, 0x79, 0xaa);
> > pci_write_config8(dev, 0x7a, 0x24);
> > pci_write_config8(dev, 0x7b, 0x00);
> > pci_write_config8(dev, 0x7c, 0x00);
> > pci_write_config8(dev, 0x7d, 0x6d);
>
> Pretty scary for someone without the datasheet. Please add more comments
> and/or use self-explanatory #defines or something alike.

For the most part these shouldn't be messed with, the values are from
the porting guide and should work fine with any board (YMMV though).
I've added some comments, but to do any real work on this would
absolutely require access to the datasheets.
> > +}
> >
> > +/**
> > Set up various ram and other control registers statically. Some of these may
> > not be needed, other should be done with spd info, but that's a project for
> > the future
> > */
> >
> > +static void sdram_set_registers(const struct mem_controller *ctrl)
> > +{
> > /* TODO: Eliminate any duplicates */
> > pci_write_config8(ctrl->d0f3, 0xd0, 0x88);
> > pci_write_config8(ctrl->d0f3, 0xd1, 0x8b);
> > pci_write_config8(ctrl->d0f3, 0xd2, 0x89);
> > pci_write_config8(ctrl->d0f3, 0x86, 0x2d);
> > /* Driving selection */
> > /* DQ / DQS ODT Driving and Range Select */
> > pci_write_config8(ctrl->d0f3, 0xd5, 0x8a);
> > /* Memory Pads Driving and Range Select */
> > pci_write_config8(ctrl->d0f3, 0xd6, 0xaa);
> > /* DRAM Driving ??? Group DQS */
> > pci_write_config8(ctrl->d0f3, 0xe0, 0xee);
> > /* DRAM Driving ??? Group DQ (DQ, MPD, DQM) */
> > pci_write_config8(ctrl->d0f3, 0xe2, 0xac);//ba
> > /* DRAM Driving ??? Group CS */
> > pci_write_config8(ctrl->d0f3, 0xe4, 0x66);
> > /* DRAM Driving ??? Group MA */
> > pci_write_config8(ctrl->d0f3, 0xe8, 0x86);
> > /* DRAM Driving ??? Group MCLK */
> > pci_write_config8(ctrl->d0f3, 0xe6, 0xaa);
>
> This is better, at least there are a few comments so we (sort of) know
> what might be going on.
>
> It's still problematic if we were to make any changes to the code
> without having access to the datasheets, but I don't know a good way
> to make that easier (short of reproducing the datasheet as code comment).

And since the datasheets are NDA'd, completely reproducing them in
comments could cause some problems. Most of these registers have
different functions for every bit, so this file could easily become very
large very fast.
> > pci_write_config16(ctrl->d0f3, 0x50, 0x0020);
> > /* BA0-2 Selection. Don't mess with */
> > pci_write_config8(ctrl->d0f3, 0x52, 0x33);
> > pci_write_config8(ctrl->d0f3, 0x53, 0x3f);
> > /* Disable bank interleaving. This feature seems useless anyways */
>
> Why useless?

It's the most messed up form of interleaving I've ever heard of. It
allows the northbridge to trick /itself/ into thinking 2 dimms are a
single dimm, via a virtual bank routine and this register. I can't come
up with any useful purpose, since the number of virtual and physical
banks supported is the same. You can also (heh) map any physical bank
(or combination of physical banks) to any virtual bank, to really mess
with someone's head. It makes me wonder if Via is taking bribes from the
makers of Excedrin or Heineken.
> > /* 6. Mode register set. */
> > PRINT_DEBUG_MEM("RAM Enable 4: Mode register set\r\n");
> > //safe value for now, BL=8, WR=5, CAS=5
> > do_ram_command(dev, RAM_COMMAND_MRS, 0x0022d8);
>
> Please list/explain the bits and their meaning, 0x0022d8 is unreadable.

It's from the porting guide, there's a comment now. It somehow conforms
to JEDEC specs, but don't ask me how it works out, the address bits seem
to be scrambled. If someone can figure it out, please, let me know.
> > Index: src/northbridge/via/cn700/raminit.h
> >
> > --- src/northbridge/via/cn700/raminit.h	(revision 0)
> > +++ src/northbridge/via/cn700/raminit.h	(revision 0)
> > @@ -0,0 +1,35 @@
> > +/*
> > This file is part of the LinuxBIOS project.
> > Copyright (C) 2007 Corey Osgood
> > This program is free software; you can redistribute it and/or modify
> > it under the terms of the GNU General Public License as published by
> > the Free Software Foundation; either version 2 of the License, or
> > (at your option) any later version.
> > This program is distributed in the hope that it will be useful,
> > but WITHOUT ANY WARRANTY; without even the implied warranty of
> > MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > GNU General Public License for more details.
> > You should have received a copy of the GNU General Public License
> > along with this program; if not, write to the Free Software
> > Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 USA
> > */
> >
> > +#ifndef RAMINIT_H
> > +#define RAMINIT_H
> >
> > +#define RAM_COMMAND_NORMAL	0x0
> > +#define RAM_COMMAND_NOP		0x1
> > +#define RAM_COMMAND_PRECHARGE	0x2
> > +#define RAM_COMMAND_MRS		0x3
> > +#define RAM_COMMAND_CBR		0x4
> >
> > +#define DIMM_SOCKETS 1 //Only one on my board.
>
> Well, maybe, but the controller supports more than one?
>
> This is src/northbridge/via/cn700/raminit.h, which should not be specific
> to your board.

The controller supports maybe 4, maybe 8, all depends on if you believe
the datasheet or the porting guide. I set it at 1 for now so that if
anyone tries to use more then 1, they'll have to hunt down why it's not
compiling, and hopefully find that note and figure out that only 1 dimm
will work. Note has been changed, too.
> > Index: src/northbridge/via/cn700/vgabios.c
> >
> > --- src/northbridge/via/cn700/vgabios.c	(revision 0)
> > +++ src/northbridge/via/cn700/vgabios.c	(revision 0)
>
> There must be a better way than cloning the whole file. Did you change
> any of the contents?

Yes, the vga rom address, which is hardcoded (2Mb rom on epia-m, 4Mb on
j7f2). Once I can get another board I plan to see if I can get
BOCHS+x86emu to work, if that works this file can be dropped. If not,
I'll do some cleanup work to make it work on both epia-m and here, and
give it a nice home somewhere. It might be a while before I do though,
and I figure other people probably want to use/play with this in the
meantime.

Thanks,
Corey