Re: ppc compile failure (__flush_icache_range etc undeclared)
Tried 2.6.26-rc1+git on ppc (arch/ppc, PReP subarch) and got this: arch/ppc/kernel/ppc_ksyms.c: At top level: arch/ppc/kernel/ppc_ksyms.c:152: error: '__flush_icache_range' undeclared here (not in a function) That's fixed by [2/3] in http://ozlabs.org/pipermail/linuxppc-dev/2008-May/056010.html Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 3/3] [POWERPC] 86xx: mpc8610_hpcd: add watchdog node
+ compatible = mpc83xx_wdt, fsl,mpc8610-wdt; You should put the most specific entry first. Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 0/4] minor fixes for 2.6.26
I think that adding zImage.* to clean-files is a bad idea, because we have zImage.lds.S, zImage.coff.lds.S and zImage.ps3.lds.S in arch/powerpc/boot that we don't want deleted. It would be OK for compiling with a separate object directory but would be bad for compiling in the source directory. Arguably this suggests we should rename either the lds files or the zImages so that we *can* get a pattern to match. Not really. Just define a $(ALL_ZIMAGES) var and use that everywhere. I would be pretty upset if make clean decided to delete my zImage.backup, etc. Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 3/4] spi: Add OF binding support for SPI busses
Ok, elegance apart:-) You can use the SPI-bridge construct to also describe simple SPI-chipselect configurations. But is it really a good idea? Wouldn't it be better to handle these two cases separately? It would be best to handle all these things that are specific to a certain SPI controller (like how CSs work) in the binding for that SPI controller, and not try to shoehorn all of this into some artificial generic framework. If you can have identical addresses on the SPI bus going to different devices based on which CS is asserted, you'll have to make the CS part of the reg. Example: spi-controller { #address-cells = 2; #size-cells = 0; [EMAIL PROTECTED],f000 { reg = 0 f000 ; } // CS 0, SPI address f000 [EMAIL PROTECTED],f000 { reg = 1 f000 ; } // CS 1, SPI address f000 [EMAIL PROTECTED],ff00 { reg = 1 ff00 ; } // CS 1, SPI address ff00 } SPI-to-SPI bridges can (and should!) be handled the same way as anything-to-anything-else bridges are handled as well: either there is a nice simple one-to-one matching (and you can use ranges) or you need a driver for that bridge that knows how to make it work (or both, ranges isn't always enough, the bridge might require some specific handling for some special situations -- error handling, suspend, whatever). Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [patch 2/2] powerpc: optimise smp_wmb
From memory, I measured lwsync is 5 times faster than eieio on a dual G5. This was on a simple microbenchmark that made use of smp_wmb for store ordering, but it did not involve any IO access (which presumably would disadvantage eieio further). This is very much specific to your particular benchmark. On the 970, there are two differences between lwsync and eieio: 1) lwsync cannot issue before all previous loads are done; eieio does not have this restriction. Then, they both fly through the execution core, it doesn't wait for the barrier insn to complete in the storage system. In both cases, a barrier is inserted into both the L2 queues and the non-cacheable queues. These barriers are both removed at the same time, that is, when both are the oldest in their queue and have done their thing. 2) For eieio, the non-cacheable unit waits for all previous (non-cacheable) stores to complete, and then arbitrates for the bus and sends an EIEIO transaction. Your benchmark doesn't do non-cacheable stores, so it would seem the five-time slowdown is caused by that bus arbitration (and the bus transaction). Maybe your cacheable stores hit the bus as well, that would make this worse. Your benchmark also doesn't see the negative effects from 1). In real code, I expect 2) to be pretty much invisible (the store queues will never be completely filled up), but 1) shouldn't be very bad either. So it's a wash. But only a real benchmark will tell. Given the G5 speedup, I'd be surprised if there is not an improvment on POWER4 and 5 as well, The 970 storage subsystem and the POWER4 one are very different. Or maybe these queues are just about the last thing that _is_ identical, I dunno, there aren't public POWER4 docs for this ;-) although no idea about POWER6 or cell... No idea about POWER6; for CBE, the backend works similar to the 970 one. Given that the architecture says to use lwsync for cases like this, it would be very surprising if it performed (much) worse than eieio, eh? ;-) So I think your patch is a win; just wanted to clarify on your five-time slowdown number. HTH, Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [patch 2/2] powerpc: optimise smp_wmb
+#ifdef __SUBARCH_HAS_LWSYNC +#define SMPWMB lwsync +#else +#define SMPWMB eieio +#endif + #define smp_mb() mb() #define smp_rmb() rmb() -#define smp_wmb() eieio() +#define smp_wmb() __asm__ __volatile__ (__stringify(SMPWMB) : : :memory) SMPWMB is used only here. Why not just #ifdef __SUBARCH_HAS_LWSYNC #define smp_wmb() lwsync() #else #define smp_wmb() eieio() #endif ? Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [patch 2/2] powerpc: optimise smp_wmb
This is mostly useless then since lwsync is just a sync to a processor that doesn't know it (it's a sync with a reservd bit set) :-) Or it's just to make gas happy if you specify a processor type that doesn't have lwsync ? GAS doesn't care (I tried with -Wa,-m405). Support for this insn was added to binutils in august 2001; do we still support older binutils than that? If so, we should just do the .long trick in a lwsync() inline function, like we do for similar cases. Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [patch 2/2] powerpc: optimise smp_wmb
The main question is do we care if the downgrade to sync on power3 hurts performances (and does it ?) and what do we do for 32 bits as currently, no 32 bits implementation has lwsync afaik (though that might not be true for long). Some time ago, I benchmarked (*) a loop of stw;sync vs. stw;eieio on a 750 (yeah, great micro-benchmark, heh); and the sync version was a factor ~100 slower. Also, we don't, I think, have verified that they all properly ignore the added bit and behave as sync rather than program checking.. The architecture books don't have a programming note about this (like they do in similar cases), so either a) it works fine on every implementation; or b) someone forgot to add such a note. We shouldn't use lwsync on 32-bit (it's an awful performance hit on classic cpus, and who knows what it does on embedded cpus (bookE *shudder*)). For POWER3 and *star, i.e., all 64-bit? Perhaps it'll be okay there, but if as you suggest some 32-bit CPUs will add proper lwsync support soonish, maybe a feature fixup or some such is in order. Segher (*) Not on purpose. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 6/7] [POWERPC] booting-without-of: add FHCI USB, FSL MCU, FSL UPM and GPIO LEDs bindings
+ - fsl,fullspeed-clock : specifies the full speed USB clock source in +clknum or brgnum format. + - fsl,lowspeed-clock : specifies the low speed USB clock source in +clknum or brgnum format. What format is num in? + - fsl,usb-mode : should be host. If that's the only possible value, this property is unnecessary, no? It probably would make sense to make this optional (and default to host) anyway. + - linux,hub-power-budget : optional, USB power budget for the root hub +in mA. Why is this linux-specific? +w) Freescale MCU with MPC8349E-mITX compatible firmware + +Required properties: + - compatible : fsl,mcu-chip-board, fsl,mcu-mpc8349emitx; + - reg : should specify I2C address (0x0a). + - #address-cells : should be 0. + - #size-cells : should be 0. +x) Freescale Localbus UPM programmed to work with NAND flash Similar here, except this one is never a GPIO controller. If the point to have #a = #s = 0 is to not have a unit-address in the child nodes: you should do that simply by not specifying a reg in the child nodes. +y) LEDs on GPIOs This one is so full of linux, stuff that I won't review it -- I wouldn't know where to start, sorry. Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [RFC PATCH 2/2] mmc: add OpenFirmware bindings for the mmc_spi driver
+ Required properties: + - #address-cells : should be 0. + - #size-cells : should be 0. Are these properties required at all? Will this node have any children. You mean, does this node define a bus. If it doesn't, there shouldn't be #a and #s; if it does, the binding should describe what the addressing is. #a = #s = 0 describes a bus without any addressing; pretty unusual :-) + - linux,mmc-ocr-mask : (optional) Linux-specific MMC OCR mask +(slot voltage). Should this property be better defined? Yes please. There's a good chance that it doesn't turn out to be Linux-specific at all (after some modification, perhaps). Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [RFC PATCH 2/2] mmc: add OpenFirmware bindings for the mmc_spi driver
The real problem is we don't yet have good method (or place) to apply a translation table from compatible values to modaliases. Ideally, the translations should be part of the drivers themselves, but that causes a chicken and egg problem of needing to load the driver to get access to the table to know if it is the correct driver... Of course, I'm really not very familiar with the whole module autoloading mechanism. Regardless; binding should be based on compatible, not on a hacky and bogus linux,modalias property. i2c exactly has the same problem. Here the compatible entry is used in drivers/of/of_i2c.c and mangled into a name to be used as modalias. It's still sort of hackish, but it seems to be a compromise acceptable by both OF and i2c folks. It's perfectly acceptable. The sole purpose of compatible is for a client (i.e., the kernel) to decide what to do with this device; most often, to decide what driver to use. It would be nice to have a completely generic thing that matches device tree nodes to drivers, and it sounds perfectly reasonable to me to go via modalias for that (i.e., just translate from device tree namespace to the kernel driver namespace). As a bonus, it would make driver matching more correct in some places: currently, whatever driver matches first wins (i.e., which compatible value is tried first), but the ordering of different values in compatible is supposed to be significant (whatever is mentioned there first should win). Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: dtc: Remove some small bashisms from test scripts
Some of the helper scripts used to run testcases contain some constructs that are bashisms. Or at least which don't work on dash, the minimal shell used as /bin/sh on recent Ubuntu systems. Both of these (the redirection, and source searching the current directory if the script isn't found in $PATH) are indeed bashisms. This patch looks good to me. Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 1/2] update crypto node definition and device tree instances
Nice cleanup! Just one thing... +- compatible : Should contain entries for all compatible SEC versions, + high to low, e.g., fsl,sec2.1, fsl,sec2.0 *All* compatible versions? That's not really correct -- for example that would include *future* versions! The first entry should describe the exact device version. If there are more entries, they should be for device versions where the driver for that device version can be reasonably expected to do something useful with this newer device (reduced functionality, perhaps). Listing *all* compatible devices is a) infeasible, b) not useful, and c) insane :-) Say you have a 3.3 device, and all 3.x devices have the same programming interface; also, the 2.x interface works with reduced functionality, and 1.x isn't useful at all; in that case, you would list 3.3, 3.0, 2.0. The driver that knows about 3.x would probe for 3.0, while an older driver would probe for 2.0. The driver doesn't need to probe for 3.3, since devices implementing 3.3 should show they are compatible with 3.0 (and the binding should say they should show this). Also, the binding should explicitly list all defined compatible entries (and what they mean), not just give a few examples. Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] [POWERPC] Cleanup mpic nodes in .dts
Removed clock-frequency and big-endian props as they aren't specified anywhere. If you remove big-endian, you'll have to provide some other way to get that information (like, some new compatible value). Dunno if we need clock-frequency. This patch also removes built-in properties. I'm all for that, but the patch description didn't say it does. Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] [POWERPC] 85xx: Add next-level-cache property
Added next-level-cache to the L1 and a reference to the new L2 label. Where is this property defined? I can't find it. The PowerPC binding defines an l2-cache property for this (it points from CPU node to L2 cache node, from L2 cache node to L3 cache node, from L3 cache node to L4 cache node, etc.) Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] [POWERPC] Cleanup mpic nodes in .dts
Removed clock-frequency and big-endian props as they aren't specified anywhere. If you remove big-endian, you'll have to provide some other way to get that information (like, some new compatible value). I'm all for big-endian but we don't spec this anywhere and aren't using it right now. So until we have an real need to start an extended mpic definition I'm getting rid of it. If we would remove everything insufficiently documented, not much would be left. This doesn't seem very productive to me. Could you instead just add some TODO somewhere? Dunno if we need clock-frequency. Not used today. Sure, the kernel might not use it today, but that's no reason to remove stuff from the device tree. I'm not against removing clock-frequency though, it's not well-defined, and what would it be useful for anyway? This patch also removes built-in properties. I'm all for that, but the patch description didn't say it does. will add that to the commit message. Thanks. Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] [POWERPC] 85xx: Add next-level-cache property
Added next-level-cache to the L1 and a reference to the new L2 label. Where is this property defined? I can't find it. The PowerPC binding defines an l2-cache property for this (it points from CPU node to L2 cache node, from L2 cache node to L3 cache node, from L3 cache node to L4 cache node, etc.) So looking at the PPC binding its not terrible clear about l3-cache being a valid property. It isn't. The property is called l2-cache at every level. I believe the discussion w/ePAPR was to create something a bit more generic and clarify/update the PPC binding. Nasty. Sure, l2-cache isn't the nicest name to point to deeper cache levels, but introducing a new property with (substantially) the same semantics is worse. There really shouldn't be a new property name until new functionality is introduced. For example, it could allow to describe more than one cache at each level (the current binding already allows more than one parent for each cache, but only one child; and cache hierarchies like that actually exist). I'm going to stick with the new binding as we don't use this linking currently. Dunno what's the best thing to do here. If you don't need the functionality yet, it might be best to postpone putting either property in there. Sigh, what a mess. Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: cell-index vs. index vs. no index in I2C device nodes
I think we should just expand the definition of cell-index to include standard device enumeration for when it's needed. The original definition is too limited, IMHO. nak if you need explicit indexing then use an alias. My opinion however is that explicit indexing is unnecessary and is just an artifact of current i2c subsystem internals. There is already enough information in the device tree to match i2c devices with i2c busses without resorting to indexes. Fully agreed. Let me say it as well, it makes me feel powerful: NAK Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: cell-index vs. index vs. no index in I2C device nodes
if you need explicit indexing then use an alias. My opinion however is that explicit indexing is unnecessary and is just an artifact of current i2c subsystem internals. There is already enough information in the device tree to match i2c devices with i2c busses without resorting to indexes. Not for ALSA SoC V2 devices. In ASoC V2, the fabric driver needs to identify the codec by its specific I2C bus and address number. The codec driver is not an OF driver (normally), so it doesn't have access to any OF data. It's just an I2C driver, so its given an I2C address and some number that represents an adapter. Therefore, the fabric driver and the codec driver need to independently determine the I2C bus number, and they need to match. The fabric driver parses the OF tree and looks up the cell-index property. The codec driver uses the adapter-nr variable. The patch I posted ensures that the two contain the same number. Sounds to me like both simply need to use adapter-nr. For access to Linux-internal data structures (and that is what this index is), you shouldn't have to go via the device tree. If the Linux data structures do not have the information you need, well, fix that. Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: arch/ppc is going away Real Soon Now
[Fixed up the collision between Grant and Olof] Grant Likely [EMAIL PROTECTED] wrote: Paulus, Can we just kill all of arch/ppc for .27 right now? Acked-by: Josh Boyer [EMAIL PROTECTED] Acked-by: Stephen Neuendorffer [EMAIL PROTECTED] Acked-by: Arnd Bergmann [EMAIL PROTECTED] Acked-by: Grant Likely [EMAIL PROTECTED] Acked-by: Olof Johansson [EMAIL PROTECTED] Acked-by: Segher Boessenkool [EMAIL PROTECTED] Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: cell-index vs. index vs. no index in I2C device nodes
No; use an alias in the aliases node. That is what aliases is designed for. Something like 'index' is a reinvention of the wheel. Do aliases work in reverse? That is, if I have a pointer to a device node, can I look up its alias directly? Or do I have to scan the aliases node and do a comparison of each phandle, one at a time, until I find a match? And when I find a match, will I need to do sscanf() in order to extract the actual index value from the property? Aliases are one-way. You can have multiple aliases point to the same node, as well (and that is quite common, even). If you need a unique identifier for an OF node, use its phandle. It sounds to me like you just need to set up a mapping between phandles and Linux i2c bus ids here? Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: cell-index vs. index vs. no index in I2C device nodes
From a device tree perspective, index and cell-index are both incorrect. The IIC macros don't share register blocks with anything, are enumerated as unique instances per macro in the device tree, and should be able to be distinguished by regs and/or unit address. Does anyone disagree with that? Hear, hear. x2. Aliases can also provide a reasonable way of enumerating devices, if reg isn't suitable on its own. Yes. In almost all cases, drivers and subsystems do not need this at all though. In OF, one device points to another by putting the phandle of that pointed-to device in some property (and buses are represented by their parent bridge). If the Linux subsystem wants to use an integer for distinguishing between its various buses, that's fine, but it can just make up these numbers -- the numbers themselves have no actual meaning, only the relationships expressed via those numbers do. In a few cases, particularly where those numbers are user-visible, like in ethN, the aliases construct is a good solution. If a driver/subsystem is relying on the aliases though, it should document this in a (platform?) binding -- and it would better have a very good reason for it! Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: Default flats for running dtc from kernel build
DTS_FLAGS ?= -P 0x1000 1k seems like more than enough default padding. Yes. So why specify 4kB? /me hides Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: cell-index vs. index vs. no index in I2C device nodes
Well, I just don't see the point of having two different properties that say the same thing. I'm not an IEE 1275 purist, so I don't think we should be hampered by old node definitions. I especially don't like having a property specifically for indexing I2C nodes that can't be used to enumerate other nodes. It's not about purity. It's about overloading something that has existing semantics just because you have one usecase that you _think_ needs it. If everyone did that, this whole device tree concept is going to just be one big cluster f*ck. One important way of preventing this overloading and death-by-complexity is to make most properties specific to a particular binding. It is good if other bindings that need similar functionality can use similar properties, or sometimes even identical ones; but there are only a few properties that are defined for *every* node. Trying to make stuff too generic just doesn't work. Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: cell-index vs. index vs. no index in I2C device nodes
Aliases can also provide a reasonable way of enumerating devices, if reg isn't suitable on its own. Yes. In almost all cases, drivers and subsystems do not need this at all though. In OF, one device points to another by putting the phandle of that pointed-to device in some property (and buses are represented by their parent bridge). If the Linux subsystem wants to use an integer for distinguishing between its various buses, that's fine, but it can just make up these numbers -- the numbers themselves have no actual meaning, only the relationships expressed via those numbers do. That's true. However if all the system documentation uses a particular numbering of the devices, it's convenient if we can use the same numbering in Linux. Sure, it's convenient, and you can use aliases to get the naming you want accessible to the user. The drivers shouldn't depend on this naming internally though, esp. since it doesn't really help anything. Incidentally, another word on cell-index. Even for its intended purpose, this was always a compromise. The strictly correct way to handle shared registers like this is for the node representing the shared resource to have a table of phandles pointing back to the devices controlled by the shared registers from which the appropriate indices can derived. IMO, there is no really good (let alone correct) way of handling this. On at least some 4xx chips, however, the shared resources are scattered around various places, but all use the same device numbering. Therefore it seemed expedient to encode that numbering in a single place - 'cell-index' - rather than having several such tables. Yeah, it seems to work out for those 4xx. Other platforms would be well advised to consider the tradeoff for themselves though, many SoCs have an even more wild design than 4xx does ;-) Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCH] powerpc: Get rid of CROSS32{AS,LD,OBJCOPY}
CROSS32AS and CROSS32LD are never used (instead, CROSS32CC is used with proper command line options). CROSS32OBJCOPY isn't used anymore either, since the wrapper stuff was added. Remove these unused variables. Signed-off-by: Segher Boessenkool [EMAIL PROTECTED] Cc: Paul Mackerras [EMAIL PROTECTED] --- arch/powerpc/Makefile |8 +--- 1 files changed, 1 insertions(+), 7 deletions(-) diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile index df59bbe..c7c1de9 100644 --- a/arch/powerpc/Makefile +++ b/arch/powerpc/Makefile @@ -18,22 +18,16 @@ HAS_BIARCH := $(call cc-option-yn, -m32) CROSS32_COMPILE ?= CROSS32CC := $(CROSS32_COMPILE)gcc -CROSS32AS := $(CROSS32_COMPILE)as -CROSS32LD := $(CROSS32_COMPILE)ld CROSS32AR := $(CROSS32_COMPILE)ar -CROSS32OBJCOPY := $(CROSS32_COMPILE)objcopy ifeq ($(HAS_BIARCH),y) ifeq ($(CROSS32_COMPILE),) CROSS32CC := $(CC) -m32 -CROSS32AS := $(AS) -a32 -CROSS32LD := $(LD) -m elf32ppc -CROSS32OBJCOPY := $(OBJCOPY) CROSS32AR := GNUTARGET=elf32-powerpc $(AR) endif endif -export CROSS32CC CROSS32AS CROSS32LD CROSS32AR CROSS32OBJCOPY +export CROSS32CC CROSS32AR ifeq ($(CROSS_COMPILE),) KBUILD_DEFCONFIG := $(shell uname -m)_defconfig -- 1.5.5.1.g377d9.dirty ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH v2] [NAND] driver extension to support NAND on TQM85xx modules
I see. I think I should then also post the bindings (update of booting-without-of.txt) separately. Yes please. They are much easier to review that way (and they should be reviewed as a separate entity anyway). Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] booting-without-of: add more bindings for FSL UPM driver
+ - chip-delay : may specify a delay value in milliseconds. Delay for what? The binding should say. chip-delay is a bit too generic name as well, it could be more descriptive perhaps. Shouldn't this be a property of the NAND device anyway, not the NAND controller? Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] booting-without-of: add more bindings for FSL UPM driver
+ - chip-delay : may specify a delay value in milliseconds. Delay for what? The binding should say. chip-delay is a bit too generic name as well, it could be more descriptive perhaps. The chip-delay property defines an appropriate maximum delay time (tR) required for read operations if the R/B pin is not connected. Yeah. So please put that in the binding. Shouldn't this be a property of the NAND device anyway, not the NAND controller? Strictly speaking, it's a property of the NAND device. Therefore it should be inside the node [EMAIL PROTECTED], I thhink: + [EMAIL PROTECTED] { + #address-cells = 1; + #size-cells = 1; + chip-delay = 25; // in micro-seconds Something like that, yes. You wrote milliseconds before; which is it? And, a better property name, please. Where should that be documented? In the binding for nand devices. If there isn't any yet, it might be best to include that with the binding for your nand controller (i.e., describe the whole sub node there). Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: starting with 2.6.26-rc1 cell_defconfig fails on QS22
Finalizing device tree... using OF tree (promptr=0e1004c4) ( 700 ) Program Exception [ e1004c4 ] The program check exception happens at 0xe1004c4 ? That looks like the OF entry point (promptr)... could it be possible that it got corrupted somewhat ? The only thing I see above there would be the stack but I fail to see how it would use that much... I don't know what this [ e1004c4 ] is, if I read the current public SLOF code (for JS2x) correctly it seems to be whatever was on the stack (or just below the stack) below the error code that was thrown. Maybe some errors put something interesting there, dunno. /me looks deeper... Nastiness :-) So, a decrementer exception gives the current decrementer value as reason code; an external exception reads some BE-specific registers (on any system!); and all other exceptions use whatever was in GPR4? Anyway, the register dump shows: CR / XER LR / CTR SRR0 / SRR1DAR / DSISR 8022 014073e8 0189e99c 2000 0140 90083000 so SRR0 is 0189e99c, which is where the exception happened. Does objdump show what's going on? It seems to happen almost immediately after the kernel starts, given the CTR value. If the SRR0 address doesn't help, the LR address should. I have tried it with gcc-3.4.2, gcc-4.1.1 and gcc-4.2.4. The binutils version is more interesting here. 2.18? Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 2/9] powerpc: Add macros to access floating point registers in thread_struct.
return user_regset_copyout(pos, count, kbuf, ubuf, - target-thread.fpr, 0, -1); + target-thread.fpr, 0, -1); is there a reason we can drop the ''? Yes, .fpr is an array. C is _such_ a fun language, heh. Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: starting with 2.6.26-rc1 cell_defconfig fails on QS22
D'oh. I spent so much time and the solution is so easy. Thanks everybody. I have updated to binutils-2.18.50.0.6-2.ppc and now it works again on QS22 and JS21. So I checked your binary, and the only differences between working and not-working are a) some section offsets in the file, and b) the load address of the segment (0 vs. 0xc000...). a) should be harmless, and b) likely is a binutils bug that got fixed. What is the exact binutils version you used for the non-working? Should we test for it in our Makefile? If we require 2.18, we can drop the tests for 2.12 and 2.14 ;-) Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] powerpc: add of_find_next_property and of_get_aliased_index
It returns 2 for both i2c-1 and i2c-2. Well, I'm assuming that the alias property names will follow the current convention of nn where is a name and nn is a number. No dashes or other punctuation. Well, yes, your suggested code doesn't allow punctuation either; but that wasn't my point, it doesn't allow numbers in names. Why don't you just parse a number from the end? Also, alias names do not have any significance in general, they are just handy shortcut names for humans to use; it would be better not to overload this. What do you want to use this for? As an alternative to cell-index or device-id for enumerating devices. The consensus from the 'cell-index vs. index vs. no index in I2C device nodes' thread is that aliases are to be used to enumerate devices. Erm, no. That wasn't the consensus as I remember it; besides, it's not a good plan. Pretty much all busses can be enumerated without anything like this. There was consensus on this. Overloading cell-index is a bad plan. There was consensus on this, as well. The only thing a platform should ever use aliases for is if it needs to (for whatever purpose) find a specific device, that it cannot identify otherwise (via reg, ...). And then that platform code should look up the device by the alias, not look up the alias by the device -- there is no 1-1 mapping from device to alias! Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 19/60] microblaze_v4: checksum support
+static inline unsigned int +csum_tcpudp_nofold(unsigned long saddr, unsigned long daddr, unsigned short len, + unsigned short proto, unsigned int sum) +{ + __asm__(add %0, %4, %1\n\t + addc %0, %4, %2\n\t + addc %0, %4, %3\n\t + addc %0, %4, r0\n\t + : =d (sum) + : d (saddr), d (daddr), d (len + proto), + 0(sum)); sum should use an earlyclobber, i.e. =d(sum) , since some inputs are used after %0 is first written to. Also, you can use + instead of = to say the argument is both input and output, and get rid of %4, if you like. Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH v2] update crypto node definition and device tree instances
I'm really don't like fsl,sec1.0 or any of the variants as a compatible property either because it can easily be abused (it's not anchored to a specific physical part so the meaning can shift over time); but that is another argument and it is well documented in other email threads (http://thread.gmane.org/gmane.linux.ports.ppc64.devel/38977/ focus=39147) Also, these made-up names make you do more work: you'll need to write up a binding for them, explaining exactly what a 1.0 device etc. is (or at least point to documentation for it). If you use a name that refers to some device that people can easily google for documentation, you can skip this (well, you might need to write a binding anyway; but at least you won't have to explain what the device _is_). Using actual model names also reduces the namespace pollution (hopefully Freescale will not create some other MPC8272 device ever, so fsl,mpc8272-whatever will never be a nice name to use for any other device; OTOH, it's likely that Freescale will create some other device called SEC (there are only so many TLAs, after all), so fsl,sec-n.m isn't as future-proof. Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [POWERPC] mpc7448hpc2.dts: remove chosen node from dts
Modern versions of u-boot create a chosen node automatically. So if we set the chosen node in the dts file, there will be 2 chosen nodes passed in to the kernel, That's a bug in uboot, then: when it creates a property, it should delete an already existing property with the same name in that node. and the kernel command line will be taken from the wrong node. So, remove the extra chosen node from the dts file. This will cause a regression for people using an older version of uboot. No idea how important that is. Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 19/60] microblaze_v4: checksum support
you find bug because there is microblaze asm code in generic folder. No, I found it because I was reading the code, I didn't even notice this was in asm-generic :-) Big thanks. I'll fix it and I'll look at parameters too. Thanks. Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: MPC83xx ipic problem
interrupt-map = 0x5800 0 0 1 ipic 0x30 0x8 - FPGA @ IRQ0 0x6000 0 0 1 ipic 0x11 0x8 - miniPCI INTA @ IRQ1 0x6000 0 0 2 ipic 0x11 0x8;- miniPCI INTB @ IRQ1 Is it legal to use a single irq pin twice ? The device tree simply describes the hardware; if the hardware connects both INTXs to the same IPIC interrupt pin, then it is correct. You'll have to ask a hardware designer whether it is okay to just tie the two lines together; I believe it is, for PCI, but you better ask someone who really knows :-) Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH v2] update crypto node definition and device tree instances
Also, these made-up names make you do more work: you'll need to who said they were made up? I did. These names do not refer to some physical part you can buy. write up a binding for them, explaining exactly what a 1.0 device etc. is (or at least point to documentation for it). If you use a name that refers to some device that people can easily google for documentation, you can skip this (well, you might need to write a binding anyway; but at least you won't have to explain what the device _is_). documentation is available in the usual places, and it specifically points out which SEC version it references. I can't find a manual online for freescale sec; googling for freescale sec-1.0 finds a manual for the PowerQUICC I; is that the right one? I don't know, so the binding needs to explain it to me. Going from SoC name - SEC version is easy, but the other way around not so. Anyway, minor stuff. Plus, as I mentioned before, a lot of the differences between the SEC versions are miniscule feature bits scattered across the programming model. I don't see how this is relevant, sorry. Using actual model names also reduces the namespace pollution (hopefully Freescale will not create some other MPC8272 device ever, so fsl,mpc8272-whatever will never be a nice name to use for any other device; OTOH, it's likely that Freescale will create some other device called SEC (there are only so many TLAs, after all), so fsl,sec-n.m isn't as future-proof. I doubt that; the SEC has been around for about a decade now and that hasn't happened. You'll have to admit a three-letter acronym is a bigger namespace squatter than a nice long name is. But it's your namespace, I don't care. i tried googling for freescale sec to find any other devices called SEC, but that didn't work out. What is insider trading? ;-) Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH v2] powerpc: Add dma nodes to 83xx, 85xx and 86xx boards
What's the cell-index in these nodes used to index? Given the confusion there's been about the proper use of this property, a comment indicating which shared registers this is used to index is probably a good idea. There's supposed to be a cell-index in the *channels* to index into the shared summary register (the reg of the dma node itself). I don't see any purpose for a cell-index in the main dma node, though. I believe this comes into play when we have more than one DMA controller and sometimes there are special uses like on 8610. There is no need to guess what it is or isn't used for. Just look it up in the binding for this device. Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH v2] update crypto node definition and device tree instances
Also, these made-up names make you do more work: you'll need to who said they were made up? I did. These names do not refer to some physical part you can buy. right, they refer to devices in multiple physical parts you can buy. Part-you-can-buy documentation clearly indicates the SEC version in that part, in the form SEC X.Y, i.e, it's not something made up that's not already in freescale documentation. Yes. As a side note, since there are multiple devices that contain e.g. a sec-1.0, it would be prudent to describe the exact incarnation in the device tree, like mpc8272-sec or something, in either model or compatible, just in case a problem shows up with one of them. write up a binding for them, explaining exactly what a 1.0 device etc. is (or at least point to documentation for it). If you use a name that refers to some device that people can easily google for documentation, you can skip this (well, you might need to write a binding anyway; but at least you won't have to explain what the device _is_). documentation is available in the usual places, and it specifically points out which SEC version it references. I can't find a manual online for freescale sec; googling for freescale sec-1.0 finds a manual for the PowerQUICC I; is that the right one? I don't know, so the binding needs to explain it to me. the binding shouldn't be responsible for google's shortcomings The binding needs to describe what device it is for. I am a stupid user, just like most users, so if the binding doesn't tell me I turn to google. Don't blame them for not finding it; the binding should have told me in the first place! (that hit is correct, btw). Okay, cool. Going from SoC name - SEC version is easy, but the other way around not so. Anyway, minor stuff. sounds like you're pointing out a lack of SEC versions guide documentation of Freescale.. Yes, that would have helped. Plus, as I mentioned before, a lot of the differences between the SEC versions are miniscule feature bits scattered across the programming model. I don't see how this is relevant, sorry. I'm under the impression that listing the differences (assuming they're easily obtainable) would lead to unnecessary b-w-of bloat. The binding at a minimum should describe how to identify each unique version from the device tree, no matter how miniscule those differences are. Just a specific compatible value will do. I don't know what google does; I'd search freescale documentation directly. Or the binding could just bloody say what it is talking about in the first place, heh. Anyway, how about we do something constructive? If you still want to use fsl,sec-N.M names, that's fine with me. Each specific device tree needs to still say which exact device it contains, so an entry would look like e.g. compatible = fsl,mpc8272-sec, fsl,sec-3.0; and the driver can just probe for fsl,sec-3.0 if it doesn't need to know about the exact version; but it _can_ use it if it _does_ need to know. Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH v2] update crypto node definition and device tree instances
Yes. As a side note, since there are multiple devices that contain e.g. a sec-1.0, it would be prudent to describe the exact incarnation in the device tree, like mpc8272-sec or something, in either model but 'fsl,sec-X.Y' /does/ describe the exact incarnation, No it doesn't. If it's on a different SoC, it can have different bugs. It might be _meant_ to be exactly the same, but that's not the same thing. whereas 'fsl,mpc8349-sec' /does not/. fsl,mpc8349-sec' might mean the SEC 2.1 or the SEC 2.4, it depends on the revision of the mpc8349. Oh, nasty. That just means you'll need to put the revision of the 8349 in there as well, though -- fsl,mpc8349-rev2-sec or something. or compatible, just in case a problem shows up with one of them. I thought 'model' was superseded by 'compatible'; model is still a valid property. model shouldn't be needed to find which device driver to use, but any specific device driver can use it just fine. that's why it's taken out here, along with device_type. device_type simply isn't useful for flat device trees (in pretty much all cases), since it describes the OF programming model of a device, and almost none of that applies to flat trees. So yeah, taking that out is a good thing (esp. in a case like this, where it isn't defined anywhere what device_type crypto means). I can't find a manual online for freescale sec; googling for freescale sec-1.0 finds a manual for the PowerQUICC I; is that the right one? I don't know, so the binding needs to explain it to me. the binding shouldn't be responsible for google's shortcomings The binding needs to describe what device it is for. I am a stupid user, just like most users, so if the binding doesn't tell me I turn to google. Don't blame them for not finding it; the binding should have told me in the first place! Again, I don't see how google's results are pertinent in this discussion. It's not about google. It's about a user who needs to find out what a certain compatible entry means, or who needs to find out what value to use for a certain device. btw, the title for the binding is: g) Freescale SOC SEC Security Engines Is that what you are looking for? I'm not looking for the binding, I know where it is, thanks; I'm looking for information in the binding that tells me what compatible value means what. If not, what precisely? a list of all the parts? There's an SEC in every mpc8[35]xxE! You could do a list of all, sure. You could also say what a compatible value looks like, and give some representative examples. The binding at a minimum should describe how to identify each unique version from the device tree, no matter how miniscule those differences are. Just a specific compatible value will do. I'm at a loss; isn't that what this patch does? I lost the patch, sorry. I came into this thread at the point where Grant said that fsl,sec1.0 is a horrible compatible value. Currently the driver matches on fsl,sec2.0, and if needs be, will call of_device_is_compatible with the version number that introduces the feature it wants to implement. That's okay I suppose. Each device tree still should put the exact version of the chip in there as well. Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH v2 1/2] powerpc: Fix building of feature-fixup tests on ppc32
We need to use PPC_LCMPI otherwise we get compile errors like: arch/powerpc/lib/feature-fixups-test.S: Assembler messages: arch/powerpc/lib/feature-fixups-test.S:142: Error: Unrecognized opcode: `cmpdi' arch/powerpc/lib/feature-fixups-test.S:149: Error: Unrecognized opcode: `cmpdi' arch/powerpc/lib/feature-fixups-test.S:164: Error: Unrecognized opcode: `cmpdi' Sorry, I definitely built (and booted) a 32-bit config, so I'm not sure how I missed that. What toolchain are you using? It's more interesting to hear what version of GCC and GAS _you_ are using :-) Does it allow 64-bit-only opcodes with -a32? Does the build system somehow not pass -a32 for you? Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: patches for 2.6.27...
Please point out any patches that have been posted but havent made it into a git tree related to Freescale chips. I haven't heard back from Segher, so: http://patchwork.ozlabs.org/linuxppc/patch?id=19313 You haven't heard back because I feel like I'm talking to a wall. The compatible definition for this node needs to be fixed. Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH v3] powerpc: update crypto node definition and device tree instances
+- compatible : Should contain entries for this and backward compatible + SEC versions, high to low, e.g., fsl,sec2.1, fsl,sec2.0 First entry should state the _exact_ version of the device. sec-N.M isn't good enough; there can be implementation bugs. There can be more entries; you make it sound like each device tree should list _all_ compatible devices, which isn't the case. A device binding should not (and can not) say how compatible should be used; instead, it should list what values of compatible this binding applies to. You can give recommendations of course. +- interrupts : a b where a is the interrupt number and b is a + field that represents an encoding of the sense and level + information for the interrupt. This should be encoded based on + the information in section 2) depending on the type of interrupt + controller you have. #interrupt-cells isn't always 2. Your device binding shouldn't describe how interrupt encoding works (that's described in the imap recommended practice, already); instead, it should describe which device interrupts are listed here, and in what order. Sounds like you only have one interrupt, so that should be easy ;-) +- interrupt-parent : the phandle for the interrupt controller that + services interrupts for this device. interrupt-parent isn't a required property. It isn't part of this device binding, either. Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: New fsl device bindings file
Just curious... why we're maintaining documentation in the .txt file? Because it is human-readable text? Or is this too wild? :-) Yes :-) Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: New fsl device bindings file
/* deprecated; */ device_type = i2c; How about deprecated but kept for compatibility with true Open Firmware implementations? Well, except a flat tree isn't compatible with OF at all here. A device_type promises a certain interface; a flat tree doesn't even have the open method. From the OF base spec: “device_type” S Standard property name to specify the implemented interface. prop-encoded-array: Text string encoded with encode-string. Specifies the “device type” of this package, thus implying a specific set of package class methods implemented by this package. Seriously, you can't have a binding for OF and then cut out that part of the standard at a whim. Nothing is cut out. There never was a device binding for device_type i2c; creating one would be a considerable effort, and since flat tree users wouldn't use it anyway, you can't be seriously suggesting they should do this. It should be there (at least for those parts which are governed by a client interface API, like display, serial etc. Huh? Nothing in the client interface mentions display or serial as far as I know. but cutting it off takes away all it's meaning, So what? There _is_ no real device interface, when a flat tree is used. plus Linux implementations STILL keep searching that property along with compatible, That's a bug. Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: New fsl device bindings file
I'm sure you'll hate for doing this, No, it's an excellent move :-) but I've asked Kim to create a new Documentation/powerpc/fsl-device-tree-bindings.txt as part of his SEC patch. As a separate patch, that (at first) _only_ moves the content into separate files, please. Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH v2 3/5] of-bindings: Add binding documentation for SPI busses and devices
+The SPI master node requires the following properties: +- #address-cells - number of cells required to define a chip select + address on the SPI bus. Hrm. Should this (and reg in the child node) be required for SPI masters that have only one chip select? +- max-speed - (required) Maximum SPI clocking speed of device in Hz The property name should include something SPI, it's way too generic otherwise. +- spi,cpol- (optional) Device requires inverse clock polarity +- spi,cpha- (optional) Device requires shifted clock phase Don't abbr the property names, there's nothing wrong with longer names. The names shouldn't start with spi, either, spi isn't a vendor; how about spi-inverse-clock-polarity or similar? +- linux,modalias - (optional, Linux specific) Force binding of SPI device + to a particular spi_device driver. Useful for changing + driver binding between spidev and a kernel SPI driver. This is a temporary workaround I hope? Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: Mikrotik RouterBoard 333
Its firmware apparently provides a flattened device tree to the OS. And while this step towards world domination is flattering, it's an example of what I feared when people first got enthusiastic about the idea of including flattened device trees in firmwares. The tree has not, AFAIK, been past this list, and has apparently not been reviewed by someone knowledgeable about device trees. In short, it's crap, and now that it's embedded in the firware we can't really fix it. Can't you build a kernel with a blob that overrides the firmware-provided blob? Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: Comiler error compiling 2.6.26
Trying to cross-compile arch/powerpc for an freescale 8280: Gcc 4.1.2 Binutils 2.17 BFD: ./vmlinux.strip.7812: section .text lma 0xc000 overlaps previous sections [etc] Could you try with binutils 2.18? If not, or if that doesn't help, we'll need to see your .config . Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [HOW] binutils-2.17 breaks the 2.6.26 kernel
. = ALIGN(0x1000) /* this align directive aparently gets lost when stripping the file */ .rodata: AT (.rodata - LOAD_OFFSET): { ... } the effects of that align were dropped during strip, shifting all following sections up in memory and the resulting failure. The ELF headers show the .rodata section to have 256 byte alignment only, but after the bad strip it isn't even aligned to that anymore (file offset). Curious. I still can't reproduce this, but getting the bad binary now (it's huge, heh). We'll see. Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] Add AMCC Arches 460GT eval board support to platforms/44x
Shouldn't it be enough to have a common compatible value in each of these boards, e.g. amcc,generic-ppc44x and then just ignore the specific type unless you need to do something special? This is bad for the same reason that amcc,44x-blah compatible values are bad in device nodes. The definition of '*-44x-*' changes over time as new parts are added. Compatible values should always reflect an exact part number. Erm, no. compatible entries should always contain the real part name to reduce the chance that separate parties come up with the same compatible name for two different devices. Other than that, the name really doesn't matter, it's just _some_ string that uniquely identifies a device; and the device bindings will document what is what. Of course, if no binding is written, it becomes a lot more important that names have a sane value ;-) Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] Add AMCC Arches 460GT eval board support to platforms/44x
And then you don't need this file at all. Just add a amcc,canyonlands string to your root node compatible property. No! Don't do this because it is not true! If the board actually _is_ compatible to the canyonlands board (it only _adds_ stuff, doesn't change things or takes away things), it is perfectly valid to declare it compatible, since it _is_. In my opinion, for the root compatible value, it is also okay to declare a board compatible if it is only largely compatible, just has some little differences -- esp. if you're declaring a board to be compatible to some reference design. This does of course have downsides as well. Meh. You're splitting hairs. It _is_ true from a kernel perspective. That doesn't matter. The device tree describes the hardware, it doesn't matter what the Linux kernel does. For one thing, the kernel isn't necessarily the only OS (or other client) to run with this DTS; also, the kernel is a moving target, while the DTS can be burned into ROM. Instead, add your board name to canyonlands.c in canyonlands_probe(). It's not the most scalable solution, but it keeps you from lying about your hardware in the .dts file. That could also be done. Frankly though, if you look at the existing board.c files in there now, none of them are special. The device tree really provides the differences these days, not the C code. Yes. It would be more scalable if the platform probe code checks the device tree for the features it expects (certain CPU, certain interrupt controller, certain chipsets / bridges), instead of it checking the root compatible property. I'm this || close to just killing them all and doing a 4xx_board.c file that does the right thing based on the few boards that have differences. Oh, the kernel code even considers these things to be different platforms? Insane :-) Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] Add AMCC Arches 460GT eval board support to platforms/44x
Shouldn't it be enough to have a common compatible value in each of these boards, e.g. amcc,generic-ppc44x and then just ignore the specific type unless you need to do something special? This is bad for the same reason that amcc,44x-blah compatible values are bad in device nodes. The definition of '*-44x-*' changes over time as new parts are added. Compatible values should always reflect an exact part number. I agree in general, but I also think that all 44x boards should be regarded as the same machine time, in the same way that all powermacs are detected as compatible with Power Macintosh or MacRISC, or how all sorts of serial ports claim compatibility with i8250. And they can do that because a binding for those compatible names exists, that describes exactly what it means for a device to be i8250 or MacRISC (well, in that last case, maybe such a binding exists, or maybe it doesn't, but we don't know about it anyway, sigh). Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] leds: implement OpenFirmare GPIO LED driver
diff --git a/Documentation/powerpc/dts-bindings/gpio/led.txt b/Documentation/powerpc/dts-bindings/gpio/led.txt new file mode 100644 index 000..7e9ce81 --- /dev/null +++ b/Documentation/powerpc/dts-bindings/gpio/led.txt @@ -0,0 +1,15 @@ +LED connected to GPIO + +Required properties: +- compatible : should be gpio-led. This compatible name is a bit too generic. No, I don't know a better name :-( +- label : (optional) the label for this LED. If omitted, the label is + taken from the node name (excluding the unit address). What is a label? It should be described here. Also, its encoding should be described (a string I guess). +- gpios : should specify LED GPIO. + +Example: + [EMAIL PROTECTED] { + compatible = gpio-led; + label = hdd; + gpios = mcu_pio 0 0; +}; You show a unit address but have no reg value. This is incorrect. What would be the parent node of this, btw? Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [HOW] binutils-2.17 breaks the 2.6.26 kernel
Previous threads have mentioned that binutil-2.17 is broken for building powerpc kernels. It is fixed in binutils-2.18. Yes. I have encountered this and upgrading to 2.18 fixed my build. The symptom is large kernel sizes and a long time in gzip. In my case it was gziping a 2GB file. Are you using a binary (non-ELF) image file? This sounds like a different problem, caused by the kernel linker script not handling the build-id section correctly -- it places it at 0, and the rest of the kernel at 3GB, you can imagine the rest. I've seen this on various embedded targets, but not on PowerPC iirc -- maybe I don't build any affected defconfig, what's yours? I have a working (tested! thanks Milton) workaround for the current problem, will send it later today. This problem funnily is hidden by the presence of build-id :-) Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: powerpc: Fix OF parsing of 64 bits PCI addresses
The OF parsing code for PCI addresses isn't always treating properly the address space indication 0b11 (ie. 0x3) as meaning 64 bits memory space. This means that it fails to parse addresses for PCI BARs that have this encoding set by the firmware, which happens on some SLOF versions and breaks offb palette handling on Powerstation. Signed-off-by: Benjamin Herrenschmidt [EMAIL PROTECTED] Looks good. Acked-by: Segher Boessenkool [EMAIL PROTECTED] Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [RFC v3 PATCH 4/4] Relocation support
This patch changes all LOAD_REG_ADDR macro calls to LOAD_REG_IMMEDIATE to make sure that we load the correct address. Did you figure out _why_ LOAD_REG_ADDR doesn't work? Using LOAD_REG_IMMEDIATE is actually a step back, it makes the kernel binary non-PIC. And LOAD_REG_ADDR _should_ work just fine with your scheme. Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: Mikrotik RouterBoard 333
Other than that quibble, I agree that burning the blob into the firmware so that the firmware must be recompiled and reburned to change the blob is very undesirable. I thought the device tree was *supposed* to be an interface between the firmware and the kernel? What if the firmware produces the tree dynamically? What if the firmware itself depends on having the device tree in order to operate? Sure. But in the case where the firmware just passes the kernel some fixed blob, it is easier for everyone to include that blob with the kernel instead. Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCH] powerpc: Fix build bug with binutils 2.18 and GCC 4.2
binutils 2.18 has a bug that makes it misbehave when taking an ELF file with all segments at load address 0 as input. This happens when running strip on vmlinux, because of the AT() magic in this linker script. People using GCC = 4.2 won't run into this problem, because the build-id support will put some data into the notes segment (at a non-zero load address). To work around this, we force some data into both the dummy segment and the kernel segment, so the dummy segment will get a non-zero load address. It's not enough to always create the notes segment, since if nothing gets assigned to it, its load address will be zero. Signed-off-by: Segher Boessenkool [EMAIL PROTECTED] Tested-By: Milton Miller [EMAIL PROTECTED] --- arch/powerpc/kernel/vmlinux.lds.S | 31 --- 1 files changed, 28 insertions(+), 3 deletions(-) diff --git a/arch/powerpc/kernel/vmlinux.lds.S b/arch/powerpc/kernel/vmlinux.lds.S index 87a72c6..a914411 100644 --- a/arch/powerpc/kernel/vmlinux.lds.S +++ b/arch/powerpc/kernel/vmlinux.lds.S @@ -9,6 +9,25 @@ ENTRY(_stext) +PHDRS { + kernel PT_LOAD FLAGS(7); /* RWX */ + notes PT_NOTE FLAGS(0); + dummy PT_NOTE FLAGS(0); + + /* binutils 2.18 has a bug that makes it misbehave when taking an + ELF file with all segments at load address 0 as input. This + happens when running strip on vmlinux, because of the AT() magic + in this linker script. People using GCC = 4.2 won't run into + this problem, because the build-id support will put some data + into the notes segment (at a non-zero load address). + + To work around this, we force some data into both the dummy + segment and the kernel segment, so the dummy segment will get a + non-zero load address. It's not enough to always create the + notes segment, since if nothing gets assigned to it, its load + address will be zero. */ +} + #ifdef CONFIG_PPC64 OUTPUT_ARCH(powerpc:common64) jiffies = jiffies_64; @@ -50,7 +69,7 @@ SECTIONS . = ALIGN(PAGE_SIZE); _etext = .; PROVIDE32 (etext = .); - } + } :kernel /* Read-only data */ RODATA @@ -62,7 +81,13 @@ SECTIONS __stop___ex_table = .; } - NOTES + NOTES :kernel :notes + + /* The dummy segment contents for the bug workaround mentioned above + near PHDRS. */ + .dummy : { + LONG(0xf177) + } :kernel :dummy /* * Init sections discarded at runtime @@ -74,7 +99,7 @@ SECTIONS _sinittext = .; INIT_TEXT _einittext = .; - } + } :kernel /* .exit.text is discarded at runtime, not link time, * to deal with references from __bug_table -- 1.5.6.4.gf1ba5 ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [RFC v3 PATCH 6/4] Use LOAD_REG_IMMEDIATE macros
All of the variables references through @got translated into relocation type R_PPC64_GOT16_DS entries. All these entries correspond to one of the above entries in the .got section. But none of the entries in .got section are relocated. If that last statement is really true, then that would be an absolute show-stopper, since you're not going to stop the compiler generating loads from the TOC to get addresses of things. However, I don't think it's true. I compiled up a kernel using --emit-relocs on the final link, and with readelf -e I can see a .rela.got section containing a bunch of R_PPC64_ADDR64 relocs for entries in the .got section. So the problem appears to be either just that you are ignoring R_PPC64_ADDR64 relocs, or else that your relocs.c program has a bug and isn't seeing the .rela.got section. I analysed this further. .rela.got does have lots of relocs in it, but _not_ for relocations create with @got in the assembler code. GCC never does this AFAICS, it explicitly creates a TOC entry and uses that. So, the workaround would be to manually create TOC entries in the LOAD_REG_ADDR code as well. I'll work on that, feel free to beat me to it of course. Now I have two options left: 1. Check for R_PPC64_GOT16_DS entries and check whether the contents addressed by r2+offset is relocated or not and apply relocation if its not. 2. Change all LOAD_REG_ADDR macros to LOAD_REG_IMMEDIATE. This I have already done. I was trying to point out that this can't possibly be a viable solution to the problem, because most of the TOC loads in the binary are generated by the C compiler, and only a few of them come from use of the LOAD_REG_ADDR macro in assembly code. binutils has a problem only with the @gotXXX relocations, where the _linker_ creates the GOT entry (it doesn't emit a reloc for -emit-relocs in that case). Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: 2.6.26 does not boot on Pegasos
If you built this kernel yourself, you need to do it from a system with an up-to-date binutils (2.18) otherwise, it does this. Note to the linux-ppc guys; is there any changelog entry which reports this requirement somewhere? I didn't find one... That's because there is no such requirement. 2.18 is the latest version, so it's always wise to test with that in case you have some problems; but it wouldn't be nice if the kernel can be built with only one binutils major version. Please try my build patch for a similar problem, it went into BenH's tree today. Would be nice if it was the same problem; if not, send a proper, _detailed_ bug report here and I'll look into it. Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] powerpc: Fix build bug with binutils 2.18 and GCC 4.2
[putting linuxppc-dev on Cc:, hope you don't mind] On 24 jul 2008, at 08:32, Chuck Meade wrote: I wanted to reply to your original message regarding this, but sadly I had already deleted it. So I am sending directly to you. This patch broke my link. Sorry. I didn't test with anything _that_ ancient. Reverting it, I am again able to link the latest git fetch of the kernel, but with your patch, my ld breaks. I am using binutils 2.15, successfully until this patch was applied yesterday. What target / what config / what exact GCC version / what exact binutils version / why are you using such an ancient toolchain anyway? :-) I have been using gcc 3.4.4 and binutils 2.15 up to this point without problems. Yes they are not cutting-edge by any means, Updating to GCC 3.4.6 might be a good idea, it's the latest bug fix release in the 3.4 series. I agree PowerPC Linux should still be buildable with GCC 3.4; I don't think we really care about 3.3 or older anymore though. but the concern here is that this patch causes the linker to fail for the first time. Up until now the link has worked fine, and if I revert this patch, the link continues to work well. Yeah, I understand. I'm not saying you need to upgrade your toolchain (or, I'm not yet saying that anyway; will have to see what causes this problem first); I just said I neglected to test with anything that old. For one of my customers, we use a customized build system that can share cross toolsets for builds of multiple platforms. So the fact that these tools have worked for us cross several 83xx platforms for a long time is valuable. It would be highly desirable to have the linker continue to work. Sure, I'll try my best to find out what is wrong, and fix it for you if possible. I am very willing to work with you and test the alternative patch ideas you have for vmlinux.lds.S on my tools here. This patch was in the general interest of backwards-compatibility with pre-2.18 binutils anyway. Yes, exactly: 2.6.26 does not build with binutils 2.17. I can help you by testing on 2.15. Send me alternate vmlinux.lds.S files to try, and I will test and get back to you ASAP. No, I will not send you randomly changed source files and hope they will do something useful for you. Instead, why not provide me the information I asked for? What target (arch/powerpc it sounds like?) What _exact_ binutils version (FSF 2.15?) What _exact_ GCC version (FSF 3.4.4?) What Linux config (either the full .config, or the name of a defconfig)? The link error, in case you were wondering, was: Yes, I forgot to ask for that :-) powerpc-8325-linux-gnu-ld: final link failed: File truncated What was the command line here? Was it the linking of vmlinux? Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: section .dummy lma 0xc0294310 overlaps previous sections
Anybody else seeing these? With Linus' latest I get three of these warnings: .tmp_vmlinux1, .tmp_vmlinux2, and vmlinux. Ah, that's useful information, I think I know what's going on now. What was your binutils version? Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: link failure: file truncated
Just tried to build the latest version from Linus' tree and I am getting a link error. I can reproduce this now, with 3.4.6 + 2.15, on powerpc64 defconfig (with the drivers that don't compile with 3.4.6 disabled). I have a fix for the warning with 2.17, but that's just a warning, so I'll wait sending it until we know what causes the error. Stay tuned :-) Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] powerpc: Fix build bug with binutils 2.18 and GCC 4.2
I can help you by testing on 2.15. Send me alternate vmlinux.lds.S files to try, and I will test and get back to you ASAP. No, I will not send you randomly changed source files and hope they will do something useful for you. Instead, why not provide me the information I asked for? What target (arch/powerpc it sounds like?) What _exact_ binutils version (FSF 2.15?) What _exact_ GCC version (FSF 3.4.4?) What Linux config (either the full .config, or the name of a defconfig)? No problem -- I thought it would be helpful to offer to test changes for you, so we could work together toward a solution. Sure, and thanks for that! I need to be able to reproduce it myself to create a patch in the first place though (or spend a few hundred hours in a senseless guessing game) :-) I now managed to reproduce it with 2.15 on powerpc64. You should see the patch soon (I sent it, but for seem reason it takes a while to get through ozlabs, half an hour or so the last time). Please test, and thank you! Reply to the patch mail saying if it worked please (Jon, Sean, you as well?). Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCH] powerpc: Fix compile error with binutils 2.15
My previous patch to fix compilation with binutils-2.17 causes a file truncated build error from ld with binutils 2.15 (and possibly older), and a warning with 2.16 and 2.17. This fixes it. Signed-off-by: Segher Boessenkool [EMAIL PROTECTED] --- arch/powerpc/kernel/vmlinux.lds.S |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/arch/powerpc/kernel/vmlinux.lds.S b/arch/powerpc/kernel/vmlinux.lds.S index a914411..4a8ce62 100644 --- a/arch/powerpc/kernel/vmlinux.lds.S +++ b/arch/powerpc/kernel/vmlinux.lds.S @@ -85,7 +85,7 @@ SECTIONS /* The dummy segment contents for the bug workaround mentioned above near PHDRS. */ - .dummy : { + .dummy : AT(ADDR(.dummy) - LOAD_OFFSET) { LONG(0xf177) } :kernel :dummy -- 1.5.6.4.gf1ba5 ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] of: i2c: improve last resort compatible entry selection
compatible = atmel,24c32wp, 24c32, eeprom; I know this is just an example; but to keep thinks clear, the second and third values in this compatible property are completely bogus (for device trees). The manufacturer prefix needs to be present and 'eeprom' is far to vague. Isn't 24c32 a generic, cross manufacturer term used for these devices? Sure it is. But compatible values are a global namespace so care needs to be taken not to cause collisions. One mechanism for that is to use vendor prefixes (and that just shifts the problem so it is less global); another is to choose good names that have a lower chance to collide with the name for another device. And the most important way to prevent collisions is to write up a binding, so everyone knows you have claimed that name. It still needs to be a good name, of course. What if I have a socket and use a different vendor's chip each week? You use sockets for your seeproms? Wow :-) But yes, it shouldn't be necessary to put the exact make of the device in the device tree, for such generic devices. It certainly doesn't hurt to do so though (if the exact model is known). A reasonable compatible value would be something like serial-eeprom-24c32. You can go a little bit more generic than that, if you write up in your binding how the driver should figure out the device size and the protocol used. eeprom is the vague Linuxism that at24 is attempting to correct. eeprom just goes and searches for anything resembling an eeprom. It will trigger on chips that aren't eeproms. Yeah. And no driver should need to probe _anything_ if it has a device tree node describing the device -- certainly it shouldn't probe for what kind of device it is! Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] of: i2c: improve last resort compatible entry selection
A reasonable compatible value would be something like serial-eeprom-24c32. You can go a little bit more generic than that, if you write up in your binding how the driver should figure out the device size and the protocol used. Matching on serial-eeprom-24c32 requires me to convince the at24 authors to add that string as an alias binding for their driver. No, it requires the IIC subsystem to get fixed and not use OF compatible values as module alias names. How about serial-eeprom,24c32 or generic,24x32? Neither serial-eeprom nor generic is the name of a vendor, so no. The comma has a well-defined meaning. Why would a comma be easier than a dash for your device matching code, anyway? Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] powerpc: EOI spurious irqs during boot so they can be reenabled later
So when we receive a spurious irq, EOI it, and then mask it. What happens when a new IRQ arrives on the interrupt controller between these EOI and mask calls? Should you instead first mask it, and only then EOI it? Or doesn't that work on XICS? Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [RFC PATCH] Link the bootwrapper as a position-independent executable
Instead we now link the bootwrapper with -pie to get a position- independent executable, and process the relocations in the dynamic relocation section that the linker puts into the executable. Hurray! Looks good, just a few nits... + bl .+4 +p_base:mflrr10 /* r10 now points to runtime addr of p_base */ bl p_base instead? +10:or. r8,r0,r9/* skip relocation if we don't have both */ beq 3f Either the code or the comment is wrong -- the code says skip relocation if we don't have either. + cmpwi r0,22 /* R_PPC_RELATIVE */ + bne 3f It would be nice if there was some way to complain (at build time?) if there are unhandled relocations present. Prevents a lot of headaches when things go wrong (and they will ;-) ) 4: dcbfr0,r9 icbir0,r9 Fix these while you're at it? It's not r0, it's 0. + .dynsym : { *(.dynsym) } + .dynstr : { *(.dynstr) } + .dynamic : + { +__dynamic_start = .; +*(.dynamic) + } + .hash : { *(.hash) } + .interp : { *(.interp) } + .rela.dyn : { *(.rela*) } Do some of these sections need alignment? Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: inline assembly r0 SOS
unsigned int get_PLL_range(unsigned int range, unsigned int config) { range = range * 8 + 23; return ((config range) | (config (32 - range))) 3; } The special pattern ((a n) | (a (32 - n))) is recognized by gcc as a rotate operation. It's only valid for 1 = n = 31 though, so for input range 0 or 1. (*) If that's the whole range needed, much simpler code is possible... Segher (*) or 0x or 0xfffe, but somehow I doubt that was intended. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [RFC/PATCH 2/3] of: add of_lookup_stdout() utility function
It's not what we do with flattened device trees blobs though. In the flattened tree we're not using a /chosen/stdout property, just the linux,stdout-path one. The question that remains is; should there be? Should the dt blobs use /chosen/stdout also? (I'm not familiar enough with real OF to know the answer. I'm assuming that an instance value is not the same as a phandle). ihandles and phandles are not the same thing in OF. Since in the flat world we cannot have instances, we should use phandles instead of ihandles for the things in /chosen. I thought we agreed on that already, perhaps I am wrong? Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [RFC PATCH] Link the bootwrapper as a position-independent executable
+ bl .+4 +p_base:mflrr10 /* r10 now points to runtime addr of p_base */ bl p_base instead? I went back and forth on that. I ended up with it that way to emphasize that the bl does need to branch to the *next* instruction for the idiom to work. Right, I see. Make it even more obvious? Use bcl 20,31,$+4 instead (so people will have to look it up before changing this code, not because it is a few cycles faster ;-) ), add an empty line before the label, or even put an actual comment there? :-) +10:or. r8,r0,r9/* skip relocation if we don't have both */ beq 3f Either the code or the comment is wrong -- the code says skip relocation if we don't have either. Ah, operator precedence rules in English. :) While I don't think that double (and triple) negations in English are not overused and confusing... I (and I think most native English speakers) would parse my comment as don't (have both) whereas I think you parsed it as (don't have) both. Similarly what you say the code says parses as don't (have either) for me rather than (don't have) either. IOW, don't have either means both are missing. ... that is exactly what I meant: the code skips relocation only if both are missing. Anyway I can change the comment to say we need both to do relocation. OK? Please do -- but also change the code to match :-) + .dynsym : { *(.dynsym) } + .dynstr : { *(.dynstr) } + .dynamic : + { +__dynamic_start = .; +*(.dynamic) + } + .hash : { *(.hash) } + .interp : { *(.interp) } + .rela.dyn : { *(.rela*) } Do some of these sections need alignment? I suppose they should ideally be 4-byte aligned. Ideally, yes; I was questioning whether the ABI requires it. It will only cost a few bytes of object size so let's just do it? We don't actually use .dynsym, .dynstr, .hash or .interp, but I couldn't find any way to make the linker omit them. Assign those input sections to the /DISCARD/ output section (and do that early enough in the linker script so they aren't picked up by some other wildcard). Something like /DISCARD/ : { *(.dynstr .dynsym .hash .interp) } Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: Strange tg3 regression with UMP fw. link reporting
I don't know yet for sure what happens, but a quick look at the commit seems to show that the driver synchronously spin-waits for up to 2.5ms That's what the comment says, but the code says 2.5 _seconds_: + /* Wait for up to 2.5 milliseconds */ + for (i = 0; i 25; i++) { + if (!(tr32(GRC_RX_CPU_EVENT) GRC_RX_CPU_DRIVER_EVENT)) + break; + udelay(10); + } (not that milliseconds wouldn't be bad already...) Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH v3] libfdt: Add support for using aliases in fdt_path_offset()
Hmm.. my reading of 1275 says that an alias pointing to another alias is not permitted, but I'm not terribly confident I'm not misreading it. Segher, do you know whether this is allowed? My reading is the same: if after expanding an alias the path does not start with /, the search starts at the current package, otherwise it starts at the root node; and no further alias expansion is done. It's a corner case for sure, but at least the standard doesn't require us to implement recursive alias expansion; let's see if common practice does ;-) Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] powerpc: Fix error path in kernel_thread function
- cmpwi 0,r3,0 /* parent or child? */ - bne 1f /* return if parent */ + bns+1f /* did system call indicate error? */ + neg r3,r3 /* if so, make return code negative */ +1: cmpwi 0,r3,0 /* parent or child? */ + bne 2f /* return if parent */ li r0,0/* make top-level stack frame */ stwur0,-16(r1) mtlrr30 /* fn addr in lr */ @@ -857,7 +859,7 @@ _GLOBAL(kernel_thread) li r0,__NR_exit/* exit if function returns */ li r3,0 sc -1: lwz r30,8(r1) +2: lwz r30,8(r1) You don't need to renumber the labels, FWIW. Some people might find it more readable this way of course (but then, you could use actual label _names_ -- what a novel idea! ;-) ) Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 5/5] of/address: restrict 'no-ranges' kludge to powerpc
Certain Apple machines don't use the ranges property correctly, but the workaround should not be applied on other architectures. This patch disables the workaround for non-powerpc architectures. I'm half tempted to add it to the quirk list (which should really be made generic) so I can disable it on more 'modern' powerpc as well. Oh please oh please oh please yes do. OTOH, it would be even better to just fix up the device tree in the early platform code. Quirks are for broken hardware; software, we can fix. Segher ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] Adjust arch/powerpc inline asms for recent gcc change
- stw%U0%X0 %L2,%1 - : =m (*ptep), =m (*((unsigned char *)ptep+4)) + stw%U1%X1 %L2,%1 + : =m (*ptep), =m (*((unsigned char *)ptep+4)) : r (pte) : memory); This still isn't correct -- the constraint says that a byte is written, but the insn changes a word. Probably should just be ptep[1] ? Segher ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 11/26] KVM: PPC: Make RMO a define
On PowerPC it's very normal to not support all of the physical RAM in real mode. Oh? Are you referring to real mode limit, or 32-bit implementations with more than 32 address lines, or something else? Either way, RMO is a really bad name for this, since that name is already used for a similar but different concept. Also, it seems you construct the physical address by masking out bits from the effective address. Most implementations will trap or machine check if you address outside of physical address space, instead. Segher ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 24/26] KVM: PPC: PV mtmsrd L=0 and mtmsr
There is also a form of mtmsr where all bits need to be addressed. While the PPC64 Linux kernel behaves resonably well here, the PPC32 one never uses the L=1 form but does mtmsr even for simple things like only changing EE. You make it sound like the 32-bit kernel does something stupid, while there is no other choice. The L=1 thing only exists for 64-bit. Segher ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 11/26] KVM: PPC: Make RMO a define
On PowerPC it's very normal to not support all of the physical RAM in real mode. Oh? Are you referring to real mode limit, or 32-bit implementations with more than 32 address lines, or something else? The former. Okay. In that case, the hypervisor can usually access all of physical ram directly, unless it limits itself; and a supervisor gets either no real mode access, or all ram, or some part -- whatever the hypervisor likes. If you access outside of ram, you will most likely get a machine check. Depends on the implementation (like most things here). Either way, RMO is a really bad name for this, since that name is already used for a similar but different concept. It's the same concept, no? Not all physical memory is accessible from real mode. I think you're looking for real mode limit: The concept is called offset real mode. RMO (real mode offset) is the value that is ORed to an effective address to make the physical address (not added, offset is a pretty bad name). RML (real mode limit) is the value that has to be bigger than the effective address or you will get an exception. RMA (real mode area) is the name for the range of addresses usable in offset real mode. Also, it seems you construct the physical address by masking out bits from the effective address. Most implementations will trap or machine check if you address outside of physical address space, instead. Well the only case where I remember to have hit a real RMO case is on the PS3 - that issues a data/instruction storage interrupt when accessing anything 8MB in real mode. So I'd argue this is heavily implementation specific. It is. So what is the behaviour you want to implement? Apart from that what I'm trying to cover is that on ppc64 accessing 0xc0 in real mode gets you 0x0. Is there a better name for this? (You missed two zeroes). In hypervisor real mode, the top few bits are magic. They are used for e.g. enabling hypervisor offset real mode. In supervisor real mode, those bits are ignored (and all other bits that do not correspond to physical address lines may also be ignored). Maybe you want to call it physical_address_mask or similar? Segher ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 24/26] KVM: PPC: PV mtmsrd L=0 and mtmsr
There is also a form of mtmsr where all bits need to be addressed. While the PPC64 Linux kernel behaves resonably well here, the PPC32 one never uses the L=1 form but does mtmsr even for simple things like only changing EE. You make it sound like the 32-bit kernel does something stupid, while there is no other choice. The L=1 thing only exists for 64-bit. Oh, so that's why :). That doesn't really change the fact that it's very hard to distinguish between a mtmsr that only changes MSR_EE vs one that changes MSR_IR for example :). Hard to predict for the CPU as well, guess why there is a separate instruction now :-P By the way, L=1 _does_ exist for mtmsr; it's just that no 32-bit classic implementations support it. Segher ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 11/26] KVM: PPC: Make RMO a define
Also, it seems you construct the physical address by masking out bits from the effective address. Most implementations will trap or machine check if you address outside of physical address space, instead. Well the only case where I remember to have hit a real RMO case is on the PS3 - that issues a data/instruction storage interrupt when accessing anything 8MB in real mode. So I'd argue this is heavily implementation specific. It is. So what is the behaviour you want to implement? The one below. I'm sorry, I lost it. Below? Apart from that what I'm trying to cover is that on ppc64 accessing 0xc0 in real mode gets you 0x0. Is there a better name for this? (You missed two zeroes). In hypervisor real mode, the top few bits are magic. They are used for e.g. enabling hypervisor offset real mode. In supervisor real mode, those bits are ignored (and all other bits that do not correspond to physical address lines may also be ignored). So which bits exactly are reserved? I couldn't find a reference to that part. If by reserved you mean cannot be used for addressing, it's the top four bits. Book III-S chapter 5.7.3 in the Power Architecture 2.06 document. Implementations are allowed to ignore more bits than that. I believe in earlier versions of the architecture it was the top two bits, not four, but maybe I misremember. Maybe you want to call it physical_address_mask or similar? PAM - doesn't sound bad :). And miraculously nothing in the Power arch uses that acronym yet! But I would spell it out if I were you, acronyms are confusing. Segher ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: kernel init exception
why there generated a signal 4 in init process? That's SIGILL; sounds like you compiled init with the wrong (sub-)arch or cpu flags. Segher ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] arch/powerpc/lib/copy_32.S: Use alternate memcpy for MPC512x and MPC52xx
These processors will corrupt data if accessing the local bus with unaligned addresses. This version fixes the typical case of copying from Flash on the local bus by keeping the source address always aligned. On many platforms accessing ROM as RAM simply doesn't work(*). You shouldn't map ROM as if it is RAM, and shouldn't use the same access functions on it. Segher (*) Example: any existing 970 system will checkstop as soon as you try to do any cacheable access to some ROM. Another example of course is unaligned accesses on pretty much any system, no matter whether it's called a bug or a feature on that system :-P ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc: fix compile errors in prom_init_check for gcc 4.5
Just whitelist these extra compiler generated symbols. Fixes these errors: [...] I've used an identical patch for almost a year now, so... Signed-off-by: Stephen Rothwell s...@canb.auug.org.au Acked-by: Segher Boessenkool seg...@kernel.crashing.org You probably also need something similar to http://git.infradead.org/ users/segher/linux.git/commitdiff/ 98194f54cc8e19ecd752bc10e2d19ef94074f502 (note: only build-tested, not run-tested). Segher --- arch/powerpc/kernel/prom_init_check.sh |6 ++ 1 files changed, 6 insertions(+), 0 deletions(-) diff --git a/arch/powerpc/kernel/prom_init_check.sh b/arch/powerpc/ kernel/prom_init_check.sh index 1ac136b..9f82f49 100644 --- a/arch/powerpc/kernel/prom_init_check.sh +++ b/arch/powerpc/kernel/prom_init_check.sh @@ -52,12 +52,18 @@ do if [ ${UNDEF:0:9} = _restgpr_ ]; then OK=1 fi + if [ ${UNDEF:0:10} = _restgpr0_ ]; then + OK=1 + fi if [ ${UNDEF:0:11} = _rest32gpr_ ]; then OK=1 fi if [ ${UNDEF:0:9} = _savegpr_ ]; then OK=1 fi + if [ ${UNDEF:0:10} = _savegpr0_ ]; then + OK=1 + fi if [ ${UNDEF:0:11} = _save32gpr_ ]; then OK=1 fi -- 1.7.1 -- Cheers, Stephen Rothwells...@canb.auug.org.au http://www.canb.auug.org.au/~sfr/ ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] Adjust arch/powerpc inline asms for recent gcc change
- stw%U0%X0 %L2,%1 - : =m (*ptep), =m (*((unsigned char *)ptep+4)) + stw%U1%X1 %L2,%1 + : =m (*ptep), =m (*((unsigned char *)ptep+4)) : r (pte) : memory); This still isn't correct -- the constraint says that a byte is written, but the insn changes a word. Probably should just be ptep[1] ? Oops, almost forgot about this. Are you guys shooting a new patch or do you want me to do it ? It's really an independent fix. Just take Jakub's patch, I'll do one on top of it? Segher ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: kernel init exception
Maybe it was caused by floating exception.I found that,system received a program check exception,the reason for it was REASON_ILLEGAL. I also use show_regs to print the NIP in exception,it seemed that ,this exception was caused by 'vmhaddshs' instruction in user mode of init process . Is vmhaddshs avaliable on e500mc? My cross compile tool is gcc-4.1.2-glibc-2.5.0 This isn't vmhaddshs (which is an AltiVec instruction), but something else that also uses primary opcode 4. It sounds like your toolchain isn't set up correctly for e500mc. Segher ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc: fix compile errors in prom_init_check for gcc 4.5
You probably also need something similar to http://git.infradead.org/ users/segher/linux.git/commitdiff/ 98194f54cc8e19ecd752bc10e2d19ef94074f502 (note: only build-tested, not run-tested). See http://patchwork.ozlabs.org/patch/57379/ submitted yesterday. Yeah saw that right after I hit send :-) Either it's way more complete than what I did, or way more complex because we still don't link with libgcc :-P Segher ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc: fix compile errors in prom_init_check for gcc 4.5
See http://patchwork.ozlabs.org/patch/57379/ submitted yesterday. Yeah saw that right after I hit send :-) Either it's way more complete than what I did, or way more complex because we still don't link with libgcc :-P Actually, the 64bit versions of the routines are supplied by the linker, so the vmlinux links fine. But for modules, the final link is actually done by the kernel module loader, thus we need to provide those routines in this case. linking against libgcc would not have helped. Right, so more complete it is :-) I now checked that patch in detail, and it looks good to me. Thank you for tackling this! Segher ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 00/27] KVM PPC PV framework
[without] debian-powerpc:~# time for i in {1..1000}; do /bin/echo hello / dev/null; done real0m14.659s user0m8.967s sys 0m5.688s [with] debian-powerpc:~# time for i in {1..1000}; do /bin/echo hello / dev/null; done real0m7.557s user0m4.121s sys 0m3.426s So this is a significant performance improvement! I'm quite happy how fast this whole thing becomes :) Yeah :-) Do you have timings for the native system as well? Segher ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 11/27] KVM: PPC: Make RMO a define
v1 - v2: - RMO - PAM Except you forgot the subject line. Segher ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 27/27] KVM: PPC: Add Documentation about PV interface
+To find out if we're running on KVM or not, we overlay the PVR register. Usually +the PVR register contains an id that identifies your CPU type. If, however, you +pass KVM_PVR_PARA in the register that you want the PVR result in, the register +still contains KVM_PVR_PARA after the mfpvr call. + + LOAD_REG_IMM(r5, KVM_PVR_PARA) + mfpvr r5 + [r5 still contains KVM_PVR_PARA] I love this part :-) + __u64 scratch3; + __u64 critical; /* Guest may not get interrupts if == r1 */ + __u64 sprg0; + __u64 sprg1; + __u64 sprg2; + __u64 sprg3; + __u64 srr0; + __u64 srr1; + __u64 dar; + __u64 msr; + __u32 dsisr; + __u32 int_pending; /* Tells the guest if we have an interrupt */ +}; + +Additions to the page must only occur at the end. Struct fields are always 32 +bit aligned. The u64s are 64-bit aligned, should they always be? +The ld and std instructions are transormed to lwz and stw instructions +respectively on 32 bit systems with an added offset of 4 to accomodate for big +endianness. Will this add never overflow? Is there anything that checks for it? +mtmsrd rX, 0 b special mtmsr section +mtmsr b special mtmsr section mtmsr rX Segher ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] arch/powerpc/lib/copy_32.S: Use alternate memcpy for MPC512x and MPC52xx
Actually, this is something which might need closer attention - and maybe some support in the device tree indicating which read or write width a device can accept? There already is device-width; the drivers never should use any other access width unless they *know* that will work. Segher ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] arch/powerpc/lib/copy_32.S: Use alternate memcpy for MPC512x and MPC52xx
Hmm, unfortunately, it's usage is not clearly documented in mtd- physmap.txt, It's pretty clear I think. Patches for making it better are welcome of course. so I never thought of this parameter. And IMHO the problem goes further - basically *any* chip which is attached to the LPB can be affected by this problem, so it might be better to have a more general approach like a chip select property. You cannot treat devices on the LPB as random access, that's all. Drivers that assume they can, cannot be used for devices on the LPB. Segher ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc:prom Export device tree physical address via proc
+ if (prop) + prom_remove_property(node, prop); + prop = of_find_property(node, linux,devietree-end, NULL); Either the indentation is wrong, or you're missing some braces here You're missing a c as well (and a dash). Segher ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH V4] powerpc/prom: Export device tree physical address via proc
V4: Fixed misspelling Any particular reason you fixed only one of the two mispelings I pointed out? (device tree is two words, not one). + prop = of_find_property(node, linux,devicetree-start, NULL); + if (prop) + prom_remove_property(node, prop); + + prop = of_find_property(node, linux,devicetree-end, NULL); + if (prop) + prom_remove_property(node, prop); + + flat_dt_start = virt_to_phys(initial_boot_params); + flat_dt_end = virt_to_phys(initial_boot_params) + + initial_boot_params-totalsize; + prom_add_property(node, flat_dt_start_prop); + prom_add_property(node, flat_dt_end_prop); You could use one property instead of two; use addr+len like every other property does. You also should use a better name for the property; is this the previous kernel's device tree? Just device-tree makes no sense, it is not pointing to the device tree for sure! Segher ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH V4] powerpc/prom: Export device tree physical address via proc
Any particular reason you fixed only one of the two mispelings I pointed out? (device tree is two words, not one). Ahh, my fault. Well I wasn't terribly clear ;-) You could use one property instead of two; use addr+len like every other property does. You also should use a better name for the property; is this the previous kernel's device tree? Just device-tree makes no sense, it is not pointing to the device tree for sure! What about just one node called flat-device-tree? But *what* flat device tree? It cannot be the flat device tree, or it would be useless information, since we are already reading it! Segher ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev