drivers/fsi: Add delay before sampling input on SDA
PATCH v2 1/1 drivers/fsi: Add delay before sampling input on SDA
During high cpu loads the SDA in line can shift relative to the clock signal which can corrupt the received input data. Slow down the time to sample input to account for this.
---
v2 - Increase delay for SDA in sampling only.
- Decrease the original delay for SDA sampling from 1us to 200ns
--- drivers/fsi/fsi-master-gpio.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/fsi/fsi-master-gpio.c +2 -1
@@ -14,6 +14,7 @@#include "fsi-master.h" #define FSI_GPIO_STD_DLY 1/* Standard pin delay in nS */ +#define FSI_GPIO_SDA_IN_DLY 200/* Wait to sample SDA line, in nS */ #define FSI_ECHO_DELAY_CLOCKS 16/* Number clocks for echo delay */ #define FSI_PRE_BREAK_CLOCKS 50/* Number clocks to prep for break */ #define FSI_BREAK_CLOCKS 256/* Number of clocks to issue break */
@@ -97,7 +98,7 @@ static int sda_in(struct fsi_master_gpio *master) {int in; -ndelay (FSI_GPIO_STD_DLY ); +ndelay (FSI_GPIO_SDA_IN_DLY ); in =gpiod_get_value (master->gpio_data);return in ?1 :0 ; }
Signature · 1 line
1.8.2.2
Quoted 34 lines · Christopher Bostic
During high cpu loads the SDA in line can shift relative to the clock signal which can corrupt the received input data. Slow down the time to sample input to account for this.
Signed-off-by Christopher Bostic <[email protected]>---
v2 - Increase delay for SDA in sampling only.
- Decrease the original delay for SDA sampling from 1us to 200ns--- drivers/fsi/fsi-master-gpio.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)drivers/fsi/fsi-master-gpio.c +2
-2@@ -14,6 +14,7 @@#include "fsi-master.h" #define FSI_GPIO_STD_DLY 1/* Standard pin delay in nS */ +#define FSI_GPIO_SDA_IN_DLY 200/* Wait to sample SDA line, in nS */ #define FSI_ECHO_DELAY_CLOCKS 16/* Number clocks for echo delay */ #define FSI_PRE_BREAK_CLOCKS 50/* Number clocks to prep for break */ #define FSI_BREAK_CLOCKS 256/* Number of clocks to issue break */ @@ -97,7 +98,7 @@ static int sda_in(struct fsi_master_gpio *master) {int in; -ndelay (FSI_GPIO_STD_DLY ); +ndelay (FSI_GPIO_SDA_IN_DLY ); in =gpiod_get_value (master->gpio_data);return in ?1 :0 ; } --
Quoted 2 lines
1.8.2.2
Hi Chris,
Quoted 3 lines
During high cpu loads the SDA in line can shift relative to the clock signal which can corrupt the received input data. Slow down the time to sample input to account for this.
This looks much better than the previous patch, but do you know why high CPU loads affect this? All of the GPIO interactions are performed with the single spinlock held, so CPU load should have no bearing on the timing of the bit-banging - the driver has exclusive use of the CPU during a FSI transaction.
Can you elaborate on the symptoms of this?
One thing that looks a little suspicious to me is that you're running the clock during the echo delay time, so you're potentially feeding random data to the slave...
Cheers,
Jeremy
Quoted 11 lines · Jeremy Kerr
Hi Chris,
Quoted 3 lines
During high cpu loads the SDA in line can shift relative to the clock signal which can corrupt the received input data. Slow down the time to sample input to account for this.
This looks much better than the previous patch, but do you know why high CPU loads affect this? All of the GPIO interactions are performed with the single spinlock held, so CPU load should have no bearing on the timing of the bit-banging - the driver has exclusive use of the CPU during a FSI transaction.
Can you elaborate on the symptoms of this?
Hi Jeremy,
Yes that's true, cpu load shouldn't be a factor in this case then. The failure mode is during the sampling of input data during the slave response phase of the command. CRC received does not match the CRC calculated due to bit flips related to sampling/clocking timing issues. This problem only appears when hostboot is accessing the BMC. Increasing delay does correct the issue. I don't have a good explanation for why this timing behavior occurs given its not cpu load related.
Quoted 5 lines
One thing that looks a little suspicious to me is that you're running the clock during the echo delay time, so you're potentially feeding random data to the slave...
Cheers,
The echo delay in the driver clocks '1's on the line so as long as the echo_delay() utility is used there is no risk of random data in this phase. Clocking during this phase also gives the slave more clocks to process the newly sent command so the entire time for processing a single command and generating a response takes less follow on clocks.
Thanks,
Chris
Quoted 3 lines
Jeremy
Hi Chris,
Quoted 3 lines
The echo delay in the driver clocks '1's on the line so as long as the
echo_delay()utility is used there is no risk of random data in this phase.
Ah, I'd missed the set_sda_output(1), there. That should be fine then.
Quoted 8 lines
Yes that's true, cpu load shouldn't be a factor in this case then. The failure mode is during the sampling of input data during the slave response phase of the command. CRC received does not match the CRC calculated due to bit flips
related to sampling/clocking timing issues. This problem only appears when hostboot is accessing the BMC. Increasing delay does correct the issue. I don't have a good explanation for why this timing behavior occurs given its not cpu load related.
My concern here is that with such a dramatic change in timing there (200x), and not knowing what the underlying problem is, we're just making the issue less likely to occur, rather than fixing it.
Do you know if we have access to HW folks to scope the data & clock? We could set a GPIO when we see a CRC error, to use as an external trigger...
Cheers,
Jeremy
Quoted 15 lines · Jeremy Kerr
Hi Chris,
Quoted 3 lines
The echo delay in the driver clocks '1's on the line so as long as the
echo_delay()utility is used there is no risk of random data in this phase.Ah, I'd missed the
set_sda_output(1), there. That should be fine then.Quoted 8 lines
Yes that's true, cpu load shouldn't be a factor in this case then. The failure mode is during the sampling of input data during the slave response phase of the command. CRC received does not match the CRC calculated due to bit flips
related to sampling/clocking timing issues. This problem only appears when hostboot is accessing the BMC. Increasing delay does correct the issue. I don't have a good explanation for why this timing behavior occurs given its not cpu load related.
Quoted 3 lines
My concern here is that with such a dramatic change in timing there (200x), and not knowing what the underlying problem is, we're just making the issue less likely to occur, rather than fixing it.
Hi Jeremy,
I understand your concern here. I agree we need to understand the root cause. I'll open up an issue to track this.
Quoted 4 lines
Do you know if we have access to HW folks to scope the data & clock? We could set a GPIO when we see a CRC error, to use as an external trigger...
I'll ask around and see what can be done. Taking a snap shot at time of recreate should be enough to see what's going on in clock and data timing when we fail, seems to be fairly consistent under some hostboot access of PNOR conditions.
Thanks,
Chris
Quoted 6 lines
Cheers,
Jeremy