| Message ID | 1389933172-22991-1-git-send-email-sachin.kamat@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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> Thank you for sending the patch. However, please CC me, because I am a maintainer of Exynos DP driver. Best regards, Jingoo Han > --- > drivers/video/exynos/Kconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > 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. > -- > 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. Now, Exynos "DP" and Exynos "MIPI" are separately maintained as below: EXYNOS DP DRIVER M: Jingoo Han <jg1.han@samsung.com> L: linux-fbdev@vger.kernel.org S: Maintained F: drivers/video/exynos/exynos_dp* F: include/video/exynos_dp* EXYNOS MIPI DISPLAY DRIVERS M: Inki Dae <inki.dae@samsung.com> M: Donghwa Lee <dh09.lee@samsung.com> M: Kyungmin Park <kyungmin.park@samsung.com> L: linux-fbdev@vger.kernel.org S: Maintained F: drivers/video/exynos/exynos_mipi* F: include/video/exynos_mipi* Thus, 2nd patch is related to "MIPI", thus, previous CC list is right. However, 1st patch is related to only "DP". So, I should be included to CC list. Best regards, Jingoo Han -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 Friday, January 17, 2014 2:44 PM, Sachin Kamat wrote: > 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. Ah, I see. Right, there is no entries for 'drivers/video/exynos/Kconfig'. Because, this Kconfig file is shared by both "EXYNOS DP DRIVER" and "EXYNOS MIPI DISPLAY DRIVERS", I did not add it to "EXYNOS DP DRIVER" entry. > > 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) If you have a good idea for get_maintainer.pl, please let me know. Thank you. Best regards, Jingoo Han -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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? Tomi
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 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. Tomi
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/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. Tomi
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/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. 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 Tomi
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(-)