* Re: [PATCH 12/18] Uartlite: Let the console be initialized earlier
From: Grant Likely @ 2007-09-28 20:01 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: linuxppc-dev
In-Reply-To: <200709282140.07000.arnd@arndb.de>
On 9/28/07, Arnd Bergmann <arnd@arndb.de> wrote:
> On Friday 28 September 2007, Grant Likely wrote:
> > +#else /* CONFIG_OF */
> > +static void __init ulite_console_of_find_device(int id) { /* do nothing */ }
> > +#endif /* CONFIG_OF */
>
> Shouldn't this be inline? It shouldn't matter much since most of the time
> gcc -funit-at-a-time takes care of this, but it's common to make the inlining
> explicit.
heh, I even had the inline in there on an earlier version of the
patch. I can add it back it.
g.
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195
^ permalink raw reply
* Re: [PATCH 00/18] Virtex support in arch/powerpc
From: Arnd Bergmann @ 2007-09-28 19:46 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <20070928181421.18608.74224.stgit@trillian.cg.shawcable.net>
Hi Grant!
On Friday 28 September 2007, Grant Likely wrote:
> This series adds Xilinx Virtex support to arch/powerpc. =A0Please review
> and comment. =A0It includes support for the uartlite and SystemACE devices
Very nice set of patches, I looked at all of them but couldn't find much
to complain about, just trivial details.
Arnd <><
^ permalink raw reply
* Re: [PATCH 10/18] Uartlite: improve in-code comments
From: Grant Likely @ 2007-09-28 20:02 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: linuxppc-dev
In-Reply-To: <200709282143.16815.arnd@arndb.de>
On 9/28/07, Arnd Bergmann <arnd@arndb.de> wrote:
> On Friday 28 September 2007, Grant Likely wrote:
> > +/* ---------------------------------------------------------------------
> > + * Core UART driver operations
> > + */
> > +
>
> This is a rather unusual style of commenting. IMHO it would be better if you
> left out the ----- line.
I find the horizontal breaks useful when parsing through the code. If
others agree with you, then I'll happily remove them.
g.
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195
^ permalink raw reply
* Re: [PATCH 01/18] Virtex: Add uartlite bootwrapper driver
From: Grant Likely @ 2007-09-28 20:04 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: linuxppc-dev
In-Reply-To: <200709282145.09411.arnd@arndb.de>
On 9/28/07, Arnd Bergmann <arnd@arndb.de> wrote:
> On Friday 28 September 2007, Grant Likely wrote:
> > +static void uartlite_putc(unsigned char c)
> > +{
> > +while ((in_be32(reg_base + 0x8) & 0x08) != 0); /* spin */
> > +out_be32(reg_base + 0x4, c);
> > +}
>
> When coding a spin-loop, it's better to do a cpu_relax() between
> each attempt.
Is cpu_relax even implemented in the bootwrapper?
g.
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195
^ permalink raw reply
* Re: [PATCH 00/18] Virtex support in arch/powerpc
From: Grant Likely @ 2007-09-28 20:05 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: linuxppc-dev
In-Reply-To: <200709282146.14944.arnd@arndb.de>
On 9/28/07, Arnd Bergmann <arnd@arndb.de> wrote:
> Hi Grant!
>
> On Friday 28 September 2007, Grant Likely wrote:
> > This series adds Xilinx Virtex support to arch/powerpc. Please review
> > and comment. It includes support for the uartlite and SystemACE devices
>
> Very nice set of patches, I looked at all of them but couldn't find much
> to complain about, just trivial details.
Thank you very much for the review!
Cheers,
g.
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195
^ permalink raw reply
* Re: [PATCH] cpm: Describe multi-user ram in its own device node.
From: Vitaly Bordug @ 2007-09-28 20:06 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev
In-Reply-To: <20070928190616.GB20213@loki.buserror.net>
Hello Scott,
Looks good, only one note:
On Fri, 28 Sep 2007 14:06:16 -0500
Scott Wood wrote:
> + im_dprambase = cpm2_immr->im_dprambase;
> +
> /* Attach the usable dpmem area */
> /* XXX: This is actually crap. CPM_DATAONLY_BASE and
> * CPM_DATAONLY_SIZE is only a subset of the available dpram. It
> * varies with the processor and the microcode patches activated.
> * But the following should be at least safe.
> */
> - rh_attach_region(&cpm_dpmem_info, 0, r.end - r.start + 1);
> + rh_attach_region(&cpm_dpmem_info, CPM_MAP_ADDR + CPM_DATAONLY_BASE,
> + CPM_DATAONLY_SIZE);
> }
>
Can we have something to address upper comment? I mean,any way to have dpram beginning and size encoded in the device tree? We seem to be adding new bus, and still pulling the information from the defines. Maybe I miss something
here, but it looks a bit odd.
--
Sincerely, Vitaly
^ permalink raw reply
* Re: [PATCH] cpm: Describe multi-user ram in its own device node.
From: Scott Wood @ 2007-09-28 20:10 UTC (permalink / raw)
To: Vitaly Bordug; +Cc: linuxppc-dev
In-Reply-To: <20070929000621.2d332e86@kernel.crashing.org>
Vitaly Bordug wrote:
> Hello Scott,
>
> Looks good, only one note:
>
> On Fri, 28 Sep 2007 14:06:16 -0500
> Scott Wood wrote:
>
>> + im_dprambase = cpm2_immr->im_dprambase;
>> +
>> /* Attach the usable dpmem area */
>> /* XXX: This is actually crap. CPM_DATAONLY_BASE and
>> * CPM_DATAONLY_SIZE is only a subset of the available dpram. It
>> * varies with the processor and the microcode patches activated.
>> * But the following should be at least safe.
>> */
>> - rh_attach_region(&cpm_dpmem_info, 0, r.end - r.start + 1);
>> + rh_attach_region(&cpm_dpmem_info, CPM_MAP_ADDR + CPM_DATAONLY_BASE,
>> + CPM_DATAONLY_SIZE);
>> }
>>
>
> Can we have something to address upper comment? I mean,any way to
> have dpram beginning and size encoded in the device tree? We seem to
> be adding new bus, and still pulling the information from the
> defines. Maybe I miss something here, but it looks a bit odd.
This bit is #ifndef CONFIG_PPC_CPM_NEW_BINDING (and can come out once
all arch/powerpc boards are converted and tested -- I think it's just
mpc866ads and CPM mpc85xx left to go). The new code in
arch/powerpc/sysdev/cpm_common.c does get it from the device tree.
-Scott
^ permalink raw reply
* Re: [PATCH 03/18] Virtex: add xilinx interrupt controller driver
From: Olof Johansson @ 2007-09-28 20:17 UTC (permalink / raw)
To: Grant Likely; +Cc: linuxppc-dev
In-Reply-To: <20070928181606.18608.30412.stgit@trillian.cg.shawcable.net>
On Fri, Sep 28, 2007 at 12:16:07PM -0600, Grant Likely wrote:
> +/*
> + * INTC Registers
> + */
> +#define ISR 0 /* Interrupt Status */
> +#define IPR 4 /* Interrupt Pending */
> +#define IER 8 /* Interrupt Enable */
> +#define IAR 12 /* Interrupt Acknowledge */
> +#define SIE 16 /* Set Interrupt Enable bits */
> +#define CIE 20 /* Clear Interrupt Enable bits */
> +#define IVR 24 /* Interrupt Vector */
> +#define MER 28 /* Master Enable */
The defines are fairly generic, I guess you haven't ran across cases
where there's naming conflicts, but you might want to prefix them with
something more unique just in case.
> +static struct irq_host *master_irqhost;
> +
> +/*
> + * IRQ Chip operations
> + */
> +static void xilinx_intc_mask(unsigned int virq)
> +{
> + int irq = irq_map[virq].hwirq;
> + void * regs = get_irq_chip_data(virq);
> + pr_debug("mask: %d\n", irq);
> + out_be32(regs + CIE, 1 << irq);
> +}
> +
> +static void xilinx_intc_unmask(unsigned int virq)
> +{
> + int irq = irq_map[virq].hwirq;
> + void * regs = get_irq_chip_data(virq);
> + pr_debug("unmask: %d\n", irq);
> + out_be32(regs + SIE, 1 << irq);
> +}
> +
> +static void xilinx_intc_ack(unsigned int virq)
> +{
> + int irq = irq_map[virq].hwirq;
> + void * regs = get_irq_chip_data(virq);
> + pr_debug("ack: %d\n", irq);
> + out_be32(regs + IAR, 1 << irq);
> +}
I guess some of the above are open-coded instead of using virq_to_hw()
for performance reasons, it could be useful to have comments regarding
this so they aren't changed by some janitor down the road. Or, in case
they're not performance-critical, change them to use virq_to_hw.
-Olof
^ permalink raw reply
* Re: [PATCH 1/2] qemu platform, v2
From: Rob Landley @ 2007-09-28 20:14 UTC (permalink / raw)
To: Segher Boessenkool
Cc: linuxppc-dev, Paul Mackerras, Christoph Hellwig, Milton Miller,
David Gibson
In-Reply-To: <4d27912d86d1373ca01d43bd296e237b@kernel.crashing.org>
On Friday 28 September 2007 11:53:28 am Segher Boessenkool wrote:
> >> I'd be following this more closely if compiling a device tree didn't
> >> currently
> >> require an external utility (dtc or some such) that doesn't come with
> >> the
> >> Linux kernel. No other target platform I've built kernels for
> >> requires such
> >> an environmental dependency.
> >
> > No? You haven't built kernels for other platforms that have external
> > dependencies such as perl, gcc, make, binutils, etc.? :)
>
> Two of the supported Linux archs cannot be built with a mainline
> compiler, even!
Strange definition of "supported"...
> And I have to install GNU sed/awk to get builds to work, too.
I extended busybox sed until it built everything I threw at it. (I even added
a "lie to autoconf" step that replies to --version to say "This is not gnu
sed 4.0" so a regex in autoconf doesn't do something stupid.)
That's how I got into busybox development in the first place...
> OTOH, it would be nice if we didn't need DTC -- it itself doesn't
> build out-of-the-box on all systems, either ;-)
I've built x86, x86-64, mips, arm, and sparc. None of them needed anything
but the seven packages I mentioned. I'm poking at adding m68k, alpha, bfin,
and powerpc, but my free time's been spoken for recently. (I'll become
interested in sh or parisc when qemu grows support for it. I'm only
interested in bfin because I was given some free blackfin hardware at OLS.)
I've even poked at running s390 under Hercules but the setup was insane enough
to throw it way the heck down on my todo list. (Step 1: download and
configure an IBM operating system image from the 1970's... Oh yeah, fills me
with enthusiasm...)
> >> (This is a problem both for hardwiring the
> >> device tree into the kernel and for building a new boot rom from the
> >> linux
> >> kernel's ppc boot wrapper that would contain such a device tree to
> >> feed to
> >> the kernel.)
> >
> > It's only really been a problem for ps3 so far, since the embedded
> > guys don't seem to have any difficulty with installing dtc. We are
> > looking at what to do for ps3 and prep, and the answer may well
> > involve bundling dtc in the kernel source (it's not too big, around
> > 3400 lines).
>
> If only a few platforms have this problem, we could instead include
> their .dtb files in the kernel source tree.
I've had it clarified that the recent qemu-ppc patches from Milton only
require dtc to build the ROM image to boot qemu with. The Linux kernel image
you build hasn't got a device tree in it (although that means it needs one
passed in from the rom image)...
Using some other patches that floated by, I did build a prep kernel a couple
of weeks ago, which qemu happily handed control off to... which then failed
to boot because it couldn't parse the incomplete residual data left over from
open hackware. The solution the ppc list recommended? Hardwire a device
tree into said linux kernel using dtc...
>
> Segher
Rob
--
"One of my most productive days was throwing away 1000 lines of code."
- Ken Thompson.
^ permalink raw reply
* Re: [PATCH 02/18] Add Kconfig macros for Xilinx Virtex support
From: Olof Johansson @ 2007-09-28 20:19 UTC (permalink / raw)
To: Grant Likely; +Cc: linuxppc-dev
In-Reply-To: <20070928181558.18608.23527.stgit@trillian.cg.shawcable.net>
On Fri, Sep 28, 2007 at 12:16:01PM -0600, Grant Likely wrote:
> From: Grant Likely <grant.likely@secretlab.ca>
>
> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
> ---
>
> arch/powerpc/platforms/40x/Kconfig | 30 +++++++++++++++++-------------
> 1 files changed, 17 insertions(+), 13 deletions(-)
>
> diff --git a/arch/powerpc/platforms/40x/Kconfig b/arch/powerpc/platforms/40x/Kconfig
> index c3dce3b..1aae0e6 100644
> --- a/arch/powerpc/platforms/40x/Kconfig
> +++ b/arch/powerpc/platforms/40x/Kconfig
> @@ -61,13 +61,14 @@ config WALNUT
> help
> This option enables support for the IBM PPC405GP evaluation board.
>
> -#config XILINX_ML300
> -# bool "Xilinx-ML300"
> -# depends on 40x
> -# default y
> -# select VIRTEX_II_PRO
> -# help
> -# This option enables support for the Xilinx ML300 evaluation board.
> +config XILINX_VIRTEX_GENERIC_BOARD
> + bool "Generic Xilinx Virtex board"
> + depends on 40x
> + default y
> + select VIRTEX_II_PRO
> + select VIRTEX_4_FX
> + help
> + This option enables generic support for Xilinx Virtex based boards.
I would appreciate a bit verboser help text here, i.e. including what
boards are considered generic. Maybe something like "...including ML403,
<x>, <y>, and other 4FX/IIPro-based boards"?
-Olof
^ permalink raw reply
* Re: [PATCH] cpm: Describe multi-user ram in its own device node.
From: Vitaly Bordug @ 2007-09-28 20:25 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev
In-Reply-To: <46FD5FCB.4010908@freescale.com>
Hello Scott,
On Fri, 28 Sep 2007 15:10:51 -0500
Scott Wood wrote:
> Vitaly Bordug wrote:
> > Hello Scott,
> >
> > Looks good, only one note:
> >
> > On Fri, 28 Sep 2007 14:06:16 -0500
> > Scott Wood wrote:
> >
> >> + im_dprambase = cpm2_immr->im_dprambase;
> >> +
> >> /* Attach the usable dpmem area */
> >> /* XXX: This is actually crap. CPM_DATAONLY_BASE and
> >> * CPM_DATAONLY_SIZE is only a subset of the available dpram. It
> >> * varies with the processor and the microcode patches activated.
> >> * But the following should be at least safe.
> >> */
> >> - rh_attach_region(&cpm_dpmem_info, 0, r.end - r.start + 1);
> >> + rh_attach_region(&cpm_dpmem_info, CPM_MAP_ADDR + CPM_DATAONLY_BASE,
> >> + CPM_DATAONLY_SIZE);
> >> }
> >>
> >
> > Can we have something to address upper comment? I mean,any way to
> > have dpram beginning and size encoded in the device tree? We seem to
> > be adding new bus, and still pulling the information from the
> > defines. Maybe I miss something here, but it looks a bit odd.
>
> This bit is #ifndef CONFIG_PPC_CPM_NEW_BINDING (and can come out once
> all arch/powerpc boards are converted and tested -- I think it's just
> mpc866ads and CPM mpc85xx left to go). The new code in
> arch/powerpc/sysdev/cpm_common.c does get it from the device tree.
>
ok, sorry for the noise. If so, I'll try to test-n-fix upper two soon. Unfortunately,
there are many 8xx in ppc, that may depend on cpm (need to check).
--
Sincerely, Vitaly
^ permalink raw reply
* Re: [PATCH 03/18] Virtex: add xilinx interrupt controller driver
From: Grant Likely @ 2007-09-28 20:26 UTC (permalink / raw)
To: Olof Johansson; +Cc: linuxppc-dev
In-Reply-To: <20070928201719.GA23749@lixom.net>
On 9/28/07, Olof Johansson <olof@lixom.net> wrote:
> On Fri, Sep 28, 2007 at 12:16:07PM -0600, Grant Likely wrote:
>
> > +/*
> > + * INTC Registers
> > + */
> > +#define ISR 0 /* Interrupt Status */
> > +#define IPR 4 /* Interrupt Pending */
> > +#define IER 8 /* Interrupt Enable */
> > +#define IAR 12 /* Interrupt Acknowledge */
> > +#define SIE 16 /* Set Interrupt Enable bits */
> > +#define CIE 20 /* Clear Interrupt Enable bits */
> > +#define IVR 24 /* Interrupt Vector */
> > +#define MER 28 /* Master Enable */
>
> The defines are fairly generic, I guess you haven't ran across cases
> where there's naming conflicts, but you might want to prefix them with
> something more unique just in case.
Will do
>
> > +static void xilinx_intc_ack(unsigned int virq)
> > +{
> > + int irq = irq_map[virq].hwirq;
> > + void * regs = get_irq_chip_data(virq);
> > + pr_debug("ack: %d\n", irq);
> > + out_be32(regs + IAR, 1 << irq);
> > +}
>
> I guess some of the above are open-coded instead of using virq_to_hw()
> for performance reasons, it could be useful to have comments regarding
> this so they aren't changed by some janitor down the road. Or, in case
> they're not performance-critical, change them to use virq_to_hw.
Or it was just that my example code from another driver wasn't using
virq_to_hw() either. I'll fix this.
g.
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195
^ permalink raw reply
* Re: [PATCH 01/18] Virtex: Add uartlite bootwrapper driver
From: Josh Boyer @ 2007-09-28 20:26 UTC (permalink / raw)
To: Grant Likely; +Cc: linuxppc-dev, Arnd Bergmann
In-Reply-To: <fa686aa40709281304p62fe5e66k99ea934a4896d876@mail.gmail.com>
On Fri, 28 Sep 2007 14:04:04 -0600
"Grant Likely" <grant.likely@secretlab.ca> wrote:
> On 9/28/07, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Friday 28 September 2007, Grant Likely wrote:
> > > +static void uartlite_putc(unsigned char c)
> > > +{
> > > +while ((in_be32(reg_base + 0x8) & 0x08) != 0); /* spin */
> > > +out_be32(reg_base + 0x4, c);
> > > +}
> >
> > When coding a spin-loop, it's better to do a cpu_relax() between
> > each attempt.
>
> Is cpu_relax even implemented in the bootwrapper?
No. And it doesn't need to be :)
josh
^ permalink raw reply
* Re: [PATCH 06/18] [POWERPC] Fix UARTLITE reg io for little-endian architectures (ie. microblaze)
From: Olof Johansson @ 2007-09-28 20:31 UTC (permalink / raw)
To: Grant Likely; +Cc: linuxppc-dev
In-Reply-To: <20070928181704.18608.40317.stgit@trillian.cg.shawcable.net>
On Fri, Sep 28, 2007 at 12:17:13PM -0600, Grant Likely wrote:
> From: Grant Likely <grant.likely@secretlab.ca>
>
> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
> Acked-by: John Williams <jwilliams@itee.uq.edu.au>
> ---
>
> arch/ppc/syslib/virtex_devices.c | 2 +-
> drivers/serial/uartlite.c | 32 ++++++++++++++++----------------
> 2 files changed, 17 insertions(+), 17 deletions(-)
>
> diff --git a/arch/ppc/syslib/virtex_devices.c b/arch/ppc/syslib/virtex_devices.c
> index ace4ec0..270ad3a 100644
> --- a/arch/ppc/syslib/virtex_devices.c
> +++ b/arch/ppc/syslib/virtex_devices.c
> @@ -28,7 +28,7 @@
> .num_resources = 2, \
> .resource = (struct resource[]) { \
> { \
> - .start = XPAR_UARTLITE_##num##_BASEADDR + 3, \
> + .start = XPAR_UARTLITE_##num##_BASEADDR, \
> .end = XPAR_UARTLITE_##num##_HIGHADDR, \
> .flags = IORESOURCE_MEM, \
> }, \
> diff --git a/drivers/serial/uartlite.c b/drivers/serial/uartlite.c
> index f5051cf..59b674a 100644
> --- a/drivers/serial/uartlite.c
> +++ b/drivers/serial/uartlite.c
> @@ -61,7 +61,7 @@ static int ulite_receive(struct uart_port *port, int stat)
> /* stats */
> if (stat & ULITE_STATUS_RXVALID) {
> port->icount.rx++;
> - ch = readb(port->membase + ULITE_RX);
> + ch = in_be32((void*)port->membase + ULITE_RX);
Hmm, I see the start changed, and you're now reading/writing a full
32-bit word instead of individual bytes. Still, looks a little fishy to
me. Wouldn't it be more appropriate to change the ULITE_RX offset to be
3 higher and still read/write bytes?
Or are the registers defined as 32-bit ones? (I don't remember, it was
so long since I touched uartlite myself. :-)
(Same for the other functions below, but the general principle applies.)
Also, I'm not sure you need to cast port->membase to void*, do you? The
math will still be right since it's declared as char *.
-Olof
^ permalink raw reply
* Re: [PATCH] cpm: Describe multi-user ram in its own device node.
From: Vitaly Bordug @ 2007-09-28 20:30 UTC (permalink / raw)
To: galak; +Cc: linuxppc-dev
In-Reply-To: <20070928190616.GB20213@loki.buserror.net>
Kumar,
Realizing this may suffer a bit from cleanest-dts flame war, but anyway I pretty much see a lot of
sense in getting this in during next merge window. Is this possible?
On Fri, 28 Sep 2007 14:06:16 -0500
Scott Wood wrote:
> The way the current CPM binding describes available multi-user (a.k.a.
> dual-ported) RAM doesn't work well when there are multiple free regions,
> and it doesn't work at all if the region doesn't begin at the start of
> the muram area (as the hardware needs to be programmed with offsets into
> this area). The latter situation can happen with SMC UARTs on CPM2, as its
> parameter RAM is relocatable, u-boot puts it at zero, and the kernel doesn't
> support moving it.
>
> It is now described with a muram node, similar to QE. The current CPM
> binding is sufficiently recent (i.e. never appeared in an official release)
> that compatibility with existing device trees is not an issue.
>
> The code supporting the new binding is shared between cpm1 and cpm2, rather
> than remain separated. QE should be able to use this code as well, once
> minor fixes are made to its device trees.
>
> Signed-off-by: Scott Wood <scottwood@freescale.com>
Acked-by: Vitaly Bordug <vitb@kernel.crashing.org>
> ---
> Documentation/powerpc/booting-without-of.txt | 40 ++++++-
> arch/powerpc/Kconfig.debug | 6 +-
> arch/powerpc/boot/cpm-serial.c | 44 +++++--
> arch/powerpc/boot/dts/ep88xc.dts | 13 ++-
> arch/powerpc/boot/dts/mpc8272ads.dts | 11 ++
> arch/powerpc/boot/dts/mpc885ads.dts | 13 ++-
> arch/powerpc/boot/dts/pq2fads.dts | 13 ++-
> arch/powerpc/sysdev/commproc.c | 11 ++-
> arch/powerpc/sysdev/cpm2_common.c | 36 ++----
> arch/powerpc/sysdev/cpm_common.c | 159 ++++++++++++++++++++++++++
> drivers/serial/cpm_uart/cpm_uart_cpm2.c | 4 +-
> include/asm-powerpc/commproc.h | 12 ++
> include/asm-powerpc/cpm.h | 14 +++
> include/asm-powerpc/cpm2.h | 10 ++
> 14 files changed, 338 insertions(+), 48 deletions(-)
--
Sincerely, Vitaly
^ permalink raw reply
* Re: [PATCH 02/18] Add Kconfig macros for Xilinx Virtex support
From: Grant Likely @ 2007-09-28 20:39 UTC (permalink / raw)
To: Olof Johansson; +Cc: linuxppc-dev
In-Reply-To: <20070928201946.GB23749@lixom.net>
On 9/28/07, Olof Johansson <olof@lixom.net> wrote:
> On Fri, Sep 28, 2007 at 12:16:01PM -0600, Grant Likely wrote:
> > From: Grant Likely <grant.likely@secretlab.ca>
> >
> > Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
> > ---
> >
> > arch/powerpc/platforms/40x/Kconfig | 30 +++++++++++++++++-------------
> > 1 files changed, 17 insertions(+), 13 deletions(-)
> >
> > diff --git a/arch/powerpc/platforms/40x/Kconfig b/arch/powerpc/platforms/40x/Kconfig
> > index c3dce3b..1aae0e6 100644
> > --- a/arch/powerpc/platforms/40x/Kconfig
> > +++ b/arch/powerpc/platforms/40x/Kconfig
> > @@ -61,13 +61,14 @@ config WALNUT
> > help
> > This option enables support for the IBM PPC405GP evaluation board.
> >
> > -#config XILINX_ML300
> > -# bool "Xilinx-ML300"
> > -# depends on 40x
> > -# default y
> > -# select VIRTEX_II_PRO
> > -# help
> > -# This option enables support for the Xilinx ML300 evaluation board.
> > +config XILINX_VIRTEX_GENERIC_BOARD
> > + bool "Generic Xilinx Virtex board"
> > + depends on 40x
> > + default y
> > + select VIRTEX_II_PRO
> > + select VIRTEX_4_FX
> > + help
> > + This option enables generic support for Xilinx Virtex based boards.
>
> I would appreciate a bit verboser help text here, i.e. including what
> boards are considered generic. Maybe something like "...including ML403,
> <x>, <y>, and other 4FX/IIPro-based boards"?
Done.
This option enables generic support for Xilinx Virtex based boards.
+ The generic virtex board support matches any device tree which
+ specifies 'xilinx,virtex' in its compatible field. This includes
+ the Xilinx ML3xx and ML4xx reference designs using the powerpc
+ core.
+
+ Most Virtex designs should use this unless it needs to do some
+ special configuration at board probe time.
+
Cheers,
g.
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195
^ permalink raw reply
* Re: [PATCH 06/18] [POWERPC] Fix UARTLITE reg io for little-endian architectures (ie. microblaze)
From: Grant Likely @ 2007-09-28 20:42 UTC (permalink / raw)
To: Olof Johansson; +Cc: linuxppc-dev
In-Reply-To: <20070928203104.GC23749@lixom.net>
On 9/28/07, Olof Johansson <olof@lixom.net> wrote:
> On Fri, Sep 28, 2007 at 12:17:13PM -0600, Grant Likely wrote:
> > From: Grant Likely <grant.likely@secretlab.ca>
> >
> > Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
> > Acked-by: John Williams <jwilliams@itee.uq.edu.au>
> > ---
> >
> > arch/ppc/syslib/virtex_devices.c | 2 +-
> > drivers/serial/uartlite.c | 32 ++++++++++++++++----------------
> > 2 files changed, 17 insertions(+), 17 deletions(-)
> >
> > diff --git a/arch/ppc/syslib/virtex_devices.c b/arch/ppc/syslib/virtex_devices.c
> > index ace4ec0..270ad3a 100644
> > --- a/arch/ppc/syslib/virtex_devices.c
> > +++ b/arch/ppc/syslib/virtex_devices.c
> > @@ -28,7 +28,7 @@
> > .num_resources = 2, \
> > .resource = (struct resource[]) { \
> > { \
> > - .start = XPAR_UARTLITE_##num##_BASEADDR + 3, \
> > + .start = XPAR_UARTLITE_##num##_BASEADDR, \
> > .end = XPAR_UARTLITE_##num##_HIGHADDR, \
> > .flags = IORESOURCE_MEM, \
> > }, \
> > diff --git a/drivers/serial/uartlite.c b/drivers/serial/uartlite.c
> > index f5051cf..59b674a 100644
> > --- a/drivers/serial/uartlite.c
> > +++ b/drivers/serial/uartlite.c
> > @@ -61,7 +61,7 @@ static int ulite_receive(struct uart_port *port, int stat)
> > /* stats */
> > if (stat & ULITE_STATUS_RXVALID) {
> > port->icount.rx++;
> > - ch = readb(port->membase + ULITE_RX);
> > + ch = in_be32((void*)port->membase + ULITE_RX);
>
> Hmm, I see the start changed, and you're now reading/writing a full
> 32-bit word instead of individual bytes. Still, looks a little fishy to
> me. Wouldn't it be more appropriate to change the ULITE_RX offset to be
> 3 higher and still read/write bytes?
>
> Or are the registers defined as 32-bit ones? (I don't remember, it was
> so long since I touched uartlite myself. :-)
All the registers are defined as 32 bit ones. I think it makes more
sense to access the registers as they are documented, and it
eliminates the 'magic' +3 needed to make it work now.
>
> (Same for the other functions below, but the general principle applies.)
>
> Also, I'm not sure you need to cast port->membase to void*, do you? The
> math will still be right since it's declared as char *.
membase is now defined as u32*, so the cast is needed.
Cheers,
g.
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195
^ permalink raw reply
* Re: [PATCH 06/18] [POWERPC] Fix UARTLITE reg io for little-endian architectures (ie. microblaze)
From: Olof Johansson @ 2007-09-28 20:47 UTC (permalink / raw)
To: Grant Likely; +Cc: linuxppc-dev
In-Reply-To: <fa686aa40709281342h4f8ff3fdv265e8098d2da6f52@mail.gmail.com>
On Fri, Sep 28, 2007 at 02:42:32PM -0600, Grant Likely wrote:
> On 9/28/07, Olof Johansson <olof@lixom.net> wrote:
>
> > Hmm, I see the start changed, and you're now reading/writing a full
> > 32-bit word instead of individual bytes. Still, looks a little fishy to
> > me. Wouldn't it be more appropriate to change the ULITE_RX offset to be
> > 3 higher and still read/write bytes?
> >
> > Or are the registers defined as 32-bit ones? (I don't remember, it was
> > so long since I touched uartlite myself. :-)
>
> All the registers are defined as 32 bit ones. I think it makes more
> sense to access the registers as they are documented, and it
> eliminates the 'magic' +3 needed to make it work now.
Ok, thaks for the clarification. Feel free to add it as motivation in
the patch description. :)
> > (Same for the other functions below, but the general principle applies.)
> >
> > Also, I'm not sure you need to cast port->membase to void*, do you? The
> > math will still be right since it's declared as char *.
>
> membase is now defined as u32*, so the cast is needed.
Hm, I must have looked at a stale tree.
Thanks,
-Olof
^ permalink raw reply
* Re: [PATCH 06/18] [POWERPC] Fix UARTLITE reg io for little-endian architectures (ie. microblaze)
From: Grant Likely @ 2007-09-28 20:50 UTC (permalink / raw)
To: Olof Johansson; +Cc: linuxppc-dev
In-Reply-To: <20070928204749.GD23749@lixom.net>
On 9/28/07, Olof Johansson <olof@lixom.net> wrote:
> On Fri, Sep 28, 2007 at 02:42:32PM -0600, Grant Likely wrote:
> > On 9/28/07, Olof Johansson <olof@lixom.net> wrote:
> > > Also, I'm not sure you need to cast port->membase to void*, do you? The
> > > math will still be right since it's declared as char *.
> >
> > membase is now defined as u32*, so the cast is needed.
>
> Hm, I must have looked at a stale tree.
No, wait. You're right. It is a char*. I'll drop the cast.
g.
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195
^ permalink raw reply
* Re: [PATCH 06/18] [POWERPC] Fix UARTLITE reg io for little-endian architectures (ie. microblaze)
From: Grant Likely @ 2007-09-28 20:52 UTC (permalink / raw)
To: Olof Johansson; +Cc: linuxppc-dev
In-Reply-To: <fa686aa40709281350y63475e93hd18e7da497c6f1b0@mail.gmail.com>
On 9/28/07, Grant Likely <grant.likely@secretlab.ca> wrote:
> On 9/28/07, Olof Johansson <olof@lixom.net> wrote:
> > On Fri, Sep 28, 2007 at 02:42:32PM -0600, Grant Likely wrote:
> > > On 9/28/07, Olof Johansson <olof@lixom.net> wrote:
> > > > Also, I'm not sure you need to cast port->membase to void*, do you? The
> > > > math will still be right since it's declared as char *.
> > >
> > > membase is now defined as u32*, so the cast is needed.
> >
> > Hm, I must have looked at a stale tree.
>
> No, wait. You're right. It is a char*. I'll drop the cast.
Wait, I'm wrong again... it's in/out_be32 that expects an (unsigned*).
The compiler complains without the cast.
g.
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195
^ permalink raw reply
* Re: Flash paritioning and JFFS2
From: Mirek23 @ 2007-09-28 21:09 UTC (permalink / raw)
To: linuxppc-embedded
In-Reply-To: <12937526.post@talk.nabble.com>
I when through the jffs2 driver code and I have noticed that during the jffs2
initialization It is a problem to access the flash memory (of my board) in
32 bit mode. All what refers to the 32 bit access gets from Flash only 16
(Most significant) bits. The remaining 16 (less significant) bits are simply
filled with 0.
I have noticed that there are two sets of functions which are used to access
Flash memory:
#define cpu_to_je32(x) ((jint32_t){x})
#define je32_to_cpu(x) ((x).v32)
#define cpu_to_je16(x) ((jint16_t){x})
#define je16_to_cpu(x) ((x).v16)
These functions which refer to 16 bits access are working fine in my case
but those which refer to 32 access do not work properly. Does anybody have
some idea how to change those functions which deal with 32 bits to force
them to access the Flash memory in two steps to get twice 16 bits numbers
and at the end form 32 bits value?
Best Regards
Mirek
Mirek23 wrote:
>
> Dear All,
>
> Finally I have sorted out few problems with linux and u-boot and I have
> tried to come back to the jffs2 and mtd partitioning issue.
>
> I am using CFI driver and I pass partitioning information from command
> line via u-boot.
>
> This part works fine:
>
> Unfortunatly I have the problem when accesing the flash partitions because
> my Avnet evaluation board is designet in such a way that the data should
> be accessed in the mode x16 (16 bits in each goal).
>
> Linux CFI driver is able to detect the x16 mode but it does not handle it
> correctly.
>
> I will give you an example:
>
> My mtd1 partition contains the u-boot environment. When I print the mtd1
> contents with (busybox) hexdump I get:
>
> 00000000 eb 92 00 00 62 6f 00 00 61 72 00 00 3d 63 00 00
> |....bo..ar..=c..|
> 00000010 73 6f 00 00 3d 74 00 00 55 4c 00 00 39 36 00 00
> |so..=t..UL..96..|
> 00000020 20 72 00 00 74 3d 00 00 65 76 00 00 66 73 00 00 |
> r..t=..ev..fs..|
> 00000030 77 20 00 00 73 72 00 00 74 3d 00 00 39 2e 00 00 |w
> ..sr..t=..9...|
> 00000040 39 2e 00 00 34 2e 00 00 33 3a 00 00 70 74 00 00
> |9...4...3:..pt..|
> 00000050 6c 64 00 00 31 2f 00 00 63 5f 00 00 78 2c 00 00
> |ld..1/..c_..x,..|
> 00000060 70 20 00 00 70 3d 00 00 3a 3a 00 00 72 74 00 00 |p
> ..p=..::..rt..|
> 00000070 34 2d 00 00 72 65 00 00 65 74 00 00 3a 64 00 00
> |4-..re..et..:d..|
>
> When I copy the contents of the /dev/mtd1 to the file by 2 bytes (bs=2) I
> get correct
> readout:
>
> dd if=/dev/mtd1 of=./u-boot_Env.txt bs=2 count=128k
>
> hexdump ./u-boot_Env.txt
> 00000000 eb 92 c9 6d 62 6f 6f 74 61 72 67 73 3d 63 6f 6e
> |...mbootargs=con|
> 00000010 73 6f 6c 65 3d 74 74 79 55 4c 30 2c 39 36 30 30
> |sole=ttyUL0,9600|
> 00000020 20 72 6f 6f 74 3d 2f 64 65 76 2f 6e 66 73 20 72 |
> root=/dev/nfs r|
> 00000030 77 20 6e 66 73 72 6f 6f 74 3d 31 32 39 2e 31 32 |w
> nfsroot=129.12|
> 00000040 39 2e 31 34 34 2e 31 31 33 3a 2f 6f 70 74 2f 65
> |9.144.113:/opt/e|
> 00000050 6c 64 6b 34 31 2f 70 70 63 5f 34 78 78 2c 74 63
> |ldk41/ppc_4xx,tc|
>
> The real problems begin when I want to deal with jffs2 file system.
>
> I have mtd3 partition which is intended to hold jffs2 fs.
> To create the jffs2 fs I do:
>
> ./mkfs.jffs2 --pad=0xA0000 --eraseblock=0x20000 --root=/tmp/bin/mtd1/work/
> --output=image2.jffs2
> flash_erase /dev/mtd3
> dd if=./image2.jffs2 of=/dev/mtd3 bs=2 count=655360
>
> After that I have jffs2 fs under /dev/mtd3.
>
> When I try to mount the jffs2 partition fun begins:
>
> mount -t jffs2 /dev/mtdblock3 /mnt
>
> [ 2828.462121] jffs2_scan_eraseblock(): Node at 0x00000000 {0x1985,
> 0x2003, 0x00000000) has invalid CRC 0xf0600000 (calculated 0xf9d690b3)
> [ 2828.610485] jffs2_scan_eraseblock(): Magic bitmask 0x1985 not found at
> 0x00000008: 0xf060 instead
> [ 2828.719326] jffs2_scan_eraseblock(): Node at 0x0000000c {0x1985,
> 0xe001, 0x00000000) has invalid CRC 0xc3200000 (calculated 0x92fedd67)
> [ 2828.870047] jffs2_scan_eraseblock(): Magic bitmask 0x1985 not found at
> 0x00000014: 0xc320 instead
> [ 2828.979854] jffs2_scan_eraseblock(): Magic bitmask 0x1985 not found at
> 0x00000024: 0x46fc instead
>
> .
> .
> .
> .
>
> [ 2834.858103] Further such events for this erase block will not be
> printed
> [ 2835.029913] Cowardly refusing to erase blocks on filesystem with no
> valid JFFS2 nodes
> [ 2835.124378] empty_blocks 0, bad_blocks 0, c->nr_blocks 5
> mount: mounting /dev/mtdblock3 on /mnt failed
>
>
> I have examined carefully the above output and what I see is that linux
> does not properly access the flash memory:
>
> I did the hexdump on the image2.jffs2 (which is placed in /dev/mtd3) and
> compared it with the jffs2 mount messages:
>
> hexdump -C image2.jffs2
>
> 00000000 19 85 20 03 00 00 00 0c f0 60 dc 98 19 85 e0 01 |..
> ......`......|
> 00000010 00 00 00 31 c3 20 dd 5d 00 00 00 01 00 00 00 00 |...1.
> .]........|
>
> jffs2_scan_eraseblock(): Node at 0x00000000 {0x1985, 0x2003, 0x00000000)
> has invalid CRC 0xf0600000 (calculated 0xf9d690b3)
>
> The linux mount complains about wrong CRC (invalid CRC 0xf0600000) but in
> the memory it is
> f0 60 dc 98 (so it means that it was read in byte by byte mode but not in
> x16 (2 bytes) mode).
>
> Does somebody know how to fix this problem?
>
> Best Regards
>
> Mirek
>
>
>
>
>
>
>
>
>
>
--
View this message in context: http://www.nabble.com/Flash-paritioning-and-JFFS2-tf4317566.html#a12948521
Sent from the linuxppc-embedded mailing list archive at Nabble.com.
^ permalink raw reply
* Powerbook shuts down hard when hot, patch found
From: Michael Buesch @ 2007-09-28 21:32 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev
Hi,
some time ago I already mailed you about this problem.
I will quickly describe what's going on, again:
My powerbook boots fine when it's cold. You can work with
it and you can also run it hot (compile something, etc...).
But when you try to boot it while it is hot, it will
automatically shutdown hard inside of the boot process.
Some part of the hardware is shutting it down.
This does not happen when it's getting hot _after_ boot.
Only while boot when it's hot.
I bisected the problem and found out that the following patch
is responsible for this. Yeah, this sounds a little bit
crazy, as this patch does not seem to be related to
this at all. But I confirmed it by booting kernel
4f71c5de19c27f2198105d3b26b398494d5c353b
That kernel triggered the bug. Then I patch -R the patch below
and it booted properly, even when hot. I also rechecked
that it really was hot enough to trigger the event by
immediately rebooting into a known bad kernel, that immediately
shut down the pbook, as expected.
So I'm pretty sure it's the patch below causing this weird
behaviour. Any idea why?
commit 4f71c5de19c27f2198105d3b26b398494d5c353b
Author: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: Fri Nov 17 15:35:00 2006 +1100
[PATCH] Fix radeon DDC regression
When radeonfb was changed to use the new "generic" ddc, a bit of
code initializing the GPIO lines was lost, causing it to not work
if the firmware didn't configure them properly, which seems to
happen on some cards.
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>
diff --git a/drivers/video/aty/radeon_i2c.c b/drivers/video/aty/radeon_i2c.c
index 6767545..869725a 100644
--- a/drivers/video/aty/radeon_i2c.c
+++ b/drivers/video/aty/radeon_i2c.c
@@ -139,7 +139,13 @@ void radeon_delete_i2c_busses(struct rad
int radeon_probe_i2c_connector(struct radeonfb_info *rinfo, int conn,
u8 **out_edid)
{
- u8 *edid = fb_ddc_read(&rinfo->i2c[conn-1].adapter);
+ u32 reg = rinfo->i2c[conn-1].ddc_reg;
+ u8 *edid;
+
+ OUTREG(reg, INREG(reg) &
+ ~(VGA_DDC_DATA_OUTPUT | VGA_DDC_CLK_OUTPUT));
+
+ edid = fb_ddc_read(&rinfo->i2c[conn-1].adapter);
if (out_edid)
*out_edid = edid;
--
Greetings Michael.
^ permalink raw reply related
* Re: Powerbook shuts down hard when hot, patch found
From: Benjamin Herrenschmidt @ 2007-09-28 23:04 UTC (permalink / raw)
To: Michael Buesch; +Cc: linuxppc-dev
In-Reply-To: <200709282332.52819.mb@bu3sch.de>
On Fri, 2007-09-28 at 23:32 +0200, Michael Buesch wrote:
> Hi,
>
> some time ago I already mailed you about this problem.
> I will quickly describe what's going on, again:
>
> My powerbook boots fine when it's cold. You can work with
> it and you can also run it hot (compile something, etc...).
> But when you try to boot it while it is hot, it will
> automatically shutdown hard inside of the boot process.
> Some part of the hardware is shutting it down.
> This does not happen when it's getting hot _after_ boot.
> Only while boot when it's hot.
>
> I bisected the problem and found out that the following patch
> is responsible for this. Yeah, this sounds a little bit
> crazy, as this patch does not seem to be related to
> this at all. But I confirmed it by booting kernel
> 4f71c5de19c27f2198105d3b26b398494d5c353b
> That kernel triggered the bug. Then I patch -R the patch below
> and it booted properly, even when hot. I also rechecked
> that it really was hot enough to trigger the event by
> immediately rebooting into a known bad kernel, that immediately
> shut down the pbook, as expected.
> So I'm pretty sure it's the patch below causing this weird
> behaviour. Any idea why?
This is very strange... Can you try also clearing VGA_DDC_CLK_OUT_EN and
VGA_DDC_DATA_OUT_EN and the same time and see if that helps ?
>
> commit 4f71c5de19c27f2198105d3b26b398494d5c353b
> Author: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Date: Fri Nov 17 15:35:00 2006 +1100
>
> [PATCH] Fix radeon DDC regression
>
> When radeonfb was changed to use the new "generic" ddc, a bit of
> code initializing the GPIO lines was lost, causing it to not work
> if the firmware didn't configure them properly, which seems to
> happen on some cards.
>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Signed-off-by: Linus Torvalds <torvalds@osdl.org>
>
> diff --git a/drivers/video/aty/radeon_i2c.c b/drivers/video/aty/radeon_i2c.c
> index 6767545..869725a 100644
> --- a/drivers/video/aty/radeon_i2c.c
> +++ b/drivers/video/aty/radeon_i2c.c
> @@ -139,7 +139,13 @@ void radeon_delete_i2c_busses(struct rad
> int radeon_probe_i2c_connector(struct radeonfb_info *rinfo, int conn,
> u8 **out_edid)
> {
> - u8 *edid = fb_ddc_read(&rinfo->i2c[conn-1].adapter);
> + u32 reg = rinfo->i2c[conn-1].ddc_reg;
> + u8 *edid;
> +
> + OUTREG(reg, INREG(reg) &
> + ~(VGA_DDC_DATA_OUTPUT | VGA_DDC_CLK_OUTPUT));
> +
> + edid = fb_ddc_read(&rinfo->i2c[conn-1].adapter);
>
> if (out_edid)
> *out_edid = edid;
>
^ permalink raw reply
* Re: [PATCH 01/18] Virtex: Add uartlite bootwrapper driver
From: Arnd Bergmann @ 2007-09-28 23:31 UTC (permalink / raw)
To: Josh Boyer; +Cc: linuxppc-dev
In-Reply-To: <20070928152651.2e185d15@weaponx.rchland.ibm.com>
On Friday 28 September 2007, Josh Boyer wrote:
>=20
> > Is cpu_relax even implemented in the bootwrapper?
>=20
> No. =A0And it doesn't need to be :)
I think I should learn to read subject lines. You are of course
both right.
Arnd <><
^ permalink raw reply
* Re: [PATCH] cpm: Describe multi-user ram in its own device node.
From: Kumar Gala @ 2007-09-28 23:53 UTC (permalink / raw)
To: Vitaly Bordug; +Cc: PowerPC dev list, Timur Tabi
In-Reply-To: <20070929003005.59afc2a0@kernel.crashing.org>
On Sep 28, 2007, at 3:30 PM, Vitaly Bordug wrote:
> Kumar,
>
> Realizing this may suffer a bit from cleanest-dts flame war, but
> anyway I pretty much see a lot of
> sense in getting this in during next merge window. Is this possible?
Probably, does QE have muram? I can't keep these things straight.
- k
^ 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