can: Unify MTU settings for CAN interfaces

PATCH In review Oliver Hartkopp 2 participants 6 messages Mar 7, 2014

Subsystems (2)

Arm/Microchip (AT91) SoC support CAN network drivers

lore

Share & tools

lore

Oliver Hartkopp

author

2014-03-07 08:23 UTC #0 lore raw

PATCH 1/1 can: Unify MTU settings for CAN interfaces

CAN interfaces only support MTU values of 16 (CAN 2.0) and 72 (CAN FD). Setting the MTU to other values is pointless but it does not really hurt. With the introduction of the CAN FD support in drivers/net/can a new function to switch the MTU for CAN FD has been introduced.

This patch makes use of this can_change_mtu() function to check for correct MTU settings also in legacy CAN (2.0) devices.

Signed-off-by Oliver Hartkopp <[email protected]>

---

In sja1000.c and mscan.c the spaces at the beginning of the line have been replaced by tabs as checkpatch.pl complained about it.

drivers/net/can/at91_can.c +1 -0

index 1d00b95..f07fa89 100644
@@ -1194,6 +1194,7 @@ static const struct net_device_ops at91_netdev_ops = {
 	.ndo_open	= at91_open,
 	.ndo_stop	= at91_close,
 	.ndo_start_xmit	= at91_start_xmit,
+	.ndo_change_mtu = can_change_mtu,
 };
 
 static ssize_t at91_sysfs_show_mb0_id(struct device *dev,

drivers/net/can/bfin_can.c +1 -0

index 8d2b89a..543ecce 100644
@@ -528,6 +528,7 @@ static const struct net_device_ops bfin_can_netdev_ops = {
 	.ndo_open               = bfin_can_open,
 	.ndo_stop               = bfin_can_close,
 	.ndo_start_xmit         = bfin_can_start_xmit,
+	.ndo_change_mtu         = can_change_mtu,
 };
 
 static int bfin_can_probe(struct platform_device *pdev)

drivers/net/can/c_can/c_can.c +1 -0

index 951bfed..9c32e9e 100644
@@ -1277,6 +1277,7 @@ static const struct net_device_ops c_can_netdev_ops = {
 	.ndo_open = c_can_open,
 	.ndo_stop = c_can_close,
 	.ndo_start_xmit = c_can_start_xmit,
+	.ndo_change_mtu = can_change_mtu,
 };
 
 int register_c_can_dev(struct net_device *dev)

drivers/net/can/cc770/cc770.c +1 -0

index 0f12abf..d837927 100644
@@ -823,6 +823,7 @@ static const struct net_device_ops cc770_netdev_ops = {
 	.ndo_open = cc770_open,
 	.ndo_stop = cc770_close,
 	.ndo_start_xmit = cc770_start_xmit,
+	.ndo_change_mtu = can_change_mtu,
 };
 
 int register_cc770dev(struct net_device *dev)

drivers/net/can/flexcan.c +1 -0

index 61376ab..5380e58 100644
@@ -1011,6 +1011,7 @@ static const struct net_device_ops flexcan_netdev_ops = {
 	.ndo_open	= flexcan_open,
 	.ndo_stop	= flexcan_close,
 	.ndo_start_xmit	= flexcan_start_xmit,
+	.ndo_change_mtu = can_change_mtu,
 };
 
 static int register_flexcandev(struct net_device *dev)

drivers/net/can/grcan.c +1 -0

index ab506d6..3fd9fd9 100644
@@ -1578,6 +1578,7 @@ static const struct net_device_ops grcan_netdev_ops = {
 	.ndo_open	= grcan_open,
 	.ndo_stop	= grcan_close,
 	.ndo_start_xmit	= grcan_start_xmit,
+	.ndo_change_mtu = can_change_mtu,
 };
 
 static int grcan_setup_netdev(struct platform_device *ofdev,

drivers/net/can/janz-ican3.c +1 -0

index 71594e5..31092c4 100644
@@ -1597,6 +1597,7 @@ static const struct net_device_ops ican3_netdev_ops = {
 	.ndo_open	= ican3_open,
 	.ndo_stop	= ican3_stop,
 	.ndo_start_xmit	= ican3_xmit,
+	.ndo_change_mtu = can_change_mtu,
 };
 
 /*

drivers/net/can/mcp251x.c +1 -0

index cdb9808..92dc132 100644
@@ -996,6 +996,7 @@ static const struct net_device_ops mcp251x_netdev_ops = {
 	.ndo_open = mcp251x_open,
 	.ndo_stop = mcp251x_stop,
 	.ndo_start_xmit = mcp251x_hard_start_xmit,
+	.ndo_change_mtu = can_change_mtu,
 };
 
 static const struct of_device_id mcp251x_of_match[] = {

drivers/net/can/mscan/mscan.c +4 -3

index b9f3faa..e0c9be5 100644
@@ -647,9 +647,10 @@ static int mscan_close(struct net_device *dev)
 }
 
 static const struct net_device_ops mscan_netdev_ops = {
-       .ndo_open               = mscan_open,
-       .ndo_stop               = mscan_close,
-       .ndo_start_xmit         = mscan_start_xmit,
+	.ndo_open	= mscan_open,
+	.ndo_stop	= mscan_close,
+	.ndo_start_xmit	= mscan_start_xmit,
+	.ndo_change_mtu	= can_change_mtu,
 };
 
 int register_mscandev(struct net_device *dev, int mscan_clksrc)

drivers/net/can/pch_can.c +1 -0

index 6c077eb..6472562 100644
@@ -950,6 +950,7 @@ static const struct net_device_ops pch_can_netdev_ops = {
 	.ndo_open		= pch_can_open,
 	.ndo_stop		= pch_close,
 	.ndo_start_xmit		= pch_xmit,
+	.ndo_change_mtu		= can_change_mtu,
 };
 
 static void pch_can_remove(struct pci_dev *pdev)

drivers/net/can/sja1000/sja1000.c +4 -3

index 55cce47..f31499a 100644
@@ -642,9 +642,10 @@ void free_sja1000dev(struct net_device *dev)
 EXPORT_SYMBOL_GPL(free_sja1000dev);
 
 static const struct net_device_ops sja1000_netdev_ops = {
-       .ndo_open               = sja1000_open,
-       .ndo_stop               = sja1000_close,
-       .ndo_start_xmit         = sja1000_start_xmit,
+	.ndo_open	= sja1000_open,
+	.ndo_stop	= sja1000_close,
+	.ndo_start_xmit	= sja1000_start_xmit,
+	.ndo_change_mtu	= can_change_mtu,
 };
 
 int register_sja1000dev(struct net_device *dev)

drivers/net/can/slcan.c +6 -0

index 3fcdae2..f5b16e0 100644
@@ -411,10 +411,16 @@ static void slc_free_netdev(struct net_device *dev)
 	slcan_devs[i] = NULL;
 }
 
+static int slcan_change_mtu(struct net_device *dev, int new_mtu)
+{
+	return -EINVAL;
+}
+
 static const struct net_device_ops slc_netdev_ops = {
 	.ndo_open               = slc_open,
 	.ndo_stop               = slc_close,
 	.ndo_start_xmit         = slc_xmit,
+	.ndo_change_mtu         = slcan_change_mtu,
 };
 
 static void slc_setup(struct net_device *dev)

drivers/net/can/softing/softing_main.c +1 -0

index 9ea0dcd..3766bd9 100644
@@ -628,6 +628,7 @@ static const struct net_device_ops softing_netdev_ops = {
 	.ndo_open = softing_netdev_open,
 	.ndo_stop = softing_netdev_stop,
 	.ndo_start_xmit	= softing_netdev_start_xmit,
+	.ndo_change_mtu = can_change_mtu,
 };
 
 static const struct can_bittiming_const softing_btr_const = {

drivers/net/can/ti_hecc.c +1 -0

index 2c62fe6..258b9c4 100644
@@ -871,6 +871,7 @@ static const struct net_device_ops ti_hecc_netdev_ops = {
 	.ndo_open		= ti_hecc_open,
 	.ndo_stop		= ti_hecc_close,
 	.ndo_start_xmit		= ti_hecc_xmit,
+	.ndo_change_mtu		= can_change_mtu,
 };
 
 static int ti_hecc_probe(struct platform_device *pdev)

drivers/net/can/usb/ems_usb.c +1 -0

index 52c42fd..00f2534 100644
@@ -883,6 +883,7 @@ static const struct net_device_ops ems_usb_netdev_ops = {
 	.ndo_open = ems_usb_open,
 	.ndo_stop = ems_usb_close,
 	.ndo_start_xmit = ems_usb_start_xmit,
+	.ndo_change_mtu = can_change_mtu,
 };
 
 static const struct can_bittiming_const ems_usb_bittiming_const = {

drivers/net/can/usb/esd_usb2.c +1 -0

index 7fbe859..1f8ce91 100644
@@ -888,6 +888,7 @@ static const struct net_device_ops esd_usb2_netdev_ops = {
 	.ndo_open = esd_usb2_open,
 	.ndo_stop = esd_usb2_close,
 	.ndo_start_xmit = esd_usb2_start_xmit,
+	.ndo_change_mtu = can_change_mtu,
 };
 
 static const struct can_bittiming_const esd_usb2_bittiming_const = {

drivers/net/can/usb/kvaser_usb.c +1 -0

index e77d110..ea596b5 100644
@@ -1388,6 +1388,7 @@ static const struct net_device_ops kvaser_usb_netdev_ops = {
 	.ndo_open = kvaser_usb_open,
 	.ndo_stop = kvaser_usb_close,
 	.ndo_start_xmit = kvaser_usb_start_xmit,
+	.ndo_change_mtu = can_change_mtu,
 };
 
 static const struct can_bittiming_const kvaser_usb_bittiming_const = {

drivers/net/can/usb/peak_usb/pcan_usb_core.c +1 -0

index 0b7a4c3..93e4a55 100644
@@ -702,6 +702,7 @@ static const struct net_device_ops peak_usb_netdev_ops = {
 	.ndo_open = peak_usb_ndo_open,
 	.ndo_stop = peak_usb_ndo_stop,
 	.ndo_start_xmit = peak_usb_ndo_start_xmit,
+	.ndo_change_mtu = can_change_mtu,
 };
 
 /*

drivers/net/can/usb/usb_8dev.c +1 -0

index a0fa1fd..cde2634 100644
@@ -887,6 +887,7 @@ static const struct net_device_ops usb_8dev_netdev_ops = {
 	.ndo_open = usb_8dev_open,
 	.ndo_stop = usb_8dev_close,
 	.ndo_start_xmit = usb_8dev_start_xmit,
+	.ndo_change_mtu = can_change_mtu,
 };
 
 static const struct can_bittiming_const usb_8dev_bittiming_const = {

Marc Kleine-Budde

maintainer

2014-03-07 11:40 UTC #1 lore raw

Quoted 15 lines · Oliver Hartkopp

CAN interfaces only support MTU values of 16 (CAN 2.0) and 72 (CAN FD). Setting the MTU to other values is pointless but it does not really hurt. With the introduction of the CAN FD support in drivers/net/can a new function to switch the MTU for CAN FD has been introduced.
This patch makes use of this can_change_mtu() function to check for correct MTU settings also in legacy CAN (2.0) devices.
Signed-off-by Oliver Hartkopp <[email protected]>
drivers/net/can/slcan.c +0 -0
index 3fcdae2..f5b16e0 100644

drivers/net/can/slcan.c static void slc_free_netdev(struct net_device *dev) · line 418

Quoted 8 lines

drivers/net/can/slcan.c +5 -0

@@ -411,10 +411,16 @@ static void slc_free_netdev(struct net_device *dev)
 	slcan_devs[i] = NULL;
 }
 
+static int slcan_change_mtu(struct net_device *dev, int new_mtu)
+{
+	return -EINVAL;
+}
+

Why is the slcan special?

drivers/net/can/slcan.c static void slc_free_netdev(struct net_device *dev) · line 426

Quoted 8 lines

drivers/net/can/slcan.c +1 -0

 static const struct net_device_ops slc_netdev_ops = {
 	.ndo_open               = slc_open,
 	.ndo_stop               = slc_close,
 	.ndo_start_xmit         = slc_xmit,
+	.ndo_change_mtu         = slcan_change_mtu,
 };
 
 static void slc_setup(struct net_device *dev)

Marc

Signature · 4 lines

Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |

Oliver Hartkopp

author

2014-03-07 12:53 UTC #2 lore raw

Quoted 19 lines · Marc Kleine-Budde

Quoted 18 lines · Oliver Hartkopp
CAN interfaces only support MTU values of 16 (CAN 2.0) and 72 (CAN FD). Setting the MTU to other values is pointless but it does not really hurt. With the introduction of the CAN FD support in drivers/net/can a new function to switch the MTU for CAN FD has been introduced.
This patch makes use of this can_change_mtu() function to check for correct MTU settings also in legacy CAN (2.0) devices.
Signed-off-by Oliver Hartkopp <[email protected]>
drivers/net/can/slcan.c +0 -0
index 3fcdae2..f5b16e0 100644
@@ -411,10 +411,16 @@ static void slc_free_netdev(struct net_device *dev)
 	slcan_devs[i] = NULL;
 }
 

Quoted 7 lines

drivers/net/can/slcan.c static void slc_free_netdev(struct net_device *dev) · line 418
Quoted 5 lines
drivers/net/can/slcan.c +5 -0
+static int slcan_change_mtu(struct net_device *dev, int new_mtu)
+{
+	return -EINVAL;
+}
+
Why is the slcan special?

slcan as well as vcan are no users of the CAN driver infrastructure. No include of dev.h, no bitrate settings, etc.
The only drivers that do not depend from 'config CAN_DEV' in Kconfig.

The -EINVAL error is only thrown when someone tries to move the slcan MTU away from the only valid CAN_MTU, see:

	http://lxr.free-electrons.com/source/net/core/dev.c#L5324

Regards,
Oliver

Marc Kleine-Budde

maintainer

2014-03-07 12:56 UTC #3 lore raw

Quoted 30 lines · Oliver Hartkopp

Quoted 19 lines · Marc Kleine-Budde
Quoted 18 lines · Oliver Hartkopp
CAN interfaces only support MTU values of 16 (CAN 2.0) and 72 (CAN FD). Setting the MTU to other values is pointless but it does not really hurt. With the introduction of the CAN FD support in drivers/net/can a new function to switch the MTU for CAN FD has been introduced.
This patch makes use of this can_change_mtu() function to check for correct MTU settings also in legacy CAN (2.0) devices.
Signed-off-by Oliver Hartkopp <[email protected]>
drivers/net/can/slcan.c +0 -0
index 3fcdae2..f5b16e0 100644
@@ -411,10 +411,16 @@ static void slc_free_netdev(struct net_device *dev)
 	slcan_devs[i] = NULL;
 }
 
Quoted 7 lines
drivers/net/can/slcan.c static void slc_free_netdev(struct net_device *dev) · line 418
Quoted 5 lines
drivers/net/can/slcan.c +5 -0
+static int slcan_change_mtu(struct net_device *dev, int new_mtu)
+{
+	return -EINVAL;
+}
+
Why is the slcan special?

Quoted 8 lines

slcan as well as vcan are no users of the CAN driver infrastructure. No include of dev.h, no bitrate settings, etc.
The only drivers that do not depend from 'config CAN_DEV' in Kconfig.
The -EINVAL error is only thrown when someone tries to move the slcan MTU away from the only valid CAN_MTU, see:
	http://lxr.free-electrons.com/source/net/core/dev.c#L5324

Thanks for the clarification.

Added yo can-next.

Marc

Signature · 4 lines

Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |

Oliver Hartkopp

author

2014-03-07 12:58 UTC #4 lore raw

Quoted 2 lines · Marc Kleine-Budde

Added yo can-next.

Ah, you run a US keyboard :-)

Marc Kleine-Budde

maintainer

2014-03-07 13:04 UTC #5 lore raw

Quoted 6 lines · Oliver Hartkopp

Quoted 2 lines · Marc Kleine-Budde
Added yo can-next.
        ^
Ah, you run a US keyboard :-)

ACK :)

Signature · 4 lines

Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |