Skip to contentSkip to conversation

drivers/fsi: FSI protocol updates to improve reliability

PATCH In review Christopher Bostic 2 participants 2 messages Apr 10, 2017 FSI

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 drivers/fsi: FSI protocol updates to improve reliability

Re-order delays in the clock toggle from: delay clock 0, delay, clock 1 to clock 0, delay, clock 1, delay. Allows removal of delay added just prior to sampling SDA line for SDA input.

Slow down the delay period from 1ns to 3us. The original 1ns delay was actually 1-2 us based on real time measurements so this isn't a factor of 3000x slowdown.

Removed echo delay on send command as its not needed.

Removed the 100x prime clocks after command and now send 50x clocks prior to command as is done by the pdbg tool.

---
 drivers/fsi/fsi-master-gpio.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)
drivers/fsi/fsi-master-gpio.c +5 -8
index a8976d5..1517c21 100644
@@ -13,7 +13,7 @@
 
 #include "fsi-master.h"
 
-#define	FSI_GPIO_STD_DLY	1	/* Standard pin delay in nS */
+#define	FSI_GPIO_STD_DLY	3	/* Standard pin delay in uS */
 #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 */
@@ -61,7 +61,6 @@
 #define	FSI_GPIO_CRC_SIZE	4
 #define	FSI_GPIO_MSG_ID_SIZE		2
 #define	FSI_GPIO_MSG_RESPID_SIZE	2
-#define	FSI_GPIO_PRIME_SLAVE_CLOCKS	100
 
 static DEFINE_SPINLOCK(fsi_gpio_cmd_lock);	/* lock around fsi commands */
 
@@ -86,10 +85,10 @@ static void clock_toggle(struct fsi_master_gpio *master, int count)
 	int i;
 
 	for (i = 0; i < count; i++) {
-		ndelay(FSI_GPIO_STD_DLY);
 		gpiod_set_value(master->gpio_clk, 0);
-		ndelay(FSI_GPIO_STD_DLY);
+		udelay(FSI_GPIO_STD_DLY);
 		gpiod_set_value(master->gpio_clk, 1);
+		udelay(FSI_GPIO_STD_DLY);
 	}
 }
 
@@ -97,7 +96,6 @@ static int sda_in(struct fsi_master_gpio *master)
 {
 	int in;
 
-	ndelay(FSI_GPIO_STD_DLY);
 	in = gpiod_get_value(master->gpio_data);
 	return in ? 1 : 0;
 }
@@ -287,8 +285,6 @@ static int poll_for_response(struct fsi_master_gpio *master, uint8_t expected,
 			fsi_master_gpio_error(master, FSI_GPIO_CRC_INVAL);
 			return -EIO;
 		}
-		/* Clock the slave enough to be ready for next operation */
-		clock_toggle(master, FSI_GPIO_PRIME_SLAVE_CLOCKS);
 		return 0;
 
 	} while (busy_count++ < FSI_GPIO_MAX_BUSY);
@@ -362,8 +358,9 @@ static int send_command(struct fsi_master_gpio *master,
 	int rc;
 
 	spin_lock_irqsave(&fsi_gpio_cmd_lock, flags);
+	set_sda_output(master, 1);
+	clock_toggle(master, 50);
 	serial_out(master, cmd);
-	echo_delay(master);
 	rc = poll_for_response(master, expected, size, val);
 	spin_unlock_irqrestore(&fsi_gpio_cmd_lock, flags);
 
Signature · 1 line
1.8.2.2
Acked-by on v1 1/1
#1 lore raw
Quoted 70 lines · Christopher Bostic

Re-order delays in the clock toggle from: delay clock 0, delay, clock 1 to clock 0, delay, clock 1, delay. Allows removal of delay added just prior to sampling SDA line for SDA input.

Slow down the delay period from 1ns to 3us. The original 1ns delay was actually 1-2 us based on real time measurements so this isn't a factor of 3000x slowdown.

Removed echo delay on send command as its not needed.

Removed the 100x prime clocks after command and now send 50x clocks prior to command as is done by the pdbg tool.

---
  drivers/fsi/fsi-master-gpio.c | 13 +++++--------
  1 file changed, 5 insertions(+), 8 deletions(-)
drivers/fsi/fsi-master-gpio.c +3 -7
index a8976d5..1517c21 100644
@@ -13,7 +13,7 @@
 
  #include "fsi-master.h"
 
-#define	FSI_GPIO_STD_DLY	1	/* Standard pin delay in nS */
+#define	FSI_GPIO_STD_DLY	3	/* Standard pin delay in uS */
  #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 */
@@ -61,7 +61,6 @@
  #define	FSI_GPIO_CRC_SIZE	4
  #define	FSI_GPIO_MSG_ID_SIZE		2
  #define	FSI_GPIO_MSG_RESPID_SIZE	2
-#define	FSI_GPIO_PRIME_SLAVE_CLOCKS	100
 
  static DEFINE_SPINLOCK(fsi_gpio_cmd_lock);	/* lock around fsi commands */
 
@@ -86,10 +85,10 @@ static void clock_toggle(struct fsi_master_gpio *master, int count)
  	int i;
 
  	for (i = 0; i < count; i++) {
-		ndelay(FSI_GPIO_STD_DLY);
  		gpiod_set_value(master->gpio_clk, 0);
-		ndelay(FSI_GPIO_STD_DLY);
+		udelay(FSI_GPIO_STD_DLY);
  		gpiod_set_value(master->gpio_clk, 1);
+		udelay(FSI_GPIO_STD_DLY);
  	}
  }
 
@@ -97,7 +96,6 @@ static int sda_in(struct fsi_master_gpio *master)
  {
  	int in;
 
-	ndelay(FSI_GPIO_STD_DLY);
  	in = gpiod_get_value(master->gpio_data);
  	return in ? 1 : 0;
  }
@@ -287,8 +285,6 @@ static int poll_for_response(struct fsi_master_gpio *master, uint8_t expected,
  			fsi_master_gpio_error(master, FSI_GPIO_CRC_INVAL);
  			return -EIO;
  		}
-		/* Clock the slave enough to be ready for next operation */
-		clock_toggle(master, FSI_GPIO_PRIME_SLAVE_CLOCKS);
  		return 0;
 
  	} while (busy_count++ < FSI_GPIO_MAX_BUSY);

drivers/fsi/fsi-master-gpio.c static int send_command(struct fsi_master_gpio *master, · line 366

Quoted 10 lines
drivers/fsi/fsi-master-gpio.c +2 -1
@@ -362,8 +358,9 @@ static int send_command(struct fsi_master_gpio *master,
  	int rc;
 
  	spin_lock_irqsave(&fsi_gpio_cmd_lock, flags);
+	set_sda_output(master, 1);
+	clock_toggle(master, 50);
  	serial_out(master, cmd);
-	echo_delay(master);
  	rc = poll_for_response(master, expected, size, val);
  	spin_unlock_irqrestore(&fsi_gpio_cmd_lock, flags);
 

Looks good to me.