* Re: Fix Firmware class name collision
From: Scott Wood @ 2007-12-04 23:52 UTC (permalink / raw)
To: Timur Tabi; +Cc: PowerPC dev list, Greg Kroah-Hartman, Markus Rechberger
In-Reply-To: <4755E6AC.9020808@freescale.com>
Timur Tabi wrote:
>
> In other words, the only thing you get is the first letter of the device name.
> You used to get the whole name. The physical address obviously isn't very helpful.
The physical address certainly is useful when you have more than one
device of the same name.
> Now, there are two solutions:
>
> 1) Change the PowerPC device names from physical_address.device_name to
> device_name.physical_address (so that e0102400.ucc becomes ucc.e0102400)
So then you'd get "firmware-ucc.e01024". What if there's another ucc at
e0102480? For devices with longer names, you'd have even less
precision in the address.
-Scott
^ permalink raw reply
* Re: ucc_uart: add support for Freescale QUICCEngine UART
From: Timur Tabi @ 2007-12-04 23:47 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: linuxppc-dev
In-Reply-To: <200712050026.07616.arnd@arndb.de>
Arnd Bergmann wrote:
> You can argue that the QS is really a DMA device, but in that case you
> should convert the driver to use the DMA mapping interfaces correctly,
> which I would consider overkill.
I'm confused. I'm already calling dma_alloc_coherent() and getting a dma_addr_t
back. Why do I need to use mapping functions to convert between virtual and
physical/bus addresses?
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply
* Fix Firmware class name collision
From: Timur Tabi @ 2007-12-04 23:45 UTC (permalink / raw)
To: Markus Rechberger, Greg Kroah-Hartman, PowerPC dev list
Markus and Greg,
I've found a problem with this patch that probably affects a number of embedded
PowerPC systems:
http://marc.info/?l=linux-kernel&m=119222892713518&w=2
The problem is that the device name for many PowerPC SoC devices is based on the
physical address. So we have stuff like this:
# ls -l /sys/devices/
drwxr-xr-x 9 root root 0 Nov 9 17:29 e0000000.soc8323
drwxr-xr-x 11 root root 0 Nov 9 17:22 e0100000.qe
[snip]
# ls -l /sys/devices/e0000000.soc8323/
drwxr-xr-x 2 root root 0 Nov 9 17:34 e0000200.wdt
drwxr-xr-x 2 root root 0 Nov 9 17:34 e0000700.pic
drwxr-xr-x 2 root root 0 Nov 9 17:34 e0001400.par_io
drwxr-xr-x 2 root root 0 Nov 9 17:34 e0003000.i2c
drwxr-xr-x 2 root root 0 Nov 9 17:34 e0004500.serial
drwxr-xr-x 2 root root 0 Nov 9 17:34 e0004600.serial
drwxr-xr-x 2 root root 0 Nov 9 17:34 e0030000.crypto
[snip]
With this patch, the device names in /sys/class/firmware look like this:
# ls -l /sys/class/firmware/
drwxr-xr-x 2 root root 0 Nov 9 17:37 firmware-e0102400.u
-rw-r--r-- 1 root root 4096 Nov 9 17:22 timeout
In other words, the only thing you get is the first letter of the device name.
You used to get the whole name. The physical address obviously isn't very helpful.
The problem is the size of the string is only 20 characters:
static inline void fw_setup_device_id(struct device *f_dev, struct device *dev)
{
snprintf(f_dev->bus_id, BUS_ID_SIZE, "firmware-%s", dev->bus_id);
}
Now, there are two solutions:
1) Change the PowerPC device names from physical_address.device_name to
device_name.physical_address (so that e0102400.ucc becomes ucc.e0102400)
2) Change fw_setup_device_id() to something like this:
snprintf(f_dev->bus_id, BUS_ID_SIZE, "fw-%s", dev->bus_id);
which do you think is a better idea?
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply
* Re: ucc_uart: add support for Freescale QUICCEngine UART
From: Scott Wood @ 2007-12-04 23:44 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: linuxppc-dev, Timur Tabi
In-Reply-To: <200712050039.39153.arnd@arndb.de>
Arnd Bergmann wrote:
> On Wednesday 05 December 2007, Scott Wood wrote:
>>> You can argue that the QS is really a DMA device, but in that
>>> case you should convert the driver to use the DMA mapping
>>> interfaces correctly, which I would consider overkill.
>> Why is it overkill?
>>
>
> Well, if the QE can never be used with an IOMMU anyway,
I'm unconvinced that that will always be the case.
> The DMA mapping API is meant for the cases where physical and dma
> addresses can be different in the first place.
It's also used for noncoherent DMA; while it's unlikely Freescale will
come out with a QE 8xx chip any time soon, there could be hardware bugs
or performance considerations that make it desireable to treat it as
non-coherent.
-Scott
^ permalink raw reply
* dtc: Implement path references
From: David Gibson @ 2007-12-04 23:43 UTC (permalink / raw)
To: Jon Loeliger; +Cc: linuxppc-dev
This patch extends dtc syntax to allow references (&label, or
&{/full/path}) directly within property definitions, rather than
inside a cell list. Such references are expanded to the full path of
the referenced node, as a string, instead of to a phandle as
references within cell lists are evaluated.
A testcase is also included.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Index: dtc/tests/run_tests.sh
===================================================================
--- dtc.orig/tests/run_tests.sh 2007-12-05 09:48:12.000000000 +1100
+++ dtc/tests/run_tests.sh 2007-12-05 10:31:51.000000000 +1100
@@ -148,6 +148,9 @@
run_test dtc.sh -I dts -O dtb -o dtc_references_dts0.test.dtb references_dts0.dts
run_test references dtc_references_dts0.test.dtb
+ run_test dtc.sh -I dts -O dtb -o dtc_path-references.test.dtb path-references.dts
+ run_test path-references dtc_path-references.test.dtb
+
# Check -Odts mode preserve all dtb information
for tree in test_tree1.dtb dtc_tree1.test.dtb dtc_escapes.test.dtb ; do
run_test dtc.sh -I dtb -O dts -o odts_$tree.test.dts $tree
Index: dtc/dtc-parser.y
===================================================================
--- dtc.orig/dtc-parser.y 2007-12-05 10:18:26.000000000 +1100
+++ dtc/dtc-parser.y 2007-12-05 10:31:51.000000000 +1100
@@ -192,6 +192,10 @@
{
$$ = data_merge($1, $3);
}
+ | propdataprefix DT_REF
+ {
+ $$ = data_add_marker($1, REF_PATH, $2);
+ }
| propdata DT_LABEL
{
$$ = data_add_marker($1, LABEL, $2);
Index: dtc/dtc.h
===================================================================
--- dtc.orig/dtc.h 2007-12-05 09:02:20.000000000 +1100
+++ dtc/dtc.h 2007-12-05 10:32:26.000000000 +1100
@@ -104,6 +104,7 @@
/* Data blobs */
enum markertype {
REF_PHANDLE,
+ REF_PATH,
LABEL,
};
@@ -139,6 +140,8 @@
struct data data_copy_file(FILE *f, size_t len);
struct data data_append_data(struct data d, const void *p, int len);
+struct data data_insert_at_marker(struct data d, struct marker *m,
+ const void *p, int len);
struct data data_merge(struct data d1, struct data d2);
struct data data_append_cell(struct data d, cell_t word);
struct data data_append_re(struct data d, const struct fdt_reserve_entry *re);
Index: dtc/tests/Makefile.tests
===================================================================
--- dtc.orig/tests/Makefile.tests 2007-12-03 15:33:10.000000000 +1100
+++ dtc/tests/Makefile.tests 2007-12-05 10:31:51.000000000 +1100
@@ -9,7 +9,7 @@
sw_tree1 \
move_and_save mangle-layout \
open_pack rw_tree1 setprop del_property del_node \
- string_escapes references \
+ string_escapes references path-references \
dtbs_equal_ordered
LIB_TESTS = $(LIB_TESTS_L:%=$(TESTS_PREFIX)%)
Index: dtc/tests/path-references.c
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ dtc/tests/path-references.c 2007-12-05 10:31:51.000000000 +1100
@@ -0,0 +1,83 @@
+/*
+ * libfdt - Flat Device Tree manipulation
+ * Testcase for string references in dtc
+ * Copyright (C) 2006 David Gibson, IBM Corporation.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public License
+ * as published by the Free Software Foundation; either version 2.1 of
+ * the License, or (at your option) any later version.
+ *
+ * This library 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
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+#include <stdint.h>
+
+#include <fdt.h>
+#include <libfdt.h>
+
+#include "tests.h"
+#include "testdata.h"
+
+void check_ref(const void *fdt, int node, const char *checkpath)
+{
+ const char *p;
+ int len;
+
+ p = fdt_getprop(fdt, node, "ref", &len);
+ if (!p)
+ FAIL("fdt_getprop(%d, \"ref\"): %s", node, fdt_strerror(len));
+ if (!streq(p, checkpath))
+ FAIL("'ref' in node at %d has value \"%s\" instead of \"%s\"",
+ node, p, checkpath);
+
+ p = fdt_getprop(fdt, node, "lref", &len);
+ if (!p)
+ FAIL("fdt_getprop(%d, \"lref\"): %s", node, fdt_strerror(len));
+ if (!streq(p, checkpath))
+ FAIL("'lref' in node at %d has value \"%s\" instead of \"%s\"",
+ node, p, checkpath);
+}
+
+int main(int argc, char *argv[])
+{
+ void *fdt;
+ const char *p;
+ int len, multilen;
+ int n1, n2;
+
+ test_init(argc, argv);
+ fdt = load_blob_arg(argc, argv);
+
+ n1 = fdt_path_offset(fdt, "/node1");
+ if (n1 < 0)
+ FAIL("fdt_path_offset(/node1): %s", fdt_strerror(n1));
+ n2 = fdt_path_offset(fdt, "/node2");
+ if (n2 < 0)
+ FAIL("fdt_path_offset(/node2): %s", fdt_strerror(n2));
+
+ check_ref(fdt, n1, "/node2");
+ check_ref(fdt, n2, "/node1");
+
+ /* Check multiple reference */
+ multilen = strlen("/node1") + strlen("/node2") + 2;
+ p = fdt_getprop(fdt, 0, "multiref", &len);
+ if (!p)
+ FAIL("fdt_getprop(0, \"multiref\"): %s", fdt_strerror(len));
+ if (len != multilen)
+ FAIL("multiref has wrong length, %d instead of %d",
+ len, multilen);
+ if ((!streq(p, "/node1") || !streq(p + strlen("/node1") + 1, "/node2")))
+ FAIL("multiref has wrong value");
+
+ PASS();
+}
Index: dtc/tests/path-references.dts
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ dtc/tests/path-references.dts 2007-12-05 10:35:55.000000000 +1100
@@ -0,0 +1,14 @@
+/dts-v1/;
+
+/ {
+ /* Check multiple references case */
+ multiref = &n1 , &n2;
+ n1: node1 {
+ ref = &{/node2}; /* reference precedes target */
+ lref = &n2;
+ };
+ n2: node2 {
+ ref = &{/node1}; /* reference after target */
+ lref = &n1;
+ };
+};
Index: dtc/data.c
===================================================================
--- dtc.orig/data.c 2007-12-05 09:02:20.000000000 +1100
+++ dtc/data.c 2007-12-05 10:34:01.000000000 +1100
@@ -202,6 +202,21 @@
return d;
}
+struct data data_insert_at_marker(struct data d, struct marker *m,
+ const void *p, int len)
+{
+ d = data_grow_for(d, len);
+ memmove(d.val + m->offset + len, d.val + m->offset, d.len - m->offset);
+ memcpy(d.val + m->offset, p, len);
+ d.len += len;
+
+ /* Adjust all markers after the one we're inserting at */
+ m = m->next;
+ for_each_marker(m)
+ m->offset += len;
+ return d;
+}
+
struct data data_append_markers(struct data d, struct marker *m)
{
struct marker **mp = &d.markers;
Index: dtc/checks.c
===================================================================
--- dtc.orig/checks.c 2007-12-05 09:48:12.000000000 +1100
+++ dtc/checks.c 2007-12-05 10:33:50.000000000 +1100
@@ -302,11 +302,36 @@
CHECK(phandle_references, NULL, NULL, fixup_phandle_references, NULL, ERROR,
&duplicate_node_names, &explicit_phandles);
+static void fixup_path_references(struct check *c, struct node *dt,
+ struct node *node, struct property *prop)
+{
+ struct marker *m = prop->val.markers;
+ struct node *refnode;
+ char *path;
+
+ for_each_marker_of_type(m, REF_PATH) {
+ assert(m->offset <= prop->val.len);
+
+ refnode = get_node_by_ref(dt, m->ref);
+ if (!refnode) {
+ FAIL(c, "Reference to non-existent node or label \"%s\"\n",
+ m->ref);
+ continue;
+ }
+
+ path = refnode->fullpath;
+ prop->val = data_insert_at_marker(prop->val, m, path,
+ strlen(path) + 1);
+ }
+}
+CHECK(path_references, NULL, NULL, fixup_path_references, NULL, ERROR,
+ &duplicate_node_names);
+
static struct check *check_table[] = {
&duplicate_node_names, &duplicate_property_names,
&name_is_string, &name_properties,
&explicit_phandles,
- &phandle_references,
+ &phandle_references, &path_references,
};
int check_semantics(struct node *dt, int outversion, int boot_cpuid_phys);
--
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: ucc_uart: add support for Freescale QUICCEngine UART
From: Arnd Bergmann @ 2007-12-04 23:39 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev, Timur Tabi
In-Reply-To: <4755E3A6.80704@freescale.com>
On Wednesday 05 December 2007, Scott Wood wrote:
>
> > You can argue that the QS is really a DMA device, but in that case you
> > should convert the driver to use the DMA mapping interfaces correctly,
> > which I would consider overkill.
>
> Why is it overkill?
>
Well, if the QE can never be used with an IOMMU anyway, you don't
lose any functionality by considering it a physical address instead of
a DMA address.
The DMA mapping API is meant for the cases where physical and dma
addresses can be different in the first place.
Arnd <><
^ permalink raw reply
* Re: [FDT][PATCH] Print out the total size as part of ftdump
From: Kumar Gala @ 2007-12-04 23:38 UTC (permalink / raw)
To: David Gibson; +Cc: linuxppc-dev, Jon Loeliger
In-Reply-To: <20071204222047.GA5771@localhost.localdomain>
On Dec 4, 2007, at 4:20 PM, David Gibson wrote:
> On Tue, Dec 04, 2007 at 10:33:20AM -0600, Kumar Gala wrote:
>> Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
>
> You know, the whole batch of fprintf()s of header information in
> dt_from_blob() are really just a debugging hack that uglifies dtc's
> output. The whole lot could be moved over to ftdump instead, which
> is, after all, *supposed* to be a debugging hack.
Yeah, I realized having a way to dump just the header info was nice.
- k
^ permalink raw reply
* [FDT][PATCH] Fix padding options
From: Kumar Gala @ 2007-12-04 23:36 UTC (permalink / raw)
To: Jon Loeliger; +Cc: linuxppc-dev
"Add an option to pad the blob that is generated" broke the padding
support. We were updating the fdt header after writing it.
Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
---
Better commit message for this, since my commit ID is different.
flattree.c | 12 +++++++-----
1 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/flattree.c b/flattree.c
index c860b63..eb9c82c 100644
--- a/flattree.c
+++ b/flattree.c
@@ -399,6 +399,12 @@ void dt_to_blob(FILE *f, struct boot_info *bi, int version,
if (padsize > 0)
padlen = padsize;
+ if (padlen > 0) {
+ int tsize = be32_to_cpu(fdt.totalsize);
+ tsize += padlen;
+ fdt.totalsize = cpu_to_be32(tsize);
+ }
+
/*
* Assemble the blob: start with the header, add with alignment
* the reserve buffer, add the reserve map terminating zeroes,
@@ -414,12 +420,8 @@ void dt_to_blob(FILE *f, struct boot_info *bi, int version,
/*
* If the user asked for more space than is used, pad out the blob.
*/
- if (padlen > 0) {
- int tsize = be32_to_cpu(fdt.totalsize);
- tsize += padlen;
+ if (padlen > 0)
blob = data_append_zeroes(blob, padlen);
- fdt.totalsize = cpu_to_be32(tsize);
- }
fwrite(blob.val, blob.len, 1, f);
--
1.5.3.4
^ permalink raw reply related
* Re: ucc_uart: add support for Freescale QUICCEngine UART
From: Scott Wood @ 2007-12-04 23:32 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: linuxppc-dev, Timur Tabi
In-Reply-To: <200712050026.07616.arnd@arndb.de>
Arnd Bergmann wrote:
> From a code clarity perspective, the interesting point is that dma_addr_t is
> what comes back from the functions in dma-mapping.h. If you don't use them,
> a physical address is phys_addr_t.
>
> You can argue that the QS is really a DMA device, but in that case you
> should convert the driver to use the DMA mapping interfaces correctly,
> which I would consider overkill.
Why is it overkill?
-Scott
^ permalink raw reply
* dtc: Generate useful error message for properties after subnodes
From: David Gibson @ 2007-12-04 23:27 UTC (permalink / raw)
To: Jon Loeliger; +Cc: linuxppc-dev
On several occasions, I've accidentally put properties after subnodes
in a dts file. I've then spent ages thinking that the resulting
syntax error was because of something else.
This patch arranges for this specific syntax error to generate a more
specific and useful error message.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Index: dtc/tests/prop-after-subnode.dts
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ dtc/tests/prop-after-subnode.dts 2007-12-05 10:24:52.000000000 +1100
@@ -0,0 +1,9 @@
+/dts-v1/;
+
+/ {
+ node1 {
+ };
+ prop;
+ node2 {
+ };
+};
Index: dtc/dtc-parser.y
===================================================================
--- dtc.orig/dtc-parser.y 2007-12-05 10:12:10.000000000 +1100
+++ dtc/dtc-parser.y 2007-12-05 10:18:26.000000000 +1100
@@ -276,6 +276,11 @@
{
$$ = chain_node($1, $2);
}
+ | subnode propdef
+ {
+ yyerror("syntax error: properties must precede subnodes\n");
+ YYERROR;
+ }
;
subnode:
--
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: ucc_uart: add support for Freescale QUICCEngine UART
From: Arnd Bergmann @ 2007-12-04 23:26 UTC (permalink / raw)
To: Timur Tabi; +Cc: linuxppc-dev
In-Reply-To: <4755D73D.7040204@freescale.com>
On Tuesday 04 December 2007, Timur Tabi wrote:
> When I program the DMA controller, I give it a dma_addr_t. =A0And yet, th=
e DMA=20
> controller and the QE are both devices on the SoC. =A0So if the DMA contr=
oller=20
> takes a dma_addr_t, then shouldn't the QE also take one?
>=20
=46rom a code clarity perspective, the interesting point is that dma_addr_t=
is
what comes back from the functions in dma-mapping.h. If you don't use them,
a physical address is phys_addr_t.
You can argue that the QS is really a DMA device, but in that case you
should convert the driver to use the DMA mapping interfaces correctly,
which I would consider overkill.
Arnd <><
^ permalink raw reply
* Re: [PATCH v2] Fix hardware IRQ time accounting problem.
From: Jörg Sommer @ 2007-12-04 22:21 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <20071204055144.GR24243@bakeyournoodle.com>
Hallo Tony,
Tony Breeds <tony@bakeyournoodle.com> wrote:
> The commit fa13a5a1f25f671d084d8884be96fc48d9b68275 (sched: restore
> deterministic CPU accounting on powerpc), unconditionally calls
> update_process_tick() in system context. In the deterministic accounting case
> this is the correct thing to do. However, in the non-deterministic accounting
> case we need to not do this, and results in the time accounted as hardware irq
> time being artificially elevated.
>
> Signed-off-by: Tony Breeds <tony@bakeyournoodle.com>
> ---
> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
> index 41e13f4..b9d8837 100644
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -350,7 +350,7 @@ struct task_struct *__switch_to(struct task_struct *prev,
> local_irq_save(flags);
>
> account_system_vtime(current);
> - account_process_tick(current, 0);
> + account_process_vtime(current);
> calculate_steal_time();
This patch works for me. The high hardware interrupt load is gone. Thanks.
Bye, Jörg.
--
Der Klügere gibt so lange nach bis er der Dumme ist.
^ permalink raw reply
* dtc: Trivial lexer cleanups
From: David Gibson @ 2007-12-04 22:50 UTC (permalink / raw)
To: Jon Loeliger; +Cc: linuxppc-dev
This patch applies a couple of tiny cleanups to the lexer. The
not-very-useful 'WS' named pattern is removed, and the debugging
printf() for single character tokens is moved to the top of the
action, which results in less confusing output when LEXDEBUG is
switched on (because it goes before the printf()s for possible
resulting lexer state changes).
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Index: dtc/dtc-lexer.l
===================================================================
--- dtc.orig/dtc-lexer.l 2007-11-22 17:29:51.000000000 +1100
+++ dtc/dtc-lexer.l 2007-11-22 17:30:55.000000000 +1100
@@ -29,7 +29,6 @@ PROPNODECHAR [a-zA-Z0-9,._+*#?@-]
PATHCHAR ({PROPNODECHAR}|[/])
LEGACYPATHCHAR [a-zA-Z0-9_@/]
LABEL [a-zA-Z_][a-zA-Z0-9_]*
-WS [[:space:]]
%{
#include "dtc.h"
@@ -193,7 +192,7 @@ static int dts_version; /* = 0 */
}
-<*>{WS}+ /* eat whitespace */
+<*>[[:space:]]+ /* eat whitespace */
<*>"/*"([^*]|\*+[^*/])*\*+"/" {
yylloc.filenum = srcpos_filenum;
@@ -207,6 +206,8 @@ static int dts_version; /* = 0 */
<*>. {
yylloc.filenum = srcpos_filenum;
yylloc.first_line = yylineno;
+ DPRINT("Char: %c (\\x%02x)\n", yytext[0],
+ (unsigned)yytext[0]);
if (yytext[0] == '[') {
DPRINT("<BYTESTRING>\n");
BEGIN(BYTESTRING);
@@ -216,9 +217,6 @@ static int dts_version; /* = 0 */
DPRINT("<PROPNODENAME>\n");
BEGIN(PROPNODENAME);
}
- DPRINT("Char: %c (\\x%02x)\n", yytext[0],
- (unsigned)yytext[0]);
-
return yytext[0];
}
--
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
* dtc: Convert "name" property checking to new infrastructure
From: David Gibson @ 2007-12-04 22:40 UTC (permalink / raw)
To: Jon Loeliger; +Cc: linuxppc-dev
This patch removes the old-style checking code for the "name" property
- i.e. verifying that the "name" property, if present, matches the
node name. It replaces it with a pair of more-or-less equivalent
checks in the new checking framework.
This also promotes this check to a "structural" check, or at least an
error-rather-than-warning test, since the structural/semantic
distinction doesn't really apply in the new framework.
A testcase for the check is also added.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Index: dtc/checks.c
===================================================================
--- dtc.orig/checks.c 2007-12-04 14:14:41.000000000 +1100
+++ dtc/checks.c 2007-12-04 14:41:46.000000000 +1100
@@ -170,6 +170,27 @@ out:
}
/*
+ * Utility check functions
+ */
+
+static void check_is_string(struct check *c, struct node *root,
+ struct node *node)
+{
+ struct property *prop;
+ char *propname = c->data;
+
+ prop = get_property(node, propname);
+ if (!prop)
+ return; /* Not present, assumed ok */
+
+ if (!data_is_one_string(prop->val))
+ FAIL(c, "\"%s\" property in %s is not a string",
+ propname, node->fullpath);
+}
+#define CHECK_IS_STRING(nm, propname, lvl) \
+ CHECK(nm, NULL, check_is_string, NULL, (propname), (lvl))
+
+/*
* Structural check functions
*/
@@ -236,6 +257,23 @@ static void check_explicit_phandles(stru
}
NODE_CHECK(explicit_phandles, NULL, ERROR);
+static void check_name_properties(struct check *c, struct node *root,
+ struct node *node)
+{
+ struct property *prop;
+
+ prop = get_property(node, "name");
+ if (!prop)
+ return; /* No name property, that's fine */
+
+ if ((prop->val.len != node->basenamelen+1)
+ || (memcmp(prop->val.val, node->name, node->basenamelen) != 0))
+ FAIL(c, "\"name\" property in %s is incorrect (\"%s\" instead"
+ " of base node name)", node->fullpath, prop->val.val);
+}
+CHECK_IS_STRING(name_is_string, "name", ERROR);
+NODE_CHECK(name_properties, NULL, ERROR, &name_is_string);
+
/*
* Reference fixup functions
*/
@@ -266,6 +304,7 @@ CHECK(phandle_references, NULL, NULL, fi
static struct check *check_table[] = {
&duplicate_node_names, &duplicate_property_names,
+ &name_is_string, &name_properties,
&explicit_phandles,
&phandle_references,
};
@@ -350,25 +389,10 @@ static int must_be_string(struct propert
return 1;
}
-static int name_prop_check(struct property *prop, struct node *node)
-{
- if ((prop->val.len != node->basenamelen+1)
- || !strneq(prop->val.val, node->name, node->basenamelen)) {
- ERRMSG("name property \"%s\" does not match node basename in %s\n",
- prop->val.val,
- node->fullpath);
- return 0;
- }
-
- return 1;
-}
-
static struct {
char *propname;
int (*check_fn)(struct property *prop, struct node *node);
} prop_checker_table[] = {
- {"name", must_be_string},
- {"name", name_prop_check},
{"linux,phandle", must_be_one_cell},
{"#address-cells", must_be_one_cell},
{"#size-cells", must_be_one_cell},
Index: dtc/tests/bad-name-property.dts
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ dtc/tests/bad-name-property.dts 2007-12-04 14:28:54.000000000 +1100
@@ -0,0 +1,7 @@
+/dts-v1/;
+
+/ {
+ node@0 {
+ name = "badthing";
+ };
+};
Index: dtc/tests/run_tests.sh
===================================================================
--- dtc.orig/tests/run_tests.sh 2007-12-04 14:29:14.000000000 +1100
+++ dtc/tests/run_tests.sh 2007-12-04 14:29:27.000000000 +1100
@@ -163,6 +163,7 @@ dtc_tests () {
run_test dtc-checkfails.sh -I dts -O dtb minusone-phandle.dts
run_test dtc-checkfails.sh -I dts -O dtb nonexist-node-ref.dts
run_test dtc-checkfails.sh -I dts -O dtb nonexist-label-ref.dts
+ run_test dtc-checkfails.sh -I dts -O dtb bad-name-property.dts
}
while getopts "vt:m" ARG ; do
--
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: ucc_uart: add support for Freescale QUICCEngine UART
From: Timur Tabi @ 2007-12-04 22:39 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: linuxppc-dev
In-Reply-To: <200712042313.58252.arnd@arndb.de>
Arnd Bergmann wrote:
> I'm guessing that you don't really mean dma_addr_t here, but rather
> phys_addr_t, which is something different.
Now that I think about it, I don't know which is correct. The value is plugged
into the pointer register of a buffer descriptor, and the QE performs a DMA-like
memory transfer from that address into its local memory. I don't know if the QE
is considered "external" enough that the address is a DMA address or a physical
address.
When I program the DMA controller, I give it a dma_addr_t. And yet, the DMA
controller and the QE are both devices on the SoC. So if the DMA controller
takes a dma_addr_t, then shouldn't the QE also take one?
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply
* Re: Merge dtc
From: Josh Boyer @ 2007-12-04 22:34 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linuxppc-dev, Gibson, David Woodhouse, David
In-Reply-To: <18261.53985.700234.923526@cargo.ozlabs.ibm.com>
On Wed, 5 Dec 2007 09:21:21 +1100
Paul Mackerras <paulus@samba.org> wrote:
> David Woodhouse writes:
>
> > I think this is a bad idea -- it's hardly a difficult for those people
> > who _do_ need dts to obtain it separately.
>
> The trouble is that it's not just people who are making a kernel for a
> specific embedded board that need dtc. These days anyone who wants to
> try cross-compiling a powerpc kernel and does a make allyesconfig, or
> who picks cell_defconfig or ps3_defconfig to try, needs dtc if their
> kernel build is to go all the way through and give them an exit status
> of 0. I can see that for people who are trying to do the right thing
> and compile-test their patch across architectures, it's annoying that
> powerpc has an extra external requirement when no other architecture
> does, and it usually just means they don't compile-test on powerpc.
>
> Of the various options for solving this, including dtc in the kernel
> sources seems best to me.
Using that same reasoning, should we merge a mkimage patch for the
boards that use U-Boot?
(That's a serious question, not a smart-ass response)
josh
^ permalink raw reply
* dtc: Fix FAIL() macro varargs
From: David Gibson @ 2007-12-04 22:34 UTC (permalink / raw)
To: Jon Loeliger; +Cc: linuxppc-dev
The way the checking subsystem FAIL() macro is currently implemented
it must take at least one paramater after the format string. This
patch corrects the problem.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Index: dtc/checks.c
===================================================================
--- dtc.orig/checks.c 2007-12-04 16:42:48.000000000 +1100
+++ dtc/checks.c 2007-12-04 17:17:42.000000000 +1100
@@ -101,11 +101,11 @@ static inline void check_msg(struct chec
fprintf(stderr, "\n");
}
-#define FAIL(c, fmt, ...) \
+#define FAIL(c, ...) \
do { \
TRACE((c), "\t\tFAILED at %s:%d", __FILE__, __LINE__); \
(c)->status = FAILED; \
- check_msg((c), fmt, __VA_ARGS__); \
+ check_msg((c), __VA_ARGS__); \
} while (0)
static void check_nodes_props(struct check *c, struct node *dt, struct node *node)
--
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: ucc_uart: add support for Freescale QUICCEngine UART
From: Timur Tabi @ 2007-12-04 22:33 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: linuxppc-dev
In-Reply-To: <200712042313.58252.arnd@arndb.de>
Arnd Bergmann wrote:
> Can you use the driver on CPUs without this particular bug when it's built
> in Soft-UART mode?
No, you have to build with Soft-UART mode turned off. Soft-UART mode actually
uses the HMC mode of the QE and hacks it up to act like a UART.
The only way to know at runtime whether Soft-UART is required is to check the
SOC model/revision and compare it to a list.
I could add code to check this list and enable Soft-UART if when there's a known
CPU with the problem. However, I can't know whether I need to upload the
microcode. I also have a U-Boot version of qe_upload_firmware(), so it's
conceivable that U-Boot could have uploaded the microcode but I still need the
Linux driver to use Soft-UART.
I also have no control over which Soft-UART microcodes are available. It's
possible that some SOCs could have this bug but Freescale won't produce a
microcode to fix it.
> Try to reduce the number of #ifdefs in your code. In particular, you should
> not do conditional #includes and #defines, as they often lead to subtle bugs.
Ok, I can remove those #defines, but my main concern was to clearly isolate the
Soft-UART code, since it is an ugly hack. I didn't want to mesh the Soft-UART
code with the normal UART code. I don't know what more I could do to limit the
number of #ifdefs.
>> +struct ucc_uart_pram {
>> + struct ucc_slow_pram common;
>> + u8 res1[8]; /* reserved */
>> + __be16 maxidl; /* Maximum idle chars */
>> + __be16 idlc; /* temp idle counter */
>> + __be16 brkcr; /* Break count register */
>> + __be16 parec; /* receive parity error counter */
>> + __be16 frmec; /* receive framing error counter */
>> + __be16 nosec; /* receive noise counter */
>> + __be16 brkec; /* receive break condition counter */
>> + __be16 brkln; /* last received break length */
>> + __be16 uaddr[2]; /* UART address character 1 & 2 */
>> + __be16 rtemp; /* Temp storage */
>> + __be16 toseq; /* Transmit out of sequence char */
>> + __be16 cchars[8]; /* control characters 1-8 */
>> + __be16 rccm; /* receive control character mask */
>> + __be16 rccr; /* receive control character register */
>> + __be16 rlbc; /* receive last break character */
>> + __be16 res2; /* reserved */
>> + __be32 res3; /* reserved, should be cleared */
>> + u8 res4; /* reserved, should be cleared */
>> + u8 res5[3]; /* reserved, should be cleared */
>> + __be32 res6; /* reserved, should be cleared */
>> + __be32 res7; /* reserved, should be cleared */
>> + __be32 res8; /* reserved, should be cleared */
>> + __be32 res9; /* reserved, should be cleared */
>> + __be32 res10; /* reserved, should be cleared */
>> + __be32 res11; /* reserved, should be cleared */
>> + __be32 res12; /* reserved, should be cleared */
>> + __be32 res13; /* reserved, should be cleared */
>> +#ifdef CONFIG_SERIAL_QE_SOFT_UART
>> + __be16 supsmr; /* 0x90, Shadow UPSMR */
>> + __be16 res92; /* 0x92, reserved, initialize to 0 */
>> + __be32 rx_state; /* 0x94, RX state, initialize to 0 */
>> + __be32 rx_cnt; /* 0x98, RX count, initialize to 0 */
>> + u8 rx_length; /* 0x9C, Char length, set to 1+CL+PEN+1+SL */
>> + u8 rx_bitmark; /* 0x9D, reserved, initialize to 0 */
>> + u8 rx_temp_dlst_qe; /* 0x9E, reserved, initialize to 0 */
>> + u8 res14[0xBC - 0x9F]; /* reserved */
>> + __be32 dump_ptr; /* 0xBC, Dump pointer */
>> + __be32 rx_frame_rem; /* 0xC0, reserved, initialize to 0 */
>> + u8 rx_frame_rem_size; /* 0xC4, reserved, initialize to 0 */
>> + u8 tx_mode; /* 0xC5, mode, 0=AHDLC, 1=UART */
>> + u16 tx_state; /* 0xC6, TX state */
>> + u8 res15[0xD0 - 0xC8]; /* reserved */
>> + __be32 resD0; /* 0xD0, reserved, initialize to 0 */
>> + u8 resD4; /* 0xD4, reserved, initialize to 0 */
>> + __be16 resD5; /* 0xD5, reserved, initialize to 0 */
>> +#endif
>> +} __attribute__ ((packed));
>
> The structure is perfectly packed even without your __attribute__ ((packed)),
> so you should leave out the attribute in order to get more efficient code
> accessing it.
If the structure is packed either way, why would the code differ? I don't see
how the code would be more efficient if I removed "__attribute__ ((packed))".
If gcc does something differently, that's news to me.
> Do you really need the debugging function like this in the code?
> Usually they are rather pointless once the code works, and will
> suffer from bitrot because nobody enables the code.
Well, I'm hesitant to remove it because I have a feeling that if I do, the next
day I'll want it.
>> +#ifdef CONFIG_SERIAL_QE_SOFT_UART
>> +#define UCC_UART_SUPSMR_SL 0x8000
>> +#define UCC_UART_SUPSMR_RPM_MASK 0x6000
>> +#define UCC_UART_SUPSMR_RPM_ODD 0x0000
>> +#define UCC_UART_SUPSMR_RPM_LOW 0x2000
>
> again, the #ifdef should be left out if it can.
Alright.
>
>> + * Given the virtual address for a character buffer, this function returns
>> + * the physical (DMA) equivalent.
>> + */
>> +static inline dma_addr_t cpu2qe_addr(void *addr, struct uart_qe_port *qe_port)
>> +{
>> + if (likely((addr >= qe_port->bd_virt)) &&
>> + (addr < (qe_port->bd_virt + qe_port->bd_size)))
>> + return qe_port->bd_phys + (addr - qe_port->bd_virt);
>> +
>> + /* something nasty happened */
>> + printk(KERN_ERR "%s: addr=%p\n", __FUNCTION__, addr);
>> + BUG();
>> + return 0;
>> +}
>
> I'm guessing that you don't really mean dma_addr_t here, but rather
> phys_addr_t, which is something different.
Now that I think about it, I think I really mean unsigned, since the returned
value is just an offset. I need to think about that.
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply
* Re: Merge dtc
From: David Woodhouse @ 2007-12-04 22:33 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linuxppc-dev, David Gibson
In-Reply-To: <18261.53985.700234.923526@cargo.ozlabs.ibm.com>
On Wed, 2007-12-05 at 09:21 +1100, Paul Mackerras wrote:
> David Woodhouse writes:
>
> > I think this is a bad idea -- it's hardly a difficult for those people
> > who _do_ need dts to obtain it separately.
>
> The trouble is that it's not just people who are making a kernel for a
> specific embedded board that need dtc. These days anyone who wants to
> try cross-compiling a powerpc kernel and does a make allyesconfig, or
> who picks cell_defconfig or ps3_defconfig to try, needs dtc if their
> kernel build is to go all the way through and give them an exit status
> of 0. I can see that for people who are trying to do the right thing
> and compile-test their patch across architectures, it's annoying that
> powerpc has an extra external requirement when no other architecture
> does, and it usually just means they don't compile-test on powerpc.
>
> Of the various options for solving this, including dtc in the kernel
> sources seems best to me.
Make vmlinux the default target instead of zImage would seem like a
better answer. I'm surprised that it isn't like that already, in fact --
and I'm only inferring that it isn't from your message. I always thought
that if I typed 'make' I got the vmlinux and not a zImage.
--
dwmw2
^ permalink raw reply
* Re: Problem compiling sequoia using DENX kernel. Xenomai-patch required?
From: Josh Boyer @ 2007-12-04 22:29 UTC (permalink / raw)
To: niklaus.giger; +Cc: linuxppc-embedded
In-Reply-To: <200712042314.38498.niklaus.giger@member.fsf.org>
On Tue, 4 Dec 2007 23:14:38 +0100
Niklaus Giger <niklaus.giger@member.fsf.org> wrote:
> Am Dienstag, 4. Dezember 2007 schrieb Wolfgang Denk:
> <...>
> > I'm afraid "normal" here still means arch/ppc - hopefully for not
> > long any more. Note: a matching Xenomai patch for arch/ppc will be in
> > Xenomai 2.4 when it comes out in a few days.
> Thanks a lot for your explanation. I took
> ksrc/arch/powerpc/patches/adeos-ipipe-2.6.23-ppc-1.6-00.patch
> from the xenomai trunk and was able to compile and boot successfully
> the yosemite board (using ARCH=ppc). Will try the sequoaia board tomorrow.
>
> I hope that this is a good starting point to get my custom PPC440EPx board
> into shape. I think once the arch/powerpc will work for PPC440 it should not
> be a lot of work to port it from ppc -> powerpc.
Sequoia is already in arch/powerpc. At least most of the base support.
josh
^ permalink raw reply
* Re: Merge dtc
From: Paul Mackerras @ 2007-12-04 22:21 UTC (permalink / raw)
To: David Woodhouse; +Cc: linuxppc-dev, David Gibson
In-Reply-To: <1196733544.13978.201.camel@pmac.infradead.org>
David Woodhouse writes:
> I think this is a bad idea -- it's hardly a difficult for those people
> who _do_ need dts to obtain it separately.
The trouble is that it's not just people who are making a kernel for a
specific embedded board that need dtc. These days anyone who wants to
try cross-compiling a powerpc kernel and does a make allyesconfig, or
who picks cell_defconfig or ps3_defconfig to try, needs dtc if their
kernel build is to go all the way through and give them an exit status
of 0. I can see that for people who are trying to do the right thing
and compile-test their patch across architectures, it's annoying that
powerpc has an extra external requirement when no other architecture
does, and it usually just means they don't compile-test on powerpc.
Of the various options for solving this, including dtc in the kernel
sources seems best to me.
Paul.
^ permalink raw reply
* Re: [FDT][PATCH] Print out the total size as part of ftdump
From: David Gibson @ 2007-12-04 22:20 UTC (permalink / raw)
To: Kumar Gala; +Cc: linuxppc-dev, Jon Loeliger
In-Reply-To: <Pine.LNX.4.64.0712041032580.29317@blarg.am.freescale.net>
On Tue, Dec 04, 2007 at 10:33:20AM -0600, Kumar Gala wrote:
> Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
You know, the whole batch of fprintf()s of header information in
dt_from_blob() are really just a debugging hack that uglifies dtc's
output. The whole lot could be moved over to ftdump instead, which
is, after all, *supposed* to be a debugging hack.
--
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: [FDT][PATCH] Fix padding options
From: Kumar Gala @ 2007-12-04 22:18 UTC (permalink / raw)
To: David Gibson; +Cc: linuxppc-dev, Jon Loeliger
In-Reply-To: <20071204220432.GA2521@localhost.localdomain>
I guess it was really git commit
2b7dc8dce549ad72ad437b254bf756d7ba4c2a5a
Add an option to pad the blob that is generated
- k
On Dec 4, 2007, at 4:04 PM, David Gibson wrote:
> On Tue, Dec 04, 2007 at 10:27:52AM -0600, Kumar Gala wrote:
>> commit 22e787ca2b1e49a9a0f3c43262564ab1038c5c3c broke the padding
>> support. We were updating the fdt header after writing it.
>
> Uh.. I can't find a commit with that SHA1. Which one are you
> referring to?
>
> --
> 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: Problem compiling sequoia using DENX kernel. Xenomai-patch required?
From: Niklaus Giger @ 2007-12-04 22:14 UTC (permalink / raw)
To: Wolfgang Denk; +Cc: linuxppc-embedded
In-Reply-To: <20071203230816.1C502242E9@gemini.denx.de>
Am Dienstag, 4. Dezember 2007 schrieb Wolfgang Denk:
<...>
> I'm afraid "normal" here still means arch/ppc - hopefully for not
> long any more. Note: a matching Xenomai patch for arch/ppc will be in
> Xenomai 2.4 when it comes out in a few days.
Thanks a lot for your explanation. I took
ksrc/arch/powerpc/patches/adeos-ipipe-2.6.23-ppc-1.6-00.patch
from the xenomai trunk and was able to compile and boot successfully
the yosemite board (using ARCH=ppc). Will try the sequoaia board tomorrow.
I hope that this is a good starting point to get my custom PPC440EPx board
into shape. I think once the arch/powerpc will work for PPC440 it should not
be a lot of work to port it from ppc -> powerpc.
Best regards
--
Niklaus Giger
^ permalink raw reply
* Re: ucc_uart: add support for Freescale QUICCEngine UART
From: Arnd Bergmann @ 2007-12-04 22:13 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Timur Tabi
In-Reply-To: <11967907173600-git-send-email-timur@freescale.com>
On Tuesday 04 December 2007, Timur Tabi wrote:
> Add support for UART serial ports using a Freescale QUICC Engine
> (found on some MPC83xx and MPC85xx SOCs).
>
> Because of a silicon bug in some QE-enabled SOCs (e.g. 8323 and 8360), a new
> microcode is required. This microcode implements UART via a work-around,
> hence it's called "Soft-UART". This driver can use QE firmware upload feature
> to upload the correct microcode to the QE.
Can you use the driver on CPUs without this particular bug when it's built
in Soft-UART mode?
> +
> +#ifdef CONFIG_SERIAL_QE_SOFT_UART_UPLOAD
> +#include <linux/firmware.h>
> +#include <asm/reg.h>
> +#endif
> +
> +#ifdef CONFIG_SERIAL_QE_SOFT_UART
> +/*
> + * The GUMR flag for Soft UART. This would normally be defined in qe.h,
> + * but Soft-UART is a hack and we want to keep everything related to it in
> + * this file.
> + */
> +#define UCC_SLOW_GUMR_H_SUART 0x00004000 /* Soft UART */
> +
> +/*
> + * firmware_loaded is 1 if the firmware has been loaded, 0 otherwise.
> + */
> +#ifdef CONFIG_SERIAL_QE_SOFT_UART_UPLOAD
> +static int firmware_loaded;
> +#endif
Try to reduce the number of #ifdefs in your code. In particular, you should
not do conditional #includes and #defines, as they often lead to subtle bugs.
> +struct ucc_uart_pram {
> + struct ucc_slow_pram common;
> + u8 res1[8]; /* reserved */
> + __be16 maxidl; /* Maximum idle chars */
> + __be16 idlc; /* temp idle counter */
> + __be16 brkcr; /* Break count register */
> + __be16 parec; /* receive parity error counter */
> + __be16 frmec; /* receive framing error counter */
> + __be16 nosec; /* receive noise counter */
> + __be16 brkec; /* receive break condition counter */
> + __be16 brkln; /* last received break length */
> + __be16 uaddr[2]; /* UART address character 1 & 2 */
> + __be16 rtemp; /* Temp storage */
> + __be16 toseq; /* Transmit out of sequence char */
> + __be16 cchars[8]; /* control characters 1-8 */
> + __be16 rccm; /* receive control character mask */
> + __be16 rccr; /* receive control character register */
> + __be16 rlbc; /* receive last break character */
> + __be16 res2; /* reserved */
> + __be32 res3; /* reserved, should be cleared */
> + u8 res4; /* reserved, should be cleared */
> + u8 res5[3]; /* reserved, should be cleared */
> + __be32 res6; /* reserved, should be cleared */
> + __be32 res7; /* reserved, should be cleared */
> + __be32 res8; /* reserved, should be cleared */
> + __be32 res9; /* reserved, should be cleared */
> + __be32 res10; /* reserved, should be cleared */
> + __be32 res11; /* reserved, should be cleared */
> + __be32 res12; /* reserved, should be cleared */
> + __be32 res13; /* reserved, should be cleared */
> +#ifdef CONFIG_SERIAL_QE_SOFT_UART
> + __be16 supsmr; /* 0x90, Shadow UPSMR */
> + __be16 res92; /* 0x92, reserved, initialize to 0 */
> + __be32 rx_state; /* 0x94, RX state, initialize to 0 */
> + __be32 rx_cnt; /* 0x98, RX count, initialize to 0 */
> + u8 rx_length; /* 0x9C, Char length, set to 1+CL+PEN+1+SL */
> + u8 rx_bitmark; /* 0x9D, reserved, initialize to 0 */
> + u8 rx_temp_dlst_qe; /* 0x9E, reserved, initialize to 0 */
> + u8 res14[0xBC - 0x9F]; /* reserved */
> + __be32 dump_ptr; /* 0xBC, Dump pointer */
> + __be32 rx_frame_rem; /* 0xC0, reserved, initialize to 0 */
> + u8 rx_frame_rem_size; /* 0xC4, reserved, initialize to 0 */
> + u8 tx_mode; /* 0xC5, mode, 0=AHDLC, 1=UART */
> + u16 tx_state; /* 0xC6, TX state */
> + u8 res15[0xD0 - 0xC8]; /* reserved */
> + __be32 resD0; /* 0xD0, reserved, initialize to 0 */
> + u8 resD4; /* 0xD4, reserved, initialize to 0 */
> + __be16 resD5; /* 0xD5, reserved, initialize to 0 */
> +#endif
> +} __attribute__ ((packed));
The structure is perfectly packed even without your __attribute__ ((packed)),
so you should leave out the attribute in order to get more efficient code
accessing it.
> +
> +#ifdef DEBUG
> +static void dump_ucc_uart_pram(struct ucc_uart_pram __iomem *uccup)
> +{
> + unsigned int i;
Do you really need the debugging function like this in the code?
Usually they are rather pointless once the code works, and will
suffer from bitrot because nobody enables the code.
> +
> +#ifdef CONFIG_SERIAL_QE_SOFT_UART
> +#define UCC_UART_SUPSMR_SL 0x8000
> +#define UCC_UART_SUPSMR_RPM_MASK 0x6000
> +#define UCC_UART_SUPSMR_RPM_ODD 0x0000
> +#define UCC_UART_SUPSMR_RPM_LOW 0x2000
again, the #ifdef should be left out if it can.
> + * Given the virtual address for a character buffer, this function returns
> + * the physical (DMA) equivalent.
> + */
> +static inline dma_addr_t cpu2qe_addr(void *addr, struct uart_qe_port *qe_port)
> +{
> + if (likely((addr >= qe_port->bd_virt)) &&
> + (addr < (qe_port->bd_virt + qe_port->bd_size)))
> + return qe_port->bd_phys + (addr - qe_port->bd_virt);
> +
> + /* something nasty happened */
> + printk(KERN_ERR "%s: addr=%p\n", __FUNCTION__, addr);
> + BUG();
> + return 0;
> +}
I'm guessing that you don't really mean dma_addr_t here, but rather
phys_addr_t, which is something different.
> +
> +/*
> + * Physical to virtual address translation.
> + *
> + * Given the physical (DMA) address for a character buffer, this function
> + * returns the virtual equivalent.
> + */
> +static inline void *qe2cpu_addr(dma_addr_t addr, struct uart_qe_port *qe_port)
same here.
Arnd <><
^ 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