[flashrom] [PATCH 3/3] serprog: add support for setting the SPI frequency

Stefan Tauner stefan.tauner at student.tuwien.ac.at
Thu Sep 15 22:52:22 CEST 2011


On Thu, 15 Sep 2011 22:14:11 +0200
Uwe Hermann <uwe at hermann-uwe.de> wrote:

> On Thu, Sep 08, 2011 at 12:56:23AM +0200, Stefan Tauner wrote:
> 
> > +		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.

basically i agree, but since it was used all over the place i thought
it makes more sense to make it consistently wrong :)
i would like to postpone this to a later patch or change 2/3
accordingly.

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

Hz is an arbitrary unit. Although you get better resolution with
smaller steps, you do not get "maximum flexibility" when using Hz:
there are frequencies less than that (e.g. 1/10 Hz). The question
is what is useful in this use case and i think that is a kHz
resolution. Most programmers will have to round to some frequency due
to fixed PLL ratios etc. anyway, so it does not make much sense to have
such a high resolution. For communication integrity one kHz more or
less does not matter either. The only argument in favor of Hz that i
would acknowledge is that is the most basic unit for humans when they
are thinking of frequencies (although logically there is no such basic
unit as explained above). And that argument is pretty weak because
anyone familiar with Hz is probably able to "think" in kHz too. And for
usability kHz is even better due to less 0s :)

-- 
Kind regards/Mit freundlichen Grüßen, Stefan Tauner




More information about the flashrom mailing list