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

[util: clock] clock utility now supports ASTCLOCK for pre-AT systems #57

Merged
merged 5 commits into from
Apr 14, 2024

Conversation

Mellvik
Copy link
Owner

@Mellvik Mellvik commented Apr 9, 2024

This PR expands the clock utility to support ASTCLOCK - seemingly the predominant RTC for surviving XT-class systems - thus bringing automatic date/time setting to this class of systems.

An integral part of AST's SixPakPlus and several other ISA-products for years, ASTCLOCK does what the CMOS RTC does on newer systems - in pretty much the same way, except for using different clock chips, and thus requiring different mechanisms for 'setting' and 'getting'.

AST started off using the NS 58167 chip, which looks a lot like the Motorola MC146818 used on the AT and later - and in the Dallas 'integrated' clock units. Then around 1986 AST switched to the simpler (and presumably cheaper) Ricoh RP5C15, a quite different animal (see pictures). The updated clock utility supports both chip-types with auto detection.

In order to tell clock to use the AST clock, an -A option has been added. Also, a /bootopts-option AST=1 has been added to simplify usage even further. ASTCLOCK= would have been easier to understand, but /bootopts keeps growing and we have severe space constraints.

IMG_9132
IMG_8947

@ghaerr
Copy link

ghaerr commented Apr 9, 2024

Hello @Mellvik,

Nice pictures, I'm pretty sure I have a few cards like that sitting around deep inside a box somewhere :)

Geez, between supporting IBM PC clock chips, PC-98 (at least for ELKS) and now AST, clock.c is a mess of ifdefs, and hard to follow. Actually it was never easy to follow, even at the start. And all of this hardware handling in a user program, not the kernel lol. One would think there's a better way to organize it all, by moving the hardware-specific routines into separate files perhaps? I never wanted to jump into it. Or, perhaps that kind of stuff should be in the kernel, but just adds porting complexity. Not sure what the best answer is to increase understandability.

I noticed your comment in init/main.c where a recently deleted /bootopts might not get detected as the low memory segment of the previous read remains... did that ever get fixed, or do we need a solution to that?

@ghaerr
Copy link

ghaerr commented Apr 9, 2024

a recently deleted /bootopts might not get detected as the low memory segment of the previous read remains... did that ever get fixed

I see, I found your enhancement to boot_minix.c that fixes the deleted /bootopts issue for MINIX, but not for FAT. Does that sound correct?

@Mellvik
Copy link
Owner Author

Mellvik commented Apr 9, 2024

Thanks @ghaerr!

Nice pictures, I'm pretty sure I have a few cards like that sitting around deep inside a box somewhere :)

There is indeed a lot of history here. It haven't checked but I'm pretty sure these cards have decent prices on ebay...

Geez, between supporting IBM PC clock chips, PC-98 (at least for ELKS) and now AST, clock.c is a mess of ifdefs, and hard to follow. Actually it was never easy to follow, even at the start. And all of this hardware handling in a user program, not the kernel lol. One would think there's a better way to organize it all, by moving the hardware-specific routines into separate files perhaps? I never wanted to jump into it. Or, perhaps that kind of stuff should be in the kernel, but just adds porting complexity. Not sure what the best answer is to increase understandability.

Messy indeed. And kinda ugly. The latter is unavoidable - since 'outside the kernel when possible' is a priority. I decided against adding #ifdef CONFIG_HW_PCXT thinking it would add to the mess rather than improve it. OTOH, some compartmentalization - as you suggest - would be beneficial. Might take a stab at that later.
that said, it doesn't seem likely that we will continue to add more clock types, does it?

This one fixes my itch for now :-)

@Mellvik
Copy link
Owner Author

Mellvik commented Apr 9, 2024

a recently deleted /bootopts might not get detected as the low memory segment of the previous read remains... did that ever get fixed

I see, I found your enhancement to boot_minix.c that fixes the deleted /bootopts issue for MINIX, but not for FAT. Does that sound correct?

