LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [RFC] Device tree for new desktop platform in arch/powerpc
From: Mark A. Greer @ 2007-06-18 20:25 UTC (permalink / raw)
  To: Gerhard Pircher; +Cc: linuxppc-dev, "list <linuxppc-dev"
In-Reply-To: <20070618194359.321010@gmx.net>

On Mon, Jun 18, 2007 at 09:43:59PM +0200, Gerhard Pircher wrote:
> Von: "Mark A. Greer" <mgreer@mvista.com>
> > On Mon, Jun 18, 2007 at 08:57:15PM +0200, Gerhard Pircher wrote:
> > 
> > > 1. The AmigaOne makes use of different G3...
> > 
> > I'm far from an authoritative figure on this but making a
> > reasonable default (in your dts file) then updating in the 
> > bootwrapper makes sense to me.  That's what I did for several 
> > properties in the prpmc2800's dts.
> So, the bootwrapper should update the name property of the CPU node. Is the cpu node name used somewhere?

Oh, sorry I really talking about the cpu frequencies, etc.
I don't think the name of the cpu node matters but I'd have to check.
Others probably know off the top of their heads.

Mark

^ permalink raw reply

* CONFIG_PS3_USE_LPAR_ADDR dramically increases boot time
From: Jimi Xenidis @ 2007-06-18 19:45 UTC (permalink / raw)
  To: linuxppc-dev list

The following change set:
   http://git.kernel.org/?p=linux/kernel/git/paulus/ 
powerpc.git;a=commit;h=261efc3f178c8c5b55d76208aee1f39ce247f723

setting CONFIG_PS3_USE_LPAR_ADDR (which is the default for PS3)  
changes increases MAX_PHYSADDR_BITS  from 44 to 47 which makes the  
array 8 time bigger and takes way longer for sparse_init() to run on  
simulator.

Is this really the right approach?
Cell is actually 42 so could not the special address space be 43 or 44?

-JX

^ permalink raw reply

* Re: [RFC] Device tree for new desktop platform in arch/powerpc
From: Gerhard Pircher @ 2007-06-18 19:43 UTC (permalink / raw)
  To: Mark A. Greer; +Cc: linuxppc-dev, "list <linuxppc-dev"
In-Reply-To: <20070618191529.GC8740@mag.az.mvista.com>


-------- Original-Nachricht --------
Datum: Mon, 18 Jun 2007 12:15:29 -0700
Von: "Mark A. Greer" <mgreer@mvista.com>
An: Gerhard Pircher <gerhard_pircher@gmx.net>
CC: linuxppc-dev@ozlabs.org, "list <linuxppc-dev"@ozlabs.org
Betreff: Re: [RFC] Device tree for new desktop platform in arch/powerpc

> On Mon, Jun 18, 2007 at 08:57:15PM +0200, Gerhard Pircher wrote:
> 
> > 1. The AmigaOne makes use of different G3...
> 
> I'm far from an authoritative figure on this but making a
> reasonable default (in your dts file) then updating in the 
> bootwrapper makes sense to me.  That's what I did for several 
> properties in the prpmc2800's dts.
So, the bootwrapper should update the name property of the CPU node. Is the cpu node name used somewhere?

> > If so, where should I specify the register address ranges (reg
> > property)? In the PCI device node or in the ISA device nodes,
> > as it is now?
> 
> Each device's node should have a reg property for its registers.
> You'll need a ranges property for the pci-isa bridge as well.
Ah yes, didn't think about that.

> P.S. You should change email clients of fix the one you're using
> to break lines up so they don't wrap in a std 80 char line 
> window.
Yeah, sorry. Forgot about that when writing the mail.

Gerhard
-- 
Ist Ihr Browser Vista-kompatibel? Jetzt die neuesten 
Browser-Versionen downloaden: http://www.gmx.net/de/go/browser

^ permalink raw reply

* Re: [RFC] Device tree for new desktop platform in arch/powerpc
From: Mark A. Greer @ 2007-06-18 19:15 UTC (permalink / raw)
  To: Gerhard Pircher; +Cc: linuxppc-dev, "list <linuxppc-dev"
In-Reply-To: <20070618185715.321010@gmx.net>

On Mon, Jun 18, 2007 at 08:57:15PM +0200, Gerhard Pircher wrote:

> 1. The AmigaOne makes use of different G3 (750Cx, 750GL, 750GX) and G4 (7400, 7450, 7455, 7457) CPUs. The current dts file defines a cpu node with the name "PowerPC,7xxx@0" and the bootwrapper just fills in the cpu-, bus- and timebase frequencies. Is this the recommended way, or should the bootwrapper create a fresh cpu node with the correct name of the CPU (e.g. "PowerPC,7455@ 0" - I guess this would require a table with all PVR values and their corresponding CPU name. Or is there a function in the kernel that do this?).

I'm far from an authoritative figure on this but making a reasonable
default (in your dts file) then updating in the bootwrapper makes sense
to me.  That's what I did for several properties in the prpmc2800's dts.

> 2. I tried to define all ISA devices, which are integrated in the southbridge (VT82C686B). Currently these are child nodes of the device tree root. Should they be child nodes of the southbridge's PCI-to-ISA device node?

I would think so.

> If so, where should I specify the register address ranges (reg property)? In the PCI device node or in the ISA device nodes, as it is now?

Each device's node should have a reg property for its registers.  You'll
need a ranges property for the pci-isa bridge as well.

Mark

P.S. You should change email clients of fix the one you're using to
     break lines up so they don't wrap in a std 80 char line window.

^ permalink raw reply

* [RFC] Device tree for new desktop platform in arch/powerpc
From: Gerhard Pircher @ 2007-06-18 18:57 UTC (permalink / raw)
  To: linuxppc-dev list

