s390/net: lcs: fix build errors when FDDI is a loadable module

Message ID 20230621213742.8245-1-rdunlap@infradead.org
State New
Headers
Series s390/net: lcs: fix build errors when FDDI is a loadable module |

Commit Message

Randy Dunlap June 21, 2023, 9:37 p.m. UTC
  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(+)
  

Comments

Alexandra Winter June 22, 2023, 7:15 a.m. UTC | #1
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
  
Simon Horman June 22, 2023, 7:53 a.m. UTC | #2
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.
  
Alexandra Winter June 22, 2023, 12:16 p.m. UTC | #3
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
  
Christian Borntraeger June 22, 2023, 12:33 p.m. UTC | #4
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.
  
Randy Dunlap June 22, 2023, 3:36 p.m. UTC | #5
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 Dunlap June 28, 2023, 2:53 a.m. UTC | #6
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 Dunlap June 28, 2023, 5:06 a.m. UTC | #7
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);
  
Alexandra Winter June 28, 2023, 1:41 p.m. UTC | #8
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.

Alexandra
  
Randy Dunlap June 28, 2023, 3:29 p.m. UTC | #9
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. :)
  

Patch

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