6 comments:

File it85spi.c:
- Patch Set #1, Line 303:
```
	data = calloc(1, sizeof(struct it85spi_data));
	if (!data) {
		msg_perr("Unable to allocate space for extra SPI master data.\n");
		return SPI_GENERIC_ERROR;
	}

	spi_master_it85xx.data = data;

	if (register_shutdown(it85xx_shutdown, data)) {
		goto it85spi_init_error;
```
  in fact don't even bother checking the return value of register_shutdown() as it's useless to do so […]
  I am keeping this check for return value of register_shutdown because as it is currently implemented seems like it can return 1 or 0. If this aspect needs to be changed/improved, I can do it later as a next step.
  Moving everything to the bottom of the function as you suggested.
- Patch Set #1, Line 339: chipaddr
  these casts are not needed
  Done
- Patch Set #1, Line 341: goto it85spi_init_error;
  this should be a basic early return the code was correct before.
  Done
- Patch Set #1, Line 346:
```
	data->ce_high = (unsigned char *)(base + 0xE00);  /* 0xFFFFFE00 */
	data->ce_low = (unsigned char *)(base + 0xD00);  /* 0xFFFFFD00 */
```
  this is common code on both sides of the pre-processor directive.
  Yes I agree! Can I take care of this code behind the flags in the next patch?
- Patch Set #1, Line 349:
```
	msg_pdbg("%s: internal_buses_supported=0x%x\n", __func__,
		  internal_buses_supported);
```
  this isn't needed.
  There are lots of similar debug messages in this file. I am not sure what exactly they are for so I don't want to remove them. In any case, if we decide to remove them can I do it in the next patch?
- Patch Set #1, Line 362: */
  this comment block is all related the to the above code and should be immediately close to the regis […]
  Not sure what do you mean here: do you mean this FIXME comment should be below /* Set this as SPI controller */ one? So just swap comments?

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