diff mbox series

[RFC,3/6] dtoc: extend dtoc to use struct driver_info when linking nodes

Message ID 20200513201345.26769-4-walter.lozano@collabora.com
State New
Headers show
Series improve OF_PLATDATA support | expand

Commit Message

Walter Lozano May 13, 2020, 8:13 p.m. UTC
In the current implementation, when dtoc parses a dtb to generate a struct
platdata it converts the information related to linked nodes as pointers
to struct platdata of destination nodes. By doing this, it makes
difficult to get pointer to udevices created based on these
information.

This patch extends dtoc to use struct driver_info when populating
information about linked nodes, which makes it easier to later get
the devices created. In this context, reimplement functions like
clk_get_by_index_platdata() which made use of the previous approach.

Signed-off-by: Walter Lozano <walter.lozano at collabora.com>
---
 drivers/clk/clk-uclass.c            |  8 +++-----
 drivers/misc/irq-uclass.c           |  4 ++--
 drivers/mmc/ftsdc010_mci.c          |  2 +-
 drivers/mmc/rockchip_dw_mmc.c       |  2 +-
 drivers/mmc/rockchip_sdhci.c        |  2 +-
 drivers/ram/rockchip/sdram_rk3399.c |  2 +-
 drivers/spi/rk_spi.c                |  2 +-
 include/clk.h                       |  2 +-
 tools/dtoc/dtb_platdata.py          | 17 ++++++++++++++---
 9 files changed, 25 insertions(+), 16 deletions(-)

Comments

Simon Glass May 20, 2020, 3:07 a.m. UTC | #1
Hi Walter,

On Wed, 13 May 2020 at 14:14, Walter Lozano <walter.lozano at collabora.com> wrote:
>
> In the current implementation, when dtoc parses a dtb to generate a struct
> platdata it converts the information related to linked nodes as pointers
> to struct platdata of destination nodes. By doing this, it makes
> difficult to get pointer to udevices created based on these
> information.
>
> This patch extends dtoc to use struct driver_info when populating
> information about linked nodes, which makes it easier to later get
> the devices created. In this context, reimplement functions like
> clk_get_by_index_platdata() which made use of the previous approach.
>
> Signed-off-by: Walter Lozano <walter.lozano at collabora.com>
> ---
>  drivers/clk/clk-uclass.c            |  8 +++-----
>  drivers/misc/irq-uclass.c           |  4 ++--
>  drivers/mmc/ftsdc010_mci.c          |  2 +-
>  drivers/mmc/rockchip_dw_mmc.c       |  2 +-
>  drivers/mmc/rockchip_sdhci.c        |  2 +-
>  drivers/ram/rockchip/sdram_rk3399.c |  2 +-
>  drivers/spi/rk_spi.c                |  2 +-
>  include/clk.h                       |  2 +-
>  tools/dtoc/dtb_platdata.py          | 17 ++++++++++++++---
>  9 files changed, 25 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c
> index 71878474eb..f24008fe00 100644
> --- a/drivers/clk/clk-uclass.c
> +++ b/drivers/clk/clk-uclass.c
> @@ -25,14 +25,12 @@ static inline const struct clk_ops *clk_dev_ops(struct udevice *dev)
>
>  #if CONFIG_IS_ENABLED(OF_CONTROL)
>  # if CONFIG_IS_ENABLED(OF_PLATDATA)
> -int clk_get_by_index_platdata(struct udevice *dev, int index,
> -                             struct phandle_1_arg *cells, struct clk *clk)
> +int clk_get_by_driver_info(struct udevice *dev, struct phandle_1_arg *cells,
> +                       struct clk *clk)
>  {
>         int ret;
>
> -       if (index != 0)
> -               return -ENOSYS;

But it looks like you only support index 0?

> -       ret = uclass_get_device(UCLASS_CLK, 0, &clk->dev);
> +       ret = device_get_by_driver_info((struct driver_info*) cells[0].node, &clk->dev);

Or do you mean [index] here instead of [0] ?

>         if (ret)
>                 return ret;
>         clk->id = cells[0].arg[0];
> diff --git a/drivers/misc/irq-uclass.c b/drivers/misc/irq-uclass.c
> index 61aa10e465..8eaebe6419 100644
> --- a/drivers/misc/irq-uclass.c
> +++ b/drivers/misc/irq-uclass.c
> @@ -63,14 +63,14 @@ int irq_read_and_clear(struct irq *irq)
>  }
>
>  #if CONFIG_IS_ENABLED(OF_PLATDATA)
> -int irq_get_by_index_platdata(struct udevice *dev, int index,
> +int irq_get_by_driver_info(struct udevice *dev,
>                               struct phandle_1_arg *cells, struct irq *irq)
>  {
>         int ret;
>
>         if (index != 0)
>                 return -ENOSYS;
> -       ret = uclass_get_device(UCLASS_IRQ, 0, &irq->dev);
> +       ret = device_get_by_driver_info(cells[0].node, &irq->dev);
>         if (ret)
>                 return ret;
>         irq->id = cells[0].arg[0];
> diff --git a/drivers/mmc/ftsdc010_mci.c b/drivers/mmc/ftsdc010_mci.c
> index 9c15eb36d6..efa92d48be 100644
> --- a/drivers/mmc/ftsdc010_mci.c
> +++ b/drivers/mmc/ftsdc010_mci.c
> @@ -437,7 +437,7 @@ static int ftsdc010_mmc_probe(struct udevice *dev)
>         chip->priv = dev;
>         chip->dev_index = 1;
>         memcpy(priv->minmax, dtplat->clock_freq_min_max, sizeof(priv->minmax));
> -       ret = clk_get_by_index_platdata(dev, 0, dtplat->clocks, &priv->clk);
> +       ret = clk_get_by_driver_info(dev, dtplat->clocks, &priv->clk);
>         if (ret < 0)
>                 return ret;
>  #endif
> diff --git a/drivers/mmc/rockchip_dw_mmc.c b/drivers/mmc/rockchip_dw_mmc.c
> index a0e1be8794..7b4479543c 100644
> --- a/drivers/mmc/rockchip_dw_mmc.c
> +++ b/drivers/mmc/rockchip_dw_mmc.c
> @@ -120,7 +120,7 @@ static int rockchip_dwmmc_probe(struct udevice *dev)
>         priv->minmax[0] = 400000;  /*  400 kHz */
>         priv->minmax[1] = dtplat->max_frequency;
>
> -       ret = clk_get_by_index_platdata(dev, 0, dtplat->clocks, &priv->clk);
> +       ret = clk_get_by_driver_info(dev, dtplat->clocks, &priv->clk);
>         if (ret < 0)
>                 return ret;
>  #else
> diff --git a/drivers/mmc/rockchip_sdhci.c b/drivers/mmc/rockchip_sdhci.c
> index b440996b26..b073f1a08d 100644
> --- a/drivers/mmc/rockchip_sdhci.c
> +++ b/drivers/mmc/rockchip_sdhci.c
> @@ -46,7 +46,7 @@ static int arasan_sdhci_probe(struct udevice *dev)
>         host->name = dev->name;
>         host->ioaddr = map_sysmem(dtplat->reg[0], dtplat->reg[1]);
>         max_frequency = dtplat->max_frequency;
> -       ret = clk_get_by_index_platdata(dev, 0, dtplat->clocks, &clk);
> +       ret = clk_get_by_driver_info(dev, dtplat->clocks, &clk);
>  #else
>         max_frequency = dev_read_u32_default(dev, "max-frequency", 0);
>         ret = clk_get_by_index(dev, 0, &clk);
> diff --git a/drivers/ram/rockchip/sdram_rk3399.c b/drivers/ram/rockchip/sdram_rk3399.c
> index d69ef01d08..87ec25f893 100644
> --- a/drivers/ram/rockchip/sdram_rk3399.c
> +++ b/drivers/ram/rockchip/sdram_rk3399.c
> @@ -3125,7 +3125,7 @@ static int rk3399_dmc_init(struct udevice *dev)
>               priv->cic, priv->pmugrf, priv->pmusgrf, priv->pmucru, priv->pmu);
>
>  #if CONFIG_IS_ENABLED(OF_PLATDATA)
> -       ret = clk_get_by_index_platdata(dev, 0, dtplat->clocks, &priv->ddr_clk);
> +       ret = clk_get_by_driver_info(dev, dtplat->clocks, &priv->ddr_clk);
>  #else
>         ret = clk_get_by_index(dev, 0, &priv->ddr_clk);
>  #endif
> diff --git a/drivers/spi/rk_spi.c b/drivers/spi/rk_spi.c
> index 95eeb8307a..bd0337272e 100644
> --- a/drivers/spi/rk_spi.c
> +++ b/drivers/spi/rk_spi.c
> @@ -181,7 +181,7 @@ static int conv_of_platdata(struct udevice *dev)
>
>         plat->base = dtplat->reg[0];
>         plat->frequency = 20000000;
> -       ret = clk_get_by_index_platdata(dev, 0, dtplat->clocks, &priv->clk);
> +       ret = clk_get_by_driver_info(dev, dtplat->clocks, &priv->clk);
>         if (ret < 0)
>                 return ret;
>         dev->req_seq = 0;
> diff --git a/include/clk.h b/include/clk.h
> index c6a2713f62..3be379de03 100644
> --- a/include/clk.h
> +++ b/include/clk.h
> @@ -89,7 +89,7 @@ struct clk_bulk {
>
>  #if CONFIG_IS_ENABLED(OF_CONTROL) && CONFIG_IS_ENABLED(CLK)
>  struct phandle_1_arg;
> -int clk_get_by_index_platdata(struct udevice *dev, int index,
> +int clk_get_by_driver_info(struct udevice *dev,
>                               struct phandle_1_arg *cells, struct clk *clk);
>
>  /**
> diff --git a/tools/dtoc/dtb_platdata.py b/tools/dtoc/dtb_platdata.py
> index 3f899a8bac..d30e2af2ec 100644
> --- a/tools/dtoc/dtb_platdata.py
> +++ b/tools/dtoc/dtb_platdata.py

Can you put the dtoc part of this change in a separate patch?

> @@ -153,6 +153,7 @@ class DtbPlatdata(object):
>          self._aliases = {}
>          self._drivers = []
>          self._driver_aliases = {}
> +        self._links = []
>
>      def get_normalized_compat_name(self, node):
>          compat_c, aliases_c = get_compat_name(node)
> @@ -507,7 +508,7 @@ class DtbPlatdata(object):
>          """
>          struct_name, _ = self.get_normalized_compat_name(node)
>          var_name = conv_name_to_c(node.name)
> -        self.buf('static const struct %s%s %s%s = {\n' %
> +        self.buf('static struct %s%s %s%s = {\n' %
>                   (STRUCT_PREFIX, struct_name, VAL_PREFIX, var_name))
>          for pname in sorted(node.props):
>              prop = node.props[pname]
> @@ -526,6 +527,7 @@ class DtbPlatdata(object):
>                  if info:
>                      # Process the list as pairs of (phandle, id)
>                      pos = 0
> +                    item = 0
>                      for args in info.args:
>                          phandle_cell = prop.value[pos]
>                          phandle = fdt_util.fdt32_to_cpu(phandle_cell)
> @@ -535,8 +537,10 @@ class DtbPlatdata(object):
>                          for i in range(args):
>                              arg_values.append(str(fdt_util.fdt32_to_cpu(prop.value[pos + 1 + i])))
>                          pos += 1 + args
> -                        vals.append('\t{&%s%s, {%s}}' % (VAL_PREFIX, name,
> -                                                     ', '.join(arg_values)))
> +                        vals.append('\t{NULL, {%s}}' % (', '.join(arg_values)))
> +                        phandle_entry = '%s%s.%s[%d].node = DM_GET_DEVICE(%s)' % (VAL_PREFIX, var_name, member_name, item, name)
> +                        self._links.append(phandle_entry)
> +                        item += 1
>                      for val in vals:
>                          self.buf('\n\t\t%s,' % val)
>                  else:
> @@ -559,6 +563,7 @@ class DtbPlatdata(object):
>          self.buf('\t.name\t\t= "%s",\n' % struct_name)
>          self.buf('\t.platdata\t= &%s%s,\n' % (VAL_PREFIX, var_name))
>          self.buf('\t.platdata_size\t= sizeof(%s%s),\n' % (VAL_PREFIX, var_name))
> +        self.buf('\t.dev\t\t= NULL,\n')

I suggest emitting a little comment here saying that it is filled in
by (function) at runtime.

>          self.buf('};\n')
>          self.buf('\n')
>
> @@ -592,6 +597,12 @@ class DtbPlatdata(object):
>              self.output_node(node)
>              nodes_to_output.remove(node)
>
> +        self.buf('void populate_phandle_data(void) {\n')
> +        for l in self._links:
> +            self.buf(('\t%s;\n' % l))
> +        self.buf('}\n')

Can you add a comment with an example line that is output? Or maybe
use a dict with a key and value for each piece (dmc_at_xxx.clocks[0]
as key and clock_controler_at_... as value) so you can have the string
formatting done here. It is a just a bit unclear what is being written
here.

> +
> +        self.out(''.join(self.get_buf()))
>
>  def run_steps(args, dtb_file, include_disabled, output):
>      """Run all the steps of the dtoc tool
> --
> 2.20.1
>

Regards,
Simon
Walter Lozano May 21, 2020, 1:16 p.m. UTC | #2
On 20/5/20 00:07, Simon Glass wrote:
> Hi Walter,
>
> On Wed, 13 May 2020 at 14:14, Walter Lozano<walter.lozano at collabora.com>  wrote:
>> In the current implementation, when dtoc parses a dtb to generate a struct
>> platdata it converts the information related to linked nodes as pointers
>> to struct platdata of destination nodes. By doing this, it makes
>> difficult to get pointer to udevices created based on these
>> information.
>>
>> This patch extends dtoc to use struct driver_info when populating
>> information about linked nodes, which makes it easier to later get
>> the devices created. In this context, reimplement functions like
>> clk_get_by_index_platdata() which made use of the previous approach.
>>
>> Signed-off-by: Walter Lozano<walter.lozano at collabora.com>
>> ---
>>   drivers/clk/clk-uclass.c            |  8 +++-----
>>   drivers/misc/irq-uclass.c           |  4 ++--
>>   drivers/mmc/ftsdc010_mci.c          |  2 +-
>>   drivers/mmc/rockchip_dw_mmc.c       |  2 +-
>>   drivers/mmc/rockchip_sdhci.c        |  2 +-
>>   drivers/ram/rockchip/sdram_rk3399.c |  2 +-
>>   drivers/spi/rk_spi.c                |  2 +-
>>   include/clk.h                       |  2 +-
>>   tools/dtoc/dtb_platdata.py          | 17 ++++++++++++++---
>>   9 files changed, 25 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c
>> index 71878474eb..f24008fe00 100644
>> --- a/drivers/clk/clk-uclass.c
>> +++ b/drivers/clk/clk-uclass.c
>> @@ -25,14 +25,12 @@ static inline const struct clk_ops *clk_dev_ops(struct udevice *dev)
>>
>>   #if CONFIG_IS_ENABLED(OF_CONTROL)
>>   # if CONFIG_IS_ENABLED(OF_PLATDATA)
>> -int clk_get_by_index_platdata(struct udevice *dev, int index,
>> -                             struct phandle_1_arg *cells, struct clk *clk)
>> +int clk_get_by_driver_info(struct udevice *dev, struct phandle_1_arg *cells,
>> +                       struct clk *clk)
>>   {
>>          int ret;
>>
>> -       if (index != 0)
>> -               return -ENOSYS;
> But it looks like you only support index 0?
>
>> -       ret = uclass_get_device(UCLASS_CLK, 0, &clk->dev);
>> +       ret = device_get_by_driver_info((struct driver_info*) cells[0].node, &clk->dev);
> Or do you mean [index] here instead of [0] ?

In my understanding, this new function clk_get_by_driver_info() which 
somehow replaces clk_get_by_index_platdata() doesn't require an index at 
all. The function device_get_by_driver_info will return the device 
associated with cells->node, which basically is a struct driver_info 
which holds a struct udevice *dev.

In the case that cells contains more than one struct phandle_1_arg, this 
function should be called as many times as required to fill the proper 
struct udevice dev.

>>          if (ret)
>>                  return ret;
>>          clk->id = cells[0].arg[0];
>> diff --git a/drivers/misc/irq-uclass.c b/drivers/misc/irq-uclass.c
>> index 61aa10e465..8eaebe6419 100644
>> --- a/drivers/misc/irq-uclass.c
>> +++ b/drivers/misc/irq-uclass.c
>> @@ -63,14 +63,14 @@ int irq_read_and_clear(struct irq *irq)
>>   }
>>
>>   #if CONFIG_IS_ENABLED(OF_PLATDATA)
>> -int irq_get_by_index_platdata(struct udevice *dev, int index,
>> +int irq_get_by_driver_info(struct udevice *dev,
>>                                struct phandle_1_arg *cells, struct irq *irq)
>>   {
>>          int ret;
>>
>>          if (index != 0)
>>                  return -ENOSYS;
>> -       ret = uclass_get_device(UCLASS_IRQ, 0, &irq->dev);
>> +       ret = device_get_by_driver_info(cells[0].node, &irq->dev);
>>          if (ret)
>>                  return ret;
>>          irq->id = cells[0].arg[0];
>> diff --git a/drivers/mmc/ftsdc010_mci.c b/drivers/mmc/ftsdc010_mci.c
>> index 9c15eb36d6..efa92d48be 100644
>> --- a/drivers/mmc/ftsdc010_mci.c
>> +++ b/drivers/mmc/ftsdc010_mci.c
>> @@ -437,7 +437,7 @@ static int ftsdc010_mmc_probe(struct udevice *dev)
>>          chip->priv = dev;
>>          chip->dev_index = 1;
>>          memcpy(priv->minmax, dtplat->clock_freq_min_max, sizeof(priv->minmax));
>> -       ret = clk_get_by_index_platdata(dev, 0, dtplat->clocks, &priv->clk);
>> +       ret = clk_get_by_driver_info(dev, dtplat->clocks, &priv->clk);
>>          if (ret < 0)
>>                  return ret;
>>   #endif
>> diff --git a/drivers/mmc/rockchip_dw_mmc.c b/drivers/mmc/rockchip_dw_mmc.c
>> index a0e1be8794..7b4479543c 100644
>> --- a/drivers/mmc/rockchip_dw_mmc.c
>> +++ b/drivers/mmc/rockchip_dw_mmc.c
>> @@ -120,7 +120,7 @@ static int rockchip_dwmmc_probe(struct udevice *dev)
>>          priv->minmax[0] = 400000;  /*  400 kHz */
>>          priv->minmax[1] = dtplat->max_frequency;
>>
>> -       ret = clk_get_by_index_platdata(dev, 0, dtplat->clocks, &priv->clk);
>> +       ret = clk_get_by_driver_info(dev, dtplat->clocks, &priv->clk);
>>          if (ret < 0)
>>                  return ret;
>>   #else
>> diff --git a/drivers/mmc/rockchip_sdhci.c b/drivers/mmc/rockchip_sdhci.c
>> index b440996b26..b073f1a08d 100644
>> --- a/drivers/mmc/rockchip_sdhci.c
>> +++ b/drivers/mmc/rockchip_sdhci.c
>> @@ -46,7 +46,7 @@ static int arasan_sdhci_probe(struct udevice *dev)
>>          host->name = dev->name;
>>          host->ioaddr = map_sysmem(dtplat->reg[0], dtplat->reg[1]);
>>          max_frequency = dtplat->max_frequency;
>> -       ret = clk_get_by_index_platdata(dev, 0, dtplat->clocks, &clk);
>> +       ret = clk_get_by_driver_info(dev, dtplat->clocks, &clk);
>>   #else
>>          max_frequency = dev_read_u32_default(dev, "max-frequency", 0);
>>          ret = clk_get_by_index(dev, 0, &clk);
>> diff --git a/drivers/ram/rockchip/sdram_rk3399.c b/drivers/ram/rockchip/sdram_rk3399.c
>> index d69ef01d08..87ec25f893 100644
>> --- a/drivers/ram/rockchip/sdram_rk3399.c
>> +++ b/drivers/ram/rockchip/sdram_rk3399.c
>> @@ -3125,7 +3125,7 @@ static int rk3399_dmc_init(struct udevice *dev)
>>                priv->cic, priv->pmugrf, priv->pmusgrf, priv->pmucru, priv->pmu);
>>
>>   #if CONFIG_IS_ENABLED(OF_PLATDATA)
>> -       ret = clk_get_by_index_platdata(dev, 0, dtplat->clocks, &priv->ddr_clk);
>> +       ret = clk_get_by_driver_info(dev, dtplat->clocks, &priv->ddr_clk);
>>   #else
>>          ret = clk_get_by_index(dev, 0, &priv->ddr_clk);
>>   #endif
>> diff --git a/drivers/spi/rk_spi.c b/drivers/spi/rk_spi.c
>> index 95eeb8307a..bd0337272e 100644
>> --- a/drivers/spi/rk_spi.c
>> +++ b/drivers/spi/rk_spi.c
>> @@ -181,7 +181,7 @@ static int conv_of_platdata(struct udevice *dev)
>>
>>          plat->base = dtplat->reg[0];
>>          plat->frequency = 20000000;
>> -       ret = clk_get_by_index_platdata(dev, 0, dtplat->clocks, &priv->clk);
>> +       ret = clk_get_by_driver_info(dev, dtplat->clocks, &priv->clk);
>>          if (ret < 0)
>>                  return ret;
>>          dev->req_seq = 0;
>> diff --git a/include/clk.h b/include/clk.h
>> index c6a2713f62..3be379de03 100644
>> --- a/include/clk.h
>> +++ b/include/clk.h
>> @@ -89,7 +89,7 @@ struct clk_bulk {
>>
>>   #if CONFIG_IS_ENABLED(OF_CONTROL) && CONFIG_IS_ENABLED(CLK)
>>   struct phandle_1_arg;
>> -int clk_get_by_index_platdata(struct udevice *dev, int index,
>> +int clk_get_by_driver_info(struct udevice *dev,
>>                                struct phandle_1_arg *cells, struct clk *clk);
>>
>>   /**
>> diff --git a/tools/dtoc/dtb_platdata.py b/tools/dtoc/dtb_platdata.py
>> index 3f899a8bac..d30e2af2ec 100644
>> --- a/tools/dtoc/dtb_platdata.py
>> +++ b/tools/dtoc/dtb_platdata.py
> Can you put the dtoc part of this change in a separate patch?
The issue is that it will cause an error at run time as the struct 
driver_info will not have the correct values for struct udevice *dev, 
which is populated by populate_phandle_data().
>> @@ -153,6 +153,7 @@ class DtbPlatdata(object):
>>           self._aliases = {}
>>           self._drivers = []
>>           self._driver_aliases = {}
>> +        self._links = []
>>
>>       def get_normalized_compat_name(self, node):
>>           compat_c, aliases_c = get_compat_name(node)
>> @@ -507,7 +508,7 @@ class DtbPlatdata(object):
>>           """
>>           struct_name, _ = self.get_normalized_compat_name(node)
>>           var_name = conv_name_to_c(node.name)
>> -        self.buf('static const struct %s%s %s%s = {\n' %
>> +        self.buf('static struct %s%s %s%s = {\n' %
>>                    (STRUCT_PREFIX, struct_name, VAL_PREFIX, var_name))
>>           for pname in sorted(node.props):
>>               prop = node.props[pname]
>> @@ -526,6 +527,7 @@ class DtbPlatdata(object):
>>                   if info:
>>                       # Process the list as pairs of (phandle, id)
>>                       pos = 0
>> +                    item = 0
>>                       for args in info.args:
>>                           phandle_cell = prop.value[pos]
>>                           phandle = fdt_util.fdt32_to_cpu(phandle_cell)
>> @@ -535,8 +537,10 @@ class DtbPlatdata(object):
>>                           for i in range(args):
>>                               arg_values.append(str(fdt_util.fdt32_to_cpu(prop.value[pos + 1 + i])))
>>                           pos += 1 + args
>> -                        vals.append('\t{&%s%s, {%s}}' % (VAL_PREFIX, name,
>> -                                                     ', '.join(arg_values)))
>> +                        vals.append('\t{NULL, {%s}}' % (', '.join(arg_values)))
>> +                        phandle_entry = '%s%s.%s[%d].node = DM_GET_DEVICE(%s)' % (VAL_PREFIX, var_name, member_name, item, name)
>> +                        self._links.append(phandle_entry)
>> +                        item += 1
>>                       for val in vals:
>>                           self.buf('\n\t\t%s,' % val)
>>                   else:
>> @@ -559,6 +563,7 @@ class DtbPlatdata(object):
>>           self.buf('\t.name\t\t= "%s",\n' % struct_name)
>>           self.buf('\t.platdata\t= &%s%s,\n' % (VAL_PREFIX, var_name))
>>           self.buf('\t.platdata_size\t= sizeof(%s%s),\n' % (VAL_PREFIX, var_name))
>> +        self.buf('\t.dev\t\t= NULL,\n')
> I suggest emitting a little comment here saying that it is filled in
> by (function) at runtime.
Sure.
>>           self.buf('};\n')
>>           self.buf('\n')
>>
>> @@ -592,6 +597,12 @@ class DtbPlatdata(object):
>>               self.output_node(node)
>>               nodes_to_output.remove(node)
>>
>> +        self.buf('void populate_phandle_data(void) {\n')
>> +        for l in self._links:
>> +            self.buf(('\t%s;\n' % l))
>> +        self.buf('}\n')
> Can you add a comment with an example line that is output? Or maybe
> use a dict with a key and value for each piece (dmc_at_xxx.clocks[0]
> as key and clock_controler_at_... as value) so you can have the string
> formatting done here. It is a just a bit unclear what is being written
> here.

Yes, I totally agree, there should be a more clear way to do this. I was 
thinking about it for a while and I initially thought that doing all the 
expansions in one place will make it easier to read but I had many doubts.

Thanks for pointing it.

>> +
>> +        self.out(''.join(self.get_buf()))
>>
>>   def run_steps(args, dtb_file, include_disabled, output):
>>       """Run all the steps of the dtoc tool
>> --
>> 2.20.1
>>
Regards,

Walter
diff mbox series

Patch

diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c
index 71878474eb..f24008fe00 100644
--- a/drivers/clk/clk-uclass.c
+++ b/drivers/clk/clk-uclass.c
@@ -25,14 +25,12 @@  static inline const struct clk_ops *clk_dev_ops(struct udevice *dev)
 
 #if CONFIG_IS_ENABLED(OF_CONTROL)
 # if CONFIG_IS_ENABLED(OF_PLATDATA)
-int clk_get_by_index_platdata(struct udevice *dev, int index,
-			      struct phandle_1_arg *cells, struct clk *clk)
+int clk_get_by_driver_info(struct udevice *dev, struct phandle_1_arg *cells,
+			struct clk *clk)
 {
 	int ret;
 
-	if (index != 0)
-		return -ENOSYS;
-	ret = uclass_get_device(UCLASS_CLK, 0, &clk->dev);
+	ret = device_get_by_driver_info((struct driver_info*) cells[0].node, &clk->dev);
 	if (ret)
 		return ret;
 	clk->id = cells[0].arg[0];
diff --git a/drivers/misc/irq-uclass.c b/drivers/misc/irq-uclass.c
index 61aa10e465..8eaebe6419 100644
--- a/drivers/misc/irq-uclass.c
+++ b/drivers/misc/irq-uclass.c
@@ -63,14 +63,14 @@  int irq_read_and_clear(struct irq *irq)
 }
 
 #if CONFIG_IS_ENABLED(OF_PLATDATA)
-int irq_get_by_index_platdata(struct udevice *dev, int index,
+int irq_get_by_driver_info(struct udevice *dev,
 			      struct phandle_1_arg *cells, struct irq *irq)
 {
 	int ret;
 
 	if (index != 0)
 		return -ENOSYS;
-	ret = uclass_get_device(UCLASS_IRQ, 0, &irq->dev);
+	ret = device_get_by_driver_info(cells[0].node, &irq->dev);
 	if (ret)
 		return ret;
 	irq->id = cells[0].arg[0];
diff --git a/drivers/mmc/ftsdc010_mci.c b/drivers/mmc/ftsdc010_mci.c
index 9c15eb36d6..efa92d48be 100644
--- a/drivers/mmc/ftsdc010_mci.c
+++ b/drivers/mmc/ftsdc010_mci.c
@@ -437,7 +437,7 @@  static int ftsdc010_mmc_probe(struct udevice *dev)
 	chip->priv = dev;
 	chip->dev_index = 1;
 	memcpy(priv->minmax, dtplat->clock_freq_min_max, sizeof(priv->minmax));
-	ret = clk_get_by_index_platdata(dev, 0, dtplat->clocks, &priv->clk);
+	ret = clk_get_by_driver_info(dev, dtplat->clocks, &priv->clk);
 	if (ret < 0)
 		return ret;
 #endif
diff --git a/drivers/mmc/rockchip_dw_mmc.c b/drivers/mmc/rockchip_dw_mmc.c
index a0e1be8794..7b4479543c 100644
--- a/drivers/mmc/rockchip_dw_mmc.c
+++ b/drivers/mmc/rockchip_dw_mmc.c
@@ -120,7 +120,7 @@  static int rockchip_dwmmc_probe(struct udevice *dev)
 	priv->minmax[0] = 400000;  /*  400 kHz */
 	priv->minmax[1] = dtplat->max_frequency;
 
-	ret = clk_get_by_index_platdata(dev, 0, dtplat->clocks, &priv->clk);
+	ret = clk_get_by_driver_info(dev, dtplat->clocks, &priv->clk);
 	if (ret < 0)
 		return ret;
 #else
diff --git a/drivers/mmc/rockchip_sdhci.c b/drivers/mmc/rockchip_sdhci.c
index b440996b26..b073f1a08d 100644
--- a/drivers/mmc/rockchip_sdhci.c
+++ b/drivers/mmc/rockchip_sdhci.c
@@ -46,7 +46,7 @@  static int arasan_sdhci_probe(struct udevice *dev)
 	host->name = dev->name;
 	host->ioaddr = map_sysmem(dtplat->reg[0], dtplat->reg[1]);
 	max_frequency = dtplat->max_frequency;
-	ret = clk_get_by_index_platdata(dev, 0, dtplat->clocks, &clk);
+	ret = clk_get_by_driver_info(dev, dtplat->clocks, &clk);
 #else
 	max_frequency = dev_read_u32_default(dev, "max-frequency", 0);
 	ret = clk_get_by_index(dev, 0, &clk);
diff --git a/drivers/ram/rockchip/sdram_rk3399.c b/drivers/ram/rockchip/sdram_rk3399.c
index d69ef01d08..87ec25f893 100644
--- a/drivers/ram/rockchip/sdram_rk3399.c
+++ b/drivers/ram/rockchip/sdram_rk3399.c
@@ -3125,7 +3125,7 @@  static int rk3399_dmc_init(struct udevice *dev)
 	      priv->cic, priv->pmugrf, priv->pmusgrf, priv->pmucru, priv->pmu);
 
 #if CONFIG_IS_ENABLED(OF_PLATDATA)
-	ret = clk_get_by_index_platdata(dev, 0, dtplat->clocks, &priv->ddr_clk);
+	ret = clk_get_by_driver_info(dev, dtplat->clocks, &priv->ddr_clk);
 #else
 	ret = clk_get_by_index(dev, 0, &priv->ddr_clk);
 #endif
diff --git a/drivers/spi/rk_spi.c b/drivers/spi/rk_spi.c
index 95eeb8307a..bd0337272e 100644
--- a/drivers/spi/rk_spi.c
+++ b/drivers/spi/rk_spi.c
@@ -181,7 +181,7 @@  static int conv_of_platdata(struct udevice *dev)
 
 	plat->base = dtplat->reg[0];
 	plat->frequency = 20000000;
-	ret = clk_get_by_index_platdata(dev, 0, dtplat->clocks, &priv->clk);
+	ret = clk_get_by_driver_info(dev, dtplat->clocks, &priv->clk);
 	if (ret < 0)
 		return ret;
 	dev->req_seq = 0;
diff --git a/include/clk.h b/include/clk.h
index c6a2713f62..3be379de03 100644
--- a/include/clk.h
+++ b/include/clk.h
@@ -89,7 +89,7 @@  struct clk_bulk {
 
 #if CONFIG_IS_ENABLED(OF_CONTROL) && CONFIG_IS_ENABLED(CLK)
 struct phandle_1_arg;
-int clk_get_by_index_platdata(struct udevice *dev, int index,
+int clk_get_by_driver_info(struct udevice *dev,
 			      struct phandle_1_arg *cells, struct clk *clk);
 
 /**
diff --git a/tools/dtoc/dtb_platdata.py b/tools/dtoc/dtb_platdata.py
index 3f899a8bac..d30e2af2ec 100644
--- a/tools/dtoc/dtb_platdata.py
+++ b/tools/dtoc/dtb_platdata.py
@@ -153,6 +153,7 @@  class DtbPlatdata(object):
         self._aliases = {}
         self._drivers = []
         self._driver_aliases = {}
+        self._links = []
 
     def get_normalized_compat_name(self, node):
         compat_c, aliases_c = get_compat_name(node)
@@ -507,7 +508,7 @@  class DtbPlatdata(object):
         """
         struct_name, _ = self.get_normalized_compat_name(node)
         var_name = conv_name_to_c(node.name)
-        self.buf('static const struct %s%s %s%s = {\n' %
+        self.buf('static struct %s%s %s%s = {\n' %
                  (STRUCT_PREFIX, struct_name, VAL_PREFIX, var_name))
         for pname in sorted(node.props):
             prop = node.props[pname]
@@ -526,6 +527,7 @@  class DtbPlatdata(object):
                 if info:
                     # Process the list as pairs of (phandle, id)
                     pos = 0
+                    item = 0
                     for args in info.args:
                         phandle_cell = prop.value[pos]
                         phandle = fdt_util.fdt32_to_cpu(phandle_cell)
@@ -535,8 +537,10 @@  class DtbPlatdata(object):
                         for i in range(args):
                             arg_values.append(str(fdt_util.fdt32_to_cpu(prop.value[pos + 1 + i])))
                         pos += 1 + args
-                        vals.append('\t{&%s%s, {%s}}' % (VAL_PREFIX, name,
-                                                     ', '.join(arg_values)))
+                        vals.append('\t{NULL, {%s}}' % (', '.join(arg_values)))
+                        phandle_entry = '%s%s.%s[%d].node = DM_GET_DEVICE(%s)' % (VAL_PREFIX, var_name, member_name, item, name)
+                        self._links.append(phandle_entry)
+                        item += 1
                     for val in vals:
                         self.buf('\n\t\t%s,' % val)
                 else:
@@ -559,6 +563,7 @@  class DtbPlatdata(object):
         self.buf('\t.name\t\t= "%s",\n' % struct_name)
         self.buf('\t.platdata\t= &%s%s,\n' % (VAL_PREFIX, var_name))
         self.buf('\t.platdata_size\t= sizeof(%s%s),\n' % (VAL_PREFIX, var_name))
+        self.buf('\t.dev\t\t= NULL,\n')
         self.buf('};\n')
         self.buf('\n')
 
@@ -592,6 +597,12 @@  class DtbPlatdata(object):
             self.output_node(node)
             nodes_to_output.remove(node)
 
+        self.buf('void populate_phandle_data(void) {\n')
+        for l in self._links:
+            self.buf(('\t%s;\n' % l))
+        self.buf('}\n')
+
+        self.out(''.join(self.get_buf()))
 
 def run_steps(args, dtb_file, include_disabled, output):
     """Run all the steps of the dtoc tool