wwarthen / RomWBW

System Software for Z80/Z180/Z280 Computers
GNU Affero General Public License v3.0
339 stars 99 forks source link

HBIOS/sd.asm should initialize CSIO to full speed regardless of whether there are SDCards present. #171

Closed durgadas311 closed 3 years ago

durgadas311 commented 3 years ago

I'd like to request that CSIO get fully initialized regardless of whether any SDCards are initialized. From what I can tell, and observe, if no SDCards are inserted then the driver leaves the CSIO initialized at the slowest speed. Here is the situation that came up:

If a non-SDCard device is attached to the second CSIO device, instead of a second SDCard, then CSIO gets initialized differently if there is an SDCard or none. In these cases, a WizNet W5500 is attached to the second CSIO device.

While the network code could force-initialize CSIO to ensure it runs at full speed, this could possibly break the SDCard driver in the future - if there is ever a reason to use a different/custom speed. I'm not sure if this will ever be the case, but it might be nice to ensure that this common hardware (the CSIO) is only initialized once, and that initialization is not reversed by other code.

durgadas311 commented 3 years ago

I'm not sure if RomWBW handles SDCard "hot plug", and whether the OS can remain running through that. But, if the driver does not always return the CSIO to full speed it could cause problems for the other device.

wwarthen commented 3 years ago

RomWBW does handle hot plugging of SD Cards during a live OS. There are some constraints depending on the OS. For example, CP/M 2.2 requires you to be at the command prompt and use restart (ctrl-C) after plugging it in.

You are absolutely correct about generic initialization of the CSIO interface. Since the use of a Z180 implies that a CSIO exists, the CSIO can and should be initialized regardless of devices using it. I will do something about this very shortly, but may be tomorrow.

Obviously, the CSIO should be initialized to either low or high speed at startup and any driver that uses CSIO should adjust the speed as needed, but leave it restored to the default speed. I guess you are suggesting that high speed be the default since that would mean less instances of the drivers needing to change the speed. The drivers would just lower the speed during the device initialization process (if needed by the device). All of the CSIO drivers will need to abide by the convention, but I'm not ready to create an API for the CSIO at this point.

Thanks!

Wayne

durgadas311 commented 3 years ago

Yeah, good point(s). I'll talk to the CP/NET CSIO developer about saving the current speed, forcing high-speed, then restoring the previous speed. That's probably the right thing to do.

We ran into problems with slow speed because, it appears, that the CSIO transmit actually clears TE before the last bit gets sent. Our code checks TE before turning off chip-select, and at slow speed it ends up truncating the transfer because the last bit was not completed. I did not dig into this area of your code, but was wondering if you had seen this issue and handled it.

wwarthen commented 3 years ago

Yeah, good point(s). I'll talk to the CP/NET CSIO developer about saving the current speed, forcing high-speed, then restoring the previous speed. That's probably the right thing to do.

I like this approach. Since the Z180 registers are all readable, any driver can get and save the state at function entry and restore at exit.

We ran into problems with slow speed because, it appears, that the CSIO transmit actually clears TE before the last bit gets sent. Our code checks TE before turning off chip-select, and at slow speed it ends up truncating the transfer because the last bit was not completed. I did not dig into this area of your code, but was wondering if you had seen this issue and handled it.

Yes!!! This also burned me at one point. In the sd.asm code, you will see a small delay function after many of the "WAIT_TX" calls. It is for this exact reason. This is challenging my memory, but I believe that the issue only existed on one one of the two variants of the chip. I think I only had to add the delay when dealing with the Rev. K chips. However, for safety, the delay is used regardless.

wwarthen commented 3 years ago

Just checked in (dev branch) an update that initializes the Z180 CSIO at system startup. A constant called Z180_CNTR_DEF has been created to hold the default value. This value can be used by drivers to restore the default value after modification. The sd.asm driver still needs to be updated to set and restore the CNTR register as needed.

@durgadas311 The current default value is the slowest speed available for the CSIO. Is this OK?

-Wayne

durgadas311 commented 3 years ago

I think that will be fine. For CP/NET, this code will have already been run so it should just save the current value and restore it after.

wwarthen commented 3 years ago

Thanks Douglas. Based on your feedback, I will close this issue. I will put the corresponding changes to sd.asm on the todo list, but those changes won't really affect your work based on the current approach.

Thanks again for all of your help with this.

-Wayne