Re: Linuxppc-dev Digest, Vol 37, Issue 84
From: lakshminarayana babu <hidden>
Date: 2007-09-13 04:31:43
i am new to the linux.....can you tell me how to access the pci configuration space using ioremap function...but it is implicit function declaration... On 9/13/07, linuxppc-dev-request@ozlabs.org [off-list ref] wrote:
Send Linuxppc-dev mailing list submissions to linuxppc-dev@ozlabs.org To subscribe or unsubscribe via the World Wide Web, visit https://ozlabs.org/mailman/listinfo/linuxppc-dev or, via email, send a message with subject or body 'help' to linuxppc-dev-request@ozlabs.org You can reach the person managing the list at linuxppc-dev-owner@ozlabs.org When replying, please edit your Subject line so it is more specific than "Re: Contents of Linuxppc-dev digest..." Today's Topics: 1. Re: [PATCH 15/15] Add DEFINE_SPUFS_ATTRIBUTE() (Michael Ellerman) 2. Re: [PATCH v3] [POWERPC] 85xx: Add basic Uniprocessor MPC8572 DS port (Segher Boessenkool) 3. Re: [PATCH v3] [POWERPC] 85xx: Add basic Uniprocessor MPC8572 DS port (Segher Boessenkool) 4. Re: [PATCH 1/5] Add Freescale DMA and DMA channel to Documentation/powerpc/booting-without-of.txt file. (Segher Boessenkool) 5. Re: [PATCH v3] [POWERPC] 85xx: Add basic Uniprocessor MPC8572 DS port (Segher Boessenkool) 6. Re: [PATCH v4] [POWERPC] 85xx: Add basic Uniprocessor MPC8572 DS port (Segher Boessenkool) 7. Re: [PATCH] [POWERPC] DTS cleanup (Segher Boessenkool) 8. Re: [PATCH] [POWERPC] 85xx: Add basic Uniprocessor MPC8572 DS port (Segher Boessenkool) 9. Re: [PATCH] [RFC][POWERPC] Merge 32 and 64 bit pci_process_bridge_OF_ranges() instances (Segher Boessenkool) 10. Re: [PATCH] [POWERPC] 85xx: Add basic Uniprocessor MPC8572 DS port (Segher Boessenkool) 11. Re: [PATCH] [POWERPC] DTS cleanup (Kumar Gala) ---------------------------------------------------------------------- Message: 1 Date: Thu, 13 Sep 2007 12:05:41 +1000 From: Michael Ellerman <redacted> Subject: Re: [PATCH 15/15] Add DEFINE_SPUFS_ATTRIBUTE() To: Arnd Bergmann <arnd@arndb.de> Cc: linuxppc-dev@ozlabs.org, Jeremy Kerr <jk@ozlabs.org>, linux-kernel@vger.kernel.org Message-ID: [ref] Content-Type: text/plain; charset="us-ascii" On Wed, 2007-09-12 at 10:47 +0200, Arnd Bergmann wrote:quoted
On Wednesday 12 September 2007, Michael Ellerman wrote:quoted
On Wed, 2007-09-12 at 17:43 +1000, Michael Ellerman wrote:quoted
This patch adds DEFINE_SPUFS_ATTRIBUTE(), a wraper around DEFINE_SIMPLE_ATTRIBUTE which does the specified locking for the get routine for us. Unfortunately we need two get routines (a locked and unlockedversion) toquoted
quoted
quoted
support the coredump code. This patch hides one of those (the lockedversion)quoted
quoted
quoted
inside the macro foo.quoted
jk said:quoted
"Good god man!"Yeah, I'm a bit lukewarm on this one. But the diffstat is nice, 50%codequoted
quoted
reduction ain't bad :)Have you looked at the change in object code size? I would expect the object code to actually become bigger. I also think that it hurts readability rather than help it.Yeah I did, it's smaller actually: text data bss dec hex filename 44898 17804 120 62822 f566 spufs-before.o 44886 17804 120 62810 f55a spufs-after.oquoted
Maybe a better solution is to change the core dump code to not require the mutex to be held in the first place. By the time we get to call the get functions, it should already be in saved state and no longer be able to get scheduled, so we might not actually need all the extra tricks with avoiding the mutex to be taken again.Well that'd be nice, but I don't see anywhere that that happens. AFAICT the acquire we do in the first coredump callback is the first the SPU contexts know about their PPE process dying. And spufs is still live, so I think we definitely need to grab the mutex, or we might race with userspace accessing spufs files. cheers -- Michael Ellerman OzLabs, IBM Australia Development Lab wwweb: http://michael.ellerman.id.au phone: +61 2 6212 1183 (tie line 70 21183) We do not inherit the earth from our ancestors, we borrow it from our children. - S.M.A.R.T Person -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 189 bytes Desc: This is a digitally signed message part Url : http://ozlabs.org/pipermail/linuxppc-dev/attachments/20070913/d7914b17/attachment-0001.pgp ------------------------------ Message: 2 Date: Wed, 12 Sep 2007 15:36:55 +0200 From: Segher Boessenkool <redacted> Subject: Re: [PATCH v3] [POWERPC] 85xx: Add basic Uniprocessor MPC8572 DS port To: Kumar Gala <redacted> Cc: linuxppc-dev@ozlabs.org Message-ID: [ref] Content-Type: text/plain; charset=US-ASCII; format=flowed Looks a lot better, thanks! Some minor nits and suggestions...quoted
+/ { + model = "fsl,MPC8572DS"; + compatible = "fsl,MPC8572DS", "fsl,MPC85xxDS";We don't want "xx" compatible entries; especially here it makes no sense at all. If the board is compatible to some other (older) board, just name that board explicitly.quoted
+ PowerPC,8572@0 {Maybe it would be good to use "PowerPC,e500" instead -- it would make it easier to probe for the actual CPU type, that way. Not that Linux uses the name/compatible here at all ;-)quoted
+ soc8572@ffe00000 {You should put an interrupt-parent in here, so you can get rid of it in all the children. And then there's the pci_bridge thing we're discussing on IRC, of course -- basically, get rid of the pci_bridge pseudo-node, and move the interrupt-map for the south-bridge devices into the south-bridge node. Segher ------------------------------ Message: 3 Date: Wed, 12 Sep 2007 16:00:38 +0200 From: Segher Boessenkool <redacted> Subject: Re: [PATCH v3] [POWERPC] 85xx: Add basic Uniprocessor MPC8572 DS port To: David Gibson <redacted> Cc: linuxppc-dev@ozlabs.org Message-ID: [ref] Content-Type: text/plain; charset=US-ASCII; format=flowedquoted
quoted
quoted
quoted
+ uli1575@0 { + reg = <0 0 0 0 0>;This looks kind of bogus...Its a PCIe to PCI bridge that is transparent.Right.... if it has no control registers, I think it should just lack 'reg', not define a zero-length register block."reg" for PCI config registers has length 0 always, it's defined that way in the PCI binding. But if this thing is transparent, it doesn't have PCI config regs.quoted
quoted
quoted
quoted
+ #size-cells = <2>; + #address-cells = <3>; + ranges = <02000000 0 80000000 + 02000000 0 80000000 + 0 20000000 + 01000000 0 00000000 + 01000000 0 00000000 + 0 00100000>;And if truly transparent, it should perhaps have just ranges; indicating that child addresses are identity mapped to parent addresses.If truly transparent, the node should just not be there at all!quoted
quoted
quoted
quoted
+ pci_bridge@0 {Ok.. why is pci_bridge nested within uli1575 - with the matching reg and ranges, it looks like they ought to be one device. Also if this is a PCI<->PCI bridge, I believe it shold have device_type = "pci".We've been using this as it stands for a while. If there are some changes here that make sense I'm willing to make them.Right, at present I don't see why you couldn't just ditch the pci_bridge node, and drop its contents straight into the uli1575 node.Yeah. The preferred name for PCI-to-PCI bridge nodes is simply "pci", btw. Segher ------------------------------ Message: 4 Date: Thu, 13 Sep 2007 04:09:29 +0200 From: Segher Boessenkool <redacted> Subject: Re: [PATCH 1/5] Add Freescale DMA and DMA channel to Documentation/powerpc/booting-without-of.txt file. To: Scott Wood <redacted> Cc: linuxppc-dev@ozlabs.org, Zhang Wei-r63237 [off-list ref], paulus@samba.org, David Gibson [off-list ref] Message-ID: [ref] Content-Type: text/plain; charset=US-ASCII; format=flowedquoted
quoted
I have a strange issue here. If I rename 'fsl,dma' to 'fsl,mpc8540-dma', the 'fsl,mpc8540-dma-channel' will be also regarded as DMA device not DMA channel.What tree are you using? Commit 804ace8881d211ac448082e871dd312132393049 in Paul's git tree should have fixed that.Strange, I don't see that commit -- maybe gitweb is broken, or maybe the patch was superseded, or maybe it just disappeared? It's still shown in patchworks with this commit id fwiw. Segher ------------------------------ Message: 5 Date: Wed, 12 Sep 2007 16:10:09 +0200 From: Segher Boessenkool <redacted> Subject: Re: [PATCH v3] [POWERPC] 85xx: Add basic Uniprocessor MPC8572 DS port To: Kumar Gala <redacted> Cc: linuxppc-dev@ozlabs.org, David Gibson [off-list ref] Message-ID: [ref] Content-Type: text/plain; charset=US-ASCII; format=flowedquoted
quoted
quoted
+ i8259: interrupt-controller@20 { + reg = <1 20 2 + 1 a0 2 + 1 4d0 2>; + clock-frequency = <0>;Hrm.. what is clock-frequency for on an i8259? I see that other 8259 descriptions have this as well, so it's not a problem with this patch specifically.Its a copy-paste thing so I don't know.If your bootwrapper doesn't fill in this value, you should get rid of this property -- better to have no value than to have the wrong value, esp. since it's probably unused anyway. Segher ------------------------------ Message: 6 Date: Thu, 13 Sep 2007 00:22:58 +0200 From: Segher Boessenkool <redacted> Subject: Re: [PATCH v4] [POWERPC] 85xx: Add basic Uniprocessor MPC8572 DS port To: Kumar Gala <redacted> Cc: linuxppc-dev@ozlabs.org Message-ID: [ref] Content-Type: text/plain; charset=US-ASCII; format=flowedquoted
+ soc8572@ffe00000 { + #address-cells = <1>; + #size-cells = <1>; + device_type = "soc"; + ranges = <00000000 ffe00000 00100000>; + reg = <ffe00000 00001000>; // CCSRBAR & soc regs,remove once parsequoted
code for immrbase fixedHrm, if you remove it here, where else are you going to describe the SoC control register block (whatever it is called -- is that IMMR?) Segher ------------------------------ Message: 7 Date: Thu, 13 Sep 2007 00:10:38 +0200 From: Segher Boessenkool <redacted> Subject: Re: [PATCH] [POWERPC] DTS cleanup To: Kumar Gala <redacted> Cc: linuxppc-dev@ozlabs.org Message-ID: [ref] Content-Type: text/plain; charset=US-ASCII; format=flowedquoted
* 32-bit in cpu node -- doesn't exist in any spec and not used by kernelYeah.quoted
* built-in for non-standard buses (ISA, PCI)"built-in" is some weird CHRP property, so yes we don't need it or want it.quoted
* Removed #interrupt-cells in places they don't need to be setGreat :-)quoted
* Fixed ranges on lite5200*This has a problem still:quoted
model = "fsl,mpc5200"; compatible = "mpc5200"; revision = ""; // from bootloader - #interrupt-cells = <3>; device_type = "soc"; - ranges = <0 f0000000 f0010000>; - reg = <f0000000 00010000>; + ranges = <0 f0000000 0000c000>; + reg = <f0000000 0000c000>;That makes "reg" and "ranges" identify an identical address range, which means no subnode can claim any address in that range, so the "ranges" property should go. Alternatively, the "reg" might be claiming too big a space. Which is it? Segher ------------------------------ Message: 8 Date: Wed, 12 Sep 2007 15:20:14 +0200 From: Segher Boessenkool <redacted> Subject: Re: [PATCH] [POWERPC] 85xx: Add basic Uniprocessor MPC8572 DS port To: Scott Wood <redacted> Cc: Olof Johansson <redacted>, linuxppc-dev@ozlabs.org Message-ID: [ref] Content-Type: text/plain; charset=US-ASCII; format=flowedquoted
quoted
quoted
well the ifdefs are orthogonal. We don't have a way of knowing primary from the device tree today.How about something like "fsl,primary-phb" in the bus device node? I don't know, maybe it's already been discussed and turned down for some reason.It's more of a Linux issue than anything to do with the hardware.Yeah, many machines actually have multiple "primary PHBs", and Linux cannot really deal with that. It's probably best to handle this from platform code.quoted
quoted
Or would it be sufficient to check children of that device node to see if the ULi is on that bus?Or more generally, see if an isa node is somewhere in that subtree.And if there is no ISA node, find any other legacy device (non-native ATA controllers, ...), etc. Segher ------------------------------ Message: 9 Date: Wed, 12 Sep 2007 16:51:25 +0200 From: Segher Boessenkool <redacted> Subject: Re: [PATCH] [RFC][POWERPC] Merge 32 and 64 bit pci_process_bridge_OF_ranges() instances To: Arnd Bergmann <arnd@arndb.de> Cc: linuxppc-dev@ozlabs.org Message-ID: [ref] Content-Type: text/plain; charset=US-ASCII; format=flowedquoted
quoted
quoted
quoted
+struct ranges_pci { + unsigned int pci_space; + u64 pci_addr; + phys_addr_t phys_addr; + u64 size; +} __attribute__((packed)); +This structure definition uses unaligned members because of the 'packed' attribute. Is that really what you intended?yes, exactly, because I'm mapping this struct on ranges extracted from the dts instead of juggling with ranges[foo] offsets.I see. It does however look wrong to me, because you are using a hardcoded phys_addr_t type. This breaks when phys_addr has a different size from what you expect, e.g. when booting a pure 32 bit kernel on a machine that has a 64 bit physical address space.More generally, you can even have a different size for the "phys_addr" for different nodes in the same device tree. You really should look at the #address-cells in this node's parent, and translate that all the way up to the root node to get a CPU address. Segher ------------------------------ Message: 10 Date: Wed, 12 Sep 2007 15:20:25 +0200 From: Segher Boessenkool <redacted> Subject: Re: [PATCH] [POWERPC] 85xx: Add basic Uniprocessor MPC8572 DS port To: Kumar Gala <redacted> Cc: linuxppc-dev@ozlabs.org Message-ID: [ref] Content-Type: text/plain; charset=US-ASCII; format=flowedquoted
quoted
quoted
+ l2-cache-controller@20000 { + compatible = "fsl,8572-l2-cache-controller"; + reg = <20000 1000>; + cache-line-size = <20>; // 32 bytes + cache-size = <80000>; // L2, 512K + interrupt-parent = <&mpic>; + interrupts = <10 2>; + };Should this node be referenced by an l2-cache property in the cpu node?No, its a front side cache.What is a "front side cache"? What exactly does it cache? If it's a cache for one CPU only, that fact should be shown in the device tree somehow.quoted
quoted
quoted
+ device_type = "pci"; + #interrupt-cells = <1>; + #size-cells = <2>; + #address-cells = <3>; + reg = <8000 1000>; + bus-range = <0 ff>; + ranges = <02000000 0 80000000 80000000 0 20000000 + 01000000 0 00000000 ffc00000 000010000>;quoted
quoted
No prefetchable mem space?we haven't normally provided prefetch on 85xx/86xx.. will deal with this later.If you don't set up prefetchable memory regions on the PCI from the firmware, this code is fine, sure. It would be a good plan to do map all BARs that say they are prefetchable in some prefetchable PCI window, it gives a nice speed boost, even when the kernel accesses it as simple non-cacheable space: the PCI bridges in between can streamline loads from these areas. In any case, the device tree should be in synch with how the firmware set up the PCI hardware, and it seems that's what you have now, so all is fine. Segher ------------------------------ Message: 11 Date: Wed, 12 Sep 2007 22:08:33 -0500 From: Kumar Gala <redacted> Subject: Re: [PATCH] [POWERPC] DTS cleanup To: Segher Boessenkool <redacted> Cc: linuxppc-dev@ozlabs.org Message-ID: [ref] Content-Type: text/plain; charset=US-ASCII; delsp=yes; format=flowed On Sep 12, 2007, at 5:10 PM, Segher Boessenkool wrote:quoted
quoted
* 32-bit in cpu node -- doesn't exist in any spec and not used by kernelYeah.quoted
* built-in for non-standard buses (ISA, PCI)"built-in" is some weird CHRP property, so yes we don't need it or want it.Do you suggest we get ride of it from ISA nodes as well?quoted
quoted
* Removed #interrupt-cells in places they don't need to be setGreat :-)quoted
* Fixed ranges on lite5200*This has a problem still:quoted
model = "fsl,mpc5200"; compatible = "mpc5200"; revision = ""; // from bootloader - #interrupt-cells = <3>; device_type = "soc"; - ranges = <0 f0000000 f0010000>; - reg = <f0000000 00010000>; + ranges = <0 f0000000 0000c000>; + reg = <f0000000 0000c000>;That makes "reg" and "ranges" identify an identical address range, which means no subnode can claim any address in that range, so the "ranges" property should go. Alternatively, the "reg" might be claiming too big a space. Which is it?Yeah, I think it should be 0x100 for the 'soc' regs on 52xx so I'll set regs to that. - k ------------------------------ _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev End of Linuxppc-dev Digest, Vol 37, Issue 84 ********************************************