[PATCH 0/2 v2] can: peak_usb: PCAN-USB bus state fix and info ordering

[PATCH 0/2 v2] can: peak_usb: PCAN-USB bus state fix and info ordering

[Stephane Grosjean]

This bundle of changes:
- corrects a potential bad transition to ERROR_PASSIVE or ERROR_WARNING
  state if BERR reporting was not explicitly requested for the can
  interface associated with the PCAN-USB,
- reorders the information displayed when the PCAN-USB is detected, when it
  can benefit from a firmware update.

Signed-off-by: Stephane Grosjean <s.grosjean@peak-system.com>

Stephane Grosjean (2):
  can: peak_usb: always ask for BERR reporting for PCAN-USB devices
  can: peak_usb: exchange the order of information messages

 drivers/net/can/usb/peak_usb/pcan_usb.c | 27 ++++++++++++-------------
 1 file changed, 13 insertions(+), 14 deletions(-)

-- 
2.25.1

[PATCH 1/2 v2] can: peak_usb: always ask for BERR reporting for PCAN-USB devices

[Stephane Grosjean]

Since for the PCAN-USB, the management of the transition to the
ERROR_WARNING or ERROR_PASSIVE state is done according to the error
counters, these must be requested unconditionally.

Signed-off-by: Stephane Grosjean <s.grosjean@peak-system.com>
---
v2:
- remove CAN_CTRLMODE_BERR_REPORTING from ctrlmode_supported

 drivers/net/can/usb/peak_usb/pcan_usb.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/net/can/usb/peak_usb/pcan_usb.c b/drivers/net/can/usb/peak_usb/pcan_usb.c
index 837b3fecd71e..af8d3dadbbb8 100644
--- a/drivers/net/can/usb/peak_usb/pcan_usb.c
+++ b/drivers/net/can/usb/peak_usb/pcan_usb.c
@@ -841,14 +841,14 @@ static int pcan_usb_start(struct peak_usb_device *dev)
 	pdev->bec.rxerr = 0;
 	pdev->bec.txerr = 0;
 
-	/* be notified on error counter changes (if requested by user) */
-	if (dev->can.ctrlmode & CAN_CTRLMODE_BERR_REPORTING) {
-		err = pcan_usb_set_err_frame(dev, PCAN_USB_BERR_MASK);
-		if (err)
-			netdev_warn(dev->netdev,
-				    "Asking for BERR reporting error %u\n",
-				    err);
-	}
+	/* always ask the device for BERR reporting, to be able to switch from
+	 * WARNING to PASSIVE state
+	 */
+	err = pcan_usb_set_err_frame(dev, PCAN_USB_BERR_MASK);
+	if (err)
+		netdev_warn(dev->netdev,
+			    "Asking for BERR reporting error %u\n",
+			    err);
 
 	/* if revision greater than 3, can put silent mode on/off */
 	if (dev->device_rev > 3) {
@@ -986,7 +986,6 @@ const struct peak_usb_adapter pcan_usb = {
 	.device_id = PCAN_USB_PRODUCT_ID,
 	.ctrl_count = 1,
 	.ctrlmode_supported = CAN_CTRLMODE_3_SAMPLES | CAN_CTRLMODE_LISTENONLY |
-			      CAN_CTRLMODE_BERR_REPORTING |
 			      CAN_CTRLMODE_CC_LEN8_DLC,
 	.clock = {
 		.freq = PCAN_USB_CRYSTAL_HZ / 2,
-- 
2.25.1

[PATCH 2/2 v2] can: peak_usb: exchange the order of information messages

[Stephane Grosjean]

Proposes the possible update of the PCAN-USB firmware after indicating its
name and current version.

Signed-off-by: Stephane Grosjean <s.grosjean@peak-system.com>
---
 drivers/net/can/usb/peak_usb/pcan_usb.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/can/usb/peak_usb/pcan_usb.c b/drivers/net/can/usb/peak_usb/pcan_usb.c
index af8d3dadbbb8..876218752766 100644
--- a/drivers/net/can/usb/peak_usb/pcan_usb.c
+++ b/drivers/net/can/usb/peak_usb/pcan_usb.c
@@ -883,6 +883,11 @@ static int pcan_usb_init(struct peak_usb_device *dev)
 		return err;
 	}
 
+	dev_info(dev->netdev->dev.parent,
+		 "PEAK-System %s adapter hwrev %u serial %08X (%u channel)\n",
+		 pcan_usb.name, dev->device_rev, serial_number,
+		 pcan_usb.ctrl_count);
+
 	/* Since rev 4.1, PCAN-USB is able to make single-shot as well as
 	 * looped back frames.
 	 */
@@ -896,11 +901,6 @@ static int pcan_usb_init(struct peak_usb_device *dev)
 			 "Firmware update available. Please contact support@peak-system.com\n");
 	}
 
