7 comments:

File sb600spi.c:
- Patch Set #1, Line 376: \n
  Maybe replace this newline with `...` and a space, and then ... (see next comment)
- Patch Set #1, Line 388: Setting SPI clock to %s
  ... this could be shortened to "failed.\n", and ...
- Patch Set #1, Line 391: Setting SPI clock to %s
  ... this as well.
- Patch Set #1, Line 397: msg_pdbg("Setting SPI read mode to %s (%i).\n", spireadmodes[mode], mode);
  Same as before.
- Patch Set #1, Line 435:
```
free(spispeed);
			return 1;
```
  nit: This path does not free(spireadmode), but it might be null.
  Instead, one can keep the declarations at the beginning and only extract spireadmode after this block.
- Patch Set #1, Line 451: (amd_gen < CHIPSET_BOLTON))
  Does this mean that generations before Bolton do not support configuring spireadmode at all?
- 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:
```
 if (spispeed_idx < 0)
         return 1;
```
```
 return set_speed(dev, spispeed_idx);
```

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