[coreboot] [RFC] [flashrom] "accelerated" high-level external programmer functions and serial external programmer protocol

[Carl-Daniel Hailfinger]

On 05.06.2009 23:12, Urja Rannikko wrote:
>> Yes, for that you need flow control. For example, you you have the
>> programmer return the amount of bytes free in the buffer after each
>> command stream you send to the device.
>>
>>     
> For a moment i was adding a "return bytes free in buffer" command to
> the protocol, but then i realized that if the code sent that command
> as the last in command stream, and waited for the answer, the answer
> would always be that the buffer is empty, because it just had to go
> though all the data in the buffer to get the command. So i'm sticking
> with the
> - query buffer size in the beginning
> - when doing opbuf writes, count bytes sent
> - if about to have sent too many bytes, instead send NOP and wait for
> the ACK and clear the counter and continue
>   

Good idea.

> I have the protocol specification ready and the AVR code compiles
> already, i'm just moving to flashrom code. I'll attach the .txt to
> this mail, it's better viewed with proper tabs and fixed-width font
> (leafpad fullscreen 1280x1024 is nice :P).

I like the protocol specification. A few minor design notes, though.

> Serial Flasher Protocol Specification
>
>
> Command And Answer Sequence - not all commands give an answer
> PC: LENGHT(8bit) COMMAND(8bit) <LENGHT PARAMETER BYTES>
> DEV: ACK/NAK(8bit) <OPTIONAL RETURN BYTES (only if ACK)> or nothing
>
> ACK = 0xAA
> NAK = 0x55
>   

Since you comunicate over serial, it may happen that one bit is
lost/inserted. Since you specified
ACK = NAK <<1
this could result in spurious acks if there are synchronization
problems. I have seen bitstream sync problems too often with SPI
communication, so I'm extra careful.
What about
ACK = 0x10 (Is Ok)
NAK = 0xBA (BAd)

Your choice, though. Anything else would be just as good if one value is
not a shift of the other.

> All multibyte values are little-endian.
>
> COMMAND	Description			Parameters			Return Value
> 0x00	NOP				none				ACK
>   

Maybe call it NOPACK.

> 0x01	Query serial buffer size	none				ACK + 16bit size / NAK
> 0x02	Query supported bustypes	none				ACK + 8-bit flags (as per flashrom) / NAK:
> 									bit 0: PARALLEL
> 									bit 1: LPC
> 									bit 2: FWH
> 									bit 3: SPI if ever supported
> 0x03	Query supported chip size	none				ACK + 8bit power of two / NAK
> 0x04	Query operation buffer size	none				ACK + 16bit size / NAK
> 0x05	Read byte			24-bit addr			ACK + BYTE / NAK
> 0x06	Read n bytes			24-bit addr + 24-bit lenght	ACK + lenght bytes / NAK
>   

lenght -> length

> 0x07	Initialize operation buffer	none				ACK / NAK
> 0x08	Write to opbuf: Write byte	24-bit addr + 8-bit byte	nothing	/ NAK (NOTE: takes 6 bytes in opbuf)
> 0x09	Write to opbuf: Write byte seq	8-bit byte			nothing / NAK (NOTE: takes 1 bytes in opbuf)
>   

Hm. I don't understand the opbuf lengths here.
By the way, having a single-byte write command (like you have) and a
multi-byte write command would probably be a more future-proof design.
0x09     Write to opbuf: Write byte seq 24-bit addr + 24-bit length + n
bytes data                   ACK/NAK (Note: takes 7+n bytes in opbuf)

> 0x0A	Write to opbuf: delay		32-bit usecs			nothing / NAK (NOTE: takes 5 bytes in opbuf)
>   

I'd like all commands to send a return code. All those write opcodes
could be returning ACK even before they are sent.

> 0x0B	Execute operation buffer	none				ACK / NAK 
> 		- Execute operation buffer will also clear it
> 0x??	unimplemented command		any				NAK
>   

0x0C     Query programmer interface version     4 bytes "pRoG"   ACK + 2
bytes version (>0)
0x0D     Query programmer name    none    ACK + 16 bytes string (NULL
padding)

I'd insert interface version and programmer name as 0x00 and 0x01,
moving everything else down. That way, we can change the interface later
and query it easily. The only thing we need to keep constant in the
interface is opcode 0x00. Everything else can be changed as needed.


Regards,
Carl-Daniel

-- 
http://www.hailfinger.org/