Attention is currently required from: Felix Singer, Alex Badea.
7 comments:
Patchset:
Hi Alex, thank you for the contribution!
The code looks already quite good. Anyway I have a questions and some minor annotations.
On which device have you tested this driver? Can you mention it in the commit message.
File asm106x.c:
Patch Set #3, Line 35: {PCI_VENDOR_ID_ASMEDIA, 0x0612, OK, "ASMedia", "ASM106x"},
This PCI-Id is assigned to the ASM1062 Serial ATA Controller,
the PCI-Id 0x625 is assigned to '106x SATA/RAID Controller'. Beside that there is a PCI-Id 0x0611 for 'ASM1061 SATA IDE Controller'. For which one is this driver?
Or does it work on all of them?
PCI-Ids from pciutils (https://pci-ids.ucw.cz/v2.2/pci.ids)
Patch Set #3, Line 51: programmer_delay(flash, 10);
`programmer_delay()` should be used outside of the programmer. It is there to determine if an programmer specific delay function should be called or the default one.
Here you can use `void default_delay(unsigned int usec)`.
With it you can also pass only the `struct pci_dev *pci` to this function.
```
static int asm106x_wait_ready(const struct pci_dev *pci, uint8_t *pval)
```
Patch Set #3, Line 59: if (pval)
You could move this 'if, assignment, return' where the break is. Then everything below the loop is a timeout.
Patch Set #3, Line 74: writecnt, readcnt);
This can be moved in the line above.
Patch Set #3, Line 115: ctrl | ASM106X_CTRL_CSN);
This function call can be in one line
File meson.build:
Patch Set #3, Line 152: 'cpu_families' : cpus_port_io,
This line should be removed. The driver uses only PCI register to communicate with the flash, not I/O ports.
To view, visit change 73037. To unsubscribe, or for help writing mail filters, visit settings.