[PATCH v2] can: mcba_usb: Fix termination command argument

[PATCH v2] can: mcba_usb: Fix termination command argument

[Yasushi SHOJI]

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>
---
 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.38.1

Re: [PATCH v2] can: mcba_usb: Fix termination command argument

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 1682 bytes --]

On 25.11.2022 00:25:03, Yasushi SHOJI wrote:
> 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>

Applied to linux-can.

Thanks,
Marc

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2] can: mcba_usb: Fix termination command argument

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (master)
by Marc Kleine-Budde <mkl@pengutronix.de>:

On Fri, 25 Nov 2022 00:25:03 +0900 you wrote:
> 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.
> 
> [...]

Here is the summary with links:
  - [v2] can: mcba_usb: Fix termination command argument
    https://git.kernel.org/netdev/net/c/1a8e3bd25f1e

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html