* Re: [PATCH 12/60] microblaze_v4: Generic dts file for platforms
From: Michal Simek @ 2008-06-26 21:41 UTC (permalink / raw)
To: Stephen Neuendorffer
Cc: linux-arch, alan, arnd, vapier.adi, matthew, microblaze-uclinux,
linux-kernel, linuxppc-dev, will.newton, hpa, John Linn, drepper,
john.williams
In-Reply-To: <20080626201813.9662411E0051@mail20-wa4.bigfish.com>
Ok. Thanks for information Steve.
Jon: Only for sure the main difference is only in value handling. Am I right?
If yes, it is easy to change.
M
> It's on my list of things to do, but I'm swamped with a few deadlines at
> the moment.
>
> Steve
>
>> -----Original Message-----
>> From: linuxppc-dev-bounces+stephen.neuendorffer=xilinx.com@ozlabs.org
> [mailto:linuxppc-dev-
>> bounces+stephen.neuendorffer=xilinx.com@ozlabs.org] On Behalf Of
> Michal Simek
>> Sent: Thursday, June 26, 2008 11:57 AM
>> To: Jon Loeliger
>> Cc: linux-arch@vger.kernel.org; alan@lxorguk.ukuu.org.uk;
> arnd@arndb.de; vapier.adi@gmail.com;
>> matthew@wil.cx; microblaze-uclinux@itee.uq.edu.au;
> linux-kernel@vger.kernel.org; drepper@redhat.com;
>> linuxppc-dev@ozlabs.org; will.newton@gmail.com; hpa@zytor.com; John
> Linn; john.williams@petalogix.com
>> Subject: Re: [PATCH 12/60] microblaze_v4: Generic dts file for
> platforms
>> OK. We have to change fdt generator with Steve.
>>
>> Steve: can you look at it?
>>
>> M
>>
>>
>>> monstr@seznam.cz wrote:
>>>> From: Michal Simek <monstr@monstr.eu>
>>>>
>>>>
>>>> Signed-off-by: Michal Simek <monstr@monstr.eu>
>>>> ---
>>>> arch/microblaze/platform/generic/system.dts | 300
>>>> +++++++++++++++++++++++++++
>>>> 1 files changed, 300 insertions(+), 0 deletions(-)
>>>> create mode 100644 arch/microblaze/platform/generic/system.dts
>>>>
>>>> diff --git a/arch/microblaze/platform/generic/system.dts
>>>> b/arch/microblaze/platform/generic/system.dts
>>>> new file mode 100644
>>>> index 0000000..724a037
>>>> --- /dev/null
>>>> +++ b/arch/microblaze/platform/generic/system.dts
>>>> @@ -0,0 +1,300 @@
>>>> +/*
>>>> + * (C) Copyright 2007-2008 Xilinx, Inc.
>>>> + * (C) Copyright 2007-2008 Michal Simek
>>>> + *
>>>> + * Michal SIMEK <monstr@monstr.eu>
>>>> + *
>>>> + * 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.
>>>> + *
>>>> + * This program is distributed in the hope that it will be useful,
>>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>>>> + * GNU General Public License for more details.
>>>> + *
>>>> + * You should have received a copy of the GNU General Public
> License
>>>> + * along with this program; if not, write to the Free Software
>>>> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
>>>> + * MA 02111-1307 USA
>>>> + *
>>>> + * CAUTION: This file is automatically generated by libgen.
>>>> + * Version: Xilinx EDK 9.2.02 EDK_Jm_SP2.3
>>>> + * Generate by FDT v1.00.a
>>>> + */
>>>> +
>>>> +/ {
>>>> + #address-cells = <1>;
>>>> + #size-cells = <1>;
>>>> + compatible = "xlnx,microblaze";
>>>> + model = "testing";
>>>> + DDR_SDRAM_32Mx16: memory@20000000 {
>>>> + device_type = "memory";
>>>> + reg = < 20000000 2000000 >;
>>>> + } ;
>>>> + chosen {
>>>> + bootargs = "console=ttyUL0,115200 loglevel=15";
>>>> + linux,stdout-path = "/plb@0/serial@40100000";
>>>> + } ;
>>>> + cpus {
>>>> + #address-cells = <1>;
>>>> + #cpus = <1>;
>>>> + #size-cells = <0>;
>>>> + microblaze_0: cpu@0 {
>>>> + clock-frequency = <2faf080>;
>>>
>>> This should really be using the /dts-v1/ format now.
>>>
>>> Thanks,
>>> jdl
>>>
>>> _______________________________________________
>>> Linuxppc-dev mailing list
>>> Linuxppc-dev@ozlabs.org
>>> https://ozlabs.org/mailman/listinfo/linuxppc-dev
>>>
>>>
>>>
> ------------------------------------------------------------------------
>>>
>>> No virus found in this incoming message.
>>> Checked by AVG.
>>> Version: 8.0.101 / Virus Database: 270.4.1/1519 - Release Date:
> 25.6.2008 04:13
>> _______________________________________________
>> Linuxppc-dev mailing list
>> Linuxppc-dev@ozlabs.org
>> https://ozlabs.org/mailman/listinfo/linuxppc-dev
>
>
> This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.
>
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
>
>
> ------------------------------------------------------------------------
>
>
> No virus found in this incoming message.
> Checked by AVG.
> Version: 8.0.101 / Virus Database: 270.4.1/1519 - Release Date: 25.6.2008 04:13
^ permalink raw reply
* Re: [PATCH 12/60] microblaze_v4: Generic dts file for platforms
From: Jon Loeliger @ 2008-06-26 21:44 UTC (permalink / raw)
To: monstr
Cc: linux-arch, alan, arnd, vapier.adi, matthew, microblaze-uclinux,
linux-kernel, linuxppc-dev, will.newton, hpa, John Linn, drepper,
john.williams
In-Reply-To: <48640CF7.7050700@seznam.cz>
Michal Simek wrote:
> Ok. Thanks for information Steve.
>
> Jon: Only for sure the main difference is only in value handling. Am I right?
> If yes, it is easy to change.
>
> M
Right. It is essentially C-like now. And has a
declarator at the top with /dts-v1/. Essentially
all of the DTS files in arch/powerpc/boot/dts should
be V1 now. Lots of examples.
Except in Byte Arrays: x = [ CA FE BA BE FE ED D0 0D ];
jdl
^ permalink raw reply
* issue with kgdb on ppc
From: SEEMA pm @ 2008-06-26 21:53 UTC (permalink / raw)
To: linuxppc-dev
[-- Attachment #1: Type: text/plain, Size: 838 bytes --]
Hello
I'm trying to get kgdb working on a mpc8540 based target machine. I took the
latest patches which are for 2.6.13(mine is 2.6.11) and build and made
changes to config by enabling :
- Compile the kernel with debug info
- KGDB: kernel debugging with remote gdb
- KGDB: Console messages through gdb
- Method for KGDB communication (KGDB: On ethernet - in kernel)
I also added bootargs through uboot as:
kgdboe=6443@10.110.1.2/,6442@10.110.1.31/00:0F:1F:6E:F3:30 kgdbwait
Now, I'm getting the messages:
netconsole: not configured, aborting
kgdboe: eth0 doesn't support polling, aborting.
kgdboe: netpoll_setup failed kgdboe failed
And after some lines;
kgdb: Defering I/O setup to kernel module.
kgdb: Waiting for connection from remote gdb...
But then it goes to load ramdisk.
Can someone please provide some help on this
thanks
[-- Attachment #2: Type: text/html, Size: 1037 bytes --]
^ permalink raw reply
* Re: [PATCH 48/60] microblaze_v4: headers simple files - empty or redirect to asm-generic
From: Arnd Bergmann @ 2008-06-26 22:09 UTC (permalink / raw)
To: H. Peter Anvin
Cc: linux-arch, alan, Adrian Bunk, vapier.adi, matthew,
microblaze-uclinux, linux-kernel, drepper, linuxppc-dev,
will.newton, Michal Simek, monstr, John.Linn, john.williams
In-Reply-To: <4863D87F.5060103@zytor.com>
On Thursday 26 June 2008, H. Peter Anvin wrote:
>
> The sanest way to do that would probably be something along the lines of:
>
> -> Change include/asm-xxx to arch/xxx/include/asm
Sam Ravnborg is already working on this part.
> -> Create arch/generic
> -> Make sure arch/xxx/include/asm and arch/generic/include/asm are both
> in the include path, in that order.
>
> That would also get rid of the symlink.
It requires two more steps:
- change all remaining users of #include <asm-generic/*.h> to include
that file with the new name, e.g. #include <generic/asm/foo.h> if we
have it in the right path.
- change the way 'make headers_install' works so that the files get
installed to the right location in $PREFIX/include/asm/ instead of
$PREFIX/include/asm-generic/ for each header that does not exist
in arch/foo/include/asm.
These changes are probably somewhat controversial, and the first
one is much more invasive than the patches that Sam has, so I'd
not want to do them at the same time.
> On the other hand, the redirection isn't all that bad.
Agreed. I just have the hope that someone comes up with the magical
solution for this problem that makes it a lot nicer than the suggestions
so far or the current way of doing it ;-)
Arnd <><
^ permalink raw reply
* Re: [PATCH 58/60] microblaze_v4: sys_microblaze.c
From: Arnd Bergmann @ 2008-06-26 22:34 UTC (permalink / raw)
To: monstr
Cc: linux-arch, alan, Michal Simek, vapier.adi, matthew,
microblaze-uclinux, linux-kernel, drepper, linuxppc-dev,
will.newton, hpa, John.Linn, john.williams
In-Reply-To: <4863E8F2.5080608@seznam.cz>
On Thursday 26 June 2008, Michal Simek wrote:
>
> >> +
> >> +/*
> >> + * sys_ipc() is the de-multiplexer for the SysV IPC calls..
> >> + *
> >> + * This is really horribly ugly.
> >> + */
> >
> > If it's so horribly ugly, don't do it this way ;-)
>
> :-) this is not my part of code. I'll remove it with syscall changes.
Yes, I was aware of that. I believe the comment was initially done in
order to prevent people from copying it to new architectures, but it
never worked.
> >> +int
> >> +sys_ipc(uint call, int first, int second, int third, void *ptr, long fifth)
> >> +{
> >> + int version, ret;
> >> +
> >> + version = call >> 16; /* hack for backward compatibility */
> >> + call &= 0xffff;
> >
> > Backwards compatibility with what?
>
> I don't know.
> John: I suppose this is your comment.
No, it was a trick question, this is also copied from i386. It was meant
for the iBCS2 compatibility layer that provides support for running
Unix binaries from the late 80s, but never quite made it into the Linux
kernel.
> >> +unsigned long sys_mmap2(unsigned long addr, size_t len,
> >> + unsigned long prot, unsigned long flags,
> >> + unsigned long fd, unsigned long pgoff)
> >> +{
> >> + return do_mmap2(addr, len, prot, flags, fd, pgoff);
> >> +}
> >> +
> >> +unsigned long sys_mmap(unsigned long addr, size_t len,
> >> + unsigned long prot, unsigned long flags,
> >> + unsigned long fd, off_t offset)
> >> +{
> >> + int err = -EINVAL;
> >> +
> >> + if (offset & ~PAGE_MASK) {
> >> + printk(KERN_INFO "no pagemask in mmap\r\n");
> >> + goto out;
> >> + }
> >> +
> >> + err = do_mmap2(addr, len, prot, flags, fd, offset >> PAGE_SHIFT);
> >> +out:
> >> + return err;
> >> +}
> >
> > Which mmap is uClibc really using? I suppose you only need mmap2, even for
> > compatibility with any binary ever built for microblaze.
>
> I don't know. Microblaze should be compiled with uClibc and glibc. In case you
> need mmap2 for uClibc and mmap for glibc it is ok.
I looked at uClibc again and found that you probably need both for compatibility
and functionality, same as for other calls that can take either an off_t or
a loff_t. Only if you changed the libc/sysdeps/linux/microblaze/mmap.c code,
you could get rid of sys_mmap(). I don't think you need to worry about glibc
at this point, because as I understand it the old port is broken anyway
and it's easier to do a new microblaze glibc port from scratch than trying
to update it to the latest glibc version...
Arnd <><
^ permalink raw reply
* Re: [PATCH 48/60] microblaze_v4: headers simple files - empty or redirect to asm-generic
From: Arnd Bergmann @ 2008-06-26 23:23 UTC (permalink / raw)
To: Adrian Bunk
Cc: linux-arch, alan, Michal Simek, vapier.adi, matthew,
microblaze-uclinux, linux-kernel, drepper, linuxppc-dev,
will.newton, hpa, monstr, John.Linn, john.williams
In-Reply-To: <20080626180530.GA22827@cs181140183.pp.htv.fi>
On Thursday 26 June 2008, Adrian Bunk wrote:
> Honestly, I do not completely like your approach of getting the
> microblaze port submitter to create the asm-generic files - I would
> personally prefer if the microblaze port would look exactly like all
> other ports and the (reasonable) changes you have in mind were not
> being discussed and done as part of the submission of a new port.
But it works really well this way ;-). My point is that a new port
should look just like all the other ports should have looked as
well, not like they did. When it comes to the ABI, you
cannot make incompatible changes after it's merged, so IMHO all
ABI defining headers should go to asm-generic if possible.
Since there doesn't seem to be anyone investing work into moving the
files there (I started it before, but got bored before submitting
them all myself), the point of adding a new architecture is exactly
the right one for putting the file to asm-generic. For Michal, there
is no difference between putting the file into asm-generic or
asm-microblaze, other than that he has to change his existing
patch once, but in return he gets fewer files to maintain afterwards.
The fundamental principle here is: if you want your code to get in,
do it in a way that makes your own code cleaner by making it cleaner
for everyone else as well.
The result is that more people look at the code, and that Michal's
name gets more widely known, so the next time he needs something
from another developer, he's more likely to get heard because
you think of him as the person that did all the useful work on
the asm-generic files.
> After all, it won't matter whether we'll unify resp. remove
> 22 or 23 files.
That wasn't my idea. The logic was that if one more file exists
in asm-generic that can be removed from the architectures,
we get 22 more files to remove without anyone having to look
at the big picture. When microblaze is in, I can compile a list
with asm-generic files that can be used to replace the architecture
specific files, so the arch maintainers can decide on their own
whether to clean their own stuff up or not.
With namei.h, I may have gone too far to request moving it
to asm-generic as part of the microblaze merge, because it's
not an ABI header, but I think it's a step in the right direction
anyway, and I may put it there myself if I ever get to do
my "how to port Linux to a new architecture the right way" paper.
Arnd <><
^ permalink raw reply
* Re: dtc: Address an assortment of portability problems
From: David Gibson @ 2008-06-26 23:47 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev, Benno Rice
In-Reply-To: <20080626152528.GA12256@loki.buserror.net>
On Thu, Jun 26, 2008 at 10:25:28AM -0500, Scott Wood wrote:
> On Thu, Jun 26, 2008 at 11:03:49AM +1000, David Gibson wrote:
> > - the endian handling functions in libfdt_env.h, based on
> > endian.h and byteswap.h are replaced with some portable open-coded
> > versions. Unfortunately, these result in fairly crappy code when
> > compiled, but as far as I can determine there doesn't seem to be any
> > POSIX, SUS or de facto standard way of determining endianness at
> > compile time, nor standard names for byteswapping functions.
>
> Since device-tree and network byte order happen to be the same, we could use
> ntohl/htonl.
Not for the 64-bit version.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
^ permalink raw reply
* [PATCH] Support NAND partitions >4GiB with Open Firmware
From: Mitch Bradley @ 2008-06-26 23:50 UTC (permalink / raw)
To: linuxppc-dev, linux-mtd
This patch modifes ofpart.c so the total size of NAND FLASH
and the size of an individual partition can exceed 4GiB. It does so
by decoding the "reg" property based on the values of "#address-cells"
and "#size-cells" in the parent node, thus allowing the base address
and size to be 64-bit numbers if necessary. It handles any combination
of #address-cells = 1 or 2 and #size-cells = 1 or 2, handles the case
where the parent node doesn't have #address-cells / #size-cells properties,
and handles the case where the value of #address-cells is incorrectly
inherited from farther up the tree than the direct parent.
This patch does not solve the problem that the MTD subsystem
itself is limited to 2 GiB NAND sizes, but it is a step in that direction.
The final assignment of the 64-bit partition offset and size values is
truncated (by the C type conversion rules) to the actual size of the
struct mtd_partition offset and size fields (which are currently u_int32's).
At some point in the future, when those fields become larger, the code
should "just work".
The patch should apply to either 2.6.25 or 2.6.26rc. It has been tested
on the OLPC variant of 2.6.25 , with additional patches to other
OLPC-specific files that aren't necessary for other architectures
that have more mature support for Open Firmware. The OLPC
patch set, plus a set of test cases that verify correct operation for
various #address-cells / #size-cells combinations, can be found at
http://dev.laptop.org/~wmb/ofpart-olpc.tgz
Signed-off-by: Mitch Bradley <wmb@firmworks.com>
diff --git a/drivers/mtd/ofpart.c b/drivers/mtd/ofpart.c
index f86e069..b83b26c 100644
--- a/drivers/mtd/ofpart.c
+++ b/drivers/mtd/ofpart.c
@@ -7,6 +7,10 @@
* Revised to handle newer style flash binding by:
* Copyright (C) 2007 David Gibson, IBM Corporation.
*
+ * Revised to handle multi-cell addresses and size in reg properties,
+ * paving the way for NAND FLASH devices > 4GiB by:
+ * Mitch Bradley <wmb@laptop.org>
+ *
* 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
@@ -15,0 +19,0 @@
#include <linux/module.h>
#include <linux/init.h>
+#include <linux/device.h>
#include <linux/of.h>
#include <linux/mtd/mtd.h>
#include <linux/mtd/partitions.h>
+u_int32_t decode_cell(const u_int8_t *prop)
+{
+ return ((prop[0] << 24) + (prop[1] << 16) + (prop[2] << 8) +
prop[3]);
+}
+
int __devinit of_mtd_parse_partitions(struct device *dev,
struct mtd_info *mtd,
struct device_node *node,
@@ -44,5 +54,7 @@ int __devinit of_mtd_parse_partitions(struct device *dev,
pp = NULL;
i = 0;
while ((pp = of_get_next_child(node, pp))) {
- const u32 *reg;
+ u_int64_t xoffset, xsize;
+ const u_int32_t *propval;
+ u_int32_t addrcells = 0, sizecells = 0;
int len;
- reg = of_get_property(pp, "reg", &len);
- if (!reg || (len != 2 * sizeof(u32))) {
+ /*
+ * Determine the layout of a "reg" entry based on the parent
+ * node's properties, if it hasn't been done already.
+ */
+
+ if (addrcells == 0)
+ addrcells = of_n_addr_cells(pp);
+ if (sizecells == 0)
+ sizecells = of_n_size_cells(pp);
+
+ propval = of_get_property(pp, "reg", &len);
+
+ /*
+ * Handle the possibility of a broken device tree that
+ * doesn't define #address-cells and #size-cells properly.
+ * In a standard device tree, if the address portion of
+ * "reg" is one cell, the direct parent should have a
+ * #address-cells property with value 1.
+ */
+ if (propval && (len == 2 * sizeof(u32)))
+ addrcells = sizecells = 1;
+
+ /* Error checks */
+
+ if (addrcells < 1 || addrcells > 2) {
+ of_node_put(pp);
+ dev_err(dev, "Invalid #address_cells %d on %s\n",
+ addrcells, node->full_name);
+ kfree(*pparts);
+ *pparts = NULL;
+ return -EINVAL;
+ }
+
+ if (sizecells < 1 || sizecells > 2) {
+ of_node_put(pp);
+ dev_err(dev, "Invalid #size-cells %d on %s\n",
+ sizecells, node->full_name);
+ kfree(*pparts);
+ *pparts = NULL;
+ return -EINVAL;
+ }
+
+ if (!propval || (len != (addrcells+sizecells) *
sizeof(u32))) {
of_node_put(pp);
dev_err(dev, "Invalid 'reg' on %s\n",
node->full_name);
kfree(*pparts);
*pparts = NULL;
return -EINVAL;
}
- (*pparts)[i].offset = reg[0];
- (*pparts)[i].size = reg[1];
+
+ /* Accumulate the base address and size into 64-bit
numbers */
+ xoffset = (u_int64_t)ntohl(propval[0]);
+ if (addrcells > 1) {
+ xoffset <<= 32;
+ xoffset += ntohl(propval[1]);
+ }
+ xsize = (u_int64_t)ntohl(propval[addrcells]);
+ if (sizecells > 1) {
+ xsize <<= 32;
+ xsize += ntohl(propval[addrcells+1]);
+ }
+
+ (*pparts)[i].offset = xoffset;
+ (*pparts)[i].size = xsize;
partname = of_get_property(pp, "label", &len);
if (!partname)
^ permalink raw reply related
* Re: [PATCH 02/60] microblaze_v4: Makefiles for Microblaze cpu
From: John Williams @ 2008-06-27 0:03 UTC (permalink / raw)
To: Adrian Bunk
Cc: linux-arch, vapier.adi, arnd, matthew, microblaze-uclinux,
linux-kernel, drepper, linuxppc-dev, will.newton, hpa,
Michal Simek, John.Linn, alan
In-Reply-To: <20080626194002.GC22827@cs181140183.pp.htv.fi>
On 6/27/08, Adrian Bunk <bunk@kernel.org> wrote:
> On Thu, Jun 26, 2008 at 08:46:44PM +0200, Michal Simek wrote:
> > Adrian Bunk napsal(a):
> > > On Thu, Jun 26, 2008 at 02:29:31PM +0200, monstr@seznam.cz wrote:
> > >> ...
> > >> --- /dev/null
> > >> +++ b/arch/microblaze/Makefile
> > >> ...
> > >> +# Work out HW multipler support. This is icky.
> > >> +# 1. Spartan2 has no HW multiplers.
> > >> +# 2. MicroBlaze v3.x always uses them, except in Spartan 2
> > >> +# 3. All other FPGa/CPU ver combos, we can trust the CONFIG_ settings
> > >> +ifeq (,$(findstring spartan2,$(CONFIG_XILINX_MICROBLAZE0_FAMILY)))
> > >> + ifeq ($(CPU_MAJOR),3)
> > >> + CPUFLAGS-1 += -mno-xl-soft-mul
> > >> + else
> > >> + # USE_HW_MUL can be 0, 1, or 2, defining a heirarchy of HW Mul support.
> > >> + CPUFLAGS-$(subst 1,,$(CONFIG_XILINX_MICROBLAZE0_USE_HW_MUL)) += -mxl-multiply-high
> > >> + CPUFLAGS-$(CONFIG_XILINX_MICROBLAZE0_USE_HW_MUL) += -mno-xl-soft-mul
> > >> + endif
> > >> +endif
> > >> +CPUFLAGS-$(CONFIG_XILINX_MICROBLAZE0_USE_DIV) += -mno-xl-soft-div
> > >> +CPUFLAGS-$(CONFIG_XILINX_MICROBLAZE0_USE_BARREL) += -mxl-barrel-shift
> > >> +CPUFLAGS-$(CONFIG_XILINX_MICROBLAZE0_USE_PCMP) += -mxl-pattern-compare
> > >> +
> > >> +CPUFLAGS-1 += $(call cc-option,-mcpu=v$(CPU_VER))
> > >> +
> > >> +# The various CONFIG_XILINX cpu features options are integers 0/1/2...
> > >> +# rather than bools y/n
> > >> +CFLAGS += $(CPUFLAGS-1)
> > >> +CFLAGS += $(CPUFLAGS-2)
> > >> ...
> > >
> > > Why are the options not bools?
> > because CONFIG_XILINX_... are 0, 1 or 2 not only y, n.
> I understood that.
>
> But _why_ are these options not bools?
The CPU has 3 levels of multiplier support -
0=none
1= 32-bit (mul produces 32 LSB of a 32x32 bit multiply)
2= 32-bit high (mulhi produces 32 MSB of a 32x32 bit multiply).
The CPU-specific options are pulled directly out of the MicroBlaze
system design tools as an auto-generated config file that we drop into
the kernel build. Doing so ensures consistency between the hardware
and the kernel running on it.
These options come out of the tools as intergers, 0,1,2. We found in
the early days of microblaze that lots of users built unbootable
kernels because they manually transcribed these options incorrectly.
So, we integrate as closely as we reasonable can to prevent these sort
of CPU configuration errors.
These options are now limited to about 6 CPU-specific options - things
we must know about the CPU at compile time to set the correct cpuflags
to GCC.
All else is set through open firmware or querying a Processor Version
Register at boot.
Regards,
John
--
John Williams, PhD, B.Eng, B.IT
PetaLogix - Linux Solutions for a Reconfigurable World
w: www.petalogix.com p: +61-7-30090663 f: +61-7-30090663
^ permalink raw reply
* Re: Microblaze init port v4
From: John Williams @ 2008-06-27 0:12 UTC (permalink / raw)
To: Adrian Bunk
Cc: linux-arch, vapier.adi, arnd, matthew, microblaze-uclinux,
linux-kernel, drepper, linuxppc-dev, will.newton, hpa, monstr,
John.Linn, alan
In-Reply-To: <20080626150121.GB22025@cs181140183.pp.htv.fi>
Hi,
On 6/27/08, Adrian Bunk <bunk@kernel.org> wrote:
> On Thu, Jun 26, 2008 at 02:29:29PM +0200, monstr@seznam.cz wrote:
> > current linux version is 2.6.26-rc8 and I think this is the right time
> > to send latest patches againts rc8 for Microblaze CPU.
> >...
>
> Thanks for your work on getting the architecture included.
>
> I have two questions:
>
> Where can I find a toolchain for compiling a Microblaze kernel?
We bundle it in the PetaLinux distro:
http://developer.petalogix.com
> What is the status of Microblaze support in upstream binutils and gcc?
noMMU uClibc toolchain is at 3.4.1, MMU/glibc is at 4.1.1 Xilinx does
their gcc/binutils stuff mostly in-house, and none of it is upstream.
We (PetaLogix) build and distribute the uClibc toolchain, and we are
slowly merging the MMU support across to our regular distro, that will
also include the glibc tools.
Cheers,
John
--
John Williams, PhD, B.Eng, B.IT
PetaLogix - Linux Solutions for a Reconfigurable World
w: www.petalogix.com p: +61-7-30090663 f: +61-7-30090663
^ permalink raw reply
* Re: [PATCH] powerpc: add of_find_next_property andof_get_aliased_index
From: David Gibson @ 2008-06-27 1:30 UTC (permalink / raw)
To: Stefan Roese; +Cc: linuxppc-dev, Timur Tabi, Sean MacLennan
In-Reply-To: <200806262041.12275.sr@denx.de>
On Thu, Jun 26, 2008 at 08:41:12PM +0200, Stefan Roese wrote:
> On Thursday 26 June 2008, Sean MacLennan wrote:
> > > Well, there's a lot of disagreement on this subject. Not only do we
> > > not agree on a method of enumerating devices, a lot of people have a
> > > problem with the concept of enumerating them in the first place!
> >
> > An interesting point is that I enforced an index in the i2c-ibm_iic
> > driver with no disagreement at all ;)
>
> You have been lucky I suppose. :)
Ah... that's because IIC does have a correct reason to have cell-index
- like most of the 4xx devices, we may need the index for programming
the CPM power control retisters - and we didn't notice you were also
using for the incorrect purpose of supplying an index to the i2c
layer.
Please note that cell-index *does* *not* *work* for the global index
if there are multiple SoC-like units in the system. For its correct
purpose of indexing the CPM registers, cell-index must be local to the
SoC, for the global index it must not.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
^ permalink raw reply
* Re: [Resend][PATCH 1/8][Version 2] MPC5121 Update MPC5121ADS device tree
From: David Gibson @ 2008-06-27 1:42 UTC (permalink / raw)
To: John Rigby; +Cc: linuxppc-dev
In-Reply-To: <20080625203051.4FE882F123B@rumpole.eng.lineo.com>
On Wed, Jun 18, 2008 at 02:24:46PM -0600, John Rigby wrote:
> Updated device tree for MPC5121ADS
[snip]
> soc@80000000 {
> compatible = "fsl,mpc5121-immr";
> + device_type = "soc";
I realise we still need the unwanted device_type value on the soc in
some cases for backwards compatibility, but why are you adding it
where it wasn't before..?
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
^ permalink raw reply
* National Semi DP83865 Phy on custom hardware with PPC 8540 CPU
From: Ed Henderson @ 2008-06-27 2:45 UTC (permalink / raw)
To: linuxppc-embedded
[-- Attachment #1: Type: text/plain, Size: 1409 bytes --]
Greetings.
I am a hardware engineer and I designed a board based on a Power PC MPC8540
CPU. The hardware was patterned after another board with a Marvel driver. I
had some difficulty locating the Marvel Chips, so I used a National
Semiconductor DP83865 Phy instead, available at Digikey. We looked at this
and felt it would not be too difficult to adapt the current Kernel code to
this phy. In practice, now that we have hardware built, it is proving to be
a bit more difficult to bring up. It is very interesting that u-boot
contains code for this Phy, and we are able to get to the u-boot command
line, setup and Ethernet address, ping and even do a tftpboot kernel
transfer. Unfortunately, in the Kernel, we do not have the same success.
This seems to indicate that the hardware is working, at least to some
degree.
We did some modifications on the Marvel driver in order to 'attempt' to
adapt it to the National Phy. To be honest, we are not that familiar with
this aspect of Kernel development, so we could really use some
pointers/tips/assistance. There are some file geared toward the DP83865,
but they are "PCI" drivers. Our Phy is directly connected to the PPC. I do
not know if it make sense to start with the natsemi.c file (which is geared
toward PCI), or to make our changes to the Marvel phy.
I would appreciate any help in resolving this issue.
Regards,
Ed Henderson
[-- Attachment #2: Type: text/html, Size: 3499 bytes --]
^ permalink raw reply
* Re: [PATCH] Support NAND partitions >4GiB with Open Firmware
From: Mitch Bradley @ 2008-06-27 3:09 UTC (permalink / raw)
To: linuxppc-dev, linux-mtd
In-Reply-To: <48642B50.9060607@firmworks.com>
A revised version of the patch, addressing some points that Segher
identified, will be issued soon.
So if you want to review the patch as submitted, please be aware that
some stylistic things have already been fixed (u64 instead of u_int64_t
etc, use of of_read_number(), removal of fallback code for broken
device trees). The info in Documentation/powerpc/booting-without-of.txt
will also be amended.
The overall scheme - using #address-cells and #size-cells to permit 64-bit
offset and sizes - remains unchanged.
^ permalink raw reply
* Re: [PATCH] Support NAND partitions >4GiB with Open Firmware
From: David Gibson @ 2008-06-27 3:20 UTC (permalink / raw)
To: Mitch Bradley; +Cc: linuxppc-dev, linux-mtd
In-Reply-To: <48642B50.9060607@firmworks.com>
On Thu, Jun 26, 2008 at 01:50:40PM -1000, Mitch Bradley wrote:
> This patch modifes ofpart.c so the total size of NAND FLASH
> and the size of an individual partition can exceed 4GiB. It does so
> by decoding the "reg" property based on the values of "#address-cells"
> and "#size-cells" in the parent node, thus allowing the base address
> and size to be 64-bit numbers if necessary. It handles any combination
> of #address-cells = 1 or 2 and #size-cells = 1 or 2, handles the case
> where the parent node doesn't have #address-cells / #size-cells properties,
> and handles the case where the value of #address-cells is incorrectly
> inherited from farther up the tree than the direct parent.
>
> This patch does not solve the problem that the MTD subsystem
> itself is limited to 2 GiB NAND sizes, but it is a step in that direction.
> The final assignment of the 64-bit partition offset and size values is
> truncated (by the C type conversion rules) to the actual size of the
> struct mtd_partition offset and size fields (which are currently u_int32's).
> At some point in the future, when those fields become larger, the code
> should "just work".
>
> The patch should apply to either 2.6.25 or 2.6.26rc. It has been tested
> on the OLPC variant of 2.6.25 , with additional patches to other
> OLPC-specific files that aren't necessary for other architectures
> that have more mature support for Open Firmware. The OLPC
> patch set, plus a set of test cases that verify correct operation for
> various #address-cells / #size-cells combinations, can be found at
> http://dev.laptop.org/~wmb/ofpart-olpc.tgz
I like the idea - but there are some uglies in the implementation.
> Signed-off-by: Mitch Bradley <wmb@firmworks.com>
>
> diff --git a/drivers/mtd/ofpart.c b/drivers/mtd/ofpart.c
> index f86e069..b83b26c 100644
> --- a/drivers/mtd/ofpart.c
> +++ b/drivers/mtd/ofpart.c
> @@ -7,6 +7,10 @@
> * Revised to handle newer style flash binding by:
> * Copyright (C) 2007 David Gibson, IBM Corporation.
> *
> + * Revised to handle multi-cell addresses and size in reg properties,
> + * paving the way for NAND FLASH devices > 4GiB by:
> + * Mitch Bradley <wmb@laptop.org>
> + *
> * 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
> @@ -15,0 +19,0 @@
>
> #include <linux/module.h>
> #include <linux/init.h>
> +#include <linux/device.h>
> #include <linux/of.h>
> #include <linux/mtd/mtd.h>
> #include <linux/mtd/partitions.h>
>
> +u_int32_t decode_cell(const u_int8_t *prop)
> +{
> + return ((prop[0] << 24) + (prop[1] << 16) + (prop[2] << 8) +
> prop[3]);
> +}
You don't appear to actually use this new function.
> int __devinit of_mtd_parse_partitions(struct device *dev,
> struct mtd_info *mtd,
> struct device_node *node,
> @@ -44,5 +54,7 @@ int __devinit of_mtd_parse_partitions(struct device *dev,
> pp = NULL;
> i = 0;
> while ((pp = of_get_next_child(node, pp))) {
> - const u32 *reg;
> + u_int64_t xoffset, xsize;
The names u64 and u32 are preferred for kernel internal things.
Certainly not u_int64_t which isn't even the C99 standard name.
> + const u_int32_t *propval;
> + u_int32_t addrcells = 0, sizecells = 0;
> int len;
>
> - reg = of_get_property(pp, "reg", &len);
> - if (!reg || (len != 2 * sizeof(u32))) {
> + /*
> + * Determine the layout of a "reg" entry based on the parent
> + * node's properties, if it hasn't been done already.
> + */
> +
> + if (addrcells == 0)
Redundant 'if'; you've just initialized this variable to zero.
> + addrcells = of_n_addr_cells(pp);
> + if (sizecells == 0)
> + sizecells = of_n_size_cells(pp);
> +
> + propval = of_get_property(pp, "reg", &len);
> +
> + /*
> + * Handle the possibility of a broken device tree that
> + * doesn't define #address-cells and #size-cells properly.
> + * In a standard device tree, if the address portion of
> + * "reg" is one cell, the direct parent should have a
> + * #address-cells property with value 1.
> + */
> + if (propval && (len == 2 * sizeof(u32)))
> + addrcells = sizecells = 1;
> +
> + /* Error checks */
> +
> + if (addrcells < 1 || addrcells > 2) {
> + of_node_put(pp);
> + dev_err(dev, "Invalid #address_cells %d on %s\n",
> + addrcells, node->full_name);
> + kfree(*pparts);
> + *pparts = NULL;
> + return -EINVAL;
> + }
> +
> + if (sizecells < 1 || sizecells > 2) {
> + of_node_put(pp);
> + dev_err(dev, "Invalid #size-cells %d on %s\n",
> + sizecells, node->full_name);
> + kfree(*pparts);
> + *pparts = NULL;
> + return -EINVAL;
> + }
> +
> + if (!propval || (len != (addrcells+sizecells) *
> sizeof(u32))) {
> of_node_put(pp);
> dev_err(dev, "Invalid 'reg' on %s\n",
> node->full_name);
> kfree(*pparts);
> *pparts = NULL;
> return -EINVAL;
> }
> - (*pparts)[i].offset = reg[0];
> - (*pparts)[i].size = reg[1];
> +
> + /* Accumulate the base address and size into 64-bit
> numbers */
> + xoffset = (u_int64_t)ntohl(propval[0]);
You shouldn't use the ntohl() names outside of networking code.
Instead use be32_to_cpu() and the like.
> + if (addrcells > 1) {
> + xoffset <<= 32;
> + xoffset += ntohl(propval[1]);
> + }
> + xsize = (u_int64_t)ntohl(propval[addrcells]);
> + if (sizecells > 1) {
> + xsize <<= 32;
> + xsize += ntohl(propval[addrcells+1]);
> + }
> +
> + (*pparts)[i].offset = xoffset;
> + (*pparts)[i].size = xsize;
>
> partname = of_get_property(pp, "label", &len);
> if (!partname)
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
^ permalink raw reply
* Re: [PATCH] Support NAND partitions >4GiB with Open Firmware
From: Mitch Bradley @ 2008-06-27 3:28 UTC (permalink / raw)
To: Mitch Bradley, linuxppc-dev, linux-mtd, david
In-Reply-To: <20080627032046.GH17621@yookeroo.seuss>
David Gibson wrote:
> On Thu, Jun 26, 2008 at 01:50:40PM -1000, Mitch Bradley wrote:
>
>> This patch modifes ofpart.c so the total size of NAND FLASH
>> and the size of an individual partition can exceed 4GiB. It does so
>> by decoding the "reg" property based on the values of "#address-cells"
>> and "#size-cells" in the parent node, thus allowing the base address
>> and size to be 64-bit numbers if necessary. It handles any combination
>> of #address-cells = 1 or 2 and #size-cells = 1 or 2, handles the case
>> where the parent node doesn't have #address-cells / #size-cells properties,
>> and handles the case where the value of #address-cells is incorrectly
>> inherited from farther up the tree than the direct parent.
>>
>> This patch does not solve the problem that the MTD subsystem
>> itself is limited to 2 GiB NAND sizes, but it is a step in that direction.
>> The final assignment of the 64-bit partition offset and size values is
>> truncated (by the C type conversion rules) to the actual size of the
>> struct mtd_partition offset and size fields (which are currently u_int32's).
>> At some point in the future, when those fields become larger, the code
>> should "just work".
>>
>> The patch should apply to either 2.6.25 or 2.6.26rc. It has been tested
>> on the OLPC variant of 2.6.25 , with additional patches to other
>> OLPC-specific files that aren't necessary for other architectures
>> that have more mature support for Open Firmware. The OLPC
>> patch set, plus a set of test cases that verify correct operation for
>> various #address-cells / #size-cells combinations, can be found at
>> http://dev.laptop.org/~wmb/ofpart-olpc.tgz
>>
>
> I like the idea - but there are some uglies in the implementation.
>
>
>> Signed-off-by: Mitch Bradley <wmb@firmworks.com>
>>
>> diff --git a/drivers/mtd/ofpart.c b/drivers/mtd/ofpart.c
>> index f86e069..b83b26c 100644
>> --- a/drivers/mtd/ofpart.c
>> +++ b/drivers/mtd/ofpart.c
>> @@ -7,6 +7,10 @@
>> * Revised to handle newer style flash binding by:
>> * Copyright (C) 2007 David Gibson, IBM Corporation.
>> *
>> + * Revised to handle multi-cell addresses and size in reg properties,
>> + * paving the way for NAND FLASH devices > 4GiB by:
>> + * Mitch Bradley <wmb@laptop.org>
>> + *
>> * 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
>> @@ -15,0 +19,0 @@
>>
>> #include <linux/module.h>
>> #include <linux/init.h>
>> +#include <linux/device.h>
>> #include <linux/of.h>
>> #include <linux/mtd/mtd.h>
>> #include <linux/mtd/partitions.h>
>>
>> +u_int32_t decode_cell(const u_int8_t *prop)
>> +{
>> + return ((prop[0] << 24) + (prop[1] << 16) + (prop[2] << 8) +
>> prop[3]);
>> +}
>>
>
> You don't appear to actually use this new function.
>
It's already gone in the new version (the announcement of which may have
been delayed by the mailing list of which I'm not a subscriber).
>
>> int __devinit of_mtd_parse_partitions(struct device *dev,
>> struct mtd_info *mtd,
>> struct device_node *node,
>> @@ -44,5 +54,7 @@ int __devinit of_mtd_parse_partitions(struct device *dev,
>> pp = NULL;
>> i = 0;
>> while ((pp = of_get_next_child(node, pp))) {
>> - const u32 *reg;
>> + u_int64_t xoffset, xsize;
>>
>
> The names u64 and u32 are preferred for kernel internal things.
> Certainly not u_int64_t which isn't even the C99 standard name.
>
Already fixed.
>
>> + const u_int32_t *propval;
>> + u_int32_t addrcells = 0, sizecells = 0;
>> int len;
>>
>> - reg = of_get_property(pp, "reg", &len);
>> - if (!reg || (len != 2 * sizeof(u32))) {
>> + /*
>> + * Determine the layout of a "reg" entry based on the parent
>> + * node's properties, if it hasn't been done already.
>> + */
>> +
>> + if (addrcells == 0)
>>
>
> Redundant 'if'; you've just initialized this variable to zero.
>
The intention is that the body of the "if" should only be executed
once during the loop, since the parent node is the same for all children.
>
>> + addrcells = of_n_addr_cells(pp);
>> + if (sizecells == 0)
>> + sizecells = of_n_size_cells(pp);
>> +
>> + propval = of_get_property(pp, "reg", &len);
>> +
>> + /*
>> + * Handle the possibility of a broken device tree that
>> + * doesn't define #address-cells and #size-cells properly.
>> + * In a standard device tree, if the address portion of
>> + * "reg" is one cell, the direct parent should have a
>> + * #address-cells property with value 1.
>> + */
>> + if (propval && (len == 2 * sizeof(u32)))
>> + addrcells = sizecells = 1;
>> +
>> + /* Error checks */
>> +
>> + if (addrcells < 1 || addrcells > 2) {
>> + of_node_put(pp);
>> + dev_err(dev, "Invalid #address_cells %d on %s\n",
>> + addrcells, node->full_name);
>> + kfree(*pparts);
>> + *pparts = NULL;
>> + return -EINVAL;
>> + }
>> +
>> + if (sizecells < 1 || sizecells > 2) {
>> + of_node_put(pp);
>> + dev_err(dev, "Invalid #size-cells %d on %s\n",
>> + sizecells, node->full_name);
>> + kfree(*pparts);
>> + *pparts = NULL;
>> + return -EINVAL;
>> + }
>> +
>> + if (!propval || (len != (addrcells+sizecells) *
>> sizeof(u32))) {
>> of_node_put(pp);
>> dev_err(dev, "Invalid 'reg' on %s\n",
>> node->full_name);
>> kfree(*pparts);
>> *pparts = NULL;
>> return -EINVAL;
>> }
>> - (*pparts)[i].offset = reg[0];
>> - (*pparts)[i].size = reg[1];
>> +
>> + /* Accumulate the base address and size into 64-bit
>> numbers */
>> + xoffset = (u_int64_t)ntohl(propval[0]);
>>
>
> You shouldn't use the ntohl() names outside of networking code.
> Instead use be32_to_cpu() and the like.
>
That whole code block has been replaced by a call to of_read_number()
>
>> + if (addrcells > 1) {
>> + xoffset <<= 32;
>> + xoffset += ntohl(propval[1]);
>> + }
>> + xsize = (u_int64_t)ntohl(propval[addrcells]);
>> + if (sizecells > 1) {
>> + xsize <<= 32;
>> + xsize += ntohl(propval[addrcells+1]);
>> + }
>> +
>> + (*pparts)[i].offset = xoffset;
>> + (*pparts)[i].size = xsize;
>>
>> partname = of_get_property(pp, "label", &len);
>> if (!partname)
>>
>
>
^ permalink raw reply
* Re: [PATCH] Support NAND partitions >4GiB with Open Firmware
From: David Gibson @ 2008-06-27 3:38 UTC (permalink / raw)
To: Mitch Bradley; +Cc: linuxppc-dev, linux-mtd
In-Reply-To: <48645E6A.6020202@firmworks.com>
On Thu, Jun 26, 2008 at 05:28:42PM -1000, Mitch Bradley wrote:
> David Gibson wrote:
>> On Thu, Jun 26, 2008 at 01:50:40PM -1000, Mitch Bradley wrote:
[snip]
>>> + const u_int32_t *propval;
>>> + u_int32_t addrcells = 0, sizecells = 0;
>>> int len;
>>>
>>> - reg = of_get_property(pp, "reg", &len);
>>> - if (!reg || (len != 2 * sizeof(u32))) {
>>> + /*
>>> + * Determine the layout of a "reg" entry based on the parent
>>> + * node's properties, if it hasn't been done already.
>>> + */
>>> +
>>> + if (addrcells == 0)
>>>
>>
>> Redundant 'if'; you've just initialized this variable to zero.
>
> The intention is that the body of the "if" should only be executed
> once during the loop, since the parent node is the same for all
> children.
But the initialization is within the loop body as well, so this won't
do it. Just factor the code getting addr and size cells right out of
the loop, instead.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
^ permalink raw reply
* Re: [Resend][PATCH 1/8][Version 2] MPC5121 Update MPC5121ADS device tree
From: John Rigby @ 2008-06-27 3:40 UTC (permalink / raw)
To: John Rigby, linuxppc-dev
In-Reply-To: <20080627014233.GB17621@yookeroo.seuss>
Because get_immrbase in fsl_soc.c does not work without it.
On Thu, Jun 26, 2008 at 7:42 PM, David Gibson
<david@gibson.dropbear.id.au> wrote:
> On Wed, Jun 18, 2008 at 02:24:46PM -0600, John Rigby wrote:
>> Updated device tree for MPC5121ADS
>
> [snip]
>> soc@80000000 {
>> compatible = "fsl,mpc5121-immr";
>> + device_type = "soc";
>
> I realise we still need the unwanted device_type value on the soc in
> some cases for backwards compatibility, but why are you adding it
> where it wasn't before..?
>
> --
> David Gibson | I'll have my music baroque, and my code
> david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
> | _way_ _around_!
> http://www.ozlabs.org/~dgibson
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
>
^ permalink raw reply
* Re: [PATCH] Support NAND partitions >4GiB with Open Firmware
From: Mitch Bradley @ 2008-06-27 3:48 UTC (permalink / raw)
To: Mitch Bradley, linuxppc-dev, linux-mtd
In-Reply-To: <20080627033817.GJ17621@yookeroo.seuss>
David Gibson wrote:
> On Thu, Jun 26, 2008 at 05:28:42PM -1000, Mitch Bradley wrote:
>
>> David Gibson wrote:
>>
>>> On Thu, Jun 26, 2008 at 01:50:40PM -1000, Mitch Bradley wrote:
>>>
> [snip]
>
>>>> + const u_int32_t *propval;
>>>> + u_int32_t addrcells = 0, sizecells = 0;
>>>> int len;
>>>>
>>>> - reg = of_get_property(pp, "reg", &len);
>>>> - if (!reg || (len != 2 * sizeof(u32))) {
>>>> + /*
>>>> + * Determine the layout of a "reg" entry based on the parent
>>>> + * node's properties, if it hasn't been done already.
>>>> + */
>>>> +
>>>> + if (addrcells == 0)
>>>>
>>>>
>>> Redundant 'if'; you've just initialized this variable to zero.
>>>
>> The intention is that the body of the "if" should only be executed
>> once during the loop, since the parent node is the same for all
>> children.
>>
>
> But the initialization is within the loop body as well, so this won't
> do it. Just factor the code getting addr and size cells right out of
> the loop, instead.
>
>
Hmmm. Perhaps it's better to move the declaration of the variables out of
the loop instead.
Moving the of_n_*_cells() calls outside the loop requires redundant calls
to of_get_child() and of_node_put(), because of_n_*_cells() implicitly
reach up to the parent node. That is almost certainly more expensive
than the "if".
^ permalink raw reply
* Re: [PATCH] Support NAND partitions >4GiB with Open Firmware
From: David Gibson @ 2008-06-27 4:04 UTC (permalink / raw)
To: Mitch Bradley; +Cc: linuxppc-dev, linux-mtd
In-Reply-To: <486462F1.9080602@firmworks.com>
On Thu, Jun 26, 2008 at 05:48:01PM -1000, Mitch Bradley wrote:
>
>
> David Gibson wrote:
>> On Thu, Jun 26, 2008 at 05:28:42PM -1000, Mitch Bradley wrote:
>>
>>> David Gibson wrote:
>>>
>>>> On Thu, Jun 26, 2008 at 01:50:40PM -1000, Mitch Bradley wrote:
>>>>
>> [snip]
>>
>>>>> + const u_int32_t *propval;
>>>>> + u_int32_t addrcells = 0, sizecells = 0;
>>>>> int len;
>>>>>
>>>>> - reg = of_get_property(pp, "reg", &len);
>>>>> - if (!reg || (len != 2 * sizeof(u32))) {
>>>>> + /*
>>>>> + * Determine the layout of a "reg" entry based on the parent
>>>>> + * node's properties, if it hasn't been done already.
>>>>> + */
>>>>> +
>>>>> + if (addrcells == 0)
>>>>>
>>>> Redundant 'if'; you've just initialized this variable to zero.
>>>>
>>> The intention is that the body of the "if" should only be executed
>>> once during the loop, since the parent node is the same for all
>>> children.
>>>
>>
>> But the initialization is within the loop body as well, so this won't
>> do it. Just factor the code getting addr and size cells right out of
>> the loop, instead.
>>
>>
>
> Hmmm. Perhaps it's better to move the declaration of the variables out of
> the loop instead.
>
> Moving the of_n_*_cells() calls outside the loop requires redundant calls
> to of_get_child() and of_node_put(), because of_n_*_cells() implicitly
> reach up to the parent node. That is almost certainly more expensive
> than the "if".
This is not exactly a hot path; clarity and (source) code size are
more important than expense. But going down to the child then back up
is ugly too. Maybe you should just directly pull #address-cells and
#size-cells from the parent node. In fact, of_n_*_cells() are wrong
by the OF spec, since they assume the values are inherited, which is
not how it's supposed to work.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
^ permalink raw reply
* Re: [PATCH 06/12] net: use linux/of_{device,platform}.h instead of asm
From: Jeff Garzik @ 2008-06-27 4:58 UTC (permalink / raw)
To: Stephen Rothwell; +Cc: ppc-dev, netdev
In-Reply-To: <20080523162854.2e6ea8f0.sfr@canb.auug.org.au>
Stephen Rothwell wrote:
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> ---
> drivers/net/fs_enet/fs_enet-main.c | 2 +-
> drivers/net/fs_enet/mac-scc.c | 2 +-
> drivers/net/fs_enet/mii-fec.c | 2 +-
> drivers/net/ibm_newemac/core.h | 2 +-
> drivers/net/ucc_geth.c | 2 +-
> drivers/net/ucc_geth_mii.c | 2 +-
> 6 files changed, 6 insertions(+), 6 deletions(-)
ACK
I'll let a ppc person queue this
^ permalink raw reply
* Re: [PATCH V2] ibm_newemac: Fixes kernel crashes when speed of cable connected changes
From: Jeff Garzik @ 2008-06-27 5:14 UTC (permalink / raw)
To: Stefan Roese; +Cc: linuxppc-dev, Sathya Narayanan, netdev
In-Reply-To: <1214246162-25293-1-git-send-email-sr@denx.de>
Stefan Roese wrote:
> From: Sathya Narayanan <sathyan@teamf1.com>
>
> The descriptor pointers were not initialized to NIL values, so it was
> poiniting to some random addresses which was completely invalid. This
> fix takes care of initializing the descriptor to NIL values and clearing
> the valid descriptors on clean ring operation.
>
> Signed-off-by: Sathya Narayanan <sathyan@teamf1.com>
> Signed-off-by: Stefan Roese <sr@denx.de>
> ---
> Changed since V1:
> - Fixed off-by-one error in for loops
>
> drivers/net/ibm_newemac/core.c | 6 +++++-
> 1 files changed, 5 insertions(+), 1 deletions(-)
I didn't see any answers to ben h's questions?
Holding this patch, waiting for ben's ack...
^ permalink raw reply
* Re: [RFC 1/3 v2] hvc_console: rework setup to replace irq functions with callbacks
From: Rusty Russell @ 2008-06-27 5:27 UTC (permalink / raw)
To: Christian Borntraeger
Cc: Linux PPC devel, Jeremy Fitzhardinge, Yajin, LKML,
Virtualization Mailing List
In-Reply-To: <200806201524.08237.borntraeger@de.ibm.com>
On Friday 20 June 2008 23:24:08 Christian Borntraeger wrote:
> I also kept hvc_struct defined in hvc_console.h so that hvc_irq.c can
> access the irq_requested element.
Added this fix:
=46ix compile of hvc_rtas.c
Moving the struct definition out to the header had bad effect under one
ppc64 config that I tried:
drivers/char/hvc_console.h:59: error: field =E2=80=98kref=E2=80=99 has inco=
mplete type
So move the include of kref.h too.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
diff -r f382d8f562a8 drivers/char/hvc_console.c
=2D-- a/drivers/char/hvc_console.c Fri Jun 27 15:17:49 2008 +1000
+++ b/drivers/char/hvc_console.c Fri Jun 27 15:24:15 2008 +1000
@@ -27,7 +27,6 @@
#include <linux/init.h>
#include <linux/kbd_kern.h>
#include <linux/kernel.h>
=2D#include <linux/kref.h>
#include <linux/kthread.h>
#include <linux/list.h>
#include <linux/module.h>
diff -r f382d8f562a8 drivers/char/hvc_console.h
=2D-- a/drivers/char/hvc_console.h Fri Jun 27 15:17:49 2008 +1000
+++ b/drivers/char/hvc_console.h Fri Jun 27 15:24:15 2008 +1000
@@ -26,6 +26,7 @@
=20
#ifndef HVC_CONSOLE_H
#define HVC_CONSOLE_H
+#include <linux/kref.h>
=20
/*
* This is the max number of console adapters that can/will be found as
^ permalink raw reply
* [PATCH] powerpc: excplictly copy elements of pt_regs
From: Stephen Rothwell @ 2008-06-27 6:18 UTC (permalink / raw)
To: paulus; +Cc: ppc-dev
Gcc 4.3 produced this warning:
arch/powerpc/kernel/signal_64.c: In function 'restore_sigcontext':
arch/powerpc/kernel/signal_64.c:161: warning: array subscript is above array bounds
This is caused by us copying to aliases of elements of the pt_regs
structure. Make those explicit.
This adds one extra __get_user and unrolls a loop.
Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
arch/powerpc/kernel/signal_64.c | 24 ++++++++++++------------
1 files changed, 12 insertions(+), 12 deletions(-)
This has been build tested for ppc64_defconfig.
diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/signal_64.c
index da7c058..8af35c5 100644
--- a/arch/powerpc/kernel/signal_64.c
+++ b/arch/powerpc/kernel/signal_64.c
@@ -137,29 +137,29 @@ static long restore_sigcontext(struct pt_regs *regs, sigset_t *set, int sig,
#endif
unsigned long err = 0;
unsigned long save_r13 = 0;
- elf_greg_t *gregs = (elf_greg_t *)regs;
unsigned long msr;
- int i;
/* If this is not a signal return, we preserve the TLS in r13 */
if (!sig)
save_r13 = regs->gpr[13];
- /* copy everything before MSR */
- err |= __copy_from_user(regs, &sc->gp_regs,
- PT_MSR*sizeof(unsigned long));
-
+ /* copy the GPRs */
+ err |= __copy_from_user(regs->gpr, sc->gp_regs, sizeof(regs->gpr));
+ err |= __get_user(regs->nip, &sc->gp_regs[PT_NIP]);
/* get MSR separately, transfer the LE bit if doing signal return */
err |= __get_user(msr, &sc->gp_regs[PT_MSR]);
if (sig)
regs->msr = (regs->msr & ~MSR_LE) | (msr & MSR_LE);
-
+ err |= __get_user(regs->orig_gpr3, &sc->gp_regs[PT_ORIG_R3]);
+ err |= __get_user(regs->ctr, &sc->gp_regs[PT_CTR]);
+ err |= __get_user(regs->link, &sc->gp_regs[PT_LNK]);
+ err |= __get_user(regs->xer, &sc->gp_regs[PT_XER]);
+ err |= __get_user(regs->ccr, &sc->gp_regs[PT_CCR]);
/* skip SOFTE */
- for (i = PT_MSR+1; i <= PT_RESULT; i++) {
- if (i == PT_SOFTE)
- continue;
- err |= __get_user(gregs[i], &sc->gp_regs[i]);
- }
+ err |= __get_user(regs->trap, &sc->gp_regs[PT_TRAP]);
+ err |= __get_user(regs->dar, &sc->gp_regs[PT_DAR]);
+ err |= __get_user(regs->dsisr, &sc->gp_regs[PT_DSISR]);
+ err |= __get_user(regs->result, &sc->gp_regs[PT_RESULT]);
if (!sig)
regs->gpr[13] = save_r13;
--
1.5.6
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/
^ permalink raw reply related
* Re: [PATCH] ibm_newemac: Fixes entry of short packets
From: SathyaNarayanan @ 2008-06-27 6:36 UTC (permalink / raw)
To: benh; +Cc: linuxppc-dev, Stefan Roese, netdev
In-Reply-To: <1214263257.8011.278.camel@pasglop>
[-- Attachment #1: Type: text/plain, Size: 1888 bytes --]
Hi benh,
Please find my comments inline.
On Tue, Jun 24, 2008 at 4:50 AM, Benjamin Herrenschmidt <
benh@kernel.crashing.org> wrote:
> On Mon, 2008-06-23 at 14:55 +0200, Stefan Roese wrote:
> > From: Sathya Narayanan <sathyan@teamf1.com>
> >
> > Short packets has to be discarded by the driver. So this patch addresses
> the
> > issue of discarding the short packets of size lesser then ethernet header
> > size.
>
> You are freeing the skb, why ? Shouldn't we just keep the skb in the
> ring for further rx ?
Actually , short packets are not allowed to flow through the higher layers,
If any of the layer tried to use the extra room available may hit wit crash
.
Since it is a invalid packet it has to be dropped and freed in driver.
Actually if you see in code, the other invalid packets are also handelled
similar.
>
> > Signed-off-by: Sathya Narayanan <sathyan@teamf1.com>
> > Signed-off-by: Stefan Roese <sr@denx.de>
> > ---
> > drivers/net/ibm_newemac/core.c | 7 +++++++
> > 1 files changed, 7 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/net/ibm_newemac/core.c
> b/drivers/net/ibm_newemac/core.c
> > index 6dfc2c9..aa407b2 100644
> > --- a/drivers/net/ibm_newemac/core.c
> > +++ b/drivers/net/ibm_newemac/core.c
> > @@ -1652,6 +1652,13 @@ static int emac_poll_rx(void *param, int budget)
> >
> > skb_put(skb, len);
> > push_packet:
> > + if (skb->len < ETH_HLEN) {
> > + dev_kfree_skb(skb);
> > + printk(KERN_WARNING "%s: short packets dropped\n",
> > + dev->ndev->name);
> > + ++dev->estats.rx_dropped_stack;
> > + goto next;
> > + }
> > skb->dev = dev->ndev;
> > skb->protocol = eth_type_trans(skb, dev->ndev);
> > emac_rx_csum(dev, skb, ctrl);
>
>
[-- Attachment #2: Type: text/html, Size: 3259 bytes --]
^ 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