That's right. The clue being (as I recollect) the 'Linux OK' replacing 'Linux loaded' in the boot sequence... I never cared about FAT boot, and never used it.

@ghaerr
Copy link

ghaerr commented Apr 9, 2024

it doesn't seem likely that we will continue to add more clock types, does it?

Famous last words!!! :)

I decided against adding #ifdef CONFIG_HW_PCXT thinking it would add to the mess rather than improve it

Agree there.

some compartmentalization - as you suggest - would be beneficial

If the new code were put into a separate file, that would automatically "create" a function API for it, as the two source files would have to communicate. I only mention it here because its still nice to think of more easily sharing between ELKS and TLVC. I've added a reminder to my list also.

@Mellvik
Copy link
Owner Author

Mellvik commented Apr 10, 2024

a recently deleted /bootopts might not get detected as the low memory segment of the previous read remains... did that ever get fixed

I see, I found your enhancement to boot_minix.c that fixes the deleted /bootopts issue for MINIX, but not for FAT. Does that sound correct?

That's right. The clue being (as I recollect) the 'Linux OK' replacing 'Linux loaded' in the boot sequence... I never cared about FAT boot, and never used it.

BTW - thanks for flagging this, that comment needs to go.

@Mellvik
Copy link
Owner Author

Mellvik commented Apr 10, 2024

it doesn't seem likely that we will continue to add more clock types, does it?

Famous last words!!! :)

Hmmmmmmmmmmm, I went to the Linux source tree looking for smart ways to probe for the 'normal' CMOS RTC (the mc146818), and found 'drivers' for maybe 100+ clock types. No sh... This world is nothing if not full of surprises. Not that we need to worry about them, they all emulate the 'original' Motorola chip, but then again, what an incredible mess.

I decided against adding #ifdef CONFIG_HW_PCXT thinking it would add to the mess rather than improve it

Agree there.

some compartmentalization - as you suggest - would be beneficial

If the new code were put into a separate file, that would automatically "create" a function API for it, as the two source files would have to communicate. I only mention it here because it's still nice to think of more easily sharing between ELKS and TLVC. I've added a reminder to my list also.

I fully agree. That said, creating it in a 'good' way is much more up your alley than mine. Kick it off and I'll contribute. In the meanwhile I'll reorganize the current mess a bit - and replace the AST= switch with a CMOS RTC probe.

@ghaerr
Copy link

ghaerr commented Apr 11, 2024

That said, creating it in a 'good' way is much more up your alley than mine. Kick it off and I'll contribute.

I took a hard look at this and ELKS clock.c and have concluded that your (this) version is already quite a bit better off than the original, thanks to your function renaming using ast_ and cmos_ in almost all places for AST and PC/CMOS clock functions. Perhaps consider renaming every hardware-specific routine with the same prefix(s) if you haven't already done so.

The entire clock.c source could probably use a pass through clang-format, (or indent -i4 -br -ce -npcs -nsob -bap -psl -l78 -nut -di8 -ldi8) in my opinion.

If you test this version for CMOS and AST hardware, I'll wait until after your final commit, copy it back to ELKS, and then rename the PC-98 routines, unless you'd like to. We can then have @tyama501 test the PC-98 version. I'm not quite sure whether you've deleted the PC-98 stuff, or just moved it around.

With all the ast_, cmos_ and pc98_ routines grouped at the top of the clock.c source file, it should be fairly straightforward to then ifdef or otherwise handle the calling the required routines in main and other general functions. The function naming scheme you've already invented should allow the cleanup to proceed nicely without having to break out hardware-specific functions into separate files, so no "API" will be needed as I had previously suggested.

Nice work!

@Mellvik
Copy link
Owner Author

Mellvik commented Apr 11, 2024

Sounds like plan, @ghaerr - I do have an update which needs some more testing. I'll add some pc98 adjustments and post tomorrow (no, the PC98 stuff is intact - I decided to keep it in TLVC for - you said it - cross compatibilty).

I need to take closer look at indent. Last time I used it, it had one (1) option, the indent. A quick look at the manpage suggests that must be aeons ago. Thanks.

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