On Wed, 14 Aug 2013 08:56:27 +0200
Carl-Daniel Hailfinger <[email protected]> wrote:
> Am 10.08.2013 03:45 schrieb Stefan Tauner:
> > diff --git a/sb600spi.c b/sb600spi.c
> > index c9be44c..2c1d400 100644
> > --- a/sb600spi.c
> > +++ b/sb600spi.c
> > @@ -272,6 +272,61 @@ static int sb600_spi_send_command(struct flashctx
> > *flash, unsigned int writecnt,
> > return 0;
> > }
> >
> > +struct spispeed {
> > + const char *const name;
> > + const int8_t speed;
> > +};
> > +
> > +static const struct spispeed spispeeds[] = {
> > + { "66 MHz", 0x00 },
> > + { "33 MHz", 0x01 },
> > + { "22 MHz", 0x02 },
> > + { "16.5 MHz", 0x03 },
> > +};
> I see struct spispeed and think it is a somewhat odd construct. The
> speed member is just the array index stored in the corresponding array
> member. Do you envision speed values which are not an array index?
I want to merge the various frequency setting functions eventually and
Dediprog for example uses non-linear values. Hence I'd rather use this
unnecessary scheme for easier refactoring later.
> > +static int set_speed(struct pci_dev *dev, const struct spispeed *spispeed)
> > +{
> > + bool success = false;
> > + uint8_t speed = spispeed->speed;
> > +
> > + msg_pdbg("Setting SPI clock to %s (0x%x).\n", spispeed->name, speed);
> > + if (amd_gen != CHIPSET_YANGTZE) {
> > + rmmio_writeb((mmio_readb(sb600_spibar + 0xd) & ~(0x3 << 4)) |
> > (speed << 4), sb600_spibar + 0xd);
> > + success = (speed == ((mmio_readb(sb600_spibar + 0xd) >> 4) &
> > 0x3));
> The code reads spispeed and then sets 16 MHz regardless of whether it's
> already 16 MHz. I also seem to remember that you were opposed to setting
> this speed permanently and advocated to restore it on shutdown.
This should not be done unconditionally, right, I will change this.
But it is done (like the (un)setting of fast speed also) with the
r*-mmio functions so it should be reverted on shutdown AFAIK?
--
Kind regards/Mit freundlichen Grüßen, Stefan Tauner
_______________________________________________
flashrom mailing list
[email protected]
http://www.flashrom.org/mailman/listinfo/flashrom
