[U-Boot] [PATCH 2/9] fdt: Support for ISA busses

[Paul Burton <paul.burton@imgtec.com>]

On Fri, Jan 29, 2016 at 07:56:11AM -0700, Simon Glass wrote:
> Hi Paul,
> 
> On 29 January 2016 at 06:54, Paul Burton <paul.burton@imgtec.com> wrote:
> > Support ISA busses in much the same way as Linux does. This allows for
> > ISA bus addresses to be translated, and only if CONFIG_OF_ISA_BUS is
> > selected in order to avoid including the code in builds which won't need
> > it.
> >
> > Signed-off-by: Paul Burton <paul.burton@imgtec.com>
> > ---
> >
> >  common/fdt_support.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++++++--
> >  drivers/core/Kconfig |  4 +++
> >  2 files changed, 85 insertions(+), 2 deletions(-)
> >
> > diff --git a/common/fdt_support.c b/common/fdt_support.c
> > index 75d0858..0aba77d 100644
> > --- a/common/fdt_support.c
> > +++ b/common/fdt_support.c
> > @@ -959,6 +959,7 @@ static void of_dump_addr(const char *s, const fdt32_t *addr, int na) { }
> >  struct of_bus {
> >         const char      *name;
> >         const char      *addresses;
> > +       int             (*match)(void *blob, int parentoffset);
> 
> Can you please add a comment to this method explaining what it does,
> args, return value?

Hi Simon,

Ok, will do (I'll try not to use "but the rest don't!" as an excuse).

> >         void            (*count_cells)(void *blob, int parentoffset,
> >                                 int *addrc, int *sizec);
> >         u64             (*map)(fdt32_t *addr, const fdt32_t *range,
> > @@ -1013,8 +1014,70 @@ static int of_bus_default_translate(fdt32_t *addr, u64 offset, int na)
> >         return 0;
> >  }
> >
> > +#ifdef CONFIG_OF_ISA_BUS
> > +
> > +/* ISA bus translator */
> > +static int of_bus_isa_match(void *blob, int parentoffset)
> > +{
> > +       const char *name;
> > +
> > +       name = fdt_get_name(blob, parentoffset, NULL);
> > +       if (!name)
> > +               return 0;
> > +
> > +       return !strcmp(name, "isa");
> > +}
> > +
> > +static void of_bus_isa_count_cells(void *blob, int parentoffset,
> > +                                  int *addrc, int *sizec)
> > +{
> > +       if (addrc)
> > +               *addrc = 2;
> > +       if (sizec)
> > +               *sizec = 1;
> > +}
> > +
> > +static u64 of_bus_isa_map(fdt32_t *addr, const fdt32_t *range,
> > +                         int na, int ns, int pna)
> > +{
> > +       u64 cp, s, da;
> > +
> > +       /* Check address type match */
> > +       if ((addr[0] ^ range[0]) & cpu_to_be32(1))
> > +               return OF_BAD_ADDR;
> > +
> > +       cp = of_read_number(range + 1, na - 1);
> > +       s  = of_read_number(range + na + pna, ns);
> > +       da = of_read_number(addr + 1, na - 1);
> > +
> > +       debug("OF: ISA map, cp=%" PRIu64 ", s=%" PRIu64
> > +             ", da=%" PRIu64 "\n", cp, s, da);
> > +
> > +       if (da < cp || da >= (cp + s))
> > +               return OF_BAD_ADDR;
> > +       return da - cp;
> > +}
> > +
> > +static int of_bus_isa_translate(fdt32_t *addr, u64 offset, int na)
> > +{
> > +       return of_bus_default_translate(addr + 1, offset, na - 1);
> > +}
> > +
> > +#endif /* CONFIG_OF_ISA_BUS */
> > +
> >  /* Array of bus specific translators */
> >  static struct of_bus of_busses[] = {
> > +#ifdef CONFIG_OF_ISA_BUS
> > +       /* ISA */
> > +       {
> > +               .name = "isa",
> > +               .addresses = "reg",
> > +               .match = of_bus_isa_match,
> > +               .count_cells = of_bus_isa_count_cells,
> > +               .map = of_bus_isa_map,
> > +               .translate = of_bus_isa_translate,
> > +       },
> > +#endif /* CONFIG_OF_ISA_BUS */
> >         /* Default */
> >         {
> >                 .name = "default",
> > @@ -1025,6 +1088,22 @@ static struct of_bus of_busses[] = {
> >         },
> >  };
> >
> > +static struct of_bus *of_match_bus(void *blob, int parentoffset)
> > +{
> > +       struct of_bus *bus;
> > +
> > +       if (ARRAY_SIZE(of_busses) == 1)
> > +               return &of_busses[0];
> > +
> > +       for (bus = &of_busses[0]; bus; bus++) {
> > +               if (!bus->match || bus->match(blob, parentoffset))
> > +                       return bus;
> > +       }
> > +
> > +       BUG();
> 
> What will this do? Can we propagate the error up instead?

It would mean we'd somehow not got the default bus struct (which has a
NULL bus->match) in the of_busses array. That should never happen, hence
if it does something has gone horribly wrong & there's nothing sane the
caller can do about it.

> > +       return NULL;
> > +}
> > +
> >  static int of_translate_one(void * blob, int parent, struct of_bus *bus,
> >                             struct of_bus *pbus, fdt32_t *addr,
> >                             int na, int ns, int pna, const char *rprop)
> > @@ -1104,7 +1183,7 @@ static u64 __of_translate_address(void *blob, int node_offset, const fdt32_t *in
> >         parent = fdt_parent_offset(blob, node_offset);
> >         if (parent < 0)
> >                 goto bail;
> > -       bus = &of_busses[0];
> > +       bus = of_match_bus(blob, parent);
> >
> >         /* Cound address cells & copy address locally */
> >         bus->count_cells(blob, parent, &na, &ns);
> > @@ -1133,7 +1212,7 @@ static u64 __of_translate_address(void *blob, int node_offset, const fdt32_t *in
> >                 }
> >
> >                 /* Get new parent bus and counts */
> > -               pbus = &of_busses[0];
> > +               pbus = of_match_bus(blob, parent);
> >                 pbus->count_cells(blob, parent, &pna, &pns);
> >                 if (!OF_CHECK_COUNTS(pna)) {
> >                         printf("%s: Bad cell count for %s\n", __FUNCTION__,
> > diff --git a/drivers/core/Kconfig b/drivers/core/Kconfig
> > index c5c9d2a..339f52f 100644
> > --- a/drivers/core/Kconfig
> > +++ b/drivers/core/Kconfig
> > @@ -178,4 +178,8 @@ config SPL_OF_TRANSLATE
> >           used for the address translation. This function is faster and
> >           smaller in size than fdt_translate_address().
> >
> > +config OF_ISA_BUS
> > +       bool "Support the ISA bus in fdt_translate_address"
> > +       depends on OF_TRANSLATE
> 
> Please add help here, perhaps with a small example.

Ok.

Thanks,
    Paul

> > +
> >  endmenu
> > --
> > 2.7.0
> >
> 
> Regards,
> Simon