linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Conor Dooley <conor@kernel.org>
To: Johan Jonker <jbx6244@gmail.com>
Cc: robh+dt@kernel.org, hjc@rock-chips.com, heiko@sntech.de,
	andy.yan@rock-chips.com, krzysztof.kozlowski+dt@linaro.org,
	conor+dt@kernel.org, airlied@gmail.com, daniel@ffwll.ch,
	maarten.lankhorst@linux.intel.com, tzimmermann@suse.de,
	mripard@kernel.org, markyao0591@gmail.com,
	dri-devel@lists.freedesktop.org, devicetree@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v1 1/6] dt-bindings: display: rockchip: rockchip,dw-hdmi: deprecate port property
Date: Tue, 30 Jan 2024 18:18:49 +0000	[thread overview]
Message-ID: <20240130-denim-capacity-e1617595ac9f@spud> (raw)
In-Reply-To: <6626d288-ecf1-436b-b0a3-3417308a8633@gmail.com>


[-- Attachment #1.1: Type: text/plain, Size: 3859 bytes --]

On Tue, Jan 30, 2024 at 03:55:43PM +0100, Johan Jonker wrote:
> The hdmi-connector nodes are now functional and the new way to model
> hdmi nodes with, so deprecate the port property and

This doesn't really explain what makes having hdmi-connector nodes
replace the usecase for "port".

> make port@0 and
> port@1 a requirement.

Why?

> Also update example.

"Also do x" is a red flag when it comes to commit messages, as it
immediately makes me think that this should be more than one commit.
I'd probably write this as "Update the example to avoid use of the
deprecated property" or something to avoid bad gut reactions.
That's not worth resending for though obviously...

> 
> Signed-off-by: Johan Jonker <jbx6244@gmail.com>
> ---
>  .../display/rockchip/rockchip,dw-hdmi.yaml    | 27 ++++++++++++++++---
>  1 file changed, 23 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml b/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml
> index 7e59dee15a5f..cd0a42f35f24 100644
> --- a/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml
> +++ b/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml
> @@ -97,8 +97,11 @@ properties:
>    ports:
>      $ref: /schemas/graph.yaml#/properties/ports
> 
> -    patternProperties:
> -      "^port(@0)?$":
> +    properties:
> +      port:
> +        $ref: /schemas/graph.yaml#/properties/port
> +        deprecated: true

This change makes the deprecated property's description incomplete,
since it doesn't cover the endpoints any more. It also doesn't make
port@0 and port mutually exclusive.
I think I'd just be inclined to delete the "port" bit entirely from the
binding if you're not gonna preserve the description of what that
property was meant to be. Otherwise, I'd be doing something like:
@@ -112,6 +112,8 @@ properties:
           endpoint@1:
             description: Connection to the VOPL
     properties:
+      port:
+        deprecated: true
       port@1:
         $ref: /schemas/graph.yaml#/properties/port
         description: Output of the DWC HDMI TX

So that the description of the deprecated property is maintained.

Cheers,
Conor.

> +      port@0:
>          $ref: /schemas/graph.yaml#/properties/port
>          description: Input of the DWC HDMI TX
>          properties:
> @@ -108,11 +111,14 @@ properties:
>              description: Connection to the VOPB
>            endpoint@1:
>              description: Connection to the VOPL
> -    properties:
>        port@1:
>          $ref: /schemas/graph.yaml#/properties/port
>          description: Output of the DWC HDMI TX
> 
> +    required:
> +      - port@0
> +      - port@1
> +
>    rockchip,grf:
>      $ref: /schemas/types.yaml#/definitions/phandle
>      description:
> @@ -147,7 +153,11 @@ examples:
>          clock-names = "iahb", "isfr";
> 
>          ports {
> -            port {
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +
> +            port@0 {
> +                reg = <0>;
>                  #address-cells = <1>;
>                  #size-cells = <0>;
> 
> @@ -155,11 +165,20 @@ examples:
>                      reg = <0>;
>                      remote-endpoint = <&vopb_out_hdmi>;
>                  };
> +
>                  hdmi_in_vopl: endpoint@1 {
>                      reg = <1>;
>                      remote-endpoint = <&vopl_out_hdmi>;
>                  };
>              };
> +
> +            port@1 {
> +                reg = <1>;
> +
> +                hdmi_out_con: endpoint {
> +                    remote-endpoint = <&hdmi_con_in>;
> +                };
> +            };
>          };
>      };
> 
> --
> 2.39.2
> 

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  parent reply	other threads:[~2024-01-30 18:19 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-30 14:55 [PATCH v1 1/6] dt-bindings: display: rockchip: rockchip,dw-hdmi: deprecate port property Johan Jonker
2024-01-30 14:57 ` [PATCH v1 2/6] dt-bindings: display: rockchip,dw-hdmi: add power-domains property Johan Jonker
2024-01-30 18:05   ` Conor Dooley
2024-01-31 13:59   ` Rob Herring
2024-01-30 14:57 ` [PATCH v1 3/6] ARM: dts: rockchip: fix rk3288 hdmi ports node Johan Jonker
2024-01-30 14:57 ` [PATCH v1 4/6] ARM: dts: rockchip: fix rk322x " Johan Jonker
2024-01-30 14:58 ` [PATCH v1 5/6] arm64: dts: rockchip: fix rk3328 " Johan Jonker
2024-01-30 14:58 ` [PATCH v1 6/6] arm64: dts: rockchip: fix rk3399 " Johan Jonker
2024-01-30 18:18 ` Conor Dooley [this message]
2024-01-31 14:15   ` [PATCH v1 1/6] dt-bindings: display: rockchip: rockchip,dw-hdmi: deprecate port property Rob Herring

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240130-denim-capacity-e1617595ac9f@spud \
    --to=conor@kernel.org \
    --cc=airlied@gmail.com \
    --cc=andy.yan@rock-chips.com \
    --cc=conor+dt@kernel.org \
    --cc=daniel@ffwll.ch \
    --cc=devicetree@vger.kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=heiko@sntech.de \
    --cc=hjc@rock-chips.com \
    --cc=jbx6244@gmail.com \
    --cc=krzysztof.kozlowski+dt@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=markyao0591@gmail.com \
    --cc=mripard@kernel.org \
    --cc=robh+dt@kernel.org \
    --cc=tzimmermann@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).