[PATCH V2 RESEND] arm: dts: Exynos5: Use pmu_system_controller phandle for dp phy

[PATCH V2 RESEND] arm: dts: Exynos5: Use pmu_system_controller phandle for dp phy

[Vivek Gautam]

DP PHY now require pmu-system-controller to handle PMU register
to control PHY's power isolation. Adding the same to dp-phy
node.

Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
Reviewed-by: Jingoo Han <jg1.han@samsung.com>
Tested-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
Cc: Kukjin Kim <kgene@kernel.org>
---
 arch/arm/boot/dts/exynos5250.dtsi |    2 +-
 arch/arm/boot/dts/exynos5420.dtsi |    4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
index 0a588b4..bebd099 100644
--- a/arch/arm/boot/dts/exynos5250.dtsi
+++ b/arch/arm/boot/dts/exynos5250.dtsi
@@ -732,7 +732,7 @@
 
 	dp_phy: video-phy@10040720 {
 		compatible = "samsung,exynos5250-dp-video-phy";
-		reg = <0x10040720 4>;
+		samsung,pmu-syscon = <&pmu_system_controller>;
 		#phy-cells = <0>;
 	};
 
diff --git a/arch/arm/boot/dts/exynos5420.dtsi b/arch/arm/boot/dts/exynos5420.dtsi
index 8617a03..1353a09 100644
--- a/arch/arm/boot/dts/exynos5420.dtsi
+++ b/arch/arm/boot/dts/exynos5420.dtsi
@@ -503,8 +503,8 @@
 	};
 
 	dp_phy: video-phy@10040728 {
-		compatible = "samsung,exynos5250-dp-video-phy";
-		reg = <0x10040728 4>;
+		compatible = "samsung,exynos5420-dp-video-phy";
+		samsung,pmu-syscon = <&pmu_system_controller>;
 		#phy-cells = <0>;
 	};
 
-- 
1.7.10.4

Re: [PATCH V2 RESEND] arm: dts: Exynos5: Use pmu_system_controller phandle for dp phy

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1889 bytes --]

On Mon, Nov 24, 2014 at 11:11:23AM +0530, Vivek Gautam wrote:
> DP PHY now require pmu-system-controller to handle PMU register
> to control PHY's power isolation. Adding the same to dp-phy
> node.
> 
> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
> Reviewed-by: Jingoo Han <jg1.han@samsung.com>
> Tested-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
> Cc: Kukjin Kim <kgene@kernel.org>
> ---
>  arch/arm/boot/dts/exynos5250.dtsi |    2 +-
>  arch/arm/boot/dts/exynos5420.dtsi |    4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
> index 0a588b4..bebd099 100644
> --- a/arch/arm/boot/dts/exynos5250.dtsi
> +++ b/arch/arm/boot/dts/exynos5250.dtsi
> @@ -732,7 +732,7 @@
>  
>  	dp_phy: video-phy@10040720 {
>  		compatible = "samsung,exynos5250-dp-video-phy";
> -		reg = <0x10040720 4>;
> +		samsung,pmu-syscon = <&pmu_system_controller>;
>  		#phy-cells = <0>;
>  	};
>  
> diff --git a/arch/arm/boot/dts/exynos5420.dtsi b/arch/arm/boot/dts/exynos5420.dtsi
> index 8617a03..1353a09 100644
> --- a/arch/arm/boot/dts/exynos5420.dtsi
> +++ b/arch/arm/boot/dts/exynos5420.dtsi
> @@ -503,8 +503,8 @@
>  	};
>  
>  	dp_phy: video-phy@10040728 {
> -		compatible = "samsung,exynos5250-dp-video-phy";
> -		reg = <0x10040728 4>;
> +		compatible = "samsung,exynos5420-dp-video-phy";
> +		samsung,pmu-syscon = <&pmu_system_controller>;
>  		#phy-cells = <0>;
>  	};
>  

It seems like these nodes have been in the Linux tree since 3.12 and
3.17, respectively and these changes break backwards-compatibility. Has
anyone thought about the possible consequences?

Although, looking more closely it seems like this isn't the first time
that backwards-compatibility was broken in these files, so perhaps
nobody cares...

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH V2 RESEND] arm: dts: Exynos5: Use pmu_system_controller phandle for dp phy

[Vivek Gautam]

Hi,


On Mon, Nov 24, 2014 at 4:02 PM, Thierry Reding
<thierry.reding@gmail.com> wrote:
> On Mon, Nov 24, 2014 at 11:11:23AM +0530, Vivek Gautam wrote:
>> DP PHY now require pmu-system-controller to handle PMU register
>> to control PHY's power isolation. Adding the same to dp-phy
>> node.
>>
>> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
>> Reviewed-by: Jingoo Han <jg1.han@samsung.com>
>> Tested-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
>> Cc: Kukjin Kim <kgene@kernel.org>
>> ---
>>  arch/arm/boot/dts/exynos5250.dtsi |    2 +-
>>  arch/arm/boot/dts/exynos5420.dtsi |    4 ++--
>>  2 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
>> index 0a588b4..bebd099 100644
>> --- a/arch/arm/boot/dts/exynos5250.dtsi
>> +++ b/arch/arm/boot/dts/exynos5250.dtsi
>> @@ -732,7 +732,7 @@
>>
>>       dp_phy: video-phy@10040720 {
>>               compatible = "samsung,exynos5250-dp-video-phy";
>> -             reg = <0x10040720 4>;
>> +             samsung,pmu-syscon = <&pmu_system_controller>;
>>               #phy-cells = <0>;
>>       };
>>
>> diff --git a/arch/arm/boot/dts/exynos5420.dtsi b/arch/arm/boot/dts/exynos5420.dtsi
>> index 8617a03..1353a09 100644
>> --- a/arch/arm/boot/dts/exynos5420.dtsi
>> +++ b/arch/arm/boot/dts/exynos5420.dtsi
>> @@ -503,8 +503,8 @@
>>       };
>>
>>       dp_phy: video-phy@10040728 {
>> -             compatible = "samsung,exynos5250-dp-video-phy";
>> -             reg = <0x10040728 4>;
>> +             compatible = "samsung,exynos5420-dp-video-phy";
>> +             samsung,pmu-syscon = <&pmu_system_controller>;
>>               #phy-cells = <0>;
>>       };
>>
>
> It seems like these nodes have been in the Linux tree since 3.12 and
> 3.17, respectively and these changes break backwards-compatibility. Has
> anyone thought about the possible consequences?

Sorry for my ignorance, but i have a doubt.
If the bindings and device node both are being changed in the same kernel
version (as fixes),
so that the stable will have both; then the only scenerio of backward
compatibility comes when kernel is upgraded but not dtbs.
Does such upgradation makes sense for distros ?

>
> Although, looking more closely it seems like this isn't the first time
> that backwards-compatibility was broken in these files, so perhaps
> nobody cares...
>
> Thierry



-- 
Best Regards
Vivek Gautam
Samsung R&D Institute, Bangalore
India

Re: [PATCH V2 RESEND] arm: dts: Exynos5: Use pmu_system_controller phandle for dp phy

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 3218 bytes --]

On Mon, Nov 24, 2014 at 04:17:18PM +0530, Vivek Gautam wrote:
> Hi,
> 
> 
> On Mon, Nov 24, 2014 at 4:02 PM, Thierry Reding
> <thierry.reding@gmail.com> wrote:
> > On Mon, Nov 24, 2014 at 11:11:23AM +0530, Vivek Gautam wrote:
> >> DP PHY now require pmu-system-controller to handle PMU register
> >> to control PHY's power isolation. Adding the same to dp-phy
> >> node.
> >>
> >> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
> >> Reviewed-by: Jingoo Han <jg1.han@samsung.com>
> >> Tested-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
> >> Cc: Kukjin Kim <kgene@kernel.org>
> >> ---
> >>  arch/arm/boot/dts/exynos5250.dtsi |    2 +-
> >>  arch/arm/boot/dts/exynos5420.dtsi |    4 ++--
> >>  2 files changed, 3 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
> >> index 0a588b4..bebd099 100644
> >> --- a/arch/arm/boot/dts/exynos5250.dtsi
> >> +++ b/arch/arm/boot/dts/exynos5250.dtsi
> >> @@ -732,7 +732,7 @@
> >>
> >>       dp_phy: video-phy@10040720 {
> >>               compatible = "samsung,exynos5250-dp-video-phy";
> >> -             reg = <0x10040720 4>;
> >> +             samsung,pmu-syscon = <&pmu_system_controller>;
> >>               #phy-cells = <0>;
> >>       };
> >>
> >> diff --git a/arch/arm/boot/dts/exynos5420.dtsi b/arch/arm/boot/dts/exynos5420.dtsi
> >> index 8617a03..1353a09 100644
> >> --- a/arch/arm/boot/dts/exynos5420.dtsi
> >> +++ b/arch/arm/boot/dts/exynos5420.dtsi
> >> @@ -503,8 +503,8 @@
> >>       };
> >>
> >>       dp_phy: video-phy@10040728 {
> >> -             compatible = "samsung,exynos5250-dp-video-phy";
> >> -             reg = <0x10040728 4>;
> >> +             compatible = "samsung,exynos5420-dp-video-phy";
> >> +             samsung,pmu-syscon = <&pmu_system_controller>;
> >>               #phy-cells = <0>;
> >>       };
> >>
> >
> > It seems like these nodes have been in the Linux tree since 3.12 and
> > 3.17, respectively and these changes break backwards-compatibility. Has
> > anyone thought about the possible consequences?
> 
> Sorry for my ignorance, but i have a doubt.
> If the bindings and device node both are being changed in the same kernel
> version (as fixes),
> so that the stable will have both; then the only scenerio of backward
> compatibility comes when kernel is upgraded but not dtbs.

