Re: [PATCH v2 06/12] ARM: dts: apq8064: Add MDP support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 






On 10/04/15 21:21, Stephen Boyd wrote:
On 04/10/15 12:39, Srinivas Kandagatla wrote:


On 10/04/15 18:04, Stephen Boyd wrote:
On 04/10/15 05:34, Srinivas Kandagatla wrote:
@@ -250,6 +265,18 @@
               };
           };

+        ext_3p3v: regulator-fixed@1 {
+            compatible = "regulator-fixed";
+            regulator-min-microvolt = <3300000>;
+            regulator-max-microvolt = <3300000>;
+            regulator-name = "ext_3p3v";
+            regulator-type = "voltage";
+            startup-delay-us = <0>;
+            gpio = <&tlmm_pinmux 77 GPIO_ACTIVE_HIGH>;
+            enable-active-high;
+            regulator-boot-on;
+        };

This shouldn't be inside the SoC node because it doesn't have a reg
property. It should be in a 'regulators' node that's in the root of the
tree:

Is this new DT requirement/style? I have not noticed such a dt style
in the past. I might have missed it. Any advantage of doing this way?

It's a style. I'm not sure if it's new, but I feel like I've seen
mention of it before more than a year ago (see
arch/arm/boot/dts/tegra30-beaver.dts for an example). The advantage of
doing it this way is we can see all the gpio/fixed regulators in one
place and they're physically placed on a separate bus from the SoC bus.
Typically nodes have reg properties too, so making up fake reg
properties for the regulator nodes when they're on the SoC bus would be
wrong and confusing. If they're under some regulators container node we
can number them from 0 to N and use that for the reg property.

Thanks for explaining.

+
+        hdmi: qcom,hdmi-tx@4a00000 {
+            compatible = "qcom,hdmi-tx-8960";
+            reg-names = "core_physical";
+            reg = <0x04a00000 0x1000>;
+            interrupts = <GIC_SPI 79 IRQ_TYPE_NONE>;
+            clock-names =
+                "core_clk",
+                "master_iface_clk",
+                "slave_iface_clk";
+            clocks =
+                <&mmcc HDMI_APP_CLK>,
+                <&mmcc HDMI_M_AHB_CLK>,
+                <&mmcc HDMI_S_AHB_CLK>;
+            qcom,hdmi-tx-ddc-clk = <&tlmm_pinmux 70
+                        GPIO_ACTIVE_HIGH>;
+            qcom,hdmi-tx-ddc-data = <&tlmm_pinmux 71
+                        GPIO_ACTIVE_HIGH>;
+            qcom,hdmi-tx-hpd = <&tlmm_pinmux 72
+                        GPIO_ACTIVE_HIGH>;

This should be done via the *-gpios method. i.e. hdmi-tx-ddc-clk-gpios,
hdmi-tx-ddc-data-gpios, etc.

Thanks for the inputs,

That's true, These are existing bindings, so I can't change it as part
of this patch, However I will make another patch to fix this in both
drivers and DT for good reasons. Just noticed that bindings are not
consistent with the examples and the driver, which I guess is another
issue.

Yes, the driver/binding should be fixed and then this patch can be
corrected and applied. There are no implementations of the DT for this
device upstream in the dts directory so there's no breakage or backwards
incompatibility problem by fixing the driver/binding.

Yep, In that case, I should pull this patch out of this series just to avoid any delays and create a new patchset for fixing bindings + driver + DT.


--
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




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux