lnls-dig / openMMC

Open source firmware for MMC controllers
GNU General Public License v3.0
39 stars 32 forks source link

Improve AFCv3.1 RTM presence detection workaround #181

Closed augustofg closed 1 year ago

augustofg commented 1 year ago

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: https://github.com/lnls-dig/openMMC/blob/3b7ae353018cbc15eb9325241074fbee9fe98cb3/port/board/rtm-8sfp/rtm_user.c#L67-L86

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 commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

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 commented 9 months ago

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