Messages in this thread | | | Subject | Re: [PATCH V2 mips/linux.git] firmware: bcm47xx_nvram: refactor finding & reading NVRAM | From | Rafał Miłecki <> | Date | Fri, 5 Mar 2021 11:16:40 +0100 |
| |
Hi,
On 05.03.2021 10:58, Philippe Mathieu-Daudé wrote: > On Fri, Mar 5, 2021 at 6:55 AM Rafał Miłecki <zajec5@gmail.com> wrote: >> >> From: Rafał Miłecki <rafal@milecki.pl> >> >> 1. Use meaningful variable names (e.g. "flash_start", "res_size" instead >> of e.g. "iobase", "end") >> 2. Always operate on "offset" instead of mix of start, end, size, etc. > > "instead of a mix" > >> 3. Add helper checking for NVRAM to avoid duplicating code >> 4. Use "found" variable instead of goto >> 5. Use simpler checking of offsets and sizes (2 nested loops with >> trivial check instead of extra function) > > This could be a series of trivial patches, why did you choose to make a mixed > bag harder to review?
It's a subjective thing and often a matter of maintainer taste. I can say that after contributing to various Linux subsystems. If you split a similar patch for MTD subsystem you'll get complains about making changes too small & too hard to review (sic!).
This isn't a bomb really: 63 insertions(+), 48 deletions(-)
That said I admit I don't know MIPS tree habits. Thomas: do you prefer smaller patches in case like this?
| |