| Message ID | 1308552287-18752-1-git-send-email-zhangfei.gao@marvell.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Monday 20 June 2011 08:44:47 Zhangfei Gao wrote: > Without depends, MMC_SDHCI_PXAV3/2 can be chosen in other platfrom. > Build error will occur since plat/sdhci.h is located at arch/arm/plat-pxa/ > > Signed-off-by: Zhangfei Gao <zhangfei.gao@marvell.com> > --- > drivers/mmc/host/Kconfig | 2 ++ > 1 files changed, 2 insertions(+), 0 deletions(-) > > diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig > index 198ddda..d33a3e2 100644 > --- a/drivers/mmc/host/Kconfig > +++ b/drivers/mmc/host/Kconfig > @@ -183,6 +183,7 @@ config MMC_SDHCI_S3C > > config MMC_SDHCI_PXAV3 > tristate "Marvell MMP2 SD Host Controller support (PXAV3)" > + depends on ARCH_MMP > select MMC_SDHCI > select MMC_SDHCI_PLTFM > default CPU_MMP2 Why not just move the header file to include/linux/platform_data/? Then you can build the driver on all platforms, which improves the build test coverage. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jun 20, 2011 at 4:07 PM, Arnd Bergmann <arnd@arndb.de> wrote: > On Monday 20 June 2011 08:44:47 Zhangfei Gao wrote: >> Without depends, MMC_SDHCI_PXAV3/2 can be chosen in other platfrom. >> Build error will occur since plat/sdhci.h is located at arch/arm/plat-pxa/ >> >> Signed-off-by: Zhangfei Gao <zhangfei.gao@marvell.com> >> --- >> drivers/mmc/host/Kconfig | 2 ++ >> 1 files changed, 2 insertions(+), 0 deletions(-) >> >> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig >> index 198ddda..d33a3e2 100644 >> --- a/drivers/mmc/host/Kconfig >> +++ b/drivers/mmc/host/Kconfig >> @@ -183,6 +183,7 @@ config MMC_SDHCI_S3C >> >> config MMC_SDHCI_PXAV3 >> tristate "Marvell MMP2 SD Host Controller support (PXAV3)" >> + depends on ARCH_MMP >> select MMC_SDHCI >> select MMC_SDHCI_PLTFM >> default CPU_MMP2 > > Why not just move the header file to include/linux/platform_data/? > > Then you can build the driver on all platforms, which improves the > build test coverage. > > Arnd Hi, Arnd, Thanks a lot again :) It is really a good suggestion, is include/linux/platform_data/ is newly added? Will move the header file to platform_data/pxa_sdhci.h arch/arm/plat-pxa/include/plat/sdhci.h => include/linux/platform_data/pxa_sdhci.h > -- > To unsubscribe from this list: send the line "unsubscribe linux-mmc" 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-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
>> >> Why not just move the header file to include/linux/platform_data/? >> >> Then you can build the driver on all platforms, which improves the >> build test coverage. >> >> Arnd > > Hi, Arnd, > > Thanks a lot again :) > It is really a good suggestion, is include/linux/platform_data/ is newly added? > Will move the header file to platform_data/pxa_sdhci.h > arch/arm/plat-pxa/include/plat/sdhci.h => > include/linux/platform_data/pxa_sdhci.h In test of build sdhci-pxav3/2 with x86_64_defconfig, link error occurs for undefined clk_*, such as clk_enable/disable, etc. The CLKDEV_LOOKUP is selected in arch/arm/Kconfig, but not selected in arch/x86/Kconfig. Any suggestion? Thanks -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jun 20, 2011 at 6:02 PM, zhangfei gao <zhangfei.gao@gmail.com> wrote: >>> >>> Why not just move the header file to include/linux/platform_data/? >>> >>> Then you can build the driver on all platforms, which improves the >>> build test coverage. >>> >>> Arnd >> >> Hi, Arnd, >> >> Thanks a lot again :) >> It is really a good suggestion, is include/linux/platform_data/ is newly added? >> Will move the header file to platform_data/pxa_sdhci.h >> arch/arm/plat-pxa/include/plat/sdhci.h => >> include/linux/platform_data/pxa_sdhci.h > > In test of build sdhci-pxav3/2 with x86_64_defconfig, link error > occurs for undefined clk_*, such as clk_enable/disable, etc. > The CLKDEV_LOOKUP is selected in arch/arm/Kconfig, but not selected in > arch/x86/Kconfig. > Any suggestion? > > Thanks > Hi Arnd Currently, clk_enable, clk_disable, clk_get_rate realization are located at arch. Driver/clk/clkdev.c only realize clk_get/put. I am afraid it may difficult to build the driver on other arch, like x86, so we may still need "depends on ARCH_MMP" for safety. Thanks -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Monday 20 June 2011, zhangfei gao wrote: > Currently, clk_enable, clk_disable, clk_get_rate realization are > located at arch. > Driver/clk/clkdev.c only realize clk_get/put. > > I am afraid it may difficult to build the driver on other arch, like > x86, so we may still need "depends on ARCH_MMP" for safety. I would recommend turning this into "depends on CLKDEV_LOOKUP" instead. Always be specific with your dependencies so you enable the option on all platforms that are able to build it, but not on any of the others. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Monday 20 June 2011, zhangfei gao wrote: > It is really a good suggestion, is include/linux/platform_data/ is newly added? > Will move the header file to platform_data/pxa_sdhci.h > arch/arm/plat-pxa/include/plat/sdhci.h => > include/linux/platform_data/pxa_sdhci.h Yes, it is fairly new. The move looks correct to me. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jun 20, 2011 at 7:49 PM, Arnd Bergmann <arnd@arndb.de> wrote: > On Monday 20 June 2011, zhangfei gao wrote: >> Currently, clk_enable, clk_disable, clk_get_rate realization are >> located at arch. >> Driver/clk/clkdev.c only realize clk_get/put. >> >> I am afraid it may difficult to build the driver on other arch, like >> x86, so we may still need "depends on ARCH_MMP" for safety. > > I would recommend turning this into "depends on CLKDEV_LOOKUP" instead. > Always be specific with your dependencies so you enable the option > on all platforms that are able to build it, but not on any of the > others. > > Arnd > Select CLKDEV_LOOKUP is compile driver/clk/clkdev.c, which only provide clk_get, clk_put. Is it unsafe to assume all clk_get_rate/clk_enable/clk_disable are provided by the arch, which CLKDEV_LOOKUP? Any possibility CLKDEV_LOOKUP is selected but clk_get_rate is not realized. Thanks -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Monday 20 June 2011, zhangfei gao wrote: > Select CLKDEV_LOOKUP is compile driver/clk/clkdev.c, which only > provide clk_get, clk_put. > Is it unsafe to assume all clk_get_rate/clk_enable/clk_disable are > provided by the arch, which CLKDEV_LOOKUP? > Any possibility CLKDEV_LOOKUP is selected but clk_get_rate is not realized. I would consider that a bug, so you can ignore that case. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" 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/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig index 198ddda..d33a3e2 100644 --- a/drivers/mmc/host/Kconfig +++ b/drivers/mmc/host/Kconfig @@ -183,6 +183,7 @@ config MMC_SDHCI_S3C config MMC_SDHCI_PXAV3 tristate "Marvell MMP2 SD Host Controller support (PXAV3)" + depends on ARCH_MMP select MMC_SDHCI select MMC_SDHCI_PLTFM default CPU_MMP2 @@ -195,6 +196,7 @@ config MMC_SDHCI_PXAV3 config MMC_SDHCI_PXAV2 tristate "Marvell PXA9XX SD Host Controller support (PXAV2)" + depends on ARCH_MMP select MMC_SDHCI select MMC_SDHCI_PLTFM default CPU_PXA910
Without depends, MMC_SDHCI_PXAV3/2 can be chosen in other platfrom. Build error will occur since plat/sdhci.h is located at arch/arm/plat-pxa/ Signed-off-by: Zhangfei Gao <zhangfei.gao@marvell.com> --- drivers/mmc/host/Kconfig | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)