netdev - Re: [PATCH 3/6] can: sja1000: Add Quirks for RZ/N1 SJA1000 CAN controller

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 2 Jul 2022 18:33:22 +0200
From:   Marc Kleine-Budde <mkl@...gutronix.de>
To:     Biju Das <biju.das.jz@...renesas.com>
Cc:     Wolfgang Grandegger <wg@...ndegger.com>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Stefan Mätje <stefan.maetje@....eu>,
        Vincent Mailhol <mailhol.vincent@...adoo.fr>,
        Oliver Hartkopp <socketcan@...tkopp.net>,
        linux-can@...r.kernel.org, netdev@...r.kernel.org,
        Geert Uytterhoeven <geert+renesas@...der.be>,
        Chris Paterson <Chris.Paterson2@...esas.com>,
        Biju Das <biju.das@...renesas.com>,
        linux-renesas-soc@...r.kernel.org
Subject: Re: [PATCH 3/6] can: sja1000: Add Quirks for RZ/N1 SJA1000 CAN
 controller

On 02.07.2022 15:01:27, Biju Das wrote:
> Chapter 6.5.16 of the RZ/N1 Peripheral Manual mentions the below
> differences compared to the reference Philips SJA1000 device.
> 
> Handling of Transmitted Messages:
>  * The CAN controller does not copy transmitted messages to the receive
>    buffer, unlike the reference device.

This is something different than....

>  int register_sja1000dev(struct net_device *dev)
>  {
> +	struct sja1000_priv *priv = netdev_priv(dev);
>  	int ret;
>  
>  	if (!sja1000_probe_chip(dev))
>  		return -ENODEV;
>  
> -	dev->flags |= IFF_ECHO;	/* we support local echo */
> +	if (!(priv->flags & SJA1000_NO_HW_LOOPBACK_QUIRK))
> +		dev->flags |= IFF_ECHO;	/* we support local echo */

... the IFF_ECHO.

IFF_ECHO set means the driver cals can_put_echo_skb() before TX and
can_get_echo_skb() after TX complete interrupt.

| irqreturn_t sja1000_interrupt(int irq, void *dev_id)
[...]
| 	while ((isrc = priv->read_reg(priv, SJA1000_IR)) &&
| 	       (n < SJA1000_MAX_IRQ)) {
| 
| 		status = priv->read_reg(priv, SJA1000_SR);
| 		/* check for absent controller due to hw unplug */
| 		if (status == 0xFF && sja1000_is_absent(priv))
| 			goto out;
| 
| 		if (isrc & IRQ_WUI)
| 			netdev_warn(dev, "wakeup interrupt\n");
| 
| 		if (isrc & IRQ_TI) {
| 			/* transmission buffer released */
| 			if (priv->can.ctrlmode & CAN_CTRLMODE_ONE_SHOT &&
| 			    !(status & SR_TCS)) {
| 				stats->tx_errors++;
| 				can_free_echo_skb(dev, 0, NULL);
| 			} else {

Please add a netdev_info() for debugging and verify that you get a TX
complete IRQ.

| 				/* transmission complete */
| 				stats->tx_bytes += can_get_echo_skb(dev, 0, NULL);
| 				stats->tx_packets++;
| 			}
| 			netif_wake_queue(dev);
| 		}


If your hardware doesn't support hardware loopback (configured via
CMD_SRR):

| 	if (priv->can.ctrlmode & CAN_CTRLMODE_LOOPBACK)
| 		cmd_reg_val |= CMD_SRR;
| 	else
| 		cmd_reg_val |= CMD_TR;

then don't set CAN_CTRLMODE_LOOPBACK in priv->can.ctrlmode_supported.

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 |

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)