Page MenuHomeFreeBSD

nexus code tidy-up
ClosedPublic

Authored by mhorne on Feb 10 2023, 5:43 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, May 12, 2:08 PM
Unknown Object (File)
Sat, Apr 27, 7:29 AM
Unknown Object (File)
Sat, Apr 27, 7:29 AM
Unknown Object (File)
Sat, Apr 27, 7:24 AM
Unknown Object (File)
Sat, Apr 27, 7:24 AM
Unknown Object (File)
Sat, Apr 27, 5:28 AM
Unknown Object (File)
Feb 6 2024, 10:42 AM
Unknown Object (File)
Jan 14 2024, 7:27 AM

Details

Summary

Make a pass at the various nexus implementations, fixing some very minor style issues, obsolete comments, etc.

The method declaration sections have become quite unwieldy in many respects. Attempt to tame it by:

  1. Using generated method typedefs
  2. Grouping methods roughly by category, and then alphabetically.

Presented as one diff for review, but on the branch this is one commit per arch.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

@jhb this will obviously cause conflicts with your branch of changes. Although cosmetic changes should generally go first, I suspect it might be easier to rebase these changes on top of yours, compared to the other way around. There is absolutely no rush on committing this one, so let me know how you'd like me to proceed.

A few other changes to use DEFINE_CLASS_0 I would also leave out.

I think if we wanted to macro-ify driver_t objects, we should define a new macro specific to new-bus (like how we have DEVMETHOD as an alias of KOBJMETHOD) rather than using DEFINE_CLASS_0 directly. I know a few places use DEFINE_CLASS_1 out of necessity currently. Maybe a DEFINE_DRIVER? You could in that case even have a DEFINE_DRIVER_NO_SOFTC for that special case?

sys/arm/arm/nexus.c
151

I would maybe leave this part as-is of an explicit struct driver_t as that is the more common pattern in the tree vs using DEFINE_CLASS_0.

306

Extra indentation?

sys/riscv/riscv/nexus.c
129 ↗(On Diff #116952)

Similar thought on using of DEFINE_CLASS_0

In D38495#875983, @jhb wrote:

A few other changes to use DEFINE_CLASS_0 I would also leave out.

I think if we wanted to macro-ify driver_t objects, we should define a new macro specific to new-bus (like how we have DEVMETHOD as an alias of KOBJMETHOD) rather than using DEFINE_CLASS_0 directly. I know a few places use DEFINE_CLASS_1 out of necessity currently. Maybe a DEFINE_DRIVER? You could in that case even have a DEFINE_DRIVER_NO_SOFTC for that special case?

Sure, makes sense. I have seen DEFINE_CLASS_0 used more often in new code. What is/should be DEFINE_CLASS()? From the comment it is the historic API for this stuff, and is not very widely used. Am I right to consider it more-or-less deprecated? DEFINE_DRIVER() certainly seems clean enough for the general case.

I would be happy to codify this stuff more clearly in our documentation. I'm also not against simply reverting to simple driver_t definitions. They really aren't too ugly, just noticeably different from the places that must use DEFINE_CLASS_1.

In D38495#875983, @jhb wrote:

A few other changes to use DEFINE_CLASS_0 I would also leave out.

I think if we wanted to macro-ify driver_t objects, we should define a new macro specific to new-bus (like how we have DEVMETHOD as an alias of KOBJMETHOD) rather than using DEFINE_CLASS_0 directly. I know a few places use DEFINE_CLASS_1 out of necessity currently. Maybe a DEFINE_DRIVER? You could in that case even have a DEFINE_DRIVER_NO_SOFTC for that special case?

Sure, makes sense. I have seen DEFINE_CLASS_0 used more often in new code. What is/should be DEFINE_CLASS()? From the comment it is the historic API for this stuff, and is not very widely used. Am I right to consider it more-or-less deprecated? DEFINE_DRIVER() certainly seems clean enough for the general case.

DEFINE_CLASS is also for kobj use and not new-bus specific whereas DEFINE_DRIVER would be a new-bus API similar to DEVMETHOD vs KOBJMETHOD.

I would be happy to codify this stuff more clearly in our documentation. I'm also not against simply reverting to simple driver_t definitions. They really aren't too ugly, just noticeably different from the places that must use DEFINE_CLASS_1.

I think for this review it would be good to just revert to simple driver_t definitions and perhaps pursue adding DEFINE_DRIVER as a separate followup.

Drop DEFINE_CLASS_0 changes. The places where it is already in use (x86 and powerpc nexus.c) are untouched.

This revision is now accepted and ready to land.Mar 20 2023, 5:18 PM