-	dev_info(dev->netdev->dev.parent,
-		 "PEAK-System %s adapter hwrev %u serial %08X (%u channel)\n",
-		 pcan_usb.name, dev->device_rev, serial_number,
-		 pcan_usb.ctrl_count);
-
 	return 0;
 }
 
-- 
2.25.1

Re: [PATCH 1/2 v2] can: peak_usb: always ask for BERR reporting for PCAN-USB devices

[Marc Kleine-Budde]

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

On 21.10.2021 10:15:04, Stephane Grosjean wrote:
> Since for the PCAN-USB, the management of the transition to the
> ERROR_WARNING or ERROR_PASSIVE state is done according to the error
> counters, these must be requested unconditionally.
> 
> Signed-off-by: Stephane Grosjean <s.grosjean@peak-system.com>

This description indicates the patch should be backported, as it fixes a
problem. Can you figure out which patch added the problem in the first
place, so that we can add a Fixed tag?

regards,
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 1/2 v2] can: peak_usb: always ask for BERR reporting for PCAN-USB devices

[Stéphane Grosjean]

​Hi,

- the rxerr/txerr counters were used for the first time to determine the new state from: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c11dcee758302702a83c6e85e4c4c3d9af42d2b3

- but their values could be read by the user space according to ctrl_mode, starting from: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=ea8b33bde76c8fcef347b9b9cf15649fe41a5a6e

Which one do you think is the most appropriate?

Regards,

Stephane



            De: Marc Kleine-Budde
Envoyé: Vendredi 29 octobre 2021 14:53
À: Stéphane Grosjean
Cc: linux-can Mailing List
Objet: Re: [PATCH 1/2 v2] can: peak_usb: always ask for BERR reporting for PCAN-USB devices




On 21.10.2021 10:15:04, Stephane Grosjean wrote:

> Since for the PCAN-USB, the management of the transition to the

> ERROR_WARNING or ERROR_PASSIVE state is done according to the error

> counters, these must be requested unconditionally.

>

> Signed-off-by: Stephane Grosjean <s.grosjean@peak-system.com>



This description indicates the patch should be backported, as it fixes a

problem. Can you figure out which patch added the problem in the first

place, so that we can add a Fixed tag?



regards,

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 |


--
PEAK-System Technik GmbH
Sitz der Gesellschaft Darmstadt - HRB 9183
Geschaeftsfuehrung: Alexander Gach / Uwe Wilhelm
Unsere Datenschutzerklaerung mit wichtigen Hinweisen
zur Behandlung personenbezogener Daten finden Sie unter
www.peak-system.com/Datenschutz.483.0.html

Re: [PATCH 1/2 v2] can: peak_usb: always ask for BERR reporting for PCAN-USB devices

[Marc Kleine-Budde]

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

On 04.11.2021 17:06:59, Stéphane Grosjean wrote:
> - the rxerr/txerr counters were used for the first time to determine
> the new state from:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c11dcee758302702a83c6e85e4c4c3d9af42d2b3
> 
> - but their values could be read by the user space according to
> ctrl_mode, starting from:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=ea8b33bde76c8fcef347b9b9cf15649fe41a5a6e
> 
> Which one do you think is the most appropriate?

I'm taking the first one:
Fixes: c11dcee75830 ("can: peak_usb: pcan_usb_decode_error(): upgrade handling of bus state changes")

regards,
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 0/2 v2] can: peak_usb: PCAN-USB bus state fix and info ordering

[Marc Kleine-Budde]

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

On 21.10.2021 10:15:03, Stephane Grosjean wrote:
> This bundle of changes:
> - corrects a potential bad transition to ERROR_PASSIVE or ERROR_WARNING
>   state if BERR reporting was not explicitly requested for the can
>   interface associated with the PCAN-USB,
> - reorders the information displayed when the PCAN-USB is detected, when it
>   can benefit from a firmware update.
> 
> Signed-off-by: Stephane Grosjean <s.grosjean@peak-system.com>

Applied both to linux-can/testing. Added stable on Cc for the 1st patch.

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 --]