On 06/12/2018 03:26 PM, Peter Rosin wrote:
On 2018-06-12 08:09, Jarkko Nikula wrote:
On 06/11/2018 06:20 PM, Andy Shevchenko wrote:
On Mon, 2018-06-11 at 17:22 +0300, Jarkko Nikula wrote:
This is rather readability update than micro-optimization, or if not
optimization at all. We take the input clock rate to a variable and
pass
that to SCL timing parameter calculation functions.
While at it, indent i2c_dw_scl_hcnt()/i2c_dw_scl_lcnt() argument list
to
the same alignment. Now first argument is off by one character.
I think it still doesn't explain why instead of moving other lines, you
split the first one in each excerpt like this.
Indentation goes too far right, each following line needs 7 spaces and
line length becomes over 80. I wouldn't like to go to that path.
But the new line break does not make any sense *for this patch*. Just look
at it!
The white space changes only makes sense when considering patch 6. And that
dependency is not mentioned in this commit message which makes any reviewer
go WFT when looking at this patch. I.e. in my view the white space changes
in this patch is just a preparation for patch 6. Which is a bit pointless,
why do you not just move the white space cleanup from patch 4 to patch 6?
Or do the cleanup here in patch 4, but do it in a way that makes sense by
itself (and then, if needed, redo it in patch 6).
I basically do here s/i2c_dw_clk_rate(dev)/ic_clk/ and while at it move
the function call to its own line due the existing misalignment. I don't
think it makes sense to split this tiny patch into two?
But what I'd like to do is to keep patch 6 as small as possible instead
of moving this minor stuff there. I'm all fine dropping the whole
misaligment fix, moving this patch before 6, changing commit log from v4
[1], splitting this or whatever looks best for this minor uninteresting
stuff :-)
1. https://www.spinics.net/lists/linux-i2c/msg35230.html
--
Jarkko
