[coreboot] [PATCH 3/5] Intel 3100, version 3

Uwe Hermann uwe at hermann-uwe.de
Fri Mar 14 01:54:49 CET 2008


On Thu, Mar 13, 2008 at 05:16:26PM -0700, Ed Swierk wrote:
> +/* This code is based on src/southbridge/intel/esb6300/chip.h */

Drop this, it's just a list of #defines.


> Index: coreboot-v2-3137/src/southbridge/intel/i3100/i3100_early_smbus.c
[...]
> +/* This code is based on src/southbridge/intel/esb6300/esb6300_early_smbus.c */

Drop, this code is trivial.


> +
> +#include "i3100_smbus.h"
> +
> +#define SMBUS_IO_BASE 0x0f00
> +
> +static void enable_smbus(void)
> +{
> +	device_t dev;
> +	dev = pci_locate_device(PCI_ID(0x8086, 0x269b), 0);

Use values from pci_id.h, please.


> +	if (dev == PCI_DEV_INVALID) {
> +		die("SMBus controller not found\r\n");
> +	}
> +	uint8_t enable;

Move all variables to the top.


> +	print_spew("SMBus controller enabled\r\n");
> +	pci_write_config32(dev, 0x20, SMBUS_IO_BASE | 1);
> +	pci_write_config8(dev, 0x40, 1);
> +	pci_write_config8(dev, 0x4, 1);
> +	/* SMBALERT_DIS */
> +        outb(4, SMBUS_IO_BASE + SMBSLVCMD);
> +
> +	/* Disable interrupt generation */
> +	outb(0, SMBUS_IO_BASE + SMBHSTCTL);
> +}
> +
> +static int smbus_read_byte(unsigned device, unsigned address)

Use u8, u16 et al.


> +{
> +	return do_smbus_read_byte(SMBUS_IO_BASE, device, address);
> +}
> Index: coreboot-v2-3137/src/southbridge/intel/i3100/i3100_ehci.c
> ===================================================================
> --- /dev/null
> +++ coreboot-v2-3137/src/southbridge/intel/i3100/i3100_ehci.c
> @@ -0,0 +1,71 @@
> +/*
> + * This file is part of the coreboot project.
> + *
> + * Copyright (C) 2008 Arastra, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * 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
> + *
> + */
> +
> +/* This code is based on src/southbridge/intel/esb6300/esb6300_ehci.c */

Drop, trivial/canonical code.


> +
> +#include <console/console.h>
> +#include <device/device.h>
> +#include <device/pci.h>
> +#include <device/pci_ids.h>
> +#include <device/pci_ops.h>
> +#include "i3100.h"
> +
> +static void ehci_init(struct device *dev)
> +{
> +	uint32_t cmd;
> +
> +	/* Not sure if this is needed on i3100: */
> +	printk_debug("EHCI: Setting up controller.. ");
> +	cmd = pci_read_config32(dev, PCI_COMMAND);
> +	pci_write_config32(dev, PCI_COMMAND, cmd | PCI_COMMAND_MASTER);
> +	printk_debug("done.\n");

Tested? I'd say drop it (i.e. make the function empty) until it's proven
that this is really required. AFAIK it's not (as seen on other boards).


> +}
> +
> +static void ehci_set_subsystem(device_t dev, unsigned vendor, unsigned device)
> +{
> +	uint8_t access_cntl;
> +	access_cntl = pci_read_config8(dev, 0x80);
> +	/* Enable writes to protected registers */
> +	pci_write_config8(dev, 0x80, access_cntl | 1);
> +	/* Write the subsystem vendor and device id */
> +	pci_write_config32(dev, PCI_SUBSYSTEM_VENDOR_ID,
> +		((device & 0xffff) << 16) | (vendor & 0xffff));
> +	/* Restore protection */
> +	pci_write_config8(dev, 0x80, access_cntl);
> +}
> +
> +static struct pci_operations lops_pci = {
> +	.set_subsystem = &ehci_set_subsystem,
> +};
> +static struct device_operations ehci_ops  = {
> +	.read_resources   = pci_dev_read_resources,
> +	.set_resources    = pci_dev_set_resources,
> +	.enable_resources = pci_dev_enable_resources,
> +	.init             = ehci_init,
> +	.scan_bus         = 0,
> +	.enable           = i3100_enable,
> +	.ops_pci          = &lops_pci,
> +};
> +
> +static struct pci_driver ehci_driver __pci_driver = {
> +	.ops    = &ehci_ops,
> +	.vendor = PCI_VENDOR_ID_INTEL,
> +	.device = PCI_DEVICE_ID_INTEL_3100_EHCI,
> +};
> Index: coreboot-v2-3137/src/southbridge/intel/i3100/i3100_lpc.c
> ===================================================================
> --- /dev/null
> +++ coreboot-v2-3137/src/southbridge/intel/i3100/i3100_lpc.c
> @@ -0,0 +1,316 @@
> +/*
> + * This file is part of the coreboot project.
> + *
> + * Copyright (C) 2004 Linux Networx
> + * Copyright (C) 2008 Arastra, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * 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
> + *
> + */
> +
> +/* This code is based on src/southbridge/intel/esb6300/esb6300_lpc.c */

