Re: [PATCH 05/10] pylibfdt: Support device-tree creation/expansion

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]



On Wed, May 23, 2018 at 10:03:41PM -0600, Simon Glass wrote:
> Add support for fdt_open_init() and fdt_create_empty_tree() from the
> Python library.
> 
> Signed-off-by: Simon Glass <sjg@xxxxxxxxxxxx>
> ---
> 
>  pylibfdt/libfdt.i       | 33 +++++++++++++++++++++++++++++++++
>  tests/pylibfdt_tests.py | 18 +++++++++++++++++-
>  2 files changed, 50 insertions(+), 1 deletion(-)
> 
> diff --git a/pylibfdt/libfdt.i b/pylibfdt/libfdt.i
> index c0cd096..129f000 100644
> --- a/pylibfdt/libfdt.i
> +++ b/pylibfdt/libfdt.i
> @@ -432,6 +432,39 @@ class Fdt:
>              return pdata
>          return Property(pdata[0], pdata[1])
>  
> +    @staticmethod
> +    def create_empty_tree(size, quiet=()):
> +        """Create an empty device tree ready for use
> +
> +        Args:
> +            size: Size of device tree in bytes
> +
> +        Returns:
> +            Fdt object containing the device tree
> +        """
> +        data = bytearray(size)
> +        err = check_err(fdt_create_empty_tree(data, size), quiet)
> +        if err:
> +            return err
> +        return Fdt(data)
> +
> +    def open_into(self, size, quiet=()):

So, usually keeping the same names here as in C is a good idea.
However, when Python is managing the buffer, rather than the caller
having to thing about it explicitly the "open_into" name is kind of
misleading.

I can see two ways to go:
    1) Call it resize() instead (which would probably need to do
       either fdt_open_into() or fdt_resize) depending on if you're in
       sw or rw mode)
    2) Change the interface so that rather than changing the buffer
       "in place", it leaves the existing Fdt object as is and returns
       a new Fdt object with the "opened" buffer.


> +        """Move the device tree into a larger or smaller space
> +
> +        This creates a new device tree of size @size and moves the existing
> +        device tree contents over to that. It can be used to create more space
> +        in a device tree.
> +
> +        Args:
> +            size: Required new size of device tree in bytes
> +        """
> +        fdt = bytearray(size)
> +        fdt[:len(self._fdt)] = self._fdt

You don't need this - fdt_open_into() will already copy data from the
old buffer into the new one.

> +        err = check_err(fdt_open_into(self._fdt, fdt, size), quiet)
> +        if err:
> +            return err
> +        self._fdt = fdt
> +
>      def pack(self, quiet=()):
>          """Pack the device tree to remove unused space
>  
> diff --git a/tests/pylibfdt_tests.py b/tests/pylibfdt_tests.py
> index ca27b0b..a538ba1 100644
> --- a/tests/pylibfdt_tests.py
> +++ b/tests/pylibfdt_tests.py
> @@ -55,7 +55,7 @@ import unittest
>  
>  sys.path.insert(0, '../pylibfdt')
>  import libfdt
> -from libfdt import FdtException, QUIET_NOTFOUND, QUIET_ALL
> +from libfdt import Fdt, FdtException, QUIET_NOTFOUND, QUIET_ALL
>  
>  def get_err(err_code):
>      """Convert an error code into an error message
> @@ -376,6 +376,22 @@ class PyLibfdtTests(unittest.TestCase):
>                            self.fdt.get_mem_rsv(0))
>          self.assertEquals([123456789, 010000], self.fdt.get_mem_rsv(1))
>  
> +    def testEmpty(self):
> +        """Test that we can create an empty tree"""
> +        self.assertEquals(-libfdt.NOSPACE,
> +                          Fdt.create_empty_tree(1, (libfdt.NOSPACE,)))
> +        fdt = Fdt.create_empty_tree(128)
> +        self.assertEquals(128, fdt.totalsize())
> +
> +    def testOpenInto(self):
> +        """Test that we can resize a tree"""
> +        fdt = Fdt.create_empty_tree(128)
> +        self.assertEquals(128, fdt.totalsize())
> +        fdt.open_into(256)
> +        self.assertEquals(256, fdt.totalsize())
> +        fdt.pack()
> +        self.assertTrue(fdt.totalsize() < 128)
> +
>  
>  if __name__ == "__main__":
>      unittest.main()

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Device Tree]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux