Ping — Stéphane > -----Original Message----- > From: Stéphane Grosjean > Sent: mardi 15 octobre 2019 14:21 > To: Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx> > Cc: linux-can Mailing List <linux-can@xxxxxxxxxxxxxxx> > Subject: RE: [PATCH 2/2] can/peak_usb/pcan_usb: add support of rxerr/txerr > counters > > Hello Marc, > > See my comments below. > > Thank you, > > — Stéphane > > > > /* > > > * start interface > > > */ > > > static int pcan_usb_start(struct peak_usb_device *dev) { > > > struct pcan_usb *pdev = container_of(dev, struct pcan_usb, dev); > > > +int err; > > > > > > /* number of bits used in timestamps read from adapter struct */ > > > peak_usb_init_time_ref(&pdev->time_ref, &pcan_usb); > > > > > > +pdev->bec.rxerr = 0; > > > +pdev->bec.txerr = 0; > > > + > > > +/* be notified on any error counter change */ > > > > Does the device offer the possibility to read the error counters? If > > so please do it in pcan_usb_get_berr_counter(). > > > > Nope! The value of the error counters can only be obtained through the > below "bus error" notification mechanism: > > > > +err = pcan_usb_set_err_frame(dev, PCAN_USB_ERR_ECC | > > > + PCAN_USB_ERR_RXERR | > > PCAN_USB_ERR_TXERR | > > > + PCAN_USB_ERR_RXERR_CNT | > > > + PCAN_USB_ERR_TXERR_CNT); > > > > Better only enable bus errors, if the user has requested them. See: > > > > > > https://elixir.bootlin.com/linux/latest/source/drivers/net/can/sja1000 > > /sja10 > > 00.c#L152 > > > > Ok. But this also means that the rx/tx err counters will always be 0 if user > didn't request "berr-reporting on". Is this ok? > > > > +if (err) > > > +netdev_warn(dev->netdev, > > > + "CAN bus error counters not available (err %u)\n", > > > + err); > > > + > > > /* if revision greater than 3, can put silent mode on/off */ > > > if (dev->device_rev > 3) { > > > -int err; > > > - > > > err = pcan_usb_set_silent(dev, > > > dev->can.ctrlmode & > > CAN_CTRLMODE_LISTENONLY); > > > if (err) > > > @@ -906,4 +1050,5 @@ const struct peak_usb_adapter pcan_usb = { > > > .dev_encode_msg = pcan_usb_encode_msg, > > > .dev_start = pcan_usb_start, > > > .dev_restart_async = pcan_usb_restart_async, > > > +.do_get_berr_counter = pcan_usb_get_berr_counter, > > > }; > > > > > > > Marc > > > > -- > > 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 | -- 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