[linux,dev-4.7] drivers/fsi: FSI protocol updates to improve reliability

Submitted by Christopher Bostic on April 10, 2017, 5:55 p.m.

Details

Message ID 20170410175506.62007-1-cbostic@linux.vnet.ibm.com
State New
Headers show

Commit Message

Christopher Bostic April 10, 2017, 5:55 p.m.
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.

Signed-off-by: Christopher Bostic <cbostic@linux.vnet.ibm.com>
---
 drivers/fsi/fsi-master-gpio.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

Comments

eajames@linux.vnet.ibm.com April 10, 2017, 7:24 p.m.
On 04/10/2017 12:55 PM, Christopher Bostic wrote:
> 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.
>
> Signed-off-by: Christopher Bostic <cbostic@linux.vnet.ibm.com>
> ---
>   drivers/fsi/fsi-master-gpio.c | 13 +++++--------
>   1 file changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/fsi/fsi-master-gpio.c b/drivers/fsi/fsi-master-gpio.c
> index a8976d5..1517c21 100644
> --- a/drivers/fsi/fsi-master-gpio.c
> +++ b/drivers/fsi/fsi-master-gpio.c
> @@ -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);
>

Looks good to me.

Acked-by: Eddie James <eajames@linux.vnet.ibm.com>

Patch hide | download patch | download mbox

diff --git a/drivers/fsi/fsi-master-gpio.c b/drivers/fsi/fsi-master-gpio.c
index a8976d5..1517c21 100644
--- a/drivers/fsi/fsi-master-gpio.c
+++ b/drivers/fsi/fsi-master-gpio.c
@@ -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);