Page MenuHomeFreeBSD

[man] Add documentation for OpenFirmware API
ClosedPublic

Authored by gonzo on Feb 26 2018, 1:34 AM.

Details

Summary

Add man pages for number of OF_* functions that are used
in OpenFirmware-compatible systems. Please note that
documentation in OF_getprop.9 does not match current
property management API but matches what it will be
once functional and style bugs are fixed. This diff
will not be committed until API is fixed I just want
to start editorial review as sson as possible.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

gonzo created this revision.Feb 26 2018, 1:34 AM
wblock added a subscriber: wblock.Mar 3 2018, 12:53 AM

There is a lot of redundancy in the "The function returns" sentences, which could be shortened by saying just "Returns" or rearranging some in a different order, like "Zero is returned..."

share/man/man9/OF_child.9
48 ↗(On Diff #39726)
returns the phandle value of the first child of the
50 ↗(On Diff #39726)
Zero is returned if there are no child nodes.
53 ↗(On Diff #39726)
returns the phandle for the parent of the
54 ↗(On Diff #39726)
.Fa node .
55 ↗(On Diff #39726)
Zero is returned if
60 ↗(On Diff #39726)
returns the phandle value of the next sibling of the
62 ↗(On Diff #39726)
Zero is returned if there is no sibling node.
share/man/man9/OF_device_from_xref.9
47 ↗(On Diff #39726)
When a device tree node references another node, the driver may
48 ↗(On Diff #39726)
need to get the device associated with the referenced node.

"get the device" needs to be more specific. Get a device_t instance?

49 ↗(On Diff #39726)
For instance, an Ethernet driver accessing a PHY device.
50 ↗(On Diff #39726)
To make this possible, the kernel maintains a table that
54 ↗(On Diff #39726)
adds a map entry from the effective phandle
58 ↗(On Diff #39726)
If a mapping entry for
60 ↗(On Diff #39726)
already exists, it is replaced with the new one.
64 ↗(On Diff #39726)
returns a device_t instance associated with the effective phandle
69 ↗(On Diff #39726)
returns the effective phandle associated with the device
71 ↗(On Diff #39726)
If no such mapping exists, the function returns 0.
share/man/man9/OF_finddevice.9
42 ↗(On Diff #39726)
returns the phandle for the node specified by the
44 ↗(On Diff #39726)
Returns -1 if the path cannot be found in the tree.
46 ↗(On Diff #39726)
The return value should only be checked with equality
47 ↗(On Diff #39726)

Make this more language-agnostic:

operators (equal to, not equal to) and not relational comparison
48 ↗(On Diff #39726)

As above, and start new sentences on new lines.

(less than, greater than).
There is a discrepancy between the IEEE 1275 standard and
49 ↗(On Diff #39726)
the
.Fx Ns 's
internal representation of a phandle: IEEE 1275
50 ↗(On Diff #39726)
requires the return value of this function to be -1 if the path
51 ↗(On Diff #39726)

Break sentence:

is not found.
But phandle_t is an unsigned type, so it cannot
52 ↗(On Diff #39726)
be relationally compared with -1 or 0, which

I'm not sure if "relationally" is the right word. I can think of others like "accurately" or "properly", but they don't seem much better.

share/man/man9/OF_getprop.9
79 ↗(On Diff #39726)
Device nodes can have associated properties.
80 ↗(On Diff #39726)
Properties consist of a name and a value.
82 ↗(On Diff #39726)
A value is an array of zero or more bytes that encode certain
86 ↗(On Diff #39726)
values or any combination of these types.
88 ↗(On Diff #39726)
Properties with a zero-byte length value usually represent boolean

Is the length being a byte value relevant if it is zero? Could we just say

Properties with a zero-length value usually represent boolean
92 ↗(On Diff #39726)

Possessive adds no value here:

A byte array is encoded as a sequence of bytes and represents
95 ↗(On Diff #39726)
A text string is a sequence of n printable characters.
It is
97 ↗(On Diff #39726)
characters represented as bytes plus a terminating null character.
107 ↗(On Diff #39726)
or 0 if the property exists but has no associated value.
108 ↗(On Diff #39726)
If
.Fa propname
does not exist, -1 is returned.
113 ↗(On Diff #39726)
copies a maximum of
121 ↗(On Diff #39726)
The function returns the actual size of the value or -1 if the
125 ↗(On Diff #39726)
copies a maximum of
129 ↗(On Diff #39726)
then converts cell values from big-endian to host byte order.
130 ↗(On Diff #39726)
The function returns the actual size of the value in bytes, or -1
133 ↗(On Diff #39726)

Spelling, but also unclear. Will it still work if len is not a multiple of 4? If not, this should say "must".

must be a multiple of 4.
138 ↗(On Diff #39726)
has a property specified by
140 ↗(On Diff #39726)
or zero if the property does not exist.
148 ↗(On Diff #39726)
If the property is found, the function copies a maximum of
153 ↗(On Diff #39726)
The function returns the actual size in bytes of the value,
154 ↗(On Diff #39726)
or -1 if the property does not exist.
162 ↗(On Diff #39726)
If the property is found, the function copies a maximum of
167 ↗(On Diff #39726)
then converts cell values from big-endian to host byte order.
168 ↗(On Diff #39726)
The function returns the actual size in bytes of the value,
169 ↗(On Diff #39726)
or -1 if the property does not exist.
172 ↗(On Diff #39726)
allocates memory large enough to hold the
178 ↗(On Diff #39726)
The function returns the actual size of the value and stores the
179 ↗(On Diff #39726)
address of the allocated memory in
181 ↗(On Diff #39726)
If the property has a zero-sized value,
184 ↗(On Diff #39726)
The function returns -1 if the property does not exist or
189 ↗(On Diff #39726)

Does "may sleep" mean I have permission to sleep while waiting for the memory allocation, or does it mean that the memory allocation might cause me to sleep? I suspect it's the second. So:

The function might sleep when allocating memory.
192 ↗(On Diff #39726)
allocates enough memory to hold the
200 ↗(On Diff #39726)
The actual size of the value is returned and the
201 ↗(On Diff #39726)
address of allocated memory is stored in

Note: many of the previous sections had this copy/paste "The function returns", which is redundant and can be shortened this way.

203 ↗(On Diff #39726)
If the property has a zero-length value,
205 ↗(On Diff #39726)
is set to NULL.
206 ↗(On Diff #39726)
The function returns -1 if the property does not exist or
211 ↗(On Diff #39726)
The function might sleep when allocating memory.
216 ↗(On Diff #39726)

Should this be

that was allocated by
218 ↗(On Diff #39726)
or
222 ↗(On Diff #39726)
copies a maximum of
224 ↗(On Diff #39726)

Not clear. Maybe:

.Fn OF_nextprop
copies
.Fa size
bytes of the name of the property following
.Fa propname
251 ↗(On Diff #39726)
If the property does not exist, the function tries to create
share/man/man9/OF_node_from_xref.9
47 ↗(On Diff #39726)
For instance, a framebuffer controller may refer to a GPIO
49 ↗(On Diff #39726)
For such implementations, the phandle is used by the kernel to
50 ↗(On Diff #39726)
identify the node does not match the value of the cell property that
51 ↗(On Diff #39726)

This sentence is difficult to understand.

52 ↗(On Diff #39726)
Otherwise, the effective phandle is equal to the kernel phandle.
56 ↗(On Diff #39726)
returns the kernel phandle for the effective phandle
58 ↗(On Diff #39726)
If one cannot be found or the OpenFirmware implementation
60 ↗(On Diff #39726)
the input value.
63 ↗(On Diff #39726)
returns the effective phandle for the kernel phandle
65 ↗(On Diff #39726)
If one cannot be found or the OpenFirmware implementation
67 ↗(On Diff #39726)
the input value.
share/man/man9/OF_package_to_path.9
44 ↗(On Diff #39726)
bytes of the fully qualified path to the device tree node
gonzo updated this revision to Diff 39924.Mar 3 2018, 7:55 PM
gonzo marked 78 inline comments as done.

Apply wblock@ edits and get rid of "The function returns"
in favor of just "Returns"

share/man/man9/OF_finddevice.9
52 ↗(On Diff #39726)

By "relationally" I meant "using greater than, less than" as opposing to equality check. Maybe it can give you some hints on how to phrase this better.

share/man/man9/OF_getprop.9
189 ↗(On Diff #39726)

It's the second.

224 ↗(On Diff #39726)

I don't think it's the right description. Function copies name to buf, if the name length is larger than size, it copies only size bytes.

share/man/man9/OF_node_from_xref.9
51 ↗(On Diff #39726)

I added more details to the example to illustrate the concept of reference.

This revision was not accepted when it landed; it landed in state Needs Review.Apr 9 2018, 10:24 PM
This revision was automatically updated to reflect the committed changes.