[PATCH AUTOSEL 6.0 07/13] can: mcba_usb: Fix termination command argument

[PATCH AUTOSEL 6.0 07/13] can: mcba_usb: Fix termination command argument

[Sasha Levin]

From: Yasushi SHOJI <yasushi.shoji@gmail.com>

[ Upstream commit 1a8e3bd25f1e789c8154e11ea24dc3ec5a4c1da0 ]

Microchip USB Analyzer can activate the internal termination resistors
by setting the "termination" option ON, or OFF to to deactivate them.
As I've observed, both with my oscilloscope and captured USB packets
below, you must send "0" to turn it ON, and "1" to turn it OFF.

From the schematics in the user's guide, I can confirm that you must
drive the CAN_RES signal LOW "0" to activate the resistors.

Reverse the argument value of usb_msg.termination to fix this.

These are the two commands sequence, ON then OFF.

> No.     Time           Source                Destination           Protocol Length Info
>       1 0.000000       host                  1.3.1                 USB      46     URB_BULK out
>
> Frame 1: 46 bytes on wire (368 bits), 46 bytes captured (368 bits)
> USB URB
> Leftover Capture Data: a80000000000000000000000000000000000a8
>
> No.     Time           Source                Destination           Protocol Length Info
>       2 4.372547       host                  1.3.1                 USB      46     URB_BULK out
>
> Frame 2: 46 bytes on wire (368 bits), 46 bytes captured (368 bits)
> USB URB
> Leftover Capture Data: a80100000000000000000000000000000000a9

Signed-off-by: Yasushi SHOJI <yashi@spacecubics.com>
Link: https://lore.kernel.org/all/20221124152504.125994-1-yashi@spacecubics.com
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/net/can/usb/mcba_usb.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/net/can/usb/mcba_usb.c b/drivers/net/can/usb/mcba_usb.c
index 218b098b261d..47619e9cb005 100644
--- a/drivers/net/can/usb/mcba_usb.c
+++ b/drivers/net/can/usb/mcba_usb.c
@@ -47,6 +47,10 @@
 #define MCBA_VER_REQ_USB 1
 #define MCBA_VER_REQ_CAN 2
 
+/* Drive the CAN_RES signal LOW "0" to activate R24 and R25 */
+#define MCBA_VER_TERMINATION_ON 0
+#define MCBA_VER_TERMINATION_OFF 1
+
 #define MCBA_SIDL_EXID_MASK 0x8
 #define MCBA_DLC_MASK 0xf
 #define MCBA_DLC_RTR_MASK 0x40
@@ -463,7 +467,7 @@ static void mcba_usb_process_ka_usb(struct mcba_priv *priv,
 		priv->usb_ka_first_pass = false;
 	}
 
-	if (msg->termination_state)
+	if (msg->termination_state == MCBA_VER_TERMINATION_ON)
 		priv->can.termination = MCBA_TERMINATION_ENABLED;
 	else
 		priv->can.termination = MCBA_TERMINATION_DISABLED;
@@ -785,9 +789,9 @@ static int mcba_set_termination(struct net_device *netdev, u16 term)
 	};
 
 	if (term == MCBA_TERMINATION_ENABLED)
-		usb_msg.termination = 1;
+		usb_msg.termination = MCBA_VER_TERMINATION_ON;
 	else
-		usb_msg.termination = 0;
+		usb_msg.termination = MCBA_VER_TERMINATION_OFF;
 
 	mcba_usb_xmit_cmd(priv, (struct mcba_usb_msg *)&usb_msg);
 
-- 
2.35.1

[PATCH AUTOSEL 6.0 08/13] net: fec: don't reset irq coalesce settings to defaults on "ip link up"

[Sasha Levin]

From: Rasmus Villemoes <linux@rasmusvillemoes.dk>

[ Upstream commit df727d4547de568302b0ed15b0d4e8a469bdb456 ]

