Hi Masahiro,
On 20 February 2015 at 05:12, Masahiro Yamada yamada.m@jp.panasonic.com wrote:
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.
In the first case you could just a local variable as is done elsewhere in the code:
const void *blob = gd->fdt_blob;
Then use 'blob'.
In the second case, you could easily restructure your loop to remove the duplication. But really you should use the alias function in fdtdec until you convert to driver model.
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.
Even now you don't need to do that, as above.
+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.
The purpose is that we then know which drivers are directly looking at the device tree. As we add driver model support, we remove these strings. When the table is empty, we can remove all that support from fdtdec, including aliases.
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/
This is up to you.
Bu why? It allows people to make adjustments if needed. The numbering of devices is an important thing in U-Boot - most commands use this although I would like one day to allow devices to be named (just use dev->name with device tree).
Anyway this is up to you.
Regards, Simon