Hi Mark, Thanks, for the fast feedback. >From: Mark Rutland [mailto:mark.rutland@xxxxxxx] >Sent: Mittwoch, 12. Juli 2017 11:47 >> +/ { >> + model = "Freescale i.MX53 based Beckhoff CX9020"; >> + compatible = "fsl,imx53-qsb", "fsl,imx53"; >> + >> + chosen { >> + stdout-path = &uart2; > >No baud-rate or bits configuration? The default config from imx53.dtsi works fine for us. >> + ccat { >> + compatible = "bhf,emi-ccat"; >> + }; >> + >> + display0: display@di0 { > >This unit-address (the bit after the @) isn't valid, as that should >match a reg or ranges, but this node has neither. > >Just call this display-0. > Okay, I will fix this >> + #address-cells =<1>; >> + #size-cells = <0>; >> + compatible = "fsl,imx-parallel-display"; >> + interface-pix-fmt = "rgb24"; >> + pinctrl-names = "default"; >> + pinctrl-0 = <&pinctrl_ipu_disp0>; >> + status = "okay"; >> + >> + port@0 { >> + reg = <0>; >> + display0_in: endpoint { >> + remote-endpoint = <&ipu_di0_disp0>; >> + }; >> + }; >> + >> + port@1 { >> + reg = <1>; >> + display0_out: endpoint { >> + remote-endpoint = <&panel_in>; >> + }; >> + }; >> + }; >> + >> + dvi_panel: display@0 { > >Likewise you have no reg here, so the unit address isn't valid. > >Surely panel-0? > Okay, I will have a closer look, too. >> + #address-cells =<1>; >> + #size-cells = <0>; >> + compatible = "simple,ddc-only"; > >I don't see that compatible string in my Linux tree, and it doesn't make >sense to me -- "simple" isn't a vendor-prefix. > >Where has this come from? > Out-of-tree, sorry. Our device has a DVI connector bound to the imx parallel interface. So my idea was to reuse the panel-simple driver and add a very simple panel with only ddc options. Unfortunately, I was too shy to post that upstream[1]. Is there a more elegant solution? Or should I remove all display related nodes from imx53-cx9020.dts? >> + ddc-i2c-bus = <&i2c2>; >> + >> + port { >> + panel_in: endpoint { >> + remote-endpoint = <&display0_out>; >> + }; >> + }; >> + }; > >[...] > >> + regulators { >> + compatible = "simple-bus"; >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + reg_3p2v: regulator@0 { >> + compatible = "regulator-fixed"; >> + reg = <0>; > >Meaningless reg entry. > Okay, I will remove this. >> + regulator-name = "3P2V"; >> + regulator-min-microvolt = <3200000>; >> + regulator-max-microvolt = <3200000>; >> + regulator-always-on; >> + }; >> + >> + reg_usb_vbus: regulator@1 { >> + compatible = "regulator-fixed"; >> + reg = <1>; > >Likewise. > this, too. >> + regulator-name = "usb_vbus"; >> + regulator-min-microvolt = <5000000>; >> + regulator-max-microvolt = <5000000>; >> + gpio = <&gpio7 8 0>; >> + enable-active-high; >> + }; >> + }; > >There's no need for a simple-bus here. It doesn't represent HW, and you >can nothing. You can put these directly under the root node, without a >synthetic reg or unnecessary container: > > reg_3p2v: regulator-3p2v { > compatible = "regulator-fixed"; > regulator-name = "3P2V"; > regulator-min-microvolt = <3200000>; > regulator-max-microvolt = <3200000>; > regulator-always-on; > }; > > reg_usb_vbus: regulator-usb-vbus { > compatible = "regulator-fixed"; > regulator-name = "usb_vbus"; > regulator-min-microvolt = <5000000>; > regulator-max-microvolt = <5000000>; > gpio = <&gpio7 8 0>; > enable-active-high; > } > Thanks, I will send a v2 with your simplified version. As soon as I get the display/ panel thing right. >Otherwise, looks fine to me. > >Thanks, >Mark. [1] https://github.com/Beckhoff/CX9020/blob/master/kernel-patches/0003-drm-panel-simple-Add-support-for-ddc-only-panel.patch Beckhoff Automation GmbH & Co. KG | Managing Director: Dipl. Phys. Hans Beckhoff Registered office: Verl, Germany | Register court: Guetersloh HRA 7075 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
