
Hi Simon,
On Tue, 17 Feb 2015 22:01:53 -0700 Simon Glass sjg@chromium.org wrote:
+#include <common.h> +#include <linux/err.h> +#include <usb.h> +#include <fdtdec.h> +#include "xhci.h"
+DECLARE_GLOBAL_DATA_PTR;
+#define FDT gd->fdt_blob
Ick, please don't do this. Just use a local variable if you like.
+#define COMPAT "panasonic,uniphier-xhci"
This too.
Why? Each of them appears twice in the code.
Also, if I directly write the strings here, the line exceeds 80 columns and the code gets unreadable for line-wrapping.
Anyway, this is not a big deal. You have already posted DM USB patch series. Once it is merged into the mainline, I will not have to scan a device tree in my own driver.
+static int get_uniphier_xhci_base(int index, struct xhci_hccr **base) +{
int offset;
for (offset = fdt_node_offset_by_compatible(FDT, 0, COMPAT);
offset >= 0;
offset = fdt_node_offset_by_compatible(FDT, offset, COMPAT)) {
if (index == 0) {
*base = (struct xhci_hccr *)
fdtdec_get_addr(FDT, offset, "reg");
return 0;
}
index--;
}
How about this"
count = fdtdec_find_aliasses_for_id(gd->fdt_blob, "usb", COMPAT_PANASONIC_XHCI, node_list, 4); if (index >= count) return -ENOENT; offset = node_list[index];
then aliases will work. You need to ad the COMPAT to include/fdtdec.h/c. See ehci-tegra.c for an example.
I do not like this idea.
fdt_compat_id is one of what I think should be removed asap because:
- The two separete tables, enum fdt_compat_id and compat_names[], must be maintained in sync. If someone modifies just one of them, it gets broken. Anybody who checks those two tables are arranged in the same order?
- It seems ridiculous to collect many devices from various platforms into the common place. If everyone starts to follow this way, the table grows too big soon.
Moreover, I am not happy about adding aliases to my device tree again.
Rather, I am searching for an idea to avoid adding an alias to every device, like this. http://patchwork.ozlabs.org/patch/441922/
Best Regards Masahiro Yamada