[-- Attachment #1: Type: text/plain, Size: 2014 bytes --]

Hi,

I'm working on support for the AmigaOne and started to write a device tree source file for use with the cuboot bootwrapper. Additionally to the booting-without-of.txt document and other dts files I found some OpenFirmware related documents on http://playground.sun.com/1275/ and tried to follow their guidelines. Note that I didn't try to compile the dts file yet, as it is not yet complete.

Some questions arose during the implementation, which I hope some of you can answer:

1. The AmigaOne makes use of different G3 (750Cx, 750GL, 750GX) and G4 (7400, 7450, 7455, 7457) CPUs. The current dts file defines a cpu node with the name "PowerPC,7xxx@0" and the bootwrapper just fills in the cpu-, bus- and timebase frequencies. Is this the recommended way, or should the bootwrapper create a fresh cpu node with the correct name of the CPU (e.g. "PowerPC,7455@ 0" - I guess this would require a table with all PVR values and their corresponding CPU name. Or is there a function in the kernel that do this?).

2. I tried to define all ISA devices, which are integrated in the southbridge (VT82C686B). Currently these are child nodes of the device tree root. Should they be child nodes of the southbridge's PCI-to-ISA device node? If so, where should I specify the register address ranges (reg property)? In the PCI device node or in the ISA device nodes, as it is now?

3. The dts files define the device_type of a serial port as "serial", whereas the OF spec says "pnpPNP,501". What's the difference between the two?

4. I couldn't find any information for a device tree representation of the i8259 interrupt controller in the booting-without-of.txt file. Is my implementation correct (inspired by the OF documents)?

I would be grateful, if you could take a look at my dts file and comment on every line that doesn't meet your views.

Thanks in advance!

regards,

Gerhard
-- 
GMX FreeMail: 1 GB Postfach, 5 E-Mail-Adressen, 10 Free SMS.
Alle Infos und kostenlose Anmeldung: http://www.gmx.net/de/go/freemail

[-- Attachment #2: amigaone.dts --]
[-- Type: application/octet-stream, Size: 4047 bytes --]

/*
 * AmigaOne Device Tree Source
 *
 * Copyright 2007 Gerhard Pircher (gerhard_pircher@gmx.net)
 *
 * 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.
 */


/ {
	model = "Eyetech,AmigaOne";
	compatible = "Eyetech,AmigaOne" "MAI,Teron";
	#address-cells = <1>;
	#size-cells = <1>;

	cpus {
		#address-cells = <1>;
		#size-cells = <0>;

		PowerPC,7xxx@0 {			// Can be 75x or 745x?
			device_type = "cpu";
			reg = <0>;
			d-cache-line-size = <20>;	// 32 bytes
			i-cache-line-size = <20>;	// 32 bytes
			d-cache-size = <8000>;		// L1, 32K
			i-cache-size = <8000>;		// L1, 32K
			timebase-frequency = <0>;	// 33.3 MHz, from U-boot
			clock-frequency = <0>;		// From U-boot
			bus-frequency = <0>;		// From U-boot
			32-bit;
		};
	};

	memory {
		device_type = "memory";
		reg = <0 0>;				// From U-boot
	};

	// Should the following nodes be childs of a ISA bus node?

	dma-controller {
		device_type = "dma-controller";
		compatible = "chrp,dma";
		reg = <0 10 80 10 c0 20>;
		/* Channel 4 reserverd, cascade mode, 2x32k transfer/counter
		 * widths and bus master capability. Is this really necessary?
		 */
/*		dma = <4 4 20 20 1>; */
		#address-cells = <1>;
		#size-cells = <0>;			// No physical address.
	};

  	interrupt-controller {
		device_type = "interrupt-controller";
		compatible = "chrp,iic";
		interrupt-controller;
		reg = <20 2 a0 2 4d0 2>;
		reserved-interrupts = <2>;
		#address-cells = <1>;
		#size-cells = <0>;
		#interrupt-cells = <2>;
	};

	8042@60 {
		device_type = "8042";
		compatible = "chrp,8042";
		reg = <60 10>;
		interrupts = <1 3 c 3>;			// IRQ1, IRQ12 (rising edge)
		interrupt-parent = <&interrupt-controller>;
		#address-cells = <1>;
		#size-cells = <0>;			// No physical address.

		keyboard {
			device_type = "keyboard";
			compatible = "pnpPNP,303";
			reg = <0>;
		};

		mouse {
			device_type = "mouse";
			compatible = "pnpPNP,f03";
			reg = <1>;
		};
	};

	timer@40 {
		device_type = "timer";
		reg = <40 20>
		#address-cells = <1>;
		#size-cells = <0>;
	};

	rtc@70 {
		device_type = "rtc";
		compatible = <ds1385-rtc>;
		reg = <70 2>;
		interrupts = <8 3>;
		#address-cells = <1>;
		#size-cells = <0>;
	};

	serial@3f8 {
		device_type = "serial";
/*		compatible = "pnpPNP,501";*/
		compatible = "ns16550";
		reg = <3f8 8>;
		interrupts = <4 3>;			// IRQ4 (rising edge)
		interrupt-parent = <&interrupt-controller>;
		clock-frequency = <0>;			// Not necessary?
		#address-cells = <1>;
		#size-cells = <0>;
	};

	serial@2f8 {
		device_type = "serial";
/*		compatible = "pnpPNP,501";*/
		compatible = "ns16550";
		reg = <2f8 8>;
		interrupts = <3 3>;			// IRQ3 (rising edge)
		interrupt-parent = <&interrupt-controller>;
		clock-frequency = <0>;			// Not necessary?
		#address-cells = <1>;
		#size-cells = <0>;
	};

	parallel@378 {
		device_type = "parallel";
		compatible = "chrp,ecp";
		reg = <378 3 778 3>;
/*		interrupts = <7>;*/			// No IRQ free on AmigaOne!
		interrupt-parent = <&interrupt-controller>;
/*		dma = <3 0 0 0>;*/			// Parallel port DMA mode?
		#address-cells = <1>;
		#size-cells = <0>;
	};

	fdc@3f0 {
		device_type = "fdc";
		compatible = "pnpPNP,700";
		reg = <3f0 8>;
		interrupts = <6 3>;			// IRQ6 (rising edge)
		interrupt-parent = <&interrupt-controller>;
/*		dma = < >;*/				// Floppy DMA mode?
		#address-cells = <1>;
		#size-cells = <0>;

		disk {
			device_type = "block";
			reg = <0>;
		};
	};	

  	pci@80000000 {
		device_type = "pci";
		bus-frequency = <01fca055>;		// 33.3MHz
		bus-range = <0 f>;
		reg = <80000000 7f000000>;			// Whole PCI space.
		ranges = <01000000 0 0 fe000000 0 00c00000	// PCI I/O
			  02000000 0 0 80000000 0 7d000000	// PCI memory
			  02000000 0 0 fd000000 0 01000000	// PCI alias memory
			  00000000 0 0 fec00000 0 00300000>;	// PCI config
		8259-interrupt-acknowledge = <fef00000>;
		#address-cells = <3>;
		#size-cells = <2>;
	};
};

[-- Attachment #3: ioports --]
[-- Type: application/octet-stream, Size: 1190 bytes --]

00000000-00bfffff : PCI host bridge
  00000000-0000001f : dma1
  00000020-00000021 : 8259 (master)
  00000040-0000005f : timer
  00000060-0000006f : i8042
  00000080-0000008f : dma page reg
  000000a0-000000a1 : 8259 (slave)
  000000c0-000000df : dma2
  00000170-00000177 : ide1
  000001f0-000001f7 : ide0
  000002f8-000002ff : serial
  00000376-00000376 : ide1
  00000378-0000037a : parport0
  000003c0-000003df : vga+
  000003e8-000003ef : serial
  000003f6-000003f6 : ide0
  000003f8-000003ff : serial
  000004d0-000004d1 : 8259 edge control
  00002000-00002fff : PCI Bus #01
    00002000-000020ff : 0000:01:00.0
  0000cc00-0000cc0f : 0000:00:07.1
    0000cc00-0000cc07 : ide0
    0000cc08-0000cc0f : ide1
  00802000-0080207f : 0000:00:06.0
  00802080-0080209f : 0000:00:07.2
    00802080-0080209f : uhci_hcd
  008020a0-008020bf : 0000:00:07.3
    008020a0-008020bf : uhci_hcd
  00802100-008021ff : 0000:00:07.5
  00802200-00802203 : 0000:00:07.5
  00802204-00802207 : 0000:00:07.5
  00802300-008023ff : 0000:00:07.6
  00802400-0080241f : 0000:00:09.0
    00802400-0080241f : EMU10K1
  00802420-00802427 : 0000:00:09.1
    00802420-00802427 : emu10k1-gp
e100e000-e100e07f : 0000:00:06.0

^ permalink raw reply

* Re: [patch 24/33] powerpc: Correct __secondary_hold comment
From: Segher Boessenkool @ 2007-06-18 16:56 UTC (permalink / raw)
  To: Milton Miller; +Cc: ppcdev
In-Reply-To: <26778ed0e343676aa0c4fd0f4fa0746b@bga.com>

> How does this text sound?
>
> This code starting at offset 0x60 is used to hold secondary processors
> before the kernel has been relocated to its linked address.  Any
> platform or pre-boot environment may transfer secondary processors to
> the kernel by copying the first 0x100 bytes to address 0, and then
> instructing the secondary cpus to branch to address 0x60 after placing
> their hardware id (as specified by the device tree) in r3.

I think it is a Really Bad Idea(tm) to make this part of
the kernel ABI.  Can't you just make the secondaries enter
the kernel by jumping to the normal kernel entry point
(with some flag set in some register I suppose)?


Segher

^ permalink raw reply

* Re: [patch 5/6] ps3: ROM Storage Driver
From: Milton Miller @ 2007-06-18 16:30 UTC (permalink / raw)
  To: Geert Uytterhoeven; +Cc: ppcdev
In-Reply-To: <20070615120848.454492000@pademelon.sonytel.be>

> From: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
>
> Add a CD/DVD/BD Storage Driver for the PS3:
>   - Implemented as a SCSI device driver
>   - Uses software scatter-gather with a 64 KiB bounce buffer as the 
> hypervisor
>     doesn't support scatter-gather
>


>
> +config PS3_ROM
> +	tristate "PS3 ROM Storage Driver"
> +	depends on PPC_PS3 && BLK_DEV_SR
> +	select PS3_STORAGE
> +	help
> +	  Include support for the PS3 ROM Storage.
> +
> +	  This support is required to access the PS3 BD/DVD/CD-ROM drive.
> +	  In general, all users will say Y or M.
> +


When I think ROM I usually dont' start with optical media.

Have you condered calling this the optical driver?

Or at least putting BD?DVD?CD-ROM on the prompt.


Why does it depend on BLK_DEV_SR?

...
> +
> +#include <linux/cdrom.h>
> +#include <linux/highmem.h>
> +#include <linux/interrupt.h>
> +#include <linux/kthread.h>
> +
> +#include <scsi/scsi.h>
> +#include <scsi/scsi_cmnd.h>
> +#include <scsi/scsi_dbg.h>
> +#include <scsi/scsi_device.h>
> +#include <scsi/scsi_host.h>
> +
> +#include <asm/lv1call.h>
> +#include <asm/ps3stor.h>
> +
> +
> +#define DEVICE_NAME			"ps3rom"
> +
> +#define BOUNCE_SIZE			(64*1024)
> +
> +#define PS3ROM_MAX_SECTORS		(BOUNCE_SIZE / CD_FRAMESIZE)
> +

the above describes the device

> +#define LV1_STORAGE_SEND_ATAPI_COMMAND	(1)
> +
>
while this one is part of the hypervisor ABI.


> +
> +struct ps3rom_private {
> +	struct ps3_storage_device *dev;
> +	struct scsi_cmnd *cmd;
> +};
> +#define ps3rom_priv(dev)	((dev)->sbd.core.driver_data)
>

The above is driver defined.

> +
> +struct lv1_atapi_cmnd_block {
> +	u8	pkt[32];	/* packet command block           */
> +	u32	pktlen;		/* should be 12 for ATAPI 8020    */
> +	u32	blocks;
> +	u32	block_size;
> +	u32	proto;		/* transfer mode                  */
> +	u32	in_out;		/* transfer direction             */
> +	u64	buffer;		/* parameter except command block */
> +	u32	arglen;		/* length above                   */
> +};
> +

Its minor, but I'd probably put the define down here with the rest of 
the ABI contants vs all defines then structs then enums.

> +enum lv1_atapi_proto {
> +	NON_DATA_PROTO     = 0,
> +	PIO_DATA_IN_PROTO  = 1,
> +	PIO_DATA_OUT_PROTO = 2,
> +	DMA_PROTO = 3
> +};
> +
> +enum lv1_atapi_in_out {
> +	DIR_WRITE = 0, /* memory -> device */
> +	DIR_READ = 1 /* device -> memory */
> +};
> +
>


...


> +
> +static int __devinit ps3rom_probe(struct ps3_system_bus_device *_dev)
> +{
> +	struct ps3_storage_device *dev = to_ps3_storage_device(&_dev->core);
> +	int error;
> +	struct Scsi_Host *host;
> +	struct ps3rom_private *priv;
> +
> +	if (dev->blk_size != CD_FRAMESIZE) {
> +		dev_err(&dev->sbd.core,
> +			"%s:%u: cannot handle block size %lu\n", __func__,
> +			__LINE__, dev->blk_size);
> +		return -EINVAL;
> +	}
> +
> +	dev->bounce_size = BOUNCE_SIZE;
> +	dev->bounce_buf = kmalloc(BOUNCE_SIZE, GFP_DMA);
> +	if (!dev->bounce_buf) {
> +		return -ENOMEM;
> +	}
> +
> +	error = ps3stor_setup(dev);
> +	if (error)
> +		goto fail_free_bounce;
> +
> +	/* override the interrupt handler */
> +	free_irq(dev->irq, dev);
> +	error = request_irq(dev->irq, ps3rom_interrupt, IRQF_DISABLED,
> +			    dev->sbd.core.driver->name, dev);
> +	if (error) {
> +		dev_err(&dev->sbd.core, "%s:%u: request_irq failed %d\n",
> +			__func__, __LINE__, error);
> +		goto fail_teardown;
> +	}
> +

Somebody's help isn't helpful?  Layering problem?


> +	host = scsi_host_alloc(&ps3rom_host_template,
> +			       sizeof(struct ps3rom_private));
> +	if (!host) {
> +		dev_err(&dev->sbd.core, "%s:%u: scsi_host_alloc failed\n",
> +			__func__, __LINE__);
> +		goto fail_teardown;
> +	}
> +
> +	priv = (struct ps3rom_private *)host->hostdata;
> +	ps3rom_priv(dev) = host;
> +	priv->dev = dev;
>

Is there code to clear ->priv when failing or unregistering?  Usually 
these fields are set to NULL to avoid accidental use of stale pointers 
to freed memory.  Discovering a NULL pointer defereence is usually 
easier than a stale usage.

> +
> +	/* One device/LUN per SCSI bus */
> +	host->max_id = 1;
> +	host->max_lun = 1;
> +
> +	error = scsi_add_host(host, &dev->sbd.core);
> +	if (error) {
> +		dev_err(&dev->sbd.core, "%s:%u: scsi_host_alloc failed %d\n",
> +			__func__, __LINE__, error);
> +		error = -ENODEV;
> +		goto fail_host_put;
> +	}
> +
> +	scsi_scan_host(host);
> +	return 0;
> +
> +fail_host_put:
> +	scsi_host_put(host);
> +fail_teardown:
> +	ps3stor_teardown(dev);
> +fail_free_bounce:
> +	kfree(dev->bounce_buf);
> +	return error;
> +}
>

^ permalink raw reply

* Re: [patch 6/6] ps3: FLASH ROM Storage Driver
From: Milton Miller @ 2007-06-18 16:30 UTC (permalink / raw)
  To: Geert Uytterhoeven; +Cc: ppcdev
In-Reply-To: <20070615120848.558678000@pademelon.sonytel.be>


> From: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
>
> Add a FLASH ROM Storage Driver for the PS3:
>   - Implemented as a misc character device driver
>   - Uses a fixed 256 KiB buffer allocated from boot memory as the 
> hypervisor
>     requires the writing of aligned 256 KiB blocks
>
> CC: Geoff Levand <geoffrey.levand@am.sony.com>
> Signed-off-by: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
> ---
> Changes since previous submission:
>       o Don't use `default y' in Kconfig
>       o #include <linux/uaccess.h> instead of <asm/uaccess.h>
>       o Set up sysfs links between misc character device and PS3 
> system device:
> 	  . /sys/class/misc/ps3flash/device -> 
> ../../../devices/ps3_system/sb_01
> 	  . /sys/devices/ps3_system/sb_01/misc:ps3flash -> 
> ../../../class/misc/ps3flash
>
>  arch/powerpc/platforms/ps3/Kconfig |   11 +
>  drivers/char/Makefile              |    2
>  drivers/char/ps3flash.c            |  404 
> +++++++++++++++++++++++++++++++++++++
>  3 files changed, 417 insertions(+)

You indicated that the primary reasons to use this driver were to boot 
back into GameOS, or reinstall the boot loader, yet your Kconfig entry 
says "In general, all users will say Y or M".  This could give a bit 
more guidance.

ps3flash_read_write_sectors chooses a string every call that is only 
used on the error path.

ps3flash_read gives priority to returning an encountered error.  If it 
made partial progress (eg partially mapped user buffer) it should 
return the count of bytes sent to the user.

ps3_write will call the hypervisor to write an aligned chunk of memory 
every call.  I'm guessing the hypervisor does no buffering of the 
flash.  If the file is written with a small block size, this will
cause unnecessary flash wear.  Have you considered requiring the 
application to write the full 256MiB, or alternatively delaying the 
actual write call until either close, the write reaches the boundary, 
or a read or write accesses another chunk?   Yes, the later would mean 
short writes would receive the error on close.

milton

^ permalink raw reply

* Re: [patch 1/6] ps3: Preallocate bootmem memory for the PS3 FLASH
From: Milton Miller @ 2007-06-18 16:25 UTC (permalink / raw)
  To: Geert Uytterhoeven; +Cc: ppcdev
In-Reply-To: <Pine.LNX.4.62.0706151529380.14899@pademelon.sonytel.be>

On Fri Jun 15 23:32:41 EST 2007, Geert Uytterhoeven wrote:
> On Fri, 15 Jun 2007, Benjamin Herrenschmidt wrote:
>> On Fri, 2007-06-15 at 13:39 +0200, Geert Uytterhoeven wrote:
>>> Preallocate 256 KiB of bootmem memory for the PS3 FLASH ROM storage 
>>> driver.
>>
>> I still very much dislike the #ifdef xxx_MODULE in main kernel code.
>
> Me too, but there's no other reliable way to allocate it after boot 
> up...
>
>> At the end of the day, is it realistic to ever use a PS3 without the
>> storage driver ? I would suggest just allocating those 
>> unconditionally.
>
> If you don't plan to (a) boot into GameOS or (b) update the kboot 
> kernel, then the answer is: yes, you can live without the FLASH ROM 
> driver, and save one promille of RAM.
>

In that case, why not make it an early_param, and only change the 
default based on the config varables?   It sounds like I might want to 
reclaim this memory even if I compiled in the driver.

Statement (a) is because one has to write the flag to say don't' boot 
OtherOS to go back to GameOS (or reset all defaults) I presume.

milton

^ permalink raw reply

* Re: Regarding MPC8540 IRQ Issue
From: sudheer @ 2007-06-18 14:37 UTC (permalink / raw)
  To: Bhupender Saharan; +Cc: linuxppc-embedded
In-Reply-To: <720399a30706161716p33411412w539dafa91923c74d@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 2734 bytes --]

Hi Bhupi
Thanks for the response.
I tried with all the free irqs as per  /proc/interrupts  in request_irq 
function one in each trial.  But in none of them it jumped to interrupt 
handler.
Please let me know whether this is the one you said to try or anything 
else. ?

Thanks
Sudheer

Bhupender Saharan wrote:
> Hi Sudhir,
>
> From the PCI dump it looks like IRQ PIN register is 0. During 
> enumeration when BIOS sees that IRQ PIN register is 0, it would not 
> allocate any interrupt for this card and that's why you are seeing IRQ 
> Line register also as 0 value.
>
> We need to do some work around for this.
>
>  IN the driver you need to register for all the pci vectors( INTA,B,C 
> and D). You would be called for any interrupt happening on the bus. 
> you have to check if the interrupt is for you and then claim it 
> otherwise return that interrupt is not for you.
>
>
> Regards
> Bhupi
>
>
> On 6/16/07, *sudheer* <urwithsudheer@gmail.com 
> <mailto:urwithsudheer@gmail.com>> wrote:
>
>     Hello All
>
>     I am working on MPC8540 board placing it in the PCI slot of the
>     x86 system.
>     After system bootup , i could see the powerpc board detected and pci
>     config space configured.
>     But i could not find any IRQ assigned for it.
>
>     SetUp:
>     Host: x86 System- Linux-2.6.9
>     HOST PCI Slot is 64-bit, 66MHz
>     Agent: MPC8540 Board.
>
>     Here is dump of "lspci -vx " for this device.
>
>     04:03.0 Power PC: Motorola MPC8540 (rev 20) (prog-if 01)
>             Flags: bus master, 66Mhz, fast devsel, latency 64
>             Memory at dee00000 (32-bit, non-prefetchable) [size=1M]
>             Capabilities: [60] #00 [0000]
>     00: 57 10 08 00 46 01 b0 00 20 01 20 0b 00 40 00 00
>     10: 00 00 e0 de 00 00 00 00 00 00 00 00 00 00 00 00
>     20: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>     30: 00 00 00 00 60 00 00 00 00 00 00 00 00 00 00 00
>
>     From the above dump - IRQ Pin & Line of config space shows zeroes.
>
>     I tried writing a small module and probing for IRQ generating a
>     message
>     interrupt. Though i could see in the 8540 message status register that
>     interrupt is generated, i could not get any IRQ when i do the
>     probe. For
>     this trial,  i have configured the PIC message enable register,
>     message
>     vector/priority and destination registers, processor current task
>     priority register (CPTR).
>
>     Can anyone give me some suggestions to try out.
>
>     Thanks
>     Sudheer
>
>     _______________________________________________
>     Linuxppc-embedded mailing list
>     Linuxppc-embedded@ozlabs.org <mailto:Linuxppc-embedded@ozlabs.org>
>     https://ozlabs.org/mailman/listinfo/linuxppc-embedded
>
>


[-- Attachment #2: Type: text/html, Size: 3621 bytes --]

^ permalink raw reply

* Re: [patch 30/33] PS3: Bootwrapper support.
From: Milton Miller @ 2007-06-18 14:20 UTC (permalink / raw)
  To: Geoff Levand; +Cc: ppcdev
In-Reply-To: <46730D8A.9090309@am.sony.com>

On Sat Jun 16 08:07:06 EST 2007, Geoff Levand wrote:
> zImage.ps3 is a wrapped image that contains a flat device tree, an lv1
> compatible entry point, a wrapper program with a standard entry point,
> and an optional initrd.  otheros.bld is the gzip compresed rom image 
> built
> from zImage.ps3.  otheros.bld is suitable for programming into the PS3 
> boot
> flash memory.
>
> Also remove the now unneeded lmb calls in the platform code.
>
> Signed-off-by: Geoff Levand <geoffrey.levand at am.sony.com>
> ---
>  arch/powerpc/boot/Makefile         |   21 ++--
>  arch/powerpc/boot/ps3-head.S       |   84 ++++++++++++++++
>  arch/powerpc/boot/ps3-hvcall.S     |  184 
> +++++++++++++++++++++++++++++++++++++
>  arch/powerpc/boot/ps3.c            |  158 
> +++++++++++++++++++++++++++++++
>  arch/powerpc/boot/wrapper          |   55 +++++++++++
>  arch/powerpc/boot/zImage.ps3.lds.S |   50 ++++++++++
>  arch/powerpc/platforms/ps3/mm.c    |    2
>  7 files changed, 543 insertions(+), 11 deletions(-)
>
> --- a/arch/powerpc/boot/Makefile
> +++ b/arch/powerpc/boot/Makefile
> @@ -41,11 +41,13 @@ zliblinuxheader := zlib.h zconf.h zutil.
>  $(addprefix $(obj)/,$(zlib) gunzip_util.o main.o): \
>         $(addprefix $(obj)/,$(zliblinuxheader)) $(addprefix 
> $(obj)/,$(zlibheader))
>
> +src-plat-$(CONFIG_PPC_PS3) += ps3-head.S ps3-hvcall.S ps3.c
> +
>  src-wlib := string.S crt0.S stdio.c main.c flatdevtree.c 
> flatdevtree_misc.c \
>                 ns16550.c serial.c simple_alloc.c div64.S util.S \
>                 gunzip_util.c elf_util.c $(zlib) devtree.c \
>                 44x.c ebony.c mv64x60.c mpsc.c mv64x60_i2c.c
> -src-plat := of.c cuboot-83xx.c cuboot-85xx.c holly.c \
> +src-plat := $(src-plat-y) of.c cuboot-83xx.c cuboot-85xx.c holly.c \
>                 cuboot-ebony.c treeboot-ebony.c prpmc2800.c
>  src-boot := $(src-wlib) $(src-plat) empty.c

The policy so far has been code for all platforms is compiled for all 
invocations of make in the boot directory.  Therefore just add your 
files directly to src-plat.  If needed, add a flags override to set the 
cpu type.

>
> @@ -75,11 +77,11 @@ $(addprefix $(obj)/,$(zliblinuxheader)):
>  $(obj)/empty.c:
>         @touch $@
>
> -$(obj)/zImage.lds $(obj)/zImage.coff.lds: $(obj)/%: 
> $(srctree)/$(src)/%.S
> +$(obj)/zImage.lds $(obj)/zImage.coff.lds $(obj)/zImage.ps3.lds: 
> $(obj)/%: $(srctree)/$(src)/%.S
>         @cp $< $@
>
>  clean-files := $(zlib) $(zlibheader) $(zliblinuxheader) \
> -               empty.c zImage.coff.lds zImage.lds
> +               empty.c zImage zImage.coff.lds zImage.ps3.lds 
> zImage.lds
>
>  quiet_cmd_bootcc = BOOTCC  $@
>        cmd_bootcc = $(CROSS32CC) -Wp,-MD,$(depfile) $(BOOTCFLAGS) -c 
> -o $@ $<
> @@ -102,7 +104,7 @@ hostprogs-y := addnote addRamDisk hack-c
>
>  targets                += $(patsubst $(obj)/%,%,$(obj-boot) wrapper.a)
>  extra-y                := $(obj)/wrapper.a $(obj-plat) $(obj)/empty.o 
> \
> -                  $(obj)/zImage.lds $(obj)/zImage.coff.lds
> +                  $(obj)/zImage.lds $(obj)/zImage.coff.lds 
> $(obj)/zImage.ps3.lds
>
>  wrapper                :=$(srctree)/$(src)/wrapper
>  wrapperbits    := $(extra-y) $(addprefix $(obj)/,addnote hack-coff 
> mktree) \
> @@ -187,11 +189,12 @@ $(obj)/zImage.%: vmlinux $(wrapperbits)
>  $(obj)/zImage.iseries: vmlinux
>         $(STRIP) -s -R .comment $< -o $@
>
> -$(obj)/zImage.ps3: vmlinux
> -       $(STRIP) -s -R .comment $< -o $@
> +$(obj)/zImage.ps3: vmlinux  $(wrapper) $(wrapperbits) 
> $(srctree)/$(src)/dts/ps3.dts
> +       $(STRIP) -s -R .comment $< -o vmlinux.strip
> +       $(call cmd,wrap,ps3,$(srctree)/$(src)/dts/ps3.dts,,)
>
> -$(obj)/zImage.initrd.ps3: vmlinux
> -       @echo "  WARNING zImage.initrd.ps3 not supported (yet)"
> +$(obj)/zImage.initrd.ps3: vmlinux  $(wrapper) $(wrapperbits) 
> $(srctree)/$(src)/dts/ps3.dts $(obj)/ramdisk.image.gz
> +       $(call 
> cmd,wrap,ps3,$(srctree)/$(src)/dts/ps3.dts,,$(obj)/ramdisk.image.gz)

The separate rule here is to pick up ps3.dts, correct?

After Marc's 3 patch series this would be handled by the config file.

>
>  $(obj)/zImage.holly-elf: vmlinux $(wrapperbits)
>         $(call if_changed,wrap,holly,$(obj)/dts/holly.dts,,)
> @@ -230,7 +233,7 @@ install: $(CONFIGURE) $(addprefix $(obj)
>
>  # anything not in $(targets)
>  clean-files += $(image-) $(initrd-) zImage zImage.initrd cuImage.* \
> -       treeImage.* zImage.dts zImage.dts_initrd
> +       treeImage.* zImage.dts zImage.dts_initrd otheros.bld
>
>  # clean up files cached by wrapper
>  clean-kernel := vmlinux.strip vmlinux.bin
> --- /dev/null
> +++ b/arch/powerpc/boot/ps3-head.S
> @@ -0,0 +1,84 @@
> +/*
> + *  PS3 bootwrapper entry.
> + *
> + *  Copyright (C) 2007 Sony Computer Entertainment Inc.
> + *  Copyright 2007 Sony Corp.
> + *
> + *  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., 59 Temple Place, Suite 330, Boston, MA  
> 02111-1307  USA
> + */
> +
> +#include "ppc_asm.h"
> +
> +       .text
> +
> +/*
> + * __system_reset_overlay - The PS3 first stage entry.
> + *
> + * The bootwraper build script copies the 0x100 bytes at symbol
> + * __system_reset_overlay to offset 0x100 of the rom image.
> + */
> +
> +       .globl __system_reset_overlay
> +__system_reset_overlay:
> +
> +       /* Switch to 32-bit mode. */
> +
> +       mfmsr   r9
> +       clrldi  r9,r9,1
> +       mtmsrd  r9
> +       nop
> +
> +       /* Get thread number in r3 and branch. */
> +
> +       mfspr   r3, 0x88
> +       cntlzw. r3, r3
> +       li      r4, 0
> +       li      r5, 0
> +       beq     1f
> +
> +       /* Secondary goes to __secondary_hold in kernel. */
> +
> +       li      r4, 0x60
> +       mtctr   r4
> +       bctr
> +
> +       /* Primary waits for __secondary_hold_acknowledge. */
> +1:
> +       li      r5, 0x10 /* __secondary_hold_acknowledge */
> +       or      28, 28, 28 /* db8cyc */
> +
> +       ld      r4, 0(r5)
> +       cmpdi   r4, 0
> +       beq     1b

(1) The address of __secondary_hold_acknowledge is not stable.  It has 
changed in the past and may change in the future.  (2) This works 
because you always have exactly 2 cpus, and your secondary cpu is not 
id 0.

Because this is part of the kernel source tree, I'm not opposed to this 
usage, but I think some checking would be in order.   A comment that 
this only works for two cpus with the slave id not being 0, and some 
check that nm on the kernel image results in the expected symbol and 
value.

Since I'm here, I'll mention that the li to r5 doesn't need to be in 
the loop.  In fact, the load to r4 could be expressed as 0x10(0) or 
better create a symbol and use secondary_ack@l(0) eliminating the need 
for r5 and easing the above check.

> +
> +       /* Primary goes to _zimage_start in wrapper. */
> +
> +       lis     r4, _zimage_start at ha
> +       addi    r4, r4, _zimage_start at l
> +       mtctr   r4
> +       bctr
> +
> +/*
> + * __system_reset_kernel - Place holder for the kernel reset vector.
> + *
> + * The bootwrapper build script copies 0x100 bytes from offset 0x100
> + * of the rom image to the symbol __system_reset_kernel.  At runtime
> + * the bootwrapper program copies the 0x100 bytes at 
> __system_reset_kernel
> + * to ram address 0x100.  This symbol must occupy 0x100 bytes.
> + */
> +
> +       .globl __system_reset_kernel
> +__system_reset_kernel:
> +
> +       . = __system_reset_kernel + 0x100

Not bad.   Since this code doesn't have to be at any particular 
location when it is built (you have no .org or similar, its all PIC), I 
don't see anything that couldn't be done from an asm block in the 
platform .c file, like the entry point in prpmc2800.c file; that would 
save a file in the boot directory.  In that case __system_reset_kernel 
would be an array of 256 bytes.

[removed ps3-hvcall.S and lmb hack removal from followup]
> --- /dev/null
> +++ b/arch/powerpc/boot/ps3.c
> @@ -0,0 +1,158 @@
> +/*
> + *  PS3 bootwrapper support.
> + *
> + *  Copyright (C) 2007 Sony Computer Entertainment Inc.
> + *  Copyright 2007 Sony Corp.
> + *
> + *  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., 59 Temple Place, Suite 330, Boston, MA  
> 02111-1307  USA
> + */
> +
> +#include <stdarg.h>
> +#include <stddef.h>
> +#include "types.h"
> +#include "elf.h"
> +#include "string.h"
> +#include "stdio.h"
> +#include "page.h"
> +#include "ops.h"
> +
> +extern s64 lv1_panic(u64 in_1);
> +extern s64 lv1_get_logical_partition_id(u64 *out_1);
> +extern s64 lv1_get_logical_ppe_id(u64 *out_1);
> +extern s64 lv1_get_repository_node_value(u64 in_1, u64 in_2, u64 in_3,
> +       u64 in_4, u64 in_5, u64 *out_1, u64 *out_2);
> +
> +#ifdef DEBUG
> +#define DBG(fmt...) printf(fmt)
> +#else
> +static inline int __attribute__ ((format (printf, 1, 2))) DBG(
> +       const char *fmt, ...) {return 0;}
> +#endif

This should be in stdio.h for all.  Maybe ops, but that is a bit of a 
catch-all and printf is not there.

> +
> +BSS_STACK(4096);
> +
> +/* A buffer that may be edited by tools operating on a zImage binary 
> so as to
> + * edit the command line passed to vmlinux (by setting 
> /chosen/bootargs).
> + * The buffer is put in it's own section so that tools may locate it 
> easier.
> + */
> +static char cmdline[COMMAND_LINE_SIZE]
> +       __attribute__((__section__("__builtin_cmdline")));
> +
> +static void prep_cmdline(void *chosen)
> +{
> +       if (cmdline[0] == '\0')
> +               getprop(chosen, "bootargs", cmdline, 
> COMMAND_LINE_SIZE-1);
> +       else
> +               setprop_str(chosen, "bootargs", cmdline);
> +
> +       printf("cmdline: '%s'\n", cmdline);
> +}
> +
> +static void ps3_console_write(const char *buf, int len)
> +{
> +}

I'm guessing your testing was done with some non-empty function that 
you are not prepared to release.  Otherwise you would not have added 
all the printf code.   I'm not complaining, just noting for my 
observations below.

> +
> +static void ps3_exit(void)
> +{
> +       printf("ps3_exit\n");
> +       lv1_panic(0); /* zero = no reboot */
> +       while (1);
> +}

Does the hypervisor give some kind of indication that the code paniced? 
  A comment telling us users what to expect might be nice.

> +
> +static int ps3_repository_read_rm_size(u64 *rm_size)
> +{
> +       s64 result;
> +       u64 lpar_id;
> +       u64 ppe_id;
> +       u64 v2;
> +
> +       result = lv1_get_logical_partition_id(&lpar_id);
> +
> +       if (result)
> +               return -1;
> +
> +       result = lv1_get_logical_ppe_id(&ppe_id);
> +
> +       if (result)
> +               return -1;
> +
> +       /*
> +        * n1: 0000000062690000 : ....bi..
> +        * n2: 7075000000000000 : pu......
> +        * n3: 0000000000000001 : ........
> +        * n4: 726d5f73697a6500 : rm_size.
> +       */
> +
> +       result = lv1_get_repository_node_value(lpar_id, 
> 0x0000000062690000ULL,
> +               0x7075000000000000ULL, ppe_id, 0x726d5f73697a6500ULL, 
> rm_size,
> +               &v2);
> +
> +       printf("%s:%d: ppe_id  %lu \n", __func__, __LINE__,
> +               (unsigned long)ppe_id);
> +       printf("%s:%d: lpar_id %lu \n", __func__, __LINE__,
> +               (unsigned long)lpar_id);
> +       printf("%s:%d: rm_size %llxh \n", __func__, __LINE__, 
> *rm_size);
> +
> +       return result ? -1 : 0;
> +}
> +
> +void ps3_copy_vectors(void)
> +{
> +       extern char __system_reset_kernel[];
> +
> +       memcpy((void *)0x100, __system_reset_kernel, 0x100);
> +       flush_cache((void *)0x100, 0x100);
> +}
> +
> +void platform_init(void)
> +{
> +       extern char _end[];
> +       extern char _dtb_start[];
> +       extern char _initrd_start[];
> +       extern char _initrd_end[];
> +       const u32 heapsize = 0x1000000 - (u32)_end; /* 16MiB */

Ok your heap is from (wrapper) _end to 16MiB.

> +       void *chosen;
> +       unsigned long ft_addr;
> +       u64 rm_size;
> +
> +       console_ops.write = ps3_console_write;
> +       platform_ops.exit = ps3_exit;
> +
> +       printf("\n-- PS3 bootwrapper --\n");
> +
> +       simple_alloc_init(_end, heapsize, 32, 64);
> +       ft_init(_dtb_start, 0, 4);

Ok this is the end of platform_init for most platforms.

> +
> +       chosen = finddevice("/chosen");
> +
> +       ps3_repository_read_rm_size(&rm_size);
> +       dt_fixup_memory(0, rm_size);

this is dt_fixups.

> +
> +       if (_initrd_end > _initrd_start) {
> +               setprop_val(chosen, "linux,initrd-start", 
> (u32)(_initrd_start));
> +               setprop_val(chosen, "linux,initrd-end", 
> (u32)(_initrd_end));
> +       }

This is part of prep_initrd.

> +
> +       prep_cmdline(chosen);
> +
> +       ft_addr = dt_ops.finalize();
> +
> +       ps3_copy_vectors();
> +
> +       printf(" flat tree at 0x%lx\n\r", ft_addr);
> +
> +       ((kernel_entry_t)0)(ft_addr, 0, NULL);
> +
> +       ps3_exit();
> +}

Based on past comments, I belive that you are not calling the normal 
_start() specifically to avoid dragging in zlib.  Oh, but you call 
prep_cmdline, which means you still get main.c and it drags zlib.  I 
guess we should move prep_initrd and prep_cmdline.  Were you just 
skipping prep_kernel?

This code and the concept in your .lds file looks like it could be used 
by other platforms in a generic way
(except for the call to ps3_exit which should be exit, and the call to 
ps3_copy_vectors, which can be moved earlier.  While its likely similar 
functionality would be needed, the details of what to patch and copy 
may vary).

> --- a/arch/powerpc/boot/wrapper
> +++ b/arch/powerpc/boot/wrapper
> @@ -144,6 +144,15 @@ miboot|uboot)
>  cuboot*)
>      gzip=
>      ;;
> +ps3)
> +    platformo="$object/ps3-head.o $object/ps3-hvcall.o $object/ps3.o"
> +    lds=$object/zImage.ps3.lds
> +    gzip=
> +    ext=bin
> +    objflags="-O binary"
> +    ksection=.kernel:vmlinux.bin
> +    isection=.kernel:initrd
> +    ;;
>  esac
>
>  vmz="$tmpdir/`basename \"$kernel\"`.$ext"
> @@ -239,4 +248,50 @@ treeboot*)
>      fi
>      exit 0
>      ;;
> +ps3)
> +    # The ps3's loader supports loading gzipped binary images from 
> flash
> +    # rom to addr zero. The loader enters the image at addr 0x100.  A
> +    # bootwrapper overlay is use to arrange for the kernel to be 
> loaded
> +    # to addr zero and to have a suitable bootwrapper entry at 0x100.
> +    # To construct the rom image, 0x100 bytes from offset 0x100 in the
> +    # kernel is copied to the bootwrapper symbol 
> __system_reset_kernel.
> +    # The 0x100 bytes at the bootwrapper symbol 
> __system_reset_overlay is
> +    # then copied to offset 0x100.  At runtime the bootwrapper program
> +    # copies the 0x100 bytes at __system_reset_kernel to addr 0x100.
> +
> +    system_reset_overlay=0x`${CROSS}nm "$ofile" \
> +        | grep ' __system_reset_overlay$'       \
> +        | cut -d' ' -f1`
> +    system_reset_overlay=`printf "%d" $system_reset_overlay`
> +    system_reset_kernel=0x`${CROSS}nm "$ofile" \
> +        | grep ' __system_reset_kernel$'       \
> +        | cut -d' ' -f1`
> +    system_reset_kernel=`printf "%d" $system_reset_kernel`
> +    overlay_dest="256"
> +    overlay_size="256"
> +
> +    rm -f "$object/otheros.bld"
> +
> +    ${CROSS}objcopy -O binary "$ofile" "$ofile.bin"
> +
> +    msg=$(dd if="$ofile.bin" of="$ofile.bin" conv=notrunc \
> +        skip=$overlay_dest seek=$system_reset_kernel      \
> +        count=$overlay_size bs=1 2>&1)
> +
> +    if [ $? -ne "0" ]; then
> +       echo $msg
> +       exit 1
> +    fi
> +
> +    msg=$(dd if="$ofile.bin" of="$ofile.bin" conv=notrunc \
> +        skip=$system_reset_overlay seek=$overlay_dest     \
> +        count=$overlay_size bs=1 2>&1)
> +
> +    if [ $? -ne "0" ]; then
> +       echo $msg
> +       exit 2
> +    fi
> +
> +    gzip --force -9 --stdout "$ofile.bin" > "$object/otheros.bld"
> +    ;;
>  esac
> --- /dev/null
> +++ b/arch/powerpc/boot/zImage.ps3.lds.S
> @@ -0,0 +1,50 @@
> +OUTPUT_ARCH(powerpc:common)
> +ENTRY(_zimage_start)
> +EXTERN(_zimage_start)
> +SECTIONS
> +{
> +  _vmlinux_start =  .;
> +  .kernel:vmlinux.bin : { *(.kernel:vmlinux.bin) }
> +  _vmlinux_end =  .;
> +
> +  . = ALIGN(8);
> +  _dtb_start = .;
> +  .kernel:dtb : { *(.kernel:dtb) }
> +  _dtb_end = .;
> +
> +  . = ALIGN(4096);
> +  _initrd_start =  .;
> +  .kernel:initrd : { *(.kernel:initrd) }
> +  _initrd_end =  .;
> +
> +  _start = .;
> +  .text      :
> +  {
> +    *(.text)
> +    *(.fixup)
> +  }
> +  _etext = .;
> +  . = ALIGN(4096);
> +  .data    :
> +  {
> +    *(.rodata*)
> +    *(.data*)
> +    *(.sdata*)
> +    __got2_start = .;
> +    *(.got2)
> +    __got2_end = .;
> +  }
> +
> +  . = ALIGN(4096);
> +  _edata  =  .;
> +
> +  . = ALIGN(4096);
> +  __bss_start = .;
> +  .bss       :
> +  {
> +   *(.sbss)
> +   *(.bss)
> +  }
> +  . = ALIGN(4096);
> +  _end = . ;
> +}

You don't seem to relocate the initrd.  Your lds file has the initrd on 
the 4k page boundary after the dtb, which is after the kernel as copied 
by objcopy -O binary.  As far as I know, objcopy only copies the load 
sections; it doesn't create space in the output binary image for alloc 
only sections.  What prevents the initrd from being in the kernel bss, 
which it clears before even looking at the device tree?  For that 
matter, what prevents the device tree from being allocated in this bss 
range?

I'm guessing that you tested with some additional code to display the 
console output, which made _end be above the kernel _end, or close 
enough that the finalized bss was above the kernel _end?  Did you test 
an initrd?  I guess an attached initrd would also solve the problem of 
the final dtb being overwritten, but not the start of the initrd.


I think your approach to the image layout has promise but still needs 
some refinement.  To solve the problem of where is the kernel end, I 
would create a section to hold the kernel elf header and use the 
elf_parse routines that are now conveniently in a separate file.  The 
wrapper script would dd off the header and then it would be linked into 
the wrapper like the other sections.  At that point the initrd could be 
moved before creating the malloc pool above it.

This approach can be used whenever there is something capable of 
installing an image at address 0 in ram before starting its execution, 
and is useful when the time to install that image is less than the time 
to decompress the kernel (and copy either it or the zImage, for the 
case where the entry point is fixed in low memory).  I can think of 
other platforms where this holds true.  In your case, this is true 
because the hypervisor decompresses the image.

What should this type of image be called?  Its not really a zImage, 
since there is nothing compressed (well, maybe the initrd/initramfs).  
uImage is already taken (u would have stood for uncompressed).  
Something like wrapped image?  flat image?  combined image?  We'd still 
do make zImage to invoke make for it.

One more point.  You indicated that this image had a standard entry 
point.  Were you thinking it will be used for kexec?  Or were you just 
trying to state you used the library _zimage_start?  While the current 
kexec-tools for 64-bit assumes the entry point to be image offset 0, 
the same address is used for both the boot cpu entry point and the 
address of the slave code to copy to address 0.  If you want the master 
to execute the wrapper code, you will need to patch the branch at 
address 0 in the kernel image to go to _zimage_start in the wrapper, 
and then from the wrapper patch it back before starting the kernel.  Or 
perhaps you don't plan to run the wrapper on the kexec path?  In that 
case the system reset handler would retained the patched handler and 
the initrd ignored would be ignored.  While you usage may not care, 
other platforms use the system reset vector.  In either case, 
_zimage_start doesn't have the necessary slave code, and therefore 
shouldn't be the entry point for the final image when we fix that bug 
in kexec-tools.


milton

^ permalink raw reply

* Re: [patch 23/33] powerpc: Localize mmu_off
From: Milton Miller @ 2007-06-18 14:08 UTC (permalink / raw)
  To: Geoff Levand; +Cc: ppcdev
In-Reply-To: <46730D5B.300@am.sony.com>

On Sat Jun 16 08:06:19 EST 2007, Geoff Levand wrote:
> This just removes the dependency __mmu_off has on the symbol
> __after_prom_start.  I found the current code inconvenient when I
> wanted to put some debugging code between the call to __mmu_off
> and the branch to __after_prom_start.

You are entering this code with the mmu on?

My first thought was that is unexpected, but aparently Apple machines 
do that, so its supported for the master -- not the slaves.

