i2c: cadence: fix Kconfig dependency

PATCH New Wolfram Sang 5 participants 18 messages Apr 6, 2014 – May 22, 2014 I2C subsystem host drivers

Share & tools

Wolfram Sang

author

2014-04-06 19:07 UTC #0 lore raw

PATCH 1/1 i2c: cadence: fix Kconfig dependency

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

Quoted 2 lines

Quoted 2 lines
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 <[email protected]>

---
 drivers/i2c/busses/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/i2c/busses/Kconfig +1 -1

index 93165ff453ab..3d3b9b3577c5 100644
@@ -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.

Signature · 1 line

1.9.1

Sören Brinkmann

2014-04-06 22:13 UTC #1 lore raw

PATCH 1/1 i2c: cadence: fix Kconfig dependency

Hi Wolfram,

Quoted 6 lines · 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

Quoted 7 lines

All error/warnings:
drivers/i2c/busses/i2c-cadence.c: In function 'cdns_i2c_clear_bus_hold':
Quoted 2 lines
Quoted 2 lines
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

Uwe Kleine-König

2014-04-07 06:31 UTC #2 lore raw

PATCH 1/1 i2c: cadence: fix Kconfig dependency

In reply to Wolfram Sang

Hello,

Quoted 24 lines · 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':
Quoted 2 lines
Quoted 2 lines
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 <[email protected]>
---
 drivers/i2c/busses/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/i2c/busses/Kconfig +0 -0
index 93165ff453ab..3d3b9b3577c5 100644

drivers/i2c/busses/Kconfig config I2C_BLACKFIN_TWI_CLK_KHZ · line 381

Quoted 5 lines

drivers/i2c/busses/Kconfig +1 -1

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

Quoted 12 lines

drivers/i2c/busses/Kconfig +0 -0

 	help
 	  Say yes here to select Cadence I2C Host Controller. This controller is
 	  e.g. used by Xilinx Zynq.

Signature · 7 lines

1.9.1


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

Signature · 2 lines

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

Alexander Shiyan

2014-04-07 06:37 UTC #3 lore raw

Mon, 7 Apr 2014 08:31:00 +0200 ?? Uwe Kleine-K?nig <[email protected]>:

Quoted 28 lines

Hello,
Quoted 25 lines · 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':
Quoted 2 lines
Quoted 2 lines
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 <[email protected]>
---
 drivers/i2c/busses/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/i2c/busses/Kconfig +0 -0
index 93165ff453ab..3d3b9b3577c5 100644
@@ -378,7 +378,7 @@ config I2C_BLACKFIN_TWI_CLK_KHZ
 

Quoted 7 lines

drivers/i2c/busses/Kconfig config I2C_BLACKFIN_TWI_CLK_KHZ · line 381
Quoted 4 lines
drivers/i2c/busses/Kconfig +1 -1
 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.

---

Uwe Kleine-König

2014-04-07 06:56 UTC #4 lore raw

PATCH 1/1 i2c: cadence: fix Kconfig dependency

Hello,

Quoted 35 lines · Alexander Shiyan

Mon, 7 Apr 2014 08:31:00 +0200 ?? Uwe Kleine-K?nig <[email protected]>:
Quoted 34 lines
Hello,
Quoted 24 lines · 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':
Quoted 2 lines
Quoted 2 lines
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 <[email protected]>
---
 drivers/i2c/busses/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/i2c/busses/Kconfig +0 -0
index 93165ff453ab..3d3b9b3577c5 100644
drivers/i2c/busses/Kconfig config I2C_BLACKFIN_TWI_CLK_KHZ · line 381
Quoted 5 lines
drivers/i2c/busses/Kconfig +1 -1
@@ -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:

Quoted 5 lines

Quoted 1 line
	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)

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

	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

Signature · 2 lines

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

Alexander Shiyan

2014-04-07 07:02 UTC #5 lore raw

Mon, 7 Apr 2014 08:56:57 +0200 ?? Uwe Kleine-K?nig <[email protected]>:

Quoted 1 line

Hello,

...

Quoted 34 lines

Quoted 34 lines
Quoted 33 lines · Wolfram Sang
Quoted 24 lines
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':
Quoted 2 lines
Quoted 2 lines
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 <[email protected]>
---
 drivers/i2c/busses/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/i2c/busses/Kconfig +0 -0
index 93165ff453ab..3d3b9b3577c5 100644
drivers/i2c/busses/Kconfig config I2C_BLACKFIN_TWI_CLK_KHZ · line 381
Quoted 5 lines
drivers/i2c/busses/Kconfig +1 -1
@@ -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)

Quoted 8 lines

Quoted 3 lines
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.

---

Sören Brinkmann

2014-04-07 16:18 UTC #6 lore raw

PATCH 1/1 i2c: cadence: fix Kconfig dependency

Quoted 39 lines · Alexander Shiyan

Mon, 7 Apr 2014 08:56:57 +0200 ?? Uwe Kleine-K?nig <[email protected]>:
Quoted 1 line
Hello,
...
Quoted 36 lines
Quoted 36 lines
Quoted 26 lines · Wolfram Sang
Quoted 25 lines
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':
Quoted 2 lines
Quoted 2 lines
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 <[email protected]>
---
 drivers/i2c/busses/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/i2c/busses/Kconfig +0 -0
index 93165ff453ab..3d3b9b3577c5 100644
@@ -378,7 +378,7 @@ config I2C_BLACKFIN_TWI_CLK_KHZ
 
Quoted 7 lines
drivers/i2c/busses/Kconfig config I2C_BLACKFIN_TWI_CLK_KHZ · line 381
Quoted 4 lines
drivers/i2c/busses/Kconfig +1 -1
 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)

Quoted 8 lines

Quoted 6 lines
Quoted 1 line
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':

Quoted 1 line

Quoted 1 line
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 <[email protected]>

Signed-off-by Soren Brinkmann <[email protected]>

---
 drivers/i2c/busses/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/i2c/busses/Kconfig +1 -1

index 66e68e48218f..9f546eecd322 100644
@@ -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.

Signature · 1 line

1.9.1.1.gbb9f595

Wolfram Sang

author

2014-05-21 08:29 UTC #7 lore raw

PATCH 1/1 i2c: cadence: fix Kconfig dependency

Quoted 4 lines

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>

Sören Brinkmann

2014-05-21 16:00 UTC #8 lore raw

PATCH 1/1 i2c: cadence: fix Kconfig dependency

Hi Wolfram,

Quoted 8 lines · Wolfram Sang

Quoted 4 lines
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

Michal Simek

2014-05-22 04:58 UTC #9 lore raw

PATCH 1/1 i2c: cadence: fix Kconfig dependency

Quoted 12 lines · S?ren Brinkmann

Hi Wolfram,
Quoted 8 lines · Wolfram Sang
Quoted 4 lines
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.

Quoted 6 lines

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

Signature · 14 lines

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>

Sören Brinkmann

2014-04-07 15:36 UTC #10 lore raw

PATCH 1/1 i2c: cadence: fix Kconfig dependency

In reply to Alexander Shiyan

Quoted 35 lines · Alexander Shiyan

Mon, 7 Apr 2014 08:31:00 +0200 ?? Uwe Kleine-K?nig <[email protected]>:
Quoted 34 lines
Hello,
Quoted 24 lines · 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':
Quoted 2 lines
Quoted 2 lines
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 <[email protected]>
---
 drivers/i2c/busses/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/i2c/busses/Kconfig +0 -0
index 93165ff453ab..3d3b9b3577c5 100644
drivers/i2c/busses/Kconfig config I2C_BLACKFIN_TWI_CLK_KHZ · line 381
Quoted 5 lines
drivers/i2c/busses/Kconfig +1 -1
@@ -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:

Quoted 3 lines

Quoted 1 line
	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

Uwe Kleine-König

2014-04-07 17:50 UTC #11 lore raw

PATCH 1/1 i2c: cadence: fix Kconfig dependency

Hello S?ren,

Quoted 8 lines · S?ren Brinkmann

Quoted 1 line · Alexander Shiyan
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

Signature · 2 lines

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

Michal Simek

2014-04-07 06:39 UTC #12 lore raw

PATCH 1/1 i2c: cadence: fix Kconfig dependency

In reply to Uwe Kleine-König

Quoted 34 lines · Uwe Kleine-K?nig

Hello,
Quoted 24 lines · 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':
Quoted 2 lines
Quoted 2 lines
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 <[email protected]>
---
 drivers/i2c/busses/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/i2c/busses/Kconfig +0 -0
index 93165ff453ab..3d3b9b3577c5 100644
drivers/i2c/busses/Kconfig config I2C_BLACKFIN_TWI_CLK_KHZ · line 381
Quoted 5 lines
drivers/i2c/busses/Kconfig +1 -1
@@ -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:

Quoted 4 lines

	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

Signature · 14 lines

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>

Wolfram Sang

author

2014-04-07 06:56 UTC #13 lore raw

PATCH 1/1 i2c: cadence: fix Kconfig dependency

Quoted 3 lines

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.

Michal Simek

2014-04-07 07:00 UTC #14 lore raw

PATCH 1/1 i2c: cadence: fix Kconfig dependency

Quoted 6 lines · Wolfram Sang

Quoted 3 lines
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

Signature · 14 lines

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>

Michal Simek

2014-04-08 08:33 UTC #15 lore raw

PATCH 1/1 i2c: cadence: fix Kconfig dependency

In reply to Wolfram Sang

Quoted 7 lines · Wolfram Sang

Quoted 3 lines
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

Signature · 14 lines

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>

Sören Brinkmann

2014-04-07 15:37 UTC #16 lore raw

PATCH 1/1 i2c: cadence: fix Kconfig dependency

In reply to Michal Simek

Quoted 35 lines · Michal Simek

Quoted 34 lines · Uwe Kleine-K?nig
Hello,
Quoted 24 lines · 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':
Quoted 2 lines
Quoted 2 lines
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 <[email protected]>
---
 drivers/i2c/busses/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/i2c/busses/Kconfig +0 -0
index 93165ff453ab..3d3b9b3577c5 100644
drivers/i2c/busses/Kconfig config I2C_BLACKFIN_TWI_CLK_KHZ · line 381
Quoted 5 lines
drivers/i2c/busses/Kconfig +1 -1
@@ -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:

Quoted 7 lines

Quoted 4 lines
	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

Sören Brinkmann

2014-04-07 16:14 UTC #17 lore raw

PATCH 1/1 i2c: cadence: fix Kconfig dependency

In reply to Uwe Kleine-König

Quoted 28 lines · Uwe Kleine-K?nig

Hello,
Quoted 25 lines · 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':
Quoted 2 lines
Quoted 2 lines
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 <[email protected]>
---
 drivers/i2c/busses/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/i2c/busses/Kconfig +0 -0
index 93165ff453ab..3d3b9b3577c5 100644
@@ -378,7 +378,7 @@ config I2C_BLACKFIN_TWI_CLK_KHZ
 

Quoted 7 lines

drivers/i2c/busses/Kconfig config I2C_BLACKFIN_TWI_CLK_KHZ · line 381
Quoted 4 lines
drivers/i2c/busses/Kconfig +1 -1
 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