Attention is currently required from: Paul Menzel, Angel Pons.
7 comments:
Patchset:
Thanks for the review!
File tf530_spi.c:
Patch Set #1, Line 4: * Copyright (C) 2019 Steven J Leary
This differs from the commit author data.
Steven did the original port and published it on his github.
Patch Set #1, Line 20: #if CONFIG_TF530_SPI == 1
Is this really necessary? I know other files have these guards, but I don't think they're actually u […]
It is modeled after the LINUX_SPI driver. I did not try to create an improvement over the other drivers but rather keep the consistency with what's there. Is this not recommended?
Patch Set #1, Line 62: static struct ConfigDev *cd = NULL;
There's an ongoing effort to remove global state from programmers. See changes to `digilent_spi. […]
I'll have a look at this again. I noticed, but since this programmer is on the CPU board itself, there is no chance that this code will ever be called reentrantly.
This is also a global (system wide) handle, e.g. the same value for all tasks in the system.
struct Library *ExpansionBase = NULL;
if ((ExpansionBase = (struct Library *)
OpenLibrary((unsigned char *)"expansion.library", 0L)) == NULL) {
Moving the assignment out of the if-block's condition should be easier to read: […]
Yeah they're necessary. :( I'll reorg.
Why return 0?
Good catch
Patch Set #1, Line 169: register_spi_master(&spi_master_tf53x, NULL);
This function returns something. Why not propagate the return value? […]
Ack
To view, visit change 54944. To unsubscribe, or for help writing mail filters, visit settings.