Re: Question about spi_transfer_one_message

[sparticvs svcitraps <sparticvs@xxxxxxxxxx>]

And I did just realize that xfr->cs_change is the outer if-conditional on the last in the list, so I do realize that I missed that in the pseudo code, which I think makes my pseudo code almost correct and would instead mean that there is no need for the `keep_cs` variable. I updated the Pseudo-code inline since I just realized the rest.

BEGIN spi_transfer_one_message
cs_acquire()

FOR EACH xfr IN msg:

	ret := transfer_one(xfr)
	IF ret > 0:
		# wait for completion timeout (estimated)
		ms := estimate_xfr_time
	ENDIF

	IF xfr.delay:
		delay
	ENDIF

- 	IF is_last(xfr):
-		KEEP_CS := TRUE
-	ELSE:
+	IF xfr.cs_change AND NOT is_last(xfr):
		cs_release
		delay
		cs_acquire
	ENDIF

-	IF ret != OK OR NOT KEEP_CS:
+	IF ret != OK OR xfr.cs_change:
		cs_release
	ENDIF

	finalize_curr_msg()

END

> On Aug 1, 2016, at 10:29 AM, sparticvs svcitraps <sparticvs@xxxxxxxxxx> wrote:
> 
> Hello.
> 
> I am writing a spi device driver, and I ran into some interesting behavior (at least interesting in terms of this spi device) regarding chip select.
> 
> For the spi_transfer_one_message, the logic for chip select bouncing is as follows (in pseudo code to ensure I understand it correctly and I am skipping abort paths):
> 
> BEGIN spi_transfer_one_message
> cs_acquire()
> 
> FOR EACH xfr IN msg:
> 	ret := transfer_one(xfr)
> 	IF ret > 0:
> 		# wait for completion timeout (estimated)
> 		ms := estimate_xfr_time
> 	ENDIF
> 
> 	IF xfr.delay:
> 		delay
> 	ENDIF
> 
>  	IF is_last(xfr):
> 		KEEP_CS := TRUE
> 	ELSE:
> 		cs_release
> 		delay
> 		cs_acquire
> 	ENDIF
> 
> 	IF ret != OK OR NOT KEEP_CS:
> 		cs_release
> 	ENDIF
> 
> 	finalize_curr_msg()
> END
> 
> 
> From my understanding (and experiments) CS is held low even when there are no more transfers. What is the reason for this to be done this way?
> 
> The issue I am running into is that this spi-device (EPSON S1D13781 LCDC) requires that CS is bounced before the next command. I am not sure if this is required of all SPI devices, but it makes sense to me.
> 
> So, the only way that I could fix this issue, and get the proper behavior on chip was to change the code to:
> 
> 
> BEGIN spi_transfer_one_message
> cs_acquire()
> 
> FOR EACH xfr IN msg:
> 
> 	ret := transfer_one(xfr)
> 	IF ret > 0:
> 		# wait for completion timeout (estimated)
> 		ms := estimate_xfr_time
> 	ENDIF
> 
> 	IF xfr.delay:
> 		delay
> 	ENDIF
> 
> - 	IF is_last(xfr):
> -		KEEP_CS := TRUE
> -	ELSE:
> +	IF xfr.cs_change AND NOT is_last(xfr):
> 		cs_release
> 		delay
> 		cs_acquire
> 	ENDIF
> 
> -	IF ret != OK OR NOT KEEP_CS:
> +	IF ret != OK OR xfr.cs_change:
> 		cs_release
> 	ENDIF
> 
> 	finalize_curr_msg()
> 
> END
> 
> Basically, this always forces the CS hold to be released when completing a message. Does this make sense for all other use-cases? If so, I can start a patch, but I just wasn’t sure if the current behavior was expected for all other operations. The only place where I can see this being in conflict is the use of `spi_transfer::cs_change’, but that said, this logic could (and probably should) use the cs_change to determine if it should keep the CS held low instead of the original logic.

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html