[PATCH] mmc: add dependence in Kconfig for pxav2/pxav3

[PATCH] mmc: add dependence in Kconfig for pxav2/pxav3

[Zhangfei Gao]

	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
@@ -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
-- 
1.7.0.4

Re: [PATCH] mmc: add dependence in Kconfig for pxav2/pxav3

[Arnd Bergmann]

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

Re: [PATCH] mmc: add dependence in Kconfig for pxav2/pxav3

[zhangfei gao]

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
>

Re: [PATCH] mmc: add dependence in Kconfig for pxav2/pxav3

[zhangfei gao]

>>
>> 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

Re: [PATCH] mmc: add dependence in Kconfig for pxav2/pxav3

[zhangfei gao]

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

Re: [PATCH] mmc: add dependence in Kconfig for pxav2/pxav3

[Arnd Bergmann]

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

Re: [PATCH] mmc: add dependence in Kconfig for pxav2/pxav3

[Arnd Bergmann]

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

Re: [PATCH] mmc: add dependence in Kconfig for pxav2/pxav3

[zhangfei gao]

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

Re: [PATCH] mmc: add dependence in Kconfig for pxav2/pxav3

[Arnd Bergmann]

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