Re: [PATCH v3 4/4] pylibfdt: Support the sequential-write interface

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



On Wed, Jun 13, 2018 at 09:44:46PM -0600, Simon Glass wrote:
> It is useful to be able to create a device tree from scratch using
> software. This is supported in libfdt but not currently available in the
> Python bindings.
> 
> Add a new FdtSw class to handle this, with various methods corresponding
> to the libfdt functions. When the tree is complete, calling as_fdt() will
> return the completed device-tree object.
> 
> The FdtSw class automatically expands the tree before each operation, to
> ensure that there is enough space. There is no need to resize the tree
> manually, although a size hint given to the constructor may improve
> performance slightly for large trees.
> 
> Signed-off-by: Simon Glass <sjg@xxxxxxxxxxxx>

[snip]
> +    def add_space(self, inc_size):
> +        """Expand the device tree enough to hold new data
> +
> +        This makes sure that the device tree is large enough to hold the data
> +        size indicated. If not, it is expanded to suit.
> +        """
> +        free_space = fdt_free_space(self._fdtsw)
> +        # allow a bit of margin for two nul terminators a node name and its
> +        # string property. The worst case is 4 bytes of nul for each if we are
> +        # unlucky with the alignment. We also add 4 more bytes in case the
> +        # previous property finished unaligned.
> +        if inc_size + 12 > free_space:
> +            new_size = ((len(self._fdtsw) + inc_size +
> +                         self.INC_SIZE - 1) & ~(self.INC_SIZE - 1))
> +            self.resize(new_size)
> +
> +    def resize(self, size):
> +        """Resize the buffer to accommodate a larger tree
> +
> +        Args:
> +            size: New size of tree
> +
> +        Raises:
> +            FdtException on any error
> +        """
> +        fdt = bytearray(size)
> +        # This line should not be needed. Without it, we get BADSTRUCTURE
> +        fdt[:len(self._fdtsw)] = self._fdtsw
> +        err = check_err(fdt_resize(self._fdtsw, fdt, size))
> +        if err:
> +            return err
> +        self._fdtsw = fdt
> +
> +    def add_reservemap_entry(self, addr, size):
> +        """Add a new memory reserve map entry
> +
> +        Once finished adding, you must call finish_reservemap().
> +
> +        Args:
> +            addr: 64-bit start address
> +            size: 64-bit size
> +
> +        Raises:
> +            FdtException on any error
> +        """
> +        self.add_space(SIZEOF_fdt_reserve_entry)

So, following on from the other thread, I don't think this approach of
pre-adding space is the right way to go.  Estimating the necessary
space requires knowledge of the dtb format internals, which we're
thereby essentially duplicating in libfdt itself and the Python
wrapper.

I think the more robust approach is to just retry the operations,
i.e. basically just:

while (err = fdt_do_whatever) == -FDT_ERR_NOSPACE:
	resize(current + 1kiB)

Now, as you pointed out elsewhere that could result in repeated
resizing which could get slow.  But.. in practice I don't think it
will happen all that often, and if you're really invested in speed,
you're probably not using Python in the first place.

So I think the way is to use this approach throughout, then look at
optimizing it only if someone has a problem with it in practice.

-- 
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