> Signed-off-by: Geoff Levand <geoffrey.levand at am.sony.com>
> ---
>  arch/powerpc/kernel/head_64.S |    4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> --- a/arch/powerpc/kernel/head_64.S
> +++ b/arch/powerpc/kernel/head_64.S
> @@ -1695,9 +1695,11 @@ _GLOBAL(__start_initialization_multiplat
>  2:
>
>         /* Switch off MMU if not already */
> -       LOAD_REG_IMMEDIATE(r4, .__after_prom_start - KERNELBASE)
> +       LOAD_REG_IMMEDIATE(r4, __mmu_off_return - KERNELBASE)
>         add     r4,r4,r30
>         bl      .__mmu_off
> +__mmu_off_return:
> +
>         b       .__after_prom_start
>
>  _STATIC(__boot_from_prom)
>
> -- 

That seems like a long label.

I could stand a 1: there (ie a local numeric label).

Hmm... how about always going directly there:

-       bl      .__mmu_off
-       b       .__after_prom_start
+       mtlr    r4
+       b       .__mmu_off


So that there is no space to add code that is skipped if the mmu was on?

milton

^ permalink raw reply

* Re: [patch 24/33] powerpc: Correct __secondary_hold comment
From: Milton Miller @ 2007-06-18 14:08 UTC (permalink / raw)
  To: Geoff Levand; +Cc: ppcdev
In-Reply-To: <46730D5F.4040701@am.sony.com>

On Sat Jun 16 08:06:23 EST 2007, Geoff Levand wrote:
> Remove references to pSeries and OpenFirmware in the __secondary_hold
> usage comment.  __secondary_hold is a generic routine and can be used
> by other platforms.

The comment is correct, just incomplete.  Well, the part that you 
changed anyways.  How about changing the subject to:

Expand comment for other uses of __secondary_hold.

>
> Signed-off-by: Geoff Levand <geoffrey.levand at am.sony.com>
> ---
>  arch/powerpc/kernel/head_64.S |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> --- a/arch/powerpc/kernel/head_64.S
> +++ b/arch/powerpc/kernel/head_64.S
> @@ -103,8 +103,8 @@ __secondary_hold_acknowledge:
>
>         . = 0x60
>  /*
> - * The following code is used on pSeries to hold secondary processors
> - * in a spin loop after they have been freed from OpenFirmware, but
> + * The following code is used to hold secondary processors
> + * in a spin loop after they have entered the kernel, but
>   * before the bulk of the kernel has been relocated.  This code
>   * is relocated to physical address 0x60 before prom_init is run.
>   * All of it must fit below the first exception vector at 0x100.
>

I don't like the resulting wording.  In particular, I don't like "after 
they have entered the kernel", as most of the kernel is not available 
to be entered.  In addition, the unchanged part of the comment refers 
to relocating from offset 0x60-0x100 to address 0x60, but in fact 
offset 0x0-0x100 is placed at address 0 (0x60 is just the entry point), 
and its done from within prom_init not before.

How does this text sound?

This code starting at offset 0x60 is used to hold secondary processors 
before the kernel has been relocated to its linked address.  Any 
platform or pre-boot environment may transfer secondary processors to 
the kernel by copying the first 0x100 bytes to address 0, and then 
instructing the secondary cpus to branch to address 0x60 after placing 
their hardware id (as specified by the device tree) in r3.  When 
booting from OpenFirmware using the client interface, the copy is 
performed by the boot cpu in prom_init and then the start-cpu method is 
used to transfer the cpus to this loop.  Although 
__secondary_hold_acknoledge is used serially to provide a positive 
handshake that each cpu has started, neither its address nor the 
protocol is considered part of the kernel entry ABI and should not be 
relied on by external code.  The code and data for the secondary cpus 
must be contained in bytes 0x4 - 0x100, that is before the first 
exception vector and after at least one instruction for the master 
entrypoint.

milton

^ permalink raw reply

* Re: [PATCH] rheap: eliminates internal fragments caused by alignment
From: Joakim Tjernlund @ 2007-06-18 13:24 UTC (permalink / raw)
  To: Pantelis Antoniou; +Cc: linuxppc-dev Development, Li Yang, Paul, Vitaly Bordug
In-Reply-To: <DA299F48-E5A6-495C-8169-A211D4C83864@gmail.com>

On Mon, 2007-06-18 at 15:31 +0300, Pantelis Antoniou wrote:
> On 18 =CE=99=CE=BF=CF=85=CE=BD 2007, at 3:25 =CE=9C=CE=9C, Joakim Tjernlu=
nd wrote:
>=20
> > On Mon, 2007-06-18 at 15:14 +0300, Pantelis Antoniou wrote:
> >> Hi Li,
> >>
> >> The patch appears fine at first glance.
> >>
> >> I'm really curious what kind of alignment do you use in order to =20
> >> have so
> >> much wasted space that re-inserting the leftovers makes so much of a
> >> difference.
>=20
> Jocke,
>=20
> Ugh, 256?

Yes, the ucc_geth needs lots of alignment. Leo can tell you more :)

>=20
> That's not nice at all.
> I'm not sure that even then you're going to have any guarantees that
> the allocations will be successful.
>=20
> I'm afraid that in those case it would be better for the board
> platform setup to have the space preallocated according to the BSP
> requirements.

Maybe, but without the PATCH you will run out of space very quickly
and you cannot bring up 3 ethernet interfaces. Rheap is plain wrong
as is today and needs this patch.

 Jocke

>=20
> Just my $0.02.
>=20
> -- Pantelis

^ permalink raw reply

* Re: [PATCH] boot: find initrd location from device-tree
From: Segher Boessenkool @ 2007-06-18 12:52 UTC (permalink / raw)
  To: Milton Miller; +Cc: ppcdev, Paul Mackerras, David Gibson
In-Reply-To: <fbae4506b772f07d2a8c1d230bea82c8@bga.com>

>>> +     /* if the initrd is above 4G, its untouchable in 32 bit mode */
>>> +     if (initrd_end <= UINT_MAX && initrd_start < initrd_end) {
>>> +             loader_info.initrd_addr = initrd_start;
>>> +             loader_info.initrd_size  = initrd_end - initrd_start;
>>> +     } else {
>>> +             printf("ignoring loader supplied initrd parameters\n");
>>
>> Saying why might be helpful.
>
> Hmm... well, that would mean seperating the && of the if; there are two
> possible reasons. (1) this 32-bit code can't handle such large
> addresses (in which case a 64-bit kernel may work), and (2) end is >=
> start (which means there really isn't any data.  I suppose I could
> reverse the sense of the if, use an else if, and make the final else
> the good path.

"ignoring loader supplied nonsensical initrd parameters"

You do need to fix the comment, though :-)

>>> +#define _LIBC_LIMITS_H_              /* don't recurse to system's
>> headers */
>>> +#include <limits.h>          /* MAX_UINT, etc */
>>> +
>>
>> I think it's really a bad idea to use any headers from outside the
>> boot context here.  We're dealing with explicit sized ints, so we can
>> safely define our own constants giving the limit values.
>
> As I said in the patch changelog, the only headers picked up here are
> libgcc.

Which the kernel doesn't use.  It _should_ be fine
nevertheless, <limits.h> is part of the "freestanding"
headers -- why do you need to do a "libc" trick though?
This doesn't work when not using glibc I guess?


Segher

^ permalink raw reply

* Re: [PATCH 1/5] Add the explanation and sample of RapidIO DTS sector to the document of booting-without-of.txt file.
From: Segher Boessenkool @ 2007-06-18 12:43 UTC (permalink / raw)
  To: Zhang Wei-r63237; +Cc: linuxppc-dev, paulus, linux-kernel
In-Reply-To: <46B96294322F7D458F9648B60E15112C527171@zch01exm26.fsl.freescale.net>

>> You don't need to mention _all_ compatible devices in
>> the "compatible" property, only the few that matter;
>> typically the oldest one, and sometimes some intermediate
>> device that has extra features over the original one.
>
> The oldest one is difficult to find out sometime. Can we only set the
> self name in dts, such as "fsl, rapidio-8641", and add this 
> 'compatible'
> property to the driver ids arrays?

You can do that, but you typically don't need to -- the
whole idea of "compatible" is to avoid this, and not need
to have huge "pci id" style tables in the device drivers
that need constant updating.  But you _can_ do it, sure.

> Such as:
>
> static struct of_device_id of_rio_rpn_ids[] = {
> 	{ .compatible = "fsl, rapidio-8540",},
> 	{ .compatible = "fsl, rapidio-8560",},
> 	{ .compatible = "fsl, rapidio-8641",},
> 	{ .compatible = "fsl, rapidio-8548",},
> 	{},
> };
>
> How about that?

I would just put 8540 in the table and in all device trees
in this case.

>> It isn't useful to add "compatible" entries that no OS
>> probes for.
>>
>>>> Concrete names are good.
>>>
>>> While I agree concrete names are good, we put these 'blocks' in so
>>> many devices that using the device to match on is pointless.
>>
>> You *definitely* should put the device name for _this_
>> device in there, in case it needs some special workaround.
>>
>>> I'm all for making up a name like 'Grande', 'Del',
>> 'Janeiro'.  This is
>>> effective what we did with gianfar.  The name gets picked up pretty
>>> quickly by people.
>>
>> That can be used as the "base" name, yes.
>
> Do you have the name list? I can change my codes according them.

Nope.

> How about 'Mercurary', 'Venus', 'Earth', 'Mars', 'Saturn', 'Jupiter',
> 'Uranus', 'Neptune',
> Or 'Aries', 'Taurus', 'Gemini', 'Cancer', 'Leo', 'Virgo', 'Libra',
> 'Scorpius', 'Sagittarius', 'Capricornus', 'Aquarius', 'Pisces' ?

I don't like making up names just for this, I don't see what good
this would do.  Using a pre-existing code name is fine of course,
as long as it is a unique identifier, since that's all that matters.


Segher

^ permalink raw reply

* Re: [PATCH] rheap: eliminates internal fragments caused by alignment
From: Pantelis Antoniou @ 2007-06-18 12:31 UTC (permalink / raw)
  To: joakim.tjernlund; +Cc: linuxppc-dev Development, Li Yang, Paul, Vitaly Bordug
In-Reply-To: <1182169549.2969.20.camel@gentoo-jocke.transmode.se>


On 18 =CE=99=CE=BF=CF=85=CE=BD 2007, at 3:25 =CE=9C=CE=9C, Joakim =
Tjernlund wrote:

> On Mon, 2007-06-18 at 15:14 +0300, Pantelis Antoniou wrote:
>> Hi Li,
>>
>> The patch appears fine at first glance.
>>
>> I'm really curious what kind of alignment do you use in order to =20
>> have so
>> much wasted space that re-inserting the leftovers makes so much of a
>> difference.

Jocke,

Ugh, 256?

That's not nice at all.
I'm not sure that even then you're going to have any guarantees that
the allocations will be successful.

I'm afraid that in those case it would be better for the board
platform setup to have the space preallocated according to the BSP
requirements.

Just my $0.02.

-- Pantelis

> This I what you get for eth0 with patch,
> all hex, decimal within()
> MURAM alloc, start:400, size:200(512), align:8(8)
> MURAM alloc, start:600, size:208(520), align:8(8)
> MURAM alloc, start:880, size:80(128), align:80(128)
> MURAM alloc, start:900, size:1a0(416), align:100(256)
> MURAM alloc, start:aa0, size:40(64), align:20(32)
> MURAM alloc, start:b00, size:100(256), align:100(256)
> MURAM alloc, start:c00, size:a0(160), align:100(256)
> MURAM alloc, start:cc0, size:c(12), align:40(64)
> MURAM alloc, start:cd0, size:30(48), align:8(8)
> MURAM alloc, start:d00, size:80(128), align:80(128)
> MURAM alloc, start:d80, size:80(128), align:80(128)
> MURAM alloc, start:e00, size:80(128), align:80(128)
> MURAM alloc, start:e80, size:80(128), align:80(128)
> MURAM alloc, start:f00, size:40(64), align:40(64)
> MURAM alloc, start:f40, size:40(64), align:40(64)
> MURAM alloc, start:f80, size:40(64), align:40(64)
> MURAM alloc, start:fc0, size:40(64), align:40(64)
> MURAM alloc, start:1000, size:5d(93), align:4(4)
>
> And this is what you get without patch:
> MURAM alloc, start:1400, size:200(512), align:8(8)
> MURAM alloc, start:1608, size:208(520), align:8(8)
> MURAM alloc, start:1880, size:80(128), align:80(128)
> MURAM alloc, start:1a00, size:1a0(416), align:100(256)
> MURAM alloc, start:1bc0, size:40(64), align:20(32)
> MURAM alloc, start:1d00, size:100(256), align:100(256)
> MURAM alloc, start:1f00, size:a0(160), align:100(256)
> MURAM alloc, start:1fc0, size:c(12), align:40(64)
> MURAM alloc, start:2000, size:30(48), align:8(8)
> MURAM alloc, start:2080, size:80(128), align:80(128)
> MURAM alloc, start:2180, size:80(128), align:80(128)
> MURAM alloc, start:2280, size:80(128), align:80(128)
> MURAM alloc, start:2380, size:80(128), align:80(128)
> MURAM alloc, start:2440, size:40(64), align:40(64)
> MURAM alloc, start:24c0, size:40(64), align:40(64)
> MURAM alloc, start:2540, size:40(64), align:40(64)
> MURAM alloc, start:25c0, size:40(64), align:40(64)
> MURAM alloc, start:262c, size:5d(93), align:4(4)
>
>  Jocke

^ permalink raw reply

* Re: [PATCH] rheap: eliminates internal fragments caused by alignment
From: Joakim Tjernlund @ 2007-06-18 12:25 UTC (permalink / raw)
  To: Pantelis Antoniou; +Cc: linuxppc-dev Development, Li Yang, Paul, Vitaly Bordug
In-Reply-To: <03F6FDB0-3D7B-4272-AC1C-8E06F82DD815@gmail.com>

On Mon, 2007-06-18 at 15:14 +0300, Pantelis Antoniou wrote:
> Hi Li,
> 
> The patch appears fine at first glance.
> 
> I'm really curious what kind of alignment do you use in order to have so
> much wasted space that re-inserting the leftovers makes so much of a
> difference.
This I what you get for eth0 with patch,
all hex, decimal within()
MURAM alloc, start:400, size:200(512), align:8(8)
MURAM alloc, start:600, size:208(520), align:8(8)
MURAM alloc, start:880, size:80(128), align:80(128)
MURAM alloc, start:900, size:1a0(416), align:100(256)
MURAM alloc, start:aa0, size:40(64), align:20(32)
MURAM alloc, start:b00, size:100(256), align:100(256)
MURAM alloc, start:c00, size:a0(160), align:100(256)
MURAM alloc, start:cc0, size:c(12), align:40(64)
MURAM alloc, start:cd0, size:30(48), align:8(8)
MURAM alloc, start:d00, size:80(128), align:80(128)
MURAM alloc, start:d80, size:80(128), align:80(128)
MURAM alloc, start:e00, size:80(128), align:80(128)
MURAM alloc, start:e80, size:80(128), align:80(128)
MURAM alloc, start:f00, size:40(64), align:40(64)
MURAM alloc, start:f40, size:40(64), align:40(64)
MURAM alloc, start:f80, size:40(64), align:40(64)
MURAM alloc, start:fc0, size:40(64), align:40(64)
MURAM alloc, start:1000, size:5d(93), align:4(4)

And this is what you get without patch:
MURAM alloc, start:1400, size:200(512), align:8(8)
MURAM alloc, start:1608, size:208(520), align:8(8)
MURAM alloc, start:1880, size:80(128), align:80(128)
MURAM alloc, start:1a00, size:1a0(416), align:100(256)
MURAM alloc, start:1bc0, size:40(64), align:20(32)
MURAM alloc, start:1d00, size:100(256), align:100(256)
MURAM alloc, start:1f00, size:a0(160), align:100(256)
MURAM alloc, start:1fc0, size:c(12), align:40(64)
MURAM alloc, start:2000, size:30(48), align:8(8)
MURAM alloc, start:2080, size:80(128), align:80(128)
MURAM alloc, start:2180, size:80(128), align:80(128)
MURAM alloc, start:2280, size:80(128), align:80(128)
MURAM alloc, start:2380, size:80(128), align:80(128)
MURAM alloc, start:2440, size:40(64), align:40(64)
MURAM alloc, start:24c0, size:40(64), align:40(64)
MURAM alloc, start:2540, size:40(64), align:40(64)
MURAM alloc, start:25c0, size:40(64), align:40(64)
MURAM alloc, start:262c, size:5d(93), align:4(4)

 Jocke

^ permalink raw reply

* Re: [PATCH] rheap: eliminates internal fragments caused by alignment
From: Pantelis Antoniou @ 2007-06-18 12:14 UTC (permalink / raw)
  To: Li Yang; +Cc: linuxppc-dev Development, Paul, Vitaly Bordug
In-Reply-To: <46766C91.6090903@freescale.com>

Hi Li,

The patch appears fine at first glance.

I'm really curious what kind of alignment do you use in order to have so
much wasted space that re-inserting the leftovers makes so much of a
difference.

-- Pantelis


On 18 =CE=99=CE=BF=CF=85=CE=BD 2007, at 2:29 =CE=9C=CE=9C, Li Yang =
wrote:

> The patch adds fragments caused by rh_alloc_align() back to free =20
> list, instead
> of allocating the whole chunk of memory.  This will greatly improve =20=

> memory
> utilization managed by rheap.
>
> It solves MURAM not enough problem with 3 UCCs enabled on MPC8323.
>
> Signed-off-by: Li Yang <leoli@freescale.com>
> ---
> arch/powerpc/lib/rheap.c |   48 ++++++++++++++++++++++++++=20
> +------------------
> 1 files changed, 29 insertions(+), 19 deletions(-)
>
> diff --git a/arch/powerpc/lib/rheap.c b/arch/powerpc/lib/rheap.c
> index 180ee29..2f24ea0 100644
> --- a/arch/powerpc/lib/rheap.c
> +++ b/arch/powerpc/lib/rheap.c
> @@ -437,27 +437,26 @@ unsigned long rh_alloc_align(rh_info_t * =20
> info, int size, int alignment, const ch
> 	struct list_head *l;
> 	rh_block_t *blk;
> 	rh_block_t *newblk;
> -	unsigned long start;
> +	unsigned long start, sp_size;
> 	/* Validate size, and alignment must be power of two */
> 	if (size <=3D 0 || (alignment & (alignment - 1)) !=3D 0)
> 		return (unsigned long) -EINVAL;
> -	/* given alignment larger that default rheap alignment */
> -	if (alignment > info->alignment)
> -		size +=3D alignment - 1;
> -
> 	/* Align to configured alignment */
> 	size =3D (size + (info->alignment - 1)) & ~(info->alignment - =
1);
> -	if (assure_empty(info, 1) < 0)
> +	if (assure_empty(info, 2) < 0)
> 		return (unsigned long) -ENOMEM;
> 	blk =3D NULL;
> 	list_for_each(l, &info->free_list) {
> 		blk =3D list_entry(l, rh_block_t, list);
> -		if (size <=3D blk->size)
> -			break;
> +		if (size <=3D blk->size) {
> +			start =3D (blk->start + alignment - 1) & =
~(alignment - 1);
> +			if (start + size <=3D blk->start + blk->size)
> +				break;
> +		}
> 		blk =3D NULL;
> 	}
> @@ -470,25 +469,36 @@ unsigned long rh_alloc_align(rh_info_t * =20
> info, int size, int alignment, const ch
> 		list_del(&blk->list);
> 		newblk =3D blk;
> 	} else {
> +		/* Fragment caused, split if needed */
> +		/* Create block for fragment in the beginning */
> +		sp_size =3D start - blk->start;
> +		if (sp_size) {
> +			rh_block_t *spblk;
> +
> +			spblk =3D get_slot(info);
> +			spblk->start =3D blk->start;
> +			spblk->size =3D sp_size;
> +			/* add before the blk */
> +			list_add(&spblk->list, blk->list.prev);
> +		}
> 		newblk =3D get_slot(info);
> -		newblk->start =3D blk->start;
> +		newblk->start =3D start;
> 		newblk->size =3D size;
> -		/* blk still in free list, with updated start, size */
> -		blk->start +=3D size;
> -		blk->size -=3D size;
> +		/* blk still in free list, with updated start and size
> +		 * for fragment in the end */
> +		blk->start =3D start + size;
> +		blk->size -=3D sp_size + size;
> +		/* No fragment in the end, remove blk */
> +		if (blk->size =3D=3D 0) {
> +			list_del(&blk->list);
> +			release_slot(info, blk);
> +		}
> 	}
> 	newblk->owner =3D owner;
> -	start =3D newblk->start;
> 	attach_taken_block(info, newblk);
> -	/* for larger alignment return fixed up pointer  */
> -	/* this is no problem with the deallocator since */
> -	/* we scan for pointers that lie in the blocks   */
> -	if (alignment > info->alignment)
> -		start =3D (start + alignment - 1) & ~(alignment - 1);
> -
> 	return start;
> }
>

^ permalink raw reply

* Re: [PATCH] rheap: eliminates internal fragments caused by alignment
From: Joakim Tjernlund @ 2007-06-18 11:48 UTC (permalink / raw)
  To: Li Yang; +Cc: linuxppc-dev Development, Paul, Vitaly Bordug
In-Reply-To: <46766C91.6090903@freescale.com>

On Mon, 2007-06-18 at 19:29 +0800, Li Yang wrote:
> The patch adds fragments caused by rh_alloc_align() back to free list, instead
> of allocating the whole chunk of memory.  This will greatly improve memory
> utilization managed by rheap.
> 
> It solves MURAM not enough problem with 3 UCCs enabled on MPC8323.
> 
> Signed-off-by: Li Yang <leoli@freescale.com>
> ---

For what it is worth:
I have tested this patch and it solves the problem with 3 UCCs
as ethernet for me. This fixes a regression introduced after 2.6.20
Please consider for 2.6.22
Acked-by: Joakim Tjernlund <joakim.tjernlund@transmode.se> 

^ permalink raw reply

* [PATCH] rheap: eliminates internal fragments caused by alignment
From: Li Yang @ 2007-06-18 11:29 UTC (permalink / raw)
  To: Paul; +Cc: linuxppc-dev Development, Vitaly Bordug

The patch adds fragments caused by rh_alloc_align() back to free list, instead
of allocating the whole chunk of memory.  This will greatly improve memory
utilization managed by rheap.

It solves MURAM not enough problem with 3 UCCs enabled on MPC8323.

Signed-off-by: Li Yang <leoli@freescale.com>
---
 arch/powerpc/lib/rheap.c |   48 +++++++++++++++++++++++++++------------------
 1 files changed, 29 insertions(+), 19 deletions(-)

diff --git a/arch/powerpc/lib/rheap.c b/arch/powerpc/lib/rheap.c
index 180ee29..2f24ea0 100644
--- a/arch/powerpc/lib/rheap.c
+++ b/arch/powerpc/lib/rheap.c
@@ -437,27 +437,26 @@ unsigned long rh_alloc_align(rh_info_t * info, int size, int alignment, const ch
 	struct list_head *l;
 	rh_block_t *blk;
 	rh_block_t *newblk;
-	unsigned long start;
+	unsigned long start, sp_size;
 
 	/* Validate size, and alignment must be power of two */
 	if (size <= 0 || (alignment & (alignment - 1)) != 0)
 		return (unsigned long) -EINVAL;
 
-	/* given alignment larger that default rheap alignment */
-	if (alignment > info->alignment)
-		size += alignment - 1;
-
 	/* Align to configured alignment */
 	size = (size + (info->alignment - 1)) & ~(info->alignment - 1);
 
-	if (assure_empty(info, 1) < 0)
+	if (assure_empty(info, 2) < 0)
 		return (unsigned long) -ENOMEM;
 
 	blk = NULL;
 	list_for_each(l, &info->free_list) {
 		blk = list_entry(l, rh_block_t, list);
-		if (size <= blk->size)
-			break;
+		if (size <= blk->size) {
+			start = (blk->start + alignment - 1) & ~(alignment - 1);
+			if (start + size <= blk->start + blk->size)
+				break;
+		}
 		blk = NULL;
 	}
 
@@ -470,25 +469,36 @@ unsigned long rh_alloc_align(rh_info_t * info, int size, int alignment, const ch
 		list_del(&blk->list);
 		newblk = blk;
 	} else {
+		/* Fragment caused, split if needed */
+		/* Create block for fragment in the beginning */
+		sp_size = start - blk->start;
+		if (sp_size) {
+			rh_block_t *spblk;
+
+			spblk = get_slot(info);
+			spblk->start = blk->start;
+			spblk->size = sp_size;
+			/* add before the blk */
+			list_add(&spblk->list, blk->list.prev);
+		}
 		newblk = get_slot(info);
-		newblk->start = blk->start;
+		newblk->start = start;
 		newblk->size = size;
 
-		/* blk still in free list, with updated start, size */
-		blk->start += size;
-		blk->size -= size;
+		/* blk still in free list, with updated start and size
+		 * for fragment in the end */
+		blk->start = start + size;
+		blk->size -= sp_size + size;
+		/* No fragment in the end, remove blk */
+		if (blk->size == 0) {
+			list_del(&blk->list);
+			release_slot(info, blk);
+		}
 	}
 
 	newblk->owner = owner;
-	start = newblk->start;
 	attach_taken_block(info, newblk);
 
-	/* for larger alignment return fixed up pointer  */
-	/* this is no problem with the deallocator since */
-	/* we scan for pointers that lie in the blocks   */
-	if (alignment > info->alignment)
-		start = (start + alignment - 1) & ~(alignment - 1);
-
 	return start;
 }

^ permalink raw reply related

* Re: [PATCH] Wire up sys_sync_file_range() on powerpc
From: Russell King @ 2007-06-18  9:41 UTC (permalink / raw)
  To: David Woodhouse
  Cc: akpm, linux-arch, linuxppc-dev, torvalds, Paul Mackerras, drepper
In-Reply-To: <1182158711.2799.78.camel@shinybook.infradead.org>

On Mon, Jun 18, 2007 at 05:25:11PM +0800, David Woodhouse wrote:
> [PATCH] Add sys_sync_file_range2() system call
> ... with more sensible ABI, for those architectures which hated
> sys_sync_file_range() for wasting argument slots by misaligning 64-bit
> values. Convert ARM to use it, wire it up on PowerPC, and prevent the
> missing syscall checker from bitching about sync_file_range if you have
> sync_file_range2 instead.
> 
> Signed-off-by: David Woodhouse <dwmw2@infradead.org>

For the ARM bits:

Acked-by: Russell King <rmk+kernel@arm.linux.org.uk>

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

^ permalink raw reply

* Re: [PATCH] Wire up sys_sync_file_range() on powerpc
From: David Woodhouse @ 2007-06-18  9:25 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: akpm, linux-arch, linuxppc-dev, torvalds, drepper, rmk
In-Reply-To: <18038.6658.854660.4233@cargo.ozlabs.ibm.com>

On Mon, 2007-06-18 at 15:37 +1000, Paul Mackerras wrote:
> David Woodhouse writes:
> 
> > Pants.
> 
> Ah.  Is that a naughty word in the UK?  If so I'll have to remember
> that when I go to kernel summit. :)

I think the most important thing you have to remember about 'pants'
while in the UK is that you don't wear them on the outside.

> > It doesn't work because the 'flags' argument ends up in r9, and
> > we can only use r3-r8 for syscall arguments. We'll need to do it the
> > same way as ARM does, with the flags as the second argument.
> 
> Yes.  We'll need a wrapper for both 32-bit and for 32-on-64.
> 
> > Or is it? Can we ditch sys_sync_file_range now and implement a new
> > sys_sync_file_range2 with the two 32-bit arguments first?
> 
> Would be nice...

It might look something like this... I can't test it sensibly till I get
home next week; anyone else want to poke at it?

(Note that glibc's syscall() doesn't get 64-bit arguments like this
right; you'll want to implement do_sync_file_range() in assembly if
you're using Andrew's original test program.)

        .section ".text"
        .globl do_sync_file_range
        .type   do_sync_file_range, @function
do_sync_file_range:
        stwu 1,-16(1)
        mflr 0
        stw 31,12(1)
        stw 0,20(1)
        li 0,308
        sc           
        mfcr 0      
        andis. 9,0,4096
        mr 31,3
        beq 0,.L6
        bl __errno_location
        li 0,-1
        stw 31,0(3)
.L6:
        mr 3,0
        lwz 0,20(1)
        lwz 31,12(1)
        addi 1,1,16
        mtlr 0
        blr
        .size   do_sync_file_range,.-do_sync_file_range

---
[PATCH] Add sys_sync_file_range2() system call
... with more sensible ABI, for those architectures which hated
sys_sync_file_range() for wasting argument slots by misaligning 64-bit
values. Convert ARM to use it, wire it up on PowerPC, and prevent the
missing syscall checker from bitching about sync_file_range if you have
sync_file_range2 instead.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>

diff --git a/arch/arm/kernel/calls.S b/arch/arm/kernel/calls.S
index 19326d7..a98d0c9 100644
--- a/arch/arm/kernel/calls.S
+++ b/arch/arm/kernel/calls.S
@@ -350,7 +350,7 @@
 		CALL(sys_set_robust_list)
 		CALL(sys_get_robust_list)
 /* 340 */	CALL(sys_splice)
-		CALL(sys_arm_sync_file_range)
+		CALL(sys_sync_file_range2)
 		CALL(sys_tee)
 		CALL(sys_vmsplice)
 		CALL(sys_move_pages)
diff --git a/arch/arm/kernel/sys_arm.c b/arch/arm/kernel/sys_arm.c
index 1ca2d51..4d25e49 100644
--- a/arch/arm/kernel/sys_arm.c
+++ b/arch/arm/kernel/sys_arm.c
@@ -328,16 +328,3 @@ asmlinkage long sys_arm_fadvise64_64(int fd, int advice,
 {
 	return sys_fadvise64_64(fd, offset, len, advice);
 }
-
-/*
- * Yet more syscall fsckage - we can't fit sys_sync_file_range's
- * arguments into the available registers with EABI.  So, let's
- * create an ARM specific syscall for this which has _sane_
- * arguments.  (This incidentally also has an ABI-independent
- * argument layout.)
- */
-asmlinkage long sys_arm_sync_file_range(int fd, unsigned int flags,
-					loff_t offset, loff_t nbytes)
-{
-	return sys_sync_file_range(fd, offset, nbytes, flags);
-}
diff --git a/arch/powerpc/kernel/sys_ppc32.c b/arch/powerpc/kernel/sys_ppc32.c
index 047246a..b42cbf1 100644
--- a/arch/powerpc/kernel/sys_ppc32.c
+++ b/arch/powerpc/kernel/sys_ppc32.c
@@ -810,3 +810,12 @@ asmlinkage long compat_sys_request_key(const char __user *_type,
 	return sys_request_key(_type, _description, _callout_info, destringid);
 }
 
+asmlinkage long compat_sys_sync_file_range2(int fd, unsigned int flags,
+				   unsigned offset_hi, unsigned offset_lo,
+				   unsigned nbytes_hi, unsigned nbytes_lo)
+{
+	loff_t offset = ((loff_t)offset_hi << 32) | offset_lo;
+	loff_t nbytes = ((loff_t)nbytes_hi << 32) | nbytes_lo;
+
+	return sys_sync_file_range(fd, offset, nbytes, flags);
+}
diff --git a/fs/sync.c b/fs/sync.c
index 2f97576..7cd005e 100644
--- a/fs/sync.c
+++ b/fs/sync.c
@@ -236,6 +236,14 @@ out:
 	return ret;
 }
 
+/* It would be nice if people remember that not all the world's an i386
+   when they introduce new system calls */
+asmlinkage long sys_sync_file_range2(int fd, unsigned int flags,
+				     loff_t offset, loff_t nbytes)
+{
+	return sys_sync_file_range(fd, offset, nbytes, flags);
+}
+
 /*
  * `endbyte' is inclusive
  */
diff --git a/include/asm-arm/unistd.h b/include/asm-arm/unistd.h
index 250d7f1..bfdbebe 100644
--- a/include/asm-arm/unistd.h
+++ b/include/asm-arm/unistd.h
@@ -367,6 +367,7 @@
 #define __NR_get_robust_list		(__NR_SYSCALL_BASE+339)
 #define __NR_splice			(__NR_SYSCALL_BASE+340)
 #define __NR_arm_sync_file_range	(__NR_SYSCALL_BASE+341)
+#define __NR_sync_file_range2		__NR_arm_sync_file_range
 #define __NR_tee			(__NR_SYSCALL_BASE+342)
 #define __NR_vmsplice			(__NR_SYSCALL_BASE+343)
 #define __NR_move_pages			(__NR_SYSCALL_BASE+344)
diff --git a/include/asm-powerpc/systbl.h b/include/asm-powerpc/systbl.h
index 700ca59..1cc3f9c 100644
--- a/include/asm-powerpc/systbl.h
+++ b/include/asm-powerpc/systbl.h
@@ -311,3 +311,4 @@ COMPAT_SYS_SPU(utimensat)
 COMPAT_SYS_SPU(signalfd)
 COMPAT_SYS_SPU(timerfd)
 SYSCALL_SPU(eventfd)
+COMPAT_SYS_SPU(sync_file_range2)
diff --git a/include/asm-powerpc/unistd.h b/include/asm-powerpc/unistd.h
index e3c28dc..f71c606 100644
--- a/include/asm-powerpc/unistd.h
+++ b/include/asm-powerpc/unistd.h
@@ -330,10 +330,11 @@
 #define __NR_signalfd		305
 #define __NR_timerfd		306
 #define __NR_eventfd		307
+#define __NR_sync_file_range2	308
 
 #ifdef __KERNEL__
 
-#define __NR_syscalls		308
+#define __NR_syscalls		309
 
 #define __NR__exit __NR_exit
 #define NR_syscalls	__NR_syscalls
diff --git a/scripts/checksyscalls.sh b/scripts/checksyscalls.sh
index f98171f..0dcc01c 100755
--- a/scripts/checksyscalls.sh
+++ b/scripts/checksyscalls.sh
@@ -99,6 +99,11 @@ cat << EOF
 #define __IGNORE_setfsuid32
 #define __IGNORE_setfsgid32
 
+/* sync_file_range had a stupid ABI. Allow sync_file_range2 instead */
+#ifdef __NR_sync_file_range2
+#define __IGNORE_sync_file_range
+#endif
+
 /* Unmerged syscalls for AFS, STREAMS, etc. */
 #define __IGNORE_afs_syscall
 #define __IGNORE_getpmsg


-- 
dwmw2

^ permalink raw reply related

* [PATCH 1/1] Add support for MSI on Axon-based Cell systems
From: Michael Ellerman @ 2007-06-18  8:49 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Milton Miller

This patch adds support for the setup and decoding of MSIs
on Axon-based Cell systems, using the MSIC mechanism.

This involves setting up an area of BE memory which the Axon
then uses as a FIFO for MSI messages. When one or more MSIs
are decoded by the MSIC we receive an interrupt on the MPIC,
and the MSI messages are written into the FIFO. At the moment
we use a 64KB FIFO, one per MSIC/BE.

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
I've updated this to use a more robust, correct and I think straight forward
way of configuring the FIFO size - hopefully y'all agree. I think I've
addressed all the other points people raised as well. Thanks for the feedback.

 arch/powerpc/platforms/cell/Makefile   |    2 +
 arch/powerpc/platforms/cell/axon_msi.c |  445 ++++++++++++++++++++++++++++++++
 2 files changed, 447 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/platforms/cell/Makefile b/arch/powerpc/platforms/cell/Makefile
index 869af89..6615d40 100644
--- a/arch/powerpc/platforms/cell/Makefile
+++ b/arch/powerpc/platforms/cell/Makefile
@@ -23,3 +23,5 @@ obj-$(CONFIG_SPU_BASE)			+= spu_callbacks.o spu_base.o \
 					   $(spu-priv1-y) \
 					   $(spu-manage-y) \
 					   spufs/
+
+obj-$(CONFIG_PCI_MSI)			+= axon_msi.o
diff --git a/arch/powerpc/platforms/cell/axon_msi.c b/arch/powerpc/platforms/cell/axon_msi.c
new file mode 100644
index 0000000..4c9ab5b
--- /dev/null
+++ b/arch/powerpc/platforms/cell/axon_msi.c
@@ -0,0 +1,445 @@
+/*
+ * Copyright 2007, Michael Ellerman, IBM Corporation.
+ *
+ * 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.
+ */
+
+
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/kernel.h>
+#include <linux/pci.h>
+#include <linux/msi.h>
+#include <linux/reboot.h>
+
+#include <asm/dcr.h>
+#include <asm/machdep.h>
+#include <asm/prom.h>
+
+
+/*
+ * MSIC registers, specified as offsets from dcr_base
+ */
+#define MSIC_CTRL_REG	0x0
+
+/* Base Address registers specify FIFO location in BE memory */
+#define MSIC_BASE_ADDR_HI_REG	0x3
+#define MSIC_BASE_ADDR_LO_REG	0x4
+
+/* Hold the read/write offsets into the FIFO */
+#define MSIC_READ_OFFSET_REG	0x5
+#define MSIC_WRITE_OFFSET_REG	0x6
+
+
+/* MSIC control register flags */
+#define MSIC_CTRL_ENABLE		0x0001
+#define MSIC_CTRL_FIFO_FULL_ENABLE	0x0002
+#define MSIC_CTRL_IRQ_ENABLE		0x0008
+#define MSIC_CTRL_FULL_STOP_ENABLE	0x0010
+
+/*
+ * The MSIC can be configured to use a FIFO of 32KB, 64KB, 128KB or 256KB.
+ * Currently we're using a 64KB FIFO size.
+ */
+#define MSIC_FIFO_SIZE_SHIFT	16
+#define MSIC_FIFO_SIZE_BYTES	(1 << MSIC_FIFO_SIZE_SHIFT)
+
+/*
+ * To configure the FIFO size as (1 << n) bytes, we write (n - 15) into bits
+ * 8-9 of the MSIC control reg.
+ */
+#define MSIC_CTRL_FIFO_SIZE	(((MSIC_FIFO_SIZE_SHIFT - 15) << 8) & 0x300)
+
+/*
+ * We need to mask the read/write offsets to make sure they stay within
+ * the bounds of the FIFO. Also they should always be 16-byte aligned.
+ */
+#define MSIC_FIFO_SIZE_MASK	((MSIC_FIFO_SIZE_BYTES - 1) & ~0xFu)
+
+/* Each entry in the FIFO is 16 bytes, the first 4 bytes hold the irq # */
+#define MSIC_FIFO_ENTRY_SIZE	0x10
+
+
+struct axon_msic {
+	struct device_node *dn;
+	struct irq_host *irq_host;
+	__le32 *fifo;
+	dcr_host_t dcr_host;
+	struct list_head list;
+	u32 read_offset;
+	u32 dcr_base;
+};
+
+static LIST_HEAD(axon_msic_list);
+
+static void msic_dcr_write(struct axon_msic *msic, unsigned int dcr_n, u32 val)
+{
+	pr_debug("axon_msi: dcr_write(0x%x, 0x%x)\n", val, dcr_n);
+
+	dcr_write(msic->dcr_host, msic->dcr_base + dcr_n, val);
+}
+
+static u32 msic_dcr_read(struct axon_msic *msic, unsigned int dcr_n)
+{
+	return dcr_read(msic->dcr_host, msic->dcr_base + dcr_n);
+}
+
+static void axon_msi_cascade(unsigned int irq, struct irq_desc *desc)
+{
+	struct axon_msic *msic = get_irq_data(irq);
+	u32 write_offset, msi;
+	int idx;
+
+	write_offset = msic_dcr_read(msic, MSIC_WRITE_OFFSET_REG);
+	pr_debug("axon_msi: original write_offset 0x%x\n", write_offset);
+
+	/* write_offset doesn't wrap properly, so we have to mask it */
+	write_offset &= MSIC_FIFO_SIZE_MASK;
+
+	while (msic->read_offset != write_offset) {
+		idx  = msic->read_offset / sizeof(__le32);
+		msi  = le32_to_cpu(msic->fifo[idx]);
+		msi &= 0xFFFF;
+
+		pr_debug("axon_msi: woff %x roff %x msi %x\n",
+			  write_offset, msic->read_offset, msi);
+
+		msic->read_offset += MSIC_FIFO_ENTRY_SIZE;
+		msic->read_offset &= MSIC_FIFO_SIZE_MASK;
+
+		if (msi < NR_IRQS && irq_map[msi].host == msic->irq_host)
+			generic_handle_irq(msi);
+		else
+			pr_debug("axon_msi: invalid irq 0x%x!\n", msi);
+	}
+
+	desc->chip->eoi(irq);
+}
+
+static struct axon_msic *find_msi_translator(struct pci_dev *dev)
+{
+	struct irq_host *irq_host;
+	struct device_node *dn, *tmp;
+	const phandle *ph;
+	struct axon_msic *msic = NULL;
+
+	dn = pci_device_to_OF_node(dev);
+	if (!dn) {
+		dev_dbg(&dev->dev, "axon_msi: no pci_dn found\n");
+		return NULL;
+	}
+
+	for (; dn; tmp = of_get_parent(dn), of_node_put(dn), dn = tmp) {
+		ph = of_get_property(dn, "msi-translator", NULL);
+		if (ph)
+			break;
+	}
+
+	if (!ph) {
+		dev_dbg(&dev->dev,
+			"axon_msi: no msi-translator property found\n");
+		goto out_error;
+	}
+
+	tmp = dn;
+	dn = of_find_node_by_phandle(*ph);
+	if (!dn) {
+		dev_dbg(&dev->dev,
+			"axon_msi: msi-translator doesn't point to a node\n");
+		goto out_error;
+	}
+
+	irq_host = irq_find_host(dn);
+	if (!irq_host) {
+		dev_dbg(&dev->dev, "axon_msi: no irq_host found for node %s\n",
+			dn->full_name);
+		goto out_error;
+	}
+
+	msic = irq_host->host_data;
+
+out_error:
+	of_node_put(dn);
+	of_node_put(tmp);
+
+	return msic;
+}
+
+static int axon_msi_check_device(struct pci_dev *dev, int nvec, int type)
+{
+	if (!find_msi_translator(dev))
+		return -ENODEV;
+
+	return 0;
+}
+
+static int setup_msi_msg_address(struct pci_dev *dev, struct msi_msg *msg)
+{
+	struct device_node *dn, *tmp;
+	struct msi_desc *entry;
+	int len;
+	const u32 *prop;
+
+	dn = pci_device_to_OF_node(dev);
+	if (!dn) {
+		dev_dbg(&dev->dev, "axon_msi: no pci_dn found\n");
+		return -ENODEV;
+	}
+
+	entry = list_first_entry(&dev->msi_list, struct msi_desc, list);
+
+	for (; dn; tmp = of_get_parent(dn), of_node_put(dn), dn = tmp) {
+		if (entry->msi_attrib.is_64) {
+			prop = of_get_property(dn, "msi-address-64", &len);
+			if (prop)
+				break;
+		}
+
+		prop = of_get_property(dn, "msi-address-32", &len);
+		if (prop)
+			break;
+	}
+
+	if (!prop) {
+		dev_dbg(&dev->dev,
+			"axon_msi: no msi-address-(32|64) properties found\n");
+		return -ENOENT;
+	}
+
+	switch (len) {
+	case 8:
+		msg->address_hi = prop[0];
+		msg->address_lo = prop[1];
+		break;
+	case 4:
+		msg->address_hi = 0;
+		msg->address_lo = prop[0];
+		break;
+	default:
+		dev_dbg(&dev->dev,
+			"axon_msi: malformed msi-address-(32|64) property\n");
+		of_node_put(dn);
+		return -EINVAL;
+	}
+
+	of_node_put(dn);
+
+	return 0;
+}
+
+static int axon_msi_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
+{
+	unsigned int virq, rc;
+	struct msi_desc *entry;
+	struct msi_msg msg;
+	struct axon_msic *msic;
+
+	msic = find_msi_translator(dev);
+	if (!msic)
+		return -ENODEV;
+
+	rc = setup_msi_msg_address(dev, &msg);
+	if (rc)
+		return rc;
+
+	/* We rely on being able to stash a virq in a u16 */
+	BUILD_BUG_ON(NR_IRQS > 65536);
+
+	list_for_each_entry(entry, &dev->msi_list, list) {
+		virq = irq_create_direct_mapping(msic->irq_host);
+		if (virq == NO_IRQ) {
+			dev_warn(&dev->dev,
+				 "axon_msi: virq allocation failed!\n");
+			return -1;
+		}
+		dev_dbg(&dev->dev, "axon_msi: allocated virq 0x%x\n", virq);
+
+		set_irq_msi(virq, entry);
+		msg.data = virq;
+		write_msi_msg(virq, &msg);
+	}
+
+	return 0;
+}
+
+static void axon_msi_teardown_msi_irqs(struct pci_dev *dev)
+{
+	struct msi_desc *entry;
+
+	dev_dbg(&dev->dev, "axon_msi: tearing down msi irqs\n");
+
+	list_for_each_entry(entry, &dev->msi_list, list) {
+		if (entry->irq == NO_IRQ)
+			continue;
+
+		set_irq_msi(entry->irq, NULL);
+		irq_dispose_mapping(entry->irq);
+	}
+}
+
+static struct irq_chip msic_irq_chip = {
+	.mask		= mask_msi_irq,
+	.unmask		= unmask_msi_irq,
+	.shutdown	= unmask_msi_irq,
+	.typename	= "AXON-MSI",
+};
+
+static int msic_host_map(struct irq_host *h, unsigned int virq,
+			 irq_hw_number_t hw)
+{
+	set_irq_chip_and_handler(virq, &msic_irq_chip, handle_simple_irq);
+
+	return 0;
+}
+
+static int msic_host_match(struct irq_host *host, struct device_node *dn)
+{
+	struct axon_msic *msic = host->host_data;
+
+	return msic->dn == dn;
+}
+
+static struct irq_host_ops msic_host_ops = {
+	.match	= msic_host_match,
+	.map	= msic_host_map,
+};
+
+static int axon_msi_notify_reboot(struct notifier_block *nb,
+				  unsigned long code, void *data)
+{
+	struct axon_msic *msic;
+	u32 tmp;
+
+	list_for_each_entry(msic, &axon_msic_list, list) {
+		pr_debug("axon_msi: disabling %s\n", msic->dn->full_name);
+		tmp  = msic_dcr_read(msic, MSIC_CTRL_REG);
+		tmp &= ~MSIC_CTRL_ENABLE & ~MSIC_CTRL_IRQ_ENABLE;
+		msic_dcr_write(msic, MSIC_CTRL_REG, tmp);
+	}
+
+	return 0;
+}
+
+static struct notifier_block axon_msi_reboot_notifier = {
+	.notifier_call = axon_msi_notify_reboot
+};
+
+static int axon_msi_setup_one(struct device_node *dn)
+{
+	struct page *page;
+	struct axon_msic *msic;
+	unsigned int virq;
+	int dcr_len;
+
+	pr_debug("axon_msi: setting up dn %s\n", dn->full_name);
+
+	msic = kzalloc(sizeof(struct axon_msic), GFP_KERNEL);
+	if (!msic) {
+		printk(KERN_ERR "axon_msi: couldn't allocate msic for %s\n",
+		       dn->full_name);
+		goto out;
+	}
+
+	msic->dcr_base = dcr_resource_start(dn, 0);
+	dcr_len = dcr_resource_len(dn, 0);
+
+	if (msic->dcr_base == 0 || dcr_len == 0) {
+		printk(KERN_ERR
+		       "axon_msi: couldn't parse dcr properties on %s\n",
+			dn->full_name);
+		goto out;
+	}
+
+	msic->dcr_host = dcr_map(dn, msic->dcr_base, dcr_len);
+	if (!DCR_MAP_OK(msic->dcr_host)) {
+		printk(KERN_ERR "axon_msi: dcr_map failed for %s\n",
+		       dn->full_name);
+		goto out_free_msic;
+	}
+
+	page = alloc_pages_node(of_node_to_nid(dn), GFP_KERNEL,
+				get_order(MSIC_FIFO_SIZE_BYTES));
+	if (!page) {
+		printk(KERN_ERR "axon_msi: couldn't allocate fifo for %s\n",
+		       dn->full_name);
+		goto out_free_msic;
+	}
+
+	msic->fifo = page_address(page);
+
+	msic->irq_host = irq_alloc_host(IRQ_HOST_MAP_NOMAP, NR_IRQS,
+					&msic_host_ops, 0);
+	if (!msic->irq_host) {
+		printk(KERN_ERR "axon_msi: couldn't allocate irq_host for %s\n",
+		       dn->full_name);
+		goto out_free_fifo;
+	}
+
+	msic->irq_host->host_data = msic;
+
+	virq = irq_of_parse_and_map(dn, 0);
+	if (virq == NO_IRQ) {
+		printk(KERN_ERR "axon_msi: irq parse and map failed for %s\n",
+		       dn->full_name);
+		goto out_free_host;
+	}
+
+	msic->dn = of_node_get(dn);
+
+	set_irq_data(virq, msic);
+	set_irq_chained_handler(virq, axon_msi_cascade);
+	pr_debug("axon_msi: irq 0x%x setup for axon_msi\n", virq);
+
+	/* Enable the MSIC hardware */
+	msic_dcr_write(msic, MSIC_BASE_ADDR_HI_REG, (u64)msic->fifo >> 32);
+	msic_dcr_write(msic, MSIC_BASE_ADDR_LO_REG,
+				  (u64)msic->fifo & 0xFFFFFFFF);
+	msic_dcr_write(msic, MSIC_CTRL_REG,
+			MSIC_CTRL_IRQ_ENABLE | MSIC_CTRL_ENABLE |
+			MSIC_CTRL_FIFO_SIZE);
+
+	list_add(&msic->list, &axon_msic_list);
+
+	printk(KERN_DEBUG "axon_msi: setup MSIC on %s\n", dn->full_name);
+
+	return 0;
+
+out_free_host:
+	kfree(msic->irq_host);
+out_free_fifo:
+	__free_pages(virt_to_page(msic->fifo), get_order(MSIC_FIFO_SIZE_BYTES));
+out_free_msic:
+	kfree(msic);
+out:
+
+	return -1;
+}
+
+static int axon_msi_init(void)
+{
+	struct device_node *dn;
+	int found = 0;
+
+	pr_debug("axon_msi: initialising ...\n");
+
+	for_each_compatible_node(dn, NULL, "ibm,axon-msic") {
+		if (axon_msi_setup_one(dn) == 0)
+			found++;
+	}
+
+	if (found) {
+		ppc_md.setup_msi_irqs = axon_msi_setup_msi_irqs;
+		ppc_md.teardown_msi_irqs = axon_msi_teardown_msi_irqs;
+		ppc_md.msi_check_device = axon_msi_check_device;
+
+		register_reboot_notifier(&axon_msi_reboot_notifier);
+
+		pr_debug("axon_msi: registered callbacks!\n");
+	}
+
+	return 0;
+}
+arch_initcall(axon_msi_init);
-- 
1.5.1.3.g7a33b

^ permalink raw reply related

* Re: [PATCH] boot: find initrd location from device-tree
From: Milton Miller @ 2007-06-18  6:08 UTC (permalink / raw)
  To: David Gibson; +Cc: ppcdev, Paul Mackerras
In-Reply-To: <4672EE95.3030407@freescale.com>

[replying to David and Scott the second sub-thread]

On Jun 15, 2007, at 2:55 PM, Scott Wood wrote:
> Milton Miller wrote:
>> The file name dtscan reflects the possibility that simiar functions
>> like the rmo_top might be added to the file.
> Aren't these kind of things what devtree.c is for?

Hmm... i guess .. I didn't need the fixups in that file, but I guess 
the xlate routines couuld be useful for find top of memory.

That file is for "not specific to device tree library (firmware 
callback vs flat tree)" utilities?

