Author: uwe Date: Sun Nov 7 00:36:49 2010 New Revision: 6030 URL: https://tracker.coreboot.org/trac/coreboot/changeset/6030
Log: Various Super I/O fixes and corrections.
- VIA VT1211: - Add missing LDNs and respective code to handle them. - Add some TODOs for other stuff that needs fixing. - Use VT1211_SP1 instead of hardcoding the LDN number (2). - Fixup pnp_dev_info[] as per datasheet, but some TODOs remain. - Various coding style fixes and changes to u8/u16/etc.
- Serverengines Pilot: Various coding style fixes and changes to u8/u16/etc.
Signed-off-by: Uwe Hermann uwe@hermann-uwe.de Acked-by: Uwe Hermann uwe@hermann-uwe.de
Modified: trunk/src/superio/renesas/m3885x/superio.c trunk/src/superio/serverengines/pilot/pilot.h trunk/src/superio/serverengines/pilot/pilot_early_init.c trunk/src/superio/serverengines/pilot/pilot_early_serial.c trunk/src/superio/via/vt1211/Makefile.inc trunk/src/superio/via/vt1211/chip.h trunk/src/superio/via/vt1211/vt1211.c trunk/src/superio/via/vt1211/vt1211.h
Modified: trunk/src/superio/renesas/m3885x/superio.c ============================================================================== --- trunk/src/superio/renesas/m3885x/superio.c Sat Nov 6 01:57:19 2010 (r6029) +++ trunk/src/superio/renesas/m3885x/superio.c Sun Nov 7 00:36:49 2010 (r6030) @@ -44,15 +44,14 @@ m3885_configure_multikey(); }
- static void m3885x_read_resources(device_t dev) { - // nothing, but this function avoids an error on serial console. + /* Nothing, but this function avoids an error on serial console. */ }
static void m3885x_enable_resources(device_t dev) { - // nothing, but this function avoids an error on serial console. + /* Nothing, but this function avoids an error on serial console. */ }
static struct device_operations ops = { @@ -74,5 +73,3 @@ CHIP_NAME("Renesas M3885x Super I/O") .enable_dev = enable_dev, }; - -
Modified: trunk/src/superio/serverengines/pilot/pilot.h ============================================================================== --- trunk/src/superio/serverengines/pilot/pilot.h Sat Nov 6 01:57:19 2010 (r6029) +++ trunk/src/superio/serverengines/pilot/pilot.h Sun Nov 7 00:36:49 2010 (r6030) @@ -21,8 +21,8 @@
/* PILOT Super I/O is only based on LPC observation done on factory system. */
-#define PILOT_SP1 0x02 /* Com1 */ #define PILOT_LD1 0x01 /* Logical device 1 */ +#define PILOT_SP1 0x02 /* Com1 */ #define PILOT_LD4 0x04 /* Logical device 4 */ #define PILOT_LD5 0x05 /* Logical device 5 */ #define PILOT_LD7 0x07 /* Logical device 7 */
Modified: trunk/src/superio/serverengines/pilot/pilot_early_init.c ============================================================================== --- trunk/src/superio/serverengines/pilot/pilot_early_init.c Sat Nov 6 01:57:19 2010 (r6029) +++ trunk/src/superio/serverengines/pilot/pilot_early_init.c Sun Nov 7 00:36:49 2010 (r6030) @@ -29,7 +29,7 @@ */ static void pilot_early_init(device_t dev) { - unsigned port = dev >> 8; + u16 port = dev >> 8;
print_debug("Using port: "); print_debug_hex16(port); @@ -55,6 +55,7 @@ pnp_set_enable(PNP_DEV(port, 0x3), 0); pnp_exit_ext_func_mode(dev); */ + pnp_enter_ext_func_mode(dev); pnp_set_logical_device(PNP_DEV(port, 0x4)); pnp_exit_ext_func_mode(dev); @@ -95,6 +96,7 @@ pnp_enter_ext_func_mode(dev); pnp_set_enable(PNP_DEV(port, 0x7), 0); pnp_exit_ext_func_mode(dev); + /* pnp_enter_ext_func_mode(dev); pnp_set_logical_device(PNP_DEV(port, 0x8));
Modified: trunk/src/superio/serverengines/pilot/pilot_early_serial.c ============================================================================== --- trunk/src/superio/serverengines/pilot/pilot_early_serial.c Sat Nov 6 01:57:19 2010 (r6029) +++ trunk/src/superio/serverengines/pilot/pilot_early_serial.c Sun Nov 7 00:36:49 2010 (r6030) @@ -25,20 +25,20 @@ #include "pilot.h"
/* Pilot uses 0x5A/0xA5 pattern to actiavte deactivate config access. */ -static inline void pnp_enter_ext_func_mode(device_t dev) +static void pnp_enter_ext_func_mode(device_t dev) { - unsigned port = dev >> 8; + u16 port = dev >> 8; outb(0x5A, port); }
static void pnp_exit_ext_func_mode(device_t dev) { - unsigned port = dev >> 8; + u16 port = dev >> 8; outb(0xA5, port); }
/* Serial config is a fairly standard procedure. */ -static void pilot_enable_serial(device_t dev, unsigned iobase) +static void pilot_enable_serial(device_t dev, u16 iobase) { pnp_enter_ext_func_mode(dev); pnp_set_logical_device(dev); @@ -51,7 +51,7 @@ { pnp_enter_ext_func_mode(dev); pnp_set_logical_device(dev); - pnp_set_iobase(dev, PNP_IDX_IO0, 0x00); + pnp_set_iobase(dev, PNP_IDX_IO0, 0x0000); pnp_set_enable(dev, 0); pnp_exit_ext_func_mode(dev); }
Modified: trunk/src/superio/via/vt1211/Makefile.inc ============================================================================== --- trunk/src/superio/via/vt1211/Makefile.inc Sat Nov 6 01:57:19 2010 (r6029) +++ trunk/src/superio/via/vt1211/Makefile.inc Sun Nov 7 00:36:49 2010 (r6030) @@ -18,5 +18,5 @@ ## Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA ##
-#config chip.h ramstage-$(CONFIG_SUPERIO_VIA_VT1211) += vt1211.c +
Modified: trunk/src/superio/via/vt1211/chip.h ============================================================================== --- trunk/src/superio/via/vt1211/chip.h Sat Nov 6 01:57:19 2010 (r6029) +++ trunk/src/superio/via/vt1211/chip.h Sun Nov 7 00:36:49 2010 (r6030) @@ -18,8 +18,8 @@ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA */
-#ifndef _SUPERIO_VIA_VT1211 -#define _SUPERIO_VIA_VT1211 +#ifndef SUPERIO_VIA_VT1211_CHIP_H +#define SUPERIO_VIA_VT1211_CHIP_H
#include <uart8250.h>
@@ -29,4 +29,4 @@ struct uart8250 com1, com2; };
-#endif /* _SUPERIO_VIA_VT1211 */ +#endif
Modified: trunk/src/superio/via/vt1211/vt1211.c ============================================================================== --- trunk/src/superio/via/vt1211/vt1211.c Sat Nov 6 01:57:19 2010 (r6029) +++ trunk/src/superio/via/vt1211/vt1211.c Sun Nov 7 00:36:49 2010 (r6030) @@ -19,33 +19,30 @@ * MA 02110-1301 USA */
- /* vt1211 routines and defines*/ - #include <arch/io.h> #include <console/console.h> #include <device/device.h> #include <device/pnp.h> #include <uart8250.h> #include <stdlib.h> - #include "vt1211.h" #include "chip.h"
-static unsigned char vt1211hwmonitorinits[]={ - 0x10,0x3, 0x11,0x10, 0x12,0xd, 0x13,0x7f, +static u8 hwm_io_regs[] = { + 0x10,0x03, 0x11,0x10, 0x12,0x0d, 0x13,0x7f, 0x14,0x21, 0x15,0x81, 0x16,0xbd, 0x17,0x8a, - 0x18,0x0, 0x19,0x0, 0x1a,0x0, 0x1b,0x0, - 0x1d,0xff, 0x1e,0x0, 0x1f,0x73, 0x20,0x67, + 0x18,0x00, 0x19,0x00, 0x1a,0x00, 0x1b,0x00, + 0x1d,0xff, 0x1e,0x00, 0x1f,0x73, 0x20,0x67, 0x21,0xc1, 0x22,0xca, 0x23,0x74, 0x24,0xc2, - 0x25,0xc7, 0x26,0xc9, 0x27,0x7f, 0x29,0x0, - 0x2a,0x0, 0x2b,0xff, 0x2c,0x0, 0x2d,0xff, - 0x2e,0x0, 0x2f,0xff, 0x30,0x0, 0x31,0xff, - 0x32,0x0, 0x33,0xff, 0x34,0x0, 0x39,0xff, - 0x3a,0x0, 0x3b,0xff, 0x3c,0xff, 0x3d,0xff, - 0x3e,0x0, 0x3f,0xb0, 0x43,0xff, 0x44,0xff, - 0x46,0xff, 0x47,0x50, 0x4a,0x3, 0x4b,0xc0, - 0x4c,0x0, 0x4d,0x0, 0x4e,0xf, 0x5d,0x77, - 0x5c,0x0, 0x5f,0x33, 0x40,0x1 + 0x25,0xc7, 0x26,0xc9, 0x27,0x7f, 0x29,0x00, + 0x2a,0x00, 0x2b,0xff, 0x2c,0x00, 0x2d,0xff, + 0x2e,0x00, 0x2f,0xff, 0x30,0x00, 0x31,0xff, + 0x32,0x00, 0x33,0xff, 0x34,0x00, 0x39,0xff, + 0x3a,0x00, 0x3b,0xff, 0x3c,0xff, 0x3d,0xff, + 0x3e,0x00, 0x3f,0xb0, 0x43,0xff, 0x44,0xff, + 0x46,0xff, 0x47,0x50, 0x4a,0x03, 0x4b,0xc0, + 0x4c,0x00, 0x4d,0x00, 0x4e,0x0f, 0x5d,0x77, + 0x5c,0x00, 0x5f,0x33, 0x40,0x01, };
static void pnp_enter_ext_func_mode(device_t dev) @@ -59,32 +56,39 @@ outb(0xaa, dev->path.pnp.port); }
-static void vt1211_set_iobase(device_t dev, unsigned index, unsigned iobase) +static void vt1211_set_iobase(device_t dev, u8 index, u16 iobase) { switch (dev->path.pnp.device) { - case VT1211_FDC: - case VT1211_PP: - case VT1211_SP1: - case VT1211_SP2: - pnp_write_config(dev, index + 0, (iobase >> 2) & 0xff); - break; - case VT1211_HWM: - default: - pnp_write_config(dev, index + 0, (iobase >> 8) & 0xff); - pnp_write_config(dev, index + 1, iobase & 0xff); - break; + case VT1211_FDC: + case VT1211_PP: + case VT1211_SP1: + case VT1211_SP2: + pnp_write_config(dev, index + 0, (iobase >> 2) & 0xff); + break; + case VT1211_ROM: + /* TODO: Error. VT1211_ROM doesn't have an I/O base. */ + break; + case VT1211_MIDI: + case VT1211_GAME: + case VT1211_GPIO: + case VT1211_WDG: + case VT1211_WUC: + case VT1211_HWM: + case VT1211_FIR: + default: + pnp_write_config(dev, index + 0, (iobase >> 8) & 0xff); + pnp_write_config(dev, index + 1, iobase & 0xff); + break; } }
-static void init_hwm(unsigned long base) +/* Initialize VT1211 hardware monitor registers, which are at 0xECXX. */ +static void init_hwm(u16 base) { int i;
- /* initialize vt1211 hardware monitor registers, which are at 0xECXX */ - for(i = 0; i < sizeof(vt1211hwmonitorinits); i += 2) { - outb(vt1211hwmonitorinits[i + 1], - base + vt1211hwmonitorinits[i]); - } + for (i = 0; i < sizeof(hwm_io_regs); i += 2) + outb(hwm_io_regs[i + 1], base + hwm_io_regs[i]); }
static void vt1211_init(struct device *dev) @@ -92,14 +96,10 @@ struct superio_via_vt1211_config *conf = dev->chip_info; struct resource *res0;
- if (!dev->enabled) { + if (!dev->enabled) return; - }
switch (dev->path.pnp.device) { - case VT1211_FDC: - case VT1211_PP: - break; case VT1211_SP1: res0 = find_resource(dev, PNP_IDX_IO0); init_uart8250(res0->base, &conf->com1); @@ -112,14 +112,25 @@ res0 = find_resource(dev, PNP_IDX_IO0); init_hwm(res0->base); break; + case VT1211_FDC: + case VT1211_PP: + case VT1211_MIDI: + case VT1211_GAME: + case VT1211_GPIO: + case VT1211_WDG: + case VT1211_WUC: + case VT1211_FIR: + case VT1211_ROM: + /* TODO: Any init needed for these LDNs? */ + break; default: - printk(BIOS_INFO, "vt1211 asked to initialise unknown device!\n"); + printk(BIOS_INFO, "VT1211: Cannot init unknown device!\n"); } }
static void vt1211_pnp_enable_resources(device_t dev) { - printk(BIOS_DEBUG, "%s - enabling\n",dev_path(dev)); + printk(BIOS_DEBUG, "%s - enabling\n", dev_path(dev)); pnp_enter_ext_func_mode(dev); pnp_enable_resources(dev); pnp_exit_ext_func_mode(dev); @@ -130,41 +141,39 @@ struct resource *res;
#if CONFIG_CONSOLE_SERIAL8250 == 1 - if( dev->path.pnp.device == 2 ){ - for(res = dev->resource_list; res; res = res->next){ + /* TODO: Do the same for SP2? */ + if (dev->path.pnp.device == VT1211_SP1) { + for (res = dev->resource_list; res; res = res->next) { res->flags |= IORESOURCE_STORED; report_resource_stored(dev, res, ""); } return; } #endif + pnp_enter_ext_func_mode(dev); - /* Select the device */ + pnp_set_logical_device(dev);
/* Paranoia says I should disable the device here... */ - for(res = dev->resource_list; res; res = res->next){ + for (res = dev->resource_list; res; res = res->next) { if (!(res->flags & IORESOURCE_ASSIGNED)) { - printk(BIOS_ERR, "ERROR: %s %02lx %s size: 0x%010Lx not assigned\n", - dev_path(dev), res->index, - resource_type(res), - res->size); + printk(BIOS_ERR, "ERROR: %s %02lx %s size: 0x%010Lx " + "not assigned\n", dev_path(dev), res->index, + resource_type(res), res->size); continue; }
- /* Now store the resource */ + /* Now store the resource. */ if (res->flags & IORESOURCE_IO) { vt1211_set_iobase(dev, res->index, res->base); - } - else if (res->flags & IORESOURCE_DRQ) { + } else if (res->flags & IORESOURCE_DRQ) { pnp_set_drq(dev, res->index, res->base); - } - else if (res->flags & IORESOURCE_IRQ) { + } else if (res->flags & IORESOURCE_IRQ) { pnp_set_irq(dev, res->index, res->base); - } - else { - printk(BIOS_ERR, "ERROR: %s %02lx unknown resource type\n", - dev_path(dev), res->index); + } else { + printk(BIOS_ERR, "ERROR: %s %02lx unknown resource " + "type\n", dev_path(dev), res->index); return; } res->flags |= IORESOURCE_STORED; @@ -177,12 +186,13 @@
static void vt1211_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); - } + if (dev->enabled) + return; + + pnp_enter_ext_func_mode(dev); + pnp_set_logical_device(dev); + pnp_set_enable(dev, 0); + pnp_exit_ext_func_mode(dev); }
struct device_operations ops = { @@ -193,21 +203,25 @@ .init = vt1211_init, };
+/* TODO: Check if 0x07f8 is correct for FDC/PP/SP1/SP2, the rest is correct. */ static struct pnp_info pnp_dev_info[] = { - { &ops, VT1211_FDC, PNP_IO0 | PNP_IRQ0 | PNP_DRQ0, { 0x07f8, 0}, }, - { &ops, VT1211_PP, PNP_IO0 | PNP_IRQ0 | PNP_DRQ0, { 0x07f8, 0}, }, - { &ops, VT1211_SP1, PNP_IO0 | PNP_IRQ0, { 0x07f8, 0}, }, - { &ops, VT1211_SP2, PNP_IO0 | PNP_IRQ0, { 0x07f8, 0}, }, - { &ops, VT1211_HWM, PNP_IO0 , { 0xff00, 0 }, }, + { &ops, VT1211_FDC, PNP_IO0 | PNP_IRQ0 | PNP_DRQ0, { 0x07f8, 0 }, }, + { &ops, VT1211_PP, PNP_IO0 | PNP_IRQ0 | PNP_DRQ0, { 0x07f8, 0 }, }, + { &ops, VT1211_SP1, PNP_IO0 | PNP_IRQ0, { 0x07f8, 0 }, }, + { &ops, VT1211_SP2, PNP_IO0 | PNP_IRQ0, { 0x07f8, 0 }, }, + { &ops, VT1211_MIDI, PNP_IO0 | PNP_IRQ0, { 0xfffc, 0 }, }, + { &ops, VT1211_GAME, PNP_IO0, { 0xfff8, 0 }, }, + { &ops, VT1211_GPIO, PNP_IO0 | PNP_IRQ0, { 0xfff0, 0 }, }, + { &ops, VT1211_WDG, PNP_IO0 | PNP_IRQ0, { 0xfff0, 0 }, }, + { &ops, VT1211_WUC, PNP_IO0 | PNP_IRQ0, { 0xfff0, 0 }, }, + { &ops, VT1211_HWM, PNP_IO0 | PNP_IRQ0, { 0xff00, 0 }, }, + { &ops, VT1211_FIR, PNP_IO0 | PNP_IRQ0 | PNP_DRQ0, { 0xff00, 0 }, }, + { &ops, VT1211_ROM, }, };
static void enable_dev(struct device *dev) { - printk(BIOS_DEBUG, "vt1211 enabling PNP devices.\n"); - pnp_enable_devices(dev, - &ops, - ARRAY_SIZE(pnp_dev_info), - pnp_dev_info); + pnp_enable_devices(dev, &ops, ARRAY_SIZE(pnp_dev_info), pnp_dev_info); }
struct chip_operations superio_via_vt1211_ops = {
Modified: trunk/src/superio/via/vt1211/vt1211.h ============================================================================== --- trunk/src/superio/via/vt1211/vt1211.h Sat Nov 6 01:57:19 2010 (r6029) +++ trunk/src/superio/via/vt1211/vt1211.h Sun Nov 7 00:36:49 2010 (r6030) @@ -18,17 +18,16 @@ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA */
-/* vt1211 PNP devices */ - -#define VT1211_FDC 0 /* Floppy */ -#define VT1211_PP 1 /* Parallel Port */ -#define VT1211_SP1 2 /* COM1 */ -#define VT1211_SP2 3 /* COM2 */ -#define VT1211_MIDI 6 /* MIDI */ -#define VT1211_GAME 7 /* Game port */ -#define VT1211_GPIO 8 /* GPIO pins */ -#define VT1211_WATCHDOG 9 /* Watchdog timer */ -#define VT1211_WAKEUP 10 /* Wakeup control */ -#define VT1211_HWM 11 /* Hardware monitor */ -#define VT1211_FIR 12 /* Irda */ -#define VT1211_ROM 13 /* ROM control */ +/* Logical device numbers (LDNs). */ +#define VT1211_FDC 0x00 /* Floppy */ +#define VT1211_PP 0x01 /* Parallel port */ +#define VT1211_SP1 0x02 /* COM1 */ +#define VT1211_SP2 0x03 /* COM2 */ +#define VT1211_MIDI 0x06 /* MIDI */ +#define VT1211_GAME 0x07 /* Game port (GMP) */ +#define VT1211_GPIO 0x08 /* GPIO */ +#define VT1211_WDG 0x09 /* Watchdog timer (WDG) */ +#define VT1211_WUC 0x0a /* Wake-up control (WUC) */ +#define VT1211_HWM 0x0b /* Hardware monitor (HM) */ +#define VT1211_FIR 0x0c /* Very fast IR (VFIR/FIR) */ +#define VT1211_ROM 0x0d /* Flash ROM */
On 11/6/10 4:36 PM, repository service wrote:
Modified: trunk/src/superio/serverengines/pilot/pilot_early_init.c
--- trunk/src/superio/serverengines/pilot/pilot_early_init.c Sat Nov 6 01:57:19 2010 (r6029) +++ trunk/src/superio/serverengines/pilot/pilot_early_init.c Sun Nov 7 00:36:49 2010 (r6030) @@ -29,7 +29,7 @@ */ static void pilot_early_init(device_t dev) {
- unsigned port = dev >> 8;
u16 port = dev >> 8;
print_debug("Using port: "); print_debug_hex16(port);
@@ -55,6 +55,7 @@ pnp_set_enable(PNP_DEV(port, 0x3), 0); pnp_exit_ext_func_mode(dev); */
- pnp_enter_ext_func_mode(dev); pnp_set_logical_device(PNP_DEV(port, 0x4)); pnp_exit_ext_func_mode(dev);
@@ -95,6 +96,7 @@ pnp_enter_ext_func_mode(dev); pnp_set_enable(PNP_DEV(port, 0x7), 0); pnp_exit_ext_func_mode(dev);
/* pnp_enter_ext_func_mode(dev); pnp_set_logical_device(PNP_DEV(port, 0x8));
I know I have been one of the biggest advocators of this kind of change. However, I just recently got reminded that there are CPUs without I/O operations. On these the IO range is usually memory mapped. So this kind of trick with shifting dev and also assuming that a port I/O address fits in 16bit is not always possible. Now, who knows if any such system (except the dead alpha) exists that actually requires Super I/O drivers. However, we should keep this in mind for other code (like the UART stuff)
Stefan