* [PATCH] i2c: cadence: fix Kconfig dependency @ 2014-04-06 19:07 Wolfram Sang 2014-04-06 22:13 ` Sören Brinkmann 2014-04-07 6:31 ` Uwe Kleine-König 0 siblings, 2 replies; 18+ messages in thread From: Wolfram Sang @ 2014-04-06 19:07 UTC (permalink / raw) To: linux-arm-kernel 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 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH] i2c: cadence: fix Kconfig dependency 2014-04-06 19:07 [PATCH] i2c: cadence: fix Kconfig dependency Wolfram Sang @ 2014-04-06 22:13 ` Sören Brinkmann 2014-04-07 6:31 ` Uwe Kleine-König 1 sibling, 0 replies; 18+ messages in thread From: Sören Brinkmann @ 2014-04-06 22:13 UTC (permalink / raw) To: linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH] i2c: cadence: fix Kconfig dependency 2014-04-06 19:07 [PATCH] i2c: cadence: fix Kconfig dependency Wolfram Sang 2014-04-06 22:13 ` Sören Brinkmann @ 2014-04-07 6:31 ` Uwe Kleine-König 2014-04-07 6:37 ` Alexander Shiyan ` (2 more replies) 1 sibling, 3 replies; 18+ messages in thread From: Uwe Kleine-König @ 2014-04-07 6:31 UTC (permalink / raw) To: linux-arm-kernel 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/ | ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] i2c: cadence: fix Kconfig dependency 2014-04-07 6:31 ` Uwe Kleine-König @ 2014-04-07 6:37 ` Alexander Shiyan 2014-04-07 6:56 ` Uwe Kleine-König 2014-04-07 15:36 ` Sören Brinkmann 2014-04-07 6:39 ` Michal Simek 2014-04-07 16:14 ` Sören Brinkmann 2 siblings, 2 replies; 18+ messages in thread From: Alexander Shiyan @ 2014-04-07 6:37 UTC (permalink / raw) To: linux-arm-kernel 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. --- ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH] i2c: cadence: fix Kconfig dependency 2014-04-07 6:37 ` Alexander Shiyan @ 2014-04-07 6:56 ` Uwe Kleine-König 2014-04-07 7:02 ` Alexander Shiyan 2014-04-07 15:36 ` Sören Brinkmann 1 sibling, 1 reply; 18+ messages in thread From: Uwe Kleine-König @ 2014-04-07 6:56 UTC (permalink / raw) To: linux-arm-kernel 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/ | ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] i2c: cadence: fix Kconfig dependency 2014-04-07 6:56 ` Uwe Kleine-König @ 2014-04-07 7:02 ` Alexander Shiyan 2014-04-07 16:18 ` Sören Brinkmann 0 siblings, 1 reply; 18+ messages in thread From: Alexander Shiyan @ 2014-04-07 7:02 UTC (permalink / raw) To: linux-arm-kernel 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. --- ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH] i2c: cadence: fix Kconfig dependency 2014-04-07 7:02 ` Alexander Shiyan @ 2014-04-07 16:18 ` Sören Brinkmann 2014-05-21 8:29 ` Wolfram Sang 0 siblings, 1 reply; 18+ messages in thread From: Sören Brinkmann @ 2014-04-07 16:18 UTC (permalink / raw) To: linux-arm-kernel 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 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH] i2c: cadence: fix Kconfig dependency 2014-04-07 16:18 ` Sören Brinkmann @ 2014-05-21 8:29 ` Wolfram Sang 2014-05-21 16:00 ` Sören Brinkmann 0 siblings, 1 reply; 18+ messages in thread From: Wolfram Sang @ 2014-05-21 8:29 UTC (permalink / raw) To: linux-arm-kernel > 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> ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH] i2c: cadence: fix Kconfig dependency 2014-05-21 8:29 ` Wolfram Sang @ 2014-05-21 16:00 ` Sören Brinkmann 2014-05-22 4:58 ` Michal Simek 0 siblings, 1 reply; 18+ messages in thread From: Sören Brinkmann @ 2014-05-21 16:00 UTC (permalink / raw) To: linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH] i2c: cadence: fix Kconfig dependency 2014-05-21 16:00 ` Sören Brinkmann @ 2014-05-22 4:58 ` Michal Simek 0 siblings, 0 replies; 18+ messages in thread From: Michal Simek @ 2014-05-22 4:58 UTC (permalink / raw) To: linux-arm-kernel 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> ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH] i2c: cadence: fix Kconfig dependency 2014-04-07 6:37 ` Alexander Shiyan 2014-04-07 6:56 ` Uwe Kleine-König @ 2014-04-07 15:36 ` Sören Brinkmann 2014-04-07 17:50 ` Uwe Kleine-König 1 sibling, 1 reply; 18+ messages in thread From: Sören Brinkmann @ 2014-04-07 15:36 UTC (permalink / raw) To: linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH] i2c: cadence: fix Kconfig dependency 2014-04-07 15:36 ` Sören Brinkmann @ 2014-04-07 17:50 ` Uwe Kleine-König 0 siblings, 0 replies; 18+ messages in thread From: Uwe Kleine-König @ 2014-04-07 17:50 UTC (permalink / raw) To: linux-arm-kernel 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/ | ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH] i2c: cadence: fix Kconfig dependency 2014-04-07 6:31 ` Uwe Kleine-König 2014-04-07 6:37 ` Alexander Shiyan @ 2014-04-07 6:39 ` Michal Simek 2014-04-07 6:56 ` Wolfram Sang 2014-04-07 15:37 ` Sören Brinkmann 2014-04-07 16:14 ` Sören Brinkmann 2 siblings, 2 replies; 18+ messages in thread From: Michal Simek @ 2014-04-07 6:39 UTC (permalink / raw) To: linux-arm-kernel 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> ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH] i2c: cadence: fix Kconfig dependency 2014-04-07 6:39 ` Michal Simek @ 2014-04-07 6:56 ` Wolfram Sang 2014-04-07 7:00 ` Michal Simek 2014-04-08 8:33 ` Michal Simek 2014-04-07 15:37 ` Sören Brinkmann 1 sibling, 2 replies; 18+ messages in thread From: Wolfram Sang @ 2014-04-07 6:56 UTC (permalink / raw) To: linux-arm-kernel > 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> ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH] i2c: cadence: fix Kconfig dependency 2014-04-07 6:56 ` Wolfram Sang @ 2014-04-07 7:00 ` Michal Simek 2014-04-08 8:33 ` Michal Simek 1 sibling, 0 replies; 18+ messages in thread From: Michal Simek @ 2014-04-07 7:00 UTC (permalink / raw) To: linux-arm-kernel 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> ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH] i2c: cadence: fix Kconfig dependency 2014-04-07 6:56 ` Wolfram Sang 2014-04-07 7:00 ` Michal Simek @ 2014-04-08 8:33 ` Michal Simek 1 sibling, 0 replies; 18+ messages in thread From: Michal Simek @ 2014-04-08 8:33 UTC (permalink / raw) To: linux-arm-kernel 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> ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH] i2c: cadence: fix Kconfig dependency 2014-04-07 6:39 ` Michal Simek 2014-04-07 6:56 ` Wolfram Sang @ 2014-04-07 15:37 ` Sören Brinkmann 1 sibling, 0 replies; 18+ messages in thread From: Sören Brinkmann @ 2014-04-07 15:37 UTC (permalink / raw) To: linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH] i2c: cadence: fix Kconfig dependency 2014-04-07 6:31 ` Uwe Kleine-König 2014-04-07 6:37 ` Alexander Shiyan 2014-04-07 6:39 ` Michal Simek @ 2014-04-07 16:14 ` Sören Brinkmann 2 siblings, 0 replies; 18+ messages in thread From: Sören Brinkmann @ 2014-04-07 16:14 UTC (permalink / raw) To: linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2014-05-22 4:58 UTC | newest] Thread overview: 18+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-04-06 19:07 [PATCH] i2c: cadence: fix Kconfig dependency Wolfram Sang 2014-04-06 22:13 ` Sören Brinkmann 2014-04-07 6:31 ` Uwe Kleine-König 2014-04-07 6:37 ` Alexander Shiyan 2014-04-07 6:56 ` Uwe Kleine-König 2014-04-07 7:02 ` Alexander Shiyan 2014-04-07 16:18 ` Sören Brinkmann 2014-05-21 8:29 ` Wolfram Sang 2014-05-21 16:00 ` Sören Brinkmann 2014-05-22 4:58 ` Michal Simek 2014-04-07 15:36 ` Sören Brinkmann 2014-04-07 17:50 ` Uwe Kleine-König 2014-04-07 6:39 ` Michal Simek 2014-04-07 6:56 ` Wolfram Sang 2014-04-07 7:00 ` Michal Simek 2014-04-08 8:33 ` Michal Simek 2014-04-07 15:37 ` Sören Brinkmann 2014-04-07 16:14 ` Sören Brinkmann
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).