Currently, when a FEC device is brought up, the irq coalesce settings
are reset to their default values (1000us, 200 frames). That's
unexpected, and breaks for example use of an appropriate .link file to
make systemd-udev apply the desired
settings (https://www.freedesktop.org/software/systemd/man/systemd.link.html),
or any other method that would do a one-time setup during early boot.

Refactor the code so that fec_restart() instead uses
fec_enet_itr_coal_set(), which simply applies the settings that are
stored in the private data, and initialize that private data with the
default values.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/net/ethernet/freescale/fec_main.c | 22 ++++++----------------
 1 file changed, 6 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 5aa254eaa8d0..b71e0c32e351 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -72,7 +72,7 @@
 #include "fec.h"
 
 static void set_multicast_list(struct net_device *ndev);
-static void fec_enet_itr_coal_init(struct net_device *ndev);
+static void fec_enet_itr_coal_set(struct net_device *ndev);
 
 #define DRIVER_NAME	"fec"
 
@@ -1164,8 +1164,7 @@ fec_restart(struct net_device *ndev)
 		writel(0, fep->hwp + FEC_IMASK);
 
 	/* Init the interrupt coalescing */
-	fec_enet_itr_coal_init(ndev);
-
+	fec_enet_itr_coal_set(ndev);
 }
 
 static void fec_enet_stop_mode(struct fec_enet_private *fep, bool enabled)
@@ -2771,19 +2770,6 @@ static int fec_enet_set_coalesce(struct net_device *ndev,
 	return 0;
 }
 
-static void fec_enet_itr_coal_init(struct net_device *ndev)
-{
-	struct ethtool_coalesce ec;
-
-	ec.rx_coalesce_usecs = FEC_ITR_ICTT_DEFAULT;
-	ec.rx_max_coalesced_frames = FEC_ITR_ICFT_DEFAULT;
-
-	ec.tx_coalesce_usecs = FEC_ITR_ICTT_DEFAULT;
-	ec.tx_max_coalesced_frames = FEC_ITR_ICFT_DEFAULT;
-
-	fec_enet_set_coalesce(ndev, &ec, NULL, NULL);
-}
-
 static int fec_enet_get_tunable(struct net_device *netdev,
 				const struct ethtool_tunable *tuna,
 				void *data)
@@ -3538,6 +3524,10 @@ static int fec_enet_init(struct net_device *ndev)
 	fep->rx_align = 0x3;
 	fep->tx_align = 0x3;
 #endif
+	fep->rx_pkts_itr = FEC_ITR_ICFT_DEFAULT;
+	fep->tx_pkts_itr = FEC_ITR_ICFT_DEFAULT;
+	fep->rx_time_itr = FEC_ITR_ICTT_DEFAULT;
+	fep->tx_time_itr = FEC_ITR_ICTT_DEFAULT;
 
 	/* Check mask of the streaming and coherent API */
 	ret = dma_set_mask_and_coherent(&fep->pdev->dev, DMA_BIT_MASK(32));
-- 
2.35.1

Re: [PATCH AUTOSEL 6.0 08/13] net: fec: don't reset irq coalesce settings to defaults on "ip link up"

[Rasmus Villemoes]

On 06/12/2022 10.49, Sasha Levin wrote:
> From: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> 
> [ Upstream commit df727d4547de568302b0ed15b0d4e8a469bdb456 ]

Well, I'm not sure this is actually -stable material, but even if it is,
it's probably better to wait until the regression which this introduced
has also been fixed in mainline:

https://lore.kernel.org/netdev/20221205204604.869853-1-linux@rasmusvillemoes.dk/

[PATCH AUTOSEL 6.0 09/13] net: loopback: use NET_NAME_PREDICTABLE for name_assign_type

[Sasha Levin]

From: Rasmus Villemoes <linux@rasmusvillemoes.dk>

[ Upstream commit 31d929de5a112ee1b977a89c57de74710894bbbf ]

When the name_assign_type attribute was introduced (commit
685343fc3ba6, "net: add name_assign_type netdev attribute"), the
loopback device was explicitly mentioned as one which would make use
of NET_NAME_PREDICTABLE:

    The name_assign_type attribute gives hints where the interface name of a
    given net-device comes from. These values are currently defined:
...
      NET_NAME_PREDICTABLE:
        The ifname has been assigned by the kernel in a predictable way
        that is guaranteed to avoid reuse and always be the same for a
        given device. Examples include statically created devices like
        the loopback device [...]

Switch to that so that reading /sys/class/net/lo/name_assign_type
produces something sensible instead of returning -EINVAL.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/net/loopback.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/loopback.c b/drivers/net/loopback.c
index 14e8d04cb434..2e9742952c4e 100644
--- a/drivers/net/loopback.c
+++ b/drivers/net/loopback.c
@@ -211,7 +211,7 @@ static __net_init int loopback_net_init(struct net *net)
 	int err;
 
 	err = -ENOMEM;
-	dev = alloc_netdev(0, "lo", NET_NAME_UNKNOWN, loopback_setup);
+	dev = alloc_netdev(0, "lo", NET_NAME_PREDICTABLE, loopback_setup);
 	if (!dev)
 		goto out;
 
-- 
2.35.1

Re: [PATCH AUTOSEL 6.0 09/13] net: loopback: use NET_NAME_PREDICTABLE for name_assign_type

[Jakub Kicinski]

On Tue,  6 Dec 2022 04:49:12 -0500 Sasha Levin wrote:
> From: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> 
> [ Upstream commit 31d929de5a112ee1b977a89c57de74710894bbbf ]
> 
> When the name_assign_type attribute was introduced (commit
> 685343fc3ba6, "net: add name_assign_type netdev attribute"), the
> loopback device was explicitly mentioned as one which would make use
> of NET_NAME_PREDICTABLE:
> 
>     The name_assign_type attribute gives hints where the interface name of a
>     given net-device comes from. These values are currently defined:
> ...
>       NET_NAME_PREDICTABLE:
>         The ifname has been assigned by the kernel in a predictable way
>         that is guaranteed to avoid reuse and always be the same for a
>         given device. Examples include statically created devices like
>         the loopback device [...]
> 
> Switch to that so that reading /sys/class/net/lo/name_assign_type
> produces something sensible instead of returning -EINVAL.

Yeah... we should have applied it to -next, I think backporting it is 
a good idea but I wish it had more time in the -next tree since it's 
a "uAPI alignment" :( 

Oh, well, very unlikely it will break anything, tho, so let's do it.

Re: [PATCH AUTOSEL 6.0 09/13] net: loopback: use NET_NAME_PREDICTABLE for name_assign_type

[Sasha Levin]

On Tue, Dec 06, 2022 at 11:49:56AM -0800, Jakub Kicinski wrote:
>On Tue,  6 Dec 2022 04:49:12 -0500 Sasha Levin wrote:
>> From: Rasmus Villemoes <linux@rasmusvillemoes.dk>
>>
>> [ Upstream commit 31d929de5a112ee1b977a89c57de74710894bbbf ]
>>
>> When the name_assign_type attribute was introduced (commit
>> 685343fc3ba6, "net: add name_assign_type netdev attribute"), the
>> loopback device was explicitly mentioned as one which would make use
>> of NET_NAME_PREDICTABLE:
>>
>>     The name_assign_type attribute gives hints where the interface name of a
>>     given net-device comes from. These values are currently defined:
>> ...
>>       NET_NAME_PREDICTABLE:
>>         The ifname has been assigned by the kernel in a predictable way
>>         that is guaranteed to avoid reuse and always be the same for a
>>         given device. Examples include statically created devices like
>>         the loopback device [...]
>>
>> Switch to that so that reading /sys/class/net/lo/name_assign_type
>> produces something sensible instead of returning -EINVAL.
>
>Yeah... we should have applied it to -next, I think backporting it is
>a good idea but I wish it had more time in the -next tree since it's
>a "uAPI alignment" :(
>
>Oh, well, very unlikely it will break anything, tho, so let's do it.

Want me to push it back a week to the next batch? It'll give it two
weeks instead of the usual week.

-- 
Thanks,
Sasha

Re: [PATCH AUTOSEL 6.0 09/13] net: loopback: use NET_NAME_PREDICTABLE for name_assign_type

[Jakub Kicinski]

On Tue, 6 Dec 2022 21:20:18 -0500 Sasha Levin wrote:
> >Yeah... we should have applied it to -next, I think backporting it is
> >a good idea but I wish it had more time in the -next tree since it's
> >a "uAPI alignment" :(
> >
> >Oh, well, very unlikely it will break anything, tho, so let's do it.  
> 
> Want me to push it back a week to the next batch? It'll give it two
> weeks instead of the usual week.

Oh, perfect, I didn't know we can hold for a week. Yes, please!

Re: [PATCH AUTOSEL 6.0 09/13] net: loopback: use NET_NAME_PREDICTABLE for name_assign_type

[Sasha Levin]

On Tue, Dec 06, 2022 at 06:49:27PM -0800, Jakub Kicinski wrote:
>On Tue, 6 Dec 2022 21:20:18 -0500 Sasha Levin wrote:
>> >Yeah... we should have applied it to -next, I think backporting it is
>> >a good idea but I wish it had more time in the -next tree since it's
>> >a "uAPI alignment" :(
>> >
>> >Oh, well, very unlikely it will break anything, tho, so let's do it.
>>
>> Want me to push it back a week to the next batch? It'll give it two
>> weeks instead of the usual week.
>
>Oh, perfect, I didn't know we can hold for a week. Yes, please!

Ack, you should get another AUTOSEL mail for this patch next week.

-- 
Thanks,
Sasha