Correct.

> Does such upgradation makes sense for distros ?

Yes. Back at the time a decision was made that device trees need to be
stable ABI because eventually they'd be shipped with the device rather
than the distribution. As such it may not at all be possible to update
them (they could be in some sort of ROM).

For that reason new kernels need to keep working with old DTBs unless an
argument can be made that would justify breaking things. I don't think I
have ever seen anyone win such an argument. One of the rare exceptions
that I know of is if you can prove that a given hardware block has never
been used in an upstream kernel, then changing the DTB in backwards-
incompatible ways would be okay because you wouldn't be breaking things
for existing users.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH V2 RESEND] arm: dts: Exynos5: Use pmu_system_controller phandle for dp phy

[Vivek Gautam]

On Mon, Nov 24, 2014 at 4:26 PM, Thierry Reding
<thierry.reding@gmail.com> wrote:
> On Mon, Nov 24, 2014 at 04:17:18PM +0530, Vivek Gautam wrote:
>> Hi,
>>
>>
>> On Mon, Nov 24, 2014 at 4:02 PM, Thierry Reding
>> <thierry.reding@gmail.com> wrote:
>> > On Mon, Nov 24, 2014 at 11:11:23AM +0530, Vivek Gautam wrote:
>> >> DP PHY now require pmu-system-controller to handle PMU register
>> >> to control PHY's power isolation. Adding the same to dp-phy
>> >> node.
>> >>
>> >> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
>> >> Reviewed-by: Jingoo Han <jg1.han@samsung.com>
>> >> Tested-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
>> >> Cc: Kukjin Kim <kgene@kernel.org>
>> >> ---
>> >>  arch/arm/boot/dts/exynos5250.dtsi |    2 +-
>> >>  arch/arm/boot/dts/exynos5420.dtsi |    4 ++--
>> >>  2 files changed, 3 insertions(+), 3 deletions(-)
>> >>
>> >> diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
>> >> index 0a588b4..bebd099 100644
>> >> --- a/arch/arm/boot/dts/exynos5250.dtsi
>> >> +++ b/arch/arm/boot/dts/exynos5250.dtsi
>> >> @@ -732,7 +732,7 @@
>> >>
>> >>       dp_phy: video-phy@10040720 {
>> >>               compatible = "samsung,exynos5250-dp-video-phy";
>> >> -             reg = <0x10040720 4>;
>> >> +             samsung,pmu-syscon = <&pmu_system_controller>;
>> >>               #phy-cells = <0>;
>> >>       };
>> >>
>> >> diff --git a/arch/arm/boot/dts/exynos5420.dtsi b/arch/arm/boot/dts/exynos5420.dtsi
>> >> index 8617a03..1353a09 100644
>> >> --- a/arch/arm/boot/dts/exynos5420.dtsi
>> >> +++ b/arch/arm/boot/dts/exynos5420.dtsi
>> >> @@ -503,8 +503,8 @@
>> >>       };
>> >>
>> >>       dp_phy: video-phy@10040728 {
>> >> -             compatible = "samsung,exynos5250-dp-video-phy";
>> >> -             reg = <0x10040728 4>;
>> >> +             compatible = "samsung,exynos5420-dp-video-phy";
>> >> +             samsung,pmu-syscon = <&pmu_system_controller>;
>> >>               #phy-cells = <0>;
>> >>       };
>> >>
>> >
>> > It seems like these nodes have been in the Linux tree since 3.12 and
>> > 3.17, respectively and these changes break backwards-compatibility. Has
>> > anyone thought about the possible consequences?
>>
>> Sorry for my ignorance, but i have a doubt.
>> If the bindings and device node both are being changed in the same kernel
>> version (as fixes),
>> so that the stable will have both; then the only scenerio of backward
>> compatibility comes when kernel is upgraded but not dtbs.
>
> Correct.
>
>> Does such upgradation makes sense for distros ?
>
> Yes. Back at the time a decision was made that device trees need to be
> stable ABI because eventually they'd be shipped with the device rather
> than the distribution. As such it may not at all be possible to update
> them (they could be in some sort of ROM).
>
> For that reason new kernels need to keep working with old DTBs unless an
> argument can be made that would justify breaking things. I don't think I
> have ever seen anyone win such an argument.
> One of the rare exceptions
> that I know of is if you can prove that a given hardware block has never
> been used in an upstream kernel, then changing the DTB in backwards-
> incompatible ways would be okay because you wouldn't be breaking things
> for existing users.

I am pretty sure about the Chrome devices (which have not been
upgraded to any kernel after
3.8).
Probably Javier may have better knowledge.

Javier, is there any other device using upstream kernel post 3.12 (any
arndale octa based) ?




-- 
Best Regards
Vivek Gautam
Samsung R&D Institute, Bangalore
India

Re: [PATCH V2 RESEND] arm: dts: Exynos5: Use pmu_system_controller phandle for dp phy

[Javier Martinez Canillas]

Hello Vivek, Thierry,

On 11/24/2014 12:29 PM, Vivek Gautam wrote:
>> Yes. Back at the time a decision was made that device trees need to be
>> stable ABI because eventually they'd be shipped with the device rather
>> than the distribution. As such it may not at all be possible to update
>> them (they could be in some sort of ROM).
>>
>> For that reason new kernels need to keep working with old DTBs unless an
>> argument can be made that would justify breaking things. I don't think I
>> have ever seen anyone win such an argument.

Although uncommon, there are cases when breaking a DT backward compatibility
could make sense IMHO. For example the DT binding for the SPI controller
found on Samsung SoCs (s3c64xx) used a custom binding to specify the chip
select (CS) line. Later the binding was changed in mainline breaking backward
compatibility and the breakage was not noticed in a year even when the change
not only broke backward compatibility but also was wrong.

So the options were to a) keep the buggy DT which also broke backward compact,
b) revert to the old binding breaking any DTS that were added the last year,
c) break backward compatibility again but take the opportunity to fix the
binding for good by dropping device specific bindings and use standard ones.

At the end it was decided that c) was the least bad option and was made in
commit 306972c ("spi: s3c64xx: use the generic SPI "cs-gpios" property").

>> One of the rare exceptions
>> that I know of is if you can prove that a given hardware block has never
>> been used in an upstream kernel, then changing the DTB in backwards-
>> incompatible ways would be okay because you wouldn't be breaking things
>> for existing users.
> 
> I am pretty sure about the Chrome devices (which have not been
> upgraded to any kernel after
> 3.8).
> Probably Javier may have better knowledge.
> 

Correct. The Exynos based Chromebooks are using a 3.8 kernel and the FDT
shipped can't be used with a mainline kernel since it used out-of-tree
drivers whose DT bindings were changed during review to upstream inclusion.

Also, the stock boot-loader loads a FIT image which has a kernel and FDT
bundled in the same image so in that case the FDT has to be shipped with
the Linux kernel anyways.

> Javier, is there any other device using upstream kernel post 3.12 (any
> arndale octa based) ?
> 

Sorry, I don't know if there are other Exynos devices using a more recent
kernel but AFAICT the DT binding for the Exynos Display Port was changed
recently (v3.17) in a non-backward compatible way with commit 5f1dcd8
("drm/exynos: dp: Modify driver to support drm_panel"). So anyone using a
more recent kernel will have to update the FDT to have display working.

Also display does not work for many Exynos5 devices that have an eDP to LVDS
bridge chip for example so I think is safe to assume that anything related
to the Exynos DP (like the DP PHY) would still not have a stable DT binding.

Best regards,
Javier

Re: [PATCH V2 RESEND] arm: dts: Exynos5: Use pmu_system_controller phandle for dp phy

[Javier Martinez Canillas]

Hello Kukjin,

On Mon, Nov 24, 2014 at 6:41 AM, Vivek Gautam <gautam.vivek@samsung.com> wrote:
> DP PHY now require pmu-system-controller to handle PMU register
> to control PHY's power isolation. Adding the same to dp-phy
> node.
>
> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
> Reviewed-by: Jingoo Han <jg1.han@samsung.com>
> Tested-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
> Cc: Kukjin Kim <kgene@kernel.org>

Any opinions about $subject?

This patch is -rc material since is needed after commit a5ec598 ("phy:
exynos-dp-video: Use syscon
support to control pmu register") which landed in 3.18. That means
that display for Exynos is currently broken in 3.18.

I think it's too late for the 3.18 -rc cycle but at least it would be
great to have this merged for 3.19 and backport to stable kernels to
have display working again.

Thierry had concerns that this change breaks DT backward compability
but actually it was already been broken by a5ec598 which changed the
DT binding for the phy-exynos-dp-video driver so we should either
apply this patch now or revert a5ec598.

Thanks a lot and best regards,
Javier

Re: [PATCH V2 RESEND] arm: dts: Exynos5: Use pmu_system_controller phandle for dp phy

[Jingoo Han]

On Tuesday, December 02, 2014 5:17 PM, Javier Martinez Canillas wrote:
> 
> Hello Kukjin,
> 
> On Mon, Nov 24, 2014 at 6:41 AM, Vivek Gautam <gautam.vivek@samsung.com> wrote:
> > DP PHY now require pmu-system-controller to handle PMU register
> > to control PHY's power isolation. Adding the same to dp-phy
> > node.
> >
> > Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
> > Reviewed-by: Jingoo Han <jg1.han@samsung.com>
> > Tested-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
> > Cc: Kukjin Kim <kgene@kernel.org>
> 
> Any opinions about $subject?
> 
> This patch is -rc material since is needed after commit a5ec598 ("phy:
> exynos-dp-video: Use syscon
> support to control pmu register") which landed in 3.18. That means
> that display for Exynos is currently broken in 3.18.
> 
> I think it's too late for the 3.18 -rc cycle but at least it would be
> great to have this merged for 3.19 and backport to stable kernels to
> have display working again.

I agree with this suggestion.

> 
> Thierry had concerns that this change breaks DT backward compability
> but actually it was already been broken by a5ec598 which changed the
> DT binding for the phy-exynos-dp-video driver so we should either
> apply this patch now or revert a5ec598.

I think that very few people might use old properties for Exynos DP.
Actually, DT backward compatibility will not be the considerable problem
in my opinion.

But, in order to keep the DT backward compatibility, we should revert
a5ec598, and send another patch for keeping the DT backward compatibility.

Best regards,
Jingoo Han

> 
> Thanks a lot and best regards,
> Javier

Re: [PATCH V2 RESEND] arm: dts: Exynos5: Use pmu_system_controller phandle for dp phy

[Javier Martinez Canillas]

[adding arm-soc maintainers to cc]

Hello Kukjin,

On 12/02/2014 09:39 AM, Jingoo Han wrote:
> On Tuesday, December 02, 2014 5:17 PM, Javier Martinez Canillas wrote:
>> On Mon, Nov 24, 2014 at 6:41 AM, Vivek Gautam <gautam.vivek@samsung.com> wrote:
>> > DP PHY now require pmu-system-controller to handle PMU register
>> > to control PHY's power isolation. Adding the same to dp-phy
>> > node.
>> >
>> > Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
>> > Reviewed-by: Jingoo Han <jg1.han@samsung.com>
>> > Tested-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
>> > Cc: Kukjin Kim <kgene@kernel.org>
>> 
>> Any opinions about $subject?
>> 
>> This patch is -rc material since is needed after commit a5ec598 ("phy:
>> exynos-dp-video: Use syscon
>> support to control pmu register") which landed in 3.18. That means
>> that display for Exynos is currently broken in 3.18.
>> 
>> I think it's too late for the 3.18 -rc cycle but at least it would be
>> great to have this merged for 3.19 and backport to stable kernels to
>> have display working again.
> 
> I agree with this suggestion.
>

Sorry for being nagging with this but 3.18 has been released and the Exynos
DP video PHY is not working because this patch was not merged :(

So, it would be good if this can be pushed before the merge window for 3.19
closes or we may end with another kernel release with a non-working display.
 
>> 
>> Thierry had concerns that this change breaks DT backward compability
>> but actually it was already been broken by a5ec598 which changed the
>> DT binding for the phy-exynos-dp-video driver so we should either
>> apply this patch now or revert a5ec598.
> 
> I think that very few people might use old properties for Exynos DP.
> Actually, DT backward compatibility will not be the considerable problem
> in my opinion.
> 
> But, in order to keep the DT backward compatibility, we should revert
> a5ec598, and send another patch for keeping the DT backward compatibility.
>

I'm not sure if is worth it to revert a5ec598 and maintain DT backward
compatibility in this case since it seems there aren't real mainline
users of Exynos DP, otherwise someone would had cared that 3.18 is broken.

IMHO just $subject has to be picked to make the DTS use the new DT binding
of the phy-exynos-dp-video driver. Maybe arm-soc maintainers can pick
$subject directly since Kukjin seems to be busy?

Thanks a lot and best regards,
Javier