* Re: [POWERPC] convert string i/o operations to C
From: Michael Ellerman @ 2006-09-20 4:38 UTC (permalink / raw)
To: Stephen Rothwell; +Cc: ppc-dev, paulus
In-Reply-To: <20060920133532.ce818ab7.sfr@canb.auug.org.au>
[-- Attachment #1: Type: text/plain, Size: 851 bytes --]
On Wed, 2006-09-20 at 13:35 +1000, Stephen Rothwell wrote:
> This produces essentially the same code and will make the iSeries i/o
> consolidation easier.
A bit more white space would be nice :D, eg:
> +void _insb(volatile u8 __iomem *port, void *buf, long count)
> +{
> + u8 *tbuf = buf;
> + u8 tmp;
> +
> + if (unlikely(count <= 0))
> + return;
> + asm volatile("sync");
> + do {
> + tmp = *port;
> + asm volatile("eieio");
> + *tbuf++ = tmp;
> + } while (--count != 0);
> + asm volatile("twi 0,%0,0; isync" : : "r" (tmp));
> +}
> +EXPORT_SYMBOL(_insb);
cheers
--
Michael Ellerman
OzLabs, IBM Australia Development Lab
wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)
We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 191 bytes --]
^ permalink raw reply
* Re: [PATCH] Start arch/powerpc/boot code reorganization
From: Josh Boyer @ 2006-09-20 3:10 UTC (permalink / raw)
To: Mark A. Greer; +Cc: linuxppc-dev, Paul Mackerras
In-Reply-To: <20060920012042.GA28489@mag.az.mvista.com>
On Tue, 2006-09-19 at 18:20 -0700, Mark A. Greer wrote:
> Paul,
>
> Regarding our earlier conversation about ft_translate_addr and
> ft_parentize. I just realized that on IRC today Matt Porter pointed
> out that using the "reg" property in ns16550.c is wrong. We--several
> of us on #mklinux--decided that the "address" property is the correct
> thing to use because we should really be using a virtual address from
> the fw (bootwrapper has no ioremap). So, unless someone objects,
> we'll all add the "address" property to our uart device nodes in our
> fdt's (if they can be used as the console).
An example of where this is needed is 4xx. Lots of boards rely on the
openbios mapping created for the UART in the zImage wrapper. However,
the kernel doesn't rely on that, and the "reg" property of the .dts
should contain the physical address.
It was pointed out on IRC that the "address" property is defined in the
OF spec for specifying virtual address mappings. This is exactly what
we need in that it allows the zImage wrapper to use the fw defined UART
mapping, but the kernel still gets the real physical address later on.
And other things that don't use zImage wrappers, like u-boot, can simply
ignore the "address" property defined within the UART node.
The options that were discussed were this, hardcoding, or implementing
ioremap in the bootwraper. This seemed to be the most elegant way of
doing things.
josh
^ permalink raw reply
* [POWERPC] convert string i/o operations to C
From: Stephen Rothwell @ 2006-09-20 3:35 UTC (permalink / raw)
To: paulus; +Cc: ppc-dev
In-Reply-To: <20060919222351.d27a1a06.sfr@canb.auug.org.au>
This produces essentially the same code and will make the iSeries i/o
consolidation easier.
The count parameter is changed to long since that will produce the same
(better) code on 32 and 64 bit builds.
Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
arch/powerpc/kernel/Makefile | 2 -
arch/powerpc/kernel/io.c | 117 +++++++++++++++++++++++++++++++++++++++
arch/powerpc/kernel/misc.S | 95 --------------------------------
arch/powerpc/kernel/ppc_ksyms.c | 7 --
include/asm-powerpc/io.h | 12 ++--
include/asm-ppc/io.h | 12 ++--
6 files changed, 130 insertions(+), 115 deletions(-)
This version has lots more C code. Paulus also said that the "sync"s
could go after the check for count <= 0.
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index 8b3f4fa..8b133af 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -51,7 +51,7 @@ extra-$(CONFIG_8xx) := head_8xx.o
extra-y += vmlinux.lds
obj-y += time.o prom.o traps.o setup-common.o \
- udbg.o misc.o
+ udbg.o misc.o io.o
obj-$(CONFIG_PPC32) += entry_32.o setup_32.o misc_32.o
obj-$(CONFIG_PPC64) += misc_64.o dma_64.o iommu.o
obj-$(CONFIG_PPC_MULTIPLATFORM) += prom_init.o
diff --git a/arch/powerpc/kernel/io.c b/arch/powerpc/kernel/io.c
new file mode 100644
index 0000000..80a3209
--- /dev/null
+++ b/arch/powerpc/kernel/io.c
@@ -0,0 +1,117 @@
+/*
+ * I/O string operations
+ * Copyright (C) 1995-1996 Gary Thomas (gdt@linuxppc.org)
+ * Copyright (C) 2006 IBM Corporation
+ *
+ * Largely rewritten by Cort Dougan (cort@cs.nmt.edu)
+ * and Paul Mackerras.
+ *
+ * Adapted for iSeries by Mike Corrigan (mikejc@us.ibm.com)
+ * PPC64 updates by Dave Engebretsen (engebret@us.ibm.com)
+ *
+ * Rewritten in C by Stephen Rothwell.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+#include <linux/kernel.h>
+#include <linux/types.h>
+#include <linux/compiler.h>
+#include <linux/module.h>
+
+#include <asm/io.h>
+
+void _insb(volatile u8 __iomem *port, void *buf, long count)
+{
+ u8 *tbuf = buf;
+ u8 tmp;
+
+ if (unlikely(count <= 0))
+ return;
+ asm volatile("sync");
+ do {
+ tmp = *port;
+ asm volatile("eieio");
+ *tbuf++ = tmp;
+ } while (--count != 0);
+ asm volatile("twi 0,%0,0; isync" : : "r" (tmp));
+}
+EXPORT_SYMBOL(_insb);
+
+void _outsb(volatile u8 __iomem *port, const void *buf, long count)
+{
+ const u8 *tbuf = buf;
+
+ if (unlikely(count <= 0))
+ return;
+ asm volatile("sync");
+ do {
+ *port = *tbuf++;
+ } while (--count != 0);
+ asm volatile("sync");
+}
+EXPORT_SYMBOL(_outsb);
+
+void _insw_ns(volatile u16 __iomem *port, void *buf, long count)
+{
+ u16 *tbuf = buf;
+ u16 tmp;
+
+ if (unlikely(count <= 0))
+ return;
+ asm volatile("sync");
+ do {
+ tmp = *port;
+ asm volatile("eieio");
+ *tbuf++ = tmp;
+ } while (--count != 0);
+ asm volatile("twi 0,%0,0; isync" : : "r" (tmp));
+}
+EXPORT_SYMBOL(_insw_ns);
+
+void _outsw_ns(volatile u16 __iomem *port, const void *buf, long count)
+{
+ const u16 *tbuf = buf;
+
+ if (unlikely(count <= 0))
+ return;
+ asm volatile("sync");
+ do {
+ *port = *tbuf++;
+ } while (--count != 0);
+ asm volatile("sync");
+}
+EXPORT_SYMBOL(_outsw_ns);
+
+void _insl_ns(volatile u32 __iomem *port, void *buf, long count)
+{
+ u32 *tbuf = buf;
+ u32 tmp;
+
+ if (unlikely(count <= 0))
+ return;
+ asm volatile("sync");
+ do {
+ tmp = *port;
+ asm volatile("eieio");
+ *tbuf++ = tmp;
+ } while (--count != 0);
+ asm volatile("twi 0,%0,0; isync" : : "r" (tmp));
+}
+EXPORT_SYMBOL(_insl_ns);
+
+void _outsl_ns(volatile u32 __iomem *port, const void *buf, long count)
+{
+ const u32 *tbuf = buf;
+
+ if (unlikely(count <= 0))
+ return;
+ asm volatile("sync");
+ do {
+ *port = *tbuf++;
+ } while (--count != 0);
+ asm volatile("sync");
+}
+EXPORT_SYMBOL(_outsl_ns);
diff --git a/arch/powerpc/kernel/misc.S b/arch/powerpc/kernel/misc.S
index 6feb391..330c9dc 100644
--- a/arch/powerpc/kernel/misc.S
+++ b/arch/powerpc/kernel/misc.S
@@ -43,98 +43,3 @@ _GLOBAL(add_reloc_offset)
add r3,r3,r5
mtlr r0
blr
-
-/*
- * I/O string operations
- *
- * insb(port, buf, len)
- * outsb(port, buf, len)
- * insw(port, buf, len)
- * outsw(port, buf, len)
- * insl(port, buf, len)
- * outsl(port, buf, len)
- * insw_ns(port, buf, len)
- * outsw_ns(port, buf, len)
- * insl_ns(port, buf, len)
- * outsl_ns(port, buf, len)
- *
- * The *_ns versions don't do byte-swapping.
- */
-_GLOBAL(_insb)
- sync
- cmpwi 0,r5,0
- mtctr r5
- subi r4,r4,1
- blelr-
-00: lbz r5,0(r3)
- eieio
- stbu r5,1(r4)
- bdnz 00b
- twi 0,r5,0
- isync
- blr
-
-_GLOBAL(_outsb)
- cmpwi 0,r5,0
- mtctr r5
- subi r4,r4,1
- blelr-
- sync
-00: lbzu r5,1(r4)
- stb r5,0(r3)
- bdnz 00b
- sync
- blr
-
-_GLOBAL(_insw_ns)
- sync
- cmpwi 0,r5,0
- mtctr r5
- subi r4,r4,2
- blelr-
-00: lhz r5,0(r3)
- eieio
- sthu r5,2(r4)
- bdnz 00b
- twi 0,r5,0
- isync
- blr
-
-_GLOBAL(_outsw_ns)
- cmpwi 0,r5,0
- mtctr r5
- subi r4,r4,2
- blelr-
- sync
-00: lhzu r5,2(r4)
- sth r5,0(r3)
- bdnz 00b
- sync
- blr
-
-_GLOBAL(_insl_ns)
- sync
- cmpwi 0,r5,0
- mtctr r5
- subi r4,r4,4
- blelr-
-00: lwz r5,0(r3)
- eieio
- stwu r5,4(r4)
- bdnz 00b
- twi 0,r5,0
- isync
- blr
-
-_GLOBAL(_outsl_ns)
- cmpwi 0,r5,0
- mtctr r5
- subi r4,r4,4
- blelr-
- sync
-00: lwzu r5,4(r4)
- stw r5,0(r3)
- bdnz 00b
- sync
- blr
-
diff --git a/arch/powerpc/kernel/ppc_ksyms.c b/arch/powerpc/kernel/ppc_ksyms.c
index 75429e5..807193a 100644
--- a/arch/powerpc/kernel/ppc_ksyms.c
+++ b/arch/powerpc/kernel/ppc_ksyms.c
@@ -95,13 +95,6 @@ #ifdef CONFIG_PPC64
EXPORT_SYMBOL(copy_4K_page);
#endif
-EXPORT_SYMBOL(_insb);
-EXPORT_SYMBOL(_outsb);
-EXPORT_SYMBOL(_insw_ns);
-EXPORT_SYMBOL(_outsw_ns);
-EXPORT_SYMBOL(_insl_ns);
-EXPORT_SYMBOL(_outsl_ns);
-
#if defined(CONFIG_PPC32) && (defined(CONFIG_BLK_DEV_IDE) || defined(CONFIG_BLK_DEV_IDE_MODULE))
EXPORT_SYMBOL(ppc_ide_md);
#endif
diff --git a/include/asm-powerpc/io.h b/include/asm-powerpc/io.h
index 51a5987..57e7d14 100644
--- a/include/asm-powerpc/io.h
+++ b/include/asm-powerpc/io.h
@@ -143,12 +143,12 @@ #define readw_relaxed(addr) readw(addr)
#define readl_relaxed(addr) readl(addr)
#define readq_relaxed(addr) readq(addr)
-extern void _insb(volatile u8 __iomem *port, void *buf, int ns);
-extern void _outsb(volatile u8 __iomem *port, const void *buf, int ns);
-extern void _insw_ns(volatile u16 __iomem *port, void *buf, int ns);
-extern void _outsw_ns(volatile u16 __iomem *port, const void *buf, int ns);
-extern void _insl_ns(volatile u32 __iomem *port, void *buf, int nl);
-extern void _outsl_ns(volatile u32 __iomem *port, const void *buf, int nl);
+extern void _insb(volatile u8 __iomem *port, void *buf, long count);
+extern void _outsb(volatile u8 __iomem *port, const void *buf, long count);
+extern void _insw_ns(volatile u16 __iomem *port, void *buf, long count);
+extern void _outsw_ns(volatile u16 __iomem *port, const void *buf, long count);
+extern void _insl_ns(volatile u32 __iomem *port, void *buf, long count);
+extern void _outsl_ns(volatile u32 __iomem *port, const void *buf, long count);
static inline void mmiowb(void)
{
diff --git a/include/asm-ppc/io.h b/include/asm-ppc/io.h
index 9fac420..3d9a9e6 100644
--- a/include/asm-ppc/io.h
+++ b/include/asm-ppc/io.h
@@ -327,12 +327,12 @@ #define outw_p(val, port) outw((val), (p
#define inl_p(port) inl((port))
#define outl_p(val, port) outl((val), (port))
-extern void _insb(volatile u8 __iomem *port, void *buf, int ns);
-extern void _outsb(volatile u8 __iomem *port, const void *buf, int ns);
-extern void _insw_ns(volatile u16 __iomem *port, void *buf, int ns);
-extern void _outsw_ns(volatile u16 __iomem *port, const void *buf, int ns);
-extern void _insl_ns(volatile u32 __iomem *port, void *buf, int nl);
-extern void _outsl_ns(volatile u32 __iomem *port, const void *buf, int nl);
+extern void _insb(volatile u8 __iomem *port, void *buf, long count);
+extern void _outsb(volatile u8 __iomem *port, const void *buf, long count);
+extern void _insw_ns(volatile u16 __iomem *port, void *buf, long count);
+extern void _outsw_ns(volatile u16 __iomem *port, const void *buf, long count);
+extern void _insl_ns(volatile u32 __iomem *port, void *buf, long count);
+extern void _outsl_ns(volatile u32 __iomem *port, const void *buf, long count);
#define IO_SPACE_LIMIT ~0
--
1.4.2.1
^ permalink raw reply related
* Re: [PATCH] Remove powerpc specific parts of 3c509 driver
From: Segher Boessenkool @ 2006-09-20 1:41 UTC (permalink / raw)
To: Linas Vepstas; +Cc: akpm, Stephen Rothwell, ppc-dev, netdev, jgarzik
In-Reply-To: <20060920011750.GS29167@austin.ibm.com>
> Well, I'm having trouble thinking of other busses that have as strong
> a sense of the "address-data" style I/O as PCI. Busses like scsi and
> ide are primarily "command-data" or "data-data" in style. Only the
> address-data style busses need readl/writel-style routines.
SBUS, JBUS, VMEbus, NuBus, RapidIO, eBus, various SoC busses, to name
a few. There are many, and most are big-endian.
Segher
^ permalink raw reply
* Re: [PATCH] Start arch/powerpc/boot code reorganization
From: Mark A. Greer @ 2006-09-20 1:28 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linuxppc-dev
In-Reply-To: <20060920012042.GA28489@mag.az.mvista.com>
On Tue, Sep 19, 2006 at 06:20:42PM -0700, Mark A. Greer wrote:
> Paul,
>
> Regarding our earlier conversation about ft_translate_addr and
...
BTW, the only changes to your patch would be removing translate_addr from
dt_ops in ops.h and removing the "dt_ops.translate_addr = NULL;" line
in of.c.
I'll respin the other patches.
Mark
^ permalink raw reply
* Re: [PATCH] Start arch/powerpc/boot code reorganization
From: Mark A. Greer @ 2006-09-20 1:20 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linuxppc-dev
In-Reply-To: <17680.30367.157642.674242@cargo.ozlabs.ibm.com>
Paul,
Regarding our earlier conversation about ft_translate_addr and
ft_parentize. I just realized that on IRC today Matt Porter pointed
out that using the "reg" property in ns16550.c is wrong. We--several
of us on #mklinux--decided that the "address" property is the correct
thing to use because we should really be using a virtual address from
the fw (bootwrapper has no ioremap). So, unless someone objects,
we'll all add the "address" property to our uart device nodes in our
fdt's (if they can be used as the console).
A couple consequences, if that happens:
- It removes the requirement for ft_translate_addr (and
dt_ops.translate_addr) in the bootwrapper. That cleans up
flatdevtree_misc.c a lot.
- Since ft_find_node/device will be the only caller of ft_parentize
now, that may change your plans for ft_parentize.
Do you have any objections to using "address" property instead of "reg"?
Mark
^ permalink raw reply
* Re: [PATCH] Remove powerpc specific parts of 3c509 driver
From: Jeff Garzik @ 2006-09-20 1:18 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: akpm, Stephen Rothwell, ppc-dev, netdev
In-Reply-To: <1192ED41-329C-4CB0-AD87-B3BF62EC4337@kernel.crashing.org>
Segher Boessenkool wrote:
>>> Sure, PCI busses are little-endian. But is readX()/writeX() for PCI
>>> only?
>>
>> Yes.
>>
>> For other buses, use foo_writel(), etc.
>
> Can this please be documented then? Never heard this before...
You have come late to the party. This has been the case for many, many
years.
And there is no point in a massive rename to pci_writel(), either.
Jeff
^ permalink raw reply
* Re: [PATCH] Remove powerpc specific parts of 3c509 driver
From: Linas Vepstas @ 2006-09-20 1:17 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: akpm, Stephen Rothwell, ppc-dev, netdev, jgarzik
In-Reply-To: <9F6F4A8E-C780-4358-97AA-570B33C0598F@kernel.crashing.org>
Hi,
I am alarmed and embarassed that sloppy comments on my part has turned
onto a long conversation.
On Wed, Sep 20, 2006 at 02:58:39AM +0200, Segher Boessenkool wrote:
> >> Sure, PCI busses are little-endian. But is readX()/writeX() for PCI
> >> only? I sure hope not.
> >
> > It's defined for PCI and possibly ISA memory. You can use it for other
> > things if you whish to, but "other things" are arch specific in any
> > case.
>
> Huh? You're saying that only PCI and ISA are standardised busses?
Well, I'm having trouble thinking of other busses that have as strong
a sense of the "address-data" style I/O as PCI. Busses like scsi and
ide are primarily "command-data" or "data-data" in style. Only the
address-data style busses need readl/writel-style routines.
I can't prove, but suspect that the "adress-data" style of access is
why PCI is wired up "close to" the CPU. What other bsses are there
that are direct-attached to the CPU? I can't think of much ...
The sbus on sparc ... hypertransport from AMD ... but hypertransport is
more or less invisible to the kernel. ... some recent attempts to
supplant the system bus with infiniband, but I get the impression that
this will be strangely engineered, and semi-invisible to the kernel as
well. The actual infiniband protocols are ipv6-like+rdma and so fall
into a "data-data" programming style.
> > Different bus -> different accessor.
>
> Then please rename readX()/writeX() to pci_readX()/pci_writeX().
Well, I don't get the impression that there will be othre busses for
which this is an issue the way it is on pci.
--linas
^ permalink raw reply
* Re: Hang with isync
From: Manoj Sharma @ 2006-09-20 1:16 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <d6dada100609181828j1804f4f5ic0d1dd8a225980c@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 791 bytes --]
>
> Hi,
>
> We use linux kernel 2.4.20 on ppc405 and the system hangs once in a while
> when isync gets called in this function:
>
> _GLOBAL(_nmask_and_or_msr)
> mfmsr r0 /* Get current msr */
> andc r0,r0,r3 /* And off the bits set in r3 (first parm) */
> or r0,r0,r4 /* Or on the bits in r4 (second parm) */
> sync /* Some chip revs have problems here... */
> isync
> mtmsr r0 /* Update machine state */
> isync
> blr /* Done */
>
> 2.5 onwards, I find that "sync; isync" has been replaced by a macro SYNC
> (defined only for 601). I don't find it in any changelog and reason for the
> change.
>
> Can someone give some information on this change?
>
> Appreciate any help.
> Manoj
>
> <linuxppc-dev@ozlabs.org>
>
[-- Attachment #2: Type: text/html, Size: 1490 bytes --]
^ permalink raw reply
* Re: [PATCH] Remove powerpc specific parts of 3c509 driver
From: Segher Boessenkool @ 2006-09-20 1:08 UTC (permalink / raw)
To: Jeff Garzik; +Cc: akpm, Stephen Rothwell, ppc-dev, netdev
In-Reply-To: <45109203.6060606@pobox.com>
>> Sure, PCI busses are little-endian. But is readX()/writeX() for PCI
>> only?
>
> Yes.
>
> For other buses, use foo_writel(), etc.
Can this please be documented then? Never heard this before...
Segher
^ permalink raw reply
* Re: JTAG debugger for MPC82xx/MPC83xx
From: Greg Weeks @ 2006-09-20 0:22 UTC (permalink / raw)
To: Steven Hein; +Cc: linuxppc-embedded
In-Reply-To: <4510664B.2020304@sgi.com>
Steven Hein wrote:
> I've been researching the available JTAG debuggers that
> support the MPC82xx/MPC83xx family, and I've found these
> choices so far:
>
> * Abatron BDI-2000
> * Green Hills Probe
> * Freescale's PowerTAP PRO for PowerPC
> * Lauterbach TRACE32-ICD
> * WindRiver ICE
> * Embedded Toolsmiths Guardian-SE (BUT...this is no longer
> being sold, according to an email response I received
> from Embedded Toolsmiths)
>
> We will definitely want one with a network connection,
> and will will run the debugger software from a Linux host.
> Ideally, we would also like one that provides a library
> to allow us to write apps to function the debugger.
>
> Can anyone comment on what JTAG debugger they are using,
> and how have your experiences been? Are there any other
> good options besides the ones mentioned above.
> I'd appreciate any input.
>
The only one I've used for power pc is the BDI. I've not tried it on the
83xx yet, but others here have. It worked well with the 82xx. I mostly
used gdb to talk to it over ethernet.
Greg Weeks
^ permalink raw reply
* Re: [PATCH] Remove powerpc specific parts of 3c509 driver
From: Segher Boessenkool @ 2006-09-20 0:58 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: akpm, Stephen Rothwell, ppc-dev, netdev, jgarzik
In-Reply-To: <1158711935.6002.226.camel@localhost.localdomain>
>> Sure, PCI busses are little-endian. But is readX()/writeX() for PCI
>> only? I sure hope not.
>
> It's defined for PCI and possibly ISA memory. You can use it for other
> things if you whish to, but "other things" are arch specific in any
> case.
Huh? You're saying that only PCI and ISA are standardised busses?
>> It would make a lot more sense if readX()/writeX() used the
>> endianness
>> of the bus they are performed on.
>
> No way ! Again, it's evil if such a simple thing start doing different
> things depending on random external factors.
That's your opinion, yes.
I'm saying it's *not* doing different things: in both cases it just does
the correct-endian access. Also it doesn't depend on "random external
factors" -- they're not random factors, and not external either: it only
depends on the bus the access is done on.
> Different bus -> different accessor.
Then please rename readX()/writeX() to pci_readX()/pci_writeX().
>> Now you can say, use readl_be() or something similar, but that's a)
>> ugly,
>> b) error-prone, c) exponential interface explosion, d) ugly.
>
> I'd rather has an interface explosion than having black endian magic
> happening inside of the accessors.
Any comments on a), b) and d) as well?
Segher
^ permalink raw reply
* Re: [PATCH] Remove powerpc specific parts of 3c509 driver
From: Jeff Garzik @ 2006-09-20 0:57 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: akpm, Stephen Rothwell, ppc-dev, netdev
In-Reply-To: <488875E7-CCBC-47E1-A273-A2D037A997B2@kernel.crashing.org>
Segher Boessenkool wrote:
> Sure, PCI busses are little-endian. But is readX()/writeX() for PCI
> only?
Yes.
For other buses, use foo_writel(), etc.
Jeff
^ permalink raw reply
* Re: [PATCH] Remove powerpc specific parts of 3c509 driver
From: Benjamin Herrenschmidt @ 2006-09-20 0:25 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: akpm, Stephen Rothwell, ppc-dev, netdev, jgarzik
In-Reply-To: <488875E7-CCBC-47E1-A273-A2D037A997B2@kernel.crashing.org>
On Wed, 2006-09-20 at 02:21 +0200, Segher Boessenkool wrote:
> > Nah. We have the basic rule that readl/writel are little endian.
> > PowerPC
> > additionally provides arch specific low level in_{be,le}32 type
> > accessors with explicit endianness. Or you can also use
> > cpu_to_le32/le32_to_cpu kind of macros to convert between native and
> > explicit endianness.
>
> Sure, PCI busses are little-endian. But is readX()/writeX() for PCI
> only? I sure hope not.
It's defined for PCI and possibly ISA memory. You can use it for other
things if you whish to, but "other things" are arch specific in any
case.
> It would make a lot more sense if readX()/writeX() used the endianness
> of the bus they are performed on.
No way ! Again, it's evil if such a simple thing start doing different
things depending on random external factors.
Different bus -> different accessor.
We defined on PowerPC that readl was fine for anything that comes out of
ioremap and is little endian, but that's also why you have the explicit
{in,out}_{le,be}{16,32}. That's what you should use in fact with non-PCI
busses unless you know you are LE.
> PowerPC byteswaps are cheap -- for 16- and 32-bit accesses. They're quite bad for 64-bit though; it would
> be a pity to end up doing two of those for a 64-bit big-endian I/O
> access
> (one on the access itself, one to convert the data back to CPU order).
>
> This would happily solve the problem of the various variations of
> byte-swapping bus bridges, too ("natural" swap, 32-bit swap, 64-bit
> swap,
> perhaps others that I thankfully have never seen or cannot remember).
>
> Now you can say, use readl_be() or something similar, but that's a)
> ugly,
> b) error-prone, c) exponential interface explosion, d) ugly.
I'd rather has an interface explosion than having black endian magic
happening inside of the accessors.
Ben.
^ permalink raw reply
* Re: [PATCH] Remove powerpc specific parts of 3c509 driver
From: Segher Boessenkool @ 2006-09-20 0:21 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: akpm, Stephen Rothwell, ppc-dev, netdev, jgarzik
In-Reply-To: <1158708269.6002.186.camel@localhost.localdomain>
> Nah. We have the basic rule that readl/writel are little endian.
> PowerPC
> additionally provides arch specific low level in_{be,le}32 type
> accessors with explicit endianness. Or you can also use
> cpu_to_le32/le32_to_cpu kind of macros to convert between native and
> explicit endianness.
Sure, PCI busses are little-endian. But is readX()/writeX() for PCI
only? I sure hope not.
It would make a lot more sense if readX()/writeX() used the endianness
of the bus they are performed on. PowerPC byteswaps are cheap -- for
16- and 32-bit accesses. They're quite bad for 64-bit though; it would
be a pity to end up doing two of those for a 64-bit big-endian I/O
access
(one on the access itself, one to convert the data back to CPU order).
This would happily solve the problem of the various variations of
byte-swapping bus bridges, too ("natural" swap, 32-bit swap, 64-bit
swap,
perhaps others that I thankfully have never seen or cannot remember).
Now you can say, use readl_be() or something similar, but that's a)
ugly,
b) error-prone, c) exponential interface explosion, d) ugly.
Segher
^ permalink raw reply
* Re: [POWERPC] convert string i/o operations to C
From: Benjamin Herrenschmidt @ 2006-09-20 0:08 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: Stephen Rothwell, paulus, ppc-dev
In-Reply-To: <F3BB850E-E6DD-4CE1-A93B-A56BD2C4B653@kernel.crashing.org>
> Why? If "ns" <= 0 there is no I/O done...
For the sake of being paranoid :) What happens on x86 if you do a rep ;
insw with a 0 count ?
Ben.
^ permalink raw reply
* Re: [POWERPC] convert string i/o operations to C
From: Segher Boessenkool @ 2006-09-20 0:03 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: Stephen Rothwell, paulus, ppc-dev
In-Reply-To: <1158707532.6002.171.camel@localhost.localdomain>
>> Perhaps this:
>>
>>> +void _insb(volatile u8 __iomem *port, void *buf, int ns)
>>> +{
>>> + asm volatile("sync");
>>> + if (ns <= 0)
>>> + return;
>>> + asm volatile(
>>
>> should be this:
>>
>>> +void _insb(volatile u8 __iomem *port, void *buf, int ns)
>>> +{
>>> + if (ns <= 0)
>>> + return;
>>> + asm volatile("sync");
>>> + asm volatile(
>
> No. The barrier should be there even if somebody is lame enough to
> give
> a count of 0.
Why? If "ns" <= 0 there is no I/O done...
Segher
^ permalink raw reply
* Re: Fw: [PATCH] Remove powerpc specific parts of 3c509 driver
From: Paul Mackerras @ 2006-09-19 23:27 UTC (permalink / raw)
To: Linas Vepstas; +Cc: akpm, Stephen Rothwell, netdev, jgarzik, ppc-dev
In-Reply-To: <20060919184243.GL29167@austin.ibm.com>
Linas Vepstas writes:
> The rest of this patch might indeed be correct, but the above comment
> bothers me. The "ns" versions of routines are supposed to be
> non-byte-swapped versions of the insl/outsl routines (which would
> byte-swap on big-endian archs such as powerpc.)
If it were true that in/outsw and in/outsl were actually used to
transfer arrays of 16-bit data items or 32-bit data items to/from an
I/O device, I would agree with you, but they aren't. They are
universally used to transfer arrays of bytes, with the optimization of
doing so 2 or 4 bytes at a time. That is why in/outsw and in/outsl
don't (and shouldn't) do byte swapping.
Paul.
^ permalink raw reply
* Re: JTAG debugger for MPC82xx/MPC83xx
From: Mark A. Greer @ 2006-09-19 23:26 UTC (permalink / raw)
To: Steven Hein; +Cc: linuxppc-embedded
In-Reply-To: <4510664B.2020304@sgi.com>
On Tue, Sep 19, 2006 at 04:51:07PM -0500, Steven Hein wrote:
> Hi,
>
> My group is starting a new project includes a custom board
> with an MPC8258E processor on it. I'm in the process
> of evaluating the state of the Linux kernel, toolchains,
> etc. for the MPC8258E/MPC8360E, as well as selecting a
> JTAG debugger for our project.
>
> I've been researching the available JTAG debuggers that
> support the MPC82xx/MPC83xx family, and I've found these
> choices so far:
>
> * Abatron BDI-2000
> * Green Hills Probe
> * Freescale's PowerTAP PRO for PowerPC
> * Lauterbach TRACE32-ICD
> * WindRiver ICE
> * Embedded Toolsmiths Guardian-SE (BUT...this is no longer
> being sold, according to an email response I received
> from Embedded Toolsmiths)
>
> We will definitely want one with a network connection,
> and will will run the debugger software from a Linux host.
> Ideally, we would also like one that provides a library
> to allow us to write apps to function the debugger.
>
> Can anyone comment on what JTAG debugger they are using,
> and how have your experiences been? Are there any other
> good options besides the ones mentioned above.
> I'd appreciate any input.
You will find, by far, more bdi2000 users on the linuxppc-dev/embedded
lists than all the others debuggers combined, times 10, etc (at least,
they're that much more vocal). Other linux arch's have a lot of bdi
users too. There's even some powerpc linux kernel support for the bdi.
In general, the bdi works really well and their support is top notch.
I do not recieve any kickbacks from abatron, btw. This question has
been asked before on this list (or -dev) so you can see other responses
in the archives.
Mark
^ permalink raw reply
* Re: Fw: [PATCH] Remove powerpc specific parts of 3c509 driver
From: Benjamin Herrenschmidt @ 2006-09-19 23:24 UTC (permalink / raw)
To: Matt Sealey; +Cc: akpm, Stephen Rothwell, jgarzik, ppc-dev, netdev
In-Reply-To: <45103C62.4080003@genesi-usa.com>
On Tue, 2006-09-19 at 20:52 +0200, Matt Sealey wrote:
> Some northbridges and PCI bridges have "clever" byteswapping in
> hardware, maybe this is just an effect of that. In theory depending on
> the host bridge, you should pass in big endian data and have it swap or
> not swap, not pick that way in the driver, UNLESS your driver expects
> bigendian data, in which case on a bigendian platform you can tell it to
> write without swapping. Voila, two functions.
It's generally considered pretty bad for a northbridge to try to muck
around with byte order. There are fairly well defined rules to plug a
little endian bus (PCI, ISA, ...) on a big endian machine.
The trick that some people didn't get a while ago is that while
accessors like inw/inl shall return a byteswapped data, string
operations like in insw/insl who are copying from a fifo basically to
memory (and opposite write versions) shall -not- byteswap since the data
isn't interpreted. it's a byte stream. It doesn't have any endian
semantic associated to it until it's actually read back from memory in
which case the appropriate endian swap (if any) has to be used depending
on the endianness and size of a given field read/written.
Since some people didn't get it, in the early days, some BE
architectures like ppc had versions of insw/insl that did byteswap,
which was wrong. The bits in this driver are remains from that era.
Note that to aggravate the problem, it still happens that HW engineers
try to be "smart" when hooking a 16 bits or 32 bits FIFO to a BE machine
and byteswap it in hardware. This is of course totally bogus but did
happen with IDE controllers typically (I think atari or amiga has one of
these, the Tivo is like that too, and a bunch of embedded things). The
net result is that you have to pump the data fifo using a byteswapping
accessor and you cannot use DMA unless you DMA controller can re-swap
the other way around.... But lots of HW people still don't get it :)
> However the existance of these PCI bridges these days? I haven't seen
> one in years, and when I have nobody has ever enabled the magic swappy
> thing as it's unreliable and can't always tell how you present the data.
>
> One wishes that there was a ntoh and hton style macro in standard use
> for PCI access.. hang on though that jsut wouldn't work would it.
Nah. We have the basic rule that readl/writel are little endian. PowerPC
additionally provides arch specific low level in_{be,le}32 type
accessors with explicit endianness. Or you can also use
cpu_to_le32/le32_to_cpu kind of macros to convert between native and
explicit endianness.
Ben.
^ permalink raw reply
* Re: Fw: [PATCH] Remove powerpc specific parts of 3c509 driver
From: Benjamin Herrenschmidt @ 2006-09-19 23:18 UTC (permalink / raw)
To: Linas Vepstas; +Cc: akpm, Stephen Rothwell, netdev, jgarzik, ppc-dev
In-Reply-To: <20060919184243.GL29167@austin.ibm.com>
> However, I presume someone added the __powerpc__ define here
> because they picked up a 3c509 at a garage sale, stuck it in
> a powerpc, found out it didn't work due to a byte-swapping bug,
> and then patched it as above. I'm disturbed that somehow
> outsl_ns() became identical to outsl() at some point, presumably
> breaking this patch.
The problem is that somebody had the "bright" idea to implement ppc
outsl as byteswapping a loooong time ago. This was totally bogus and got
fixed, but it looks like this driver holds a remain of that period.
Ben.
^ permalink raw reply
* Re: [POWERPC] convert string i/o operations to C
From: Benjamin Herrenschmidt @ 2006-09-19 23:17 UTC (permalink / raw)
To: Matt Sealey; +Cc: sfr, paulus, linuxppc-dev
In-Reply-To: <45104304.3000205@genesi-usa.com>
> But it couldn't hurt, right? There has to be an application note per-CPU
> on the correct sequence of operations for such an access (I seem to have
> collected a directory full for firmware development), it seems a little
> odd to pick and choose one instruction over another for one thing, and
> then say you need to do it to support the 601 of all things, and run
> this code against the G3/G4/G5 which perhaps doesn't care or is more
> intelligent about it (or is guaranteed to have a more intelligent host
> bridge at least).
>
> Maybe I'm talking crap, please say so :D
No, your basic idea is valid. One of the thing we are tackling doing is
to add to our cpu feature "nop'ing out" mecanism something closer to
what x86 does nowadays which is to have alternate sequence of
instructions depending on the CPU.
I just think however that the MMIO loads aren't a very high priority
spot for such an optimisation :) Spinlocks are more interesting and we
are looking into doing something like x86 where you don't need to build
a UP kernel, the SMP kernel will automatically replace the spinlocks
with simple preempt_disable/enable at boot if running on only one CPU.
We could do similar things with some barriers etc...
Ben.
^ permalink raw reply
* Re: [POWERPC] convert string i/o operations to C
From: Benjamin Herrenschmidt @ 2006-09-19 23:14 UTC (permalink / raw)
To: Matt Sealey; +Cc: sfr, paulus, linuxppc-dev
In-Reply-To: <45103DF0.9050409@genesi-usa.com>
On Tue, 2006-09-19 at 20:58 +0200, Matt Sealey wrote:
> Shouldn't this stuff be optimized out depending on what processor you're
> ACTUALLY running?
>
> For a generic "powerpc" kernel it can be understood, but when you
> consider that on 970/POWER4 and above they use lwsync instead of sync
> (google for them and see the mailing list posts :), just to breathe back
> some performance in spinlocks and so on, surely this can be rejigged so
> that processors don't do more work than necessary..? Even a noop takes
> time doesn't it?
The architecture is precise enough there. It should be twi,isync. The
goal of the isync is to make sure no subsequent instruction can execute
until the previous conditional branch has been fully resolved, which
implies loading the dependent data. An isync is the only instruction in
this specific case that is guaranteed to provide what we need by the
architecture.
Ben
^ permalink raw reply
* Re: [POWERPC] convert string i/o operations to C
From: Benjamin Herrenschmidt @ 2006-09-19 23:12 UTC (permalink / raw)
To: Linas Vepstas; +Cc: Stephen Rothwell, paulus, ppc-dev
In-Reply-To: <20060919182953.GK29167@austin.ibm.com>
On Tue, 2006-09-19 at 13:29 -0500, Linas Vepstas wrote:
> On Tue, Sep 19, 2006 at 10:23:51PM +1000, Stephen Rothwell wrote:
>
> Perhaps this:
>
> > +void _insb(volatile u8 __iomem *port, void *buf, int ns)
> > +{
> > + asm volatile("sync");
> > + if (ns <= 0)
> > + return;
> > + asm volatile(
>
> should be this:
>
> > +void _insb(volatile u8 __iomem *port, void *buf, int ns)
> > +{
> > + if (ns <= 0)
> > + return;
> > + asm volatile("sync");
> > + asm volatile(
No. The barrier should be there even if somebody is lame enough to give
a count of 0.
> to assuage David Howell's concern. Not that ns should be negative
> in the first place ... but what the hey.
>
> Re Davids other comment: "ns" stands for "no byte-swap", and
> "s" stands for "string" so "insl_ns" is input string of longs,
> no byte swap.
David's comment was about the "ns" argument which is the count (dunno
why "ns").
> Here's a question:
>
> > + asm volatile(
> > + "mtctr %2\n"
> > + "subi %1,%1,1\n"
> > + "0: lbz %2,0(%0)\n"
> > + "eieio\n"
> > + "stbu %2,1(%1)\n"
> > + "bdnz 0b\n"
> > + "twi 0,%2,0\n"
>
> What does this twi do? According to my powerpc docs, this would be a
> no-op. Does this have some magic synchronizing powers on certain
> implementations? If so, there should be at least a comment card added
> about why the twi is there. (This special ability of twi might be
> well-known to some, but still, this is not immediately obvious,
> and not immedately documented in e.g. the PEM.)
>
> --linas
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
^ permalink raw reply
* Re: [POWERPC] convert string i/o operations to C
From: Stephen Rothwell @ 2006-09-19 23:07 UTC (permalink / raw)
To: David Howells; +Cc: ppc-dev, paulus
In-Reply-To: <28673.1158669778@warthog.cambridge.redhat.com>
On Tue, 19 Sep 2006 13:42:58 +0100 David Howells <dhowells@redhat.com> wrote:
>
> Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> > +void _insb(volatile u8 __iomem *port, void *buf, int ns)
> > +{
> > + asm volatile("sync");
> > + if (ns <= 0)
> > + return;
>
> What is "ns" meant to do? It seems to degrade the _insb() op to just a sync,
> but is that correct?
"ns" is the count (maybe I should rename the parameter, but that is what
was in the prototype in io.h). The "sync" is in the same place as it was
in the assembler code that this replaces.
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox