diff mbox series

[2/2] ARM: dts: exynos: arndale: fix HDMI-HPD line handling

Message ID 20180726101228.14956-2-a.hajda@samsung.com (mailing list archive)
State Accepted
Headers show
Series [1/2] ARM: dts: exynos: arndale: use i2c-gpio for HDMI-DDC | expand

Commit Message

Andrzej Hajda July 26, 2018, 10:12 a.m. UTC
HDMI-HPD was set active low, moreover by default pincontrol
chip sets pull-down on the pin. As a result HDMI driver
assumes TV is always connected regardless of actual state.
The patch fixes it.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 arch/arm/boot/dts/exynos5250-arndale.dts  | 4 +++-
 arch/arm/boot/dts/exynos5250-pinctrl.dtsi | 5 +++++
 2 files changed, 8 insertions(+), 1 deletion(-)

Comments

Krzysztof Kozlowski July 26, 2018, 10:44 a.m. UTC | #1
On 26 July 2018 at 12:12, Andrzej Hajda <a.hajda@samsung.com> wrote:
> HDMI-HPD was set active low, moreover by default pincontrol
> chip sets pull-down on the pin. As a result HDMI driver
> assumes TV is always connected regardless of actual state.
> The patch fixes it.
>
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
>  arch/arm/boot/dts/exynos5250-arndale.dts  | 4 +++-
>  arch/arm/boot/dts/exynos5250-pinctrl.dtsi | 5 +++++
>  2 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/exynos5250-arndale.dts b/arch/arm/boot/dts/exynos5250-arndale.dts
> index 591023391f7d..26bae5157177 100644
> --- a/arch/arm/boot/dts/exynos5250-arndale.dts
> +++ b/arch/arm/boot/dts/exynos5250-arndale.dts
> @@ -210,9 +210,11 @@
>  };
>
>  &hdmi {
> +       pinctrl-names = "default";
> +       pinctrl-0 = <&hdmi_hpd>;
>         status = "okay";
>         ddc = <&i2c_ddc>;
> -       hpd-gpios = <&gpx3 7 GPIO_ACTIVE_LOW>;
> +       hpd-gpios = <&gpx3 7 GPIO_ACTIVE_HIGH>;

The gpx3-7 (HDMI-HPD) has external pull up to... so maybe it should be
active low? How can it go high if it is already high?
I am not sure if this matters because the driver ignores the flags and
responds on both falling and rising edge.

Best regards,
Krzysztof

>         vdd_osc-supply = <&ldo10_reg>;
>         vdd_pll-supply = <&ldo8_reg>;
>         vdd-supply = <&ldo8_reg>;
> diff --git a/arch/arm/boot/dts/exynos5250-pinctrl.dtsi b/arch/arm/boot/dts/exynos5250-pinctrl.dtsi
> index b25d520393b8..d31a68672bfa 100644
> --- a/arch/arm/boot/dts/exynos5250-pinctrl.dtsi
> +++ b/arch/arm/boot/dts/exynos5250-pinctrl.dtsi
> @@ -599,6 +599,11 @@
>                 samsung,pin-pud = <EXYNOS_PIN_PULL_NONE>;
>                 samsung,pin-drv = <EXYNOS4_PIN_DRV_LV1>;
>         };
> +
> +       hdmi_hpd: hdmi-hpd {
> +               samsung,pins = "gpx3-7";
> +               samsung,pin-pud = <EXYNOS_PIN_PULL_NONE>;
> +       };
>  };
>
>  &pinctrl_1 {
> --
> 2.18.0
>
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andrzej Hajda July 26, 2018, 11:50 a.m. UTC | #2
On 26.07.2018 12:44, Krzysztof Kozlowski wrote:
> On 26 July 2018 at 12:12, Andrzej Hajda <a.hajda@samsung.com> wrote:
>> HDMI-HPD was set active low, moreover by default pincontrol
>> chip sets pull-down on the pin. As a result HDMI driver
>> assumes TV is always connected regardless of actual state.
>> The patch fixes it.
>>
>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>> ---
>>  arch/arm/boot/dts/exynos5250-arndale.dts  | 4 +++-
>>  arch/arm/boot/dts/exynos5250-pinctrl.dtsi | 5 +++++
>>  2 files changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/boot/dts/exynos5250-arndale.dts b/arch/arm/boot/dts/exynos5250-arndale.dts
>> index 591023391f7d..26bae5157177 100644
>> --- a/arch/arm/boot/dts/exynos5250-arndale.dts
>> +++ b/arch/arm/boot/dts/exynos5250-arndale.dts
>> @@ -210,9 +210,11 @@
>>  };
>>
>>  &hdmi {
>> +       pinctrl-names = "default";
>> +       pinctrl-0 = <&hdmi_hpd>;
>>         status = "okay";
>>         ddc = <&i2c_ddc>;
>> -       hpd-gpios = <&gpx3 7 GPIO_ACTIVE_LOW>;
>> +       hpd-gpios = <&gpx3 7 GPIO_ACTIVE_HIGH>;
> The gpx3-7 (HDMI-HPD) has external pull up to... so maybe it should be
> active low? How can it go high if it is already high?
> I am not sure if this matters because the driver ignores the flags and
> responds on both falling and rising edge.

HDMI specification says HPD is active high!
Arndale schematics are quite misleading - pull-up resistors near level
shifter are marked as not-connected :) so there are no pull-ups.
And there is pull-down resistor on right side of the level shifter.
And finally with this patch it works as expected, without it it works
sometimes.

Regards
Andrzej

