Re: [Patch 3/6] Modify ptrace code to use Hardware Breakpoint interfaces

2009-06-14 Thread David Gibson
On Wed, Jun 10, 2009 at 12:20:02PM +0530, K.Prasad wrote:
> On Fri, Jun 05, 2009 at 03:13:45PM +1000, David Gibson wrote:
> > On Wed, Jun 03, 2009 at 10:05:24PM +0530, K.Prasad wrote:
> > > Modify the ptrace code to use the hardware breakpoint interfaces for 
> > > user-space.
> > > 
> > > Signed-off-by: K.Prasad 
> > > ---
> > >  arch/powerpc/kernel/ptrace.c |   47 
> > > +++
> > >  1 file changed, 47 insertions(+)
> > > 
> > > Index: linux-2.6-tip.hbkpt/arch/powerpc/kernel/ptrace.c
> > > ===
> > > --- linux-2.6-tip.hbkpt.orig/arch/powerpc/kernel/ptrace.c
> > > +++ linux-2.6-tip.hbkpt/arch/powerpc/kernel/ptrace.c
> > > @@ -37,6 +37,7 @@
> > >  #include 
> > >  #include 
> > >  #include 
> > > +#include 
> > >  
> > >  /*
> > >   * does not yet catch signals sent when the child dies.
> > > @@ -735,9 +736,26 @@ void user_disable_single_step(struct tas
> > >   clear_tsk_thread_flag(task, TIF_SINGLESTEP);
> > >  }
> > >  
> > > +void ptrace_triggered(struct hw_breakpoint *bp, struct pt_regs *regs)
> > > +{
> > > + /*
> > > +  * Unregister the breakpoint request here since ptrace has defined a
> > > +  * one-shot behaviour for breakpoint exceptions in PPC64.
> > > +  * The SIGTRAP signal is generated automatically for us in do_dabr().
> > > +  * We don't have to do anything here
> > > +  */
> > > + unregister_user_hw_breakpoint(current, bp);
> > > + kfree(bp);
> > 
> > Couldn't you also clear the saved dabr info here, to avoid having to
> > special case this in the actual breakpoint handler.
> 
> The saved dabr_data is created as a static variable and I didn't want to
> modify its value across files.

Hrm.  I'm not sure which of these options is the uglier, to be honest.

> > Also, I think you should be delivering the signal here - for gdb
> > compatibility I think we'll need to match the old behaviour which has
> > the TRAP delivered before executing the breakpointed instruction.
> > 
> 
> We could do it either way. Return a NOTIFY_DONE from
> hw_breakpoint_handler() and allow the do_dabr()
> code to deliver the signal, or deliver a signal here and return a
> NOTIFY_STOP in the exception handler. I chose the former as it doesn't
> duplicate the code.

I see.  The thing is, since you're aiming to make *the* hardware
breakpoint interface here, I think you really should be rewriting
do_dabr entirely as part of this framework, not just adding your stuff
as one option in there alongside the old-style ways of doing it.

-- 
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@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH v3] powerpc: add ioremap_early() for mapping IO regions before MMU_init()

2009-06-14 Thread Benjamin Herrenschmidt
On Wed, 2009-05-27 at 12:55 -0600, Grant Likely wrote:
> From: Grant Likely 
> 
> ioremap_early() is useful for things like mapping SoC internally registers
> and early debug output because it allows mappings to devices to be setup
> early in the boot process where they are needed.  It also give a
> performance boost since BAT mapped registers don't get flushed out of
> the TLB.
> 
> Without ioremap_early(), early mappings are set up in an ad-hoc manner
> and they get lost when the MMU is set up.  Drivers then have to perform
> hacky fixups to transition over to new mappings.
> 
> Signed-off-by: Grant Likely 
> ---

Approach looks sane at first glance.

However, I'm reluctant to but that in until we have all MMU types
covered or we'll have "interesting" surprises. Also, the CPM patch
doesn't actually fix the massive bogon in there :-)

> + /* Be loud and annoying if someone calls this too late.
> +  * No need to crash the kernel though */
> + WARN_ON(mem_init_done);
> + if (mem_init_done)
> + return NULL;

Can't we write

if (WARN_ON(mem_init_done))
return NULL;

nowadays ?

> + /* Make sure request is sane */
> + if (size == 0)
> + return NULL;
> +
> + /* If the region is already block mapped, then there is nothing
> +  * to do; just return the mapped address */
> + v = p_mapped_by_bats(addr);
> + if (v)
> + return (void __iomem *)v;

Should we check the size ?

> + /* Align region size */
> + for (bl = 128<<10; bl < (256<<20); bl <<= 1) {
> + p = _ALIGN_DOWN(addr, bl); /* BATs align on 128k boundaries */
> + size = ALIGN(addr - p + size, bl);
> + if (bl >= size)
> + break;
> + }
> +
> + /* Complain loudly if too much is requested */
> + if (bl >= (256<<20)) {
> + WARN_ON(1);
> + return NULL;
> + }

Do we avoid that running into the linear mapping ?

> + /* Allocate the aligned virtual base address.  ALIGN_DOWN is used
> +  * to ensure no overlaps occur with normal 4k ioremaps. */
> + ioremap_bot = _ALIGN_DOWN(ioremap_bot, bl) - size;
> +
> + /* Set up a BAT for this IO region */
> + i = loadbat(ioremap_bot, p, size, PAGE_KERNEL_NCG);
> + if (i < 0)
> + return NULL;
> +
> + return (void __iomem *) (ioremap_bot + (addr - p));
>  }
>  
>  /*
> diff --git a/arch/powerpc/platforms/52xx/mpc52xx_common.c 
> b/arch/powerpc/platforms/52xx/mpc52xx_common.c
> index 8e3dd5a..2c49148 100644
> --- a/arch/powerpc/platforms/52xx/mpc52xx_common.c
> +++ b/arch/powerpc/platforms/52xx/mpc52xx_common.c
> @@ -146,7 +146,20 @@ static struct of_device_id mpc52xx_cdm_ids[] __initdata 
> = {
>  void __init
>  mpc52xx_map_common_devices(void)
>  {
> + const struct of_device_id immr_ids[] = {
> + { .compatible = "fsl,mpc5200-immr", },
> + { .compatible = "fsl,mpc5200b-immr", },
> + { .type = "soc", .compatible = "mpc5200", }, /* lite5200 */
> + { .type = "builtin", .compatible = "mpc5200", }, /* efika */
> + {}
> + };
>   struct device_node *np;
> + struct resource res;
> +
> + /* Pre-map the whole register space using a BAT entry */
> + np = of_find_matching_node(NULL, immr_ids);
> + if (np && (of_address_to_resource(np, 0, &res) == 0))
> + ioremap_early(res.start, res.end - res.start + 1);
>  
>   /* mpc52xx_wdt is mapped here and used in mpc52xx_restart,
>* possibly from a interrupt context. wdt is only implement
> diff --git a/arch/powerpc/sysdev/cpm_common.c 
> b/arch/powerpc/sysdev/cpm_common.c
> index e4b6d66..370723e 100644
> --- a/arch/powerpc/sysdev/cpm_common.c
> +++ b/arch/powerpc/sysdev/cpm_common.c
> @@ -56,7 +56,7 @@ void __init udbg_init_cpm(void)
>  {
>   if (cpm_udbg_txdesc) {
>  #ifdef CONFIG_CPM2
> - setbat(1, 0xf000, 0xf000, 1024*1024, PAGE_KERNEL_NCG);
> + setbat(0xf000, 0xf000, 1024*1024, PAGE_KERNEL_NCG);
>  #endif
>   udbg_putc = udbg_putc_cpm;
>   }

That needs to be properly fixed ... maybe using ioremap_early() ? :-)

Also, make the initial call ioremap_early_init() just to make things
clear that one can't just call ioremap(), we are limited to a very
specific thing here.

For 8xx I'm not sure what the right approach is. For 40x, 440 and FSL
BookE we probably want to allow to bolt some TLB entries.

Cheers,
Ben.

Cheers,
Ben.


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH][Resend 2][BUILD FAILURE 04/04] Next June 04:PPC64 randconfig [drivers/net/ucc_geth.o]

2009-06-14 Thread Subrata Modak
Hi Li/Nathan,

On Thu, 2009-06-11 at 09:07 +0530, Subrata Modak wrote: 
> Hi Nathan,
> 
> >On Wed, 2009-06-10 at 21:28 -0500, Nathan Lynch wrote:
> >Subrata Modak  writes:
> > 
> > > On Thu, 2009-06-11 at 11:05 +1000, Stephen Rothwell wrote:
> > >> Hi Subrata,
> > >> 
> > >> On Wed, 10 Jun 2009 23:13:23 +0530 Subrata Modak 
> > >>  wrote:
> > >> >
> > >> >/* Find the TBI PHY.  If it's not there, we don't support SGMII 
> > >> > */
> > >> > -  ph = of_get_property(np, "tbi-handle", NULL);
> > >> > +  ph = (phandle *)of_get_property(np, "tbi-handle", NULL);
> > >> 
> > >> You don't need this cast because of_get_property() returns "void *".
> > >
> > > Stephen,
> > >
> > > True. But without this gcc complains:
> > >
> > > CC [M]  drivers/net/ucc_geth.o
> > > drivers/net/ucc_geth.c: In function bucc_geth_probeb:
> > > drivers/net/ucc_geth.c:3824: warning: assignment discards qualifiers
> > > from pointer target type
> > 
> > ph should be declared const phandle *.  Look at other uses of
> > of_get_property.
> >
> 
> Ok fine. Here is a revised patch again.
> 
> Subject: [PATCH][Resend 2][BUILD FAILURE 04/04] Next June 04:PPC64 randconfig 
> [drivers/net/ucc_geth.o]
> Reference(s):
> http://lkml.org/lkml/2009/6/4/241,
> http://lkml.org/lkml/2009/6/10/338,
> 
> Fix the following build error:
> 
> drivers/net/ucc_geth.c: In function bucc_geth_probeb:
> drivers/net/ucc_geth.c:3822: error: 'ph' undeclared (first use in this 
> function)
> drivers/net/ucc_geth.c:3822: error: (Each undeclared identifier is reported 
> only once
> drivers/net/ucc_geth.c:3822: error: for each function it appears in.)
> drivers/net/ucc_geth.c:3832: error: 'mdio' undeclared (first use in this 
> function)
> make[2]: *** [drivers/net/ucc_geth.o] Error 1
> 
> Signed-off-by: Subrata Modak 
> ---

Is there anything else to be done in this patch. If this is OK, can this
be applied. I am not sure, but, i find Li Yang as the maintainer for
this in linux*/MAINTAINERS file. Kindly let me know if this patch needs
to be revisited for some other issue(s).

Regards--
Subrata

> 
> --- linux-2.6.30-rc8/drivers/net/ucc_geth.c.orig  2009-06-10 
> 11:58:39.0 -0500
> +++ linux-2.6.30-rc8/drivers/net/ucc_geth.c   2009-06-10 22:28:13.0 
> -0500
> @@ -3595,6 +3595,8 @@ static const struct net_device_ops ucc_g
> 
>  static int ucc_geth_probe(struct of_device* ofdev, const struct of_device_id 
> *match)
>  {
> + struct device_node *mdio;
> + const phandle *ph;
>   struct device *device = &ofdev->dev;
>   struct device_node *np = ofdev->node;
>   struct net_device *dev = NULL;
> 
> ---
> Regards--
> Subrata
> 

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH][BUILD FAILURE 03/04] Next June 04:PPC64 randconfig [drivers/net/lance.o]

2009-06-14 Thread Subrata Modak

On Thu, 2009-06-11 at 09:20 +0530, Subrata Modak wrote: 
> Hi Benjamin/Paul,
> 
> >On Thu, 2009-06-04 at 19:02 +0530, Subrata Modak wrote:
> >CC  drivers/net/lance.o
> > drivers/net/lance.c: In function 'lance_probe1':
> > drivers/net/lance.c:575: error: implicit declaration of function 
> > 'isa_virt_to_bus'
> > drivers/net/lance.c: In function 'lance_rx':
> > drivers/net/lance.c:1197: error: implicit declaration of function 
> > 'isa_bus_to_virt'
> > make[2]: *** [drivers/net/lance.o] Error 1
> > make[1]: *** [drivers/net] Error 2
> > make: *** [drivers] Error 2
> 
> Reference: http://lkml.org/lkml/2009/6/4/240,
> To fix the following build error:
> 
> drivers/net/lance.c: In function 'lance_probe1':
> drivers/net/lance.c:575: error: implicit declaration of function 
> 'isa_virt_to_bus'
> drivers/net/lance.c: In function 'lance_rx':
> drivers/net/lance.c:1197: error: implicit declaration of function 
> 'isa_bus_to_virt'
> make[2]: *** [drivers/net/lance.o] Error 1
> make[1]: *** [drivers/net] Error 2
> make: *** [drivers] Error 2
> 
> I would like to propose the following patch. The prototypes for the functions:
> 'isa_virt_to_bus' & 'isa_virt_to_bus' are existing for some archs like the
> mips, x86, parisc, arm & alpha, but, is missing for powerpc. Is it safe to
> introduce the following soultion for powerpc ? It fixes the build problem
> i reported earlier.
> 
> Signed-off-by: Subrata Modak 
> ---

Benjamin,

I am not sure whether you liked the following patch to solve the above
problem. Do, you want me address some other issue(s)/fixes for this ?

