LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] [02/10] pasemi_mac: Stop using the pci config space accessors for register read/writes
From: Olof Johansson @ 2007-08-23 18:13 UTC (permalink / raw)
  To: jgarzik; +Cc: netdev, linuxppc-dev
In-Reply-To: <20070822141248.GC16830@lixom.net>

Move away from using the pci config access functions for simple register
access.  Our device has all of the registers in the config space (hey,
from the hardware point of view it looks reasonable :-), so we need to
somehow get to it. Newer firmwares have it in the device tree such that
we can just get it and ioremap it there (in case it ever moves in future
products). For now, provide a hardcoded fallback for older firmwares.


Signed-off-by: Olof Johansson <olof@lixom.net>


---

Updated: Removed explicit inlines, cleaned up read functions, fixed
grammar.


Index: mainline/drivers/net/pasemi_mac.c
===================================================================
--- mainline.orig/drivers/net/pasemi_mac.c
+++ mainline/drivers/net/pasemi_mac.c
@@ -83,44 +83,35 @@ static struct pasdma_status *dma_status;
 
 static unsigned int read_iob_reg(struct pasemi_mac *mac, unsigned int reg)
 {
-	unsigned int val;
-
-	pci_read_config_dword(mac->iob_pdev, reg, &val);
-	return val;
+	return in_le32(mac->iob_regs+reg);
 }
 
 static void write_iob_reg(struct pasemi_mac *mac, unsigned int reg,
 			  unsigned int val)
 {
-	pci_write_config_dword(mac->iob_pdev, reg, val);
+	out_le32(mac->iob_regs+reg, val);
 }
 
 static unsigned int read_mac_reg(struct pasemi_mac *mac, unsigned int reg)
 {
-	unsigned int val;
-
-	pci_read_config_dword(mac->pdev, reg, &val);
-	return val;
+	return in_le32(mac->regs+reg);
 }
 
 static void write_mac_reg(struct pasemi_mac *mac, unsigned int reg,
 			  unsigned int val)
 {
-	pci_write_config_dword(mac->pdev, reg, val);
+	out_le32(mac->regs+reg, val);
 }
 
 static unsigned int read_dma_reg(struct pasemi_mac *mac, unsigned int reg)
 {
-	unsigned int val;
-
-	pci_read_config_dword(mac->dma_pdev, reg, &val);
-	return val;
+	return in_le32(mac->dma_regs+reg);
 }
 
 static void write_dma_reg(struct pasemi_mac *mac, unsigned int reg,
 			  unsigned int val)
 {
-	pci_write_config_dword(mac->dma_pdev, reg, val);
+	out_le32(mac->dma_regs+reg, val);
 }
 
 static int pasemi_get_mac_addr(struct pasemi_mac *mac)
@@ -585,7 +576,6 @@ static int pasemi_mac_clean_tx(struct pa
 	}
 	mac->tx->next_to_clean += count;
 	spin_unlock_irqrestore(&mac->tx->lock, flags);
-
 	netif_wake_queue(mac->netdev);
 
 	return count;
@@ -1076,6 +1066,73 @@ static int pasemi_mac_poll(struct net_de
 	}
 }
 
+static void __iomem * __devinit map_onedev(struct pci_dev *p, int index)
+{
+	struct device_node *dn;
+	void __iomem *ret;
+
+	dn = pci_device_to_OF_node(p);
+	if (!dn)
+		goto fallback;
+
+	ret = of_iomap(dn, index);
+	if (!ret)
+		goto fallback;
+
+	return ret;
+fallback:
+	/* This is hardcoded and ugly, but we have some firmware versions
+	 * that don't provide the register space in the device tree. Luckily
+	 * they are at well-known locations so we can just do the math here.
+	 */
+	return ioremap(0xe0000000 + (p->devfn << 12), 0x2000);
+}
+
+static int __devinit pasemi_mac_map_regs(struct pasemi_mac *mac)
+{
+	struct resource res;
+	struct device_node *dn;
+	int err;
+
+	mac->dma_pdev = pci_get_device(PCI_VENDOR_ID_PASEMI, 0xa007, NULL);
+	if (!mac->dma_pdev) {
+		dev_err(&mac->pdev->dev, "Can't find DMA Controller\n");
+		return -ENODEV;
+	}
+
+	mac->iob_pdev = pci_get_device(PCI_VENDOR_ID_PASEMI, 0xa001, NULL);
+	if (!mac->iob_pdev) {
+		dev_err(&mac->pdev->dev, "Can't find I/O Bridge\n");
+		return -ENODEV;
+	}
+
+	mac->regs = map_onedev(mac->pdev, 0);
+	mac->dma_regs = map_onedev(mac->dma_pdev, 0);
+	mac->iob_regs = map_onedev(mac->iob_pdev, 0);
+
+	if (!mac->regs || !mac->dma_regs || !mac->iob_regs) {
+		dev_err(&mac->pdev->dev, "Can't map registers\n");
+		return -ENODEV;
+	}
+
+	/* The dma status structure is located in the I/O bridge, and
+	 * is cache coherent.
+	 */
+	if (!dma_status) {
+		dn = pci_device_to_OF_node(mac->iob_pdev);
+		if (dn)
+			err = of_address_to_resource(dn, 1, &res);
+		if (!dn || err) {
+			/* Fallback for old firmware */
+			res.start = 0xfd800000;
+			res.end = res.start + 0x1000;
+		}
+		dma_status = __ioremap(res.start, res.end-res.start, 0);
+	}
+
+	return 0;
+}
+
 static int __devinit
 pasemi_mac_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 {
@@ -1104,21 +1161,6 @@ pasemi_mac_probe(struct pci_dev *pdev, c
 
 	mac->pdev = pdev;
 	mac->netdev = dev;
-	mac->dma_pdev = pci_get_device(PCI_VENDOR_ID_PASEMI, 0xa007, NULL);
-
-	if (!mac->dma_pdev) {
-		dev_err(&pdev->dev, "Can't find DMA Controller\n");
-		err = -ENODEV;
-		goto out_free_netdev;
-	}
-
-	mac->iob_pdev = pci_get_device(PCI_VENDOR_ID_PASEMI, 0xa001, NULL);
-
-	if (!mac->iob_pdev) {
-		dev_err(&pdev->dev, "Can't find I/O Bridge\n");
-		err = -ENODEV;
-		goto out_put_dma_pdev;
-	}
 
 	/* These should come out of the device tree eventually */
 	mac->dma_txch = index;
@@ -1161,12 +1203,9 @@ pasemi_mac_probe(struct pci_dev *pdev, c
 	dev->poll = pasemi_mac_poll;
 	dev->features = NETIF_F_HW_CSUM;
 
-	/* The dma status structure is located in the I/O bridge, and
-	 * is cache coherent.
-	 */
-	if (!dma_status)
-		/* XXXOJN This should come from the device tree */
-		dma_status = __ioremap(0xfd800000, 0x1000, 0);
+	err = pasemi_mac_map_regs(mac);
+	if (err)
+		goto out;
 
 	mac->rx_status = &dma_status->rx_sta[mac->dma_rxch];
 	mac->tx_status = &dma_status->tx_sta[mac->dma_txch];
@@ -1193,10 +1232,17 @@ pasemi_mac_probe(struct pci_dev *pdev, c
 	return err;
 
 out:
-	pci_dev_put(mac->iob_pdev);
-out_put_dma_pdev:
-	pci_dev_put(mac->dma_pdev);
-out_free_netdev:
+	if (mac->iob_pdev)
+		pci_dev_put(mac->iob_pdev);
+	if (mac->dma_pdev)
+		pci_dev_put(mac->dma_pdev);
+	if (mac->dma_regs)
+		iounmap(mac->dma_regs);
+	if (mac->iob_regs)
+		iounmap(mac->iob_regs);
+	if (mac->regs)
+		iounmap(mac->regs);
+
 	free_netdev(dev);
 out_disable_device:
 	pci_disable_device(pdev);
@@ -1220,6 +1266,10 @@ static void __devexit pasemi_mac_remove(
 	pci_dev_put(mac->dma_pdev);
 	pci_dev_put(mac->iob_pdev);
 
+	iounmap(mac->regs);
+	iounmap(mac->dma_regs);
+	iounmap(mac->iob_regs);
+
 	pci_set_drvdata(pdev, NULL);
 	free_netdev(netdev);
 }
Index: mainline/drivers/net/pasemi_mac.h
===================================================================
--- mainline.orig/drivers/net/pasemi_mac.h
+++ mainline/drivers/net/pasemi_mac.h
@@ -52,6 +52,9 @@ struct pasemi_mac_rxring {
 
 struct pasemi_mac {
 	struct net_device *netdev;
+	void __iomem *regs;
+	void __iomem *dma_regs;
+	void __iomem *iob_regs;
 	struct pci_dev *pdev;
 	struct pci_dev *dma_pdev;
 	struct pci_dev *iob_pdev;

^ permalink raw reply

* Re: [PATCH] [02/10] pasemi_mac: Stop using the pci config space accessors for register read/writes
From: Olof Johansson @ 2007-08-23 18:12 UTC (permalink / raw)
  To: Stephen Rothwell; +Cc: netdev, jgarzik, linuxppc-dev
In-Reply-To: <20070823103103.a1815a07.sfr@canb.auug.org.au>

On Thu, Aug 23, 2007 at 10:31:03AM +1000, Stephen Rothwell wrote:
> On Wed, 22 Aug 2007 09:12:48 -0500 Olof Johansson <olof@lixom.net> wrote:
> >
> > -static unsigned int read_iob_reg(struct pasemi_mac *mac, unsigned int reg)
> > +static inline unsigned int read_iob_reg(struct pasemi_mac *mac, unsigned int reg)
>           ^^^^^^
> For static functions in C files, we tend not to bother marking them
> inline any more as the compiler does a pretty good job theses days.

Yeah, sloppy coding on my behalf. It was still there from when I
explicitly added noinline during debugging, forgot to take it out
alltogether.

> > -	pci_read_config_dword(mac->iob_pdev, reg, &val);
> > +	val = in_le32(mac->iob_regs+reg);
> > +
> >  	return val;
> 
> Why not just "return in_le32(mac->iob_regs+reg);" ?
> And similarly below?

Residual from debugging as well, I had debug hooks showing what was
read/written that I took out, but didn't fix up the surrounding stuff.


Refreshed patch posted separately. Thanks for the feedback.

-Olof

^ permalink raw reply

* Re: asm-ppc header issues when building ARCH=powerpc
From: Brad Boyer @ 2007-08-23 18:56 UTC (permalink / raw)
  To: Kumar Gala; +Cc: linuxppc-dev@ozlabs.org list, Paul Mackerras, David Gibson
In-Reply-To: <F19C9986-9E53-47F3-835D-C93DA9C3ADED@kernel.crashing.org>

On Wed, Aug 22, 2007 at 11:16:03PM -0500, Kumar Gala wrote:
> On Aug 22, 2007, at 10:33 PM, David Gibson wrote:
> >> My analysis of <asm/bootinfo.h> usage:
> >>
> >> ./drivers/macintosh/adb-iop.c:#include <asm/bootinfo.h>	 remove
> >> ./drivers/char/vme_scc.c:#include <asm/bootinfo.h>		68k only
> >> ./drivers/char/serial167.c:#include <asm/bootinfo.h>		68k only
> >> ./drivers/serial/dz.c:#include <asm/bootinfo.h>		 decstation
> >> ./drivers/mtd/devices/ms02-nv.c:#include <asm/bootinfo.h>	decstation
> >> ./drivers/net/macsonic.c:#include <asm/bootinfo.h>		68k
> >> ./drivers/net/jazzsonic.c:#include <asm/bootinfo.h>		mips
> >> ./drivers/video/pmag-aa-fb.c:#include <asm/bootinfo.h>		mips
> >> ./drivers/video/maxinefb.c:#include <asm/bootinfo.h>		mips
> >> ./drivers/video/logo/logo.c:#include <asm/bootinfo.h>		mips
> >> ./drivers/video/macfb.c:#include <asm/bootinfo.h>		68k
> >> ./drivers/video/valkyriefb.c:#include <asm/bootinfo.h>		68k
> >
> > Uh.. I'm pretty sure valkyriefb.c is for (old) PowerMacs, not 68k.
> 
> It appears to be both.  If you look at the include its protected by a  
> #ifdef CONFIG_MAC which we is only defined on m68k.

According to drivers/video/macfb.c, both the Quadra 630 and Performa 588
have the Valkyrie chip for their video. I don't remember which ppc based
models have it, but it's definitely on both architectures.

Just as an extra note, the file drivers/macintosh/adb-iop.c is m68k only,
so you should probably leave that alone as well. It probably doesn't need
that header, but the change should really come from the 68k side of things.

	Brad Boyer
	flar@allandria.com

^ permalink raw reply

* Re: [PATCH 05/20] bootwrapper: flatdevtree fixes
From: Segher Boessenkool @ 2007-08-23 18:00 UTC (permalink / raw)
  To: David Gibson; +Cc: linuxppc-dev, paulus
In-Reply-To: <20070823020147.GB7042@localhost.localdomain>

> Actually, in any case, I don't think we want to implement get_path()
> this way for real OF.  Better to have get_path() itself as a callback:
> on real OF I believe we can directly ask for the full path to a given
> phandle,

Yes.  "package-to-path" from the client interface.


Segher

^ permalink raw reply

* Re: asm-ppc header issues when building ARCH=powerpc
From: Kumar Gala @ 2007-08-23 18:00 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev@ozlabs.org list, Paul Mackerras
In-Reply-To: <20070823173314.GA18412@ld0162-tx32.am.freescale.net>


On Aug 23, 2007, at 12:33 PM, Scott Wood wrote:

> On Thu, Aug 23, 2007 at 12:47:57PM +1000, David Gibson wrote:
>>> ./drivers/mtd/maps/tqm834x.c:#include <asm/ppcboot.h>
>>> ./drivers/mtd/maps/pq2fads.c:#include <asm/ppcboot.h>
>>
>> Although these both have an extern of type bd_t (defined in
>> ppcboot.h), afaict they don't actually use it, so these should be
>> removable.
>
> They look like they're using it to me...  See the bd->bi_flashstart  
> and
> bd->bi_flashsize references in init_pq2fads_mtd.

Both of these are dead in one way or another.  pq2fads.c isn't in the  
makefile and you can't Kconfig tqm834x.c into existence.  Thus my  
patch to remove them.

- k

^ permalink raw reply

* Re: [patch 1/2] powerpc: rmb fix
From: Segher Boessenkool @ 2007-08-23 17:57 UTC (permalink / raw)
  To: Nick Piggin; +Cc: linuxppc-dev, Paul Mackerras
In-Reply-To: <20070822035539.GB26374@wotan.suse.de>

>> The powerpc kernel needs to have full sync insns in every I/O
>> accessor in order to enforce all the ordering rules Linux demands.
>> It's a bloody shame, but the alternative would be to make the
>> barriers lots more expensive.  A third alternative would be to
>
> Well lots more expensive compared to what you have now. But what
> you have now is like having those expensive barriers between
> *every* io access.

Yeah.  But I/O reads are very expensive anyway, and the barriers
are used for more than just I/O ordering.

I/O writes are a different thing; ideally, they would use only
eieio, if anything at all.

Maybe the tradeoff isn't optimal.  The I/O primitives didn't have
all those "sync"s in there before, they got added because some bad
interaction with spinlocks was discovered, if my memory isn't failing
me.

>> have barrier ops that do not order everything, but just A vs. B
>> for various choices of A and B (coherent accesses, MMIO accesses,
>> etc.)
>
> The non-smp_ variant is supposed to order everything, AFAIK. Maybe
> you could get more fancy and have PIO vs MMIO etc etc. but it looks
> like this whole area is in a pretty sticky state anyway so let's
> not think about that.

*Thinking* about it is fun.  Trying to get the code merged would be
a different thing ;-)


Segher

^ permalink raw reply

* Re: [patch 1/2] powerpc: rmb fix
From: Segher Boessenkool @ 2007-08-23 17:49 UTC (permalink / raw)
  To: Nick Piggin; +Cc: linuxppc-dev, Paul Mackerras
In-Reply-To: <20070822040504.GC26374@wotan.suse.de>

>>> Drivers are definitely using these __raw_ accessors, and from a quick
>>> look, they do appear to be hoping that *mb() is going to order access
>>> for
>>> them.
>>
>> Which drivers?
>
> There are maybe a dozen that use the raw accessors, and use non-smp_
> memory barriers. I just looked at drivers/video/tgafb.c, which
> indeed appears to intermix them.

Hrm yeah.  It also looks like old buggy code that could use a
cleanup or two (or three or four).  I wonder if all __raw_ users
are like that?


Segher

^ permalink raw reply

* Re: [PATCH 05/20] bootwrapper: flatdevtree fixes
From: Scott Wood @ 2007-08-23 17:48 UTC (permalink / raw)
  To: David Gibson; +Cc: linuxppc-dev, paulus
In-Reply-To: <20070823020147.GB7042@localhost.localdomain>

David Gibson wrote:
> Actually, no - sorry, that's the other problem with this, which I
> forgot to mention.  On real OF, the "name" property contains the
> node's name *without the unit address*; that is, only the portion
> before the '@'.  So your getprop change does not match real OF
> behaviour - and real OF behaviour will not do what you want for
> dt_get_path().

Ah, OK.

> Actually, in any case, I don't think we want to implement get_path()
> this way for real OF.  Better to have get_path() itself as a callback:
> on real OF I believe we can directly ask for the full path to a given
> phandle, the get name based implementation can then be made specific
> to the flat device tree.
> 
> Or actually, I think we might be able to come up with a get_path()
> implementation for flat tree that's less hideous than repeatedly
> calling get_parent() which is an ugly, ugly operation on the flat tree

It's likely to be ugly no matter what, though I'll try to come up with 
something slightly nicer.  If I were doing this code from scratch, I'd 
probably liven the tree first and reflatten it to pass to the kernel.

> (and will get worse with libfdt).

Why is that?

>>Plus, something might come along that needs to dynamically look for
>>either name or something else.  It's more flexible this way.
> 
> Hrm... "something might come along" just seems contrived to me.

Well, I generally prefer doing things the more flexible way in the 
absence of a good reason not to.  OF returning the bare name is a good 
reason not to.

-Scott

^ permalink raw reply

* Re: asm-ppc header issues when building ARCH=powerpc
From: Scott Wood @ 2007-08-23 17:33 UTC (permalink / raw)
  To: Kumar Gala, linuxppc-dev@ozlabs.org list, Paul Mackerras
In-Reply-To: <20070823024757.GC7042@localhost.localdomain>

On Thu, Aug 23, 2007 at 12:47:57PM +1000, David Gibson wrote:
> > ./drivers/mtd/maps/tqm834x.c:#include <asm/ppcboot.h>
> > ./drivers/mtd/maps/pq2fads.c:#include <asm/ppcboot.h>
> 
> Although these both have an extern of type bd_t (defined in
> ppcboot.h), afaict they don't actually use it, so these should be
> removable.

They look like they're using it to me...  See the bd->bi_flashstart and
bd->bi_flashsize references in init_pq2fads_mtd.

-Scott

^ permalink raw reply

* Any success with EP8248 and u-boot?
From: daniel.garoz @ 2007-08-23 16:57 UTC (permalink / raw)
  To: linuxppc-embedded

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

Hi everybody.

Has anyone successfully started linux using u-boot on the EmbeddedPlanet 
EP8248 evaluation board?

Thanks.

Daniel Garoz.





Q No imprima este e-mail si no es necesario. Protejamos el Medio Ambiente.
E


Este mensaje y sus ficheros adjuntos se dirigen exclusivamente a su 
destinatario y pueden contener información confidencial y/o sujeta a 
secreto profesional. Si no es usted el destinatario indicado, queda 
notificado de que la utilización, divulgación y/o copia sin autorización 
está prohibida en virtud de la legislación vigente. Si ha recibido este 
mensaje por error, le rogamos que nos lo comunique inmediatamente por esta 
misma vía y proceda a su destrucción. Los ficheros adjuntos han sido 
comprobados con un programa anti virus antes de su transmisión, pero les 
recomendamos que efectúen sus propias comprobaciones antes de abrir 
cualquiera de ellos. No aceptamos responsabilidad alguna por daños o 
pérdidas causados por virus informáticos.


La empresa no asume ningún tipo de responsabilidad legal por el contenido 
de este mensaje. Cualquier opinión manifestada en el pertenece solo al 
autor y no representa necesariamente la opinión de la empresa salvo que 
expresamente se especifique lo contrario.


Dimetronic, S.A. con domicilio en Avda. Castilla nº2, Parque Empresarial 
San Fernando (Edificio Grecia), 28830 San Fernando de Henares, Madrid, 
España y CIF A-28512598 se encuentra inscrita en el Registro Mercantil de 
Madrid al tomo 4.948 general, sección 3ª, folio 23, hoja nº 39135 y 
Dimetronic S.A., Sucursal Portuguesa con domicilio en Avenida D. João II, 
Lote 1.16.05, Edificio Infante 3ºQ, 1990-083 Lisboa, Freguesia de Santa 
Maria dos Olivais, Concelho de Lisboa, (Portugal), NIPC 980 020 891 se 
encuentra inscrita en el Registro Comercial de Lisboa con el nº 
68850/880725.



Dimetronic, S.A. y Dimetronic, S.A. Sucursal Portuguesa son filiales de 
Invensys, Plc, cuyo domicilio social está en Portland House, Bressenden 
Place, London SW1E 5BF. Registrada en Inglaterra y Gales con el nº 
1641421.




English translation


Tradução portuguesa




[-- Attachment #2: Type: text/html, Size: 3558 bytes --]

^ permalink raw reply

* Re: wmb vs mmiowb
From: Jesse Barnes @ 2007-08-23 17:02 UTC (permalink / raw)
  To: Nick Piggin; +Cc: linux-ia64, Linus Torvalds, linuxppc-dev
In-Reply-To: <20070823022043.GB18788@wotan.suse.de>

> > Yeah, they keep threatening to use this instead, but I'm not sure
> > how easy it would be.  Also they may have more devices/drivers to
> > worry about than sn2, so maybe changing over would mean too much
> > driver debugging (well auditing really since it's not that hard to
> > know where to put them).  Irix actually had an io_unlock() routine
> > that did this implicitly, but iirc that was shot down for Linux...
>
> Why was it shot down? Seems like a pretty good idea to me ;)

Well, like Linus said, it had some significant downsides (though I think 
Irix had fewer lock types, so the multiplicative effect wasn't so bad 
there).

> I'm clueless when it comes to drivers, but I see a lot of mmiowb()
> that are not paired with spin_unlock. How are these obvious? (ie.
> what is the pattern?) It looks like some might be lockless FIFOs (or
> maybe I'm just not aware of where the locks are). Can you just
> quickly illustrate the problem being solved?

Wow, it certainly has proliferated since it was added to the tree. :)

I didn't audit all the uses, but it seems like many of them get it 
right, i.e. mmiowb() before spin_unlock() where PIO has been done.  I'd 
have to look carefully to see whether lockless usages are correct, it's 
likely they're not.

Jesse

^ permalink raw reply

* Re: wmb vs mmiowb
From: Jesse Barnes @ 2007-08-23 16:56 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: Nick Piggin, linux-ia64, Linus Torvalds, linuxppc-dev
In-Reply-To: <1187854035.5972.4.camel@localhost.localdomain>

On Thursday, August 23, 2007 12:27 am Benjamin Herrenschmidt wrote:
> > Of course, the normal memory barrier would usually be a
> > "spin_unlock()" or something like that, not a "wmb()". In fact, I
> > don't think the powerpc implementation (as an example of this) will
> > actually synchronize with anything *but* a spin_unlock().
>
> We are even more sneaky in the sense that we set a per-cpu flag on
> any MMIO write and do the sync automatically in spin_unlock() :-)

Yeah, that's a reasonable thing to do, and in fact I think there's code 
to do something similar when a task is switched out (this keeps user 
level drivers from having do mmiowb() type things).

FWIW, I think I had an earlier version of the patch that used the name 
pioflush() or something similar, the only confusing thing about that 
name is that the primitive doesn't actually force I/Os down to the 
device level, just to the closest bridge.

It'll be interesting to see if upcoming x86 designs share this problem 
(e.g. large HT or CSI topologies).

Jesse

^ permalink raw reply

* Re: signals handling in the kernel
From: David Hawkins @ 2007-08-23 16:32 UTC (permalink / raw)
  To: Mirek23; +Cc: linuxppc-embedded
In-Reply-To: <12291367.post@talk.nabble.com>

Hi Mirek,

> Thank you for your hints. I was not aware about race
> conditions in signal handling routines. So far I did
> not noticed any anomalies when running my server
> since I use only one interrupt which refers to only one
> signal. I would be interested however in the solution
> you have suggested with: SIGIO and fasync() 
> 
> Would you be so kind to provide me with some example code.

Look at simple_driver.c and simple_driver_test.c in

http://www.ovro.caltech.edu/~dwh/correlator/software/driver_design.tar.gz
http://www.ovro.caltech.edu/~dwh/correlator/cobra_docs.html

Note that I used the old signals API in the user-space test:

    /* Connect up the signal handler */
    signal(SIGIO, &sigio_handler);

But you should really use the new signals API. I can't
remember the reason, it'll be explained in the Robbins&Robbins
book I reference below.

Note that another issue with signals is that they are
not queued. So if you have multiple events, you might
miss one.

I found a reference to unreliable signals. Take a look
at the signals chapter in "Advanced Programming in the Unix
Environment", by Stevens. However, that book is a bit old
now. The lack of safety may now only apply to that specific
signals API.

Another nice book is "Unix Systems Programming", by Robbins
and Robbins. Look in Chapter 8. Especially Section 8.6. That
section discusses the problems you'll face when your signal
interrupts POSIX calls, and functions that are signal handler
safe.

These types of issues would not be faced using select().
Hence the reason I can't remember the details on using
signals; as I rarely use them. The exception is that I
do catch ctrl-C to then shut down a process cleanly.
However, most of my user-space code is written using the
ACE C++ classes and infra-structure, and in that case you
deal with signals by creating an event handler, and
registering it with a reactor, which is implemented with,
you guessed it, select().  Notice the theme ... :)

The ACE C++ libraries are detailed in: "C++ Network
Programming" Volume 1 & 2 by Huston and Schmidt, and
in "The ACE programmers guide", by Huston et al.

Cheers,
Dave

^ permalink raw reply

* Re: wmb vs mmiowb
From: Benjamin Herrenschmidt @ 2007-08-23 16:27 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Nick Piggin, linux-ia64, Jesse Barnes, linuxppc-dev
In-Reply-To: <alpine.LFD.0.999.0708230915000.30176@woody.linux-foundation.org>

On Thu, 2007-08-23 at 09:16 -0700, Linus Torvalds wrote:
> 
> On Thu, 23 Aug 2007, Nick Piggin wrote:
> > 
> > Also, FWIW, there are some advantages of deferring the mmiowb thingy
> > until the point of unlock.
> 
> And that is exactly what ppc64 does.
> 
> But you're missing a big point: for 99.9% of all hardware, mmiowb() is a 
> total no-op. So when you talk about "advantages", you're not talking about 
> any *real* advantage, are you?

I wonder whether it might be worth removing mmiowb and having all archs
that matter do like ppc64 though... It's just yet another confusing
barrier that most driver writers get wrong..

Cheers,
Ben.

^ permalink raw reply

* Re: wmb vs mmiowb
From: Linus Torvalds @ 2007-08-23 16:16 UTC (permalink / raw)
  To: Nick Piggin; +Cc: linux-ia64, Jesse Barnes, linuxppc-dev
In-Reply-To: <20070823042038.GI18788@wotan.suse.de>



On Thu, 23 Aug 2007, Nick Piggin wrote:
> 
> Also, FWIW, there are some advantages of deferring the mmiowb thingy
> until the point of unlock.

And that is exactly what ppc64 does.

But you're missing a big point: for 99.9% of all hardware, mmiowb() is a 
total no-op. So when you talk about "advantages", you're not talking about 
any *real* advantage, are you?

			Linus

^ permalink raw reply

* Re: wmb vs mmiowb
From: Linus Torvalds @ 2007-08-23 16:14 UTC (permalink / raw)
  To: Nick Piggin; +Cc: linux-ia64, Jesse Barnes, linuxppc-dev
In-Reply-To: <20070823035448.GG18788@wotan.suse.de>



On Thu, 23 Aug 2007, Nick Piggin wrote:
> 
> OK, but we'd have some kind of functions that are called not to
> serialise the CPUs, but to serialise the IO. It would be up to
> the calling code to already provide CPU synchronisation.
> 
> serialize_io(); / unserialize_io(); / a nicer name

We could call it "mmiowb()", for example?

Radical idea, I know.

> If we could pass in some kind of relevant resoure (eg. the IO
> memory or device or something), then we might even be able to
> put debug checks there to ensure two CPUs are never inside the
> same critical IO section at once.

We could certainly give it the spinlock as an argument.

		Linus

^ permalink raw reply

* Re: [Cbe-oss-dev] [patch 1/5] spu_manage: use newer physical-id
From: Arnd Bergmann @ 2007-08-23 16:12 UTC (permalink / raw)
  To: kou.ishizaki; +Cc: linuxppc-dev, paulus, linux-kernel, cbe-oss-dev, jk
In-Reply-To: <20070823.180152.-1300528381.kouish@swc.toshiba.co.jp>

On Thursday 23 August 2007, kou.ishizaki@toshiba.co.jp wrote:
> Please check "unit-id" if "physical-id" doesn't exist. Because Celleb
> uses "unit-id" to provide spe_id.

Ok, I need to discuss this with Christian then, to make sure we get to
a version that works everywhere. Paul, please ignore this patch for now.

I've update the git repository to only have the other four patches.

	Arnd <><

^ permalink raw reply

* Re: Flash paritioning and JFFS2
From: Michael Brian Willis @ 2007-08-23 16:08 UTC (permalink / raw)
  To: Mirek23; +Cc: linuxppc-embedded
In-Reply-To: <12293748.post@talk.nabble.com>

On Thu, 2007-08-23 at 06:36 -0700, Mirek23 wrote:
> Could someone advice me how to configure Linux kernel to make use of the
> Flash (with above mentioned partitions and JFFS2.

The Denx U-boot and Linux Guide (DULG) will tell you how to configure
the kernel for MTD devices and will give some good general information: 
http://www.denx.de/wiki/view/DULG/FlashFilesystemsMTD



> I do not know however which specific options to enable for MTD/JFFS2 since
> it has many sub options.

The following post has some relevant information for your setup:
http://osdir.com/ml/linux.uclinux.devel/2005-12/msg00130.html



> I do not know also where to place:
> -the partition information (which I have mentioned above) 

The partition information probably should go in a file that you create
for your board located at "drivers/mtd/maps/<your_file>.c"
 

> -the start (0xFF800000) and end (0xFFBFFFFF) physical location of my Flash
> memory.

I think this information just needs to be defined in your board's .h
file in u-boot. I don't think you need to specify this in any linux
config files. 



I hope this info helps. 


Regards, 

Michael Willis
Applied Research Labs - University of Texas
willis@arlut.utexas.edu
 

^ permalink raw reply

* Flash paritioning and JFFS2
From: Mirek23 @ 2007-08-23 13:36 UTC (permalink / raw)
  To: linuxppc-embedded


Dear All,

     I am running linux 2.6.21 (by Grant) on the ppc405 which is build in
Virtex-4 of Avnet (xlinx ml403 like) evaluation board. The Avnet board has
the 4 MB of  NOR Flash (Intel TE28F640) . I just wanted to make use of the
Flash to store
u-boot, linux kernel and some config files.

To do so I wanted first to partition Flash to three partitions:
- U-Boot
- Linux-kernel
- JFFS2 (for some config files)

Could someone advice me how to configure Linux kernel to make use of the
Flash (with above mentioned partitions and JFFS2.

The information which I have collected is as following that I have to:
enable in kernel:
Device Drivers->Memory Technology Devices (MTD)
File Systems -> Miscellaneous file systems -> JFFS2

I do not know however which specific options to enable for MTD/JFFS2 since
it has many sub options.
I do not no also where to place:

-the partition information (which I have mentioned above) 
-the start (0xFF800000) and end (0xFFBFFFFF) physical location of my Flash
memory.

My aim is to mount (after booting Linux) the JFFS2 partition to store some
config data.

Best Regards and thank you in advance for any hint on that

Mirek 
   

 
-- 
View this message in context: http://www.nabble.com/Flash-paritioning-and-JFFS2-tf4317566.html#a12293748
Sent from the linuxppc-embedded mailing list archive at Nabble.com.

^ permalink raw reply

* Re: [PATCH 2/2] [POWERPC] Size swapper_pg_dir correctly
From: Kumar Gala @ 2007-08-23 13:17 UTC (permalink / raw)
  To: Stephen Rothwell; +Cc: ppc-dev, paulus
In-Reply-To: <20070823161351.2a5f8096.sfr@canb.auug.org.au>


On Aug 23, 2007, at 1:13 AM, Stephen Rothwell wrote:

> On Wed, 22 Aug 2007 22:26:35 -0500 Kumar Gala  
> <galak@kernel.crashing.org> wrote:
>>
>>> +#ifdef CONFIG_PPC64
>>> +	DEFINE(PGD_TABLE_SIZE, PGD_TABLE_SIZE);
>>> +#endif
>>
>> Why limit this to ppc64?  The cleanup should be reasonable for all  
>> ppc.
>
> Because PGD_TABLE_SIZE only exists for ppc64 :-) for ppc32 the
> swapper_pg_dir will always be 1 page, right?

No, it can be 2 pages if we do > 32-bit physical on 44x/fsl_booke.

> So we could have
> #define PGD_TABLE_SIZE PAGE_SIZE
> for 32 bit and then go around and change the swapper_pg_dir's, but  
> that
> would be a separate patch.

- k

^ permalink raw reply

* [PATCH v7 3/3] [POWERPC] MPC832x_RDB: update dts to use SPI1 in QE, register mmc_spi stub
From: Anton Vorontsov @ 2007-08-23 11:36 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <20070823113349.GA11870@localhost.localdomain>

mmc_spi already tested to work. When it will hit mainline
the only change that will be needed is replacing "spidev"
with "mmc_spi".

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 arch/powerpc/boot/dts/mpc832x_rdb.dts     |    2 +-
 arch/powerpc/platforms/83xx/mpc832x_rdb.c |   46 +++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/boot/dts/mpc832x_rdb.dts b/arch/powerpc/boot/dts/mpc832x_rdb.dts
index e9c332f..1ac0025 100644
--- a/arch/powerpc/boot/dts/mpc832x_rdb.dts
+++ b/arch/powerpc/boot/dts/mpc832x_rdb.dts
@@ -211,7 +211,7 @@
 			reg = <4c0 40>;
 			interrupts = <2>;
 			interrupt-parent = <&qeic>;
-			mode = "cpu";
+			mode = "cpu-qe";
 		};
 
 		spi@500 {
diff --git a/arch/powerpc/platforms/83xx/mpc832x_rdb.c b/arch/powerpc/platforms/83xx/mpc832x_rdb.c
index e021b08..7ddb527 100644
--- a/arch/powerpc/platforms/83xx/mpc832x_rdb.c
+++ b/arch/powerpc/platforms/83xx/mpc832x_rdb.c
@@ -15,6 +15,7 @@
  */
 
 #include <linux/pci.h>
+#include <linux/spi/spi.h>
 
 #include <asm/of_platform.h>
 #include <asm/time.h>
@@ -22,6 +23,7 @@
 #include <asm/udbg.h>
 #include <asm/qe.h>
 #include <asm/qe_ic.h>
+#include <sysdev/fsl_soc.h>
 
 #include "mpc83xx.h"
 
@@ -32,6 +34,50 @@
 #define DBG(fmt...)
 #endif
 
+static void mpc83xx_spi_activate_cs(u8 cs, u8 polarity)
+{
+	pr_debug("%s %d %d\n", __func__, cs, polarity);
+	par_io_data_set(3, 13, polarity);
+}
+
+static void mpc83xx_spi_deactivate_cs(u8 cs, u8 polarity)
+{
+	pr_debug("%s %d %d\n", __func__, cs, polarity);
+	par_io_data_set(3, 13, !polarity);
+}
+
+static struct spi_board_info mpc832x_spi_boardinfo = {
+	.bus_num = 0x4c0,
+	.chip_select = 0,
+	.max_speed_hz = 50000000,
+	/*
+	 * XXX: This is spidev (spi in userspace) stub, should
+	 * be replaced by "mmc_spi" when mmc_spi will hit mainline.
+	 */
+	.modalias = "spidev",
+};
+
+static int __init mpc832x_spi_init(void)
+{
+	if (!machine_is(mpc832x_rdb))
+		return 0;
+
+	par_io_config_pin(3,  0, 3, 0, 1, 0); /* SPI1 MOSI, I/O */
+	par_io_config_pin(3,  1, 3, 0, 1, 0); /* SPI1 MISO, I/O */
+	par_io_config_pin(3,  2, 3, 0, 1, 0); /* SPI1 CLK,  I/O */
+	par_io_config_pin(3,  3, 2, 0, 1, 0); /* SPI1 SEL,  I   */
+
+	par_io_config_pin(3, 13, 1, 0, 0, 0); /* !SD_CS,    O */
+	par_io_config_pin(3, 14, 2, 0, 0, 0); /* SD_INSERT, I */
+	par_io_config_pin(3, 15, 2, 0, 0, 0); /* SD_PROTECT,I */
+
+	return fsl_spi_init(&mpc832x_spi_boardinfo, 1,
+			    mpc83xx_spi_activate_cs,
+			    mpc83xx_spi_deactivate_cs);
+}
+
+device_initcall(mpc832x_spi_init);
+
 /* ************************************************************************
  *
  * Setup the architecture
-- 
1.5.0.6

^ permalink raw reply related

* [PATCH v7 2/3] [POWERPC] fsl_soc: add support for fsl_spi
From: Anton Vorontsov @ 2007-08-23 11:35 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <20070823113349.GA11870@localhost.localdomain>

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 arch/powerpc/sysdev/fsl_soc.c |   87 +++++++++++++++++++++++++++++++++++++++++
 arch/powerpc/sysdev/fsl_soc.h |    7 +++
 2 files changed, 94 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/sysdev/fsl_soc.c b/arch/powerpc/sysdev/fsl_soc.c
index 1cf29c9..2f11323 100644
--- a/arch/powerpc/sysdev/fsl_soc.c
+++ b/arch/powerpc/sysdev/fsl_soc.c
@@ -24,6 +24,7 @@
 #include <linux/platform_device.h>
 #include <linux/of_platform.h>
 #include <linux/phy.h>
+#include <linux/spi/spi.h>
 #include <linux/fsl_devices.h>
 #include <linux/fs_enet_pd.h>
 #include <linux/fs_uart_pd.h>
@@ -1187,3 +1188,89 @@ err:
 arch_initcall(cpm_smc_uart_of_init);
 
 #endif /* CONFIG_8xx */
+
+int __init fsl_spi_init(struct spi_board_info *board_infos,
+			unsigned int num_board_infos,
+			void (*activate_cs)(u8 cs, u8 polarity),
+			void (*deactivate_cs)(u8 cs, u8 polarity))
+{
+	struct device_node *np;
+	unsigned int i;
+	const u32 *sysclk;
+
+	np = of_find_node_by_type(NULL, "qe");
+	if (!np)
+		return -ENODEV;
+
+	sysclk = of_get_property(np, "bus-frequency", NULL);
+	if (!sysclk)
+		return -ENODEV;
+
+	for (np = NULL, i = 1;
+	     (np = of_find_compatible_node(np, "spi", "fsl_spi")) != NULL;
+	     i++) {
+		int ret = 0;
+		unsigned int j;
+		const void *prop;
+		struct resource res[2];
+		struct platform_device *pdev;
+		struct fsl_spi_platform_data pdata = {
+			.activate_cs = activate_cs,
+			.deactivate_cs = deactivate_cs,
+		};
+
+		memset(res, 0, sizeof(res));
+
+		pdata.sysclk = *sysclk;
+
+		prop = of_get_property(np, "reg", NULL);
+		if (!prop)
+			goto err;
+		pdata.bus_num = *(u32 *)prop;
+
+		prop = of_get_property(np, "mode", NULL);
+		if (prop && !strcmp(prop, "cpu-qe"))
+			pdata.qe_mode = 1;
+
+		for (j = 0; j < num_board_infos; j++) {
+			if (board_infos[j].bus_num == pdata.bus_num)
+				pdata.max_chipselect++;
+		}
+
+		if (!pdata.max_chipselect)
+			goto err;
+
+		ret = of_address_to_resource(np, 0, &res[0]);
+		if (ret)
+			goto err;
+
+		ret = of_irq_to_resource(np, 0, &res[1]);
+		if (ret == NO_IRQ)
+			goto err;
+
+		pdev = platform_device_alloc("mpc83xx_spi", i);
+		if (!pdev)
+			goto err;
+
+		ret = platform_device_add_data(pdev, &pdata, sizeof(pdata));
+		if (ret)
+			goto unreg;
+
+		ret = platform_device_add_resources(pdev, res,
+						    ARRAY_SIZE(res));
+		if (ret)
+			goto unreg;
+
+		ret = platform_device_register(pdev);
+		if (ret)
+			goto unreg;
+
+		continue;
+unreg:
+		platform_device_del(pdev);
+err:
+		continue;
+	}
+
+	return spi_register_board_info(board_infos, num_board_infos);
+}
diff --git a/arch/powerpc/sysdev/fsl_soc.h b/arch/powerpc/sysdev/fsl_soc.h
index 04e145b..618d91d 100644
--- a/arch/powerpc/sysdev/fsl_soc.h
+++ b/arch/powerpc/sysdev/fsl_soc.h
@@ -8,5 +8,12 @@ extern phys_addr_t get_immrbase(void);
 extern u32 get_brgfreq(void);
 extern u32 get_baudrate(void);
 
+struct spi_board_info;
+
+extern int fsl_spi_init(struct spi_board_info *board_infos,
+			unsigned int num_board_infos,
+			void (*activate_cs)(u8 cs, u8 polarity),
+			void (*deactivate_cs)(u8 cs, u8 polarity));
+
 #endif
 #endif
-- 
1.5.0.6

^ permalink raw reply related

* [PATCH v7 1/3] [POWERPC] QE lib: extern par_io_config_pin and par_io_data_set funcs
From: Anton Vorontsov @ 2007-08-23 11:35 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <20070823113349.GA11870@localhost.localdomain>

This is needed to configure and control QE pario pins from the kernel.

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 include/asm-powerpc/qe.h |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/include/asm-powerpc/qe.h b/include/asm-powerpc/qe.h
index 9d304b1..ad23c58 100644
--- a/include/asm-powerpc/qe.h
+++ b/include/asm-powerpc/qe.h
@@ -32,6 +32,9 @@
 extern void qe_reset(void);
 extern int par_io_init(struct device_node *np);
 extern int par_io_of_config(struct device_node *np);
+extern int par_io_config_pin(u8 port, u8 pin, int dir, int open_drain,
+			     int assignment, int has_irq);
+extern int par_io_data_set(u8 port, u8 pin, u8 val);
 
 /* QE internal API */
 int qe_issue_cmd(u32 cmd, u32 device, u8 mcn_protocol, u32 cmd_input);
-- 
1.5.0.6

^ permalink raw reply related

* [PATCH v7 0/3] [POWERPC] fsl_soc: add support for fsl_spi
From: Anton Vorontsov @ 2007-08-23 11:33 UTC (permalink / raw)
  To: Stephen Rothwell; +Cc: linuxppc-dev
In-Reply-To: <20070823132421.d2b230af.sfr@canb.auug.org.au>

On Thu, Aug 23, 2007 at 01:24:21PM +1000, Stephen Rothwell wrote:
> Hi Anton,
> 
> On Wed, 22 Aug 2007 18:57:32 +0400 Anton Vorontsov <avorontsov@ru.mvista.com> wrote:
> >
> > +	sysclk = *(u32 *)of_get_property(np, "bus-frequency", NULL);
> 
> I just cringe everytime I see someone dereference a pointer they got from
> somewhere (effectively) external without checking for NULL.

Actually, sitting at the editor I thought twice before _removing_ NULL
checks, and obviously was wrong with final decision. ;-)

I just knew that there is already many places in fsl_soc that don't
check for NULLs, especially when:

> I realise
> that sometimes "that can't happen" ...

Probably to save some code space. But now I seem to comprehend it: not
checking for NULL properties is not a some kind of rule for fsl_soc,
but exceptions which probably should be fixed someday.

Thanks.


[Not related to this particular answer]

Heh.. honestly speaking, I myself don't like externs I introducing in
the board file. Okay, let's risk, and do whole thing correctly:
placing externs into asm/qe.h, trivial patch that could end up with
long discussion. ;-)

Lucky numbered v7 is following.

-- 
Anton Vorontsov
email: cbou@mail.ru
backup email: ya-cbou@yandex.ru
irc://irc.freenode.net/bd2

^ permalink raw reply

* Re: signals handling in the kernel
From: Mirek23 @ 2007-08-23 10:57 UTC (permalink / raw)
  To: linuxppc-embedded
In-Reply-To: <46C9C70D.9080903@ovro.caltech.edu>


Hi David,

    Thank you for your hints. I was not aware about race conditions in
signal handling routines. So far I did not noticed any anomalies when
running my server since I use only one interrupt which refers to only one
signal. 
I would be interested however in the solution you have suggested with:
SIGIO and fasync() 

Would you be so kind to provide me with some example code.

Best Regards

Mirek




David Hawkins-3 wrote:
> 
> Hi Mirek,
> 
>> In my case I found somehow more convenient to deal with
>> signals.
> 
> Ok.
> 
>> The server program which I use was originally written
>> for VxWorks. In VxWorks there was no separation betwenn
>> the user and kernel space. When the interrupt occured
>> in VxWorks the interrupt service routine was called.
>> The interrupt service routine was implemented in the server. 
>> 
>> I found it somehow easier to use signals to trigger signal handler
>> (previously in VxWorks interrupt service routine) than changing the
>> structure of the server to deal with select().
> 
> I guess it depends on what you consider 'easier'.
> Signals have potential race conditions, and so using
> select() is safer. I find it 'easier' to have less
> problems, so would spend the time to make the server
> use select().
> 
> But, you are free to ignore this advice. :)
> 
>> I hope however that there is no fundamental problem with
>> sending signals from kernel (interrupt service routine)
>> to the user space.
> 
> There are potential race conditions. I'm not sure if this
> problem was 2.4 kernel specific, or 2.6 kernel specific,
> or signals specific. I think its signals specific.
> 
> A web search should yield more info on this. Try googling
> 'signals race condition', and it looks like its a problem
> still.
> 
> So it depends on whether your server is running in
> a critical, and secure system, as to whether you want
> to stick with signals.
> 
>> I do not know why the function kill_proc_info does not 
>> export its symbol within the kernel 2.6.21 .
>> With previous version of the kernel 2.4 and early 2.6.*
>> the kill_proc_info symbol was exported.
> 
> If SIGIO is sufficient for you, then just use the driver
> fasync() call-back mechanism. The example code I referred
> to has an example. If its not clear to you, I can
> explain it.
> 
> If you're having to modify some corner of the kernel not
> used by many, then I'm sure your solution is not the
> correct one, and you won't get anyone helping you
> when things go wrong.
> 
> So, take the experience of others; re-write the server
> to use signals. If the server was well written to start
> with you should be able to call the 'signal handler'
> function after returning from your select() call with
> the handle ready. It shouldn't be that hard.
> 
> Come on, you've just returned from holiday, it should
> be no sweat to code up a new server :)
> 
> Regards,
> Dave
> 
> 
> _______________________________________________
> Linuxppc-embedded mailing list
> Linuxppc-embedded@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-embedded
> 
> 

-- 
View this message in context: http://www.nabble.com/signals-handling-in-the-kernel-tf4229566.html#a12291367
Sent from the linuxppc-embedded mailing list archive at Nabble.com.

^ 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