On Wed, 14 Aug 2013 08:56:27 +0200 Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net 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?