This is ok, this file is huge and non-trivial.


> Index: coreboot-v2-3137/src/southbridge/intel/i3100/i3100_pci.c
> ===================================================================
> --- /dev/null
> +++ coreboot-v2-3137/src/southbridge/intel/i3100/i3100_pci.c
> @@ -0,0 +1,58 @@
> +/*
> + * This file is part of the coreboot project.
> + *
> + * Copyright (C) 2008 Arastra, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * 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
> + *
> + */
> +
> +/* This code is based on src/southbridge/intel/esb6300/esb6300_pci.c */

Drop, trivial.


> +
> +#include <console/console.h>
> +#include <device/device.h>
> +#include <device/pci.h>
> +#include <device/pci_ids.h>
> +#include <device/pci_ops.h>
> +#include "i3100.h"
> +
> +static void pci_init(struct device *dev)
> +{
> +	uint16_t word;
> +
> +	/* Clear system errors */
> +	word = pci_read_config16(dev, 0x06);
> +	word |= 0xf900; /* Clear possible errors */
> +	pci_write_config16(dev, 0x06, word);
> +
> +	word = pci_read_config16(dev, 0x1e);
> +	word |= 0xf800; /* Clear possible errors */
> +	pci_write_config16(dev, 0x1e, word);

Needed? Maybe yes, not sure. Have you tested dropping it?


> +}
> +
> +static struct device_operations pci_ops  = {
> +	.read_resources   = pci_bus_read_resources,
> +	.set_resources    = pci_dev_set_resources,
> +	.enable_resources = pci_bus_enable_resources,
> +	.init             = pci_init,
> +	.scan_bus         = pci_scan_bridge,
> +	.ops_pci          = 0,
> +};
> +
> +static struct pci_driver pci_driver __pci_driver = {
> +	.ops    = &pci_ops,
> +	.vendor = PCI_VENDOR_ID_INTEL,
> +	.device = PCI_DEVICE_ID_INTEL_3100_PCI,
> +};
> +
> Index: coreboot-v2-3137/src/southbridge/intel/i3100/i3100_sata.c
> ===================================================================
[...]
> +static void sata_set_subsystem(device_t dev, unsigned vendor, unsigned device)

u16 for vendor/device.


> +{
> +	pci_write_config32(dev, PCI_SUBSYSTEM_VENDOR_ID,
> +		((device & 0xffff) << 16) | (vendor & 0xffff));
> +}
> +
> +static struct pci_operations lops_pci = {
> +	.set_subsystem = sata_set_subsystem,
> +};
> +static struct device_operations sata_ops  = {
> +	.read_resources   = pci_dev_read_resources,
> +	.set_resources    = pci_dev_set_resources,
> +	.enable_resources = pci_dev_enable_resources,
> +	.init             = sata_init,
> +	.scan_bus         = 0,
> +	.ops_pci          = &lops_pci,
> +};
> +
> +static struct pci_driver sata_driver __pci_driver = {
> +        .ops    = &sata_ops,
> +        .vendor = PCI_VENDOR_ID_INTEL,
> +        .device = PCI_DEVICE_ID_INTEL_3100_SATA,
> +};
> +
> +static struct pci_driver sata_driver_nr __pci_driver = {
> +        .ops    = &sata_ops,
> +        .vendor = PCI_VENDOR_ID_INTEL,
> +        .device = PCI_DEVICE_ID_INTEL_3100_SATA_R,
> +};


