Attention is currently required from: Nico Huber, Paul Menzel, Edward O'Callaghan, Angel Pons, Nikolai Artemiev, Sergii Dmytruk.

Patch set 19:Code-Review +1

View Change

3 comments:

• File cli_classic.c:

  • Patch Set #15, Line 66:

    	       "      --wp-range=<start>,<len>      set write protect range\n"
    	       "      --wp-region <region>          set write protect region\n"
    

    Cool thanks! Your explanation: […]

    Looks like it is done in the latest patchset. I mark this comment resolved.

• File cli_classic.c:

  • Patch Set #17, Line 220:

    		ret = flashrom_wp_set_range(flash, &cfg, &range);
    		if (ret) {
    			msg_gerr("The chip does not support the requested range.\n");
    			return ret;
    		}
    
    		/* Write range before other operations (i.e. enabling HW protection) */
    		ret = write_wp_config(flash, &cfg);
    		if (ret)
    			return ret;
    

    We could print messages from libflashrom, but it's generally undesirable and should be removed as mu […]

    Few days later, and after re-reading the discussion, I think I understand. Especially if there are potential cases where only 1,2 needed, not necessarily followed by 3.

    Existing printing of messages in libflashrom seems like a separate conversation.

    Re: tests for API directly: actually yes, and Sergii already added a bunch of those in CB:59075 :)

  • Patch Set #17, Line 236:

    		ret = flashrom_wp_set_mode(&cfg, mode);
    		if (ret) {
    			msg_gerr("The chip does not support the requested mode.\n");
    			return ret;
    		}
    
    		/* Write mode before other operations */
    		ret = write_wp_config(flash, &cfg);
    		if (ret)
    			return ret;
    

    Same comment, seems like an operation consists of two steps.

    closing as dup

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