Re: [PATCH] can: sja1000: Add support for listen-only mode and one-shot mode

[Marc Kleine-Budde <mkl@pengutronix.de>]

[-- Attachment #1: Type: text/plain, Size: 2759 bytes --]

On 09/17/2012 05:58 PM, Andreas Larsson wrote:
> Packages that fails to be transmitted in one-shot mode are never the
> less echoed back as sja1000 does not give any indication when a single
> transmission has failed.

Uhh... can any of the SJA1000 gurus comment to this?

Some write-more-readable-code comments inline :)

> Signed-off-by: Andreas Larsson <andreas@gaisler.com>
> ---
>  drivers/net/can/sja1000/sja1000.c |   10 +++++++---
>  1 files changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/can/sja1000/sja1000.c b/drivers/net/can/sja1000/sja1000.c
> index 4c4f33d..cc771d9 100644
> --- a/drivers/net/can/sja1000/sja1000.c
> +++ b/drivers/net/can/sja1000/sja1000.c
> @@ -141,6 +141,7 @@ static void set_normal_mode(struct net_device *dev)
>  	struct sja1000_priv *priv = netdev_priv(dev);
>  	unsigned char status = priv->read_reg(priv, REG_MOD);
>  	int i;
> +	u8 normal = priv->can.ctrlmode & CAN_CTRLMODE_LISTENONLY ? MOD_LOM : 0;

What about a more readable:
	u8 normal = 0;

	if (priv->can.ctrlmode & CAN_CTRLMODE_LISTENONLY)
		normal = MOD_LOM;

>  
>  	for (i = 0; i < 100; i++) {
>  		/* check reset bit */
> @@ -156,7 +157,7 @@ static void set_normal_mode(struct net_device *dev)
>  		}
>  
>  		/* set chip to normal mode */
> -		priv->write_reg(priv, REG_MOD, 0x00);
> +		priv->write_reg(priv, REG_MOD, normal);
>  		udelay(10);
>  		status = priv->read_reg(priv, REG_MOD);
>  	}
> @@ -278,6 +279,7 @@ static netdev_tx_t sja1000_start_xmit(struct sk_buff *skb,
>  	canid_t id;
>  	uint8_t dreg;
>  	int i;
> +	u8 oneshot;
>  
>  	if (can_dropped_invalid_skb(dev, skb))
>  		return NETDEV_TX_OK;
> @@ -310,7 +312,8 @@ static netdev_tx_t sja1000_start_xmit(struct sk_buff *skb,
>  
>  	can_put_echo_skb(skb, dev, 0);
>  
> -	sja1000_write_cmdreg(priv, CMD_TR);
> +	oneshot = (priv->can.ctrlmode & CAN_CTRLMODE_ONE_SHOT) ? CMD_AT : 0;
> +	sja1000_write_cmdreg(priv, CMD_TR | oneshot);

something simmilar here, too

>  	return NETDEV_TX_OK;
>  }
> @@ -605,7 +608,8 @@ struct net_device *alloc_sja1000dev(int sizeof_priv)
>  	priv->can.do_set_mode = sja1000_set_mode;
>  	priv->can.do_get_berr_counter = sja1000_get_berr_counter;
>  	priv->can.ctrlmode_supported = CAN_CTRLMODE_3_SAMPLES |
> -		CAN_CTRLMODE_BERR_REPORTING;
> +		CAN_CTRLMODE_BERR_REPORTING | CAN_CTRLMODE_LISTENONLY |
> +		CAN_CTRLMODE_ONE_SHOT;
>  
>  	spin_lock_init(&priv->cmdreg_lock);
>  
> 

Marc

-- 
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   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]