Re: [PATCH v3 4/7] net: can: flexcan: move rx offload_add from probe to start

[Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx>]

On 08/01/2018 04:06 PM, Pankaj Bansal wrote:
> rx offload depends on number of message buffers, which in turn depends on
> messgae buffer size. with the upcoming LX2160A SOC the message buffer size
> can be configured to 72 bytes if it were to be used in CAN FD mode.
> 
> The current mode in which the flexcan is being operated is known at the
> time of flexcan_open and not at the time of flexcan_probe.
> 
> Therefore, move the rx offload_add from flexcan_probe to flexcan_open.
> correspondingly, move rx offload_delete form flexcan_remove to
> flexcan_close.
> 
> Signed-off-by: Pankaj Bansal <pankaj.bansal@xxxxxxx>
> ---
> 
> Notes:
>     V3:
>     - New patch
>     V2:
>     - Not existed
> 
>  drivers/net/can/flexcan.c | 73 +++++++++++++++++++------------------
>  1 file changed, 37 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index 755a761eef02..04aa264cb771 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -1116,6 +1116,41 @@ static int flexcan_open(struct net_device *dev)
>  	if (err)
>  		goto out_close;
>  
> +	if (priv->devtype_data->quirks & FLEXCAN_QUIRK_USE_OFF_TIMESTAMP) {
> +		priv->tx_mb_idx = FLEXCAN_TX_MB_OFF_TIMESTAMP;
> +		priv->tx_mb_reserved = &regs->mb[FLEXCAN_TX_MB_RESERVED_OFF_TIMESTAMP];
> +	} else {
> +		priv->tx_mb_idx = FLEXCAN_TX_MB_OFF_FIFO;
> +		priv->tx_mb_reserved = &regs->mb[FLEXCAN_TX_MB_RESERVED_OFF_FIFO];
> +	}
> +	priv->tx_mb = &regs->mb[priv->tx_mb_idx];
> +
> +	priv->reg_imask1_default = FLEXCAN_IFLAG_MB(priv->tx_mb_idx);
> +	priv->reg_imask2_default = 0;
> +
> +	priv->offload.mailbox_read = flexcan_mailbox_read;
> +
> +	if (priv->devtype_data->quirks & FLEXCAN_QUIRK_USE_OFF_TIMESTAMP) {
> +		u64 imask;
> +
> +		priv->offload.mb_first = FLEXCAN_RX_MB_OFF_TIMESTAMP_FIRST;
> +		priv->offload.mb_last = FLEXCAN_RX_MB_OFF_TIMESTAMP_LAST;
> +
> +		imask = GENMASK_ULL(priv->offload.mb_last,
> +				    priv->offload.mb_first);
> +		priv->reg_imask1_default |= imask;
> +		priv->reg_imask2_default |= imask >> 32;
> +
> +		err = can_rx_offload_add_timestamp(dev, &priv->offload);
> +	} else {
> +		priv->reg_imask1_default |= FLEXCAN_IFLAG_RX_FIFO_OVERFLOW |
> +			FLEXCAN_IFLAG_RX_FIFO_AVAILABLE;
> +		err = can_rx_offload_add_fifo(dev, &priv->offload,
> +					      FLEXCAN_NAPI_WEIGHT);
> +	}
> +	if (err)
> +		goto failed_offload;

Please keep the naming scheme for the labels in this function, this
would be "out_free_irq".

> +
>  	/* start chip and queuing */
>  	err = flexcan_chip_start(dev);
>  	if (err)

You need to cleanup the rx_offload here. goto out_offload_del;

> @@ -1128,6 +1163,7 @@ static int flexcan_open(struct net_device *dev)
>  
>  	return 0;
>  
> + failed_offload:

Replace by:

out_offload_del:
	can_rx_offload_del();

>   out_free_irq:
>  	free_irq(dev->irq, dev);
>   out_close:
> @@ -1148,6 +1184,7 @@ static int flexcan_close(struct net_device *dev)
>  	can_rx_offload_disable(&priv->offload);
>  	flexcan_chip_stop(dev);
>  
> +	can_rx_offload_del(&priv->offload);
>  	free_irq(dev->irq, dev);
>  	clk_disable_unprepare(priv->clk_per);
>  	clk_disable_unprepare(priv->clk_ipg);
> @@ -1359,39 +1396,6 @@ static int flexcan_probe(struct platform_device *pdev)
>  	priv->devtype_data = devtype_data;
>  	priv->reg_xceiver = reg_xceiver;
>  
> -	if (priv->devtype_data->quirks & FLEXCAN_QUIRK_USE_OFF_TIMESTAMP) {
> -		priv->tx_mb_idx = FLEXCAN_TX_MB_OFF_TIMESTAMP;
> -		priv->tx_mb_reserved = &regs->mb[FLEXCAN_TX_MB_RESERVED_OFF_TIMESTAMP];
> -	} else {
> -		priv->tx_mb_idx = FLEXCAN_TX_MB_OFF_FIFO;
> -		priv->tx_mb_reserved = &regs->mb[FLEXCAN_TX_MB_RESERVED_OFF_FIFO];
> -	}
> -	priv->tx_mb = &regs->mb[priv->tx_mb_idx];
> -
> -	priv->reg_imask1_default = FLEXCAN_IFLAG_MB(priv->tx_mb_idx);
> -	priv->reg_imask2_default = 0;
> -
> -	priv->offload.mailbox_read = flexcan_mailbox_read;
> -
> -	if (priv->devtype_data->quirks & FLEXCAN_QUIRK_USE_OFF_TIMESTAMP) {
> -		u64 imask;
> -
> -		priv->offload.mb_first = FLEXCAN_RX_MB_OFF_TIMESTAMP_FIRST;
> -		priv->offload.mb_last = FLEXCAN_RX_MB_OFF_TIMESTAMP_LAST;
> -
> -		imask = GENMASK_ULL(priv->offload.mb_last, priv->offload.mb_first);
> -		priv->reg_imask1_default |= imask;
> -		priv->reg_imask2_default |= imask >> 32;
> -
> -		err = can_rx_offload_add_timestamp(dev, &priv->offload);
> -	} else {
> -		priv->reg_imask1_default |= FLEXCAN_IFLAG_RX_FIFO_OVERFLOW |
> -			FLEXCAN_IFLAG_RX_FIFO_AVAILABLE;
> -		err = can_rx_offload_add_fifo(dev, &priv->offload, FLEXCAN_NAPI_WEIGHT);
> -	}
> -	if (err)
> -		goto failed_offload;
> -
>  	err = register_flexcandev(dev);
>  	if (err) {
>  		dev_err(&pdev->dev, "registering netdev failed\n");
> @@ -1405,7 +1409,6 @@ static int flexcan_probe(struct platform_device *pdev)
>  
>  	return 0;
>  
> - failed_offload:
>   failed_register:
>  	free_candev(dev);
>  	return err;
> @@ -1414,10 +1417,8 @@ static int flexcan_probe(struct platform_device *pdev)
>  static int flexcan_remove(struct platform_device *pdev)
>  {
>  	struct net_device *dev = platform_get_drvdata(pdev);
> -	struct flexcan_priv *priv = netdev_priv(dev);
>  
>  	unregister_flexcandev(dev);
> -	can_rx_offload_del(&priv->offload);
>  	free_candev(dev);
>  
>  	return 0;
> 

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: signature.asc
Description: OpenPGP digital signature