* Re: CONFIG_PS3_USE_LPAR_ADDR dramically increases boot time
From: Jimi Xenidis @ 2007-06-19 0:13 UTC (permalink / raw)
To: Geoff Levand; +Cc: linuxppc-dev, Arnd Bergmann
In-Reply-To: <46771140.5000205@am.sony.com>
On Jun 18, 2007, at 7:12 PM, Geoff Levand wrote:
> Hi Jimi.
>
> Jimi Xenidis wrote:
>> On Jun 18, 2007, at 5:14 PM, Arnd Bergmann wrote:
>>
>>> On Monday 18 June 2007, Jimi Xenidis wrote:
>>>>
>>>> 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?
>>>
>>> Unfortunately, the hypervisor chooses the addresses where things get
>>> mapped, and since they are guest-real, there is no hard limit where
>>> the hypervisor puts them, like the 42 bit limit on bare metal.
>>>
>>> I guess this will improve if we get virt_mem_map support on powerpc,
>>> but I'm not sure if that's currently being worked on.
>>
>> I feel a little nit-picky worrying about slowing down boot, I'm sure
>> it negligible on HW, and complaining about 2K vs 16K memsection. But
>> it is important to point out that this effects all PPC64 targets if a
>> distro decides to include the PS3 config in their PPC64 image.
>
>
> When CONFIG_PS3_USE_LPAR_ADDR=n a translation in the platform code
> sets up the page table to map the region to low addresses (continuous
> with the boot mem region), but that translation is not compatible
> with the mapping scheme used by the spu regions, which use sparse mem.
hmm.. sparse mem is for the linear map.
Why is are SPE areas in the linear map?
>
> Arnd mention that he planed to re-do the spu region management, and
> I think with that we can use CONFIG_PS3_USE_LPAR_ADDR=n and the spufs
> at the same time. Arnd, what is the status?
>
> -Geoff
>
^ permalink raw reply
* Xilinx OPB_PCI support in 2.6
From: John Williams @ 2007-06-19 0:20 UTC (permalink / raw)
To: Linux PPC
Hi folks,
Has anyone tried getting the Xilinx opb_pci bridge going in a recent 2.6
kernel? - ML410 board or similar?
Mailing list archives seems silent about it.
Thanks,
John
^ permalink raw reply
* Re: CONFIG_PS3_USE_LPAR_ADDR dramically increases boot time
From: Benjamin Herrenschmidt @ 2007-06-19 1:23 UTC (permalink / raw)
To: Jimi Xenidis; +Cc: linuxppc-dev, Arnd Bergmann
In-Reply-To: <7F58A02C-5C4C-4717-8D32-65C75C342ADB@pobox.com>
On Mon, 2007-06-18 at 20:13 -0400, Jimi Xenidis wrote:
> > When CONFIG_PS3_USE_LPAR_ADDR=n a translation in the platform code
> > sets up the page table to map the region to low addresses
> (continuous
> > with the boot mem region), but that translation is not compatible
> > with the mapping scheme used by the spu regions, which use sparse
> mem.
>
> hmm.. sparse mem is for the linear map.
SPE local store used to have special struct pages for it in order to
return things from nopage(). It was dodgy and racy. I killed that
recently, it was causing more problem than it was solving. So it should
be fine now. I'm now using nopfn() and inserting the PTEs directly from
the nopfn() hook.
So Geoff, you can remove the option for now or make it n by default.
Ben.
^ permalink raw reply
* Re: [PATCH] boot: find initrd location from device-tree
From: David Gibson @ 2007-06-19 1:25 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: ppcdev, Paul Mackerras, Milton Miller
In-Reply-To: <bfbd4f126c427f590198a3e50a789b0c@kernel.crashing.org>
On Mon, Jun 18, 2007 at 02:52:12PM +0200, Segher Boessenkool wrote:
> >>> + /* 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?
IMO if it needs this define magic to work properly, it's not a good
idea to include it at all. Especially since avoiding it is easy and
safe in this case.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
^ permalink raw reply
* RE: [PATCH] rheap: eliminates internal fragments caused by alignment
From: Li Yang-r58472 @ 2007-06-19 4:02 UTC (permalink / raw)
To: Pantelis Antoniou, joakim.tjernlund
Cc: linuxppc-dev Development, Paul, Vitaly Bordug
In-Reply-To: <DA299F48-E5A6-495C-8169-A211D4C83864@gmail.com>
> -----Original Message-----
> From: Pantelis Antoniou [mailto:pantelis.antoniou@gmail.com]
> Sent: Monday, June 18, 2007 8:31 PM
> To: joakim.tjernlund@transmode.se
> Cc: Li Yang-r58472; Paul; Vitaly Bordug; linuxppc-dev Development
> Subject: Re: [PATCH] rheap: eliminates internal fragments caused by =
alignment
>=20
>=20
> On 18 =C9=EF=F5=ED 2007, at 3:25 =CC=CC, Joakim Tjernlund 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
> >> have so
> >> much wasted space that re-inserting the leftovers makes so much of =
a
> >> difference.
>=20
> Jocke,
>=20
> Ugh, 256?
>=20
> That's not nice at all.
Pantelis,
For QE, most of the MURAM allocation requires relative large alignment. =
SDMA buffer even needs a 4k alignment.
> 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.
Dynamic allocation surely can't match the efficiency of manual =
allocation. But with this patch applied, the utilization efficiency =
will be good enough for most systems to work correctly, and without the =
great trouble to partition the space yourself. Moreover, given the fact =
that in QE drivers some MURAM usage can be tunable, it will be more =
suitable to use the dynamic allocation.
A general policy will be helpful, that allocations with huge alignment =
(like 4k) should always be done as early as possible.
- Leo
^ permalink raw reply
* [PATCH] fix snd-powermac refounting bugs
From: Benjamin Herrenschmidt @ 2007-06-19 4:37 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linuxppc-dev list
The old snd-powermac driver has some serious refcounting issues when
initialisation fails, which is the case on all new machines with
a layout-id since those are handled by the new snd-powermac driver.
Some of those bugs seem to have been under the radar for some time
(like double pci_dev_put), but one was actually added in 2.6.22 with
Stephen attempt at teaching refcounting to the driver which didn't
do it at all.
This patch fixes both, thus removing all sort of kref errors that
would happen if that driver gets loaded on a G5 machine or a recent
PowerBook due to OF nodes left around with a 0 refcount.
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
Index: linux-work/sound/ppc/pmac.c
===================================================================
--- linux-work.orig/sound/ppc/pmac.c 2007-06-19 14:18:09.000000000 +1000
+++ linux-work/sound/ppc/pmac.c 2007-06-19 14:27:08.000000000 +1000
@@ -775,7 +775,8 @@ static int snd_pmac_free(struct snd_pmac
out_le32(&chip->awacs->control, in_le32(&chip->awacs->control) & 0xfff);
}
- snd_pmac_sound_feature(chip, 0);
+ if (chip->node)
+ snd_pmac_sound_feature(chip, 0);
/* clean up mixer if any */
if (chip->mixer_free)
@@ -925,6 +926,7 @@ static int __init snd_pmac_detect(struct
}
if (! sound) {
of_node_put(chip->node);
+ chip->node = NULL;
return -ENODEV;
}
prop = of_get_property(sound, "sub-frame", NULL);
@@ -937,7 +939,9 @@ static int __init snd_pmac_detect(struct
printk(KERN_INFO "snd-powermac no longer handles any "
"machines with a layout-id property "
"in the device-tree, use snd-aoa.\n");
+ of_node_put(sound);
of_node_put(chip->node);
+ chip->node = NULL;
return -ENODEV;
}
/* This should be verified on older screamers */
@@ -1297,8 +1301,6 @@ int __init snd_pmac_new(struct snd_card
return 0;
__error:
- if (chip->pdev)
- pci_dev_put(chip->pdev);
snd_pmac_free(chip);
return err;
}
^ permalink raw reply
* Re: [PATCH] kexec ppc64: fix misaligned cmdline
From: Horms @ 2007-06-19 5:06 UTC (permalink / raw)
To: Michael Neuling; +Cc: linuxppc-dev, kexec, Milton Miller
In-Reply-To: <10188.1181179162@neuling.org>
On Thu, Jun 07, 2007 at 11:19:22AM +1000, Michael Neuling wrote:
> If the cmdline changes between boots, we can get misalignment of the
> bootargs entry, which in turn corrupts our device tree blob and hence
> kills our kexec boot. Also, if there was no /chosen/bootargs
> previously, we'd never add a new one.
>
> This ignores the bootargs while parsing the tree and inserts it later.
>
> Signed-off-by: Michael Neuling <mikey@neuling.org>
Thanks, applied.
--
Horms
H: http://www.vergenet.net/~horms/
W: http://www.valinux.co.jp/en/
^ permalink raw reply
* Re: [RFC] Device tree for new desktop platform in arch/powerpc
From: David Gibson @ 2007-06-19 5:08 UTC (permalink / raw)
To: Mark A. Greer; +Cc: "list <linuxppc-dev", linuxppc-dev
In-Reply-To: <20070618202518.GG8740@mag.az.mvista.com>
On Mon, Jun 18, 2007 at 01:25:18PM -0700, Mark A. Greer wrote:
> 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.
I don't believe the name is actually used. It would be nice to get it
right, though - I think it can be considered option for the first cut,
though.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
^ permalink raw reply
* Please pull powerpc.git merge branch
From: Paul Mackerras @ 2007-06-19 5:39 UTC (permalink / raw)
To: torvalds; +Cc: linuxppc-dev
Linus,
Please do:
git pull \
git://git.kernel.org/pub/scm/linux/kernel/git/paulus/powerpc.git merge
to get a fix from Ben H for some refcounting bugs in the snd-powermac
driver.
Thanks,
Paul.
sound/ppc/pmac.c | 8 +++++---
1 files changed, 5 insertions(+), 3 deletions(-)
commit 41e904dee22066ae7ac3a7f695b2c8bd43ce4e04
Author: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: Tue Jun 19 14:37:39 2007 +1000
[POWERPC] Fix snd-powermac refcounting bugs
The old snd-powermac driver has some serious refcounting issues when
initialisation fails, which is the case on all new machines with
a layout-id since those are handled by the new snd-aoa driver.
Some of those bugs seem to have been under the radar for some time
(like double pci_dev_put), but one was actually added in 2.6.22 with
Stephen attempt at teaching refcounting to the driver which didn't
do it at all.
This patch fixes both, thus removing all sort of kref errors that
would happen if that driver gets loaded on a G5 machine or a recent
PowerBook due to OF nodes left around with a 0 refcount.
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Signed-off-by: Paul Mackerras <paulus@samba.org>
^ permalink raw reply
* Re: [RFC] Device tree for new desktop platform in arch/powerpc
From: David Gibson @ 2007-06-19 5:42 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:
> 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?
They should be children of the PCI<->ISA bridge. They should specify
there own address ranges in their reg properties - but those address
ranges are in the address space of the ISA bus. The PCI<->ISA bridge
will need a correct ranges property to map that into the PCI address
space, and the PCI host bridge will need a ranges which maps that into
the processor's
> 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?
Err... device_type == "pnpPNP,501", or compatible == "pnpPNP,501"?
> 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.
> /*
> * 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?
If they're ISA devices, then yes, they should.
> 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.
This node has no children, so #address-cells and #size-cells values
are meaningless.
> };
> interrupt-controller {
> device_type = "interrupt-controller";
> compatible = "chrp,iic";
Is there a device binding defined somewhere for "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>;
This should cause a dtc error. Either you want &/interrupt-controller
or give the interrupt-controller node a label and refer to that. It's
either '&/some/full/path' or '&label'.
> #address-cells = <1>;
> #size-cells = <0>; // No physical address.
Comment is arguably incorrect.
> keyboard {
> device_type = "keyboard";
> compatible = "pnpPNP,303";
> reg = <0>;
> };
> mouse {
> device_type = "mouse";
> compatible = "pnpPNP,f03";
> reg = <1>;
> };
> };
> timer@40 {
> device_type = "timer";
For flat device trees we're generally avoiding setting the device_type
property unless there is a clearly defined "class binding" which
applies. There are a number of cases here where I'm not sure if
that's true.
> reg = <40 20>
> #address-cells = <1>;
> #size-cells = <0>;
No children, so #a and #s are pointless.
> };
> rtc@70 {
> device_type = "rtc";
> compatible = <ds1385-rtc>;
> reg = <70 2>;
> interrupts = <8 3>;
> #address-cells = <1>;
> #size-cells = <0>;
Ditto
> };
> 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?
Probably necessary, but may need to be filled in from the bootwrapper.
> #address-cells = <1>;
> #size-cells = <0>;
Ditto.
> };
> 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>;
> };
Don't think you need this subnode.
> };
> 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>;
> };
> };
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
^ permalink raw reply
* Re: [patch 4/6] ps3: Disk Storage Driver
From: Christoph Hellwig @ 2007-06-19 5:43 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Jens Axboe, James E.J. Bottomley, linux-scsi, linux-kernel,
linuxppc-dev, Paul Mackerras
In-Reply-To: <20070615120848.352093000@pademelon.sonytel.be>
On Fri, Jun 15, 2007 at 01:39:23PM +0200, Geert Uytterhoeven wrote:
> From: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
>
> Add a Disk Storage Driver for the PS3:
> - Implemented as a block device driver with a dynamic major
> - Disk names (and partitions) are of the format ps3d%c(%u)
> - Uses software scatter-gather with a 64 KiB bounce buffer as the hypervisor
> doesn't support scatter-gather
Looks good to me. Only nitpicks are:
- ps3disk_priv should probably be an inline function instead of a macro
- ps3disk_open is not needed at all and should be removed completely.
- please remove PS3DISK_MAJOR - and just pass 0 to register_blkdev.
Passing 0 to get an unassigned dev_t range is the normal convention
and giving it a suprising symbolic names makes reading the code a
little harder.
- put_disk in ps3disk_remove should be done after you finished tearing
down the device, not before
- calling free_irq directly followed by request_irq in ps3disk_probe
looks rather dumb. I'd rather move the request_irq out of
ps3stor_setup into the low-level driver.
^ permalink raw reply
* Re: [patch 5/6] ps3: ROM Storage Driver
From: Christoph Hellwig @ 2007-06-19 5:44 UTC (permalink / raw)
To: Milton Miller; +Cc: Geert Uytterhoeven, ppcdev
In-Reply-To: <d46c75da8a86ab5cd83b63f9dbfee33f@bga.com>
On Mon, Jun 18, 2007 at 11:30:39AM -0500, Milton Miller wrote:
> When I think ROM I usually dont' start with optical media.
>
> Have you condered calling this the optical driver?
Actually the right name would probably be ps3atapi :)
^ permalink raw reply
* Re: for-2.6.23 branch in powerpc.git created
From: Paul Mackerras @ 2007-06-19 5:47 UTC (permalink / raw)
To: Olof Johansson; +Cc: linuxppc-dev
In-Reply-To: <20070615135349.GA28486@lixom.net>
Olof Johansson writes:
> The following two were postponed to 2.6.23 (CF driver and IDE glue):
>
> http://patchwork.ozlabs.org/linuxppc/patch?id=11094
> http://patchwork.ozlabs.org/linuxppc/patch?id=11148
>
> These two are marked "awaiting upstream" but never showed up:
>
> http://patchwork.ozlabs.org/linuxppc/patch?id=11147
> http://patchwork.ozlabs.org/linuxppc/patch?id=11452
It looks to me like 11148 (Electra CF driver) and 11147 (pcmcia: ppc64
needs 32-bit ioaddr_t) need to go to the pcmcia maintainer. You can
put Acked-by: Paul Mackerras <paulus@samba.org> on them.
I'll put the other two in.
Paul.
^ permalink raw reply
* Re: [patch 5/6] ps3: ROM Storage Driver
From: Christoph Hellwig @ 2007-06-19 5:51 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Jens Axboe, James E.J. Bottomley, linux-scsi, linux-kernel,
linuxppc-dev, Paul Mackerras
In-Reply-To: <20070615120848.454492000@pademelon.sonytel.be>
Looks pretty nice.
Some more comments:
- no need for the BLK_DEV_SR dependency, it should depend on SCSI
instead (which might be implied by beeing in the scsi menu, don't
remember that detail)
- ps3rom_priv should probably be an inline
- the cmd field in struct ps3rom_private should probably be renamed
to curr_cmd or something along the line to make sure this is the
current command we deal with and the driver can only deal with one
command at a time.
- The comment in ps3rom_queuecommand should explain why we prefer
lv1_storage_{read,write} over just sending down the scsi cdb,
not just that we prefer it.
- I don't think you need to decode the 6 byte commands at all,
ATAPI only has the 10 byte variants and you can tell the scsi
layer not to ever send the 6 byte ones by setting the use_10_for_rw
flag in ->slave_configure
- same comment about the free_irq/request_irq pair as in the disk
driver.
- please use the new shost_priv helper in scsi-misc for accessing
the hostdata file in struct Scsi_Host so that the ugly casts
can go away.
^ permalink raw reply
* Re: [patch 4/6] ps3: Disk Storage Driver
From: Christoph Hellwig @ 2007-06-19 5:53 UTC (permalink / raw)
To: David Miller
Cc: axboe, James.Bottomley, linux-scsi, linux-kernel, linuxppc-dev,
paulus, Geert.Uytterhoeven, dwmw2, alan
In-Reply-To: <20070615.141917.85409547.davem@davemloft.net>
On Fri, Jun 15, 2007 at 02:19:17PM -0700, David Miller wrote:
> Another quirk I have to deal with is that under LDOMs you
> can export full disks and also just slices. So I'll have
> to get down into the partition machinery to support that
> somehow.
What's the problem with that? Partition machinery is perfectly
fine with just getting a raw disk without a partition table,
and it obviously doesn't care if the raw disk it sees is part
of a partition on the storage server.
^ permalink raw reply
* Re: [patch 4/6] ps3: Disk Storage Driver
From: Christoph Hellwig @ 2007-06-19 5:56 UTC (permalink / raw)
To: David Miller
Cc: James.Bottomley, axboe, linux-scsi, linux-kernel, linuxppc-dev,
paulus, Geert.Uytterhoeven, dwmw2, alan
In-Reply-To: <20070615.160858.26276547.davem@davemloft.net>
On Fri, Jun 15, 2007 at 04:08:58PM -0700, David Miller wrote:
> That's not gonna work, it's a totally different model.
>
> I have a predefined protocol over hypervisor provided "channels" and
> page flipping also done by the hypervisor for the bulk data transfer.
> For the client side I cannot change the hypervisor nor the server
> speaking on the other end. And when I do write a server I do want
> it to be able to speak to all of the existing clients.
>
> There's SCSI command pass through as well, as I keep mentioning as
> it's an important reason I don't want to go with any of the non-SCSI
> solutions (other than perhaps ATA) being suggested.
A SCSI pass through is of course perfectly fine. If you have a separate
block passthrough that has additional magic a separate block driver is
the way to go because it actually is simpler than a scsi driver decoding
command blocks and translating them to deep magic. The ps3 storage
drivers this thread discussed are a good example for that. We now
have a very nice and simple disk, scsi and flash chardev driver each
that don't include abstractions layers and cruft. Combine that with
their initial scsi layer driver that was full of internal dispatches
because each of these device types speaks a completely different command
set.
^ permalink raw reply
* Re: [patch 30/33] PS3: Bootwrapper support.
From: Mark A. Greer @ 2007-06-19 5:58 UTC (permalink / raw)
To: Geoff Levand; +Cc: ppcdev, Milton Miller
In-Reply-To: <46771CE7.8040901@am.sony.com>
On Mon, Jun 18, 2007 at 05:01:43PM -0700, Geoff Levand wrote:
> Mark A. Greer wrote:
> > As for the rest of Geoff's patch, I think I get the gist of what's going
> > on but it would be really nice to have it described in English (and put
> > in the patch description). That way others (like me) could understand
> > the problem you're solving and what your solution is.
>
> This is from the comment the patch adds to the wrapper script. I think
> it gives you what you need:
>
> +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.
Yes, that was what I needed. I should have found that myself...my apologies.
Mark
^ permalink raw reply
* Re: [patch 4/6] ps3: Disk Storage Driver
From: David Miller @ 2007-06-19 6:03 UTC (permalink / raw)
To: hch
Cc: axboe, James.Bottomley, linux-scsi, linux-kernel, linuxppc-dev,
paulus, Geert.Uytterhoeven, dwmw2, alan
In-Reply-To: <20070619055358.GD18542@lst.de>
From: Christoph Hellwig <hch@lst.de>
Date: Tue, 19 Jun 2007 07:53:58 +0200
> On Fri, Jun 15, 2007 at 02:19:17PM -0700, David Miller wrote:
> > Another quirk I have to deal with is that under LDOMs you
> > can export full disks and also just slices. So I'll have
> > to get down into the partition machinery to support that
> > somehow.
>
> What's the problem with that? Partition machinery is perfectly
> fine with just getting a raw disk without a partition table,
> and it obviously doesn't care if the raw disk it sees is part
> of a partition on the storage server.
Applications care :-)
One way of managing the partition table is by asking the storage
server for it. It supports giving back something that looks
like the Sun partition table layout, and also something that's
EFI based.
But the client can ignore that and read/write the partition
table itself.
What I really care about is what will work transparently for existing
userspace. In particular, distribution installers and existing tools
like fdisk.
When a slice it being exported, it's not being exported like that so
that the client can just spam a disk label into it, it's for exporting
one slice. So we might want (using the scsi naming as an arbitrary
example case) /dev/sda1 to be created but not /dev/sda
How exactly do you envision this kind of thing working?
Supporting the disk-server-provided partition table management
means providing ioctls() or whatever interface to get/set
the table. I had one idea to intercept reads/writes to the
first block(s) and translating those into partition management
commands to the disk server, but that is just too ugly to live. :-)
^ permalink raw reply
* Re: [patch 4/6] ps3: Disk Storage Driver
From: David Miller @ 2007-06-19 6:07 UTC (permalink / raw)
To: hch
Cc: James.Bottomley, axboe, linux-scsi, linux-kernel, linuxppc-dev,
paulus, Geert.Uytterhoeven, dwmw2, alan
In-Reply-To: <20070619055629.GE18542@lst.de>
From: Christoph Hellwig <hch@lst.de>
Date: Tue, 19 Jun 2007 07:56:29 +0200
> A SCSI pass through is of course perfectly fine. If you have a separate
> block passthrough that has additional magic a separate block driver is
> the way to go because it actually is simpler than a scsi driver decoding
> command blocks and translating them to deep magic. The ps3 storage
> drivers this thread discussed are a good example for that. We now
> have a very nice and simple disk, scsi and flash chardev driver each
> that don't include abstractions layers and cruft. Combine that with
> their initial scsi layer driver that was full of internal dispatches
> because each of these device types speaks a completely different command
> set.
That's how I'm currently writing my virtual disk client driver
for the Sun LDOMS stuff, as a block device.
The remaining issues are the partitioning (which we're discussing in
another thread) and how to export the scsi passthru support in such a
non-scsi block driver.
The main disk I/O block read and write is done using descriptors
sent to the disk server. SCSI pass-through is provided (optionally)
so that disk analysis tools can do things like MODE_SENSE on the
disk.
^ permalink raw reply
* [PATCH 1/3] Check for the root node in of_detach_node()
From: Michael Ellerman @ 2007-06-19 6:07 UTC (permalink / raw)
To: linuxppc-dev
It's not sensible to call of_detach_node() on the root node,
but we should check for it just to be safe.
Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
arch/powerpc/kernel/prom.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index af42dda..6ea94cf 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -1432,6 +1432,8 @@ void of_detach_node(const struct device_node *np)
write_lock(&devtree_lock);
parent = np->parent;
+ if (!parent)
+ goto out_unlock;
if (allnodes == np)
allnodes = np->allnext;
@@ -1455,6 +1457,7 @@ void of_detach_node(const struct device_node *np)
prevsib->sibling = np->sibling;
}
+out_unlock:
write_unlock(&devtree_lock);
}
--
1.5.1.3.g7a33b
^ permalink raw reply related
* [PATCH 2/3] Generalise device_node flag interface
From: Michael Ellerman @ 2007-06-19 6:07 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <9ac0032bf8356ff6e41b0860b6f67dddf9bda2f5.1182233192.git.michael@ellerman.id.au>
The struct device_node currently has a _flags variable, although
it's only used for one flag - OF_DYNAMIC. Generalise the flag
accessors so we can use them with other flags in future.
Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
arch/powerpc/kernel/prom.c | 2 +-
arch/powerpc/platforms/pseries/reconfig.c | 2 +-
include/asm-powerpc/prom.h | 14 +++++++++++---
3 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index 6ea94cf..4f70764 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -1375,7 +1375,7 @@ static void of_node_release(struct kref *kref)
struct device_node *node = kref_to_device_node(kref);
struct property *prop = node->properties;
- if (!OF_IS_DYNAMIC(node))
+ if (!of_node_check_flag(node, OF_DYNAMIC))
return;
while (prop) {
struct property *next = prop->next;
diff --git a/arch/powerpc/platforms/pseries/reconfig.c b/arch/powerpc/platforms/pseries/reconfig.c
index 5aa97af..c02f874 100644
--- a/arch/powerpc/platforms/pseries/reconfig.c
+++ b/arch/powerpc/platforms/pseries/reconfig.c
@@ -123,7 +123,7 @@ static int pSeries_reconfig_add_node(const char *path, struct property *proplist
strcpy(np->full_name, path);
np->properties = proplist;
- OF_MARK_DYNAMIC(np);
+ of_node_set_flag(np, OF_DYNAMIC);
kref_init(&np->kref);
np->parent = derive_parent(path);
diff --git a/include/asm-powerpc/prom.h b/include/asm-powerpc/prom.h
index 6845af9..6eb98aa 100644
--- a/include/asm-powerpc/prom.h
+++ b/include/asm-powerpc/prom.h
@@ -98,10 +98,18 @@ struct device_node {
extern struct device_node *of_chosen;
/* flag descriptions */
-#define OF_DYNAMIC 1 /* node and properties were allocated via kmalloc */
+#define OF_DYNAMIC 1 /* node and properties were allocated via kmalloc */
+
+static inline int of_node_check_flag(struct device_node *n, unsigned long flag)
+{
+ return test_bit(flag, &n->_flags);
+}
+
+static inline void of_node_set_flag(struct device_node *n, unsigned long flag)
+{
+ set_bit(flag, &n->_flags);
+}
-#define OF_IS_DYNAMIC(x) test_bit(OF_DYNAMIC, &x->_flags)
-#define OF_MARK_DYNAMIC(x) set_bit(OF_DYNAMIC, &x->_flags)
#define HAVE_ARCH_DEVTREE_FIXUPS
--
1.5.1.3.g7a33b
^ permalink raw reply related
* [PATCH 3/3] Add a warning to help trackdown device_node refcounting bugs
From: Michael Ellerman @ 2007-06-19 6:08 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <9ac0032bf8356ff6e41b0860b6f67dddf9bda2f5.1182233192.git.michael@ellerman.id.au>
When the refcount for a device node goes to 0, we call the
destructor - of_node_release(). This should only happen if we've
already detached the node from the device tree.
So add a flag OF_DETACHED which tracks detached-ness, and if we
find ourselves in of_node_release() without it set, issue a
warning and don't free the device_node. To avoid warning
continuously reinitialise the kref to a sane value.
Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
The warning looks something like the following. In this case it shows that
I erroneously did an of_node_put() inside the of_find_node_by_type() loop
in setup_arch().
WARNING: Bad of_node_put() on /memory@b8000000
Call Trace:
[c0000000007e3ba0] [c00000000000f74c] .show_stack+0x6c/0x1a0 (unreliable)
[c0000000007e3c40] [c00000000002397c] .of_node_release+0x44/0xe8
[c0000000007e3cd0] [c00000000021f4e0] .kref_put+0x74/0x94
[c0000000007e3d50] [c000000000022b34] .of_node_put+0x20/0x34
[c0000000007e3dc0] [c000000000022fcc] .of_find_node_by_type+0x90/0xc8
[c0000000007e3e50] [c00000000063ff78] .setup_arch+0x1b8/0x234
[c0000000007e3ee0] [c0000000006376ac] .start_kernel+0xd8/0x3e4
[c0000000007e3f90] [c0000000000084c8] .start_here_common+0x54/0x8c
arch/powerpc/kernel/prom.c | 11 +++++++++++
include/asm-powerpc/prom.h | 1 +
2 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index 4f70764..e0f61bb 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -1375,8 +1375,17 @@ static void of_node_release(struct kref *kref)
struct device_node *node = kref_to_device_node(kref);
struct property *prop = node->properties;
+ /* We should never be releasing nodes that haven't been detached. */
+ if (!of_node_check_flag(node, OF_DETACHED)) {
+ printk("WARNING: Bad of_node_put() on %s\n", node->full_name);
+ dump_stack();
+ kref_init(&node->kref);
+ return;
+ }
+
if (!of_node_check_flag(node, OF_DYNAMIC))
return;
+
while (prop) {
struct property *next = prop->next;
kfree(prop->name);
@@ -1457,6 +1466,8 @@ void of_detach_node(const struct device_node *np)
prevsib->sibling = np->sibling;
}
+ of_node_set_flag(np, OF_DETACHED);
+
out_unlock:
write_unlock(&devtree_lock);
}
diff --git a/include/asm-powerpc/prom.h b/include/asm-powerpc/prom.h
index 6eb98aa..01b27f2 100644
--- a/include/asm-powerpc/prom.h
+++ b/include/asm-powerpc/prom.h
@@ -99,6 +99,7 @@ extern struct device_node *of_chosen;
/* flag descriptions */
#define OF_DYNAMIC 1 /* node and properties were allocated via kmalloc */
+#define OF_DETACHED 2 /* node has been detached from the device tree */
static inline int of_node_check_flag(struct device_node *n, unsigned long flag)
{
--
1.5.1.3.g7a33b
^ permalink raw reply related
* Re: [RFC] Device tree for new desktop platform in arch/powerpc
From: Segher Boessenkool @ 2007-06-19 6:08 UTC (permalink / raw)
To: Gerhard Pircher; +Cc: ppcdev list
In-Reply-To: <20070618185715.321010@gmx.net>
[Your "To:" header was broken...]
> 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?).
Just name the cpu nodes "cpu@0" etc. The "generic names"
recommended practice wants this, and the node names here
aren't actually used by anything anyway, except they look
nice to the user. The PowerPC ISA binding predates the
generic names r.p. btw.
> 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.
This cannot work on PowerPC since PowerPC doesn't have
legacy I/O on the system bus.
> Should they be child nodes of the southbridge's PCI-to-ISA device node?
Yes.
> 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?
Both. Each device has a "reg" property that describes that
device's registers relative to the addressing on the bus it
sits on. All busses that do a direct one-to-one mapping of
some address range (perhaps with a translation) have a "ranges"
property describing that.
> 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?
"device_type" is "serial", "compatible" is "pnpPNP,whatever".
Where in the OF specs did you find this pnpPNP thing btw?
> 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)?
"name" = "device_type" = "interrupt-controller".
"compatible" is one of those pnpPNP things.
> 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.
Please send the thing inline, not as an attachment.
Segher
^ permalink raw reply
* Re: [RFC] Device tree for new desktop platform in arch/powerpc
From: Segher Boessenkool @ 2007-06-19 6:16 UTC (permalink / raw)
To: David Gibson; +Cc: ppcdev list
In-Reply-To: <20070619054232.GB32039@localhost.localdomain>
>> interrupt-controller {
>> device_type = "interrupt-controller";
>> compatible = "chrp,iic";
>
> Is there a device binding defined somewhere for "chrp,iic"?
In the CHRP platform binding, which doesn't apply here.
>> keyboard {
>> device_type = "keyboard";
>> compatible = "pnpPNP,303";
>> reg = <0>;
>> };
>
>> mouse {
>> device_type = "mouse";
>> compatible = "pnpPNP,f03";
>> reg = <1>;
>> };
>> };
>
>> timer@40 {
>> device_type = "timer";
>
> For flat device trees we're generally avoiding setting the device_type
> property unless there is a clearly defined "class binding" which
> applies. There are a number of cases here where I'm not sure if
> that's true.
Keyboard and mouse have defined "device_type"s. Timer I
don't think so.
Either way, I'd leave out the device_type here, the OS
isn't supposed to use it anyway.
>> 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?
>
> Probably necessary, but may need to be filled in from the bootwrapper.
If absent, "clock-frequency" should be taken to be the
default thing. Actually, for pnpPNP (PC-style) serial,
the clock frequency is fixed.
>> pci@80000000 {
>> device_type = "pci";
>> bus-frequency = <01fca055>; // 33.3MHz
So write it in decimal...
Segher
^ permalink raw reply
* Re: for-2.6.23 branch in powerpc.git created
From: Paul Mackerras @ 2007-06-19 6:30 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev, Guennadi Liakhovetski
In-Reply-To: <4672D694.5060701@freescale.com>
Scott Wood writes:
> Guennadi Liakhovetski wrote:
> > These two i2c patches:
> >
> > http://ozlabs.org/pipermail/linuxppc-dev/2007-June/037327.html
>
> This looks OK as an interim measure, though at some point I'd like to
> get around to letting both devices and drivers specify a list of match
> names.
>
> > http://ozlabs.org/pipermail/linuxppc-dev/2007-June/037328.html
>
> I'd leave out the device_type, and the "rtc" in "rtc-r5c372" is
> redundant if that's all the r5c372 chip does, but other than that it
> looks good.
>
> > also would be nice to get in, although, they only make sense with an ack
> > from Scott Wood and another patch from him, as explained in links above...
>
> I assume you mean "Call of_register_i2c_devices() for fsl-i2c"
> (http://ozlabs.org/pipermail/linuxppc-dev/2007-May/036333.html)?
Isn't that the one where you replied to yourself saying you'd posted
the wrong patch? Did you ever post the right patch?
Paul.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox