Re: RFC: ARM Boot standard for passing device tree blob
On Fri, Mar 26, 2010 at 07:43:20AM -1000, Mitch Bradley wrote: What is the reason for turning off the data caches? Leaving all caches turned on and coherent with one another has always worked well for me at the interface from firmware to a booted program. With the data caches on, you need the MMU to be setup and operational. That's impractical when you're passing control between differing pieces of software which may have different ideas about how to setup the MMU and where to place the page tables. If you accidentally overwrite the page tables, things quickly become undebuggable. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: Request review of device tree documentation
On Sun, Jun 13, 2010 at 11:23:45PM -0600, Grant Likely wrote: Or perhaps the MMU and caches can be turned off for the duration of the callback. I don't have the details of ARM MMUs and caches reloaded into my head yet. Maybe next week... We've had these kinds of questions in the past. Doing what you're asking above is not really an option - it requires: 1. disable all IRQs 2. setup 1:1 MMU mappings for code to turn off MMU (requires new page table) 3. disable imprecise exceptions 4. flush caches and TLBS 5. jump to 1:1 mapping area for code to disable MMU 6. disable caches and mmu 7. call function 8. flush caches and TLBs 9. re-enable caches and mmu 10. re-enable imprecise exceptions 11. switch back to original MMU mappings 12. re-enable all IRQs This is fine if you don't care at all about interrupt latency. Unfortunately, most people do care about interrupt latency because that directly affects interactivity and system performance. The called function could not enable interrupts or exceptions - as the CPU vectors are in virtual space, disabling the MMU effectively makes them disappear. Moreover, with the MMU and caches disabled, the CPU performance is extremely poor, so the called function will run slowly. So, disabling the MMU isn't really viable. Now, if the external code was fully PIC, we could then run it with the MMU enabled. However, this wouldn't really help - the external code could not access any devices without knowledge of how the kernel setup the V:P translations. So you'd need to pass some kind of data structure giving locations of devices to the called code - but then what if the kernel doesn't have the device mapped? ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: Request review of device tree documentation
On Sun, Jun 13, 2010 at 09:45:50PM -1000, Mitch Bradley wrote: None of this is a deal-breaker for the kind of debugging tasks that are the primary use case for the callback. Would you mind explaining what kind of tasks these callbacks will be used for? ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: Request review of device tree documentation
On Mon, Jun 14, 2010 at 07:36:10PM +1000, Benjamin Herrenschmidt wrote: However, there's a lot of room for abuse here and I'm worried that if it becomes widespread, we'll start seeing vendors use that as a way to do some kind of HAL and hide various platform methods in there (clock control, nvram, etc...). This is what I'm worried about too. As I said in my first reply in this thread, calling out from the kernel will kill performance due to the time taken to shut down the caches and MMU, which can only be done safely with all exceptions turned off. The only time that it can be seriously considered is if you're calling out to reboot, shutdown or kexec. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: Platform data with function pointers
On Fri, Jun 18, 2010 at 01:47:28PM +0100, Lorenzo Pieralisi wrote: I think chip-select addressing should be used if that is the way HW handles it. If the device is described through a memory-mapping, ex. snippet follows: ser...@101f2000 { compatible = arm,pl011; reg = 0x101f2000 0x1000 ; }; Primecell devices aren't platform devices. They have no 'id' field as such. Instead, modern implementations have PCI-like IDs. struct map_desc { unsigned long virtual; unsigned long pfn; unsigned long length; unsigned int type; }; static struct map_desc realview_eb_io_desc[] __initdata = { { .virtual= IO_ADDRESS(REALVIEW_SYS_BASE), .pfn= __phys_to_pfn(REALVIEW_SYS_BASE), .length = SZ_4K, .type = MT_DEVICE, }, ... These mappings are entirely arbitary, and change according to the implementation of the platform. Some platforms want to avoid using ioremap() to create 4K page mappings for their devices, so instead they statically map them and arrange for ioremap() to know about that static mapping. Given that PAGE_OFFSET can be changed, it would be absolutely silly to put this into the device tree. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] ARM: map ATAGs when not in first 1MB of RAM
On Thu, Jan 27, 2011 at 09:50:43AM -0600, Rob Herring wrote: + /* + * Otherwise map the 1MB region r2 points to (atags or dtb) + */ +1: mov r0, r2, lsr #20 + mov r0, r0, lsl #20 + sub r3, r0, #(PHYS_OFFSET 0xff00) + .if (PHYS_OFFSET 0x00f0) + sub r3, r3, #(PHYS_OFFSET 0x00f0) This introduces new PHYS_OFFSET uses which we're trying hard to get rid of. This will need to be reworked. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] ARM: map ATAGs when not in first 1MB of RAM
On Thu, Jan 27, 2011 at 11:41:30AM -0600, Rob Herring wrote: Russell, On 01/27/2011 10:31 AM, Russell King - ARM Linux wrote: On Thu, Jan 27, 2011 at 09:50:43AM -0600, Rob Herring wrote: + /* +* Otherwise map the 1MB region r2 points to (atags or dtb) +*/ +1: mov r0, r2, lsr #20 + mov r0, r0, lsl #20 + sub r3, r0, #(PHYS_OFFSET 0xff00) + .if (PHYS_OFFSET 0x00f0) + sub r3, r3, #(PHYS_OFFSET 0x00f0) This introduces new PHYS_OFFSET uses which we're trying hard to get rid of. This will need to be reworked. Yeah, I didn't really like that either. How about this? It's untested. It replaces the whole section mapping the 1st 1MB and should make replacing this instance of PHYS_OFFSET with a variable easier. With the p2v patches, queued for the next merge window, PHYS_OFFSET becomes: extern unsigned long __pv_phys_offset; #define PHYS_OFFSET __pv_phys_offset so using PHYS_OFFSET in any way in assembly isn't going to work too well. Luckily, the p2v patches supply __create_page_tables with a value of PHYS_OFFSET in r8. What this means is that there's an fundamental interdependence between your patches and mine. Your patches can work without the p2v patches. Or they can be made to work with the p2v patches applied. So I think this needs to wait until after the next merge window, or it needs to be prepared against the p2v patches (see my p2v branch) and applied there once everyone's happy with it. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v2] ARM: map ATAGs when not in first 1MB of RAM
On Fri, Jan 28, 2011 at 08:00:46AM -0600, Rob Herring wrote: @@ -206,11 +206,17 @@ __create_page_tables: #endif /* - * Then map first 1MB of ram in case it contains our boot params. + * Then map boot params address in r2 or + * the first 1MB of ram if boot params address is not specified. */ - add r0, r4, #PAGE_OFFSET 18 - orr r6, r7, r8 - str r6, [r0] + mov r0, r2, lsr #20 + movsr0, r0, lsl #20 + moveq r0, r8 + sub r3, r0, r8 + add r3, r3, #PAGE_OFFSET + add r3, r4, r3, lsr #18 + orr r6, r7, r0 + str r6, [r3] Wouldn't: str r6, [r4, r3, lsr #18] work here? ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCHv3] ARM:boot:device tree: Allow the device tree binary to be appended to zImage
On Fri, Apr 29, 2011 at 09:16:39AM -0400, Nicolas Pitre wrote: On Fri, 29 Apr 2011, Tony Lindgren wrote: If the compressed image is smaller than BSS, then we end up having DT data in the BSS area. In this case the compressed image is about 2.3 MB for LZMA. The uncompress code does not know about the kernel BSS, and does not necessarily relocate anything depending on the compressed image load address. So in which code do we want to relocate the DT data? We could do it based on estimated BSS size in uncompress code, or based on the real BSS size in __mmap_switched before BSS gets reset. Estimations for that kind of thing is always bound to create problems some day. The DT data should probably be moved out of the way from arch/arm/kernel/head.S before the .bss is cleared, and even before enabling the MMU, like in __vet_atags. Err, no. Moving stuff around becomes quite expensive when the cache is not on. It's far better to work out where to place it first time around so its not in the way. Remember that there is a section of the community which cares deeply about boot time and has already put quite some resources into sorting that out. CELF springs to mind. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC PATCH 4/4] ARM: Xilinx: Adding Xilinx board support
On Mon, May 02, 2011 at 09:50:11PM +, johnl...@comcast.net wrote: Seems easy enough assuming we don't need the SCU early for the core count and can get that from the device tree. Beware. Things may not be that trivial when you start considering some of the weirdnesses that some vendors start coming out with. Since the SCU and part of the GIC end up in the same 4K page they need to be together (unless I'm thinking about that wrong). SCU 0xF800 GIC_CPU 0xF8000100 GIC_DIS 0xF8001000 It is preferable to use the same mapping, as it ensures that you're not going to create incompatible aliases. But, before we get too bogged down with this, please realise that it is possible to setup early mappings from the device tree for things like the SCU. Maybe not using ioremap(), but I'm sure we can find some way to use the iotable_init() stuff to setup some early mappings from DT for this stuff. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC PATCH 4/4] ARM: Xilinx: Adding Xilinx board support
On Tue, May 03, 2011 at 09:58:14AM +0200, Arnd Bergmann wrote: On Tuesday 03 May 2011 01:01:18 Russell King - ARM Linux wrote: It is preferable to use the same mapping, as it ensures that you're not going to create incompatible aliases. Is aliasing a problem for MMIO mappings? I would think that you can ioremap registers anywhere and as often as you want because they are never cacheable. Provided the type and sharability is identical then there is no problem. I wasn't referring to multiple ioremap()s of the same region - that should be fine as the same attributes will be used. If you mix ioremap() and iotable_init() then you _could_ have problems if you don't use MT_DEVICE in the iotable. What I'm basically saying is that mixing the methods of creating these mappings makes it much easier to get tripped up over these issues. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: device_tree binding for amba bus
On Tue, May 17, 2011 at 05:05:12PM -0700, Stephen Neuendorffer wrote: In drivers/amba/bus.c: static int amba_get_enable_pclk(struct amba_device *pcdev) { struct clk *pclk = clk_get(pcdev-dev, apb_pclk); static int amba_get_enable_vcore(struct amba_device *pcdev) { struct regulator *vcore = regulator_get(pcdev-dev, vcore); So users of this bus have to have a clock and a regulator with those exact names. Is it your expectation that the clock/regulator names are standardized across arch/arm? Let's fix one thing here. The second argument to clk_get is *not* a unique clock name. It is a _connection_ name for the device, to distinguish between different clocks on the same device. It does not identify _any_ particular clock on its own. And a little grepping suggests that almost all of the machines that possibly use this don't do anything useful with it (just implementing duplicated dummy code so that apb_pclk has something to refer to). apb_pclk is the _bus_ clock for the device, which must be enabled for any register access to the device, including the Primecell ID registers. Most platforms do not have any facilities to control this clock, but there are some which do. One of those which does is the ST stuff: mach-u300/clock.c: * The MMCI apb_pclk is hardwired to the same terminal as the mach-u300/clock.c: * The SPI apb_pclk is hardwired to the same terminal as the mach-u300/clock.c: DEF_LOOKUP_CON(intcon, apb_pclk, intcon_clk), mach-u300/clock.c: DEF_LOOKUP_CON(pl172, apb_pclk, emif_clk), mach-u300/clock.c: DEF_LOOKUP_CON(mmci, apb_pclk, mmcsd_clk), mach-u300/clock.c: DEF_LOOKUP_CON(pl022, apb_pclk, spi_clk), mach-u300/clock.c: DEF_LOOKUP_CON(uart1, apb_pclk, uart1_pclk), mach-u300/clock.c: DEF_LOOKUP_CON(uart0, apb_pclk, uart0_pclk), For these, they gate the bus clock along with the functional clock for the device. The side effect of that is using the clock enable points in the driver-based code which are designed for the functional clock results in the bus clock being turned off, and then the driver tries to access the device registers - resulting in the device not being accessible. So, we decided to separate out this bus clock, call it 'apb_pclk' (it's the ARM Primecell Bus, PCLK input on the device) and allow platforms which have this problem to deal with it _without_ having to add ifdefs or other shite to all the drivers. Note that you need to turn on this very clock to even read the IDs out of the device, in order to match the driver. So, it really doesn't make sense for the driver to do it when the bus level code which understands the extraction of the IDs needs to fiddle with it anyway. I didn't want to go around _all_ the primecell drivers adding yet another probe step, failure path in the probe function, and additional call in their remove functions - every additional thing which needs to be done in order is another thing that can get out of order or be forgotten in the failure cleanup path. The ST devices can alias the apb_pclk to the functional clock, thereby ensuring that while the device needs to be accessible, both the device PCLK and functional clock remain enabled. Meanwhile others which don't allow the PCLK to be turned on and off can control their functional clock as the driver desires. I hope this helps to understand what's going on here. I think what we have today is an elegant solution to the problem presented by some SoCs. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 2/2] drivers/amba: probe via device tree
On Sat, May 21, 2011 at 11:42:34AM -0600, Grant Likely wrote: Russell, it seems to me that the primary behaviour that amba_bus has over platform_bus is the clock management, and secondarily verification of the type of device by the device id. Am I correct, or am I missing something? It matches by vendor/device ID just like PCI does, and does the bus clock management and power management in a really nice way, which I doubt platform devices will ever do. The way this discussion is going, I'm going to suggest that we also convert PCI stuff to being platform devices too. I don't see the point of PCI existing for all the same reasons being given in this thread. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 2/2] drivers/amba: probe via device tree
On Mon, May 23, 2011 at 08:00:15AM -0700, Stephen Neuendorffer wrote: To be specific (whether this is 'to shreds' or not, you can decide). 1. amba_bus expects the old ARM primcell ID. The one in the new A9 IP appears to be different. (b105900d instead of b105f00d) Ok, so we can update to accept the A9 ID if the other IDs are compatible. My guess is that its changed because the format of the IDs has changed, and so it serves to distinguish the format. At the moment I have no information what so ever on this change of ID. Some documentation on this would be useful. 2. Not only does amba_bus reference apb_pclk, but the driver directly references emu_src_clk. Neither of these has direct analogs on our hardware. apb_pclk doesn't reference anything on most APB-using hardware either. As I've already tried to explain, it's there for those who _do_ have that, those being the ST Ericsson people, whose cores would lockup or die in a kernel oops without it. We're not having platform specific crap appearing in generic drivers, so a _generic_ solution to the problem is to have everyone provide an apb_pclk whether they have it or not. emu_src_clk is there for the OMAP, and again if it's not required then you just need to provide it with a dummy. 3. Implementing the workaround for 2 that you've described requires a dummy implementation of the 'standard' clock stuff, which is currently not implemented by mach-xilinx, because we simply haven't needed it. Is the right solution here to add more machine-specific code? In that case, why not just do this: #define DEV_NAME whatever_your_etb_device_is_called void clk_disable(struct clk *clk) { } int clk_enable(struct clk *clk) { return 0; } struct clk *clk_get(struct device *dev, const char *id) { return dev strcmp(dev_name(dev), DEV_NAME) == 0 ? NULL : ERR_PTR(-ENOENT); } void clk_put(struct clk *clk) { } That results in very little actual code, and this is precisely one of the reasons why I don't like having a huge big pile of code behind the clk API implementing lots of useless stuff which platforms like yours just don't want. 4. etm is old, we have ptm and other custom cores, but the driver combines the etm and etb functionality together. If we have hardware coming along which re-uses etb without etm, then the driver needs to be re-worked to allow the OK, some of this is trivial to fix, but some of it isn't. Apart from (4), nothing you've said is anything but trivial. So you seem to be blaming the bus-level code for problems which have precisely nothing to do with the bus-level code at all. I really can't understand that, other than by assigning it to either gross misunderstanding (due to not being able to separate the issues) or maybe a religious or political agenda. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC 1/2] ARM:Tegra: Device Tree Support: Initialize the audio card from the device tree.
On Thu, Jun 02, 2011 at 10:04:45AM -0600, Grant Likely wrote: Right now we can't do dynamic registration for on-chip devices in a lot of cases because we don't have the infrastructure to hook up the associated struct clks. I've been wondering about this, and I don't see it as a blocking problem as you seem to be. I assume platform devices have stable names when they're created from the device tree? If yes, there's no problem having the DT start to describe the SoC specific devices _today_ - all that the clk API using clkdev requires is a stable device name. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC 1/2] ARM:Tegra: Device Tree Support: Initialize the audio card from the device tree.
On Fri, Jun 03, 2011 at 12:21:50AM +0800, Barry Song wrote: Arnd has required me to use device tree in our new SoC for the coming upstream. so i am trying to define a property like clock = uart in dts. then in drivers, i get this string by: clk = of_get_property(np, clock, NULL); then request this clock by clk_get(). This is entirely wrong. clk_get() takes two things. It takes a struct device. We should know what the struct device is (provided they're named in a stable manner.) The other parameter, the string, is up to the driver. It's not a device property. It's not a SoC-wide clock name. It's a connection name for the clock on the device. This won't change from one instance of the device to another instance of the device - it's effectively a constant. So there's no point in having the DT describe that name - that's out of its realm. One of the problems is that clk_get() hides the mapping of device+connection internally, which it has had to as we haven't had a device tree to look things up. In essence, clk_get() is looking up a property (the clock connection name) for the struct device. When clks get converted to the device tree, the DT stuff should hook inside clk_get() to do a property lookup to discover which clock the driver wants. Drivers should definitely not be looking up a property in the device tree and using that as a connection name into clk_get(). ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC 1/2] ARM:Tegra: Device Tree Support: Initialize the audio card from the device tree.
On Fri, Jun 03, 2011 at 10:32:52AM +0800, Barry Song wrote: but there is really no an unified rule by now, for exmaple, samsung just required platform device names matched with the string parameter to get a clock. it looks like clk_get in plat-samsung depends on the string more than device and the clock name is in SoC level. Samsung has been broken in respect of this for quite some time, and I've been nagging Ben about it ever since I provided clkdev. The problem is that Ben doesn't have the time to fix Samsung... same situation for mach-at91/clock.c: /* clocks cannot be de-registered no refcounting necessary */ struct clk *clk_get(struct device *dev, const char *id) { struct clk *clk; list_for_each_entry(clk, clocks, node) { if (strcmp(id, clk-name) == 0) return clk; if (clk-function (dev == clk-dev) strcmp(id, clk-function) == 0) return clk; } return ERR_PTR(-ENOENT); } EXPORT_SYMBOL(clk_get); That's broken, and it's incompatible with DT in any case because the only way to set 'clk-dev' is to have devices statically declared. OMAP used to be broken until I converted it to clkdev, and when I did their drivers became more simple because they didn't need to ifdef clocknames and such like. msm required device struct matched: struct clk *clk_get(struct device *dev, const char *id) { struct clk *clk; mutex_lock(clocks_mutex); list_for_each_entry(clk, clocks, list) if (!strcmp(id, clk-name) clk-dev == dev) goto found_it; list_for_each_entry(clk, clocks, list) if (!strcmp(id, clk-name) clk-dev == NULL) goto found_it; clk = ERR_PTR(-ENOENT); found_it: mutex_unlock(clocks_mutex); return clk; } EXPORT_SYMBOL(clk_get); Again, that's incompatible with DT in any case, as we don't know what 'clk-dev' would be if the devices aren't statically declared. So this is broken too. Each need to be converted to clkdev _before_ they even start thinking about device trees. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 0/3] patches to allow DTB to be appended to the ARM zImage
On Sun, Jun 12, 2011 at 02:06:37AM -0400, Nicolas Pitre wrote: This is a resend of those patches with fixups after the latest changes in mainline. [PATCH 1/3] ARM: zImage: ensure it is always a multiple of 64 bits in size This one is new and trivial. [PATCH 2/3] ARM: zImage: Allow the appending of a device tree binary Mostly John Bonesio's version with some adjustments and cleanups. [PATCH 3/3] ARM: zImage: make sure appended DTB doesn't get overwritten by kernel .bss New, simpler alternative to Tony Lindgren's version. One thing which has been bugging me for some time is that the DT stuff completely overrides the ATAGs. This is wrong with solutions like this. We have a set of perfectly good boot loaders which provide correct information to the kernel via ATAGs. If we start moving everything over to DT, then we run into a problem because the ATAGs are ignored - stuff such as the RAM size and command line passed from the boot loader will be entirely ignored, instead these having to be encoded into the kernel at build time. This is clearly not the right thing to be doing, and this will _prevent_ certain platforms being converted over (I'm going to refuse to convert the ones which I have an interest in with this restriction in place.) This restriction needs to be fixed without forcing people to rewrite their boot loaders. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 0/3] patches to allow DTB to be appended to the ARM zImage
On Sun, Jun 12, 2011 at 04:34:15PM +0800, Shawn Guo wrote: On Sun, Jun 12, 2011 at 09:15:41AM +0100, Russell King - ARM Linux wrote: On Sun, Jun 12, 2011 at 02:06:37AM -0400, Nicolas Pitre wrote: This is a resend of those patches with fixups after the latest changes in mainline. [PATCH 1/3] ARM: zImage: ensure it is always a multiple of 64 bits in size This one is new and trivial. [PATCH 2/3] ARM: zImage: Allow the appending of a device tree binary Mostly John Bonesio's version with some adjustments and cleanups. [PATCH 3/3] ARM: zImage: make sure appended DTB doesn't get overwritten by kernel .bss New, simpler alternative to Tony Lindgren's version. One thing which has been bugging me for some time is that the DT stuff completely overrides the ATAGs. This is wrong with solutions like this. We have a set of perfectly good boot loaders which provide correct information to the kernel via ATAGs. If we start moving everything over to DT, then we run into a problem because the ATAGs are ignored - stuff such as the RAM size and command line passed from the boot loader will be entirely ignored, instead these having to be encoded into the kernel at build time. What u-boot does right now is replacing the parameters in dtb with its for those it's interested in, for example command line is one, before it passes dtb to kernel. What if your platform doesn't use uboot? ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 0/3] patches to allow DTB to be appended to the ARM zImage
On Sun, Jun 12, 2011 at 06:42:16PM +0800, Shawn Guo wrote: On Sun, Jun 12, 2011 at 10:52:17AM +0100, Russell King - ARM Linux wrote: On Sun, Jun 12, 2011 at 05:38:23PM +0800, Shawn Guo wrote: On Sun, Jun 12, 2011 at 10:21:31AM +0100, Russell King - ARM Linux wrote: What if your platform doesn't use uboot? Add dtb parsing support with the help from libfdt, I guess. It is some amount of work, but it's not a rewrite of bootloader, IMHO. I guess you're suggesting that this wrapper uses libfdt to merge the ATAGs with the DT info? No, ATAGs does not play at all in this case. For u-boot example again, if it boots a dt kernel, dtb will be parsed to get cmdline node overwritten as bootargs env value, and then it boots the dt kernel with this updated dtb. You've missed my point entirely. What you're saying is that we have to re-build and replace the boot loader in order to pass a command line into a kernel using the DT wrapper. I'm saying that you shouldn't have to, and the kernel should accept the memory size and command line from the ATAGs _in addition_ to the appended DT blob, and the ATAGs in that case should take precidence. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 0/3] patches to allow DTB to be appended to the ARM zImage
On Sun, Jun 12, 2011 at 01:22:19PM +0200, Petr Štetiar wrote: Shawn Guo shawn@freescale.com [2011-06-12 16:34:15]: On Sun, Jun 12, 2011 at 09:15:41AM +0100, Russell King - ARM Linux wrote: One thing which has been bugging me for some time is that the DT stuff completely overrides the ATAGs. This is wrong with solutions like this. We have a set of perfectly good boot loaders which provide correct information to the kernel via ATAGs. If we start moving everything over to DT, then we run into a problem because the ATAGs are ignored - stuff such as the RAM size and command line passed from the boot loader will be entirely ignored, instead these having to be encoded into the kernel at build time. What u-boot does right now is replacing the parameters in dtb with its for those it's interested in, for example command line is one, before it passes dtb to kernel. If I understand it all correctly, there must be some 'legacy bootloader support' added to the kernel, layer which would convert the ATAGs to the DT. Or something like that. Please note, that there are some boards out there, which use some kind of proprietary bootloaders, so you can't update or change them easily. You either don't have source code, you're limited by the available space (2KB eeprom) so you can't add much more things into that bootloader or you're limited by the size of the MBR (yes, even such bootloaders exists). Exactly my point - I have quite a number of platforms here which will never be able to have a boot loader capable of modifying a DT blob for the kernel. One of the points of Nicolas' patch set is to allow existing boot loaders to boot kernels where the hardware description is contained in a DT blob encapsulated with the kernel. That's great but the way things are currently setup, it means that the boot loader does nothing more than loading and executing - and we lose the _existing_ flexibility for the boot loader to pass platform specific information to the kernel. So, what I'm considering to do is update the boot protocol such that the base address of the DT blob is provided in r3, separately from the ATAG pointer in r2. This means that boot loaders can provide a DT blob (r2 = 0 r3 = DT) or they can provide ATAGs (r2 = atag, r3 = indeterminant). We can then cater for the situation where we have an ATAG boot loader, but a kernel with an appended DT description (r2 = atag, r3 = DT) and have the ATAG information override the DT for things like memory layout and the command line string. This is the only sensible way of maintaining compatibility with the existing boot protocol, which is an absolute _must_ if we are going to convert some of the existing SoCs (like PXA) to DT and get rid of the legacy static descriptions of the on-board devices (which is the whole point we're going down this path.) It's either that or we'll have to refuse to convert existing SoCs to DT to maintain compatibility for existing boards - which makes DT support totally pointless. Let me give a situation where this matters: you have a boot loader which loads a kernel from disk and executes it. You have 256MB of RAM fitted to the machine. You replace this kernel with a DT-enabled kernel which has the DT blob appended to the kernel. The DT blob says you have 256MB of RAM. You remove a 128MB DIMM because its gone faulty. You try to boot. The boot loader provides the kernel with an ATAG telling it that there's 128MB of RAM. However, the kernel ignores the ATAGs and instead looks at the encapsulated DT information which tells it that there's 256MB of RAM. Your kernel OOPSes. You reboot, and try passing a command line argument of 'mem=128M'. The kernel again ignores this because its an ATAG. The result: you can't boot the platform. Another case: your flash has become corrupted, and the kernel won't mount the flash based rootfs. You want to boot from a root-NFS export to sort the problem out, but the kernel ignores your new command line telling it to do so. The result: you can't boot the platform. Another case: you have a Thecus N2100 acting as a server, with a pair of drives setup as raid 1. You reboot it one day and it refuses to build the raid 1 rootfs, and so panics at boot. You want to change the kernel command line so that it mounts root from somewhere else, but because you're using a DT based kernel, it ignores you. The result: you can't boot the platform. This behaviour from a DT based kernel is just not acceptable, and it's also not acceptable to expect platforms to update their boot loaders to cope with DT. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 0/3] patches to allow DTB to be appended to the ARM zImage
On Sun, Jun 12, 2011 at 04:15:23PM +0200, Arnd Bergmann wrote: On Sunday 12 June 2011 13:58:20 Russell King - ARM Linux wrote: Exactly my point - I have quite a number of platforms here which will never be able to have a boot loader capable of modifying a DT blob for the kernel. One of the points of Nicolas' patch set is to allow existing boot loaders to boot kernels where the hardware description is contained in a DT blob encapsulated with the kernel. That's great but the way things are currently setup, it means that the boot loader does nothing more than loading and executing - and we lose the existing flexibility for the boot loader to pass platform specific information to the kernel. So, what I'm considering to do is update the boot protocol such that the base address of the DT blob is provided in r3, separately from the ATAG pointer in r2. This means that boot loaders can provide a DT blob (r2 = 0 r3 = DT) or they can provide ATAGs (r2 = atag, r3 = indeterminant). We can then cater for the situation where we have an ATAG boot loader, but a kernel with an appended DT description (r2 = atag, r3 = DT) and have the ATAG information override the DT for things like memory layout and the command line string. But when you have both atag and DT and the atag overrides the DT, that means we have incorrect information in the DT, and code might later rely on that information. I don't think you're considering real-world usage scenarios, but instead concentrating on the use issues. If you only do that you're boxing yourself into a corner and will cause a world of pain for folk who would just like the kernel to be usable. The information in ATAGs which will either never be in DT or which should override what is in DT is: - memory parameters - command line - initrd location - system serial number - system revision number Things like the serial number will _never_ be in DT (do you want to build a DT image unique to each platform?) initrd location will never be in DT either (it depends where the boot loader placed it.) ... and so forth. This is the kind of information which should override whatever is in DT because the DT contained information could well be wrong or outdated, and its not the kind of information that anything other than core code should be dealing with in any case. IMHO when we allow passing a DT to a kernel while booting from an existing boot loader that only knows about atag, the code that loads the DT should be responsible for updating the DT with the atag information, not pass two conflicting sets of data into the actual kernel. So, that means Nicolas' patches which allow a DT to be appended to the image need to have DT and ATAG parsing in them to update the DT stuff with the ATAG information. Let me give a situation where this matters: you have a boot loader which loads a kernel from disk and executes it. You have 256MB of RAM fitted to the machine. You replace this kernel with a DT-enabled kernel which has the DT blob appended to the kernel. The DT blob says you have 256MB of RAM. You remove a 128MB DIMM because its gone faulty. You try to boot. The boot loader provides the kernel with an ATAG telling it that there's 128MB of RAM. However, the kernel ignores the ATAGs and instead looks at the encapsulated DT information which tells it that there's 256MB of RAM. Your kernel OOPSes. You reboot, and try passing a command line argument of 'mem=128M'. The kernel again ignores this because its an ATAG. The result: you can't boot the platform. Another case: your flash has become corrupted, and the kernel won't mount the flash based rootfs. You want to boot from a root-NFS export to sort the problem out, but the kernel ignores your new command line telling it to do so. The result: you can't boot the platform. Another case: you have a Thecus N2100 acting as a server, with a pair of drives setup as raid 1. You reboot it one day and it refuses to build the raid 1 rootfs, and so panics at boot. You want to change the kernel command line so that it mounts root from somewhere else, but because you're using a DT based kernel, it ignores you. The result: you can't boot the platform. So we need to at least the command line and the memory layout to be adapted in the in-memory DT, from the atags. Any other tags? See above. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 0/3] patches to allow DTB to be appended to the ARM zImage
On Sun, Jun 12, 2011 at 08:57:51AM -0600, Grant Likely wrote: On Sun, Jun 12, 2011 at 04:15:23PM +0200, Arnd Bergmann wrote: But when you have both atag and DT and the atag overrides the DT, that means we have incorrect information in the DT, and code might later rely on that information. IMHO when we allow passing a DT to a kernel while booting from an existing boot loader that only knows about atag, the code that loads the DT should be responsible for updating the DT with the atag information, not pass two conflicting sets of data into the actual kernel. I completely agree here. I /started/ from the position that ATAGs and DTB would coexist, and after extensive debate[1] my opinion turned around to it should be one or the other. Otherwise there are all kinds of questions about accuracy of the information and which takes precedence. And we've ended up with a fucked up situation which is extremely fragile, and actually makes me _NOT_ want to convert any existing platforms to use DT in the least. This I view as a fundamental blocker which needs addressing before anyone can make use of DT on ARM. DT is _not_ the authoritive source of information on systems with ATAGs. Imagine this situation: you have your PC. It provides memory information through the E820 interface. You convert your kernel to use DT and it only uses the information passed from the DT blob, which it loaded as part of your kernel off disk. However, your RAM size has changed. Should the kernel continue to believe the memory information found in the encapsulated DT blob, or should it continue to get it from the E820 interface? It's precisely the same problem here. The E820 interface _has_ to take precedence because that is the _authoritive_ source of information. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 0/3] patches to allow DTB to be appended to the ARM zImage
On Sun, Jun 12, 2011 at 05:01:22PM +0200, Arnd Bergmann wrote: Of course it should override the device tree, I'm not arguing that. All I'm saying is that we don't need to special-case this or support both formats once the kernel is there as long as we move the information into appropriate DT-representation of the same data. You're arguing for it to be parsed by whatever fragment of assembly code we have to place around the zImage to merge it into DT. And yes, it's going to have to be assembly code because we don't have a C environment at that point which can run in a relocatable way. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 0/3] patches to allow DTB to be appended to the ARM zImage
On Sun, Jun 12, 2011 at 11:47:59AM -0400, Nicolas Pitre wrote: On Sun, 12 Jun 2011, Russell King - ARM Linux wrote: And we've ended up with a fucked up situation which is extremely fragile, and actually makes me _NOT_ want to convert any existing platforms to use DT in the least. Agreed. I don't think that anything older than OMAP2 is worth converting to DT. The return on the investment is simply not worth it, other than for experimental purposes. I think you haven't appreciated the situation - let's take PXA as an example. PXA has been around for years, and IP in the latest silicon is present in many of the older silicons too. There's two issues here: 1. If we port existing drivers over to use DT as a means to shrink the size of the kernel, we need _all_ PXA using platforms to use DT. 2. If we continue having board support for PXA submitted, we want it to use DT support. The result will be a mess of some bits of PXA using DT, other bits using statically declared stuff. It may get to the point where on some PXA platforms DT is used to describe some of the system, and on a different PXA platform, it describes some other but needs some static stuff. I don't see this as a sustainable way forward. If we're going to move a particular SoC over to DT, we need to move the entire SoC over. We can't do this half-heartedly. And that means we _must_ deal with accepting ATAGs from existing boot loaders, with that information taking precidence over the DT blob supplied with the kernel. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 1/3] ARM: zImage: ensure it is always a multiple of 64 bits in size
On Sun, Jun 12, 2011 at 02:06:38AM -0400, Nicolas Pitre wrote: diff --git a/arch/arm/boot/compressed/vmlinux.lds.in b/arch/arm/boot/compressed/vmlinux.lds.in index ea80abe..6c02db1 100644 --- a/arch/arm/boot/compressed/vmlinux.lds.in +++ b/arch/arm/boot/compressed/vmlinux.lds.in @@ -47,6 +47,9 @@ SECTIONS .got : { *(.got) } _got_end = .; .got.plt : { *(.got.plt) } + + /* ensure the zImage file size is always a multiple of 64 bits */ + .pad : { BYTE(0); . = ALIGN(8); } Why add a byte before aligning? Isn't merely aligning to 64-bit sufficient? ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 2/3] ARM: zImage: Allow the appending of a device tree binary
On Mon, Jun 13, 2011 at 03:46:49AM -0700, Tony Lindgren wrote: * Nicolas Pitre nicolas.pi...@linaro.org [110611 23:03]: From: John Bonesio bo...@secretlab.ca This patch provides the ability to boot using a device tree that is appended to the raw binary zImage (e.g. cat zImage filename.dtb zImage_w_dtb). Signed-off-by: John Bonesio bo...@secretlab.ca [nico: adjusted to latest zImage changes plus additional cleanups] Signed-off-by: Nicolas Pitre nicolas.pi...@linaro.org Looks like this rebased version has two nice improvments: It uses r5 instead of lr, and keeps stack usable early on :) One of the comments still refers to lr though. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 0/3] patches to allow DTB to be appended to the ARM zImage
On Mon, Jun 13, 2011 at 10:14:07AM -0400, Nicolas Pitre wrote: On Mon, 13 Jun 2011, Tony Lindgren wrote: * Nicolas Pitre nicolas.pi...@linaro.org [110612 11:55]: On Sun, 12 Jun 2011, Russell King - ARM Linux wrote: I don't see this as a sustainable way forward. If we're going to move a particular SoC over to DT, we need to move the entire SoC over. We can't do this half-heartedly. And that means we _must_ deal with accepting ATAGs from existing boot loaders, with that information taking precidence over the DT blob supplied with the kernel. Well... OK. Let's see how this can be accommodated with the existing patch floating around doing that. I agree that we need to parse the user configurable ATAGs to support existing hardware properly. Otherwise we have edit the .dts for each board to change the user configurable things, which is not nice for distros. You mean existing bootloaders, right? Updated bootloaders should translate user configurable information into proper DT records and pass the resulting DTB to the kernel separately. OMAP is one of the code bases where this really matters - they have a _lot_ of existing platforms with boot loaders which do the ATAG stuff. They also have a lot of code in arch/arm that needs to be converted to a DT representation. With the current situation where you can have either ATAGs or DT but not both, they're currently facing either having to break all the existing platforms by ignoring the ATAGs _or_ keeping two copies of a considerable amount of data - one in DT form and one in its existing form. At present, DT can only be used sensibly on brand new SoCs where there are no existing platforms with ATAG based boot loaders to worry about. As things stand at present, even with your patch series, existing SoCs have no viable path to transition to DT. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 2/3] ARM: gic: add OF based initialization
On Mon, Jun 13, 2011 at 10:53:16AM -0600, Grant Likely wrote: On Tue, Jun 07, 2011 at 09:22:20AM -0500, Rob Herring wrote: +- interrupt-controller : Identifies the node as an interrupt controller +- #interrupt-cells : Specifies the number of cells needed to encode an + interrupt source. The type shall be a u32 and the value shall be 1. +- reg : Specifies base physical address(s) and size of the GIC registers. The + first 2 values are the GIC distributor register base and size. The 2nd 2 + values are the GIC cpu interface register base and size. +- irq-start : The first actual interrupt that is connected to h/w. Drop irq-start. That's a Linux internal implementation detail, and Linux can easily handle dynamic assignment of irq ranges. Something has to be done with the IRQs on GIC, because Linux probably won't have a 1:1 mapping between the hardware IRQ numbers and the Linux IRQ numbers Have you seen the patches from Marc which deal with the per-CPU interrupts by creating individual Linux IRQ numbers for each CPU for each per-CPU interrupt? So you can end up with 16 per-CPU x 4 CPUs = 64 Linux interrupts for 16 hardware interrupts. How would DT deal with that - and how would you specify a connection between a per-CPU PMU and one of the per-CPU interrupts? The sensible thing from a DT point of view I think would be to ignore that abstraction, and have some kind of mapping layer between DT and drivers which knew about that. But that sounds like a world of pain. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v2] ARM: CSR: Adding CSR SiRFprimaII board support
On Fri, Jul 01, 2011 at 06:19:43PM +0200, Arnd Bergmann wrote: On Friday 01 July 2011, Barry Song wrote: It looks like we can new a common function named as of_io_earlymap() or something in drivers/of/address.c. of_iomap() does ioremap, of_io_earlymap() does early static mapping? Then all SoCs can call this function to do early static mapping. if so, some lines can be deleted in sirfsoc_of_clk_init(). How do you think about newing the function in drivers/of/address.c? I think that's a good idea, but the ARM specific implementation cannot be in common code. Other architectures have stuff similar to iotable_init in asm/fixmap.h. If we decide on a function prototype for this, the implementation can be arch/*/. One of the issues with fixmap is that its based around single pages and indexing an area. It's idiotic to use such a thing if you have to map the ISA memory regions for VGA. Plus, of course, forcing everything down the route of ioremap() and fixmap forces everyone to use 2-level page tables and 4K page table entries, avoiding the possibility of having just a single 1st level page table entry covering their IO space. Not only does it increase TLB pressure but it also makes page table walking more expensive. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 1/2] Input: gpio-keys: do not reference platform_data after .probe exits
On Tue, Jul 19, 2011 at 09:17:26AM +0800, Shawn Guo wrote: On Mon, Jul 18, 2011 at 10:02:44AM -0700, Dmitry Torokhov wrote: On Monday, July 18, 2011 09:45:07 AM Shawn Guo wrote: The patch makes a copy of platform data into driver data, so that any reference to platform_data after .probe exits can be avoided. And why is this beneficial? I am of the opinion that platform data should stay on (and be accessed through a const pointer to ensure that the driver will not alter it). To me, it's a common sense that platform data should not be referenced after .probe exits, so that any platform code providing the data can claim the data as __initconst. That's totally buggered, and that's putting it kindly. Consider a driver built as a module, vs built-in. If you build it as a module, your driver data is stale by the time you insert the module. It's not much better with it built-in - if you have hotplug enabled, you can unbind and rebind the driver, which means that the .probe function can be called long after the .init sections have been discarded. So no, this is no justification for the patch. Don't *ever* make any platform devices or any data pointed to by a platform device discardable after init time. It's an oops waiting to happen. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 4/7] tty: serial: support device tree in pxa
On Tue, Jul 19, 2011 at 01:53:53PM -0600, Grant Likely wrote: On Tue, Jul 19, 2011 at 1:48 PM, Arnd Bergmann a...@arndb.de wrote: On Tuesday 19 July 2011 13:40:10 Grant Likely wrote: On Tue, Jul 19, 2011 at 10:24:47AM +0800, Haojian Zhuang wrote: Support both normal platform driver and device tree driver in serial pxa. Signed-off-by: Haojian Zhuang haojian.zhu...@marvell.com --- drivers/tty/serial/Kconfig | 4 +- drivers/tty/serial/of_serial.c | 12 + drivers/tty/serial/pxa.c | 93 ++- include/linux/serial_pxa.h | 17 +++ 4 files changed, 122 insertions(+), 4 deletions(-) create mode 100644 include/linux/serial_pxa.h serial_pxa is already a platform_driver. Instead of modifying of_serial, an of_match_table should be added to this driver and it should decode the DT data inside the existing probe hook. No need to create all of this extra infrastructure. Right. We should probably rename of_serial to 8250_of and remove the qpace parts from the driver. In fact, I think we've got about 3 devtree drivers for 8250 serial ports. I think it is about time for some consolidation work. :-) I wonder if we can roll of_serial directly into the 8250.c driver. The original serial.c got split into 8250.c, plus several probe modules (8250_pnp.c, 8250_pci.c, etc) to get around the problem of lots of bus specific crap appearing in the main driver. 8250_of.c would follow on that theme. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC/PATCH 2/7] OMAP3: beagle: don't touch omap_device internals
On Thu, Jul 28, 2011 at 12:53:47AM -0500, Nishanth Menon wrote: +struct device *omap_hwmod_to_device(const char *oh_name) +{ + struct omap_hwmod *oh; + + if (!oh_name) { + WARN(1, %s: no hwmod name!\n, __func__); + return ERR_PTR(-EINVAL); + } + + oh = _lookup(oh_name); + if (IS_ERR_OR_NULL(oh)) { + WARN(1, %s: no hwmod for %s\n, __func__, + oh_name); + return ERR_PTR(-ENODEV); It is good practice to always propagate back the error codes from functions which failed. So you may need something like: return ERR_PTR(oh ? PTR_ERR(oh) : -ENODEV); here. + } + if (IS_ERR_OR_NULL(oh-od)) { + WARN(1, %s: no omap_device for %s\n, __func__, + oh_name); + return ERR_PTR(-ENODEV); Same here. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v2 4/7] tty: serial: support device tree in pxa
On Fri, Jul 29, 2011 at 10:45:39AM -0600, Grant Likely wrote: On Thu, Jul 28, 2011 at 02:41:30PM +0800, Haojian Zhuang wrote: +#ifdef CONFIG_OF + for (i = 0; i PXA_SERIAL_NR; i++) { + if (serial_pxa_ports[i] == NULL) + break; + } + if (i = PXA_SERIAL_NR) { + pr_warn(can't find pxa serial port\n); + return -ENODEV; + } + + if (of_property_read_u32(np, clock-frequency, clk)) { + pr_warn(no clock-frequency property set\n); + return -ENODEV; + } + if (of_property_read_u32(np, current-speed, spd) == 0) + sport-port.custom_divisor = clk / (16 * spd); + + sprintf(name, pxa2xx-uart.%d, i); + sport-clk = clk_get_sys(name, NULL); + if (IS_ERR(sport-clk)) { + ret = PTR_ERR(sport-clk); + goto err_free; + } + sport-port.uartclk = clk; +#else + i = dev-id; sport-clk = clk_get(dev-dev, NULL); if (IS_ERR(sport-clk)) { ret = PTR_ERR(sport-clk); goto err_free; } + sport-port.uartclk = clk_get_rate(sport-clk); +#endif This means a kernel build can either support DT or non-DT, but not both. DT non-DT booting are full supported with the same kernel image, so don't do it this way. Instead, check for the presence of an of_node. If it is there, do the DT parsing. If now, still support the old method. Oh ffs, why is this DT stuff causing all the clk stuff to have to change. And specifically why is stuff converting to use clk_get_sys(). clk_get_sys() is there to allow system devices to get their clocks. It's not for general drivers to use. Please, stop this madness. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v2 6/7] i2c: pxa: support i2c controller from DT
On Fri, Jul 29, 2011 at 10:52:22AM -0600, Grant Likely wrote: On Thu, Jul 28, 2011 at 02:41:32PM +0800, Haojian Zhuang wrote: - /* -* If dev-id is negative we consider it as zero. -* The reason to do so is to avoid sysfs names that only make -* sense when there are multiple adapters. -*/ - i2c-adap.nr = dev-id; - snprintf(i2c-adap.name, sizeof(i2c-adap.name), pxa_i2c-i2c.%u, -i2c-adap.nr); - i2c-clk = clk_get(dev-dev, NULL); + if (np) { + i2c-adap.nr = idx++; Use this so that a bus number gets dynamically assigned: i2c-adap.nr = -1; + snprintf(i2c-adap.name, sizeof(i2c-adap.name), + pxa2xx-i2c.%u, i2c-adap.nr); + i2c-clk = clk_get_sys(i2c-adap.name, NULL); Missing i2c-adap.dev.of_node = dev-dev.of_node; And here we go again. Is it really the case that this DT stuff doesn't have stable device names? ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC/PATCH 7/7] WIP: HACK/RFC: omap_device: begin to decouple platform_device from omap_device
On Thu, Jul 21, 2011 at 04:52:18PM -0700, Kevin Hilman wrote: Rather than embedding a struct platform_device inside a struct omap_device, decouple them, leaving only a pointer to the platform_device inside the omap_device. This patch uses devres to allocate and attach the omap_device to the struct device, so finding an omap_device from a struct device means using devres_find(), and the to_omap_device() helper function was modified accordingly. RFC/Hack alert: Currently the driver core (drivers/base/dd.c) doesn't expect any devres resources to exist before the driver's -probe() is called. In this patch, I just comment out the warning, but we'll need to understand why that limitation exists, and if it's a real limitation. A first glance suggests that it's not really needed. If this is a true limitation, we'll need to find some way other than devres to attach an omap_device to a struct device. On OMAP, we will need an omap_device attached to a struct device before probe because device HW may be disabled in probe and drivers are expected to use runtime PM in -probe() to activate the hardware before access. Because the runtime PM API calls use omap_device (via our PM domain layer), we need omap_device attached to a platform_device before probe. This feels really wrong to overload devres with this. devres purpose is to provide the device's _drivers_ with a way to allocate and free resources in such a way to avoid leaks on .remove or probe failure. So I think that overloading it with something that has a different lifetime is completely wrong. We could add a new member to struct dev_archdata or pdev_archdata to carry a pointer to this data, which I think would be a far cleaner (and saner) way to deal with this. In much the same way as we already have an of_node member in struct device. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v2 6/7] i2c: pxa: support i2c controller from DT
On Sat, Jul 30, 2011 at 04:29:50AM -1000, Mitch Bradley wrote: On 7/29/2011 6:55 AM, Russell King - ARM Linux wrote: On Fri, Jul 29, 2011 at 10:52:22AM -0600, Grant Likely wrote: On Thu, Jul 28, 2011 at 02:41:32PM +0800, Haojian Zhuang wrote: - /* - * If dev-id is negative we consider it as zero. - * The reason to do so is to avoid sysfs names that only make - * sense when there are multiple adapters. - */ - i2c-adap.nr = dev-id; - snprintf(i2c-adap.name, sizeof(i2c-adap.name), pxa_i2c-i2c.%u, - i2c-adap.nr); - i2c-clk = clk_get(dev-dev, NULL); + if (np) { + i2c-adap.nr = idx++; Use this so that a bus number gets dynamically assigned: i2c-adap.nr = -1; + snprintf(i2c-adap.name, sizeof(i2c-adap.name), + pxa2xx-i2c.%u, i2c-adap.nr); + i2c-clk = clk_get_sys(i2c-adap.name, NULL); Missing i2c-adap.dev.of_node = dev-dev.of_node; And here we go again. Is it really the case that this DT stuff doesn't have stable device names? Device tree names are completely stable, based on hardware addresses that don't change from boot to boot. Even for buses where access addresses are dynamically assigned, the device tree reg property address is based on a stable addressing form. For example, PCI devices use the (stable) configuration address as the reg property and USB devices use the (stable) hub port number. People's tendency to want to assign sequential small integers in Linux has nothing to do with the device tree. I suspect that it's a carryover from the historical Unix major/minor device numbering model, but in any case, there's nothing unstable about the device tree naming model. Quite the opposite - stable naming was a fundamental criterion when I designed Open Firmware. Which means - if Grant is accepting the conversion of ARM to DT and upstreaming it, he needs to keep an eye on this madness and reject stuff which tries to do as per this patch. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC/PATCH 7/7] WIP: HACK/RFC: omap_device: begin to decouple platform_device from omap_device
On Sat, Jul 30, 2011 at 08:58:07PM -0600, Grant Likely wrote: On Sat, Jul 30, 2011 at 01:03:32PM +0100, Russell King - ARM Linux wrote: On Thu, Jul 21, 2011 at 04:52:18PM -0700, Kevin Hilman wrote: Rather than embedding a struct platform_device inside a struct omap_device, decouple them, leaving only a pointer to the platform_device inside the omap_device. This patch uses devres to allocate and attach the omap_device to the struct device, so finding an omap_device from a struct device means using devres_find(), and the to_omap_device() helper function was modified accordingly. RFC/Hack alert: Currently the driver core (drivers/base/dd.c) doesn't expect any devres resources to exist before the driver's -probe() is called. In this patch, I just comment out the warning, but we'll need to understand why that limitation exists, and if it's a real limitation. A first glance suggests that it's not really needed. If this is a true limitation, we'll need to find some way other than devres to attach an omap_device to a struct device. On OMAP, we will need an omap_device attached to a struct device before probe because device HW may be disabled in probe and drivers are expected to use runtime PM in -probe() to activate the hardware before access. Because the runtime PM API calls use omap_device (via our PM domain layer), we need omap_device attached to a platform_device before probe. This feels really wrong to overload devres with this. devres purpose is to provide the device's _drivers_ with a way to allocate and free resources in such a way to avoid leaks on .remove or probe failure. So I think that overloading it with something that has a different lifetime is completely wrong. I disagree; extending devres to also handle device lifetime scoped resources makes perfect sense. It is a clean extension, and struct device is really getting rather huge. I certainly prefer re-scoping devres to adding more elements to struct device. The point is you're asking something which is designed to have a well defined lifetime of driver-bind...driver-unbind to do a lot more than that - extending its lifetime conditional on some flag to the entire device lifetime instead. Such extensions tend to be a disaster - and a recipe for confusion as people will no longer have a clear idea of the lifetime issues. We already know that people absolutely struggle to understand lifed objects - we see it time and time again where people directly kfree stuff like platform devices without thinking about whether they're still in use. So no, extending devres is a _big_ mistake and is far from clean. Not only that, but if most of the platform devices are omap devices, (as it would be on OMAP), you'll consume more memory through having to have the additional management structs for the omap_device stuff than a simple pointer. As for struct device getting rather huge, last time I looked it contained a load of stuff which was there whether or not a platform used it. Eg, you get 4 bytes wasted for an of_node whether you have DT support or not. If sizeof(struct device) really is a problem, then it needs the unused stuff ifdef'd away. So I don't buy the its getting rather huge argument. We could add a new member to struct dev_archdata or pdev_archdata to carry a pointer to this data, which I think would be a far cleaner (and saner) way to deal with this. In much the same way as we already have an of_node member in struct device. Since this is just for omap_device, which by definition is arm-only, I don't have any strong objection to using pdev_archdata. If it was cross-architecture, then I'd argue strongly for the devres approach. Indeed, which is why I think putting it in the platform device archdata makes total sense, more sense than buggering up the clean devres lifetime rules that we have today. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v2 1/7] ARM: mmp: parse irq from DT
On Thu, Jul 28, 2011 at 02:41:27PM +0800, Haojian Zhuang wrote: + unsigned int status, mask, irq_base, nr, data; + int cascade; + ... + mux_info = kzalloc(sizeof(*mux_info), GFP_KERNEL); + if (mux_info == NULL) + goto out; + status += (unsigned int)mmp_info-virt_base; + mux_info-status = (void __iomem *)status; This is silly. Why not just do: mux_info-status = mmp_info-virt_base + status; and avoid those horrible casts? ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v2 4/7] tty: serial: support device tree in pxa
On Mon, Aug 01, 2011 at 10:50:47AM +0800, Haojian Zhuang wrote: On Sat, Jul 30, 2011 at 12:49 AM, Russell King - ARM Linux li...@arm.linux.org.uk wrote: Oh ffs, why is this DT stuff causing all the clk stuff to have to change. And specifically why is stuff converting to use clk_get_sys(). clk_get_sys() is there to allow system devices to get their clocks. It's not for general drivers to use. Please, stop this madness. So how could I get these clocks? Add the new _stable_ DT device name to the clkdev list. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC/PATCH 7/7] WIP: HACK/RFC: omap_device: begin to decouple platform_device from omap_device
On Tue, Aug 02, 2011 at 01:55:55AM +0300, Felipe Balbi wrote: Hi, On Mon, Aug 01, 2011 at 03:11:57PM -0700, Kevin Hilman wrote: Russell King - ARM Linux li...@arm.linux.org.uk writes: Help the typechecker do its job. As we have only one (at the moment...) And make it: +struct omap_device; struct pdev_archdata { +#ifdef CONFIG_ARCH_OMAP + struct omap_device *omap; +#endif }; for bonus points, so we only get the additional pointer for OMAP. OK, will do it this way. this has the tendency to grow larger, no ? What if all other ARMs decide to add their own pointers there too ? Their pointers for what? It's only OMAP which has this special omap_device thing. Should that spread, instead of adding more pointers here, the work should be to consolidate between those various implementations. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 1/2] arm/mx5: parse iomuxc pad configuratoin from device tree
On Fri, Aug 05, 2011 at 01:36:56PM -0500, Matt Sealey wrote: On Fri, Aug 5, 2011 at 2:07 AM, David Brown dav...@codeaurora.org wrote: On Thu, Aug 04, 2011 at 06:07:15PM -0500, Matt Sealey wrote: Hi Grant, Shawn, On Mon, Jul 25, 2011 at 3:46 PM, Grant Likely grant.lik...@secretlab.ca wrote: This could get really verbose in a really big hurry. Fortunately the dtb format is sophisticated enough to only store each unique property name once, so the data shouldn't be huge, but it is still going to make for huge source files. Can you think of a more concise representation? Yes: no representation at all. The correct place for IOMUX setup being done is *inside the boot firmware as soon as physically possible* and not seconds into boot after U-Boot has made a console, done a boot timeout, loaded scripts, kernels and ramdisks from media and then uncompressed and entered a Linux kernel. This is true in situations where we have control over the bootloader, but that isn't always the case. Indeed it is not, but then it is their fault the board won't boot Linux, and not yours, right? :) Not normally. What happens is that you're provided with the boot loader. You're told to get Linux working on the board. Anything which the boot loader doesn't do ends up being hacked around in the kernel. Don't say that isn't what happens, because that comment is made with a substantial body experience in dealing with patches from multiple different sources containing crap to work around boot loader bugs. Most boot loader bugs go _unfixed_ on hardware even after being reported. So please, don't tell us that boot loaders should do things right. We know, and we know (again from a vast body of experience) that relying on boot loaders is plain idiotic and totally unworkable. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: How to handle named resources with DT?
On Tue, Aug 09, 2011 at 11:23:20AM +0200, Cousson, Benoit wrote: Hi Grant, Trying to bind hwmod informations with DT, I'm facing a little limitation. A bunch of drivers are using the platform_get_resource_byname, so the name for the resource is needed. The name is used so far for IORESOURCE_MEM, IORESOURCE_IRQ and IORESOURCE_DMA types of resources. Do you have any plan to add that, or do you have any clean way to avoid that without having to use the resource index? Named resources are evil. You only have to look at /proc/iomem to see why that's the case. Here's an example: b7a01000-b7a01003 : set b7a01000-b7a01003 : set b7a01004-b7a01007 : dat b7a01004-b7a01007 : dat b7a01008-b7a0100b : dirout b7a01008-b7a0100b : dirout b7a01020-b7a01023 : set b7a01020-b7a01023 : set b7a01024-b7a01027 : dat b7a01024-b7a01027 : dat b7a01028-b7a0102b : dirout b7a01028-b7a0102b : dirout b7a01040-b7a01043 : set b7a01040-b7a01043 : set b7a01044-b7a01047 : dat b7a01044-b7a01047 : dat b7a01048-b7a0104b : dirout b7a01048-b7a0104b : dirout b7a01060-b7a01063 : set b7a01060-b7a01063 : set b7a01064-b7a01067 : dat b7a01064-b7a01067 : dat b7a01068-b7a0106b : dirout b7a01068-b7a0106b : dirout b7a01080-b7a01083 : set b7a01080-b7a01083 : set b7a01084-b7a01087 : dat b7a01084-b7a01087 : dat b7a01088-b7a0108b : dirout b7a01088-b7a0108b : dirout How very informative. Not. So what device owns those? Who knows, that's lost by the crappy named resource stuff. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: Subject: L2x0 OF properties do not include interrupt #
On Thu, Aug 11, 2011 at 11:06:23AM -0500, Rob Herring wrote: No, either you have 1 interrupt and it is the combined one. or you have the 9? separate interrupts. Having both combined and separate hooked up is a bit dumb, so I would not worry about that case. I would just define the event counter interrupt 1st as that is probably the primary use. Also, I think that was the only interrupt on the L2x0 controllers IIRC. It's also conceivable that some of the interrupts get routed somewhere else rather than just into the GIC. A lot of other primecells have the one-combined and individual interrupts. So far, in the last 10 years, no one's used the individual interrupts in Linux (or even afaik wired them up.) Not with either the original ARM primecells or the vendor modified ones. That tells us something about the individual interrupts. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: How to handle named resources with DT?
On Thu, Aug 25, 2011 at 02:16:14AM +0300, Felipe Balbi wrote: on top of all that, for IPs which are used on many SoCs (such as MUSB) it's quite silly to force all users to provide resources in a certain order. It sounds to me that this will be prone to error in many ways until everything is synced up and on the correct order. Ditching _byname is a very bad idea. I continue to disagree. The current _byname is an abonimation and hack to try to fix this problem. _byname should have been implemented differently - rather than overriding the resources name field (which is _normally_ specified to be the device or driver name), a new field should have been introduced in struct resource to carry the resource sub-name (which is really what it is.) That would have avoided making /proc/iomem completely illegible with multiple devices using this feature. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 4/6] DMA: PL330: Add device tree support
On Fri, Aug 26, 2011 at 08:16:11AM -0500, Rob Herring wrote: Thomas, On 08/26/2011 03:40 AM, Thomas Abraham wrote: + - arm,pl330-peri-reqs: number of actual peripheral requests connected to the +dma controller. Maximum value is 32. Perhaps could be a bitmask for sparsely populated requests. May not matter since phandles will define the connections. Can be optional and not present means 00 requests (mem-to-mem only). The number of peripheral requests is readable from configuration register zero, so this is discoverable. Why should we put this information into DT if its provided by the hardware? The number of DMA channels available is also configurable by the SoC designer, yet you don't specify that in DT. And there's a whole bunch of other configuration options available to the SoC designer, most of which are discoverable from the configuration registers. So, I don't think you should be specifying the number of requests. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: How to handle named resources with DT?
On Sun, Aug 28, 2011 at 05:06:43PM -0600, Paul Walmsley wrote: DMA resource data are usually DMA request line ID numbers. Not always. On ARMs development platforms, we ended up using strings - because we've ended up with a DMA controller with N request signals, where the first three request signals come from a MUX which assigns them to other devices in the system. An opaque number doesn't hack it because you start having to define arbitary ranges for such things, and that very quickly starts getting you into sticky problems when you have to work out that request signal S1 is routed through mux M1 which is connected to DMA request signal D1, and if M1 is in use, then maybe M2 can be used instead which routes to DMA request signal D2. The idea that DMA should be small integers is very limiting. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 6/7] OMAP3: board-dt: Add generic board file for DT support
On Fri, Sep 02, 2011 at 10:46:56AM +0200, Cousson, Benoit wrote: Now, that kernel.org is back, I'll pull you branches :-). Umm. Are you sure? What are you checking - that some kernel.org servers are reachable or that master.kernel.org is reachable (it isn't.) ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 5/5] ARM: gic: add OF based initialization
On Thu, Sep 15, 2011 at 12:07:25PM +0200, Cousson, Benoit wrote: On OMAP4 the SoC interrupts external to the MPU (SPI) have an offset of 32. Only the internal PPI are between 0 and 31. SGIs are 0 to 15, PPIs are 16 to 31, and SPIs are 32+ - that's the numbering given to us by the GIC. The real HW physical number start at 0, and thus this is that value that should be in the irq binding of the device. That depends whether you're counting SPI number or whether you're counting IRQ number in the GIC interfaces. SPI0 will be reported to us from the GIC as 32, not 0, so to start numbering from 0 (which is already frowned upon for many reasons) we'd have to subtract 32 after checking that the IRQ is not a SGI nor PPI in the assembly code instead. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 5/5] ARM: gic: add OF based initialization
On Thu, Sep 15, 2011 at 02:28:06PM +0200, Cousson, Benoit wrote: The HW specs is obviously counting the IRQ number at the GIC interface. That offset is not known outside the MPUSS. Please have a look at the OMAP4430 TRM p4761 (NDA vM version). As far as I know, I have no access to that. I've certainly never been pointed to any documentation on OMAP4. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] pata-generic/of: Make probing via device tree non-powerpc-specific
On Fri, Sep 16, 2011 at 03:38:10PM +0100, Dave Martin wrote: This patch enables device-tree-based probing of the pata-generic platform driver across all architectures: * make the pata_of_generic module depend on OF instead of PPC_OF; * supply some missing inclues; * replace endianness-sensitive raw access to device tree data with of_property_read_u32() calls. Signed-off-by: Dave Martin dave.mar...@linaro.org --- Tested on ARM Versatile Express, with my soon-to-be-posted device tree support patches. This may not be the correct way to support the CF slot on Versatile Express - it depends whether the CF slot on VE supports just CF memory cards or whether it can take any CF card. If the latter, then what may be inserted could be a CF network card, and that means it's probably wrong to tell the kernel that what's there is a PATA interface. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] arm/dt: Add SoC detection macros
On Fri, Sep 09, 2011 at 01:02:19AM -0700, Allen Martin wrote: +#ifdef CONFIG_ARCH_TEGRA_2x_SOC +# ifdef SOC_NAME +# undef MULTI_SOC +# define MULTI_SOC +# else +# define SOC_NAME tegra2 +# endif +#endif +#ifdef CONFIG_ARCH_TEGRA_3x_SOC +# ifdef SOC_NAME +# undef MULTI_SOC +# define MULTI_SOC +# else +# define SOC_NAME tegra3 +# endif +#endif + +#define soc_is_tegra2() 0 +#define soc_is_tegra3() 0 + +#if defined(MULTI_SOC) +# if defined(CONFIG_ARCH_TEGRA_2x_SOC) +# undef soc_is_tegra2 +# define soc_is_tegra2()is_tegra2() +# endif +# if defined(CONFIG_ARCH_TEGRA_3x_SOC) +# undef soc_is_tegra3 +# define soc_is_tegra3()is_tegra3() +# endif +#else /* non-multi, only one architecture is on */ +# if defined(CONFIG_ARCH_TEGRA_2x_SOC) +# undef soc_is_tegra2 +# define soc_is_tegra2()1 +# elif defined(CONFIG_ARCH_TEGRA_3x_SOC) +# undef soc_is_tegra3 +# define soc_is_tegra3()1 +# endif +#endif This is not the way to do this, especially for a file in asm/*.h. Look at the way machine_is_xxx() is dealt with in include/generated/mach-types.h. #define MULTI_SOC 0 #undef SOC_SELECTED #ifdef CONFIG_ARCH_TEGRA_2x_SOC # ifdef SOC_SELECTED # undef MULTI_SOC # define MULTI_SOC 1 # else # define SOC_SELECTED # endif # define soc_is_tegra2()(!MULTI_SOC || is_tegra2()) #else # define soc_is_tegra2()0 #endif #ifdef CONFIG_ARCH_TEGRA_3x_SOC # ifdef SOC_SELECTED # undef MULTI_SOC # define MULTI_SOC 1 # else # define SOC_SELECTED # endif # define soc_is_tegra3()(!MULTI_SOC || is_tegra3()) #else # define soc_is_tegra3()0 #endif #undef SOC_SELECTED The above is nicely extensible - if other SoCs need to extend this they just need to add another outer ifdef..endif section to the file. + +enum soc_version { + SOC_UNKNOWN = 0, + TEGRA_T20, + TEGRA_T30, I'd suggest prefixing these with SOC_ to avoid any namespace problems. +}; + +void soc_init_version(void); +enum soc_version soc_get_version(void); + +static inline int is_tegra2(void) +{ + return soc_get_version() == TEGRA_T20; +} + +static inline int is_tegra3(void) +{ + return soc_get_version() == TEGRA_T30; +} If we require all SoCs to provide a value in soc_version, then we can use exactly the same method as mach-types.h uses - and while at this, please get rid of soc_get_version(). It's far cheaper to access the variable directly rather than indirect through a function, just like we do with __machine_arch_type. Mark it __read_mostly too. One last point to raise here is - and it's quite a fundamental one - do we really want this? If we're making decisions based on the SoC type, that suggests to me that the hardware description in DT is incomplete, and we're hiding stuff in the kernel behind the SoC type. That doesn't sound particularly appealing given the point of DT is to encode the hardware specific information outside the kernel code. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] arm/dt: Add SoC detection macros
On Sat, Sep 17, 2011 at 12:34:57PM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote: On 11:28 Sat 17 Sep , Russell King - ARM Linux wrote: One last point to raise here is - and it's quite a fundamental one - do we really want this? If we're making decisions based on the SoC type, that suggests to me that the hardware description in DT is incomplete, and we're hiding stuff in the kernel behind the SoC type. That doesn't sound particularly appealing given the point of DT is to encode the hardware specific information outside the kernel code. except if a machine can run on 2 soc so detect it will avoid to have 2 Device Tree This code is structured to match the SoC based upon an entry in the DT, so for tegra2 vs tegra3 it's already having to have two different DTs to distinguish between them. However, I still go back to my original point: the point of DT is to provide a description of the hardware which the kernel is running on - not only for current hardware but possibly future hardware as well. Eg, if Tegra4 comes along with more peripherals than Tegra3 but has basic hardware which the kernel already supports, just wired up differently, then Tegra4 should just work with a new DT file and no code changes. What I'm saying is that in that scenario it should not be necessary to edit the kernel to invent new SoC types, and then teach it that Tegra4 is mostly the same as Tegra3. That information should all be encoded into the DT rather than the C code in the kernel. So, I think adding this SoC type stuff is the wrong approach to the problem. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 4/5] ARM: gic: allow irq_start to be 0
On Wed, Sep 14, 2011 at 11:31:39AM -0500, Rob Herring wrote: From: Rob Herring rob.herr...@calxeda.com There's really no need to set irq_start per platform for the primary gic. The SGIs and PPIs are not handled as normal irqs, so how irqs 0-31 are setup doesn't really matter. So allow irq_start to be set to 0 to match the linux irq numbering. That's not correct. The hardware starts numbering SPI IRQs from 32 when reading the INTACK register. The number which gets passed through into asm_do_IRQ() will therefore be from 32 and above. There's several reasons for this: 1. To avoid IRQ0, which is commonly used to indicate 'no interrupt' to drivers. 2. To avoid the ISA IRQ range 1-15 which are hard-coded into various drivers (and we want those to fail.) 3. It's wasteful and pointless to manipulate the IRQ number given that we have sparse irq support. Also, bear in mind that gic_irq(SPI0) needs to return 32 to hit the right registers - so you'd have to set gic-irq_start to -32 to make SPI0 = IRQ0 work. Finally, the valid range for irq_start is 16 to 32 + the Linux IRQ base for the first GIC (SGI) interrupt. We should probably make gic_init() BUG() if its passed values less than 16. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 5/5] ARM: gic: add OF based initialization
On Mon, Sep 19, 2011 at 11:47:18AM +0200, Cousson, Benoit wrote: Since the cpumask is not relevant for the SPI, maybe having two interrupt controllers will be more relevant. Or maybe 3, since there is some SGIs as well. I don't think anyone uses SGIs outside of the common SMP code. Therefore they're handled entirely separately for the root GIC. (If there's two GICs - some platforms do have two - then the SGIs on the non-root GIC are unused.) ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 5/5] ARM: gic: add OF based initialization
On Mon, Sep 19, 2011 at 02:09:46PM +0200, Cousson, Benoit wrote: Every CortexA9 based SoC have to add the 32 offset to the SoC level interrupt number line. The ID numbering scheme is relevant only inside the GIC, but at SoC level only the IRQ lines that entered the MP core are relevant. That ID is a pure internal GIC encoding. As far as SPIs go, I think what should be done is that the DT should refer to a SPI phandle plus the SPI number, and be done with just that. The point as to whether SoCs internally use SPIs themselves is a complete distraction - if they're using SPIs internally then we _also_ need some way for the on-SoC peripherals to refer to them too. What the GIC exports are 16 PPIs per CPU, 16 SGIs and N SPIs. That's what we should be modelling for the GIC, not something else. So peripherals connect to an SPI numbered N where N = 0. How we want SPIs to map to Linux IRQ numbers is the issue, and as things stand at present, we want SPI0 to map to IRQ32 on all platforms where the GIC is the root, to avoid any unnecessary complexity (because the hardware tells us that SPI0 gives us ID32 in the interrupt acknowledge register.) Doing anything else requires computation or a lookup table, and we shouldn't be doing that kind of thing unless there's a real reason to do so (there isn't, especially with sparse irq support.) As far as PPIs go, support for that is still being worked on, and most of that at present does not go through genirq stuff (and it isn't relevant to use the standard genirq interfaces for PPIs _anyway_.) SGIs don't use genirq in any way, and are used for SMP IPIs. That's completely separate from the way IRQs are used - they're not connected to devices at all. (They're provided as an inter-processor communication method.) So forget SGIs. They may apparantly occupy IRQ IDs 0-15, but reality is they leave those IDs unused, IRQs 0-15 are not requestable, which is a definite *good* thing. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH V2] AT91: dt: at91sam9g45 family and board device tree files
On Mon, Oct 03, 2011 at 12:00:56PM +0200, Nicolas Ferre wrote: diff --git a/arch/arm/mach-at91/board-dt.c b/arch/arm/mach-at91/board-dt.c new file mode 100644 index 000..7bcb9a9 --- /dev/null +++ b/arch/arm/mach-at91/board-dt.c @@ -0,0 +1,122 @@ +/* + * Setup code for AT91SAM Evaluation Kits with Device Tree support + * + * Covers: * AT91SAM9G45-EKES board + * * AT91SAM9M10-EKES board + * * AT91SAM9M10G45-EK board + * + * Copyright (C) 2011 Atmel, + *2011 Nicolas Ferre nicolas.fe...@atmel.com + * + * Licensed under GPLv2 or later. + */ + +#include linux/types.h +#include linux/init.h +#include linux/module.h +#include linux/irqdomain.h +#include linux/of_irq.h +#include linux/of_platform.h + +#include mach/hardware.h +#include mach/board.h +#include mach/gpio.h Is linux/gpio.h broken for you? ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 2/9] regulator: helper routine to extract regulator_init_data
On Tue, Sep 27, 2011 at 01:10:04PM +0100, Mark Brown wrote: On Tue, Sep 27, 2011 at 03:42:45PM +0530, Rajendra Nayak wrote: + init_data = devm_kzalloc(dev, sizeof(struct regulator_init_data), +GFP_KERNEL); + if (!init_data) + return NULL; /* Out of memory? */ This means that the init data will be kept around for the entire lifetime of the device rather than being discarded. May I remind you that devm_* lifetime expires whenever the associated driver is unbound, which can be much shorter than the lifetime of the struct device. It expires when any of the following occurs: 1. userspace asks the associated driver to be unbound 2. the driver is removed 3. any driver probe for this struct device fails 4. the struct device is unregistered. So: don't use devm_* for anything other than stuff inside a driver being associated with the struct device itself. Other uses are a bug waiting to happen. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 1/2] arm: samsung: move timer irq numbers to end of linux irq space
On Fri, Oct 07, 2011 at 02:41:42AM +0530, Thomas Abraham wrote: All of Samsung's s5p platforms have timer irqs statically mapped from linux irq numbers 11 to 15. These timer irqs are moved to end of the statically mapped linux irq space and the hardware irqs, which were statically mapped starting from 32 is moved to start from 0. The NR_IRQS macro is consolidated for all the s5p platforms in this process. Am I reading this patch correctly - in that on platforms with the GIC, you add 32 to the GIC IRQ number _just_ to avoid the possibility that IRQs 11-15 are seen as valid? Or to put it another way, you're trying to ensure that if these timer IRQs are requested, they will fail? ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCHv2 02/10] ARM: vic: MULTI_IRQ_HANDLER handler
On Thu, Nov 03, 2011 at 01:29:08PM +0100, Linus Walleij wrote: No, if we receive another IRQ *after* the read of the register was the question, right? Just replace stat = ~(1 irq); with a second stat = readl_relaxed(vic-base + VIC_IRQ_STATUS); It'll work just fine, the IRQ line should be low when you read it the second time, else it is probably fully proper to call the IRQ handler again anyway. It depends on what kind of behaviour you want. There are two solutions: stat = readl_relaxed(vic-base + VIC_IRQ_STATUS); while (stat) { irq = ffs(stat) - 1; handle_irq(irq); stat = readl_relaxed(vic-base + VIC_IRQ_STATUS); } This gives priority to the lowest numbered interrupts; if these get stuck then they can exclude higher numbered interrupts. This is what we implement in the assembly code versions, and as far as I know, no one has ever complained about that behaviour. stat = readl_relaxed(vic-base + VIC_IRQ_STATUS); while (stat) { while (stat) { irq = ffs(stat) - 1; stat = ~(1 irq); handle_irq(irq); } stat = readl_relaxed(vic-base + VIC_IRQ_STATUS); } This ensures that we process all interrupts found pending before we re-check for any new interrupts pending. Arguably this is a much fairer implementation (and may mean if things get irrevokably stuck, things like sysrq via the console uart may still work.) ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCHv2 02/10] ARM: vic: MULTI_IRQ_HANDLER handler
On Thu, Nov 03, 2011 at 02:00:15PM +0100, Linus Walleij wrote: On Thu, Nov 3, 2011 at 1:51 PM, Russell King - ARM Linux li...@arm.linux.org.uk wrote: stat = readl_relaxed(vic-base + VIC_IRQ_STATUS); while (stat) { while (stat) { irq = ffs(stat) - 1; stat = ~(1 irq); handle_irq(irq); } stat = readl_relaxed(vic-base + VIC_IRQ_STATUS); } This ensures that we process all interrupts found pending before we re-check for any new interrupts pending. Arguably this is a much fairer implementation (and may mean if things get irrevokably stuck, things like sysrq via the console uart may still work.) I really like the looks of this, Jamie can you do it like that? Maybe some smallish comment about what's going on can be good for future generations reading that code... Bear in mind that it gets a little more complex when you have more than one VIC, because the outer loop should be across all VICs. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCHv2 02/10] ARM: vic: MULTI_IRQ_HANDLER handler
On Thu, Nov 03, 2011 at 03:03:37PM +, Jamie Iles wrote: On Thu, Nov 03, 2011 at 01:31:02PM +, Russell King - ARM Linux wrote: On Thu, Nov 03, 2011 at 02:00:15PM +0100, Linus Walleij wrote: On Thu, Nov 3, 2011 at 1:51 PM, Russell King - ARM Linux li...@arm.linux.org.uk wrote: stat = readl_relaxed(vic-base + VIC_IRQ_STATUS); while (stat) { while (stat) { irq = ffs(stat) - 1; stat = ~(1 irq); handle_irq(irq); } stat = readl_relaxed(vic-base + VIC_IRQ_STATUS); } This ensures that we process all interrupts found pending before we re-check for any new interrupts pending. Arguably this is a much fairer implementation (and may mean if things get irrevokably stuck, things like sysrq via the console uart may still work.) I really like the looks of this, Jamie can you do it like that? Maybe some smallish comment about what's going on can be good for future generations reading that code... Bear in mind that it gets a little more complex when you have more than one VIC, because the outer loop should be across all VICs. OK, so I think what I posted yesterday does that (updated for slightly better naming) and with a description. In the spirit of fairness iterating over the VIC's this way seemed right to me. Yes. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 2/5] ARM: vexpress: Remove platform SMP functions from ct_desc
On Fri, Nov 11, 2011 at 06:27:03PM +, Pawel Moll wrote: This patch removes platform SMP callbacks from ct_desc struct and replaces them with global symbols in preparation for DT-based support code. Will and myself discussed how to do this, and we came up with the ct_desc solution. Now you're doing something different. It seems to me like there's a disconnect between various different parts of ARM Ltd between people who have different ideas about how problems are to be solved. So, what's the technical reason for this change? I can't see how this improves anything. In fact, this patch reintroduces a bug which have been previously fixed: +static void ct_ca9x4_init_cpu_map(void) +{ + int i, ncores; + ncores = scu_get_core_count(V2T_PERIPH_P2V(A9_MPCORE_SCU)); + + for (i = 0; i ncores; ++i) + set_cpu_possible(i, true); + + set_smp_cross_call(gic_raise_softirq); +} vs -static void ct_ca9x4_init_cpu_map(void) -{ - int i, ncores = scu_get_core_count(V2TILE_PERIPH_P2V(A9_MPCORE_SCU)); - - if (ncores nr_cpu_ids) { - pr_warn(SMP: %u cores greater than maximum (%u), clipping\n, - ncores, nr_cpu_ids); - ncores = nr_cpu_ids; - } - - for (i = 0; i ncores; ++i) - set_cpu_possible(i, true); - - set_smp_cross_call(gic_raise_softirq); -} When you rebase, please pay better attention to the conflicts. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 1/5] ARM: vexpress: Get rid of MMIO_P2V
On Fri, Nov 11, 2011 at 06:27:02PM +, Pawel Moll wrote: @@ -17,3 +14,12 @@ struct amba_device name##_device = { \ .irq= IRQ_##base, \ /* .dma = DMA_##base,*/ \ } + +/* 2MB large area for motherboard's peripherals static mapping */ +#define V2M_PERIPH 0xf800 +#define V2M_PERIPH_P2V(offset) ((void __iomem *)(V2M_PERIPH | (offset))) + +/* Tile's peripherals static mappings should start here */ +#define V2T_PERIPH 0xf820 +#define V2T_PERIPH_P2V(offset) ((void __iomem *)(V2T_PERIPH | (offset))) + Please get rid of these blank lines at the end of files. diff --git a/arch/arm/mach-vexpress/v2m.c b/arch/arm/mach-vexpress/v2m.c index 1fafc32..b84fa45 100644 --- a/arch/arm/mach-vexpress/v2m.c +++ b/arch/arm/mach-vexpress/v2m.c @@ -39,29 +39,41 @@ static struct map_desc v2m_io_desc[] __initdata = { { - .virtual= __MMIO_P2V(V2M_PA_CS7), + .virtual= V2M_PERIPH, .pfn= __phys_to_pfn(V2M_PA_CS7), .length = SZ_128K, .type = MT_DEVICE, }, }; +static void __iomem *v2m_sysreg_base; + + + More useless blank lines. static void __init v2m_timer_init(void) { + void *sysctl_base; + void *timer01_base; Do you not use sparse? __iomem. + unsigned int timer01_irq; u32 scctrl; + sysctl_base = ioremap(V2M_SYSCTL, SZ_4K); + BUG_ON(!sysctl_base); + timer01_base = ioremap(V2M_TIMER01, SZ_4K); + BUG_ON(!timer01_base); + timer01_irq = IRQ_V2M_TIMER0; What's going on with the indentation here? @@ -413,6 +431,10 @@ static void __init v2m_populate_ct_desc(void) static void __init v2m_map_io(void) { iotable_init(v2m_io_desc, ARRAY_SIZE(v2m_io_desc)); + + /* Will become an ioremap() when possible */ + v2m_sysreg_base = V2M_PERIPH_P2V(V2M_SYSREGS); It won't if it stays here. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 3/5] ARM: vexpress: Add DT support in v2m
On Wed, Nov 16, 2011 at 05:07:51PM +, Pawel Moll wrote: On Wed, 2011-11-16 at 16:59 +, Rob Herring wrote: It has nothing to do with taste and obviously documentation changes over time. I'm going to start naming everything with legacy because someday it all will be... It's about how you create compatible strings. They should not be generic, but specific to particular hardware version. If you happen to be compatible with older h/w then you can claim compatibility with that older h/w. Notice that it's not: compatible=legacy not even: compatible=arm,legacy It's: compatible=arm,vexpress-legacy A specific variant of Versatile Express hardware. It's just that the legacy word carries some meaning. Would it looked better if it was called: compatible=arm,vexpress-nalatenskap ? (thanks, google translate ;-) You're totally failing to understand the point. The point is that when Versatile Express first came out, it had a memory map. At this point in time, there was nothing 'legacy' about it, it was the latest and greatest thing. If DT support was created at this point, it would have been called something without 'legacy' in its name. A year or so on, it's been superseded by something else, and is now called legacy. The way DT works, you don't go around renaming stuff because it's become legacy. So, the original DT support still stands using whatever named properties would have been invented at the time. So, when we create support for it *now*, we should not be creating a DT file with properties named as 'legacy'. The fact that it has become legacy _today_ is irrelevant to DT. If we follow through the argument that we _should_ name things legacy, then it follows that we need to constantly patch DT files to rename stuff to 'legacy' when it's no longer the latest and greatest - and then you need the right DT file corresponding to the right kernel version. Again, this is _not_ how DT is supposed to work. DT is about describing the hardware, not describing the state of something. A DT description of a hardware system created 10 years ago should still work _today_ even though the hardware system may now be 'legacy'. So, get rid of that 'legacy' crap in DT naming. It doesn't belong. Or we'll start talling the Cortex-A5 stuff 'legacy' right now as well. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 3/5] ARM: vexpress: Add DT support in v2m
On Fri, Nov 11, 2011 at 06:27:04PM +, Pawel Moll wrote: +#if defined(CONFIG_ARCH_VEXPRESS_DT) + int err; + const char *path; + struct device_node *node; + + node = of_find_compatible_node(NULL, NULL, arm,sp810); + BUG_ON(!node); + sysctl_base = of_iomap(node, 0); + BUG_ON(!sysctl_base); + + err = of_property_read_string(of_aliases, timer, path); + BUG_ON(err); + node = of_find_node_by_path(path); + BUG_ON(!node); + timer01_base = of_iomap(node, 0); + BUG_ON(!timer01_base); + timer01_irq = irq_of_parse_and_map(node, 0); Are you sure you have enough BUG_ON()s there? It's well worth reading Linus' various messages on the use of BUG(), which can be found: http://yarchive.net/comp/linux/BUG.html The lack of the timer and sysctl registers are really a 'report and maybe we could get a message out' kind of scenario rather than a 'oops, kill the machine'. +#endif + } else { sysctl_base = ioremap(V2M_SYSCTL, SZ_4K); BUG_ON(!sysctl_base); timer01_base = ioremap(V2M_TIMER01, SZ_4K); BUG_ON(!timer01_base); timer01_irq = IRQ_V2M_TIMER0; + } And in any case, both these paths have a common set of BUG_ON()s: BUG_ON(!sysctl_base); BUG_ON(!timer01_base); So do we really need them having this independently? @@ -383,11 +412,18 @@ static struct clk_lookup v2m_lookups[] = { }, }; +static void __init v2m_system_id(void) +{ + if (!system_rev) + system_rev = readl(v2m_sysreg_base + V2M_SYS_ID); You do understand that system_rev is for the system _revision_ not for some kind of system ID. For example, it's to identify whether we're on a revision 4, 5 or 6 system. However, with DT the differences in system revision should be encoded into the DT itself, and the kernel should not be making choices about the hardware off this. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 04/10] arm/tegra: prepare early init for multiple tegra variants
On Thu, Nov 17, 2011 at 06:19:18PM +0200, Peter De Schrijver wrote: This patch splits the early init code in a common and a tegra20 specific part. L2 cache initialization is generalized and discovers the cache associativity at runtime. Also use arm_pm_restart instead of arm_arch_reset and reset the the system using the PMC reset feature rather then the CAR system reset. I'm already carrying this, and the follow-on patch to remove the arch_reset() from tegra entirely. 8=== ARM: restart: tegra: use new restart hook Hook these platforms restart code into the arm_pm_restart hook rather than using arch_reset(). Signed-off-by: Russell King rmk+ker...@arm.linux.org.uk --- arch/arm/mach-tegra/common.c |5 ++--- arch/arm/mach-tegra/include/mach/system.h |6 +++--- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/arch/arm/mach-tegra/common.c b/arch/arm/mach-tegra/common.c index 690b888..1374d10 100644 --- a/arch/arm/mach-tegra/common.c +++ b/arch/arm/mach-tegra/common.c @@ -31,9 +31,7 @@ #include clock.h #include fuse.h -void (*arch_reset)(char mode, const char *cmd) = tegra_assert_system_reset; - -void tegra_assert_system_reset(char mode, const char *cmd) +static void tegra_assert_system_reset(char mode, const char *cmd) { void __iomem *reset = IO_ADDRESS(TEGRA_CLK_RESET_BASE + 0x04); u32 reg; @@ -76,6 +74,7 @@ static void __init tegra_init_cache(void) void __init tegra_init_early(void) { + arm_pm_restart = tegra_assert_system_reset; tegra_init_fuse(); tegra_init_clock(); tegra_clk_init_from_table(common_clk_init_table); diff --git a/arch/arm/mach-tegra/include/mach/system.h b/arch/arm/mach-tegra/include/mach/system.h index 027c421..b87b8a4 100644 --- a/arch/arm/mach-tegra/include/mach/system.h +++ b/arch/arm/mach-tegra/include/mach/system.h @@ -21,9 +21,9 @@ #ifndef __MACH_TEGRA_SYSTEM_H #define __MACH_TEGRA_SYSTEM_H -#include mach/iomap.h - -extern void (*arch_reset)(char mode, const char *cmd); +static inline void arch_reset(char mode, const char *cmd) +{ +} static inline void arch_idle(void) { -- 1.7.4.4 ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 1/5] ARM: vexpress: Get rid of MMIO_P2V
On Fri, Nov 18, 2011 at 12:20:48PM +, Pawel Moll wrote: On Thu, 2011-11-17 at 15:43 +, Russell King - ARM Linux wrote: +/* Tile's peripherals static mappings should start here */ +#define V2T_PERIPH 0xf820 +#define V2T_PERIPH_P2V(offset) ((void __iomem *)(V2T_PERIPH | (offset))) + Please get rid of these blank lines at the end of files. Patch splitting leftover. Will fix. +static void __iomem *v2m_sysreg_base; + + + More useless blank lines. Ok, will change that. I just like the way that the data are visually separated from the code like here: ceade897 (Russell King 2010-02-11 21:44:53 + 68) .init = v2m_timer_init, ceade897 (Russell King 2010-02-11 21:44:53 + 69) }; ceade897 (Russell King 2010-02-11 21:44:53 + 70) ceade897 (Russell King 2010-02-11 21:44:53 + 71) ceade897 (Russell King 2010-02-11 21:44:53 + 72) static DEFINE_SPINLOCK(v2m_cfg_lock); or here: ceade897 (Russell King 2010-02-11 21:44:53 + 289) rtc_device, ceade897 (Russell King 2010-02-11 21:44:53 + 290) }; ceade897 (Russell King 2010-02-11 21:44:53 + 291) ceade897 (Russell King 2010-02-11 21:44:53 + 292) ceade897 (Russell King 2010-02-11 21:44:53 + 293) static long v2m_osc_round(struct clk *clk, unsigned long rate) Note the difference in the number. Two is acceptable to split sections of code. Three is getting excessive. static void __init v2m_timer_init(void) { + void *sysctl_base; + void *timer01_base; Do you not use sparse? __iomem. Ok. + unsigned int timer01_irq; u32 scctrl; + sysctl_base = ioremap(V2M_SYSCTL, SZ_4K); + BUG_ON(!sysctl_base); + timer01_base = ioremap(V2M_TIMER01, SZ_4K); + BUG_ON(!timer01_base); + timer01_irq = IRQ_V2M_TIMER0; What's going on with the indentation here? Patch-splitting artefact again, will fix. @@ -413,6 +431,10 @@ static void __init v2m_populate_ct_desc(void) static void __init v2m_map_io(void) { iotable_init(v2m_io_desc, ARRAY_SIZE(v2m_io_desc)); + + /* Will become an ioremap() when possible */ + v2m_sysreg_base = V2M_PERIPH_P2V(V2M_SYSREGS); It won't if it stays here. Excuse my inferior English language capabilities, but what do you suggest here? What I'm saying is your comment is wrong. ioremap() will never be possible in the map_io function. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 3/5] ARM: vexpress: Add DT support in v2m
On Fri, Nov 18, 2011 at 12:20:49PM +, Pawel Moll wrote: On Thu, 2011-11-17 at 15:53 +, Russell King - ARM Linux wrote: Again, this is _not_ how DT is supposed to work. DT is about describing the hardware, not describing the state of something. And that's exactly what I was trying to do. Describe the hardware, using terminology *used by the hardware designers* - fact that you have chosen to ignore, fair enough. Again, you're totally FAILING to get the point. Had DT support been created before the new version came out there would be no 'legacy' in terminology what so ever. _That's_ the whole point. What you're using is *todays* terminology, not the terminology used at the point of *creation*. Why can't you get it through your thick skull that at some day, the entire Cortex-A5 series of CPUs is going to be called 'legacy'. And at that point, will you be wanting to rename all the properties because of that change? No. That's insane. So why the fuck do it now to the CA9x5? Try thinking. It's good for the mind. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 3/5] ARM: vexpress: Add DT support in v2m
On Thu, Nov 17, 2011 at 06:37:26PM +, Dave Martin wrote: On Thu, Nov 17, 2011 at 04:05:37PM +, Russell King - ARM Linux wrote: You do understand that system_rev is for the system _revision_ not for some kind of system ID. For example, it's to identify whether we're on a revision 4, 5 or 6 system. However, with DT the differences in system revision should be encoded into the DT itself, and the kernel should not be making choices about the hardware off this. I can't comment on whether this is an abuse of system_rev, since I'm not too familiar with that. I feel that Whether the value of the V2M_SYS_ID register should be put in the DT is more doubtful though: the DT must describe the hardware which cannot be probed. That is exactly my point - but we don't want V2M_SYS_ID controlling what hardware is there because then you'll end up having _drivers_ having to be aware of the system revision. Instead, the differences in device IP - when they're not discoverable from the device itself - should be encoded in DT to allow the device IP to be reused on different platforms which may not even have a V2M_SYS_ID register. That's what my objection is about: nothing should be using the overall system revision to determine anything about it. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 4/5] ARM: vexpress: Initial RS1 memory map support
On Fri, Nov 18, 2011 at 12:20:50PM +, Pawel Moll wrote: On Thu, 2011-11-17 at 15:36 +, Russell King - ARM Linux wrote: You do know that this will probably cause uboot to load the uImage at 0x80008000 instead of 0x60008000, and therefore we'll lose half the RAM on this board? I missed that - I don't use uboot, so I don't build uImages. As the ARCH_VEXPRESS_LEGACY will be no more (see my other response) I'll revert the order, so the _RS1 will enforce P2V, AUTO_ZRELADDR and change the zrealaddr-y friends (as in: 0x6* when no _RS1, 0x8* when _RS1=y). Is there any other way of having single binary kernel booting from both 0x60008000 and 0x80008000? Not with uboot - uboot has been broken for many years in regard of being able to load kernel images inteligently - it has to be told explicitly where to load the image in system memory, so if the uImage is built for 0x80008000, uboot will load it at that address no matter if there's RAM there or not. That interacts with the P2V stuff and makes PHYS_OFFSET be 0x8000 rather than 0x6000 - and my guess is that it'll still pass in memory information describing the RAM at 0x6000 which may result in the kernel failing to boot. There's no way around this with uboot at the moment, and I don't expect the uboots in the field already on devices to be updated to solve this. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v2 1/4] ARM: vexpress: Get rid of MMIO_P2V
On Fri, Nov 25, 2011 at 04:15:00PM +, Dave Martin wrote: diff --git a/arch/arm/plat-versatile/headsmp.S b/arch/arm/plat-versatile/headsmp.S index d397a1f..0be2efc 100644 --- a/arch/arm/plat-versatile/headsmp.S +++ b/arch/arm/plat-versatile/headsmp.S @@ -28,6 +28,7 @@ ENTRY(versatile_secondary_startup) pen: ldr r7, [r6] cmp r7, r0 bne pen +ENDPROC(versatile_secondary_startup) This is wrong. You're telling the assembler that this function ends here. It doesn't, it continues on. Put the ENDPROC at the end of the function. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v3 4/5] ARM: vexpress: Initial RS1 memory map support
On Wed, Nov 30, 2011 at 04:38:26PM -0500, Nicolas Pitre wrote: On Wed, 30 Nov 2011, Mark Brown wrote: On Wed, Nov 30, 2011 at 03:43:50PM -0500, Nicolas Pitre wrote: annoying. If nothing moves I might just go ahead with those changes and simply rip the uImage make target out of the kernel as well. Maybe the inconvenience will be a sufficient incentive for people to lobby proper u-Boot support. And it is not like if the u-Boot patches didn't exist Oh, dear. Any pointers to the discussions on the u-boot side? Certainly. Many different threads actually. Here's a few: http://news.gmane.org/group/gmane.comp.boot-loaders.u-boot/thread=114981 http://news.gmane.org/group/gmane.comp.boot-loaders.u-boot/thread=115774 And this is not the first time this issue has come up: http://news.gmane.org/group/gmane.comp.boot-loaders.u-boot/thread=83824 http://news.gmane.org/group/gmane.linux.ports.arm.kernel/thread=81546/force_load=t/focus=84138 Yet more u-Boot woes: http://news.gmane.org/group/gmane.linux.ports.arm.kernel/thread=53973 I'm sure there are others, but that's what I've quickly dug out. So why are we still supporting uboot in the mainline kernel on ARM? ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v3 4/5] ARM: vexpress: Initial RS1 memory map support
On Thu, Dec 01, 2011 at 11:10:50AM +, Mark Brown wrote: On Wed, Nov 30, 2011 at 04:38:26PM -0500, Nicolas Pitre wrote: On Wed, 30 Nov 2011, Mark Brown wrote: Oh, dear. Any pointers to the discussions on the u-boot side? Certainly. Many different threads actually. Here's a few: OK, thanks - I see Stephen just followed up and Wolfgang seems moderately happy so hopefully there will be some progress. It also occurs to me that there's at least Qi also using uImages, hopefully other bootloaders are going to be easier to deal with (or already cope). Most boot loaders on ARM prior to uboot either allow the load address to be specified via the command script, or they already load the kernel to a fixed address into system RAM - using the kernels native zImage binary format. So these other boot loaders shouldn't be any problem what so ever. In fact they've already been working well with relocatable zImages for many many years. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] ata: Don't use NO_IRQ in pata_of_platform driver
On Tue, Dec 06, 2011 at 09:37:09AM +, Dave Martin wrote: To clarify, you're suggesting that the meanings of all other IRQ values would not change initially? (i.e., we remap HW irq 0, if there is one, to some other random number but have a 1:1 mapping for everything else). Even better. Avoid the first 16 IRQ numbers altogether - so that ISA drivers which have these numbers hard-encoded in them will see failures if they're expecting standard ISA IRQ numbering. We already do that with the GIC, partly because of the hardware design. We do that on Footbridge based systems, because they may or may not have a real ISA IRQ controller. But.. let's make one thing clear: Alan Cox and Linus have been going on about how IRQ0 should not be used. Let's be crystal clear: even x86 uses IRQ0. It happens to be the PIC timer, and that gets claimed early on during the x86 boot. So please don't tell me that x86 avoids IRQ0. It doesn't. It just happens that x86 never shows IRQ0 to anything but the i8253 PIC driver. So lets see how x86 squeels if we make the i8253 PIC driver reject IRQ0. I bet that there'd be absolute fury at such a suggestion. When x86 sorts this out, there _might_ be some more motivation to take such comments seriously. Until then it's more like a joke. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] ata: Don't use NO_IRQ in pata_of_platform driver
On Tue, Dec 06, 2011 at 09:30:00AM +, Dave Martin wrote: Do we expect there to be any platform drivers which are shared between legacy platforms and newer DT-ised platforms? Those drivers would be pain points since they would need to understand both conventions. So far as I can see, only boards which are not DT-ised, which do not use DT-ised drivers and which do not use drivers which use interrupts and are either used by DT-ised boards or by arches with a non-zero NO_IRQ could safely carry on using a non-zero NO_IRQ. Tracking down exactly which boards and drivers this applies to could be hard. We could have a CONFIG_NO_IRQ and make them depend on it, but we still have to find that list of boards and drivers in the first place. You're digging too deeply into this. Drivers which need to know whether an IRQ is valid need to know this if they wish to do something different for 'this device doesn't have an IRQ wired'. These are the drivers which have problems because of the -1 vs 0 thing. That is different from 'this is an invalid IRQ number', which is what you find out when you call request_irq(). So please, stop thinking 'we need to convert drivers to check for = 0'. We don't. We just need to make sure that we're not passing a zero IRQ number to any driver. On platforms where IRQ0 is special like x86, request_irq() will fail with -EBUSY on drivers which don't care (or other kind of refusal to initialize), and will cause 'polling mode' with the 8250 driver. So, all that we need to do is to ensure that all the IRQ chip stuff is fixed up so that IRQ0 is only used for the same purpose as x86 - the PIC timer on systems with an ISA 8253 timer. Everything else should not pass IRQ0 outside core platform code. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] ata: Don't use NO_IRQ in pata_of_platform driver
On Tue, Dec 06, 2011 at 12:00:12PM +0100, Geert Uytterhoeven wrote: On Tue, Dec 6, 2011 at 11:46, Russell King - ARM Linux li...@arm.linux.org.uk wrote: But.. let's make one thing clear: Alan Cox and Linus have been going on about how IRQ0 should not be used. Let's be crystal clear: even x86 uses IRQ0. It happens to be the PIC timer, and that gets claimed early on during the x86 boot. So please don't tell me that x86 avoids IRQ0. It doesn't. It just happens that x86 never shows IRQ0 to anything but the i8253 PIC driver. It's shown in /proc/interrupts due to a bug in show_interrupts(). The (gmail damaged) patch below fixes this bug. So we now try to hide the fact that there _is_ an interrupt called 0 on x86 systems? Sorry, I can't that that seriously in any way. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] ata: Don't use NO_IRQ in pata_of_platform driver
On Tue, Dec 06, 2011 at 11:05:54AM +, Alan Cox wrote: Even better. Avoid the first 16 IRQ numbers altogether - so that ISA drivers which have these numbers hard-encoded in them will see failures if they're expecting standard ISA IRQ numbering. The ISA bus space is non-discoverable so that doesn't make any sense. But.. let's make one thing clear: Alan Cox and Linus have been going on about how IRQ0 should not be used. Let's be crystal clear: even x86 uses IRQ0. It happens to be the PIC timer, and that gets claimed early on during the x86 boot. So please don't tell me that x86 avoids IRQ0. It doesn't. It just happens that x86 never shows IRQ0 to anything but the i8253 PIC driver. x86 has an internal invisible IRQ 0 on some platforms. It's never exposed beyond the arch code. So lets see how x86 squeels if we make the i8253 PIC driver reject IRQ0. I bet that there'd be absolute fury at such a suggestion. Actually it would be about ten minutes work to remap it to some other number that isn't used. It never goes via request_irq or via any core or driver layer code however. In the ARM case the same is going to be true. If you have IRQ 0 plumbing that only goes internally in the arch/arm code - eg an ARM with IRQ 0 wired to something totally arch specific and non-driver then it's not going to break and like the internals of x86 doesn't matter. When x86 sorts this out, there _might_ be some more motivation to take such comments seriously. Until then it's more like a joke. Pity you feel that way, but if ARM wants to continue to break more and more as we clean up NO_IRQ for everything else that's your privilege, but don't expect any sympathy. For the platforms I care about, it probably won't break. For those which do break, it's a matter of fixing their include/mach/irqs.h and the code in their irqchips to convert the IRQ number to the correct bitmask for the register. However, I have suggested in the past that new platforms _should_ avoid not just IRQ0 but IRQ0-15 (for a completely different reason to that of 'IRQ0 means no IRQ'.) But such comments just get ignored, so I just don't see the point in doing anything about this. If people experience breakage, so be it. I too will have little sympathy but not for the same reason. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] ata: Don't use NO_IRQ in pata_of_platform driver
On Tue, Dec 06, 2011 at 11:37:35AM +, Dave Martin wrote: 1) All OF code and drivers should be migrating to use 0 instead of NO_IRQ for the no-interrupt case. Code which receives irq numbers directly from the OF framework and refers to NO_IRQ, or expects 0 to be a valid needs to be fixed. 2) Where we hit a problem, board code needs to be adapted to remap HW IRQs 0-15 to different software values. (This could be done using irq domains, or not) No AMBA driver I'm aware of ever uses an IRQ number 0 or is passed such an IRQ number. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] ata: Don't use NO_IRQ in pata_of_platform driver
On Tue, Dec 06, 2011 at 11:20:49AM -0800, Linus Torvalds wrote: Not for any device driver, though. It's used entirely internally, and it doesn't even use request_irq(). It uses the magic internal setup_irq() and never *ever* exposes irq0 as anything that a driver can see. That's what matters. You can use irq0 in ARM land all you like, AS LONG AS IT'S SOME HIDDEN INTERNAL USE. No drivers. No *nothing* that ever uses that absolutely *idiotic* NO_IRQ crap. In fact, you may be *forced* to use what is physically irq0 - it's just that you should never expose it as such to drivers. And x86 doesn't. So Russell, if you think this has anything to do with NO_IRQ, and how x86 isn't doing things right, you're wrong. It's just like the internal exception thing, or the magical cascade interrupt, or the x87 exception mapped through the PIC. They are magic hidden interrupts that are set up in one place (well, one place *each*), and are never exposed anywhere else. The problem with NO_IRQ is that stupid we expose our mind-numbingly stupid interfaces across the whole kernel. x86 never did that. ARM still does. x86 doesn't have to fix anything. ARM does. Remember you said that I shouldn't take things personally? Well, this is one issue I really don't care about. I don't think any platform I _actually_ have will be impacted by any change in this area. Other platform maintainers may have their own issues but that's not _my_ problem. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v5 5/9] ARM: versatile: Map local timers using Device Tree when possible
On Fri, Dec 09, 2011 at 07:03:48PM +, Pawel Moll wrote: If twd_base is not set, try to map the TWD registers from arm,smp-twd Device Tree node (compatible value as used in Highbank's DT). Please do it the other way around - allow DT to override twd_base if the DT node is present. The propsed way gets in the way of the single zImage stuff which Linaro is working on. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v4 4/7] cpufreq: add clk-reg cpufreq driver
On Mon, Dec 26, 2011 at 09:44:52PM +0800, Richard Zhao wrote: On Mon, Dec 26, 2011 at 11:10:30AM +, Mark Brown wrote: The *call* is there in the regulator subsystem, it's just that none of the drivers back it up with an actual implementation yet. Which turns out to be a good thing as cpufreq can't currently understand variable latencies and the governors don't deal well with non-trivial latencies anyway. but clk API don't have such calls. and many SoCs only adjust clk frequencies, using one single voltage. That's because it's often not known - especially in the case of PLLs, data sheets don't tend to specify how long it takes for the PLL to relock after a requested change. If it's important that the PLL be locked, there will be a bit to poll (or they'll cause the CPU itself to stall while the PLL is not locked.) So, for these kinds of situations, how do you suggest that the clk API provides this information? ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v3] Input: keyboard - add device tree bindings for simple key matrixes
On Tue, Jan 03, 2012 at 08:22:21AM -0800, Simon Glass wrote: Can the Linux key codes fit in 8 bits? That depends on your point of view. If you hack on X, then the answer is yes and you ignore the squeels of your users when certain key presses get misinterpreted. (The Psion LX platform, otherwise known as the Netbook Pro, suffered with this problem.) If you are a kernel hacker, the answer is no, because key codes currently go all the way to 0x300. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v3] Input: keyboard - add device tree bindings for simple key matrixes
On Tue, Jan 03, 2012 at 08:44:32AM -0800, Dmitry Torokhov wrote: On Tue, Jan 03, 2012 at 04:29:30PM +, Russell King - ARM Linux wrote: On Tue, Jan 03, 2012 at 08:22:21AM -0800, Simon Glass wrote: Can the Linux key codes fit in 8 bits? That depends on your point of view. If you hack on X, then the answer is yes and you ignore the squeels of your users when certain key presses get misinterpreted. (The Psion LX platform, otherwise known as the Netbook Pro, suffered with this problem.) If you are a kernel hacker, the answer is no, because key codes currently go all the way to 0x300. For bootloader environment 0-255 range is probably sufficient though, the upper keys are somewhat recent additions to the maps... I assume you deem 'recent' to mean 8 years ago - they've been there since at least 2.6.9, which is where the problem I refer to above was first noticed. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v2 3/6] ARM: at91/gpio: add DT support
On Tue, Jan 03, 2012 at 07:34:48PM +0100, Nicolas Ferre wrote: + at91_gpio-clock = clk_get_sys(NULL, at91_gpio-chip.label); + if (!at91_gpio-clock) { + pr_err(at91_gpio.%d, failed to get clock, ignoring.\n, + alias_id); + goto ioremap_err; + } + + /* enable PIO controller's clock */ + if(clk_enable(at91_gpio-clock)) { + pr_err(at91_gpio.%d, failed to enable clock, ignoring.\n, + alias_id); + goto clk_err; + } No new code should be added to the kernel which uses clk_enable() without using clk_prepare() first. Ditto clk_disable() and clk_unprepare(). + at91_gpio-clock = clk_get_sys(NULL, at91_gpio-chip.label); + if (!at91_gpio-clock) { + pr_err(at91_gpio.%d, failed to get clock, ignoring.\n, i); + goto ioremap_err; + } + + if(clk_enable(at91_gpio-clock)) { + pr_err(at91_gpio.%d, failed to enable clock, ignoring.\n, i); + goto clk_err; + } I've seen this code somewhere before... Couldn't this be separated out into a helper function? ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC PATCH v2 1/9] arm: avoid using on_each_cpu hard coded ret value
On Sun, Jan 08, 2012 at 03:32:21PM +0200, Gilad Ben-Yossef wrote: on_each_cpu always returns a hard coded return code of zero. Removing all tests based on this return value saves run time cycles for compares and code bloat for branches. Signed-off-by: Gilad Ben-Yossef gi...@benyossef.com Acked-by: Peter Zijlstra a.p.zijls...@chello.nl Reviewed-by: Michal Nazarewicz min...@mina86.com CC: Will Deacon will.dea...@arm.com CC: Peter Zijlstra a.p.zijls...@chello.nl CC: Paul Mackerras pau...@samba.org CC: Ingo Molnar mi...@elte.hu CC: Arnaldo Carvalho de Melo a...@ghostprotocols.net CC: Russell King li...@arm.linux.org.uk Reviewed-by: Russell King rmk+ker...@arm.linux.org.uk CC: Grant Likely grant.lik...@secretlab.ca CC: Rob Herring rob.herr...@calxeda.com CC: linux-arm-ker...@lists.infradead.org CC: devicetree-discuss@lists.ozlabs.org --- arch/arm/kernel/perf_event.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/arch/arm/kernel/perf_event.c b/arch/arm/kernel/perf_event.c index 5bb91bf..6e9acb7 100644 --- a/arch/arm/kernel/perf_event.c +++ b/arch/arm/kernel/perf_event.c @@ -616,7 +616,7 @@ static int __init cpu_pmu_reset(void) { if (cpu_pmu cpu_pmu-reset) - return on_each_cpu(cpu_pmu-reset, NULL, 1); + on_each_cpu(cpu_pmu-reset, NULL, 1); There's not much to review here... Thanks. return 0; } arch_initcall(cpu_pmu_reset); -- 1.7.0.4 ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC PATCH v2 8/9] smp: refactor on_each_cpu to void returning func
On Sun, Jan 08, 2012 at 03:32:28PM +0200, Gilad Ben-Yossef wrote: on_each_cpu returns the retunr value of smp_call_function which is hard coded to 0. Refactor on_each_cpu to a void function and the few callers that check the return value to save compares and branches. Signed-off-by: Gilad Ben-Yossef gi...@benyossef.com Acked-by: Peter Zijlstra a.p.zijls...@chello.nl Reviewed-by: Michal Nazarewicz min...@mina86.com CC: David Airlie airl...@linux.ie CC: dri-de...@lists.freedesktop.org CC: Benjamin Herrenschmidt b...@kernel.crashing.org CC: Paul Mackerras pau...@samba.org CC: Grant Likely grant.lik...@secretlab.ca CC: Rob Herring rob.herr...@calxeda.com CC: linuxppc-...@lists.ozlabs.org CC: devicetree-discuss@lists.ozlabs.org CC: Richard Henderson r...@twiddle.net CC: Ivan Kokshaysky i...@jurassic.park.msu.ru CC: Matt Turner matts...@gmail.com CC: linux-al...@vger.kernel.org CC: Thomas Gleixner t...@linutronix.de CC: Ingo Molnar mi...@redhat.com CC: H. Peter Anvin h...@zytor.com CC: x...@kernel.org CC: Tony Luck tony.l...@intel.com CC: Fenghua Yu fenghua...@intel.com CC: linux-i...@vger.kernel.org CC: Will Deacon will.dea...@arm.com CC: Peter Zijlstra a.p.zijls...@chello.nl CC: Arnaldo Carvalho de Melo a...@ghostprotocols.net CC: Russell King li...@arm.linux.org.uk As there's only one place in the ARM code where we look at the return value, and you've patched that away in patch 1, this looks fine. I've not checked for users outside of arch/arm, so: Acked-by: Russell King rmk+ker...@arm.linux.org.uk Thanks. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 1/2] versatile: sic: add device tree bindings
On Fri, Jan 13, 2012 at 12:44:00AM +, Jamie Iles wrote: +#ifdef CONFIG_OF +int __init sic_of_init(struct device_node *np, struct device_node *parent) +{ + struct fpga_irq_data *sic_data = kzalloc(sizeof(*sic_data), GFP_KERNEL); + int err = -ENOMEM, irq; + + if (WARN_ON(!sic_data)) + return err; + + sic_data-base = of_iomap(np, 0); + if (WARN_ON(!sic_data-base)) + goto out_free_data; + + irq = irq_of_parse_and_map(np, 0); + if (irq 0) + goto out_free_data; + + sic_data-irq_start = irq_alloc_descs(-1, 0, 32, numa_node_id()); + if (WARN_ON(sic_data-irq_start 0)) { + err = sic_data-irq_start; + goto out_unmap; + } + sic_data-domain.of_node = of_node_get(np); + + fpga_irq_init(irq, ~0, sic_data); I notice that you completely ignore the validity mask, and initialize all 32 interrupts. This is wrong. You don't think I put the validity mask there just to obfuscate the code? And this shouldn't be called 'sic' at all. It may be the secondary interrupt controller on the Versatile, but it's also the PIC, SIC and CIC on Integrator platforms. Calling anything in this file (which is entirely separate of any platform) 'sic' is _wrong_. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 1/2] versatile: sic: add device tree bindings
On Fri, Jan 13, 2012 at 11:41:51AM -0700, Grant Likely wrote: On Fri, Jan 13, 2012 at 10:58:34AM +, Jamie Iles wrote: On Fri, Jan 13, 2012 at 10:48:42AM +, Russell King - ARM Linux wrote: On Fri, Jan 13, 2012 at 12:44:00AM +, Jamie Iles wrote: +#ifdef CONFIG_OF +int __init sic_of_init(struct device_node *np, struct device_node *parent) +{ + struct fpga_irq_data *sic_data = kzalloc(sizeof(*sic_data), GFP_KERNEL); + int err = -ENOMEM, irq; + + if (WARN_ON(!sic_data)) + return err; + + sic_data-base = of_iomap(np, 0); + if (WARN_ON(!sic_data-base)) + goto out_free_data; + + irq = irq_of_parse_and_map(np, 0); + if (irq 0) + goto out_free_data; + + sic_data-irq_start = irq_alloc_descs(-1, 0, 32, numa_node_id()); + if (WARN_ON(sic_data-irq_start 0)) { + err = sic_data-irq_start; + goto out_unmap; + } + sic_data-domain.of_node = of_node_get(np); + + fpga_irq_init(irq, ~0, sic_data); I notice that you completely ignore the validity mask, and initialize all 32 interrupts. This is wrong. You don't think I put the validity mask there just to obfuscate the code? OK, that is wrong. The validity mask is used for the non-DT platforms, for the DT one I'll add a property stating the valid mask: arm,valid-irq-mask = 0x001f; to the node in the trees. Shouldn't be necessary when the driver is converted to using the linear mapping. When that happens, only the requested irqs will get mapped. What I'm saying is that they shouldn't even be _published_ as potentially valid IRQs. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 1/2] versatile: sic: add device tree bindings
On Fri, Jan 13, 2012 at 02:00:41PM -0700, Grant Likely wrote: Let me look at it a bit. You can leave the initialization loop in fpga_irq_init() for now. I'm looking at adding a mask field to irq_domain_add_legacy in addition to the host_data pointer. I also want to investigate replacing fpga_irq with generic-chip.c. The problem I've currently got though is that irq_domain_add_legacy() can currently support any number of irqs, but adding a simple mask argument will limit it to 32. That won't work for large irq controllers like the gic. The easy answer is not to register the IRQs which don't actually exist or we don't want to be used out of the set of 32. However, the problem comes with: #define PIC_MASK0xFFD0 where we have an IRQ23 but no IRQ22. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC PATCH 0/5] ARM: introducing DT topology
On Wed, Jan 18, 2012 at 02:36:43PM +, Lorenzo Pieralisi wrote: Current code in the kernel, in particular the boot sequence, hinges upon a sequential mapping of MPIDR values for cpus and related interrupt controller CPU interfaces to logical cpu indexing. I don't believe it does. What it does rely upon is having cpu_logical_map() correctly setup for the logical-to-hardware mapping - which, if it's different, should be done in smp_init_cpus(), taking note that logical CPU 0 is _always_ the boot CPU. (That's a restriction caused by the way suspend/resume unplugs all but the lowest numbered logical online CPU.) So, with the code today, there's nothing in the code which prevents this from happening: a) you boot on h/w CPU 1, which becomes logical CPU 0. b) you boot h/w CPU 3 as logical CPU 1. c) you boot h/w CPU 0 as logical CPU 2. d) you boot h/w CPU 2 as logical CPU 3. You just need to ensure that cpu_logical_map() contains the array {1, 3, 0, 2} instead of the default (because we _have_ to have a default) of {1, 0, 2, 3}. This hypothesis is not valid when the concept of cluster is introduced since the MPIDR cannot be represented as a single index and interrupt controller CPU interfaces might be wired with a numbering scheme following per-SoC design parameters which cannot be extrapolated easily through generic functions by the primary CPU. So what you're saying is that the GIC CPU index may not be the CPU number given by MPIDR? Furthermore, relying on the MPIDR to be wired according to real topology levels might turn out to be an unreliable solution, hence a SW representation is needed to override possibly incorrect MPIDR register values. This sounds like you're saying that the contents of MPIDR might be buggy sometime in the future? Do we actually know of any situations where the information in there is currently wrong (outside of the development lab)? If not, it's not something we should cater for until it's actually happened, and then the pain should be felt by those silly enough to allow the chip to go out the door. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC PATCH 0/5] ARM: introducing DT topology
On Thu, Jan 19, 2012 at 12:18:32PM +, Catalin Marinas wrote: On Wed, Jan 18, 2012 at 06:04:53PM +, Russell King - ARM Linux wrote: On Wed, Jan 18, 2012 at 05:50:28PM +, Lorenzo Pieralisi wrote: This sounds like you're saying that the contents of MPIDR might be buggy sometime in the future? Do we actually know of any situations where the information in there is currently wrong (outside of the development lab)? If not, it's not something we should cater for until it's actually happened, and then the pain should be felt by those silly enough to allow the chip to go out the door. I share your view Russell. Having said that: MPIDR is IMPLEMENTATION DEFINED. I'll assume you mean that it's left to the implementation to set MPIDR appropriately, and you're expecting implementations to make mistakes with it. I think what Lorenzo means is that MPIDR is indeed a mandated register with certain bits specified by the ARM ARM. However, the affinity bits are left to the implementation. The ARM ARM makes recommendations and gives some examples but they are not mandatory. An implementer is under no legal obligation to follow the examples. It is not that unlikely that someone in the future may use a different cluster/processor counting scheme than ours. Also in the context of virtualisation, the lower affinity field may be used for virtual CPUs counting). So what you're saying is that the definition of this register has become soo loose that it's completely useless. Oh what a really great move. Your designers really need their heads bashed against a brick wall to knock some sense into them over stuff like this. It's exactly this kind of idiotic 'infinite flexibility' that Linus had a moan about last year. What it means is that we need _more_ code to support whatever manufacturers come up in their scheme of things - rather than having a standard way to represent these details, which software can parse in a consistent way. Instead, we have something that's effectively mandated to be present but is basically useless because no software can parse it properly without specific information from the manufacturer, and even then its not guaranteed. Well done ARM Ltd. Again. Stop doing this. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v6 9/9] ARM: vexpress: Add Device Tree for V2P-CA15 core tile (TC1 variant)
On Thu, Jan 19, 2012 at 05:00:56PM +, David Vrabel wrote: I don't expect any real production vexpress system to use this config options I do - _if_ I decide to try DT on my Versatile Express. That'll be with the existing boot setup, which will be ATAG based. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v6 9/9] ARM: vexpress: Add Device Tree for V2P-CA15 core tile (TC1 variant)
On Thu, Jan 19, 2012 at 05:27:15PM +, Pawel Moll wrote: On Thu, 2012-01-19 at 17:00 +, David Vrabel wrote: The problem wasn't with including skeleton.dtsi. Including as it is creates two device_type=memory nodes, one with regs=0 0, which is definitely wrong. With CONFIG_ARM_ATAG_DTB_COMPAT the zImage decompressor modifies the appended DTB using information from the ATAGs (see atags_to_fdt()). If there's an ATAG giving the amount of RAM the DTB's memory node is replaced with a new one. Since the vexpress DTBs don't have a memory node it's added and the DTB ends up with two nodes describing memory. The memory@address node name is in my opinion perfectly legal - p. 3.4 of the DT spec says The name component of the node name (see 2.2.1) shall be memory.. So the decompressor code may be wrong in looking for adress-less memory node... I don't think you can expect such early code to properly parse a DT tree with a variability in how memory stuff is declared into that DT tree. What if you have two memory nodes specified in the DT file, and the ATAG data contains one? The more I look at this, the more I'm convinced that Grant's idea that DT should entirely override ATAGs all the way to the kernel proper was the wrong solution - at least in the kernel, if we had both available, we could make a choice there, and have the full DT library to be able to manipulate the DT blob. Unfortunately, that's not so, so we're just going to have to accept that on ARM it should be memory if we want the ATAG code to override it. Expecting the decompressor to figure out that it needs to delete DT nodes and all that I think is asking far too much. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
[PATCH 00/31] Clean up amba/primecell bus support
This patch set cleans up the AMBA/Primecell bus support. Contained within this set are: 1. Patches 1-5 - Allocation APIs It seems several places in the kernel want to allocate and register amba_device structures. Let's make this official, and common code. So we introduce amba_device_alloc(), amba_device_add(), and amba_device_put() APIs, which provide the necessary support safely for this. Convert the DT, ux500, Integrator/AP and mxs code to use these APIs rather than their private version. 2. Patches 6-17 - NO_IRQ cleanup Making IRQ number 0 mean 'no irq' for the AMBA drivers and bus layer. We get rid of NO_IRQ initializers in this across various platforms, and make the bus layer warn if '-1' is passed in a device as an IRQ. This prevents new occurances of NO_IRQ initializers for this bus type going by unspotted. 3. Patches 18-31 - static initializers Provide a standardized set of initializers for statically declared AMBA devices. We provide two - one for the simpler APB devices, and one for AHB devices. The major difference between these two is the setting of the DMA mask - APB peripherals do not have DMA capability themselves. While updating (3), I came across new additions in Spear3xx and the Samsung directories which added AMBA devices, which are declared globally. The Spear3xx devices are used in other files, so that's fine. Samsung devices don't seem to be - and that needs fixing (did someone not run checkpatch or sparse on these changes?) Please send fixes. Acks for these patches are welcome. arch/arm/mach-bcmring/core.c | 23 +- arch/arm/mach-ep93xx/core.c| 46 ++- arch/arm/mach-exynos/dma.c | 38 ++--- arch/arm/mach-integrator/core.c| 70 +++- arch/arm/mach-integrator/impd1.c |9 +-- arch/arm/mach-integrator/integrator_cp.c | 49 ++- arch/arm/mach-lpc32xx/phy3250.c| 32 +-- arch/arm/mach-mxs/devices.c|8 +- arch/arm/mach-mxs/devices/amba-duart.c |2 +- arch/arm/mach-netx/fb.c| 13 +--- arch/arm/mach-nomadik/board-nhk8815.c | 17 +--- arch/arm/mach-nomadik/cpu-8815.c |9 +-- arch/arm/mach-omap2/emu.c | 26 +-- arch/arm/mach-realview/core.h | 20 + arch/arm/mach-realview/realview_eb.c | 78 +- arch/arm/mach-realview/realview_pb1176.c | 78 +- arch/arm/mach-realview/realview_pb11mp.c | 78 +- arch/arm/mach-realview/realview_pba8.c | 78 +- arch/arm/mach-realview/realview_pbx.c | 78 +- arch/arm/mach-s5p64x0/dma.c| 22 + arch/arm/mach-s5pc100/dma.c| 38 ++--- arch/arm/mach-s5pv210/dma.c| 38 ++--- arch/arm/mach-spear3xx/spear300.c | 14 +--- arch/arm/mach-spear3xx/spear3xx.c | 27 +- arch/arm/mach-spear6xx/spear6xx.c | 10 +- arch/arm/mach-u300/core.c | 85 arch/arm/mach-ux500/devices-common.c | 13 +-- arch/arm/mach-versatile/core.c | 70 arch/arm/mach-versatile/core.h | 20 + arch/arm/mach-versatile/versatile_pb.c | 18 ++-- arch/arm/mach-vexpress/core.h | 17 arch/arm/mach-vexpress/ct-ca9x4.c |8 +- arch/arm/mach-vexpress/include/mach/ct-ca9x4.h |2 +- arch/arm/mach-vexpress/v2m.c | 20 ++-- drivers/amba/bus.c | 105 ++-- drivers/mmc/host/mmci.c|2 +- drivers/of/platform.c |6 +- include/linux/amba/bus.h | 36 38 files changed, 488 insertions(+), 815 deletions(-) ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
[PATCH 16/31] ARM: amba: u300: get rid of NO_IRQ initializers
Signed-off-by: Russell King rmk+ker...@arm.linux.org.uk --- arch/arm/mach-u300/core.c |6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/arch/arm/mach-u300/core.c b/arch/arm/mach-u300/core.c index b4c6926..ed92967 100644 --- a/arch/arm/mach-u300/core.c +++ b/arch/arm/mach-u300/core.c @@ -105,7 +105,7 @@ static struct amba_device uart0_device = { .end = U300_UART0_BASE + SZ_4K - 1, .flags = IORESOURCE_MEM, }, - .irq = { IRQ_U300_UART0, NO_IRQ }, + .irq = { IRQ_U300_UART0 }, }; /* The U335 have an additional UART1 on the APP CPU */ @@ -129,7 +129,7 @@ static struct amba_device uart1_device = { .end = U300_UART1_BASE + SZ_4K - 1, .flags = IORESOURCE_MEM, }, - .irq = { IRQ_U300_UART1, NO_IRQ }, + .irq = { IRQ_U300_UART1 }, }; #endif @@ -160,7 +160,7 @@ static struct amba_device pl022_device = { .end = U300_SPI_BASE + SZ_4K - 1, .flags = IORESOURCE_MEM, }, - .irq = {IRQ_U300_SPI, NO_IRQ }, + .irq = {IRQ_U300_SPI }, /* * This device has a DMA channel but the Linux driver does not use * it currently. -- 1.7.4.4 ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
[PATCH 17/31] ARM: amba: make use of -1 IRQs warn
Make the core warn about the use of -1 (NO_IRQ) Signed-off-by: Russell King rmk+ker...@arm.linux.org.uk --- drivers/amba/bus.c |3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/drivers/amba/bus.c b/drivers/amba/bus.c index d15acbb..01c2cf4 100644 --- a/drivers/amba/bus.c +++ b/drivers/amba/bus.c @@ -511,6 +511,9 @@ int amba_device_add(struct amba_device *dev, struct resource *parent) void __iomem *tmp; int i, ret; + WARN_ON(dev-irq[0] == (unsigned int)-1); + WARN_ON(dev-irq[1] == (unsigned int)-1); + ret = request_resource(parent, dev-res); if (ret) goto err_out; -- 1.7.4.4 ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
[PATCH 18/31] ARM: amba: provide common initializers for static amba devices
Signed-off-by: Russell King rmk+ker...@arm.linux.org.uk --- include/linux/amba/bus.h | 33 + 1 files changed, 33 insertions(+), 0 deletions(-) diff --git a/include/linux/amba/bus.h b/include/linux/amba/bus.h index e192962..a9fab83 100644 --- a/include/linux/amba/bus.h +++ b/include/linux/amba/bus.h @@ -92,4 +92,37 @@ void amba_release_regions(struct amba_device *); #define amba_manf(d) AMBA_MANF_BITS((d)-periphid) #define amba_part(d) AMBA_PART_BITS((d)-periphid) +#define __AMBA_DEV(busid, data, mask) \ + { \ + .coherent_dma_mask = mask, \ + .init_name = busid, \ + .platform_data = data, \ + } + +/* + * APB devices do not themselves have the ability to address memory, + * so DMA masks should be zero (much like USB peripheral devices.) + * The DMA controller DMA masks should be used instead (much like + * USB host controllers in conventional PCs.) + */ +#define AMBA_APB_DEVICE(name, busid, id, base, irqs, data) \ +struct amba_device name##_device = { \ + .dev = __AMBA_DEV(busid, data, 0), \ + .res = DEFINE_RES_MEM(base, SZ_4K), \ + .irq = irqs,\ + .periphid = id, \ +} + +/* + * AHB devices are DMA capable, so set their DMA masks + */ +#define AMBA_AHB_DEVICE(name, busid, id, base, irqs, data) \ +struct amba_device name##_device = { \ + .dev = __AMBA_DEV(busid, data, ~0ULL), \ + .res = DEFINE_RES_MEM(base, SZ_4K), \ + .dma_mask = ~0ULL, \ + .irq = irqs,\ + .periphid = id, \ +} + #endif -- 1.7.4.4 ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
[PATCH 19/31] ARM: amba: vexpress: get rid of private platform amba_device initializer
Signed-off-by: Russell King rmk+ker...@arm.linux.org.uk --- arch/arm/mach-vexpress/core.h | 17 - arch/arm/mach-vexpress/ct-ca9x4.c |8 arch/arm/mach-vexpress/v2m.c | 20 ++-- 3 files changed, 14 insertions(+), 31 deletions(-) diff --git a/arch/arm/mach-vexpress/core.h b/arch/arm/mach-vexpress/core.h index f439715..9f0f282 100644 --- a/arch/arm/mach-vexpress/core.h +++ b/arch/arm/mach-vexpress/core.h @@ -1,19 +1,2 @@ #define __MMIO_P2V(x) (((x) 0xf) | (((x) 0x0f00) 4) | 0xf800) #define MMIO_P2V(x)((void __iomem *)__MMIO_P2V(x)) - -#define AMBA_DEVICE(name,busid,base,plat) \ -struct amba_device name##_device = { \ - .dev= { \ - .coherent_dma_mask = ~0UL, \ - .init_name = busid, \ - .platform_data = plat, \ - }, \ - .res= { \ - .start = base, \ - .end= base + SZ_4K - 1, \ - .flags = IORESOURCE_MEM, \ - }, \ - .dma_mask = ~0UL, \ - .irq= IRQ_##base, \ - /* .dma = DMA_##base,*/ \ -} diff --git a/arch/arm/mach-vexpress/ct-ca9x4.c b/arch/arm/mach-vexpress/ct-ca9x4.c index 2b1e836..3fde21c 100644 --- a/arch/arm/mach-vexpress/ct-ca9x4.c +++ b/arch/arm/mach-vexpress/ct-ca9x4.c @@ -109,10 +109,10 @@ static struct clcd_board ct_ca9x4_clcd_data = { .remove = versatile_clcd_remove_dma, }; -static AMBA_DEVICE(clcd, ct:clcd, CT_CA9X4_CLCDC, ct_ca9x4_clcd_data); -static AMBA_DEVICE(dmc, ct:dmc, CT_CA9X4_DMC, NULL); -static AMBA_DEVICE(smc, ct:smc, CT_CA9X4_SMC, NULL); -static AMBA_DEVICE(gpio, ct:gpio, CT_CA9X4_GPIO, NULL); +static AMBA_AHB_DEVICE(clcd, ct:clcd, 0, CT_CA9X4_CLCDC, IRQ_CT_CA9X4_CLCDC, ct_ca9x4_clcd_data); +static AMBA_APB_DEVICE(dmc, ct:dmc, 0, CT_CA9X4_DMC, IRQ_CT_CA9X4_DMC, NULL); +static AMBA_APB_DEVICE(smc, ct:smc, 0, CT_CA9X4_SMC, IRQ_CT_CA9X4_SMC, NULL); +static AMBA_APB_DEVICE(gpio, ct:gpio, 0, CT_CA9X4_GPIO, IRQ_CT_CA9X4_GPIO, NULL); static struct amba_device *ct_ca9x4_amba_devs[] __initdata = { clcd_device, diff --git a/arch/arm/mach-vexpress/v2m.c b/arch/arm/mach-vexpress/v2m.c index b4a28ca..ad64f97 100644 --- a/arch/arm/mach-vexpress/v2m.c +++ b/arch/arm/mach-vexpress/v2m.c @@ -266,16 +266,16 @@ static struct mmci_platform_data v2m_mmci_data = { .status = v2m_mmci_status, }; -static AMBA_DEVICE(aaci, mb:aaci, V2M_AACI, NULL); -static AMBA_DEVICE(mmci, mb:mmci, V2M_MMCI, v2m_mmci_data); -static AMBA_DEVICE(kmi0, mb:kmi0, V2M_KMI0, NULL); -static AMBA_DEVICE(kmi1, mb:kmi1, V2M_KMI1, NULL); -static AMBA_DEVICE(uart0, mb:uart0, V2M_UART0, NULL); -static AMBA_DEVICE(uart1, mb:uart1, V2M_UART1, NULL); -static AMBA_DEVICE(uart2, mb:uart2, V2M_UART2, NULL); -static AMBA_DEVICE(uart3, mb:uart3, V2M_UART3, NULL); -static AMBA_DEVICE(wdt, mb:wdt, V2M_WDT, NULL); -static AMBA_DEVICE(rtc, mb:rtc, V2M_RTC, NULL); +static AMBA_APB_DEVICE(aaci, mb:aaci, 0, V2M_AACI, IRQ_V2M_AACI, NULL); +static AMBA_APB_DEVICE(mmci, mb:mmci, 0, V2M_MMCI, IRQ_V2M_MMCI, v2m_mmci_data); +static AMBA_APB_DEVICE(kmi0, mb:kmi0, 0, V2M_KMI0, IRQ_V2M_KMI0, NULL); +static AMBA_APB_DEVICE(kmi1, mb:kmi1, 0, V2M_KMI1, IRQ_V2M_KMI1, NULL); +static AMBA_APB_DEVICE(uart0, mb:uart0, 0, V2M_UART0, IRQ_V2M_UART0, NULL); +static AMBA_APB_DEVICE(uart1, mb:uart1, 0, V2M_UART1, IRQ_V2M_UART1, NULL); +static AMBA_APB_DEVICE(uart2, mb:uart2, 0, V2M_UART2, IRQ_V2M_UART2, NULL); +static AMBA_APB_DEVICE(uart3, mb:uart3, 0, V2M_UART3, IRQ_V2M_UART3, NULL); +static AMBA_APB_DEVICE(wdt, mb:wdt, 0, V2M_WDT, IRQ_V2M_WDT, NULL); +static AMBA_APB_DEVICE(rtc, mb:rtc, 0, V2M_RTC, IRQ_V2M_RTC, NULL); static struct amba_device *v2m_amba_devs[] __initdata = { aaci_device, -- 1.7.4.4 ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
[PATCH 21/31] ARM: amba: realview: get rid of private platform amba_device initializer
Signed-off-by: Russell King rmk+ker...@arm.linux.org.uk --- arch/arm/mach-realview/core.h| 20 --- arch/arm/mach-realview/realview_eb.c | 38 +++--- arch/arm/mach-realview/realview_pb1176.c | 38 +++--- arch/arm/mach-realview/realview_pb11mp.c | 38 +++--- arch/arm/mach-realview/realview_pba8.c | 38 +++--- arch/arm/mach-realview/realview_pbx.c| 38 +++--- 6 files changed, 100 insertions(+), 110 deletions(-) diff --git a/arch/arm/mach-realview/core.h b/arch/arm/mach-realview/core.h index 735b57a..b1c6097 100644 --- a/arch/arm/mach-realview/core.h +++ b/arch/arm/mach-realview/core.h @@ -28,21 +28,11 @@ #include asm/setup.h #include asm/leds.h -#define AMBA_DEVICE(name,busid,base,plat) \ -static struct amba_device name##_device = {\ - .dev= { \ - .coherent_dma_mask = ~0,\ - .init_name = busid, \ - .platform_data = plat, \ - }, \ - .res= { \ - .start = REALVIEW_##base##_BASE, \ - .end= (REALVIEW_##base##_BASE) + SZ_4K - 1, \ - .flags = IORESOURCE_MEM, \ - }, \ - .dma_mask = ~0, \ - .irq= base##_IRQ, \ -} +#define APB_DEVICE(name, busid, base, plat)\ +static AMBA_APB_DEVICE(name, busid, 0, base, base##_IRQ, plat) + +#define AHB_DEVICE(name, busid, base, plat)\ +static AMBA_AHB_DEVICE(name, busid, 0, base, base##_IRQ, plat) struct machine_desc; diff --git a/arch/arm/mach-realview/realview_eb.c b/arch/arm/mach-realview/realview_eb.c index 5c810e5..fbca43c 100644 --- a/arch/arm/mach-realview/realview_eb.c +++ b/arch/arm/mach-realview/realview_eb.c @@ -176,27 +176,27 @@ static struct pl022_ssp_controller ssp0_plat_data = { #define EB_SSP_IRQ { IRQ_EB_SSP } /* FPGA Primecells */ -AMBA_DEVICE(aaci, fpga:aaci, AACI, NULL); -AMBA_DEVICE(mmc0, fpga:mmc0, MMCI0,realview_mmc0_plat_data); -AMBA_DEVICE(kmi0, fpga:kmi0, KMI0, NULL); -AMBA_DEVICE(kmi1, fpga:kmi1, KMI1, NULL); -AMBA_DEVICE(uart3, fpga:uart3, EB_UART3, NULL); +APB_DEVICE(aaci, fpga:aaci, AACI, NULL); +APB_DEVICE(mmc0, fpga:mmc0, MMCI0,realview_mmc0_plat_data); +APB_DEVICE(kmi0, fpga:kmi0, KMI0, NULL); +APB_DEVICE(kmi1, fpga:kmi1, KMI1, NULL); +APB_DEVICE(uart3, fpga:uart3, EB_UART3, NULL); /* DevChip Primecells */ -AMBA_DEVICE(smc, dev:smc, EB_SMC, NULL); -AMBA_DEVICE(clcd, dev:clcd, EB_CLCD, clcd_plat_data); -AMBA_DEVICE(dmac, dev:dmac, DMAC, NULL); -AMBA_DEVICE(sctl, dev:sctl, SCTL, NULL); -AMBA_DEVICE(wdog, dev:wdog, EB_WATCHDOG, NULL); -AMBA_DEVICE(gpio0, dev:gpio0, EB_GPIO0, gpio0_plat_data); -AMBA_DEVICE(gpio1, dev:gpio1, GPIO1,gpio1_plat_data); -AMBA_DEVICE(gpio2, dev:gpio2, GPIO2,gpio2_plat_data); -AMBA_DEVICE(rtc, dev:rtc, EB_RTC, NULL); -AMBA_DEVICE(sci0, dev:sci0, SCI, NULL); -AMBA_DEVICE(uart0, dev:uart0, EB_UART0, NULL); -AMBA_DEVICE(uart1, dev:uart1, EB_UART1, NULL); -AMBA_DEVICE(uart2, dev:uart2, EB_UART2, NULL); -AMBA_DEVICE(ssp0, dev:ssp0, EB_SSP, ssp0_plat_data); +AHB_DEVICE(smc, dev:smc, EB_SMC, NULL); +AHB_DEVICE(clcd, dev:clcd, EB_CLCD, clcd_plat_data); +AHB_DEVICE(dmac, dev:dmac, DMAC, NULL); +AHB_DEVICE(sctl, dev:sctl, SCTL, NULL); +APB_DEVICE(wdog, dev:wdog, EB_WATCHDOG, NULL); +APB_DEVICE(gpio0, dev:gpio0, EB_GPIO0, gpio0_plat_data); +APB_DEVICE(gpio1, dev:gpio1, GPIO1,gpio1_plat_data); +APB_DEVICE(gpio2, dev:gpio2, GPIO2,gpio2_plat_data); +APB_DEVICE(rtc, dev:rtc, EB_RTC, NULL); +APB_DEVICE(sci0, dev:sci0, SCI, NULL); +APB_DEVICE(uart0, dev:uart0, EB_UART0, NULL); +APB_DEVICE(uart1, dev:uart1, EB_UART1, NULL); +APB_DEVICE(uart2, dev:uart2, EB_UART2, NULL); +APB_DEVICE(ssp0, dev:ssp0, EB_SSP, ssp0_plat_data); static struct amba_device *amba_devs[] __initdata = { dmac_device, diff --git a/arch/arm/mach-realview/realview_pb1176.c b/arch/arm/mach-realview/realview_pb1176.c index 9506de9..1b6e60c 100644 --- a/arch/arm/mach-realview/realview_pb1176.c +++ b/arch/arm/mach-realview/realview_pb1176.c @@ -155,27 +155,27 @@ static struct pl022_ssp_controller ssp0_plat_data = { #define PB1176_SSP_IRQ { IRQ_DC1176_SSP } /* FPGA Primecells */ -AMBA_DEVICE(aaci, fpga:aaci,AACI, NULL); -AMBA_DEVICE(mmc0, fpga:mmc0,MMCI0, realview_mmc0_plat_data); -AMBA_DEVICE(kmi0,