Re: [PATCH v4 1/2] ARM64: dts: sun50i-h6-pine-h64: state that the DT supports the modelA

[Maxime Ripard]

Hi,

On Sat, Nov 09, 2019 at 04:20:32PM +0000, Corentin Labbe wrote:
> The current sun50i-h6-pine-h64 DT does not specify which model (A or B)
> it supports.
> When this file was created, only modelA was existing, but now both model
> exists and with the time, this DT drifted to support the model B since it is
> the most common one.
> Furtheremore, some part of the model A does not work with it like ethernet and
> HDMI connector (as confirmed by Jernej on IRC).
>
> So it is time to settle the issue, and the easiest way was to state that
> this DT is for model B.
> Easiest since only a small name changes is required.
> Doing the opposite (stating this file is for model A) will add changes (for
> ethernet and HDMI) and so, will break too many setup.
>
> But as asked by the maintainer this patch state this file is for model A.
> In the process this patch adds the missing compoments to made it work on
> model A.
>
> Signed-off-by: Corentin Labbe <clabbe@xxxxxxxxxxxx>
> ---
>  .../devicetree/bindings/arm/sunxi.yaml        |  4 ++--
>  .../boot/dts/allwinner/sun50i-h6-pine-h64.dts | 19 +++++++++++++++----
>  2 files changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/arm/sunxi.yaml b/Documentation/devicetree/bindings/arm/sunxi.yaml
> index 8a1e38a1d7ab..b8ec616c2538 100644
> --- a/Documentation/devicetree/bindings/arm/sunxi.yaml
> +++ b/Documentation/devicetree/bindings/arm/sunxi.yaml
> @@ -599,9 +599,9 @@ properties:
>            - const: pine64,pine64-plus
>            - const: allwinner,sun50i-a64
>
> -      - description: Pine64 PineH64
> +      - description: Pine64 PineH64 model A
>          items:
> -          - const: pine64,pine-h64
> +          - const: pine64,pine-h64-modelA

You can change the description to make it more obvious if you want to,
but changing the compatible is a no-go.

>            - const: allwinner,sun50i-h6
>
>        - description: Pine64 LTS
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-pine-h64.dts b/arch/arm64/boot/dts/allwinner/sun50i-h6-pine-h64.dts
> index 74899ede00fb..1d9afde4d3d7 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-h6-pine-h64.dts
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-pine-h64.dts
> @@ -10,8 +10,8 @@
>  #include <dt-bindings/gpio/gpio.h>
>
>  / {
> -	model = "Pine H64";
> -	compatible = "pine64,pine-h64", "allwinner,sun50i-h6";
> +	model = "Pine H64 model A";
> +	compatible = "pine64,pine-h64-modelA", "allwinner,sun50i-h6";

Same thing here, changing the model is fine, the compatible isn't

>  	aliases {
>  		ethernet0 = &emac;
> @@ -22,9 +22,10 @@
>  		stdout-path = "serial0:115200n8";
>  	};
>
> -	connector {
> +	hdmi_connector: connector {

Why do you need to add the label?

Thanks!
Maxime

Attachment: signature.asc
Description: PGP signature