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]
Date:   Mon, 4 Dec 2023 12:52:57 +0000
From:   Conor Dooley <conor.dooley@...rochip.com>
To:     Markuss Broks <markuss.broks@...il.com>
CC:     Conor Dooley <conor@...nel.org>,
        Karel Balej <karelb@...li.ms.mff.cuni.cz>,
        Dmitry Torokhov <dmitry.torokhov@...il.com>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley <conor+dt@...nel.org>,
        Henrik Rydberg <rydberg@...math.org>,
        <linux-input@...r.kernel.org>, <devicetree@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>,
        Duje Mihanović <duje.mihanovic@...le.hr>,
        <~postmarketos/upstreaming@...ts.sr.ht>,
        <phone-devel@...r.kernel.org>, Karel Balej <balejk@...fyz.cz>
Subject: Re: [PATCH v3 2/5] dt-bindings: input/touchscreen: Add compatible
 for IST3038B

On Mon, Dec 04, 2023 at 02:40:44PM +0200, Markuss Broks wrote:
> On 12/3/23 13:20, Conor Dooley wrote:
> > On Sat, Dec 02, 2023 at 01:48:33PM +0100, Karel Balej wrote:
> > > From: Markuss Broks <markuss.broks@...il.com>
> > > 
> > > Imagis IST3038B is a variant (firmware?) of Imagis IST3038 IC,
> > > add the compatible for it to the IST3038C bindings.
> > This one is better, but would be well served by mentioning what
> > specifically is different (register addresses or firmware commands?)
> 
> I don't think anyone knows this other than Imagis itself. I would guess it's
> different hardware, since register addresses are indeed different, but on
> the other hand, there is a possibility that firmware on the MCU could be
> responding to those commands. I suppose "... IST3038B is a hardware variant
> of ... IST3038" would be more correct.

Only Imagis might know the specifics, but you (plural) have made driver
changes so you know what is different in terms of the programming model.
I'm just asking for you to mention how the programming model varies in
the commit message. Otherwise I can't know whether you should have added
a fallback compatible, without going and reading your driver change. The
commit message for the bindings should stand on its own merit in that
regard.
"Variant" alone does not suffice, as many variants of devices have a
compatible programming model, be that for a subset of features or
complete compatibility.

> The reason why I think it could be firmware-defined is because we have a lot
> of variants (30xxA, 30xxB, 30xxC, plain 30xx), and the numbers usually mean
> feature level/completeness, e.g. some don't support the touch pressure or
> touchkeys, and we don't know what A/B/C/none means.

Ultimately whether it is due to firmware or the hardware isn't
particular important, just mention what is incompatibly different.

Cheers,
Conor.


> > > Signed-off-by: Markuss Broks <markuss.broks@...il.com>
> > > Signed-off-by: Karel Balej <balejk@...fyz.cz>
> > > ---
> > >   .../devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml   | 1 +
> > >   1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml b/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml
> > > index 0d6b033fd5fb..b5372c4eae56 100644
> > > --- a/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml
> > > +++ b/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml
> > > @@ -18,6 +18,7 @@ properties:
> > >     compatible:
> > >       enum:
> > > +      - imagis,ist3038b
> > >         - imagis,ist3038c
> > >     reg:
> > > -- 
> > > 2.43.0
> > > 
> - Markuss

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