Hi Simon,
On Tue, 17 Feb 2015 22:01:53 -0700
Simon Glass <s...@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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
