[coreboot] patch: add mcp55 pcie device

[Carl-Daniel Hailfinger]

On 11.08.2008 07:37, ron minnich wrote:
> Note discussion in the patch.
>
> Not that I want another long-running discussion which results in
> nothing being done!
>
> Just "noting for the log"
>
> ron
>   
> pcie device for mcp55. 
>
> This compiles.
>
> Note this is not as convenient in one way as v2. In v2, due to the use of 
> linker sets, we simply get all 6 devices just by compiling. 
>
> In v3, 
> in the mainboard dts, we will need a separate dts node for each device, viz:
> pci at 0,a{};
> pci at 0,b{};
>
> etc. 
>
> This strongly argues for Uwe's request to put multiple device dts nodes
> in one file. I just don't know how to do that yet; suggestions welcome. 

How about multi-level dts instead of the current two-level model? An
intermediate level could aggregate device-level dts. The mainboard dts
would include intermediate-level dts unless low-level stuff was needed.

> Note that it *is* clearer from the point of view of seeing exactly what 
> is being built in in one place: the dts. In v2, the magic of linker scripts
> added considerable confusion, and we got complaints about that, so 
> we are trying to avoid such magic in v3. 

Point taken.

> The overall question of which is better? We'll see. 
>
> Note that in an earlier version of v3, we had this same capabilty 
> (albeit done without linker sets), and again we removed it as people 
> preferred to reduce the magic. 

Could you dig up the revision? I'd like to take a look. Thanks.

> Signed-off-by: Ronald G. Minnich <rminnich at gmail.com>

With the comments below addressed:
Acked-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>


> Index: southbridge/nvidia/mcp55/pcie.c
> ===================================================================
> --- southbridge/nvidia/mcp55/pcie.c	(revision 0)
> +++ southbridge/nvidia/mcp55/pcie.c	(revision 0)
> @@ -0,0 +1,117 @@
> +/*
> + * This file is part of the coreboot project.
> + *
> + * Copyright (C) 2004 Tyan Computer
> + * Written by Yinghai Lu <yhlu at tyan.com> for Tyan Computer.
> + * Copyright (C) 2006,2007 AMD
> + * Written by Yinghai Lu <yinghai.lu at amd.com> 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 pcie_init(struct device *dev)
> +{
> +
> +	/* Enable pci error detecting */
> +	u32 dword;
> +
> +	/* System error enable */
> +	dword = pci_read_config32(dev, 0x04);
> +	dword |= (1<<8); /* System error enable */
> +	dword |= (1<<30); /* Clear possible errors */
> +	pci_write_config32(dev, 0x04, dword);
> +
> +}
> +
> +struct device_operations mcp55_pcia = {

Please change the name to mcp55_pcie_a. Especially a few lines below,
where you have a mcp55_pcie, the naming gets really confusing really fast.

> +	.id = {.type = DEVICE_ID_PCI,
> +		{.pci = {.vendor = PCI_VENDOR_ID_NVIDIA,
> +			      .device = PCI_DEVICE_ID_NVIDIA_MCP55_PCIE_A}}},
> +	.constructor		 = default_device_constructor,
> +	.reset_bus		= pci_bus_reset,
> +	.phase3_scan		 = pci_scan_bridge,
> +	.phase4_read_resources	 = pci_dev_read_resources,
> +	.phase4_set_resources	 = pci_dev_set_resources,
> +	.phase5_enable_resources = pci_bus_enable_resources,
> +	.phase6_init		 = pcie_init,
> +	.ops_pci		 = &pci_dev_ops_pci,
> +};
> +
> +struct device_operations mcp55_pcib_c = {

mcp55_pcie_bc

> +	.id = {.type = DEVICE_ID_PCI,
> +		{.pci = {.vendor = PCI_VENDOR_ID_NVIDIA,
> +			      .device = PCI_DEVICE_ID_NVIDIA_MCP55_PCIE_B_C}}},
> +	.constructor		 = default_device_constructor,
> +	.reset_bus		= pci_bus_reset,
> +	.phase3_scan		 = pci_scan_bridge,
> +	.phase4_read_resources	 = pci_dev_read_resources,
> +	.phase4_set_resources	 = pci_dev_set_resources,
> +	.phase5_enable_resources = pci_bus_enable_resources,
> +	.phase6_init		 = pcie_init,
> +	.ops_pci		 = &pci_dev_ops_pci,
> +};
> +
> +struct device_operations mcp55_pcid = {

mcp55_pcie_d

> +	.id = {.type = DEVICE_ID_PCI,
> +		{.pci = {.vendor = PCI_VENDOR_ID_NVIDIA,
> +			      .device = PCI_DEVICE_ID_NVIDIA_MCP55_PCIE_D}}},
> +	.constructor		 = default_device_constructor,
> +	.reset_bus		= pci_bus_reset,
> +	.phase3_scan		 = pci_scan_bridge,
> +	.phase4_read_resources	 = pci_dev_read_resources,
> +	.phase4_set_resources	 = pci_dev_set_resources,
> +	.phase5_enable_resources = pci_bus_enable_resources,
> +	.phase6_init		 = pcie_init,
> +	.ops_pci		 = &pci_dev_ops_pci,
> +};
> +
> +struct device_operations mcp55_pcie = {

mcp55_pcie_e. Right now the name looks like it is the only PCIe device
operations. Bad.

> +	.id = {.type = DEVICE_ID_PCI,
> +		{.pci = {.vendor = PCI_VENDOR_ID_NVIDIA,
> +			      .device = PCI_DEVICE_ID_NVIDIA_MCP55_PCIE_D}}},
> +	.constructor		 = default_device_constructor,
> +	.reset_bus		= pci_bus_reset,
> +	.phase3_scan		 = pci_scan_bridge,
> +	.phase4_read_resources	 = pci_dev_read_resources,
> +	.phase4_set_resources	 = pci_dev_set_resources,
> +	.phase5_enable_resources = pci_bus_enable_resources,
> +	.phase6_init		 = pcie_init,
> +	.ops_pci		 = &pci_dev_ops_pci,
> +};
> +
> +struct device_operations mcp55_pcif = {

mcp55_pcie_f

> +	.id = {.type = DEVICE_ID_PCI,
> +		{.pci = {.vendor = PCI_VENDOR_ID_NVIDIA,
> +			      .device = PCI_DEVICE_ID_NVIDIA_MCP55_PCIE_F}}},
> +	.constructor		 = default_device_constructor,
> +	.reset_bus		= pci_bus_reset,
> +	.phase3_scan		 = pci_scan_bridge,
> +	.phase4_read_resources	 = pci_dev_read_resources,
> +	.phase4_set_resources	 = pci_dev_set_resources,
> +	.phase5_enable_resources = pci_bus_enable_resources,
> +	.phase6_init		 = pcie_init,
> +	.ops_pci		 = &pci_dev_ops_pci,
> +};

Regards,
Carl-Daniel

-- 
http://www.hailfinger.org/