* 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
* 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: [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: 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: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 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] 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 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 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 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] 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 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 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 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] 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] 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 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 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 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 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 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 01/18] Virtex: Add uartlite bootwrapper driver
From: Arnd Bergmann @ 2007-09-28 19:45 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <20070928181551.18608.88418.stgit@trillian.cg.shawcable.net>
On Friday 28 September 2007, Grant Likely wrote:
> +static void uartlite_putc(unsigned char c)
> +{
> +=A0=A0=A0=A0=A0=A0=A0while ((in_be32(reg_base + 0x8) & 0x08) !=3D 0); /*=
spin */
> +=A0=A0=A0=A0=A0=A0=A0out_be32(reg_base + 0x4, c);
> +}
When coding a spin-loop, it's better to do a cpu_relax() between
each attempt.
Arnd <><
^ permalink raw reply
* Re: [PATCH 10/18] Uartlite: improve in-code comments
From: Arnd Bergmann @ 2007-09-28 19:43 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <20070928181740.18608.67126.stgit@trillian.cg.shawcable.net>
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.
Arnd <><
^ permalink raw reply
* Re: [PATCH 12/18] Uartlite: Let the console be initialized earlier
From: Arnd Bergmann @ 2007-09-28 19:40 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <20070928181754.18608.52779.stgit@trillian.cg.shawcable.net>
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.
Arnd <><
^ permalink raw reply
* Re: [PATCH 11/18] Virtex: Port UARTLITE driver to of-platform-bus
From: Arnd Bergmann @ 2007-09-28 19:32 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <20070928181748.18608.62409.stgit@trillian.cg.shawcable.net>
T24gRnJpZGF5IDI4IFNlcHRlbWJlciAyMDA3LCBHcmFudCBMaWtlbHkgd3JvdGU6Cj4gLaCgoKCg
oKByZXQgPSB1YXJ0X3JlZ2lzdGVyX2RyaXZlcigmdWxpdGVfdWFydF9kcml2ZXIpOwo+IC2goKCg
oKCgaWYgKHJldCkKPiAtoKCgoKCgoKCgoKCgoKCgcmV0dXJuIHJldDsKPiAroKCgoKCgoHByX2Rl
YnVnKCJ1YXJ0bGl0ZTogY2FsbGluZyB1YXJ0X3JlZ2lzdGVyX2RyaXZlcigpXG4iKTsKPiAroKCg
oKCgoGlmICgocmV0ID0gdWFydF9yZWdpc3Rlcl9kcml2ZXIoJnVsaXRlX3VhcnRfZHJpdmVyKSkg
IT0gMCkKPiAroKCgoKCgoKCgoKCgoKCgZ290byBlcnJfdWFydDsKPiCgCj4gLaCgoKCgoKByZXQg
PSBwbGF0Zm9ybV9kcml2ZXJfcmVnaXN0ZXIoJnVsaXRlX3BsYXRmb3JtX2RyaXZlcik7Cj4gLaCg
oKCgoKBpZiAocmV0KQo+IC2goKCgoKCgoKCgoKCgoKB1YXJ0X3VucmVnaXN0ZXJfZHJpdmVyKCZ1
bGl0ZV91YXJ0X2RyaXZlcik7Cj4gK6CgoKCgoKBpZiAoKHJldCA9IHVsaXRlX29mX3JlZ2lzdGVy
KCkpICE9IDApCj4gK6CgoKCgoKCgoKCgoKCgoGdvdG8gZXJyX29mOwo+IKAKPiAroKCgoKCgoHBy
X2RlYnVnKCJ1YXJ0bGl0ZTogY2FsbGluZyBwbGF0Zm9ybV9kcml2ZXJfcmVnaXN0ZXIoKVxuIik7
Cj4gK6CgoKCgoKBpZiAoKHJldCA9IHBsYXRmb3JtX2RyaXZlcl9yZWdpc3RlcigmdWxpdGVfcGxh
dGZvcm1fZHJpdmVyKSkgIT0gMCkKPiAroKCgoKCgoKCgoKCgoKCgZ290byBlcnJfcGxhdDsKPiAr
Cj4gK6CgoKCgoKByZXR1cm4gMDsKCkkgZG9uJ3Qgc2VlIHRoaXMgYXMgbXVjaCBvZiBhbiBpbXBy
b3ZlbWVudC4gRG9pbmcgYW4gYXNzaWdubWVudCBpbiB0aGUKY29uZGl0aW9uIG1ha2VzIHRoZSBj
b2RlIGxlc3MgcmVhZGFibGUgSU1ITywgc28gaXQgc2hvdWxkIHJlYWxseQpiZSB3cml0dGVuIGFz
CgoJcmV0ID0gdHJ5X3NvbWV0aGluZygpOwoJaWYgKHJldCkKCQlnb3RvIHNvbWV0aGluZ19mYWls
ZWQ7CgpUaGlzIGtlZXBzIHRoZSBub3JtYWwgY29kZSB0byB0aGUgbGVmdCwgYW5kIHRoZSBlcnJv
ciBoYW5kbGluZyBpbmRlbnRlZAp0byB0aGUgcmlnaHQuCgoKCUFybmQgPD48Cg==
^ 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