Page MenuHomeFreeBSD

Add libfdt user space library
Needs ReviewPublic

Authored by stevek on Jun 11 2016, 7:32 PM.

Details

Reviewers
andrew
imp
Group Reviewers
ARM
Summary

Build libfdt for user space to use.

Sources for libfdt come from contrib/dtc/libfdt.

Test Plan

Built and tested on RPi2.
Used by Juniper on ARMv7-based system to get access to I2C information in the FDT blob.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 20307
Build 19765: arc lint + arc unit

Event Timeline

stevek updated this revision to Diff 17522.Jun 11 2016, 7:32 PM
stevek retitled this revision from to Add libfdt user space library and expose FDT blob via sysctl on ARM.
stevek updated this object.
stevek edited the test plan for this revision. (Show Details)
stevek set the repository for this revision to rS FreeBSD src repository.
stevek added subscribers: jtl, sjg.
stevek added a reviewer: ARM.Jun 11 2016, 8:18 PM
manu added a subscriber: manu.Jun 11 2016, 8:24 PM

Why can't you use ofwdump(8) for this ? Isn't all the needed information is in there ?

In D6814#143173, @manu wrote:

Why can't you use ofwdump(8) for this ? Isn't all the needed information is in there ?

Not if you're trying to do something programmatically with libfdt.

We use this interface at Juniper to feed the kernel's representation of the DTB to libfdt. We also have another library that looks up I2C nodes in the FDT using libfdt to determine the slave IDs for I2C devices in order to access them.

Having thought about this I'm not opposed to this. I think it should be two commits, one for the kernel bits, and another to build libfdt.a.

sys/arm/arm/machdep.c
264–276 ↗(On Diff #17522)

This should be in a MI source file. I think the most appropriate would be sys/dev/ofw/ofw_fdt.c. It should be under hw.fdt, e.g. hw.fdt.dtbp, and only exposed when we have installed a pointer. The last point will be important on arm64 as we may not have a devicetree to point to, e.g. when booting with ACPI.

Having thought about this I'm not opposed to this. I think it should be two commits, one for the kernel bits, and another to build libfdt.a.

Sure, makes sense to me.

sys/arm/arm/machdep.c
264–276 ↗(On Diff #17522)

Would it make sense to call it hw.fdt.dtb instead? Since it would be the entire DTB?

imp added inline comments.Jul 30 2016, 1:29 AM
lib/libfdt/Makefile
11–18

where are all these files?

sys/arm/arm/machdep.c
264–276 ↗(On Diff #17522)

Yes.

stevek added inline comments.Aug 3 2016, 1:00 AM
lib/libfdt/Makefile
11–18

They come from contrib/dtc/libfdt (the LIBFDTDIR setting)

stevek added a comment.EditedAug 3 2016, 11:42 PM

Having thought about this I'm not opposed to this. I think it should be two commits, one for the kernel bits, and another to build libfdt.a.

See D7411 for the updated kernel piece. I will update D6814 to be just the user space library part.

stevek updated this revision to Diff 19024.Aug 3 2016, 11:47 PM

Remove the kernel pieces, which are now being handled in D7411

stevek updated this object.Aug 3 2016, 11:49 PM
stevek retitled this revision from Add libfdt user space library and expose FDT blob via sysctl on ARM to Add libfdt user space library.Aug 4 2016, 1:36 AM
andrew edited edge metadata.Aug 11 2016, 9:28 AM

This should be attached to the build, and used by the GNU dtc.

lib/libfdt/Makefile
7

Do we want this as a shared library? I'm mostly concerned about future maintenance.

This should be attached to the build, and used by the GNU dtc.

I'm wondering should libfdt be conditionally built and, if so, should it be based on MK_FDT and MK_GPL_DTC (since the dtc build will be using it after the suggested changes)?

lib/libfdt/Makefile
7

Do you mean from a compatibility perspective?

andrew added inline comments.Aug 24 2016, 6:14 PM
lib/libfdt/Makefile
7

It would be for future compatibility, i.e. if we import a future version that changes it we will need a version bump. By just building a static library this isn't an issue.

stevek added inline comments.Sep 23 2016, 4:21 AM
lib/libfdt/Makefile
7

It might not be bad to start it as a static lib, especially considering the sources from dtc don't have much in the way of documentation.

ian added a subscriber: ian.Sep 3 2018, 3:09 PM

This should be attached to the build, and used by the GNU dtc.

I'm wondering should libfdt be conditionally built and, if so, should it be based on MK_FDT and MK_GPL_DTC (since the dtc build will be using it after the suggested changes)?

I think it should be conditional on MK_FDT, but not on MK_GPL_DTC, because the library itself is BSD-licensed.

imp added a comment.Sep 3 2018, 3:43 PM
In D6814#362680, @ian wrote:

This should be attached to the build, and used by the GNU dtc.

I'm wondering should libfdt be conditionally built and, if so, should it be based on MK_FDT and MK_GPL_DTC (since the dtc build will be using it after the suggested changes)?

I think it should be conditional on MK_FDT, but not on MK_GPL_DTC, because the library itself is BSD-licensed.

Yes. And there will be a day, I think, where we don't have the gpl dtc in the tree, but still have libfdt.

In D6814#362680, @ian wrote:

This should be attached to the build, and used by the GNU dtc.

I'm wondering should libfdt be conditionally built and, if so, should it be based on MK_FDT and MK_GPL_DTC (since the dtc build will be using it after the suggested changes)?

I think it should be conditional on MK_FDT, but not on MK_GPL_DTC, because the library itself is BSD-licensed.

Makes sense to me, I'll do the appropriate changes.

stevek updated this revision to Diff 49344.Oct 20 2018, 7:03 PM

Build libfdt as static library only
Update to latest head libnames.mk changes.

stevek updated this revision to Diff 49346.Oct 20 2018, 7:16 PM

Added fdt_addresses.c and fdt_overlay.c

kevans added a subscriber: kevans.Oct 23 2018, 5:07 PM
kevans added inline comments.
lib/libfdt/Makefile
3

In addition to question on IRC about pulling this from ^/sys/contrib/libfdt; I realize this probably predates the usage of SRCTOP becoming more common, but this should probably be w.r.t. ${SRCTOP} instead of ${.CURDIR}.