[PATCH] i2c: cadence: fix Kconfig dependency

[PATCH] i2c: cadence: fix Kconfig dependency

[Wolfram Sang]

During development, the driver first really needed to depend on
COMMON_CLK only. Later, it was switched to writel_relaxed, but it was
forgotten to update the dependencies, so build errors occured:

config: make ARCH=i386 allyesconfig

All error/warnings:

   drivers/i2c/busses/i2c-cadence.c: In function 'cdns_i2c_clear_bus_hold':
>> drivers/i2c/busses/i2c-cadence.c:168:3: error: implicit declaration
>> of function 'writel_relaxed' [-Werror=implicit-function-declaration]

Use a very safe dependency for now.

Signed-off-by: Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>
---
 drivers/i2c/busses/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 93165ff453ab..3d3b9b3577c5 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -378,7 +378,7 @@ config I2C_BLACKFIN_TWI_CLK_KHZ
 
 config I2C_CADENCE
 	tristate "Cadence I2C Controller"
-	depends on COMMON_CLK
+	depends on ARCH_ZYNQ
 	help
 	  Say yes here to select Cadence I2C Host Controller. This controller is
 	  e.g. used by Xilinx Zynq.
-- 
1.9.1

[PATCH] i2c: cadence: fix Kconfig dependency

[Wolfram Sang]

During development, the driver first really needed to depend on
COMMON_CLK only. Later, it was switched to writel_relaxed, but it was
forgotten to update the dependencies, so build errors occured:

config: make ARCH=i386 allyesconfig

All error/warnings:

   drivers/i2c/busses/i2c-cadence.c: In function 'cdns_i2c_clear_bus_hold':
>> drivers/i2c/busses/i2c-cadence.c:168:3: error: implicit declaration
>> of function 'writel_relaxed' [-Werror=implicit-function-declaration]

Use a very safe dependency for now.

Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
---
 drivers/i2c/busses/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 93165ff453ab..3d3b9b3577c5 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -378,7 +378,7 @@ config I2C_BLACKFIN_TWI_CLK_KHZ
 
 config I2C_CADENCE
 	tristate "Cadence I2C Controller"
-	depends on COMMON_CLK
+	depends on ARCH_ZYNQ
 	help
 	  Say yes here to select Cadence I2C Host Controller. This controller is
 	  e.g. used by Xilinx Zynq.
-- 
1.9.1

Re: [PATCH] i2c: cadence: fix Kconfig dependency

[Sören Brinkmann]

Hi Wolfram,

On Sun, 2014-04-06 at 09:07PM +0200, Wolfram Sang wrote:
> During development, the driver first really needed to depend on
> COMMON_CLK only. Later, it was switched to writel_relaxed, but it was
> forgotten to update the dependencies, so build errors occured:
> 
> config: make ARCH=i386 allyesconfig
> 
> All error/warnings:
> 
>    drivers/i2c/busses/i2c-cadence.c: In function 'cdns_i2c_clear_bus_hold':
> >> drivers/i2c/busses/i2c-cadence.c:168:3: error: implicit declaration
> >> of function 'writel_relaxed' [-Werror=implicit-function-declaration]
> 
> Use a very safe dependency for now.

Thanks for taking care of this. Looks like the '_relaxed' IO helpers are
only available on ARM. I'll give that a try and might send a patch
eventually. I suspect 'ARM && COMMON_CLK' would work and be a little
less restrictive.

	Thanks,
	Sören

[PATCH] i2c: cadence: fix Kconfig dependency

[Sören Brinkmann]

Hi Wolfram,

On Sun, 2014-04-06 at 09:07PM +0200, Wolfram Sang wrote:
> During development, the driver first really needed to depend on
> COMMON_CLK only. Later, it was switched to writel_relaxed, but it was
> forgotten to update the dependencies, so build errors occured:
> 
> config: make ARCH=i386 allyesconfig
> 
> All error/warnings:
> 
>    drivers/i2c/busses/i2c-cadence.c: In function 'cdns_i2c_clear_bus_hold':
> >> drivers/i2c/busses/i2c-cadence.c:168:3: error: implicit declaration
> >> of function 'writel_relaxed' [-Werror=implicit-function-declaration]
> 
> Use a very safe dependency for now.

Thanks for taking care of this. Looks like the '_relaxed' IO helpers are
only available on ARM. I'll give that a try and might send a patch
eventually. I suspect 'ARM && COMMON_CLK' would work and be a little
less restrictive.

	Thanks,
	S?ren

Re: [PATCH] i2c: cadence: fix Kconfig dependency

[Uwe Kleine-König]

Hello,

On Sun, Apr 06, 2014 at 09:07:07PM +0200, Wolfram Sang wrote:
> During development, the driver first really needed to depend on
> COMMON_CLK only. Later, it was switched to writel_relaxed, but it was
> forgotten to update the dependencies, so build errors occured:
> 
> config: make ARCH=i386 allyesconfig
> 
> All error/warnings:
> 
>    drivers/i2c/busses/i2c-cadence.c: In function 'cdns_i2c_clear_bus_hold':
> >> drivers/i2c/busses/i2c-cadence.c:168:3: error: implicit declaration
> >> of function 'writel_relaxed' [-Werror=implicit-function-declaration]
> 
> Use a very safe dependency for now.
> 
> Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
> ---
>  drivers/i2c/busses/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index 93165ff453ab..3d3b9b3577c5 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -378,7 +378,7 @@ config I2C_BLACKFIN_TWI_CLK_KHZ
>  
>  config I2C_CADENCE
>  	tristate "Cadence I2C Controller"
> -	depends on COMMON_CLK
> +	depends on ARCH_ZYNQ
I'd suggest:

	depends on ARM && (ARCH_ZYNC || COMPILE_TEST)

This way it's available for compile testing but still it doesn't appear
when configuring a kernel for a real machine.

Best regards
Uwe

>  	help
>  	  Say yes here to select Cadence I2C Host Controller. This controller is
>  	  e.g. used by Xilinx Zynq.
> -- 
> 1.9.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[PATCH] i2c: cadence: fix Kconfig dependency

[Uwe Kleine-König]

Hello,

On Sun, Apr 06, 2014 at 09:07:07PM +0200, Wolfram Sang wrote:
> During development, the driver first really needed to depend on
> COMMON_CLK only. Later, it was switched to writel_relaxed, but it was
> forgotten to update the dependencies, so build errors occured:
> 
> config: make ARCH=i386 allyesconfig
> 
> All error/warnings:
> 
>    drivers/i2c/busses/i2c-cadence.c: In function 'cdns_i2c_clear_bus_hold':
> >> drivers/i2c/busses/i2c-cadence.c:168:3: error: implicit declaration
> >> of function 'writel_relaxed' [-Werror=implicit-function-declaration]
> 
> Use a very safe dependency for now.
> 
> Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
> ---
>  drivers/i2c/busses/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index 93165ff453ab..3d3b9b3577c5 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -378,7 +378,7 @@ config I2C_BLACKFIN_TWI_CLK_KHZ
>  
>  config I2C_CADENCE
>  	tristate "Cadence I2C Controller"
> -	depends on COMMON_CLK
> +	depends on ARCH_ZYNQ
I'd suggest:

	depends on ARM && (ARCH_ZYNC || COMPILE_TEST)

This way it's available for compile testing but still it doesn't appear
when configuring a kernel for a real machine.

Best regards
Uwe

>  	help
>  	  Say yes here to select Cadence I2C Host Controller. This controller is
>  	  e.g. used by Xilinx Zynq.
> -- 
> 1.9.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Re: [PATCH] i2c: cadence: fix Kconfig dependency

[Alexander Shiyan]

Mon, 7 Apr 2014 08:31:00 +0200 от Uwe Kleine-König <u.kleine-koenig@pengutronix.de>:
> Hello,
> 
> On Sun, Apr 06, 2014 at 09:07:07PM +0200, Wolfram Sang wrote:
> > During development, the driver first really needed to depend on
> > COMMON_CLK only. Later, it was switched to writel_relaxed, but it was
> > forgotten to update the dependencies, so build errors occured:
> > 
> > config: make ARCH=i386 allyesconfig
> > 
> > All error/warnings:
> > 
> >    drivers/i2c/busses/i2c-cadence.c: In function 'cdns_i2c_clear_bus_hold':
> > >> drivers/i2c/busses/i2c-cadence.c:168:3: error: implicit declaration
> > >> of function 'writel_relaxed' [-Werror=implicit-function-declaration]
> > 
> > Use a very safe dependency for now.
> > 
> > Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
> > ---
> >  drivers/i2c/busses/Kconfig | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> > index 93165ff453ab..3d3b9b3577c5 100644
> > --- a/drivers/i2c/busses/Kconfig
> > +++ b/drivers/i2c/busses/Kconfig
> > @@ -378,7 +378,7 @@ config I2C_BLACKFIN_TWI_CLK_KHZ
> >  
> >  config I2C_CADENCE
> >  	tristate "Cadence I2C Controller"
> > -	depends on COMMON_CLK
> > +	depends on ARCH_ZYNQ
> I'd suggest:
> 
> 	depends on ARM && (ARCH_ZYNC || COMPILE_TEST)

ARCH_ZYNC || (ARM && COMPILE_TEST)

Same, but more clear.

---

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] i2c: cadence: fix Kconfig dependency

[Alexander Shiyan]

Mon, 7 Apr 2014 08:31:00 +0200 ?? Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>:
> Hello,
> 
> On Sun, Apr 06, 2014 at 09:07:07PM +0200, Wolfram Sang wrote:
> > During development, the driver first really needed to depend on
> > COMMON_CLK only. Later, it was switched to writel_relaxed, but it was
> > forgotten to update the dependencies, so build errors occured:
> > 
> > config: make ARCH=i386 allyesconfig
> > 
> > All error/warnings:
> > 
> >    drivers/i2c/busses/i2c-cadence.c: In function 'cdns_i2c_clear_bus_hold':
> > >> drivers/i2c/busses/i2c-cadence.c:168:3: error: implicit declaration
> > >> of function 'writel_relaxed' [-Werror=implicit-function-declaration]
> > 
> > Use a very safe dependency for now.
> > 
> > Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
> > ---
> >  drivers/i2c/busses/Kconfig | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> > index 93165ff453ab..3d3b9b3577c5 100644
> > --- a/drivers/i2c/busses/Kconfig
> > +++ b/drivers/i2c/busses/Kconfig
> > @@ -378,7 +378,7 @@ config I2C_BLACKFIN_TWI_CLK_KHZ
> >  
> >  config I2C_CADENCE
> >  	tristate "Cadence I2C Controller"
> > -	depends on COMMON_CLK
> > +	depends on ARCH_ZYNQ
> I'd suggest:
> 
> 	depends on ARM && (ARCH_ZYNC || COMPILE_TEST)

ARCH_ZYNC || (ARM && COMPILE_TEST)

Same, but more clear.

---

Re: [PATCH] i2c: cadence: fix Kconfig dependency

[Michal Simek]

[-- Attachment #1: Type: text/plain, Size: 1913 bytes --]

On 04/07/2014 08:31 AM, Uwe Kleine-König wrote:
> Hello,
> 
> On Sun, Apr 06, 2014 at 09:07:07PM +0200, Wolfram Sang wrote:
>> During development, the driver first really needed to depend on
>> COMMON_CLK only. Later, it was switched to writel_relaxed, but it was
>> forgotten to update the dependencies, so build errors occured:
>>
>> config: make ARCH=i386 allyesconfig
>>
>> All error/warnings:
>>
>>    drivers/i2c/busses/i2c-cadence.c: In function 'cdns_i2c_clear_bus_hold':
>>>> drivers/i2c/busses/i2c-cadence.c:168:3: error: implicit declaration
>>>> of function 'writel_relaxed' [-Werror=implicit-function-declaration]
>>
>> Use a very safe dependency for now.
>>
>> Signed-off-by: Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>
>> ---
>>  drivers/i2c/busses/Kconfig | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
>> index 93165ff453ab..3d3b9b3577c5 100644
>> --- a/drivers/i2c/busses/Kconfig
>> +++ b/drivers/i2c/busses/Kconfig
>> @@ -378,7 +378,7 @@ config I2C_BLACKFIN_TWI_CLK_KHZ
>>  
>>  config I2C_CADENCE
>>  	tristate "Cadence I2C Controller"
>> -	depends on COMMON_CLK
>> +	depends on ARCH_ZYNQ
> I'd suggest:
> 
> 	depends on ARM && (ARCH_ZYNC || COMPILE_TEST)
> 
> This way it's available for compile testing but still it doesn't appear
> when configuring a kernel for a real machine.

I would suggest to add relaxed IO helper function to asm-generic/io.h
to be available for all archs. Then we can avoid this type of errors.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

[PATCH] i2c: cadence: fix Kconfig dependency

[Michal Simek]

On 04/07/2014 08:31 AM, Uwe Kleine-K?nig wrote:
> Hello,
> 
> On Sun, Apr 06, 2014 at 09:07:07PM +0200, Wolfram Sang wrote:
>> During development, the driver first really needed to depend on
>> COMMON_CLK only. Later, it was switched to writel_relaxed, but it was
>> forgotten to update the dependencies, so build errors occured:
>>
>> config: make ARCH=i386 allyesconfig
>>
>> All error/warnings:
>>
>>    drivers/i2c/busses/i2c-cadence.c: In function 'cdns_i2c_clear_bus_hold':
>>>> drivers/i2c/busses/i2c-cadence.c:168:3: error: implicit declaration
>>>> of function 'writel_relaxed' [-Werror=implicit-function-declaration]
>>
>> Use a very safe dependency for now.
>>
>> Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
>> ---
>>  drivers/i2c/busses/Kconfig | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
>> index 93165ff453ab..3d3b9b3577c5 100644
>> --- a/drivers/i2c/busses/Kconfig
>> +++ b/drivers/i2c/busses/Kconfig
>> @@ -378,7 +378,7 @@ config I2C_BLACKFIN_TWI_CLK_KHZ
>>  
>>  config I2C_CADENCE
>>  	tristate "Cadence I2C Controller"
>> -	depends on COMMON_CLK
>> +	depends on ARCH_ZYNQ
> I'd suggest:
> 
> 	depends on ARM && (ARCH_ZYNC || COMPILE_TEST)
> 
> This way it's available for compile testing but still it doesn't appear
> when configuring a kernel for a real machine.

I would suggest to add relaxed IO helper function to asm-generic/io.h
to be available for all archs. Then we can avoid this type of errors.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 263 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140407/2635f234/attachment.sig>

Re: [PATCH] i2c: cadence: fix Kconfig dependency

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 251 bytes --]


> I would suggest to add relaxed IO helper function to asm-generic/io.h
> to be available for all archs. Then we can avoid this type of errors.

I favour this, too. For the cadence driver, I needed a quick solution
before linux-next breaks, though.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH] i2c: cadence: fix Kconfig dependency

[Wolfram Sang]

> I would suggest to add relaxed IO helper function to asm-generic/io.h
> to be available for all archs. Then we can avoid this type of errors.

I favour this, too. For the cadence driver, I needed a quick solution
before linux-next breaks, though.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140407/b36acb98/attachment.sig>

Re: [PATCH] i2c: cadence: fix Kconfig dependency

[Uwe Kleine-König]

Hello,

On Mon, Apr 07, 2014 at 10:37:36AM +0400, Alexander Shiyan wrote:
> Mon, 7 Apr 2014 08:31:00 +0200 от Uwe Kleine-König <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>:
> > Hello,
> > 
> > On Sun, Apr 06, 2014 at 09:07:07PM +0200, Wolfram Sang wrote:
> > > During development, the driver first really needed to depend on
> > > COMMON_CLK only. Later, it was switched to writel_relaxed, but it was
> > > forgotten to update the dependencies, so build errors occured:
> > > 
> > > config: make ARCH=i386 allyesconfig
> > > 
> > > All error/warnings:
> > > 
> > >    drivers/i2c/busses/i2c-cadence.c: In function 'cdns_i2c_clear_bus_hold':
> > > >> drivers/i2c/busses/i2c-cadence.c:168:3: error: implicit declaration
> > > >> of function 'writel_relaxed' [-Werror=implicit-function-declaration]
> > > 
> > > Use a very safe dependency for now.
> > > 
> > > Signed-off-by: Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>
> > > ---
> > >  drivers/i2c/busses/Kconfig | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> > > index 93165ff453ab..3d3b9b3577c5 100644
> > > --- a/drivers/i2c/busses/Kconfig
> > > +++ b/drivers/i2c/busses/Kconfig
> > > @@ -378,7 +378,7 @@ config I2C_BLACKFIN_TWI_CLK_KHZ
> > >  
> > >  config I2C_CADENCE
> > >  	tristate "Cadence I2C Controller"
> > > -	depends on COMMON_CLK
> > > +	depends on ARCH_ZYNQ
> > I'd suggest:
> > 
> > 	depends on ARM && (ARCH_ZYNC || COMPILE_TEST)
> 
> ARCH_ZYNC || (ARM && COMPILE_TEST)
> 
> Same, but more clear.
"more clear" might be subjective, at least for me I don't see a benefit
from one over the other.

hmm, considering a (hypothetical) driver that depends on I2C and is
available on Zync but compiles on ARM, it would need either: 

	ARM && I2C && (ZYNC || COMPILE_TEST)

or

	ZYNC && I2C || (ARM && I2C && COMPILE_TEST)

or

	I2C && (ARCH_ZYNC || (ARM && COMPILE_TEST))

In this case the first variant is the best, isn't it? Is this a reason
to stick to this scheme even if there in the simpler case that is in
question here? Probably not?!

But anyhow, my main concern was the introduction of COMPILE_TEST, I
don't care much about the actual expression used. So use whatever you
want.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[PATCH] i2c: cadence: fix Kconfig dependency

[Uwe Kleine-König]

Hello,

On Mon, Apr 07, 2014 at 10:37:36AM +0400, Alexander Shiyan wrote:
> Mon, 7 Apr 2014 08:31:00 +0200 ?? Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>:
> > Hello,
> > 
> > On Sun, Apr 06, 2014 at 09:07:07PM +0200, Wolfram Sang wrote:
> > > During development, the driver first really needed to depend on
> > > COMMON_CLK only. Later, it was switched to writel_relaxed, but it was
> > > forgotten to update the dependencies, so build errors occured:
> > > 
> > > config: make ARCH=i386 allyesconfig
> > > 
> > > All error/warnings:
> > > 
> > >    drivers/i2c/busses/i2c-cadence.c: In function 'cdns_i2c_clear_bus_hold':
> > > >> drivers/i2c/busses/i2c-cadence.c:168:3: error: implicit declaration
> > > >> of function 'writel_relaxed' [-Werror=implicit-function-declaration]
> > > 
> > > Use a very safe dependency for now.
> > > 
> > > Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
> > > ---
> > >  drivers/i2c/busses/Kconfig | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> > > index 93165ff453ab..3d3b9b3577c5 100644
> > > --- a/drivers/i2c/busses/Kconfig
> > > +++ b/drivers/i2c/busses/Kconfig
> > > @@ -378,7 +378,7 @@ config I2C_BLACKFIN_TWI_CLK_KHZ
> > >  
> > >  config I2C_CADENCE
> > >  	tristate "Cadence I2C Controller"
> > > -	depends on COMMON_CLK
> > > +	depends on ARCH_ZYNQ
> > I'd suggest:
> > 
> > 	depends on ARM && (ARCH_ZYNC || COMPILE_TEST)
> 
> ARCH_ZYNC || (ARM && COMPILE_TEST)
> 
> Same, but more clear.
"more clear" might be subjective, at least for me I don't see a benefit
from one over the other.

hmm, considering a (hypothetical) driver that depends on I2C and is
available on Zync but compiles on ARM, it would need either: 

	ARM && I2C && (ZYNC || COMPILE_TEST)

or

	ZYNC && I2C || (ARM && I2C && COMPILE_TEST)

or

	I2C && (ARCH_ZYNC || (ARM && COMPILE_TEST))

In this case the first variant is the best, isn't it? Is this a reason
to stick to this scheme even if there in the simpler case that is in
question here? Probably not?!

But anyhow, my main concern was the introduction of COMPILE_TEST, I
don't care much about the actual expression used. So use whatever you
want.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Re: [PATCH] i2c: cadence: fix Kconfig dependency

[Michal Simek]

[-- Attachment #1: Type: text/plain, Size: 782 bytes --]

On 04/07/2014 08:56 AM, Wolfram Sang wrote:
> 
>> I would suggest to add relaxed IO helper function to asm-generic/io.h
>> to be available for all archs. Then we can avoid this type of errors.
> 
> I favour this, too. For the cadence driver, I needed a quick solution
> before linux-next breaks, though.

Sure. That's understandable.

Before my comment I have sent this to Russell too.
http://www.spinics.net/lists/arm-kernel/msg320285.html

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

[PATCH] i2c: cadence: fix Kconfig dependency

[Michal Simek]

On 04/07/2014 08:56 AM, Wolfram Sang wrote:
> 
>> I would suggest to add relaxed IO helper function to asm-generic/io.h
>> to be available for all archs. Then we can avoid this type of errors.
> 
> I favour this, too. For the cadence driver, I needed a quick solution
> before linux-next breaks, though.

Sure. That's understandable.

Before my comment I have sent this to Russell too.
http://www.spinics.net/lists/arm-kernel/msg320285.html

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 263 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140407/9bcfaa59/attachment.sig>

Re: [PATCH] i2c: cadence: fix Kconfig dependency

[Alexander Shiyan]

Mon, 7 Apr 2014 08:56:57 +0200 от Uwe Kleine-König <u.kleine-koenig@pengutronix.de>:
> Hello,
...
> > > On Sun, Apr 06, 2014 at 09:07:07PM +0200, Wolfram Sang wrote:
> > > > During development, the driver first really needed to depend on
> > > > COMMON_CLK only. Later, it was switched to writel_relaxed, but it was
> > > > forgotten to update the dependencies, so build errors occured:
> > > > 
> > > > config: make ARCH=i386 allyesconfig
> > > > 
> > > > All error/warnings:
> > > > 
> > > >    drivers/i2c/busses/i2c-cadence.c: In function 'cdns_i2c_clear_bus_hold':
> > > > >> drivers/i2c/busses/i2c-cadence.c:168:3: error: implicit declaration
> > > > >> of function 'writel_relaxed' [-Werror=implicit-function-declaration]
> > > > 
> > > > Use a very safe dependency for now.
> > > > 
> > > > Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
> > > > ---
> > > >  drivers/i2c/busses/Kconfig | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> > > > index 93165ff453ab..3d3b9b3577c5 100644
> > > > --- a/drivers/i2c/busses/Kconfig
> > > > +++ b/drivers/i2c/busses/Kconfig
> > > > @@ -378,7 +378,7 @@ config I2C_BLACKFIN_TWI_CLK_KHZ
> > > >  
> > > >  config I2C_CADENCE
> > > >  	tristate "Cadence I2C Controller"
> > > > -	depends on COMMON_CLK
> > > > +	depends on ARCH_ZYNQ
> > > I'd suggest:
> > > 
> > > 	depends on ARM && (ARCH_ZYNC || COMPILE_TEST)
> > 
> > ARCH_ZYNC || (ARM && COMPILE_TEST)
> > 
> > Same, but more clear.
> "more clear" might be subjective, at least for me I don't see a benefit
> from one over the other.
> 
> hmm, considering a (hypothetical) driver that depends on I2C and is
> available on Zync but compiles on ARM, it would need either: 

No. Look at drivers/i2c/Kconfig:

"source drivers/i2c/busses/Kconfig" is under "if I2C" already.

---

Re: [PATCH] i2c: cadence: fix Kconfig dependency

[Alexander Shiyan]

Mon, 7 Apr 2014 08:56:57 +0200 ?? Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>:
> Hello,
...
> > > On Sun, Apr 06, 2014 at 09:07:07PM +0200, Wolfram Sang wrote:
> > > > During development, the driver first really needed to depend on
> > > > COMMON_CLK only. Later, it was switched to writel_relaxed, but it was
> > > > forgotten to update the dependencies, so build errors occured:
> > > > 
> > > > config: make ARCH=i386 allyesconfig
> > > > 
> > > > All error/warnings:
> > > > 
> > > >    drivers/i2c/busses/i2c-cadence.c: In function 'cdns_i2c_clear_bus_hold':
> > > > >> drivers/i2c/busses/i2c-cadence.c:168:3: error: implicit declaration
> > > > >> of function 'writel_relaxed' [-Werror=implicit-function-declaration]
> > > > 
> > > > Use a very safe dependency for now.
> > > > 
> > > > Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
> > > > ---
> > > >  drivers/i2c/busses/Kconfig | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> > > > index 93165ff453ab..3d3b9b3577c5 100644
> > > > --- a/drivers/i2c/busses/Kconfig
> > > > +++ b/drivers/i2c/busses/Kconfig
> > > > @@ -378,7 +378,7 @@ config I2C_BLACKFIN_TWI_CLK_KHZ
> > > >  
> > > >  config I2C_CADENCE
> > > >  	tristate "Cadence I2C Controller"
> > > > -	depends on COMMON_CLK
> > > > +	depends on ARCH_ZYNQ
> > > I'd suggest:
> > > 
> > > 	depends on ARM && (ARCH_ZYNC || COMPILE_TEST)
> > 
> > ARCH_ZYNC || (ARM && COMPILE_TEST)
> > 
> > Same, but more clear.
> "more clear" might be subjective, at least for me I don't see a benefit
> from one over the other.
> 
> hmm, considering a (hypothetical) driver that depends on I2C and is
> available on Zync but compiles on ARM, it would need either: 

No. Look at drivers/i2c/Kconfig:

"source drivers/i2c/busses/Kconfig" is under "if I2C" already.

---

Re: [PATCH] i2c: cadence: fix Kconfig dependency

[Sören Brinkmann]

On Mon, 2014-04-07 at 10:37AM +0400, Alexander Shiyan wrote:
> Mon, 7 Apr 2014 08:31:00 +0200 от Uwe Kleine-König <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>:
> > Hello,
> > 
> > On Sun, Apr 06, 2014 at 09:07:07PM +0200, Wolfram Sang wrote:
> > > During development, the driver first really needed to depend on
> > > COMMON_CLK only. Later, it was switched to writel_relaxed, but it was
> > > forgotten to update the dependencies, so build errors occured:
> > > 
> > > config: make ARCH=i386 allyesconfig
> > > 
> > > All error/warnings:
> > > 
> > >    drivers/i2c/busses/i2c-cadence.c: In function 'cdns_i2c_clear_bus_hold':
> > > >> drivers/i2c/busses/i2c-cadence.c:168:3: error: implicit declaration
> > > >> of function 'writel_relaxed' [-Werror=implicit-function-declaration]
> > > 
> > > Use a very safe dependency for now.
> > > 
> > > Signed-off-by: Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>
> > > ---
> > >  drivers/i2c/busses/Kconfig | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> > > index 93165ff453ab..3d3b9b3577c5 100644
> > > --- a/drivers/i2c/busses/Kconfig
> > > +++ b/drivers/i2c/busses/Kconfig
> > > @@ -378,7 +378,7 @@ config I2C_BLACKFIN_TWI_CLK_KHZ
> > >  
> > >  config I2C_CADENCE
> > >  	tristate "Cadence I2C Controller"
> > > -	depends on COMMON_CLK
> > > +	depends on ARCH_ZYNQ
> > I'd suggest:
> > 
> > 	depends on ARM && (ARCH_ZYNC || COMPILE_TEST)
> 
> ARCH_ZYNC || (ARM && COMPILE_TEST)

Why would you add Zynq to the dependencies? This driver should work on
all ARM platforms (more or less, given that the IP is implemented
similarly). I think ARM, if it selects COMMON_CLK, is enough. Otherwise
ARM && COMMON_CLK. I don't see a good reason to restrict the driver to
Zynq only.

	Sören

[PATCH] i2c: cadence: fix Kconfig dependency

[Sören Brinkmann]

On Mon, 2014-04-07 at 10:37AM +0400, Alexander Shiyan wrote:
> Mon, 7 Apr 2014 08:31:00 +0200 ?? Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>:
> > Hello,
> > 
> > On Sun, Apr 06, 2014 at 09:07:07PM +0200, Wolfram Sang wrote:
> > > During development, the driver first really needed to depend on
> > > COMMON_CLK only. Later, it was switched to writel_relaxed, but it was
> > > forgotten to update the dependencies, so build errors occured:
> > > 
> > > config: make ARCH=i386 allyesconfig
> > > 
> > > All error/warnings:
> > > 
> > >    drivers/i2c/busses/i2c-cadence.c: In function 'cdns_i2c_clear_bus_hold':
> > > >> drivers/i2c/busses/i2c-cadence.c:168:3: error: implicit declaration
> > > >> of function 'writel_relaxed' [-Werror=implicit-function-declaration]
> > > 
> > > Use a very safe dependency for now.
> > > 
> > > Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
> > > ---
> > >  drivers/i2c/busses/Kconfig | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> > > index 93165ff453ab..3d3b9b3577c5 100644
> > > --- a/drivers/i2c/busses/Kconfig
> > > +++ b/drivers/i2c/busses/Kconfig
> > > @@ -378,7 +378,7 @@ config I2C_BLACKFIN_TWI_CLK_KHZ
> > >  
> > >  config I2C_CADENCE
> > >  	tristate "Cadence I2C Controller"
> > > -	depends on COMMON_CLK
> > > +	depends on ARCH_ZYNQ
> > I'd suggest:
> > 
> > 	depends on ARM && (ARCH_ZYNC || COMPILE_TEST)
> 
> ARCH_ZYNC || (ARM && COMPILE_TEST)

Why would you add Zynq to the dependencies? This driver should work on
all ARM platforms (more or less, given that the IP is implemented
similarly). I think ARM, if it selects COMMON_CLK, is enough. Otherwise
ARM && COMMON_CLK. I don't see a good reason to restrict the driver to
Zynq only.

	S?ren

Re: [PATCH] i2c: cadence: fix Kconfig dependency

[Sören Brinkmann]

On Mon, 2014-04-07 at 08:39AM +0200, Michal Simek wrote:
> On 04/07/2014 08:31 AM, Uwe Kleine-König wrote:
> > Hello,
> > 
> > On Sun, Apr 06, 2014 at 09:07:07PM +0200, Wolfram Sang wrote:
> >> During development, the driver first really needed to depend on
> >> COMMON_CLK only. Later, it was switched to writel_relaxed, but it was
> >> forgotten to update the dependencies, so build errors occured:
> >>
> >> config: make ARCH=i386 allyesconfig
> >>
> >> All error/warnings:
> >>
> >>    drivers/i2c/busses/i2c-cadence.c: In function 'cdns_i2c_clear_bus_hold':
> >>>> drivers/i2c/busses/i2c-cadence.c:168:3: error: implicit declaration
> >>>> of function 'writel_relaxed' [-Werror=implicit-function-declaration]
> >>
> >> Use a very safe dependency for now.
> >>
> >> Signed-off-by: Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>
> >> ---
> >>  drivers/i2c/busses/Kconfig | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> >> index 93165ff453ab..3d3b9b3577c5 100644
> >> --- a/drivers/i2c/busses/Kconfig
> >> +++ b/drivers/i2c/busses/Kconfig
> >> @@ -378,7 +378,7 @@ config I2C_BLACKFIN_TWI_CLK_KHZ
> >>  
> >>  config I2C_CADENCE
> >>  	tristate "Cadence I2C Controller"
> >> -	depends on COMMON_CLK
> >> +	depends on ARCH_ZYNQ
> > I'd suggest:
> > 
> > 	depends on ARM && (ARCH_ZYNC || COMPILE_TEST)
> > 
> > This way it's available for compile testing but still it doesn't appear
> > when configuring a kernel for a real machine.
> 
> I would suggest to add relaxed IO helper function to asm-generic/io.h
> to be available for all archs. Then we can avoid this type of errors.

ACK

	Sören

[PATCH] i2c: cadence: fix Kconfig dependency

[Sören Brinkmann]

On Mon, 2014-04-07 at 08:39AM +0200, Michal Simek wrote:
> On 04/07/2014 08:31 AM, Uwe Kleine-K?nig wrote:
> > Hello,
> > 
> > On Sun, Apr 06, 2014 at 09:07:07PM +0200, Wolfram Sang wrote:
> >> During development, the driver first really needed to depend on
> >> COMMON_CLK only. Later, it was switched to writel_relaxed, but it was
> >> forgotten to update the dependencies, so build errors occured:
> >>
> >> config: make ARCH=i386 allyesconfig
> >>
> >> All error/warnings:
> >>
> >>    drivers/i2c/busses/i2c-cadence.c: In function 'cdns_i2c_clear_bus_hold':
> >>>> drivers/i2c/busses/i2c-cadence.c:168:3: error: implicit declaration
> >>>> of function 'writel_relaxed' [-Werror=implicit-function-declaration]
> >>
> >> Use a very safe dependency for now.
> >>
> >> Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
> >> ---
> >>  drivers/i2c/busses/Kconfig | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> >> index 93165ff453ab..3d3b9b3577c5 100644
> >> --- a/drivers/i2c/busses/Kconfig
> >> +++ b/drivers/i2c/busses/Kconfig
> >> @@ -378,7 +378,7 @@ config I2C_BLACKFIN_TWI_CLK_KHZ
> >>  
> >>  config I2C_CADENCE
> >>  	tristate "Cadence I2C Controller"
> >> -	depends on COMMON_CLK
> >> +	depends on ARCH_ZYNQ
> > I'd suggest:
> > 
> > 	depends on ARM && (ARCH_ZYNC || COMPILE_TEST)
> > 
> > This way it's available for compile testing but still it doesn't appear
> > when configuring a kernel for a real machine.
> 
> I would suggest to add relaxed IO helper function to asm-generic/io.h
> to be available for all archs. Then we can avoid this type of errors.

ACK

	S?ren

Re: [PATCH] i2c: cadence: fix Kconfig dependency

[Sören Brinkmann]

On Mon, 2014-04-07 at 08:31AM +0200, Uwe Kleine-König wrote:
> Hello,
> 
> On Sun, Apr 06, 2014 at 09:07:07PM +0200, Wolfram Sang wrote:
> > During development, the driver first really needed to depend on
> > COMMON_CLK only. Later, it was switched to writel_relaxed, but it was
> > forgotten to update the dependencies, so build errors occured:
> > 
> > config: make ARCH=i386 allyesconfig
> > 
> > All error/warnings:
> > 
> >    drivers/i2c/busses/i2c-cadence.c: In function 'cdns_i2c_clear_bus_hold':
> > >> drivers/i2c/busses/i2c-cadence.c:168:3: error: implicit declaration
> > >> of function 'writel_relaxed' [-Werror=implicit-function-declaration]
> > 
> > Use a very safe dependency for now.
> > 
> > Signed-off-by: Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>
> > ---
> >  drivers/i2c/busses/Kconfig | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> > index 93165ff453ab..3d3b9b3577c5 100644
> > --- a/drivers/i2c/busses/Kconfig
> > +++ b/drivers/i2c/busses/Kconfig
> > @@ -378,7 +378,7 @@ config I2C_BLACKFIN_TWI_CLK_KHZ
> >  
> >  config I2C_CADENCE
> >  	tristate "Cadence I2C Controller"
> > -	depends on COMMON_CLK
> > +	depends on ARCH_ZYNQ
> I'd suggest:
> 
> 	depends on ARM && (ARCH_ZYNC || COMPILE_TEST)

This misses COMMON_CLK in the COMPILE_TEST case.

	Sören

[PATCH] i2c: cadence: fix Kconfig dependency

[Sören Brinkmann]

On Mon, 2014-04-07 at 08:31AM +0200, Uwe Kleine-K?nig wrote:
> Hello,
> 
> On Sun, Apr 06, 2014 at 09:07:07PM +0200, Wolfram Sang wrote:
> > During development, the driver first really needed to depend on
> > COMMON_CLK only. Later, it was switched to writel_relaxed, but it was
> > forgotten to update the dependencies, so build errors occured:
> > 
> > config: make ARCH=i386 allyesconfig
> > 
> > All error/warnings:
> > 
> >    drivers/i2c/busses/i2c-cadence.c: In function 'cdns_i2c_clear_bus_hold':
> > >> drivers/i2c/busses/i2c-cadence.c:168:3: error: implicit declaration
> > >> of function 'writel_relaxed' [-Werror=implicit-function-declaration]
> > 
> > Use a very safe dependency for now.
> > 
> > Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
> > ---
> >  drivers/i2c/busses/Kconfig | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> > index 93165ff453ab..3d3b9b3577c5 100644
> > --- a/drivers/i2c/busses/Kconfig
> > +++ b/drivers/i2c/busses/Kconfig
> > @@ -378,7 +378,7 @@ config I2C_BLACKFIN_TWI_CLK_KHZ
> >  
> >  config I2C_CADENCE
> >  	tristate "Cadence I2C Controller"
> > -	depends on COMMON_CLK
> > +	depends on ARCH_ZYNQ
> I'd suggest:
> 
> 	depends on ARM && (ARCH_ZYNC || COMPILE_TEST)

This misses COMMON_CLK in the COMPILE_TEST case.

	S?ren

Re: [PATCH] i2c: cadence: fix Kconfig dependency

[Sören Brinkmann]

On Mon, 2014-04-07 at 11:02AM +0400, Alexander Shiyan wrote:
> Mon, 7 Apr 2014 08:56:57 +0200 от Uwe Kleine-König <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>:
> > Hello,
> ...
> > > > On Sun, Apr 06, 2014 at 09:07:07PM +0200, Wolfram Sang wrote:
> > > > > During development, the driver first really needed to depend on
> > > > > COMMON_CLK only. Later, it was switched to writel_relaxed, but it was
> > > > > forgotten to update the dependencies, so build errors occured:
> > > > > 
> > > > > config: make ARCH=i386 allyesconfig
> > > > > 
> > > > > All error/warnings:
> > > > > 
> > > > >    drivers/i2c/busses/i2c-cadence.c: In function 'cdns_i2c_clear_bus_hold':
> > > > > >> drivers/i2c/busses/i2c-cadence.c:168:3: error: implicit declaration
> > > > > >> of function 'writel_relaxed' [-Werror=implicit-function-declaration]
> > > > > 
> > > > > Use a very safe dependency for now.
> > > > > 
> > > > > Signed-off-by: Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>
> > > > > ---
> > > > >  drivers/i2c/busses/Kconfig | 2 +-
> > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> > > > > index 93165ff453ab..3d3b9b3577c5 100644
> > > > > --- a/drivers/i2c/busses/Kconfig
> > > > > +++ b/drivers/i2c/busses/Kconfig
> > > > > @@ -378,7 +378,7 @@ config I2C_BLACKFIN_TWI_CLK_KHZ
> > > > >  
> > > > >  config I2C_CADENCE
> > > > >  	tristate "Cadence I2C Controller"
> > > > > -	depends on COMMON_CLK
> > > > > +	depends on ARCH_ZYNQ
> > > > I'd suggest:
> > > > 
> > > > 	depends on ARM && (ARCH_ZYNC || COMPILE_TEST)
> > > 
> > > ARCH_ZYNC || (ARM && COMPILE_TEST)
> > > 
> > > Same, but more clear.
> > "more clear" might be subjective, at least for me I don't see a benefit
> > from one over the other.
> > 
> > hmm, considering a (hypothetical) driver that depends on I2C and is
> > available on Zync but compiles on ARM, it would need either: 
> 
> No. Look at drivers/i2c/Kconfig:

I'd propose the below fix. The relaxed IO helpers are currently
available on ARM only and the clock notifier used, depends on
COMMON_CLK. I2C-related dependencies should be okay due to the location in
Kconfig of this option.

	Thanks,
	Sören

---------------8<---------------8<----------------8<-----------------8<-----------8<-------
Date: Sun, 6 Apr 2014 15:19:34 -0700
Subject: [PATCH] i2c: cadence: Add dependency on ARM

Due to switching to the (read|write)l_relaxed IO helpers, building this
driver fails on non-ARM architectures, which don't provide these helpers
with the following error:
  All error/warnings:

     drivers/i2c/busses/i2c-cadence.c: In function 'cdns_i2c_clear_bus_hold':
  >> drivers/i2c/busses/i2c-cadence.c:168:3: error: implicit declaration of function 'writel_relaxed' [-Werror=implicit-function-declaration]
        cdns_i2c_writereg(reg & ~CDNS_I2C_CR_HOLD, CDNS_I2C_CR_OFFSET);
        ^
     cc1: some warnings being treated as errors

Make the driver additionally depend on CONFIG_ARM to fix this.

Reported-by: kbuild test robot <fengguang.wu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Soren Brinkmann <soren.brinkmann-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
---
 drivers/i2c/busses/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 66e68e48218f..9f546eecd322 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -377,7 +377,7 @@ config I2C_BLACKFIN_TWI_CLK_KHZ
 
 config I2C_CADENCE
 	tristate "Cadence I2C Controller"
-	depends on COMMON_CLK
+	depends on ARM && COMMON_CLK
 	help
 	  Say yes here to select Cadence I2C Host Controller. This controller is
 	  e.g. used by Xilinx Zynq.
-- 
1.9.1.1.gbb9f595

[PATCH] i2c: cadence: fix Kconfig dependency

[Sören Brinkmann]

On Mon, 2014-04-07 at 11:02AM +0400, Alexander Shiyan wrote:
> Mon, 7 Apr 2014 08:56:57 +0200 ?? Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>:
> > Hello,
> ...
> > > > On Sun, Apr 06, 2014 at 09:07:07PM +0200, Wolfram Sang wrote:
> > > > > During development, the driver first really needed to depend on
> > > > > COMMON_CLK only. Later, it was switched to writel_relaxed, but it was
> > > > > forgotten to update the dependencies, so build errors occured:
> > > > > 
> > > > > config: make ARCH=i386 allyesconfig
> > > > > 
> > > > > All error/warnings:
> > > > > 
> > > > >    drivers/i2c/busses/i2c-cadence.c: In function 'cdns_i2c_clear_bus_hold':
> > > > > >> drivers/i2c/busses/i2c-cadence.c:168:3: error: implicit declaration
> > > > > >> of function 'writel_relaxed' [-Werror=implicit-function-declaration]
> > > > > 
> > > > > Use a very safe dependency for now.
> > > > > 
> > > > > Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
> > > > > ---
> > > > >  drivers/i2c/busses/Kconfig | 2 +-
> > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> > > > > index 93165ff453ab..3d3b9b3577c5 100644
> > > > > --- a/drivers/i2c/busses/Kconfig
> > > > > +++ b/drivers/i2c/busses/Kconfig
> > > > > @@ -378,7 +378,7 @@ config I2C_BLACKFIN_TWI_CLK_KHZ
> > > > >  
> > > > >  config I2C_CADENCE
> > > > >  	tristate "Cadence I2C Controller"
> > > > > -	depends on COMMON_CLK
> > > > > +	depends on ARCH_ZYNQ
> > > > I'd suggest:
> > > > 
> > > > 	depends on ARM && (ARCH_ZYNC || COMPILE_TEST)
> > > 
> > > ARCH_ZYNC || (ARM && COMPILE_TEST)
> > > 
> > > Same, but more clear.
> > "more clear" might be subjective, at least for me I don't see a benefit
> > from one over the other.
> > 
> > hmm, considering a (hypothetical) driver that depends on I2C and is
> > available on Zync but compiles on ARM, it would need either: 
> 
> No. Look at drivers/i2c/Kconfig:

I'd propose the below fix. The relaxed IO helpers are currently
available on ARM only and the clock notifier used, depends on
COMMON_CLK. I2C-related dependencies should be okay due to the location in
Kconfig of this option.

	Thanks,
	S?ren

---------------8<---------------8<----------------8<-----------------8<-----------8<-------
Date: Sun, 6 Apr 2014 15:19:34 -0700
Subject: [PATCH] i2c: cadence: Add dependency on ARM

Due to switching to the (read|write)l_relaxed IO helpers, building this
driver fails on non-ARM architectures, which don't provide these helpers
with the following error:
  All error/warnings:

     drivers/i2c/busses/i2c-cadence.c: In function 'cdns_i2c_clear_bus_hold':
  >> drivers/i2c/busses/i2c-cadence.c:168:3: error: implicit declaration of function 'writel_relaxed' [-Werror=implicit-function-declaration]
        cdns_i2c_writereg(reg & ~CDNS_I2C_CR_HOLD, CDNS_I2C_CR_OFFSET);
        ^
     cc1: some warnings being treated as errors

Make the driver additionally depend on CONFIG_ARM to fix this.

Reported-by: kbuild test robot <fengguang.wu@intel.com>
Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
---
 drivers/i2c/busses/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 66e68e48218f..9f546eecd322 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -377,7 +377,7 @@ config I2C_BLACKFIN_TWI_CLK_KHZ
 
 config I2C_CADENCE
 	tristate "Cadence I2C Controller"
-	depends on COMMON_CLK
+	depends on ARM && COMMON_CLK
 	help
 	  Say yes here to select Cadence I2C Host Controller. This controller is
 	  e.g. used by Xilinx Zynq.
-- 
1.9.1.1.gbb9f595

Re: [PATCH] i2c: cadence: fix Kconfig dependency

[Uwe Kleine-König]

Hello Sören,

On Mon, Apr 07, 2014 at 08:36:29AM -0700, Sören Brinkmann wrote:
> On Mon, 2014-04-07 at 10:37AM +0400, Alexander Shiyan wrote:
> > ARCH_ZYNC || (ARM && COMPILE_TEST)
> 
> Why would you add Zynq to the dependencies? This driver should work on
> all ARM platforms (more or less, given that the IP is implemented
> similarly). I think ARM, if it selects COMMON_CLK, is enough. Otherwise
> ARM && COMMON_CLK. I don't see a good reason to restrict the driver to
> Zynq only.
The idea is that if COMPILE_TEST is off you can only enable the driver
for machines where the device in question really exists to simplify
kernel configuration. Sometimes this isn't really possible, e.g. for i2c
devices that can be put on more or less every machine. I had the
impression that the cadence i2c bus device only exists on Zync? Ok, you
could implement this device on every SoC that has some programmable
hardware, but that is not the point.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[PATCH] i2c: cadence: fix Kconfig dependency

[Uwe Kleine-König]

Hello S?ren,

On Mon, Apr 07, 2014 at 08:36:29AM -0700, S?ren Brinkmann wrote:
> On Mon, 2014-04-07 at 10:37AM +0400, Alexander Shiyan wrote:
> > ARCH_ZYNC || (ARM && COMPILE_TEST)
> 
> Why would you add Zynq to the dependencies? This driver should work on
> all ARM platforms (more or less, given that the IP is implemented
> similarly). I think ARM, if it selects COMMON_CLK, is enough. Otherwise
> ARM && COMMON_CLK. I don't see a good reason to restrict the driver to
> Zynq only.
The idea is that if COMPILE_TEST is off you can only enable the driver
for machines where the device in question really exists to simplify
kernel configuration. Sometimes this isn't really possible, e.g. for i2c
devices that can be put on more or less every machine. I had the
impression that the cadence i2c bus device only exists on Zync? Ok, you
could implement this device on every SoC that has some programmable
hardware, but that is not the point.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Re: [PATCH] i2c: cadence: fix Kconfig dependency

[Michal Simek]

[-- Attachment #1: Type: text/plain, Size: 721 bytes --]

On 04/07/2014 08:56 AM, Wolfram Sang wrote:
> 
>> I would suggest to add relaxed IO helper function to asm-generic/io.h
>> to be available for all archs. Then we can avoid this type of errors.
> 
> I favour this, too. For the cadence driver, I needed a quick solution
> before linux-next breaks, though.
> 

FYI: Here is the patch
http://www.spinics.net/lists/kernel/msg1718606.html

Thanks,
Michal
-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

[PATCH] i2c: cadence: fix Kconfig dependency

[Michal Simek]

On 04/07/2014 08:56 AM, Wolfram Sang wrote:
> 
>> I would suggest to add relaxed IO helper function to asm-generic/io.h
>> to be available for all archs. Then we can avoid this type of errors.
> 
> I favour this, too. For the cadence driver, I needed a quick solution
> before linux-next breaks, though.
> 

FYI: Here is the patch
http://www.spinics.net/lists/kernel/msg1718606.html

Thanks,
Michal
-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 263 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140408/19ac1649/attachment.sig>

Re: [PATCH] i2c: cadence: fix Kconfig dependency

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 328 bytes --]

> I'd propose the below fix. The relaxed IO helpers are currently
> available on ARM only and the clock notifier used, depends on
> COMMON_CLK. I2C-related dependencies should be okay due to the location in
> Kconfig of this option.

If somebody thinks an update is still needed, please resend against
latest i2c-next. Thanks.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH] i2c: cadence: fix Kconfig dependency

[Wolfram Sang]

> I'd propose the below fix. The relaxed IO helpers are currently
> available on ARM only and the clock notifier used, depends on
> COMMON_CLK. I2C-related dependencies should be okay due to the location in
> Kconfig of this option.

If somebody thinks an update is still needed, please resend against
latest i2c-next. Thanks.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140521/25330a54/attachment.sig>

Re: [PATCH] i2c: cadence: fix Kconfig dependency

[Sören Brinkmann]

Hi Wolfram,

On Wed, 2014-05-21 at 10:29AM +0200, Wolfram Sang wrote:
> > I'd propose the below fix. The relaxed IO helpers are currently
> > available on ARM only and the clock notifier used, depends on
> > COMMON_CLK. I2C-related dependencies should be okay due to the location in
> > Kconfig of this option.
> 
> If somebody thinks an update is still needed, please resend against
> latest i2c-next. Thanks.
> 

I think we're fine for now. Once the *_relaxed IO helpers are available
on all archs it makes sense to revisit the COMPILE_TEST option. Until
then we are restricted to ARM anyway and the additional Zynq restriction
doesn't make a big difference, IMHO.

@Michal: Do you have an update on the IO helpers?

	Thanks,
	Sören

[PATCH] i2c: cadence: fix Kconfig dependency

[Sören Brinkmann]

Hi Wolfram,

On Wed, 2014-05-21 at 10:29AM +0200, Wolfram Sang wrote:
> > I'd propose the below fix. The relaxed IO helpers are currently
> > available on ARM only and the clock notifier used, depends on
> > COMMON_CLK. I2C-related dependencies should be okay due to the location in
> > Kconfig of this option.
> 
> If somebody thinks an update is still needed, please resend against
> latest i2c-next. Thanks.
> 

I think we're fine for now. Once the *_relaxed IO helpers are available
on all archs it makes sense to revisit the COMPILE_TEST option. Until
then we are restricted to ARM anyway and the additional Zynq restriction
doesn't make a big difference, IMHO.

@Michal: Do you have an update on the IO helpers?

	Thanks,
	S?ren

Re: [PATCH] i2c: cadence: fix Kconfig dependency

[Michal Simek]

[-- Attachment #1: Type: text/plain, Size: 1283 bytes --]

On 05/21/2014 06:00 PM, Sören Brinkmann wrote:
> Hi Wolfram,
> 
> On Wed, 2014-05-21 at 10:29AM +0200, Wolfram Sang wrote:
>>> I'd propose the below fix. The relaxed IO helpers are currently
>>> available on ARM only and the clock notifier used, depends on
>>> COMMON_CLK. I2C-related dependencies should be okay due to the location in
>>> Kconfig of this option.
>>
>> If somebody thinks an update is still needed, please resend against
>> latest i2c-next. Thanks.
>>
> 
> I think we're fine for now. Once the *_relaxed IO helpers are available
> on all archs it makes sense to revisit the COMPILE_TEST option. Until
> then we are restricted to ARM anyway and the additional Zynq restriction
> doesn't make a big difference, IMHO.
> 
> @Michal: Do you have an update on the IO helpers?

My patch was replaced by Will's one but I can't see it in linux-next
that's why we should ask directly Will.
Will: What's the status on it?

Thanks,
Michal


-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

[PATCH] i2c: cadence: fix Kconfig dependency

[Michal Simek]

On 05/21/2014 06:00 PM, S?ren Brinkmann wrote:
> Hi Wolfram,
> 
> On Wed, 2014-05-21 at 10:29AM +0200, Wolfram Sang wrote:
>>> I'd propose the below fix. The relaxed IO helpers are currently
>>> available on ARM only and the clock notifier used, depends on
>>> COMMON_CLK. I2C-related dependencies should be okay due to the location in
>>> Kconfig of this option.
>>
>> If somebody thinks an update is still needed, please resend against
>> latest i2c-next. Thanks.
>>
> 
> I think we're fine for now. Once the *_relaxed IO helpers are available
> on all archs it makes sense to revisit the COMPILE_TEST option. Until
> then we are restricted to ARM anyway and the additional Zynq restriction
> doesn't make a big difference, IMHO.
> 
> @Michal: Do you have an update on the IO helpers?

My patch was replaced by Will's one but I can't see it in linux-next
that's why we should ask directly Will.
Will: What's the status on it?

Thanks,
Michal


-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 263 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140522/036b8ba4/attachment.sig>