[coreboot] patch: add in smbus support for mcp55
Carl-Daniel Hailfinger
c-d.hailfinger.devel.2006 at gmx.net
Wed Aug 6 13:34:45 CEST 2008
On 06.08.2008 02:23, ron minnich wrote:
> This patch adds in a bit more support, this is the mcp55 smbus.
>
> This also illustrates the need to figure out the pci stuff in a better
> way. It works for now, but we need to do better.
>
> But we'll figure out it once we have a full port under our belts, and
> we can come up with something that works right. We're still
> getting over the v2 practice of including .c files :-) [Which, lest we
> forget, was the best way to do things given that we did NOT have
> cache as ram -- I intend no criticism at all!]
>
> Signed-off-by: Ronald G. Minnich <rminnich at gmail.com>
>
A few comments below. In general, using printk more often would be nice
since we're not constrained by romcc anymore. IMO we also should settle
for a delay mechanism which doesn't clobber POST codes.
> Index: southbridge/nvidia/mcp55/mcp55_smbus.h
> ===================================================================
> --- southbridge/nvidia/mcp55/mcp55_smbus.h (revision 0)
> +++ southbridge/nvidia/mcp55/mcp55_smbus.h (revision 0)
> @@ -0,0 +1,194 @@
> +/*
> + * This file is part of the coreboot project.
> + *
> + * Copyright (C) 2004 Tyan Computer
> + * Written by Yinghai Lu <yhlu at tyan.com> for Tyan Computer.
> + * Copyright (C) 2006,2007 AMD
> + * Written by Yinghai Lu <yinghai.lu at amd.com> for AMD.
> + *
> + * 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/smbus_def.h>
> +
> +#define SMBHSTSTAT 0x1
> +#define SMBHSTPRTCL 0x0
> +#define SMBHSTCMD 0x3
> +#define SMBXMITADD 0x2
> +#define SMBHSTDAT0 0x4
> +#define SMBHSTDAT1 0x5
> +
> +/* Between 1-10 seconds, We should never timeout normally
> + * Longer than this is just painful when a timeout condition occurs.
> + */
> +#define SMBUS_TIMEOUT (100*1000*10)
> +
> +static inline void smbus_delay(void)
> +{
> + outb(0x80, 0x80);
> +}
Can we use inb(0x80) or outb(0x80,0x84) instead? That way, we won't mess
us POST code logging.
> +
> +static int smbus_wait_until_ready(unsigned smbus_io_base)
Can we use explicit length types like u16 instead of unsigned? That
applies to almost all functions.
> +{
> + unsigned long loops;
> + loops = SMBUS_TIMEOUT;
> + do {
> + unsigned char val;
> + smbus_delay();
> + val = inb(smbus_io_base + SMBHSTSTAT);
> + val &= 0x1f;
> + if (val == 0) {
> + return 0;
> + }
> + outb(val,smbus_io_base + SMBHSTSTAT);
> + } while(--loops);
> + return -2;
The -2 is a magic value. A #define would be appreciated.
> +}
> +
> +static int smbus_wait_until_done(unsigned smbus_io_base)
u16
> +{
> + unsigned long loops;
> + loops = SMBUS_TIMEOUT;
> + do {
> + unsigned char val;
> + smbus_delay();
> +
> + val = inb(smbus_io_base + SMBHSTSTAT);
> + if ( (val & 0xff) != 0) {
> + return 0;
> + }
> + } while(--loops);
> + return -3;
Magic value.
> +}
Missing empty line between functions.
> +static int do_smbus_recv_byte(unsigned smbus_io_base, unsigned device)
u16/u32
> +{
> + unsigned char global_status_register;
> + unsigned char byte;
> +
> + /* set the device I'm talking too */
> + outb(((device & 0x7f) << 1)|1 , smbus_io_base + SMBXMITADD);
> + smbus_delay();
> +
> + /* byte data recv */
> + outb(0x05, smbus_io_base + SMBHSTPRTCL);
> + smbus_delay();
> +
> + /* poll for transaction completion */
> + if (smbus_wait_until_done(smbus_io_base) < 0) {
> + return -3;
Magic value.
> + }
> +
> + global_status_register = inb(smbus_io_base + SMBHSTSTAT) & 0x80; /* lose check */
Do we want to lose a check here or do we want to check loosely?
> +
> + /* read results of transaction */
> + byte = inb(smbus_io_base + SMBHSTCMD);
> +
> + if (global_status_register != 0x80) { // lose check, otherwise it should be 0
> + return -1;
Magic value.
> + }
> + return byte;
> +}
Missing empty line between functions.
> +static int do_smbus_send_byte(unsigned smbus_io_base, unsigned device, unsigned char val)
u16/u32
> +{
> + unsigned global_status_register;
> +
> + outb(val, smbus_io_base + SMBHSTDAT0);
> + smbus_delay();
> +
> + /* set the command... */
> + outb(val, smbus_io_base + SMBHSTCMD);
> + smbus_delay();
> +
> + /* set the device I'm talking too */
> + outb(((device & 0x7f) << 1) | 0, smbus_io_base + SMBXMITADD);
> + smbus_delay();
> +
> + /* set up for a byte data write */
> + outb(0x04, smbus_io_base + SMBHSTPRTCL);
> + smbus_delay();
> +
> + /* poll for transaction completion */
> + if (smbus_wait_until_done(smbus_io_base) < 0) {
> + return -3;
Magic value.
> + }
> + global_status_register = inb(smbus_io_base + SMBHSTSTAT) & 0x80; /* lose check */;
> +
> + if (global_status_register != 0x80) {
> + return -1;
Magic value.
> + }
> + return 0;
> +}
Missing empty line between functions.
> +static int do_smbus_read_byte(unsigned smbus_io_base, unsigned device, unsigned address)
u16/u32/u8
> +{
> + unsigned char global_status_register;
> + unsigned char byte;
> +
> + /* set the device I'm talking too */
> + outb(((device & 0x7f) << 1)|1 , smbus_io_base + SMBXMITADD);
> + smbus_delay();
> + /* set the command/address... */
> + outb(address & 0xff, smbus_io_base + SMBHSTCMD);
> + smbus_delay();
> + /* byte data read */
> + outb(0x07, smbus_io_base + SMBHSTPRTCL);
> + smbus_delay();
> +
> + /* poll for transaction completion */
> + if (smbus_wait_until_done(smbus_io_base) < 0) {
> + return -3;
> + }
> +
> + global_status_register = inb(smbus_io_base + SMBHSTSTAT) & 0x80; /* lose check */
> +
> + /* read results of transaction */
> + byte = inb(smbus_io_base + SMBHSTDAT0);
> +
> + if (global_status_register != 0x80) { // lose check, otherwise it should be 0
> + return -1;
> + }
> + return byte;
> +}
> +
> +
Two empty lines.
> +static int do_smbus_write_byte(unsigned smbus_io_base, unsigned device, unsigned address, unsigned char val)
u16/u32/u8
> +{
> + unsigned global_status_register;
> +
> + outb(val, smbus_io_base + SMBHSTDAT0);
> + smbus_delay();
> +
> + /* set the device I'm talking too */
> + outb(((device & 0x7f) << 1) | 0, smbus_io_base + SMBXMITADD);
> + smbus_delay();
> +
> + outb(address & 0xff, smbus_io_base + SMBHSTCMD);
> + smbus_delay();
> +
> + /* set up for a byte data write */
> + outb(0x06, smbus_io_base + SMBHSTPRTCL);
> + smbus_delay();
> +
> + /* poll for transaction completion */
> + if (smbus_wait_until_done(smbus_io_base) < 0) {
> + return -3;
> + }
> + global_status_register = inb(smbus_io_base + SMBHSTSTAT) & 0x80; /* lose check */;
> +
> + if (global_status_register != 0x80) {
> + return -1;
> + }
> + return 0;
> +}
> +
> Index: southbridge/nvidia/mcp55/stage1_smbus.c
> ===================================================================
> --- southbridge/nvidia/mcp55/stage1_smbus.c (revision 0)
> +++ southbridge/nvidia/mcp55/stage1_smbus.c (revision 0)
> @@ -0,0 +1,93 @@
> +/*
> + * This file is part of the coreboot project.
> + *
> + * Copyright (C) 2004 Tyan Computer
> + * Written by Yinghai Lu <yhlu at tyan.com> for Tyan Computer.
> + * Copyright (C) 2006,2007 AMD
> + * Written by Yinghai Lu <yinghai.lu at amd.com> for AMD.
> + *
> + * 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 <console.h>
> +#include <io.h>
> +#include <device/device.h>
> +#include <device/pci.h>
> +#include "mcp55.h"
> +#define pci_read_config32(bus, dev, where) pci_cf8_conf1.read32(NULL, bus, dev, where)
> +#define pci_write_config32(bus, dev, where, what) pci_cf8_conf1.write32(NULL, bus, dev, where, what)
> +#define pci_read_config16(bus, dev, where) pci_cf8_conf1.read32(NULL, bus, dev, where)
> +#define pci_write_config16(bus, dev, where, what) pci_cf8_conf1.write32(NULL, bus, dev, where, what)
> +#include "mcp55_smbus.h"
> +
> +#define SMBUS0_IO_BASE 0x1000
> +#define SMBUS1_IO_BASE (0x1000+(1<<8))
> +/*SIZE 0x40 */
> +
> +static void enable_smbus(void)
> +{
> + u32 dev;
> + dev = pci_locate_device(PCI_ID(0x10de, 0x0368), 0);
> +#if 0
> + if (dev == PCI_DEV_INVALID) {
> + die("SMBUS controller not found\r\n");
> + }
> +
> + print_debug("SMBus controller enabled\r\n");
printk
> +#endif
> + /* set smbus iobase */
> + pci_write_config32(0, dev, 0x20, SMBUS0_IO_BASE | 1);
> + pci_write_config32(0, dev, 0x24, SMBUS1_IO_BASE | 1);
> + /* Set smbus iospace enable */
> + pci_write_config16(0, dev, 0x4, 0x01);
> + /* clear any lingering errors, so the transaction will run */
> + outb(inb(SMBUS0_IO_BASE + SMBHSTSTAT), SMBUS0_IO_BASE + SMBHSTSTAT);
> + outb(inb(SMBUS1_IO_BASE + SMBHSTSTAT), SMBUS1_IO_BASE + SMBHSTSTAT);
> +}
> +
> +static int smbus_recv_byte(unsigned device)
u32?
> +{
> + return do_smbus_recv_byte(SMBUS0_IO_BASE, device);
> +}
Missing empty line between functions.
> +static int smbus_send_byte(unsigned device, unsigned char val)
u32?
> +{
> + return do_smbus_send_byte(SMBUS0_IO_BASE, device, val);
> +}
Missing empty line between functions.
> +static int smbus_read_byte(unsigned device, unsigned address)
u32?
> +{
> + return do_smbus_read_byte(SMBUS0_IO_BASE, device, address);
> +}
Missing empty line between functions.
> +static int smbus_write_byte(unsigned device, unsigned address, unsigned char val)
u32/u16?
> +{
> + return do_smbus_write_byte(SMBUS0_IO_BASE, device, address, val);
> +}
> +
> +static int smbusx_recv_byte(unsigned smb_index, unsigned device)
u8/u32?
> +{
> + return do_smbus_recv_byte(SMBUS0_IO_BASE + (smb_index<<8), device);
> +}
Missing empty line between functions.
> +static int smbusx_send_byte(unsigned smb_index, unsigned device, unsigned char val)
> +{
> + return do_smbus_send_byte(SMBUS0_IO_BASE + (smb_index<<8), device, val);
> +}
Missing empty line between functions.
> +static int smbusx_read_byte(unsigned smb_index, unsigned device, unsigned address)
> +{
> + return do_smbus_read_byte(SMBUS0_IO_BASE + (smb_index<<8), device, address);
> +}
Missing empty line between functions.
> +static int smbusx_write_byte(unsigned smb_index, unsigned device, unsigned address, unsigned char val)
> +{
> + return do_smbus_write_byte(SMBUS0_IO_BASE + (smb_index<<8), device, address, val);
> +}
> +
> Index: mainboard/gigabyte/m57sli/Makefile
> ===================================================================
> --- mainboard/gigabyte/m57sli/Makefile (revision 719)
> +++ mainboard/gigabyte/m57sli/Makefile (working copy)
> @@ -20,7 +20,8 @@
> ##
>
> STAGE0_MAINBOARD_OBJ := $(obj)/mainboard/$(MAINBOARDDIR)/stage1.o \
> - $(obj)/mainboard/$(MAINBOARDDIR)/option_table.c
> + $(obj)/mainboard/$(MAINBOARDDIR)/option_table.c \
> + $(obj)/southbridge/nvidia/mcp55/stage1_smbus.o
>
> INITRAM_SRC = $(src)/mainboard/$(MAINBOARDDIR)/initram.c
>
Regards,
Carl-Daniel
--
http://www.hailfinger.org/
More information about the coreboot
mailing list