Page MenuHomeFreeBSD

Introduce aw_syscon(4) for earlier attachment
ClosedPublic

Authored by kevans on Jan 7 2018, 5:57 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Dec 5, 9:25 AM
Unknown Object (File)
Nov 8 2024, 8:21 AM
Unknown Object (File)
Oct 28 2024, 10:55 AM
Unknown Object (File)
Sep 30 2024, 2:32 AM
Unknown Object (File)
Sep 30 2024, 1:50 AM
Unknown Object (File)
Sep 27 2024, 8:07 PM
Unknown Object (File)
Sep 27 2024, 2:36 PM
Unknown Object (File)
Sep 26 2024, 6:39 PM
Subscribers

Details

Summary

Attaching syscon_generic earlier than BUS_PASS_DEFAULT makes it more difficult for specific syscon drivers to attach to the syscon node and to get ordering right. Further discussion yielded the following set of decisions:

  • Move syscon_generic to BUS_PASS_DEFAULT
  • If a platform needs a syscon with different attach order or probe behavior , it should subclass syscon_generic and match on the SoC specific compat string
  • When we come across a need for a syscon that attaches earlier but only specifies compatible = "syscon", we should create a syscon_exclusive driver that provides generic access but probes earlier and only matches if "syscon" is the only compatible. Such fdt nodes do exist in the wild right now, but we don't really use them at the moment.

Additionally:

  • Any syscon provider that has needs any more complex than a spinlock solely for syscon access and a single memory resource should subclass syscon directly rather than attempting to subclass syscon_generic or add complexity to it. syscon_generic's attach/detach methods may be made public should the need arise to subclass it with additional attach/detach behavior.

Introduce aw_syscon(4) that just subclasses syscon_generic but probes earlier to meet our requirements for if_awg and implements #2 above for this specific situation. It currently only matches a64/a83t/h3 since these are the only platforms that really need it at the time being.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 14178

Event Timeline

You should make aw_syscon(4) a subclass of syscon_generic. This will allow it to pick up future changes without you having to change all the SoC specific drivers.

sys/arm/allwinner/aw_syscon.c
69–81

You don't need these in a subclass.

91–95

Replace this with DEFINE_CLASS_1(..., syscon_generic);

sys/dev/extres/syscon/syscon_generic.c
145

These can stay static.

sys/dev/extres/syscon/syscon_generic.h
30

Add DECLARE_CLASS(syscon_generic); to make it subclassable.

bcr added a subscriber: bcr.

OK from manpages.

sys/dev/extres/syscon/syscon_generic.h
41

Does this file need to contain anything other than DECLARE_CLASS? Seems like the softc can remain private.

sys/dev/extres/syscon/syscon_generic.h
41

Forgive my ignorance here- does the subclassing driver need knowledge of the size of the softc, given that it'll generally need one the same size?

Address concerns by @andrew - subclass syscon_generic_driver, providing only aw_syscon_probe.

syscon_generic_{attach,detach} going back to being static and local to syscon_generic.c, and the syscon_generic_driver class gets declared in the header.

andrew added inline comments.
sys/dev/extres/syscon/syscon_generic.h
41

It needs to know the size so the child softc is large enough. The child may also need to access the contents, however this will depend on the driver.

kevans edited the summary of this revision. (Show Details)

I've been told I should also be supplying methods for initializing/destroying the base class softc, so turn the attach/detach methods into calls to softc_{init,destroy} and move their logic into those functions.

It was also suggested that syscon methods could be invoked from non-sleepable context, so the built-in mutex has been turned into a spinlock instead.

Further notes have been added to the summary.

I've been told I should also be supplying methods for initializing/destroying the base class softc, so turn the attach/detach methods into calls to softc_{init,destroy} and move their logic into those functions.

I would only worry about this when it's needed. I would normally then make the attach/detach functions public.

kevans edited the summary of this revision. (Show Details)

As per @andrew's comment, go back to private init/teardown bits and make it explicit in the commit message that syscon_generic's attach/detach may be made public as necessary. It seems like a good approach to discourage future me at the very least from again implementing an attach that just calls base attach.

This revision is now accepted and ready to land.Jan 13 2018, 5:15 PM
This revision was automatically updated to reflect the committed changes.