| Message ID | 1389933172-22991-1-git-send-email-sachin.kamat@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
On 17 January 2014 10:17, Jingoo Han <jg1.han@samsung.com> wrote: > On Friday, January 17, 2014 1:33 PM, Sachin Kamat wrote: >> >> Exynos is now a DT only platform. Hence there is no need >> for an explicit OF dependency. Remove it. >> >> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org> > > Acked-by: Jingoo Han <jg1.han@samsung.com> Thanks. > > Thank you for sending the patch. However, please CC me, > because I am a maintainer of Exynos DP driver. Sorry for missing you on the CC list. I think you probably need to update the MAINTAINER file entry to reflect this. --- With warm regards, Sachin
On 17 January 2014 10:42, Jingoo Han <jg1.han@samsung.com> wrote: > On Friday, January 17, 2014 1:58 PM, Jingoo Han wrote: >> On 17 January 2014 10:17, Jingoo Han <jg1.han@samsung.com> wrote: >> > On Friday, January 17, 2014 1:33 PM, Sachin Kamat wrote: >> >> >> >> Exynos is now a DT only platform. Hence there is no need >> >> for an explicit OF dependency. Remove it. >> >> >> >> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org> >> > >> > Acked-by: Jingoo Han <jg1.han@samsung.com> >> >> Thanks. >> > >> > Thank you for sending the patch. However, please CC me, >> > because I am a maintainer of Exynos DP driver. >> >> Sorry for missing you on the CC list. I think you probably need to >> update the MAINTAINER file >> entry to reflect this. > > Um, there is no problem in the MAINTAINER file about this. > Maybe, you are confused. No confusion from my side :) The entries below do not list maintainer for the Kconfig file since you have added specific filters. Please verify using get_maintainers script. scripts/get_maintainer.pl -f drivers/video/exynos/Kconfig This is what it gives (and you are not listed as a maintainer in this case): Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com> (maintainer:FRAMEBUFFER LAYER) Tomi Valkeinen <tomi.valkeinen@ti.com> (maintainer:FRAMEBUFFER LAYER,commit_signer:1/4=25%) Kukjin Kim <kgene.kim@samsung.com> (maintainer:ARM/S5P EXYNOS AR...) Greg Kroah-Hartman <gregkh@linuxfoundation.org> (commit_signer:2/4=50%) Kishon Vijay Abraham I <kishon@ti.com> (commit_signer:2/4=50%) Sachin Kamat <sachin.kamat@linaro.org> (commit_signer:2/4=50%,authored:2/4=50%,added_lines:3/5=60%,removed_lines:2/3=67%) Jingoo Han <jg1.han@samsung.com> (commit_signer:1/4=25%,authored:1/4=25%,added_lines:1/5=20%,removed_lines:1/3=33%) Sylwester Nawrocki <sylvester.nawrocki@gmail.com> (authored:1/4=25%,added_lines:1/5=20%) linux-fbdev@vger.kernel.org (open list:FRAMEBUFFER LAYER) linux-arm-kernel@lists.infradead.org (moderated list:ARM/S5P EXYNOS AR...) linux-samsung-soc@vger.kernel.org (moderated list:ARM/S5P EXYNOS AR...) linux-kernel@vger.kernel.org (open list)
On 17 January 2014 14:33, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote: > On 2014-01-17 06:32, Sachin Kamat wrote: >> Exynos is now a DT only platform. Hence there is no need >> for an explicit OF dependency. Remove it. > > Is Exynos a DT-only platform in v3.13? Or only in v3.14? It has been so since v3.11.
Hi Tomi, On 17 January 2014 14:51, Sachin Kamat <sachin.kamat@linaro.org> wrote: > On 17 January 2014 14:33, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote: >> On 2014-01-17 06:32, Sachin Kamat wrote: >>> Exynos is now a DT only platform. Hence there is no need >>> for an explicit OF dependency. Remove it. >> >> Is Exynos a DT-only platform in v3.13? Or only in v3.14? > > It has been so since v3.11. > Any other comments on this series?
On 10 February 2014 17:48, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote: > On 17/01/14 06:32, Sachin Kamat wrote: >> Exynos is now a DT only platform. Hence there is no need >> for an explicit OF dependency. Remove it. > > But the driver still depends on OF, doesn't it? I don't think it's very > good for the driver Kconfig to make presumptions about what ARCH_* > depend on. Depending upon nested dependencies is redundant IMHO.
On 11 February 2014 19:57, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote: > On 11/02/14 14:01, Sachin Kamat wrote: >> On 10 February 2014 17:48, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote: >>> On 17/01/14 06:32, Sachin Kamat wrote: >>>> Exynos is now a DT only platform. Hence there is no need >>>> for an explicit OF dependency. Remove it. >>> >>> But the driver still depends on OF, doesn't it? I don't think it's very >>> good for the driver Kconfig to make presumptions about what ARCH_* >>> depend on. >> >> Depending upon nested dependencies is redundant IMHO. > > Well, a driver should be independent of the underlying arch. In > practice, we have ARCH dependencies, as many of the devices only exist > on that arch. But I think the drivers should still be designed to be > arch-independent, as far as possible (omapdss compiles fine on x86). > > If the driver depends on OF, it should depend on OF in the Kconfig, no > matter if the arch also depends on OF. > > I don't really care if the EXYNOS_LCD_S6E8AX0 has OF dependency or not, > but to me this just looks unneeded cleanup, cluttering git logs, and in > my opinion it's even going to the wrong direction. Your argument makes sense. Upon further experimentation I found that even the Exynos video drivers are ARCH independent (i.e., they build on x86 too) and do not need to depend on OF for compilation. So I believe, we can remove both these dependencies. What is your opinion?
On 12 February 2014 12:55, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote: > On 12/02/14 09:08, Sachin Kamat wrote: >> On 11 February 2014 19:57, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote: >>> On 11/02/14 14:01, Sachin Kamat wrote: >>>> On 10 February 2014 17:48, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote: >>>>> On 17/01/14 06:32, Sachin Kamat wrote: >>>>>> Exynos is now a DT only platform. Hence there is no need >>>>>> for an explicit OF dependency. Remove it. >>>>> >>>>> But the driver still depends on OF, doesn't it? I don't think it's very >>>>> good for the driver Kconfig to make presumptions about what ARCH_* >>>>> depend on. >>>> >>>> Depending upon nested dependencies is redundant IMHO. >>> >>> Well, a driver should be independent of the underlying arch. In >>> practice, we have ARCH dependencies, as many of the devices only exist >>> on that arch. But I think the drivers should still be designed to be >>> arch-independent, as far as possible (omapdss compiles fine on x86). >>> >>> If the driver depends on OF, it should depend on OF in the Kconfig, no >>> matter if the arch also depends on OF. >>> >>> I don't really care if the EXYNOS_LCD_S6E8AX0 has OF dependency or not, >>> but to me this just looks unneeded cleanup, cluttering git logs, and in >>> my opinion it's even going to the wrong direction. >> >> Your argument makes sense. Upon further experimentation I found that even the >> Exynos video drivers are ARCH independent (i.e., they build on x86 too) and do >> not need to depend on OF for compilation. So I believe, we can remove both these >> dependencies. What is your opinion? > > Indeed, the driver doesn't even seem to call any of_* funcs. Looking at > the commit f9b1e013f1c6723798b8f7f5b83297e2837aaef7 (video: exynos_dp: > remove non-DT support for Exynos Display Port), it kind of sounds to me > that the OF dependency was put there just to prevent non-DT use. > > I'm fine with removing OF dependency, if the commit description is > updated to say that it can be removed as the driver doesn't actually > depend on OF at all. > > As for the ARCH dependency, I think we should keep it. I once removed > ARCH_OMAP dependency from omapdss, but Linus wasn't impressed when his > kernel compilation started to ask him if he wants to enable OMAPDSS > this, OMAPDSS that =). So I think it's fine to keep ARCH dependencies in > cases where the driver is clearly used only on some architecture. Yes, I remember that :) > > However, you can use COMPILE_TEST kconfig option if you want to compile > test on other archs. I.e.: > > depends on ARCH_EXYNOS || COMPILE_TEST For now I will update the commit description and re-send the patch. Thanks for your comments Tomi.
diff --git a/drivers/video/exynos/Kconfig b/drivers/video/exynos/Kconfig index 1129d0e9e640..976594d578a9 100644 --- a/drivers/video/exynos/Kconfig +++ b/drivers/video/exynos/Kconfig @@ -30,7 +30,7 @@ config EXYNOS_LCD_S6E8AX0 config EXYNOS_DP bool "EXYNOS DP driver support" - depends on OF && ARCH_EXYNOS + depends on ARCH_EXYNOS default n help This enables support for DP device.
Exynos is now a DT only platform. Hence there is no need for an explicit OF dependency. Remove it. Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org> --- drivers/video/exynos/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)