On Thu, Sep 08, 2011 at 12:56:23AM +0200, Stefan Tauner wrote:
> +0x14 Set SPI clock frequency 16-bit requested frequency ACK +
> 16-bit set frequency / NAK
^
"Set SPI clock frequency (in Hz)"
> + 0x14 (S_SPI_SCK):
Please rename to S_SPI_FREQ (also all other places with 'SCK'). SCK is
often used to name a clock pin, this is unnecessarily confusing here.
> + Set the SPI clock frequency.
^
in Hz
> + f_spi = extract_programmer_param("sck");
sck -> freq
> + if (f_spi && strlen(f_spi)) {
> + uint8_t buf[2];
> + uint16_t tmp = atoi(f_spi);
Hm, maybe use strtol() later and check for errors, atoi() is not so
good for that.
> + if (sp_check_commandavail(S_CMD_S_SPI_SCK) == 0)
> + msg_pdbg(MSGHEADER "Setting SPI clock rate is "
> + "not supported!\n");
> + else if (sp_docommand(S_CMD_S_SPI_SCK, 2, buf, 2, buf)
> + == 0) {
> + tmp = buf[0] | (buf[1]<<8);
^^
Missing spaces.
> + msg_pdbg(MSGHEADER "Requested to set SPI clock "
> + "frequency to %s kHz. It was actually "
> + "set to %d kHz\n", f_spi, tmp);
kHz -> Hz
I would highly recommend to use Hz as the unit for setting the frequency
(in the API). The frontend (flashrom) can easily allow freq=1000,
or freq=1khz, or freq=1mhz etc. for user friendlyness, but the API
should maintain max. flexibility IMHO.
Uwe.
--
http://hermann-uwe.de | http://sigrok.org
http://randomprojects.org | http://unmaintained-free-software.org
_______________________________________________
flashrom mailing list
[email protected]
http://www.flashrom.org/mailman/listinfo/flashrom
