[5/5] dtoc: Tidy up Python style in dtb_platdata

Update this, mostly to add comments for argument and return types. It is
probably still too early to use type hinting since it was introduced in
3.5.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 tools/dtoc/dtb_platdata.py | 71 ++++++++++++++++++++++----------------
 1 file changed, 42 insertions(+), 29 deletions(-)

Hi Simon,

Thanks for this series. I've tried to test it but I had issues to apply 
it. I have tried in u-boot, both master and next, and u-boot-dm.

Could you please point me to the right tree/version?

On 9/11/20 00:36, Simon Glass wrote:
> Update this, mostly to add comments for argument and return types. It is
> probably still too early to use type hinting since it was introduced in
> 3.5.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>   tools/dtoc/dtb_platdata.py | 71 ++++++++++++++++++++++----------------
>   1 file changed, 42 insertions(+), 29 deletions(-)
>
> diff --git a/tools/dtoc/dtb_platdata.py b/tools/dtoc/dtb_platdata.py
> index 0ef245397a9..6b10eabafb9 100644
> --- a/tools/dtoc/dtb_platdata.py
> +++ b/tools/dtoc/dtb_platdata.py
> @@ -9,6 +9,8 @@
>   
>   This supports converting device tree data to C structures definitions and
>   static data.
> +
> +See doc/driver-model/of-plat.rst for more informaiton
*information
>   """
>   
>   import collections
> @@ -19,9 +21,9 @@ import sys
>   
>   from dtoc import fdt
>   from dtoc import fdt_util
> -from patman import tools
>   
> -# When we see these properties we ignore them - i.e. do not create a structure member
> +# When we see these properties we ignore them - i.e. do not create a structure
> +# member
>   PROP_IGNORE_LIST = [
>       '#address-cells',
>       '#gpio-cells',
> @@ -69,9 +71,9 @@ def conv_name_to_c(name):
>       (400ms for 1m calls versus 1000ms for the 're' version).
>   
>       Args:
> -        name:   Name to convert
> +        name (str): Name to convert
>       Return:
> -        String containing the C version of this name
> +        str: String containing the C version of this name
>       """
>       new = name.replace('@', '_at_')
>       new = new.replace('-', '_')
> @@ -83,11 +85,11 @@ def tab_to(num_tabs, line):
>       """Append tabs to a line of text to reach a tab stop.
>   
>       Args:
> -        num_tabs: Tab stop to obtain (0 = column 0, 1 = column 8, etc.)
> -        line: Line of text to append to
> +        num_tabs (int): Tab stop to obtain (0 = column 0, 1 = column 8, etc.)
> +        line (str): Line of text to append to
>   
>       Returns:
> -        line with the correct number of tabs appeneded. If the line already
> +        str: line with the correct number of tabs appeneded. If the line already
>           extends past that tab stop then a single space is appended.
>       """
>       if len(line) >= num_tabs * 8:
> @@ -103,28 +105,31 @@ def get_value(ftype, value):
>       For booleans this return 'true'
>   
>       Args:
> -        type: Data type (fdt_util)
> -        value: Data value, as a string of bytes
> +        ftype (fdt.Type): Data type (fdt_util)
> +        value (bytes): Data value, as a string of bytes
> +
> +    Returns:
> +        str: String representation of the value
>       """
>       if ftype == fdt.Type.INT:
>           return '%#x' % fdt_util.fdt32_to_cpu(value)
>       elif ftype == fdt.Type.BYTE:
>           ch = value[0]
> -        return '%#x' % (ord(ch) if type(ch) == str else ch)
> +        return '%#x' % (ord(ch) if isinstance(ch, str) else ch)
>       elif ftype == fdt.Type.STRING:
>           # Handle evil ACPI backslashes by adding another backslash before them.
>           # So "\\_SB.GPO0" in the device tree effectively stays like that in C
>           return '"%s"' % value.replace('\\', '\\\\')
>       elif ftype == fdt.Type.BOOL:
>           return 'true'
> -    elif ftype == fdt.Type.INT64:
> +    else:  # ftype == fdt.Type.INT64:
>           return '%#x' % value
>   
>   def get_compat_name(node):
>       """Get the node's list of compatible string as a C identifiers
>   
>       Args:
> -        node: Node object to check
> +        node (fdt.Node): Node object to check
>       Return:
>           List of C identifiers for all the compatible strings
>       """
> @@ -213,7 +218,7 @@ class DtbPlatdata(object):
>           file.
>   
>           Args:
> -            fname: Filename to send output to, or '-' for stdout
> +            fname (str): Filename to send output to, or '-' for stdout
>           """
>           if fname == '-':
>               self._outfile = sys.stdout
> @@ -224,7 +229,7 @@ class DtbPlatdata(object):
>           """Output a string to the output file
>   
>           Args:
> -            line: String to output
> +            line (str): String to output
>           """
>           self._outfile.write(line)
>   
> @@ -232,7 +237,7 @@ class DtbPlatdata(object):
>           """Buffer up a string to send later
>   
>           Args:
> -            line: String to add to our 'buffer' list
> +            line (str): String to add to our 'buffer' list
>           """
>           self._lines.append(line)
>   
> @@ -240,7 +245,7 @@ class DtbPlatdata(object):
>           """Get the contents of the output buffer, and clear it
>   
>           Returns:
> -            The output buffer, which is then cleared for future use
> +            list(str): The output buffer, which is then cleared for future use
>           """
>           lines = self._lines
>           self._lines = []
> @@ -263,9 +268,14 @@ class DtbPlatdata(object):
>           or not. As an interim measure, use a list of known property names.
>   
>           Args:
> -            prop: Prop object to check
> -        Return:
> -            Number of argument cells is this is a phandle, else None
> +            prop (fdt.Prop): Prop object to check
> +            node_name (str): Node name, only used for raising an error
> +        Returns:
> +            int or None: Number of argument cells is this is a phandle,
> +                else None
> +        Raises:
> +            ValueError: if the phandle cannot be parsed or the required property
> +                is not present
>           """
>           if prop.name in ['clocks', 'cd-gpios']:
>               if not isinstance(prop.value, list):
> @@ -294,7 +304,7 @@ class DtbPlatdata(object):
>                           break
>                   if not cells:
>                       raise ValueError("Node '%s' has no cells property" %
> -                            (target.name))
> +                                     (target.name))
>                   num_args = fdt_util.fdt32_to_cpu(cells.value)
>                   max_args = max(max_args, num_args)
>                   args.append(num_args)
> @@ -404,7 +414,7 @@ class DtbPlatdata(object):
>           """Get the number of cells in addresses and sizes for this node
>   
>           Args:
> -            node: Node to check
> +            node (fdt.None): Node to check
>   
>           Returns:
>               Tuple:
> @@ -440,9 +450,10 @@ class DtbPlatdata(object):
>                   raise ValueError("Node '%s' reg property is not an int" %
>                                    node.name)
>               if len(reg.value) % total:
> -                raise ValueError("Node '%s' reg property has %d cells "
> -                        'which is not a multiple of na + ns = %d + %d)' %
> -                        (node.name, len(reg.value), na, ns))
> +                raise ValueError(
> +                    "Node '%s' reg property has %d cells "
> +                    'which is not a multiple of na + ns = %d + %d)' %
> +                    (node.name, len(reg.value), na, ns))
>               reg.na = na
>               reg.ns = ns
>               if na != 1 or ns != 1:
> @@ -585,7 +596,7 @@ class DtbPlatdata(object):
>           """Output the C code for a node
>   
>           Args:
> -            node: node to output
> +            node (fdt.Node): node to output
>           """
>           def _output_list(node, prop):
>               """Output the C code for a devicetree property that holds a list
> @@ -707,10 +718,12 @@ def run_steps(args, dtb_file, include_disabled, output, warning_disabled=False,
>       """Run all the steps of the dtoc tool
>   
>       Args:
> -        args: List of non-option arguments provided to the problem
> -        dtb_file: Filename of dtb file to process
> -        include_disabled: True to include disabled nodes
> -        output: Name of output file
> +        args (list): List of non-option arguments provided to the problem
> +        dtb_file (str): Filename of dtb file to process
> +        include_disabled (bool): True to include disabled nodes
> +        output (str): Name of output file
> +    Raises:
> +        ValueError: if args has no command, or an unknown command
>       """
>       if not args:
>           raise ValueError('Please specify a command: struct, platdata')


Regards,

Walter

Hi Simon,

Thanks for this series. I've tried to test it but I had issues to apply
it. I have tried in u-boot, both master and next, and u-boot-dm.

Could you please point me to the right tree/version?

On 9/11/20 00:36, Simon Glass wrote:
> Update this, mostly to add comments for argument and return types. It is
> probably still too early to use type hinting since it was introduced in
> 3.5.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>   tools/dtoc/dtb_platdata.py | 71 ++++++++++++++++++++++----------------
>   1 file changed, 42 insertions(+), 29 deletions(-)
>
Applied to u-boot-dm, thanks!

Hi Walter,

On Thu, 12 Nov 2020 at 20:43, Walter Lozano <walter.lozano@collabora.com> wrote:
>
> Hi Simon,
>
> Thanks for this series. I've tried to test it but I had issues to apply
> it. I have tried in u-boot, both master and next, and u-boot-dm.
>
> Could you please point me to the right tree/version?
>

Sorry I missed this email...it is in u-boot-dm/miscb-working

But now it is in dm/next

I am on the trail of a series to instantiate devices in dt_pladata.c a
bit like the tiny series. I'm not sure when I'll get more time for it,
but hopefully will have patches out in a few weeks. If you'd like to
see WIP let me know, but it is a bit rough.

Regards,
Simon