Hi Andy,
As discussed here are some dt patches which depend on
pmic header "dt-bindings/pinctrl/qcom,pmic-gpio.h" which is availble in
linux-next.
One more thing we discussed w.r.t MDP patch is that it can use
something like OPPs. Which IMHO can be done at later stage as suggested by Rob.
Having this patch atleast in mainline would enable people to work on mainline
code rather than maintaining it locally. And with iommu patches people can get
a working display too :-)
Rest of the patch series is to do with power-sequence for WLAN and
an heart-beat LED for IFC6410.
Thanks,
srini
Rob Clark (1):
ARM: dts: apq8064: Add MDP support
Srinivas Kandagatla (6):
ARM: dts: ifc6410: Add pwrseq support for WLAN
ARM: dts: qs600: add pwrseq support to WLAN
ARM: dts: apq8064-ifc6410: add heart-beat led support.
ARM: dts: ifc6410: enable MDP support
ARM: dts: qs600: enable MDP support
ARM: dts: ifc6410: add inforce LVDS panel support
arch/arm/boot/dts/qcom-apq8064-cm-qs600.dts | 36 ++++++++
arch/arm/boot/dts/qcom-apq8064-ifc6410.dts | 122 ++++++++++++++++++++++++++++
arch/arm/boot/dts/qcom-apq8064.dtsi | 87 ++++++++++++++++++++
3 files changed, 245 insertions(+)
--
1.9.1
This patch adds pwrseq for WLAN which resets the WLAN just before the
SDIO bus is up.
Signed-off-by: Srinivas Kandagatla <[email protected]>
---
arch/arm/boot/dts/qcom-apq8064-ifc6410.dts | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/arch/arm/boot/dts/qcom-apq8064-ifc6410.dts b/arch/arm/boot/dts/qcom-apq8064-ifc6410.dts
index 88d6655..df560cf 100644
--- a/arch/arm/boot/dts/qcom-apq8064-ifc6410.dts
+++ b/arch/arm/boot/dts/qcom-apq8064-ifc6410.dts
@@ -1,5 +1,6 @@
#include "qcom-apq8064-v2.0.dtsi"
#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/pinctrl/qcom,pmic-gpio.h>
/ {
model = "Qualcomm APQ8064/IFC6410";
@@ -206,6 +207,28 @@
status = "okay";
};
+ qcom,ssbi@500000 {
+ pmicintc: pmic@0 {
+ pm8921_gpio: gpio@150 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&wlan_default_gpios>;
+ wlan_default_gpios: wlan-gpios {
+ pios {
+ pins = "gpio43";
+ function = "normal";
+ bias-disable;
+ power-source = <PM8921_GPIO_S4>;
+ };
+ };
+ };
+ };
+ };
+
+ sdcc4_pwrseq:pwrseq {
+ compatible = "mmc-pwrseq-simple";
+ reset-gpios = <&pm8921_gpio 43 GPIO_ACTIVE_LOW>;
+ };
+
amba {
/* eMMC */
sdcc1: sdcc@12400000 {
@@ -227,6 +250,7 @@
status = "okay";
vmmc-supply = <&ext_3p3v>;
vqmmc-supply = <&pm8921_lvs1>;
+ mmc-pwrseq = <&sdcc4_pwrseq>;
};
};
};
--
1.9.1
Add pwrseq support to sdcc4 which would enable a proper reset of WLAN
without ugly hacks in the board support file.
Signed-off-by: Srinivas Kandagatla <[email protected]>
---
arch/arm/boot/dts/qcom-apq8064-cm-qs600.dts | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/arch/arm/boot/dts/qcom-apq8064-cm-qs600.dts b/arch/arm/boot/dts/qcom-apq8064-cm-qs600.dts
index 34ccb26..3ca1b80 100644
--- a/arch/arm/boot/dts/qcom-apq8064-cm-qs600.dts
+++ b/arch/arm/boot/dts/qcom-apq8064-cm-qs600.dts
@@ -1,4 +1,6 @@
#include "qcom-apq8064-v2.0.dtsi"
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/pinctrl/qcom,pmic-gpio.h>
/ {
model = "CompuLab CM-QS600";
@@ -155,6 +157,29 @@
regulator-always-on;
};
+ qcom,ssbi@500000 {
+ pmicintc: pmic@0 {
+ pm8921_gpio: gpio@150 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&wlan_default_gpios>;
+ wlan_default_gpios: wlan-gpios {
+ pios {
+ pins = "gpio43";
+ function = "normal";
+ bias-disable;
+ power-source = <PM8921_GPIO_S4>;
+ };
+ };
+ };
+ };
+ };
+
+ sdcc4_pwrseq:pwrseq {
+ compatible = "mmc-pwrseq-simple";
+ /* WLAN reset */
+ reset-gpios = <&pm8921_gpio 42 GPIO_ACTIVE_LOW>;
+ };
+
amba {
/* eMMC */
sdcc1: sdcc@12400000 {
@@ -173,6 +198,7 @@
status = "okay";
vmmc-supply = <&v3p3_fixed>;
vqmmc-supply = <&v3p3_fixed>;
+ mmc-pwrseq = <&sdcc4_pwrseq>;
};
};
};
--
1.9.1
This patch adds heart-beat led support on IFC6410.
Signed-off-by: Srinivas Kandagatla <[email protected]>
---
arch/arm/boot/dts/qcom-apq8064-ifc6410.dts | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/arch/arm/boot/dts/qcom-apq8064-ifc6410.dts b/arch/arm/boot/dts/qcom-apq8064-ifc6410.dts
index df560cf..1fc2ea5 100644
--- a/arch/arm/boot/dts/qcom-apq8064-ifc6410.dts
+++ b/arch/arm/boot/dts/qcom-apq8064-ifc6410.dts
@@ -207,6 +207,19 @@
status = "okay";
};
+ leds {
+ compatible = "gpio-leds";
+ pinctrl-names = "default";
+ pinctrl-0 = <¬ify_led>;
+
+ led@1 {
+ label = "apq8064:green:user1";
+ gpios = <&pm8921_gpio 18 GPIO_ACTIVE_HIGH>;
+ linux,default-trigger = "heartbeat";
+ default-state = "on";
+ };
+ };
+
qcom,ssbi@500000 {
pmicintc: pmic@0 {
pm8921_gpio: gpio@150 {
@@ -220,6 +233,15 @@
power-source = <PM8921_GPIO_S4>;
};
};
+
+ notify_led: nled {
+ pios {
+ pins = "gpio18";
+ function = "normal";
+ bias-disable;
+ power-source = <PM8921_GPIO_S4>;
+ };
+ };
};
};
};
--
1.9.1
From: Rob Clark <[email protected]>
This patch adds MDP node to APQ8064 dt.
Signed-off-by: Rob Clark <[email protected]>
[Srinivas Kandagatla] : updated with new style rpm regulators
Signed-off-by: Srinivas Kandagatla <[email protected]>
---
arch/arm/boot/dts/qcom-apq8064.dtsi | 87 +++++++++++++++++++++++++++++++++++++
1 file changed, 87 insertions(+)
diff --git a/arch/arm/boot/dts/qcom-apq8064.dtsi b/arch/arm/boot/dts/qcom-apq8064.dtsi
index cba4ccb..7d2cc45 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";
@@ -618,5 +633,77 @@
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 0>;
+ 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>;
+ 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 0>;
+ 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 0>;
+ 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>;
+ };
};
};
--
1.9.1
This patch enables MDP support on IFC6410.
Signed-off-by: Srinivas Kandagatla <[email protected]>
---
arch/arm/boot/dts/qcom-apq8064-ifc6410.dts | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/arch/arm/boot/dts/qcom-apq8064-ifc6410.dts b/arch/arm/boot/dts/qcom-apq8064-ifc6410.dts
index 1fc2ea5..1ab71f1 100644
--- a/arch/arm/boot/dts/qcom-apq8064-ifc6410.dts
+++ b/arch/arm/boot/dts/qcom-apq8064-ifc6410.dts
@@ -111,6 +111,16 @@
regulator-boot-on;
};
+ hdmi: qcom,hdmi-tx@4a00000 {
+ status = "okay";
+ core-vdda-supply = <&pm8921_hdmi_switch>;
+ hdmi-mux-supply = <&ext_3p3v>;
+ };
+
+ mdp: qcom,mdp@5100000 {
+ status = "okay";
+ };
+
gsbi3: gsbi@16200000 {
status = "okay";
qcom,mode = <GSBI_PROT_I2C>;
--
1.9.1
This patch enables MDP support on QS600 platform.
Signed-off-by: Srinivas Kandagatla <[email protected]>
---
arch/arm/boot/dts/qcom-apq8064-cm-qs600.dts | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/arch/arm/boot/dts/qcom-apq8064-cm-qs600.dts b/arch/arm/boot/dts/qcom-apq8064-cm-qs600.dts
index 3ca1b80..75411ab 100644
--- a/arch/arm/boot/dts/qcom-apq8064-cm-qs600.dts
+++ b/arch/arm/boot/dts/qcom-apq8064-cm-qs600.dts
@@ -83,6 +83,16 @@
};
};
+ hdmi: qcom,hdmi-tx@4a00000 {
+ status = "okay";
+ core-vdda-supply = <&pm8921_hdmi_switch>;
+ hdmi-mux-supply = <&v3p3_fixed>;
+ };
+
+ mdp: qcom,mdp@5100000 {
+ status = "okay";
+ };
+
gsbi@12440000 {
status = "okay";
qcom,mode = <GSBI_PROT_I2C>;
--
1.9.1
This patch adds LVDS panel for IFC6410.
Signed-off-by: Rob Clark <[email protected]>
[Rob Clark: WIP patch]
Signed-off-by: Srinivas Kandagatla <[email protected]>
---
arch/arm/boot/dts/qcom-apq8064-ifc6410.dts | 66 ++++++++++++++++++++++++++++++
1 file changed, 66 insertions(+)
diff --git a/arch/arm/boot/dts/qcom-apq8064-ifc6410.dts b/arch/arm/boot/dts/qcom-apq8064-ifc6410.dts
index 1ab71f1..3bdac02 100644
--- a/arch/arm/boot/dts/qcom-apq8064-ifc6410.dts
+++ b/arch/arm/boot/dts/qcom-apq8064-ifc6410.dts
@@ -63,6 +63,12 @@
qcom,switch-mode-frequency = <3200000>;
};
+ pm8921_l2: l2 {
+ regulator-min-microvolt = <1200000>;
+ regulator-max-microvolt = <1200000>;
+ bias-pull-down;
+ };
+
pm8921_l3: l3 {
regulator-min-microvolt = <3050000>;
regulator-max-microvolt = <3300000>;
@@ -96,6 +102,10 @@
pm8921_lvs1: lvs1 {
bias-pull-down;
};
+
+ pm8921_lvs7: lvs7 {
+ bias-pull-down;
+ };
};
};
@@ -119,6 +129,41 @@
mdp: qcom,mdp@5100000 {
status = "okay";
+ qcom,lvds-panel = <&panel>;
+ lvds-vccs-3p3v-supply = <&ext_3p3v>;
+ lvds-pll-vdda-supply = <&pm8921_l2>;
+ lvds-vdda-supply = <&pm8921_lvs7>;
+ };
+
+ panel_3p3v: panel_3p3v {
+ compatible = "regulator-fixed";
+ pinctrl-0 = <&disp_en_gpios>;
+ pinctrl-names = "default";
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ regulator-name = "panel_en_3p3v";
+ regulator-type = "voltage";
+ startup-delay-us = <0>;
+ gpio = <&pm8921_gpio 36 GPIO_ACTIVE_HIGH>;
+ enable-active-high;
+ regulator-boot-on;
+ };
+
+ backlight: backlight{
+ pinctrl-0 = <&pwm_bl_gpios>;
+ pinctrl-names = "default";
+ compatible = "gpio-backlight";
+ gpios = <&pm8921_gpio 26 GPIO_ACTIVE_HIGH>;
+ default-on;
+ };
+
+ panel: auo,b101xtn01 {
+ status = "okay";
+ compatible = "auo,b101xtn01";
+
+ ddc-i2c-bus = <&i2c3>;
+ backlight = <&backlight>;
+ power-supply = <&panel_3p3v>;
};
gsbi3: gsbi@16200000 {
@@ -235,6 +280,27 @@
pm8921_gpio: gpio@150 {
pinctrl-names = "default";
pinctrl-0 = <&wlan_default_gpios>;
+
+ pwm_bl_gpios: pwm-bl-gpios {
+ pios {
+ pins = "gpio26";
+ bias-disable;
+ function = "normal";
+ qcom,drive-strength = <PMIC_GPIO_STRENGTH_HIGH>;
+ power-source = <PM8921_GPIO_S4>;
+ };
+ };
+
+ disp_en_gpios: disp-en-gpios {
+ pios {
+ pins = "gpio36";
+ bias-disable;
+ function = "normal";
+ qcom,drive-strength = <PMIC_GPIO_STRENGTH_HIGH>;
+ power-source = <PM8921_GPIO_S4>;
+ };
+ };
+
wlan_default_gpios: wlan-gpios {
pios {
pins = "gpio43";
--
1.9.1
Hi,
Am 28.07.2015 um 14:54 schrieb Srinivas Kandagatla:
> From: Rob Clark <[email protected]>
>
> This patch adds MDP node to APQ8064 dt.
>
> Signed-off-by: Rob Clark <[email protected]>
> [Srinivas Kandagatla] : updated with new style rpm regulators
> Signed-off-by: Srinivas Kandagatla <[email protected]>
> ---
> arch/arm/boot/dts/qcom-apq8064.dtsi | 87 +++++++++++++++++++++++++++++++++++++
> 1 file changed, 87 insertions(+)
>
> diff --git a/arch/arm/boot/dts/qcom-apq8064.dtsi b/arch/arm/boot/dts/qcom-apq8064.dtsi
> index cba4ccb..7d2cc45 100644
> --- a/arch/arm/boot/dts/qcom-apq8064.dtsi
> +++ b/arch/arm/boot/dts/qcom-apq8064.dtsi
[...]
> + gpu: qcom,adreno-3xx@4300000 {
> + compatible = "qcom,adreno-3xx";
I thought that wildcards were forbidden in compatible strings? Then this
should be replaced by the real number, with a fallback to the first
compatible one.
And can't we just name the node qcom,adreno without version suffix?
Regards,
Andreas
> + reg = <0x04300000 0x20000>;
> + reg-names = "kgsl_3d0_reg_memory";
> + interrupts = <GIC_SPI 80 0>;
> + 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 0>;
> + 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>;
> + };
> };
> };
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 N?rnberg, Germany
GF: Felix Imend?rffer, Jane Smithard, Dilip Upmanyu, Graham Norton; HRB
21284 (AG N?rnberg)
Am 28.07.2015 um 14:54 schrieb Srinivas Kandagatla:
> This patch adds heart-beat led support on IFC6410.
>
> Signed-off-by: Srinivas Kandagatla <[email protected]>
> ---
> arch/arm/boot/dts/qcom-apq8064-ifc6410.dts | 22 ++++++++++++++++++++++
> 1 file changed, 22 insertions(+)
>
> diff --git a/arch/arm/boot/dts/qcom-apq8064-ifc6410.dts b/arch/arm/boot/dts/qcom-apq8064-ifc6410.dts
> index df560cf..1fc2ea5 100644
> --- a/arch/arm/boot/dts/qcom-apq8064-ifc6410.dts
> +++ b/arch/arm/boot/dts/qcom-apq8064-ifc6410.dts
> @@ -207,6 +207,19 @@
> status = "okay";
> };
>
> + leds {
> + compatible = "gpio-leds";
> + pinctrl-names = "default";
> + pinctrl-0 = <¬ify_led>;
> +
> + led@1 {
> + label = "apq8064:green:user1";
> + gpios = <&pm8921_gpio 18 GPIO_ACTIVE_HIGH>;
> + linux,default-trigger = "heartbeat";
Can we please drop the heartbeat default? While that's nice as proof of
concept, it's annoying to deliver it that way in Linux distros. Users
can enable that trigger with a simple boot script writing to /sys.
Thanks,
Andreas
> + default-state = "on";
> + };
> + };
> +
> qcom,ssbi@500000 {
> pmicintc: pmic@0 {
> pm8921_gpio: gpio@150 {
[snip]
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 N?rnberg, Germany
GF: Felix Imend?rffer, Jane Smithard, Dilip Upmanyu, Graham Norton; HRB
21284 (AG N?rnberg)
Am 28.07.2015 um 14:54 schrieb Srinivas Kandagatla:
> This patch adds LVDS panel for IFC6410.
>
> Signed-off-by: Rob Clark <[email protected]>
> [Rob Clark: WIP patch]
> Signed-off-by: Srinivas Kandagatla <[email protected]>
> ---
> arch/arm/boot/dts/qcom-apq8064-ifc6410.dts | 66 ++++++++++++++++++++++++++++++
> 1 file changed, 66 insertions(+)
>
> diff --git a/arch/arm/boot/dts/qcom-apq8064-ifc6410.dts b/arch/arm/boot/dts/qcom-apq8064-ifc6410.dts
> index 1ab71f1..3bdac02 100644
> --- a/arch/arm/boot/dts/qcom-apq8064-ifc6410.dts
> +++ b/arch/arm/boot/dts/qcom-apq8064-ifc6410.dts
> @@ -63,6 +63,12 @@
> qcom,switch-mode-frequency = <3200000>;
> };
>
> + pm8921_l2: l2 {
> + regulator-min-microvolt = <1200000>;
> + regulator-max-microvolt = <1200000>;
> + bias-pull-down;
> + };
> +
> pm8921_l3: l3 {
> regulator-min-microvolt = <3050000>;
> regulator-max-microvolt = <3300000>;
> @@ -96,6 +102,10 @@
> pm8921_lvs1: lvs1 {
> bias-pull-down;
> };
> +
> + pm8921_lvs7: lvs7 {
> + bias-pull-down;
> + };
> };
> };
>
> @@ -119,6 +129,41 @@
>
> mdp: qcom,mdp@5100000 {
> status = "okay";
> + qcom,lvds-panel = <&panel>;
In my testing this broke/uglified HDMI output. Since not everyone has
that LVDS panel attached, or might have a different panel type, I don't
think the panel node belongs in the generic -ifc6410.dts file.
I suggest you add an -ifc6410+inforce-lvds.dts or so for that.
> + lvds-vccs-3p3v-supply = <&ext_3p3v>;
> + lvds-pll-vdda-supply = <&pm8921_l2>;
> + lvds-vdda-supply = <&pm8921_lvs7>;
> + };
> +
> + panel_3p3v: panel_3p3v {
s/_/-/ ?
> + compatible = "regulator-fixed";
> + pinctrl-0 = <&disp_en_gpios>;
> + pinctrl-names = "default";
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + regulator-name = "panel_en_3p3v";
> + regulator-type = "voltage";
> + startup-delay-us = <0>;
> + gpio = <&pm8921_gpio 36 GPIO_ACTIVE_HIGH>;
> + enable-active-high;
> + regulator-boot-on;
> + };
> +
> + backlight: backlight{
> + pinctrl-0 = <&pwm_bl_gpios>;
> + pinctrl-names = "default";
> + compatible = "gpio-backlight";
> + gpios = <&pm8921_gpio 26 GPIO_ACTIVE_HIGH>;
> + default-on;
> + };
> +
> + panel: auo,b101xtn01 {
panel: panel { ?
Regards,
Andreas
> + status = "okay";
> + compatible = "auo,b101xtn01";
> +
> + ddc-i2c-bus = <&i2c3>;
> + backlight = <&backlight>;
> + power-supply = <&panel_3p3v>;
> };
>
> gsbi3: gsbi@16200000 {
> @@ -235,6 +280,27 @@
> pm8921_gpio: gpio@150 {
> pinctrl-names = "default";
> pinctrl-0 = <&wlan_default_gpios>;
> +
> + pwm_bl_gpios: pwm-bl-gpios {
> + pios {
> + pins = "gpio26";
> + bias-disable;
> + function = "normal";
> + qcom,drive-strength = <PMIC_GPIO_STRENGTH_HIGH>;
> + power-source = <PM8921_GPIO_S4>;
> + };
> + };
> +
> + disp_en_gpios: disp-en-gpios {
> + pios {
> + pins = "gpio36";
> + bias-disable;
> + function = "normal";
> + qcom,drive-strength = <PMIC_GPIO_STRENGTH_HIGH>;
> + power-source = <PM8921_GPIO_S4>;
> + };
> + };
> +
> wlan_default_gpios: wlan-gpios {
> pios {
> pins = "gpio43";
>
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 N?rnberg, Germany
GF: Felix Imend?rffer, Jane Smithard, Dilip Upmanyu, Graham Norton; HRB
21284 (AG N?rnberg)
On Tue, Jul 28, 2015 at 1:31 PM, Andreas Färber <[email protected]> wrote:
> Hi,
>
> Am 28.07.2015 um 14:54 schrieb Srinivas Kandagatla:
>> From: Rob Clark <[email protected]>
>>
>> This patch adds MDP node to APQ8064 dt.
>>
>> Signed-off-by: Rob Clark <[email protected]>
>> [Srinivas Kandagatla] : updated with new style rpm regulators
>> Signed-off-by: Srinivas Kandagatla <[email protected]>
>> ---
>> arch/arm/boot/dts/qcom-apq8064.dtsi | 87 +++++++++++++++++++++++++++++++++++++
>> 1 file changed, 87 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/qcom-apq8064.dtsi b/arch/arm/boot/dts/qcom-apq8064.dtsi
>> index cba4ccb..7d2cc45 100644
>> --- a/arch/arm/boot/dts/qcom-apq8064.dtsi
>> +++ b/arch/arm/boot/dts/qcom-apq8064.dtsi
> [...]
>> + gpu: qcom,adreno-3xx@4300000 {
>> + compatible = "qcom,adreno-3xx";
>
> I thought that wildcards were forbidden in compatible strings? Then this
> should be replaced by the real number, with a fallback to the first
> compatible one.
That would at least result in a big number of different compatibles,
esp. when you consider patchlevels of the different gpu's, which in
some cases needs to be known (esp. to userspace).. ie. a320.0 is not
the same thing as a320.2. And the "real numbers" themselves are
confusing as a result of meddling by marketeers.. (ie. a305 vs a305b
vs a306.. which actually map to 305.x, 306.x and 307.x). The current
scheme is easy to figure out how to setup the dt nodes, ie. easy to
know a3xx vs a4xx, and then copy the chipid property from downstream
dt, and everything works.
The current scheme groups by major gpu revision (ie. things where
basically all the registers change, ie. a3xx/a4xx/a5xx) with using
chipid and a few if statements here and there (kernel and userspace)
to deal with the intra-generation differences. (The chipid is
something which in theory should be read out of a version register,
but seems that cannot be trusted.)
Not to mention that a3xx/a4xx/etc is how userspace code in mesa is partitioned.
BR,
-R
> And can't we just name the node qcom,adreno without version suffix?
>
> Regards,
> Andreas
>
>> + reg = <0x04300000 0x20000>;
>> + reg-names = "kgsl_3d0_reg_memory";
>> + interrupts = <GIC_SPI 80 0>;
>> + 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 0>;
>> + 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>;
>> + };
>> };
>> };
>
> --
> SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Felix Imendörffer, Jane Smithard, Dilip Upmanyu, Graham Norton; HRB
> 21284 (AG Nürnberg)
On Tue, Jul 28, 2015 at 1:50 PM, Andreas Färber <[email protected]> wrote:
> Am 28.07.2015 um 14:54 schrieb Srinivas Kandagatla:
>> This patch adds LVDS panel for IFC6410.
>>
>> Signed-off-by: Rob Clark <[email protected]>
>> [Rob Clark: WIP patch]
>> Signed-off-by: Srinivas Kandagatla <[email protected]>
>> ---
>> arch/arm/boot/dts/qcom-apq8064-ifc6410.dts | 66 ++++++++++++++++++++++++++++++
>> 1 file changed, 66 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/qcom-apq8064-ifc6410.dts b/arch/arm/boot/dts/qcom-apq8064-ifc6410.dts
>> index 1ab71f1..3bdac02 100644
>> --- a/arch/arm/boot/dts/qcom-apq8064-ifc6410.dts
>> +++ b/arch/arm/boot/dts/qcom-apq8064-ifc6410.dts
>> @@ -63,6 +63,12 @@
>> qcom,switch-mode-frequency = <3200000>;
>> };
>>
>> + pm8921_l2: l2 {
>> + regulator-min-microvolt = <1200000>;
>> + regulator-max-microvolt = <1200000>;
>> + bias-pull-down;
>> + };
>> +
>> pm8921_l3: l3 {
>> regulator-min-microvolt = <3050000>;
>> regulator-max-microvolt = <3300000>;
>> @@ -96,6 +102,10 @@
>> pm8921_lvs1: lvs1 {
>> bias-pull-down;
>> };
>> +
>> + pm8921_lvs7: lvs7 {
>> + bias-pull-down;
>> + };
>> };
>> };
>>
>> @@ -119,6 +129,41 @@
>>
>> mdp: qcom,mdp@5100000 {
>> status = "okay";
>> + qcom,lvds-panel = <&panel>;
>
> In my testing this broke/uglified HDMI output. Since not everyone has
> that LVDS panel attached, or might have a different panel type, I don't
> think the panel node belongs in the generic -ifc6410.dts file.
> I suggest you add an -ifc6410+inforce-lvds.dts or so for that.
I think the problem is not so much the dt bindings, since we have an
i2c/ddc where we can probe the panel's edid and see if it is actually
attached.
Probably panel_simple_probe() should attempt to read edid (or at least
first block) if ddc is not null, and return an error if that times
out. That way we could tell at runtime whether the panel is there or
not.
BR,
-R
>> + lvds-vccs-3p3v-supply = <&ext_3p3v>;
>> + lvds-pll-vdda-supply = <&pm8921_l2>;
>> + lvds-vdda-supply = <&pm8921_lvs7>;
>> + };
>> +
>> + panel_3p3v: panel_3p3v {
>
> s/_/-/ ?
>
>> + compatible = "regulator-fixed";
>> + pinctrl-0 = <&disp_en_gpios>;
>> + pinctrl-names = "default";
>> + regulator-min-microvolt = <3300000>;
>> + regulator-max-microvolt = <3300000>;
>> + regulator-name = "panel_en_3p3v";
>> + regulator-type = "voltage";
>> + startup-delay-us = <0>;
>> + gpio = <&pm8921_gpio 36 GPIO_ACTIVE_HIGH>;
>> + enable-active-high;
>> + regulator-boot-on;
>> + };
>> +
>> + backlight: backlight{
>> + pinctrl-0 = <&pwm_bl_gpios>;
>> + pinctrl-names = "default";
>> + compatible = "gpio-backlight";
>> + gpios = <&pm8921_gpio 26 GPIO_ACTIVE_HIGH>;
>> + default-on;
>> + };
>> +
>> + panel: auo,b101xtn01 {
>
> panel: panel { ?
>
> Regards,
> Andreas
>
>> + status = "okay";
>> + compatible = "auo,b101xtn01";
>> +
>> + ddc-i2c-bus = <&i2c3>;
>> + backlight = <&backlight>;
>> + power-supply = <&panel_3p3v>;
>> };
>>
>> gsbi3: gsbi@16200000 {
>> @@ -235,6 +280,27 @@
>> pm8921_gpio: gpio@150 {
>> pinctrl-names = "default";
>> pinctrl-0 = <&wlan_default_gpios>;
>> +
>> + pwm_bl_gpios: pwm-bl-gpios {
>> + pios {
>> + pins = "gpio26";
>> + bias-disable;
>> + function = "normal";
>> + qcom,drive-strength = <PMIC_GPIO_STRENGTH_HIGH>;
>> + power-source = <PM8921_GPIO_S4>;
>> + };
>> + };
>> +
>> + disp_en_gpios: disp-en-gpios {
>> + pios {
>> + pins = "gpio36";
>> + bias-disable;
>> + function = "normal";
>> + qcom,drive-strength = <PMIC_GPIO_STRENGTH_HIGH>;
>> + power-source = <PM8921_GPIO_S4>;
>> + };
>> + };
>> +
>> wlan_default_gpios: wlan-gpios {
>> pios {
>> pins = "gpio43";
>>
>
>
> --
> SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Felix Imendörffer, Jane Smithard, Dilip Upmanyu, Graham Norton; HRB
> 21284 (AG Nürnberg)
On Tue, Jul 28, 2015 at 01:52:36PM +0100, Srinivas Kandagatla wrote:
> Hi Andy,
>
> As discussed here are some dt patches which depend on
> pmic header "dt-bindings/pinctrl/qcom,pmic-gpio.h" which is availble in
> linux-next.
Ok. I took all of these and put them on top of my 4.3 tags on the
qcom/for-next. I compiled against the current next master and all was well.
<snip>
--
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Hi Srini,
On 07/28/2015 06:24 PM, Srinivas Kandagatla wrote:
> This patch adds LVDS panel for IFC6410.
>
> Signed-off-by: Rob Clark <[email protected]>
> [Rob Clark: WIP patch]
> Signed-off-by: Srinivas Kandagatla <[email protected]>
> ---
> arch/arm/boot/dts/qcom-apq8064-ifc6410.dts | 66 ++++++++++++++++++++++++++++++
> 1 file changed, 66 insertions(+)
>
> diff --git a/arch/arm/boot/dts/qcom-apq8064-ifc6410.dts b/arch/arm/boot/dts/qcom-apq8064-ifc6410.dts
> index 1ab71f1..3bdac02 100644
> --- a/arch/arm/boot/dts/qcom-apq8064-ifc6410.dts
> +++ b/arch/arm/boot/dts/qcom-apq8064-ifc6410.dts
> @@ -63,6 +63,12 @@
> qcom,switch-mode-frequency = <3200000>;
> };
>
> + pm8921_l2: l2 {
> + regulator-min-microvolt = <1200000>;
> + regulator-max-microvolt = <1200000>;
> + bias-pull-down;
> + };
> +
> pm8921_l3: l3 {
> regulator-min-microvolt = <3050000>;
> regulator-max-microvolt = <3300000>;
> @@ -96,6 +102,10 @@
> pm8921_lvs1: lvs1 {
> bias-pull-down;
> };
> +
> + pm8921_lvs7: lvs7 {
> + bias-pull-down;
> + };
> };
> };
>
> @@ -119,6 +129,41 @@
>
> mdp: qcom,mdp@5100000 {
> status = "okay";
> + qcom,lvds-panel = <&panel>;
We're trying to switch to the of_graph way of representing connected
panels. With that, the above phandle will go away. This needs to be
replaced with:
port {
lvds_out: endpoint {
remote_endpoint = <&auo_in>;
};
};
> + lvds-vccs-3p3v-supply = <&ext_3p3v>;
> + lvds-pll-vdda-supply = <&pm8921_l2>;
> + lvds-vdda-supply = <&pm8921_lvs7>;
> + };
> +
> + panel_3p3v: panel_3p3v {
> + compatible = "regulator-fixed";
> + pinctrl-0 = <&disp_en_gpios>;
> + pinctrl-names = "default";
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + regulator-name = "panel_en_3p3v";
> + regulator-type = "voltage";
> + startup-delay-us = <0>;
> + gpio = <&pm8921_gpio 36 GPIO_ACTIVE_HIGH>;
> + enable-active-high;
> + regulator-boot-on;
> + };
> +
> + backlight: backlight{
> + pinctrl-0 = <&pwm_bl_gpios>;
> + pinctrl-names = "default";
> + compatible = "gpio-backlight";
> + gpios = <&pm8921_gpio 26 GPIO_ACTIVE_HIGH>;
> + default-on;
> + };
> +
> + panel: auo,b101xtn01 {
> + status = "okay";
> + compatible = "auo,b101xtn01";
> +
> + ddc-i2c-bus = <&i2c3>;
> + backlight = <&backlight>;
> + power-supply = <&panel_3p3v>;
and for the panel:
port {
auo_in: endpoint {
remote-endpoint = <&lvds_out>;
};
};
Thanks,
Archit
> };
>
> gsbi3: gsbi@16200000 {
> @@ -235,6 +280,27 @@
> pm8921_gpio: gpio@150 {
> pinctrl-names = "default";
> pinctrl-0 = <&wlan_default_gpios>;
> +
> + pwm_bl_gpios: pwm-bl-gpios {
> + pios {
> + pins = "gpio26";
> + bias-disable;
> + function = "normal";
> + qcom,drive-strength = <PMIC_GPIO_STRENGTH_HIGH>;
> + power-source = <PM8921_GPIO_S4>;
> + };
> + };
> +
> + disp_en_gpios: disp-en-gpios {
> + pios {
> + pins = "gpio36";
> + bias-disable;
> + function = "normal";
> + qcom,drive-strength = <PMIC_GPIO_STRENGTH_HIGH>;
> + power-source = <PM8921_GPIO_S4>;
> + };
> + };
> +
> wlan_default_gpios: wlan-gpios {
> pios {
> pins = "gpio43";
>
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
On 28/07/15 18:36, Andreas F?rber wrote:
> Am 28.07.2015 um 14:54 schrieb Srinivas Kandagatla:
>> This patch adds heart-beat led support on IFC6410.
>>
>> Signed-off-by: Srinivas Kandagatla <[email protected]>
>> ---
>> arch/arm/boot/dts/qcom-apq8064-ifc6410.dts | 22 ++++++++++++++++++++++
>> 1 file changed, 22 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/qcom-apq8064-ifc6410.dts b/arch/arm/boot/dts/qcom-apq8064-ifc6410.dts
>> index df560cf..1fc2ea5 100644
>> --- a/arch/arm/boot/dts/qcom-apq8064-ifc6410.dts
>> +++ b/arch/arm/boot/dts/qcom-apq8064-ifc6410.dts
>> @@ -207,6 +207,19 @@
>> status = "okay";
>> };
>>
>> + leds {
>> + compatible = "gpio-leds";
>> + pinctrl-names = "default";
>> + pinctrl-0 = <¬ify_led>;
>> +
>> + led@1 {
>> + label = "apq8064:green:user1";
>> + gpios = <&pm8921_gpio 18 GPIO_ACTIVE_HIGH>;
>> + linux,default-trigger = "heartbeat";
>
> Can we please drop the heartbeat default? While that's nice as proof of
> concept, it's annoying to deliver it that way in Linux distros. Users
> can enable that trigger with a simple boot script writing to /sys.
>
Thanks for the input, I will change to be none, Let the users decide the
trigger.
thanks,
srini
> Thanks,
> Andreas
>
>> + default-state = "on";
>> + };
>> + };
>> +
>> qcom,ssbi@500000 {
>> pmicintc: pmic@0 {
>> pm8921_gpio: gpio@150 {
> [snip]
>
On 29/07/15 06:39, Archit Taneja wrote:
> Hi Srini,
>
> On 07/28/2015 06:24 PM, Srinivas Kandagatla wrote:
>> This patch adds LVDS panel for IFC6410.
>>
>> Signed-off-by: Rob Clark <[email protected]>
>> [Rob Clark: WIP patch]
>> Signed-off-by: Srinivas Kandagatla <[email protected]>
>> ---
>> arch/arm/boot/dts/qcom-apq8064-ifc6410.dts | 66
>> ++++++++++++++++++++++++++++++
>> 1 file changed, 66 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/qcom-apq8064-ifc6410.dts
>> b/arch/arm/boot/dts/qcom-apq8064-ifc6410.dts
>> index 1ab71f1..3bdac02 100644
>> --- a/arch/arm/boot/dts/qcom-apq8064-ifc6410.dts
>> +++ b/arch/arm/boot/dts/qcom-apq8064-ifc6410.dts
>> @@ -63,6 +63,12 @@
>> qcom,switch-mode-frequency = <3200000>;
>> };
>>
>> + pm8921_l2: l2 {
>> + regulator-min-microvolt = <1200000>;
>> + regulator-max-microvolt = <1200000>;
>> + bias-pull-down;
>> + };
>> +
>> pm8921_l3: l3 {
>> regulator-min-microvolt = <3050000>;
>> regulator-max-microvolt = <3300000>;
>> @@ -96,6 +102,10 @@
>> pm8921_lvs1: lvs1 {
>> bias-pull-down;
>> };
>> +
>> + pm8921_lvs7: lvs7 {
>> + bias-pull-down;
>> + };
>> };
>> };
>>
>> @@ -119,6 +129,41 @@
>>
>> mdp: qcom,mdp@5100000 {
>> status = "okay";
>> + qcom,lvds-panel = <&panel>;
>
> We're trying to switch to the of_graph way of representing connected
> panels. With that, the above phandle will go away. This needs to be
> replaced with:
>
Thanks for the input, I modified the panel support to use graphs which
works perfectly fine.
thanks,
srini
> port {
> lvds_out: endpoint {
> remote_endpoint = <&auo_in>;
> };
> };
>
>> + lvds-vccs-3p3v-supply = <&ext_3p3v>;
>> + lvds-pll-vdda-supply = <&pm8921_l2>;
>> + lvds-vdda-supply = <&pm8921_lvs7>;
>> + };
>> +
>> + panel_3p3v: panel_3p3v {
>> + compatible = "regulator-fixed";
>> + pinctrl-0 = <&disp_en_gpios>;
>> + pinctrl-names = "default";
>> + regulator-min-microvolt = <3300000>;
>> + regulator-max-microvolt = <3300000>;
>> + regulator-name = "panel_en_3p3v";
>> + regulator-type = "voltage";
>> + startup-delay-us = <0>;
>> + gpio = <&pm8921_gpio 36 GPIO_ACTIVE_HIGH>;
>> + enable-active-high;
>> + regulator-boot-on;
>> + };
>> +
>> + backlight: backlight{
>> + pinctrl-0 = <&pwm_bl_gpios>;
>> + pinctrl-names = "default";
>> + compatible = "gpio-backlight";
>> + gpios = <&pm8921_gpio 26 GPIO_ACTIVE_HIGH>;
>> + default-on;
>> + };
>> +
>> + panel: auo,b101xtn01 {
>> + status = "okay";
>> + compatible = "auo,b101xtn01";
>> +
>> + ddc-i2c-bus = <&i2c3>;
>> + backlight = <&backlight>;
>> + power-supply = <&panel_3p3v>;
>
> and for the panel:
>
> port {
> auo_in: endpoint {
> remote-endpoint = <&lvds_out>;
> };
> };
>
> Thanks,
> Archit
>
>> };
>>
>> gsbi3: gsbi@16200000 {
>> @@ -235,6 +280,27 @@
>> pm8921_gpio: gpio@150 {
>> pinctrl-names = "default";
>> pinctrl-0 = <&wlan_default_gpios>;
>> +
>> + pwm_bl_gpios: pwm-bl-gpios {
>> + pios {
>> + pins = "gpio26";
>> + bias-disable;
>> + function = "normal";
>> + qcom,drive-strength =
>> <PMIC_GPIO_STRENGTH_HIGH>;
>> + power-source = <PM8921_GPIO_S4>;
>> + };
>> + };
>> +
>> + disp_en_gpios: disp-en-gpios {
>> + pios {
>> + pins = "gpio36";
>> + bias-disable;
>> + function = "normal";
>> + qcom,drive-strength =
>> <PMIC_GPIO_STRENGTH_HIGH>;
>> + power-source = <PM8921_GPIO_S4>;
>> + };
>> + };
>> +
>> wlan_default_gpios: wlan-gpios {
>> pios {
>> pins = "gpio43";
>>
>
On 07/28/2015 05:54 AM, Srinivas Kandagatla wrote:
> +
> + panel_3p3v: panel_3p3v {
> + compatible = "regulator-fixed";
> + pinctrl-0 = <&disp_en_gpios>;
> + pinctrl-names = "default";
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + regulator-name = "panel_en_3p3v";
> + regulator-type = "voltage";
> + startup-delay-us = <0>;
> + gpio = <&pm8921_gpio 36 GPIO_ACTIVE_HIGH>;
> + enable-active-high;
> + regulator-boot-on;
> + };
We should put gpio regulators into their own container in the root of
the tree. Similar to what was done for 8960 gpio regulators.
> +
> + backlight: backlight{
> + pinctrl-0 = <&pwm_bl_gpios>;
> + pinctrl-names = "default";
> + compatible = "gpio-backlight";
> + gpios = <&pm8921_gpio 26 GPIO_ACTIVE_HIGH>;
> + default-on;
> + };
> +
> + panel: auo,b101xtn01 {
> + status = "okay";
> + compatible = "auo,b101xtn01";
> +
> + ddc-i2c-bus = <&i2c3>;
> + backlight = <&backlight>;
> + power-supply = <&panel_3p3v>;
> };
These two nodes shouldn't be under the SoC node. They don't have
registers so they should be at the root of the tree. And we don't need
to put labels twice on nodes. If we're modifying things in board
specific dtsi files it should be fine to leave the label off if the
label is in the SoC dtsi file.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
On 07/28/2015 05:53 AM, Srinivas Kandagatla wrote:
> +
> + sdcc4_pwrseq:pwrseq {
Missing space here between label and node name.
> + compatible = "mmc-pwrseq-simple";
> + reset-gpios = <&pm8921_gpio 43 GPIO_ACTIVE_LOW>;
> + };
> +
Also this node is not part of the SoC so it should be in the root,
possibly under some sort of power-sequences node so that all the
sequences are grouped in one place.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
On Tue 28 Jul 05:53 PDT 2015, Srinivas Kandagatla wrote:
> This patch adds pwrseq for WLAN which resets the WLAN just before the
> SDIO bus is up.
>
Does this mean that we have the !power-of-2 patch in mmc on the way?
> Signed-off-by: Srinivas Kandagatla <[email protected]>
> ---
> arch/arm/boot/dts/qcom-apq8064-ifc6410.dts | 24 ++++++++++++++++++++++++
> 1 file changed, 24 insertions(+)
>
> diff --git a/arch/arm/boot/dts/qcom-apq8064-ifc6410.dts b/arch/arm/boot/dts/qcom-apq8064-ifc6410.dts
> index 88d6655..df560cf 100644
> --- a/arch/arm/boot/dts/qcom-apq8064-ifc6410.dts
> +++ b/arch/arm/boot/dts/qcom-apq8064-ifc6410.dts
> @@ -1,5 +1,6 @@
> #include "qcom-apq8064-v2.0.dtsi"
> #include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/pinctrl/qcom,pmic-gpio.h>
>
> / {
> model = "Qualcomm APQ8064/IFC6410";
> @@ -206,6 +207,28 @@
> status = "okay";
> };
>
> + qcom,ssbi@500000 {
> + pmicintc: pmic@0 {
> + pm8921_gpio: gpio@150 {
These should already be labeled in the apq8064 dtsi (or in a
qcom-pm8921.dtsi).
> + pinctrl-names = "default";
> + pinctrl-0 = <&wlan_default_gpios>;
Please move these to the pwrseq node, as that's the consumer of this.
> + wlan_default_gpios: wlan-gpios {
> + pios {
> + pins = "gpio43";
> + function = "normal";
> + bias-disable;
> + power-source = <PM8921_GPIO_S4>;
> + };
> + };
> + };
> + };
> + };
Regards,
Bjorn
On 07/28/2015 05:54 AM, Srinivas Kandagatla wrote:
> @@ -618,5 +633,77 @@
> 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 0>;
> + 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>;
These should use standard "-gpios" syntax. Please fix the binding/driver.
> + 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 0>;
> + interrupt-names = "kgsl_3d0_irq";
> + clock-names =
> + "core_clk",
> + "iface_clk",
> + "mem_clk",
> + "mem_iface_clk";
Please drop "_clk" and fix driver code as we've done for all other drivers.
> + 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>;
> + };
> + };
These should be OPPs. I see in the cover-letter we're asking to ignore
this part of the binding. When is the binding and driver going to be
fixed? The dts has been out of tree for quite some time and I assume
work has been progressing on the driver even without this dts being in
mainline so it doesn't make sense how pushing half-baked dts upstream is
going to help. In fact, it's going to make things worse if someone
complains that we're not maintaining backwards compatibility and then
have to hack up driver code to work both ways. At which point there's
practically zero incentive to change anything in the binding or driver.
It's better to fix the driver now and avoid these sorts of problems
entirely.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
On Wed, Aug 5, 2015 at 9:27 PM, Stephen Boyd <[email protected]> wrote:
> On 07/28/2015 05:54 AM, Srinivas Kandagatla wrote:
>>
>> @@ -618,5 +633,77 @@
>> 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 0>;
>> + 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>;
>
>
> These should use standard "-gpios" syntax. Please fix the binding/driver.
With this, and a couple of the other bindings, I'd chosen to follow to
follow the downstream bindings simply because I still need to use
downstream kernel on a handful of devices (for example, I have no a4xx
device which runs anything close to an upstream kernel, yet still
quite a lot of work to do in userspace/mesa there).. this might be a
scenario that other kernel dev's in other subsystems aren't really
familiar with, but when it comes to gpu's, really only 10% or less of
the driver is in the kernel. There is a massive sh**-ton of
complexity in terms of cmdstream building in userspace.
But if the change to do things upstream is not too intrusive I'd be
willing to carry the 'revert' patch as a downstream patch on top of
upstream.. or if we could reasonably support both bindings, that is
also an option. Mostly I just haven't had time to look into it, since
unlike other drivers, when it comes to gpu stuff, the kernel is only
the tip of the (inverted) iceberg..
>> + 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 0>;
>> + interrupt-names = "kgsl_3d0_irq";
>> + clock-names =
>> + "core_clk",
>> + "iface_clk",
>> + "mem_clk",
>> + "mem_iface_clk";
>
>
> Please drop "_clk" and fix driver code as we've done for all other drivers.
>
>> + 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>;
>> + };
>> + };
>
>
> These should be OPPs. I see in the cover-letter we're asking to ignore this
> part of the binding. When is the binding and driver going to be fixed? The
> dts has been out of tree for quite some time and I assume work has been
> progressing on the driver even without this dts being in mainline so it
> doesn't make sense how pushing half-baked dts upstream is going to help. In
> fact, it's going to make things worse if someone complains that we're not
> maintaining backwards compatibility and then have to hack up driver code to
> work both ways. At which point there's practically zero incentive to change
> anything in the binding or driver. It's better to fix the driver now and
> avoid these sorts of problems entirely.
again, it is a bit awkward thanks to having to deal with downstream
kernels while upstream catches up..
It does seem interesting to move to cpufreq for managing the gpu
speed, and maybe doing something more clever than all-or-nothing freq
scaling.. but yet again something where I really haven't had time to
investigate beyond "yeah, that seems like a good idea"..
It would be *really* nice if we had some mechanism to mark bindings
that we are not ready yet to call ABI, since on one hand we have an
upstream gpu driver and corresponding userspace to actually have some
interesting some interesting (ie. not just buildroot and serial
console) stuff running on an upstream kernel on actual devices. And
that is a nice situation that I wouldn't have even thought possible a
few years ago. But otoh, I agree that we shouldn't call these
bindings final abi.
Anyways, patches welcome.. bonus points if they are not too intrusive
to revert (for the downstream kernels I have to deal with) or if they
continue to support the downstream bindings (for now, until I don't
have to care about downstream kernels)..
BR,
-R
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
On 29/07/15 23:55, Stephen Boyd wrote:
> On 07/28/2015 05:53 AM, Srinivas Kandagatla wrote:
>> +
>> + sdcc4_pwrseq:pwrseq {
>
> Missing space here between label and node name.
>
I will fix this in next version.
>> + compatible = "mmc-pwrseq-simple";
>> + reset-gpios = <&pm8921_gpio 43 GPIO_ACTIVE_LOW>;
>> + };
>> +
>
> Also this node is not part of the SoC so it should be in the root,
> possibly under some sort of power-sequences node so that all the
> sequences are grouped in one place.
Good spot and suggestion. I will do that in next version.
--srini
>
On 29/07/15 23:46, Stephen Boyd wrote:
> On 07/28/2015 05:54 AM, Srinivas Kandagatla wrote:
>> +
>> + panel_3p3v: panel_3p3v {
>> + compatible = "regulator-fixed";
>> + pinctrl-0 = <&disp_en_gpios>;
>> + pinctrl-names = "default";
>> + regulator-min-microvolt = <3300000>;
>> + regulator-max-microvolt = <3300000>;
>> + regulator-name = "panel_en_3p3v";
>> + regulator-type = "voltage";
>> + startup-delay-us = <0>;
>> + gpio = <&pm8921_gpio 36 GPIO_ACTIVE_HIGH>;
>> + enable-active-high;
>> + regulator-boot-on;
>> + };
>
> We should put gpio regulators into their own container in the root of
> the tree. Similar to what was done for 8960 gpio regulators.
I agree, Will do this in next version.
>
>> +
>> + backlight: backlight{
>> + pinctrl-0 = <&pwm_bl_gpios>;
>> + pinctrl-names = "default";
>> + compatible = "gpio-backlight";
>> + gpios = <&pm8921_gpio 26 GPIO_ACTIVE_HIGH>;
>> + default-on;
>> + };
>> +
>> + panel: auo,b101xtn01 {
>> + status = "okay";
>> + compatible = "auo,b101xtn01";
>> +
>> + ddc-i2c-bus = <&i2c3>;
>> + backlight = <&backlight>;
>> + power-supply = <&panel_3p3v>;
>> };
>
> These two nodes shouldn't be under the SoC node. They don't have
> registers so they should be at the root of the tree. And we don't need
> to put labels twice on nodes. If we're modifying things in board
> specific dtsi files it should be fine to leave the label off if the
> label is in the SoC dtsi file.
Yep, will fix it in next version.
>
On Tue 28 Jul 05:54 PDT 2015, Srinivas Kandagatla wrote:
> This patch adds LVDS panel for IFC6410.
>
My ifc6410 doesn't have a LVDS display, so I don't think these nodes
belong in this dts file.
I was thinking we could have a variant including this dts file adding
the panel, but I assume that you can mount other panels than the auo
panel defined below?
Either way, I think we should keep this file to define the stuff that's
on the IFC6410 itself and then figure out how to aid people in adding
the rest of their additions...
Regards,
Bjorn
On Mon, Aug 10, 2015 at 7:25 PM, Bjorn Andersson
<[email protected]> wrote:
> On Tue 28 Jul 05:54 PDT 2015, Srinivas Kandagatla wrote:
>
>> This patch adds LVDS panel for IFC6410.
>>
>
> My ifc6410 doesn't have a LVDS display, so I don't think these nodes
> belong in this dts file.
fwiw, my hope was that we could implement a panel-picker for lvds+ddc,
in a similar way to handling devices that could have multiple
different dsi panels, to make this a bit more automagically
plug-and-play.. srini has done some experiments with probing panel's
edid, so we just need to figure out if vendor+device-id in edid is
reliable enough to have a global table mapping them back to panel
compat strings or whether that table needs to go in dt..
BR,
-R
> I was thinking we could have a variant including this dts file adding
> the panel, but I assume that you can mount other panels than the auo
> panel defined below?
>
>
> Either way, I think we should keep this file to define the stuff that's
> on the IFC6410 itself and then figure out how to aid people in adding
> the rest of their additions...
>
> Regards,
> Bjorn