patch: stage 2 ide device for mcp55

[ron minnich]

attached. This was pretty trivial. I expect to be able to do the next
few devices and take at most 15 mins apiece for them.

First I intend, since Peter keeps asking, to modify dtc so we can have
ide.dtc and not just ide.

This actually compiles and builds and the dtc works and ... well it's nice.

ron

[Carl-Daniel Hailfinger]

On 11.08.2008 06:42, ron minnich wrote:
> attached. This was pretty trivial. I expect to be able to do the next
> few devices and take at most 15 mins apiece for them.
>
> First I intend, since Peter keeps asking, to modify dtc so we can have
> ide.dtc and not just ide.
>
> This actually compiles and builds and the dtc works and ... well it's nice.
>
> ron
>
> Here is the first stage2 device. It was really easy to bring over.
>
> For Peter and others, the next thing I'll do is make it so dtc can
> use 'ide.dtc' and still get the name right.
>
> Stand by.
> For those of you who want to see how to bring other chips across, 
> watch this space: this is a tutorial of sorts.
>
> So far, it's going well. 
> Signed-off-by: Ronald G. Minnich

A few minor comments, feel free to commit anyway.
Acked-by: Carl-Daniel Hailfinger
> Index: southbridge/nvidia/mcp55/ide.c
>
> --- southbridge/nvidia/mcp55/ide.c	(revision 0)
> +++ southbridge/nvidia/mcp55/ide.c	(revision 0)
> @@ -0,0 +1,93 @@
> +/*
> This file is part of the coreboot project.
> Copyright (C) 2004 Tyan Computer
> Written by Yinghai Lu  for Tyan Computer.
> Copyright (C) 2006,2007 AMD
> Written by Yinghai Lu  for AMD.
> 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
> */
>
> +#include <types.h>
> +#include <lib.h>
> +#include <console.h>
> +#include <device/pci.h>
> +#include <msr.h>
> +#include <legacy.h>
> +#include <device/pci_ids.h>
> +#include <statictree.h>
> +#include <config.h>
> +#include "mcp55.h"
>
> +static void ide_init(struct device *dev)
> +{
> struct southbridge_nvidia_mcp55_ide_config *conf =
>    (struct southbridge_nvidia_mcp55_ide_config *)dev->device_configuration;
> /* Enable ide devices so the linux ide driver will work */
> u32 dword;
> u16 word;
> u8 byte;
> word = pci_read_config16(dev, 0x50);
> /* Ensure prefetch is disabled */
> word &= ~((1 << 15) | (1 << 13));
> if (conf->ide1_enable) {
> /* Enable secondary ide interface */
> word |= (1<<0);
> printk(BIOS_DEBUG, "IDE1 \t");

Can you move the comment into the printk? Same value for readers of the
code, great improvement for the logs.
> }
> if (conf->ide0_enable) {
> /* Enable primary ide interface */
> word |= (1<<1);
> printk(BIOS_DEBUG, "IDE0\n");

Same here.
> }
> word |= (1<<12);
> word |= (1<<14);
> pci_write_config16(dev, 0x50, word);
> byte = 0x20 ; // Latency: 64-->32
> pci_write_config8(dev, 0xd, byte);
> dword = pci_read_config32(dev, 0xf8);
> dword |= 12;
> pci_write_config32(dev, 0xf8, dword);
>
> +#ifdef CONFIG_PCI_ROM_RUN
> pci_dev_init(dev);
>
> +#endif
>
> +#warning set subsystem id on mcp55 ide
> +#if 0
> pci_write_config32(dev, 0x40,
> ((device & 0xffff) << 16) | (vendor & 0xffff));
>
> +#endif

Maybe move the hunk above into an extra function so we can use the
generic subsystem ID infrastructure?
> +}
>
> +struct device_operations mcp55_ide = {
> .id = {.type = DEVICE_ID_PCI,
> {.pci = {.vendor = PCI_VENDOR_ID_NVIDIA,
> 	      .device = PCI_DEVICE_ID_NVIDIA_MCP55_IDE}}},
> .constructor		 = default_device_constructor,
> .phase3_scan		 = 0,
> .phase4_read_resources	 = pci_dev_read_resources,
> .phase4_set_resources	 = pci_dev_set_resources,
> .phase5_enable_resources = pci_dev_enable_resources,
> .phase6_init		 = ide_init,
> .ops_pci		 = &pci_dev_ops_pci,
>
> +};

Regards,
Carl-Daniel

[ron minnich]

On Mon, Aug 11, 2008 at 3:20 AM, Carl-Daniel Hailfinger
> Maybe move the hunk above into an extra function so we can use the
> generic subsystem ID infrastructure?

It's gone. There is not one on v3 and I see no need to add it. The
propagation of subsystem stuff can be done by the dtc, AFAICS. We do
this:
1. add subsystem info to dts and device struct
2. allow users to specify subsystem to any node in the dts. All nodes
under that
   node will inherit that id unless they similarly set it.
3. at the root, we set subsystem to (I assume) mainboard VID and DID
4. at flatten tree time, we walk the tree and propagate subsystem VID
and DID to
children. This ensures that all devices have it at compile time
5. In a final pass at runtime (probably phase6) we walk the tree and,
for devices that set 'support subsystem VID/DID', we set it into the
hardware .

Done. Run time mess becomes compile time mess. Unless I'm missing something.

If I am, let me know.

ron

[Uwe Hermann]

On Mon, Aug 11, 2008 at 09:03:14AM -0700, ron minnich wrote:
> On Mon, Aug 11, 2008 at 3:20 AM, Carl-Daniel Hailfinger
> > Maybe move the hunk above into an extra function so we can use the
> > generic subsystem ID infrastructure?
>
> It's gone. There is not one on v3 and I see no need to add it. The
> propagation of subsystem stuff can be done by the dtc, AFAICS. We do
> this:
> add subsystem info to dts and device struct
> allow users to specify subsystem to any node in the dts. All nodes
>
> under that
>    node will inherit that id unless they similarly set it.
> 3. at the root, we set subsystem to (I assume) mainboard VID and DID
> 4. at flatten tree time, we walk the tree and propagate subsystem VID
> and DID to
> children. This ensures that all devices have it at compile time
> 5. In a final pass at runtime (probably phase6) we walk the tree and,
> for devices that set 'support subsystem VID/DID', we set it into the
> hardware .
>
> Done. Run time mess becomes compile time mess. Unless I'm missing something.
>
> If I am, let me know.

Sounds good, but will need some testing on actual hardware, of course.

I think we'll also need a mechanism to 'not set' or 'unset' subsystem
IDs for certain devices. Some (PCI) devices on some chipsets just can't
have subsystem IDs at all, for some we don't know _how_ to set them
(missing datasheets) and some inherit their IDs from other devices and
don't have a mechanism to set their own (e.g. some Intel audio inherits
the subsystem IDs of IDE and similar stuff).

Uwe.

[ron minnich]

On Mon, Aug 11, 2008 at 4:12 PM, Uwe Hermann  wrote:
> I think we'll also need a mechanism to 'not set' or 'unset' subsystem
> IDs for certain devices. Some (PCI) devices on some chipsets just can't
> have subsystem IDs at all, for some we don't know _how_ to set them
> (missing datasheets) and some inherit their IDs from other devices and
> don't have a mechanism to set their own (e.g. some Intel audio inherits
> the subsystem IDs of IDE and similar stuff).

yes, and the easiest place to capture all this behavior is in the
phase6_init code. That's why I put it there. So the dtc can compile in
subsystem ids but the real determinant of how it gets set is the
device itself.

ron

[Carl-Daniel Hailfinger]

On 11.08.2008 18:03, ron minnich wrote:
> On Mon, Aug 11, 2008 at 3:20 AM, Carl-Daniel Hailfinger
> > Maybe move the hunk above into an extra function so we can use the
> > generic subsystem ID infrastructure?
>
> It's gone. There is not one on v3 and I see no need to add it. The

We have a generic subsystem ID infrastructure in v3. Look at
pci_dev_enable_resources().
> propagation of subsystem stuff can be done by the dtc, AFAICS. We do
> this:
> add subsystem info to dts and device struct

We only have per-mainboard subsystem info in the dts. We need to add
per-device subsystem info to the dts.
> allow users to specify subsystem to any node in the dts. All nodes
>
> under that
>    node will inherit that id unless they similarly set it.

Not yet done.
> at the root, we set subsystem to (I assume) mainboard VID and DID

We set a special variable mainboard_subsystem_vendor and ..._id as part
of the mainboard struct.
> at flatten tree time, we walk the tree and propagate subsystem VID
>
> and DID to
> children. This ensures that all devices have it at compile time

Not yet.
> In a final pass at runtime (probably phase6) we walk the tree and,
>
> for devices that set 'support subsystem VID/DID', we set it into the
> hardware.

We do that in pci_dev_enable_resources().
> Done. Run time mess becomes compile time mess. Unless I'm missing something.
>
> If I am, let me know.

See above.

A few seconds ago, I posted a patch to move the mcp55 subsystem ID code
to the existing v3 infrastructure.

Regards,
Carl-Daniel

[ron minnich]

On Mon, Aug 11, 2008 at 6:36 PM, Carl-Daniel Hailfinger
> We have a generic subsystem ID infrastructure in v3. Look at
> pci_dev_enable_resources().

True. I left it there by mistake :-)
> > propagation of subsystem stuff can be done by the dtc, AFAICS. We do
> > this:
> > add subsystem info to dts and device struct
>
> We only have per-mainboard subsystem info in the dts. We need to add
> per-device subsystem info to the dts.

it's easy. Add two properties, and if they are set, then put then in
the device, and if they are not, leave them 0.
> > allow users to specify subsystem to any node in the dts. All nodes
> >
> > under that
> >    node will inherit that id unless they similarly set it.
>
> Not yet done.

good point.
> > at the root, we set subsystem to (I assume) mainboard VID and DID
>
> We set a special variable mainboard_subsystem_vendor and ..._id as part
> of the mainboard struct.

yep, I'm counting on that. See, most fo the work is already done :-)
> > at flatten tree time, we walk the tree and propagate subsystem VID
> >
> > and DID to
> > children. This ensures that all devices have it at compile time
>
> Not yet.

but we could, right? Then we'd have it at compile time and when we
visualize the tree. That would be cool!
> We do that in pci_dev_enable_resources().

good point.

I would much rather do this in dtc, but I'm not that worried about it,
but how about we drop the pci_ops and just put the setting of
subsystem ops into the generic device. It's too much messing around to
all this pci_ops stuff for one simple operation.

ron

[ron minnich]

Committed revision 739.