Re: [PATCH 1/3] Fix Unlikely(x) == y
On Tue, Feb 19, 2008 at 10:28:46AM +0100, Andi Kleen wrote: > > Sometimes, for performance critical paths, I would like gcc to be dumb and > > follow *my* code and not its hard-coded probabilities. > > If you really want that, simple: just disable optimization @) already tried. It fixed some difficulties, but create new expected issues with data being reloaded often from memory instead of being passed along a few registers. Don't forget that optimizing for x86 requires a lot of smartness from the compiler because of the very small number of registers! > > Maybe one thing we would need would be the ability to assign probabilities > > to each branch based on what we expect, so that gcc could build a better > > tree keeping most frequently used code tight. > > Just use profile feedback then for user space. I don't think it's a good > idea for kernel code though because it leads to unreproducible binaries > which would wreck the development model. I never found this to be practically usable in fact, because you have to use it on the *exact* same source. End of game for cross-compilation. It would be good to be able to use a few pragmas in the code to say "hey, I want this block optimized like this". This is what I understood the __builtin_expect() was for, except that it tends to throw unpredicted branches too far away. > > Hmm I've just noticed -fno-guess-branch-probability in the man, I never > > tried > > it. > > Or -fno-reorder-blocks Thanks for the hint, I will try it. Willy ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 2/2] i2c-ibm_iic driver
Hi Sean, On Tue, 19 Feb 2008 16:58:01 -0500, Sean MacLennan wrote: > Signed-off-by: Sean MacLennan <[EMAIL PROTECTED]> > > --- for-2.6.25/drivers/i2c/busses/orig-i2c-ibm_iic.c 2008-02-18 > 16:36:30.0 -0500 > +++ for-2.6.25/drivers/i2c/busses/i2c-ibm_iic.c 2008-02-19 > 16:46:35.0 -0500 > @@ -6,6 +6,9 @@ > * Copyright (c) 2003, 2004 Zultys Technologies. > * Eugene Surovegin <[EMAIL PROTECTED]> or <[EMAIL PROTECTED]> > * > + * Copyright (c) 2008 PIKA Technologies > + * Sean MacLennan <[EMAIL PROTECTED]> > + * > * Based on original work by > * Ian DaSilva <[EMAIL PROTECTED]> > * Armin Kuster <[EMAIL PROTECTED]> > (...) > --- a/drivers/i2c/busses/Kconfig > +++ b/drivers/i2c/busses/Kconfig > @@ -244,7 +244,7 @@ config I2C_PIIX4 > > config I2C_IBM_IIC > tristate "IBM PPC 4xx on-chip I2C interface" > - depends on IBM_OCP > + depends on 4xx > help > Say Y here if you want to use IIC peripheral found on > embedded IBM PPC 4xx based systems. I've applied this version. I see that there are discussions going on about the best "depends on" statement in Kconfig, feel free to submit an updated (or incremental) patch as soon as a decision will have been taken. Thanks, -- Jean Delvare ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 2/2] i2c-ibm_iic driver
Hi Arnd, On Tue, 19 Feb 2008 23:55:16 +0100, Arnd Bergmann wrote: > On Tuesday 19 February 2008, Stefan Roese wrote: > > On Tuesday 19 February 2008, Jean Delvare wrote: > > > > > > With this Kconfig change, "make menuconfig" lets me select the > > > i2c-ibm_iic driver on x86_64, but it fails to build horribly. I think > > > that you want to restrict the build to PPC machines somehow, or at > > > least make sure that either IBM_OCP or OF support is present. > > > > How about this: > > > > - depends on IBM_OCP > > + depends on 4xx > > I think we should allow it to be built on other platforms as well, > as long as they have of_platform_device support. > > The Axon south bridge used on IBMs QS21 blade probably has an ibm_iic, > even though it's managed by the firmware and we probably don't want > to use it at this time, someone could use the same chip in a new > design and actually do that. > > In general, I also like to make it possible to enable drivers just > for the benefit of compile testing, even for stuff that you can't > find in any existing HW configuration, so as long as it builds on > a platform, I think we shouldn't forbid it: Fine with me as long as the default is set appropriately (i.e. default to not building the driver on archs/platforms where it builds but is known to be useless.) > > - depends on IBM_OCP > + depends on IBM_OCP || PPC_MERGE -- Jean Delvare ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [patch 0/4] RFC: PCI: consolidate several pcibios_enable_resources() implementations
On Mon, Feb 18, 2008 at 09:39:52PM -0700, Bjorn Helgaas wrote: > There are many implementations of pcibios_enable_resources() that differ > in minor ways that look more like bugs than architectural differences. > This patch series consolidates most of them to use the x86 version. > > This series is for discussion only at this point. I'm interested in > feedback about whether any of the differences are "real" and need to > be preserved. > > ARM and PA-RISC, in particular, have interesting differences: > - ARM always enables bridge devices, which no other arch does > - PA-RISC always turns on SERR and PARITY, which no other arch does > > Should other arches do the same thing, or are these somehow related to > ARM and PA-RISC architecture? My impression was most x86 BIOS's did NOT turn on SERR/PERR when I added that code to parisc-linux port (2000 or 2001 so) . HPUX was turning on SERR/PERR and so I was comfortable the HW was stable and it not crash under normal use unless something was really broken. There is certainly nothing architectural specific about SERR/PERR. I felt (at the time) this is more of a case of "if it's not reported to the user, we won't get blamed for it not working well." hth, grant ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
[GIT PULL] Please pull spufs.git master branch
Hi Paul, Please do a: git pull \ git://git.kernel.org/pub/scm/linux/kernel/git/jk/spufs.git master I have two new bugfixes for spufs: these are both fixes for regressions since .24. Cheers, Jeremy --- 2 commits: [POWERPC] spufs: fix scheduler starvation by idle contexts Jeremy Kerr <[EMAIL PROTECTED]> arch/powerpc/platforms/cell/spufs/run.c |1 + arch/powerpc/platforms/cell/spufs/sched.c |8 +++- 2 files changed, 4 insertions(+), 5 deletions(-) [POWERPC] cell: fix spurious false return from spu_trap_data_{map,seg} Andre Detsch <[EMAIL PROTECTED]> arch/powerpc/platforms/cell/spu_base.c | 12 arch/powerpc/platforms/cell/spufs/switch.c |6 +++--- include/asm-powerpc/spu.h |3 +-- 3 files changed, 4 insertions(+), 17 deletions(-) ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] [LMB]: Fix lmb_add_region if region should be added at the head
On Feb 19, 2008, at 11:26 PM, David Miller wrote: > From: Kumar Gala <[EMAIL PROTECTED]> > Date: Tue, 19 Feb 2008 23:16:18 -0600 > >> The for loop above the code I added will move all the existing slots >> up one. Its just the tail cleanup we are missing. > > Aha, I see how this works now, thanks! > > I'll add this to my LMB tree. Sounds good. Now just convince Paul or Linus to pull this in for 2.6.25 :) - k ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] [LMB]: Fix lmb_add_region if region should be added at the head
From: Kumar Gala <[EMAIL PROTECTED]> Date: Tue, 19 Feb 2008 23:16:18 -0600 > The for loop above the code I added will move all the existing slots > up one. Its just the tail cleanup we are missing. Aha, I see how this works now, thanks! I'll add this to my LMB tree. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] [LMB]: Fix lmb_add_region if region should be added at the head
On Feb 19, 2008, at 10:45 PM, David Miller wrote: > From: Kumar Gala <[EMAIL PROTECTED]> > Date: Tue, 19 Feb 2008 22:27:48 -0600 (CST) > >> We introduced a bug in fixing lmb_add_region to handle an initial >> region being non-zero. Before that fix it was impossible to insert >> a region at the head of the list since the first region always >> started >> at zero. >> >> Now that its possible for the first region to be non-zero we need to >> check to see if the new region should be added at the head and if so >> actually add it. >> >> Signed-off-by: Kumar Gala <[EMAIL PROTECTED]> > ... >> @@ -184,6 +184,11 @@ static long __init lmb_add_region(struct >> lmb_region *rgn, u64 base, u64 size) >> break; >> } >> } >> + >> +if (base < rgn->region[0].base) { >> +rgn->region[0].base = base; >> +rgn->region[0].size = size; >> +} >> rgn->cnt++; >> >> return 0; > > Are you sure this is sufficient? > > It seems to me, to handle this properly, you'll need to handle > the case where the lower addressed entry you are inserting is > not contiguous with the existing entry 0. > > Therefore, you need to move all existing entries up a slot, > then you can set the 0 entry to 'base' and 'size'. The for loop above the code I added will move all the existing slots up one. Its just the tail cleanup we are missing. - k ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] [LMB]: Fix lmb_add_region if region should be added at the head
From: Kumar Gala <[EMAIL PROTECTED]> Date: Tue, 19 Feb 2008 22:27:48 -0600 (CST) > We introduced a bug in fixing lmb_add_region to handle an initial > region being non-zero. Before that fix it was impossible to insert > a region at the head of the list since the first region always started > at zero. > > Now that its possible for the first region to be non-zero we need to > check to see if the new region should be added at the head and if so > actually add it. > > Signed-off-by: Kumar Gala <[EMAIL PROTECTED]> ... > @@ -184,6 +184,11 @@ static long __init lmb_add_region(struct lmb_region > *rgn, u64 base, u64 size) > break; > } > } > + > + if (base < rgn->region[0].base) { > + rgn->region[0].base = base; > + rgn->region[0].size = size; > + } > rgn->cnt++; > > return 0; Are you sure this is sufficient? It seems to me, to handle this properly, you'll need to handle the case where the lower addressed entry you are inserting is not contiguous with the existing entry 0. Therefore, you need to move all existing entries up a slot, then you can set the 0 entry to 'base' and 'size'. What do you think? ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCH] [LMB]: Fix lmb_add_region if region should be added at the head
We introduced a bug in fixing lmb_add_region to handle an initial region being non-zero. Before that fix it was impossible to insert a region at the head of the list since the first region always started at zero. Now that its possible for the first region to be non-zero we need to check to see if the new region should be added at the head and if so actually add it. Signed-off-by: Kumar Gala <[EMAIL PROTECTED]> --- lib/lmb.c |5 + 1 files changed, 5 insertions(+), 0 deletions(-) diff --git a/lib/lmb.c b/lib/lmb.c index e3c8dcb..3c43b95 100644 --- a/lib/lmb.c +++ b/lib/lmb.c @@ -184,6 +184,11 @@ static long __init lmb_add_region(struct lmb_region *rgn, u64 base, u64 size) break; } } + + if (base < rgn->region[0].base) { + rgn->region[0].base = base; + rgn->region[0].size = size; + } rgn->cnt++; return 0; -- 1.5.3.8 ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Please pull powerpc.git merge branch
Linus, Please do git pull \ git://git.kernel.org/pub/scm/linux/kernel/git/paulus/powerpc.git merge to get a few more bug and warning fixes for powerpc. The diffstat is bloated by the defconfig updates -- the actual code changes are only a few dozen lines. Thanks, Paul. arch/powerpc/boot/Makefile |8 arch/powerpc/boot/dts/bamboo.dts |3 arch/powerpc/boot/dts/ebony.dts|2 arch/powerpc/boot/dts/katmai.dts |2 arch/powerpc/boot/dts/kilauea.dts |3 arch/powerpc/boot/dts/makalu.dts |3 arch/powerpc/boot/dts/rainier.dts |4 arch/powerpc/boot/dts/sequoia.dts |4 arch/powerpc/boot/dts/taishan.dts |4 arch/powerpc/configs/bamboo_defconfig | 81 ++- arch/powerpc/configs/ebony_defconfig | 79 ++- arch/powerpc/configs/ep405_defconfig | 92 ++- arch/powerpc/configs/kilauea_defconfig | 69 ++ arch/powerpc/configs/makalu_defconfig | 69 ++ arch/powerpc/configs/ppc44x_defconfig | 904 arch/powerpc/configs/rainier_defconfig | 82 ++- arch/powerpc/configs/sequoia_defconfig | 77 ++- arch/powerpc/configs/taishan_defconfig | 81 ++- arch/powerpc/configs/walnut_defconfig | 81 ++- arch/powerpc/configs/warp_defconfig| 139 +++-- arch/powerpc/kernel/kprobes.c |9 arch/powerpc/kernel/prom.c | 13 arch/powerpc/platforms/44x/Kconfig | 10 arch/powerpc/platforms/pseries/power.c |2 arch/ppc/platforms/4xx/ibm440ep.c |6 drivers/net/ibm_newemac/rgmii.c|1 26 files changed, 1497 insertions(+), 331 deletions(-) create mode 100644 arch/powerpc/configs/ppc44x_defconfig Ananth N Mavinakayanahalli (1): [POWERPC] Kill sparse warnings in kprobes Becky Bruce (1): [POWERPC] Fix dt_mem_next_cell() to read the full address Josh Boyer (4): [POWERPC] 4xx: Update defconfigs for 2.6.25 [POWERPC] 44x: Fix Kconfig formatting [POWERPC] 44x: Add multiplatform defconfig [POWERPC] Fix bootwrapper builds with older gcc versions Stefan Roese (2): [POWERPC] net: NEWEMAC: Remove "rgmii-interface" from rgmii matching table [POWERPC] 4xx: Remove "i2c" and "xxmii-interface" device_types from dts Stephen Rothwell (1): [POWERPC] Fix warning in pseries/power.c Wolfgang Ocker (1): [POWERPC] PPC440EP Interrupt Triggering and Level Settings ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH v2][POWERPC] Fix initial lmb add region with a non-zero base
From: Kumar Gala <[EMAIL PROTECTED]> Date: Tue, 19 Feb 2008 21:02:04 -0600 > np. Are we trying to get this into 2.6.25 or .26? I'm ambivalent but I would obviously prefer 2.6.25 because it would allow me to proceed more easily with my sparc64 NUMA work as well as get your bug fixes in more smoothly. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH v2][POWERPC] Fix initial lmb add region with a non-zero base
On Feb 19, 2008, at 6:30 PM, David Miller wrote: > From: Kumar Gala <[EMAIL PROTECTED]> > Date: Tue, 19 Feb 2008 13:51:37 -0600 (CST) > >> If we add to an empty lmb region with a non-zero base we will not >> coalesce >> the number of regions down to one. This causes problems on ppc32 >> for the >> memory region as its assumed to only have one region. >> >> We can fix this easily by causing the initial add to replace the >> dummy >> region. >> >> Signed-off-by: Kumar Gala <[EMAIL PROTECTED]> >> --- >> >> Fix a bug the initial patch introduced if we have a region that >> gets added >> at the beginning of the list we wouldn't actually add it. >> >> Dave can you replace the patch in you tree with this one. > > I think my tree has already or will soon be pulled in so > I don't want to rebase it. > > Why don't you simply send me the relative bug fix instead? np. Are we trying to get this into 2.6.25 or .26? - k ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: libfdt: More tests of NOP handling behaviour
On Tue, Feb 19, 2008 at 08:18:19PM -0500, Jerry Van Baren wrote: > Jon Loeliger wrote: > > So, like, the other day David Gibson mumbled: > >> In light of the recently discovered bug with NOP handling, this adds > >> some more testcases for NOP handling. Specifically, it adds a helper > >> program which will add a NOP tag after every existing tag in a dtb, > >> and runs the standard battery of tests over trees mangled in this way. > >> > >> For now, this does not add a NOP at the very beginning of the > >> structure block. This causes problems for libfdt at present, because > >> we assume in many places that the root node's BEGIN_NODE tag is at > >> offset 0. I'm still contemplating what to do about this (with one > >> option being simply to declare such dtbs invalid). > >> > >> Signed-off-by: David Gibson <[EMAIL PROTECTED]> > > > > Applied. > > > > BTW, declaring DTBs with BEGIN_NODES not at offset 0 > > as invalid seems like a fine choice to me. > > > > jdl > > FWIIW, I vote ditto on declaring DTBs with BEGIN_NODES not at offset 0 > as invalid. The root being at offset 0 assumption is pretty well > entrenched and I cannot think of any reason to change it that would be > worth the effort. Well, it's actually not that hard to deal with. I've already been planning to add a helper function/macro which validates a node offset (something currently open-coded in a whole bunch of places). It would be fairly easy to make it skip over nops as well. But, likewise I can think of no reason that NOPs before the root node would be useful or likely to occur in practice. -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: libfdt: More tests of NOP handling behaviour
Jon Loeliger wrote: > So, like, the other day David Gibson mumbled: >> In light of the recently discovered bug with NOP handling, this adds >> some more testcases for NOP handling. Specifically, it adds a helper >> program which will add a NOP tag after every existing tag in a dtb, >> and runs the standard battery of tests over trees mangled in this way. >> >> For now, this does not add a NOP at the very beginning of the >> structure block. This causes problems for libfdt at present, because >> we assume in many places that the root node's BEGIN_NODE tag is at >> offset 0. I'm still contemplating what to do about this (with one >> option being simply to declare such dtbs invalid). >> >> Signed-off-by: David Gibson <[EMAIL PROTECTED]> > > Applied. > > BTW, declaring DTBs with BEGIN_NODES not at offset 0 > as invalid seems like a fine choice to me. > > jdl FWIIW, I vote ditto on declaring DTBs with BEGIN_NODES not at offset 0 as invalid. The root being at offset 0 assumption is pretty well entrenched and I cannot think of any reason to change it that would be worth the effort. Best regards, gvb ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [Linux-fbdev-devel] [PATCH 1/2] fb: add support for foreign endianness
Andrew Morton writes: > Bizarrely, the original author of the patch (Anton) has fallen off the cc. > Could whoever did that please thwap himself? > > Anyway, my head is now officially spinning. Did anyone actually have a > reason why we shouldn't proceed with Anton's patch? I was wondering if it would be sufficient to provide alternative versions of fb_readl, fb_writel etc. that do byte-swapping. That would mean that all framebuffers would have to have the same endianness, but that would suffice for embedded systems such as Anton's and would end up a lot simpler IMHO. Paul. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCH] powerpc: don't create two devices for each cpm_uart device tree node
powerpc: don't create two devices for each cpm_uart device tree node Code in arch/powerpc/sysdev/fsl_soc.c used to create two 'struct device' objects for each cpm_uart device tree node - one "fsl-cpm-scc:uart" in cpm_uart_of_init() and one "fsl-cpm-smc:uart" in cpm_smc_uart_of_init(). This patch adds additional test to those routines, such that cpm_uart_of_init() only registers devices with model "SCC", and cpm_smc_uart_of_init() only registers devices with model "SMC". Signed-off-by: Nikita Youshchenko <[EMAIL PROTECTED]> diff --git a/arch/powerpc/sysdev/fsl_soc.c b/arch/powerpc/sysdev/fsl_soc.c index f1b8412..536f8c9 100644 --- a/arch/powerpc/sysdev/fsl_soc.c +++ b/arch/powerpc/sysdev/fsl_soc.c @@ -890,6 +890,10 @@ static int __init cpm_uart_of_init(void) const int *id; const char *model; + model = of_get_property(np, "model", NULL); + if (!model || strcmp(model, "SCC") != 0) + continue; + memset(r, 0, sizeof(r)); memset(&cpm_uart_data, 0, sizeof(cpm_uart_data)); @@ -917,7 +921,6 @@ static int __init cpm_uart_of_init(void) id = of_get_property(np, "device-id", NULL); cpm_uart_data.fs_no = *id; - model = of_get_property(np, "model", NULL); strcpy(cpm_uart_data.fs_type, model); cpm_uart_data.uart_clk = ppc_proc_freq; @@ -1175,6 +1178,10 @@ static int __init cpm_smc_uart_of_init(void) const int *id; const char *model; + model = of_get_property(np, "model", NULL); + if (!model || strcmp(model, "SMC") != 0) + continue; + memset(r, 0, sizeof(r)); memset(&cpm_uart_data, 0, sizeof(cpm_uart_data)); @@ -1200,7 +1207,6 @@ static int __init cpm_smc_uart_of_init(void) goto err; } - model = of_get_property(np, "model", NULL); strcpy(cpm_uart_data.fs_type, model); id = of_get_property(np, "device-id", NULL); signature.asc Description: This is a digitally signed message part. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [Linux-fbdev-devel] [PATCH 1/2] fb: add support for foreign endianness
On Tue, 2008-02-19 at 19:47 -0500, [EMAIL PROTECTED] wrote: > On Tue, 19 Feb 2008 04:05:30 PST, Andrew Morton said: > > On Tue, 19 Feb 2008 12:27:54 +0100 Clemens Koller <[EMAIL PROTECTED] > > wrote: > > > That's not an issue in my case. The SM50x can be connected to > > > either an PCI or some Local/CPU-whateverbus IF. > > > I.e. on the MPC85xx PowerPC, PCI and LocalBus are separate bussses. > > > If the sm501 is attached to the MPC85xx' PCI like any other video card, > > > the PCI config-space is can be accessed as usual, whereas the framebuffer > > > memory area is byte-swapped compared to other common video cards. > > > Anyway, my head is now officially spinning. Did anyone actually have a > > reason why we shouldn't proceed with Anton's patch? > > Clemens answered my question regarding the real-life existence of hardware > that would benefit. I'd say if Anton's patch works on Clemens' hardware and > otherwise passes review, we should proceed... No objection here neither. Ben. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [Linux-fbdev-devel] [PATCH 1/2] fb: add support for foreign endianness
On Tue, 19 Feb 2008 04:05:30 PST, Andrew Morton said: > On Tue, 19 Feb 2008 12:27:54 +0100 Clemens Koller <[EMAIL PROTECTED] > wrote: > > That's not an issue in my case. The SM50x can be connected to > > either an PCI or some Local/CPU-whateverbus IF. > > I.e. on the MPC85xx PowerPC, PCI and LocalBus are separate bussses. > > If the sm501 is attached to the MPC85xx' PCI like any other video card, > > the PCI config-space is can be accessed as usual, whereas the framebuffer > > memory area is byte-swapped compared to other common video cards. > Anyway, my head is now officially spinning. Did anyone actually have a > reason why we shouldn't proceed with Anton's patch? Clemens answered my question regarding the real-life existence of hardware that would benefit. I'd say if Anton's patch works on Clemens' hardware and otherwise passes review, we should proceed... pgpUuNatloOiz.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH v2][POWERPC] Fix initial lmb add region with a non-zero base
From: Kumar Gala <[EMAIL PROTECTED]> Date: Tue, 19 Feb 2008 13:51:37 -0600 (CST) > If we add to an empty lmb region with a non-zero base we will not coalesce > the number of regions down to one. This causes problems on ppc32 for the > memory region as its assumed to only have one region. > > We can fix this easily by causing the initial add to replace the dummy > region. > > Signed-off-by: Kumar Gala <[EMAIL PROTECTED]> > --- > > Fix a bug the initial patch introduced if we have a region that gets added > at the beginning of the list we wouldn't actually add it. > > Dave can you replace the patch in you tree with this one. I think my tree has already or will soon be pulled in so I don't want to rebase it. Why don't you simply send me the relative bug fix instead? Thanks! ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCH] [POWERPC] fix warning in pseries/power.c
Introduced by commit 79393fc46ede43451a500a132e5de9856f5a4c83 ("kobject: convert pseries/power.c to kobj_attr interface"). sys_create_file takes a "struct attrbute *" not a "struct kobj_addribute *". arch/powerpc/platforms/pseries/power.c: In function 'apo_pm_init': arch/powerpc/platforms/pseries/power.c:78: warning: passing argument 2 of 'sysfs_create_file' from incompatible pointer type Signed-off-by: Stephen Rothwell <[EMAIL PROTECTED]> --- arch/powerpc/platforms/pseries/power.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/arch/powerpc/platforms/pseries/power.c b/arch/powerpc/platforms/pseries/power.c index e95fc15..6d62662 100644 --- a/arch/powerpc/platforms/pseries/power.c +++ b/arch/powerpc/platforms/pseries/power.c @@ -75,7 +75,7 @@ core_initcall(pm_init); #else static int __init apo_pm_init(void) { - return (sysfs_create_file(power_kobj, &auto_poweron_attr)); + return (sysfs_create_file(power_kobj, &auto_poweron_attr.attr)); } __initcall(apo_pm_init); #endif -- 1.5.4.2 -- Cheers, Stephen Rothwell[EMAIL PROTECTED] http://www.canb.auug.org.au/~sfr/ ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [patch v7 3/4] USB: add Cypress c67x00 OTG controller HCD driver
On Tue, Feb 19, 2008 at 04:09:19PM +0100, Peter Korsgaard wrote: > This patch adds HCD support for the Cypress c67x00 family of devices. > > Signed-off-by: Peter Korsgaard <[EMAIL PROTECTED]> And it doesn't build: CC [M] drivers/usb/c67x00/c67x00-hcd.o distcc[2413] ERROR: compile /home/gregkh/.ccache/c67x00-hcd.tmp.mini.2409.i on localhost failed drivers/usb/c67x00/c67x00-hcd.c:345: error: redefinition of 'c67x00_hcd_probe' drivers/usb/c67x00/c67x00-hcd.h:119: error: previous definition of 'c67x00_hcd_probe' was here drivers/usb/c67x00/c67x00-hcd.c:402: error: redefinition of 'c67x00_hcd_remove' drivers/usb/c67x00/c67x00-hcd.h:126: error: previous definition of 'c67x00_hcd_remove' was here make[2]: *** [drivers/usb/c67x00/c67x00-hcd.o] Error 1 make[1]: *** [drivers/usb/c67x00] Error 2 make: *** [_module_drivers/usb] Error 2 This is _after_ removing the obviously incorrect usb_disabled() function that you included in your .h file. I'm going to hold off applying any of these for now, as it doesn't look like something is configured properly here. thanks, greg k-h ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 2/2] i2c-ibm_iic driver
On Wednesday 20 February 2008, Stephen Rothwell wrote: > > > - depends on IBM_OCP > > > + depends on IBM_OCP || PPC_MERGE > > not PPC_OF? or even OF (give the sparc guys the opportunity to build it > for us :-))? > Right, I was looking for that option but couldn't find it. I would guess sparc doesn't work because it's lacking the necessary I/O functions, in_8 and out_8 that are powerpc specific. so maybe: depends on PPC depends on IBM_OCP || OF Arnd <>< ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 2/2] i2c-ibm_iic driver
> > - depends on IBM_OCP > > + depends on IBM_OCP || PPC_MERGE not PPC_OF? or even OF (give the sparc guys the opportunity to build it for us :-))? -- Cheers, Stephen Rothwell[EMAIL PROTECTED] http://www.canb.auug.org.au/~sfr/ pgpCiK7cr2CGX.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 2/2] i2c-ibm_iic driver
Arnd Bergmann wrote: > On Tuesday 19 February 2008, Stefan Roese wrote: > >> On Tuesday 19 February 2008, Jean Delvare wrote: >> >>> With this Kconfig change, "make menuconfig" lets me select the >>> i2c-ibm_iic driver on x86_64, but it fails to build horribly. I think >>> that you want to restrict the build to PPC machines somehow, or at >>> least make sure that either IBM_OCP or OF support is present. >>> >> How about this: >> >> - depends on IBM_OCP >> + depends on 4xx >> > > I think we should allow it to be built on other platforms as well, > as long as they have of_platform_device support. > > The Axon south bridge used on IBMs QS21 blade probably has an ibm_iic, > even though it's managed by the firmware and we probably don't want > to use it at this time, someone could use the same chip in a new > design and actually do that. > > In general, I also like to make it possible to enable drivers just > for the benefit of compile testing, even for stuff that you can't > find in any existing HW configuration, so as long as it builds on > a platform, I think we shouldn't forbid it: > > - depends on IBM_OCP > + depends on IBM_OCP || PPC_MERGE > > I have no problem with this change. If everybody agrees, I can respin the patch. Cheers, Sean ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: Difference between vmlinux and vmlinux.bin
Timur Tabi wrote: > So when Kbuild creates vmlinux.bin, what does it do besides extract the > PT_LOAD > segment? Never mind, I was doing something stupid which was trashing my in-memory copy of vmlinux.bin. -- Timur Tabi Linux kernel developer at Freescale ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 2/2] i2c-ibm_iic driver
On Tuesday 19 February 2008, Stefan Roese wrote: > On Tuesday 19 February 2008, Jean Delvare wrote: > > > > With this Kconfig change, "make menuconfig" lets me select the > > i2c-ibm_iic driver on x86_64, but it fails to build horribly. I think > > that you want to restrict the build to PPC machines somehow, or at > > least make sure that either IBM_OCP or OF support is present. > > How about this: > > - depends on IBM_OCP > + depends on 4xx I think we should allow it to be built on other platforms as well, as long as they have of_platform_device support. The Axon south bridge used on IBMs QS21 blade probably has an ibm_iic, even though it's managed by the firmware and we probably don't want to use it at this time, someone could use the same chip in a new design and actually do that. In general, I also like to make it possible to enable drivers just for the benefit of compile testing, even for stuff that you can't find in any existing HW configuration, so as long as it builds on a platform, I think we shouldn't forbid it: - depends on IBM_OCP + depends on IBM_OCP || PPC_MERGE Arnd <>< ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Difference between vmlinux and vmlinux.bin
I'm trying to write an ELF loader for a PowerPC vmlinux, and I've come across something I don't understand. In vmlinux, there are two Program Segments, the first of which is PT_LOAD. What is the difference between the block of data inside this section, and vmlinux.bin? I thought that vmlinux.bin is nothing more than the PT_LOAD section of vmlinux. However, when I do a memcmp, there is a difference at offset 3080192 (vmlinux.bin is 4874532 bytes long): memcmp @ 3080192 125 != 31 32 bytes @ 412f: 7d 20 00 28 31 29 ff ff 7d 20 01 2d 40 a2 ff f4 2f 89 00 00 41 9e 01 9c 7f e3 fb 78 4b d7 b0 75 32 bytes @ 4230: 1f 8b 08 08 b7 e7 7a 44 00 03 62 75 73 79 62 6f 78 2d 31 2e 31 2e 33 2e 69 6d 67 00 ec 5d 7d 74 So when Kbuild creates vmlinux.bin, what does it do besides extract the PT_LOAD segment? -- Timur Tabi Linux kernel developer at Freescale ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 1/3] Fix Unlikely(x) == y
On Tuesday 19 February 2008 20:57, Andi Kleen wrote: > On Tue, Feb 19, 2008 at 08:46:46PM +1100, Nick Piggin wrote: > > I think it was just a simple context switch benchmark, but not lmbench > > (which I found to be a bit too variable). But it was a long time ago... > > Do you still have it? > > I thought about writing my own but ended up being too lazy for that @) Had a quick look but couldn't find it. It was just two threads running and switching to each other with a couple of mutexes or yield. If I find it, then I'll send it over. > > > > Actually one thing I don't like about gcc is that I think it still > > > > emits cmovs for likely/unlikely branches, > > > > > > That's -Os. > > > > And -O2 and -O3, on the gccs that I'm using, AFAIKS. > > Well if it still happens on gcc 4.2 with P4 tuning you should > perhaps open a gcc PR. They tend to ignore these bugs mostly in > my experience, but sometimes they act on them. I'm not sure about P4 tuning... But even IMO it should not on predictable branches too much for any (especially OOOE) CPU. > > > > which is silly (the gcc developers > > > > > > It depends on the CPU. e.g. on K8 and P6 using CMOV if possible > > > makes sense. P4 doesn't like it though. > > > > If the branch is completely predictable (eg. annotated), then I > > think branches should be used anyway. Even on well predicted > > branches, cmov is similar speed on microbenchmarks, but it will > > increase data hazards I think, so it will probably be worse for > > some real world situations. > > At least the respective optimization manuals say they should be used. > I presume they only made this recommendation after some extensive > benchmarking. What I have seen is that they tell you definitely not to use it for predictable branches. Eg. the Intel optimization manual says Use the setcc and cmov instructions to eliminate unpredictable conditional branches where possible. Do not do this for predictable branches. Do not use these instructions to eliminate all unpredictable conditional branches, because using these instructions will incur execution overhead due to executing both paths of a conditional branch. In addition, converting conditional branches to cmovs or setcc trades control-flow dependence for data dependence and restricts the capability of the out-of-order engine. > > But a likely branch will be _strongly_ predicted to be taken, > > wheras a lot of the gcc heuristics simply have slightly more or > > slightly less probability. So it's not just a question of which > > way is more likely, but also _how_ likely it is to go that way. > > Yes, but a lot of the heuristics are pretty strong (>80%) and gcc will > act on them unless it has a very strong contra cue. And that should > normally not be the case. True, but if you know a branch is 99%+, then use of likely/unlikely can still be a good idea. 80% may not be enough to choose a branch over a cmov for example. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 2/2] i2c-ibm_iic driver
Stefan Roese wrote: > How about this: > > - depends on IBM_OCP > + depends on 4xx > That's what I did, great minds think alike ;) But the email does not seem to have come through, so I will repost the patch. If you already got an email with the above patch, you can ignore this one ;) Cheers, Sean Signed-off-by: Sean MacLennan <[EMAIL PROTECTED]> --- for-2.6.25/drivers/i2c/busses/orig-i2c-ibm_iic.c2008-02-18 16:36:30.0 -0500 +++ for-2.6.25/drivers/i2c/busses/i2c-ibm_iic.c 2008-02-19 16:46:35.0 -0500 @@ -6,6 +6,9 @@ * Copyright (c) 2003, 2004 Zultys Technologies. * Eugene Surovegin <[EMAIL PROTECTED]> or <[EMAIL PROTECTED]> * + * Copyright (c) 2008 PIKA Technologies + * Sean MacLennan <[EMAIL PROTECTED]> + * * Based on original work by * Ian DaSilva <[EMAIL PROTECTED]> * Armin Kuster <[EMAIL PROTECTED]> @@ -39,12 +42,17 @@ #include #include #include + +#ifdef CONFIG_IBM_OCP #include #include +#else +#include +#endif #include "i2c-ibm_iic.h" -#define DRIVER_VERSION "2.1" +#define DRIVER_VERSION "2.2" MODULE_DESCRIPTION("IBM IIC driver v" DRIVER_VERSION); MODULE_LICENSE("GPL"); @@ -657,6 +665,7 @@ return (u8)((opb + 9) / 10 - 1); } +#ifdef CONFIG_IBM_OCP /* * Register single IIC interface */ @@ -828,5 +837,181 @@ ocp_unregister_driver(&ibm_iic_driver); } +#else /* !CONFIG_IBM_OCP */ + +static int __devinit iic_request_irq(struct of_device *ofdev, +struct ibm_iic_private *dev) +{ + struct device_node *np = ofdev->node; + int irq; + + if (iic_force_poll) + return NO_IRQ; + + irq = irq_of_parse_and_map(np, 0); + if (irq == NO_IRQ) { + dev_err(&ofdev->dev, "irq_of_parse_and_map failed\n"); + return NO_IRQ; + } + + /* Disable interrupts until we finish initialization, assumes +* level-sensitive IRQ setup... +*/ + iic_interrupt_mode(dev, 0); + if (request_irq(irq, iic_handler, 0, "IBM IIC", dev)) { + dev_err(&ofdev->dev, "request_irq %d failed\n", irq); + /* Fallback to the polling mode */ + return NO_IRQ; + } + + return irq; +} + +/* + * Register single IIC interface + */ +static int __devinit iic_probe(struct of_device *ofdev, + const struct of_device_id *match) +{ + struct device_node *np = ofdev->node; + struct ibm_iic_private *dev; + struct i2c_adapter *adap; + const u32 *indexp, *freq; + int ret; + + dev = kzalloc(sizeof(*dev), GFP_KERNEL); + if (!dev) { + dev_err(&ofdev->dev, "failed to allocate device data\n"); + return -ENOMEM; + } + + dev_set_drvdata(&ofdev->dev, dev); + + indexp = of_get_property(np, "index", NULL); + if (!indexp) { + dev_err(&ofdev->dev, "no index specified\n"); + ret = -EINVAL; + goto error_cleanup; + } + dev->idx = *indexp; + + dev->vaddr = of_iomap(np, 0); + if (dev->vaddr == NULL) { + dev_err(&ofdev->dev, "failed to iomap device\n"); + ret = -ENXIO; + goto error_cleanup; + } + + init_waitqueue_head(&dev->wq); + + dev->irq = iic_request_irq(ofdev, dev); + if (dev->irq == NO_IRQ) + dev_warn(&ofdev->dev, "using polling mode\n"); + + /* Board specific settings */ + if (iic_force_fast || of_get_property(np, "fast-mode", NULL)) + dev->fast_mode = 1; + + freq = of_get_property(np, "clock-frequency", NULL); + if (freq == NULL) { + freq = of_get_property(np->parent, "clock-frequency", NULL); + if (freq == NULL) { + dev_err(&ofdev->dev, "Unable to get bus frequency\n"); + ret = -EINVAL; + goto error_cleanup; + } + } + + dev->clckdiv = iic_clckdiv(*freq); + dev_dbg(&ofdev->dev, "clckdiv = %d\n", dev->clckdiv); + + /* Initialize IIC interface */ + iic_dev_init(dev); + + /* Register it with i2c layer */ + adap = &dev->adap; + adap->dev.parent = &ofdev->dev; + strlcpy(adap->name, "IBM IIC", sizeof(adap->name)); + i2c_set_adapdata(adap, dev); + adap->id = I2C_HW_OCP; + adap->class = I2C_CLASS_HWMON; + adap->algo = &iic_algo; + adap->timeout = 1; + adap->nr = dev->idx; + + ret = i2c_add_numbered_adapter(adap); + if (ret < 0) { + dev_err(&ofdev->dev, "failed to register i2c adapter\n"); + goto error_cleanup; + } + + dev_info(&ofdev->dev, "using %s mode\n", +dev->fast_mode ? "fast (400 kHz)" : "standard (100 kHz)"); + + return 0; + +error_cleanup: + if (dev->irq != NO_IRQ) { + iic_interrupt_mode(dev, 0);
Re: [PATCH 2/2] i2c-ibm_iic driver
Signed-off-by: Sean MacLennan <[EMAIL PROTECTED]> --- for-2.6.25/drivers/i2c/busses/orig-i2c-ibm_iic.c2008-02-18 16:36:30.0 -0500 +++ for-2.6.25/drivers/i2c/busses/i2c-ibm_iic.c 2008-02-19 16:46:35.0 -0500 @@ -6,6 +6,9 @@ * Copyright (c) 2003, 2004 Zultys Technologies. * Eugene Surovegin <[EMAIL PROTECTED]> or <[EMAIL PROTECTED]> * + * Copyright (c) 2008 PIKA Technologies + * Sean MacLennan <[EMAIL PROTECTED]> + * * Based on original work by * Ian DaSilva <[EMAIL PROTECTED]> * Armin Kuster <[EMAIL PROTECTED]> @@ -39,12 +42,17 @@ #include #include #include + +#ifdef CONFIG_IBM_OCP #include #include +#else +#include +#endif #include "i2c-ibm_iic.h" -#define DRIVER_VERSION "2.1" +#define DRIVER_VERSION "2.2" MODULE_DESCRIPTION("IBM IIC driver v" DRIVER_VERSION); MODULE_LICENSE("GPL"); @@ -657,6 +665,7 @@ return (u8)((opb + 9) / 10 - 1); } +#ifdef CONFIG_IBM_OCP /* * Register single IIC interface */ @@ -828,5 +837,181 @@ ocp_unregister_driver(&ibm_iic_driver); } +#else /* !CONFIG_IBM_OCP */ + +static int __devinit iic_request_irq(struct of_device *ofdev, +struct ibm_iic_private *dev) +{ + struct device_node *np = ofdev->node; + int irq; + + if (iic_force_poll) + return NO_IRQ; + + irq = irq_of_parse_and_map(np, 0); + if (irq == NO_IRQ) { + dev_err(&ofdev->dev, "irq_of_parse_and_map failed\n"); + return NO_IRQ; + } + + /* Disable interrupts until we finish initialization, assumes +* level-sensitive IRQ setup... +*/ + iic_interrupt_mode(dev, 0); + if (request_irq(irq, iic_handler, 0, "IBM IIC", dev)) { + dev_err(&ofdev->dev, "request_irq %d failed\n", irq); + /* Fallback to the polling mode */ + return NO_IRQ; + } + + return irq; +} + +/* + * Register single IIC interface + */ +static int __devinit iic_probe(struct of_device *ofdev, + const struct of_device_id *match) +{ + struct device_node *np = ofdev->node; + struct ibm_iic_private *dev; + struct i2c_adapter *adap; + const u32 *indexp, *freq; + int ret; + + dev = kzalloc(sizeof(*dev), GFP_KERNEL); + if (!dev) { + dev_err(&ofdev->dev, "failed to allocate device data\n"); + return -ENOMEM; + } + + dev_set_drvdata(&ofdev->dev, dev); + + indexp = of_get_property(np, "index", NULL); + if (!indexp) { + dev_err(&ofdev->dev, "no index specified\n"); + ret = -EINVAL; + goto error_cleanup; + } + dev->idx = *indexp; + + dev->vaddr = of_iomap(np, 0); + if (dev->vaddr == NULL) { + dev_err(&ofdev->dev, "failed to iomap device\n"); + ret = -ENXIO; + goto error_cleanup; + } + + init_waitqueue_head(&dev->wq); + + dev->irq = iic_request_irq(ofdev, dev); + if (dev->irq == NO_IRQ) + dev_warn(&ofdev->dev, "using polling mode\n"); + + /* Board specific settings */ + if (iic_force_fast || of_get_property(np, "fast-mode", NULL)) + dev->fast_mode = 1; + + freq = of_get_property(np, "clock-frequency", NULL); + if (freq == NULL) { + freq = of_get_property(np->parent, "clock-frequency", NULL); + if (freq == NULL) { + dev_err(&ofdev->dev, "Unable to get bus frequency\n"); + ret = -EINVAL; + goto error_cleanup; + } + } + + dev->clckdiv = iic_clckdiv(*freq); + dev_dbg(&ofdev->dev, "clckdiv = %d\n", dev->clckdiv); + + /* Initialize IIC interface */ + iic_dev_init(dev); + + /* Register it with i2c layer */ + adap = &dev->adap; + adap->dev.parent = &ofdev->dev; + strlcpy(adap->name, "IBM IIC", sizeof(adap->name)); + i2c_set_adapdata(adap, dev); + adap->id = I2C_HW_OCP; + adap->class = I2C_CLASS_HWMON; + adap->algo = &iic_algo; + adap->timeout = 1; + adap->nr = dev->idx; + + ret = i2c_add_numbered_adapter(adap); + if (ret < 0) { + dev_err(&ofdev->dev, "failed to register i2c adapter\n"); + goto error_cleanup; + } + + dev_info(&ofdev->dev, "using %s mode\n", +dev->fast_mode ? "fast (400 kHz)" : "standard (100 kHz)"); + + return 0; + +error_cleanup: + if (dev->irq != NO_IRQ) { + iic_interrupt_mode(dev, 0); + free_irq(dev->irq, dev); + } + + if (dev->vaddr) + iounmap(dev->vaddr); + + dev_set_drvdata(&ofdev->dev, NULL); + kfree(dev); + return ret; +} + +/* + * Cleanup initialized IIC interface + */ +static int __devexit iic_remove(struct of_device *ofdev) +{ +
Re: [patch 4/4] RFC: PCI: consolidate several pcibios_enable_resources() implementations
> > That is, whatever the arch code decides to use to decide whether > > resources are assigned by firmware or by the first pass assignment code > > or not and collide or not, once that phase is finished (which is the > > case when calling pcibios_enable_device(), having the resource in the > > resource-tree or not is, I believe, the proper way to test whether it's > > a useable resource. > > So should x86 adopt that collision check? I don't hear anything about > actual architecture differences that are behind this implementation > difference. Well, on powerpc we do allow under some circumstances a 0 start value in BARs, which is why I wanted to use a different check. Ben. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCH v2][POWERPC] Fix initial lmb add region with a non-zero base
If we add to an empty lmb region with a non-zero base we will not coalesce the number of regions down to one. This causes problems on ppc32 for the memory region as its assumed to only have one region. We can fix this easily by causing the initial add to replace the dummy region. Signed-off-by: Kumar Gala <[EMAIL PROTECTED]> --- Fix a bug the initial patch introduced if we have a region that gets added at the beginning of the list we wouldn't actually add it. Dave can you replace the patch in you tree with this one. arch/powerpc/mm/lmb.c | 12 1 files changed, 12 insertions(+), 0 deletions(-) diff --git a/arch/powerpc/mm/lmb.c b/arch/powerpc/mm/lmb.c index 4ce23bc..4bf8f19 100644 --- a/arch/powerpc/mm/lmb.c +++ b/arch/powerpc/mm/lmb.c @@ -141,6 +141,12 @@ static long __init lmb_add_region(struct lmb_region *rgn, unsigned long base, unsigned long coalesced = 0; long adjacent, i; + if ((rgn->cnt == 1) && (rgn->region[0].size == 0)) { + rgn->region[0].base = base; + rgn->region[0].size = size; + return 0; + } + /* First try and coalesce this LMB with another. */ for (i=0; i < rgn->cnt; i++) { unsigned long rgnbase = rgn->region[i].base; @@ -185,6 +191,12 @@ static long __init lmb_add_region(struct lmb_region *rgn, unsigned long base, break; } } + + if (base < rgn->region[0].base) { + rgn->region[0].base = base; + rgn->region[0].size = size; + } + rgn->cnt++; return 0; -- 1.5.3.8 ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [patch 0/4] RFC: PCI: consolidate several pcibios_enable_resources() implementations
On Tuesday 19 February 2008 03:08:32 am Benjamin Herrenschmidt wrote: > On Tue, 2008-02-19 at 08:09 +, Russell King wrote: > > On Mon, Feb 18, 2008 at 09:39:52PM -0700, Bjorn Helgaas wrote: > > > > > > ARM and PA-RISC, in particular, have interesting differences: > > > - ARM always enables bridge devices, which no other arch does > > > > ARM does this because there is nothing else which would do that - which > > means devices behind bridges would be completely inaccessible. > > That's normally done by pci_enable_bridges() called by > pci_assign_unassigned_resources(). alpha, mips, mn10300, powerpc, and x86 use pci_enable_bridges() via pci_assign_unassigned_resources(). parisc uses pci_enable_bridges() directly from lba_driver_probe(). I guess the other arches (except arm) rely on firmware to enable bridge. I think pci_assign_unassigned_resources() is a bit of an anachronism -- it's a boot-time thing that does it for all root buses at once. A more hot-plug oriented scheme would do it like parisc, where every time we discover a root bridge, we do any necessary resource assignment and bridge enablement underneath it. For ARM, maybe that could happen in pcibios_init_hw() or something similar. > > > - PA-RISC always turns on SERR and PARITY, which no other arch does > > > > ARM also does this, unless pdev_bad_for_parity(dev) is true. See > > ARMs pcibios_fixup_bus(). > > While that sounds like a good idea to generalize, I think it should > remain arch stuff tho, not move to generic code. > > On some platforms, weirdo firmwares handle error handling and will be > unhappy if the kernel mucks around (such as pSeries). I agree the SERR and PARITY stuff probably needs to remain arch code. But it would be nice to at least have this and the bridge enable done in similar places across arches. Bjorn ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [patch 4/4] RFC: PCI: consolidate several pcibios_enable_resources() implementations
On Tue, Feb 19, 2008 at 09:11:55AM -0700, Bjorn Helgaas wrote: > > That is, whatever the arch code decides to use to decide whether > > resources are assigned by firmware or by the first pass assignment code > > or not and collide or not, once that phase is finished (which is the > > case when calling pcibios_enable_device(), having the resource in the > > resource-tree or not is, I believe, the proper way to test whether it's > > a useable resource. > > So should x86 adopt that collision check? Yes, and other arches as well, I believe. > I don't hear anything about > actual architecture differences that are behind this implementation > difference. On ppc64 "0" can be a valid value for resource start, as far as I know. Ivan. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Status of I2C on mpc5200
Hello all, I have a question regarding the stability of I2C on mpc52xx. According to menuconfig's help it isn't still stable as it says: "The driver may also work on 52xx family processors, though interrupts are known not to work" However there has been a patch from Domen Puncer that apparently fixed the driver in 2.6.23. commit 254db9b5e7b1b0d38a4f177c2c23a5685c78221a Author: Domen Puncer <[EMAIL PROTECTED]> Date: Thu Jul 12 14:12:31 2007 +0200 i2c-mpc: work around missing-9th-clock-pulse bug Work around a problem reported on: http://ozlabs.org/pipermail/linuxppc-embedded/2005-July/019038.html Without this patch I2C on mpc5200 becomes unusable after a while. Tested on mpc5200 boards by Matthias Fechner and me. Signed-off-by: Domen Puncer <[EMAIL PROTECTED]> Signed-off-by: Jean Delvare <[EMAIL PROTECTED]> If any kind soul could provide a status I'd be glad to provide the patch for the help file... Thanks, Eric Dujardin " Ce courriel et les documents qui y sont attaches peuvent contenir des informations confidentielles. Si vous n'etes pas le destinataire escompte, merci d'en informer l'expediteur immediatement et de detruire ce courriel ainsi que tous les documents attaches de votre systeme informatique. Toute divulgation, distribution ou copie du present courriel et des documents attaches sans autorisation prealable de son emetteur est interdite." " This e-mail and any attached documents may contain confidential or proprietary information. If you are not the intended recipient, please advise the sender immediately and delete this e-mail and all attached documents from your computer system. Any unauthorised disclosure, distribution or copying hereof is prohibited."___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] mpc i2c driver, compare to NO_IRQ instead of zero
Hi Jon, On Mon, 21 Jan 2008 15:07:40 -0500, Jon Smirl wrote: > Alter the mpc i2c driver to use the NO_IRQ symbol instead of > the constant zero when checking for valid interrupts. NO_IRQ=-1 > on ppc and NO_IRQ=0 on powerpc so the checks against zero are > not correct. Using NO_IRQ sounds good, just one question: > > Signed-off-by: Jon Smirl <[EMAIL PROTECTED]> > --- > > drivers/i2c/busses/i2c-mpc.c | 10 +- > 1 files changed, 5 insertions(+), 5 deletions(-) > > > diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c > index bbe787b..d20959d 100644 > --- a/drivers/i2c/busses/i2c-mpc.c > +++ b/drivers/i2c/busses/i2c-mpc.c > @@ -99,7 +99,7 @@ static int i2c_wait(struct mpc_i2c *i2c, unsigned timeout, > int writing) > u32 x; > int result = 0; > > - if (i2c->irq == 0) > + if (i2c->irq == NO_IRQ) > { > while (!(readb(i2c->base + MPC_I2C_SR) & CSR_MIF)) { > schedule(); > @@ -329,7 +329,7 @@ static int fsl_i2c_probe(struct platform_device *pdev) > return -ENOMEM; > > i2c->irq = platform_get_irq(pdev, 0); > - if (i2c->irq < 0) { > + if (i2c->irq < NO_IRQ) { I am skeptical about this one. Can platform_get_irq() really return NO_IRQ? I thought that the IRQ resource would be plain missing if the device has no IRQ, so I would expect: i2c->irq = platform_get_irq(pdev, 0); if (i2c->irq < 0) i2c->irq = NO_IRQ; /* Use polling */ Testing against NO_IRQ suggests that devices with no IRQ would still have an IRQ resource defined and explicitly set to NO_IRQ. Sounds weird to me. Can you please clarify this point? For what it's worth, no other kernel driver checks for irq < NO_IRQ. They all check for irq < 0 after calling platform_get_irq(). > result = -ENXIO; > goto fail_get_irq; > } > @@ -344,7 +344,7 @@ static int fsl_i2c_probe(struct platform_device *pdev) > goto fail_map; > } > > - if (i2c->irq != 0) > + if (i2c->irq != NO_IRQ) > if ((result = request_irq(i2c->irq, mpc_i2c_isr, > IRQF_SHARED, "i2c-mpc", i2c)) < 0) { > printk(KERN_ERR > @@ -367,7 +367,7 @@ static int fsl_i2c_probe(struct platform_device *pdev) > return result; > >fail_add: > - if (i2c->irq != 0) > + if (i2c->irq != NO_IRQ) > free_irq(i2c->irq, i2c); >fail_irq: > iounmap(i2c->base); > @@ -384,7 +384,7 @@ static int fsl_i2c_remove(struct platform_device *pdev) > i2c_del_adapter(&i2c->adap); > platform_set_drvdata(pdev, NULL); > > - if (i2c->irq != 0) > + if (i2c->irq != NO_IRQ) > free_irq(i2c->irq, i2c); > > iounmap(i2c->base); The rest looks good. -- Jean Delvare ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] Convert pfc8563 i2c driver from old style to new style
On Tue, 19 Feb 2008 17:24:50 +0100, Alessandro Zummo wrote: > On Tue, 19 Feb 2008 16:10:20 +0100 > Jean Delvare <[EMAIL PROTECTED]> wrote: > > > Hi Jon, > > > > On Mon, 21 Jan 2008 15:09:01 -0500, Jon Smirl wrote: > > > Convert pfc8563 i2c driver from old style to new style. > > > > > > Signed-off-by: Jon Smirl <[EMAIL PROTECTED]> > > > --- > > > > > > drivers/rtc/rtc-pcf8563.c | 107 > > > +++-- > > > 1 files changed, 27 insertions(+), 80 deletions(-) > > > > Preliminary note: this driver belongs to the RTC subsystem, so it will > > be up to Alessandro Zummo (Cc'd) to push this patch upstream... when it > > is ready to go there. > > A conversion to new style for this driver has already been > pushed upstream. Adapting platforms to instantiate the device > should be fairly easy, especially when you use the dts. Ah, excellent. I didn't know, thanks for pointing it out. I'm glad I added you to Cc :) -- Jean Delvare ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] Convert pfc8563 i2c driver from old style to new style
On Tue, 19 Feb 2008 16:10:20 +0100 Jean Delvare <[EMAIL PROTECTED]> wrote: > Hi Jon, > > On Mon, 21 Jan 2008 15:09:01 -0500, Jon Smirl wrote: > > Convert pfc8563 i2c driver from old style to new style. > > > > Signed-off-by: Jon Smirl <[EMAIL PROTECTED]> > > --- > > > > drivers/rtc/rtc-pcf8563.c | 107 > > +++-- > > 1 files changed, 27 insertions(+), 80 deletions(-) > > Preliminary note: this driver belongs to the RTC subsystem, so it will > be up to Alessandro Zummo (Cc'd) to push this patch upstream... when it > is ready to go there. A conversion to new style for this driver has already been pushed upstream. Adapting platforms to instantiate the device should be fairly easy, especially when you use the dts. -- Best regards, Alessandro Zummo, Tower Technologies - Torino, Italy http://www.towertech.it ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Unaligned LocalPlus Bus access on MPC5200
Hello, I've encountered with the problem of unaligned word access to external devices (Flash memory) connected to Local Plus bus of MPC5200 processor. Any comments on this would be very appreciated. And the essence of the issue is as follows: - when I try to read a data word from LPB-connected Flash using some even address (0xFF00, 0xFF02, 0xFF04, etc), then everything works fine (it's not a typo, 2bytes-aligned word accesses pass well too); - when I try to read a data word from LPB-connected Flash using some odd address (0xFF01, 0xFF03, ...) then LP returns only 1 byte of word correctly (the 3 bytes remained are filled with zeros). (a) Here is what I have when I read from LPB using word-aligned accesses with MPC5200 rev.A and LPB configured in Non_Multiplexed mode + 8-bit data bus: lwz from 0xc3082000:0xba476bc7 lwz from 0xc3082004:0xb95d77de ... Now I try the unaligned reads: lwz from 0xc3082000:0xba476bc7 lwz from 0xc3082001:0x00b9 lwz from 0xc3082002:0x6bc7b95d lwz from 0xc3082003:0xc700 (b) With MPC5200 rev.B situation is similar, and different only in fact that unaligned read results to 2-bytes reading for 0x1 addresses cases (LPB again configured in Non_Multiplexed mode + 8-bit data bus): lwz from 0xd1082000:0x2f459eaf lwz from 0xd1082004:0x388ff68d ... lwz from 0xd1082000:0x2f459eaf lwz from 0xd1082001:0xaf38 lwz from 0xd1082002:0x9eaf388f lwz from 0xd1082003:0xaf00 (c) When LPB operates in the Multiplexed mode with 32-bit data bus, the erroneous result is observed for 0x3 addresses cases only (MPC5200 has rev.A in these tests): lwz from 0xc3082000:0x7e9043a6 lwz from 0xc3082004:0x7eb143a6 ... lwz from 0xc3082000:0x7e9043a6 lwz from 0xc3082001:0x9043a67e lwz from 0xc3082002:0x43a67eb1 lwz from 0xc3082003:0xa600 I used the following platforms for tests: - TQM5200 board, which is based on MPC5200rev.A CPU, and has AMD Flash connected to LPB configured in Multiplexed mode with 32-bit data bus; - some customed board, which is based on MPC5200rev.A CPU, and has Intel Flash connected to LPB configured in Non-Multiplexed mode with 8-bit data bus; - Lite5200B board, which is based on MPC5200rev.B CPU, and has AMD Flash connected to LPB configured in Non-Multiplexed mode with 8-bit data bus. The Linux source tree I used is linux-2.6.23.16 (DENX linux-2.6.23-stable branch). The toolchain is ELDK-4.2. As an example, these LPB-related issue leads to the incorrect operation of JFFS2 file-system created on the top of MTD device built on a Flash chip from Intel/Sharp (drivers/mtd/chips/cfi_cmdset_0001.c). With these Flash chips implementation of point/unpoint API is possible, so the cfi_cmdset_0001.c driver exports the corresponding point/unpoint methods for the MTD device, and JFFS2 then uses these methods to operate with data directly from Flash (without copying them to RAM memory). One of these operations is memcpy() in the jffs2_scan_dirent_node() function, which copies the file name from some address at Flash (aligned) to, unfortunately, unaligned destination in RAM ("name" field of the jffs2_full_dirent structure). The implementation of memcpy() in lib_powerpc first does byte-to-byte transfers to achieve the aligned destination, and then does word-to-word transfers, but by this moment the source is unaligned, so memcpy() does lwz-s from unaligned addresses on LPB. Just FYI, a simple work-around for the issue with the Intel/Sharp Flash chips connected to LPB of MPC5200 is to mark your struct map_info as .phys = NO_XIP, and implement read/write/copy_from/copy_to byte-to-byte functions (in your drivers/mtd/maps/ board file). Regards, Yuri -- Yuri Tikhonov, Senior Software Engineer Emcraft Systems, www.emcraft.com ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [patch 4/4] RFC: PCI: consolidate several pcibios_enable_resources() implementations
On Monday 18 February 2008 11:31:07 pm Benjamin Herrenschmidt wrote: > > On Mon, 2008-02-18 at 21:39 -0700, Bjorn Helgaas wrote: > > powerpc: has a different collision check at (5) > > I've always found the collision check dodgy. I tend to want to keep > the way powerpc does it here. > > pci_enable_device() should only enable resources that have successfully > been added to the resource tree (that have passed all the collision > check etc...). There is a simple & clear indication of that: res->parent > is non-NULL. I think that is a better check than the test x86 does on > start and end. > > That is, whatever the arch code decides to use to decide whether > resources are assigned by firmware or by the first pass assignment code > or not and collide or not, once that phase is finished (which is the > case when calling pcibios_enable_device(), having the resource in the > resource-tree or not is, I believe, the proper way to test whether it's > a useable resource. So should x86 adopt that collision check? I don't hear anything about actual architecture differences that are behind this implementation difference. Bjorn ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Réf. : Re: [PATCH] fix 2.6.25-rc2 compilation with CONFIG_SPI_MPC52xx_PSC
OK great. Additionnally, the FIFO_52xx macro probably belongs to mpc52xx_psc.h. Eric Dujardin --- Sagem DS - DP Combat Terrestre 178 rue de Paris - F-91344 Massy Cedex (33) 1 69196792 (direct) (33) 1 69196904 (fax) --- "Grant Likely" <[EMAIL PROTECTED]> Envoyé par : [EMAIL PROTECTED] 19/02/2008 16:09 Remis le : 19/02/2008 16:09 Pour : EricDuj <[EMAIL PROTECTED]> cc :linuxppc-dev@ozlabs.org, Rigby John-R61273 <[EMAIL PROTECTED]> Objet : Re: [PATCH] fix 2.6.25-rc2 compilation with CONFIG_SPI_MPC52xx_PSC On Feb 19, 2008 7:46 AM, EricDuj <[EMAIL PROTECTED]> wrote: Yes, your right. There were changes made to the psc structures to support the 5121 but as you've discovered they didn't get propagated to the spi driver. Your patch is close, but I need to look at it a bit deeper today to decide if it is the correct fix. Cheers, g. -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev " Ce courriel et les documents qui y sont attaches peuvent contenir des informations confidentielles. Si vous n'etes pas le destinataire escompte, merci d'en informer l'expediteur immediatement et de detruire ce courriel ainsi que tous les documents attaches de votre systeme informatique. Toute divulgation, distribution ou copie du present courriel et des documents attaches sans autorisation prealable de son emetteur est interdite." " This e-mail and any attached documents may contain confidential or proprietary information. If you are not the intended recipient, please advise the sender immediately and delete this e-mail and all attached documents from your computer system. Any unauthorised disclosure, distribution or copying hereof is prohibited."___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Patch fixing rmmod -> kernel panic for Peak CAN driver for MPC5200B
Here is a patch that solves the problem with kernel panic when removing the peak-linux-driver-3.17 on a MPC5200B. If someone else apart from us have experienced that. The patches to peak-linux-driver-3.17 needed to run on MPC5200B are: peak-linux-driver-3.17-mpc5200.patch (DENX) peak-linux-driver-3.17-mpc5200-platform_driver.patch (Sylvain Munaut & MontaVista) Mattias diff --exclude CVS -uNr peak-linux-driver-3.17/driver/src/pcan_mpc5200.c peak-linux-driver-3.17.modified/driver/src/pcan_mpc5200.c --- peak-linux-driver-3.17/driver/src/pcan_mpc5200.c2008-02-19 16:06:54.0 +0100 +++ peak-linux-driver-3.17.modified/driver/src/pcan_mpc5200.c 2008-02-19 16:06:46.0 +0100 @@ -752,7 +752,16 @@ { DPRINTK(KERN_DEBUG "%s: mgt_mscan_cleanup()\n", DEVICE_NAME); - platform_driver_unregister(&mscan_driver); + /* In mgt_mscan_init platform_driver_register is called only once +* for module 1. +* We should only call platform_driver_unregister once and we do it +* for module 1 since it is the last module to cleanup. +*/ + if (!(dev->port.mscan.module_num-1)) { + DPRINTK(KERN_DEBUG "platform_driver_unregister(&mscan_driver)\n"); + (void) platform_driver_unregister(&mscan_driver); + } + #if 0 /* seems like it's been freed already */ mgt_mscan_free_irq(dev); ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] Convert pfc8563 i2c driver from old style to new style
On Tue, 19 Feb 2008 10:17:43 -0500, Jon Smirl wrote: > On 2/19/08, Jean Delvare <[EMAIL PROTECTED]> wrote: > > Hi Jon, > > > > On Mon, 21 Jan 2008 15:09:01 -0500, Jon Smirl wrote: > > > Convert pfc8563 i2c driver from old style to new style. > > Let's just forget about this patch until the dynamic module loading > support goes into the base. Why? I don't see the rationale. New-style drivers are good even without dynamic module loading. > The Phytec PCM030 mpc5200 board uses this chip. They are staying out > of tree because they've integrated real-time support which is not in > mainline yet. -- Jean Delvare ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
[patch v7 2/4] USB: add Cypress c67x00 OTG controller core driver
This patch add the core driver for the c67x00 USB OTG controller. The core driver is responsible for the platform bus binding and creating either USB HCD or USB Gadget instances for each of the serial interface engines on the chip. This driver does not directly implement the HCD or gadget behaviours; it just controls access to the chip. Signed-off-by: Peter Korsgaard <[EMAIL PROTECTED]> Acked-by: David Brownell <[EMAIL PROTECTED]> --- MAINTAINERS |6 + drivers/usb/c67x00/c67x00-drv.c | 232 include/linux/usb/c67x00.h | 48 3 files changed, 286 insertions(+) Index: linux-2.6/drivers/usb/c67x00/c67x00-drv.c === --- /dev/null +++ linux-2.6/drivers/usb/c67x00/c67x00-drv.c @@ -0,0 +1,232 @@ +/* + * c67x00-drv.c: Cypress C67X00 USB Common infrastructure + * + * Copyright (C) 2006-2008 Barco N.V. + *Derived from the Cypress cy7c67200/300 ezusb linux driver and + *based on multiple host controller drivers inside the linux kernel. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, + * MA 02110-1301 USA. + */ + +/* + * This file implements the common infrastructure for using the c67x00. + * It is both the link between the platform configuration and subdrivers and + * the link between the common hardware parts and the subdrivers (e.g. + * interrupt handling). + * + * The c67x00 has 2 SIE's (serial interface engine) wich can be configured + * to be host, device or OTG (with some limitations, E.G. only SIE1 can be OTG). + * + * Depending on the platform configuration, the SIE's are created and + * the corresponding subdriver is initialized (c67x00_probe_sie). + */ + +#include +#include +#include +#include +#include + +#include "c67x00.h" + +static void c67x00_probe_sie(struct c67x00_sie *sie, +struct c67x00_device *dev, int sie_num) +{ + spin_lock_init(&sie->lock); + sie->dev = dev; + sie->sie_num = sie_num; + sie->mode = c67x00_sie_config(dev->pdata->sie_config, sie_num); + + switch (sie->mode) { + case C67X00_SIE_UNUSED: + dev_info(sie_dev(sie), +"Not using SIE %d as requested\n", sie->sie_num); + break; + + default: + dev_err(sie_dev(sie), + "Unsupported configuration: 0x%x for SIE %d\n", + sie->mode, sie->sie_num); + break; + } +} + +static void c67x00_remove_sie(struct c67x00_sie *sie) +{ +} + +static irqreturn_t c67x00_irq(int irq, void *__dev) +{ + struct c67x00_device *c67x00 = __dev; + struct c67x00_sie *sie; + u16 msg, int_status; + int i, count = 8; + + int_status = c67x00_ll_hpi_status(c67x00); + if (!int_status) + return IRQ_NONE; + + while (int_status != 0 && (count-- >= 0)) { + c67x00_ll_irq(c67x00, int_status); + for (i = 0; i < C67X00_SIES; i++) { + sie = &c67x00->sie[i]; + msg = 0; + if (int_status & SIEMSG_FLG(i)) + msg = c67x00_ll_fetch_siemsg(c67x00, i); + if (sie->irq) + sie->irq(sie, int_status, msg); + } + int_status = c67x00_ll_hpi_status(c67x00); + } + + if (int_status) + dev_warn(&c67x00->pdev->dev, "Not all interrupts handled! " +"status = 0x%04x\n", int_status); + + return IRQ_HANDLED; +} + +/* - */ + +static int __devinit c67x00_drv_probe(struct platform_device *pdev) +{ + struct c67x00_device *c67x00; + struct c67x00_platform_data *pdata; + struct resource *res, *res2; + int ret, i; + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + if (!res) + return -ENODEV; + + res2 = platform_get_resource(pdev, IORESOURCE_IRQ, 0); + if (!res2) + return -ENODEV; + + pdata = pdev->dev.platform_data; + if (!pdata) + return -ENODEV; + + c67x00 = kzalloc(sizeof(*c67x00), GFP_KERNEL); +
[patch v7 4/4] USB: add Cypress c67x00 OTG controller gadget driver
This patch adds USB gadget support for the Cypress c67x00 family of devices. This is work in progress and not ready to be committed yet. I'm posting this to show how it fits with the rest of the driver and to collect feedback. The driver works good enought to use g_serial, but there are still issues to be solved. The biggest issue is that endpoint 0 is currently handled by the BIOS inside the c67x00, so the gadget stack never sees the data. The BIOS also has other deficiencies, E.G. see the patching done in c67x00_ll_susb_init(). --- drivers/usb/Kconfig|2 drivers/usb/Makefile |2 drivers/usb/c67x00/Kconfig | 21 drivers/usb/c67x00/Makefile|7 drivers/usb/c67x00/c67x00-drv.c| 11 drivers/usb/c67x00/c67x00-ll-hpi.c | 201 drivers/usb/c67x00/c67x00-udc.c| 905 + drivers/usb/c67x00/c67x00-udc.h| 50 ++ drivers/usb/c67x00/c67x00.h| 21 drivers/usb/gadget/Kconfig |7 drivers/usb/gadget/gadget_chips.h |8 drivers/usb/host/Kconfig | 12 12 files changed, 1232 insertions(+), 15 deletions(-) Index: linux-2.6/drivers/usb/c67x00/c67x00-udc.c === --- /dev/null +++ linux-2.6/drivers/usb/c67x00/c67x00-udc.c @@ -0,0 +1,905 @@ +/* + * c67x00-udc.c: Cypress C67X00 USB device controller + * + * Copyright (C) 2006-2008 Barco N.V. + *Derived from the Cypress cy7c67200/300 ezusb linux driver and + *based on multiple device controller drivers inside the linux kernel. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, + * MA 02110-1301 USA. + */ + +#include +#include +#include +#include +#include + +#include "c67x00.h" +#include "c67x00-udc.h" + +/* Defined in DEVICE n ENDPOINT STATUS REGISTERS */ +#define OVERFLOW_FLG 0x0800 /* Receive overflow */ +#define UNDERFLOW_FLG 0x0400 /* Receive underflow */ +#define OUT_EXCEPTION_FLG 0x0200 /* OUT received when armed for IN */ +#define IN_EXCEPTION_FLG 0x0100 /* IN received when armed for OUT */ +#define STALL_FLG 0x0080 /* Stall sent */ +#define NAK_FLG0x0040 /* NAK sent */ +#define LENGTH_EXCEPT_FLG 0x0020 /* Overflow or Underflow occured */ +#define SETUP_FLG 0x0010 /* SETUP packet received */ +#define SEQ_STAT 0x0008 /* Last Data Toggle Sequence bit sent + or received */ +#define TIMEOUT_FLG0x0004 /* Last transmission timed out */ +#define ERROR_FLG 0x0002 /* CRC Err detected in last + reception*/ +#define ACK_FLG0x0001 /* Last transaction ACK'D (sent + or received) */ + +/* Defined in DEVICE n ENDPOINT CONTROL REGISTERS */ +#define DIR_SEL_IN 0x0004 /* Last transmission timed out */ +#define EP_ENABLE 0x0002 /* Enable Endpoint */ + + + +struct c67x00_request { + struct usb_request req; + struct list_head queue; +}; + + + +struct c67x00_udc_ep { + struct usb_ep ep; + struct c67x00_udc *udc; + + struct list_head queue; + int ep_num; + int is_ep_in; + int enable; + int stopped; + int start_io; +}; + +#define C67X00_MAX_NB_END_POINTS 8 + +struct c67x00_udc { + spinlock_t lock; + struct c67x00_sie *sie; + struct usb_gadget gadget; + struct usb_gadget_driver *driver; + struct c67x00_udc_ep ep[C67X00_MAX_NB_END_POINTS]; + struct work_struct io_work; + int config_nr; + /* The highest string descriptor entry + (used to retrieve descriptors from gadget driver) */ + int top_str_id; + u16 string_desc_addr; +}; + +const static unsigned char get_descriptor_device[] = { + USB_DIR_IN | USB_TYPE_STANDARD | USB_RECIP_DEVICE, + USB_REQ_GET_DESCRIPTOR, + 0x00, + USB_DT_DEVICE, + 0x00, + 0x00, + 0x12, + 0x00 +}; + +const static unsigned char get_descriptor_config[] = { + USB_DIR_IN | USB_TYPE_STANDARD | USB_RECIP_DEVICE, + USB_REQ_GET_DESCRIPTOR, + 0x00, + USB_DT_CONFIG, + 0x00, + 0x00, +
[patch v7 0/4] Cypress c67x00 (EZ-Host/EZ-OTG) support
The Cypress c67x00 (EZ-Host/EZ-OTG) controllers are multi-role low/fullspeed USB controllers. This patch series implements a HCD driver and shows the work-in-progress status of a gadget driver. I believe patch 1..3 are ready, and I would like to see them queued up for mainline. Changes since v6: - Addressed David and Alan's comments (removed done list + tasklet) Changes since v5: - Merged c67x00_ll_{get,set}_siemsg() into c67x00_ll_fetch_siemsg(). - Fix for interrupt race condition at probe time (reported by Grant) Changes since v4: - Addressed Grant's comments (c67x00_dev->c67x00_hcd_dev, label indent) - Moved c67x00_ll_set_ep_{ctrl,packet_size}_reg() to patch 4 as they are only needed for gadget support. - Added c67x00_ prefix to struct lcp_int_data Changes since v3: - Lots of cleanups: checkpatch, interrupt handling, c67x00_ prefixes, .. - The dummy platform_device's created per serial engine are gone. - Gadget driver (WIP) -- Bye, Peter Korsgaard ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
[patch v7 1/4] USB: add Cypress c67x00 low level interface code
This patch adds the low level support code for the Cypress c67x00 family of OTG controllers. The low level code is responsible for register access and implements the software protocol for communicating with the 16bit microcontroller inside the c67x00 device. Communication is done over the HPI interface (16bit SRAM-like parallel bus). Signed-off-by: Peter Korsgaard <[EMAIL PROTECTED]> Acked-by: David Brownell <[EMAIL PROTECTED]> --- drivers/usb/c67x00/c67x00-ll-hpi.c | 410 + drivers/usb/c67x00/c67x00.h| 285 + 2 files changed, 695 insertions(+) Index: linux-2.6/drivers/usb/c67x00/c67x00.h === --- /dev/null +++ linux-2.6/drivers/usb/c67x00/c67x00.h @@ -0,0 +1,285 @@ +/* + * c67x00.h: Cypress C67X00 USB register and field definitions + * + * Copyright (C) 2006-2008 Barco N.V. + *Derived from the Cypress cy7c67200/300 ezusb linux driver and + *based on multiple host controller drivers inside the linux kernel. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, + * MA 02110-1301 USA. + */ + +#ifndef _USB_C67X00_H +#define _USB_C67X00_H + +#include +#include +#include +#include + +/* - + * Cypress C67x00 register definitions + */ + +/* Hardware Revision Register */ +#define HW_REV_REG 0xC004 + +/* General USB registers */ +/* = */ + +/* USB Control Register */ +#define USB_CTL_REG(x) ((x) ? 0xC0AA : 0xC08A) + +#define LOW_SPEED_PORT(x) ((x) ? 0x0800 : 0x0400) +#define HOST_MODE 0x0200 +#define PORT_RES_EN(x) ((x) ? 0x0100 : 0x0080) +#define SOF_EOP_EN(x) ((x) ? 0x0002 : 0x0001) + +/* USB status register - Notice it has different content in hcd/udc mode */ +#define USB_STAT_REG(x)((x) ? 0xC0B0 : 0xC090) + +#define EP0_IRQ_FLG0x0001 +#define EP1_IRQ_FLG0x0002 +#define EP2_IRQ_FLG0x0004 +#define EP3_IRQ_FLG0x0008 +#define EP4_IRQ_FLG0x0010 +#define EP5_IRQ_FLG0x0020 +#define EP6_IRQ_FLG0x0040 +#define EP7_IRQ_FLG0x0080 +#define RESET_IRQ_FLG 0x0100 +#define SOF_EOP_IRQ_FLG0x0200 +#define ID_IRQ_FLG 0x4000 +#define VBUS_IRQ_FLG 0x8000 + +/* USB Host only registers */ +/* === */ + +/* Host n Control Register */ +#define HOST_CTL_REG(x)((x) ? 0xC0A0 : 0xC080) + +#define PREAMBLE_EN0x0080 /* Preamble enable */ +#define SEQ_SEL0x0040 /* Data Toggle Sequence Bit Select */ +#define ISO_EN 0x0010 /* Isochronous enable */ +#define ARM_EN 0x0001 /* Arm operation */ + +/* Host n Interrupt Enable Register */ +#define HOST_IRQ_EN_REG(x) ((x) ? 0xC0AC : 0xC08C) + +#define SOF_EOP_IRQ_EN 0x0200 /* SOF/EOP Interrupt Enable */ +#define SOF_EOP_TMOUT_IRQ_EN 0x0800 /* SOF/EOP Timeout Interrupt Enable */ +#define ID_IRQ_EN 0x4000 /* ID interrupt enable */ +#define VBUS_IRQ_EN0x8000 /* VBUS interrupt enable */ +#define DONE_IRQ_EN0x0001 /* Done Interrupt Enable */ + +/* USB status register */ +#define HOST_STAT_MASK 0x02FD +#define PORT_CONNECT_CHANGE(x) ((x) ? 0x0020 : 0x0010) +#define PORT_SE0_STATUS(x) ((x) ? 0x0008 : 0x0004) + +/* Host Frame Register */ +#define HOST_FRAME_REG(x) ((x) ? 0xC0B6 : 0xC096) + +#define HOST_FRAME_MASK0x07FF + +/* USB Peripheral only registers */ +/* = */ + +/* Device n Port Sel reg */ +#define DEVICE_N_PORT_SEL(x) ((x) ? 0xC0A4 : 0xC084) + +/* Device n Interrupt Enable Register */ +#define DEVICE_N_IRQ_EN_REG(x) ((x) ? 0xC0AC : 0xC08C) + +#define DEVICE_N_ENDPOINT_N_CTL_REG(dev, ep) ((dev) \ +? (0x0280 + (ep << 4)) \ +: (0x0200 + (ep << 4))) +#define DEVICE_N_ENDPOINT_N_STAT_REG(dev, ep) ((dev) \ +? (0x0286 + (ep << 4)) \ +: (0x0206 + (ep << 4))) + +
[patch v7 3/4] USB: add Cypress c67x00 OTG controller HCD driver
This patch adds HCD support for the Cypress c67x00 family of devices. Signed-off-by: Peter Korsgaard <[EMAIL PROTECTED]> --- drivers/usb/Makefile |2 drivers/usb/c67x00/Makefile| 11 drivers/usb/c67x00/c67x00-drv.c| 13 drivers/usb/c67x00/c67x00-hcd.c| 409 drivers/usb/c67x00/c67x00-hcd.h| 150 drivers/usb/c67x00/c67x00-ll-hpi.c | 75 ++ drivers/usb/c67x00/c67x00-sched.c | 1170 + drivers/usb/c67x00/c67x00.h|9 drivers/usb/host/Kconfig | 12 9 files changed, 1851 insertions(+) Index: linux-2.6/drivers/usb/c67x00/c67x00-hcd.c === --- /dev/null +++ linux-2.6/drivers/usb/c67x00/c67x00-hcd.c @@ -0,0 +1,409 @@ +/* + * c67x00-hcd.c: Cypress C67X00 USB Host Controller Driver + * + * Copyright (C) 2006-2008 Barco N.V. + *Derived from the Cypress cy7c67200/300 ezusb linux driver and + *based on multiple host controller drivers inside the linux kernel. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, + * MA 02110-1301 USA. + */ + +#include +#include +#include + +#include "c67x00.h" +#include "c67x00-hcd.h" + +/* -- + * Root Hub Support + */ + +static __u8 c67x00_hub_des[] = { + 0x09, /* __u8 bLength; */ + 0x29, /* __u8 bDescriptorType; Hub-descriptor */ + 0x02, /* __u8 bNbrPorts; */ + 0x00, /* __u16 wHubCharacteristics; */ + 0x00, /* (per-port OC, no power switching) */ + 0x32, /* __u8 bPwrOn2pwrGood; 2ms */ + 0x00, /* __u8 bHubContrCurrent; 0 mA */ + 0x00, /* __u8 DeviceRemovable; ** 7 Ports max ** */ + 0xff, /* __u8 PortPwrCtrlMask; ** 7 ports max ** */ +}; + +static void c67x00_hub_reset_host_port(struct c67x00_sie *sie, int port) +{ + struct c67x00_hcd *c67x00 = sie->private_data; + unsigned long flags; + + c67x00_ll_husb_reset(sie, port); + + spin_lock_irqsave(&c67x00->lock, flags); + c67x00_ll_husb_reset_port(sie, port); + spin_unlock_irqrestore(&c67x00->lock, flags); + + c67x00_ll_set_husb_eot(sie->dev, DEFAULT_EOT); +} + +static int c67x00_hub_status_data(struct usb_hcd *hcd, char *buf) +{ + struct c67x00_hcd *c67x00 = hcd_to_c67x00_hcd(hcd); + struct c67x00_sie *sie = c67x00->sie; + u16 status; + int i; + + *buf = 0; + status = c67x00_ll_usb_get_status(sie); + for (i = 0; i < C67X00_PORTS; i++) + if (status & PORT_CONNECT_CHANGE(i)) + *buf |= (1 << i); + + /* bit 0 denotes hub change, b1..n port change */ + *buf <<= 1; + + return !!*buf; +} + +static int c67x00_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue, + u16 wIndex, char *buf, u16 wLength) +{ + struct c67x00_hcd *c67x00 = hcd_to_c67x00_hcd(hcd); + struct c67x00_sie *sie = c67x00->sie; + u16 status, usb_status; + int len = 0; + unsigned int port = wIndex-1; + u16 wPortChange, wPortStatus; + + switch (typeReq) { + + case GetHubStatus: + *(__le32 *) buf = cpu_to_le32(0); + len = 4;/* hub power */ + break; + + case GetPortStatus: + if (wIndex > C67X00_PORTS) + return -EPIPE; + + status = c67x00_ll_usb_get_status(sie); + usb_status = c67x00_ll_get_usb_ctl(sie); + + wPortChange = 0; + if (status & PORT_CONNECT_CHANGE(port)) + wPortChange |= USB_PORT_STAT_C_CONNECTION; + + wPortStatus = USB_PORT_STAT_POWER; + if (!(status & PORT_SE0_STATUS(port))) + wPortStatus |= USB_PORT_STAT_CONNECTION; + if (usb_status & LOW_SPEED_PORT(port)) { + wPortStatus |= USB_PORT_STAT_LOW_SPEED; + c67x00->low_speed_ports |= (1 << port); + } else + c67x00->low_speed_ports &= ~(1 << port);
Re: [PATCH] Convert pfc8563 i2c driver from old style to new style
On 2/19/08, Jean Delvare <[EMAIL PROTECTED]> wrote: > Hi Jon, > > On Mon, 21 Jan 2008 15:09:01 -0500, Jon Smirl wrote: > > Convert pfc8563 i2c driver from old style to new style. Let's just forget about this patch until the dynamic module loading support goes into the base. The Phytec PCM030 mpc5200 board uses this chip. They are staying out of tree because they've integrated real-time support which is not in mainline yet. -- Jon Smirl [EMAIL PROTECTED] ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] fix 2.6.25-rc2 build with CONFIG_SPI_MPC52xx_PSC
On Feb 19, 2008 7:50 AM, EricDuj <[EMAIL PROTECTED]> wrote: > > Hello again, > > Here is a second patch as the link phase fails in the same configuration. > The error is the following: > > > Find the patch below. > Best, > Eric Thanks. This looks right. In order to pick up this patch, I need you to reply with your "Signed-off-by:" line. Cheers, g. > > === > --- arch/powerpc/platforms/52xx/mpc52xx_common.c.orig 2008-02-19 > 14:14:52.0 +0100 > +++ arch/powerpc/platforms/52xx/mpc52xx_common.c2008-02-19 > 14:12:56.0 +0100 > @@ -199,6 +199,7 @@ > > return 0; > } > +EXPORT_SYMBOL(mpc52xx_set_psc_clkdiv); > > /** > * mpc52xx_restart: ppc_md->restart hook for mpc5200 using the watchdog > timer > > > > -- > View this message in context: > http://www.nabble.com/-PATCH--fix-2.6.25-rc2-compilation-with-CONFIG_SPI_MPC52xx_PSC-tp15560559p15560565.html > Sent from the linuxppc-dev mailing list archive at Nabble.com. > > ___ > Linuxppc-dev mailing list > Linuxppc-dev@ozlabs.org > https://ozlabs.org/mailman/listinfo/linuxppc-dev > -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] Convert pfc8563 i2c driver from old style to new style
Hi Jon, On Mon, 21 Jan 2008 15:09:01 -0500, Jon Smirl wrote: > Convert pfc8563 i2c driver from old style to new style. > > Signed-off-by: Jon Smirl <[EMAIL PROTECTED]> > --- > > drivers/rtc/rtc-pcf8563.c | 107 > +++-- > 1 files changed, 27 insertions(+), 80 deletions(-) Preliminary note: this driver belongs to the RTC subsystem, so it will be up to Alessandro Zummo (Cc'd) to push this patch upstream... when it is ready to go there. I am also adding Clemens Koller to Cc. Clemens, as you sent patches to the pcf8563 driver, I guess that you are using this driver. Please let us know on which architecture and whether the platform code was already updated to instantiate the pcf8563 (or rtc8564) device. Right now it seems that only two ixp4xx platforms have been updated to do so (dsmg600 and nas100d). I am worried that the conversion to a new-style driver could break some systems, although that should be fairly easy to fix. Review: > diff --git a/drivers/rtc/rtc-pcf8563.c b/drivers/rtc/rtc-pcf8563.c > index b3317fc..8eff549 100644 > --- a/drivers/rtc/rtc-pcf8563.c > +++ b/drivers/rtc/rtc-pcf8563.c > @@ -25,10 +25,6 @@ > * located at 0x51 will pass the validation routine due to > * the way the registers are implemented. > */ > -static const unsigned short normal_i2c[] = { I2C_CLIENT_END }; > - > -/* Module parameters */ > -I2C_CLIENT_INSMOD; The comment above can probably go away as well now, it no longer makes sense without the normal_i2c declaration. > > #define PCF8563_REG_ST1 0x00 /* status */ > #define PCF8563_REG_ST2 0x01 > @@ -72,9 +68,6 @@ struct pcf8563 { > int c_polarity; /* 0: MO_C=1 means 19xx, otherwise MO_C=1 means 20xx */ > }; > > -static int pcf8563_probe(struct i2c_adapter *adapter, int address, int kind); > -static int pcf8563_detach(struct i2c_client *client); > - > /* > * In the routines that deal directly with the pcf8563 hardware, we use > * rtc_time -- month 0-11, hour 0-23, yr = calendar year-epoch. > @@ -257,98 +250,52 @@ static const struct rtc_class_ops pcf8563_rtc_ops = { > .set_time = pcf8563_rtc_set_time, > }; > > -static int pcf8563_attach(struct i2c_adapter *adapter) > +static int pcf8563_remove(struct i2c_client *client) > { > - return i2c_probe(adapter, &addr_data, pcf8563_probe); > + struct rtc_device *rtc = i2c_get_clientdata(client); > + > + if (rtc) > + rtc_device_unregister(rtc); > + > + return 0; > } Moving this function where pcf8563_detach() was would result in a smaller, more readable patch. > > +static struct i2c_device_id pcf8563_id[] = { This structure and all the surrounding infrastructure don't exist yet, so you can't use this. This part of the code will be added later to all the new-style i2c device drivers at once, you don't have to worry about this. > + {"pcf8563", 0}, > + {"rtc8564", 0}, > + {}, > +}; > +MODULE_DEVICE_TABLE(i2c, pcf8563_id); > + > +static int pcf8563_probe(struct i2c_client *client, const struct > i2c_device_id *id); > + > static struct i2c_driver pcf8563_driver = { > .driver = { > - .name = "pcf8563", > + .name = "rtc-pcf8563", > }, > .id = I2C_DRIVERID_PCF8563, It's probably the right time to get rid of I2C_DRIVERID_PCF8563 (here and in ). > - .attach_adapter = &pcf8563_attach, > - .detach_client = &pcf8563_detach, > + .probe = &pcf8563_probe, > + .remove = &pcf8563_remove, Use tabs before the equal sign to be consistent with the rest of the structure declaration. > + .id_table = pcf8563_id, > }; > > -static int pcf8563_probe(struct i2c_adapter *adapter, int address, int kind) > +static int pcf8563_probe(struct i2c_client *client, const struct > i2c_device_id *id) > { > - struct pcf8563 *pcf8563; > - struct i2c_client *client; > + int result; > struct rtc_device *rtc; > > - int err = 0; > - > - dev_dbg(&adapter->dev, "%s\n", __FUNCTION__); > - > - if (!i2c_check_functionality(adapter, I2C_FUNC_I2C)) { > - err = -ENODEV; > - goto exit; > - } > - > - if (!(pcf8563 = kzalloc(sizeof(struct pcf8563), GFP_KERNEL))) { > - err = -ENOMEM; > - goto exit; > - } So you no longer allocate a struct pcf8563. How are pcf8563_get_datetime() and pcf8563_set_datetime() supposed to work? Both do: struct pcf8563 *pcf8563 = container_of(client, struct pcf8563, client); While I agree that struct pcf8563 must no longer contain a struct i2c_client for a new-style driver, there's still c_polarity which needs to be carried around and handled. You did not actually test your patch, did you? I think that you will have to add a struct rtc_device *rtc; to struct pcf8563, as you can't set the client data to both the struct rtc_device address and the struct pcf8563 address. This is what the pcf8583 driv
Re: [PATCH] fix 2.6.25-rc2 compilation with CONFIG_SPI_MPC52xx_PSC
On Feb 19, 2008 7:46 AM, EricDuj <[EMAIL PROTECTED]> wrote: > > Hi all, > > I am trying to cross compile the vanilla 2.6.25-rc2 kernel with the Denx 4.1 > toolchain (gcc-4.0.0 based). > The build fails with the following error when CONFIG_SPI_MPC52xx_PSC is set: > > CC drivers/spi/spi.o > LD drivers/spi/built-in.o > CC [M] drivers/spi/mpc52xx_psc_spi.o > drivers/spi/mpc52xx_psc_spi.c: In function 'mpc52xx_psc_spi_transfer_rxtx': > drivers/spi/mpc52xx_psc_spi.c:193: error: 'struct mpc52xx_psc' has no member > named 'rfalarm' > drivers/spi/mpc52xx_psc_spi.c:197: error: 'struct mpc52xx_psc' has no member > named 'rfnum' > drivers/spi/mpc52xx_psc_spi.c: In function 'mpc52xx_psc_spi_port_config': > drivers/spi/mpc52xx_psc_spi.c:349: error: 'struct mpc52xx_psc' has no member > named 'rfcntl' > make[2]: *** [drivers/spi/mpc52xx_psc_spi.o] Erreur 1 > make[1]: *** [drivers/spi] Erreur 2 > make: *** [drivers] Erreur 2 > > > A quick look at the code shows a mismatch between mpc52xx_psc.c and > mpc52xx_psc_spi.c. > Assuming mpc52xx_psc.c is right, here's my take at a patch (not tested on > real hardware, > needs review !). At least it fixes the build. Yes, your right. There were changes made to the psc structures to support the 5121 but as you've discovered they didn't get propagated to the spi driver. Your patch is close, but I need to look at it a bit deeper today to decide if it is the correct fix. Cheers, g. -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [i2c] [PATCH] Convert pfc8563 i2c driver from old style to new style
Hi Jon, On Fri, 25 Jan 2008 12:16:46 -0500, Jon Smirl wrote: > Any final objections to this patch? Jean can you pick it up? Sorry for the delay, I'll review your patch right now. Note that the driver is really named pcf8563 not pfc8563. -- Jean Delvare ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [BUG][PATCH] 2.6.25-rc2 build fails on mpc52xx_psc_spi
Hello again, Here is a second patch as the link fails with the same configuration. The error is the following: Image Name: Linux-2.6.25-rc2 Created: Tue Feb 19 13:57:46 2008 Image Type: PowerPC Linux Kernel Image (gzip compressed) Data Size:1662959 Bytes = 1623.98 kB = 1.59 MB Load Address: 0x0040 Entry Point: 0x00400550 Building modules, stage 2. MODPOST 24 modules ERROR: "mpc52xx_set_psc_clkdiv" [drivers/spi/mpc52xx_psc_spi.ko] undefined! make[1]: *** [__modpost] Erreur 1 make: *** [modules] Erreur 2 Here is the patch: --- arch/powerpc/platforms/52xx/mpc52xx_common.c.orig 2008-02-19 14:14:52.0 +0100 +++ arch/powerpc/platforms/52xx/mpc52xx_common.c2008-02-19 14:12:56.0 +0100 @@ -199,6 +199,7 @@ return 0; } +EXPORT_SYMBOL(mpc52xx_set_psc_clkdiv); /** * mpc52xx_restart: ppc_md->restart hook for mpc5200 using the watchdog timer Best, Eric EricDuj wrote: > > Hi all, > > I am trying to cross compile the vanilla 2.6.25-rc2 kernel with the Denx > 4.1 toolchain (gcc-4.0.0 based). > > -- View this message in context: http://www.nabble.com/-BUG--PATCH--2.6.25-rc2-build-fails-on-mpc52xx_psc_spi-tp15560559p15560565.html Sent from the linuxppc-dev mailing list archive at Nabble.com. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
[BUG][PATCH] 2.6.25-rc2 build fails on mpc52xx_psc_spi
Hi all, I am trying to cross compile the vanilla 2.6.25-rc2 kernel with the Denx 4.1 toolchain (gcc-4.0.0 based). The build fails with the following error: CC drivers/spi/spi.o LD drivers/spi/built-in.o CC [M] drivers/spi/mpc52xx_psc_spi.o drivers/spi/mpc52xx_psc_spi.c: In function 'mpc52xx_psc_spi_transfer_rxtx': drivers/spi/mpc52xx_psc_spi.c:193: error: 'struct mpc52xx_psc' has no member named 'rfalarm' drivers/spi/mpc52xx_psc_spi.c:197: error: 'struct mpc52xx_psc' has no member named 'rfnum' drivers/spi/mpc52xx_psc_spi.c: In function 'mpc52xx_psc_spi_port_config': drivers/spi/mpc52xx_psc_spi.c:349: error: 'struct mpc52xx_psc' has no member named 'rfcntl' make[2]: *** [drivers/spi/mpc52xx_psc_spi.o] Erreur 1 make[1]: *** [drivers/spi] Erreur 2 make: *** [drivers] Erreur 2 See the .config below. A quick look at the code shows a mismatch between mpc52xx_psc.c and mpc52xx_psc_spi.c. Assuming mpc52xx_psc.c is right, here's my take at a patch (not tested on real hardware, needs review !). At least it fixes the build. --- drivers/spi/mpc52xx_psc_spi.c.orig 2008-02-19 13:47:54.0 +0100 +++ drivers/spi/mpc52xx_psc_spi.c 2008-02-19 13:47:58.0 +0100 @@ -134,11 +134,15 @@ /* wake up when 80% fifo full */ #define MPC52xx_PSC_RFALARM (MPC52xx_PSC_BUFSIZE * 20 / 100) +#define FIFO_52xx_PSC(psc) ((struct mpc52xx_psc_fifo __iomem *)(psc+1)) + static int mpc52xx_psc_spi_transfer_rxtx(struct spi_device *spi, struct spi_transfer *t) { struct mpc52xx_psc_spi *mps = spi_master_get_devdata(spi->master); struct mpc52xx_psc __iomem *psc = mps->psc; + struct mpc52xx_psc_fifo __iomem *fifo = FIFO_52xx_PSC(psc); + unsigned rb = 0;/* number of bytes receieved */ unsigned sb = 0;/* number of bytes sent */ unsigned char *rx_buf = (unsigned char *)t->rx_buf; @@ -190,11 +194,11 @@ out_8(&psc->mode, 0); } else { out_8(&psc->mode, MPC52xx_PSC_MODE_FFULL); - out_be16(&psc->rfalarm, rfalarm); + out_be16(&fifo->rfalarm, rfalarm); } out_be16(&psc->mpc52xx_psc_imr, MPC52xx_PSC_IMR_RXRDY); wait_for_completion(&mps->done); - recv_at_once = in_be16(&psc->rfnum); + recv_at_once = in_be16(&fifo->rfnum); dev_dbg(&spi->dev, "%d bytes received\n", recv_at_once); send_at_once = recv_at_once; @@ -331,6 +335,8 @@ static int mpc52xx_psc_spi_port_config(int psc_id, struct mpc52xx_psc_spi *mps) { struct mpc52xx_psc __iomem *psc = mps->psc; + struct mpc52xx_psc_fifo __iomem *fifo = FIFO_52xx_PSC(psc); + u32 mclken_div; int ret = 0; @@ -346,7 +352,7 @@ /* Disable interrupts, interrupts are based on alarm level */ out_be16(&psc->mpc52xx_psc_imr, 0); out_8(&psc->command, MPC52xx_PSC_SEL_MODE_REG_1); - out_8(&psc->rfcntl, 0); + out_8(&fifo->rfcntl, 0); out_8(&psc->mode, MPC52xx_PSC_MODE_FFULL); /* Configure 8bit codec mode as a SPI master and use EOF flags */ Thanks for your attention, Eric - # # Automatically generated make config: don't edit # Linux kernel version: 2.6.25-rc2 # Tue Feb 19 12:50:28 2008 # # CONFIG_PPC64 is not set # # Processor support # CONFIG_6xx=y # CONFIG_PPC_85xx is not set # CONFIG_PPC_8xx is not set # CONFIG_40x is not set # CONFIG_44x is not set # CONFIG_E200 is not set CONFIG_PPC_FPU=y # CONFIG_ALTIVEC is not set CONFIG_PPC_STD_MMU=y CONFIG_PPC_STD_MMU_32=y # CONFIG_PPC_MM_SLICES is not set # CONFIG_SMP is not set CONFIG_PPC32=y CONFIG_WORD_SIZE=32 CONFIG_PPC_MERGE=y CONFIG_MMU=y CONFIG_GENERIC_CMOS_UPDATE=y CONFIG_GENERIC_TIME=y CONFIG_GENERIC_TIME_VSYSCALL=y CONFIG_GENERIC_CLOCKEVENTS=y CONFIG_GENERIC_HARDIRQS=y # CONFIG_HAVE_SETUP_PER_CPU_AREA is not set CONFIG_IRQ_PER_CPU=y CONFIG_RWSEM_XCHGADD_ALGORITHM=y CONFIG_ARCH_HAS_ILOG2_U32=y CONFIG_GENERIC_HWEIGHT=y CONFIG_GENERIC_CALIBRATE_DELAY=y CONFIG_GENERIC_FIND_NEXT_BIT=y # CONFIG_ARCH_NO_VIRT_TO_BUS is not set CONFIG_PPC=y CONFIG_EARLY_PRINTK=y CONFIG_GENERIC_NVRAM=y CONFIG_SCHED_NO_NO_OMIT_FRAME_POINTER=y CONFIG_ARCH_MAY_HAVE_PC_FDC=y CONFIG_PPC_OF=y CONFIG_OF=y # CONFIG_PPC_UDBG_16550 is not set # CONFIG_GENERIC_TBSYNC is not set CONFIG_AUDIT_ARCH=y CONFIG_GENERIC_BUG=y CONFIG_DEFAULT_UIMAGE=y CONFIG_ARCH_SUSPEND_POSSIBLE=y # CONFIG_PPC_DCR_NATIVE is not set # CONFIG_PPC_DCR_MMIO is not set CONFIG_DEFCONFIG_LIST="/lib/modules/$UNAME_RELEASE/.config" # # General setup # CONFIG_EXPERIMENTAL=y CONFIG_BROKEN_ON_SMP=y CONFIG_LOCK_KERNEL=y CONFIG_INIT_ENV_ARG_LIMIT=32 CONFIG_LOCALVERSION="" CONFIG_LOCALVERSION_AUTO=y CONFIG_SWAP=y CONFIG_SYSVIPC=y CONFIG_SYSVIPC_SYSCTL=y # CONFIG_POSIX_MQUEUE is not set # CONFIG_BSD_PROCESS_ACCT is not set # CONFIG_TASKSTATS is not set # CONFI
Re: [BUG] Linux 2.6.25-rc2 - Regression from 2.6.24-rc1-git1 softlockup while bootup on powerpc
Jens Axboe wrote: > On Tue, Feb 19 2008, KAMEZAWA Hiroyuki wrote: >> On Sun, 17 Feb 2008 20:29:13 +0100 >> Jens Axboe <[EMAIL PROTECTED]> wrote: >> >>> It's odd stuff. Could you perhaps try and add some printks to >>> block/cfq-iosched.c:call_for_each_cic(), like dumping the 'nr' return >>> from radix_tree_gang_lookup() and the pointer value of cics[i] in the >>> for() loop after the lookup? >>> >> I met the same issue on ia64/NUMA box. >> seems cisc[]->key is NULL and index for radix_tree_gang_lookup() was >> always '1'. > > Why does it keep repeating then? If ->key is NULL, the next lookup index > should be 1UL. > > But I think the radix 'scan over entire tree' is a bit fragile. This > patch adds a parallel hlist for ease of properly browsing the members, > does that work for you? It compiles, but I haven't booted it here yet... > >> Attached patch works well for me, but I don't know much about cfq. >> please confirm. > > It doesn't make a lot of sense, I'm afraid. > > block/blk-ioc.c | 35 +++ > block/cfq-iosched.c | 37 +++-- > include/linux/iocontext.h |2 ++ > 3 files changed, 28 insertions(+), 46 deletions(-) > > diff --git a/block/blk-ioc.c b/block/blk-ioc.c > index 80245dc..73c7002 100644 > --- a/block/blk-ioc.c Hi Jens, Thanks for the patch. The patch works fine, machine boots up without the kernel panic. -- Thanks & Regards, Kamalesh Babulal, Linux Technology Center, IBM, ISTL. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [Linux-fbdev-devel] [PATCH 1/2] fb: add support for foreign endianness
Andrew Morton schrieb: > On Tue, 19 Feb 2008 12:27:54 +0100 Clemens Koller <[EMAIL PROTECTED]> wrote: >> Benjamin Herrenschmidt schrieb: >>> On Tue, 2008-02-19 at 00:35 +0100, Clemens Koller wrote: [EMAIL PROTECTED] schrieb: > On Mon, 18 Feb 2008 08:18:47 +0100, Krzysztof Helt said: >> I know two fb drivers which use endianess information (pm2fb and >> s3c2410fb). >> Both resolve endianess at driver level. Actually, both handle it by >> setting special >> bits so the graphics chip itself reorder bytes to transform foreign >> endianess. >> I understand that this patch is for chips which cannot reorder bytes by >> themselves. > Does anybody know of such a chip that's actually available in the wild? > Or are > we writing drivers for speculative possible chips? > I had troubles with the Silicon Motion SM501/SM502 endianess on PowerPC PCI vs. LocalBus. The chip also has a register to swap endianess, but that seems to only affect some LocalBus modes. The current fb and X drivers are working, but when it comes to font aliasing and hw-acceleration, the problems start to rise again... >>> Most "sane" gfx chips nowadays provide configurable surfaces that allow >>> to perform the swap when writing/reading from regions of the >>> framebuffer, with the ability to set a different swapper setting (based >>> on bit depth) per region. >> Most! But not the SM50x. I still hope I would be wrong here. :-( >> >>> Then there is also the risk that your PCI<->Localbus has been wired >>> improperly :-) >> That's not an issue in my case. The SM50x can be connected to >> either an PCI or some Local/CPU-whateverbus IF. >> I.e. on the MPC85xx PowerPC, PCI and LocalBus are separate bussses. >> If the sm501 is attached to the MPC85xx' PCI like any other video card, >> the PCI config-space is can be accessed as usual, whereas the framebuffer >> memory area is byte-swapped compared to other common video cards. >> >> So, to get back on topic: >> I would welcome endianess swapping in SW. Some architectures (PowerPC) >> should also be able to do swapped-endian mmapping. I just haven't >> had time for a closer look but it looks also interesting way to do it >> that way. > > Bizarrely, the original author of the patch (Anton) has fallen off the cc. > Could whoever did that please thwap himself? Propably my bad, being subscribed to several CCed lists... > Anyway, my head is now officially spinning. Did anyone actually have a > reason why we shouldn't proceed with Anton's patch? Since it seem that there are some odd chips out in the wild, I guess Valdis (also readded to CC:) has no more objections to give it a try. :-) Regards, Clemens ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [Linux-fbdev-devel] [PATCH 1/2] fb: add support for foreign endianness
On Tue, 19 Feb 2008 12:27:54 +0100 Clemens Koller <[EMAIL PROTECTED]> wrote: > Benjamin Herrenschmidt schrieb: > > On Tue, 2008-02-19 at 00:35 +0100, Clemens Koller wrote: > >> [EMAIL PROTECTED] schrieb: > >>> On Mon, 18 Feb 2008 08:18:47 +0100, Krzysztof Helt said: > I know two fb drivers which use endianess information (pm2fb and > s3c2410fb). > Both resolve endianess at driver level. Actually, both handle it by > setting special > bits so the graphics chip itself reorder bytes to transform foreign > endianess. > I understand that this patch is for chips which cannot reorder bytes by > themselves. > >>> Does anybody know of such a chip that's actually available in the wild? > >>> Or are > >>> we writing drivers for speculative possible chips? > >>> > >> I had troubles with the Silicon Motion SM501/SM502 endianess on PowerPC > >> PCI vs. LocalBus. > >> The chip also has a register to swap endianess, but that seems to only > >> affect some > >> LocalBus modes. > >> The current fb and X drivers are working, but when it comes to font > >> aliasing and hw-acceleration, the problems start to rise again... > > > > Most "sane" gfx chips nowadays provide configurable surfaces that allow > > to perform the swap when writing/reading from regions of the > > framebuffer, with the ability to set a different swapper setting (based > > on bit depth) per region. > > Most! But not the SM50x. I still hope I would be wrong here. :-( > > > Then there is also the risk that your PCI<->Localbus has been wired > > improperly :-) > > That's not an issue in my case. The SM50x can be connected to > either an PCI or some Local/CPU-whateverbus IF. > I.e. on the MPC85xx PowerPC, PCI and LocalBus are separate bussses. > If the sm501 is attached to the MPC85xx' PCI like any other video card, > the PCI config-space is can be accessed as usual, whereas the framebuffer > memory area is byte-swapped compared to other common video cards. > > So, to get back on topic: > I would welcome endianess swapping in SW. Some architectures (PowerPC) > should also be able to do swapped-endian mmapping. I just haven't > had time for a closer look but it looks also interesting way to do it > that way. Bizarrely, the original author of the patch (Anton) has fallen off the cc. Could whoever did that please thwap himself? Anyway, my head is now officially spinning. Did anyone actually have a reason why we shouldn't proceed with Anton's patch? ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [Linux-fbdev-devel] [PATCH 1/2] fb: add support for foreign endianness
Benjamin Herrenschmidt schrieb: > On Tue, 2008-02-19 at 00:35 +0100, Clemens Koller wrote: >> [EMAIL PROTECTED] schrieb: >>> On Mon, 18 Feb 2008 08:18:47 +0100, Krzysztof Helt said: I know two fb drivers which use endianess information (pm2fb and s3c2410fb). Both resolve endianess at driver level. Actually, both handle it by setting special bits so the graphics chip itself reorder bytes to transform foreign endianess. I understand that this patch is for chips which cannot reorder bytes by themselves. >>> Does anybody know of such a chip that's actually available in the wild? Or >>> are >>> we writing drivers for speculative possible chips? >>> >> I had troubles with the Silicon Motion SM501/SM502 endianess on PowerPC PCI >> vs. LocalBus. >> The chip also has a register to swap endianess, but that seems to only >> affect some >> LocalBus modes. >> The current fb and X drivers are working, but when it comes to font >> aliasing and hw-acceleration, the problems start to rise again... > > Most "sane" gfx chips nowadays provide configurable surfaces that allow > to perform the swap when writing/reading from regions of the > framebuffer, with the ability to set a different swapper setting (based > on bit depth) per region. Most! But not the SM50x. I still hope I would be wrong here. :-( > Then there is also the risk that your PCI<->Localbus has been wired > improperly :-) That's not an issue in my case. The SM50x can be connected to either an PCI or some Local/CPU-whateverbus IF. I.e. on the MPC85xx PowerPC, PCI and LocalBus are separate bussses. If the sm501 is attached to the MPC85xx' PCI like any other video card, the PCI config-space is can be accessed as usual, whereas the framebuffer memory area is byte-swapped compared to other common video cards. So, to get back on topic: I would welcome endianess swapping in SW. Some architectures (PowerPC) should also be able to do swapped-endian mmapping. I just haven't had time for a closer look but it looks also interesting way to do it that way. Regards, Clemens ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [patch 0/4] RFC: PCI: consolidate several pcibios_enable_resources() implementations
On Tue, 2008-02-19 at 08:09 +, Russell King wrote: > On Mon, Feb 18, 2008 at 09:39:52PM -0700, Bjorn Helgaas wrote: > > There are many implementations of pcibios_enable_resources() that differ > > in minor ways that look more like bugs than architectural differences. > > This patch series consolidates most of them to use the x86 version. > > > > This series is for discussion only at this point. I'm interested in > > feedback about whether any of the differences are "real" and need to > > be preserved. > > > > ARM and PA-RISC, in particular, have interesting differences: > > - ARM always enables bridge devices, which no other arch does > > ARM does this because there is nothing else which would do that - which > means devices behind bridges would be completely inaccessible. That's normally done by pci_enable_bridges() called by pci_assign_unassigned_resources(). (The later has a weird naming, it does more than just assign unassigned resources in fact). > > - PA-RISC always turns on SERR and PARITY, which no other arch does > > ARM also does this, unless pdev_bad_for_parity(dev) is true. See > ARMs pcibios_fixup_bus(). While that sounds like a good idea to generalize, I think it should remain arch stuff tho, not move to generic code. On some platforms, weirdo firmwares handle error handling and will be unhappy if the kernel mucks around (such as pSeries). > > Should other arches do the same thing, or are these somehow related to > > ARM and PA-RISC architecture? > > I suspect they're architecture specific; I wouldn't like to do either > on x86, but they're either required or preferred on ARM. Ben. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 1/3] Fix Unlikely(x) == y
On Tue, Feb 19, 2008 at 08:46:46PM +1100, Nick Piggin wrote: > On Tuesday 19 February 2008 20:25, Andi Kleen wrote: > > On Tue, Feb 19, 2008 at 01:33:53PM +1100, Nick Piggin wrote: > > > > I actually once measured context switching performance in the scheduler, > > > and removing the unlikely hint for testing RT tasks IIRC gave about 5% > > > performance drop. > > > > OT: what benchmarks did you use for that? I had a change some time > > ago to the CFS scheduler to avoid unpredicted indirect calls for > > the common case, but I wasn't able to benchmark a difference with the usual > > suspect benchmark (lmbench). Since it increased code size by > > a few bytes it was rejected then. > > I think it was just a simple context switch benchmark, but not lmbench > (which I found to be a bit too variable). But it was a long time ago... Do you still have it? I thought about writing my own but ended up being too lazy for that @) > > > > However, the P4's branch predictor is pretty good, and it should easily > > > > I think it depends on the generation. Prescott class branch > > prediction should be much better than the earlier ones. > > I was using a Nocona Xeon, which I think is a Prescott class? Yes. > And don't they have much higher mispredict penalty (than older P4s)? They do have a longer pipeline, so yes more penalty (5 or 6 stages more iirc), but also a lot better branch predictor which makes up for that. > > > > > Actually one thing I don't like about gcc is that I think it still emits > > > cmovs for likely/unlikely branches, > > > > That's -Os. > > And -O2 and -O3, on the gccs that I'm using, AFAIKS. Well if it still happens on gcc 4.2 with P4 tuning you should perhaps open a gcc PR. They tend to ignore these bugs mostly in my experience, but sometimes they act on them. > > > > > which is silly (the gcc developers > > > > It depends on the CPU. e.g. on K8 and P6 using CMOV if possible > > makes sense. P4 doesn't like it though. > > If the branch is completely predictable (eg. annotated), then I > think branches should be used anyway. Even on well predicted > branches, cmov is similar speed on microbenchmarks, but it will > increase data hazards I think, so it will probably be worse for > some real world situations. At least the respective optimization manuals say they should be used. I presume they only made this recommendation after some extensive benchmarking. > > > > > the quite good numbers that cold CPU predictors can attain. However > > > for really performance critical code (or really "never" executed > > > code), then I think it is OK to have the hints and not have to rely > > > on gcc heuristics. > > > > But only when the explicit hints are different from what the implicit > > branch predictors would predict anyways. And if you look at the > > heuristics that is not often the case... > > But a likely branch will be _strongly_ predicted to be taken, > wheras a lot of the gcc heuristics simply have slightly more or > slightly less probability. So it's not just a question of which > way is more likely, but also _how_ likely it is to go that way. Yes, but a lot of the heuristics are pretty strong (>80%) and gcc will act on them unless it has a very strong contra cue. And that should normally not be the case. -Andi ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 1/3] Fix Unlikely(x) == y
On Tue, Feb 19, 2008 at 01:33:53PM +1100, Nick Piggin wrote: > On Tuesday 19 February 2008 01:39, Andi Kleen wrote: > > Arjan van de Ven <[EMAIL PROTECTED]> writes: > > > you have more faith in the authors knowledge of how his code actually > > > behaves than I think is warranted :) > > > > iirc there was a mm patch some time ago to keep track of the actual > > unlikely values at runtime and it showed indeed some wrong ones. But the > > far majority of them are probably correct. > > > > > Or faith in that he knows what "unlikely" means. > > > I should write docs about this; but unlikely() means: > > > 1) It happens less than 0.01% of the cases. > > > 2) The compiler couldn't have figured this out by itself > > >(NULL pointer checks are compiler done already, same for some other > > > conditions) 3) It's a hot codepath where shaving 0.5 cycles (less even on > > > x86) matters (and the author is ok with taking a 500 cycles hit if he's > > > wrong) > > > > One more thing unlikely() does is to move the unlikely code out of line. > > So it should conserve some icache in critical functions, which might > > well be worth some more cycles (don't have numbers though). > > I actually once measured context switching performance in the scheduler, > and removing the unlikely hint for testing RT tasks IIRC gave about 5% > performance drop. OT: what benchmarks did you use for that? I had a change some time ago to the CFS scheduler to avoid unpredicted indirect calls for the common case, but I wasn't able to benchmark a difference with the usual suspect benchmark (lmbench). Since it increased code size by a few bytes it was rejected then. > > This was on a P4 which is very different from more modern CPUs both in > terms of branch performance characteristics, > and icache characteristics. Hmm, the P4 the trace cache actually should not care about inline code that is not executed. > However, the P4's branch predictor is pretty good, and it should easily I think it depends on the generation. Prescott class branch prediction should be much better than the earlier ones. > Actually one thing I don't like about gcc is that I think it still emits > cmovs for likely/unlikely branches, That's -Os. > which is silly (the gcc developers It depends on the CPU. e.g. on K8 and P6 using CMOV if possible makes sense. P4 doesn't like it though. > the quite good numbers that cold CPU predictors can attain. However > for really performance critical code (or really "never" executed > code), then I think it is OK to have the hints and not have to rely > on gcc heuristics. But only when the explicit hints are different from what the implicit branch predictors would predict anyways. And if you look at the heuristics that is not often the case... Also I really think that mm patch that measured unlikely efficiency should be dug out and merged to mainline and them regularly rechecked. -Andi ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 1/3] Fix Unlikely(x) == y
> Sometimes, for performance critical paths, I would like gcc to be dumb and > follow *my* code and not its hard-coded probabilities. If you really want that, simple: just disable optimization @) > Maybe one thing we would need would be the ability to assign probabilities > to each branch based on what we expect, so that gcc could build a better > tree keeping most frequently used code tight. Just use profile feedback then for user space. I don't think it's a good idea for kernel code though because it leads to unreproducible binaries which would wreck the development model. > Hmm I've just noticed -fno-guess-branch-probability in the man, I never tried > it. Or -fno-reorder-blocks -Andi ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 1/3] Fix Unlikely(x) == y
On Tuesday 19 February 2008 20:25, Andi Kleen wrote: > On Tue, Feb 19, 2008 at 01:33:53PM +1100, Nick Piggin wrote: > > I actually once measured context switching performance in the scheduler, > > and removing the unlikely hint for testing RT tasks IIRC gave about 5% > > performance drop. > > OT: what benchmarks did you use for that? I had a change some time > ago to the CFS scheduler to avoid unpredicted indirect calls for > the common case, but I wasn't able to benchmark a difference with the usual > suspect benchmark (lmbench). Since it increased code size by > a few bytes it was rejected then. I think it was just a simple context switch benchmark, but not lmbench (which I found to be a bit too variable). But it was a long time ago... > > This was on a P4 which is very different from more modern CPUs both in > > terms of branch performance characteristics, > > > > and icache characteristics. > > Hmm, the P4 the trace cache actually should not care about inline > code that is not executed. Yeah, which is why it is a bit different than other CPUs. Although the L2 cache I guess is still going to suffer from sparse code, but I guess that is a bit less important. > > However, the P4's branch predictor is pretty good, and it should easily > > I think it depends on the generation. Prescott class branch > prediction should be much better than the earlier ones. I was using a Nocona Xeon, which I think is a Prescott class? And don't they have much higher mispredict penalty (than older P4s)? > > Actually one thing I don't like about gcc is that I think it still emits > > cmovs for likely/unlikely branches, > > That's -Os. And -O2 and -O3, on the gccs that I'm using, AFAIKS. > > which is silly (the gcc developers > > It depends on the CPU. e.g. on K8 and P6 using CMOV if possible > makes sense. P4 doesn't like it though. If the branch is completely predictable (eg. annotated), then I think branches should be used anyway. Even on well predicted branches, cmov is similar speed on microbenchmarks, but it will increase data hazards I think, so it will probably be worse for some real world situations. > > the quite good numbers that cold CPU predictors can attain. However > > for really performance critical code (or really "never" executed > > code), then I think it is OK to have the hints and not have to rely > > on gcc heuristics. > > But only when the explicit hints are different from what the implicit > branch predictors would predict anyways. And if you look at the > heuristics that is not often the case... But a likely branch will be _strongly_ predicted to be taken, wheras a lot of the gcc heuristics simply have slightly more or slightly less probability. So it's not just a question of which way is more likely, but also _how_ likely it is to go that way. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [BUG] Linux 2.6.25-rc2 - Regression from 2.6.24-rc1-git1 softlockup while bootup on powerpc
On Tue, Feb 19 2008, KAMEZAWA Hiroyuki wrote: > On Tue, 19 Feb 2008 09:58:38 +0100 > Jens Axboe <[EMAIL PROTECTED]> wrote: > > > when I inserted printk here > > > == > > > for (i = 0; i < nr; i++) > > > func(ioc, cics[i]); > > > printk("%d %lx\n", nr, index); > > > == > > > index was always "1" and nr was always 32. > > > > > > So, cics[31]->key was always NULL when index=1 is passed to > > > radix_tree_gang_lookup(). > > > > Hang on, it returned 32? It should not return more than 16, since that > > is what we have room for and asked for. > sorry. Of course, it was 16 ;( I expected so, otherwise we would have had far more serious problems :-) > your patch works well. thank you. It's committed now and posted in the relevant bugzilla as well (#9948). -- Jens Axboe ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [BUG] Linux 2.6.25-rc2 - Regression from 2.6.24-rc1-git1 softlockup while bootup on powerpc
On Tue, 19 Feb 2008 09:58:38 +0100 Jens Axboe <[EMAIL PROTECTED]> wrote: > > when I inserted printk here > > == > > for (i = 0; i < nr; i++) > > func(ioc, cics[i]); > > printk("%d %lx\n", nr, index); > > == > > index was always "1" and nr was always 32. > > > > So, cics[31]->key was always NULL when index=1 is passed to > > radix_tree_gang_lookup(). > > Hang on, it returned 32? It should not return more than 16, since that > is what we have room for and asked for. sorry. Of course, it was 16 ;( your patch works well. thank you. -Kame ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [BUG] Linux 2.6.25-rc2 - Regression from 2.6.24-rc1-git1 softlockup while bootup on powerpc
On Tue, Feb 19 2008, KAMEZAWA Hiroyuki wrote: > On Tue, 19 Feb 2008 09:36:34 +0100 > Jens Axboe <[EMAIL PROTECTED]> wrote: > > > On Tue, Feb 19 2008, KAMEZAWA Hiroyuki wrote: > > > On Sun, 17 Feb 2008 20:29:13 +0100 > > > Jens Axboe <[EMAIL PROTECTED]> wrote: > > > > > > > It's odd stuff. Could you perhaps try and add some printks to > > > > block/cfq-iosched.c:call_for_each_cic(), like dumping the 'nr' return > > > > from radix_tree_gang_lookup() and the pointer value of cics[i] in the > > > > for() loop after the lookup? > > > > > > > I met the same issue on ia64/NUMA box. > > > seems cisc[]->key is NULL and index for radix_tree_gang_lookup() was > > > always '1'. > > > > Why does it keep repeating then? If ->key is NULL, the next lookup index > > should be 1UL. > > > > But I think the radix 'scan over entire tree' is a bit fragile. This > > patch adds a parallel hlist for ease of properly browsing the members, > > does that work for you? It compiles, but I haven't booted it here yet... > > > Works well for me and my box booted ! Super, I'll get it upstream. Thanks for testing and debugging! -- Jens Axboe ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 2/2] i2c-ibm_iic driver
On Tuesday 19 February 2008, Jean Delvare wrote: > > diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig > > index b61f56b..44c0984 100644 > > --- a/drivers/i2c/busses/Kconfig > > +++ b/drivers/i2c/busses/Kconfig > > @@ -244,7 +244,6 @@ config I2C_PIIX4 > > > > config I2C_IBM_IIC > > tristate "IBM PPC 4xx on-chip I2C interface" > > - depends on IBM_OCP > > help > > Say Y here if you want to use IIC peripheral found on > > embedded IBM PPC 4xx based systems. > > With this Kconfig change, "make menuconfig" lets me select the > i2c-ibm_iic driver on x86_64, but it fails to build horribly. I think > that you want to restrict the build to PPC machines somehow, or at > least make sure that either IBM_OCP or OF support is present. How about this: - depends on IBM_OCP + depends on 4xx Best regards, Stefan ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [BUG] Linux 2.6.25-rc2 - Regression from 2.6.24-rc1-git1 softlockup while bootup on powerpc
On Tue, 19 Feb 2008 09:36:34 +0100 Jens Axboe <[EMAIL PROTECTED]> wrote: > On Tue, Feb 19 2008, KAMEZAWA Hiroyuki wrote: > > On Sun, 17 Feb 2008 20:29:13 +0100 > > Jens Axboe <[EMAIL PROTECTED]> wrote: > > > > > It's odd stuff. Could you perhaps try and add some printks to > > > block/cfq-iosched.c:call_for_each_cic(), like dumping the 'nr' return > > > from radix_tree_gang_lookup() and the pointer value of cics[i] in the > > > for() loop after the lookup? > > > > > I met the same issue on ia64/NUMA box. > > seems cisc[]->key is NULL and index for radix_tree_gang_lookup() was > > always '1'. > > Why does it keep repeating then? If ->key is NULL, the next lookup index > should be 1UL. > > But I think the radix 'scan over entire tree' is a bit fragile. This > patch adds a parallel hlist for ease of properly browsing the members, > does that work for you? It compiles, but I haven't booted it here yet... > Works well for me and my box booted ! Thanks, -Kame ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [BUG] Linux 2.6.25-rc2 - Regression from 2.6.24-rc1-git1 softlockup while bootup on powerpc
On Tue, Feb 19 2008, KAMEZAWA Hiroyuki wrote: > On Tue, 19 Feb 2008 09:36:34 +0100 > Jens Axboe <[EMAIL PROTECTED]> wrote: > > > On Tue, Feb 19 2008, KAMEZAWA Hiroyuki wrote: > > > On Sun, 17 Feb 2008 20:29:13 +0100 > > > Jens Axboe <[EMAIL PROTECTED]> wrote: > > > > > > > It's odd stuff. Could you perhaps try and add some printks to > > > > block/cfq-iosched.c:call_for_each_cic(), like dumping the 'nr' return > > > > from radix_tree_gang_lookup() and the pointer value of cics[i] in the > > > > for() loop after the lookup? > > > > > > > I met the same issue on ia64/NUMA box. > > > seems cisc[]->key is NULL and index for radix_tree_gang_lookup() was > > > always '1'. > > > > Why does it keep repeating then? If ->key is NULL, the next lookup index > > should be 1UL. > > > when I inserted printk here > == > for (i = 0; i < nr; i++) > func(ioc, cics[i]); > printk("%d %lx\n", nr, index); > == > index was always "1" and nr was always 32. > > So, cics[31]->key was always NULL when index=1 is passed to > radix_tree_gang_lookup(). Hang on, it returned 32? It should not return more than 16, since that is what we have room for and asked for. Using ->dead_key when ->key is NULL is correct btw, since that is the correct location in the tree once the process has exited. But that should not happen until AFTER the func() call, so I still think the list patch is safer. > > But I think the radix 'scan over entire tree' is a bit fragile. This > > patch adds a parallel hlist for ease of properly browsing the > > members, does that work for you? It compiles, but I haven't booted > > it here yet... > > > will try. please wait a bit. It boots here, so at least it passes normal sanity tests. It should solve your problem as well, hopefully. -- Jens Axboe ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [BUG] Linux 2.6.25-rc2 - Regression from 2.6.24-rc1-git1 softlockup while bootup on powerpc
On Tue, 19 Feb 2008 09:36:34 +0100 Jens Axboe <[EMAIL PROTECTED]> wrote: > On Tue, Feb 19 2008, KAMEZAWA Hiroyuki wrote: > > On Sun, 17 Feb 2008 20:29:13 +0100 > > Jens Axboe <[EMAIL PROTECTED]> wrote: > > > > > It's odd stuff. Could you perhaps try and add some printks to > > > block/cfq-iosched.c:call_for_each_cic(), like dumping the 'nr' return > > > from radix_tree_gang_lookup() and the pointer value of cics[i] in the > > > for() loop after the lookup? > > > > > I met the same issue on ia64/NUMA box. > > seems cisc[]->key is NULL and index for radix_tree_gang_lookup() was > > always '1'. > > Why does it keep repeating then? If ->key is NULL, the next lookup index > should be 1UL. > when I inserted printk here == for (i = 0; i < nr; i++) func(ioc, cics[i]); printk("%d %lx\n", nr, index); == index was always "1" and nr was always 32. So, cics[31]->key was always NULL when index=1 is passed to radix_tree_gang_lookup(). > But I think the radix 'scan over entire tree' is a bit fragile. This > patch adds a parallel hlist for ease of properly browsing the members, > does that work for you? It compiles, but I haven't booted it here yet... > will try. please wait a bit. Thanks, -Kame ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [patch 0/4] RFC: PCI: consolidate several pcibios_enable_resources() implementations
On Mon, Feb 18, 2008 at 09:39:52PM -0700, Bjorn Helgaas wrote: > There are many implementations of pcibios_enable_resources() that differ > in minor ways that look more like bugs than architectural differences. > This patch series consolidates most of them to use the x86 version. > > This series is for discussion only at this point. I'm interested in > feedback about whether any of the differences are "real" and need to > be preserved. > > ARM and PA-RISC, in particular, have interesting differences: > - ARM always enables bridge devices, which no other arch does ARM does this because there is nothing else which would do that - which means devices behind bridges would be completely inaccessible. > - PA-RISC always turns on SERR and PARITY, which no other arch does ARM also does this, unless pdev_bad_for_parity(dev) is true. See ARMs pcibios_fixup_bus(). > Should other arches do the same thing, or are these somehow related to > ARM and PA-RISC architecture? I suspect they're architecture specific; I wouldn't like to do either on x86, but they're either required or preferred on ARM. -- Russell King Linux kernel2.6 ARM Linux - http://www.arm.linux.org.uk/ maintainer of: ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [BUG] Linux 2.6.25-rc2 - Regression from 2.6.24-rc1-git1 softlockup while bootup on powerpc
On Tue, Feb 19 2008, KAMEZAWA Hiroyuki wrote: > On Sun, 17 Feb 2008 20:29:13 +0100 > Jens Axboe <[EMAIL PROTECTED]> wrote: > > > It's odd stuff. Could you perhaps try and add some printks to > > block/cfq-iosched.c:call_for_each_cic(), like dumping the 'nr' return > > from radix_tree_gang_lookup() and the pointer value of cics[i] in the > > for() loop after the lookup? > > > I met the same issue on ia64/NUMA box. > seems cisc[]->key is NULL and index for radix_tree_gang_lookup() was > always '1'. Why does it keep repeating then? If ->key is NULL, the next lookup index should be 1UL. But I think the radix 'scan over entire tree' is a bit fragile. This patch adds a parallel hlist for ease of properly browsing the members, does that work for you? It compiles, but I haven't booted it here yet... > Attached patch works well for me, but I don't know much about cfq. > please confirm. It doesn't make a lot of sense, I'm afraid. block/blk-ioc.c | 35 +++ block/cfq-iosched.c | 37 +++-- include/linux/iocontext.h |2 ++ 3 files changed, 28 insertions(+), 46 deletions(-) diff --git a/block/blk-ioc.c b/block/blk-ioc.c index 80245dc..73c7002 100644 --- a/block/blk-ioc.c +++ b/block/blk-ioc.c @@ -17,17 +17,13 @@ static struct kmem_cache *iocontext_cachep; static void cfq_dtor(struct io_context *ioc) { - struct cfq_io_context *cic[1]; - int r; + if (!hlist_empty(&ioc->cic_list)) { + struct cfq_io_context *cic; - /* -* We don't have a specific key to lookup with, so use the gang -* lookup to just retrieve the first item stored. The cfq exit -* function will iterate the full tree, so any member will do. -*/ - r = radix_tree_gang_lookup(&ioc->radix_root, (void **) cic, 0, 1); - if (r > 0) - cic[0]->dtor(ioc); + cic = list_entry(ioc->cic_list.first, struct cfq_io_context, + cic_list); + cic->dtor(ioc); + } } /* @@ -57,18 +53,16 @@ EXPORT_SYMBOL(put_io_context); static void cfq_exit(struct io_context *ioc) { - struct cfq_io_context *cic[1]; - int r; - rcu_read_lock(); - /* -* See comment for cfq_dtor() -*/ - r = radix_tree_gang_lookup(&ioc->radix_root, (void **) cic, 0, 1); - rcu_read_unlock(); - if (r > 0) - cic[0]->exit(ioc); + if (!hlist_empty(&ioc->cic_list)) { + struct cfq_io_context *cic; + + cic = list_entry(ioc->cic_list.first, struct cfq_io_context, + cic_list); + cic->exit(ioc); + } + rcu_read_unlock(); } /* Called by the exitting task */ @@ -105,6 +99,7 @@ struct io_context *alloc_io_context(gfp_t gfp_flags, int node) ret->nr_batch_requests = 0; /* because this is 0 */ ret->aic = NULL; INIT_RADIX_TREE(&ret->radix_root, GFP_ATOMIC | __GFP_HIGH); + INIT_HLIST_HEAD(&ret->cic_list); ret->ioc_data = NULL; } diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c index ca198e6..62eda3f 100644 --- a/block/cfq-iosched.c +++ b/block/cfq-iosched.c @@ -1145,38 +1145,19 @@ static void cfq_put_queue(struct cfq_queue *cfqq) /* * Call func for each cic attached to this ioc. Returns number of cic's seen. */ -#define CIC_GANG_NR16 static unsigned int call_for_each_cic(struct io_context *ioc, void (*func)(struct io_context *, struct cfq_io_context *)) { - struct cfq_io_context *cics[CIC_GANG_NR]; - unsigned long index = 0; - unsigned int called = 0; - int nr; + struct cfq_io_context *cic; + struct hlist_node *n; + int called = 0; rcu_read_lock(); - - do { - int i; - - /* -* Perhaps there's a better way - this just gang lookups from -* 0 to the end, restarting after each CIC_GANG_NR from the -* last key + 1. -*/ - nr = radix_tree_gang_lookup(&ioc->radix_root, (void **) cics, - index, CIC_GANG_NR); - if (!nr) - break; - - called += nr; - index = 1 + (unsigned long) cics[nr - 1]->key; - - for (i = 0; i < nr; i++) - func(ioc, cics[i]); - } while (nr == CIC_GANG_NR); - + hlist_for_each_entry_rcu(cic, n, &ioc->cic_list, cic_list) { + func(ioc, cic); + called++; + } rcu_read_unlock(); return called; @@ -1190,6 +1171,7 @@ static void cic_free_func(struct io_context *ioc, struct cfq_io_context *cic) spin_lock_irqsave(&ioc->lock, flags); radix_tr
Re: [BUG] Linux 2.6.25-rc2 - Regression from 2.6.24-rc1-git1 softlockup while bootup on powerpc
On Sun, 17 Feb 2008 20:29:13 +0100 Jens Axboe <[EMAIL PROTECTED]> wrote: > It's odd stuff. Could you perhaps try and add some printks to > block/cfq-iosched.c:call_for_each_cic(), like dumping the 'nr' return > from radix_tree_gang_lookup() and the pointer value of cics[i] in the > for() loop after the lookup? > I met the same issue on ia64/NUMA box. seems cisc[]->key is NULL and index for radix_tree_gang_lookup() was always '1'. Attached patch works well for me, but I don't know much about cfq. please confirm. Regards, -Kame == cics[]->key can be NULL. In that case, cics[]->dead_key has key value. Signed-off-by: KAMEZAWA Hiroyuki <[EMAIL PROTECTED]> Index: linux-2.6.25-rc2/block/cfq-iosched.c === --- linux-2.6.25-rc2.orig/block/cfq-iosched.c +++ linux-2.6.25-rc2/block/cfq-iosched.c @@ -1171,7 +1171,11 @@ call_for_each_cic(struct io_context *ioc break; called += nr; - index = 1 + (unsigned long) cics[nr - 1]->key; + + if (!cics[nr - 1]->key) + index = 1 + (unsigned long) cics[nr - 1]->dead_key; + else + index = 1 + (unsigned long) cics[nr - 1]->key; for (i = 0; i < nr; i++) func(ioc, cics[i]); ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 2/2] i2c-ibm_iic driver
Hi Sean, On Mon, 18 Feb 2008 20:42:46 -0500, Sean MacLennan wrote: > An updated version of the patch. This one should answer all of Jean's > concerns. > > Cheers, >Sean > > Signed-off-by: Sean MacLennan <[EMAIL PROTECTED]> > --- > --- for-2.6.25/drivers/i2c/busses/orig-i2c-ibm_iic.c 2008-02-18 > 16:36:30.0 -0500 > +++ for-2.6.25/drivers/i2c/busses/i2c-ibm_iic.c 2008-02-18 > 17:39:53.0 -0500 > @@ -6,6 +6,9 @@ > * Copyright (c) 2003, 2004 Zultys Technologies. > * Eugene Surovegin <[EMAIL PROTECTED]> or <[EMAIL PROTECTED]> > * > + * Copyright (c) 2008 PIKA Technologies > + * Sean MacLennan <[EMAIL PROTECTED]> > + * > * Based on original work by > * Ian DaSilva <[EMAIL PROTECTED]> > * Armin Kuster <[EMAIL PROTECTED]> > @@ -39,12 +42,17 @@ > #include > #include > #include > + > +#ifdef CONFIG_IBM_OCP > #include > #include > +#else > +#include > +#endif > > #include "i2c-ibm_iic.h" > > -#define DRIVER_VERSION "2.1" > +#define DRIVER_VERSION "2.2" > > MODULE_DESCRIPTION("IBM IIC driver v" DRIVER_VERSION); > MODULE_LICENSE("GPL"); > @@ -657,6 +665,7 @@ > return (u8)((opb + 9) / 10 - 1); > } > > +#ifdef CONFIG_IBM_OCP > /* > * Register single IIC interface > */ > @@ -828,5 +837,182 @@ > ocp_unregister_driver(&ibm_iic_driver); > } > > +#else /* !CONFIG_IBM_OCP */ > + > +static int __devinit iic_request_irq(struct of_device *ofdev, > + struct ibm_iic_private *dev) > +{ > + struct device_node *np = ofdev->node; > + int irq; > + > + if (iic_force_poll) > + return NO_IRQ; > + > + irq = irq_of_parse_and_map(np, 0); > + if (irq == NO_IRQ) { > + dev_err(&ofdev->dev, "irq_of_parse_and_map failed\n"); > + return NO_IRQ; > + } > + > + /* Disable interrupts until we finish initialization, assumes > + * level-sensitive IRQ setup... > + */ > + iic_interrupt_mode(dev, 0); > + if (request_irq(irq, iic_handler, 0, "IBM IIC", dev)) { > + dev_err(&ofdev->dev, "request_irq %d failed\n", irq); > + /* Fallback to the polling mode */ > + return NO_IRQ; > + } > + > + return irq; > +} > + > +/* > + * Register single IIC interface > + */ > +static int __devinit iic_probe(struct of_device *ofdev, > +const struct of_device_id *match) > +{ > + struct device_node *np = ofdev->node; > + struct ibm_iic_private *dev; > + struct i2c_adapter *adap; > + const u32 *indexp, *freq; > + int ret; > + > + Double blank line is prohibited inside a function. > + dev = kzalloc(sizeof(*dev), GFP_KERNEL); > + if (!dev) { > + dev_err(&ofdev->dev, "failed to allocate device data\n"); > + return -ENOMEM; > + } > + > + dev_set_drvdata(&ofdev->dev, dev); > + > + indexp = of_get_property(np, "index", NULL); > + if (!indexp) { > + dev_err(&ofdev->dev, "no index specified.\n"); This is the only error message that has a trailing dot. Remove it? > + ret = -EINVAL; Isn't it a bit inconsistent to return -EINVAL here... > + goto error_cleanup; > + } > + dev->idx = *indexp; > + > + dev->vaddr = of_iomap(np, 0); > + if (dev->vaddr == NULL) { > + dev_err(&ofdev->dev, "failed to iomap device\n"); > + ret = -ENXIO; > + goto error_cleanup; > + } > + > + init_waitqueue_head(&dev->wq); > + > + dev->irq = iic_request_irq(ofdev, dev); > + if (dev->irq == NO_IRQ) > + dev_warn(&ofdev->dev, "using polling mode\n"); > + > + /* Board specific settings */ > + if (iic_force_fast || of_get_property(np, "fast-mode", NULL)) > + dev->fast_mode = 1; > + > + freq = of_get_property(np, "clock-frequency", NULL); > + if (freq == NULL) { > + freq = of_get_property(np->parent, "clock-frequency", NULL); > + if (freq == NULL) { > + dev_err(&ofdev->dev, "Unable to get bus frequency\n"); > + ret = -EBUSY; ... but -EBUSY there? > + goto error_cleanup; > + } > + } > + > + dev->clckdiv = iic_clckdiv(*freq); > + dev_dbg(&ofdev->dev, "clckdiv = %d\n", dev->clckdiv); > + > + /* Initialize IIC interface */ > + iic_dev_init(dev); > + > + /* Register it with i2c layer */ > + adap = &dev->adap; > + adap->dev.parent = &ofdev->dev; > + strlcpy(adap->name, "IBM IIC", sizeof(adap->name)); > + i2c_set_adapdata(adap, dev); > + adap->id = I2C_HW_OCP; > + adap->class = I2C_CLASS_HWMON; > + adap->algo = &iic_algo; > + adap->timeout = 1; > + adap->nr = dev->idx; > + > + ret = i2c_add_numbered_adapter(adap); > + if (ret < 0) { > + dev_err(&ofdev->dev, "failed to register i2c adapter\n"); > + goto error_cleanup; > + } > + > + dev_dbg(&ofde