Message ID | 20230621213742.8245-1-rdunlap@infradead.org |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:994d:0:b0:3d9:f83d:47d9 with SMTP id k13csp4670579vqr; Wed, 21 Jun 2023 14:58:53 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ6Z0MIxuZ4/sZ9HxmlyDAA8uyyZTwIE2tYXz9U54/hj38FDOOV06FvD44eLtSYDb0VWNNQe X-Received: by 2002:a05:6a00:194d:b0:666:2889:32bd with SMTP id s13-20020a056a00194d00b00666288932bdmr7946746pfk.9.1687384733462; Wed, 21 Jun 2023 14:58:53 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1687384733; cv=none; d=google.com; s=arc-20160816; b=DUCiyu0gmVs516FkQPgen9zRwYH0iHqqT/dOuAfB1X/BHHws9gXDDq7S7CH2cV1LxY O5ebSsxF80+DztiHbDRn9D6Q+XAuCtaQUV7DGMFE4SOPlOCJQmkXhZHgy3w6TeQ6VekX kWVhefkAefNjVq8Choh0UDRereeQqpOOeA4f59vYJ8LQ+GBodmgiR2x4FCPK3468ufdI lJM+us/LK2aA8umjazGepmDGolmdsy5kNFDE3y/VzL+NQ+NWebYuWfMO8wZMkFQR+bJa bEvgl7aRJoSMwqjT+Be4zjZl09wipq9OeyJoD8xJ2OlqgXwMg69F2g/JGPRAxMkrFgwx LLTQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from:dkim-signature; bh=pkZgplOLZ8AEWdttiF8Ut3/Ee7D1W2OVcYqdc7VQf0E=; b=qq6rzl0YJhcM4m6trtPv9CFiok42xfHP+qtAMqxdCs7JhshPAGuiNjB4rJPSRDHQBH Qbg44grMp9w/FAzU39XiPr35Fhm6QTL0ZA7ZVt+6FD6BtLXrNYFP9PNlqKPZ6n9N69GQ UCPnXYA9cW4fDcgcgSWPxLj7ecl8w9C97svs5jRSh3UGx6gDUaubCtRGjJEJu4kvJxb+ aJ/e+SPbny+CcEDWQIhuj5u5GIihK94Rih5fMEnLqO3Msy1t4+evUI3hkRhMB5sda1+n ClCpEFisVYvT/PgVjLeK3t1gIS9MthgJGPAIKF95dpF5arshaSXBbBhU9OtNdLxBSwzy AtHw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@infradead.org header.s=bombadil.20210309 header.b=hahh76qv; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id b5-20020a63eb45000000b005404f8dd0eesi5080550pgk.602.2023.06.21.14.58.41; Wed, 21 Jun 2023 14:58:53 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@infradead.org header.s=bombadil.20210309 header.b=hahh76qv; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230475AbjFUVia (ORCPT <rfc822;maxin.john@gmail.com> + 99 others); Wed, 21 Jun 2023 17:38:30 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:53746 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230347AbjFUVi2 (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Wed, 21 Jun 2023 17:38:28 -0400 Received: from bombadil.infradead.org (bombadil.infradead.org [IPv6:2607:7c80:54:3::133]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 29D1B1988; Wed, 21 Jun 2023 14:37:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=bombadil.20210309; h=Content-Transfer-Encoding: MIME-Version:Message-ID:Date:Subject:Cc:To:From:Sender:Reply-To:Content-Type: Content-ID:Content-Description:In-Reply-To:References; bh=pkZgplOLZ8AEWdttiF8Ut3/Ee7D1W2OVcYqdc7VQf0E=; b=hahh76qvYl0Fq2GFFDkufVXBNW oV2SJ1FhPuHkEy8W4LoBtBWC42GP7UJZe6fuCMix3NeaJpAMLLIDUBjqVvjoIUE/i86SVeM+lcP7q 9MPBcOg8Kn7W/8izfWD6BIgq9RTItyF4Jqke2LfeA0YFjc3LooxYwi759wIfBbiqjnPYbkIhktmeO +Rapg1fUGOb2nKLhosk0mXDcggxZ5z0UPa5t+Gu7XDxSjYNDuXKtge4g+Z9SpIGSY4y3inPey7A3b Qar56EuAacwqJ+UjC4NlgqHAcBeA6g60aEiHm5rWzAWAErbCDlZwPley9kZivrg7iSRYP/QgOtKDW Z/v97k4g==; Received: from [2601:1c2:980:9ec0::2764] (helo=bombadil.infradead.org) by bombadil.infradead.org with esmtpsa (Exim 4.96 #2 (Red Hat Linux)) id 1qC5WJ-00FnwL-1q; Wed, 21 Jun 2023 21:37:43 +0000 From: Randy Dunlap <rdunlap@infradead.org> To: linux-kernel@vger.kernel.org Cc: Randy Dunlap <rdunlap@infradead.org>, kernel test robot <lkp@intel.com>, Simon Horman <simon.horman@corigine.com>, Alexandra Winter <wintera@linux.ibm.com>, Wenjia Zhang <wenjia@linux.ibm.com>, linux-s390@vger.kernel.org, netdev@vger.kernel.org, Heiko Carstens <hca@linux.ibm.com>, Vasily Gorbik <gor@linux.ibm.com>, Alexander Gordeev <agordeev@linux.ibm.com>, Christian Borntraeger <borntraeger@linux.ibm.com>, Sven Schnelle <svens@linux.ibm.com>, "David S . Miller" <davem@davemloft.net>, Eric Dumazet <edumazet@google.com>, Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com> Subject: [PATCH] s390/net: lcs: fix build errors when FDDI is a loadable module Date: Wed, 21 Jun 2023 14:37:42 -0700 Message-ID: <20230621213742.8245-1-rdunlap@infradead.org> X-Mailer: git-send-email 2.41.0 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,SPF_HELO_NONE, SPF_NONE,T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1769351134289358515?= X-GMAIL-MSGID: =?utf-8?q?1769351134289358515?= |
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
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
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.
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
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.
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.
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.
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);
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
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. :)
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