* Re: [PATCH] 64K page support for kexec
From: Milton Miller @ 2007-04-25 5:48 UTC (permalink / raw)
To: Olof Johansson, Luke Browning, Benjamin Herrenschmidt; +Cc: ppcdev, cbe-oss-dev
In-Reply-To: <20070424230708.GA10401@lixom.net>
>
>
> > > > + /*
> > > > + * FIXME, this could be made more efficient by storing the type
> > > > + * of hash algorithm in mmu_psize_defs[]. The code below
> assumes
> > > > + * the number of bits in the va representing the offset in the
> > > > + * page is less than 23. This affects the hash algorithm that
> is
> > > > + * used. When 16G pages are supported, a new hash algorithm
> > > > + * needs to be provided. See POWER ISA Book III.
> > > > + *
> > > > + * The code below works for 16M, 64K, and 4K pages.
> > > > + */
> > >
> > > A BUG_ON() when other sizes are hit could be a good idea?
> >
> > a BUG_ON if the B bit is set would be useful too. (that is 1T segment
> > HPTE).
>
> Yep.
NNNNOOOOO!!!!
Do NOT add any BUG() is this code!
Look at the context:
(1) We are tearing down *ALL* mappings. That includes the
kernel linear mapping and the mapping of the kernel text.
(2) We are in real mode. There is no way back to virtural
mode. See (1).
(3) We hove put the new kernel in memory. There is no data
that was not a part of the static data or bss sections. There
are no per-cpu variables. Nothing with vmalloc. Nothing with
kmalloc. Nothing with alloc_pages.
(4) If this is the panic kernel case, we are allready crashed
and trying to get into a new envrionment to dump memory.
Intentionally failing that is the last thing you want to do.
(5) This hook is only used by kexec_sequence, called by
machine_kexec. At the beginning of that function there
is the comment "its too late to fail here."
Bottom line: Don't even think of causing any trap here, just
execute as best as you know how. You dohn't have any way to
tell the user you failed anyways.
milton
^ permalink raw reply
* Re: arch/powerpc for lite5200b
From: Domen Puncer @ 2007-04-25 5:29 UTC (permalink / raw)
To: Pradyumna Sampath; +Cc: linuxppc-embedded
In-Reply-To: <f87351060704240431m2218910cs65510472218459b0@mail.gmail.com>
On 24/04/07 17:01 +0530, Pradyumna Sampath wrote:
> Hi Everyone,
>
> I have been trying to get the linux-2.6.20-rt3 for arch/powerpc to
> boot on my lite5200b. Here are the things that I have tried so far. I
> have a custom board based on the lite5200b and then the evaluation
> board too.
>
> - I have a working 2.6.16-rt29 kernel compiled using ARCH=ppc for both
> these boards.
>
> - I tried the default configs for lite5200 for both the custom board
> and the EVB. Both hang at "Transferring control to Linux (at address
> 00000000) ..."
Device tree missing?
You need to compile it with dtc, then in u-boot use something like:
bootp; tftp 0x400000 uImage.ppc; tftp 0x600000 lite5200b.dtb
set your bootargs
bootm 0x400000 - 0x600000
>
> - I downloaded roberts
> http://www.pengutronix.de/oselas/bsp/phytec/index_en.html#phyCORE-MPC5200B-tiny
> patches, but still ended up with the same result.
>
> I read ( http://www.nabble.com/Re%3A-MPC52xx-support-rework-for-2.6.20-p7568655.html
> ) that the arch/powerpc port for lite5200 works right out of the box.
> Am I doing something wrong ?
>
> The one thing I am yet to try is to do a git checkout from
> git.secretlabs.ca. Unfortunately thanks to some internal firewalling
> policies I still have not been able to do that.
kernel.org tree will also work with lite5200b, if you need ethernet
download fec/bestcomm patches from Grant's tree.
Domen
^ permalink raw reply
* Re: [PATCH 3/6] Consolidate of_find_property
From: Benjamin Herrenschmidt @ 2007-04-25 5:07 UTC (permalink / raw)
To: Paul Mackerras; +Cc: Stephen Rothwell, David S. Miller, ppc-dev
In-Reply-To: <17966.45645.902486.793443@cargo.ozlabs.ibm.com>
> The question is, what ppc drivers would break if we used strcmp
> instead of strcasecmp? I have a dim memory that some Apple machines
> had "ata" and others had "ATA" for the hard disk, for instance.
ata/ATA and ide/IDE is the main one that comes to mind, but I wouldn't
exclude something like Powermac vs. PowerMac or that sort of thing... I
remember a case related bug about 2 years ago but can't find what it
was ...
I think it's safe to settle on strcasecmp for most things for now
Ben.
^ permalink raw reply
* Re: [PATCH 3/6] Consolidate of_find_property
From: David Miller @ 2007-04-25 4:31 UTC (permalink / raw)
To: paulus; +Cc: sfr, linuxppc-dev
In-Reply-To: <17966.45645.902486.793443@cargo.ozlabs.ibm.com>
From: Paul Mackerras <paulus@samba.org>
Date: Wed, 25 Apr 2007 11:43:41 +1000
> The question is, what ppc drivers would break if we used strcmp
> instead of strcasecmp? I have a dim memory that some Apple machines
> had "ata" and others had "ATA" for the hard disk, for instance.
To be honest, I am sure there are a lot more examples of this
on Apple machines.
^ permalink raw reply
* Re: [PATCH 1/6] Start split out of common open firmware code
From: David Miller @ 2007-04-25 4:29 UTC (permalink / raw)
To: segher; +Cc: sfr, paulus, linuxppc-dev
In-Reply-To: <c65c2fcf295f9b4a40292156b5258012@kernel.crashing.org>
From: Segher Boessenkool <segher@kernel.crashing.org>
Date: Wed, 25 Apr 2007 03:03:31 +0200
> end up with the OF analogue to an MSI layer that only
> ever can work on intel compatible platforms.
Please don't use this straw-man.
These days it works perfectly fine on sparc64.
^ permalink raw reply
* Re: [PATCH 1/6] Start split out of common open firmware code
From: David Miller @ 2007-04-25 4:27 UTC (permalink / raw)
To: segher; +Cc: sfr, paulus, linuxppc-dev
In-Reply-To: <baa646899e4d0f3f1111f62709a2c66b@kernel.crashing.org>
From: Segher Boessenkool <segher@kernel.crashing.org>
Date: Wed, 25 Apr 2007 02:50:13 +0200
> The SPARC tree carries the same broken code -- is
> this just a case of code copying, or are there
> actual SPARC machines with such broken trees?
There have been many cases of missed or even erroneous
properties on sparc tree, but I am not too sure about
the cell counting cases.
Let's do this, once the consolidation is done keeping
behavior as-is, give me a patch to review that changing
the cell counting bits and I'll verify it against
all the machines I have here plus some OFW tree dumps.
^ permalink raw reply
* Re: [PATCH 3/6] Consolidate of_find_property
From: David Miller @ 2007-04-25 4:24 UTC (permalink / raw)
To: benh; +Cc: sfr, paulus, linuxppc-dev
In-Reply-To: <1177459235.14873.155.camel@localhost.localdomain>
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: Wed, 25 Apr 2007 10:00:35 +1000
> On Tue, 2007-04-24 at 22:39 +1000, Stephen Rothwell wrote:
> > The only change here is that a readlock is taken while the property list
> > is being traversed on Sparc where it was not taken previously.
> >
> > Also, Sparc uses strcasecmp to compare property names while PowerPC
> > uses strcmp.
>
> Ok, so that's the opposite as the previous one... property names are
> supposed to be case sensitive though, no ? Dave, how do you think we
> should converge here ? I don't think we would take much risk on ppc by
> using strncasecmp but it shouldn't be necessary...
The difference is that some properties are all-caps on powerpc
when they are lowecase on sparc, the Radeon properties are a
good example.
I think if we use strncasecmp across the board, nothing will
break.
^ permalink raw reply
* Re: [PATCH 2/6] Consolidate of_device_is_compatible
From: David Miller @ 2007-04-25 4:23 UTC (permalink / raw)
To: benh; +Cc: sfr, paulus, linuxppc-dev
In-Reply-To: <1177459165.14873.153.camel@localhost.localdomain>
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: Wed, 25 Apr 2007 09:59:24 +1000
> On Tue, 2007-04-24 at 22:38 +1000, Stephen Rothwell wrote:
> > The only difference here is that Sparc uses strncmp to match compatibility
> > names while PowerPC uses strncasecmp.
>
> We should settle for a single implementation. In theory, strncmp would
> be the way to go but there have been "bugs" here or there, especially in
> Apple DTs, that made me use strncasecmp instead in the past.
>
> Dave, what do you reckon ? I should try to find out the bogus machines
> and add workarounds in the various drivers etc... instead or we can just
> go common on strncasecmp ?
I think strncasecmp is the safest thing to start with.
^ permalink raw reply
* RE: PTE entries
From: Benjamin Herrenschmidt @ 2007-04-25 3:50 UTC (permalink / raw)
To: Siva Prasad; +Cc: linuxppc-dev, linuxppc-embedded
In-Reply-To: <D83235F0F3C86D4D889D8B9A0DA8C6D762CA85@corpexc01.corp.networkrobots.com>
On Tue, 2007-04-24 at 20:34 -0700, Siva Prasad wrote:
> Thanks Ben.
>
> I managed to fix this. I am working on how best we can rewrite the self
> modifying code we have for create_hpte routine. I would prefer to read
> from SDR1, than to hardcode (or self modify the code) the way it is done
> now.
>
> Feel free to let me know what you think.
Self-modifying allows better performances in that very critical code path...
Why would you need to change that code anyway ?
Ben.
^ permalink raw reply
* RE: PTE entries
From: Siva Prasad @ 2007-04-25 3:34 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, linuxppc-embedded
In-Reply-To: <1177376700.14873.40.camel@localhost.localdomain>
Thanks Ben.
I managed to fix this. I am working on how best we can rewrite the self
modifying code we have for create_hpte routine. I would prefer to read
from SDR1, than to hardcode (or self modify the code) the way it is done
now.
Feel free to let me know what you think.
Thanks
Siva
-----Original Message-----
From: Benjamin Herrenschmidt [mailto:benh@kernel.crashing.org]=20
Sent: Monday, April 23, 2007 6:05 PM
To: Siva Prasad
Cc: linuxppc-dev@ozlabs.org; linuxppc-embedded@ozlabs.org
Subject: Re: PTE entries
On Thu, 2007-04-19 at 21:34 -0700, Siva Prasad wrote:
> Hi,
>=20
> =20
>=20
> Where exactly the PTE are setup for any given new user program?
>=20
> =20
>=20
> In other words, if you are trying to execute a user level program,
> kernel needs to load it and setup PTEs for that program. Where does
> kernel do it?
They are faulted in.
See do_page_fault
Ben.
^ permalink raw reply
* Ramdisk Vs NFS
From: Siva Prasad @ 2007-04-25 3:16 UTC (permalink / raw)
To: linuxppc-dev, linuxppc-embedded
Hi,
What is the primary difference between Ramdisk and NFS with respect to
the wait_queue's?
If I use ramdisk, every thing works fine, but with NFS (or you may read
as 'no ramdisk') kernel/sched.c:__wake_up_common() routines has a
problem. Basically the value of "&q->task_list->next" is out of our
memory range (not between 0xc0000000 and 0xF0000000), and this causes
trouble of accessing non-existing memory.=20
Interesting thing is, this happens much before we even load the ramdisk
drivers.
Appreciate if any one has some insight into this.
Thanks
Siva
^ permalink raw reply
* Re: [PATCH] powerpc: uninline of_iomap function
From: Paul Mackerras @ 2007-04-25 2:15 UTC (permalink / raw)
To: Christian Krafft, Benjamin Herrenschmidt, Arnd Bergmann,
linuxppc-dev, Arnd Bergmann
In-Reply-To: <17966.41335.429268.945199@cargo.ozlabs.ibm.com>
I wrote:
> Looks like your mailer mangled the patch - could you resend it with a
> different mailer or with different settings of your mailer?
Never mind, looks like the problem is on my end.
Paul.
^ permalink raw reply
* Re: [PATCH 3/6] Consolidate of_find_property
From: Segher Boessenkool @ 2007-04-25 2:10 UTC (permalink / raw)
To: Paul Mackerras; +Cc: ppc-dev, David S. Miller, Stephen Rothwell
In-Reply-To: <17966.45645.902486.793443@cargo.ozlabs.ibm.com>
>> Property names aren't supposed to contain uppercase
>> characters.
>
> Somebody forgot to tell Apple:
Yes I know. Sigh.
>> I can swing both ways on which of strcmp()
>> or strcasecmp() is better, but there seems no reason
>> to do this differently on each platform.
>
> The question is, what ppc drivers would break if we used strcmp
> instead of strcasecmp? I have a dim memory that some Apple machines
> had "ata" and others had "ATA" for the hard disk, for instance.
Hrm I don't remember that one, but yeah something
similar is bound to prop up.
The question is, should we use strcasecmp(), or adjust
those drivers to probe for the upper case version
in addition to the lower case version? That option
at least has the advantage that you can document
the workaround right where the actual problem is met :-)
Segher
^ permalink raw reply
* Re: [PATCH 1/6] Start split out of common open firmware code
From: Segher Boessenkool @ 2007-04-25 2:07 UTC (permalink / raw)
To: Josh Boyer; +Cc: Stephen Rothwell, paulus, David S. Miller, ppc-dev
In-Reply-To: <20070425015123.GE23995@crusty.rchland.ibm.com>
>>> If you can get me access to all of the old PowerMac or CHRP crap with
>>> broken device-tree's ...
>>
>> No I can't, which is exactly why I cannot fix it myself.
>>
>>> Note that I don't think there's much cases of such breakage on the
>>> field,
>>
>> For PowerPC it is mostly the G3 PowerMacs I believe.
>
> I have one of those. What exactly do you need from it? I can
> probably do
> some testing with it if needs be.
I'll cook up a patch when Stephen's patch series hits
a Git tree. Testing would basically be trying to run
a kernel with all possible devices enabled, and seeing
if they all still work as expected.
Thanks for offering your help! Let's see what happens
next week or so :-)
Segher
^ permalink raw reply
* Re: [PATCH 3/6] Consolidate of_find_property
From: Segher Boessenkool @ 2007-04-25 2:03 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: ppc-dev, paulus, David S. Miller, Stephen Rothwell
In-Reply-To: <1177463538.14873.179.camel@localhost.localdomain>
>> The OF standard doesn't allow upper case in property
>> names at all.
>
> Ok, fair enough, but in practice, they exist :-)
Yes.
>>> I don't think we would take much risk on ppc by
>>> using strncasecmp but it shouldn't be necessary...
>>
>> There are some examples where upper case is actually
>> used in property names, unfortunately. These can
>> be solved by renaming/copying those properties to
>> all lower case in a bootwrapper, or by strcmp() to
>> the "cased" string, or by strcasecmp(). I for one
>> don't like either option, you figure it out ;-)
>
> Oh well, I don't care either way though fixup from bootwrapper doesn't
> appeal to me at all...
It doesn't to me either, not for this isolated case;
but when looking at the bigger picture, all the really
bad dev tree problems we need to work around on certain
platforms, it starts to sound appealing. And when you
start doing some fixes in a bootwrapper, why not all.
Just something to think about.
Segher
^ permalink raw reply
* Re: [PATCH] generic check_legacy_ioport
From: Segher Boessenkool @ 2007-04-25 1:54 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: Christian Krafft, Olaf Hering, linuxppc-dev
In-Reply-To: <200704250001.18162.arnd@arndb.de>
>>> Do you think a device_type fdc, i8042 or ipmi will appear outside an
>>> isa
>>> node?
>>
>> Does anyone know where those ipmi devices appear in the device-tree?
>
> There are actually _no_ ipmi devices that we expect to appear here.
> the reason why the check is in the ipmi driver is in order not to crash
> on powerpc machines that load the ipmi driver but have no ipmi nodes
> in the device tree.
> I don't think there are any powerpc machines where it can find
> something
> there, but we decided to leave the code architecture independent in
> case there ever are, and just to add the check_legacy_ioport call in
> there.
SLOF/JS21 (at least some versions of it) have an "ipmi"
node on the "isa" bus. And the kernel ipmi driver actually
works on it, too (no idea about the currently proposed
scanning though -- but the principle is correct at least).
Segher
^ permalink raw reply
* Re: [PATCH 3/6] Consolidate of_find_property
From: Paul Mackerras @ 2007-04-25 1:43 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: Stephen Rothwell, David S. Miller, ppc-dev
In-Reply-To: <06c51eb59847d80d9225cd6454f1957e@kernel.crashing.org>
Segher Boessenkool writes:
> Property names aren't supposed to contain uppercase
> characters.
Somebody forgot to tell Apple:
$ ls /proc/device-tree/ht@0,f2000000/pci@9/ata-6@d
total 23
1 #address-cells 1 cable-type 1 interrupt-parent 1 name
1 #size-cells 1 class-code 1 interrupts 1 reg
1 AAPL,bus-id 1 compatible 0 lba-48 1 revision-id
1 AAPL,connector 1 device-id 1 linux,phandle 1 vendor-id
1 AAPL,pio-timing 1 device_type 1 max-latency
1 AAPL,requested-priorities 1 devsel-speed 1 min-grant
1 assigned-addresses 0 disk/ 1 model
> I can swing both ways on which of strcmp()
> or strcasecmp() is better, but there seems no reason
> to do this differently on each platform.
The question is, what ppc drivers would break if we used strcmp
instead of strcasecmp? I have a dim memory that some Apple machines
had "ata" and others had "ATA" for the hard disk, for instance.
Paul.
^ permalink raw reply
* Re: [PATCH 1/6] Start split out of common open firmware code
From: Josh Boyer @ 2007-04-25 1:51 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: Stephen Rothwell, paulus, David S. Miller, ppc-dev
In-Reply-To: <baa646899e4d0f3f1111f62709a2c66b@kernel.crashing.org>
On Wed, Apr 25, 2007 at 02:50:13AM +0200, Segher Boessenkool wrote:
> >> While I realise your patch is just moving stuff around,
> >> can this be fixed please? A missing "#address-cells"
> >> property means "two cells", not "inherit from parent".
> >> The few platforms with a broken tree should just be
> >> fixed (patch the tree in the bootwrapper, or perhaps
> >> _do_ use this workaround, but only on the affected
> >> platforms).
> >
> > If you can get me access to all of the old PowerMac or CHRP crap with
> > broken device-tree's ...
>
> No I can't, which is exactly why I cannot fix it myself.
>
> > Note that I don't think there's much cases of such breakage on the
> > field,
>
> For PowerPC it is mostly the G3 PowerMacs I believe.
I have one of those. What exactly do you need from it? I can probably do
some testing with it if needs be.
josh
^ permalink raw reply
* Re: [PATCH 3/6] Consolidate of_find_property
From: Benjamin Herrenschmidt @ 2007-04-25 1:12 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: ppc-dev, paulus, David S. Miller, Stephen Rothwell
In-Reply-To: <a8ef1d22d218c74620bc38ed9d35830e@kernel.crashing.org>
On Wed, 2007-04-25 at 02:43 +0200, Segher Boessenkool wrote:
> The OF standard doesn't allow upper case in property
> names at all.
Ok, fair enough, but in practice, they exist :-)
> > I don't think we would take much risk on ppc by
> > using strncasecmp but it shouldn't be necessary...
>
> There are some examples where upper case is actually
> used in property names, unfortunately. These can
> be solved by renaming/copying those properties to
> all lower case in a bootwrapper, or by strcmp() to
> the "cased" string, or by strcasecmp(). I for one
> don't like either option, you figure it out ;-)
Oh well, I don't care either way though fixup from bootwrapper doesn't
appeal to me at all...
Ben.
^ permalink raw reply
* Re: [PATCH 1/6] Start split out of common open firmware code
From: Segher Boessenkool @ 2007-04-25 1:03 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, paulus, David Miller, sfr
In-Reply-To: <1177459651.14873.164.camel@localhost.localdomain>
> Dave, no need to shout :-) I think it's pretty well agreed that
> Stephen's patches are good to go (unless nits I haven't seen), we are
> really just discussing what we should do on top of them.
Yes. And I dare say that we should agree that certain
fixes *should* go on top or this consolidation shouldn't
happen at all -- at least not as a consolidation to a
"generic" OF layer. For example, the OLPC device tree / OF
kernel efforts would be dead in the water if we cannot
have a *proper* "#address-cells" parser.
Again, I have *nothing* against Stephen's patches, I find
them quite nice actually. It's just that if this stuff
gets moved to a "for all archs" layer, some "design
qualms" need to be cleared up; either that, or we'll
end up with the OF analogue to an MSI layer that only
ever can work on intel compatible platforms.
Segher
^ permalink raw reply
* Re: [PATCH 1/6] Start split out of common open firmware code
From: Segher Boessenkool @ 2007-04-25 0:50 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: ppc-dev, paulus, David S. Miller, Stephen Rothwell
In-Reply-To: <1177459397.14873.160.camel@localhost.localdomain>
>> While I realise your patch is just moving stuff around,
>> can this be fixed please? A missing "#address-cells"
>> property means "two cells", not "inherit from parent".
>> The few platforms with a broken tree should just be
>> fixed (patch the tree in the bootwrapper, or perhaps
>> _do_ use this workaround, but only on the affected
>> platforms).
>
> If you can get me access to all of the old PowerMac or CHRP crap with
> broken device-tree's ...
No I can't, which is exactly why I cannot fix it myself.
> Note that I don't think there's much cases of such breakage on the
> field,
For PowerPC it is mostly the G3 PowerMacs I believe.
The SPARC tree carries the same broken code -- is
this just a case of code copying, or are there
actual SPARC machines with such broken trees?
> but I'd be annoyed to change a behaviour we had from day 1.
Yes exactly -- we shouldn't just completely break
the kernel working on those machines. Of course,
if no tester can be found, perhaps support for
those boxes should just be abandoned.
Segher
^ permalink raw reply
* Re: [PATCH 3/6] Consolidate of_find_property
From: Segher Boessenkool @ 2007-04-25 0:43 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: ppc-dev, paulus, David S. Miller, Stephen Rothwell
In-Reply-To: <1177459235.14873.155.camel@localhost.localdomain>
>> The only change here is that a readlock is taken while the property
>> list
>> is being traversed on Sparc where it was not taken previously.
>>
>> Also, Sparc uses strcasecmp to compare property names while PowerPC
>> uses strcmp.
>
> Ok, so that's the opposite as the previous one... property names are
> supposed to be case sensitive though, no ?
The OF standard doesn't allow upper case in property
names at all.
> I don't think we would take much risk on ppc by
> using strncasecmp but it shouldn't be necessary...
There are some examples where upper case is actually
used in property names, unfortunately. These can
be solved by renaming/copying those properties to
all lower case in a bootwrapper, or by strcmp() to
the "cased" string, or by strcasecmp(). I for one
don't like either option, you figure it out ;-)
Segher
^ permalink raw reply
* Re: [PATCH] powerpc: uninline of_iomap function
From: Paul Mackerras @ 2007-04-25 0:31 UTC (permalink / raw)
To: Christian Krafft; +Cc: Arnd Bergmann, Arnd Bergmann, linuxppc-dev
In-Reply-To: <20070424173202.15e5ce42@localhost>
Christian Krafft writes:
> @@ -1042,3 +1042,15 @@ const void *of_get_mac_address(struct de
> }
> EXPORT_SYMBOL(of_get_mac_address);
>
> +void __iomem *of_iomap(struct device_node *np, int index)
> +{
> + struct resource res;
>
> + if (of_address_to_resource(np, index, &res))
> + return NULL;
>
> + return ioremap(res.start, 1 + res.end - res.start);
> +}
> +EXPORT_SYMBOL(of_iomap);
Looks like your mailer mangled the patch - could you resend it with a
different mailer or with different settings of your mailer?
Paul.
^ permalink raw reply
* Re: [PATCH 2/2] POWERPC: Remove global CPM mappings
From: Vitaly Bordug @ 2007-04-24 23:57 UTC (permalink / raw)
To: Dan Malek; +Cc: linuxppc-dev, Paul Mackerras
In-Reply-To: <3AC6566B-FCB5-4B45-B282-B737F51545ED@embeddedalley.com>
On Tue, 24 Apr 2007 16:24:42 -0400
Dan Malek wrote:
>
> On Apr 24, 2007, at 1:18 AM, Vitaly Bordug wrote:
>
> > At first I was thinking of make_everybody_happy solution,
>
> Just make me happy :-) The problem with these
> "works in progress" submissions is no one ever
> goes back to properly finish them, so I suspect
> this code you are checking in, which I don't
> like at all, is going to just stay there and bug
> me until _I_ fix it.
>
Hmm - it seems I want to check stuff in and forget about it? That's just not right, and there is zero value in it.
>
> > I know it can be more efficient. And I am looking at this way, but
> > it just cannot
> > be achieved via single step.
>
> Why? Because it's more work than you want to do?
>
Because one_step_for_all is breakage-prone; and small changes are easier to debug. I want to make CPM layer
stuff better while keeping it working, and definitely not just to do some specific amount of work/code, sorry.
> > TODO list includes rehaul of GPIO (with long-time-grown
> > feature_call + device tree bindings that were implemented for 8360
> > but looks reasonable), muxing, etc.
>
> Nothing you have done so far affects any of this.
> The things you are changing are the very basic
> support functions. You'll never go back and
> fix these because then the excuse will be
> "all of the drivers now use it."
>
It does not matter what "all of the drivers now use". I just dumped long-term plan.
Anyway let's refrain from predicting future - this is kinda off topic here ;).
> > This patch just fixes what already exist in kernel, removing the
> > global IMMR pointer
>
> It doesn't "fix" anything, it just changes the model to make it
> more heavyweight.
>
This approach assumes keeping whole mapped immr just to toggle a bit in mux or issue cpm_cr_cmd
is not a good thing. If the driver needs to initialize values in mux(once), it makes sense to remap that part of immr and do just that.
for "It doesn't "fix" anything", this particular patch wraps up the cleanup and does not actually introduce new
model. The only critial path I can admit is CPM command stuff inside fs_enet and uart drivers, I'll fix it if that is the point.
> > and bringing all remaining code paths to the same need_stuff-
> > >immr_map->use_it->immr_unmap model.
>
> That's not the model. The model is: driver init maps IMMR,
> driver uses it's local mapping, driver unloaded unmaps IMMR.
> We don't map/unmap on every use. The macros should be
> given this mapped immr as a parameter, not always
> map and unmap because it takes more than five minutes
> to properly change all of the APIs.
>
So you see good in each CPM-related driver keeping its own remapped immr? It's up to driver/implementation how to handle stuff: it can remap needed things upon init and keep offsets around in internal struct, or have it ioremapped and modified in case of single-shot init/reconfigure thing. Macro is just a tool to access only necessary part of immr, and is kind of hacky just to keep arch/ppc in the loop.
> > Current code is messy at some parts, and this what I am trying to
> > address (with current patch and upcomings).
>
> IMHO this does nothing to clean it up. I don't understand
> the big deal about needing to remove the IMMR as a global
> pointer. The kernel has lots of other global variables that
> no one seems concerned about and admit they must be
> present. These crappy macros and hacks in the name of
> removing a global IMMR just isn't right.
>
Again, macros is used to keep arch/ppc/ working. direct immr usage may be efficient but pretty hacky too.
The patch is changing only arch/powerpc stuff and is sort of formal because does not really affect anything(in arch/powerpc of course) except cpm_reset and brg setting.
> > Thanks for looking at it!
>
> I'm not in any agreement this is correct nor do I want to
> see the crap checked in. You're turning a simple,
> couple of clock cycle memory access into huge overhead,
> probably context switchable operation. Someone is
> going to grab a spin lock, and end up crashing on
> what appears to be a trivial memory access.....
>
> This is not progress.
Erm.. Where exactly in this patch - cr_cmd stuff in drivers/? Overhead relies on implementation - nobody would be happy of the incremental ioremapping.
-V
^ permalink raw reply
* Re: [PATCH] generic check_legacy_ioport
From: Benjamin Herrenschmidt @ 2007-04-25 0:12 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: linuxppc-dev, Olaf Hering, Christian Krafft
In-Reply-To: <200704250001.18162.arnd@arndb.de>
On Wed, 2007-04-25 at 00:01 +0200, Arnd Bergmann wrote:
> On Tuesday 24 April 2007, Olaf Hering wrote:
> >
> > > Do you think a device_type fdc, i8042 or ipmi will appear outside an isa
> > > node?
> >
> > Does anyone know where those ipmi devices appear in the device-tree?
>
> There are actually _no_ ipmi devices that we expect to appear here.
> the reason why the check is in the ipmi driver is in order not to crash
> on powerpc machines that load the ipmi driver but have no ipmi nodes
> in the device tree.
To be totally correct, there might well be, but we shouldn't care.
That is, if a device XXX (let's say XXX is ipmi but it could be i8042 or
whatever else) exist in the device-tree outside of the ISA bus, then it
needs it's own platform or of_platform device to be probed and shouldn't
make the check_legacy_io_port() go true.
There are various cases (especially in embedded world) where "standard"
components have been puts in ASICs but at funky/different addresses, and
they shouldn't be hit by the legacy probe.
Thus, the default implementation of check_legacy_ioport() should only
match things that are under an "isa" bus. Anything else needs to be
handled either by a platform specific check_legacy_ioport() override via
ppc_md (if it's really some legacy stuff and under a fucked up
device-tree) or via driver specific alternate probing method (like an
of_platform_device).
Ben.
^ 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