[coreboot] Patch for Nokia-IP530, now with working PIRQ table

Stefan Reinauer stepan at coresystems.de
Tue May 25 19:31:10 CEST 2010


Marc,

something went very wrong with this patch. Please try again. See
comments below.

On 5/25/10 6:20 PM, mbertens wrote:
> Index: src/mainboard/Kconfig
> ===================================================================
> --- src/mainboard/Kconfig	(revision 5583)
> +++ src/mainboard/Kconfig	(working copy)
> @@ -104,8 +104,13 @@
>  	bool "VIA"
>  config VENDOR_WINENT
>  	bool "Win Enterprises"
> +<<<<<<< .mine
> +config VENDOR_NOKIA
> +	bool "Nokia"
> +=======
>  config VENDOR_WYSE
>  	bool "Wyse"
> +>>>>>>> .r5583
>  
>   
Conflicts in this file... The whole changes in this file can be dropped,
they have been merged already.
Please be more thorough when creating patch files. This doesn't even
survive "make menuconfig".


> Index: src/mainboard/nokia/ip530/Kconfig
> ===================================================================
> --- src/mainboard/nokia/ip530/Kconfig	(revision 5583)
> +++ src/mainboard/nokia/ip530/Kconfig	(working copy)
> @@ -5,7 +5,8 @@
>   
> +## Based on romstage.c from src/mainboard/asus/p2b
> +##
>   
That makes no sense, how can you base Kconfig on a romstage.c file?

>  config BOARD_NOKIA_IP530
>  	bool "IP530"
>  	select ARCH_X86
> @@ -26,8 +28,10 @@
>  	select SUPERIO_SMSC_SMSCSUPERIO
>  	select ROMCC
>  	select HAVE_PIRQ_TABLE
> +    select PIRQ_ROUTE
>  	select UDELAY_TSC
> -	select BOARD_ROMSIZE_KB_256
> +#	select HAVE_ACPI_TABLES
> +#	select HAVE_SMI_HANDLER
>   
I think these two should go away
>  
>  config MAINBOARD_DIR
>  	string
> @@ -49,3 +53,27 @@
>  	default 6
>  	depends on BOARD_NOKIA_IP530
>  
> +config CONFIG_IRQ_SLOT_COUNT
> +	int
> +	default 1
> +	depends on BOARD_NOKIA_IP530
>   
Only one entry? Also, that entry already exists. Take care that you
don't add duplicate entries.
Also, CONFIG_ must be omitted in Kconfig

> +config CONFIG_GENERATE_PIRQ_TABLE
> +	int
> +	default	1
> +	depends on BOARD_NOKIA_IP530
>   
CONFIG_ must be omitted in Kconfig. This variable is a "bool", so it
should be "select"ed above

> +
> +config HEAP_SIZE
> +	hex
> +	default 0x60000
> +	depends on BOARD_NOKIA_IP530
>   
Very bad. Why?

> +config CONFIG_PIRQ_ROUTE
> +	int
> +	default 1
> +	depends on BOARD_NOKIA_IP530
>   
CONFIG_ must be omitted in Kconfig.
same variable is "select"ed above

> +
> +config CONFIG_DEBUG
> +	int
> +	default 1
> +	depends on BOARD_NOKIA_IP530
>   
CONFIG_ must be omitted in Kconfig.

> Index: src/mainboard/nokia/ip530/romstage.c
> ===================================================================
> --- src/mainboard/nokia/ip530/romstage.c	(revision 5583)
> +++ src/mainboard/nokia/ip530/romstage.c	(working copy)
> @@ -18,6 +18,8 @@
>   * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 USA
>   */
>  
> +/* Based on romstage.c from src/mainboard/asus/p2b */
> +
>   
This comment is not needed nor helpful
>  #include <stdint.h>
>  #include <device/pci_def.h>
>  #include <arch/io.h>
> @@ -25,7 +27,8 @@
>  #include <arch/romcc_io.h>
>  #include <arch/hlt.h>
>  #include <stdlib.h>
> -#include <console/console.h>
> +#include "pc80/serial.c"
> +#include "console/console.c"
>   
This just backs out a fix that went in.
Please be more thorough when creating patch files. This doesn't even
survive "make".


>  #include "lib/ramtest.c"
>  #include "southbridge/intel/i82371eb/i82371eb_enable_rom.c"
>  #include "southbridge/intel/i82371eb/i82371eb_early_smbus.c"
> @@ -37,33 +40,35 @@
>  #include "cpu/x86/bist.h"
>  #include "superio/smsc/smscsuperio/smscsuperio_early_serial.c"
>  
> -#define SERIAL_DEV PNP_DEV(0x3f0, SMSCSUPERIO_SP1)
> -
> +#define SERIAL_DEV PNP_DEV( 0x3f0, SMSCSUPERIO_SP1 )
>   
Unnecessary white space changes  against coding conventions.

