On Mon, Sep 14, 2020 at 6:26 PM Dmitry Osipenko [off-list ref] wrote:

14.09.2020 16:49, Andy Shevchenko пишет:

quoted

On Sun, Sep 13, 2020 at 3:57 PM Dmitry Osipenko [off-list ref] wrote:

...

quoted

quoted

quoted

quoted

+       ret = i2c_transfer(client->adapter, xfer, ARRAY_SIZE(xfer));
+       if (ret != ARRAY_SIZE(xfer)) {

...> Also why switch from positive to negative conditional?

This will make code less readable because of the goto, and thus, there
will be two branches for handling of the returned value instead of one +
goto. Hence this part is good to me as-is.

But it's not the purpose of this patch, right?
Style changes should be really separated from the fix.

This should be up to a particular maintainer to decide. Usually nobody
requires patches to be overly pedantic, this may turn away contributors
because it feels like an unnecessary bikeshedding.

Let's see what Wolfram thinks about this.

It's more preferred
to accept patch as-is if it does right thing and then maintainer could
modify the patch, making cosmetic changes.

It depends on the maintainer's workflow (which may be different from
maintainer to maintainer).

quoted

And since it's a fix it should have a Fixes tag.

It shouldn't be a fix, but a new feature because apparently the 1386
controller wasn't ever intended to be properly supported before this patch.

Thanks for clarification. Indeed in this case no tag is needed.

-- 
With Best Regards,
Andy Shevchenko