[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAD=FV=X1F3QC=eSXcCn-78iQBzHMzT3z9Sis3yXKW_Bzun3+EA@mail.gmail.com>
Date: Fri, 25 Oct 2024 08:29:13 -0700
From: Doug Anderson <dianders@...omium.org>
To: Charles Wang <charles.goodix@...il.com>
Cc: dmitry.torokhov@...il.com, hbarnor@...omium.org,
conor.dooley@...rochip.com, jikos@...nel.org, bentiss@...nel.org,
linux-input@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, Krzysztof Kozlowski <krzk@...nel.org>
Subject: Re: [PATCH] dt-bindings: input: Goodix SPI HID Touchscreen
Charles,
On Fri, Oct 25, 2024 at 5:03 AM Krzysztof Kozlowski <krzk@...nel.org> wrote:
>
> > +properties:
> > + compatible:
> > + enum:
> > + - goodix,gt7986u-spi
>
> Compatible is already documented and nothing here explains why we should
> spi variant.
>
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + interrupts:
> > + maxItems: 1
> > +
> > + reset-gpios:
> > + maxItems: 1
> > +
> > + goodix,hid-report-addr:
>
> I do not see this patch addressing previous review. Sending something
> like this as v1 after long discussions also does not help.
Krzysztof is right that it's better to wait until we get consensus on
the previous discussion before sending a new patch. I know you were
just trying to help move things forward, but because of the way the
email workflow works, sending a new version tends to fork the
discussion into two threads and adds confusion.
I know Krzysztof and Rob have been silent during our recent
discussion, but it's also a long discussion. I've been assuming that
they will take some time to digest and reply in a little bit. If they
didn't, IMO it would have been reasonable to explicitly ask them for
feedback in the other thread after giving a bit of time.
As Krzysztof mentioned, if/when you send the "goodix,gt7986u-spi"
bindings again you'd want to:
* Make sure it's marked as v2.
* Make sure any previous review feedback has been addressed. For
instance, I think Krzysztof requested that you _remove_ the
goodix,hid-report-addr from the bindings and hardcode this into the
driver because every GT7986U will have the same hid-report-addr. I
know that kinda got lost in the discussion but it still needs to be
addressed or at least responded to. I guess there was at least one
other comment about "additionalProperties" that you should look for
and address.
* Make sure there's some type of version history after-the-cut. Tools
like "patman" and "b4" can help with this.
* The commit message should proactively address concerns that came up
during the review process. In this case if we go with
"goodix,gt7986u-spi" the commit message would want to say something
explaining _why_ the "-spi" suffix is appropriate here even though
normally it wouldn't be. That will help anyone digging through
history.
-Doug
Powered by blists - more mailing lists