Messages in this thread |  | | Date | Fri, 10 Apr 2015 21:30:29 +0100 | From | Srinivas Kandagatla <> | Subject | Re: [PATCH v2 06/12] ARM: dts: apq8064: Add MDP support |
| |
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.
|  |