[flashrom] please reject my two previous letters

Boris Baykov dev at borisbaykov.com
Sat Jan 10 11:31:08 CET 2015


> On Sat, 10 Jan 2015 01:34:34 +0300
> Boris Baykov <dev at borisbaykov.com> wrote:

>> On Fri, 9 Jan 2015 22:49:56 +0100
>> Stefan Tauner <stefan.tauner at alumni.tuwien.ac.at>
>>
>> > On Fri, 9 Jan 2015 23:54:28 +0300
>> > Boris Baykov <dev at borisbaykov.com> wrote:
>> 
> [...]
>> >> Please explain how is better to split this patch to make it easy for review.
>> 
>> > We want to verify all changes to flashrom in quite some detail to keep
>> > it as stable as it is. A single huge patch like yours takes a very long
>> > time to fully understand. It is probably not even possible to review
>> > such a big change in one go without making some mistakes or overlook
>> > error of the original author. Smaller independent patches could be
>> > reviewed entirely without getting too much of a headache. Does that
>> > make sense to you?
>> 
> [...]
>> 
>> Unfortunately I can't imagine how to split this patch in parts because all
>> the patch logically has one idea and that's why my changes in different
>> files are connected to other changes and they work together.
>> 

> As you said, splitting by files in general does not make sense, but
> splitting by "feature" does.

> The idea is that an incremental patchset is a coarse (and cleaned up)
> version of your development history.

Thanks, Antonio for the excellent explanation! It looks like a set of 
nesting dolls from traditional russian culture where each next doll goes 
inside of previous one :-)

Basing on your list I developed the following order of 10 + 2 patches,
where basically 10 patches and added extra 2 small patches 3a and 4a for
explanation purposes only to provide an example how to use new features.
This means that to make 4BA version we should patch 1+2+3+4+5+6+7+8+9+10,
and 3a and 4a can be after 3 and 4 to show code examples. Is it ok?

Please explain why do you think to split the first patch into two parts ?
>   - base support for the JEDEC extensions can go into a separate patch;
>   - flashrom hooking for the extensions and for 4 byte addressing can go
>     into a dedicated patch;
From one side the idea is good from another side we'll receive the patch
(first one) which doesn't make any sense besides of source size increase.

1. 4BA: Basic support for 4-bytes addressing mode
2. 4BA: Winbond W25Q256.V chip declaration
3. 4BA: Support of Extended Address Register
3a.4BA: Temporary W25Q256.V declaration update to work using Ext.Addr.Reg
4. 4BA: Support for new direct 4BA instructions + W25Q256.V declaration update
4a.4BA: Temporary W25Q256.V declaration update to work via direct funcs 
5. 4BA: Progress visualization
6. 4BA: SFDP 1.6 parser for compliance with JESD216B
7. 4BA: SFDP 1.6 simulation (for testing purposes)
8. 4BA: SFDP downgrade to 1.5 to support Micron chips
9. 4BA: Some small visual changes for 4-byte addresses
10.4BA: Temporary banner fix about the 4BA patch

So, I can split the patch today and resubmit the patches set here. 

We have now build 1866 already. Do I need to update my patch for this build?






More information about the flashrom mailing list