On 09.10.2022 00:11:49, Jeroen Hofstee wrote:
> 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)
You've convinced me :) This is overengineered, and it "breaks" the nice
1-to-1 mapping of feature and mode bits :)
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:
signature.asc
Description: PGP signature
