On 07/09/2014 09:31 PM, Nikita Edward Baruzdin wrote:
> This adds support for hardware loopback in SJA1000 by utilising its self
> reception request (SRR) feature. Upon SRR the message is transmitted and
> received simultaneously, meaning you can't have hardware loopback
> without actually sending a message to the CAN bus in case of SJA1000.
> 
> Signed-off-by: Nikita Edward Baruzdin <nebaruzdin@gmail.com>

Looks good, coding style nitpick inline...

> ---
>  drivers/net/can/sja1000/sja1000.c | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/can/sja1000/sja1000.c b/drivers/net/can/sja1000/sja1000.c
> index f31499a..c480bce 100644
> --- a/drivers/net/can/sja1000/sja1000.c
> +++ b/drivers/net/can/sja1000/sja1000.c
> @@ -278,6 +278,7 @@ static netdev_tx_t sja1000_start_xmit(struct sk_buff *skb,
>  	uint8_t dlc;
>  	canid_t id;
>  	uint8_t dreg;
> +	u8 cmd_reg_val = 0x00;
>  	int i;
>  
>  	if (can_dropped_invalid_skb(dev, skb))
> @@ -312,9 +313,14 @@ static netdev_tx_t sja1000_start_xmit(struct sk_buff *skb,
>  	can_put_echo_skb(skb, dev, 0);
>  
>  	if (priv->can.ctrlmode & CAN_CTRLMODE_ONE_SHOT)
> -		sja1000_write_cmdreg(priv, CMD_TR | CMD_AT);
> +		cmd_reg_val |= CMD_AT;
> +
> +	if (priv->can.ctrlmode & CAN_CTRLMODE_LOOPBACK)
> +		cmd_reg_val |= CMD_SRR;
>  	else
> -		sja1000_write_cmdreg(priv, CMD_TR);
> +		cmd_reg_val |= CMD_TR;
> +
> +	sja1000_write_cmdreg(priv, cmd_reg_val);
>  
>  	return NETDEV_TX_OK;
>  }
> @@ -622,9 +628,11 @@ struct net_device *alloc_sja1000dev(int sizeof_priv)
>  	priv->can.do_set_bittiming = sja1000_set_bittiming;
>  	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_LISTENONLY |
> -		CAN_CTRLMODE_ONE_SHOT;
> +	priv->can.ctrlmode_supported = CAN_CTRLMODE_LOOPBACK
> +				     | CAN_CTRLMODE_LISTENONLY
> +				     | CAN_CTRLMODE_3_SAMPLES
> +				     | CAN_CTRLMODE_ONE_SHOT
> +				     | CAN_CTRLMODE_BERR_REPORTING;

Please keep the '|' at the end of the lines.

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