LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH 2/2] uio: add an of_genirq driver
From: Hans J. Koch @ 2009-06-14 20:34 UTC (permalink / raw)
  To: Wolfgang Grandegger
  Cc: devicetree-discuss, Hans J. Koch, linux-kernel, linuxppc-dev,
	Greg KH
In-Reply-To: <4A355155.4020500@grandegger.com>

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.

^ permalink raw reply

* Re: [PATCH 2/2] uio: add an of_genirq driver
From: Greg KH @ 2009-06-14 19:27 UTC (permalink / raw)
  To: Wolfram Sang; +Cc: linuxppc-dev, Hans J. Koch, linux-kernel, devicetree-discuss
In-Reply-To: <20090614190533.GA7387@pengutronix.de>

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

^ permalink raw reply

* Re: [PATCH 2/2] uio: add an of_genirq driver
From: Wolfgang Grandegger @ 2009-06-14 19:36 UTC (permalink / raw)
  To: Hans J. Koch; +Cc: devicetree-discuss, Greg KH, linux-kernel, linuxppc-dev
In-Reply-To: <20090614192359.GG3639@local>

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.

^ permalink raw reply

* Re: [PATCH 2/2] uio: add an of_genirq driver
From: Hans J. Koch @ 2009-06-14 19:23 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: devicetree-discuss, Hans J. Koch, linux-kernel, linuxppc-dev,
	Greg KH
In-Reply-To: <20090614190533.GA7387@pengutronix.de>

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/  |

^ permalink raw reply

* Re: [PATCH 2/2] uio: add an of_genirq driver
From: Wolfram Sang @ 2009-06-14 19:05 UTC (permalink / raw)
  To: Hans J. Koch; +Cc: linuxppc-dev, devicetree-discuss, linux-kernel, Greg KH
In-Reply-To: <20090614183357.GE3639@local>

