Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752433AbaBZNWS (ORCPT ); Wed, 26 Feb 2014 08:22:18 -0500 Received: from metis.ext.pengutronix.de ([92.198.50.35]:38224 "EHLO metis.ext.pengutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751368AbaBZNWR (ORCPT ); Wed, 26 Feb 2014 08:22:17 -0500 Message-ID: <530DEA7C.6010609@pengutronix.de> Date: Wed, 26 Feb 2014 14:22:04 +0100 From: Marc Kleine-Budde User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Icedove/24.3.0 MIME-Version: 1.0 To: Appana Durga Kedareswara Rao , "wg@grandegger.com" , Michal Simek , "grant.likely@linaro.org" , "robh+dt@kernel.org" , "linux-can@vger.kernel.org" CC: "netdev@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , "linux-kernel@vger.kernel.org" , "devicetree@vger.kernel.org" Subject: Re: [PATCH v4] can: xilinx CAN controller support. References: <74d607e9-ac9c-4a9b-ac49-e84ae49d20c2@TX2EHSMHS031.ehs.local> <530D0A9C.5090402@pengutronix.de> In-Reply-To: X-Enigmail-Version: 1.6 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="WLaE7bP6u1l5645uGMVJRtbhg45ixMqLi" X-SA-Exim-Connect-IP: 2001:6f8:1178:4:5054:ff:fe8d:eefb X-SA-Exim-Mail-From: mkl@pengutronix.de X-SA-Exim-Scanned: No (on metis.ext.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: linux-kernel@vger.kernel.org Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --WLaE7bP6u1l5645uGMVJRtbhg45ixMqLi Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 02/26/2014 02:07 PM, Appana Durga Kedareswara Rao wrote: >> This loop looks broken. Can you explain how it works. >> >> What it shoud do is: >> We have put (priv->tx_head - priv->tx_tail) CAN frames into the FIFO. >> This means at maximum there could be this amount of CAN frames which >> have been successfully transmitted. For every cycle in this while loop= you >> should: >> a) check if a CAN frame has successfully been transmitted >> (as this CAN core uses a FIFO it should be "oldest") >> A read_reg() of some kind is missing in your loop. >> b) if needed, remove this event from the FIFO or >> mark the interrupt as done. Whatever you hardware needs. >> c) update your statistics >> d) Use can_get_echo_skb to push this frame into the networking stack >> e) As a CAN frame has been transmitted successfully, wake the tx_queue= =2E >> >>> + while (priv->tx_head - priv->tx_tail > 0) { >>> + if (isr & XCAN_IXR_TXFLL_MASK) { >>> + priv->write_reg(priv, XCAN_ICR_OFFSET, >>> + XCAN_IXR_TXFLL_MASK); >>> + netif_stop_queue(ndev); >> >> Why do you stop the queue here? A CAN frame has successfully been >> transmitted, there should be room in the FIFO. >> >>> + break; >>> + } >>> + can_get_echo_skb(ndev, priv->tx_tail % >>> + priv->xcan_echo_skb_max_tx); >>> + priv->tx_tail++; >>> + } >>> + >=20 > The below are the bit fields available for the Transmit FIFO. > 1) In the ISR(interrupt status register) Tx Ok interrupt and Tx fifo f= ull interrupt. > 2) in the SR(Status Register) Tx fifo full condition. >=20 >=20 > I am modifying the entire tx interrupt logic to like below. >=20 > static void xcan_tx_interrupt(struct net_device *ndev, u32 isr) > { > struct xcan_priv *priv =3D netdev_priv(ndev); > struct net_device_stats *stats =3D &ndev->stats; >=20 > while (priv->tx_head - priv->tx_tail > 0) { > if (isr & XCAN_IXR_TXFLL_MASK) { > priv->write_reg(priv, XCAN_ICR_OFFSET, > XCAN_IXR_TXFLL_MASK); > break; > } > can_get_echo_skb(ndev, priv->tx_tail % > priv->xcan_echo_skb_max_tx); > priv->tx_tail++; > stats->tx_packets++; > netif_wake_queue(ndev); > can_led_event(ndev, CAN_LED_EVENT_TX); >=20 > } You just need to wake the queue once. > } >=20 >=20 > Are you Ok with the above logic? No, how can you tell how many frames have been transmitted? Marc --=20 Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions | Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917-5555 | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | --WLaE7bP6u1l5645uGMVJRtbhg45ixMqLi Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 Comment: Using GnuPG with Icedove - http://www.enigmail.net/ iEYEARECAAYFAlMN6nwACgkQjTAFq1RaXHPQGQCffOrc4qe/mhVZnM1NvNN3XTMq JfAAn3poCqSBHxbeZtkG8DiW4dHP6rDk =fs9R -----END PGP SIGNATURE----- --WLaE7bP6u1l5645uGMVJRtbhg45ixMqLi-- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/