From: Andri Yngvason <andri.yngvason@marel.com>
To: "Thomas Körper" <Thomas.Koerper@esd.eu>
Cc: "linux-can@vger.kernel.org" <linux-can@vger.kernel.org>
Subject: Re: AW: [PATCH V5 1/1] can: Add support for esd CAN PCIe/402 card
Date: Tue, 17 Mar 2015 10:33:59 +0000 [thread overview]
Message-ID: <20150317103359.11690.3930@shannon> (raw)
In-Reply-To: <8CE1D0B9BFD2404DA079DDE1814A6F2E03294EFD39B3@esd-s3.esd.local>
Hi Thomas,
Quoting Thomas Körper (2015-03-17 06:30:02)
> Hi Andri,
>
> I've looked at the sources you mentioned, but I'm a little bit unsure now /
> the handling seems not perfectly consistent to me. (flexcan calls
> can_change_state() with tx/rx_state of 0 in the bus off path. Only
That's an oversight on my part.
> kvaser_usb counts rx_dropped++ if skb alloc failed, and setting
Yes, Darwish is very diligent in handling OOM.
> cf->data[6]/cf->data[7] to the counter values seems also rarely used)
Some chips don't expose the error counters.
>
> ...so let me show the reworked function first before I post a new patch:
>
> static void
> handle_core_msg_errstatechange(struct acc_core *core,
> const struct acc_bmmsg_errstatechange *msg)
> {
> struct acc_net_priv *priv = netdev_priv(core->net_dev);
> struct net_device_stats *stats = &core->net_dev->stats;
> struct can_frame *cf;
> struct sk_buff *skb;
>
> skb = alloc_can_err_skb(core->net_dev, &cf);
> if (skb) {
> enum can_state new_state;
> u8 txerr;
> u8 rxerr;
>
> txerr = (u8)(msg->reg_status >> 8);
> rxerr = (u8)msg->reg_status;
>
> cf->data[6] = txerr;
> cf->data[7] = rxerr;
>
> if (msg->reg_status & ACC_REG_STATUS_MASK_STATUS_BS) {
> new_state = CAN_STATE_BUS_OFF;
> } else if (msg->reg_status & ACC_REG_STATUS_MASK_STATUS_EP) {
> new_state = CAN_STATE_ERROR_PASSIVE;
> } else if (msg->reg_status & ACC_REG_STATUS_MASK_STATUS_ES) {
> new_state = CAN_STATE_ERROR_WARNING;
> } else {
> new_state = CAN_STATE_ERROR_ACTIVE;
> }
>
> if (new_state != priv->can.state) {
> enum can_state tx_state, rx_state;
>
> tx_state = (txerr >= rxerr) ? new_state : 0;
> rx_state = (rxerr >= txerr) ? new_state : 0;
>
> can_change_state(core->net_dev, cf, tx_state, rx_state);
> }
>
> netif_rx(skb);
> stats->rx_packets++;
> stats->rx_bytes += cf->can_dlc;
> } else {
> stats->rx_dropped++;
> }
>
> if (msg->reg_status & ACC_REG_STATUS_MASK_STATUS_BS) {
> acc_write32(core, ACC_CORE_OF_TX_ABORT_MASK, 0xffff);
> can_bus_off(core->net_dev);
> }
> }
>
Looks good!
Best regards,
Andri
next prev parent reply other threads:[~2015-03-17 10:49 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-03-16 12:15 [PATCH V5 1/1] can: Add support for esd CAN PCIe/402 card Thomas Körper
2015-03-16 12:46 ` Andri Yngvason
2015-03-16 13:35 ` Marc Kleine-Budde
2015-03-17 6:30 ` AW: " Thomas Körper
2015-03-17 7:26 ` Ahmed S. Darwish
2015-03-17 10:10 ` Andri Yngvason
2015-03-17 10:33 ` Andri Yngvason [this message]
2015-03-17 21:27 ` Marc Kleine-Budde
2015-03-18 5:08 ` Thomas Körper
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20150317103359.11690.3930@shannon \
--to=andri.yngvason@marel.com \
--cc=Thomas.Koerper@esd.eu \
--cc=linux-can@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).