* [PATCH] s390/net: lcs: fix build errors when FDDI is a loadable module
@ 2023-06-21 21:37 Randy Dunlap
2023-06-22 7:15 ` Alexandra Winter
0 siblings, 1 reply; 9+ messages in thread
From: Randy Dunlap @ 2023-06-21 21:37 UTC (permalink / raw)
To: linux-kernel
Cc: Randy Dunlap, kernel test robot, Simon Horman, Alexandra Winter,
Wenjia Zhang, linux-s390, netdev, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger, Sven Schnelle,
David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
Require FDDI to be built-in if it is used. LCS needs FDDI to be
built-in to build without errors.
Prevents these build errors:
s390-linux-ld: drivers/s390/net/lcs.o: in function `lcs_new_device':
drivers/s390/net/lcs.c:2150: undefined reference to `fddi_type_trans'
s390-linux-ld: drivers/s390/net/lcs.c:2151: undefined reference to `alloc_fddidev'
This FDDI requirement effectively restores the previous condition
before the blamed patch, when #ifdef CONFIG_FDDI was used, without
testing for CONFIG_FDDI_MODULE.
Fixes: 128272336120 ("s390/net: lcs: use IS_ENABLED() for kconfig detection")
Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
Reported-by: kernel test robot <lkp@intel.com>
Link: lore.kernel.org/r/202306202129.pl0AqK8G-lkp@intel.com
Suggested-by: Simon Horman <simon.horman@corigine.com>
Cc: Alexandra Winter <wintera@linux.ibm.com>
Cc: Wenjia Zhang <wenjia@linux.ibm.com>
Cc: linux-s390@vger.kernel.org
Cc: netdev@vger.kernel.org
Cc: Heiko Carstens <hca@linux.ibm.com>
Cc: Vasily Gorbik <gor@linux.ibm.com>
Cc: Alexander Gordeev <agordeev@linux.ibm.com>
Cc: Christian Borntraeger <borntraeger@linux.ibm.com>
Cc: Sven Schnelle <svens@linux.ibm.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
---
drivers/s390/net/Kconfig | 2 ++
1 file changed, 2 insertions(+)
diff -- a/drivers/s390/net/Kconfig b/drivers/s390/net/Kconfig
--- a/drivers/s390/net/Kconfig
+++ b/drivers/s390/net/Kconfig
@@ -6,11 +6,13 @@ config LCS
def_tristate m
prompt "Lan Channel Station Interface"
depends on CCW && NETDEVICES && (ETHERNET || FDDI)
+ depends on FDDI=y || FDDI=n
help
Select this option if you want to use LCS networking on IBM System z.
This device driver supports FDDI (IEEE 802.7) and Ethernet.
To compile as a module, choose M. The module name is lcs.
If you do not know what it is, it's safe to choose Y.
+ If FDDI is used, it must be built-in (=y).
config CTCM
def_tristate m
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] s390/net: lcs: fix build errors when FDDI is a loadable module
2023-06-21 21:37 [PATCH] s390/net: lcs: fix build errors when FDDI is a loadable module Randy Dunlap
@ 2023-06-22 7:15 ` Alexandra Winter
2023-06-22 7:53 ` Simon Horman
2023-06-22 15:36 ` Randy Dunlap
0 siblings, 2 replies; 9+ messages in thread
From: Alexandra Winter @ 2023-06-22 7:15 UTC (permalink / raw)
To: Randy Dunlap, linux-kernel
Cc: kernel test robot, Simon Horman, Wenjia Zhang, linux-s390, netdev,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni
On 21.06.23 23:37, Randy Dunlap wrote:
> Require FDDI to be built-in if it is used. LCS needs FDDI to be
> built-in to build without errors.
>
> Prevents these build errors:
> s390-linux-ld: drivers/s390/net/lcs.o: in function `lcs_new_device':
> drivers/s390/net/lcs.c:2150: undefined reference to `fddi_type_trans'
> s390-linux-ld: drivers/s390/net/lcs.c:2151: undefined reference to `alloc_fddidev'
>
> This FDDI requirement effectively restores the previous condition
> before the blamed patch, when #ifdef CONFIG_FDDI was used, without
> testing for CONFIG_FDDI_MODULE.
>
> Fixes: 128272336120 ("s390/net: lcs: use IS_ENABLED() for kconfig detection")
> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> Reported-by: kernel test robot <lkp@intel.com>
> Link: lore.kernel.org/r/202306202129.pl0AqK8G-lkp@intel.com
> Suggested-by: Simon Horman <simon.horman@corigine.com>
> Cc: Alexandra Winter <wintera@linux.ibm.com>
> Cc: Wenjia Zhang <wenjia@linux.ibm.com>
> Cc: linux-s390@vger.kernel.org
> Cc: netdev@vger.kernel.org
> Cc: Heiko Carstens <hca@linux.ibm.com>
> Cc: Vasily Gorbik <gor@linux.ibm.com>
> Cc: Alexander Gordeev <agordeev@linux.ibm.com>
> Cc: Christian Borntraeger <borntraeger@linux.ibm.com>
> Cc: Sven Schnelle <svens@linux.ibm.com>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: Paolo Abeni <pabeni@redhat.com>
> ---
> drivers/s390/net/Kconfig | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff -- a/drivers/s390/net/Kconfig b/drivers/s390/net/Kconfig
> --- a/drivers/s390/net/Kconfig
> +++ b/drivers/s390/net/Kconfig
> @@ -6,11 +6,13 @@ config LCS
> def_tristate m
> prompt "Lan Channel Station Interface"
> depends on CCW && NETDEVICES && (ETHERNET || FDDI)
> + depends on FDDI=y || FDDI=n
> help
> Select this option if you want to use LCS networking on IBM System z.
> This device driver supports FDDI (IEEE 802.7) and Ethernet.
> To compile as a module, choose M. The module name is lcs.
> If you do not know what it is, it's safe to choose Y.
> + If FDDI is used, it must be built-in (=y).
>
> config CTCM
> def_tristate m
>
Wow Randy and Simon, you are reacting faster than I was able to evaluate this yesterday.
2 thoughts:
1) As ETHERNET cannot be a module and this patch prevents FDDI from being a module, then
128272336120 ("s390/net: lcs: use IS_ENABLED() for kconfig detection")
is kind of pointless and can as well be reverted instead of doing this fix.
Or am I missing something?
2) I wonder whether
depends on CCW && NETDEVICES && (ETHERNET || FDDI)
+ depends on FDDI || FDDI=n
would do what we want here:
When FDDI is a loadable module, LCS mustn't be built-in.
I will do some experiments and let you know.
Alexandra
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] s390/net: lcs: fix build errors when FDDI is a loadable module
2023-06-22 7:15 ` Alexandra Winter
@ 2023-06-22 7:53 ` Simon Horman
2023-06-22 12:16 ` Alexandra Winter
2023-06-22 15:36 ` Randy Dunlap
1 sibling, 1 reply; 9+ messages in thread
From: Simon Horman @ 2023-06-22 7:53 UTC (permalink / raw)
To: Alexandra Winter
Cc: Randy Dunlap, linux-kernel, kernel test robot, Wenjia Zhang,
linux-s390, netdev, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger, Sven Schnelle,
David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
On Thu, Jun 22, 2023 at 09:15:24AM +0200, Alexandra Winter wrote:
>
>
> On 21.06.23 23:37, Randy Dunlap wrote:
> > Require FDDI to be built-in if it is used. LCS needs FDDI to be
> > built-in to build without errors.
> >
> > Prevents these build errors:
> > s390-linux-ld: drivers/s390/net/lcs.o: in function `lcs_new_device':
> > drivers/s390/net/lcs.c:2150: undefined reference to `fddi_type_trans'
> > s390-linux-ld: drivers/s390/net/lcs.c:2151: undefined reference to `alloc_fddidev'
> >
> > This FDDI requirement effectively restores the previous condition
> > before the blamed patch, when #ifdef CONFIG_FDDI was used, without
> > testing for CONFIG_FDDI_MODULE.
> >
> > Fixes: 128272336120 ("s390/net: lcs: use IS_ENABLED() for kconfig detection")
> > Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> > Reported-by: kernel test robot <lkp@intel.com>
> > Link: lore.kernel.org/r/202306202129.pl0AqK8G-lkp@intel.com
> > Suggested-by: Simon Horman <simon.horman@corigine.com>
> > Cc: Alexandra Winter <wintera@linux.ibm.com>
> > Cc: Wenjia Zhang <wenjia@linux.ibm.com>
> > Cc: linux-s390@vger.kernel.org
> > Cc: netdev@vger.kernel.org
> > Cc: Heiko Carstens <hca@linux.ibm.com>
> > Cc: Vasily Gorbik <gor@linux.ibm.com>
> > Cc: Alexander Gordeev <agordeev@linux.ibm.com>
> > Cc: Christian Borntraeger <borntraeger@linux.ibm.com>
> > Cc: Sven Schnelle <svens@linux.ibm.com>
> > Cc: David S. Miller <davem@davemloft.net>
> > Cc: Eric Dumazet <edumazet@google.com>
> > Cc: Jakub Kicinski <kuba@kernel.org>
> > Cc: Paolo Abeni <pabeni@redhat.com>
> > ---
> > drivers/s390/net/Kconfig | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff -- a/drivers/s390/net/Kconfig b/drivers/s390/net/Kconfig
> > --- a/drivers/s390/net/Kconfig
> > +++ b/drivers/s390/net/Kconfig
> > @@ -6,11 +6,13 @@ config LCS
> > def_tristate m
> > prompt "Lan Channel Station Interface"
> > depends on CCW && NETDEVICES && (ETHERNET || FDDI)
> > + depends on FDDI=y || FDDI=n
> > help
> > Select this option if you want to use LCS networking on IBM System z.
> > This device driver supports FDDI (IEEE 802.7) and Ethernet.
> > To compile as a module, choose M. The module name is lcs.
> > If you do not know what it is, it's safe to choose Y.
> > + If FDDI is used, it must be built-in (=y).
> >
> > config CTCM
> > def_tristate m
> >
>
>
> Wow Randy and Simon, you are reacting faster than I was able to evaluate this yesterday.
> 2 thoughts:
>
> 1) As ETHERNET cannot be a module and this patch prevents FDDI from being a module, then
> 128272336120 ("s390/net: lcs: use IS_ENABLED() for kconfig detection")
> is kind of pointless and can as well be reverted instead of doing this fix.
> Or am I missing something?
I'll leave that one to Randy at this point.
> 2) I wonder whether
>
> depends on CCW && NETDEVICES && (ETHERNET || FDDI)
> + depends on FDDI || FDDI=n
>
> would do what we want here:
> When FDDI is a loadable module, LCS mustn't be built-in.
>
> I will do some experiments and let you know.
It does seem to on my side.
But checking would be much appreciated.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] s390/net: lcs: fix build errors when FDDI is a loadable module
2023-06-22 7:53 ` Simon Horman
@ 2023-06-22 12:16 ` Alexandra Winter
2023-06-22 12:33 ` Christian Borntraeger
2023-06-28 2:53 ` Randy Dunlap
0 siblings, 2 replies; 9+ messages in thread
From: Alexandra Winter @ 2023-06-22 12:16 UTC (permalink / raw)
To: Simon Horman
Cc: Randy Dunlap, linux-kernel, kernel test robot, Wenjia Zhang,
linux-s390, netdev, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger, Sven Schnelle,
David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
On 22.06.23 09:53, Simon Horman wrote:
> On Thu, Jun 22, 2023 at 09:15:24AM +0200, Alexandra Winter wrote:
>>
>>
>> On 21.06.23 23:37, Randy Dunlap wrote:
>>> Require FDDI to be built-in if it is used. LCS needs FDDI to be
>>> built-in to build without errors.
>>>
>>> Prevents these build errors:
>>> s390-linux-ld: drivers/s390/net/lcs.o: in function `lcs_new_device':
>>> drivers/s390/net/lcs.c:2150: undefined reference to `fddi_type_trans'
>>> s390-linux-ld: drivers/s390/net/lcs.c:2151: undefined reference to `alloc_fddidev'
>>>
>>> This FDDI requirement effectively restores the previous condition
>>> before the blamed patch, when #ifdef CONFIG_FDDI was used, without
>>> testing for CONFIG_FDDI_MODULE.
>>>
>>> Fixes: 128272336120 ("s390/net: lcs: use IS_ENABLED() for kconfig detection")
[...]
>
>> 2) I wonder whether
>>
>> depends on CCW && NETDEVICES && (ETHERNET || FDDI)
>> + depends on FDDI || FDDI=n
>>
>> would do what we want here:
>> When FDDI is a loadable module, LCS mustn't be built-in.
>>
>> I will do some experiments and let you know.
>
> It does seem to on my side.
> But checking would be much appreciated.
Here are my experiments:
Current net-next:
-----------------
if !IS_ENABLED(CONFIG_ETHERNET) && !IS_ENABLED(CONFIG_FDDI)
drivers/s390/net/KConfig:
config LCS
def_tristate m
depends on CCW && NETDEVICES && (ETHERNET || FDDI)
.config:
ETHERNET | FDDI | LCS choices | LCS | compile
--------------------------------------------------------
n m m,n m success (failed before Randy's fix)
y m y,m,n m success (failed before Randy's fix)
y m y fails: undefined reference to `fddi_type_trans'
Simon's proposal:
-----------------
depends on CCW && NETDEVICES && (ETHERNET || FDDI)
+ depends on FDDI=y || FDDI=n
ETHERNET | FDDI | LCS choices | LCS | compile
--------------------------------------------------------
n m -
y m -
y m -
y n y,m,n y success
y n y,m,n m success
y y y,m,n m success
Alexandra's proposal:
---------------------
depends on CCW && NETDEVICES && (ETHERNET || FDDI)
+ depends on FDDI || FDDI=n
ETHERNET | FDDI | LCS choices | LCS | compile
--------------------------------------------------------
n m m,n m success
y m m,n m success
y n y,m,n y success
y n y,m,n m success
y y y,m,n m success
-----------------------------------------------------------
Seems that
A[tristate] depends on B[tristate]
means that A cannot be 'higher' than B.
Meaning, if B=n -> A= must be n
if B=m -> A can be m or n
if B=y -> A can be y or m or n
Although I did not find documentation confirming that.
@Randy, do you want give a v2 a try with that?
I guess then it is safe to delete from drivers/s390/net/lcs.c
-#if !IS_ENABLED(CONFIG_ETHERNET) && !IS_ENABLED(CONFIG_FDDI)
-#error Cannot compile lcs.c without some net devices switched on.
-#endif
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] s390/net: lcs: fix build errors when FDDI is a loadable module
2023-06-22 12:16 ` Alexandra Winter
@ 2023-06-22 12:33 ` Christian Borntraeger
2023-06-28 2:53 ` Randy Dunlap
1 sibling, 0 replies; 9+ messages in thread
From: Christian Borntraeger @ 2023-06-22 12:33 UTC (permalink / raw)
To: Alexandra Winter, Simon Horman
Cc: Randy Dunlap, linux-kernel, kernel test robot, Wenjia Zhang,
linux-s390, netdev, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Sven Schnelle, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni
Am 22.06.23 um 14:16 schrieb Alexandra Winter:
>
>
> On 22.06.23 09:53, Simon Horman wrote:
>> On Thu, Jun 22, 2023 at 09:15:24AM +0200, Alexandra Winter wrote:
>>>
>>>
>>> On 21.06.23 23:37, Randy Dunlap wrote:
>>>> Require FDDI to be built-in if it is used. LCS needs FDDI to be
>>>> built-in to build without errors.
>>>>
>>>> Prevents these build errors:
>>>> s390-linux-ld: drivers/s390/net/lcs.o: in function `lcs_new_device':
>>>> drivers/s390/net/lcs.c:2150: undefined reference to `fddi_type_trans'
>>>> s390-linux-ld: drivers/s390/net/lcs.c:2151: undefined reference to `alloc_fddidev'
>>>>
>>>> This FDDI requirement effectively restores the previous condition
>>>> before the blamed patch, when #ifdef CONFIG_FDDI was used, without
>>>> testing for CONFIG_FDDI_MODULE.
>>>>
>>>> Fixes: 128272336120 ("s390/net: lcs: use IS_ENABLED() for kconfig detection")
> [...]
>>
>>> 2) I wonder whether
>>>
>>> depends on CCW && NETDEVICES && (ETHERNET || FDDI)
>>> + depends on FDDI || FDDI=n
>>>
>>> would do what we want here:
>>> When FDDI is a loadable module, LCS mustn't be built-in.
>>>
>>> I will do some experiments and let you know.
>>
>> It does seem to on my side.
>> But checking would be much appreciated.
>
>
> Here are my experiments:
Another suggestion. Why not remove the FDDI part of the lcs driver? This seems unused
without hardware for years now.Longterm we could even remove the whole lcs driver.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] s390/net: lcs: fix build errors when FDDI is a loadable module
2023-06-22 7:15 ` Alexandra Winter
2023-06-22 7:53 ` Simon Horman
@ 2023-06-22 15:36 ` Randy Dunlap
1 sibling, 0 replies; 9+ messages in thread
From: Randy Dunlap @ 2023-06-22 15:36 UTC (permalink / raw)
To: Alexandra Winter, linux-kernel
Cc: kernel test robot, Simon Horman, Wenjia Zhang, linux-s390, netdev,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni
On 6/22/23 00:15, Alexandra Winter wrote:
>
>
> On 21.06.23 23:37, Randy Dunlap wrote:
>> Require FDDI to be built-in if it is used. LCS needs FDDI to be
>> built-in to build without errors.
>>
>> Prevents these build errors:
>> s390-linux-ld: drivers/s390/net/lcs.o: in function `lcs_new_device':
>> drivers/s390/net/lcs.c:2150: undefined reference to `fddi_type_trans'
>> s390-linux-ld: drivers/s390/net/lcs.c:2151: undefined reference to `alloc_fddidev'
>>
>> This FDDI requirement effectively restores the previous condition
>> before the blamed patch, when #ifdef CONFIG_FDDI was used, without
>> testing for CONFIG_FDDI_MODULE.
>>
>> Fixes: 128272336120 ("s390/net: lcs: use IS_ENABLED() for kconfig detection")
>> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
>> Reported-by: kernel test robot <lkp@intel.com>
>> Link: lore.kernel.org/r/202306202129.pl0AqK8G-lkp@intel.com
>> Suggested-by: Simon Horman <simon.horman@corigine.com>
>> Cc: Alexandra Winter <wintera@linux.ibm.com>
>> Cc: Wenjia Zhang <wenjia@linux.ibm.com>
>> Cc: linux-s390@vger.kernel.org
>> Cc: netdev@vger.kernel.org
>> Cc: Heiko Carstens <hca@linux.ibm.com>
>> Cc: Vasily Gorbik <gor@linux.ibm.com>
>> Cc: Alexander Gordeev <agordeev@linux.ibm.com>
>> Cc: Christian Borntraeger <borntraeger@linux.ibm.com>
>> Cc: Sven Schnelle <svens@linux.ibm.com>
>> Cc: David S. Miller <davem@davemloft.net>
>> Cc: Eric Dumazet <edumazet@google.com>
>> Cc: Jakub Kicinski <kuba@kernel.org>
>> Cc: Paolo Abeni <pabeni@redhat.com>
>> ---
>> drivers/s390/net/Kconfig | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff -- a/drivers/s390/net/Kconfig b/drivers/s390/net/Kconfig
>> --- a/drivers/s390/net/Kconfig
>> +++ b/drivers/s390/net/Kconfig
>> @@ -6,11 +6,13 @@ config LCS
>> def_tristate m
>> prompt "Lan Channel Station Interface"
>> depends on CCW && NETDEVICES && (ETHERNET || FDDI)
>> + depends on FDDI=y || FDDI=n
>> help
>> Select this option if you want to use LCS networking on IBM System z.
>> This device driver supports FDDI (IEEE 802.7) and Ethernet.
>> To compile as a module, choose M. The module name is lcs.
>> If you do not know what it is, it's safe to choose Y.
>> + If FDDI is used, it must be built-in (=y).
>>
>> config CTCM
>> def_tristate m
>>
>
>
> Wow Randy and Simon, you are reacting faster than I was able to evaluate this yesterday.
> 2 thoughts:
>
> 1) As ETHERNET cannot be a module and this patch prevents FDDI from being a module, then
> 128272336120 ("s390/net: lcs: use IS_ENABLED() for kconfig detection")
> is kind of pointless and can as well be reverted instead of doing this fix.
> Or am I missing something?
Hi,
I'll just send a revert for now and then work on the next step(s).
thanks.
--
~Randy
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] s390/net: lcs: fix build errors when FDDI is a loadable module
2023-06-22 12:16 ` Alexandra Winter
2023-06-22 12:33 ` Christian Borntraeger
@ 2023-06-28 2:53 ` Randy Dunlap
2023-06-28 5:06 ` Randy Dunlap
1 sibling, 1 reply; 9+ messages in thread
From: Randy Dunlap @ 2023-06-28 2:53 UTC (permalink / raw)
To: Alexandra Winter, Simon Horman
Cc: linux-kernel, kernel test robot, Wenjia Zhang, linux-s390, netdev,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni
Hi,
Sorry for the delay.
On 6/22/23 05:16, Alexandra Winter wrote:
>
>
> On 22.06.23 09:53, Simon Horman wrote:
>> On Thu, Jun 22, 2023 at 09:15:24AM +0200, Alexandra Winter wrote:
>>>
>>>
>>> On 21.06.23 23:37, Randy Dunlap wrote:
>>>> Require FDDI to be built-in if it is used. LCS needs FDDI to be
>>>> built-in to build without errors.
>>>>
>>>> Prevents these build errors:
>>>> s390-linux-ld: drivers/s390/net/lcs.o: in function `lcs_new_device':
>>>> drivers/s390/net/lcs.c:2150: undefined reference to `fddi_type_trans'
>>>> s390-linux-ld: drivers/s390/net/lcs.c:2151: undefined reference to `alloc_fddidev'
>>>>
>>>> This FDDI requirement effectively restores the previous condition
>>>> before the blamed patch, when #ifdef CONFIG_FDDI was used, without
>>>> testing for CONFIG_FDDI_MODULE.
>>>>
>>>> Fixes: 128272336120 ("s390/net: lcs: use IS_ENABLED() for kconfig detection")
> [...]
>>
>>> 2) I wonder whether
>>>
>>> depends on CCW && NETDEVICES && (ETHERNET || FDDI)
>>> + depends on FDDI || FDDI=n
>>>
>>> would do what we want here:
>>> When FDDI is a loadable module, LCS mustn't be built-in.
>>>
>>> I will do some experiments and let you know.
>>
>> It does seem to on my side.
>> But checking would be much appreciated.
>
>
> Here are my experiments:
>
> Current net-next:
> -----------------
> if !IS_ENABLED(CONFIG_ETHERNET) && !IS_ENABLED(CONFIG_FDDI)
>
> drivers/s390/net/KConfig:
> config LCS
> def_tristate m
> depends on CCW && NETDEVICES && (ETHERNET || FDDI)
>
> .config:
> ETHERNET | FDDI | LCS choices | LCS | compile
> --------------------------------------------------------
> n m m,n m success (failed before Randy's fix)
> y m y,m,n m success (failed before Randy's fix)
> y m y fails: undefined reference to `fddi_type_trans'
>
>
> Simon's proposal:
> -----------------
> depends on CCW && NETDEVICES && (ETHERNET || FDDI)
> + depends on FDDI=y || FDDI=n
>
> ETHERNET | FDDI | LCS choices | LCS | compile
> --------------------------------------------------------
> n m -
> y m -
> y m -
> y n y,m,n y success
> y n y,m,n m success
> y y y,m,n m success
>
>
> Alexandra's proposal:
> ---------------------
> depends on CCW && NETDEVICES && (ETHERNET || FDDI)
> + depends on FDDI || FDDI=n
>
> ETHERNET | FDDI | LCS choices | LCS | compile
> --------------------------------------------------------
> n m m,n m success
> y m m,n m success
> y n y,m,n y success
> y n y,m,n m success
> y y y,m,n m success
>
> -----------------------------------------------------------
>
> Seems that
> A[tristate] depends on B[tristate]
> means that A cannot be 'higher' than B.
> Meaning, if B=n -> A= must be n
> if B=m -> A can be m or n
> if B=y -> A can be y or m or n
Looks correct.
> Although I did not find documentation confirming that.
I think that it's in Documentation/kbuild/kconfig-language.rst,
under "Menu dependencies", but not quite in that format. :)
>
> @Randy, do you want give a v2 a try with that?
Sure, I'll try that.
> I guess then it is safe to delete from drivers/s390/net/lcs.c
> -#if !IS_ENABLED(CONFIG_ETHERNET) && !IS_ENABLED(CONFIG_FDDI)
> -#error Cannot compile lcs.c without some net devices switched on.
> -#endif
Yes, I was planning to do that as well.
Thanks for the time that you have spent on this.
--
~Randy
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] s390/net: lcs: fix build errors when FDDI is a loadable module
2023-06-28 2:53 ` Randy Dunlap
@ 2023-06-28 5:06 ` Randy Dunlap
[not found] ` <a05a7c3a-0f2f-c3be-3630-6774a26b994f@linux.ibm.com>
0 siblings, 1 reply; 9+ messages in thread
From: Randy Dunlap @ 2023-06-28 5:06 UTC (permalink / raw)
To: Alexandra Winter, Simon Horman
Cc: linux-kernel, kernel test robot, Wenjia Zhang, linux-s390, netdev,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman
Hi Alexandra, Simon, others,
Here is v2 of this patch. I will send it formally after the merge window closes.
Thanks for all of your help.
---
From: Randy Dunlap <rdunlap@infradead.org>
Subject: [PATCH v2 net] s390/net: lcs: use IS_ENABLED() for kconfig detection
When CONFIG_ETHERNET is disabled and CONFIG_FDDI=m, lcs.s has build
errors or warnings:
../drivers/s390/net/lcs.c:40:2: error: #error Cannot compile lcs.c without some net devices switched on.
40 | #error Cannot compile lcs.c without some net devices switched on.
../drivers/s390/net/lcs.c: In function 'lcs_startlan_auto':
../drivers/s390/net/lcs.c:1601:13: warning: unused variable 'rc' [-Wunused-variable]
1601 | int rc;
Solve this by using IS_ENABLED(CONFIG_symbol) instead of ifdef
CONFIG_symbol. The latter only works for builtin (=y) values
while IS_ENABLED() works for builtin or modular values.
Modify the LCS Kconfig entry to allow combinations of builtin and
modular drivers to work as long as LCS <= FDDI (where n < m < y)
if FDDI is enabled. If FDDI is not enabled, ETHERNET must be =y,
so LCS can be builtin or modular since ETHERNET is a bool.
Remove the #error directive in the source file since the Kconfig
modification prevents that error combination.
Tested successfully with all possible combinations of ETHERNET, FDDI,
and LCS.
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
Suggested-by: Alexandra Winter <wintera@linux.ibm.com>
Cc: Wenjia Zhang <wenjia@linux.ibm.com>
Cc: linux-s390@vger.kernel.org
Cc: netdev@vger.kernel.org
Cc: Heiko Carstens <hca@linux.ibm.com>
Cc: Vasily Gorbik <gor@linux.ibm.com>
Cc: Alexander Gordeev <agordeev@linux.ibm.com>
Cc: Christian Borntraeger <borntraeger@linux.ibm.com>
Cc: Sven Schnelle <svens@linux.ibm.com>
Cc: Simon Horman <simon.horman@corigine.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
---
drivers/s390/net/Kconfig | 1 +
drivers/s390/net/lcs.c | 13 ++++---------
2 files changed, 5 insertions(+), 9 deletions(-)
diff -- a/drivers/s390/net/Kconfig b/drivers/s390/net/Kconfig
--- a/drivers/s390/net/Kconfig
+++ b/drivers/s390/net/Kconfig
@@ -6,6 +6,7 @@ config LCS
def_tristate m
prompt "Lan Channel Station Interface"
depends on CCW && NETDEVICES && (ETHERNET || FDDI)
+ depends on FDDI || FDDI=n
help
Select this option if you want to use LCS networking on IBM System z.
This device driver supports FDDI (IEEE 802.7) and Ethernet.
diff -- a/drivers/s390/net/lcs.c b/drivers/s390/net/lcs.c
--- a/drivers/s390/net/lcs.c
+++ b/drivers/s390/net/lcs.c
@@ -35,11 +35,6 @@
#include "lcs.h"
-
-#if !defined(CONFIG_ETHERNET) && !defined(CONFIG_FDDI)
-#error Cannot compile lcs.c without some net devices switched on.
-#endif
-
/*
* initialization string for output
*/
@@ -1601,14 +1596,14 @@ lcs_startlan_auto(struct lcs_card *card)
int rc;
LCS_DBF_TEXT(2, trace, "strtauto");
-#ifdef CONFIG_ETHERNET
+#if IS_ENABLED(CONFIG_ETHERNET)
card->lan_type = LCS_FRAME_TYPE_ENET;
rc = lcs_send_startlan(card, LCS_INITIATOR_TCPIP);
if (rc == 0)
return 0;
#endif
-#ifdef CONFIG_FDDI
+#if IS_ENABLED(CONFIG_FDDI)
card->lan_type = LCS_FRAME_TYPE_FDDI;
rc = lcs_send_startlan(card, LCS_INITIATOR_TCPIP);
if (rc == 0)
@@ -2140,13 +2135,13 @@ lcs_new_device(struct ccwgroup_device *c
goto netdev_out;
}
switch (card->lan_type) {
-#ifdef CONFIG_ETHERNET
+#if IS_ENABLED(CONFIG_ETHERNET)
case LCS_FRAME_TYPE_ENET:
card->lan_type_trans = eth_type_trans;
dev = alloc_etherdev(0);
break;
#endif
-#ifdef CONFIG_FDDI
+#if IS_ENABLED(CONFIG_FDDI)
case LCS_FRAME_TYPE_FDDI:
card->lan_type_trans = fddi_type_trans;
dev = alloc_fddidev(0);
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] s390/net: lcs: fix build errors when FDDI is a loadable module
[not found] ` <a05a7c3a-0f2f-c3be-3630-6774a26b994f@linux.ibm.com>
@ 2023-06-28 15:29 ` Randy Dunlap
0 siblings, 0 replies; 9+ messages in thread
From: Randy Dunlap @ 2023-06-28 15:29 UTC (permalink / raw)
To: Alexandra Winter, Simon Horman
Cc: linux-kernel, kernel test robot, Wenjia Zhang, linux-s390, netdev,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni
On 6/28/23 06:41, Alexandra Winter wrote:
>
>
> On 28.06.23 07:06, Randy Dunlap wrote:
>> Hi Alexandra, Simon, others,
>>
>> Here is v2 of this patch. I will send it formally after the merge window closes.
>>
>> Thanks for all of your help.
>> ---
>
> Thank you for the patch, Randy.
>
> As suggested by Christian Bornträger, I did some research, whether the FDDI part of the LCS driver
> could be removed. And actually there is no s390 machine above the minimum architecture level that
> can have an FDDI interface.
> I will send a patch to remove the FDDI option from the lcs driver.
>
> I apologize that I was not aware of that earlier. And thank you again for pointing out the issue
> with FDDI as a module.
No problem. Thanks for the new patch.
I will trash all of my LCS patches. :)
--
~Randy
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2023-06-28 15:29 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-06-21 21:37 [PATCH] s390/net: lcs: fix build errors when FDDI is a loadable module Randy Dunlap
2023-06-22 7:15 ` Alexandra Winter
2023-06-22 7:53 ` Simon Horman
2023-06-22 12:16 ` Alexandra Winter
2023-06-22 12:33 ` Christian Borntraeger
2023-06-28 2:53 ` Randy Dunlap
2023-06-28 5:06 ` Randy Dunlap
[not found] ` <a05a7c3a-0f2f-c3be-3630-6774a26b994f@linux.ibm.com>
2023-06-28 15:29 ` Randy Dunlap
2023-06-22 15:36 ` Randy Dunlap
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).