[coreboot] [PATCH 2/5] Intel 3100, version 3: SuperIO and UART support

[Uwe Hermann]

On Thu, Mar 13, 2008 at 02:42:18PM -0700, Ed Swierk wrote:
> Index: coreboot-v2-3137/src/superio/intel/i3100/i3100.h
> ===================================================================
> --- /dev/null
> +++ coreboot-v2-3137/src/superio/intel/i3100/i3100.h
> @@ -0,0 +1,25 @@
> +/*
> + * 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 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
> + */
> +
> +/* Datasheet: http://www.intel.com/design/intarch/datashts/313458.htm */
> +
> +#define I3100_SP1 0x04 /* Com1 */
> +#define I3100_SP2 0x05 /* Com2 */
> +#define I3100_WDT 0x06 /* Watchdog timer */

Maybe add an include guard here, too, just in case.


> Index: coreboot-v2-3137/src/superio/intel/i3100/i3100_early_serial.c
> ===================================================================
> --- /dev/null
> +++ coreboot-v2-3137/src/superio/intel/i3100/i3100_early_serial.c
> @@ -0,0 +1,52 @@
> +/*
> + * 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 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 <arch/romcc_io.h>
> +#include "i3100.h"
> +
> +static void i3100_sio_write(uint8_t port, uint8_t ldn, uint8_t index,
> +			    uint8_t value)

You can use u8 instead of uint8_t et al everywhere, this changes is
required when porting to v3 anyway, so we should do it from the
beginning.


> Index: coreboot-v2-3137/src/superio/intel/i3100/superio.c
> ===================================================================
> --- /dev/null
> +++ coreboot-v2-3137/src/superio/intel/i3100/superio.c
> @@ -0,0 +1,113 @@
> +/*
> + * This file is part of the coreboot project.
> + *
> + * Copyright (C) 2006 Uwe Hermann <uwe at hermann-uwe.de>
> + *
> + * Copyright (C) 2007 AMD
> + * Written by Yinghai Lu <yinghai.lu at amd.com> for AMD.

Nah, drop these two. This is trivial code which you really can't write
much differently, even if you tried.

A lot of this should be in some global lib file anyway (will probably be
in v3), but it's fine for now to leave it here.


> + *
> + * 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 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 <device/device.h>
> +#include <device/pnp.h>
> +#include <uart8250.h>
> +#include "chip.h"
> +#include "i3100.h"
> +#include <arch/io.h>
> +
> +static void pnp_enter_ext_func_mode(device_t dev)
> +{
> +	outb(0x80, dev->path.u.pnp.port);
> +	outb(0x86, dev->path.u.pnp.port);
> +}
> +
> +static void pnp_exit_ext_func_mode(device_t dev)
> +{
> +	outb(0x68, dev->path.u.pnp.port);
> +	outb(0x08, dev->path.u.pnp.port);
> +}
> +
> +static void i3100_init(device_t dev)
> +{
> +	struct superio_intel_i3100_config *conf;
> +	struct resource *res0;
> +
> +	if (!dev->enabled) {
> +		return;
> +	}
> +
> +	conf = dev->chip_info;
> +
> +	switch (dev->path.u.pnp.device) {
> +	case I3100_SP1:
> +		res0 = find_resource(dev, PNP_IDX_IO0);
> +		init_uart8250(res0->base, &conf->com1);
> +		break;
> +	case I3100_SP2:
> +		res0 = find_resource(dev, PNP_IDX_IO0);
> +		init_uart8250(res0->base, &conf->com2);
> +		break;
> +	}
> +}
> +
> +static void i3100_pnp_set_resources(device_t dev)
> +{
> +	pnp_enter_ext_func_mode(dev);
> +	pnp_set_resources(dev);
> +	pnp_exit_ext_func_mode(dev);
> +}
> +
> +static void i3100_pnp_enable_resources(device_t dev)
> +{
> +	pnp_enter_ext_func_mode(dev);
> +	pnp_enable_resources(dev);
> +	pnp_exit_ext_func_mode(dev);
> +}
> +
> +static void i3100_pnp_enable(device_t dev)
> +{
> +	pnp_enter_ext_func_mode(dev);
> +	pnp_set_logical_device(dev);
> +	pnp_set_enable(dev, dev->enabled);
> +	pnp_exit_ext_func_mode(dev);
> +}
> +
> +static struct device_operations ops = {
> +	.read_resources   = pnp_read_resources,
> +	.set_resources    = i3100_pnp_set_resources,
> +	.enable_resources = i3100_pnp_enable_resources,
> +	.enable           = i3100_pnp_enable,
> +	.init             = i3100_init,
> +};
> +
> +static struct pnp_info pnp_dev_info[] = {
> +	{ &ops, I3100_SP1, PNP_IO0 | PNP_IRQ0, { 0x7f8, 0 }, },
> +	{ &ops, I3100_SP2, PNP_IO0 | PNP_IRQ0, { 0x7f8, 0 }, },
> +};
> +
> +static void enable_dev(struct device *dev)
> +{
> +	pnp_enable_devices(dev, &ops,
> +		sizeof(pnp_dev_info)/sizeof(pnp_dev_info[0]), pnp_dev_info);

Use ARRAY_SIZE here, please. It's in stdlib.h.


> +}
> +
> +struct chip_operations superio_intel_i3100_ops = {
> +	CHIP_NAME("Intel 3100 Super I/O")
> +	.enable_dev = enable_dev,
> +};
> +

Otherwise:
Acked-by: Uwe Hermann <uwe at hermann-uwe.de>


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