Page MenuHomeFreeBSD

openfirm: Add OF_device_unregister_xref
AcceptedPublic

Authored by manu on Dec 28 2019, 6:44 PM.
Tags
None
Referenced Files
Unknown Object (File)
Jan 15 2024, 3:01 PM
Unknown Object (File)
Dec 20 2023, 5:52 AM
Unknown Object (File)
Nov 25 2023, 7:35 AM
Unknown Object (File)
Aug 18 2023, 12:43 AM
Unknown Object (File)
Jun 16 2023, 10:56 AM
Unknown Object (File)
Mar 4 2023, 10:48 AM
Unknown Object (File)
Feb 7 2023, 9:07 PM
Unknown Object (File)
Jan 14 2023, 2:12 AM
Subscribers

Details

Reviewers
gonzo
nwhitehorn
ian
imp
Group Reviewers
ARM
arm64
manpages
Summary

When using modules we want to unregister the xref as on a following load the device will not exist anymore.
The situation is the following :

  • One module have different drivers in it with dependencies between them.
  • Due to EARLY_DRIVER pass not being honored for modules (as we are already at the last pass) the order of probing will be the node order in the DTB.
  • During the first load driver1 which depend on driver2 will not get the xref and defers its attach. The next pass will get the xref and finish the attach.
  • After an unload the xref stays
  • During the second load, driver1 will get the old device_t reference for driver2 and we will fail if we try to call any methods from it.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

bcr added a subscriber: bcr.

OK from manpages. Don't forget to bump the .Dd when committing.

So devices are just detached on unload, not deleted, so the device_t shouldn't be stale ever...

Of course, it would be better to have a newbus invalidate callback when deleting a device node, but until then, this is fine

ian added a subscriber: ian.
In D22945#502734, @imp wrote:

So devices are just detached on unload, not deleted, so the device_t shouldn't be stale ever...

Of course, it would be better to have a newbus invalidate callback when deleting a device node, but until then, this is fine

IMO, detach needs to undo whatever attach does, and since the register is always done in attach (or delayed-attach), then detach should undo the registration.

Also, having an xref handle registered implicitly says "here is a device instance which is ready to service your calls to its methods". There may be some other device in the system with delayed-attach logic that is waiting for a co-device to appear. If an old instance of the co-device was still registered even though detached, that would mess up the "wait until my peer is ready" logic. (See freescale/imx/imx6_hdmi.c for an example of such code).

This revision is now accepted and ready to land.Dec 31 2019, 6:00 PM
In D22945#503622, @ian wrote:
In D22945#502734, @imp wrote:

So devices are just detached on unload, not deleted, so the device_t shouldn't be stale ever...

Of course, it would be better to have a newbus invalidate callback when deleting a device node, but until then, this is fine

IMO, detach needs to undo whatever attach does, and since the register is always done in attach (or delayed-attach), then detach should undo the registration.

Also, having an xref handle registered implicitly says "here is a device instance which is ready to service your calls to its methods". There may be some other device in the system with delayed-attach logic that is waiting for a co-device to appear. If an old instance of the co-device was still registered even though detached, that would mess up the "wait until my peer is ready" logic. (See freescale/imx/imx6_hdmi.c for an example of such code).

We likely need to move the peering logic down into newbus... This interface confuses the attached and not there states in many ways... having it in newbus can avoid the current ambiguity... the phandle associated with a device logically doesn't care, but the semantics of how this has been used differs from this ideal.

So this is good for the current state.