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

Improve AFCv3.1 RTM presence detection workaround #181

Closed
augustofg opened this issue Oct 20, 2023 · 4 comments · Fixed by #182
Closed

Improve AFCv3.1 RTM presence detection workaround #181

augustofg opened this issue Oct 20, 2023 · 4 comments · Fixed by #182

Comments

@augustofg
Copy link
Member

Due to a design oversight in the AFCv3.1, it is not possible to configure P0.29 (RTM_PS#) and P0.30 (EN_I2C_RTM) direction independently, thus you can't access the RTM I2C bus while checking the RTM presence signal RTM_PS#. The current workaround ignores the RTM_PS# signal and uses the ACK / NACK I2C response of some RTM device to check for its presence, but there are two problems with this approach. First the code is implemented in the RTM board driver (port/board/rtm-8sfp/rtm_user.c), second it is not necessary for the AFCv4.

My proposal is to remove the rtm_check_presence function from the RTM board driver:

void rtm_check_presence( uint8_t *status )
{
/* Due to a hardware limitation in the AFC board, we can't rely on reading the PS signal
since this pin doesn't have a pull-up resistor, it's always read as 0.
A very dirty workaround is to 'ping' the RTM IO Expander(PCA9554), if it responds, then the board is connected */
rtm_enable_i2c();
uint8_t i2c_addr, i2c_interface;
uint8_t dumb;
/* Defaults to absent - in case of I2C failure */
*status = HOTSWAP_STATE_URTM_ABSENT;
if (i2c_take_by_chipid( CHIP_ID_RTM_PCA9554, &i2c_addr, &i2c_interface, 100)) {
if (xI2CMasterRead( i2c_interface, i2c_addr, &dumb, 1)) {
*status = HOTSWAP_STATE_URTM_PRSENT;
}
i2c_give(i2c_interface);
}
}

Move it to a new file located in port/board/afc-v3/rtm.c and port/board/afc-v4/rtm.c, and replace it with a better implementation that doesn't assume the I2C mapping of the RTM card in question. My idea for AFCv3.1 is: try locking the RTM I2C bus using i2c_take_by_busid, configure P0.29 and P0.30 as inputs then check the RTM presence via RTM_PS#. Finally, configure pins P0.29 and P0.30 to outputs. Please use the mmc_err return type instead of void so errors and timeouts can be properly dealt with.

For the AFCv4.0 the new rtm_check_presence function can just check the RTM_PS# signal without any I2C locking since it doesn't suffer from the AFCv3.1 hardware bug.

@augustofg
Copy link
Member Author

Humm, I forgot that there is no pull-up resistor in the RTM_PS# signal in the AFCv3.1. We should check if it is possible to enable internal pull-ups for these pins, otherwise my proposed fix will not be possible.

@gustavosr8
Copy link
Contributor

@augustofg according to 8.10.1 section of the LPC17XX datasheet:

Pull-up/pull-down resistor configuration and open-drain configuration can be programmed through the pin connect block for each GPIO pin.

@augustofg
Copy link
Member Author

Unfortunately, it is not possible to implement my proposed workaround due to the lack of a pull-up resistor in the AFCv3.1 and the fact that the internal pull-ups are not available for the P0.29 pin, so checking for an I2C device is the only way to detect the RTM presence on AFCv3.1 cards.

Still, I think that we can improve the code by moving the rtm_check_presence function to the AFC board port and check for the RTM FRU EEPROM, so it can be 'generic' for any RTM card.

@augustofg
Copy link
Member Author

Obs: the EN_I2C_RTM signal is not necessary because the I2C buffer is not populated in the AFCv3.1 BPM and Timing variants. The RTM I2C bus is accessed via the I2C MUX port 3:
screenshot-2024-02-20_10-48-06

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

Successfully merging a pull request may close this issue.

2 participants