[coreboot] patch: mcp55 smbus

Carl-Daniel Hailfinger c-d.hailfinger.devel.2006 at gmx.net
Mon Aug 11 23:12:21 CEST 2008


On 11.08.2008 19:19, ron minnich wrote:
> move things into stage0, make them shared, add new device.
>
> ron
>   
> mcp55 smbus support. Includes moving a bunch of functions to stage0 and making them SHARED. 
>
> Less code!
>
> Signed-off-by: Ronald G. Minnich <rminnich at gmail.com>

Reviewed below.
Acked-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>


> Index: southbridge/nvidia/mcp55/mcp55_smbus.h
> ===================================================================
> --- southbridge/nvidia/mcp55/mcp55_smbus.h	(revision 736)
> +++ southbridge/nvidia/mcp55/mcp55_smbus.h	(working copy)
> @@ -22,7 +22,9 @@
>   */
>  
>  #include <device/smbus_def.h>
> +#include <shared.h> /* We share symbols from stage 0 */
>  
> +
>  #define SMBHSTSTAT	0x1
>  #define SMBHSTPRTCL	0x0
>  #define SMBHSTCMD	0x3
> @@ -41,157 +43,8 @@
>  	(void) inb(0x80);
>  }
>  
> -static int smbus_wait_until_ready(u16 smbus_io_base)
> -{
> -	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;
> -}
> -
> -static int smbus_wait_until_done(u16 smbus_io_base)
> -{
> -	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;
> -}
> -
> -static int do_smbus_recv_byte(u16 smbus_io_base, u8 device)
> -{
> -	u8 global_status_register;
> -	u8 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;
> -	}
> -
> -	global_status_register = inb(smbus_io_base + SMBHSTSTAT) & 0x80; /* lose check */
> -
> -	/* read results of transaction */
> -	byte = inb(smbus_io_base + SMBHSTCMD);
> -
> -	if (global_status_register != 0x80) { // loose check, otherwise it should be 0
> -		return -1;
> -	}
> -	return byte;
> -}
> -
> -static int do_smbus_send_byte(u16 smbus_io_base, u8 device, u8 val)
> -{
> -	u8 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;
> -	}
> -	global_status_register = inb(smbus_io_base + SMBHSTSTAT) & 0x80; /* lose check */;
> -
> -	if (global_status_register != 0x80) {
> -		return -1;
> -	}
> -	return 0;
> -}
> -
> -static int do_smbus_read_byte(u16 smbus_io_base, u8 device, u8 address)
> -{
> -	u8 global_status_register;
> -	u8 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;
> -}
> -
> -static int do_smbus_write_byte(u16 smbus_io_base, u8 device, u8 address, u8 val)
> -{
> -	u8 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;
> -}
> -
> +SHARED(do_smbus_recv_byte, int, u16 smbus_io_base, u8 device);
> +SHARED(do_smbus_send_byte, int, u16 smbus_io_base, u8 device, u8 val);
> +SHARED(do_smbus_read_byte, int, u16 smbus_io_base, u8 device, u8 address);
> +SHARED(do_smbus_write_byte, int, u16 smbus_io_base, u8 device, u8 address, u8 val);
> +SHARED(enable_smbus, void, void);
> Index: southbridge/nvidia/mcp55/smbus.c
> ===================================================================
> --- southbridge/nvidia/mcp55/smbus.c	(revision 0)
> +++ southbridge/nvidia/mcp55/smbus.c	(revision 0)

Is that file really for mcp55 only?