Regards--
Subrata

> 
> --- linux-2.6.30-rc8/arch/powerpc/include/asm/io.h.orig   2009-06-10 
> 21:56:49.0 -0500
> +++ linux-2.6.30-rc8/arch/powerpc/include/asm/io.h2009-06-10 
> 22:21:35.0 -0500
> @@ -680,6 +680,9 @@ extern void __iounmap_at(void *ea, unsig
>  #define mmio_outsw(addr, src, count) writesw(addr, src, count)
>  #define mmio_outsl(addr, src, count) writesl(addr, src, count)
> 
> +#define isa_virt_to_bus  virt_to_phys
> +#define isa_bus_to_virt  phys_to_virt
> +
>  /**
>   *   virt_to_phys-   map virtual addresses to physical
>   *   @address: address to remap
> 
> ---
> Regards--
> Subrata
> 

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [BUILD FAILURE 02/04] Next June 04:PPC64 randconfig [drivers/usb/host/ohci-hcd.o]

2009-06-14 Thread Subrata Modak
On Fri, 2009-06-12 at 15:05 +0530, Subrata Modak wrote: 
> On Tue, 2009-06-09 at 17:38 -0700, David Brownell wrote:
> > On Friday 05 June 2009, Subrata Modak wrote:
> > > Correct, it fixes the issue. However, since few changes might have gone
> > > to the Kconfig, the patch does not apply cleanly. Below is the patch, just
> > > a retake of the earlier one, but on the latest code. 
> > 
> > And it got mangled a bit along the way.  Plus, the original one
> > goofed up Kconfig dependency displays ... both issues fixed in
> > this version, against current mainline GIT.
> > 
> > If someone can verify all four PPC/OF/OHCI configs build on
> > on PPC64, I'm OK with it.
> > 
> > - Dave
> 
> Dave,
> 
> Sorry for being late. The patch fixes the issue on the latest git for
> PPC64. Infact, the whole drivers/usb/host/ builds just fine:
> 
> linux-2.6 # make drivers/usb/host/
>   CHK include/linux/version.h
>   CHK include/linux/utsrelease.h
>   SYMLINK include/asm -> include/asm-powerpc
>   CALLscripts/checksyscalls.sh
>   CC  drivers/usb/host/ohci-hcd.o
>   CC  drivers/usb/host/pci-quirks.o
>   CC  drivers/usb/host/uhci-hcd.o
>   LD  drivers/usb/host/built-in.o
>   CC [M]  drivers/usb/host/isp116x-hcd.o
>   CC [M]  drivers/usb/host/u132-hcd.o
> 
> You can check in the patch now.

Dave,

Have you checked in this patch ? Or, does it require some more
modification/updates from somebody´s side ?

Regards--
Subrata

> 
> Regards--
> Subrata
> 
> > 
> > 
> > == CUT HERE
> > From: Arnd Bergmann 
> > Subject: fix build failure for PPC64 randconfig [usb/ohci]
> > 
> > We could just make the USB_OHCI_HCD_PPC_OF option implicit
> > and selected only if at least one of USB_OHCI_HCD_PPC_OF_BE
> > and USB_OHCI_HCD_PPC_OF_LE are set.
> > 
> > [ dbrown...@users.sourceforge.net: fix patch manglation and dependencies ]
> > 
> > Signed-off-by: Arnd Bergmann 
> > Resent-by: Subrata Modak 
> > Signed-off-by: David Brownell 
> > ---
> >  drivers/usb/host/Kconfig |   29 +++--
> >  1 file changed, 15 insertions(+), 14 deletions(-)
> > 
> > --- a/drivers/usb/host/Kconfig
> > +++ b/drivers/usb/host/Kconfig
> > @@ -180,26 +180,27 @@ config USB_OHCI_HCD_PPC_SOC
> >   Enables support for the USB controller on the MPC52xx or
> >   STB03xxx processor chip.  If unsure, say Y.
> > 
> > -config USB_OHCI_HCD_PPC_OF
> > -   bool "OHCI support for PPC USB controller on OF platform bus"
> > -   depends on USB_OHCI_HCD && PPC_OF
> > -   default y
> > -   ---help---
> > - Enables support for the USB controller PowerPC present on the
> > - OpenFirmware platform bus.
> > -
> >  config USB_OHCI_HCD_PPC_OF_BE
> > -   bool "Support big endian HC"
> > -   depends on USB_OHCI_HCD_PPC_OF
> > -   default y
> > +   bool "OHCI support for OF platform bus (big endian)"
> > +   depends on USB_OHCI_HCD && PPC_OF
> > select USB_OHCI_BIG_ENDIAN_DESC
> > select USB_OHCI_BIG_ENDIAN_MMIO
> > +   ---help---
> > + Enables support for big-endian USB controllers present on the
> > + OpenFirmware platform bus.
> > 
> >  config USB_OHCI_HCD_PPC_OF_LE
> > -   bool "Support little endian HC"
> > -   depends on USB_OHCI_HCD_PPC_OF
> > -   default n
> > +   bool "OHCI support for OF platform bus (little endian)"
> > +   depends on USB_OHCI_HCD && PPC_OF
> > select USB_OHCI_LITTLE_ENDIAN
> > +   ---help---
> > + Enables support for little-endian USB controllers present on the
> > + OpenFirmware platform bus.
> > +
> > +config USB_OHCI_HCD_PPC_OF
> > +   bool
> > +   depends on USB_OHCI_HCD && PPC_OF
> > +   default USB_OHCI_HCD_PPC_OF_BE || USB_OHCI_HCD_PPC_OF_LE
> > 
> >  config USB_OHCI_HCD_PCI
> > bool "OHCI support for PCI-bus USB controllers"
> > 

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [Patch 2/6] Introduce PPC64 specific Hardware Breakpoint interfaces

2009-06-14 Thread David Gibson
On Wed, Jun 10, 2009 at 12:13:49PM +0530, K.Prasad wrote:
> On Fri, Jun 05, 2009 at 03:11:58PM +1000, David Gibson wrote:
> > On Wed, Jun 03, 2009 at 10:05:11PM +0530, K.Prasad wrote:
> 
> Hi David,
>   Sorry for the delay in response below. In the meanwhile, I
> discovered an issue in detecting stray exceptions that affected
> user-space handling of breakpoints. I've made some changes to correct
> that behaviour which will be included in version VI of the patchset.

[snip]
> > > + /*
> > > +  * User-space requests will always have the address field populated
> > > +  * For kernel-addresses, either the address or symbol name can be
> > > +  * specified.
> > > +  */
> > > + if (bp->info.name)
> > > + bp->info.address = (unsigned long)
> > > + kallsyms_lookup_name(bp->info.name);
> > 
> > Archs don't have to implement this name lookup stuff, but it looks
> > like most of them would - so it looks like there ought to be a helper
> > function in generic code that will do the check / name lookup stuff.
> 
> It doesn't turn out to be very generic. The IO breakpoints in x86, the
> address-range (only) breakpoints in S390 and perhaps 4xx powerpc
> processors were what made me think that this should remain in
> arch-specific code. In these cases, we might have to deal only with
> breakpoint addresses and not names.

Hrm, ok.  I was suggesting a helper function that those archs which
can use it call at need, not moving the address lookup to generic code
in the sense of being used everywhere.  Whatever, it's not that
important at this stage.

> > > + if (bp->info.address)
> > > + return 0;
> > 
> > Hrm.. you realise there's no theoretical reason a userspace program
> > couldn't put a breakpoint at address 0...?
> 
> I agree. I think there must be parts of code that works based on this
> assumption. Will check and remove them.

Ok, good.

[snip]
> > > + else {
> > > + /*
> > > +  * This exception is triggered not because of a memory access on
> > > +  * the monitored variable but in the double-word address range
> > > +  * in which it is contained. We will consume this exception,
> > > +  * considering it as 'noise'.
> > > +  */
> > > + rc = NOTIFY_STOP;
> > > + goto out;
> > > + }
> > > + is_one_shot = (bp->triggered == ptrace_triggered) ? 1 : 0;
> > 
> > Ouch, explicitly special-casing ptrace_triggered is pretty nasty.
> > Since the bp_info is already arch specific, maybe it should include a
> > flag to indicate whether the breakpoint is one-shot or not.
> > 
> 
> The reason to check for ptrace_triggered is to contain the one-shot
> behaviour only to ptrace (thus retaining the semantics) and not to extend
> them to all user-space requests through
> register_user_hw_breakpoint().

Right, but couldn't you implement that withing ptrace_triggered
itself, without a special test here, by having it cancel the
breakpoint.

> A one-shot behaviour for all user-space requests would create more work
> for the user-space programs (such as re-registration) and will leave open
> a small window of opportunity for debug register grabbing by kernel-space
> requests.
> 
> So, in effect a request through register_user_hw_breakpoint() interface
> will behave as under:
> - Single-step over the causative instruction that triggered the
>   breakpoint exception handler.
> - Deliver the SIGTRAP signal to user-space after executing the causative
>   instruction.
> 
> This behaviour is in consonance with that of kernel-space requests and
> those on x86 processors, and helps define a consistent behaviour across
> architectures for user-space.
> 
> Let me know what you think on the same.

I certainly see the value in consistent semantics across archs.
However, I can also see uses for the powerpc trap-before-execute
behaviour.  That's why I'm suggesting it might be worth having an
arch-specific flag.

[snip]
> > > +int __kprobes single_step_dabr_instruction(struct die_args *args)
> > > +{
> > > + struct pt_regs *regs = args->regs;
> > > + int cpu = get_cpu();
> > > + int ret = NOTIFY_DONE;
> > > + siginfo_t info;
> > > + unsigned long this_dabr_data = per_cpu(dabr_data, cpu);
> > > +
> > > + /*
> > > +  * Check if we are single-stepping as a result of a
> > > +  * previous HW Breakpoint exception
> > > +  */
> > > + if (this_dabr_data == 0)
> > > + goto out;
> > > +
> > > + regs->msr &= ~MSR_SE;
> > > + /* Deliver signal to user-space */
> > > + if (this_dabr_data < TASK_SIZE) {
> > > + info.si_signo = SIGTRAP;
> > > + info.si_errno = 0;
> > > + info.si_code = TRAP_HWBKPT;
> > > + info.si_addr = (void __user *)(per_cpu(dabr_data, cpu));
> > > + force_sig_info(SIGTRAP, &info, current);
> > 
> > Uh.. I recall mentioning in my previous review that in order to match
> > previous behaviour we need to deliver the userspace signal *before*
> > stepping over the breakpointed instruction, rather

Re: [PATCH 24/33] usb/ps3: Add missing annotations

2009-06-14 Thread Benjamin Herrenschmidt
On Wed, 2009-06-10 at 16:38 +0200, Geert Uytterhoeven wrote:
> probe functions should be __devinit
> initialization functions should be __init

Please send to USB maintainers or get an ack from them.

Cheers,
Ben.

> Signed-off-by: Geert Uytterhoeven 
> Cc: Geoff Levand 
> ---
>  drivers/usb/host/ehci-ps3.c |4 ++--
>  drivers/usb/host/ohci-ps3.c |4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/usb/host/ehci-ps3.c b/drivers/usb/host/ehci-ps3.c
> index bb870b8..3e8844e 100644
> --- a/drivers/usb/host/ehci-ps3.c
> +++ b/drivers/usb/host/ehci-ps3.c
> @@ -76,7 +76,7 @@ static const struct hc_driver ps3_ehci_hc_driver = {
>   .port_handed_over   = ehci_port_handed_over,
>  };
>  
> -static int ps3_ehci_probe(struct ps3_system_bus_device *dev)
> +static int __devinit ps3_ehci_probe(struct ps3_system_bus_device *dev)
>  {
>   int result;
>   struct usb_hcd *hcd;
> @@ -224,7 +224,7 @@ static int ps3_ehci_remove(struct ps3_system_bus_device 
> *dev)
>   return 0;
>  }
>  
> -static int ps3_ehci_driver_register(struct ps3_system_bus_driver *drv)
> +static int __init ps3_ehci_driver_register(struct ps3_system_bus_driver *drv)
>  {
>   return firmware_has_feature(FW_FEATURE_PS3_LV1)
>   ? ps3_system_bus_driver_register(drv)
> diff --git a/drivers/usb/host/ohci-ps3.c b/drivers/usb/host/ohci-ps3.c
> index 1d56259..7009504 100644
> --- a/drivers/usb/host/ohci-ps3.c
> +++ b/drivers/usb/host/ohci-ps3.c
> @@ -75,7 +75,7 @@ static const struct hc_driver ps3_ohci_hc_driver = {
>  #endif
>  };
>  
> -static int ps3_ohci_probe(struct ps3_system_bus_device *dev)
> +static int __devinit ps3_ohci_probe(struct ps3_system_bus_device *dev)
>  {
>   int result;
>   struct usb_hcd *hcd;
> @@ -224,7 +224,7 @@ static int ps3_ohci_remove(struct ps3_system_bus_device 
> *dev)
>   return 0;
>  }
>  
> -static int ps3_ohci_driver_register(struct ps3_system_bus_driver *drv)
> +static int __init ps3_ohci_driver_register(struct ps3_system_bus_driver *drv)
>  {
>   return firmware_has_feature(FW_FEATURE_PS3_LV1)
>   ? ps3_system_bus_driver_register(drv)

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH 23/33] net/ps3: gelic - Add missing annotations

2009-06-14 Thread Benjamin Herrenschmidt
On Wed, 2009-06-10 at 16:38 +0200, Geert Uytterhoeven wrote:
> probe functions should be __devinit
> 
> Signed-off-by: Geert Uytterhoeven 
> Cc: Geoff Levand 

Please send to netdev or get an ack from davem.

Cheers,
Ben.

> ---
>  drivers/net/ps3_gelic_net.c  |   22 --
>  drivers/net/ps3_gelic_wireless.c |6 +++---
>  2 files changed, 15 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/net/ps3_gelic_net.c b/drivers/net/ps3_gelic_net.c
> index 2b38f39..d1a5fb4 100644
> --- a/drivers/net/ps3_gelic_net.c
> +++ b/drivers/net/ps3_gelic_net.c
> @@ -214,9 +214,10 @@ static void gelic_card_free_chain(struct gelic_card 
> *card,
>   *
>   * returns 0 on success, <0 on failure
>   */
> -static int gelic_card_init_chain(struct gelic_card *card,
> -  struct gelic_descr_chain *chain,
> -  struct gelic_descr *start_descr, int no)
> +static int __devinit gelic_card_init_chain(struct gelic_card *card,
> +struct gelic_descr_chain *chain,
> +struct gelic_descr *start_descr,
> +int no)
>  {
>   int i;
>   struct gelic_descr *descr;
> @@ -407,7 +408,7 @@ rewind:
>   *
>   * returns 0 on success, < 0 on failure
>   */
> -static int gelic_card_alloc_rx_skbs(struct gelic_card *card)
> +static int __devinit gelic_card_alloc_rx_skbs(struct gelic_card *card)
>  {
>   struct gelic_descr_chain *chain;
>   int ret;
> @@ -1422,8 +1423,8 @@ static const struct net_device_ops gelic_netdevice_ops 
> = {
>   *
>   * fills out function pointers in the net_device structure
>   */
> -static void gelic_ether_setup_netdev_ops(struct net_device *netdev,
> -  struct napi_struct *napi)
> +static void __devinit gelic_ether_setup_netdev_ops(struct net_device *netdev,
> +struct napi_struct *napi)
>  {
>   netdev->watchdog_timeo = GELIC_NET_WATCHDOG_TIMEOUT;
>   /* NAPI */
> @@ -1443,7 +1444,8 @@ static void gelic_ether_setup_netdev_ops(struct 
> net_device *netdev,
>   * gelic_ether_setup_netdev initializes the net_device structure
>   * and register it.
>   **/
> -int gelic_net_setup_netdev(struct net_device *netdev, struct gelic_card 
> *card)
> +int __devinit gelic_net_setup_netdev(struct net_device *netdev,
> +  struct gelic_card *card)
>  {
>   int status;
>   u64 v1, v2;
> @@ -1491,7 +1493,7 @@ int gelic_net_setup_netdev(struct net_device *netdev, 
> struct gelic_card *card)
>   * the card and net_device structures are linked to each other
>   */
>  #define GELIC_ALIGN (32)
> -static struct gelic_card *gelic_alloc_card_net(struct net_device **netdev)
> +static struct gelic_card * __devinit gelic_alloc_card_net(struct net_device 
> **netdev)
>  {
>   struct gelic_card *card;
>   struct gelic_port *port;
> @@ -1542,7 +1544,7 @@ static struct gelic_card *gelic_alloc_card_net(struct 
> net_device **netdev)
>   return card;
>  }
>  
> -static void gelic_card_get_vlan_info(struct gelic_card *card)
> +static void __devinit gelic_card_get_vlan_info(struct gelic_card *card)
>  {
>   u64 v1, v2;
>   int status;
> @@ -1616,7 +1618,7 @@ static void gelic_card_get_vlan_info(struct gelic_card 
> *card)
>  /**
>   * ps3_gelic_driver_probe - add a device to the control of this driver
>   */
> -static int ps3_gelic_driver_probe(struct ps3_system_bus_device *dev)
> +static int __devinit ps3_gelic_driver_probe(struct ps3_system_bus_device 
> *dev)
>  {
>   struct gelic_card *card;
>   struct net_device *netdev;
> diff --git a/drivers/net/ps3_gelic_wireless.c 
> b/drivers/net/ps3_gelic_wireless.c
> index 4f3ada6..b6b3ca9 100644
> --- a/drivers/net/ps3_gelic_wireless.c
> +++ b/drivers/net/ps3_gelic_wireless.c
> @@ -2442,7 +2442,7 @@ static const struct iw_handler_def 
> gelic_wl_wext_handler_def = {
>  #endif
>  };
>  
> -static struct net_device *gelic_wl_alloc(struct gelic_card *card)
> +static struct net_device * __devinit gelic_wl_alloc(struct gelic_card *card)
>  {
>   struct net_device *netdev;
>   struct gelic_port *port;
> @@ -2722,7 +2722,7 @@ static struct ethtool_ops gelic_wl_ethtool_ops = {
>   .set_rx_csum= gelic_net_set_rx_csum,
>  };
>  
> -static void gelic_wl_setup_netdev_ops(struct net_device *netdev)
> +static void __devinit gelic_wl_setup_netdev_ops(struct net_device *netdev)
>  {
>   struct gelic_wl_info *wl;
>   wl = port_wl(netdev_priv(netdev));
> @@ -2738,7 +2738,7 @@ static void gelic_wl_setup_netdev_ops(struct net_device 
> *netdev)
>  /*
>   * driver probe/remove
>   */
> -int gelic_wl_driver_probe(struct gelic_card *card)
> +int __devinit gelic_wl_driver_probe(struct gelic_card *card)
>  {
>   int ret;
>   struct net_device *netdev;

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.

Re: [PATCH 26/33] sound/ps3: Fix checkpatch issues

2009-06-14 Thread Takashi Iwai
At Mon, 15 Jun 2009 15:51:03 +1000,
Benjamin Herrenschmidt wrote:
> 
> On Mon, 2009-06-15 at 07:43 +0200, Takashi Iwai wrote:
> > The alsa part also already reached to the upstream, so we have to give
> > revert patches if needed.  But, in the case of sound bits, I think
> > they can remain there as they are independent changes.
> > But, if you think it's better to revert for the whole maintainability,
> > I'll revert in the next pull request that'll be sent soon later.
> > 
> No, that's ok, I'll remove those from my pile.

OK, thanks!


Takashi
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH 05/33] block: Add bio_list_peek()

2009-06-14 Thread Jens Axboe
On Mon, Jun 15 2009, Benjamin Herrenschmidt wrote:
> On Wed, 2009-06-10 at 16:38 +0200, Geert Uytterhoeven wrote:
> > Introduce bio_list_peek(), to obtain a pointer to the first bio on the 
> > bio_list
> > without actually removing it from the list. This is needed when you want to
> > serialize based on the list being empty or not.
> 
> Leaving that one (and the next one) out for now until Jens Ack them.

You can add my acked-by, this one is trivial and a good addition.

> 
> Cheers,
> Ben.
> 
> > Signed-off-by: Geert Uytterhoeven 
> > Cc: Jens Axboe 
> > ---
> >  include/linux/bio.h |5 +
> >  1 files changed, 5 insertions(+), 0 deletions(-)
> > 
> > diff --git a/include/linux/bio.h b/include/linux/bio.h
> > index 7b214fd..618bb7d 100644
> > --- a/include/linux/bio.h
> > +++ b/include/linux/bio.h
> > @@ -590,6 +590,11 @@ static inline void bio_list_merge_head(struct bio_list 
> > *bl,
> > bl->head = bl2->head;
> >  }
> >  
> > +static inline struct bio *bio_list_peek(struct bio_list *bl)
> > +{
> > +   return bl->head;
> > +}
> > +
> >  static inline struct bio *bio_list_pop(struct bio_list *bl)
> >  {
> > struct bio *bio = bl->head;
> 

-- 
Jens Axboe

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH 26/33] sound/ps3: Fix checkpatch issues

2009-06-14 Thread Benjamin Herrenschmidt
On Mon, 2009-06-15 at 07:43 +0200, Takashi Iwai wrote:
> The alsa part also already reached to the upstream, so we have to give
> revert patches if needed.  But, in the case of sound bits, I think
> they can remain there as they are independent changes.
> But, if you think it's better to revert for the whole maintainability,
> I'll revert in the next pull request that'll be sent soon later.
> 
No, that's ok, I'll remove those from my pile.

Cheers,
Ben.


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH 26/33] sound/ps3: Fix checkpatch issues

2009-06-14 Thread Takashi Iwai
At Mon, 15 Jun 2009 12:22:08 +1000,
Benjamin Herrenschmidt wrote:
> 
> On Wed, 2009-06-10 at 16:55 +0200, Takashi Iwai wrote:
> > At Wed, 10 Jun 2009 16:39:01 +0200,
> > Geert Uytterhoeven wrote:
> > > 
> > > Signed-off-by: Geert Uytterhoeven 
> > > Cc: alsa-de...@alsa-project.org
> > > Cc: Takashi Iwai 
> > 
> > Thanks, applied these three patches (26,27,28) to sound git tree.
> 
> Are you sure they don't depend on the previous patches ? We're already
> having fallover from the SCSI tree merging some of them while I haven't
> merge the previous ones yet...

These three patches are just trivial changes: coding-style fixes,
shuffle of function lines, and addition of __devinit/__devexit
notations.  So they should be fairly safe and independent (thus I
applied them).  At least, the test cross-build was OK.


> Geert, there are several "procedural" issues with your series here: In
> addition to the whole thing being submitted too late that is :-)
> 
> You don't have the same CC list on all patches. That is fine except that
> you also don't provide merge instructions after the signature, such as
> "this patch depends on XX from this series" etc...
> 
> A bunch of them do have such dependencies. In fact, there's even a patch
> that changes something in the block layer (it's not a big deal, just
> adding an accessor, but I still -need- an Ack from Jens to merge it) and
> so I have to block all patches that depend on this one. But then, we
> have indirect dependencies where some patch don't apply because they
> touch the same file that was patched by the patch who had the dependency
> etc...
> 
> I ended up stopping mid-way through the series. I'll push some of the
> patches to my -next branch tonight, and the rest once I get the Ack from
> Jens.
> 
> I recommend all maintainers (scsi, alsa) just drop these patches for now
> and instead give me Ack's so I can merge them via the powerpc tree along
> with their respective dependencies. James, feel free to do a revert if
> you don't want to rebase, that will break bisect on ps3 for a little
> while, but that's the price Geert will have to pay for his mistakes :-)

The alsa part also already reached to the upstream, so we have to give
revert patches if needed.  But, in the case of sound bits, I think
they can remain there as they are independent changes.
But, if you think it's better to revert for the whole maintainability,
I'll revert in the next pull request that'll be sent soon later.


thanks,

Takashi
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH 1/2] lib: Provide generic atomic64_t implementation

2009-06-14 Thread Paul Mackerras
Roland Dreier writes:

> FWIW, Nehalem EX actually goes to 8 cores/16 threads per socket.  But
> worrying about 32-bit performance on Nehalem is a little silly -- this
> simplest solution is simply to run a 64-bit kernel.

I'm not worried about ANY x86 processor, 32-bit or 64-bit, in fact,
since x86 already has an atomic64_t implementation for both 32-bit and
64-bit.

It is interesting, though, that arch/x86/include/asm/atomic_32.h
unconditionally uses cmpxchg8b to implement atomic64_t, but I thought
that cmpxchg8b didn't exist in processors prior to the Pentium.
Presumably you can't use perf_counters on a 386 or 486.

Paul.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH 1/2] lib: Provide generic atomic64_t implementation

2009-06-14 Thread Roland Dreier

 > The new Nehalems provide 8 logical threads in a single socket.  All
 > those threads share a cache, and they have cmpxchg8b anyway, so this
 > won't matter.

FWIW, Nehalem EX actually goes to 8 cores/16 threads per socket.  But
worrying about 32-bit performance on Nehalem is a little silly -- this
simplest solution is simply to run a 64-bit kernel.

 - R.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH v4] 83xx: add support for the kmeter1 board.

2009-06-14 Thread David Gibson
On Fri, Jun 12, 2009 at 07:27:42AM +0200, Heiko Schocher wrote:
> Hello David,
> 
> David Gibson wrote:
> > On Thu, Jun 11, 2009 at 08:10:41PM +0200, Heiko Schocher wrote:
> >> The following series implements basic board support for
> >> the kmeter1 board from keymile, based on a MPC8360.
> > 
> > [snip]
> >> +  par...@1400 {
> >> +  reg = <0x1400 0x100>;
> >> +  device_type = "par_io";
> > 
> > This should have a compatible value instead of a device_type value.
> 
> compatible = "fsl,mpc8360-par_io"; ?

Sounds reasonable, if there isn't an existing precedent to follow.

> > [snip]
> > 
> >> +  num-ports = <7>;
> >> +
> >> +  pio_ucc1: ucc_...@00 {
> > 
> > Since these nodes have addresses, they should also have reg
> > properties.  And the parent should have #address-cells and
> > #size-cells.
> 
> added "#address-cells" and "#size-cells", but couldn;t find what
> I should write in a "reg" entry ... I think something like that:
> 
> reg = <0x00 0x01>;

If you just want a plain index number, which is what it seems you have
here, then you'll want #address-cells = 1, #size-cells = 0 and reg
will then have one cell (i.e. 1 u32) with the index number

so  subn...@0 {
reg = <0>;
}
subn...@1 {
reg = <1>;
}

Note also that it's usual for the unit addresses (that's the bits
after the @ sign) to be formatted without leading zeroes.

> 
> ?
> 
> > [snip]
> >> +  q...@10 {
> >> +  #address-cells = <1>;
> >> +  #size-cells = <1>;
> >> +  device_type = "qe";
> > 
> > This device_type should not be here.
> 
> deleted.

Ok.

-- 
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@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH 26/33] sound/ps3: Fix checkpatch issues

2009-06-14 Thread Benjamin Herrenschmidt
On Wed, 2009-06-10 at 16:55 +0200, Takashi Iwai wrote:
> At Wed, 10 Jun 2009 16:39:01 +0200,
> Geert Uytterhoeven wrote:
> > 
> > Signed-off-by: Geert Uytterhoeven 
> > Cc: alsa-de...@alsa-project.org
> > Cc: Takashi Iwai 
> 
> Thanks, applied these three patches (26,27,28) to sound git tree.

Are you sure they don't depend on the previous patches ? We're already
having fallover from the SCSI tree merging some of them while I haven't
merge the previous ones yet...

Geert, there are several "procedural" issues with your series here: In
addition to the whole thing being submitted too late that is :-)

You don't have the same CC list on all patches. That is fine except that
you also don't provide merge instructions after the signature, such as
"this patch depends on XX from this series" etc...

A bunch of them do have such dependencies. In fact, there's even a patch
that changes something in the block layer (it's not a big deal, just
adding an accessor, but I still -need- an Ack from Jens to merge it) and
so I have to block all patches that depend on this one. But then, we
have indirect dependencies where some patch don't apply because they
touch the same file that was patched by the patch who had the dependency
etc...

I ended up stopping mid-way through the series. I'll push some of the
patches to my -next branch tonight, and the rest once I get the Ack from
Jens.

I recommend all maintainers (scsi, alsa) just drop these patches for now
and instead give me Ack's so I can merge them via the powerpc tree along
with their respective dependencies. James, feel free to do a revert if
you don't want to rebase, that will break bisect on ps3 for a little
while, but that's the price Geert will have to pay for his mistakes :-)

Cheers,
Ben.

> Takashi
> 
> > ---
> >  sound/ppc/snd_ps3.c |   32 +++-
> >  1 files changed, 15 insertions(+), 17 deletions(-)
> > 
> > diff --git a/sound/ppc/snd_ps3.c b/sound/ppc/snd_ps3.c
> > index f361c26..d660b0f 100644
> > --- a/sound/ppc/snd_ps3.c
> > +++ b/sound/ppc/snd_ps3.c
> > @@ -165,8 +165,7 @@ static const struct snd_pcm_hardware snd_ps3_pcm_hw = {
> > .fifo_size = PS3_AUDIO_FIFO_SIZE
> >  };
> >  
> > -static struct snd_pcm_ops snd_ps3_pcm_spdif_ops =
> > -{
> > +static struct snd_pcm_ops snd_ps3_pcm_spdif_ops = {
> > .open = snd_ps3_pcm_open,
> > .close = snd_ps3_pcm_close,
> > .prepare = snd_ps3_pcm_prepare,
> > @@ -183,7 +182,7 @@ static int snd_ps3_verify_dma_stop(struct 
> > snd_ps3_card_info *card,
> > int dma_ch, done, retries, stop_forced = 0;
> > uint32_t status;
> >  
> > -   for (dma_ch = 0; dma_ch < 8; dma_ch ++) {
> > +   for (dma_ch = 0; dma_ch < 8; dma_ch++) {
> > retries = count;
> > do {
> > status = read_reg(PS3_AUDIO_KICK(dma_ch)) &
> > @@ -259,9 +258,7 @@ static void snd_ps3_kick_dma(struct snd_ps3_card_info 
> > *card)
> >  /*
> >   * convert virtual addr to ioif bus addr.
> >   */
> > -static dma_addr_t v_to_bus(struct snd_ps3_card_info *card,
> > -  void * paddr,
> > -  int ch)
> > +static dma_addr_t v_to_bus(struct snd_ps3_card_info *card, void *paddr, 
> > int ch)
> >  {
> > return card->dma_start_bus_addr[ch] +
> > (paddr - card->dma_start_vaddr[ch]);
> > @@ -321,7 +318,7 @@ static int snd_ps3_program_dma(struct snd_ps3_card_info 
> > *card,
> > spin_lock_irqsave(&card->dma_lock, irqsave);
> > for (ch = 0; ch < 2; ch++) {
> > start_vaddr = card->dma_next_transfer_vaddr[0];
> > -   for (stage = 0; stage < fill_stages; stage ++) {
> > +   for (stage = 0; stage < fill_stages; stage++) {
> > dma_ch = stage * 2 + ch;
> > if (silent)
> > dma_addr = card->null_buffer_start_dma_addr;
> > @@ -619,7 +616,7 @@ static int snd_ps3_change_avsetting(struct 
> > snd_ps3_card_info *card)
> >   PS3_AUDIO_AO_3WMCTRL_ASOEN(2) |
> >   PS3_AUDIO_AO_3WMCTRL_ASOEN(3)),
> > 0);
> > -   wmb();  /* ensure the hardware sees the change */
> > +   wmb();  /* ensure the hardware sees the change */
> > /* wait for actually stopped */
> > retries = 1000;
> > while ((read_reg(PS3_AUDIO_AO_3WMCTRL) &
> > @@ -798,20 +795,20 @@ static struct snd_kcontrol_new spdif_ctls[] = {
> > {
> > .access = SNDRV_CTL_ELEM_ACCESS_READ,
> > .iface = SNDRV_CTL_ELEM_IFACE_PCM,
> > -   .name = SNDRV_CTL_NAME_IEC958("",PLAYBACK,CON_MASK),
> > +   .name = SNDRV_CTL_NAME_IEC958("", PLAYBACK, CON_MASK),
> > .info = snd_ps3_spdif_mask_info,
> > .get = snd_ps3_spdif_cmask_get,
> > },
> > {
> > .access = SNDRV_CTL_ELEM_ACCESS_READ,
> > .iface = SNDRV_CTL_ELEM_IFACE_PCM,
> > -   .name = SNDRV_CTL_NAME_IEC958("",PLAYBACK,PRO_MASK),
> > +   .name = SNDRV_CTL_NAME_IEC958("", PLAYBAC

[PATCH] powerpc: Add memory clobber to mtspr()

2009-06-14 Thread Benjamin Herrenschmidt
Without this clobber, mtspr can be re-ordered by gcc vs. surrounding
memory accesses. While this might be ok for some cases, it's not in
others and I'm not confident that all callers get it right (In fact
I'm sure some of them don't).

So for now, let's make mtspr() itself contain a memory clobber until
we can audit and fix everything, at which point we can remove it
if we think it's worth doing so.

Signed-off-by: Benjamin Herrenschmidt 
---

 arch/powerpc/include/asm/reg.h |3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

--- linux-work.orig/arch/powerpc/include/asm/reg.h  2009-06-15 
12:01:06.0 +1000
+++ linux-work/arch/powerpc/include/asm/reg.h   2009-06-15 12:01:17.0 
+1000
@@ -755,7 +755,8 @@
 #define mfspr(rn)  ({unsigned long rval; \
asm volatile("mfspr %0," __stringify(rn) \
: "=r" (rval)); rval;})
-#define mtspr(rn, v)   asm volatile("mtspr " __stringify(rn) ",%0" : : "r" (v))
+#define mtspr(rn, v)   asm volatile("mtspr " __stringify(rn) ",%0" : : "r" (v)\
+: "memory")
 
 #ifdef __powerpc64__
 #ifdef CONFIG_PPC_CELL
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


[git pull] Please pull powerpc.git merge branch

2009-06-14 Thread Benjamin Herrenschmidt
Hi Linus !

Here's the first half of the powerpc merge for 2.6.31. I'll have the
rest in a couple of days, just dealing with some of my own backlog here
plus some collisions.

The few things outside of arch/powerpc are powerpc-specific drivers (and
an addition to pci_ids.h). There's also a cleanup of
drivers/pci/Makefile removing another unused ppc specific bit.

No major highlight this time around. Lots of minor fixes, tweaks and
cleanups, swiotlb is now available on some crazy embedded platforms (not
something to be -that- happy about), some new embedded boards, ...

There's a rather big ps3 update queued up for the next batch, but it
didn't make it here. Pauls atomic64_t for 32-bit platforms is in the
next batch too. Hopefully I'll have it ready for you in a day or two (I
want to let it simmer in -next for a couple of days).

Cheers,
Ben.
  
The following changes since commit 45e3e1935e2857c54783291107d33323b3ef33c8:
  Linus Torvalds (1):
Merge branch 'master' of git://git.kernel.org/.../sam/kbuild-next

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/benh/powerpc.git next

Anton Blanchard (2):
  powerpc: Improve decrementer accuracy
  powerpc: Convert RTAS event scan from kernel thread to workqueue

Anton Vorontsov (6):
  powerpc/85xx: Add PCI IDs for MPC8569 family processors
  powerpc/85xx: Fix mpc8569emds crypto node to include SNOW unit
  powerpc/85xx: Fix reg & interrupts for mpc8569emds localbus added NAND
  powerpc/85xx: Add eSDHC support for MPC8569E-MDS boards
  powerpc/85xx: Enable Serial RapidIO for MPC85xx MDS boards
  powerpc/85xx: Add STMicro M25P40 serial flash support for MPC8569E-MDS

Becky Bruce (4):
  powerpc/86xx: Add 36-bit device tree for mpc8641hpcn
  powerpc: make dma_window_* in pci_controller struct avail on 32b
  powerpc: Use sg->dma_length in sg_dma_len() macro on 32-bit
  powerpc: Add support for swiotlb on 32-bit

Benjamin Herrenschmidt (12):
  Merge branch 'merge' into next
  powerpc/mm: Fix some SMP issues with MMU context handling
  powerpc/mm: Fix a AB->BA deadlock scenario with nohash MMU context lock
  powerpc: Set init_bootmem_done on NUMA platforms as well
  powerpc: Move VMX and VSX asm code to vector.S
  powerpc: Introduce CONFIG_PPC_BOOK3S
  powerpc: Split exception handling out of head_64.S
  powerpc: Separate PACA fields for server CPUs
  powerpc: Shield code specific to 64-bit server processors
  Merge commit 'jwb/next' into next
  powerpc: Fix bug in move of altivec code to vector.S
  Merge commit 'origin/master' into next

Geert Uytterhoeven (1):
  powerpc: Keep track of emulated instructions

Geoff Levand (1):
  powerpc/ps3: Use smp_request_message_ipi

Grant Likely (1):
  powerpc/virtex: refactor intc driver and add support for i8259 cascading

Haiying Wang (7):
  powerpc/85xx: clean up for mpc8568_mds name
  powerpc/qe: update risc allocation for QE
  net/ucc_geth: update riscTx and riscRx in ucc_geth
  powerpc/qe: update QE Serial Number
  net/ucc_geth: Assign six threads to Rx for UEC
  powerpc/85xx: Add MPC8569MDS board support
  powerpc/qe: add new qe properties for QE based chips

Jan Blunck (1):
  powerpc/spufs: Remove double check for non-negative dentry

John Linn (1):
  fbdev: Add PLB support and cleanup DCR in xilinxfb driver.

Josh Boyer (3):
  powerpc/4xx: Disable PCI_LEGACY
  powerpc/40x: Convert AMCC Makalu board to ppc40x_simple
  powerpc/40x: Convert AMCC Kilauea/Halekala boards to ppc40x_simple

Kumar Gala (32):
  powerpc/fsl: Remove cell-index from PCI nodes
  powerpc: Refactor board check for PCI quirks on FSL boards with uli1575
  powerpc/fsl: use of_iomap() for rstcr mapping
  powerpc/85xx: Add binding for LAWs and ECM
  powerpc/85xx: Add new LAW & ECM device tree nodes for all 85xx systems
  powerpc/86xx: Add binding for LAWs and MCM
  powerpc/86xx: Add new LAW & MCM device tree nodes for all 86xx systems
  powerpc/cpm: Remove some cruft code and defines
  powerpc/86xx: clean up smp init code
  powerpc/fsl: Removed reg property from 85xx/86xx soc node
  fsldma: Fix compile warnings
  powerpc/85xx: Add MSI nodes for MPC8568/9 MDS systems
  powerpc/fsl: Support unique MSI addresses per PCIe Root Complex
  powerpc/8xxx: Update PCI outbound window addresses for 36-bit configs
  powerpc/fsl_rio: Fix compile warnings
  powerpc/fsl: Update FSL esdhc binding
  powerpc/85xx: Add P2020DS board support
  powerpc/fsl: Setup PCI inbound window based on actual amount of memory
  powerpc: Fix up elf_read_implies_exec() usage
  powerpc/pci: Clean up direct access to sysdata by indirect ops
  powerpc/pci: Clean up direct access to sysdata by FSL platforms
  powerpc/pci: Clean up direct access to sysdata by 52xx platforms
  powerpc/pci: Clean up direct access

Re: [PATCH] powerpc: Make RTAS instantiation depend on CONFIG_PPC_RTAS

2009-06-14 Thread Benjamin Herrenschmidt
On Fri, 2009-06-12 at 14:08 +1000, Michael Ellerman wrote:
> Currently prom_init.c always instantiates RTAS, even if the kernel
> is built without RTAS support - that seems wrong.

Nak :-)

We want to always instantiate it from prom_init.c because we can't do
it any more later. There's the vague possibility that you may want to
boot a non-RTAS kernel which then kexec's into an RTAS kernel, and that
isn't possible if the initial prom_init.c didn't instanciate RTAS and
put a reference to it in the flat device-tree.

Cheers,
Ben.

> Signed-off-by: Michael Ellerman 
> ---
>  arch/powerpc/kernel/prom_init.c |4 
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
> index 2f0e64b..6c2dc59 100644
> --- a/arch/powerpc/kernel/prom_init.c
> +++ b/arch/powerpc/kernel/prom_init.c
> @@ -1052,6 +1052,7 @@ static void __init prom_init_mem(void)
>  }
>  
> 
> +#ifdef CONFIG_PPC_RTAS
>  /*
>   * Allocate room for and instantiate RTAS
>   */
> @@ -1109,6 +1110,9 @@ static void __init prom_instantiate_rtas(void)
>  
>   prom_debug("prom_instantiate_rtas: end...\n");
>  }
> +#else
> +static inline void prom_instantiate_rtas(void) { }
> +#endif /* CONFIG_PPC_RTAS */
>  
>  #ifdef CONFIG_PPC64
>  /*

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH 10/33] powerpc/cell: Extract duplicated IOPTE_* to

2009-06-14 Thread Benjamin Herrenschmidt
On Wed, 2009-06-10 at 16:38 +0200, Geert Uytterhoeven wrote:
> Both arch/powerpc/platforms/cell/iommu.c and arch/powerpc/platforms/ps3/mm.c
> contain the same Cell IOMMU page table entry definitions. Extract them and 
> move
> them to , while adding a CBE_ prefix.
> This also allows them to be used by drivers.

Are we sure include/asm/iommu.h is the right place for that ?

I'm hesitating here...

Cheers,
Ben.

> Signed-off-by: Geert Uytterhoeven 
> ---
> v2: Add CBE_ prefix
> 
>  arch/powerpc/include/asm/iommu.h|   10 
>  arch/powerpc/platforms/cell/iommu.c |   37 --
>  arch/powerpc/platforms/ps3/mm.c |7 -
>  arch/powerpc/platforms/ps3/platform.h   |   10 
>  arch/powerpc/platforms/ps3/system-bus.c |   15 +++-
>  5 files changed, 39 insertions(+), 40 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/iommu.h 
> b/arch/powerpc/include/asm/iommu.h
> index 7464c0d..7ead7c1 100644
> --- a/arch/powerpc/include/asm/iommu.h
> +++ b/arch/powerpc/include/asm/iommu.h
> @@ -35,6 +35,16 @@
>  #define IOMMU_PAGE_MASK   (~((1 << IOMMU_PAGE_SHIFT) - 1))
>  #define IOMMU_PAGE_ALIGN(addr) _ALIGN_UP(addr, IOMMU_PAGE_SIZE)
>  
> +/* Cell page table entries */
> +#define CBE_IOPTE_PP_W   0x8000ul /* protection: 
> write */
> +#define CBE_IOPTE_PP_R   0x4000ul /* protection: 
> read */
> +#define CBE_IOPTE_M  0x2000ul /* coherency required */
> +#define CBE_IOPTE_SO_R   0x1000ul /* ordering: 
> writes */
> +#define CBE_IOPTE_SO_RW  0x1800ul /* ordering: r & w 
> */
> +#define CBE_IOPTE_RPN_Mask   0x07fff000ul /* RPN */
> +#define CBE_IOPTE_H  0x0800ul /* cache hint */
> +#define CBE_IOPTE_IOID_Mask  0x07fful /* ioid */
> +
>  /* Boot time flags */
>  extern int iommu_is_off;
>  extern int iommu_force_on;
> diff --git a/arch/powerpc/platforms/cell/iommu.c 
> b/arch/powerpc/platforms/cell/iommu.c
> index bed4690..5b34fc2 100644
> --- a/arch/powerpc/platforms/cell/iommu.c
> +++ b/arch/powerpc/platforms/cell/iommu.c
> @@ -100,16 +100,6 @@
>  #define IOSTE_PS_1M  0x0005ul /*   - 1MB  */
>  #define IOSTE_PS_16M 0x0007ul /*   - 16MB */
>  
> -/* Page table entries */
> -#define IOPTE_PP_W   0x8000ul /* protection: write */
> -#define IOPTE_PP_R   0x4000ul /* protection: read */
> -#define IOPTE_M  0x2000ul /* coherency 
> required */
> -#define IOPTE_SO_R   0x1000ul /* ordering: writes */
> -#define IOPTE_SO_RW 0x1800ul /* ordering: r & w */
> -#define IOPTE_RPN_Mask   0x07fff000ul /* RPN */
> -#define IOPTE_H  0x0800ul /* cache hint */
> -#define IOPTE_IOID_Mask  0x07fful /* ioid */
> -
>  
>  /* IOMMU sizing */
>  #define IO_SEGMENT_SHIFT 28
> @@ -193,19 +183,21 @@ static int tce_build_cell(struct iommu_table *tbl, long 
> index, long npages,
>*/
>   const unsigned long prot = 0xc48;
>   base_pte =
> - ((prot << (52 + 4 * direction)) & (IOPTE_PP_W | IOPTE_PP_R))
> - | IOPTE_M | IOPTE_SO_RW | (window->ioid & IOPTE_IOID_Mask);
> + ((prot << (52 + 4 * direction)) &
> +  (CBE_IOPTE_PP_W | CBE_IOPTE_PP_R)) |
> + CBE_IOPTE_M | CBE_IOPTE_SO_RW |
> + (window->ioid & CBE_IOPTE_IOID_Mask);
>  #else
> - base_pte = IOPTE_PP_W | IOPTE_PP_R | IOPTE_M | IOPTE_SO_RW |
> - (window->ioid & IOPTE_IOID_Mask);
> + base_pte = CBE_IOPTE_PP_W | CBE_IOPTE_PP_R | CBE_IOPTE_M |
> + CBE_IOPTE_SO_RW | (window->ioid & CBE_IOPTE_IOID_Mask);
>  #endif
>   if (unlikely(dma_get_attr(DMA_ATTR_WEAK_ORDERING, attrs)))
> - base_pte &= ~IOPTE_SO_RW;
> + base_pte &= ~CBE_IOPTE_SO_RW;
>  
>   io_pte = (unsigned long *)tbl->it_base + (index - tbl->it_offset);
>  
>   for (i = 0; i < npages; i++, uaddr += IOMMU_PAGE_SIZE)
> - io_pte[i] = base_pte | (__pa(uaddr) & IOPTE_RPN_Mask);
> + io_pte[i] = base_pte | (__pa(uaddr) & CBE_IOPTE_RPN_Mask);
>  
>   mb();
>  
> @@ -231,8 +223,9 @@ static void tce_free_cell(struct iommu_table *tbl, long 
> index, long npages)
>  #else
>   /* spider bridge does PCI reads after freeing - insert a mapping
>* to a scratch page instead of an invalid entry */
> - pte = IOPTE_PP_R | IOPTE_M | IOPTE_SO_RW | __pa(window->iommu->pad_page)
> - | (window->ioid & IOPTE_IOID_Mask);
> + pte = CBE_IOPTE_PP_R | CBE_IOPTE_M | CBE_IOPTE_SO_RW |
> + __pa(window->iommu->pad_page) |
> + (window->ioid & CBE_IOPTE_IOID_Mask);
>  #endif
>  
>   io_pte = (unsigned long *)tbl->it_base + (index - tbl->it_offset);
> @@ -1001,7 +994,7 @@ static void insert_16M

Re: [PATCH 05/33] block: Add bio_list_peek()

2009-06-14 Thread Benjamin Herrenschmidt
On Wed, 2009-06-10 at 16:38 +0200, Geert Uytterhoeven wrote:
> Introduce bio_list_peek(), to obtain a pointer to the first bio on the 
> bio_list
> without actually removing it from the list. This is needed when you want to
> serialize based on the list being empty or not.

Leaving that one (and the next one) out for now until Jens Ack them.

Cheers,
Ben.

> Signed-off-by: Geert Uytterhoeven 
> Cc: Jens Axboe 
> ---
>  include/linux/bio.h |5 +
>  1 files changed, 5 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/bio.h b/include/linux/bio.h
> index 7b214fd..618bb7d 100644
> --- a/include/linux/bio.h
> +++ b/include/linux/bio.h
> @@ -590,6 +590,11 @@ static inline void bio_list_merge_head(struct bio_list 
> *bl,
>   bl->head = bl2->head;
>  }
>  
> +static inline struct bio *bio_list_peek(struct bio_list *bl)
> +{
> + return bl->head;
> +}
> +
>  static inline struct bio *bio_list_pop(struct bio_list *bl)
>  {
>   struct bio *bio = bl->head;

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [OOPS] hugetlbfs tests with 2.6.30-rc8-git1

2009-06-14 Thread Stephen Rothwell
Hi Michael,

On Mon, 15 Jun 2009 10:56:36 +1000 Michael Ellerman  
wrote:
>
> Rather than "-git7" can you tell us the actual SHA, I don't know what
> git7 is.

$ cat tools/get_gitid 
#!/bin/sh

wget -q -O - http://www.kernel.org/pub/linux/kernel/v2.6/snapshots/patch-$1.id

-- 
Cheers,
Stephen Rothwells...@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/


pgpOt5dxhiwcI.pgp
Description: PGP signature
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [OOPS] hugetlbfs tests with 2.6.30-rc8-git1

2009-06-14 Thread Michael Ellerman
On Sun, 2009-06-14 at 17:08 +0530, Sachin Sant wrote:
> Benjamin Herrenschmidt wrote:
> > On Fri, 2009-06-05 at 16:59 +0530, Sachin Sant wrote:
> >   
> >> While executing Hugetlbfs tests against 2.6.30-rc8-git1 on a
> >> Power 6 box observed the following OOPS message.
> I was able to recreate this with 2.6.30-git7.

Hi Sachin,

Rather than "-git7" can you tell us the actual SHA, I don't know what
git7 is.

> Here is the supporting data.
> 
>  cpu 0x1: Vector: 300 (Data Access) at [c000fe9b3220]
> pc: c003d620: .hpte_need_flush+0x1bc/0x2d8
> lr: c003d4d0: .hpte_need_flush+0x6c/0x2d8
> sp: c000fe9b34a0
>msr: 80009032
>dar: c000283b0d78

This address looks pretty innocuous, but I notice you have
DEBUG_PAGEALLOC=y, so presumably that's why it's triggering.

I can't see from the snippet of disassembly you posted where in the C
code we are, can you work it out?

cheers



signature.asc
Description: This is a digitally signed message part
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH] powerpc: Fix invalid construct in our CPU selection Kconfig

2009-06-14 Thread Benjamin Herrenschmidt
commit 5b7c3c918c9c26c50d220b2b50359208cb5a1dbe introduced an invalid
construct in our CPU selection. This caused warnings, though it still
appeared to do the right thing.

This fixes it properly by having separate formal definitions of
PPC_BOOK3S_32 and PPC_BOOK3S_64 and one statement defining
PPC_BOOK3S based on the two above.

Signed-off-by: Benjamin Herrenschmidt 
---

 arch/powerpc/platforms/Kconfig.cputype |9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

--- linux-work.orig/arch/powerpc/platforms/Kconfig.cputype  2009-06-15 
10:39:54.0 +1000
+++ linux-work/arch/powerpc/platforms/Kconfig.cputype   2009-06-15 
10:41:48.0 +1000
@@ -21,7 +21,7 @@ choice
 
  If unsure, select 52xx/6xx/7xx/74xx/82xx/83xx/86xx.
 
-config PPC_BOOK3S
+config PPC_BOOK3S_32
bool "512x/52xx/6xx/7xx/74xx/82xx/83xx/86xx"
select PPC_FPU
 
@@ -57,11 +57,14 @@ config E200
 
 endchoice
 
-config PPC_BOOK3S
-   default y
+config PPC_BOOK3S_64
+   def_bool y
depends on PPC64
select PPC_FPU
 
+config PPC_BOOK3S
+   def_bool y
+   depends on PPC_BOOK3S_32 || PPC_BOOK3S_64
 
 config POWER4_ONLY
bool "Optimize for POWER4"
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH 2/2] uio: add an of_genirq driver

2009-06-14 Thread Hans J. Koch
On Mon, Jun 15, 2009 at 01:46:43AM +0200, Wolfram Sang wrote:
> 
> > driver. A user _has_ to setup irq, if there is none, he still has to set
> > irq=UIO_IRQ_NONE. For that matter, 'not specified' and 'not found' is both
> > the same bad thing.
> 
> Hmm, what should I do?
> 
> A typical interrupts-property in a device-tree is specified as:
> 
>   interrupts = <&irq_controller_node irq_number irq_sense>;
> 
> Something like UIO_IRQ_NONE does not fit into this scheme, even more as it is
> Linux-specific and device trees need to be OS independant.
> 
> I'm pretty sure the correct way to state that you don't need an interrupt in
> the device-tree is to simply not specify the above interrupt property.
> 
> Well, yes, that means you can't distinguish between 'forgotten' and
> 'intentionally left out'. I wonder if it is really that bad? If something does
> not work (= one is missing interrupts), the first place to look at is the
> device tree. If one does not see an interrupt-property, voila, problem solved.
> 
> (Note that with my latest suggestion, a _wrong_ interrupt is handled the same
> way as with platform_data. request_irq() should equally fail if the
> return-value from irq_of_parse_and_map() is simply forwarded.)

I agree. And assuming Alan is right, forget what I said about IRQ 0 being a
valid interrupt number.

Thanks,
Hans

> 
> -- 
> Pengutronix e.K.   | Wolfram Sang|
> Industrial Linux Solutions | http://www.pengutronix.de/  |


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH 2/2] uio: add an of_genirq driver

2009-06-14 Thread Wolfram Sang

> driver. A user _has_ to setup irq, if there is none, he still has to set
> irq=UIO_IRQ_NONE. For that matter, 'not specified' and 'not found' is both
> the same bad thing.

Hmm, what should I do?

A typical interrupts-property in a device-tree is specified as:

interrupts = <&irq_controller_node irq_number irq_sense>;

Something like UIO_IRQ_NONE does not fit into this scheme, even more as it is
Linux-specific and device trees need to be OS independant.

I'm pretty sure the correct way to state that you don't need an interrupt in
the device-tree is to simply not specify the above interrupt property.

Well, yes, that means you can't distinguish between 'forgotten' and
'intentionally left out'. I wonder if it is really that bad? If something does
not work (= one is missing interrupts), the first place to look at is the
device tree. If one does not see an interrupt-property, voila, problem solved.

(Note that with my latest suggestion, a _wrong_ interrupt is handled the same
way as with platform_data. request_irq() should equally fail if the
return-value from irq_of_parse_and_map() is simply forwarded.)

-- 
Pengutronix e.K.   | Wolfram Sang|
Industrial Linux Solutions | http://www.pengutronix.de/  |


signature.asc
Description: Digital signature
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 2/2] uio: add an of_genirq driver

2009-06-14 Thread Hans J. Koch
On Mon, Jun 15, 2009 at 12:12:07AM +0100, Alan Cox wrote:
> > > + if (!uioinfo->irq)
> > > + uioinfo->irq = UIO_IRQ_NONE;
> > 
> > Please don't do this. It's inconsistent if all other UIO drivers require
> > people to use UIO_IRQ_NONE and you also allow zero. UIO_IRQ_NONE was
> > introduced because 0 may be a legal interrupt number on some platforms.
> 
> Zero is not a valid IRQ number in the kernel (except in arch specific
> depths). IRQ numbers are also *unsigned* so -1 isn't a safe definition.

The above uioinfo->irq is a signed long.

> 
> Zero means no IRQ. If any old UIO code is assuming otherwise it wants
> fixing.

It doesn't. It won't complain about IRQ 0 and will pass it on to
request_irq, which will probably fail if it is as you say.

I did it that way because I saw IRQ 0 in /proc/interrupts on every PC...

> 
> It is the job of the platform to map a physical IRQ 0 to some other
> representation if it exists outside of arch specific code.

Funny.

> This was
> decided some years ago and a large part of the kernel simply doesn't
> support any notion of a real IRQ 0.

Can you tell me the reason for that decision or point me to some ml archive?

Thanks,
Hans

> 
> Alan
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH 2/2] uio: add an of_genirq driver

2009-06-14 Thread Alan Cox
> > +   if (!uioinfo->irq)
> > +   uioinfo->irq = UIO_IRQ_NONE;
> 
> Please don't do this. It's inconsistent if all other UIO drivers require
> people to use UIO_IRQ_NONE and you also allow zero. UIO_IRQ_NONE was
> introduced because 0 may be a legal interrupt number on some platforms.

Zero is not a valid IRQ number in the kernel (except in arch specific
depths). IRQ numbers are also *unsigned* so -1 isn't a safe definition.

Zero means no IRQ. If any old UIO code is assuming otherwise it wants
fixing.

It is the job of the platform to map a physical IRQ 0 to some other
representation if it exists outside of arch specific code. This was
decided some years ago and a large part of the kernel simply doesn't
support any notion of a real IRQ 0.

Alan
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH 2/2] uio: add an of_genirq driver

2009-06-14 Thread Hans J. Koch
On Mon, Jun 15, 2009 at 12:00:22AM +0200, Wolfram Sang wrote:
> > > For x86 it's not defined at all. But as this code is for the PowerPC,
> > 
> > No, it isn't. What makes you say that? The Kconfig entry doesn't depend
> > on PowerPC. I compiled it on x86...
> 
> It depends on OF. You could compile on x86? Have to check that...

Ooops, forget it. It cannot be selected on x86. I was a bit distracted when
I wrote that, sorry.

> 
> > > where using NO_IRQ seems still to be OK.
> > 
> > No. uio_pdrv_genirq can be used on all platforms, and not all platforms have
> > NO_IRQ. NO_IRQ can be used in platform specific code only.
> 
> Well, the wrapper uses irq_of_parse_and_map(). So far, it returns NO_IRQ if an
> IRQ was not specified (or not found). I could check if there was an
> interrupt-property at all, so I can distinguish between 'not specified' and
> 'not found'. Then, UIO_IRQ_NONE would only be used, if there was none
> specified. Otherwise it will always be the result from irq_of_parse_and_map(),
> whatever that is (even NO_IRQ). Is this what you have in mind?

I would find it better if probe() failed if no irq is specified, printing a
message that tells the user to setup his data correctly before loading the
driver. A user _has_ to setup irq, if there is none, he still has to set
irq=UIO_IRQ_NONE. For that matter, 'not specified' and 'not found' is both
the same bad thing.

Thanks,
Hans

> 
> Regards,
> 
>Wolfram
> 
> -- 
> Pengutronix e.K.   | Wolfram Sang|
> Industrial Linux Solutions | http://www.pengutronix.de/  |


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH 2/2] uio: add an of_genirq driver

2009-06-14 Thread Wolfram Sang
> > For x86 it's not defined at all. But as this code is for the PowerPC,
> 
> No, it isn't. What makes you say that? The Kconfig entry doesn't depend
> on PowerPC. I compiled it on x86...

It depends on OF. You could compile on x86? Have to check that...

> > where using NO_IRQ seems still to be OK.
> 
> No. uio_pdrv_genirq can be used on all platforms, and not all platforms have
> NO_IRQ. NO_IRQ can be used in platform specific code only.

Well, the wrapper uses irq_of_parse_and_map(). So far, it returns NO_IRQ if an
IRQ was not specified (or not found). I could check if there was an
interrupt-property at all, so I can distinguish between 'not specified' and
'not found'. Then, UIO_IRQ_NONE would only be used, if there was none
specified. Otherwise it will always be the result from irq_of_parse_and_map(),
whatever that is (even NO_IRQ). Is this what you have in mind?

Regards,

   Wolfram

-- 
Pengutronix e.K.   | Wolfram Sang|
Industrial Linux Solutions | http://www.pengutronix.de/  |


signature.asc
Description: Digital signature
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 2/2] uio: add an of_genirq driver

2009-06-14 Thread Wolfram Sang
On Sun, Jun 14, 2009 at 12:27:07PM -0700, Greg KH wrote:
> On Sun, Jun 14, 2009 at 09:05:33PM +0200, Wolfram Sang wrote:
> > Well, I assume that issues regarding checkpatch do not have the highest
> > priority (especially while the merge-window is open), which is 
> > understandable.
> 
> Hm, the "merge-window" for new stuff like these patches is pretty much
> already closed, as you didn't send them _before_ the merge window opened
> up.  You need to get them to us sooner, so we can test stuff out in the
> -next tree for a while before we can merge them.

Seems you got me wrong here :)

As I stated in the introduction ("PATCH [0/2]"), this patch series is _not_
meant for the current merge-window. I just happened to be done with it now.

With the above sentence I just wanted to give a hint, why there was not a reply
to my checkpatch-mail (as Hans seemed to be concerned about that there was
none).

Regards,

   Wolfram

-- 
Pengutronix e.K.   | Wolfram Sang|
Industrial Linux Solutions | http://www.pengutronix.de/  |


signature.asc
Description: Digital signature
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 2/2] uio: add an of_genirq driver

2009-06-14 Thread Hans J. Koch
On Sun, Jun 14, 2009 at 09:36:53PM +0200, Wolfgang Grandegger wrote:
>   if (uioinfo->irq == NO_IRQ)
>   uioinfo->irq = UIO_IRQ_NONE;
> >>> Sorry for my ignorance, but what is NO_IRQ? If I do
> 
> It's 0 on PowerPC but ARM seems still to use -1.

Using 0 is simply wrong, especially if people do something like

if (!irq)
return -ERROR;

IRQ number 0 _is_ a valid irq. Maybe not on all platforms, but in generic
code (like UIO) you have to assume it is.

> 
> http://lxr.linux.no/linux+v2.6.30/arch/powerpc/include/asm/irq.h#L29
> 
> For x86 it's not defined at all. But as this code is for the PowerPC,

No, it isn't. What makes you say that? The Kconfig entry doesn't depend
on PowerPC. I compiled it on x86...

> where using NO_IRQ seems still to be OK.

No. uio_pdrv_genirq can be used on all platforms, and not all platforms have
NO_IRQ. NO_IRQ can be used in platform specific code only.

Anyway, if someone fills in an invalid irq in his platform data, he deserves
to crash. No need for that test. UIO docs state that irq is a _required_
element. If you forget to set it, it will probably be 0. On most platforms,
register_irq will fail with that, and you'll notice. If you silently
replace it with UIO_IRQ_NONE, you simply cover up wrong code.

Thanks,
Hans

> 
> Wolfgang.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH 2/2] uio: add an of_genirq driver

2009-06-14 Thread Greg KH
On Sun, Jun 14, 2009 at 09:05:33PM +0200, Wolfram Sang wrote:
> Well, I assume that issues regarding checkpatch do not have the highest
> priority (especially while the merge-window is open), which is understandable.

Hm, the "merge-window" for new stuff like these patches is pretty much
already closed, as you didn't send them _before_ the merge window opened
up.  You need to get them to us sooner, so we can test stuff out in the
-next tree for a while before we can merge them.

thanks,

greg k-h
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH 2/2] uio: add an of_genirq driver

2009-06-14 Thread Wolfgang Grandegger
Hans J. Koch wrote:
> On Sun, Jun 14, 2009 at 09:05:33PM +0200, Wolfram Sang wrote:
>>> Anyway, 0 is a valid IRQ number, so it cannot be used as "no irq".
>> May I point you to this thread?
>>
>> http://lkml.org/lkml/2005/11/21/221
> 
> Linus is just plain wrong in this 4 year old mail.

See also this related thread.

http://groups.google.com/group/rtc-linux/browse_thread/thread/9816648d5a8a1c9e/9968968188b5ab5a?lnk=gst&q=rx8025#9968968188b5ab5a

> 
>> (The issue comes up once in a while as some archs still use NO_IRQ, some with
>> 0 some with -1)
>>
if (uioinfo->irq == NO_IRQ)
uioinfo->irq = UIO_IRQ_NONE;
>>> Sorry for my ignorance, but what is NO_IRQ? If I do

It's 0 on PowerPC but ARM seems still to use -1.

http://lxr.linux.no/linux+v2.6.30/arch/powerpc/include/asm/irq.h#L29

For x86 it's not defined at all. But as this code is for the PowerPC,
where using NO_IRQ seems still to be OK.

Wolfgang.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH 2/2] uio: add an of_genirq driver

2009-06-14 Thread Hans J. Koch
On Sun, Jun 14, 2009 at 09:05:33PM +0200, Wolfram Sang wrote:
> 
> > Anyway, 0 is a valid IRQ number, so it cannot be used as "no irq".
> 
> May I point you to this thread?
> 
> http://lkml.org/lkml/2005/11/21/221

Linus is just plain wrong in this 4 year old mail.

> 
> (The issue comes up once in a while as some archs still use NO_IRQ, some with
> 0 some with -1)
> 
> > >   if (uioinfo->irq == NO_IRQ)
> > >   uioinfo->irq = UIO_IRQ_NONE;
> > 
> > Sorry for my ignorance, but what is NO_IRQ? If I do a
> > 
> > grep -r NO_IRQ include/
> > 
> > I get nothing.
> 
> Try a 'cd arch' before that :)

no such luck in arch/x86/ ...

> 
> > Well, you claim it's a false positive. So far, you did not get any 
> > responses,
> > AFAICS. I tend to agree with you, but I'd like to avoid patches that don't
> > pass checkpatch.pl, whatever the reason. Either the false positive gets
> > confirmed and fixed, or you should fix your patch.
> 
> Well, I assume that issues regarding checkpatch do not have the highest
> priority (especially while the merge-window is open), which is understandable.
> Fixing this bug (I take any bets that this is one ;)) might not be so trivial,
> as modifying $Attributes can easily have lots of side-effects.
> 
> Now, all this does not matter much, as the objections Grant raised are valid
> and there might be a totally different outcome to bind devices to UIO. But at
> least, we have some code to discuss...

OK, I'm looking forward to your next version.

Thanks,
Hans


> 
> Regards,
> 
>Wolfram
> 
> -- 
> Pengutronix e.K.   | Wolfram Sang|
> Industrial Linux Solutions | http://www.pengutronix.de/  |


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH 2/2] uio: add an of_genirq driver

2009-06-14 Thread Wolfram Sang

> Anyway, 0 is a valid IRQ number, so it cannot be used as "no irq".

May I point you to this thread?

http://lkml.org/lkml/2005/11/21/221

(The issue comes up once in a while as some archs still use NO_IRQ, some with
0 some with -1)

> > if (uioinfo->irq == NO_IRQ)
> > uioinfo->irq = UIO_IRQ_NONE;
> 
> Sorry for my ignorance, but what is NO_IRQ? If I do a
> 
> grep -r NO_IRQ include/
> 
> I get nothing.

Try a 'cd arch' before that :)

> Well, you claim it's a false positive. So far, you did not get any responses,
> AFAICS. I tend to agree with you, but I'd like to avoid patches that don't
> pass checkpatch.pl, whatever the reason. Either the false positive gets
> confirmed and fixed, or you should fix your patch.

Well, I assume that issues regarding checkpatch do not have the highest
priority (especially while the merge-window is open), which is understandable.
Fixing this bug (I take any bets that this is one ;)) might not be so trivial,
as modifying $Attributes can easily have lots of side-effects.

Now, all this does not matter much, as the objections Grant raised are valid
and there might be a totally different outcome to bind devices to UIO. But at
least, we have some code to discuss...

Regards,

   Wolfram

-- 
Pengutronix e.K.   | Wolfram Sang|
Industrial Linux Solutions | http://www.pengutronix.de/  |


signature.asc
Description: Digital signature
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 2/2] uio: add an of_genirq driver

2009-06-14 Thread Hans J. Koch
On Sun, Jun 14, 2009 at 07:14:06PM +0200, Wolfram Sang wrote:
> Hello Hans,
> 
> > > + uioinfo->irq = irq_of_parse_and_map(op->node, 0);
> > > + if (!uioinfo->irq)
> > > + uioinfo->irq = UIO_IRQ_NONE;
> > 
> > Please don't do this. It's inconsistent if all other UIO drivers require
> > people to use UIO_IRQ_NONE and you also allow zero. UIO_IRQ_NONE was
> > introduced because 0 may be a legal interrupt number on some platforms.
> 
> Yes, well, the '0' vs. 'NO_IRQ' thing is still not fully sorted out AFAIK.

A "cat /proc/interrupts" on any common x86 PC shows you that IRQ 0 is used
there. OK, it's unlikely someone wants to write a UIO driver for that one,
but that might be different on other platforms.
Anyway, 0 is a valid IRQ number, so it cannot be used as "no irq".

> But you are possibly right here, as long as irq_of_parse_and_map does return
> NO_IRQ, I should explicitly check for it, like this:
> 
>   if (uioinfo->irq == NO_IRQ)
>   uioinfo->irq = UIO_IRQ_NONE;

Sorry for my ignorance, but what is NO_IRQ? If I do a

grep -r NO_IRQ include/

I get nothing.

> 
> > > +/* Match table for of_platform binding */
> > > +static const struct of_device_id __devinitconst uio_of_genirq_match[] = {
> > 
> > checkpatch.pl complains about that. Please check.
> 
> Did that, it is a false positive. See here:
> 
> http://lkml.indiana.edu/hypermail/linux/kernel/0906.1/02780.html

Well, you claim it's a false positive. So far, you did not get any responses,
AFAICS. I tend to agree with you, but I'd like to avoid patches that don't
pass checkpatch.pl, whatever the reason. Either the false positive gets
confirmed and fixed, or you should fix your patch.

Thanks,
Hans

> 
> Regards,
> 
>Wolfram
> 
> -- 
> Pengutronix e.K.   | Wolfram Sang|
> Industrial Linux Solutions | http://www.pengutronix.de/  |


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH 2/2] uio: add an of_genirq driver

2009-06-14 Thread Wolfram Sang
Hello Hans,

> > +   uioinfo->irq = irq_of_parse_and_map(op->node, 0);
> > +   if (!uioinfo->irq)
> > +   uioinfo->irq = UIO_IRQ_NONE;
> 
> Please don't do this. It's inconsistent if all other UIO drivers require
> people to use UIO_IRQ_NONE and you also allow zero. UIO_IRQ_NONE was
> introduced because 0 may be a legal interrupt number on some platforms.

Yes, well, the '0' vs. 'NO_IRQ' thing is still not fully sorted out AFAIK. But
you are possibly right here, as long as irq_of_parse_and_map does return
NO_IRQ, I should explicitly check for it, like this:

if (uioinfo->irq == NO_IRQ)
uioinfo->irq = UIO_IRQ_NONE;

> > +/* Match table for of_platform binding */
> > +static const struct of_device_id __devinitconst uio_of_genirq_match[] = {
> 
> checkpatch.pl complains about that. Please check.

Did that, it is a false positive. See here:

http://lkml.indiana.edu/hypermail/linux/kernel/0906.1/02780.html

Regards,

   Wolfram

-- 
Pengutronix e.K.   | Wolfram Sang|
Industrial Linux Solutions | http://www.pengutronix.de/  |


signature.asc
Description: Digital signature
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 2/2] uio: add an of_genirq driver

