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-spiCompatible 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