Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20200912005521.26319-1-jiada_wang@mentor.com> <CAHp75Vc5YCb-6oRRfVOE5bL_Dmzy0LwDpetxqD-G+E9M=EwA=w@mail.gmail.com>
 <bd668b99-5b14-f54d-101d-7d56e0c8c4c0@gmail.com> <CAHp75VdTv-uCQue3VU=czZJd4iTG+XBVe2kFtnP+fZ1XQuFbzA@mail.gmail.com>
 <137be969-f99a-66e0-ebe4-b86f4be2b5d3@gmail.com>
In-Reply-To: <137be969-f99a-66e0-ebe4-b86f4be2b5d3@gmail.com>
From:   Andy Shevchenko <andy.shevchenko@gmail.com>
Date:   Mon, 14 Sep 2020 18:50:38 +0300
Message-ID: <CAHp75Vfg6=5u1fthsub3xw3dxAKTGPUHfamjK_A2b5hcyw25PA@mail.gmail.com>
Subject: Re: [PATCH v4 1/1] Input: atmel_mxt_ts - implement I2C retries
To:     Dmitry Osipenko <digetx@gmail.com>
Cc:     Jiada Wang <jiada_wang@mentor.com>, nick@shmanahar.org,
        Dmitry Torokhov <dmitry.torokhov@gmail.com>,
        linux-input <linux-input@vger.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Eugeniu Rosca <erosca@de.adit-jv.com>,
        Andrew_Gabbasov@mentor.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Mon, Sep 14, 2020 at 6:26 PM Dmitry Osipenko <digetx@gmail.com> wrote:
> 14.09.2020 16:49, Andy Shevchenko =D0=BF=D0=B8=D1=88=D0=B5=D1=82:
> > On Sun, Sep 13, 2020 at 3:57 PM Dmitry Osipenko <digetx@gmail.com> wrot=
e:

...

> >>>> +       ret =3D i2c_transfer(client->adapter, xfer, ARRAY_SIZE(xfer)=
);
> >>>> +       if (ret !=3D 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 patc=
h.

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

--=20
With Best Regards,
Andy Shevchenko