Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix A/UX tablet panic take 2 #5

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

peads
Copy link

@peads peads commented Nov 10, 2024

Successfully removes driver when A/UX boots. Now to reconnect it afterwards.

Successfully reattaches, but does not yet work.

incremental

removed unnecessary debug info, reverted changed debug outputs

Formatted for readability

Removed cruft. Added another case where a different control signal value is sent by A/UX to the device

Added heap init flag to slotexec, s.t. the device now receives the signal when, for example, the machine goes down for reboot, and it can then handle the finalization correctly. Removed extraneous cases in the switch since it appears they are not invoked. It now comes back up properly after a reboot, but still no dice on after starting A/UX.

Reverted having moved InitLog for debugging purposes. Added it to finalize incase it hasn't already been so.

Removed extraneous flag for kDriverSupportDMSuspendAndResume, which is not used. Cleaned up doubly-applied gcc unused indicator.

Looks like the reboot issue in os8 was a sync problem. So, added an extra IO sync. Removed unused local in VFinal.

Added more verbose debug message. Fixed incorrect return value in finalize.

Changed control determination to use more accurate consts from the API. Removed unnecessary debug output(s). More reverse-engineering/rtfm of the dev docs trying to get A/UX to recognize the tablet.

Revert mask I added on the code switch

Revert pointless changes. Justify changes with commentary. Sqaush commits

Successfully removes driver when A/UX boots. Now to reconnect it afterwards.

Successfully reattaches, but does not yet work.

incremental

removed unnecessary debug info, reverted changed debug outputs

Formatted for readability

Removed cruft. Added another case where a different control signal value is sent by A/UX to the device

Added heap init flag to slotexec, s.t. the device now receives the signal when, for example, the machine goes down for reboot, and it can then handle the finalization correctly. Removed extraneous cases in the switch since it appears they are not invoked. It now comes back up properly after a reboot, but still no dice on after starting A/UX.

Reverted having moved InitLog for debugging purposes. Added it to finalize incase it hasn't already been so.

Removed extraneous flag for kDriverSupportDMSuspendAndResume, which is not used. Cleaned up doubly-applied gcc unused indicator.

Looks like the reboot issue in os8 was a sync problem. So, added an extra IO sync. Removed unused local in VFinal.

Added more verbose debug message. Fixed incorrect return value in finalize.

Changed control determination to use more accurate consts from the API. Removed unnecessary debug output(s). More reverse-engineering/rtfm of the dev docs trying to get A/UX to recognize the tablet.

Revert mask I added on the code switch

Revert pointless changes. Justify changes with commentary. Sqaush commits
@peads peads changed the title init Fix A/UX tablet panic take 2 Nov 10, 2024
@elliotnunn
Copy link
Owner

Thanks for revising the patchset. I really appreciate you addressing this serious deficiency in the drivers. Few thoughts:

  1. The transport-ndrv.c file will need a VFinal function too, or the PowerPC version won't compile.
  2. The QFinal and VFinal functions could be simpler: the former only needs to call FreePages, and the latter only needs to issue a device reset and remove the interrupt handlers. I was thinking of writing something like "atexit" to handle cleanup when the driver closes... what do you think of that?
  3. I understand that the dNeedGoodByeMask flag is specifically for Desk Accessories to handle the system heap going away on pre-MultiFinder systems -- not useful for device drivers. Does driver shutdown still work if it is removed?
  4. Would you mind separating out the changes to prevent warnings into a separate code cleanup commit?

@peads
Copy link
Author

peads commented Nov 12, 2024

  1. Indeed. Sorry about that. Comes from not emulating the ppc side at all; I wasn't compiling the ppc side at all. I'll stub it out.
  2. Yeah, I can't see there being a problem with adding an atexit call to make sure everything is cleaned-up when destroyed, but there's a caveat here: it'd probably need to be custom-implemented since we'd want to have it be able to take an argument like on_exit, but that function isn't POSIX-compliant (iirc, it's GNU-specific).
  3. Unfortunately, yes, it is required for this purpose.dNeedGoodByeMask is the only reliable way I can get the signal that rebooting/starting-A/UX has occurred. In fact, I chose it specifically because the documentation indicates that when the heap is initialized, a driver should respond to either killCode, or goodbye by cleaning up its resources (pp. 1-27 and 1-35 https://developer.apple.com/library/archive/documentation/mac/pdf/Devices/Device_Manager.pdf). Technically, we could have it jump to IODone in the case of goodbye, but while also taking care that killCode must return control via an rts. Although, I suspect the branch testing the kind against kImmediateIOCommandKind at the end of the function handles this already.
  4. Certainly, I'll push another commit after I've addressed the concerns in 1 and 2.

@elliotnunn
Copy link
Owner

Patrick, thanks for your help with this. I have pushed a change that makes the drivers clean up neatly on close. (I wrote a longer reply to this message but it seems to have disappeared.) Let me know if there are still problems starting A/UX.

Moving the cursor while restarting with still cause a dsBadSlotInt though, because the shutdown process doesn't close drivers. I'm not sure what to do about this.

@elliotnunn
Copy link
Owner

Warnings also fixed and -Wall enabled. Are you interested in taking a crack at true A/UX driver support?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants