Page MenuHomeFreeBSD

Expand OpenFirmware API with ofw_bus_node_status_okay method
ClosedPublic

Authored by mw_semihalf.com on Jan 17 2017, 3:58 PM.
Tags
None
Referenced Files
F106387918: D9218.id24206.diff
Sun, Dec 29, 9:10 PM
Unknown Object (File)
Sat, Dec 28, 8:49 PM
Unknown Object (File)
Fri, Dec 20, 1:16 PM
Unknown Object (File)
Fri, Dec 20, 12:27 PM
Unknown Object (File)
Sat, Dec 7, 4:49 AM
Unknown Object (File)
Nov 17 2024, 10:10 AM
Unknown Object (File)
Nov 16 2024, 6:07 PM
Unknown Object (File)
Oct 28 2024, 10:29 PM

Details

Summary

Method could be used before we can access device_t structure.
As per simple phandle_t handle we can access FDT to check
if specified node has been enabled.
It will be used in Marvell's common configuration code.

Diff Detail

Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

mw_semihalf.com retitled this revision from to Expand OpenFirmware API with ofw_bus_node_status_okay method.
mw_semihalf.com updated this object.
mw_semihalf.com edited the test plan for this revision. (Show Details)
mw_semihalf.com added reviewers: ian, imp, zbb, fabient.
mw_semihalf.com set the repository for this revision to rS FreeBSD src repository - subversion.
mw_semihalf.com added a subscriber: ARM.
zbb edited edge metadata.

This is a useful change. +1

This revision is now accepted and ready to land.Jan 17 2017, 6:31 PM
mmel requested changes to this revision.Jan 18 2017, 7:17 AM
mmel added a reviewer: mmel.
mmel added a subscriber: mmel.

We have fdt_is_enabled() function, why you want new, duplicated, one?
Also note that 'ok' is also valid value for 'status' property (but yes, fdt_is_enabled() have same issue).

This revision now requires changes to proceed.Jan 18 2017, 7:17 AM

fdt_is_enabled is deprecated and should be removed as soon as there is an appropriate OpenFirmware function.

It was also decided to implement new OFW function, because fdt_is_enabled would be used by Marvell platforms at an early stage and it simply fails there with:

panic: pmap_fault: pm_pt1 abort
cpuid = 0
Uptime: 1s

because malloc is not yet available.

So, once a check for 'status = "ok"' is added, are there any objections to patch?

mmel edited edge metadata.

No, of course, not. And sorry for late response.
But remember, /sys/dev/ofw is under Nathan's jurisdiction. So you still needs his positive review.

This revision is now accepted and ready to land.Jan 18 2017, 9:32 PM

Love the concept, have issues with the implementation. :)

sys/dev/ofw/ofw_bus_subr.c
189–191

So if the property doesn't exist, we assume it's OK?

194

Wouldn't it be better to do something like:

if (len == 4 and bcmp(status, "okay", len) == 0)

since this will match 'o', 'ok' and 'oka' which isn't quite right. And properties aren't 'C' strings so bcmp would be better, no?

sys/dev/ofw/ofw_bus_subr.c
189–191

Yes. Node is enabled in 3 cases:

  • lack of status property
  • status = "ok"
  • status = "okay"
194

Im ok with both fixing len and bcmp instead of strncmp.

mw_semihalf.com edited edge metadata.
mw_semihalf.com removed rS FreeBSD src repository - subversion as the repository for this revision.
  • Add checking 'status = "ok"' and 'status = "okay"' with fixed lenght argument.
  • Also use brcmp instead of strncmp.
This revision now requires review to proceed.Jan 19 2017, 6:02 PM
This revision was automatically updated to reflect the committed changes.