Re: [PATCH 06/16] c_can: consider set bittiming may fail

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

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

On 09/09/2013 09:25 AM, Benedikt Spranger wrote:
> While setting the bittiming the C_CAN/D_CAN may fail. Do not bring up the
> CAN interface in this case.
> 
> Signed-off-by: Benedikt Spranger <b.spranger@linutronix.de>
> ---
>  drivers/net/can/c_can/c_can.c | 25 ++++++++++++++++++++-----
>  1 file changed, 20 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
> index b3cfb85..3fa5347 100644
> --- a/drivers/net/can/c_can/c_can.c
> +++ b/drivers/net/can/c_can/c_can.c
> @@ -689,9 +689,10 @@ static void c_can_configure_msg_objects(struct net_device *dev)
>   * - set operating mode
>   * - configure message objects
>   */
> -static void c_can_chip_config(struct net_device *dev)
> +static int c_can_chip_config(struct net_device *dev)
>  {
>  	struct c_can_priv *priv = netdev_priv(dev);
> +	int ret;
>  
>  	/* enable automatic retransmission */
>  	priv->write_reg(priv, C_CAN_CTRL_REG,
> @@ -726,15 +727,20 @@ static void c_can_chip_config(struct net_device *dev)
>  	priv->write_reg(priv, C_CAN_STS_REG, LEC_UNUSED);
>  
>  	/* set bittiming params */
> -	c_can_set_bittiming(dev);
> +	ret = c_can_set_bittiming(dev);
> +
> +	return ret;

Maybe just:
	return c_can_set_bittiming(dev);
>  }
>  
> -static void c_can_start(struct net_device *dev)
> +static int c_can_start(struct net_device *dev)
>  {
>  	struct c_can_priv *priv = netdev_priv(dev);
> +	int ret;
>  
>  	/* basic c_can configuration */
> -	c_can_chip_config(dev);
> +	ret = c_can_chip_config(dev);
> +	if (ret)
> +		goto out;

No need for a goto here, IMHO.

>  
>  	priv->can.state = CAN_STATE_ERROR_ACTIVE;
>  
> @@ -743,6 +749,9 @@ static void c_can_start(struct net_device *dev)
>  
>  	/* enable status change, error and module interrupts */
>  	c_can_enable_all_interrupts(priv, ENABLE_ALL_INTERRUPTS);
> +
> +out:
> +	return ret;
>  }
>  
>  static void c_can_stop(struct net_device *dev)
> @@ -758,9 +767,15 @@ static void c_can_stop(struct net_device *dev)
>  
>  static int c_can_set_mode(struct net_device *dev, enum can_mode mode)
>  {
> +	int ret;
> +
>  	switch (mode) {
>  	case CAN_MODE_START:
> -		c_can_start(dev);
> +		ret = c_can_start(dev);
> +		if (ret) {
> +			c_can_stop(dev);
> +			return ret;
> +		}
>  		netif_wake_queue(dev);
>  		break;
>  	default:
> 

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