[coreboot] patch: mcp55 usb for v3

[Carl-Daniel Hailfinger]

On 11.08.2008 08:14, ron minnich wrote:
> attached.
>
> This one was easy.
>
> BTW there was a bug in my script so some earlier patches show struct
> device dev instead of
> struct device *dev. I'm happy to be told this but I'm also aware of it :-)
>
> thanks
>
> ron
>
> Add support for mcp55 usb and usb2.
>
> These both compile. There is an unresolved issue w.r.t. the DEBUG 
> check in usb2. How do we want this in v3?
>
> Signed-off-by: Ronald G. Minnich <rminnich at gmail.com>
>   

No Ack yet, I'd like an explanation of what the unresolved issue is
before I decide.
Review below.

Regards,
Carl-Daniel

> Index: southbridge/nvidia/mcp55/usb2.c
> ===================================================================
> --- southbridge/nvidia/mcp55/usb2.c	(revision 0)
> +++ southbridge/nvidia/mcp55/usb2.c	(revision 0)
> @@ -0,0 +1,90 @@
> +/*
> + * 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"
> +
> +#warning Figure out if we want usb debug direct
>   

Hm?

> +#define CONFIG_USBDEBUG_DIRECT 0
>   

Move that one to a Kconfig variable.

> +#if CONFIG_USBDEBUG_DIRECT
> +#include <usbdebug_direct.h>
> +#endif
> +
> +// yeah right. not yet. extern struct ehci_debug_info dbg_info;
>   

More context, please.

> +
> +static void usb2_init(struct device *dev)
> +{
> +	u32 dword;
> +	dword = pci_read_config32(dev, 0xf8);
> +	dword |= 40;
> +	pci_write_config32(dev, 0xf8, dword);
> +#warning mange set subsystem in mcp55 usb2
> +#if 0
>   

Separate function please.

> +	pci_write_config32(dev, 0x40,
> +		((device & 0xffff) << 16) | (vendor & 0xffff));
> +#endif
> +}
> +
> +static void usb2_set_resources(struct device *dev)
> +{
> +#if CONFIG_USBDEBUG_DIRECT
> +	struct resource *res;
> +	unsigned base;
> +	unsigned old_debug;
> +
> +	old_debug = get_ehci_debug();
> +	set_ehci_debug(0);
> +#endif
> +	pci_dev_set_resources(dev);
> +
> +#if CONFIG_USBDEBUG_DIRECT
> +	res = find_resource(dev, 0x10);
> +	set_ehci_debug(old_debug);
> +	if (!res) return;
> +	base = res->base;
> +	set_ehci_base(base);
> +	report_resource_stored(dev, res, "");
> +#endif
> +
> +}
> +
> +struct device_operations mcp55_usb2 = {
> +	.id = {.type = DEVICE_ID_PCI,
> +		{.pci = {.vendor = PCI_VENDOR_ID_NVIDIA,
> +			      .device = PCI_DEVICE_ID_NVIDIA_MCP55_USB2}}},
> +	.constructor		 = default_device_constructor,
> +	.phase3_scan		 = 0,
> +	.phase4_read_resources	 = pci_dev_read_resources,
> +	.phase4_set_resources	 = usb2_set_resources,
> +	.phase5_enable_resources = pci_dev_enable_resources,
> +	.phase6_init		 = usb2_init,
> +	.ops_pci		 = &pci_dev_ops_pci,
>   

Exactly here you'd reference the set_subsystem function. That applies to
all other patches as well.

> +};
> Index: southbridge/nvidia/mcp55/usb.c
> ===================================================================
> --- southbridge/nvidia/mcp55/usb.c	(revision 0)
> +++ southbridge/nvidia/mcp55/usb.c	(revision 0)
> @@ -0,0 +1,55 @@
> +/*
> + * 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 usb_init(struct device * dev)
> +{
> +#warning handle subsystem set in mcp55 usb
> +#if 0
>   

Separate function.

> +	pci_write_config32(dev, 0x40,
> +		((device & 0xffff) << 16) | (vendor & 0xffff));
> +#endif
> +}
> +
> +struct device_operations mcp55_usb = {
> +	.id = {.type = DEVICE_ID_PCI,
> +		{.pci = {.vendor = PCI_VENDOR_ID_NVIDIA,
> +			      .device = PCI_DEVICE_ID_NVIDIA_MCP55_USB}}},
> +	.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		 = usb_init,
> +	.ops_pci		 = &pci_dev_ops_pci,
>   

set_subsystem usage please.

> +};
>
>   

Regards,
Carl-Daniel

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