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

[Ed Swierk]

This patch implements support for the Intel 3100 integrated SuperIO and UART.

--Ed

[Peter Stuge]

On Thu, Mar 13, 2008 at 02:42:18PM -0700, Ed Swierk wrote:
> This patch implements support for the Intel 3100 integrated SuperIO and UART.

Would ack this too.

//Peter

[Ed Swierk]

On Thu, Mar 13, 2008 at 3:18 PM, Peter Stuge  wrote:
> Would ack this too.

Doh!

Signed-off-by: Ed Swierk

--Ed

[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
> Copyright (C) 2007 AMD
> Written by Yinghai Lu  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.

[Ed Swierk]

Here is an updated patch addressing Uwe's comments.

Signed-off-by: Ed Swierk

--Ed