Hi,
On 10/6/22 18:36, Marc Kleine-Budde wrote:
On 06.10.2022 18:24:50, Marc Kleine-Budde wrote:
From: Jeroen Hofstee <jhofstee@xxxxxxxxxxxxxxxxx>
The open source firmware candleLight report bus errors
unconditionally. This adds support to enable / disable bus error
reporting with the standard netlink property.
I haven't checked the candleLight firmware, yet.
If the unmodified firmware sends bus errors per default and we introduce
BERR_REPORTING as suggested in this patch, we have to modify the default
behavior for bus errors: By default the firmware will not send bus
errors, but only if GS_CAN_MODE_BERR_REPORTING is requested during
open().
I'm not sure if we want to change the default behavior of the
firmware....To work around this backwards compatibility issue we can
explicitly turn BERR reporting on or off during open via
GS_CAN_MODE_BERR_REPORTING_ON or GS_CAN_MODE_BERR_REPORTING_OFF.
It would look like this, untested:
subject: can: gs_usb: add ability to enable / disable berr rerporting
The open source firmware candleLight report bus errors
unconditionally. This adds support to enable / disable bus error
reporting with the standard netlink property.
Signed-off-by: Jeroen Hofstee <jeroen@xxxxxxxxxxxxx>
Signed-off-by: Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx>
---
drivers/net/can/usb/gs_usb.c | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c
index 7c1f13a90419..039e8d91ad88 100644
--- a/drivers/net/can/usb/gs_usb.c
+++ b/drivers/net/can/usb/gs_usb.c
@@ -135,6 +135,13 @@ struct gs_device_config {
/* GS_CAN_FEATURE_BT_CONST_EXT BIT(10) */
/* GS_CAN_FEATURE_TERMINATION BIT(11) */
+/* Previous versions of this driver didn't support the berr-reporting flag,
+ * this flag tells if berr-reporting is supported. When not set, the device
+ * can default to its own preference to be backwards compatible.
+ */
+#define GS_CAN_MODE_BERR_REPORTING_SET BIT(12)
+#define GS_CAN_MODE_BERR_REPORTING BIT(13)
+
struct gs_device_mode {
__le32 mode;
__le32 flags;
@@ -174,7 +181,9 @@ struct gs_device_termination_state {
#define GS_CAN_FEATURE_REQ_USB_QUIRK_LPC546XX BIT(9)
#define GS_CAN_FEATURE_BT_CONST_EXT BIT(10)
#define GS_CAN_FEATURE_TERMINATION BIT(11)
-#define GS_CAN_FEATURE_MASK GENMASK(11, 0)
+/* #define GS_CAN_FEATURE_BERR_REPORTING BIT_SET(12) */
+#define GS_CAN_FEATURE_BERR_REPORTING BIT(13)
+#define GS_CAN_FEATURE_MASK GENMASK(13, 0)
/* internal quirks - keep in GS_CAN_FEATURE space for now */
@@ -919,6 +928,10 @@ static int gs_can_open(struct net_device *netdev)
if (ctrlmode & CAN_CTRLMODE_ONE_SHOT)
flags |= GS_CAN_MODE_ONE_SHOT;
+ flags |= GS_CAN_MODE_BERR_REPORTING_SET;
+ if (ctrlmode & CAN_CTRLMODE_BERR_REPORTING)
+ flags |= GS_CAN_MODE_BERR_REPORTING;
+
if (ctrlmode & CAN_CTRLMODE_FD)
flags |= GS_CAN_MODE_FD;
@@ -1223,6 +1236,9 @@ static struct gs_can *gs_make_candev(unsigned int
channel,
}
}
+ if (feature & GS_CAN_FEATURE_BERR_REPORTING)
+ dev->can.ctrlmode_supported |= CAN_CTRLMODE_BERR_REPORTING;
+
/* The CANtact Pro from LinkLayer Labs is based on the
* LPC54616 µC, which is affected by the NXP LPC USB transfer
* erratum. However, the current firmware (version 2) doesn't
--
2.25.1
That claims another bit, but at least prevents the !NOT_BIT_ERRORS road.
Well, as far as I am concerned it is up to you guys. As long as there is
a way
to suppress them I would be happy. With stats + counters you generally know
more then enough, no reason to flood the bus with useless error messages.
Regards,
Jeroen
