Re: [PATCH v3 5/7] net: can: flexcan: Add provision for variable payload size

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 

On 08/01/2018 04:06 PM, Pankaj Bansal wrote:
> Till now the flexcan module supported 8 byte payload size as per CAN 2.0
> specifications.
> But now upcoming flexcan module in NXP LX2160A SOC supports CAN FD protocol
> too.The Message buffers need to be configured to have payload size 64
> bytes.
> 
> Therefore, added provision in the driver for payload size to be 64 bytes.
> 
> Signed-off-by: Pankaj Bansal <pankaj.bansal@xxxxxxx>
> ---
> 
> Notes:
>     V3:
>      - Removed payload_size from devtype_data. Now change MB size based on
>        flexcan control mode being FD, which in turn supports on FD mode being
>        supported. This is controlled by quirk.
>     V2:
>      - Change data from u32 to __be32 in flexcan_mailbox_read
>      - Added function flexcan_get_mb to get mailbox address from mailbox number
> 
>  drivers/net/can/flexcan.c | 87 ++++++++++++++++++++++++++-----------
>  1 file changed, 62 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index 04aa264cb771..232b70f371da 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -140,7 +140,6 @@
>  #define FLEXCAN_TX_MB_RESERVED_OFF_TIMESTAMP	0
>  #define FLEXCAN_TX_MB_OFF_TIMESTAMP		1
>  #define FLEXCAN_RX_MB_OFF_TIMESTAMP_FIRST	(FLEXCAN_TX_MB_OFF_TIMESTAMP + 1)
> -#define FLEXCAN_RX_MB_OFF_TIMESTAMP_LAST	63
>  #define FLEXCAN_IFLAG_MB(x)		BIT(x)
>  #define FLEXCAN_IFLAG_RX_FIFO_OVERFLOW	BIT(7)
>  #define FLEXCAN_IFLAG_RX_FIFO_WARN	BIT(6)
> @@ -190,12 +189,13 @@
>  #define FLEXCAN_QUIRK_USE_OFF_TIMESTAMP	BIT(5) /* Use timestamp based offloading */
>  #define FLEXCAN_QUIRK_BROKEN_PERR_STATE	BIT(6) /* No interrupt for error passive */
>  #define FLEXCAN_QUIRK_DEFAULT_BIG_ENDIAN	BIT(7) /* default to BE register access */
> +#define FLEXCAN_QUIRK_USE_FD	BIT(8) /* Supports CAN FD mode */
>  
>  /* Structure of the message buffer */
>  struct flexcan_mb {
>  	u32 can_ctrl;
>  	u32 can_id;
> -	u32 data[2];
> +	u32 data[];
>  };
>  
>  /* Structure of the hardware registers */
> @@ -224,7 +224,7 @@ struct flexcan_regs {
>  	u32 rxfgmask;		/* 0x48 */
>  	u32 rxfir;		/* 0x4c */
>  	u32 _reserved3[12];	/* 0x50 */
> -	struct flexcan_mb mb[64];	/* 0x80 */
> +	u32 mb[256];		/* 0x80 */
>  	/* FIFO-mode:
>  	 *			MB
>  	 * 0x080...0x08f	0	RX message buffer
> @@ -353,6 +353,25 @@ static inline void flexcan_write_le(u32 val, void __iomem *addr)
>  	iowrite32(val, addr);
>  }
>  
> +static struct flexcan_mb *flexcan_get_mb(const struct flexcan_priv *priv,
> +					 u8 mb_index)
> +{
> +	u8 mb_size;
> +	u8 mb_count;
> +
> +	if (priv->can.ctrlmode & CAN_CTRLMODE_FD)
> +		mb_size = sizeof(struct flexcan_mb) + CANFD_MAX_DLEN;
> +	else
> +		mb_size = sizeof(struct flexcan_mb) + CAN_MAX_DLEN;
> +	mb_count = (sizeof(priv->regs->mb) / mb_size);

Maybe cache the mb_count value in priv.

> +
> +	if (mb_index >= mb_count)
> +		return NULL;

Later on you're make use of this, but if you have mb_count cached, you
can write a propper for loop and this should not happen anymore. Make it
with WARN_ON() or remove it.

> +
> +	return (struct flexcan_mb __iomem *)((u8 *)&priv->regs->mb +
> +					     (mb_size * mb_index));
> +}
> +
>  static inline void flexcan_error_irq_enable(const struct flexcan_priv *priv)
>  {
>  	struct flexcan_regs __iomem *regs = priv->regs;
> @@ -519,6 +538,7 @@ static netdev_tx_t flexcan_start_xmit(struct sk_buff *skb, struct net_device *de
>  	u32 can_id;
>  	u32 data;
>  	u32 ctrl = FLEXCAN_MB_CODE_TX_DATA | (cf->can_dlc << 16);
> +	u8 can_dlc_dword, i;
>  
>  	if (can_dropped_invalid_skb(dev, skb))
>  		return NETDEV_TX_OK;
> @@ -535,13 +555,10 @@ static netdev_tx_t flexcan_start_xmit(struct sk_buff *skb, struct net_device *de
>  	if (cf->can_id & CAN_RTR_FLAG)
>  		ctrl |= FLEXCAN_MB_CNT_RTR;
>  
> -	if (cf->can_dlc > 0) {
> -		data = be32_to_cpup((__be32 *)&cf->data[0]);
> -		priv->write(data, &priv->tx_mb->data[0]);
> -	}
> -	if (cf->can_dlc > 4) {
> -		data = be32_to_cpup((__be32 *)&cf->data[4]);
> -		priv->write(data, &priv->tx_mb->data[1]);
> +	can_dlc_dword = (cf->can_dlc + sizeof(u32) - 1) / sizeof(u32);

Use DIV_ROUND_UP() here.

> +	for (i = 0; i < can_dlc_dword; i++) {
> +		data = be32_to_cpup((__be32 *)&cf->data[(i * sizeof(u32))]);
> +		priv->write(data, &priv->tx_mb->data[i]);
>  	}
>  
>  	can_put_echo_skb(skb, dev, 0);
> @@ -666,8 +683,12 @@ static unsigned int flexcan_mailbox_read(struct can_rx_offload *offload,
>  {
>  	struct flexcan_priv *priv = rx_offload_to_priv(offload);
>  	struct flexcan_regs __iomem *regs = priv->regs;
> -	struct flexcan_mb __iomem *mb = &regs->mb[n];
> +	struct flexcan_mb __iomem *mb;
>  	u32 reg_ctrl, reg_id, reg_iflag1;
> +	__be32 data;

move data into the for loop

> +	u8 can_dlc_dword, i;
> +
> +	mb = flexcan_get_mb(priv, n);
>  
>  	if (priv->devtype_data->quirks & FLEXCAN_QUIRK_USE_OFF_TIMESTAMP) {
>  		u32 code;
> @@ -708,8 +729,11 @@ static unsigned int flexcan_mailbox_read(struct can_rx_offload *offload,
>  		cf->can_id |= CAN_RTR_FLAG;
>  	cf->can_dlc = get_can_dlc((reg_ctrl >> 16) & 0xf);
>  
> -	*(__be32 *)(cf->data + 0) = cpu_to_be32(priv->read(&mb->data[0]));
> -	*(__be32 *)(cf->data + 4) = cpu_to_be32(priv->read(&mb->data[1]));
> +	can_dlc_dword = (cf->can_dlc + sizeof(u32) - 1) / sizeof(u32);

same here, maybe add a helper function.

> +	for (i = 0; i < can_dlc_dword; i++) {
> +		data = cpu_to_be32(priv->read(&mb->data[i]));
> +		*(__be32 *)(cf->data + (i * sizeof(u32))) = data;
> +	}
>  
>  	/* mark as read */
>  	if (priv->devtype_data->quirks & FLEXCAN_QUIRK_USE_OFF_TIMESTAMP) {
> @@ -906,6 +930,7 @@ static int flexcan_chip_start(struct net_device *dev)
>  	struct flexcan_regs __iomem *regs = priv->regs;
>  	u32 reg_mcr, reg_ctrl, reg_ctrl2, reg_mecr;
>  	int err, i;
> +	struct flexcan_mb __iomem *mb;
>  
>  	/* enable module */
>  	err = flexcan_chip_enable(priv);
> @@ -987,15 +1012,19 @@ static int flexcan_chip_start(struct net_device *dev)
>  	}
>  
>  	/* clear and invalidate all mailboxes first */
> -	for (i = priv->tx_mb_idx; i < ARRAY_SIZE(regs->mb); i++) {
> -		priv->write(FLEXCAN_MB_CODE_RX_INACTIVE,
> -			    &regs->mb[i].can_ctrl);
> -	}
> +	i = priv->tx_mb_idx;
> +	mb = flexcan_get_mb(priv, i);
> +	do {
> +		priv->write(FLEXCAN_MB_CODE_RX_INACTIVE, &mb->can_ctrl);
> +	} while ((mb = flexcan_get_mb(priv, ++i)) != NULL);

Keep it a for loop please.

>  
>  	if (priv->devtype_data->quirks & FLEXCAN_QUIRK_USE_OFF_TIMESTAMP) {
> -		for (i = priv->offload.mb_first; i <= priv->offload.mb_last; i++)
> -			priv->write(FLEXCAN_MB_CODE_RX_EMPTY,
> -				    &regs->mb[i].can_ctrl);
> +		for (i = priv->offload.mb_first;
> +		     i <= priv->offload.mb_last;
> +		     i++) {
> +			mb = flexcan_get_mb(priv, i);
> +			priv->write(FLEXCAN_MB_CODE_RX_EMPTY, &mb->can_ctrl);
> +		}
>  	}
>  
>  	/* Errata ERR005829: mark first TX mailbox as INACTIVE */
> @@ -1015,7 +1044,7 @@ static int flexcan_chip_start(struct net_device *dev)
>  		priv->write(0x0, &regs->rxfgmask);
>  
>  	/* clear acceptance filters */
> -	for (i = 0; i < ARRAY_SIZE(regs->mb); i++)
> +	for (i = 0; i < ARRAY_SIZE(regs->rximr); i++)
>  		priv->write(0, &regs->rximr[i]);

What does the new IP say when accessing rximr[i] with i >= the mailbox
available due to CAN-FD?

>  
>  	/* On Vybrid, disable memory error detection interrupts
> @@ -1099,6 +1128,7 @@ static int flexcan_open(struct net_device *dev)
>  {
>  	struct flexcan_priv *priv = netdev_priv(dev);
>  	int err;
> +	u8 mb_size;
>  
>  	err = clk_prepare_enable(priv->clk_ipg);
>  	if (err)
> @@ -1116,14 +1146,21 @@ static int flexcan_open(struct net_device *dev)
>  	if (err)
>  		goto out_close;
>  
> +	if (priv->can.ctrlmode & CAN_CTRLMODE_FD)
> +		mb_size = sizeof(struct flexcan_mb) + CANFD_MAX_DLEN;
> +	else
> +		mb_size = sizeof(struct flexcan_mb) + CAN_MAX_DLEN;
> +
>  	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];
> +		priv->tx_mb_reserved = flexcan_get_mb(priv,
> +						      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_reserved = flexcan_get_mb(priv,
> +						      FLEXCAN_TX_MB_RESERVED_OFF_FIFO);
>  	}
> -	priv->tx_mb = &regs->mb[priv->tx_mb_idx];
> +	priv->tx_mb = flexcan_get_mb(priv, priv->tx_mb_idx);
>  
>  	priv->reg_imask1_default = FLEXCAN_IFLAG_MB(priv->tx_mb_idx);
>  	priv->reg_imask2_default = 0;
> @@ -1134,7 +1171,7 @@ static int flexcan_open(struct net_device *dev)
>  		u64 imask;
>  
>  		priv->offload.mb_first = FLEXCAN_RX_MB_OFF_TIMESTAMP_FIRST;
> -		priv->offload.mb_last = FLEXCAN_RX_MB_OFF_TIMESTAMP_LAST;
> +		priv->offload.mb_last = (sizeof(priv->regs->mb) / mb_size) - 1;
>  
>  		imask = GENMASK_ULL(priv->offload.mb_last,
>  				    priv->offload.mb_first);
> 

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

[Index of Archives]     [Automotive Discussions]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]     [CAN Bus]

  Powered by Linux