linux-kernel - Re: [PATCH v2 1/3] dt-bindings: display: st7701: Add Anbernic RG28XX panel

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: <20240702-magnetism-amends-d535f626e433@spud>
Date: Tue, 2 Jul 2024 16:34:42 +0100
From: Conor Dooley <conor@...nel.org>
To: Hironori KIKUCHI <kikuchan98@...il.com>
Cc: linux-kernel@...r.kernel.org, Jagan Teki <jagan@...rulasolutions.com>,
	Neil Armstrong <neil.armstrong@...aro.org>,
	Jessica Zhang <quic_jesszhan@...cinc.com>,
	Sam Ravnborg <sam@...nborg.org>, David Airlie <airlied@...il.com>,
	Daniel Vetter <daniel@...ll.ch>,
	Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
	Maxime Ripard <mripard@...nel.org>,
	Thomas Zimmermann <tzimmermann@...e.de>,
	Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>, dri-devel@...ts.freedesktop.org,
	devicetree@...r.kernel.org
Subject: Re: [PATCH v2 1/3] dt-bindings: display: st7701: Add Anbernic RG28XX
 panel

On Tue, Jul 02, 2024 at 02:28:27AM +0900, Hironori KIKUCHI wrote:
> On Sun, Jun 30, 2024 at 11:17 PM Conor Dooley <conor@...nel.org> wrote:
> > On Sat, Jun 29, 2024 at 05:26:56PM +0900, Hironori KIKUCHI wrote:
> > > On Sat, Jun 29, 2024 at 1:27 AM Conor Dooley <conor@...nel.org> wrote:
> > > > On Fri, Jun 28, 2024 at 02:10:15PM +0900, Hironori KIKUCHI wrote:

> > > > >            - densitron,dmt028vghmcmi-1a
> > > > >            - elida,kd50t048a
> > > > >            - techstar,ts8550b
> > > > >        - const: sitronix,st7701
> > > > >
> > > > >    reg:
> > > > > -    description: DSI virtual channel used by that screen
> > > > > +    description: DSI / SPI channel used by that screen
> > > > >      maxItems: 1
> > > > >
> > > > >    VCC-supply:
> > > > > @@ -43,6 +41,13 @@ properties:
> > > > >    IOVCC-supply:
> > > > >      description: I/O system regulator
> > > > >
> > > > > +  dc-gpios:
> > > > > +    maxItems: 1
> > > > > +    description:
> > > > > +      Controller data/command selection (D/CX) in 4-line SPI mode.
> > > > > +      If not set, the controller is in 3-line SPI mode.
> > > > > +      Disallowed for DSI.
> > > > > +
> > > > >    port: true
> > > > >    reset-gpios: true
> > > > >    rotation: true
> > > > > @@ -57,7 +62,38 @@ required:
> > > > >    - port
> > > > >    - reset-gpios
> > > > >
> > > > > -additionalProperties: false
> > > > > +allOf:
> > > > > +  - $ref: panel-common.yaml#
> > > > > +  - if:
> > > > > +      properties:
> > > > > +        compatible:
> > > > > +          contains:
> > > > > +            # SPI connected panels
> > > > > +            enum:
> > > > > +              - anbernic,rg28xx-panel
> > > > > +    then:
> > > > > +      $ref: /schemas/spi/spi-peripheral-props.yaml
> > > >
> > > > I'm not really keen on this. I'd rather see a different binding for the
> > > > SPI version compared to the MIPI ones. Is doing things like this common
> > > > for panels? If it is, I'll turn a blind eye..
> > >
> > > This might be the first case that a driver supports both DSI and SPI
> > > for a panel.
> > > The panel can be either a DSI device or an SPI device.
> >
> > The commit message sounded like the panel was capable of doing SPI
> > instead of DSI, is that not the case and the panel is actually capable
> > of doing both, just happens to be connected as SPI in this particular
> > device?
> 
> Sorry for the confusion.
> I think it depends on what the "panel" refers to...
> Although the "panel driver IC" (ST7701 variant) is capable of doing
> both, the "panel assy" (including its cable) of the RG28XX only has an
> SPI interface in its pinout.
> If the compatible string "rg28xx-panel" represents the assy, then I
> could say the "panel" only supports SPI, and the other panels only
> support DSI.
> But if it only represents a specific LCD panel and its driver IC with
> control parameters, then it theoretically supports both, and it might
> be sufficient to just include spi-peripheral-props, as in v1.
> v1: https://lore.kernel.org/linux-kernel/20240618081515.1215552-2-kikuchan98@gmail.com/

I thought about it some more, and I think what you've got for this in
the binding at present is fine. Splitting the binding I think would
require a select and wouldn't really be any "cleaner" of an
implementation.

Thanks,
Conor.

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