2009-06-14 Thread Grant Likely
On Thu, Jun 11, 2009 at 6:04 PM, Wolfram Sang wrote:
> Picking up the now exported generic probe function from the
> platform-variant of this driver, this patch adds an of-version. Also add
> the binding documentation.
>
> Signed-off-by: Wolfram Sang 
> Cc: Magnus Damm 
> Cc: Hans J. Koch 
> Cc: Greg KH 
> ---
>
> In probe, I put the resources-array on the stack to simplify the code. If this
> is considered too huge for the stack (140 byte for a 32-bit system at the
> moment), I can also post a version using kzalloc.
>
>  Documentation/powerpc/dts-bindings/uio-generic.txt |   16 +++
>  drivers/uio/Kconfig                                |    6 +
>  drivers/uio/Makefile                               |    1 +
>  drivers/uio/uio_of_genirq.c                        |   98 
> 
>  4 files changed, 121 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/powerpc/dts-bindings/uio-generic.txt
>  create mode 100644 drivers/uio/uio_of_genirq.c
>
> diff --git a/Documentation/powerpc/dts-bindings/uio-generic.txt 
> b/Documentation/powerpc/dts-bindings/uio-generic.txt
> new file mode 100644
> index 000..8ad9861
> --- /dev/null
> +++ b/Documentation/powerpc/dts-bindings/uio-generic.txt
> @@ -0,0 +1,16 @@
> +UIO for custom devices
> +
> +A device which will be mapped using the UIO subsystem.
> +
> +Properties:
> + - compatible : should contain the specific model used, followed by
> +                "generic-uio".
> + - reg : address range(s) of the device (up to MAX_UIO_MAPS)
> + - interrupts : interrupt of the device
> +
> +Example:
> +        c64f...@0 {
> +                compatible = "ptx,c64fpga001", "generic-uio";
> +                reg = <0x0 0x1>;
> +                interrupts = <0 0 3>;
> +        };

