* Re: [RFC][PATCH] MPC832x_RDB: update dts to use spi, register mmc_spi stub
From: Segher Boessenkool @ 2007-08-06 22:08 UTC (permalink / raw)
To: Kim Phillips; +Cc: linuxppc-dev
In-Reply-To: <20070806163137.136cc689.kim.phillips@freescale.com>
>>>>>>> + max-speed-hz = <bebc20>; /* 12500000 Hz */
>>>>
>>>> Just max-speed.
>>>
>>> Segher, how is this different from:
>>>
>>> http://ozlabs.org/pipermail/linuxppc-dev/2007-April/034557.html
>>
>> Not sure what you mean. I'm just saying that "speed-hz" is a
>> terrible name, I'm not saying that "max-speed" is perfect at all.
>
> yet you suggest a /more/ generic name, contrary to your prior comments.
Uh, you mean "hz" doesn't mean "Hertz"? What a great name,
then</sarcasm>.
> My interpretation of your recent comments is that 'max-speed' is now a
> valid property name for devices such as ucc_geth. Do I have that
> right?
It is (and always was) a _valid_ name. Whether it is a _good_
name depends on the context; if there is only one speed it can
be (reasonably) referring to, it is okay; if not (or even if so),
you're better off being a bit more verbose in your property names.
No need to go over the top though, it's all a tradeoff.
Segher
^ permalink raw reply
* [PATCH] powerpc: Fix initialization and usage of dma_mask
From: Benjamin Herrenschmidt @ 2007-08-06 22:05 UTC (permalink / raw)
To: Olaf Hering
Cc: Robert Hancock, linux-kernel, linuxppc-dev, Stefan Richter,
Paul Mackerras, stable
In-Reply-To: <1186436597.938.71.camel@localhost.localdomain>
powerpc has a couple of bugs in the usage of dma_masks that tend to
break when drivers explicitely try to set a 32 bits mask for example.
First the code that generates the pci devices from the OF device-tree
doesn't initialize the mask properly, then our implementation of
set_dma_mask() was trying to validate the -previous- mask value, not the
one passed in as an argument.
This patch should fix these.
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
Does this fix the problem you've noticed ?
(I do still think that sbp2 isn't the right place for that call btw :-)
Index: linux-work/include/asm-powerpc/dma-mapping.h
===================================================================
--- linux-work.orig/include/asm-powerpc/dma-mapping.h 2007-08-07 07:59:05.000000000 +1000
+++ linux-work/include/asm-powerpc/dma-mapping.h 2007-08-07 07:59:09.000000000 +1000
@@ -96,7 +96,7 @@
return -EIO;
if (dma_ops->set_dma_mask != NULL)
return dma_ops->set_dma_mask(dev, dma_mask);
- if (!dev->dma_mask || !dma_supported(dev, *dev->dma_mask))
+ if (!dev->dma_mask || !dma_supported(dev, dma_mask))
return -EIO;
*dev->dma_mask = dma_mask;
return 0;
Index: linux-work/arch/powerpc/kernel/pci_64.c
===================================================================
--- linux-work.orig/arch/powerpc/kernel/pci_64.c 2007-08-07 08:00:20.000000000 +1000
+++ linux-work/arch/powerpc/kernel/pci_64.c 2007-08-07 08:00:47.000000000 +1000
@@ -372,6 +372,7 @@
dev->current_state = 4; /* unknown power state */
dev->error_state = pci_channel_io_normal;
+ dev->dma_mask = 0xffffffff;
if (!strcmp(type, "pci") || !strcmp(type, "pciex")) {
/* a PCI-PCI bridge */
^ permalink raw reply
* Re: [PATCH] powerpc: Pegasos keyboard detection
From: Segher Boessenkool @ 2007-08-06 21:57 UTC (permalink / raw)
To: Matt Sealey; +Cc: linuxppc-dev, Alan Curry, linux-kernel
In-Reply-To: <46B79562.9020202@genesi-usa.com>
> 2) The fix was in the wrong place anyway, if it was going to be done
> anywhere
> at all it needs to be in
> arch/powerpc/kernel/prom_init.c:fixup_device_tree_chrp()
> like the ISA ranges breakage (which is on Briq) and IDE IRQ
> misnumbering fix.
> Not the keyboard platform driver.
Yeah. In the bootwrapper.
> 3) In any case this should be something that is fixed in the firmware,
> as any
> stalwart, stubborn Linux developer will rant at you about.
Sure, if you *can* get a fix for the firmware. Until every
user has this update, fixing it in the kernel wrapper helps
users.
> As for Segher, bootwrapper not such a good place as that's still
> mussing up
> the kernel with these fixes. Let the boot loader do it for the OS, and
> don't
> mess up the OS with device-tree fixups.
Sure, the bootloader can do it too, but then we need to fix
every bootloader that's used with Linux on this platform.
Maybe that's just one, that would make things simple :-)
> After all it may not just be Linux
> that stumbles on it. Why have the same patch in every OS?
This is just pragmatics: Linux needs the workaround -> Linux
implements the workaround. Sure it is not a _proper_ fix, but
a correctly implemented workaround "fixes" it for all users,
forever.
> With nvramrc, the fix is done for EVERY operating system from firmware
> upwards.
But it's something every user has to do separately.
> The semi-official Genesi line of support and what I have been told by
> the board
> designer is if you need to fix something in the device tree, that is
> what nvramrc
> is for, and that is why Open Firmware runs Forth scripts.
That's hardly the only reason. But yeah, that's one way to
implement the workaround, but _we_ (the Linux community) cannot
do it like that (easily) for all users.
Segher
^ permalink raw reply
* Re: [PATCH 2.6.22.y] ieee1394: revert "sbp2: enforce 32bit DMA mapping"
From: Benjamin Herrenschmidt @ 2007-08-06 21:47 UTC (permalink / raw)
To: Olaf Hering
Cc: Robert Hancock, linuxppc-dev, Stefan Richter, stable,
linux-kernel
In-Reply-To: <20070806135124.GA2900@suse.de>
On Mon, 2007-08-06 at 15:51 +0200, Olaf Hering wrote:
> On Mon, Aug 06, Benjamin Herrenschmidt wrote:
>
> > BTW. Any reason why you don't set the DMA mask in the ohci driver rather
> > than the sbp2 one ?
>
> I used this patch, and the attached CD was found.
> What dma mask should be used in ohci_probe()?
Allright. So I see two problems here:
- in the code that powerpc uses to generate the PCI tree based on the
open firmware device-tree (instead of probing the bus), we don't set the
dma mask to the default ffffffff.
- our implementation of dma_supported() incorrectly tests against the
-previous- dma mask instead of the one we pass in as an argument.
I'll send a patch later today for you guys to test.
In addition, make sure that ieee1394 properly uses the parent PCI dev
and not some other intermediary struct device for the dma mask. Oh and,
don't do the set_dma_mask() in sbp2, it has nothing to do there. It
should be in the ohci1394 driver.
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH 2.6.22.y] ieee1394: revert "sbp2: enforce 32bit DMA mapping"
From: Benjamin Herrenschmidt @ 2007-08-06 21:43 UTC (permalink / raw)
To: Olaf Hering
Cc: Robert Hancock, linuxppc-dev, Stefan Richter, stable,
linux-kernel
In-Reply-To: <20070806115804.GA1734@suse.de>
On Mon, 2007-08-06 at 13:58 +0200, Olaf Hering wrote:
> On Sun, Aug 05, Stefan Richter wrote:
>
> > Benjamin Herrenschmidt wrote:
> > >>> If setting 32-bit DMA mask fails on ppc64, that sounds like a problem
> > >>> with the DMA implementation on that architecture. There are enough cards
> > >>> out there that only support 32-bit DMA that this really needs to work..
> > >> Yes, could the PPC folks please have a look at it? Thanks.
> > >
> > > Smells like we may have a bug there. No worries though, all current PPC
> > > machines have an iommu that will not give out addresses above 32 bits
> > > anyway, but I'll double check what's up.
> > >
> > > Do you see something in dmesg when that happens ?
> >
> > There was nothing in Olaf's report, except for trouble in sbp2 _after_
> > the failure. http://lkml.org/lkml/2007/8/1/344 (I don't have a PMac.)
>
> sbp2util_remove_command_orb_pool() does not check for lu->hi being NULL.
>
> dev->dma_mask is NULL too, thats why dma_direct_dma_supported() returns
> false, and dma_set_mask() will return -EIO.
Strange... PCI devices should never have a NULL dma mask. I wonder how
we get there...
Ben.
^ permalink raw reply
* Re: [PATCH] powerpc: Pegasos keyboard detection
From: Matt Sealey @ 2007-08-06 21:40 UTC (permalink / raw)
To: Alan Curry; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <200708020440.l724edCT290676@shell01.TheWorld.com>
Okay before you add to the nvramrc you also need to add probe-all to build the
device tree first; I assumed this was common knowledge.
probe-all
" /pci/isa/8042" find-device
" 8042" encode-string device-type
install-console
banner
That should do it. Without probe-all/install-console/banner in the nvramrc they
will be run by the boot process (this is documented in the IEEE 1275 specs)
Yeah it will break for people on their first boot but let's put a few things
across;
1) Pegasos has been discontinued. The number of people with this bug are in
the low thousands - if they upgrade to 2.6.22 at all.
2) The fix was in the wrong place anyway, if it was going to be done anywhere
at all it needs to be in arch/powerpc/kernel/prom_init.c:fixup_device_tree_chrp()
like the ISA ranges breakage (which is on Briq) and IDE IRQ misnumbering fix.
Not the keyboard platform driver.
3) In any case this should be something that is fixed in the firmware, as any
stalwart, stubborn Linux developer will rant at you about. If you can't get a
firmware update or it's not fixed, this is the best place to do it.
As for Segher, bootwrapper not such a good place as that's still mussing up
the kernel with these fixes. Let the boot loader do it for the OS, and don't
mess up the OS with device-tree fixups. After all it may not just be Linux
that stumbles on it. Why have the same patch in every OS?
With nvramrc, the fix is done for EVERY operating system from firmware upwards.
The semi-official Genesi line of support and what I have been told by the board
designer is if you need to fix something in the device tree, that is what nvramrc
is for, and that is why Open Firmware runs Forth scripts.
I have a fixup script for Pegasos and one for Efika which I may publish at some
point in the very near future. We may ship a small patch for Marcin Kurek's
"BootCreator" (http://tbs-software.com/morgoth/projects.html) which includes
all the stuff. We may write our own binary bootloader.. I am waiting for the
result of the new firmware feature requests before we waste time on stuff
bplan is silently fixing.
--
Matt Sealey <matt@genesi-usa.com>
Genesi, Manager, Developer Relations
Alan Curry wrote:
> Matt Sealey writes the following:
>> Yeah please do a fixup for the boot wrapper.
>>
>> Or, if you have trouble, go into the firmware and type "nvedit", add
>> these lines;
>>
>> " /isa/8042" find-device
>> " 8042" encode-string device-type
>>
>> (then ctrl-c to exit and nvstore to run it on next reboot. Try it without
>> the patch first, on the firmware console, just to be sure I got it right,
>> because I can't test it here)
>
> It works from the ok prompt but in the nvramrc it doesn't find the device.
> (pci/isa nodes not created yet?)
>
> But the larger point:
>
>> You don't need to patch Linux at all. In fact for silly things like this
>> I would recommend against it :)
>
> If the workaround doesn't go into the kernel, everybody with affected
> hardware has to individually find out about the bug (probably by experiencing
> an annoying keyboardless boot) and fix it himself. Is that worth the
> reduction in kernel clutter?
>
^ permalink raw reply
* Re: [RFC][PATCH] MPC832x_RDB: update dts to use spi, register mmc_spi stub
From: Kim Phillips @ 2007-08-06 21:31 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: linuxppc-dev
In-Reply-To: <5e9344956c9e6d81f75d14d67064b78e@kernel.crashing.org>
On Mon, 6 Aug 2007 20:25:13 +0200
Segher Boessenkool <segher@kernel.crashing.org> wrote:
> >>>>> + max-speed-hz = <bebc20>; /* 12500000 Hz */
> >>
> >> Just max-speed.
> >
> > Segher, how is this different from:
> >
> > http://ozlabs.org/pipermail/linuxppc-dev/2007-April/034557.html
>
> Not sure what you mean. I'm just saying that "speed-hz" is a
> terrible name, I'm not saying that "max-speed" is perfect at all.
yet you suggest a /more/ generic name, contrary to your prior comments.
My interpretation of your recent comments is that 'max-speed' is now a
valid property name for devices such as ucc_geth. Do I have that right?
Kim
^ permalink raw reply
* Re: [PATCH v2] cell: move SPU affinity init to spu_management_of_ops
From: Geoff Levand @ 2007-08-06 21:29 UTC (permalink / raw)
To: Paul Mackerras
Cc: linuxppc-dev, cbe-oss-dev, hch, Arnd Bergmann, Andre Detsch
In-Reply-To: <200708040925.49098.arnd@arndb.de>
Arnd Bergmann wrote:
> On Saturday 04 August 2007, Geoff Levand wrote:
>>
>> From: Andre Detsch <adetsch@br.ibm.com>
>>
>> This patch moves affinity initialization code from spu_base.c to a
>> new spu_management_of_ops function (init_affinity), which is empty
>> in the case of PS3. This fixes a linking problem that was happening
>> when compiling for PS3.
>> Also, some small code style changes were made.
>>
>> Signed-off-by: Andre Detsch <adetsch@br.ibm.com>
>> Signed-off-by: Geoff Levand <geoffrey.levand@am.sony.com>
>
>
> Acked-by: Arnd Bergmann <arnd.bergmann@de.ibm.com>
Hi Paul,
As this one fixes a build breakage, I would like you to queue it
up for Linus to pull at the next opportunity.
-Geoff
^ permalink raw reply
* Re: 2.6.23-rc1-mm2
From: Segher Boessenkool @ 2007-08-06 21:25 UTC (permalink / raw)
To: Mariusz Kozlowski
Cc: linux-usb-devel, gregkh, linux-kernel, linuxppc-dev, paulus,
Andrew Morton
In-Reply-To: <200708062134.16643.m.kozlowski@tuxland.pl>
>>> Second issue as reported earilier allmodconfig fails to build on imac
>>> g3.
>>>
>>> CC arch/powerpc/kernel/lparmap.s
>>> AS arch/powerpc/kernel/head_64.o
>>> lparmap.c: Assembler messages:
>>> lparmap.c:84: Error: file number 1 already allocated
>>> make[1]: *** [arch/powerpc/kernel/head_64.o] Blad 1
>>> make: *** [arch/powerpc/kernel] Blad 2
>>
>> Please send me the full output of:
>>
>> gcc --version (or whatever your gcc is called)
>> ld --version
>> ld --help (I know no better way to get the supported binutils
>> targets, and the default target)
>>
>> and the lparmap.s file. You might want to skip sending it
>> to the lists, it will be a bit big (and off-topic on most
>> of those lists, anyway).
>
> Well ... its 66kB. Not that bad. Please find it attached.
> Needed gcc and ld info below.
Thanks.
It seems like things go wrong when lparmap.s is generated with
(DWARF) debug info; could you try building it (manually) with -g0
added on the end of the compile line, and see if head_64.o compiles
okay for you then? If so, I'll prepare a proper patch for it, I
have a similar one (also for lparmap!) in my queue already...
Segher
^ permalink raw reply
* Re: my cpu is MPC860, kernel version is 2.6.20.14, the kernel start info in the mail. why does the kernel panic?
From: Scott Wood @ 2007-08-06 21:24 UTC (permalink / raw)
To: Dan Malek; +Cc: poorbeyond, Linux-ppc mail list
In-Reply-To: <A3D38E79-6921-4C9A-BD82-5796B8995BA4@embeddedalley.com>
Dan Malek wrote:
> On Aug 6, 2007, at 8:52 AM, Scott Wood wrote:
>
>> It wouldn't surprise me if the 8xx code has issues with PREEMPT; try
>> turning it off for now.
>
>
> What is your reason that 8xx would be any different?
It's just a suspicion based on my dealings with the CPM code, and the
fact that the crash appeared to happen in the CPM ethernet driver. It
could be wrong, but it doesn't hurt to try without PREEMPT and see if
the problem remains.
-Scott
^ permalink raw reply
* Re: my cpu is MPC860, kernel version is 2.6.20.14, the kernel start info in the mail. why does the kernel panic?
From: Dan Malek @ 2007-08-06 21:12 UTC (permalink / raw)
To: Scott Wood; +Cc: poorbeyond, Linux-ppc mail list
In-Reply-To: <46B743C6.1030905@freescale.com>
On Aug 6, 2007, at 8:52 AM, Scott Wood wrote:
> It wouldn't surprise me if the 8xx code has issues with PREEMPT; try
> turning it off for now.
What is your reason that 8xx would be any different?
-- Dan
^ permalink raw reply
* [PING][PATCH] PPC 4xx HW Watchpoint
From: rrbranco @ 2007-08-06 21:10 UTC (permalink / raw)
To: linuxppc-dev
[-- Attachment #1: Type: text/plain, Size: 218 bytes --]
Hello guys,
I'm just writting to know if someone have some opinion about the patch I
sent a week ago. I'm not sending it again cause I'm traveling and don't
have it here in this machine.
Tks,
Rodrigo (BSDaemon).
[-- Attachment #2: Type: text/html, Size: 406 bytes --]
^ permalink raw reply
* Re: [PATCH 2/6] PowerPC 440EPx: Sequoia DTS
From: Segher Boessenkool @ 2007-08-06 21:03 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: linuxppc-dev, David Gibson
In-Reply-To: <46B76A5A.3030300@ru.mvista.com>
>> "bit-swizzling" or something which can be defined to describe some odd
>> connections. If absent we'd default to direct mapping. Segher, is
>> that idea going to cause you to scream?
>
> Actually, checking for the presence of all possible perverted
> mapping
> props doesn't seem a good idea -- it's simpler to check for the
> presence of
> one prop (like "direct-mapped" I was thinking about, or maybe
> "simple-map").
There shouldn't be many "perverted" properties -- really weird
mappings can just do a special "compatible" value, the standard
kernel driver can't handle them either, anyway.
A few extra property lookups are dirt cheap. This isn't
speed-critical code anyway.
>> Well, now you do. I believe USB usually uses that format, too.
>
> USB what, hosts or devices?
The unit-address of a device is specified in its parent's
address space. USB hosts don't typically have a USB bus as
parent ;-) (but they can still have a comma in the unit
address. Confused? Don't be :-) ).
> Hm, right... the option here would be to always have at least one
> partition and no "reg" property in the MTD node itself... or have
> "reg" with
> no partition and "ranges" if partitions are there... :-)
No way. "reg" is needed *no matter what* -- I'll not list the
other problems with that proposal.
Segher
^ permalink raw reply
* Re: [PATCH 2/6] PowerPC 440EPx: Sequoia DTS
From: Segher Boessenkool @ 2007-08-06 20:55 UTC (permalink / raw)
To: David Gibson; +Cc: linuxppc-dev
In-Reply-To: <20070806043147.GC6103@localhost.localdomain>
>>> + - compatible : should contain the specific model of flash
>>> chip(s) used
>>> + followed by either "cfi-flash" or "jedec-flash"
>>
>> Duh, have nearly forgotten to complain about "-flash" suffix.
>> Isn't it
>> superfluous?
>
> For CFI, I guess so. But don't JEDEC standardise other things as well
> as flash? I think "-flash" makes the description a bit more obvious,
> but I'll be swayed if a few other people chime in with opinions on
> this.
How about I'll just veto making the names any shorter. Problem
solved :-)
Segher
^ permalink raw reply
* Re: [PATCH 2/6] PowerPC 440EPx: Sequoia DTS
From: Segher Boessenkool @ 2007-08-06 20:54 UTC (permalink / raw)
To: David Gibson; +Cc: linuxppc-dev
In-Reply-To: <20070806042109.GB6103@localhost.localdomain>
> Aha! Ok, now I understand the sorts of situations you're talking
> about. By "not direct mapped", I thought you were talking about some
> kind of access via address/data registers on some indirect bus
> controller, rather than weird variations on endianness and
> bit-swizzling.
>
> Hrm.. this is a property of how the flash is wired onto the bus,
> rather than of the flash chips themselves, so I'm not entirely sure
> where description of it belongs.
>
> Simplest option seems to me to add a property "endianness" or
> "bit-swizzling" or something which can be defined to describe some odd
> connections. If absent we'd default to direct mapping. Segher, is
> that idea going to cause you to scream?
No, that's fine with me. I would recommend either using a
_good_ _descriptive_ name for such a property describing the
swizzling, if this swizzling is common; or just put the whole
bloody weirdo address permutation into some nice big array,
something like
address-permutation = <0 1 3 2 4 5 7 6 e f d c a b 9 8>;
>> Well, "device-width" is not the thing that we care about either.
>> ;-)
>
> Well, yes but we need to encode it somehow. Segher preferred
> device-width to interleave, because it's closer to a description of
> the actual hardware, rather than an abstration decribing its wiring.
>
> In other words I *still* don't see any reason to prefer giving the
> interleave.
You *need* to know the device width. You *need* to know
the bank width. And for 99% of the cases you don't need
to know anything more since most hardware designs aren't
insane. Case closed I'd say.
>> Didn't know that the spec allows commas after @...
>
> Well, now you do. I believe USB usually uses that format, too.
PCI too, and it even has an official binding :-)
>> Don't we need "ranges" here, at least from the formal PoV -- as
>> the parent
>> and child address spaces differ? I know the physmap_of parser doesn't
>> care but
>> still...
>
> That's one I've wondered about. To describe the partitions address
> space as lying (ultimately) in the physical address space, which in a
> sense it does, yes we'd need a ranges property here. But we also have
> a 'reg' at the top level which would overlap with that hypothetical
> ranges which would be weird. Or we could exclude the top-level reg,
> but then that's a pain if we do want to map the flash as a whole.
>
> So I left out ranges, on the grounds that there isn't actually
> anything at present which will attempt to access flash partitions
> "generically" as a device tree device.
It looks good to me like this.
In a real OF, the "register" access for the flash partition
node would be handled by its parent node, which would know
to do the direct-mapping thing (at least mapping it to _its_
parent, which typically asks the nodes further up, etc.)
For the kernel world, we should just document it in the binding.
> I'm not sold on this approach, but I haven't heard you give a better
> argument yet.
I haven't heard or thought of anything better either. Using "ranges"
is conceptually wrong, even ignoring the technical problems that come
with it.
Segher
^ permalink raw reply
* Re: [PATCH 2/6] PowerPC 440EPx: Sequoia DTS
From: Segher Boessenkool @ 2007-08-06 20:41 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: linuxppc-dev, David Gibson
In-Reply-To: <46B357D3.1040608@ru.mvista.com>
>> + - compatible : should contain the specific model of flash
>> chip(s) used
>> + followed by either "cfi-flash" or "jedec-flash"
>
> Duh, have nearly forgotten to complain about "-flash" suffix.
> Isn't it superfluous?
No, it describes what kind of thing this is. "cfi" and "jedec"
don't say much: "cfi" could equally well mean the "cute friendly imp",
and who knows what kind of JEDEC-interface device this is supposed
to indicate. "cfi-flash" and "jedec-flash" on the other hand are
sufficiently clear, even if they aren't completely specific; this
is a very common device so some leeway is in order.
Segher
^ permalink raw reply
* Re: [PATCH 2/6] PowerPC 440EPx: Sequoia DTS
From: Segher Boessenkool @ 2007-08-06 20:35 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: linuxppc-dev, David Gibson
In-Reply-To: <46B34E1F.5060009@ru.mvista.com>
>> To be honest, I'm not sure that describing the mapping is really the
>> job of the compatible property. That the flash is mapped into the
>> address space is kind of implicit in the way the reg interacts with
>> the parents' ranges property.
>
> Ah, I keep forgetting about implied 1:1 parent/child address
> correspondence... :-<
It's not implied, it is explicitly specified.
> But does it really imply how the (low) address bits of the *child*
> bus
> ("ebc" in this case) are connected to the chip? I don't think so...
The currently proposed binding assumes linear mapping.
Strange setups will need to extend this binding; this
does mean that the burden for that is not on the "normal"
case.
There are so many weird ways in which people wire up memory
devices that there is no chance to define a "generic" binding
that works for all in less than 400 pages of documentation.
> Well, "device-width" is not the thing that we care about either.
> ;-)
You need to know the device-width to drive the chip (and no,
it isn't enough to know the exact chip model even) -- why do
you say you don't care about it?
>>> Hmm... what does @2,0 mean? :-O
>
>> EBC chip select 2, offset 0.
>
> Well, so this node is under some kind of local bus node -- that's
> good.
> Didn't know that the spec allows commas after @...
Most characters are allowed in the unit-address... The following
is just fine: "my-secret-base@the-moon". ISA uses letters to
distinguish between its different address spaces, for example.
There is a direct correspondence between the first "reg"
address and the text representation of the unit address;
this correspondence is bus-dependent however.
David, can multiple devices sit on the same chip-select
on EBC, or on the same "minor" address? If not, you can
simplify your unit address representation.
>> "direct-mapped" is simply not a sufficiently specific compatible
>> property, no two ways about it.
>
> Yes, for example "direct-mapped-cfi" and "direct-mapped-jedec"
> would have
> been better...
Nah. These memory devices are meant to sit at some address/data
bus; whether it is direct-mapped or not is obvious from the node
hierarchy the flash node hangs under already; let's make the simple
case simple and the complicated cases possible.
Segher
^ permalink raw reply
* Re: [PATCH 2/6] PowerPC 440EPx: Sequoia DTS
From: Segher Boessenkool @ 2007-08-06 20:20 UTC (permalink / raw)
To: David Gibson; +Cc: linuxppc-dev
In-Reply-To: <20070803031349.GD6418@localhost.localdomain>
>>> + - compatible : should contain the specific model of flash
>>> chip(s) used
>>> + followed by either "cfi-flash" or "jedec-flash"
>>
>> This "compatible" prop (and the node in whole) doesn't say a
>> thing about how the flash is mapped into the CPU address space. I
>> strongly disagree that this node provides enough info to select a
>> driver. :-/
>
> To be honest, I'm not sure that describing the mapping is really the
> job of the compatible property. That the flash is mapped into the
> address space is kind of implicit in the way the reg interacts with
> the parents' ranges property.
Exactly.
> Can you describe some of the options for *not* direct mapped flash
> chips - I can't reasonably come up with a way of describing the
> distinction when I've never seen NOR flash other than direct mapped.
What, you've never seen SPI flash or IIC flash? :-)
Not that these are handled by the MTD framework AFAIK.
Segher
^ permalink raw reply
* Re: [PATCH 2/6] PowerPC 440EPx: Sequoia DTS
From: Segher Boessenkool @ 2007-08-06 20:15 UTC (permalink / raw)
To: Josh Boyer; +Cc: linuxppc-dev, David Gibson
In-Reply-To: <20070802151513.12e9caf7@weaponx.rchland.ibm.com>
>>> + UIC0: interrupt-controller0 {
>>> + compatible = "ibm,uic-440gp","ibm,uic";
>>
>> The first compatible entry should always be the precise model, so in
>> this case "ibm,uic-440epx". If it is (supposed to be) identical to
>> the UIC in the 440GP, it can also have an "ibm,uic-440gp" entry, but
>> since I believe all the UICs are supposed to operate the same, I think
>> that's implicit in the "ibm,uic" entry.
>
> Most UICs are the same. There are some oddball chips that either hide
> particular registers because they are unused, or they change the
> addressing stride. I'm not sure that is a common enough case to worry
> about now though.
You only need to worry about the oddball cases in the device
trees for a device that uses one off those.
It is prudent to put the exact name of the device you're
working with in there whenever possible, in case you later
discover it has some quirks. If that doesn't happen, the
kernel can happily keep probing on the more generic name.
Segher
^ permalink raw reply
* Re: [PATCH 2/6] PowerPC 440EPx: Sequoia DTS
From: Segher Boessenkool @ 2007-08-06 20:12 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: linuxppc-dev, David Gibson
In-Reply-To: <46B1F6D4.3070707@ru.mvista.com>
>> + - compatible : should contain the specific model of flash
>> chip(s) used
>> + followed by either "cfi-flash" or "jedec-flash"
>
> This "compatible" prop (and the node in whole) doesn't say a thing
> about how the flash is mapped into the CPU address space.
...and it shouldn't. That's what "ranges" properties in all
the various (grand-)parents of the node are for.
> I strongly disagree that this node provides enough info to select a
> driver. :-/
If Linux needs more information than what the device _is_, but
also needs information about how it is _used_ on some particular
hardware, to select a driver; then it can bloody well do so.
Nowhere is it said that an OS can _only_ use "compatible" properties
to do its driver selection.
>> + - reg : Address range of the flash chip
>> + - bank-width : Width (in bytes) of the flash bank. Equal to
>> the device width
>> + times the number of interleaved chips.
>> + - device-width : (optional) Width of a single flash chip. If
>> omitted,
>> + assumed to be equal to 'bank-width'.
>
> Why then not just introduce the "interleave" prop and obsolete the
> "bank-width"?
Because "interleave" is overly complicated and still doesn't handle
all cases. Also, it's a more confusing name.
The goal here is to handle 98% (or just 90% even) of all cases in
as simple a way as possible; everything else can get special handling
later.
>> + Flash partitions
>> + - reg :
>> + - read-only : (optional)
>
> All that would look nice but partition is even less of a device
> than the
> original "rom" node was... well, who cares now? :-)
Some partitions can be useful for the firmware itself, or for
early boot stages; those should be described in the device
tree in some way. And yes, you certainly can consider an
(aligned) flash partition to be a subdevice of the whole flash
bank.
> Oh, I see that the new partition representation have really
> simplified
> parsing -- this function is hardly shorter than the old one... :-P
Neither simplifying machine-parsing nor compacting the kernel
code are a goal at all, I don't see why you bring this up.
>> static struct of_device_id of_physmap_match[] = {
>> {
>> + .compatible = "cfi-flash",
>> + .data = (void *)"cfi_probe",
>> + },
>> + {
>> + .compatible = "jedec-flash",
>> + .data = (void *)"jedec_probe",
>> + },
>> + {
>
> This would also trigger on non-linearly mapped CFI or JEDEC flashes
No, it wouldn't.
>> large-flash@2,0 {
>
> Hmm... what does @2,0 mean? :-O
>
>> reg = <2 0 400000>;
"2,0" is the text representation for the unit address <2 0>
on this bus.
Segher
^ permalink raw reply
* Re: [PATCH 2/6] PowerPC 440EPx: Sequoia DTS
From: Segher Boessenkool @ 2007-08-06 19:59 UTC (permalink / raw)
To: David Gibson; +Cc: linuxppc-dev
In-Reply-To: <20070801054751.GM31391@localhost.localdomain>
>>> Yeah, better names please -- if possible, something that someone
>>> without knowledge of this SoC will understand what it is.
>>
>> I think the names are probably ok - I'm assuming they're in keeping
>> with the convention I've used of using the same names / abbreviations
>> as in the CPU user manual. I'm asking just for my own information,
>> although a comment might not be a bad idea.
Fine with me -- I personally prefer "system-device-controller"
and "clock-power-controller" or similar, but that is mostly a
matter of taste. As long as it's human readable it's fine.
> - Required properties:
> + - compatible : should contain the specific model of flash
> chip(s) used
"if known".
> + followed by either "cfi-flash" or "jedec-flash"
> + Flash partitions
> + - reg :
> + - read-only : (optional)
I'll hold off commenting on this until you've finish writing it,
you probably know my opinion about it anyway :-)
One thing though -- what _exactly_ does "read-only" signify?
Segher
^ permalink raw reply
* Re: 8250.c::autoconfig() fails loopback test on MPC824[15]
From: Guennadi Liakhovetski @ 2007-08-06 19:54 UTC (permalink / raw)
To: Segher Boessenkool
Cc: linuxppc-dev, Jon Loeliger, linux-serial, Arnd Bergmann
In-Reply-To: <1288ff079974aa806119584018bab973@kernel.crashing.org>
On Mon, 6 Aug 2007, Segher Boessenkool wrote:
> > Maybe the best solution would be for 824[15] to not claim compatibility
> > with 8250 at all then.
>
> Or at least it should have a more specific entry for this
> "special" 16x50 UART, and that one should be probed first.
>
> > If the device tree contains an entry that matches
> > what the generic driver looks for, it better be something that can
> > be handled by that driver.
>
> Pretty much; you can't make this rule too strict though,
> if a device mostly works with the generic driver, you can
> claim compatibility with it -- keep in mind that that can
> come back to bite you though, like in this case. The
> advantages do outweigh the disadvantages sometimes, it's
> all a tradeoff; avoid it if possible.
Well, the 8250 driver DOES already support devices without the loopback
test support, that's what that bit there is for, isn't it? So, we just
have to use it, as well as others do (grep -r UPF_SKIP_TEST
drivers/serial/). I'll go for the extra compatible property, as suggested,
seems like the most commonly accepted wa to handle this (and the easiest
to implement).
Thanks
Guennadi
---
Guennadi Liakhovetski
^ permalink raw reply
* Re: [PATCH] mark PCI resource with start 0 as unassigned
From: Alan Cox @ 2007-08-06 19:52 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: linuxppc-dev, Olaf Hering, linux-ide
In-Reply-To: <04ce93c63dfaa543b4068d448c8115d8@kernel.crashing.org>
O> That's of course the smarter choice, _if_ we have a choice at
> all -- on PowerPC, the PCI setup on certain platforms is done
> by the firmware (and we don't want to mess with it for various
> reasons), and some _do_ map PCI legacy I/O at 0.
>
> Not in this case though, so let's just ignore that possibility
> until it hits us in the face :-)
An SFF controller mapped at I/O is basically not going to work. Take it
up with the firmware
^ permalink raw reply
* Re: [PATCH 3/3] First cut at PReP support for arch/powerpc
From: Segher Boessenkool @ 2007-08-06 19:45 UTC (permalink / raw)
To: David Gibson; +Cc: linuxppc-dev, Paul Mackerras, Yoder Stuart-B08248
In-Reply-To: <20070806040249.GA6103@localhost.localdomain>
>> However, the interrupt mapping spec says that all interrupt
>> controller (regardless of device_type) must have a
>> property named "interrupt-controller" to identify
>> the device node as an interrupt controller and root of
>> a interrupt tree.
>> See: http://playground.sun.com/1275/practice/imap/imap0_9d.html
>
> Ah, yes. Added to both the mpic and 8259.
See? Even if I make mistakes some good comes from it!
;-) ;-) ;-)
Segher
^ permalink raw reply
* Re: [PATCH 3/3] First cut at PReP support for arch/powerpc
From: Segher Boessenkool @ 2007-08-06 19:43 UTC (permalink / raw)
To: Jon Loeliger; +Cc: linuxppc-dev@ozlabs.org, Paul Mackerras, David Gibson
In-Reply-To: <1186154678.9004.22.camel@ld0161-tx32>
>>>> + MPIC: interrupt-controller@d {
>>>> + device_type = "open-pic";
>>>>
>>>> device_type = "interrupt-controller".
>>
>> Not according to the binding in booting-without-of.txt
>
> My understanding here, though possibly flawed, is that the current
> implementation has "open-pic" but _should_ have "interrupt-controller"
> as that is the officially correct name.
>
> I _think_ this means we need a transitional period where we update
> the code to look for "interrupt-controller", and obsoletedly, looks
> for the "open-pic", while we transition to the new, correct name.
>
> I'm just betting that if I'm wrong Segher will tell me! :-)
You're wrong :-)
The kernel shouldn't look at device_type at all.
Segher
^ 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