On Mon, Feb 05, 2018 at 09:24:34AM -0600, Andrew F. Davis wrote: > On 02/01/2018 10:35 PM, Rob Herring wrote: > > On Thu, Feb 1, 2018 at 5:04 PM, Andrew F. Davis <afd@xxxxxx> wrote: > >> From: Benjamin Fair <b-fair@xxxxxx> > >> > >> This patch extends the capability of libfdt to parse the contents of device > >> trees in a similar manner to fdt_address_cells and fdt_size_cells. > >> > >> It adds a helper function which reads the address and size of a device from > >> the reg property and performs basic sanity checks. > >> > >> It does not perform translation to a physical address using the ranges > >> properties of parents, but this enhancement may be added as a separate > >> function in the future. > > > > I'm concerned about merging this without translation support. First, > > I'd question its usefulness without it. Second, it may encourage > > people to write their DTs without any ranges just so this will work. > > It can be useful for nodes where ranges is not appropriate for whatever > reason, or more likely as a base for other helpers that do perform > translation (hence the "simple" in this ones name), it will need to > still be extended for many use-cases. Yeah, I have very mixed feelings about this. If I'd just seen the patches without the threads associated with them, I'd have been happy enough. Although it certainly doesn't cover anything, there are definitely cases where an untranslated retrieval is useful. Not least as something for a translating helper to use underneath, but also because this can be useful for buses which have no direct mapping into MMIO space (/cpus, i2c amongst others). However a number of the comments on versions of the patch - and indeed the function types on this revision - seem to specifically imply this is intended for the case where an untranslated fetch is somewhere between fragile and dead wrong. Like Rob, I'm concerned that will encourage poor ways of doing things. Perhaps a version of this which will error if the parent node has any 'ranges' property (including empty)? That restricts it to the case of buses that are not mapped into their parent address space. > > I tried to upstream the kernel and u-boot implementations to libfdt in > > April 2014 (I couldn't find an archive). We've got to get it > > relicensed and then David didn't like the implementation. He said he > > was about to post his version, but then it derailed into PCI handling. Ugh.. sorry. Clearly I completely lost that train of thought. > > There's another implementation in the kernel powerpc boot code that > > may be easier to re-license. > > > > My worry with adding translation is this kind of feature creep. PCI and > other odd cases will block this effort with little gain for the users > where this simple helper is sufficient. > > > Rob > > -- 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
