Thread (17 messages) 17 messages, 5 authors, 2024-11-06

Re: [PATCH] dt-bindings: input: Goodix SPI HID Touchscreen

From: Charles Wang <hidden>
Date: 2024-10-30 04:35:00
Also in: linux-devicetree, lkml

Hi Doug,

On Fri, Oct 25, 2024 at 08:29:13AM -0700, Doug Anderson wrote:
Charles,

On Fri, Oct 25, 2024 at 5:03 AM Krzysztof Kozlowski [off-list ref] wrote:
quoted
quoted
+properties:
+  compatible:
+    enum:
+      - goodix,gt7986u-spi
Compatible is already documented and nothing here explains why we should
spi variant.
quoted
+
+  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.
I apologize for any confusion caused. As a newcomer, I am still learning
about the community practices.

Thank you very much for your patience and clear explanation. I will recheck
the previous review feedback and provide a new patch marked as v2.

Best regards,
Charles
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help