MIO reset_gpio collisions #105

Closed
opened 2023-10-18 16:38:02 +08:00 by mwojcik · 2 comments

Referring to m-labs/artiq-zynq#262 - it's a zynq-rs issue.

And on a bit more fundamental level.

In theory, there's nothing wrong with c15b54f92b - it's not too complicated, points to the correct pin, resets the GPIO to set the parameters, all good... wait, resets the GPIO? I remember something related to this... Oh yeah. Getting ETH parameters caused the red LED to disappear, so it had to be re-enabled.

That's because MIO pins for I2C were re-configured, and I2C was called to get Ethernet parameters to re-setup them for post-panic handling, and the I2C setup code reset the GPIO...

PHY_RSTn, I2C_*, ERR_LED and SD* are all on the same MIO bank. A reset to a single pin causes the others to go down and become disabled.

So what happens to cause m-labs/artiq-zynq#262?

After a boot, the following sequence happens:

  1. I2C is reset, MIO pins for I2C are configured, one reset_gpio().
  2. Mac addresses is pulled from EEPROM which uses I2C, I2C pins are configured again, second reset_gpio()
  3. PHY_RSTn is pulsed while configuring Ethernet, third reset_gpio(), but with only PHY_RSTn pin, disabling the rest.
  4. A DRTIO link is seen and there's an attempt to establish it, but since I2C pins are disabled, and when the LED I2C expander service tries to set the LEDs, no response is received from PCA954X, causing a panic.
  5. Ethernet is configured, another I2C configuration + reset_gpio; PHY_RSTn is also called...

There's also getting the config from the SD card before I2C and after first soft panic, but that doesn't reset the GPIO, so that probably doesn't need to be taken into consideration.

At some point another panic due to nested block_on! comes, but I'm not exactly sure where, didn't go that far. The main issue is multiple slcr.gpio_rst_ctrl.reset_gpio().

In my opinion, what should be done is all pins within one MIO bank should be configured in one place, with only one GPIO reset. Preferably make it a singleton too, or at least have a flag, to avoid resetting it multiple times. From that, particular modules could get control over their respective signals.

Quick hack that would get a working system would be resetting PHY before getting address information - but that's impossible, so re-configuring I2C after resetting PHY would get the code to work again. But that sounds like setting ourselves up for another failure down the line. We can do that before a proper solution is created.

Referring to m-labs/artiq-zynq#262 - it's a zynq-rs issue. And on a bit more fundamental level. In theory, there's nothing wrong with c15b54f92b3d4e125ae47a0dce7abe4b2bc9e054 - it's not too complicated, points to the correct pin, [resets the GPIO](https://git.m-labs.hk/M-Labs/zynq-rs/src/commit/c15b54f92b3d4e125ae47a0dce7abe4b2bc9e054/libboard_zynq/src/eth/mod.rs#L511) to set the parameters, all good... wait, resets the GPIO? I remember something related to this... [Oh yeah.](https://git.m-labs.hk/M-Labs/artiq-zynq/src/branch/master/src/runtime/src/comms.rs#L818) Getting ETH parameters caused the red LED to disappear, so it had to be re-enabled. That's because MIO pins for I2C were re-configured, and I2C was called to get Ethernet parameters to re-setup them for post-panic handling, and the I2C setup code reset the GPIO... ``PHY_RSTn``, ``I2C_*``, ``ERR_LED`` and ``SD*`` are all on the same MIO bank. A reset to a single pin causes the others to go down and become disabled. So what happens to cause m-labs/artiq-zynq#262? After a boot, the following sequence happens: 1. I2C is reset, MIO pins for I2C are configured, one ``reset_gpio()``. 2. Mac addresses is pulled from EEPROM which uses I2C, I2C pins are configured again, second ``reset_gpio()`` 3. PHY_RSTn is pulsed while configuring Ethernet, third ``reset_gpio()``, but with only PHY_RSTn pin, disabling the rest. 4. A DRTIO link is seen and there's an attempt to establish it, but since I2C pins are disabled, and when the LED I2C expander service tries to set the LEDs, no response is received from PCA954X, causing a panic. 5. Ethernet is configured, another I2C configuration + ``reset_gpio``; PHY_RSTn is also called... There's also getting the config from the SD card before I2C and after first soft panic, but that doesn't reset the GPIO, so that probably doesn't need to be taken into consideration. At some point another panic due to ``nested block_on!`` comes, but I'm not exactly sure where, didn't go that far. The main issue is multiple ``slcr.gpio_rst_ctrl.reset_gpio()``. In my opinion, what should be done is all pins within one MIO bank should be configured in one place, with only one GPIO reset. Preferably make it a singleton too, or at least have a flag, to avoid resetting it multiple times. From that, particular modules could get control over their respective signals. Quick hack that would get a working system would be resetting PHY before getting address information - but that's impossible, so re-configuring I2C after resetting PHY would get the code to work again. But that sounds like setting ourselves up for another failure down the line. We can do that before a proper solution is created.
Poster
Owner

Actually, it's even simpler than that. SD code was an inspiration.

Seems like reset_gpio is not necessary. I can boot normally and connect with DRTIO.

Actually, it's even simpler than that. SD code was an inspiration. Seems like ``reset_gpio`` is not necessary. I can boot normally and connect with DRTIO.
Poster
Owner

A quick study of the Zynq7000 TRM in the topic of MIO, GPIO, etc. confirms that the reset is indeed not necessary.

Two things to note:
An example on programming a MIO pin:
https://docs.xilinx.com/r/en-US/ug585-zynq-7000-SoC-TRM/MIO-Programming?tocId=P9bCXjn94eS8csby0C4Gaw

Reset affects bus interface, but not controller logic:
https://docs.xilinx.com/r/en-US/ug585-zynq-7000-SoC-TRM/Resets?tocId=1X11X3nBw3NHmOaoPknMew

Can't say for sure without experimenting too far, but I assume that means that pin configuration is not changed, but the state is. Possibly also resetting output enables, killing any possibility of I2C communication.

Reset was introduced long time ago, with EEPROM writing. I assume its purpose was to make sure there was no incorrect state on the I2C lines at the moment of configuration; but with multiple peripherals using the same MIO bank, it would mean interfering between each other; and it does seem unlikely that there has been anything else between bootup and I2C configuration that would interfere on the bus.

Anyway, the example in TRM shows that GPIO reset is not necessary to configure the function of a pin. But that behavior was blindly copied by me and @jmatyas, causing issues.

A quick study of the Zynq7000 TRM in the topic of MIO, GPIO, etc. confirms that the reset is indeed not necessary. Two things to note: An example on programming a MIO pin: https://docs.xilinx.com/r/en-US/ug585-zynq-7000-SoC-TRM/MIO-Programming?tocId=P9bCXjn94eS8csby0C4Gaw Reset affects bus interface, but not controller logic: https://docs.xilinx.com/r/en-US/ug585-zynq-7000-SoC-TRM/Resets?tocId=1X11X3nBw3NHmOaoPknMew Can't say for sure without experimenting too far, but I assume that means that pin configuration is not changed, but the state is. Possibly also resetting output enables, killing any possibility of I2C communication. Reset was introduced long time ago, with EEPROM writing. I assume its purpose was to make sure there was no incorrect state on the I2C lines at the moment of configuration; but with multiple peripherals using the same MIO bank, it would mean interfering between each other; and it does seem unlikely that there has been anything else between bootup and I2C configuration that would interfere on the bus. Anyway, the example in TRM shows that GPIO reset is not necessary to configure the function of a pin. But that behavior was blindly copied by me and @jmatyas, causing issues.
sb10q closed this issue 2023-10-20 17:41:39 +08:00
Sign in to join this conversation.
No Label
No Milestone
No Assignees
1 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: M-Labs/zynq-rs#105
There is no content yet.