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

[Urja Rannikko]

On Sat, Jun 6, 2009 at 00:37, Carl-Daniel Hailfinger
<c-d.hailfinger.devel.2006 at gmx.net> wrote:
> 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.

0x10 and 0xBA are ok.

>> 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)
Firstly, realize that the above command would have a second length
parameter of different size than the first in parameters - a parser
without knowledge of the commands parameters would surely fail.

the opbuf lenghts are because the driver must take note not to buffer
too much operations in the AVR before executing them. A write command
format in the AVR memory is essentially 0x00 (writeop) 0xnn (lenght)
0xaa 0xbb 0cc (begin address) + n bytes of data. the 0x08 operation
creates the initial 1-lenght write op that takes 6 bytes (1 op, 1
lenght, 3 addr, 1 data) and each 0x09 operation adds one to lenght and
appends the data. In case the length is already 0 (meaning 256), a new
write operation with modified address (+256) is created, etc. So a
continouous write of 256 bytes takes 261 bytes in the buffer.
I considered adding a write n function, but realized that it would
make the parameter lenght variable - now i can check that it is
correct with a simple op2len table (a double-check that the parameters
are correct). I dont see it worth the hassle, and it could code at a
maximum 252 bytes of data with the current opcode lenght format - and
the AVR's memory would surely run out if i asked it to take in some
>1k operation anyways.

The read n operation is in many ways different - it can have the
length in parameters because that is the return data length (not
command parameter length), and the AVR doesnt need to buffer the data
it sends in a reply.

>> 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.

Huh? (My AVR doesnt know the future... And the computer would surely
be baffled about an ACK before sending the command :P)

>> 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.
>

Implementing...

-- 
urjaman