| Message ID | 1428669271-11032-1-git-send-email-srinivas.kandagatla@linaro.org |
|---|---|
| State | New |
| Headers | show |
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? > > regulators { > compatible = "simple-bus"; > > ext_3p3v: fixedregulator@0 { > compatible = "regulator-fixed"; > ... > }; > }; > I will move this to the suggested style in next version. > >> + >> qcom,ssbi@500000 { >> compatible = "qcom,ssbi"; >> reg = <0x00500000 0x1000>; >> @@ -522,5 +549,82 @@ >> compatible = "qcom,tcsr-apq8064", "syscon"; >> reg = <0x1a400000 0x100>; >> }; >> + >> + 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. >> + core-vdda-supply = <&pm8921_hdmi_switch>; >> + hdmi-mux-supply = <&ext_3p3v>; >> + pinctrl-names = "default"; >> + pinctrl-0 = <&hdmi_pinctrl>; >> + }; >> + >> + gpu: qcom,adreno-3xx@4300000 { >> + compatible = "qcom,adreno-3xx"; >> + reg = <0x04300000 0x20000>; >> + reg-names = "kgsl_3d0_reg_memory"; >> + interrupts = <GIC_SPI 80 IRQ_TYPE_NONE>; >> + interrupt-names = "kgsl_3d0_irq"; >> + clock-names = >> + "core_clk", >> + "iface_clk", >> + "mem_clk", >> + "mem_iface_clk"; >> + clocks = >> + <&mmcc GFX3D_CLK>, >> + <&mmcc GFX3D_AHB_CLK>, >> + <&mmcc GFX3D_AXI_CLK>, >> + <&mmcc MMSS_IMEM_AHB_CLK>; >> + qcom,chipid = <0x03020002>; >> + qcom,gpu-pwrlevels { >> + compatible = "qcom,gpu-pwrlevels"; >> + qcom,gpu-pwrlevel@0 { >> + qcom,gpu-freq = <450000000>; >> + }; >> + qcom,gpu-pwrlevel@1 { >> + qcom,gpu-freq = <27000000>; >> + }; >> + }; > > This should be an OPP. Yes, that looks reasonable approch, But as I said before the driver and the bindings are still using this style, so I cant change this as part of this patch. I will create another patch to for better bindings with driver fixes too. > -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
diff --git a/arch/arm/boot/dts/qcom-apq8064.dtsi b/arch/arm/boot/dts/qcom-apq8064.dtsi index 56cc65e..c88470c 100644 --- a/arch/arm/boot/dts/qcom-apq8064.dtsi +++ b/arch/arm/boot/dts/qcom-apq8064.dtsi @@ -1,6 +1,7 @@ /dts-v1/; #include "skeleton.dtsi" +#include <dt-bindings/gpio/gpio.h> #include <dt-bindings/clock/qcom,gcc-msm8960.h> #include <dt-bindings/reset/qcom,gcc-msm8960.h> #include <dt-bindings/clock/qcom,mmcc-msm8960.h> @@ -107,6 +108,20 @@ }; }; + hdmi_pinctrl: hdmi-pinctrl { + mux1 { + pins = "gpio69", "gpio70", "gpio71"; + function = "hdmi"; + bias-pull-up; + drive-strength = <2>; + }; + mux2 { + pins = "gpio72"; + function = "hdmi"; + bias-pull-down; + drive-strength = <16>; + }; + }; ps_hold: ps_hold { mux { pins = "gpio78"; @@ -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; + }; + qcom,ssbi@500000 { compatible = "qcom,ssbi"; reg = <0x00500000 0x1000>; @@ -522,5 +549,82 @@ compatible = "qcom,tcsr-apq8064", "syscon"; reg = <0x1a400000 0x100>; }; + + 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>; + core-vdda-supply = <&pm8921_hdmi_switch>; + hdmi-mux-supply = <&ext_3p3v>; + pinctrl-names = "default"; + pinctrl-0 = <&hdmi_pinctrl>; + }; + + gpu: qcom,adreno-3xx@4300000 { + compatible = "qcom,adreno-3xx"; + reg = <0x04300000 0x20000>; + reg-names = "kgsl_3d0_reg_memory"; + interrupts = <GIC_SPI 80 IRQ_TYPE_NONE>; + interrupt-names = "kgsl_3d0_irq"; + clock-names = + "core_clk", + "iface_clk", + "mem_clk", + "mem_iface_clk"; + clocks = + <&mmcc GFX3D_CLK>, + <&mmcc GFX3D_AHB_CLK>, + <&mmcc GFX3D_AXI_CLK>, + <&mmcc MMSS_IMEM_AHB_CLK>; + qcom,chipid = <0x03020002>; + qcom,gpu-pwrlevels { + compatible = "qcom,gpu-pwrlevels"; + qcom,gpu-pwrlevel@0 { + qcom,gpu-freq = <450000000>; + }; + qcom,gpu-pwrlevel@1 { + qcom,gpu-freq = <27000000>; + }; + }; + }; + + mdp: qcom,mdp@5100000 { + compatible = "qcom,mdp"; + reg = <0x05100000 0xf0000>; + interrupts = <GIC_SPI 75 IRQ_TYPE_NONE>; + connectors = <&hdmi>; + gpus = <&gpu>; + clock-names = + "core_clk", + "iface_clk", + "lut_clk", + "src_clk", + "hdmi_clk", + "mdp_clk", + "mdp_axi_clk"; + clocks = + <&mmcc MDP_CLK>, + <&mmcc MDP_AHB_CLK>, + <&mmcc MDP_LUT_CLK>, + <&mmcc TV_SRC>, + <&mmcc HDMI_TV_CLK>, + <&mmcc MDP_TV_CLK>, + <&mmcc MDP_AXI_CLK>; + }; }; };