On Mon, Sep 14, 2020 at 6:26 PM Dmitry Osipenko <digetx@xxxxxxxxx> wrote:
> 14.09.2020 16:49, Andy Shevchenko пишет:
> > On Sun, Sep 13, 2020 at 3:57 PM Dmitry Osipenko <digetx@xxxxxxxxx> wrote:
...
> >>>> + 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).
> > 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
