Attention is currently required from: Xiang Wang, Stefan Reinauer, Angel Pons.

View Change

9 comments:

• Commit Message:

  • Patch Set #1, Line 8:

    https://en.wikipedia.org/wiki/Serial_Peripheral_Interface […]

    Hi Xiang, So I think what Angel and I are saying here is that the rational should be explained in more detail in the commit message for such a substantial change in behavior. Please take your time here to detail more precisely what you have in mind to help bridge the gap in what you are trying to archive vs the communities understanding of the current code.

• File bitbang_spi.c:

  • Patch Set #6, Line 52:

    	}
    
    	master->set_sck(sck);
    	master->set_mosi(mosi);
    }
    

    spurious changes?

  • Patch Set #6, Line 62:

    ditto

  • Patch Set #6, Line 89: for (int i = 0; i < 8; i++) {

    Leave style changes to another commit and out of the meat of this one, it gets in the way of reviewing the more important details.

  • Patch Set #6, Line 90: ret = ret << 1;

    same here, just leave as `ret <<= 1;` in this commit

  • Patch Set #6, Line 107: for (int i = 0; i < 8; i++) {

    ditto

  • Patch Set #6, Line 119: val = val << 1;

    ditto

  • Patch Set #6, Line 136:

    programmer_delay(master->half_period);
    	bitbang_spi_set_cs(master, 0);
    	programmer_delay(master->half_period);
    
    

    this feels like its own little patch.

  • Patch Set #6, Line 186:

    	char *mode = extract_programmer_param("mode");
    	if (mode) {
    		if (strlen(mode)) {
    			char *endptr;
    			int v = strtol(mode, &endptr, 0);
    			if (mode == endptr || v > 3 || v < 0) {
    				msg_perr("bitbang spi only work with mode 0-3\n");
    				free(mode);
    				return ERROR_FLASHROM_BUG;
    			}
    			cpol = (v >> 1) & 1;
    			cpha = (v >> 0) & 1;
    		}
    		free(mode);
    	}
    

    static parse_mode function that takes `(&int, &int)` as a type ?

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