> Index: coreboot-v2-3137/src/southbridge/intel/i3100/i3100_smbus.c
> ===================================================================
> --- /dev/null
> +++ coreboot-v2-3137/src/southbridge/intel/i3100/i3100_smbus.c
> @@ -0,0 +1,69 @@
> +/*
> + * This file is part of the coreboot project.
> + *
> + * Copyright (C) 2008 Arastra, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * 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
> + *
> + */
> +
> +/* This code is based on src/southbridge/intel/esb6300/esb6300_smbus.c */

Drop, trivial.


> Index: coreboot-v2-3137/src/southbridge/intel/i3100/i3100_uhci.c
> ===================================================================
> --- /dev/null
> +++ coreboot-v2-3137/src/southbridge/intel/i3100/i3100_uhci.c
> @@ -0,0 +1,71 @@
> +/*
> + * This file is part of the coreboot project.
> + *
> + * Copyright (C) 2008 Arastra, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * 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
> + *
> + */
> +
> +/* This code is based on src/southbridge/intel/esb6300/esb6300_uhci.c */

Drop, trivial file.


> +
> +#include <console/console.h>
> +#include <device/device.h>
> +#include <device/pci.h>
> +#include <device/pci_ids.h>
> +#include <device/pci_ops.h>
> +#include "i3100.h"
> +
> +static void uhci_init(struct device *dev)
> +{
> +	uint32_t cmd;
> +
> +	/* Not sure if this is needed on i3100: */
> +	printk_debug("UHCI: Setting up controller.. ");
> +	cmd = pci_read_config32(dev, PCI_COMMAND);
> +	pci_write_config32(dev, PCI_COMMAND, cmd | PCI_COMMAND_MASTER);
> +	printk_debug("done.\n");

Likely also not needed, please test.


> +}
> +
> +static void uhci_set_subsystem(device_t dev, unsigned vendor, unsigned device)
> +{
> +	pci_write_config32(dev, PCI_SUBSYSTEM_VENDOR_ID,
> +		((device & 0xffff) << 16) | (vendor & 0xffff));
> +}

Hm, looks redundant, should be in some global file later maybe. Won't
hold up the commit, though.


> +
> +static struct pci_operations lops_pci = {
> +	.set_subsystem = &uhci_set_subsystem,
> +};
> +
> +static struct device_operations uhci_ops  = {
> +	.read_resources   = pci_dev_read_resources,
> +	.set_resources    = pci_dev_set_resources,
> +	.enable_resources = pci_dev_enable_resources,
> +	.init             = uhci_init,
> +	.scan_bus         = 0,
> +	.enable           = i3100_enable,
> +	.ops_pci          = &lops_pci,
> +};
> +
> +static struct pci_driver uhci_driver __pci_driver = {
> +	.ops    = &uhci_ops,
> +	.vendor = PCI_VENDOR_ID_INTEL,
> +	.device = PCI_DEVICE_ID_INTEL_3100_USB,
> +};
> +
> +static struct pci_driver usb2_driver __pci_driver = {
> +	.ops    = &uhci_ops,
> +	.vendor = PCI_VENDOR_ID_INTEL,
> +	.device = PCI_DEVICE_ID_INTEL_3100_USB2,
> +};
> Index: coreboot-v2-3137/src/southbridge/intel/i3100/i3100_early_lpc.c
> ===================================================================
> --- /dev/null
> +++ coreboot-v2-3137/src/southbridge/intel/i3100/i3100_early_lpc.c
> @@ -0,0 +1,32 @@
> +/*
> + * This file is part of the coreboot project.
> + *
> + * Copyright (C) 2008 Arastra, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * 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
> + *
> + */
> +
> +static void i3100_enable_superio(void)
> +{
> +	device_t dev;
> +	dev = pci_locate_device(PCI_ID(0x8086, 0x2670), 0);

Use pci_ids.h.


> +	if (dev == PCI_DEV_INVALID) {
> +		outb(0xd0, 0x80);

Why 0xd0?


> +		die("LPC bridge not found\r\n");
> +	}
> +
> +	/* Enable decoding of I/O locations for SuperIO devices */
> +	pci_write_config16(dev, 0x82, 0x340f);
> +}


Uwe.
-- 
http://www.hermann-uwe.de  | http://www.holsham-traders.de
http://www.crazy-hacks.org | http://www.unmaintained-free-software.org




More information about the coreboot mailing list