Re: [PATCH] net-next:can: add TI CAN (HECC) driver
From: Wolfgang Grandegger <hidden>
Date: 2009-10-05 11:25:16
Possibly related (same subject, not in this thread)
- 2009-10-05 · [PATCH] net-next:can: add TI CAN (HECC) driver · Anant Gole <hidden>
- 2009-09-01 · RE: [PATCH] net-next:can: add TI CAN (HECC) driver · Gole, Anant <hidden>
- 2009-09-01 · Re: [PATCH] net-next:can: add TI CAN (HECC) driver · Wolfram Sang <hidden>
- 2009-08-31 · Re: [PATCH] net-next:can: add TI CAN (HECC) driver · Jan Kiszka <jan.kiszka@siemens.com>
- 2009-08-31 · Re: [PATCH] net-next:can: add TI CAN (HECC) driver · Oliver Hartkopp <socketcan@hartkopp.net>
Anant Gole wrote:
TI HECC (High End CAN Controller) module is found on many TI devices. It has 32 hardware mailboxes with full implementation of CAN protocol 2.0B with bus speeds up to 1Mbps. Specifications of the module are available on TI web <http://www.ti.com> Signed-off-by: Anant Gole <redacted>
I already reviewed this driver on the Socketcan-core ML and it's almost OK from the Socket-CAN point of view. Just one issue... [snip]
quoted hunk
diff --git a/drivers/net/can/ti_hecc.c b/drivers/net/can/ti_hecc.c new file mode 100644 index 0000000..9090103 --- /dev/null +++ b/drivers/net/can/ti_hecc.c
[snip]
+static int ti_hecc_set_btc(struct ti_hecc_priv *priv)
+{
+ struct can_bittiming *bit_timing = &priv->can.bittiming;
+ u32 can_btc;
+
+ can_btc = (bit_timing->phase_seg2 - 1) & 0x7;
+ can_btc |= ((bit_timing->phase_seg1 + bit_timing->prop_seg - 1)
+ & 0xF) << 3;
+ if (bit_timing->brp > 4 && priv->can.ctrlmode & CAN_CTRLMODE_3_SAMPLES)
+ can_btc |= HECC_CANBTC_SAM;
+ else
+ dev_info(priv->ndev->dev.parent,
+ "WARN: Triple sampling not set due to h/w limitations"
+ " at %d bitrate", bit_timing->bitrate);
That's not correct from my point of view. The warning message should
only be printed when the user tries to set triple sampling. I think it
should be similar to:
if (priv->can.ctrlmode & CAN_CTRLMODE_3_SAMPLES) {
if (bit_timing->brp > 4)
can_btc |= HECC_CANBTC_SAM;
else
dev_warn(priv->ndev->dev.parent,
"Triple sampling not set due to h/w "
limitations");
}
Apart from that, the patch looks OK.
Wolfgang.