Attention is currently required from: ZhiYuanNJ.
8 comments:
Patchset:
Sorry for not reviewing this earlier; I was busy with other things and missed this patch.
File ch347_spi.c:
Patch Set #4, Line 56: speed
I think `divisor` might be a better term here, as the values passed to the CH347 divide the reference clock by 2^divisor_value. These values are passed as the second argument for the `ch347_spi_config`, and the function signature for it already names it as the divisor: `ch347_spi_config(struct ch347_spi_data *ch347_data, uint8_t divisor)`
There isn't really a need to write the speeds in hexadecimal, so just drop the `0x` prefix
Just use decimal
```suggestion
int spispeed = 0; /* defaulet 60M SPI */
```
Also, in my experience it's quite difficult to get SPI flash chips working at 60 MHz, which is why I had hard coded a divisor of 2 (15 MHz) in the original code. It seems like flashrom only uses the standard JEDEC read (0x03) command, and many flash chips only support a maximum of 50 MHz or lower for standard reads.
Patch Set #4, Line 287: defaulet
default
Patch Set #4, Line 365: defaul
default
I would use MHz for more clarity instead of just M
Patch Set #4, Line 365: (60M clock spi)
I don't think the parenthesis really add any value here; I would just drop them.
To view, visit change 82776. To unsubscribe, or for help writing mail filters, visit settings.