> +//-----------------------------------------------------------------------------
>  static inline int spd_read_byte(unsigned int device, unsigned int address)
>  {
>  	return smbus_read_byte(device, address);
>  }
> -
> +//-----------------------------------------------------------------------------
>  #include "northbridge/intel/i440bx/raminit.c"
>  #include "northbridge/intel/i440bx/debug.c"
> -
> +//-----------------------------------------------------------------------------
>  static void main(unsigned long bist)
>  {
> -	if (bist == 0)
> +	if ( bist == 0 )
>   
Unnecessary white space changes  against coding conventions.
> +	{
>  		early_mtrr_init();
> -
> -	smscsuperio_enable_serial(SERIAL_DEV, CONFIG_TTYS0_BASE);
> +	}
> +	smscsuperio_enable_serial( SERIAL_DEV, CONFIG_TTYS0_BASE );
>   
Unnecessary white space changes  against coding conventions.
>  	uart_init();
>  	console_init();
> -	report_bist_failure(bist);
> +	report_bist_failure( bist );
>   
Unnecessary white space changes  against coding conventions.
>  
>  	/* Enable access to the full ROM chip, needed very early by CBFS. */
> -	i82371eb_enable_rom(PCI_DEV(0, 7, 0) ); /* ISA bridge at 00:07.0. */
> -
> +	i82371eb_enable_rom( PCI_DEV( 0, 7, 0 ) ); /* ISA bridge at 00:07.0. */
>   
Unnecessary white space changes  against coding conventions.
> Index: src/mainboard/nokia/ip530/devicetree.cb
> ===================================================================
> --- src/mainboard/nokia/ip530/devicetree.cb	(revision 5583)
> +++ src/mainboard/nokia/ip530/devicetree.cb	(working copy)
> @@ -1,94 +1,114 @@
>  ##
> -## This file is part of the coreboot project.
> +## Based on romstage.c from src/mainboard/asus/p2b
>   
Bogus change

>  ##
> -## Copyright (C) 2010 Marc Bertens <mbertens at xs4all.nl>
> -##
> -## 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
> -##
>   

Don't remove license headers

> -
>  chip northbridge/intel/i440bx		# Northbridge
> -  device lapic_cluster 0 on		# APIC cluster
> -    chip cpu/intel/socket_PGA370	# CPU
> -      device lapic 0 on end		# APIC
> -    end
> -  end
> -  device pci_domain 0 on		# PCI domain
> -    device pci 0.0 on end		# Host bridge
> -    device pci 1.0 on end		# PCI/AGP bridge
> -    chip southbridge/intel/i82371eb	# Southbridge
> -      device pci 7.0 on			# ISA bridge
> -        chip superio/smsc/smscsuperio	# Super I/O (SMSC FDC37C878)
> -          device pnp 3f0.0 on		# Floppy
> -            io 0x60 = 0x3f0
> -            irq 0x70 = 6
> -            drq 0x74 = 2
> -          end
> -          device pnp 3f0.3 on		# Parallel port
> -            io 0x60 = 0x378
> -            irq 0x70 = 7
> -            drq 0x74 = 4
> -          end
> -          device pnp 3f0.4 on		# COM1
> -            io 0x60 = 0x3f8
> -            irq 0x70 = 4
> -          end
> -          device pnp 3f0.5 on		# COM2 / IR
> -            io 0x60 = 0x2f8
> -            irq 0x70 = 3
> -          end
> -          device pnp 3f0.7 on		# PS/2 keyboard / mouse
> -            io 0x60 = 0x60
> -            io 0x62 = 0x64
> -            irq 0x70 = 1		# PS/2 keyboard interrupt
> -            irq 0x72 = 12		# PS/2 mouse interrupt
> -          end
> -          device pnp 3f0.9 on		# Game port
> -            io 0x60 = 0x201
> -          end
> -          device pnp 3f0.a on		# Power-management events (PME)
> -            io 0x60 = 0x600
> -          end
> -          device pnp 3f0.b on		# MIDI port (MPU-401)
> -            io 0x60 = 0x330
> -            irq 0x70 = 5
> -          end
> -        end
> -      end
> -      device pci 7.1 on end		# IDE
> -      device pci 7.2 on end		# USB
> -      device pci 7.3 on end		# ACPI
> -      register "ide0_enable" = "1"
> -      register "ide1_enable" = "1"
> -      register "ide_legacy_enable" = "1"
> -      # Enable UDMA/33 for higher speed if your IDE device(s) support it.
> -      register "ide0_drive0_udma33_enable" = "0"
> -      register "ide0_drive1_udma33_enable" = "0"
> -      register "ide1_drive0_udma33_enable" = "0"
> -      register "ide1_drive1_udma33_enable" = "0"
> -    end
> -    device pci 0d.0 on end		# NIC (DEC DECchip 21142/43)
> -    device pci 0e.0 on end		# NIC (DEC DECchip 21142/43)
> -    device pci 0f.0 on end		# CardBus bridge (TI PCI1225)
> -    device pci 0f.1 on end		# CardBus bridge (TI PCI1225)
> -  end
> -  device pci_domain 1 on		# PCI domain 1
> -    device pci 00.0 on end		# PCI bridge (DEC DECchip 21150)
> -  end
> -  device pci_domain 2 on		# PCI domain 2
> -    device pci 04.0 on end		# NIC (DECchip 21142/43)
> -    device pci 04.0 on end		# NIC (DECchip 21142/43)
> -  end
> +	device apic_cluster 0 on		# APIC cluster
> +		chip cpu/intel/socket_PGA370		# CPU
> +			device apic 0 on 				# APIC
> +			end
> +		end
> +	end
>   
This backs out a fix.

Won't even compile anymore

> +	device pci_domain 0 on		# PCI domain
> +		device pci 0.0 on 			# Host bridge
> +		end
> +		device pci 1.0 on 		# PCI/AGP bridge
> +		end
> +		chip southbridge/intel/i82371eb	# Southbridge
> +			device pci 7.0 on			# ISA bridge
> +		    	chip superio/smsc/smscsuperio		# Super I/O FDC 37C878
> +  		  			device pnp 3f0.0 off		# Floppy
> +						#	LDN 0x00 (Floppy)
> +						#
> +						#	Register 0x30 ( Activate ) needs to be set to 0x00 (disable de FDD)
> +  		  			end
> +				    device pnp 3f0.3 off		# Parallel port
> +						#	LDN 0x03 (Parallel port)
> +						#
> +						#	Register 0x30 ( Activate ) needs to be set to 0x00 (disable de PP)
> +						#	Registers 0x60, 0x61, 0x70 are set to 0x00
> +					end
>   
inconsistend indentation. Please fix.

> -
> Index: src/mainboard/nokia/ip530/irq_tables.c
> ===================================================================
> --- src/mainboard/nokia/ip530/irq_tables.c	(revision 5583)
> +++ src/mainboard/nokia/ip530/irq_tables.c	(working copy)
> @@ -19,31 +19,74 @@
>   */
>  
>  #include <arch/pirq_routing.h>
> +#include <console/console.h>
>  
> +#define PIRQ_MAX_DEVICES( no )	(32 + ( 16 * no ))
> +//-----------------------------------------------------------------------------
>  const struct irq_routing_table intel_irq_routing_table = {
> -	PIRQ_SIGNATURE,		/* u32 signature */
> -	PIRQ_VERSION,		/* u16 version */
> -	32 + 16 * CONFIG_IRQ_SLOT_COUNT,/* Max. number of devices on the bus */
> -	0x00,			/* Interrupt router bus */
> -	(0x07 << 3) | 0x0,	/* Interrupt router dev */
> -	0,			/* IRQs devoted exclusively to PCI usage */
> -	0x8086,			/* Vendor */
> -	0x122e,			/* Device */
> -	0,			/* Miniport */
> -	{ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 }, /* u8 rfu[11] */
> -	0x36,			/* Checksum */
> +	/* u32 signature */
> +	PIRQ_SIGNATURE,
> +	/* u16 version */
> +	PIRQ_VERSION,
> +	/* Max. number of devices on the bus, set this through Kconfig */
> +	PIRQ_MAX_DEVICES( CONFIG_IRQ_SLOT_COUNT ),
> +	/* Interrupt router bus */
> +	0x00,
> +	/* Interrupt router dev */
> +	(0x07 << 3) | 0x0,
> +	/* IRQs devoted exclusively to PCI usage */
> +	0,
> +	/* Vendor, device */
> +	0x8086,	0x122e,
> +	/* Miniport */
> +	0,
> +	/* u8 rfu[11] */
> +	{ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 },
> +	/* Checksum (has to be set to some value that would give 0 after the sum of all bytes
> +	 * for this structure (including checksum).
> +	 */
> +	0x08,
>  	{
> -		/* bus,        dev | fn,   {link, bitmap}, {link, bitmap}, {link, bitmap}, {link, bitmap}, slot, rfu */
> -		{0x00, (0x07 << 3) | 0x0, {{0x00, 0x0ea8}, {0x00, 0x0ea8}, {0x00, 0x0ea8}, {0x63, 0x0ea8}}, 0x0, 0x0},
> -		{0x00, (0x0c << 3) | 0x0, {{0x61, 0x06a8}, {0x62, 0x06a8}, {0x00, 0x06a8}, {0x00, 0x06a8}}, 0x0, 0x0},
> -		{0x00, (0x0d << 3) | 0x0, {{0x60, 0x0ea8}, {0x61, 0x0ea8}, {0x00, 0x0ea8}, {0x00, 0x0ea8}}, 0x1, 0x0},
> -		{0x00, (0x09 << 3) | 0x0, {{0x62, 0x0ea8}, {0x63, 0x0ea8}, {0x60, 0x0ea8}, {0x61, 0x0ea8}}, 0x2, 0x0},
> -		{0x00, (0x0a << 3) | 0x0, {{0x63, 0x0ea8}, {0x00, 0x0ea8}, {0x00, 0x0ea8}, {0x00, 0x0ea8}}, 0x0, 0x0},
> -		{0x01, (0x00 << 3) | 0x0, {{0x60, 0x0ea8}, {0x00, 0x0ea8}, {0x00, 0x0ea8}, {0x00, 0x0ea8}}, 0x0, 0x0},
> +		// Table by CareBear:
> +		// 0x1E20	= 0001111000100000 (IRQ5, IRQ9, IRQ10, IRQ11, IRQ12)
> +		//
> +		// Got some unexpected behaviour for the USB, orginal set 63
> +		{ 0x00, (0x07 << 3) | 0x0, {{0x00, 0x1E20}, {0x00, 0x1E20}, {0x00, 0x1E20}, {0x63, 0x1E20}}, 0x0, 0x0 },
> +				// 62
> +		{ 0x00, (0x0d << 3) | 0x0, {{0x62, 0x1E20}, {0x00, 0x0000}, {0x00, 0x0000}, {0x00, 0x0000}}, 0x0, 0x0 },
> +				// 63
> +		{ 0x00, (0x0e << 3) | 0x0, {{0x63, 0x1E20}, {0x00, 0x0000}, {0x00, 0x0000}, {0x00, 0x0000}}, 0x0, 0x0 },
> +		{ 0x02, (0x04 << 3) | 0x0, {{0x60, 0x1E20}, {0x00, 0x0000}, {0x00, 0x0000}, {0x00, 0x0000}}, 0x0, 0x0 },
> +		{ 0x02, (0x05 << 3) | 0x0, {{0x61, 0x1E20}, {0x00, 0x0000}, {0x00, 0x0000}, {0x00, 0x0000}}, 0x0, 0x0 },
> +		// TODO: fix the PCI1225.INTRTIE_BIT = 0
> +		{ 0x00, (0x0f << 3) | 0x0, {{0x60, 0x1E20}, {0x61, 0x1E20}, {0x00, 0x0000}, {0x00, 0x0000}}, 0x0, 0x0 },
>  	}
>  };
> +//-----------------------------------------------------------------------------
> +static int calc_checksum( const struct irq_routing_table *rt )
> +{
> +	long i;
> +	uint8_t *addr, sum = 0;
>  
> +	addr = (uint8_t *) rt;
> +	for (i = 0; i < rt->size; i++)
> +	{
> +		sum += addr[i];
> +	}
> +	return ( sum );
> +}
>   
unneeded.
> +//-----------------------------------------------------------------------------
>  unsigned long write_pirq_routing_table(unsigned long addr)
>  {
> -	return copy_pirq_routing_table(addr);
> +	int iResult = calc_checksum( &intel_irq_routing_table );
> +	printk( BIOS_DEBUG, "PIRQ_TABLE_CHECKSUM is %x\n", 0x100 - ( ( iResult - intel_irq_routing_table.checksum ) & 0xFF ) );
> +	return ( copy_pirq_routing_table( addr ) );
>  }
> +//-----------------------------------------------------------------------------
> +void pirq_assign_irqs(const unsigned char pIntAtoD[4])
> +{
> +	// This is just here as a placeholder until the point is solved in the
> +	// main code of coreboot
> +	return;
> +}
> +//-----------------------------------------------------------------------------
> Index: src/mainboard/nokia/ip530/mainboard.c
> ===================================================================
> --- src/mainboard/nokia/ip530/mainboard.c	(revision 5583)
> +++ src/mainboard/nokia/ip530/mainboard.c	(working copy)
> @@ -20,7 +20,67 @@
>  
>  #include <device/device.h>
>  #include "chip.h"
> +#include <device/pci_def.h>
> +#include <arch/io.h>
> +#include <device/pnp_def.h>
> +#include <console/console.h>
> +#define OUTB	outb
> +#define INB		inb
>   
Please don't


> +//-----------------------------------------------------------------------------
> +/*
> +*	Taken from flashrom project
> +*	Generic Super I/O helper functions
> +*/
> +static uint8_t sio_read(uint16_t port, uint8_t reg)
> +{
> +	OUTB( reg, port );
> +	return ( INB( port + 1 ) );
> +}
> +//-----------------------------------------------------------------------------
> +static void sio_write(uint16_t port, uint8_t reg, uint8_t data)
> +{
> +	OUTB( reg, port );
> +	OUTB( data, port + 1 );
> +	return;
> +}
>   

Please use the coreboot functions for this, no need to copy stuff from
other projects. In fact, below should go into devicetree.cb

> +//-----------------------------------------------------------------------------
> +static void nokia_ip530_board_enable( device_t dev )
> +{
> +	print_debug( "Setting up IP530-Super I/O devices\n");
> +	/*
> +	*	Register 03 (Index Address) needs to be set to 0x80:
> +	*
> +	*		Enable GP1, WDT_CTRL, GP5, GP6, Soft Power Enable and Status
> +	*		Register access when not in configuration mode (0x80)
> +	*		Sets GP1 etc. selection regis ter used when in Run mode
> +	*		(not in Configuration Mode), 00 = 0xE0
> +	*
> +	*	Register 22: (Power Control) need to be set to 0x30:
> +	*
> +	*		Serial Port 1 Power, = 1 Power on or enabled	(0x10)
> +	*		Serial Port 2 Power, = 1 Power on or enabled	(0x20)
> +	*		*Note* the rest of the devices are disabled.
> +	*
> +	*	Register 24: ( OSC ) need to be set to 0x84:
> +	*
> +	*		Osc is on, BRG clock is on. (0x04)
> +	*		IRQ8 Polarity is IRQ8 is active low (0x80)
> +	*
> +	*	This is done outside the config mode
> +	*/
> +	printk( BIOS_DEBUG, "--Correcting direct registers\n");
> +	sio_write( 0x20, 0x03, 0x80 );
> +	printk( BIOS_DEBUG, "--Register 0x03 = %X := 0x80\n", sio_read( 0x20, 0x03 ) );
> +	sio_write( 0x20, 0x22, 0x30 );
> +	printk( BIOS_DEBUG, "--Register 0x22 = %X := 0x30\n", sio_read( 0x20, 0x22 ) );
> +	sio_write( 0x20, 0x24, 0x84 );
> +	printk( BIOS_DEBUG, "--Register 0x24 = %X := 0x84\n", sio_read( 0x20, 0x24 ) );
> +	return;
> +}
> +//-----------------------------------------------------------------------------
>  
>  struct chip_operations mainboard_ops = {
> -	CHIP_NAME("Nokia IP530 Mainboard")
> +	CHIP_NAME( "NOKIA IP530 Mainboard" )
> +	.enable_dev = nokia_ip530_board_enable,
>  };
> +
> Index: src/arch/i386/boot/pirq_routing.c
> ===================================================================
> --- src/arch/i386/boot/pirq_routing.c	(revision 5583)
> +++ src/arch/i386/boot/pirq_routing.c	(working copy)
> @@ -1,3 +1,26 @@
> +/*
> + *	This file is part of the coreboot project.
> + *
> + *	coreboot PIRQ Table support
> + *
> + * 	Copyright (C) 2005-2010 coresystems GmbH
> + *	Copyright (C) 2010 Marc Bertens <mbertens at xs4all.nl>
> + *
>   
broken indentation

> + * 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; version 2 of
> + * the License.
> + *
> + * 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/console.h>
>  #include <arch/pirq_routing.h>
>  #include <string.h>
> @@ -121,7 +144,11 @@
>  
>  			printk(BIOS_DEBUG, "INT: %c link: %x bitmap: %x  ",
>  				'A' + j, link, bitmap);
> -
> +// fix made by Marc Bertens <mbertens at xs4all.nl>
> +			if (link > 0x5f)
> +			{
> +				link -= 0x5f;
> +			}
>   

What does this fix? That would be far more interesting than who fixed it
(that belongs in the copyright lines)

Stefan
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.coreboot.org/pipermail/coreboot/attachments/20100525/6c46b9df/attachment.html>


More information about the coreboot mailing list