Wolfgang Grandegger wrote:
>
> [PATCH/RFC] make bus-error generation configurable via netlink interface
>
> By default, bus-error generation is *disabled* and it can be enabled
> with: "ip link set can0 up type can bitrate 500000 bus-error 1".
>
> Signed-off-by: Wolfgang Grandegger <[email protected]>
> ---
Nice patch and incredible easy ;-)
This shows how easy we can add new functionalities to the concept ...
Two nitpicks:
1. I don't feel really comfortable with the naming of "bus-error" itself.
When i would see "bus-error 1" in my ip link show i would be confused, if my
CAN controller currently has currently a bus-error or not.
IMO it should be named like "enable-bus-errors" or "enable-berr" or
"berr-msgs" or "bus-err-msgs" or something like this.
2.
> Index: 2.6/drivers/net/can/sja1000/sja1000.c
> ===================================================================
> --- 2.6.orig/drivers/net/can/sja1000/sja1000.c
> +++ 2.6/drivers/net/can/sja1000/sja1000.c
> @@ -133,9 +133,13 @@ static void set_normal_mode(struct net_d
> for (i = 0; i < 100; i++) {
> /* check reset bit */
> if ((status & MOD_RM) == 0) {
> + u8 reg_ier = IRQ_ALL;
> +
> + if (!(priv->can.ctrlmode & CAN_CTRLMODE_BUS_ERROR))
> + reg_ier &= ~IRQ_BEI;
> priv->can.state = CAN_STATE_ERROR_ACTIVE;
> - /* enable all interrupts */
> - priv->write_reg(priv, REG_IER, IRQ_ALL);
> + /* enable interrupts */
> + priv->write_reg(priv, REG_IER, reg_ier);
> return;
> }
Why did you introduce the extra variable here?
priv->can.state = CAN_STATE_ERROR_ACTIVE;
/* enable interrupts */
if (!(priv->can.ctrlmode & CAN_CTRLMODE_BUS_ERROR))
priv->write_reg(priv, REG_IER, (IRQ_ALL & ~IRQ_BEI));
else
priv->write_reg(priv, REG_IER, IRQ_ALL);
Won't this work also?
Regards,
Oliver
_______________________________________________
Socketcan-users mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/socketcan-users
