[U-Boot] [PATCH 11/14] usb: UniPhier: add UniPhier on-chip xHCI host driver support

[Masahiro Yamada]

Hi Simon,



On Tue, 17 Feb 2015 22:01:53 -0700
Simon Glass <sjg at 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