[coreboot] Super IO: Winbond W83697HF Support
Uwe Hermann
uwe at hermann-uwe.de
Fri Jul 18 15:12:33 CEST 2008
Hi,
thanks for you patch!
On Thu, Jul 17, 2008 at 07:51:56PM -0700, Sean Nelson wrote:
> Here's a patch for coreboot v2 for Winbond W83697HF Support.
>
> Patch Attached.
Please add a Signed-off-by to every patch, otherwise we cannot commit,
see http://www.coreboot.org/Development_Guidelines#Sign-off_Procedure.
Quick review below.
> Index: src/superio/winbond/w83697hf/w83697hf_early_init.c
> ===================================================================
> --- src/superio/winbond/w83697hf/w83697hf_early_init.c (revision 0)
> +++ src/superio/winbond/w83697hf/w83697hf_early_init.c (revision 0)
> @@ -0,0 +1,35 @@
> +/*
> + * This file is part of the coreboot project.
> + *
> + * Copyright (C) 2008 Sean Nelson <snelson at nmt.edu>
> + *
> + * 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 "w83697hf.h"
> +
> +static void w83697hf_disable_dev(device_t dev)
> +{
> + pnp_set_logical_device(dev);
> + pnp_set_enable(dev, 0);
> +}
> +static void w83697hf_enable_dev(device_t dev, unsigned iobase)
> +{
> + pnp_set_logical_device(dev);
> + pnp_set_enable(dev, 0);
> + pnp_set_iobase(dev, PNP_IDX_IO0, iobase);
> + pnp_set_enable(dev, 1);
> +}
This file will probably not be required in the usual case. I'd drop
it for now unless you explicitly need it or use it in practice.
> Index: src/superio/winbond/w83697hf/superio.c
> ===================================================================
> --- src/superio/winbond/w83697hf/superio.c (revision 0)
> +++ src/superio/winbond/w83697hf/superio.c (revision 0)
> @@ -0,0 +1,205 @@
> +/*
> + * This file is part of the coreboot project.
> + *
> + * Copyright (C) 2008 Sean Nelson <snelson at nmt.edu>
> + *
> + * 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/io.h>
> +#include <device/device.h>
> +#include <device/pnp.h>
> +#include <console/console.h>
> +#include <string.h>
> +#include <bitops.h>
> +#include <uart8250.h>
> +#include <pc80/keyboard.h>
See above, keyboard.h is likely not needed.
> +#include <pc80/mc146818rtc.h>
> +#include "chip.h"
> +#include "w83697hf.h"
> +
> +
> +static void pnp_enter_ext_func_mode(device_t dev)
> +{
> + outb(0x87, dev->path.u.pnp.port);
> + outb(0x87, dev->path.u.pnp.port);
> +}
> +static void pnp_exit_ext_func_mode(device_t dev)
> +{
> + outb(0xaa, dev->path.u.pnp.port);
> +}
> +
> +static void pnp_write_index(unsigned long port_base, uint8_t reg, uint8_t value)
> +{
> + outb(reg, port_base);
> + outb(value, port_base + 1);
> +}
> +
> +static uint8_t pnp_read_index(unsigned long port_base, uint8_t reg)
> +{
> + outb(reg, port_base);
> + return inb(port_base + 1);
> +}
These two are likely not needed if you drop the HWM code (see below).
> +
> +static void enable_hwm_smbus(device_t dev) {
> + /* set the pin 91,92 as I2C bus */
> + uint8_t reg, value;
> + reg = 0x2b;
> + value = pnp_read_config(dev, reg);
> + value &= 0x3f;
> + pnp_write_config(dev, reg, value);
> +}
This will not work on this Super I/O, there's no 0x2b register.
I suggest to drop HWM init completely for now and/or (if needed)
fix this with another patch when it is tested on hardware...
> +static void init_acpi(device_t dev)
> +{
> + uint8_t value = 0x20;
> + int power_on = 1;
> +
> + get_option(&power_on, "power_on_after_fail");
> + pnp_enter_ext_func_mode(dev);
> + pnp_write_index(dev->path.u.pnp.port,7,0x0a);
> + value = pnp_read_config(dev, 0xE4);
> + value &= ~(3<<5);
> + if(power_on) {
> + value |= (1<<5);
> + }
> + pnp_write_config(dev, 0xE4, value);
> + pnp_exit_ext_func_mode(dev);
> +}
Can also be dropped, there's no such feature on this Super I/O it seems,
and there's no 0xe4 register on LDN 0xa (ACPI).
> +static void init_hwm(unsigned long base)
> +{
> + uint8_t reg, value;
> + int i;
> +
> + unsigned hwm_reg_values[] = {
> +/* reg mask data */
> + 0x40, 0xff, 0x81, /* start HWM */
> + //0x48, 0xaa, 0x2a, /* set SMBus base to 0x54>>1 */
> + //0x4a, 0x21, 0x21, /* set T2 SMBus base to 0x92>>1 and T3 SMBus base to 0x94>>1 */
> + 0x4e, 0x80, 0x00,
> + 0x43, 0x00, 0xfd, /* disable some SMI interrupts */
> + 0x44, 0x00, 0x17, /* disable more SMI interrupts */
> + 0x4c, 0xbf, 0x04
> + //0x4d, 0xff, 0x80 /* turn off beep */
> +
> + };
> +
> + for(i = 0; i< sizeof(hwm_reg_values)/sizeof(hwm_reg_values[0]); i+=3 ) {
> + reg = hwm_reg_values[i];
> + value = pnp_read_index(base, reg);
> + value &= 0xff & hwm_reg_values[i+1];
> + value |= 0xff & hwm_reg_values[i+2];
> +#if 0
> + printk_debug("base = 0x%04x, reg = 0x%02x, value = 0x%02x\r\n", base, reg,value);
> +#endif
> + pnp_write_index(base, reg, value);
> + }
> +}
Drop this, see above.
> +static void w83697hf_init(device_t dev)
> +{
> + struct superio_winbond_w83697hf_config *conf;
> + struct resource *res0, *res1;
> + if (!dev->enabled) {
> + return;
> + }
> + conf = dev->chip_info;
> + switch(dev->path.u.pnp.device) {
> + case W83697HF_SP1:
> + res0 = find_resource(dev, PNP_IDX_IO0);
> + init_uart8250(res0->base, &conf->com1);
> + break;
> + case W83697HF_SP2:
> + res0 = find_resource(dev, PNP_IDX_IO0);
> + init_uart8250(res0->base, &conf->com2);
> + break;
This is ok...
> + case W83697HF_HWM:
> + res0 = find_resource(dev, PNP_IDX_IO0);
> +#define HWM_INDEX_PORT 5
> + init_hwm(res0->base + HWM_INDEX_PORT);
> + break;
> + //case W83697HF_ACPI:
> + // init_acpi(dev);
> + // break;
... these should be dropped.
> + }
> +}
> +
> +void w83697hf_pnp_set_resources(device_t dev)
> +{
> + pnp_enter_ext_func_mode(dev);
> + pnp_set_resources(dev);
> + pnp_exit_ext_func_mode(dev);
> +}
> +
> +void w83697hf_pnp_enable_resources(device_t dev)
> +{
> + pnp_enter_ext_func_mode(dev);
> + pnp_enable_resources(dev);
> + switch(dev->path.u.pnp.device) {
> + case W83697HF_HWM:
> + printk_debug("w83697hf hwm smbus enabled\n");
> + enable_hwm_smbus(dev);
> + break;
> + }
Drop.
> + pnp_exit_ext_func_mode(dev);
> +}
> +
> +void w83697hf_pnp_enable(device_t dev)
> +{
> +
> + if (!dev->enabled) {
> + pnp_enter_ext_func_mode(dev);
> +
> + pnp_set_logical_device(dev);
> + pnp_set_enable(dev, 0);
> +
> + pnp_exit_ext_func_mode(dev);
> + }
> +}
> +
> +static struct device_operations ops = {
> + .read_resources = pnp_read_resources,
> + .set_resources = w83697hf_pnp_set_resources,
> + .enable_resources = pnp_enable_resources,
> + .enable = w83697hf_pnp_enable,
> + .init = w83697hf_init,
> +};
> +
> +static struct pnp_info pnp_dev_info[] = {
> + { &ops, W83697HF_FDC, PNP_IO0 | PNP_IRQ0 | PNP_DRQ0, { 0x07f8, 0}, },
> + { &ops, W83697HF_PP, PNP_IO0 | PNP_IRQ0 | PNP_DRQ0, { 0x07f8, 0}, },
> + { &ops, W83697HF_SP1, PNP_IO0 | PNP_IRQ0, { 0x7f8, 0 }, },
> + { &ops, W83697HF_SP2, PNP_IO0 | PNP_IRQ0, { 0x7f8, 0 }, },
> + // No 4 { 0,},
> + // No 5 { 0,},
> + { &ops, W83697HF_CIR, PNP_IO0 | PNP_IRQ0, { 0x7f8, 0 }, },
> + { &ops, W83697HF_GAME_GPIO1, PNP_IO0 | PNP_IO1 | PNP_IRQ0, { 0x7ff, 0 }, {0x7fe, 0x4}, },
> + { &ops, W83697HF_MIDI_GPIO5, },
> + { &ops, W83697HF_GPIO2, },
> + { &ops, W83697HF_ACPI, },
> + { &ops, W83697HF_HWM, PNP_IO0 | PNP_IRQ0, { 0xff8, 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 (stdlib.h).
> +}
> +
> +struct chip_operations superio_winbond_w83697hf_ops = {
> + CHIP_NAME("Winbond W83697HF Super I/O")
> + .enable_dev = enable_dev,
> +};
> Index: src/superio/winbond/w83697hf/chip.h
> ===================================================================
> --- src/superio/winbond/w83697hf/chip.h (revision 0)
> +++ src/superio/winbond/w83697hf/chip.h (revision 0)
> @@ -0,0 +1,28 @@
> +/*
> + * This file is part of the coreboot project.
> + *
> + * Copyright (C) 2008 Sean Nelson <snelson at nmt.edu>
> + *
> + * 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 <pc80/keyboard.h>
This chip doesn't seem to have keyboard support so this is #include is
not needed.
> +#include <uart8250.h>
> +
> +extern struct chip_operations superio_winbond_w83697hf_ops;
> +
> +struct superio_winbond_w83697hf_config {
> + struct uart8250 com1, com2;
> +};
> Index: src/superio/winbond/w83697hf/w83697hf.h
> ===================================================================
> --- src/superio/winbond/w83697hf/w83697hf.h (revision 0)
> +++ src/superio/winbond/w83697hf/w83697hf.h (revision 0)
> @@ -0,0 +1,30 @@
> +/*
> + * This file is part of the coreboot project.
> + *
> + * Copyright (C) 2008 Sean Nelson <snelson at nmt.edu>
> + *
> + * 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
> + */
> +
> +#define W83697HF_FDC 0 /* Floppy */
> +#define W83697HF_PP 1 /* Parallel Port */
> +#define W83697HF_SP1 2 /* Com1 */
> +#define W83697HF_SP2 3 /* Com2 */
> +#define W83697HF_CIR 6
> +#define W83697HF_GAME_GPIO1 7
> +#define W83697HF_MIDI_GPIO5 8
> +#define W83697HF_GPIO2 9
Maybe W83697HF_GPIO234, as this is for GPIOs 2-4.
> +#define W83697HF_ACPI 10
> +#define W83697HF_HWM 11 /* Hardware Monitor */
HTH, 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