[-- Attachment #1: Type: text/plain, Size: 1437 bytes --]


> 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/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply

* Re: [PATCH 2/2] uio: add an of_genirq driver
From: Hans J. Koch @ 2009-06-14 18:33 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: devicetree-discuss, Hans J. Koch, linux-kernel, linuxppc-dev,
	Greg KH
In-Reply-To: <20090614171406.GA1010@pengutronix.de>

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/  |

^ permalink raw reply

* Re: [PATCH 2/2] uio: add an of_genirq driver
From: Wolfram Sang @ 2009-06-14 17:14 UTC (permalink / raw)
  To: Hans J. Koch; +Cc: linuxppc-dev, devicetree-discuss, linux-kernel, Greg KH
In-Reply-To: <20090614122136.GD3639@local>

[-- Attachment #1: Type: text/plain, Size: 1123 bytes --]

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/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply

* Re: [PATCH 2/2] uio: add an of_genirq driver
From: Grant Likely @ 2009-06-14 14:40 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: devicetree-discuss, Hans J. Koch, linux-kernel, linuxppc-dev,
	Greg KH
In-Reply-To: <1244765062-14144-3-git-send-email-w.sang@pengutronix.de>

On Thu, Jun 11, 2009 at 6:04 PM, Wolfram Sang<w.sang@pengutronix.de> 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 <w.sang@pengutronix.de>
> Cc: Magnus Damm <magnus.damm@gmail.com>
> Cc: Hans J. Koch <hjk@linutronix.de>
> Cc: Greg KH <gregkh@suse.de>
> ---
>
> 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.
>
> =A0Documentation/powerpc/dts-bindings/uio-generic.txt | =A0 16 +++
> =A0drivers/uio/Kconfig =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0| =A0 =A06 +
> =A0drivers/uio/Makefile =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 | =A0 =A01 +
> =A0drivers/uio/uio_of_genirq.c =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0| =A0 98 ++++++++++++++++++++
> =A04 files changed, 121 insertions(+), 0 deletions(-)
> =A0create mode 100644 Documentation/powerpc/dts-bindings/uio-generic.txt
> =A0create mode 100644 drivers/uio/uio_of_genirq.c
>
> diff --git a/Documentation/powerpc/dts-bindings/uio-generic.txt b/Documen=
tation/powerpc/dts-bindings/uio-generic.txt
> new file mode 100644
> index 0000000..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
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0"generic-uio".
> + - reg : address range(s) of the device (up to MAX_UIO_MAPS)
> + - interrupts : interrupt of the device
> +
> +Example:
> + =A0 =A0 =A0 =A0c64fpga@0 {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0compatible =3D "ptx,c64fpga001", "generi=
c-uio";
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0reg =3D <0x0 0x10000>;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0interrupts =3D <0 0 3>;
> + =A0 =A0 =A0 =A0};

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.

--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

^ permalink raw reply

* Re: [PATCH v2] i2c-mpc: generate START condition after STOP caused by read i2c_msg
From: Esben Haabendal @ 2009-06-14 14:04 UTC (permalink / raw)
  To: Ben Dooks; +Cc: linuxppc-dev, linux-i2c
In-Reply-To: <20090614131605.GK20446@fluff.org.uk>

Ben Dooks wrote:
> is there a new version of this patch available?
>
>   
I will catch up on it ASAP.

/Esben

^ permalink raw reply

* Re: [PATCH v2] i2c-mpc: generate START condition after STOP caused by read i2c_msg
From: Ben Dooks @ 2009-06-14 13:16 UTC (permalink / raw)
  To: Esben Haabendal; +Cc: linuxppc-dev, linux-i2c
In-Reply-To: <4A124202.4010201@doredevelopment.dk>

is there a new version of this patch available?

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

  'a smiley only costs 4 bytes'

^ permalink raw reply

* Re: [PATCH 1/2] lib: Provide generic atomic64_t implementation
From: Avi Kivity @ 2009-06-14 13:04 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: akpm, Linus Torvalds, linux-kernel, linuxppc-dev
In-Reply-To: <18996.60235.178618.531664@cargo.ozlabs.ibm.com>

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

^ permalink raw reply

* Re: [PATCH 1/2] uio/pdrv_genirq: Refactor probe routine to expose a generic part
From: Hans J. Koch @ 2009-06-14 12:15 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: devicetree-discuss, Hans J. Koch, linux-kernel, linuxppc-dev,
	Greg KH
In-Reply-To: <1244765062-14144-2-git-send-email-w.sang@pengutronix.de>

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 <hjk@linutronix.de>

> 
> Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
> Cc: Magnus Damm <magnus.damm@gmail.com>
> Cc: Hans J. Koch <hjk@linutronix.de>
> Cc: Greg KH <gregkh@suse.de>
> ---
>  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 <linux/bitops.h>
>  #include <linux/interrupt.h>
>  #include <linux/stringify.h>
> +#include <linux/uio_pdrv_genirq.h>
>  
>  #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 0000000..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 flags;
> +};
> +
> +extern int __uio_pdrv_genirq_probe(struct device *dev, struct uio_info *uioinfo,
> +	struct resource *resources, unsigned int num_resources);
> +
> +#endif
> -- 
> 1.6.3.1

^ permalink raw reply

* Re: [PATCH 2/2] uio: add an of_genirq driver
From: Hans J. Koch @ 2009-06-14 12:21 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: devicetree-discuss, Hans J. Koch, linux-kernel, linuxppc-dev,
	Greg KH
In-Reply-To: <1244765062-14144-3-git-send-email-w.sang@pengutronix.de>

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 <w.sang@pengutronix.de>
> Cc: Magnus Damm <magnus.damm@gmail.com>
> Cc: Hans J. Koch <hjk@linutronix.de>
> Cc: Greg KH <gregkh@suse.de>
> ---
> 
> 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 0000000..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:
> +        c64fpga@0 {
> +                compatible = "ptx,c64fpga001", "generic-uio";
> +                reg = <0x0 0x10000>;
> +                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 0000000..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 <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/uio_driver.h>
> +#include <linux/of_device.h>
> +#include <linux/of_platform.h>
> +#include <linux/uio_pdrv_genirq.h>
> +
> +#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->uioinfo->irq != UIO_IRQ_NONE)
> +		irq_dispose_mapping(priv->uioinfo->irq);
> +
> +	kfree(priv->uioinfo);
> +	kfree(priv);
> +	return 0;
> +}
> +
> +/* Match table for of_platform binding */
> +static const struct of_device_id __devinitconst uio_of_genirq_match[] = {

checkpatch.pl complains about that. Please check.

> +	{ .compatible = "generic-uio", },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, uio_of_genirq_match);
> +
> +static struct of_platform_driver uio_of_genirq_driver = {
> +	.owner = THIS_MODULE,
> +	.name = "uio-of-genirq",
> +	.match_table = uio_of_genirq_match,
> +	.probe = uio_of_genirq_probe,
> +	.remove = __devexit_p(uio_of_genirq_remove),
> +};
> +
> +static inline int __init uio_of_genirq_init(void)
> +{
> +	return of_register_platform_driver(&uio_of_genirq_driver);
> +}
> +module_init(uio_of_genirq_init);
> +
> +static inline void __exit uio_of_genirq_exit(void)
> +{
> +	of_unregister_platform_driver(&uio_of_genirq_driver);
> +}
> +module_exit(uio_of_genirq_exit);
> +
> +MODULE_AUTHOR("Wolfram Sang");
> +MODULE_DESCRIPTION("Userspace I/O OF driver with generic IRQ handling");
> +MODULE_LICENSE("GPL v2");
> -- 
> 1.6.3.1

^ permalink raw reply

* Re: [PATCH 1/2] lib: Provide generic atomic64_t implementation
From: Paul Mackerras @ 2009-06-14 12:21 UTC (permalink / raw)
  To: Avi Kivity; +Cc: akpm, Linus Torvalds, linux-kernel, linuxppc-dev
In-Reply-To: <4A34E4A5.3040306@redhat.com>

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.

^ permalink raw reply

* Re: [PATCH 1/2] lib: Provide generic atomic64_t implementation
From: Avi Kivity @ 2009-06-14 11:53 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: akpm, Paul Mackerras, linux-kernel, linuxppc-dev
In-Reply-To: <alpine.LFD.2.01.0906131323270.3237@localhost.localdomain>

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

^ permalink raw reply

* Re: [OOPS] hugetlbfs tests with 2.6.30-rc8-git1
From: Sachin Sant @ 2009-06-14 11:38 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: Mel Gorman, linuxppc-dev
In-Reply-To: <1244233062.31984.6.camel@pasglop>

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 [c0000000fe9b3220]
    pc: c00000000003d620: .hpte_need_flush+0x1bc/0x2d8
    lr: c00000000003d4d0: .hpte_need_flush+0x6c/0x2d8
    sp: c0000000fe9b34a0
   msr: 8000000000009032
   dar: c0000000283b0d78
 dsisr: 40000000
  current = 0xc0000000feb54700
  paca    = 0xc000000000c02600
    pid   = 6116, comm = stack_grow_into
enter ? for help
[c0000000fe9b34a0] c0000000fc6c0d80 (unreliable)
[c0000000fe9b3560] c00000000003f384 .huge_ptep_get_and_clear+0x40/0x5c
[c0000000fe9b35e0] c000000000149ea0 .__unmap_hugepage_range+0x178/0x2b8
[c0000000fe9b36d0] c00000000014a034 .unmap_hugepage_range+0x54/0x88
[c0000000fe9b3770] c000000000133db0 .unmap_vmas+0x178/0x8f4
[c0000000fe9b38c0] c0000000001396ec .exit_mmap+0xf4/0x1d0
[c0000000fe9b3960] c00000000008cf4c .mmput+0x68/0x14c
[c0000000fe9b39f0] c000000000091dc0 .exit_mm+0x16c/0x190
[c0000000fe9b3aa0] c0000000000941c4 .do_exit+0x204/0x748
[c0000000fe9b3b80] c00000000009479c .do_group_exit+0x94/0xc8
[c0000000fe9b3c10] c0000000000a321c .get_signal_to_deliver+0x384/0x3e4
[c0000000fe9b3cf0] c000000000013d48 .do_signal+0x6c/0x2f8
[c0000000fe9b3e30] c000000000008afc do_work+0x24/0x28
--- Exception: 300 (Data Access) at 0000000010002154
SP (ef8b8820) is in userspace
1:mon> r
R00 = 0000000000000000   R16 = 0000000040040000
R01 = c0000000fe9b34a0   R17 = 0000000040040000
R02 = c000000000b07610   R18 = ffffffffffffffff
R03 = 0000000000000004   R19 = 0000000000000000
R04 = 00000000ef000000   R20 = c0000000fc070000
R05 = c0000000283a8d78   R21 = 00000000f0000000
R06 = 00001b4008001393   R22 = c0000000283a8d78
R07 = 0000000000000001   R23 = 00000000007a0000
R08 = 0000000000000004   R24 = 0000000000000000
R09 = c0000000283b8d78   R25 = 00001b4008001393
R10 = 00000000000000ef   R26 = c0000000fc070000
R11 = 0000000000250000   R27 = 0000000000000004
R12 = 0000000024022422   R28 = c0000000008e0518
R13 = c000000000c02600   R29 = 0000000000000000
R14 = c00000001bed0f00   R30 = c000000001080518
R15 = c0000000db0c0020   R31 = fe967b124f000000
pc  = c00000000003d620 .hpte_need_flush+0x1bc/0x2d8
lr  = c00000000003d4d0 .hpte_need_flush+0x6c/0x2d8
msr = 8000000000009032   cr  = 44022422
ctr = 000000000000bba4   xer = 0000000000000001   trap =  300
dar = c0000000283b0d78   dsisr = 40000000
1:mon> di $.hpte_need_flush
....... snip .......
c00000000003d604  4800000c      b       c00000000003d610        # .hpte_need_flush+0x1ac/0x2d8
c00000000003d608  792945c6      rldicr  r9,r9,40,23
c00000000003d60c  7be00600      clrldi  r0,r31,24
c00000000003d610  7d3f0378      or      r31,r9,r0
c00000000003d614  7c1cb82e      lwzx    r0,r28,r23
c00000000003d618  3d360001      addis   r9,r22,1
c00000000003d61c  2f800000      cmpwi   cr7,r0,0
c00000000003d620  eb898000      ld      r28,-32768(r9)
c00000000003d624  409e0028      bne     cr7,c00000000003d64c    # .hpte_need_flush+0x1e8/0x2d8
c00000000003d628  7fe3fb78      mr      r3,r31
c00000000003d62c  7f24cb78      mr      r4,r25
c00000000003d630  7f85e378      mr      r5,r28
c00000000003d634  7f6607b4      extsw   r6,r27
c00000000003d638  7fa7eb78      mr      r7,r29
c00000000003d63c  39000000      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
---------------------------------

^ permalink raw reply

* sym scsi driver problem with 2.6.26 or newer debian kernel on p610 (fwd)
From: Guennadi Liakhovetski @ 2009-06-14  9:10 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: debian-powerpc, Laszlo Fekete

[-- Attachment #1: Type: TEXT/PLAIN, Size: 772 bytes --]

Ok, first attempt to forward this to scsi was wrong, as pointed out by 
Matthew Wilcox this does indeed look like an interrupt problem - no 
interrupts drom SCSI, IDE, keyboar. Might be a known problem, I guess. In 
any case, I think, the OP would be grateful for any hints.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

---------- Forwarded message ----------
Date: Sat, 13 Jun 2009 16:22:07 +0200
From: Laszlo Fekete <blackluck@ktk.bme.hu>
To: debian-powerpc@lists.debian.org
Subject: sym scsi driver problem with 2.6.26 or newer debian kernel on p610
Resent-Date: Sat, 13 Jun 2009 14:29:55 +0000 (UTC)
Resent-From: debian-powerpc@lists.debian.org

This is a multi-part message in MIME format.

[-- Attachment #2: Type: TEXT/PLAIN, Size: 468 bytes --]

Hello!





Pls help me with sym scsi driver problem.



I have Ibm P610 (and tested it on P630 and P640 too), installed debian

etch and upgraded to lenny.



But with 2.6.26 or newer kernel it's not booting, it's hang on sym scsi

bus scan.





Whats the problem with it, or how can I fix this?





I attach the output from minicom with 2.6.29, 2.6.26, and the working

2.6.24 kernel booting.





Thank you very much!




[-- Attachment #3: Type: APPLICATION/CAP, Size: 69688 bytes --]

^ permalink raw reply

* Re: [PATCH 1/2] lib: Provide generic atomic64_t implementation
From: Arnd Bergmann @ 2009-06-13 21:53 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: akpm, torvalds, linux-kernel, linuxppc-dev
In-Reply-To: <18995.20685.227683.561827@cargo.ozlabs.ibm.com>

On Saturday 13 June 2009, Paul Mackerras wrote:
> +extern long long atomic64_read(const atomic64_t *v);
> +extern void     atomic64_set(atomic64_t *v, long long i);
> +extern void     atomic64_add(long long a, atomic64_t *v);
> +extern long long atomic64_add_return(long long a, atomic64_t *v);
> +extern void     atomic64_sub(long long a, atomic64_t *v);
> +extern long long atomic64_sub_return(long long a, atomic64_t *v);
> +extern long long atomic64_dec_if_positive(atomic64_t *v);
> +extern long long atomic64_cmpxchg(atomic64_t *v, long long o, long long n);
> +extern long long atomic64_xchg(atomic64_t *v, long long new);
> +extern int      atomic64_add_unless(atomic64_t *v, long long a, long long u);
> +
> +#define atomic64_add_negative(a, v)    (atomic64_add_return((a), (v)) < 0)
> +#define atomic64_inc(v)                        atomic64_add(1LL, (v))
> +#define atomic64_inc_return(v)         atomic64_add_return(1LL, (v))
> +#define atomic64_inc_and_test(v)       (atomic64_inc_return(v) == 0)
> +#define atomic64_sub_and_test(a, v)    (atomic64_sub_return((a), (v)) == 0)
> +#define atomic64_dec(v)                        atomic64_sub(1LL, (v))
> +#define atomic64_dec_return(v)         atomic64_sub_return(1LL, (v))
> +#define atomic64_dec_and_test(v)       (atomic64_dec_return((v)) == 0)
> +#define atomic64_inc_not_zero(v)       atomic64_add_unless((v), 1LL, 0LL)
> +

How about also doing these:?

#define atomic64_sub(a, v)		atomic64_add(-a, v)
#define atomic64_sub_return(a, v)	atomic64_add_return(-a, v)
#define atomic64_add(a, v)		(void)atomic64_add_return(a, v)

The cost to the caller (one or two instruction per call site)
seems to be about the same as for the other wrapper macros.

	Arnd <><

^ permalink raw reply

* Re: [PATCH RFC] powerpc: perf_counter: Enable use of software counters on 32-bit powerpc
From: Ingo Molnar @ 2009-06-13 21:16 UTC (permalink / raw)
  To: Paul Mackerras, Mike Frysinger; +Cc: Peter Zijlstra, linux-kernel, linuxppc-dev
In-Reply-To: <18995.39910.109998.926269@cargo.ozlabs.ibm.com>


* Paul Mackerras <paulus@samba.org> wrote:

> +extern void set_perf_counter_pending(void);

btw., Mike Frysinger pointed out that this prototype should be in 
include/linux/perf_counter.h, not spread out in every architecture 
pointlessly.

	Ingo

^ permalink raw reply

* Re: [PATCH 1/2] lib: Provide generic atomic64_t implementation
From: Ingo Molnar @ 2009-06-13 20:56 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: akpm, Paul Mackerras, linux-kernel, linuxppc-dev
In-Reply-To: <alpine.LFD.2.01.0906131323270.3237@localhost.localdomain>


* Linus Torvalds <torvalds@linux-foundation.org> 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?

We account all sorts of non-hw bits via atomic64_t as well - for 
example time related counters in nanoseconds - which wrap 32 bits at 
4 seconds.

There's also security/stability relevant bits:

        counter->id             = atomic64_inc_return(&perf_counter_id);

We dont really want that ID to wrap ever - it could create a leaking 
of one PMU context into another. (We could rewrite it by putting a 
global lock around it, but still - this is a convenient primitive.)

In select places we might be able to reduce the use of atomic64_t 
(that might make performance sense anyway) - but to get rid of all 
of them would be quite painful. We initially started with a 32-bit 
implementation and it was quite painful with fast-paced units.

So since Paul has already coded the wrappers up ... i'd really 
prefer that, unless there's really compelling reasons not to do it.

	Ingo

^ permalink raw reply

* Re: [PATCH 1/2] lib: Provide generic atomic64_t implementation
From: Linus Torvalds @ 2009-06-13 20:25 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: akpm, linux-kernel, linuxppc-dev
In-Reply-To: <alpine.LFD.2.01.0906131312470.3237@localhost.localdomain>



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?

		Linus

^ permalink raw reply

* Re: [PATCH 1/2] lib: Provide generic atomic64_t implementation
From: Linus Torvalds @ 2009-06-13 20:13 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: akpm, linux-kernel, linuxppc-dev
In-Reply-To: <18995.20685.227683.561827@cargo.ozlabs.ibm.com>



On Sat, 13 Jun 2009, Paul Mackerras wrote:
>
> Linus, Andrew: OK if this goes in via the powerpc tree?

Ok by me.

		Linus

^ permalink raw reply

* Re: [PATCH] powerpc/mpc52xx/mtd: fix mtd-ram access for 16-bit Local Plus Bus
From: Grant Likely @ 2009-06-13 17:09 UTC (permalink / raw)
  To: Albrecht Dreß; +Cc: Linux PPC Development, devicetree-discuss, dwmw2
In-Reply-To: <1244911551.3423.0@antares>

On Sat, Jun 13, 2009 at 10:45 AM, Albrecht Dre=DF<albrecht.dress@arcor.de> =
wrote:
> Am 11.06.09 18:27 schrieb(en) Grant Likely:
>>>
>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 *(u16 *)buf =3D *((volatile u16 *)(vdest =
- 1));
>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 buf[1] =3D *((u8 *)src);
>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 *((volatile u16 *)(vdest - 1)) =3D *(u16 =
*)buf;
>>
>> what is the purpose of volatile here? =A0If you need io barriers, then u=
se
>> the in_/out_be16 macros.
>
> Yes, you're right - should be completely superfluous here. =A0A result of=
 copy
> & paste without thinking... :-(
>
>> Blech. =A0ugly #ifdef and not really multiplatform safe (yeah, I know it
>> shouldn't break non-5200 platforms, but it does have an undesirable impa=
ct).
>> =A0There's got to be a better way.
>
> Ouch, yes - I completely forgot the possibility of multi-platform builds.
>
> Am 11.06.09 19:28 schrieb(en) Grant Likely:
>>
>> So; the solution to me seems to be on an MPC5200 platform replace the
>> offending hooks with MPC5200 specific variants at runtime.
>
> Will re-work the patch that way! =A0BTW, a dumb question: what is the pro=
per
> way to determine which cpu the system is running on? =A0Check the CPU nod=
e of
> the of tree?

In this case I'd walk up the parent nodes in the tree looking for an
"fsl,mpc5200-lpb" compatible value.  That way you only apply the
workaround if the device is on the locaplus bus, and not for example
below the PCI bus.

g.

--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

^ permalink raw reply

* Re: [PATCH] powerpc/mpc52xx/mtd: fix mtd-ram access for 16-bit Local Plus Bus
From: Albrecht Dreß @ 2009-06-13 16:45 UTC (permalink / raw)
  To: Grant Likely; +Cc: Linux PPC Development, dwmw2
In-Reply-To: <fa686aa40906110927w60a5a74alaa3420b9bf3f3e44@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 1099 bytes --]

Am 11.06.09 18:27 schrieb(en) Grant Likely:
>> +               *(u16 *)buf = *((volatile u16 *)(vdest - 1));
>> +               buf[1] = *((u8 *)src);
>> +               *((volatile u16 *)(vdest - 1)) = *(u16 *)buf;
> 
> what is the purpose of volatile here?  If you need io barriers, then  
> use the in_/out_be16 macros.

Yes, you're right - should be completely superfluous here.  A result of  
copy & paste without thinking... :-(

> Blech.  ugly #ifdef and not really multiplatform safe (yeah, I know  
> it shouldn't break non-5200 platforms, but it does have an  
> undesirable impact).  There's got to be a better way.

Ouch, yes - I completely forgot the possibility of multi-platform  
builds.

Am 11.06.09 19:28 schrieb(en) Grant Likely:
> So; the solution to me seems to be on an MPC5200 platform replace the  
> offending hooks with MPC5200 specific variants at runtime.

Will re-work the patch that way!  BTW, a dumb question: what is the  
proper way to determine which cpu the system is running on?  Check the  
CPU node of the of tree?

Thanks, Albrecht.

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* Re: invalid Kconfig "select" for MPC5121_ADS_CPLD
From: Grant Likely @ 2009-06-13 16:21 UTC (permalink / raw)
  To: Robert P. J. Day; +Cc: Linux PPC Mailing List, Wolfgang Denk
In-Reply-To: <alpine.LFD.2.00.0906131109260.23298@localhost>

On Sat, Jun 13, 2009 at 9:11 AM, Robert P. J. Day<rpjday@crashcourse.ca> wr=
ote:
>
> =A0arch/powerpc/platforms/512x/Kconfig: =A0select MPC5121_ADS_CPLD

5121 support in mainline is half-baked.  Wolfgang is working on a
patch series to update it (undergone one review cycle on the mailing
list).  I don't know if he has published a git tree containing the
current state.

BTW, it is helpful to cc: the relevant maintainers when asking
questions about particular SoCs.

g.

--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox