Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1685723
| From | Patrick Brünn <P.Bruenn@beckhoff.com> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | RE: [PATCH] ARM: dts: imx: add CX9020 Embedded PC device tree |
| Date | 2017-07-12 14:00 +0200 |
| Message-ID | <u2otb-7MP-1@gated-at.bofh.it> (permalink) |
| References | <u2lYm-6m0-31@gated-at.bofh.it> <u2mro-6xz-15@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
Hi Mark,
Thanks, for the fast feedback.
>From: Mark Rutland [mailto:mark.rutland@arm.com]
>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
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
[PATCH] ARM: dts: imx: add CX9020 Embedded PC device tree <linux-kernel-dev@beckhoff.com> - 2017-07-12 11:20 +0200
Re: [PATCH] ARM: dts: imx: add CX9020 Embedded PC device tree Mark Rutland <mark.rutland@arm.com> - 2017-07-12 11:50 +0200
RE: [PATCH] ARM: dts: imx: add CX9020 Embedded PC device tree Patrick Brünn <P.Bruenn@beckhoff.com> - 2017-07-12 14:00 +0200
Re: [PATCH] ARM: dts: imx: add CX9020 Embedded PC device tree Andrew Lunn <andrew@lunn.ch> - 2017-07-12 16:50 +0200
RE: [PATCH] ARM: dts: imx: add CX9020 Embedded PC device tree Patrick Brünn <P.Bruenn@beckhoff.com> - 2017-07-13 05:20 +0200
[PATCH v2 0/2] Add CX9020 device tree <linux-kernel-dev@beckhoff.com> - 2017-07-13 13:10 +0200
[PATCH v2 2/2] drm/panel: simple: Add support for ddc-only panel <linux-kernel-dev@beckhoff.com> - 2017-07-13 13:10 +0200
Re: [PATCH v2 2/2] drm/panel: simple: Add support for ddc-only panel Rob Herring <robh@kernel.org> - 2017-07-14 16:20 +0200
[PATCH v2 1/2] ARM: dts: imx: add CX9020 Embedded PC device tree <linux-kernel-dev@beckhoff.com> - 2017-07-13 13:20 +0200
[PATCH v3 1/2] ARM: dts: imx: add CX9020 Embedded PC device tree <linux-kernel-dev@beckhoff.com> - 2017-07-13 13:20 +0200
Re: [PATCH v3 1/2] ARM: dts: imx: add CX9020 Embedded PC device tree Shawn Guo <shawnguo@kernel.org> - 2017-07-14 04:20 +0200
[PATCH v3 0/2] Add CX9020 device tree <linux-kernel-dev@beckhoff.com> - 2017-07-13 13:20 +0200
[PATCH v3 2/2] drm/panel: simple: Add support for ddc-only panel <linux-kernel-dev@beckhoff.com> - 2017-07-13 13:20 +0200
csiph-web