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
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
Unknown Object (File)
Oct 28 2024, 4:27 PM
Unknown Object (File)
Oct 28 2024, 8:44 AM
Unknown Object (File)
Oct 18 2024, 1:44 PM
Unknown Object (File)
Oct 17 2024, 8:48 AM

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

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

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 ↗(On Diff #24122)

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

194 ↗(On Diff #24122)

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 ↗(On Diff #24122)

Yes. Node is enabled in 3 cases:

  • lack of status property
  • status = "ok"
  • status = "okay"
194 ↗(On Diff #24122)

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.