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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZyG3aciw6OvqoFZ1@ux-UP-WHL01>
Date: Wed, 30 Oct 2024 12:34:49 +0800
From: Charles Wang <charles.goodix@...il.com>
To: Doug Anderson <dianders@...omium.org>
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

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

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