On Mon Jun 18 13:18:22 EST 2007, David Gibson wrote:
> On Fri, Jun 15, 2007 at 12:34:30PM -0500, Milton Miller wrote:
> >
> > The types.h header now includes libgcc limits.h for UINT_MAX.
> >


> > ---
> > The file name dtscan reflects the possibility that simiar functions
> > like the rmo_top might be added to the file.
>
> Hrm, not sure if it's worth creating a new file for this, it could
> probably go in devtree.c.

Ok, two votes.  I'll move it next post.

> > +void dt_find_initrd(void)
> > +{
> > +     int rc;
> > +     unsigned long long initrd_start, initrd_end;
> > +     void *devp;
> > +     static const char start_prop[] = "linux,initrd-start";
> > +     static const char end_prop[] = "linux,initrd-end";
>
> I see no point to using these constants, just use literals in the
> getprop() calls.

When I had them inline, the get-property and the printf calls exceeded 
80 characters, making lots of multi-line function calls.  And 
seperating them out out made the error printfs the same, so they could 
be collapsed by gcc.  Either isn't a real strong argument, so if you 
insist I will write it out, but I thought it was nicer this way.

>
> > +     devp = finddevice("/chosen");
> > +     if (! devp) {
> > +             return;
> > +     }
> > +
> > +     /* The properties had to be 8 bytes until 2.6.22  */
> > +     rc = getprop(devp, start_prop, &initrd_start, 
> sizeof(initrd_start));
> > +     if (rc < 0)
> > +             return;
> > +     if (rc == sizeof(unsigned long)) {
> > +             unsigned long tmp;
> > +             memcpy(&tmp, &initrd_start, rc);
> > +             initrd_start = tmp;
> > +     } else if (rc != sizeof(initrd_start)) {
> > +             printf("unexpected length of %s in /chosen!\n\r", 
> start_prop);
> > +             return;
> > +     }
> > +
> > +     rc = getprop(devp, end_prop, &initrd_end, sizeof(initrd_end));
> > +     if (rc < 0) {
> > +             printf("chosen has %s but no %s!\n\r", start_prop, 
> end_prop);
> > +             return;
> > +     }
> > +     if (rc == sizeof(unsigned long)) {
> > +             unsigned long tmp;
> > +             memcpy(&tmp, &initrd_end, rc);
> > +             initrd_end = tmp;
> > +     } else if (rc != sizeof(initrd_end)) {
> > +             printf("unexpected length of %s in /chosen!\n\r", 
> end_prop);
> > +             return;
> > +     }
> > +
> > +     if (!initrd_start)
> > +             return;
> > +
> > +     /* if the initrd is above 4G, its untouchable in 32 bit mode */
> > +     if (initrd_end <= UINT_MAX && initrd_start < initrd_end) {
> > +             loader_info.initrd_addr = initrd_start;
> > +             loader_info.initrd_size  = initrd_end - initrd_start;
> > +     } else {
> > +             printf("ignoring loader supplied initrd parameters\n");
>
> Saying why might be helpful.

Hmm... well, that would mean seperating the && of the if; there are two 
possible reasons. (1) this 32-bit code can't handle such large 
addresses (in which case a 64-bit kernel may work), and (2) end is >= 
start (which means there really isn't any data.  I suppose I could 
reverse the sense of the if, use an else if, and make the final else 
the good path.



>
> > +     }
> > +}
...
> > Index: kernel/arch/powerpc/boot/types.h
> > ===================================================================
> > --- kernel.orig/arch/powerpc/boot/types.h     2007-06-15 
> 03:36:21.000000000 -0500
> > +++ kernel/arch/powerpc/boot/types.h  2007-06-15 03:44:34.000000000 
> -0500
> > @@ -1,6 +1,9 @@
> >  #ifndef _TYPES_H_
> >  #define _TYPES_H_
> >
> > +#define _LIBC_LIMITS_H_              /* don't recurse to system's 
> headers */
> > +#include <limits.h>          /* MAX_UINT, etc */
> > +
>
> I think it's really a bad idea to use any headers from outside the
> boot context here.  We're dealing with explicit sized ints, so we can
> safely define our own constants giving the limit values.

As I said in the patch changelog, the only headers picked up here are 
libgcc.  The compiler is part of the boot context, and we already pick 
up stdarg from there.   The flag there appears to be what libgcc 
expects the library to use.

I could change the comment to say /* get libgcc header only */ or 
something else you suggest.

milton

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox