Hi David,
FYI, kernel build failed on
tree: git://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git master
head: c0b8b99287235626a5850ef7e5bfc842d1ebcecd
commit: da1586461e53a4dd045738cce309ab488970f0ef [1/9] pch_gbe: Fix PTP dependencies.
config: x86_64-randconfig-s052 (attached as .config)
All error/warnings:
drivers/built-in.o: In function `pch_gbe_ioctl':
pch_gbe_main.c:(.text+0x510370): undefined reference to `pch_ch_control_write'
pch_gbe_main.c:(.text+0x510393): undefined reference to `pch_ch_control_write'
pch_gbe_main.c:(.text+0x5103b3): undefined reference to `pch_ch_control_write'
pch_gbe_main.c:(.text+0x5103e1): undefined reference to `pch_set_station_address'
pch_gbe_main.c:(.text+0x510403): undefined reference to `pch_ch_control_write'
pch_gbe_main.c:(.text+0x510431): undefined reference to `pch_set_station_address'
pch_gbe_main.c:(.text+0x51043e): undefined reference to `pch_ch_event_write'
drivers/built-in.o: In function `pch_gbe_xmit_frame':
pch_gbe_main.c:(.text+0x510fe4): undefined reference to `pch_ch_event_read'
pch_gbe_main.c:(.text+0x511049): undefined reference to `pch_tx_snap_read'
pch_gbe_main.c:(.text+0x51106f): undefined reference to `pch_ch_event_write'
drivers/built-in.o: In function `pch_gbe_napi_poll':
pch_gbe_main.c:(.text+0x511537): undefined reference to `pch_ch_event_read'
pch_gbe_main.c:(.text+0x511562): undefined reference to `pch_src_uuid_lo_read'
pch_gbe_main.c:(.text+0x51156d): undefined reference to `pch_src_uuid_hi_read'
pch_gbe_main.c:(.text+0x511659): undefined reference to `pch_ch_event_write'
pch_gbe_main.c:(.text+0x511dc1): undefined reference to `pch_rx_snap_read'
---
0-DAY kernel build testing backend Open Source Technology Center
Fengguang Wu, Yuanhan Liu Intel Corporation
7c236c43b838221e17220bcb39e8e8d8c7123713 does something like the patch
below for the sfc driver. It would be worth checking if gianfar and
intel get it right too.
/me must leave.
From: Haicheng Li <hidden> Date: 2012-10-06 12:07:14
The failure is due to the CONFIG_PPS is not set there, consequently CONFIG_PTP_1588_CLOCK can not be set as =y anyway.
So David's patch of "da1586461e53a4dd045738cce309ab488970f0ef [1/9] pch_gbe: Fix PTP dependencies" is buggy. Furthermore, I think using "selects" to resolve such dependency issue is not good idea as it won't visit the dependencies.
David, I would still suggest to take my original patch:
https://lkml.org/lkml/2012/9/28/70
+ depends on PTP_1588_CLOCK_PCH && (PCH_GBE=m || PTP_1588_CLOCK_PCH=y)
or simply like:
---
From: Haicheng Li <redacted>
Fix build error caused by broken PCH_PTP module dependency.
The .config is:
CONFIG_PCH_GBE=y
CONFIG_PCH_PTP=y
CONFIG_PTP_1588_CLOCK=m
The build error:
drivers/built-in.o: In function `pch_tx_timestamp':
.../pch_gbe_main.c:215: undefined reference to `pch_ch_event_read'
.../pch_gbe_main.c:225: undefined reference to `pch_tx_snap_read'
.../pch_gbe_main.c:231: undefined reference to `pch_ch_event_write'
.../pch_gbe_main.c:170: undefined reference to `pch_ch_event_read'
.../pch_gbe_main.c:175: undefined reference to `pch_src_uuid_lo_read'
.../pch_gbe_main.c:176: undefined reference to `pch_src_uuid_hi_read'
.../pch_gbe_main.c:190: undefined reference to `pch_ch_event_write'
.../pch_gbe_main.c:184: undefined reference to `pch_rx_snap_read'
.../pch_gbe_main.c:267: undefined reference to `pch_ch_control_write'
.../pch_gbe_main.c:271: undefined reference to `pch_ch_control_write'
.../pch_gbe_main.c:275: undefined reference to `pch_ch_control_write'
.../pch_gbe_main.c:281: undefined reference to `pch_ch_control_write'
.../pch_gbe_main.c:283: undefined reference to `pch_set_station_address'
.../pch_gbe_main.c:290: undefined reference to `pch_ch_event_write'
Signed-off-by: Haicheng Li <redacted>
---
drivers/net/ethernet/oki-semi/pch_gbe/Kconfig | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
Hi David,
FYI, kernel build failed on
tree: git://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git master
head: c0b8b99287235626a5850ef7e5bfc842d1ebcecd
commit: da1586461e53a4dd045738cce309ab488970f0ef [1/9] pch_gbe: Fix PTP dependencies.
config: x86_64-randconfig-s052 (attached as .config)
All error/warnings:
drivers/built-in.o: In function `pch_gbe_ioctl':
pch_gbe_main.c:(.text+0x510370): undefined reference to `pch_ch_control_write'
pch_gbe_main.c:(.text+0x510393): undefined reference to `pch_ch_control_write'
pch_gbe_main.c:(.text+0x5103b3): undefined reference to `pch_ch_control_write'
pch_gbe_main.c:(.text+0x5103e1): undefined reference to `pch_set_station_address'
pch_gbe_main.c:(.text+0x510403): undefined reference to `pch_ch_control_write'
pch_gbe_main.c:(.text+0x510431): undefined reference to `pch_set_station_address'
pch_gbe_main.c:(.text+0x51043e): undefined reference to `pch_ch_event_write'
drivers/built-in.o: In function `pch_gbe_xmit_frame':
pch_gbe_main.c:(.text+0x510fe4): undefined reference to `pch_ch_event_read'
pch_gbe_main.c:(.text+0x511049): undefined reference to `pch_tx_snap_read'
pch_gbe_main.c:(.text+0x51106f): undefined reference to `pch_ch_event_write'
drivers/built-in.o: In function `pch_gbe_napi_poll':
pch_gbe_main.c:(.text+0x511537): undefined reference to `pch_ch_event_read'
pch_gbe_main.c:(.text+0x511562): undefined reference to `pch_src_uuid_lo_read'
pch_gbe_main.c:(.text+0x51156d): undefined reference to `pch_src_uuid_hi_read'
pch_gbe_main.c:(.text+0x511659): undefined reference to `pch_ch_event_write'
pch_gbe_main.c:(.text+0x511dc1): undefined reference to `pch_rx_snap_read'
---
0-DAY kernel build testing backend Open Source Technology Center
Fengguang Wu, Yuanhan Liu Intel Corporation
From: David Miller <davem@davemloft.net> Date: 2012-10-06 13:22:13
From: Haicheng Li <redacted>
Date: Sat, 06 Oct 2012 20:07:08 +0800
The failure is due to the CONFIG_PPS is not set there, consequently
CONFIG_PTP_1588_CLOCK can not be set as =y anyway.
So David's patch of "da1586461e53a4dd045738cce309ab488970f0ef [1/9]
pch_gbe: Fix PTP dependencies" is buggy. Furthermore, I think using
"selects" to resolve such dependency issue is not good idea as it
won't visit the dependencies.
David, I would still suggest to take my original patch:
https://lkml.org/lkml/2012/9/28/70
+ depends on PTP_1588_CLOCK_PCH && (PCH_GBE=m || PTP_1588_CLOCK_PCH=y)
or simply like:
This is all very rediculous if you ask me.
Why should the user have to know a detail like the underlying
PTP chip type just to enable PTP on his networking card?
Because that is what you are making him do with your change.
Select removed the necessity of the user having to know these
things.
7c236c43b838221e17220bcb39e8e8d8c7123713 does something like the patch
below for the sfc driver. It would be worth checking if gianfar and
intel get it right too.
They use select and it should work here too, I don't want to
force users to have to know about an obscure PTP chip type
in order to enable PTP for their networking card.
It should be completely hidden from them.
That's why we should use select for this.
From: Haicheng Li <hidden> Date: 2012-10-06 14:07:27
On 10/06/2012 09:22 PM, David Miller wrote:
From: Haicheng Li<redacted>
Date: Sat, 06 Oct 2012 20:07:08 +0800
quoted
The failure is due to the CONFIG_PPS is not set there, consequently
CONFIG_PTP_1588_CLOCK can not be set as =y anyway.
So David's patch of "da1586461e53a4dd045738cce309ab488970f0ef [1/9]
pch_gbe: Fix PTP dependencies" is buggy. Furthermore, I think using
"selects" to resolve such dependency issue is not good idea as it
won't visit the dependencies.
David, I would still suggest to take my original patch:
https://lkml.org/lkml/2012/9/28/70
+ depends on PTP_1588_CLOCK_PCH&& (PCH_GBE=m || PTP_1588_CLOCK_PCH=y)
or simply like:
This is all very rediculous if you ask me.
Why should the user have to know a detail like the underlying
PTP chip type just to enable PTP on his networking card?
Because that is what you are making him do with your change.
Select removed the necessity of the user having to know these
things.
However it possibly breaks the build...
IMHO, the reason why the dependency of PCH_PTP becomes so tricky is that the code of these two modules call the functions of each other (bad code structure?). To fix it neatly, either we restructure the code or just simply make it:
+ depends on PTP_1588_CLOCK_PCH=y
For PCH_GBE=m case, it does be able to pass the build test, but I'm afraid it won't be smoothly workable via "insmod" due to the codependency of these two when PCH_PTP is enabled.
From: David Miller <davem@davemloft.net> Date: 2012-10-06 14:21:38
From: Haicheng Li <redacted>
Date: Sat, 06 Oct 2012 22:07:23 +0800
On 10/06/2012 09:22 PM, David Miller wrote:
quoted
From: Haicheng Li<redacted>
Date: Sat, 06 Oct 2012 20:07:08 +0800
quoted
The failure is due to the CONFIG_PPS is not set there, consequently
CONFIG_PTP_1588_CLOCK can not be set as =y anyway.
So David's patch of "da1586461e53a4dd045738cce309ab488970f0ef [1/9]
pch_gbe: Fix PTP dependencies" is buggy. Furthermore, I think using
"selects" to resolve such dependency issue is not good idea as it
won't visit the dependencies.
David, I would still suggest to take my original patch:
https://lkml.org/lkml/2012/9/28/70
+ depends on PTP_1588_CLOCK_PCH&& (PCH_GBE=m || PTP_1588_CLOCK_PCH=y)
or simply like:
This is all very rediculous if you ask me.
Why should the user have to know a detail like the underlying
PTP chip type just to enable PTP on his networking card?
Because that is what you are making him do with your change.
Select removed the necessity of the user having to know these
things.
However it possibly breaks the build...
IMHO, the reason why the dependency of PCH_PTP becomes so tricky is
that the code of these two modules call the functions of each other
(bad code structure?). To fix it neatly, either we restructure the
code or just simply make it:
+ depends on PTP_1588_CLOCK_PCH=y
For PCH_GBE=m case, it does be able to pass the build test, but I'm
afraid it won't be smoothly workable via "insmod" due to the
codependency of these two when PCH_PTP is enabled.
Then why does it work for IXGBE and others who use select?
From: Richard Cochran <richardcochran@gmail.com> Date: 2012-10-06 14:56:48
On Sat, Oct 06, 2012 at 10:07:23PM +0800, Haicheng Li wrote:
IMHO, the reason why the dependency of PCH_PTP becomes so tricky is
that the code of these two modules call the functions of each other
(bad code structure?).
Yes, that is the source of the problem. I don't recall how this habit
of having the PTP option as a module got started (and I hope it wasn't
me), but I think the right way to handle this option is with a simple
boolean connected to ifdefs for the MAC driver.
Come to think of it, it may have all started with the gianfar ptp
module. In principle, the time stamping function of a MAC driver can
be completely separate from the clock function, and indeed that is how
the pair of gianfar drivers work.
But for other hardware, it might not be practical to keep the
functions separate, and in that case I would say, just keep it as one
driver.
Thanks,
Richard
PS It looks like no one is really looking after this PCH thing, so
does anyone want to send me a board so I can take care of it? Let me
know off list.
From: Haicheng Li <hidden> Date: 2012-10-06 17:21:14
On 10/06/2012 10:21 PM, David Miller wrote:
From: Haicheng Li<redacted>
Date: Sat, 06 Oct 2012 22:07:23 +0800
quoted
On 10/06/2012 09:22 PM, David Miller wrote:
quoted
From: Haicheng Li<redacted>
Date: Sat, 06 Oct 2012 20:07:08 +0800
quoted
The failure is due to the CONFIG_PPS is not set there, consequently
CONFIG_PTP_1588_CLOCK can not be set as =y anyway.
So David's patch of "da1586461e53a4dd045738cce309ab488970f0ef [1/9]
pch_gbe: Fix PTP dependencies" is buggy. Furthermore, I think using
"selects" to resolve such dependency issue is not good idea as it
won't visit the dependencies.
David, I would still suggest to take my original patch:
https://lkml.org/lkml/2012/9/28/70
+ depends on PTP_1588_CLOCK_PCH&& (PCH_GBE=m || PTP_1588_CLOCK_PCH=y)
or simply like:
This is all very rediculous if you ask me.
Why should the user have to know a detail like the underlying
PTP chip type just to enable PTP on his networking card?
Because that is what you are making him do with your change.
Select removed the necessity of the user having to know these
things.
However it possibly breaks the build...
IMHO, the reason why the dependency of PCH_PTP becomes so tricky is
that the code of these two modules call the functions of each other
(bad code structure?). To fix it neatly, either we restructure the
code or just simply make it:
+ depends on PTP_1588_CLOCK_PCH=y
For PCH_GBE=m case, it does be able to pass the build test, but I'm
afraid it won't be smoothly workable via "insmod" due to the
codependency of these two when PCH_PTP is enabled.
Then why does it work for IXGBE and others who use select?
They explicitly select all the possible dependencies if they are bug-free (I didn't strictly check them).
Take IXGBE_PTP as example, it explicitly selects PPS, and also depends on EXPERIMENTAL:
config IXGBE_PTP
bool "PTP Clock Support"
default n
depends on IXGBE && EXPERIMENTAL
select PPS
select PTP_1588_CLOCK
So if you stick to use "select" as the convention of such build issue fixing, fengguang's build failure would be fixed by:
+ depends on EXPERIMENTAL
+ select PPS
+ select PTP_1588_CLOCK
would you prefer this way?
From: David Miller <davem@davemloft.net> Date: 2012-10-06 21:17:53
From: Haicheng Li <redacted>
Date: Sun, 07 Oct 2012 01:21:09 +0800
Take IXGBE_PTP as example, it explicitly selects PPS, and also depends
on EXPERIMENTAL:
config IXGBE_PTP
bool "PTP Clock Support"
default n
depends on IXGBE && EXPERIMENTAL
select PPS
select PTP_1588_CLOCK
So if you stick to use "select" as the convention of such build issue
fixing, fengguang's build failure would be fixed by:
+ depends on EXPERIMENTAL
+ select PPS
+ select PTP_1588_CLOCK
would you prefer this way?
From: Haicheng Li <hidden> Date: 2012-10-07 14:14:41
Fengguang reported a kernel build failure as following:
drivers/built-in.o: In function `pch_gbe_ioctl':
pch_gbe_main.c:(.text+0x510370): undefined reference to `pch_ch_control_write'
pch_gbe_main.c:(.text+0x510393): undefined reference to `pch_ch_control_write'
pch_gbe_main.c:(.text+0x5103b3): undefined reference to `pch_ch_control_write'
...
It's a regression by commit da1586461. The root cause is that
the CONFIG_PPS is not set there, consequently CONFIG_PTP_1588_CLOCK
can not be set anyway, which finally causes ptp_pch and pch_gbe_main
build failures.
As David prefers to use *select* to fix such module co-dependency issues,
this patch explicitly selects all the possible dependencies of PCH_PTP.
Reported-by: Fengguang Wu <redacted>
Reviewed-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Haicheng Li <redacted>
---
drivers/net/ethernet/oki-semi/pch_gbe/Kconfig | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
This patch is corrupted by your email client.
DO NOT resend this patch without first emailing it to yourself
and verifying that you can in fact apply what you receive in
the email cleanly.
From: Haicheng Li <hidden> Date: 2012-10-09 09:45:41
Fengguang reported a kernel build failure as following:
drivers/built-in.o: In function `pch_gbe_ioctl':
pch_gbe_main.c:(.text+0x510370): undefined reference to `pch_ch_control_write'
pch_gbe_main.c:(.text+0x510393): undefined reference to `pch_ch_control_write'
pch_gbe_main.c:(.text+0x5103b3): undefined reference to `pch_ch_control_write'
...
It's a regression by commit da1586461. The root cause is that
the CONFIG_PPS is not set there, consequently CONFIG_PTP_1588_CLOCK
can not be set anyway, which finally causes ptp_pch and pch_gbe_main
build failures.
As David prefers to use *select* to fix such module co-dependency issues,
this patch explicitly selects all the possible dependencies of PCH_PTP.
Reported-by: Fengguang Wu <redacted>
Reviewed-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Haicheng Li <redacted>
---
drivers/net/ethernet/oki-semi/pch_gbe/Kconfig | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
From: David Miller <davem@davemloft.net> Date: 2012-10-09 17:51:25
From: Haicheng Li <redacted>
Date: Tue, 9 Oct 2012 17:43:12 +0800
Fengguang reported a kernel build failure as following:
drivers/built-in.o: In function `pch_gbe_ioctl':
pch_gbe_main.c:(.text+0x510370): undefined reference to `pch_ch_control_write'
pch_gbe_main.c:(.text+0x510393): undefined reference to `pch_ch_control_write'
pch_gbe_main.c:(.text+0x5103b3): undefined reference to `pch_ch_control_write'
...
It's a regression by commit da1586461. The root cause is that
the CONFIG_PPS is not set there, consequently CONFIG_PTP_1588_CLOCK
can not be set anyway, which finally causes ptp_pch and pch_gbe_main
build failures.
As David prefers to use *select* to fix such module co-dependency issues,
this patch explicitly selects all the possible dependencies of PCH_PTP.
Reported-by: Fengguang Wu <redacted>
Reviewed-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Haicheng Li <redacted>