[PATCH v2 1/6] dt-bindings: display: Add yamls for JH7110 display system
Keith Zhao
keith.zhao at starfivetech.com
Wed Nov 29 03:13:26 UTC 2023
On 2023/10/25 20:50, Krzysztof Kozlowski wrote:
> On 25/10/2023 12:39, Keith Zhao wrote:
>> StarFive SoCs JH7110 display system:
>
> A nit, subject: drop second/last, redundant "yamls for". The
> "dt-bindings" prefix is already stating that these are bindings, so
> format is fixed.
>
>> lcd-controller bases verisilicon dc8200 IP,
>> and hdmi bases Innosilicon IP. Add bindings for them.
>
> Please make it a proper sentences, with proper wrapping.
>
>>
>> also update MAINTAINERS for dt-bindings
>
> Not a sentence, but also not really needed.ok I see.
>
>>
>> about this patch, I tested the dtbs_check and dt_binding_check
>> with the result pass.
>> Based on the feedback of the previous version, the corresponding arrangement is made
>
> Not relevant, so not really suitable for commit msg.
>
>>
>> Signed-off-by: Keith Zhao <keith.zhao at starfivetech.com>
>> ---
>> .../starfive/starfive,display-subsystem.yaml | 41 +++++++
>> .../starfive/starfive,jh7110-dc8200.yaml | 109 ++++++++++++++++++
>> .../starfive/starfive,jh7110-inno-hdmi.yaml | 85 ++++++++++++++
>> MAINTAINERS | 7 ++
>> 4 files changed, 242 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/display/starfive/starfive,display-subsystem.yaml
>> create mode 100644 Documentation/devicetree/bindings/display/starfive/starfive,jh7110-dc8200.yaml
>> create mode 100644 Documentation/devicetree/bindings/display/starfive/starfive,jh7110-inno-hdmi.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/display/starfive/starfive,display-subsystem.yaml b/Documentation/devicetree/bindings/display/starfive/starfive,display-subsystem.yaml
>> new file mode 100644
>> index 000000000..f45b97b08
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/display/starfive/starfive,display-subsystem.yaml
>> @@ -0,0 +1,41 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/display/starfive/starfive,display-subsystem.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Starfive DRM master device
>
> What is DRM in hardware? I know Digital Rights Management, but then
> subsystem seems wrong. If you mean Linux DRM, then Linux is not a
> hardware, so drop all Linuxisms and describe hardware.
ok , will only keep hardware describe in my next version
>
>
>> +
>> +maintainers:
>> + - Keith Zhao <keith.zhao at starfivetech.com>
>> + - ShengYang Chen <shengyang.chen at starfivetech.com>
>> +
>> +description:
>> + The Starfive DRM master device is a virtual device needed to list all
>
> Virtual device? Then not suitable for bindings, sorry.
>
>> + display controller or other display interface nodes that comprise the
>> + graphics subsystem.
>> +
>> +properties:
>> + compatible:
>> + const: starfive,display-subsystem
>> +
>> + ports:
>> + $ref: /schemas/types.yaml#/definitions/phandle-array
>
> No, ports is not phandle-array. ports is object, always.
>
>> + description:
>> + Should contain a list of phandles pointing to display interface ports
>> + of display controller devices. Display controller definitions as defined
>> + in Documentation/devicetree/bindings/display/starfive/
>> + starfive,jh7110-dc8200.yaml
>
> Use standard graph ports, not some own, custom property.
>
> Anyway, entire binding should be dropped. You do not need it even.
Hi Krzysztof:
Virtual device is not suitable for bindings, matbe I need associate it with the real hardware.
such as the top clocks & reset , irq , etc.
Currently I configure them in another yaml file. Logically speaking, this is more suitable.
Can adding the corresponding hardware description change its fate of being deleted?
>
>> +
>> +required:
>> + - compatible
>> + - ports
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> + - |
>> + display-subsystem {
>> + compatible = "starfive,display-subsystem";
>> + ports = <&dc_out>;
>> + };
>> diff --git a/Documentation/devicetree/bindings/display/starfive/starfive,jh7110-dc8200.yaml b/Documentation/devicetree/bindings/display/starfive/starfive,jh7110-dc8200.yaml
>> new file mode 100644
>> index 000000000..87051cddf
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/display/starfive/starfive,jh7110-dc8200.yaml
>> @@ -0,0 +1,109 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/display/starfive/starfive,jh7110-dc8200.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: StarFive display controller
>> +
>> +description:
>> + The StarFive SoC uses the display controller based on Verisilicon IP
>> + to transfer the image data from a video memory buffer to an external
>> + LCD interface.
>> +
>> +maintainers:
>> + - Keith Zhao <keith.zhao at starfivetech.com>
>> +
>> +properties:
>> + compatible:
>> + const: starfive,jh7110-dc8200
>> +
>> + reg:
>> + minItems: 1
>> + items:
>> + - description:
>> + host interface
>
> In one line.
> - description: host interface
>
>> + - description:
>> + display physical base address and length.
>
> Drop redundant parts: base address and length. Everything in reg for
> MMIO would have it...
>
>> +
>> + interrupts:
>> + items:
>> + - description: The interrupt will be generated when DC finish one frame
>> +
>> + clocks:
>> + items:
>> + - description: Clock for display system noc bus.
>> + - description: Pixel clock for display channel 0.
>> + - description: Pixel clock for display channel 1.
>> + - description: Core clock for display controller.
>> + - description: Clock for axi bus to access ddr.
>> + - description: Clock for ahb bus to R/W the phy regs.
>> + - description: External HDMI pixel clock.
>> + - description: Parent clock for pixel clock
>> +
>> + clock-names:
>> + items:
>> + - const: noc_bus
>> + - const: channel0
>> + - const: channel1
>> + - const: dc_core
>> + - const: axi_core
>> + - const: ahb
>> + - const: hdmi_tx
>> + - const: dc_parent
>> +
>> + resets:
>> + items:
>> + - description: Reset for axi bus.
>> + - description: Reset for ahb bus.
>> + - description: Core reset of display controller.
>> +
>> + reset-names:
>> + items:
>> + - const: axi
>> + - const: ahb
>> + - const: core
>> +
>> + port:
>> + $ref: /schemas/graph.yaml#/properties/port
>> +
>> +required:
>> + - compatible
>> + - reg
>> + - interrupts
>> + - clocks
>> + - clock-names
>> + - resets
>> + - reset-names
>> + - port
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> + - |
>> + dc8200: lcd-controller at 29400000 {
>> + compatible = "starfive,jh7110-dc8200";
>> + reg = <0x29400000 0x100>, <0x29400800 0x2000>;
>> + interrupts = <95>;
>> + clocks = <&syscrg 60>,
>> + <&voutcrg 7>,
>> + <&voutcrg 8>,
>> + <&voutcrg 4>,
>> + <&voutcrg 5>,
>> + <&voutcrg 6>,
>> + <&hdmitx0_pixelclk>,
>> + <&voutcrg 1>;
>> + clock-names = "noc_bus", "channel0", "channel1",
>> + "dc_core", "axi_core", "ahb",
>> + "hdmi_tx","dc_parent";
>> + resets = <&voutcrg 0>, <&voutcrg 1>, <&voutcrg 2>;
>> + reset-names = "axi", "ahb", "core";
>> + dc_out: port {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + dc_out_hdmi: endpoint at 0 {
>> + reg = <0>;
>> + remote-endpoint = <&hdmi_in_dc>;
>> + };
>> + };
>> + };
>> diff --git a/Documentation/devicetree/bindings/display/starfive/starfive,jh7110-inno-hdmi.yaml b/Documentation/devicetree/bindings/display/starfive/starfive,jh7110-inno-hdmi.yaml
>> new file mode 100644
>> index 000000000..f6c473a10
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/display/starfive/starfive,jh7110-inno-hdmi.yaml
>> @@ -0,0 +1,85 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/display/starfive/starfive,jh7110-inno-hdmi.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Starfive JH7110 HDMI controller
>> +
>> +description:
>> + The StarFive JH7110 SoC uses the HDMI signal transmiter based on innosilicon IP
>> + to generate HDMI signal from its input and transmit the signal to the screen.
>> +
>> +maintainers:
>> + - Keith Zhao <keith.zhao at starfivetech.com>
>> +
>> +properties:
>> + compatible:
>> + const: "starfive,jh7110-inno-hdmi"
>
> It does not look like you tested the bindings, at least after quick
> look. Please run `make dt_binding_check` (see
> Documentation/devicetree/bindings/writing-schema.rst for instructions).
> Maybe you need to update your dtschema and yamllint.
>
> Best regards,
> Krzysztof
>
More information about the dri-devel
mailing list