Re: [PATCH v5 1/3] arm64: dts: fsl: librem5: Add a device tree for the Librem5 devkit

[Angus Ainslie <angus@xxxxxxxx>]

Hi Rob,


On 2019-04-18 08:13, Rob Herring wrote:
> On Wed, Apr 17, 2019 at 09:55:12AM -0700, Angus Ainslie (Purism) wrote:
> > This is the development kit board for the Librem 5. The current level 
> > of
> > support yields a working console and is able to boot userspace from 
> > the
> > Network or eMMC.
> >
> > Additional subsystems that are active :
> >
> > - Both USB ports
> > - SD card socket
> > - WiFi usdhc
> > - WWAN modem
> > - GNSS
> > - GPIO keys
> > - LEDs
> > - gyro
> > - magnetometer
> > - touchscreen
> > - pwm
> > - backlight
> > - haptic motor
> >
> > Signed-off-by: Angus Ainslie (Purism) <angus@xxxxxxxx>
> > ---
> >  arch/arm64/boot/dts/freescale/Makefile        |   1 +
> >  .../dts/freescale/imx8mq-librem5-devkit.dts   | 833 
> > ++++++++++++++++++
> >  2 files changed, 834 insertions(+)
> >  create mode 100644 
> > arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts
> >
> > diff --git a/arch/arm64/boot/dts/freescale/Makefile 
> > b/arch/arm64/boot/dts/freescale/Makefile
> > index 0bd122f60549..c043aca66572 100644
> > --- a/arch/arm64/boot/dts/freescale/Makefile
> > +++ b/arch/arm64/boot/dts/freescale/Makefile
> > @@ -22,6 +22,7 @@ dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-lx2160a-rdb.dtb
> >
> >  dtb-$(CONFIG_ARCH_MXC) += imx8mm-evk.dtb
> >  dtb-$(CONFIG_ARCH_MXC) += imx8mq-evk.dtb
> > +dtb-$(CONFIG_ARCH_MXC) += imx8mq-librem5-devkit.dtb
> >  dtb-$(CONFIG_ARCH_MXC) += imx8mq-zii-ultra-rmb3.dtb
> >  dtb-$(CONFIG_ARCH_MXC) += imx8mq-zii-ultra-zest.dtb
> >  dtb-$(CONFIG_ARCH_MXC) += imx8qxp-mek.dtb
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts 
> > b/arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts
> > new file mode 100644
> > index 000000000000..846a72dc7640
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts
>
> > +&usb3_phy0 {
> > +	#address-cells = <1>;
> > +	#size-cells = <0>;
> > +	status = "okay";
> > +
> > +	port@0 {
> > +		reg = <0>;
> > +
> > +		typec_hs: endpoint {
> > +			remote-endpoint = <&usb_con_hs>;
> > +		};
> > +	};
> > +
> > +	port@1 {
> > +		reg = <1>;
> > +
> > +		typec_ss: endpoint {
> > +			remote-endpoint = <&usb_con_ss>;
> > +		};
> > +	};
>
> This is wrong. The graph should be between the USB controller and the
> connector. The phy is not involved.
>
> > +};
> > +
> > +&usb3_phy1 {
> > +	status = "okay";
> > +};
> > +
> > +&usb_dwc3_0 {
> > +	extcon = <&typec_ptn5100>;
>
> extcon is deprecated and you should only have the usb-connector 
> binding.
> From the controller, you can walk the graph to the connector.

I not sure that I'm clear on what you mean. Here's what I think you want 
it changed to.

diff --git a/arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts 
b/arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts
index 846a72dc7640..7b8770fdc5fb 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts
+++ b/arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts
@@ -755,51 +755,50 @@
 };

 &uart4 { /* BT */
        pinctrl-names = "default";
        pinctrl-0 = <&pinctrl_uart4>, <&pinctrl_bt>;
        uart-has-rtscts;
        status = "okay";
 };

 &usb3_phy0 {
+       status = "okay";
+};
+
+&usb3_phy1 {
+       status = "okay";
+};
+
+&usb_dwc3_0 {
        #address-cells = <1>;
        #size-cells = <0>;
+       dr_mode = "otg";
        status = "okay";

        port@0 {
                reg = <0>;

                typec_hs: endpoint {
                        remote-endpoint = <&usb_con_hs>;
                };
        };

        port@1 {
                reg = <1>;

                typec_ss: endpoint {
                        remote-endpoint = <&usb_con_ss>;
                };
        };
 };

-&usb3_phy1 {
-       status = "okay";
-};
-
-&usb_dwc3_0 {
-       extcon = <&typec_ptn5100>;
-       dr_mode = "otg";
-       status = "okay";
-};
-
 &usb_dwc3_1 {
        dr_mode = "host";
        status = "okay";
 };



Thanks
Angus

> > +	dr_mode = "otg";
> > +	status = "okay";
> > +};