Re: [PATCH v2 02/14] dt-bindings: display: Add Solomon SSD16xx e-paper controller binding

[Devarsh Thakkar]

Hi Krzysztof,

Thanks for the review.

On 01/10/26 11:58, Krzysztof Kozlowski wrote:
> On Sun, Sep 27, 2026 at 11:53:17PM +0530, Devarsh Thakkar wrote:
> > Add device tree binding for the Solomon Systech SSD16xx family of e-Paper
> > display controllers (SSD1683 [1], SSD1673 [3], SSD1680 [4], SSD1681 [5]).
>
> A nit, subject: drop second/last, redundant "binding". The
> "dt-bindings" prefix is already stating that these are bindings.
> See also:
> https://elixir.bootlin.com/linux/v7.1-rc7/source/Documentation/devicetree/bindings/submitting-patches.rst#L23

Agreed.

> > The binding covers the 4-wire SPI interface with required GPIO pins for
> > reset (active-low), busy status (active-high), and data/command selection.
> > The spi-max-frequency is capped at 20 MHz per the SSD1683 datasheet [1]
> > which is also the maximum supported frequency amongst the family.
> >
> > An optional rotation property allows static display orientation to be
> > configured from device tree.
> >
> > The data/command selection GPIO pin is also kept as optional since various
> > SSD16XX controllers such as SSD1683 [1] support 3-wire mode too with
> > data/command selection bit transmitted as first bit before sending the data
> > sequence.
> >
> > The first supported compatible is for Gooddisplay GDEY042T81 which is 4.2",
> > 400x300 resolution black/white e-paper display using SSD1683 controller.
> >
> > Links:
> > [1] : https://www.crystalfontz.com/controllers/SolomonSystech/SSD1683
> > [2] : https://files.seeedstudio.com/wiki/Other_Display/42-epaper/GDEY042T81.pdf
> > [3] : https://www.crystalfontz.com/controllers/SolomonSystech/SSD1673
> > [4] : https://www.crystalfontz.com/controllers/SolomonSystech/SSD1680
> > [5] : https://www.crystalfontz.com/controllers/SolomonSystech/SSD1681
> >
> > Signed-off-by: Devarsh Thakkar <devarsht@xxxxxx>
> > ---
> > Changes from v1:
> >    - Added fallback compatible for the controller
> >    - Wrap lines to 80
> >
> >   .../bindings/display/solomon,ssd16xx.yaml     | 87 +++++++++++++++++++
> >   1 file changed, 87 insertions(+)
> >   create mode 100644 Documentation/devicetree/bindings/display/solomon,ssd16xx.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/display/solomon,ssd16xx.yaml b/Documentation/devicetree/bindings/display/solomon,ssd16xx.yaml
> > new file mode 100644
> > index 000000000000..b31fb862de04
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/solomon,ssd16xx.yaml
>
> Filename must match compatible.

Sure, will fix.

> > @@ -0,0 +1,87 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/display/solomon,ssd16xx.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Solomon Systech SSD16xx controller based e-paper display panels
> > +
> > +maintainers:
> > +  - Devarsh Thakkar <devarsht@xxxxxx>
> > +
> > +description:
> > +  The SSD16xx family includes e-paper display controllers (SSD1673,
> > +  SSD1680, SSD1681, SSD1683, SSD1677) with integrated gate driver,
> > +  source driver and timing controller. Different panels use these
> > +  controllers in monochrome or 3-color (black/white/red) configurations.
> > +  The controller uses a 4-wire SPI interface and requires GPIO pins for
> > +  reset, busy status, and data/command control. A 3-wire mode is also
> > +  available where the D/C bit is transmitted along the SPI data line
> > +  before each byte.
> > +
> > +allOf:
> > +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> > +
> > +properties:
> > +  compatible:
> > +    items:
> > +      - enum:
> > +          # Good Display 4.2" 400x300 monochrome e-ink panel
> > +          - gooddisplay,gdey042t81
> > +      - enum:
>
> No, fallback cannot be enumeration.
>
> > +          - solomon,ssd1683

Agreed, will fix in this patch itself.

> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  spi-max-frequency:
> > +    maximum: 20000000
> > +
> > +  reset-gpios:
> > +    maxItems: 1
> > +    description:
> > +      GPIO connected to the RSTB (reset) pin. Active low.
> > +
> > +  busy-gpios:
> > +    maxItems: 1
> > +    description:
> > +      GPIO connected to the BUSY pin. Active high when the controller
> > +      is busy updating the display.
> > +
> > +  dc-gpios:
> > +    maxItems: 1
> > +    description:
> > +      GPIO connected to the D/C (Data/Command) pin. Low for command,
> > +      high for data.
> > +
> > +  rotation:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    enum: [0, 90, 180, 270]
> > +    description:
> > +      Display rotation in degrees clockwise.
> > +
>
> Is this display controller or panel? If panel, then missing ref to
> panel-common. If display controller, then description is misleading.

Yes, I think it should be possible to ref panel-common, Thanks for pointing.

Regards
Devarsh

> Best regards,
> Krzysztof