| Message ID | 1573316433-40669-2-git-send-email-clabbe@baylibre.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | ARM64: dts: allwinner: Add devicetree for pineH64 modelB | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | warning | "total: 0 errors, 3 warnings, 56 lines checked" |
| robh/dt-meta-schema | success |
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@baylibre.com> > --- > .../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
On Tue, Nov 12, 2019 at 01:02:19PM +0100, Maxime Ripard wrote: > 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@baylibre.com> > > --- > > .../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 > Hello I will erase compatible changes in next version. > > aliases { > > ethernet0 = &emac; > > @@ -22,9 +22,10 @@ > > stdout-path = "serial0:115200n8"; > > }; > > > > - connector { > > + hdmi_connector: connector { > > Why do you need to add the label? > For dropping the ddc-en-gpios property in model B. If you want, I can split this line change in an extra patch. Regards
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 - 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"; aliases { ethernet0 = &emac; @@ -22,9 +22,10 @@ stdout-path = "serial0:115200n8"; }; - connector { + hdmi_connector: connector { compatible = "hdmi-connector"; type = "a"; + ddc-en-gpios = <&pio 7 2 GPIO_ACTIVE_HIGH>; /* PH2 */ port { hdmi_con_in: endpoint { @@ -52,6 +53,16 @@ }; }; + reg_gmac_3v3: gmac-3v3 { + compatible = "regulator-fixed"; + regulator-name = "vcc-gmac-3v3"; + regulator-min-microvolt = <3300000>; + regulator-max-microvolt = <3300000>; + startup-delay-us = <100000>; + gpio = <&pio 2 16 GPIO_ACTIVE_HIGH>; + enable-active-high; + }; + reg_usb_vbus: vbus { compatible = "regulator-fixed"; regulator-name = "usb-vbus"; @@ -68,7 +79,7 @@ pinctrl-0 = <&ext_rgmii_pins>; phy-mode = "rgmii"; phy-handle = <&ext_rgmii_phy>; - phy-supply = <®_aldo2>; + phy-supply = <®_gmac_3v3>; allwinner,rx-delay-ps = <200>; allwinner,tx-delay-ps = <200>; status = "okay";
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@baylibre.com> --- .../devicetree/bindings/arm/sunxi.yaml | 4 ++-- .../boot/dts/allwinner/sun50i-h6-pine-h64.dts | 19 +++++++++++++++---- 2 files changed, 17 insertions(+), 6 deletions(-)