> @@ -0,0 +1,150 @@
> +/*
> + * 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 <types.h>
> +#include <lib.h>
> +#include <console.h>
> +#include <device/pci.h>
> +#include <msr.h>
> +#include <legacy.h>
> +#include <device/pci_ids.h>
> +#include <device/smbus.h>
> +#include <io.h>
> +#include <statictree.h>
> +#include <config.h>
> +#include "mcp55.h"
> +
> +#include "mcp55_smbus.h"
> +
> +static int lsmbus_recv_byte(struct device *dev)

Please change the function name to mcp55_smbus_recv_byte.

> +{
> +	unsigned device;

u8/u32?

> +	struct resource *res;
> +	struct bus *pbus;
> +
> +	device = dev->path.i2c.device;
> +	pbus = get_pbus_smbus(dev);
> +
> +	res = find_resource(pbus->dev, 0x20 + (pbus->link * 4));
> +
> +	return do_smbus_recv_byte(res->base, device);
> +}
> +
> +static int lsmbus_send_byte(struct device *dev, u8 val)

name

> +{
> +	unsigned device;

u8/u32

> +	struct resource *res;
> +	struct bus *pbus;
> +
> +	device = dev->path.i2c.device;
> +	pbus = get_pbus_smbus(dev);
> +
> +	res = find_resource(pbus->dev, 0x20 + (pbus->link * 4));
> +
> +	return do_smbus_send_byte(res->base, device, val);
> +}
> +
> +static int lsmbus_read_byte(struct device *dev, u8 address)

name

> +{
> +	unsigned device;

u8/u32

> +	struct resource *res;
> +	struct bus *pbus;
> +
> +	device = dev->path.i2c.device;
> +	pbus = get_pbus_smbus(dev);
> +
> +	res = find_resource(pbus->dev, 0x20 + (pbus->link * 4));
> +
> +	return do_smbus_read_byte(res->base, device, address);
> +}
> +
> +static int lsmbus_write_byte(struct device *dev, u8 address, u8 val)

name

> +{
> +	unsigned device;

u8/u32

> +	struct resource *res;
> +	struct bus *pbus;
> +
> +	device = dev->path.i2c.device;
> +	pbus = get_pbus_smbus(dev);
> +
> +	res = find_resource(pbus->dev, 0x20 + (pbus->link * 4));
> +
> +	return do_smbus_write_byte(res->base, device, address, val);
> +}

missing empty line

> +static struct smbus_bus_operations lops_smbus_bus = {
> +	.recv_byte	= lsmbus_recv_byte,
> +	.send_byte	= lsmbus_send_byte,
> +	.read_byte	= lsmbus_read_byte,
> +	.write_byte	= lsmbus_write_byte,

change all four function pointers to the new names

> +};
> +
> +#ifdef HAVE_ACPI_TABLES
> +unsigned pm_base;

No global variables until you ack and commit my global var patch.
By the way, not only is the current state of global vars non-functional,
it also has the nice side-effect of corrupting the CAR area randomly.

> +#endif
> +
> +static void mcp55_sm_read_resources(struct device *dev)
> +{
> +	struct resource *res;
> +	unsigned long index;
> +
> +	/* Get the normal pci resources of this device */
> +	pci_dev_read_resources(dev);
> +
> +	for (index = 0x60; index <= 0x68; index+=4) { // We got another 3.
> +		pci_get_resource(dev, index);
> +	}
> +	compact_resources(dev);
> +
> +}
> +
> +static void mcp55_sm_init(struct device *dev)
> +{
> +#ifdef CONFIG_HAVE_ACPI_TABLES
> +	struct resource *res;
> +
> +	res = find_resource(dev, 0x60);
> +
> +	if (res)
> +		pm_base = res->base;
> +#endif
> +#warning finish subsystem set in mcp55 smbus
> +#if 0
> +	pci_write_config32(dev, 0x40,
> +		((device & 0xffff) << 16) | (vendor & 0xffff));
> +#endif

separate set_subsystem function, please.

> +}
> +
> +struct device_operations mcp55_smbus = {
> +	.id = {.type = DEVICE_ID_PCI,
> +		{.pci = {.vendor = PCI_VENDOR_ID_NVIDIA,
> +			      .device = PCI_DEVICE_ID_NVIDIA_MCP55_SM2}}},
> +	.constructor		 	= default_device_constructor,
> +	.phase3_scan		 	= scan_static_bus,
> +	.phase4_read_resources	 = mcp55_sm_read_resources,
> +	.phase4_set_resources	 = pci_dev_set_resources,
> +	.phase5_enable_resources = pci_dev_enable_resources,
> +	.phase6_init		 	= mcp55_sm_init,
> +	.ops_pci		 		= &pci_dev_ops_pci,
> +	.ops_smbus_bus		= &lops_smbus_bus,
> +};
> +

Regards,
Carl-Daniel

-- 
http://www.hailfinger.org/





More information about the coreboot mailing list