[PATCH] s390/lcs: Remove FDDI option

[PATCH] s390/lcs: Remove FDDI option

[Alexandra Winter]

The last s390 machine that supported FDDI was z900 ('7th generation',
released in 2000). The oldest machine generation currently supported by
the Linux kernel is MARCH_Z10 (released 2008). If there is still a usecase
for connecting a Linux on s390 instance to a LAN Channel Station (LCS), it
can only do so via Ethernet.

Randy Dunlap[1] found that LCS over FDDI has never worked, when FDDI
was compiled as module. Instead of fixing that, remove the FDDI option
from the lcs driver.

While at it, make the CONFIG_LCS description a bit more helpful.

References:
[1] https://lore.kernel.org/netdev/20230621213742.8245-1-rdunlap@infradead.org/

Signed-off-by: Alexandra Winter <wintera@linux.ibm.com>
---
 drivers/s390/net/Kconfig |  5 ++---
 drivers/s390/net/lcs.c   | 39 ++++++---------------------------------
 2 files changed, 8 insertions(+), 36 deletions(-)

diff --git a/drivers/s390/net/Kconfig b/drivers/s390/net/Kconfig
index 9c67b97faba2..74760c1a163b 100644
--- a/drivers/s390/net/Kconfig
+++ b/drivers/s390/net/Kconfig
@@ -5,12 +5,11 @@ menu "S/390 network device drivers"
 config LCS
 	def_tristate m
 	prompt "Lan Channel Station Interface"
-	depends on CCW && NETDEVICES && (ETHERNET || FDDI)
+	depends on CCW && NETDEVICES && ETHERNET
 	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 you do not use LCS, choose N.
 
 config CTCM
 	def_tristate m
diff --git a/drivers/s390/net/lcs.c b/drivers/s390/net/lcs.c
index 9fd8e6f07a03..a1f2acd6fb8f 100644
--- a/drivers/s390/net/lcs.c
+++ b/drivers/s390/net/lcs.c
@@ -17,7 +17,6 @@
 #include <linux/if.h>
 #include <linux/netdevice.h>
 #include <linux/etherdevice.h>
-#include <linux/fddidevice.h>
 #include <linux/inetdevice.h>
 #include <linux/in.h>
 #include <linux/igmp.h>
@@ -36,10 +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,19 +1596,11 @@ lcs_startlan_auto(struct lcs_card *card)
 	int rc;
 
 	LCS_DBF_TEXT(2, trace, "strtauto");
-#ifdef 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
-	card->lan_type = LCS_FRAME_TYPE_FDDI;
-	rc = lcs_send_startlan(card, LCS_INITIATOR_TCPIP);
-	if (rc == 0)
-		return 0;
-#endif
 	return -EIO;
 }
 
@@ -1806,22 +1793,16 @@ lcs_get_frames_cb(struct lcs_channel *channel, struct lcs_buffer *buffer)
 			card->stats.rx_errors++;
 			return;
 		}
-		/* What kind of frame is it? */
-		if (lcs_hdr->type == LCS_FRAME_TYPE_CONTROL) {
-			/* Control frame. */
+		if (lcs_hdr->type == LCS_FRAME_TYPE_CONTROL)
 			lcs_get_control(card, (struct lcs_cmd *) lcs_hdr);
-		} else if (lcs_hdr->type == LCS_FRAME_TYPE_ENET ||
-			   lcs_hdr->type == LCS_FRAME_TYPE_TR ||
-			   lcs_hdr->type == LCS_FRAME_TYPE_FDDI) {
-			/* Normal network packet. */
+		else if (lcs_hdr->type == LCS_FRAME_TYPE_ENET)
 			lcs_get_skb(card, (char *)(lcs_hdr + 1),
 				    lcs_hdr->offset - offset -
 				    sizeof(struct lcs_header));
-		} else {
-			/* Unknown frame type. */
-			; // FIXME: error message ?
-		}
-		/* Proceed to next frame. */
+		else
+			dev_info_once(&card->dev->dev,
+				      "Unknown frame type %d\n",
+				      lcs_hdr->type);
 		offset = lcs_hdr->offset;
 		lcs_hdr->offset = LCS_ILLEGAL_OFFSET;
 		lcs_hdr = (struct lcs_header *) (buffer->data + offset);
@@ -2140,18 +2121,10 @@ lcs_new_device(struct ccwgroup_device *ccwgdev)
 		goto netdev_out;
 	}
 	switch (card->lan_type) {
-#ifdef CONFIG_ETHERNET
 	case LCS_FRAME_TYPE_ENET:
 		card->lan_type_trans = eth_type_trans;
 		dev = alloc_etherdev(0);
 		break;
-#endif
-#ifdef CONFIG_FDDI
-	case LCS_FRAME_TYPE_FDDI:
-		card->lan_type_trans = fddi_type_trans;
-		dev = alloc_fddidev(0);
-		break;
-#endif
 	default:
 		LCS_DBF_TEXT(3, setup, "errinit");
 		pr_err(" Initialization failed\n");
-- 
2.37.1 (Apple Git-137.1)

Re: [PATCH] s390/lcs: Remove FDDI option

[Christian Borntraeger]

Am 28.06.23 um 15:57 schrieb Alexandra Winter:
> The last s390 machine that supported FDDI was z900 ('7th generation',
> released in 2000). The oldest machine generation currently supported by
> the Linux kernel is MARCH_Z10 (released 2008). If there is still a usecase
> for connecting a Linux on s390 instance to a LAN Channel Station (LCS), it
> can only do so via Ethernet.
> 
> Randy Dunlap[1] found that LCS over FDDI has never worked, when FDDI
> was compiled as module. Instead of fixing that, remove the FDDI option
> from the lcs driver.
> 
> While at it, make the CONFIG_LCS description a bit more helpful.
> 
> References:
> [1] https://lore.kernel.org/netdev/20230621213742.8245-1-rdunlap@infradead.org/
> 
> Signed-off-by: Alexandra Winter <wintera@linux.ibm.com>

Makes perfect sense given the complexity.

Acked-by: Christian Borntraeger <borntraeger@linux.ibm.com>


> ---
>   drivers/s390/net/Kconfig |  5 ++---
>   drivers/s390/net/lcs.c   | 39 ++++++---------------------------------
>   2 files changed, 8 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/s390/net/Kconfig b/drivers/s390/net/Kconfig
> index 9c67b97faba2..74760c1a163b 100644
> --- a/drivers/s390/net/Kconfig
> +++ b/drivers/s390/net/Kconfig
> @@ -5,12 +5,11 @@ menu "S/390 network device drivers"
>   config LCS
>   	def_tristate m
>   	prompt "Lan Channel Station Interface"
> -	depends on CCW && NETDEVICES && (ETHERNET || FDDI)
> +	depends on CCW && NETDEVICES && ETHERNET
>   	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 you do not use LCS, choose N.
>   
>   config CTCM
>   	def_tristate m
> diff --git a/drivers/s390/net/lcs.c b/drivers/s390/net/lcs.c
> index 9fd8e6f07a03..a1f2acd6fb8f 100644
> --- a/drivers/s390/net/lcs.c
> +++ b/drivers/s390/net/lcs.c
> @@ -17,7 +17,6 @@
>   #include <linux/if.h>
>   #include <linux/netdevice.h>
>   #include <linux/etherdevice.h>
> -#include <linux/fddidevice.h>
>   #include <linux/inetdevice.h>
>   #include <linux/in.h>
>   #include <linux/igmp.h>
> @@ -36,10 +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,19 +1596,11 @@ lcs_startlan_auto(struct lcs_card *card)
>   	int rc;
>   
>   	LCS_DBF_TEXT(2, trace, "strtauto");
> -#ifdef 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
> -	card->lan_type = LCS_FRAME_TYPE_FDDI;
> -	rc = lcs_send_startlan(card, LCS_INITIATOR_TCPIP);
> -	if (rc == 0)
> -		return 0;
> -#endif
>   	return -EIO;
>   }
>   
> @@ -1806,22 +1793,16 @@ lcs_get_frames_cb(struct lcs_channel *channel, struct lcs_buffer *buffer)
>   			card->stats.rx_errors++;
>   			return;
>   		}
> -		/* What kind of frame is it? */
> -		if (lcs_hdr->type == LCS_FRAME_TYPE_CONTROL) {
> -			/* Control frame. */
> +		if (lcs_hdr->type == LCS_FRAME_TYPE_CONTROL)
>   			lcs_get_control(card, (struct lcs_cmd *) lcs_hdr);
> -		} else if (lcs_hdr->type == LCS_FRAME_TYPE_ENET ||
> -			   lcs_hdr->type == LCS_FRAME_TYPE_TR ||
> -			   lcs_hdr->type == LCS_FRAME_TYPE_FDDI) {
> -			/* Normal network packet. */
> +		else if (lcs_hdr->type == LCS_FRAME_TYPE_ENET)
>   			lcs_get_skb(card, (char *)(lcs_hdr + 1),
>   				    lcs_hdr->offset - offset -
>   				    sizeof(struct lcs_header));
> -		} else {
> -			/* Unknown frame type. */
> -			; // FIXME: error message ?
> -		}
> -		/* Proceed to next frame. */
> +		else
> +			dev_info_once(&card->dev->dev,
> +				      "Unknown frame type %d\n",
> +				      lcs_hdr->type);
>   		offset = lcs_hdr->offset;
>   		lcs_hdr->offset = LCS_ILLEGAL_OFFSET;
>   		lcs_hdr = (struct lcs_header *) (buffer->data + offset);
> @@ -2140,18 +2121,10 @@ lcs_new_device(struct ccwgroup_device *ccwgdev)
>   		goto netdev_out;
>   	}
>   	switch (card->lan_type) {
> -#ifdef CONFIG_ETHERNET
>   	case LCS_FRAME_TYPE_ENET:
>   		card->lan_type_trans = eth_type_trans;
>   		dev = alloc_etherdev(0);
>   		break;
> -#endif
> -#ifdef CONFIG_FDDI
> -	case LCS_FRAME_TYPE_FDDI:
> -		card->lan_type_trans = fddi_type_trans;
> -		dev = alloc_fddidev(0);
> -		break;
> -#endif
>   	default:
>   		LCS_DBF_TEXT(3, setup, "errinit");
>   		pr_err(" Initialization failed\n");

Re: [PATCH] s390/lcs: Remove FDDI option

[Simon Horman]

On Wed, Jun 28, 2023 at 03:57:36PM +0200, Alexandra Winter wrote:
> The last s390 machine that supported FDDI was z900 ('7th generation',
> released in 2000). The oldest machine generation currently supported by
> the Linux kernel is MARCH_Z10 (released 2008). If there is still a usecase
> for connecting a Linux on s390 instance to a LAN Channel Station (LCS), it
> can only do so via Ethernet.
> 
> Randy Dunlap[1] found that LCS over FDDI has never worked, when FDDI
> was compiled as module. Instead of fixing that, remove the FDDI option
> from the lcs driver.
> 
> While at it, make the CONFIG_LCS description a bit more helpful.
> 
> References:
> [1] https://lore.kernel.org/netdev/20230621213742.8245-1-rdunlap@infradead.org/
> 
> Signed-off-by: Alexandra Winter <wintera@linux.ibm.com>

[text from Jakub]

## Form letter - net-next-closed

The merge window for v6.5 has begun and therefore net-next is closed
for new drivers, features, code refactoring and optimizations.
We are currently accepting bug fixes only.

Please repost when net-next reopens after July 10th.

RFC patches sent for review only are obviously welcome at any time.

See: https://www.kernel.org/doc/html/next/process/maintainer-netdev.html#development-cycle
--
pw-bot: defer

Re: [PATCH] s390/lcs: Remove FDDI option

[Alexandra Winter]

On 28.06.23 22:11, Simon Horman wrote:
> [text from Jakub]
> 
> ## Form letter - net-next-closed
> 
> The merge window for v6.5 has begun and therefore net-next is closed
> for new drivers, features, code refactoring and optimizations.
> We are currently accepting bug fixes only.
> 
> Please repost when net-next reopens after July 10th.
> 
> RFC patches sent for review only are obviously welcome at any time.
> 
> See: https://www.kernel.org/doc/html/next/process/maintainer-netdev.html#development-cycle
> --
> pw-bot: defer

Thank you Simon for the information.

So http://vger.kernel.org/~davem/net-next.html
is no longer relevant?
(I was using that page to check, whether net-next was still open.)

Re: [PATCH] s390/lcs: Remove FDDI option

[Simon Horman]

On Thu, Jun 29, 2023 at 09:59:15AM +0200, Alexandra Winter wrote:
> 
> 
> On 28.06.23 22:11, Simon Horman wrote:
> > [text from Jakub]
> > 
> > ## Form letter - net-next-closed
> > 
> > The merge window for v6.5 has begun and therefore net-next is closed
> > for new drivers, features, code refactoring and optimizations.
> > We are currently accepting bug fixes only.
> > 
> > Please repost when net-next reopens after July 10th.
> > 
> > RFC patches sent for review only are obviously welcome at any time.
> > 
> > See: https://www.kernel.org/doc/html/next/process/maintainer-netdev.html#development-cycle
> > --
> > pw-bot: defer
> 
> Thank you Simon for the information.
> 
> So http://vger.kernel.org/~davem/net-next.html
> is no longer relevant?
> (I was using that page to check, whether net-next was still open.)

Unfortunately it seems that page sometimes gets out of sync.

Re: [PATCH] s390/lcs: Remove FDDI option

[Randy Dunlap]

On 6/28/23 06:57, Alexandra Winter wrote:
> The last s390 machine that supported FDDI was z900 ('7th generation',
> released in 2000). The oldest machine generation currently supported by
> the Linux kernel is MARCH_Z10 (released 2008). If there is still a usecase
> for connecting a Linux on s390 instance to a LAN Channel Station (LCS), it
> can only do so via Ethernet.
> 
> Randy Dunlap[1] found that LCS over FDDI has never worked, when FDDI
> was compiled as module. Instead of fixing that, remove the FDDI option
> from the lcs driver.
> 
> While at it, make the CONFIG_LCS description a bit more helpful.
> 
> References:
> [1] https://lore.kernel.org/netdev/20230621213742.8245-1-rdunlap@infradead.org/
> 
> Signed-off-by: Alexandra Winter <wintera@linux.ibm.com>


Reviewed-by: Randy Dunlap <rdunlap@infradead.org>

Thanks.

> ---
>  drivers/s390/net/Kconfig |  5 ++---
>  drivers/s390/net/lcs.c   | 39 ++++++---------------------------------
>  2 files changed, 8 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/s390/net/Kconfig b/drivers/s390/net/Kconfig
> index 9c67b97faba2..74760c1a163b 100644
> --- a/drivers/s390/net/Kconfig
> +++ b/drivers/s390/net/Kconfig
> @@ -5,12 +5,11 @@ menu "S/390 network device drivers"
>  config LCS
>  	def_tristate m
>  	prompt "Lan Channel Station Interface"
> -	depends on CCW && NETDEVICES && (ETHERNET || FDDI)
> +	depends on CCW && NETDEVICES && ETHERNET
>  	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 you do not use LCS, choose N.
>  
>  config CTCM
>  	def_tristate m
> diff --git a/drivers/s390/net/lcs.c b/drivers/s390/net/lcs.c
> index 9fd8e6f07a03..a1f2acd6fb8f 100644
> --- a/drivers/s390/net/lcs.c
> +++ b/drivers/s390/net/lcs.c
> @@ -17,7 +17,6 @@
>  #include <linux/if.h>
>  #include <linux/netdevice.h>
>  #include <linux/etherdevice.h>
> -#include <linux/fddidevice.h>
>  #include <linux/inetdevice.h>
>  #include <linux/in.h>
>  #include <linux/igmp.h>
> @@ -36,10 +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,19 +1596,11 @@ lcs_startlan_auto(struct lcs_card *card)
>  	int rc;
>  
>  	LCS_DBF_TEXT(2, trace, "strtauto");
> -#ifdef 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
> -	card->lan_type = LCS_FRAME_TYPE_FDDI;
> -	rc = lcs_send_startlan(card, LCS_INITIATOR_TCPIP);
> -	if (rc == 0)
> -		return 0;
> -#endif
>  	return -EIO;
>  }
>  
> @@ -1806,22 +1793,16 @@ lcs_get_frames_cb(struct lcs_channel *channel, struct lcs_buffer *buffer)
>  			card->stats.rx_errors++;
>  			return;
>  		}
> -		/* What kind of frame is it? */
> -		if (lcs_hdr->type == LCS_FRAME_TYPE_CONTROL) {
> -			/* Control frame. */
> +		if (lcs_hdr->type == LCS_FRAME_TYPE_CONTROL)
>  			lcs_get_control(card, (struct lcs_cmd *) lcs_hdr);
> -		} else if (lcs_hdr->type == LCS_FRAME_TYPE_ENET ||
> -			   lcs_hdr->type == LCS_FRAME_TYPE_TR ||
> -			   lcs_hdr->type == LCS_FRAME_TYPE_FDDI) {
> -			/* Normal network packet. */
> +		else if (lcs_hdr->type == LCS_FRAME_TYPE_ENET)
>  			lcs_get_skb(card, (char *)(lcs_hdr + 1),
>  				    lcs_hdr->offset - offset -
>  				    sizeof(struct lcs_header));
> -		} else {
> -			/* Unknown frame type. */
> -			; // FIXME: error message ?
> -		}
> -		/* Proceed to next frame. */
> +		else
> +			dev_info_once(&card->dev->dev,
> +				      "Unknown frame type %d\n",
> +				      lcs_hdr->type);
>  		offset = lcs_hdr->offset;
>  		lcs_hdr->offset = LCS_ILLEGAL_OFFSET;
>  		lcs_hdr = (struct lcs_header *) (buffer->data + offset);
> @@ -2140,18 +2121,10 @@ lcs_new_device(struct ccwgroup_device *ccwgdev)
>  		goto netdev_out;
>  	}
>  	switch (card->lan_type) {
> -#ifdef CONFIG_ETHERNET
>  	case LCS_FRAME_TYPE_ENET:
>  		card->lan_type_trans = eth_type_trans;
>  		dev = alloc_etherdev(0);
>  		break;
> -#endif
> -#ifdef CONFIG_FDDI
> -	case LCS_FRAME_TYPE_FDDI:
> -		card->lan_type_trans = fddi_type_trans;
> -		dev = alloc_fddidev(0);
> -		break;
> -#endif
>  	default:
>  		LCS_DBF_TEXT(3, setup, "errinit");
>  		pr_err(" Initialization failed\n");

-- 
~Randy