Hi,
Am 27.09.2021 um 19:07 schrieb maxime@cerno.tech:
Hi,
On Mon, Sep 27, 2021 at 06:44:21PM +0200, H. Nikolaus Schaller wrote:
From: Sam Ravnborg sam@ravnborg.org
Add DT bindings for the hdmi driver for the Ingenic JZ4780 SoC. Based on .txt binding from Zubair Lutfullah Kakakhel
Signed-off-by: Sam Ravnborg sam@ravnborg.org Signed-off-by: H. Nikolaus Schaller hns@goldelico.com Cc: Rob Herring robh@kernel.org Cc: devicetree@vger.kernel.org
.../bindings/display/ingenic-jz4780-hdmi.yaml | 85 +++++++++++++++++++ 1 file changed, 85 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/ingenic-jz4780-hdmi.yaml
diff --git a/Documentation/devicetree/bindings/display/ingenic-jz4780-hdmi.yaml b/Documentation/devicetree/bindings/display/ingenic-jz4780-hdmi.yaml new file mode 100644 index 000000000000..5e60cdac4f63 --- /dev/null +++ b/Documentation/devicetree/bindings/display/ingenic-jz4780-hdmi.yaml @@ -0,0 +1,85 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/ingenic-jz4780-hdmi.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml#
+title: Bindings for Ingenic JZ4780 HDMI Transmitter
+maintainers:
- H. Nikolaus Schaller hns@goldelico.com
+description: |
- The HDMI Transmitter in the Ingenic JZ4780 is a Synopsys DesignWare HDMI 1.4
- TX controller IP with accompanying PHY IP.
+allOf:
- $ref: panel/panel-common.yaml#
Is it a panel though?
Good question.
Appears to have to be changed to
- $ref: bridge/synopsys,dw-hdmi.yaml#
+properties:
- compatible:
- items:
- const: ingenic,jz4780-dw-hdmiThis can just be a const, there's no need for the items
Maybe starting with an enum is better if more compatible strings are to be added.
- reg:
- maxItems: 1
- description: the address & size of the LCD controller registers
There's no need for that description, it's obvious enough
Indeed.
- reg-io-width:
- const: 4
If it's fixed, why do you need it in the first place?
There is a fixed default of 1 if not specified.
- interrupts:
- maxItems: 1
- description: Specifies the interrupt provided by parent
There's no need for that description, it's obvious enough
Indeed.
- clocks:
- maxItems: 2
- description: Clock specifiers for isrf and iahb clocks
This can be defined as
clocks: items:
- description: isrf
- description: iahb
A better description about what these clocks are would be nice as well
Generally I see that this all is nowadays not independent of
Documentation/devicetree/bindings/display/bridge/synopsys,dw-hdmi.yaml
where there is already a description.
On the other hand every SoC specialization runs its own copy. e.g.
Documentation/devicetree/bindings/display/imx/fsl,imx6-hdmi.yaml Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yam
- clock-names:
- items:
- const: isfrIs it isfr or isrf?
isfr. Seems to be a typo in the description. See bridge/synopsys,dw-hdmi.yaml#
One question to the yaml specialists:
since ../bridge/synopsys,dw-hdmi.yaml# already defines this, do we have to repeat? Or can we reduce to just the changes?
[I am still not familiar enough with the yaml stuff to understand if it has sort of inheritance like device tree include files, so that you just have to change relevant properties]
- const: iahb
would it make sense to add additionalItems: false here?
In the jz4780 case there are just two clocks while other specializations use more and synopsys,dw-hdmi.yaml# defines additionalItems: true.
- hdmi-regulator: true
- description: Optional regulator to provide +5V at the connector
regulators need to be suffixed by -supply
My omission...
And, it should be "hdmi-5v-supply" to match driver and device tree.
You also can just provide the description, you don't need the true there
- ddc-i2c-bus: true
ditto
Ok
- description: An I2C interface if the internal DDC I2C driver is not to be used
- ports: true
If there's a single port, you don't need ports
There can be two ports - one for input from LCDC and one for output (HDMI connector). But explicitly defining an output port is optional to some extent (depending on driver structure).
You should also include /schemas/graph.yaml#/$defs/port-base
Ok.
BR and thanks, Nikolaus