Hmmm, I'm not happy about this.  The device tree describes the
hardware, not the way Linux uses the hardware.  UIO definitely falls
into the category of Linux implementation detail.

This should be approached from the other way around.  Either the
generic-uio of_platform driver should contain an explicit list of
devices to be handled by UIO, or the OF infrastructure should be
modified to allow things like force binding of_devices to of_drivers
at runtime.

g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH v2] i2c-mpc: generate START condition after STOP caused by read i2c_msg

2009-06-14 Thread Esben Haabendal

Ben Dooks wrote:

is there a new version of this patch available?

  

I will catch up on it ASAP.

/Esben

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH v2] i2c-mpc: generate START condition after STOP caused by read i2c_msg

2009-06-14 Thread Ben Dooks
is there a new version of this patch available?

-- 
Ben (b...@fluff.org, http://www.fluff.org/)

  'a smiley only costs 4 bytes'
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH 1/2] lib: Provide generic atomic64_t implementation

2009-06-14 Thread Avi Kivity

Paul Mackerras wrote:

Avi Kivity writes:

  
An alternative implementation using 64-bit cmpxchg will recover most of 
the costs of hashed spinlocks.  I assume most serious 32-bit 
architectures have them?



Have a 64-bit cmpxchg, you mean?  x86 is the only one I know of, and
it already has an atomic64_t implementation using cmpxchg8b (or
whatever it's called).
  


Yes (and it is cmpxchg8b).  I'm surprised powerpc doesn't have DCAS support.


My thinking is that the 32-bit non-x86 architectures will be mostly
UP, so the overhead is just an interrupt enable/restore.  Those that
are SMP I would expect to be small SMP -- mostly just 2 cpus and maybe
a few 4-way systems.
  


The new Nehalems provide 8 logical threads in a single socket.  All 
those threads share a cache, and they have cmpxchg8b anyway, so this 
won't matter.


--
error compiling committee.c: too many arguments to function

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH 1/2] uio/pdrv_genirq: Refactor probe routine to expose a generic part

2009-06-14 Thread Hans J. Koch
On Fri, Jun 12, 2009 at 02:04:21AM +0200, Wolfram Sang wrote:
> This patch refactors the probe routine, so that an of-version of a similiar
> driver can pick it up later.

Looks good to me.

Signed-off-by: Hans J. Koch 

