| Message ID | 1416807683-2257-1-git-send-email-gautam.vivek@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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
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
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) ?
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 -- 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
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 -- 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
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 -- 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
[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 -- 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
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>; };