5 comments:
ce_high = ((unsigned char *)base) + 0xE00; /* 0xFFFFFE00 */
ce_low = ((unsigned char *)base) + 0xD00; /* 0xFFFFFD00 */
this is common code, move it to after the ifdef blocks.
no need for '{ .. }' for single statements under a branch.
msg_pdbg("%s: internal_buses_supported=0x%x\n", __func__,
internal_buses_supported);
/* Check for FWH because IT85 listens to FWH cycles.
* FIXME: The big question is whether FWH cycles are necessary
* for communication even if LPC_IO is defined.
*/
if (internal_buses_supported & BUS_FWH)
msg_pdbg("Registering IT85 SPI.\n");
this whole block constitutes checks and prints of inputs parameters to this function and therefore should be at the beginning of the function - probably after the
`if (!(internal_buses_supported & BUS_FWH)) {` block above.
Patch Set #2, Line 354: if (!data) {
You are checking for nullity _after_ you deference here. Remember the pattern 'check-then-do'.
Patch Set #2, Line 360: free(data);
It maybe a bit more clear now for you that this just papers over the issue that register_shutdown() should never fail. Notice now that physmap() leaks and that the shutdown callback is responsible for unmapping it. Putting a free() here (not that it matters) is just distracting away from the real leaks that do matter.
To view, visit change 48196. To unsubscribe, or for help writing mail filters, visit settings.