>
> Best regards,
> Krzysztof
>
>>         vdd_osc-supply = <&ldo10_reg>;
>>         vdd_pll-supply = <&ldo8_reg>;
>>         vdd-supply = <&ldo8_reg>;
>> diff --git a/arch/arm/boot/dts/exynos5250-pinctrl.dtsi b/arch/arm/boot/dts/exynos5250-pinctrl.dtsi
>> index b25d520393b8..d31a68672bfa 100644
>> --- a/arch/arm/boot/dts/exynos5250-pinctrl.dtsi
>> +++ b/arch/arm/boot/dts/exynos5250-pinctrl.dtsi
>> @@ -599,6 +599,11 @@
>>                 samsung,pin-pud = <EXYNOS_PIN_PULL_NONE>;
>>                 samsung,pin-drv = <EXYNOS4_PIN_DRV_LV1>;
>>         };
>> +
>> +       hdmi_hpd: hdmi-hpd {
>> +               samsung,pins = "gpx3-7";
>> +               samsung,pin-pud = <EXYNOS_PIN_PULL_NONE>;
>> +       };
>>  };
>>
>>  &pinctrl_1 {
>> --
>> 2.18.0
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Krzysztof Kozlowski July 26, 2018, 11:55 a.m. UTC | #3
On 26 July 2018 at 13:50, Andrzej Hajda <a.hajda@samsung.com> wrote:
> On 26.07.2018 12:44, Krzysztof Kozlowski wrote:
>> On 26 July 2018 at 12:12, Andrzej Hajda <a.hajda@samsung.com> wrote:
>>> HDMI-HPD was set active low, moreover by default pincontrol
>>> chip sets pull-down on the pin. As a result HDMI driver
>>> assumes TV is always connected regardless of actual state.
>>> The patch fixes it.
>>>
>>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>>> ---
>>>  arch/arm/boot/dts/exynos5250-arndale.dts  | 4 +++-
>>>  arch/arm/boot/dts/exynos5250-pinctrl.dtsi | 5 +++++
>>>  2 files changed, 8 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm/boot/dts/exynos5250-arndale.dts b/arch/arm/boot/dts/exynos5250-arndale.dts
>>> index 591023391f7d..26bae5157177 100644
>>> --- a/arch/arm/boot/dts/exynos5250-arndale.dts
>>> +++ b/arch/arm/boot/dts/exynos5250-arndale.dts
>>> @@ -210,9 +210,11 @@
>>>  };
>>>
>>>  &hdmi {
>>> +       pinctrl-names = "default";
>>> +       pinctrl-0 = <&hdmi_hpd>;
>>>         status = "okay";
>>>         ddc = <&i2c_ddc>;
>>> -       hpd-gpios = <&gpx3 7 GPIO_ACTIVE_LOW>;
>>> +       hpd-gpios = <&gpx3 7 GPIO_ACTIVE_HIGH>;
>> The gpx3-7 (HDMI-HPD) has external pull up to... so maybe it should be
>> active low? How can it go high if it is already high?
>> I am not sure if this matters because the driver ignores the flags and
>> responds on both falling and rising edge.
>
> HDMI specification says HPD is active high!
> Arndale schematics are quite misleading - pull-up resistors near level
> shifter are marked as not-connected :) so there are no pull-ups.
> And there is pull-down resistor on right side of the level shifter.

Ah, I was looking at schematics and looking and looking and still did
not see "NC"... My mistake.

> And finally with this patch it works as expected, without it it works
> sometimes.

Everything is good. I already sent last pull request so this will go
after merge window, for v4.20 (or whatever number it will be).

Best regards,
Krzysztof
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Krzysztof Kozlowski Aug. 29, 2018, 6:58 p.m. UTC | #4
On Thu, Jul 26, 2018 at 12:12:28PM +0200, Andrzej Hajda wrote:
> HDMI-HPD was set active low, moreover by default pincontrol
> chip sets pull-down on the pin. As a result HDMI driver
> assumes TV is always connected regardless of actual state.
> The patch fixes it.
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
>  arch/arm/boot/dts/exynos5250-arndale.dts  | 4 +++-
>  arch/arm/boot/dts/exynos5250-pinctrl.dtsi | 5 +++++

Thanks, applied.

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/arch/arm/boot/dts/exynos5250-arndale.dts b/arch/arm/boot/dts/exynos5250-arndale.dts
index 591023391f7d..26bae5157177 100644
--- a/arch/arm/boot/dts/exynos5250-arndale.dts
+++ b/arch/arm/boot/dts/exynos5250-arndale.dts
@@ -210,9 +210,11 @@ 
 };
 
 &hdmi {
+	pinctrl-names = "default";
+	pinctrl-0 = <&hdmi_hpd>;
 	status = "okay";
 	ddc = <&i2c_ddc>;
-	hpd-gpios = <&gpx3 7 GPIO_ACTIVE_LOW>;
+	hpd-gpios = <&gpx3 7 GPIO_ACTIVE_HIGH>;
 	vdd_osc-supply = <&ldo10_reg>;
 	vdd_pll-supply = <&ldo8_reg>;
 	vdd-supply = <&ldo8_reg>;
diff --git a/arch/arm/boot/dts/exynos5250-pinctrl.dtsi b/arch/arm/boot/dts/exynos5250-pinctrl.dtsi
index b25d520393b8..d31a68672bfa 100644
--- a/arch/arm/boot/dts/exynos5250-pinctrl.dtsi
+++ b/arch/arm/boot/dts/exynos5250-pinctrl.dtsi
@@ -599,6 +599,11 @@ 
 		samsung,pin-pud = <EXYNOS_PIN_PULL_NONE>;
 		samsung,pin-drv = <EXYNOS4_PIN_DRV_LV1>;
 	};
+
+	hdmi_hpd: hdmi-hpd {
+		samsung,pins = "gpx3-7";
+		samsung,pin-pud = <EXYNOS_PIN_PULL_NONE>;
+	};
 };
 
 &pinctrl_1 {