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

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

[Randy Dunlap]

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

Re: [PATCH] s390/net: lcs: fix build errors when FDDI is a loadable module

[Alexandra Winter]

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

Re: [PATCH] s390/net: lcs: fix build errors when FDDI is a loadable module

[Simon Horman]

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.

Re: [PATCH] s390/net: lcs: fix build errors when FDDI is a loadable module

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

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

Re: [PATCH] s390/net: lcs: fix build errors when FDDI is a loadable module

[Christian Borntraeger]

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.

Re: [PATCH] s390/net: lcs: fix build errors when FDDI is a loadable module

[Randy Dunlap]

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

Re: [PATCH] s390/net: lcs: fix build errors when FDDI is a loadable module

[Randy Dunlap]

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);

Re: [PATCH] s390/net: lcs: fix build errors when FDDI is a loadable module

[Randy Dunlap]

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

Re: [PATCH] s390/net: lcs: fix build errors when FDDI is a loadable module

[Randy Dunlap]

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