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] [day] [month] [year] [list]

Message-ID: <ZyrgbC5SPe_YXoMt@ux-UP-WHL01>
Date: Wed, 6 Nov 2024 11:20:12 +0800
From: Charles Wang <charles.goodix@...il.com>
To: Doug Anderson <dianders@...omium.org>
Cc: dmitry.torokhov@...il.com, hbarnor@...omium.org, jikos@...nel.org,
	bentiss@...nel.org, linux-input@...r.kernel.org,
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
	conor.dooley@...rochip.com, Krzysztof Kozlowski <krzk@...nel.org>
Subject: Re: [PATCH] dt-bindings: input: Goodix SPI HID Touchscreen

On Mon, Nov 04, 2024 at 11:36:50AM -0800, Doug Anderson wrote:
> Charles,
> 
> On Thu, Oct 31, 2024 at 6:33 PM Charles Wang <charles.goodix@...il.com> wrote:
> >
> > Hi Doug,
> >
> > On Thu, Oct 31, 2024 at 10:58:07AM -0700, Doug Anderson wrote:
> > > Hi,
> > >
> > > On Wed, Oct 30, 2024 at 7:37 PM Charles Wang <charles.goodix@...il.com> wrote:
> > > >
> > > > > > +  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.
> > > > >
> > > > > No, you keep sending the same and the same, without improvements.
> > > > >
> > > >
> > > > I apologize for overlooking the discussions regarding this issue.
> > > >
> > > > I would like to clarify that while the current boards use the same address,
> > > > but newly designed boards in the future may require different addresses.
> > > >
> > > > Retaining this property would likely offer more flexibility.
> > >
> > > I don't feel very strongly about it, but maybe Krzysztof does?
> > > Possibly the path of least resistance would be:
> > >
> > > 1. You drop the property from the bindings.
> > >
> > > 2. You hardcode it in the driver to be the normal value.
> > >
> > > 3. If/when someone actually needs a different value then we can add it
> > > as an optional property in the bindings and fall back to the default
> > > value if the property isn't present.
> > >
> > > What do you think? If you feel strongly about keeping the
> > > "hid-report-addr" then you can certainly keep making your case.
> > > However, it's probably best to wait to get agreement from Krzysztof
> > > (or one of the other DT maintainers) before sending your next version
> > > unless you're going to take the "path of least resistance" that I talk
> > > about above.
> > >
> >
> > Agreed, let's wait and see the opinions of Krzysztof (or the other DT
> > maintainers).
> 
> As I went back and looked at this again, I'm curious: I don't know
> much about how your protocol works, but is there any reason why your
> driver can't figure out this "hid-report-addr" dynamically? Is there
> some reason you can't talk to the device and ask it what the
> "hid-report-addr" should be? From skimming through your driver there
> appear to already be a few hardcoded addresses. Can one of those
> provide you the info you need?
> 

Similar to a standard i2c-hid driver, which requires configuring the
address for hid-descr-addr in the DTS, other address information is
obtained using this address.

In theory, we can dynamically obtain most of the addresses from the chip.
However, for this chip, there always needs to be a known address for the
driver to communicate with, whether this address is 0x0000 or 0x0001,
and this address is related to the firmware.

Regarding this issue, since no further review comments received.
I think I can first remove the address as your previous suggestion
and use a default address for communication in the driver. In the
future, we can upgrade the firmware and driver to achieve dynamic
address acquisition.

Thanks,
Charles