> 
> Signed-off-by: Wolfram Sang 
> Cc: Magnus Damm 
> Cc: Hans J. Koch 
> Cc: Greg KH 
> ---
>  drivers/uio/uio_pdrv_genirq.c   |   60 --
>  include/linux/uio_pdrv_genirq.h |   13 
>  2 files changed, 45 insertions(+), 28 deletions(-)
>  create mode 100644 include/linux/uio_pdrv_genirq.h
> 
> diff --git a/drivers/uio/uio_pdrv_genirq.c b/drivers/uio/uio_pdrv_genirq.c
> index 3f06818..8f8a0f9 100644
> --- a/drivers/uio/uio_pdrv_genirq.c
> +++ b/drivers/uio/uio_pdrv_genirq.c
> @@ -20,15 +20,10 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #define DRIVER_NAME "uio_pdrv_genirq"
>  
> -struct uio_pdrv_genirq_platdata {
> - struct uio_info *uioinfo;
> - spinlock_t lock;
> - unsigned long flags;
> -};
> -
>  static irqreturn_t uio_pdrv_genirq_handler(int irq, struct uio_info 
> *dev_info)
>  {
>   struct uio_pdrv_genirq_platdata *priv = dev_info->priv;
> @@ -68,29 +63,18 @@ static int uio_pdrv_genirq_irqcontrol(struct uio_info 
> *dev_info, s32 irq_on)
>   return 0;
>  }
>  
> -static int uio_pdrv_genirq_probe(struct platform_device *pdev)
> +int __uio_pdrv_genirq_probe(struct device *dev, struct uio_info *uioinfo,
> + struct resource *resources, unsigned int num_resources)
>  {
> - struct uio_info *uioinfo = pdev->dev.platform_data;
>   struct uio_pdrv_genirq_platdata *priv;
>   struct uio_mem *uiomem;
> - int ret = -EINVAL;
> - int i;
> -
> - if (!uioinfo || !uioinfo->name || !uioinfo->version) {
> - dev_err(&pdev->dev, "missing platform_data\n");
> - goto bad0;
> - }
> -
> - if (uioinfo->handler || uioinfo->irqcontrol ||
> - uioinfo->irq_flags & IRQF_SHARED) {
> - dev_err(&pdev->dev, "interrupt configuration error\n");
> - goto bad0;
> - }
> + unsigned int i;
> + int ret;
>  
>   priv = kzalloc(sizeof(*priv), GFP_KERNEL);
>   if (!priv) {
>   ret = -ENOMEM;
> - dev_err(&pdev->dev, "unable to kmalloc\n");
> + dev_err(dev, "unable to kmalloc\n");
>   goto bad0;
>   }
>  
> @@ -100,14 +84,14 @@ static int uio_pdrv_genirq_probe(struct platform_device 
> *pdev)
>  
>   uiomem = &uioinfo->mem[0];
>  
> - for (i = 0; i < pdev->num_resources; ++i) {
> - struct resource *r = &pdev->resource[i];
> + for (i = 0; i < num_resources; ++i) {
> + struct resource *r = resources + i;
>  
>   if (r->flags != IORESOURCE_MEM)
>   continue;
>  
>   if (uiomem >= &uioinfo->mem[MAX_UIO_MAPS]) {
> - dev_warn(&pdev->dev, "device has more than "
> + dev_warn(dev, "device has more than "
>   __stringify(MAX_UIO_MAPS)
>   " I/O memory resources.\n");
>   break;
> @@ -138,19 +122,39 @@ static int uio_pdrv_genirq_probe(struct platform_device 
> *pdev)
>   uioinfo->irqcontrol = uio_pdrv_genirq_irqcontrol;
>   uioinfo->priv = priv;
>  
> - ret = uio_register_device(&pdev->dev, priv->uioinfo);
> + ret = uio_register_device(dev, priv->uioinfo);
>   if (ret) {
> - dev_err(&pdev->dev, "unable to register uio device\n");
> + dev_err(dev, "unable to register uio device\n");
>   goto bad1;
>   }
>  
> - platform_set_drvdata(pdev, priv);
> + dev_set_drvdata(dev, priv);
>   return 0;
>   bad1:
>   kfree(priv);
>   bad0:
>   return ret;
>  }
> +EXPORT_SYMBOL_GPL(__uio_pdrv_genirq_probe);
> +
> +static int uio_pdrv_genirq_probe(struct platform_device *pdev)
> +{
> + struct uio_info *uioinfo = pdev->dev.platform_data;
> +
> + if (!uioinfo || !uioinfo->name || !uioinfo->version) {
> + dev_err(&pdev->dev, "missing platform_data\n");
> + return -EINVAL;
> + }
> +
> + if (uioinfo->handler || uioinfo->irqcontrol ||
> + uioinfo->irq_flags & IRQF_SHARED) {
> + dev_err(&pdev->dev, "interrupt configuration error\n");
> + return -EINVAL;
> + }
> +
> + return __uio_pdrv_genirq_probe(&pdev->dev, uioinfo, pdev->resource,
> + pdev->num_resources);
> +}
>  
>  static int uio_pdrv_genirq_remove(struct platform_device *pdev)
>  {
> diff --git a/include/linux/uio_pdrv_genirq.h b/include/linux/uio_pdrv_genirq.h
> new file mode 100644
> index 000..a410390
> --- /dev/null
> +++ b/include/linux/uio_pdrv_genirq.h
> @@ -0,0 +1,13 @@
> +#ifndef _LINUX_UIO_PDRV_GENIRQ_H
> +#define _LINUX_UIO_PDRV_GENIRQ_H
> +
> +struct uio_pdrv_genirq_platdata {
> + struct uio_info *uioinfo;
> + spinlock_t lock;
> + unsigned long fl

Re: [PATCH 2/2] uio: add an of_genirq driver

2009-06-14 Thread Hans J. Koch
On Fri, Jun 12, 2009 at 02:04:22AM +0200, Wolfram Sang wrote:
> Picking up the now exported generic probe function from the
> platform-variant of this driver, this patch adds an of-version. Also add
> the binding documentation.

Some minor problems, see below.

> 
> Signed-off-by: Wolfram Sang 
> Cc: Magnus Damm 
> Cc: Hans J. Koch 
> Cc: Greg KH 
> ---
> 
> In probe, I put the resources-array on the stack to simplify the code. If this
> is considered too huge for the stack (140 byte for a 32-bit system at the
> moment), I can also post a version using kzalloc.
> 
>  Documentation/powerpc/dts-bindings/uio-generic.txt |   16 +++
>  drivers/uio/Kconfig|6 +
>  drivers/uio/Makefile   |1 +
>  drivers/uio/uio_of_genirq.c|   98 
> 
>  4 files changed, 121 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/powerpc/dts-bindings/uio-generic.txt
>  create mode 100644 drivers/uio/uio_of_genirq.c
> 
> diff --git a/Documentation/powerpc/dts-bindings/uio-generic.txt 
> b/Documentation/powerpc/dts-bindings/uio-generic.txt
> new file mode 100644
> index 000..8ad9861
> --- /dev/null
> +++ b/Documentation/powerpc/dts-bindings/uio-generic.txt
> @@ -0,0 +1,16 @@
> +UIO for custom devices
> +
> +A device which will be mapped using the UIO subsystem.
> +
> +Properties:
> + - compatible : should contain the specific model used, followed by
> +"generic-uio".
> + - reg : address range(s) of the device (up to MAX_UIO_MAPS)
> + - interrupts : interrupt of the device
> +
> +Example:
> +c64f...@0 {
> +compatible = "ptx,c64fpga001", "generic-uio";
> +reg = <0x0 0x1>;
> +interrupts = <0 0 3>;
> +};
> diff --git a/drivers/uio/Kconfig b/drivers/uio/Kconfig
> index 7f86534..18efe38 100644
> --- a/drivers/uio/Kconfig
> +++ b/drivers/uio/Kconfig
> @@ -46,6 +46,12 @@ config UIO_PDRV_GENIRQ
>  
> If you don't know what to do here, say N.
>  
> +config UIO_OF_GENIRQ
> + tristate "Userspace I/O OF driver with generic IRQ handling"
> + depends on UIO_PDRV_GENIRQ && OF
> + help
> +   OF wrapper for the above platform driver.
> +
>  config UIO_SMX
>   tristate "SMX cryptengine UIO interface"
>   default n
> diff --git a/drivers/uio/Makefile b/drivers/uio/Makefile
> index 5c2586d..089fd56 100644
> --- a/drivers/uio/Makefile
> +++ b/drivers/uio/Makefile
> @@ -2,6 +2,7 @@ obj-$(CONFIG_UIO) += uio.o
>  obj-$(CONFIG_UIO_CIF)+= uio_cif.o
>  obj-$(CONFIG_UIO_PDRV)   += uio_pdrv.o
>  obj-$(CONFIG_UIO_PDRV_GENIRQ)+= uio_pdrv_genirq.o
> +obj-$(CONFIG_UIO_OF_GENIRQ)  += uio_of_genirq.o
>  obj-$(CONFIG_UIO_SMX)+= uio_smx.o
>  obj-$(CONFIG_UIO_AEC)+= uio_aec.o
>  obj-$(CONFIG_UIO_SERCOS3)+= uio_sercos3.o
> diff --git a/drivers/uio/uio_of_genirq.c b/drivers/uio/uio_of_genirq.c
> new file mode 100644
> index 000..254ec5b
> --- /dev/null
> +++ b/drivers/uio/uio_of_genirq.c
> @@ -0,0 +1,98 @@
> +/*
> + * OF wrapper to make use of the uio_pdrv_genirq-driver.
> + *
> + * Copyright (C) 2009 Wolfram Sang, Pengutronix
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published 
> by
> + * the Free Software Foundation.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define OF_DRIVER_VERSION "1"
> +
> +static __devinit int uio_of_genirq_probe(struct of_device *op,
> + const struct of_device_id *match)
> +{
> + struct uio_info *uioinfo;
> + struct resource resources[MAX_UIO_MAPS];
> + int i, ret;
> +
> + uioinfo = kzalloc(sizeof(*uioinfo), GFP_KERNEL);
> + if (!uioinfo)
> + return -ENOMEM;
> +
> + uioinfo->name = op->node->name;
> + uioinfo->version = OF_DRIVER_VERSION;
> + uioinfo->irq = irq_of_parse_and_map(op->node, 0);
> + if (!uioinfo->irq)
> + uioinfo->irq = UIO_IRQ_NONE;

Please don't do this. It's inconsistent if all other UIO drivers require
people to use UIO_IRQ_NONE and you also allow zero. UIO_IRQ_NONE was
introduced because 0 may be a legal interrupt number on some platforms.

> +
> + for (i = 0; i < MAX_UIO_MAPS; ++i)
> + if (of_address_to_resource(op->node, i, &resources[i]))
> + break;
> +
> + ret = __uio_pdrv_genirq_probe(&op->dev, uioinfo, &resources, i);
> + if (ret)
> + goto err_cleanup;
> +
> + return 0;
> +
> +err_cleanup:
> + if (uioinfo->irq != UIO_IRQ_NONE)
> + irq_dispose_mapping(uioinfo->irq);
> +
> + kfree(uioinfo);
> + return ret;
> +}
> +
> +static __devexit int uio_of_genirq_remove(struct of_device *op)
> +{
> + struct uio_pdrv_genirq_platdata *priv = dev_get_drvdata(&op->dev);
> +
> + uio_unregister_device(priv->uioinfo);
> +
> + if (priv-

Re: [PATCH 1/2] lib: Provide generic atomic64_t implementation

2009-06-14 Thread Paul Mackerras
Avi Kivity writes:

> An alternative implementation using 64-bit cmpxchg will recover most of 
> the costs of hashed spinlocks.  I assume most serious 32-bit 
> architectures have them?

Have a 64-bit cmpxchg, you mean?  x86 is the only one I know of, and
it already has an atomic64_t implementation using cmpxchg8b (or
whatever it's called).

My thinking is that the 32-bit non-x86 architectures will be mostly
UP, so the overhead is just an interrupt enable/restore.  Those that
are SMP I would expect to be small SMP -- mostly just 2 cpus and maybe
a few 4-way systems.

Paul.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH 1/2] lib: Provide generic atomic64_t implementation

2009-06-14 Thread Avi Kivity

Linus Torvalds wrote:

On Sat, 13 Jun 2009, Linus Torvalds wrote:
  

On Sat, 13 Jun 2009, Paul Mackerras wrote:


Linus, Andrew: OK if this goes in via the powerpc tree?
  

Ok by me.



Btw, do 32-bit architectures really necessarily want 64-bit performance 
counters? 

I realize that 32-bit counters will overflow pretty easily, but I do 
wonder about the performance impact of doing things like hashed spinlocks 
for 64-bit counters. Maybe the downsides of 64-bit perf counters on such 
architectures might outweight the upsides?
  


An alternative implementation using 64-bit cmpxchg will recover most of 
the costs of hashed spinlocks.  I assume most serious 32-bit 
architectures have them?


--
error compiling committee.c: too many arguments to function

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [OOPS] hugetlbfs tests with 2.6.30-rc8-git1

2009-06-14 Thread Sachin Sant

Benjamin Herrenschmidt wrote:

On Fri, 2009-06-05 at 16:59 +0530, Sachin Sant wrote:
  

While executing Hugetlbfs tests against 2.6.30-rc8-git1 on a
Power 6 box observed the following OOPS message.

I was able to recreate this with 2.6.30-git7.
Here is the supporting data.

cpu 0x1: Vector: 300 (Data Access) at [c000fe9b3220]
   pc: c003d620: .hpte_need_flush+0x1bc/0x2d8
   lr: c003d4d0: .hpte_need_flush+0x6c/0x2d8
   sp: c000fe9b34a0
  msr: 80009032
  dar: c000283b0d78
dsisr: 4000
 current = 0xc000feb54700
 paca= 0xc0c02600
   pid   = 6116, comm = stack_grow_into
enter ? for help
[c000fe9b34a0] c000fc6c0d80 (unreliable)
[c000fe9b3560] c003f384 .huge_ptep_get_and_clear+0x40/0x5c
[c000fe9b35e0] c0149ea0 .__unmap_hugepage_range+0x178/0x2b8
[c000fe9b36d0] c014a034 .unmap_hugepage_range+0x54/0x88
[c000fe9b3770] c0133db0 .unmap_vmas+0x178/0x8f4
[c000fe9b38c0] c01396ec .exit_mmap+0xf4/0x1d0
[c000fe9b3960] c008cf4c .mmput+0x68/0x14c
[c000fe9b39f0] c0091dc0 .exit_mm+0x16c/0x190
[c000fe9b3aa0] c00941c4 .do_exit+0x204/0x748
[c000fe9b3b80] c009479c .do_group_exit+0x94/0xc8
[c000fe9b3c10] c00a321c .get_signal_to_deliver+0x384/0x3e4
[c000fe9b3cf0] c0013d48 .do_signal+0x6c/0x2f8
[c000fe9b3e30] c0008afc do_work+0x24/0x28
--- Exception: 300 (Data Access) at 10002154
SP (ef8b8820) is in userspace
1:mon> r
R00 =    R16 = 4004
R01 = c000fe9b34a0   R17 = 4004
R02 = c0b07610   R18 = 
R03 = 0004   R19 = 
R04 = ef00   R20 = c000fc07
R05 = c000283a8d78   R21 = f000
R06 = 1b4008001393   R22 = c000283a8d78
R07 = 0001   R23 = 007a
R08 = 0004   R24 = 
R09 = c000283b8d78   R25 = 1b4008001393
R10 = 00ef   R26 = c000fc07
R11 = 0025   R27 = 0004
R12 = 24022422   R28 = c08e0518
R13 = c0c02600   R29 = 
R14 = c0001bed0f00   R30 = c1080518
R15 = c000db0c0020   R31 = fe967b124f00
pc  = c003d620 .hpte_need_flush+0x1bc/0x2d8
lr  = c003d4d0 .hpte_need_flush+0x6c/0x2d8
msr = 80009032   cr  = 44022422
ctr = bba4   xer = 0001   trap =  300
dar = c000283b0d78   dsisr = 4000
1:mon> di $.hpte_need_flush
... snip ...
c003d604  480c  b   c003d610# 
.hpte_need_flush+0x1ac/0x2d8
c003d608  792945c6  rldicr  r9,r9,40,23
c003d60c  7be00600  clrldi  r0,r31,24
c003d610  7d3f0378  or  r31,r9,r0
c003d614  7c1cb82e  lwzxr0,r28,r23
c003d618  3d360001  addis   r9,r22,1
c003d61c  2f80  cmpwi   cr7,r0,0
c003d620  eb898000  ld  r28,-32768(r9)
c003d624  409e0028  bne cr7,c003d64c# 
.hpte_need_flush+0x1e8/0x2d8
c003d628  7fe3fb78  mr  r3,r31
c003d62c  7f24cb78  mr  r4,r25
c003d630  7f85e378  mr  r5,r28
c003d634  7f6607b4  extsw   r6,r27
c003d638  7fa7eb78  mr  r7,r29
c003d63c  3900  li  r8,0
... snip ...

This is on a power6 box with 2.4 hugetlbfs tests.
hugetlb test run log ...

truncate_above_4GB (16M: 32):   PASS
truncate_above_4GB (16M: 64):   PASS
brk_near_huge (16M: 32):PASS
brk_near_huge (16M: 64):PASS
task-size-overrun (16M: 32):PASS
task-size-overrun (16M: 64):PASS
stack_grow_into_huge (16M: 32):  ==> fails here


Thanks
-Sachin

--

-
Sachin Sant
IBM Linux Technology Center
India Systems and Technology Labs
Bangalore, India
-

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev