Re: [PATCH 5/6] net: can: flexcan: split the Message Buffer RAM area

[Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx>]

On 07/30/2018 06:07 PM, Pankaj Bansal wrote:
> The message buffer RAM area is not a contiguous 1KB area but 2 partitions
> of 512 bytes each. Till now, we used Message buffers with payload size 8
> bytes, which translates to 32 MBs per partition and no spare space is left
> in any partition.
> However, in upcoming SOC LX2160A the message buffers can have payload size
> greater than 8 bytes. This results in less than 32 MBs per partition and
> some empty area is left at the end of each partition.This empty area should
> not be accessed.
> Therefore, split the Message Buffer RAM area into two partitions.
> 
> Signed-off-by: Pankaj Bansal <pankaj.bansal@xxxxxxx>
> ---
>  drivers/net/can/flexcan.c | 66 ++++++++++++++++++++++++++++---------
>  1 file changed, 50 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index 15fa983a8237..147405e703f0 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -223,7 +223,8 @@ struct flexcan_regs {
>  	u32 rxfgmask;		/* 0x48 */
>  	u32 rxfir;		/* 0x4c */
>  	u32 _reserved3[12];	/* 0x50 */
> -	u32 mb[256];		/* 0x80 */
> +	u32 mb1[128];		/* 0x80 */
> +	u32 mb2[128];		/* 0x280 */
>  	/* FIFO-mode:
>  	 *			MB
>  	 * 0x080...0x08f	0	RX message buffer
> @@ -262,6 +263,8 @@ struct flexcan_priv {
>  	struct flexcan_mb __iomem *tx_mb;
>  	struct flexcan_mb __iomem *tx_mb_reserved;
>  	u8 tx_mb_idx;
> +	u8 mb_count_block1;
> +	u8 mb_count_block2;

What's the difference between these two?

>  	u32 reg_ctrl_default;
>  	u32 reg_imask1_default;
>  	u32 reg_imask2_default;
> @@ -676,7 +679,13 @@ static unsigned int flexcan_mailbox_read(struct can_rx_offload *offload,
>  
>  	mb_size = sizeof(struct flexcan_mb) + priv->devtype_data->payload_size;
>  
> -	mb = (struct flexcan_mb __iomem *)((u8 *)&regs->mb + (mb_size * n));
> +	if (n < priv->mb_count_block1)
> +		mb = (struct flexcan_mb __iomem *)
> +		     ((u8 *)&regs->mb1 + (mb_size * n));
> +	else
> +		mb = (struct flexcan_mb __iomem *)
> +		     ((u8 *)&regs->mb2 +
> +		     (mb_size * (n - priv->mb_count_block1)));

At the first glance there are 4 copies of this functionality in the
code. Better introduce a helper function returning the mb with the
mailbox number as an input.

>  
>  	if (priv->devtype_data->quirks & FLEXCAN_QUIRK_USE_OFF_TIMESTAMP) {
>  		u32 code;
> @@ -1003,9 +1012,14 @@ static int flexcan_chip_start(struct net_device *dev)
>  	}
>  
>  	/* clear and invalidate all mailboxes first */
> -	for (i = priv->tx_mb_idx; i < (sizeof(regs->mb) / mb_size); i++) {
> +	for (i = priv->tx_mb_idx; i < priv->mb_count_block1; i++) {
> +		mb = (struct flexcan_mb __iomem *)
> +		     ((u8 *)&regs->mb1 + (mb_size * i));
> +		priv->write(FLEXCAN_MB_CODE_RX_INACTIVE, &mb->can_ctrl);
> +	}
> +	for (i -= priv->mb_count_block1; i < priv->mb_count_block2; i++) {
>  		mb = (struct flexcan_mb __iomem *)
> -		     ((u8 *)&regs->mb + (mb_size * i));
> +		     ((u8 *)&regs->mb2 + (mb_size * i));
>  		priv->write(FLEXCAN_MB_CODE_RX_INACTIVE, &mb->can_ctrl);
>  	}
>  
> @@ -1013,8 +1027,13 @@ static int flexcan_chip_start(struct net_device *dev)
>  		for (i = priv->offload.mb_first;
>  		     i <= priv->offload.mb_last;
>  		     i++) {
> -			mb = (struct flexcan_mb __iomem *)
> -			     ((u8 *)&regs->mb + (mb_size * i));
> +			if (i < priv->mb_count_block1)
> +				mb = (struct flexcan_mb __iomem *)
> +				     ((u8 *)&regs->mb1 + (mb_size * i));
> +			else
> +				mb = (struct flexcan_mb __iomem *)
> +				     ((u8 *)&regs->mb2 +
> +				     (mb_size * (i - priv->mb_count_block1)));
>  			priv->write(FLEXCAN_MB_CODE_RX_EMPTY, &mb->can_ctrl);
>  		}
>  	}
> @@ -1300,7 +1319,7 @@ static int flexcan_probe(struct platform_device *pdev)
>  	struct flexcan_regs __iomem *regs;
>  	int err, irq;
>  	u32 clock_freq = 0;
> -	u8 mb_size;
> +	u8 mb_size, tx_mb_reserved_idx;
>  
>  	reg_xceiver = devm_regulator_get(&pdev->dev, "xceiver");
>  	if (PTR_ERR(reg_xceiver) == -EPROBE_DEFER)
> @@ -1376,6 +1395,8 @@ static int flexcan_probe(struct platform_device *pdev)
>  	}
>  	mb_size = sizeof(struct flexcan_mb) + devtype_data->payload_size;
>  
> +	priv->mb_count_block1 = sizeof(regs->mb1) / mb_size;
> +	priv->mb_count_block2 = sizeof(regs->mb2) / mb_size;
>  	priv->can.clock.freq = clock_freq;
>  	priv->can.bittiming_const = &flexcan_bittiming_const;
>  	priv->can.do_set_mode = flexcan_set_mode;
> @@ -1391,17 +1412,29 @@ static int flexcan_probe(struct platform_device *pdev)
>  
>  	if (priv->devtype_data->quirks & FLEXCAN_QUIRK_USE_OFF_TIMESTAMP) {
>  		priv->tx_mb_idx = FLEXCAN_TX_MB_OFF_TIMESTAMP;
> -		priv->tx_mb_reserved = (struct flexcan_mb __iomem *)
> -				       ((u8 *)&regs->mb + (mb_size *
> -				       FLEXCAN_TX_MB_RESERVED_OFF_TIMESTAMP));
> +		tx_mb_reserved_idx = FLEXCAN_TX_MB_RESERVED_OFF_TIMESTAMP;
>  	} else {
>  		priv->tx_mb_idx = FLEXCAN_TX_MB_OFF_FIFO;
> -		priv->tx_mb_reserved = (struct flexcan_mb __iomem *)
> -				       ((u8 *)&regs->mb + (mb_size *
> -				       FLEXCAN_TX_MB_RESERVED_OFF_FIFO));
> +		tx_mb_reserved_idx = FLEXCAN_TX_MB_RESERVED_OFF_FIFO;
>  	}
> -	priv->tx_mb = (struct flexcan_mb __iomem *)
> -		      ((u8 *)&regs->mb + (mb_size * priv->tx_mb_idx));
> +
> +	if (priv->tx_mb_idx < priv->mb_count_block1)
> +		priv->tx_mb = (struct flexcan_mb __iomem *)
> +			      ((u8 *)&regs->mb1 + (mb_size * priv->tx_mb_idx));
> +	else
> +		priv->tx_mb = (struct flexcan_mb __iomem *)
> +			      ((u8 *)&regs->mb2 + (mb_size *
> +			      (priv->tx_mb_idx - priv->mb_count_block1)));
> +
> +	if (tx_mb_reserved_idx < priv->mb_count_block1)
> +		priv->tx_mb_reserved = (struct flexcan_mb __iomem *)
> +				       ((u8 *)&regs->mb1 +
> +				       (mb_size * tx_mb_reserved_idx));
> +	else
> +		priv->tx_mb_reserved = (struct flexcan_mb __iomem *)
> +				       ((u8 *)&regs->mb2 + (mb_size *
> +				       (tx_mb_reserved_idx -
> +				       priv->mb_count_block1)));
>  
>  	priv->reg_imask1_default = FLEXCAN_IFLAG_MB(priv->tx_mb_idx);
>  	priv->reg_imask2_default = 0;
> @@ -1412,7 +1445,8 @@ static int flexcan_probe(struct platform_device *pdev)
>  		u64 imask;
>  
>  		priv->offload.mb_first = FLEXCAN_RX_MB_OFF_TIMESTAMP_FIRST;
> -		priv->offload.mb_last = (sizeof(regs->mb) / mb_size) - 1;
> +		priv->offload.mb_last = priv->mb_count_block1 +
> +					priv->mb_count_block2 - 1;
>  
>  		imask = GENMASK_ULL(priv->offload.mb_last, priv->offload.mb_first);
>  		priv->reg_imask1_default |= imask;
> 

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