7 comments:

File sb600spi.c:
- Patch Set #1, Line 376: \n
  Maybe replace this newline with `...` and a space, and then ... […]
  Ack
- Patch Set #1, Line 388: Setting SPI clock to %s
  ... this could be shortened to "failed.\n", and ...
  Ack
- Patch Set #1, Line 391: Setting SPI clock to %s
  ... this as well.
  Ack
- Patch Set #1, Line 397: msg_pdbg("Setting SPI read mode to %s (%i).\n", spireadmodes[mode], mode);
  Same as before.
  Ack
- Patch Set #1, Line 435:
```
free(spispeed);
			return 1;
```
  nit: This path does not free(spireadmode), but it might be null. […]
  Ack
- Patch Set #1, Line 451: (amd_gen < CHIPSET_BOLTON))
  Does this mean that generations before Bolton do not support configuring spireadmode at all?
  I'm not sure. The code already here only set spi mode for >= Bolton which is why I restricted this option to >= Bolton.
- Patch Set #1, Line 509:
```
if (spispeed_idx >= 0 && set_speed(dev, spispeed_idx) != 0) {
		return 1;
	}
	return 0;
```
  This is a bit hard to parse. How about: […]
  Agree. Except it would return 0 (success) if spispeed_idx < 0.

To view, visit change 38833. To unsubscribe, or for help writing mail filters, visit settings.